feat(common): team models #1986
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces team management functionality across the codebase, adding models and API support for team-based project organization.
- Added new team models in
common/src/models/team.rswithTeamResponse,TeamMembership, andTeamRolestructures - Added optional
team_idfield toProjectResponseincommon/src/models/project.rsto support team-owned projects - Modified
UserResponseincommon/src/models/user.rsto include Auth0 ID documentation and optional flags field
6 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
There was a problem hiding this comment.
PR Summary
(updates since last review)
This PR continues to expand the team management functionality with additional features and refinements. Here's what's new:
- Added
AddTeamMemberRequestincommon/src/models/team.rssupporting both user ID and email-based invitations - Enhanced
ProjectUpdateRequestwith new fields for team transfers and ownership changes - Generated corresponding TypeScript interfaces in
common/types.tsfor frontend integration
The changes maintain consistency with existing patterns and provide proper documentation. The implementation appears well-structured but should be reviewed for potential validation logic around mutually exclusive operations in project transfers.
4 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings | Greptile
| pub user_id: Option<String>, | ||
| pub email: Option<String>, | ||
| /// Role of the user in the team | ||
| pub role: Option<TeamRole>, |
There was a problem hiding this comment.
style: Consider making role field required since TeamMembership requires a role. Optional roles could lead to inconsistent states
| pub struct ProjectUpdateRequest { | ||
| /// Change display name | ||
| pub name: Option<String>, | ||
| /// Transfer to other user | ||
| pub user_id: Option<String>, | ||
| /// Transfer to a team | ||
| pub team_id: Option<String>, | ||
| /// Transfer away from current team | ||
| pub remove_from_team: Option<bool>, | ||
| /// Change compute tier | ||
| pub compute_tier: Option<ComputeTier>, |
There was a problem hiding this comment.
logic: These fields could create conflicting states if multiple transfer options are set simultaneously (user_id, team_id, and remove_from_team). Consider adding validation or documenting the precedence rules.
| /// Transfer away from current team | ||
| pub remove_from_team: Option<bool>, |
There was a problem hiding this comment.
style: remove_from_team field could be redundant since setting team_id to None could achieve the same result. Consider consolidating the team management logic.
| user_id?: string; | ||
| email?: string; | ||
| /** Role of the user in the team */ | ||
| role?: TeamRole; |
There was a problem hiding this comment.
logic: role field should be required since TeamMembership requires it. Optional role could lead to runtime errors when adding members.
| team_id?: string; | ||
| /** Transfer away from current team */ | ||
| remove_from_team?: boolean; |
There was a problem hiding this comment.
style: team_id and remove_from_team are mutually exclusive - consider using a discriminated union type instead of allowing both to be set
There was a problem hiding this comment.
PR Summary
(updates since last review)
This PR finalizes the team management implementation with account tier adjustments and permission model changes.
- Renamed
Teamaccount tier toGrowthand removedDeployertier incommon/src/models/user.rs - Removed
has_access_to_betafield fromUserResponseas part of permission model cleanup - Added team-related fields to
ProjectResponseandProjectUpdateRequestfor project ownership management
The changes streamline the permission model by consolidating special access flags and moving towards a role-based system. Consider documenting the migration path for existing users with the Deployer tier.
2 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile
| subscriptions: Subscription[]; | ||
| has_access_to_beta?: boolean; | ||
| flags?: string[]; |
There was a problem hiding this comment.
style: consider documenting the purpose and possible values of the new flags field
No description provided.