Set Teams and Roles for Invites #9062
Conversation
📝 WalkthroughWalkthroughAdds invite-time role and team assignment: invitations, DB schema, services, and controllers now carry Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas needing extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This reverts commit 72e5980.
|
@copilot write a testing protocol including setting the permissions and team roles upon invite, which is also making sure that the old team and permissions modal is still working fine |
|
@knollengewaechs I've opened a new pull request, #9086, to work on those changes. Once the pull request is ready, I'll request review from you. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/models/team/Team.scala (1)
23-37: DefaultingrequestingUsertoNonecurrently filters out all teams
removeForeignOrganizationTeamsuses:teams.filter(team => requestingUser.map(_._organization).contains(team._organization))With the default
requestingUser: Option[User] = None,requestingUser.map(...).contains(...)is alwaysfalse, so bothallowedTeamsForFolderandallowedTeamsForDatasetwill return an empty list whenever callers omit the new argument (which is likely for existing call sites).To preserve previous behavior and only apply the extra filter when a user is actually provided, consider:
- private def removeForeignOrganizationTeams(teams: List[Team], requestingUser: Option[User]): List[Team] = - teams.filter(team => requestingUser.map(_._organization).contains(team._organization)) + private def removeForeignOrganizationTeams(teams: List[Team], requestingUser: Option[User]): List[Team] = + requestingUser match { + case Some(user) => teams.filter(_. _organization == user._organization) + case None => teams + }(or equivalently
teams.filter(team => requestingUser.forall(_._organization == team._organization))).This keeps old behavior for existing callers while enforcing same‑organization filtering when
requestingUseris provided.Also applies to: 90-92
app/models/user/UserService.scala (1)
139-167:initialTeamMembershipscan create duplicate memberships for the same team
initialTeamMembershipscurrently does:organizationTeamMembership = Seq(TeamMembership(organizationTeamId, isTeamManager = false)) inviteTeamMemberships <- ... uniqueTeamMemberships = inviteTeamMemberships ++ organizationTeamMembershipDespite the name,
uniqueTeamMembershipsis just a concatenation. If an invite already contains a membership for the organization’s default team, you’ll end up with two entries for the sameteamId. When passed intoupdateTeamMembershipsForUser, this can:
- Cause duplicate rows in
user_team_roles(or hit a unique constraint, depending on schema).- Lead to confusing behavior in code that assumes one membership per team.
A safer approach is to deduplicate by
teamIdand reconcileisTeamManager(e.g., prefertrueif any entry for that team is a manager):- inviteTeamMemberships <- inviteIdOpt match { - case Some(inviteId) => inviteDAO.findTeamMembershipsFor(inviteId) ?~> "failed to get invite team memberships" - case None => Fox.successful(Seq.empty) - } - uniqueTeamMemberships = inviteTeamMemberships ++ organizationTeamMembership - } yield uniqueTeamMemberships + inviteTeamMemberships <- inviteIdOpt match { + case Some(inviteId) => inviteDAO.findTeamMembershipsFor(inviteId) ?~> "failed to get invite team memberships" + case None => Fox.successful(Seq.empty) + } + combined = inviteTeamMemberships ++ organizationTeamMembership + uniqueTeamMemberships = combined + .groupBy(_.teamId) + .values + .map { membershipsForTeam => + val teamId = membershipsForTeam.head.teamId + val isTeamManager = membershipsForTeam.exists(_.isTeamManager) + TeamMembership(teamId, isTeamManager) + } + .toSeq + } yield uniqueTeamMembershipsThis keeps the implicit “everyone is in the organization team” behavior while avoiding duplicates and preserving manager roles from invites.
🧹 Nitpick comments (2)
frontend/javascripts/admin/onboarding.tsx (1)
238-238: Remove debug console.log statement.This console.log statement should be removed before merging to production.
Apply this diff:
- console.log("Setting default team in invite modal");app/models/user/Invite.scala (1)
46-57: Consider wrapping invite and team-membership inserts in a single transactionRight now,
inviteDAO.insertOne(invite)andinviteDAO.insertTeamMemberships(invite._id, teamMemberships)are two separate DBIO runs. If the second one fails, you can end up with an invite without its intended team roles.It would be more robust to compose both operations into a single transactional DBIO so that either both the invite and its
invite_team_rolesare written, or neither is.Also applies to: 59-75, 139-156
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
app/controllers/AuthenticationController.scala(7 hunks)app/controllers/OrganizationController.scala(2 hunks)app/controllers/UserController.scala(2 hunks)app/models/team/Team.scala(1 hunks)app/models/team/TeamMembership.scala(1 hunks)app/models/user/Invite.scala(6 hunks)app/models/user/User.scala(1 hunks)app/models/user/UserService.scala(4 hunks)conf/evolutions/146-invite-roles.sql(1 hunks)conf/evolutions/reversions/146-invite-roles.sql(1 hunks)frontend/javascripts/admin/onboarding.tsx(7 hunks)frontend/javascripts/admin/rest_api.ts(2 hunks)frontend/javascripts/admin/user/permissions_and_teams_modal_view.tsx(7 hunks)frontend/javascripts/dashboard/dataset/helper_components.tsx(2 hunks)frontend/javascripts/viewer/view/action-bar/share_modal_view.tsx(3 hunks)tools/postgres/schema.sql(3 hunks)unreleased_changes/9062.md(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-04-23T08:51:57.756Z
Learnt from: frcroth
Repo: scalableminds/webknossos PR: 8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/MeshFileService.scala:170-173
Timestamp: 2025-04-23T08:51:57.756Z
Learning: In the webknossos codebase, classes extending `FoxImplicits` have access to an implicit conversion from `Option[A]` to `Fox[A]`, where `None` is converted to an empty Fox that fails gracefully in for-comprehensions.
Applied to files:
app/models/user/Invite.scala
📚 Learning: 2025-04-28T14:18:04.368Z
Learnt from: frcroth
Repo: scalableminds/webknossos PR: 8236
File: webknossos-datastore/app/com/scalableminds/webknossos/datastore/services/mesh/NeuroglancerPrecomputedMeshFileService.scala:161-166
Timestamp: 2025-04-28T14:18:04.368Z
Learning: In Scala for-comprehensions with the Fox error handling monad, `Fox.fromBool()` expressions should use the `<-` binding operator instead of the `=` assignment operator to properly propagate error conditions. Using `=` will cause validation failures to be silently ignored.
Applied to files:
app/controllers/UserController.scala
📚 Learning: 2025-08-14T14:12:03.293Z
Learnt from: hotzenklotz
Repo: scalableminds/webknossos PR: 8849
File: frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/collapsible_split_merger_evaluation_settings.tsx:39-51
Timestamp: 2025-08-14T14:12:03.293Z
Learning: Since Ant Design version 5.18, Form.Item components support the layout prop (layout="horizontal", layout="vertical", etc.), allowing individual form items to override the parent Form's layout setting. This is a newer API addition that provides more granular control over form item layouts.
Applied to files:
frontend/javascripts/dashboard/dataset/helper_components.tsx
📚 Learning: 2025-08-14T14:12:03.293Z
Learnt from: hotzenklotz
Repo: scalableminds/webknossos PR: 8849
File: frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/collapsible_split_merger_evaluation_settings.tsx:39-51
Timestamp: 2025-08-14T14:12:03.293Z
Learning: Since Ant Design version 5.18, Form.Item components support the layout prop (layout="horizontal", layout="vertical", layout="inline"), allowing individual form items to override the parent Form's layout setting. This enables mixed layouts within a single form and was added as a new API feature in v5.18 according to the official changelog.
Applied to files:
frontend/javascripts/dashboard/dataset/helper_components.tsx
📚 Learning: 2025-11-03T17:50:57.587Z
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9025
File: frontend/javascripts/dashboard/advanced_dataset/dataset_table.tsx:10-10
Timestamp: 2025-11-03T17:50:57.587Z
Learning: In the webknossos codebase, both `import { Component } from "antd";` and `import { Component } from "antd/lib";` are valid import paths for antd components. Do not flag imports from "antd/lib" as errors.
Applied to files:
frontend/javascripts/dashboard/dataset/helper_components.tsx
🧬 Code graph analysis (9)
frontend/javascripts/viewer/view/action-bar/share_modal_view.tsx (1)
frontend/javascripts/dashboard/dataset/helper_components.tsx (1)
DividerWithSubtitle(80-82)
frontend/javascripts/admin/rest_api.ts (1)
frontend/javascripts/types/api_types.ts (1)
APITeamMembership(308-312)
app/models/user/Invite.scala (2)
app/models/team/TeamMembership.scala (2)
TeamMembership(11-11)TeamMembership(12-17)app/models/user/User.scala (1)
insertTeamMembershipQuery(558-561)
app/controllers/OrganizationController.scala (2)
app/models/user/UserService.scala (2)
initialTeamMemberships(157-168)joinOrganization(168-198)app/controllers/AuthenticationController.scala (1)
joinOrganization(415-444)
app/models/user/UserService.scala (4)
app/models/user/Invite.scala (1)
findTeamMembershipsFor(149-157)app/models/user/User.scala (1)
updateTeamMembershipsForUser(561-569)app/models/organization/Organization.scala (2)
isEmpty(107-113)findOrganizationTeamId(145-152)app/controllers/AuthenticationController.scala (1)
joinOrganization(415-444)
frontend/javascripts/admin/onboarding.tsx (5)
frontend/javascripts/libs/react_helpers.tsx (1)
useFetch(35-52)frontend/javascripts/admin/rest_api.ts (2)
getEditableTeams(333-339)sendInvitesForOrganization(1723-1740)frontend/javascripts/types/api_types.ts (1)
APITeamMembership(308-312)frontend/javascripts/dashboard/dataset/helper_components.tsx (1)
DividerWithSubtitle(80-82)frontend/javascripts/admin/user/permissions_and_teams_modal_view.tsx (1)
PermissionsAndTeamsComponent(63-249)
app/models/team/TeamMembership.scala (1)
app/models/team/Team.scala (2)
publicWrites(39-51)findOne(120-127)
frontend/javascripts/admin/user/permissions_and_teams_modal_view.tsx (4)
frontend/javascripts/types/api_types.ts (2)
APITeamMembership(308-312)APIUser(338-353)frontend/javascripts/libs/react_helpers.tsx (1)
useFetch(35-52)frontend/javascripts/admin/rest_api.ts (2)
getEditableTeams(333-339)updateUser(194-199)frontend/javascripts/dashboard/dataset/helper_components.tsx (1)
DividerWithSubtitle(80-82)
app/controllers/AuthenticationController.scala (3)
app/models/team/TeamMembership.scala (2)
TeamMembership(11-11)TeamMembership(12-17)app/models/user/UserService.scala (5)
initialTeamMemberships(157-168)insert(93-137)joinOrganization(168-198)isTeamManagerOrAdminOf(321-327)isTeamManagerOrAdminOf(327-333)app/models/user/Invite.scala (1)
inviteOneRecipient(46-59)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: frontend-tests
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (22)
app/models/user/User.scala (1)
561-568: Switch toSeq[TeamMembership]looks fineThe method logic (clear then re‑insert in one transaction) is unchanged and
Seqintegrates cleanly with the existingList(clearQuery) ++ insertQueriespattern. No issues from this refactor.app/models/team/TeamMembership.scala (1)
11-17: JSON Reads forTeamMembershipaligns with existing writerThe new
jsonReadsmatches the fields emitted bypublicWrites(id,isTeamManager), and unknown fields likenameare ignored by Play JSON, so round‑tripping is consistent. No issues here.Also applies to: 20-28
app/models/team/Team.scala (1)
221-227: GeneralizingallowedTeamstoSeqis safeThe method still deletes then reinserts all allowed teams in a single transactional replacement; switching the parameter from
ListtoSeqdoesn’t change behavior and aligns with other APIs moving toSeq.unreleased_changes/9062.md (1)
1-5: Changelog entry is clear and sufficientThe note accurately summarizes the new invite capability and correctly links the evolution script. Looks good.
frontend/javascripts/viewer/view/action-bar/share_modal_view.tsx (1)
27-27: SharedDividerWithSubtitleusage looks goodSwitching the two section headers to
DividerWithSubtitlecentralizes styling without touching the sharing logic. Import path and usage match the exported helper component.Also applies to: 374-377, 441-444
frontend/javascripts/dashboard/dataset/helper_components.tsx (2)
2-2: LGTM!The Divider import is used correctly in the new
DividerWithSubtitlecomponent below.
80-82: LGTM!The
DividerWithSubtitlecomponent is clean and reusable. It provides consistent styling for section dividers across the application.tools/postgres/schema.sql (3)
24-24: LGTM!The schema version bump to 146 correctly reflects the database changes for invite roles and team memberships.
442-447: LGTM!The
invite_team_rolestable structure is well-designed with appropriate constraints and a composite primary key on(_invite, _team).
574-575: LGTM!The new columns
isAdminandisDatasetManageron the invites table have appropriate types, non-null constraints, and default values.frontend/javascripts/admin/onboarding.tsx (2)
355-364: Note: TextArea is now a controlled component.By adding the
valueprop alongsidedefaultValue, the TextArea becomes a controlled component. ThedefaultValueprop will be ignored. This appears intentional given thehandleInviteesStringChangehandler, but ensure this behavior is expected.
269-299: LGTM!The
sendInvitefunction correctly:
- Validates at least one email address is provided
- Computes
isAdminandisDatasetManagerflags from selected permissions- Extracts team memberships for the invite
- Passes all required parameters to the backend
- Resets fields after successful invite
conf/evolutions/146-invite-roles.sql (1)
1-21: LGTM!The forward migration is well-structured with:
- Proper transaction boundaries
- Version check precondition
- Correct SQL syntax for adding columns and creating tables
- Appropriate schema version update
frontend/javascripts/admin/rest_api.ts (2)
48-48: LGTM!The
APITeamMembershipimport is correctly added and used in the updated function signature below.
1723-1740: LGTM!The
sendInvitesForOrganizationfunction signature is correctly extended to include:
isAdmin: boolean flag for admin permissionisDatasetManager: boolean flag for dataset manager permissionteamMemberships: array of team membership assignmentsAll parameters are properly passed to the backend API endpoint.
app/controllers/OrganizationController.scala (2)
80-87: LGTM!The organization creation flow correctly:
- Retrieves initial team memberships (organization team by default)
- Passes
isDatasetManager = falseto maintain existing behavior- Preserves
isOrganizationOwner = truefor the owner- Supplies team memberships to the join flow
185-191: LGTM!The
addUsermethod consistently follows the same pattern as the create flow:
- Retrieves initial team memberships before joining
- Sets appropriate default permissions (
isAdmin = false,isDatasetManager = false)- Provides team memberships to the service layer
app/controllers/UserController.scala (2)
185-185: LGTM!The change from
teamMembershipService.publicReads()to a directList[TeamMembership]reader indicates thatTeamMembershipnow has its ownReadsimplementation, which simplifies the code.
189-200: Simplified authorization check is sufficient and safe.The authorization logic is secure due to the upstream filter at lines 340-341, which restricts
teamsWithUpdateto only teams where the issuing user is already a team manager or organization admin. Regular org members cannot pass this filter, so the organization membership check inensureProperTeamAdministration(line 342) provides a secondary safety layer but is not the only barrier.The privilege escalation concern is not valid: a regular user cannot reach
ensureProperTeamAdministrationwith teams unless they're already authorized to manage those teams viaisTeamManagerOrAdminOf.Note: The method
couldBeAdministratedBydoes not exist in the current codebase, so the claim about simplification cannot be verified. If this method was recently removed as part of the changes, please clarify the rationale for that removal.app/models/user/Invite.scala (1)
23-33: Invite model and DAO mapping look consistent
Invitenow includesisAdminandisDatasetManager, and the DAO correctly parses and inserts these fields. This aligns the in-memory model with the new DB columns; no issues spotted here.Also applies to: 107-119, 128-136
app/models/user/UserService.scala (1)
93-137: User creation/join flows now consistently use teamMembershipsThe extended
insertandjoinOrganizationsignatures (includingisDatasetManagerandteamMemberships) plus the calls touserDAO.updateTeamMembershipsForUsergive a single, consistent place where memberships are applied. That’s a good consolidation and matches how the rest of the code (e.g.,teamMembershipsFor) expects to read memberships.Also applies to: 168-197
app/controllers/AuthenticationController.scala (1)
284-308: User creation and org-join now correctly derive memberships from invites/organizationThe changes to
createUser,joinOrganization, andcreateOrganizationWithAdminthat calluserService.initialTeamMembershipsand passteamMemberships(along with invite-derivedisAdmin/isDatasetManager) intouserService.insert/joinOrganizationlook consistent and match the new membership model. New users now start with the organization’s default team plus any invite-specified team roles.Also applies to: 415-444, 878-895
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
frontend/javascripts/admin/onboarding.tsx (3)
226-252: MakeresetFieldsrobust when no “Default” team exists
setDefaultTeamonly updatesselectedTeamswhen a team named"Default"is present. If an organization ever lacks such a team,resetFieldswill leave whatever teams were previously selected in place, so closing/reopening the modal won’t actually reset team selections even though the permissions and email list reset.You can make this robust and still keep the “Default” behavior by clearing
selectedTeamswhen no default is found:- const setDefaultTeam = useCallback(() => { - if (defaultTeam != null) { - setSelectedTeams({ - [defaultTeam.name]: { - id: defaultTeam.id, - name: defaultTeam.name, - isTeamManager: false, - }, - }); - } - }, [defaultTeam]); + const setDefaultTeam = useCallback(() => { + if (defaultTeam != null) { + setSelectedTeams({ + [defaultTeam.name]: { + id: defaultTeam.id, + name: defaultTeam.name, + isTeamManager: false, + }, + }); + } else { + setSelectedTeams({}); + } + }, [defaultTeam, setSelectedTeams]);That way
resetFieldsgenuinely resets both permissions and team selections regardless of team setup.Also applies to: 262-267, 381-382
268-298: Invite sending flow matches API contract; consider minor UX hardeningThe
sendInvitehandler correctly validates that at least one email is present, derivesisAdmin/isDatasetManagerfromselectedPermission, and passesObject.values(selectedTeams)intosendInvitesForOrganization, so all created invites share the configured permissions/teams as per the PR description. Resetting fields after a successful send and on cancel keeps the modal tidy.If you want to harden UX slightly, you could:
- Guard against double-submits by disabling the “Send Invite Emails” button while the request is in-flight.
- Wrap
sendInvitesForOrganizationin atry/catchand show a failureToastso backend errors don’t silently fail.These are optional polish items; the current flow is functionally sound.
Also applies to: 300-307
337-353: Avoid mixingdefaultValueandvalueon the invitee textareaThe additional copy and
DividerWithSubtitlearound the email input improve clarity, and making the textarea controlled viavalue={inviteesString}is appropriate, especially sinceresetFieldsclears it.However, you now pass both
defaultValue={inviteesString}andvalue={inviteesString}toInput.TextArea. Withvaluepresent,defaultValueis redundant and can be confusing. Consider simplifying to:- <Input.TextArea + <Input.TextArea spellCheck={false} autoSize={{ minRows: 6 }} onChange={handleInviteesStringChange} placeholder={"[email protected]\[email protected]"} - defaultValue={inviteesString} - value={inviteesString} + value={inviteesString} />This keeps the field fully controlled without any ambiguity.
Also applies to: 355-363
frontend/javascripts/admin/user/permissions_and_teams_modal_view.tsx (2)
63-72: PermissionsAndTeamsComponent extraction looks good; watch for minor duplication/Radio defaultsThe extracted
PermissionsAndTeamsComponentnicely centralizes permission radio selection and per-team manager/member roles, and therenderSubtitlesWithDividerflag lets onboarding reuse it withDividerWithSubtitlewhile preserving the original modal’s look.Two small follow-ups you might consider:
You now call
useFetch(getEditableTeams, [], [])inside this component, andInviteUsersModalinonboarding.tsxalso callsuseFetch(getEditableTeams, [], [])just to discover the default team. That results in two parallel/api/teams?isEditable=truecalls whenever the invite modal is used. If this endpoint is at all heavy or rate-limited, consider lifting the fetch to the parent and optionally passingteamsinto the component, or providing a “pre-fetched teams” prop to avoid the second call.Similar to the textarea in onboarding,
Radio.Groupis effectively controlled viavalue={selectedPermission}, sodefaultValue={selectedPermission}is redundant. DroppingdefaultValuewould simplify the control and match React’s standard controlled-component pattern.These are non-blocking refinements; the current implementation is functionally correct.
Also applies to: 79-141, 143-153, 155-223, 224-249
251-291: Permission update logic now preserves multi-user flags; consider tightening payload and state resetThe new permission update flow fixes the earlier multi-user bug:
setPermissionsAndTeamsnow seedspermissionsfrom each user ({ isAdmin: user.isAdmin, isDatasetManager: user.isDatasetManager }) and only overrides them whenactiveUser.isAdmin && selectedUserIds.length === 1, so bulk “teams only” edits no longer silently demote admins or dataset managers. This addresses the previous review concern.Two refinements worth considering:
Avoid sending permission fields for multi-user edits: For bulk updates, you could omit
isAdmin/isDatasetManagerentirely instead of re-sending unchanged values, which makes the intent (“only teams change”) explicit and resilient to future server-side interpretation:
const newTeams = Utils.values(selectedTeams);let permissions = { isAdmin: user.isAdmin, isDatasetManager: user.isDatasetManager };if (activeUser.isAdmin && selectedUserIds.length === 1) {permissions = { isAdmin: false, isDatasetManager: false };
const newTeams = Utils.values(selectedTeams);let permissions: Partial<Pick<APIUser, "isAdmin" | "isDatasetManager">> = {};if (activeUser.isAdmin && selectedUserIds.length === 1) {permissions = { isAdmin: false, isDatasetManager: false }; // If the current user is admin and only one user is edited we also update the permissions. if (selectedPermission === PERMISSIONS.admin) { permissions["isAdmin"] = true; permissions["isDatasetManager"] = false; } else if (selectedPermission === PERMISSIONS.datasetManager) { permissions["isDatasetManager"] = true; permissions["isAdmin"] = false; } }
const newUser = { ...user, ...permissions, teams: newTeams };
const newUser = { ...user, ...permissions, teams: newTeams };
Reset local state when the selection/modal changes:
selectedTeams/selectedPermissionare only reinitialized when exactly one user is selected; if the component stays mounted across openings, switching from editing a single user to multiple users may keep staleselectedTeamsfrom the prior session. If that’s not desired, consider resetting state when selection mode changes or when the modal is (re)opened, e.g.:useEffect(() => { if (selectedUserIds.length === 1) { const singleUserMaybe = getSingleUserMaybe(selectedUserIds, users); if (singleUserMaybe) { setSelectedTeams(_.keyBy(singleUserMaybe.teams, "name")); setSelectedPermission(getPermissionGroupOfUser(singleUserMaybe)); } } else { // For multi-user edits, start with no teams selected setSelectedTeams({}); } }, [selectedUserIds, users]);Both points are about making behavior more explicit and predictable; the current code already satisfies the “don’t revoke permissions on multi-user edits” requirement.
Also applies to: 293-329, 331-358
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/javascripts/admin/onboarding.tsx(7 hunks)frontend/javascripts/admin/rest_api.ts(2 hunks)frontend/javascripts/admin/user/permissions_and_teams_modal_view.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/admin/rest_api.ts
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/javascripts/admin/onboarding.tsx (5)
frontend/javascripts/libs/react_helpers.tsx (1)
useFetch(35-52)frontend/javascripts/admin/rest_api.ts (2)
getEditableTeams(333-339)sendInvitesForOrganization(1729-1746)frontend/javascripts/types/api_types.ts (1)
APITeamMembership(308-312)frontend/javascripts/dashboard/dataset/helper_components.tsx (1)
DividerWithSubtitle(80-82)frontend/javascripts/admin/user/permissions_and_teams_modal_view.tsx (1)
PermissionsAndTeamsComponent(63-249)
frontend/javascripts/admin/user/permissions_and_teams_modal_view.tsx (4)
frontend/javascripts/types/api_types.ts (2)
APITeamMembership(308-312)APIUser(338-353)frontend/javascripts/libs/react_helpers.tsx (1)
useFetch(35-52)frontend/javascripts/admin/rest_api.ts (2)
getEditableTeams(333-339)updateUser(194-199)frontend/javascripts/dashboard/dataset/helper_components.tsx (1)
DividerWithSubtitle(80-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (3)
frontend/javascripts/admin/onboarding.tsx (1)
19-36: New invite permissions/teams wiring looks consistentThe added imports (REST helpers,
DividerWithSubtitle,useFetch,APITeamMembership,PERMISSIONS,PermissionsAndTeamsComponent) and their usage inInviteUsersModalline up cleanly: the modal now delegates all permission/team UI toPermissionsAndTeamsComponentand passes the resultingselectedTeams/selectedPermissionintosendInvitesForOrganizationas expected. No integration issues spotted here.Also applies to: 364-372
frontend/javascripts/admin/user/permissions_and_teams_modal_view.tsx (2)
3-4: Enums and shared props/helpers are clear and reusableExporting
ROLES/PERMISSIONSand introducingTeamRoleComponentProps/TeamRoleModalPropsmakes the permissions/teams model explicit and reusable across the admin UI (including onboarding). ThegetSingleUserMaybehelper cleanly encapsulates the “only when a single user is selected” logic. No issues here.Also applies to: 14-22, 24-41, 55-61
360-377: Modal integration and confirmation flow look good; ensure App context is presentHooking into
App.useApp().modal.confirmfor the permission-change confirmation keeps the dialog styling consistent with the rest of Ant Design, and wiringPermissionsAndTeamsComponentinto theModalwithonOk={handleUpdatePermissionsAndTeams}is straightforward.Just double-check that
PermissionsAndTeamsModalViewis always rendered under an<App>provider fromantd; otherwiseApp.useApp()won’t supply a realmodalinstance and the confirmation dialog will fail at runtime.
fm3
changed the title
Allow specifying user roles and teams already when creating invites.
URL of deployed dev instance (used for testing):
Steps to test:
TODOs:
Issues:
Context
$PR_NUMBER.mdfile inunreleased_changesor use./tools/create-changelog-entry.py)