5536 feature view accept pending invitations - #122
Conversation
The teams table and the members dialog each hardcoded bg-yellow-500. The invitations dialog needs the same square, so give it a token named for the domain rather than shipping a third copy. Value is #ffd200 from the design, not yellow-500's #eab308, so the existing squares change shade slightly. No .dark override, for the reason the other icon-bg tokens already state. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Transport for the invitee's own invitations, for #5536. Two of the three routes do not exist yet and are the subject of #6010, which also leaves the inbox path undecided. Both unknowns are confined to this module, so settling them is a one-line change here. TeamInvitation re-exports the generated TeamInvitationResponse rather than restating it, so regenerating the client turns any drift between what this assumed and what the backend ships into a compile error. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Its own bundle rather than keys in teams.json, since the components consuming it sit outside components/teams and a later surface elsewhere in the app should not reach into the teams bundle. The invitation sentence is one message with named placeholders so translators control word order, and the count uses ICU plural. Signed-off-by: Anna Effort <anna.effort@ibm.com>
One invitation as a sentence plus either its two actions or the outcome that replaced them. Accepted and declined are deliberately asymmetric, per node 5966:29865: accepted gets a check and full contrast, declined is muted with no icon. Roles map to display names with a fallback to the raw string, so an unrecognised role stays readable. Renders a div rather than an li, so a surface showing one invitation inline is not forced into list markup. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Presentational: it takes the list and callbacks and owns no fetching. Once nothing is left to act on it holds the confirmation for 4000ms and closes itself, reusing sonner's default duration so the app has one dwell rather than two. Pointer entry cancels the timer outright rather than pausing it, since a timer that resumes on pointer exit closes at an unpredictable moment. Radix's usual pause-while-focused signal is unusable because the dialog traps focus. Scroll is scoped to the list rather than the dialog. DialogContent's own overflow puts the close button inside the scroll container, so past roughly seven invitations the X scrolls out of reach. Resolving a row unmounts the focused button, so focus moves to the next actionable row or to the close button, which is what the new data-slot on the primitive is for. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Resolutions are committed after the server confirms, not optimistically: both actions are irreversible and the dialog stays open, so a rollback would visibly flip a row back. Named Data to keep it distinct from the context consumer of the same idea. An accidental second instance holds its own resolutions, which is the divergent count the provider exists to prevent, so the two should not be one import typo apart. Signed-off-by: Anna Effort <anna.effort@ibm.com>
One provider, one dialog, N triggers. Two mounted copies of a self-contained component would hold separate resolutions, so resolving in one would leave the other's count stale with nothing logged. Mounted in AppShell, which an app-wide fetch would not justify on its own. The fetch is gated on a mounted consumer instead, so a page with no trigger issues no request. That gating is also what lets this land ahead of #6010 with no feature flag: nothing calls the missing routes until a trigger is rendered. Focus return is handled here rather than by a DialogTrigger, which would force every trigger into one dialog's subtree. The element that opened the dialog is commonly gone by close, since resolving the last invitation unmounts every trigger. Signed-off-by: Anna Effort <anna.effort@ibm.com>
The Settings > Teams trigger, hidden at zero pending. Not yet rendered anywhere. Mounting it registers the consumer that starts the fetch, so the Teams page wiring waits on #6010. Takes the height and padding of the Create team button it will sit beside rather than the design's standalone 36px chip. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Measured against the Figma frame: 8px padding on all four sides with a content-driven height, 4px corners, and an explicit foreground on the label, which was inheriting its colour. The icon square is 17.5px rather than the drawn 20px. At 20px the chip stands 38px tall, and the Settings tab row reserves 32px for its toolbar contents, so the drawn size pushes the whole row taller. The three outline buttons opt out of the variant's shadow-xs, which reads as an unwanted shadow in light mode. Removing it at the variant belongs to #35, so these join the dead overrides that PR already sweeps up. Signed-off-by: Anna Effort <anna.effort@ibm.com>
The step this feature could not land earlier. Mounting the chip is what registers a consumer, which is what enables the fetch, so this is the only change that calls the routes IBM/mcp-context-forge#6010 has yet to add. Everything before it merges inert. Teams reacts to an accepted invitation through a counter in context rather than a callback prop, so a later surface elsewhere in the app can react independently of whichever one rendered the trigger. e2e/invitations.spec.ts stubs the three routes through the api-mock fixture, one test per visual state. Under E2E_REAL_API=true it becomes the integration test, once e2e/seed/seed.ts can seed a pending invitation for the test user. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Cut the build-time narrative: which routes did not exist yet, which alternatives were weighed, why a structure was chosen over another. That belongs in the PR and the issues, and goes stale in the code. What is left says what a reader needs to work with it: mount one hook instance, the row is not an <li>, the dwell never resumes, focus return is manual because the opener is usually gone. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Corners at 12px. The sentence and the action row lose their 16px inset, so both sit flush with the header against the dialog's own padding. Roles render lowercase, matching teams.json, with the English article inside the value: "an owner", "a member". pt-BR and es-ES carry no article, since "como proprietário" is right and "como um proprietário" is not. The resolved row keeps the height of the buttons it replaces, so accepting or declining no longer shifts the invitation below it, and the accepted check uses the success token. The list no longer scrolls. scrollbar-gutter-stable goes on DialogContent, which is a scroll container through the primitive; the property does not inherit, so it needs its own. Signed-off-by: Anna Effort <anna.effort@ibm.com>
The chip renders when invitations are pending and not otherwise, and the teams list refetches when acceptedCount increments. The refetch is what puts a newly joined team in the table, and it had no coverage at all. Signed-off-by: Anna Effort <anna.effort@ibm.com>
It was debug instrumentation for the HeaderProfileMenu scroll lock, with no assertions, and it failed both lint and format:check. Signed-off-by: Anna Effort <anna.effort@ibm.com>
The dwell now cancels on pointer movement or a key press during it, rather than on pointer entry: resolving the last invitation means clicking inside the dialog, so entry alone cancelled the dwell for every mouse user. The accept button's accessible name now contains its visible label in all three locales, so voice control can address it. The load error gets a localised headline over the sanitised detail, the pattern the Teams page already uses. sanitizeError only returns English. The chip moves out of the has-teams branch, so someone invited to a first team can still reach the invitation. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Expired invitations are filtered as they arrive. The inbox filters on is_active, which stays true past the expiry date, so they would otherwise be counted and offered a Join that cannot succeed. The dialog no longer stays open with nothing in it: a refetch that comes back empty closes it, and open() no-ops at a count of zero. An error keeps it open so the retry stays reachable. The sr-only region now names the row in flight and each outcome as it lands, rather than only reporting once everything is resolved. The toolbar no longer renders empty scaffolding, and the chip drops its trailing margin when it has no siblings. Signed-off-by: Anna Effort <anna.effort@ibm.com>
A role=status count of zero can never hold: HeaderQuickNav mounts a permanent announcer on every page. An open modal puts the chip in an aria-hidden subtree, so the role query has to opt into it. Outcome text now also appears in the announcer, so the row assertions are scoped to the row. Signed-off-by: Anna Effort <anna.effort@ibm.com>
There was a problem hiding this comment.
🟡 Changes recommended
Non-admin invitees cannot reach the feature, and refresh races can bypass the required confirmation dwell.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds invitee-facing pending team invitation management, including accept/decline actions, shared state, localization, accessibility, and tests.
Changes:
- Adds invitation API, state provider, count chip, and resolution dialog.
- Integrates invitations into Teams and refreshes memberships after acceptance.
- Adds localized strings, styling tokens, and unit/E2E coverage.
File summaries
| File | Description |
|---|---|
src/types/team.ts |
Re-exports invitation API type. |
src/types/invitation.ts |
Defines invitation action and resolution types. |
src/pages/Teams.tsx |
Adds invitation trigger and team refresh behavior. |
src/pages/Teams.test.tsx |
Tests Teams integration. |
src/index.css |
Adds the team icon color token. |
src/i18n/locales/pt-BR/invitations.json |
Adds Portuguese translations. |
src/i18n/locales/pt-BR/index.ts |
Registers Portuguese translations. |
src/i18n/locales/es-ES/invitations.json |
Adds Spanish translations. |
src/i18n/locales/es-ES/index.ts |
Registers Spanish translations. |
src/i18n/locales/en-US/invitations.json |
Adds English invitation messages. |
src/i18n/locales/en-US/index.ts |
Registers English messages. |
src/hooks/usePendingInvitationsData.ts |
Implements invitation queries and mutations. |
src/hooks/usePendingInvitationsData.test.ts |
Tests invitation data behavior. |
src/components/ui/dialog.tsx |
Identifies the dialog close control. |
src/components/teams/TeamsTable.tsx |
Applies the team icon token. |
src/components/teams/ManageTeamMembersDialog.tsx |
Applies the team icon token. |
src/components/layout/AppShell.tsx |
Mounts the shared invitation provider. |
src/components/invitations/PendingInvitationsProvider.tsx |
Coordinates shared invitation state and dialog. |
src/components/invitations/PendingInvitationsProvider.test.tsx |
Tests provider lifecycle and focus behavior. |
src/components/invitations/PendingInvitationsDialog.tsx |
Renders invitation states and auto-close behavior. |
src/components/invitations/PendingInvitationsDialog.test.tsx |
Tests dialog behavior and announcements. |
src/components/invitations/PendingInvitationsChip.tsx |
Adds the pending-count trigger. |
src/components/invitations/PendingInvitationsChip.test.tsx |
Tests trigger rendering and behavior. |
src/components/invitations/PendingInvitationItem.tsx |
Renders individual invitations and actions. |
src/components/invitations/PendingInvitationItem.test.tsx |
Tests invitation rows. |
src/api/invitations.ts |
Adds invitation API functions. |
src/api/invitations.test.ts |
Tests invitation API paths. |
e2e/invitations.spec.ts |
Covers invitation UI states end-to-end. |
Review details
- Files reviewed: 28/28 changed files
- Comments generated: 4
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
A refetch answered after the user acted no longer publishes: it would drop the row they just resolved along with its confirmation. The dialog also stays open while a request is in flight, so its outcome can be shown. The sr-only summary counts what is still pending rather than the original total, which it was calling pending after rows resolved. The post-accept teams refresh catches its rejection. useQuery.execute rethrows after recording the error, so the bare call leaked an unhandled rejection; the other refresh paths in the page already catch. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Settings lists the Teams tab only for is_admin and redirects everyone else off /app/settings/teams, so the badge was unreachable by the invitees it is for. The header renders on every authenticated page for every role, and the provider already wraps it. The badge itself is unchanged. The Teams toolbar goes back to what it was, and createTeamRef goes with it, since it existed only as the badge's focus fallback. The e2e suite missed this because its default user is an admin, so the new case signs in as a non-admin. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Drops the box from 38px to 28px: the h-auto override is gone, so size="sm" applies, with 6px of vertical padding and a 16px icon square to match. The half-pixel sizes go with it. Team icon chips move to the brand cyan, which --server-icon-bg already carries. That recolours the Teams table and the manage-members dialog too, both of which moved onto this token earlier in the branch. The count names the entity, since the badge is now app-wide rather than on the Teams page. Signed-off-by: Anna Effort <anna.effort@ibm.com>
|
Thanks for the additional review round @marekdano. 🙌 Fixed 1, 2 and 4 in abee073. No changes added for feedback item 3. Details:
Still blocked on IBM/mcp-context-forge#6010. @jonpspri |
marekdano
left a comment
marekdano
left a comment
There was a problem hiding this comment.
Not blocking, but good to fix
🟡 Medium — Stale invitation stays actionable while dialog is open
File: src/hooks/usePendingInvitationsData.ts:113
The staleness refresh (focus/visibilitychange) pauses while the invitations dialog is open. If an invitation's expires_at passes while the dialog sits open without the tab losing focus, the row still renders with an enabled "Join team" button. Clicking it calls acceptInvitation(), which the server rejects with a 400 — the exact failure the PR's expiry filtering was meant to prevent, just reached via a different path.
🟢 Low — Pending-count badge can go stale indefinitely
File: src/hooks/usePendingInvitationsData.ts:113
In a tab that's never blurred, nothing re-triggers a load() (only mount, focus/visibilitychange, or opening the dialog do). The header badge can keep showing an already-expired invitation as pending indefinitely. Display-only — no incorrect action results.
The expiry sweep runs while the dialog is closed, so a tab that is never blurred stops reporting lapsed invitations as pending. It refilters state rather than refetching. Accept rechecks expiry at the click, covering the open dialog where the sweep is paused. Decline stays unguarded. Signed-off-by: Anna Effort <anna.effort@ibm.com>
|
@marekdano, I confirmed the non-blocking feedback from your third review round: Feedback marked as low priority: The interval does not run while paused. Removing a row from the open dialog would unmount a focused button, discard the confirmation for an accept still in flight, and empty the list, which triggers the auto-close at Feedback marked as medium priority: Decline is deliberately not guarded. Declining an expired invitation should succeed, since it is the only way the invitee can clear the row, and I have proposed that behaviour for the decline route on IBM/mcp-context-forge#6010. Both endpoints this PR calls are still unimplemented on backend main. Four tests added, one of which asserts the interval does not run while paused. |
marekdano
left a comment
marekdano
left a comment
There was a problem hiding this comment.
🔴 High — expiry-click load() bypasses the paused guard and can clobber a sibling's accept confirmation
File: src/hooks/usePendingInvitationsData.ts:149-154
resolve()'s new early-return branch calls void load() unconditionally when accept is clicked on an expired invitation. Every other load() trigger in this file is gated by paused (the interval sweep at line 126, and refreshIfStale at line 102 both live inside if (!enabled || paused) return;) specifically because, as the author's own PR comment states: "Removing a row from the open dialog would... discard the confirmation for an accept still in flight, and empty the list, which triggers the auto-close at PendingInvitationsProvider.tsx:89." This new call sidesteps that guard by design ("covering the open dialog where the sweep is paused"), but load()'s actedSince check (line 79-83) only protects against a resolution that lands during the fetch — not one already committed and still on-screen dwelling before the fetch starts.
Failure scenario: Dialog is open with invitation A (valid) and B (about to expire). User accepts A — resolutions = {A: "accepted"}, A's row shows its confirmation check, dialog stays open because B is still pending. B then lapses while the dialog is open (sweep paused, row correctly stays visible per the new fix) and the user clicks "Join team" on it. isActionable(B) is false, so resolve() shows the toast and calls load(). Since nothing else is in-flight and A's resolution predates this fetch, actedSince is false, so the fresh server list (which the code's own "drop resolutions for invitations no longer published" comment at line 88-91 shows is expected to omit an already-accepted invitation) overwrites invitations, dropping A. PendingInvitationsProvider's invitations.length === 0 effect then closes the dialog immediately, cutting off A's confirmation before its 4s dwell — the same bug as review-round-1, reached through a new path.
None of the four new tests cover this: each exercises the expired invitation in isolation, with no prior resolution on a sibling invitation still displayed.
Suggested fix: skip the load() call (or skip applying its result) while paused/the dialog is open — the toast alone is enough feedback for the click; let the existing pause-aware sweep or the next open-triggered refetch() clear the row once it's safe to.
Accept on a lapsed row showed the toast and refetched. The server drops an invitation already accepted in the same dialog, so the fresh list took that row and its confirmation with it, emptying the dialog and triggering the auto-close. The toast alone is the response now. Signed-off-by: Anna Effort <anna.effort@ibm.com>
|
Thanks for the additional feedback, @marekdano ! To help keep track of prior rounds of feedback and fixes:
-> Fix applied: remove the call. The toast is the whole response now, and the row clears on the sweep or the next refetch on open. Gating The row stays on screen and still looks actionable until then, and a second click just re-toasts. Disabling it live would mutate the open dialog. Tests: the existing lapsed-accept case now asserts that no request is issued, and a new case covers the sibling scenario. Both tests fail against the old hook. |
There was a problem hiding this comment.
🟡 Changes recommended
The retry path can bypass the confirmation dwell, and in-flight controls have accessible names inconsistent with their visible labels.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
src/components/invitations/PendingInvitationItem.tsx:101
- When an accept is in flight, the visible label changes to “Joining...” (and its localized equivalents), but this unchanged
aria-labelremains “Join team...”. The visible text is therefore absent from the control's accessible name, contrary to the label-in-name requirement this UI is intended to satisfy. Let the busy-state text provide the accessible name; the live region already announces the team.
aria-label={intl.formatMessage(
{ id: "invitations.action.accept.aria" },
{ team: invitation.team_name },
)}
- Files reviewed: 33/33 changed files
- Comments generated: 2
- Review effort level: Balanced
Signed-off-by: Anna Effort <anna.effort@ibm.com>
marekdano
left a comment
marekdano
left a comment
There was a problem hiding this comment.
Happy with the changes!
The PR can be merged
- after implementation of GET /users/me/invitations and the decline route from IBM/mcp-context-forge#6010
- after the current conflicts are resolved
Signed-off-by: Anna Effort <anna.effort@ibm.com>
TeamSwitcher now reads the shared list from useTeamsContext, so the accepted-invitation effect calls the context refetch instead of its own useQuery. Tests that re-render outside the harness needed TeamsProvider, and the Teams remount assertion counts the provider's fetch too. Signed-off-by: Anna Effort <anna.effort@ibm.com>
Signed-off-by: Anna Effort <anna.effort@ibm.com>
|
The backend blocker is resolved. IBM/mcp-context-forge#6010 is closed, merged as IBM/mcp-context-forge#6960, so both endpoints this PR calls now exist on main:
The paths match what the client already calls, both are mapped in I refreshed Ready to merge, @gcgoncalves @gandhipratik203 @vishu-bh @marekdano |
Summary
Adds the invitee-facing UI for IBM/mcp-context-forge#5536. A count badge in the app header opens a dialog listing every pending invitation. Accepting decrements the count behind the dialog. Once nothing is left to act on, the dialog holds the confirmation for 4 seconds and closes itself.
V2-updatedSept14.mp4
PendingInvitationsProvider mounts in AppShell and owns the data; triggers are consumers of usePendingInvitations(). The fetch is gated on a mounted consumer, so a surface with no trigger issues no request.
The header renders inside AppShell inside AuthGuard on every authenticated page for every role, and the provider already wraps it, so this needs no new route and no change to the admin gate.
Blocked on IBM/mcp-context-forge#6010 and not mergeable until it lands: 2 of 3 routes do not exist yet: nothing lists the invitations addressed to the caller, and there is no decline route. The only DELETE on an invitation is cancel_team_invitation, which is gated on teams.manage_members and owner-or-inviter, so it is revoke rather than decline. Without the list there is no token, so even the working accept route is unreachable.
Resolutions are committed after the server confirms rather than optimistically, since both actions are irreversible and the dialog stays open. The inviter renders as an email address, because TeamInvitationResponse carries no name field.
The dwell that closes the dialog is cancelled by pointer movement or a key press while it is running, not by pointer entry. Entry cannot be the signal: resolving the last invitation means clicking inside the dialog, so entry would cancel the dwell for every mouse user and leave the self-close reachable only by keyboard. Arming clears any cancel from before the confirmation appeared, so the click that produced it does not count against it.
Expired invitations are filtered out as they arrive. The inbox filters on is_active, which stays true past the expiry date, so an expired invitation would otherwise be counted by the chip and offered a Join that returns 400. The dialog also never shows with nothing in it: a refetch that comes back empty closes it, and open() no-ops at a count of zero. A failed refetch keeps it open, since that is the only path on which the error and retry are reachable.
The accept button's accessible name contains its visible label in all three locales, so voice control can address it. The load error carries a localised headline over the sanitised detail, matching the Teams page, because sanitizeError only ever returns English. The sr-only region names the row in flight and each outcome as it lands, rather than only reporting once everything is resolved.
Also updated the Teams color and used it for the badge as the previous yellow option might have read as a warning.
Follow-up, not in scope: IBM/mcp-context-forge#6796 covers the untranslated role picker.