Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor Group Chats #772

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from
Draft

Conversation

SamShanks1
Copy link
Contributor

@SamShanks1 SamShanks1 commented Jun 6, 2022

Pull Request Description
A full rework of the group system

Feature Specs:

  • Group Avatar
  • Leave group chats
  • Add to group chats
  • Edit group chat name

Tasks Progress:

  • Leave Groups
  • Add to groups
  • Remove people from groups
  • Store who created groups
  • Change group name
  • Group avatar with edit
  • System messages (join/leave/add)
  • If owner leaves then assign someone else owner (first in participant list)

Preview

The following SQL needs to be ran

ALTER TABLE npwd_messages ADD COLUMN `is_system` tinyint(4) NOT NULL DEFAULT 0;
ALTER TABLE npwd_messages ADD COLUMN `system_type` varchar(48) NOT NULL DEFAULT '';
ALTER TABLE npwd_messages ADD COLUMN `system_number` varchar(48) NOT NULL DEFAULT '';
ALTER TABLE npwd_messages_conversations ADD COLUMN `owner` varchar(48) NOT NULL DEFAULT '';

#610

Pull Request Checklist:

  • Have you followed the guidelines in our Contributing document and Code of Conduct?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you built and tested NPWD in-game after the relevant change?

@SamShanks1
Copy link
Contributor Author

One problem I just thought about will be groups that were created before this and don't have the "createdBy" defined, I could make it so any groups that don't have that will allow any member to remove another member. I also don't know if any errors will be created if "createdBy" is left blank

@itschip itschip self-assigned this Jun 6, 2022
@itschip itschip added Awaiting Code Review Awaiting code review. Awaiting Testing Awaiting in-game testing. labels Jun 6, 2022
@SamShanks1
Copy link
Contributor Author

Tested with no createBy date, everything still works however no one can remove people (may add it so everyone can remove in the future). Also just added leaving groups

@SamShanks1 SamShanks1 changed the title feat(messages): Remove Members from groups feat(messages): Remove Members & Leave Groups Jun 6, 2022
Copy link
Collaborator

@LiamDormon LiamDormon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far, only issue I have is that members cannot re-join a group and a few minor things

import.sql Outdated Show resolved Hide resolved
resources/server/messages/messages.service.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@TasoOneAsia TasoOneAsia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple small things I noticed that might need some cleanup but all in all good work, thank you for the PR :)

resources/server/messages/messages.service.ts Outdated Show resolved Hide resolved
resources/server/messages/messages.service.ts Outdated Show resolved Hide resolved
resources/server/messages/messages.service.ts Outdated Show resolved Hide resolved
typings/messages.ts Outdated Show resolved Hide resolved
@TasoOneAsia
Copy link
Contributor

Also appears CI is failing for the frontend due to a couple small ESLint complaints.

Screen Shot 2022-06-09 at 9 30 59 PM

@SamShanks1
Copy link
Contributor Author

Just need to do some last testing but this should be finished, @Mojito-Fivem would you prefer I do a separate PR for adding members to groups or keep it here

@LiamDormon
Copy link
Collaborator

It's all under the same feature and I think it would create more issues having this without the adding members functionality

@SamShanks1 SamShanks1 marked this pull request as draft June 11, 2022 00:06
@SamShanks1
Copy link
Contributor Author

SamShanks1 commented Jun 11, 2022

Added system messages

Preview

The following SQL is now needed (added to the original message)

ALTER TABLE npwd_messages ADD COLUMN `is_system` tinyint(4) NOT NULL DEFAULT 0;
ALTER TABLE npwd_messages ADD COLUMN `system_type` varchar(48) NOT NULL DEFAULT '';
ALTER TABLE npwd_messages ADD COLUMN `system_number` varchar(48) NOT NULL DEFAULT '';

@SamShanks1 SamShanks1 changed the title feat(messages): Remove Members & Leave Groups Refactor Group Chats Jun 12, 2022
@itschip itschip assigned SamShanks1 and unassigned itschip Aug 28, 2022
@AlamoRoleplayFiveM
Copy link

Will there not be any chance of group calling?

@stale
Copy link

stale bot commented Mar 7, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Mar 7, 2023
@TonybynMp4
Copy link
Contributor

this needs to be revived 😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Code Review Awaiting code review. Awaiting Testing Awaiting in-game testing. wontfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants