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

Add documentation to Team API #581

Merged
merged 3 commits into from
Feb 27, 2024
Merged

Add documentation to Team API #581

merged 3 commits into from
Feb 27, 2024

Conversation

patriciaahuang
Copy link
Contributor

@patriciaahuang patriciaahuang commented Feb 12, 2024

Added documentation to teamAPI.ts. The @throws were not included due to the error messages being self-explanatory.

@patriciaahuang patriciaahuang requested a review from a team as a code owner February 12, 2024 04:54
@dti-github-bot
Copy link
Member

dti-github-bot commented Feb 12, 2024

[diff-counting] Significant lines: 32.

@@ -106,6 +131,12 @@ export const setTeam = async (teamBody: Team, member: IdolMember): Promise<Team>
return teamBody;
};

/**
* Deletes a team by removing all members, leaders, and formerMembers.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tracing this down, it seems like additionally, it removes all references between the member, leaders, and formerMembers from this team. I.e. member, leader, formerMember will no longer have his team under subteams field and formerSubteams field.

Maybe add that in the docstring?

* @param teamBody - The Team object that will be deleted.
* @param member - The IdolMember that is requesting to delete the team.
* @returns A promise that resolves to the deleted Team object.
*/
export const deleteTeam = async (teamBody: Team, member: IdolMember): Promise<Team> => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be in a separate PR, but I think this function should be renamed to clearTeam, since the team document doesn't actually get deleted from the DB. Assuming that is intentional, all we're doing here is removing all members, leaders, and formerMembers from the team.

@henry-li-06
Copy link
Collaborator

FYI the Team data type in IDOL is aggregated data from the members collection. Unlike a lot of the data types for the IDOL BE, an instance of a Team object isn't correlated to some doc in the teams collection for example.

In a sense it's a "view" on the members collection.

So Team is

export type Team = {
  uuid: string;
  name: string;
  leaders: IdolMember[];
  members: IdolMember[];
  formerMembers: IdolMember[];
};

A team exists if there are 1+ members in the members collection where the team's name is in a member's subteams or formerSubteamsfields. The team object is created by grouping members by their teams in a many to many relationship kinda situation (a member can have multiple teams and a team has multiple members).

The team's former members are just the members where the team name is listed under the formerSubteams field for the member. The team's leaders are members whose roles are either pm or tpm and have the team name listed under the members field. The team's members are the same as before just without pm or tpm role.

Any operations on teams in the BE are basically just operations on members docs to reflect/persist these team changes.

And teams really aren't used at all within IDOL (could be an interesting idea to explore, maybe for coffee chats or whatever). The main reason the teams collection was removed in favor of this implementation where teams are just a view on the members collection is related to usage of IDOL for member profiles. The members collection was the source of truth for member profile information for the website (updated through the pull-from-idol job). Teams used to be separate from members which led to 2 representations of essentially the same data (who is on what team). That led to some problems like data being out of sync and then just managing the same data in 2 places is just extra work.

The original PR if you want more info: #199

@patriciaahuang patriciaahuang merged commit deea5fa into main Feb 27, 2024
17 checks passed
@patriciaahuang patriciaahuang deleted the team-api-documentation branch February 27, 2024 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants