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

GROUP-22 Integrate Group Sync #9

Merged
merged 12 commits into from
Sep 27, 2023
Merged

GROUP-22 Integrate Group Sync #9

merged 12 commits into from
Sep 27, 2023

Conversation

makmn1
Copy link
Member

@makmn1 makmn1 commented Sep 27, 2023

Description of Changes

The following changes were made in response to the changes in GROUP-21 Group Sync Service. Reasons vary from updating common classes to ensure consistency, all the way to updating business logic based on bugs found and lacking features.

Updated Configuration

Mainly updated the names of Spring Cloud Functions. Before, their names were tied to their action (e.g. handleGroupCreateRequests vs. the now groupCreateRequests). Now they are tied to their topic. This was done since the name needed to be the same for the corresponding supplier functions in Group Sync.

Updated Group Demo Loader to Follow Event Flow

This was supposed to be done in an earlier issue. It's absence was noticed during the creation of Group Sync when Group Create Requests were not being sent when the demo loader created groups. It is now going through the event flow as initially intended.

Updated Business Logic

Testing in Group Sync revealed that we were missing some business logic to prevent unintended behavior for groups and members. The following business rules have been enforced and are subject to change in the future:

  • Users cannot join the same group twice
  • Users cannot be in two different groups at the same time
  • Users cannot send a Group Leave Request on behalf of another user if they know that user's member ID. Previously we relied on the member ID to prove the user's ownership of the member. However, this is not enough since the member ID is implemented as a serial value in the database starting from 1, meaning that a user could guess the member ID pretty quickly. Now the user must know the websocketId value that is now attached to a member's record. Since this ID is a universal unique identifier (UUID), users will not be able to practically guess the ID.
  • Updated groups to be returned by most recent group first. Initially, this was thought to be already done, but I confused ascending and descending order when it came to time.

Additional Info / Concerns

Acceptance tests were meant to be the same between Group Service and Group Sync. While the scenarios technically achieve the same thing, Group Sync is more readable. In the future, an attempt should be made to synchronize the two. Perhaps this can be considered when we have a more robust acceptance testing strategy (see the PR for Group Sync issue GROUP-21 for more details).

@makmn1 makmn1 added the enhancement New feature or request label Sep 27, 2023
@makmn1 makmn1 self-assigned this Sep 27, 2023
@makmn1 makmn1 merged commit 97c1db1 into main Sep 27, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant