-
Notifications
You must be signed in to change notification settings - Fork 74
Refactor: [Web] get team invitation handling to support role filtering and use single request #4003
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
Conversation
… simplify API requests. Updated comments for clarity and removed workarounds now that the Gauzy API bug is fixed.
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughSwitches invitation fetching from a two-request workaround to a single-request flow with optional multi-role filtering. Updates server requests, client service, and invite types accordingly. API route comments reflect the all-roles retrieval. Validation and error handling are aligned to the new single-call response. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as API Route (UI)
participant S as Server Request Layer
participant INV as Invite API
participant VAL as Validation
rect rgba(200,220,255,0.25)
note over UI,S: New flow — single request with optional multi-role filtering
UI->>S: getAllTeamInvitationsRequest({ teamId, tenantId, organizationId })
S->>INV: GET /invite?where[teamId]=...&where[role][name][i]=... (optional)
INV-->>S: 200 OK { data, total, ... }
S->>VAL: validatePaginationResponse(data)
VAL-->>S: Validated result
S-->>UI: Paginated invitations
end
sequenceDiagram
autonumber
participant S as Server Request Layer
participant INV as Invite API
participant M as Merge/Dedup
participant VAL as Validation
rect rgba(255,240,200,0.25)
note over S,VAL: Previous flow (replaced)
par EMPLOYEE
S->>INV: GET /invite?where[role][name]=EMPLOYEE
INV-->>S: Page A
and non-EMPLOYEE
S->>INV: GET /invite?where[role][name][0..n]=others
INV-->>S: Page B
end
S->>M: Combine + dedup
M-->>S: Merged page
S->>VAL: validatePaginationResponse(merged)
VAL-->>S: Result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ Finishing Touches
🧪 Generate unit tests
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 |
Please review these files and clean up the unused code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Summary
This PR refactors the team invitation API handling to eliminate a workaround that was necessary due to a backend Gauzy API bug. Previously, the API excluded EMPLOYEE roles by default, requiring the frontend to make two parallel requests (one for EMPLOYEE roles explicitly, one for non-EMPLOYEE roles) and then merge and deduplicate the results.
The main changes include:
Interface Updates: The ITeamInvitationsRequest
type is updated to accept roles?: ERoleName[]
instead of role?: ERoleName
, enabling multiple role filtering in a single request. Similarly, IGetInvitationRequest
is updated to use roles?: string[]
.
API Request Logic: The getTeamInvitationsRequest()
function now handles role arrays with proper query parameter formatting - using single role format for one role and indexed array format for multiple roles. The getAllTeamInvitationsRequest()
function is dramatically simplified from complex parallel request logic to a single API call.
Client Service: The client-side getTeamInvitations()
method is refactored to use a single request instead of the previous Promise.all
approach that combined two separate API calls and handled result deduplication.
Documentation Updates: Comments across multiple files are updated to remove "WORKAROUND" language and reflect the current single-request approach.
This refactor improves the codebase architecture by removing technical debt, enhances performance by reducing network calls from 2 to 1, and provides more flexible role filtering capabilities while maintaining full backward compatibility.
Confidence score: 4/5
- This PR is safe to merge with minimal risk as it removes workaround logic and simplifies the codebase
- Score reflects well-structured refactoring with proper fallback handling and maintained backward compatibility
- Pay close attention to the role filtering logic in
getTeamInvitationsRequest()
to ensure query parameter formatting works correctly
5 files reviewed, 2 comments
apps/web/core/services/client/api/organizations/teams/invites/invite.service.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Please review these files and clean up the unused code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/api/invite/route.ts (1)
8-10
: Unauthorized response lacks 401 status codeThis path returns 200 with
{ error: 'Unauthorized' }
, which can break clients and caching layers expecting proper HTTP semantics. Align with other routes (e.g., roles/options) and return 401.Apply:
- if (!user) { - return NextResponse.json({ error: 'Unauthorized' }); - } + if (!user) { + return NextResponse.json({ error: 'Unauthorized' }, { status: 401 }); + }
🧹 Nitpick comments (4)
apps/web/core/types/interfaces/user/invite.ts (1)
76-81
: Unify single vs multi-role query params (keep BC).Keep role? for backward-compat, add roles?: ERoleName[] and mark role? deprecated.
export interface TeamInvitationsQueryParams { tenantId: string; organizationId: string; - role?: ERoleName; + /** @deprecated use roles instead */ + role?: ERoleName; + roles?: ERoleName[]; teamId: string; }apps/web/core/services/client/api/organizations/teams/invites/invite.service.ts (2)
111-122
: Fix stray characters in comment + small cleanup.There are two spurious “˝˝” characters; remove to avoid noise in diffs/search.
- // Add role filter if roles are specified˝˝ + // Add role filter if roles are specified
99-106
: End-to-end enum typing for roles.IGetInvitationRequest currently exposes roles?: string[]; prefer ERoleName[] to catch typos at compile time (server request already uses ERoleName[]).
Also applies to: 111-122
apps/web/app/api/invite/route.ts (1)
8-10
: Normalize error payload shape across routesThis file uses
{ error: ... }
whileroles/options
uses{ errors: ... }
. Consider standardizing to one key to simplify client handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/api/invite/route.ts
(1 hunks)apps/web/app/api/roles/options/route.ts
(1 hunks)apps/web/core/services/client/api/organizations/teams/invites/invite.service.ts
(2 hunks)apps/web/core/services/server/requests/invite.ts
(3 hunks)apps/web/core/types/interfaces/user/invite.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/core/services/server/requests/invite.ts (1)
apps/web/core/types/schemas/user/invite.schema.ts (1)
TInvite
(159-159)
apps/web/core/services/client/api/organizations/teams/invites/invite.service.ts (3)
apps/web/core/types/interfaces/user/invite.ts (1)
IGetInvitationRequest
(83-87)apps/web/core/types/schemas/user/invite.schema.ts (2)
TInvite
(159-159)inviteSchema
(39-42)apps/web/core/types/schemas/utils/validation.ts (1)
validatePaginationResponse
(79-90)
🪛 GitHub Actions: Knip Review - Cleanup Unused Code - WEB
apps/web/core/types/interfaces/user/invite.ts
[error] 1-1: Unused code detected in changed file during unused-code-check step.
apps/web/core/services/server/requests/invite.ts
[error] 1-1: Unused code detected in changed file during unused-code-check step.
⏰ 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: Codacy Static Code Analysis
- GitHub Check: deploy
🔇 Additional comments (7)
apps/web/core/types/interfaces/user/invite.ts (1)
1-88
: All exported types/interfaces in invite.ts are referenced; no unused exports foundLikely an incorrect or invalid review comment.
apps/web/core/services/client/api/organizations/teams/invites/invite.service.ts (1)
131-138
: Single-call fetch + schema validation — LGTM.This removes the workaround cleanly and validates the paginated payload.
apps/web/core/services/server/requests/invite.ts (3)
54-60
: Multi-role support implementation — LGTM.Role-array handling and query construction look correct and consistent with client.
Also applies to: 68-100
102-120
: Simplified “all roles” path — LGTM.Single request with no role filter is the right fix post-backend change.
1-225
: Trim unused exports
Remove exports inapps/web/core/services/server/requests/invite.ts
that aren’t referenced elsewhere.apps/web/app/api/roles/options/route.ts (1)
72-80
: Add inline note clarifying roles filteringThe updated comment correctly highlights that omitting
roles
fetches all invitations. Add a brief inline note that passingroles?: ERoleName[]
will restrict results to those roles. Manual verification required—placeholder-based curl commands didn’t execute, so please test this behavior in your environment.apps/web/app/api/invite/route.ts (1)
12-20
: Comment accurately documents all-roles retrievalThe note matches the new single-request flow and default behavior when
roles
is omitted.
apps/web/core/services/client/api/organizations/teams/invites/invite.service.ts
Show resolved
Hide resolved
@CREDO23 please review PR and let us know if you spot any issues / things to improve etc |
@CREDO23 it based on this Slack Conversation https://evereq.slack.com/archives/GL1QGDRD5/p1756140738057259 Let me know if you need more info, take time to review it carefully please |
Refactor Get team invitation API handling to support role filtering and use single request
Previously we made 2 parallel requests to get all team invitations due to a Gauzy API bug that excluded EMPLOYEE roles by default. The backend bug is now fixed.
invitation-request.mp4
Solution
ITeamInvitationsRequest
to acceptroles?: ERoleName[]
instead ofrole?: ERoleName
getTeamInvitationsRequest()
to handle role arraysgetAllTeamInvitationsRequest()
to use single API callgetTeamInvitations()
to use single requestBenefits
No breaking changes. All existing functionality preserved.
Summary by CodeRabbit