Multi-organization membership - #433
Merged
Merged
Conversation
Adds the UI for the new backend capability: an admin can grant an existing Refactor account membership of another organization, and can remove a member from one organization without touching their account. - UserApi.lookupByEmail hits GET /users?email= and returns 0 or 1 result. A null result deliberately covers both "no such email" and "a real user you may not see"; the copy says only "No user found with that email." so the UI can't be used to enumerate accounts. - AddMemberDialog gains an "Add existing member" tab for admins and super admins, with an explicit Find step, a confirmation card, and a role select whose Member option submits the User role. - MemberCard gains a non-destructive "Remove from organization" action that DELETEs the /role sub-route, kept visually distinct from Delete. - Surfaces the user_already_in_organization, last_organization_admin and user_belongs_to_multiple_organizations conflicts with their own messages. - Drops the dead top-level useUserList/UserApi.list, which called a GET /users route that now requires an email parameter.
Adds a Clear button to the lookup result card, so finding the wrong person is recoverable without closing the dialog. Also invalidates the match when the email field is edited. Previously the found user survived an edit, so an admin who searched one address, then retyped another without pressing Find, would add the first person while the field displayed the second.
Both the create-new and add-existing flows now offer an optional coach
picker listing the organization's current members. The selection posts a
coaching relationship after the member exists, reusing the existing
POST /organizations/{id}/coaching_relationships endpoint.
The two calls are not atomic, so a failed assignment reports the member as
added with guidance to assign a coach from the member list. The reverse
failure cannot happen, and a coachless member is recoverable from the UI.
A found user is kept off their own coach list.
Adding a member and assigning their coach were two requests, and the backend sends the invitation email at the end of the first one. A failed coach assignment therefore left the person invited with no coach, which the dialog could only report as a warning. Both endpoints now take an optional coach_id and create the relationship in the same transaction, so the coach rides along with the member request and a failure fails the whole add. The partial-failure handling (assignSelectedCoach, the warning toasts) is gone with the state it described. coach_id is omitted rather than sent as null when no coach is chosen.
Drives the real stack rather than mocking the API like the other e2e specs: creates a member with a coach, creates an organization, attaches an existing user as its admin, and schedules a session across organizations. Skipped unless LIVE_E2E=1, since it is not idempotent and needs a freshly seeded database. Step d pins a finding rather than a success: a brand new organization's admin cannot add anyone to it, because the lookup only returns users who share an organization they already administer, and a new organization is empty.
The Upcoming and Previous lists fetched every session the user participates in, ignoring the organization switcher, so a user in two organizations saw both organizations' sessions at once. The defect predates multi-org membership but was unreachable until a user could belong to two organizations. Both hooks now take an optional organizationId and put organization_id in the params object, which is also the SWR key. The key matters: without it, switching organizations would serve the previous organization's cached list. The dashboard card threads its currentOrganizationId down through the bucket components; omitting the value keeps the previous unscoped behavior.
…ed org
The avatar rendered a hardcoded "RG" at all three sites: the collapsed
sidebar, the expanded trigger, and every row of the dropdown. Every
organization looked like Refactor Group, which only became visible once a
user could belong to more than one.
organizationInitials derives up to two letters from the name, taking the
first letter of the first two words ("Refactor Group" -> "RG") or the first
two letters of a single-word name ("BigTable" -> "BI"), and falls back to
"?" when there is no name.
Also drops the LOGO placeholder constant. It pointed at /placeholder.svg,
which does not exist in public/, so every avatar issued a 404 before
falling through to the initials. Leaving src undefined reaches the fallback
directly.
The switcher's existing tests now scope their assertions to the listbox:
the trigger renders the selected organization's name too, so a bare
getByText matched twice.
Live verification of the session org-scoping fix found the dashboard still
bleeding, just not on the surface that was reported. The Coaching Sessions
card's Upcoming and Previous tabs were correct, but the Upcoming Session
card and the Goals Overview beside it still showed another organization's
session, because useTodaysSessions never passed the selected organization
to the hook that now accepts one.
Also treats internal capitals as word boundaries when deriving avatar
initials, so a camel or Pascal case name reads as its own parts
("BigTable" -> "BT", not "BI"). Multi-word names are unchanged.
Adds a repeatable read-only live e2e spec covering both the avatar and the
absence of cross-organization session bleed. Unlike multi-org-live.spec.ts
it creates nothing, so it can be re-run against an already-seeded database.
join-session-popover needs no change: it filters by coaching_relationship_id,
and a relationship belongs to exactly one organization.
jhodapp
changed the title
- lookupByEmail returns Option<UserLookupResult> instead of a nullable union, per the Strict Typing and Nullability standard. New API boundary, so there was no legacy pressure to match. - TODO(rs#374) at the two remaining unscoped session fetches, both in the actions path, recording that they are deliberate and what unblocks them. - The Find button shows "Finding..." while in flight, matching Remove. - attachExisting returns Promise<void>. The response was typed User but never parsed or consumed, so the type claimed more than it delivered. - Deleting a member refreshes only on success; the failure path left the list unchanged. - One surrogate-safe idiom in both branches of organizationInitials. - Drop a stray console.log in member-card, and the isACoach binding it was the only consumer of.
The Add Member dialog hardcoded "Refactor", which is not the product name. Both mentions now read siteConfig.name, and the existing-member copy names the organization being added to rather than saying "this organization". Falls back to "you are viewing" for the brief window before the current organization resolves, so the sentence never renders with a gap in it.
jhodapp
marked this pull request as ready for review
August 7, 2026 16:48
Contributor
Greptile SummaryThe PR adds multi-organization membership management and scopes session data to the selected organization.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains from the previously reported issues. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/components/ui/members/add-member-dialog.tsx | Adds guarded existing-user lookup, role selection, coach assignment, and correctly resolves the previously reported stale-response and configuration-import issues. |
| src/components/ui/dashboard/coaching-sessions-card.tsx | Defers session bucket rendering until an organization is selected, resolving the previously reported unscoped cold-load request. |
| src/lib/api/coaching-sessions.ts | Adds organization scoping to session request parameters and cache-key inputs. |
| src/lib/hooks/use-todays-sessions.ts | Scopes today-session fetching to the selected organization and disables fetching before selection. |
| src/components/ui/organization-switcher.tsx | Displays initials derived from the currently selected organization. |
| src/components/ui/members/member-card.tsx | Changes member removal to delete only the organization membership rather than the shared account. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Select organization] --> B[currentOrganizationId available]
B --> C[Mount session buckets]
C --> D[Organization-scoped SWR key]
D --> E[Scoped session request]
F[Add member dialog] --> G{New or existing user}
G -->|New| H[Create user membership]
G -->|Existing| I[Lookup and attach membership]
H --> J[Optional coach assignment]
I --> J
Reviews (8): Last reviewed commit: "fix(dashboard): wait for the organizatio..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Editing the email cleared the found user, but an in-flight lookup that resolved afterwards wrote its result back over that. The confirmation card then showed an account the email field no longer named, and Add attached that account instead of the one on screen. Each lookup now carries a token. Editing the field or starting another lookup bumps it, and a reply whose token is stale is dropped. Also takes the product name as a prop rather than importing siteConfig. The coding standards prohibit leaf components reading global config; the members page reads it once and threads it down.
The annotation referenced React without importing it. Types are erased and __tests__ sits outside the tsc include, so it never surfaced.
Finding someone who already belongs to the organization built the whole confirmation card, so an admin picked a role and a coach before the request came back 409. The lookup now checks the member list it already has and says so immediately. The server check stays: the member list can be stale, and is absent entirely for callers that do not pass one. Both paths share one message string so they cannot drift. This makes the coach picker's self-exclusion unreachable, since the picker and the new check read the same member list, so a found user can no longer appear in it. Removed the filter and reframed the test that covered it.
Removing a member with session history returns a 409 the member card had no handler for, so it fell through to "Error removing member from this organization" and the actionable message never reached the toast. Maps user_has_coaching_history like the other organization conflicts.
The lookup behind that tab only returns users in organizations the requester administers, and adding someone to an organization they already belong to is rejected. For an admin of a single organization those two sets are exact complements, so the tab can only ever end in "already a member". canAddExistingMembers answers whether the flow has candidates: true for a super admin, or for an admin of two or more organizations. The container computes it from the session's roles and threads it down, leaving the dialog free of policy. Membership in a second organization does not count. Visibility requires Admin there, so a member-only organization contributes nobody.
Ten cases against the real backend and the seeded local users, in real Chrome. The @single-org cases are read-only. The @multi-org cases need an operator to promote ehab to Admin of Refactor Group first, since no seeded user administers two organizations, and that is the whole condition under test. Verified by reverting ehab to a single-org admin, which fails the tab test against unchanged code, so the gate is what drives it rather than the assertion happening to hold.
A cold load fetched sessions before the organization resolved. The hooks kept valid SWR keys with organization_id omitted, which the backend reads as unscoped, so eight requests went out and rendered every organization's sessions before the scoped result replaced them. The card no longer mounts the buckets until the organization is known, and the today's-sessions hook passes a null user id until then, which is the hook's existing way to skip a fetch. The new e2e asserts no session request ever leaves without an organization. Reverting both gates fails it with eight offenders. Pins browser time in that spec. Two assertions depended on where a fixed seeded session fell relative to the current day, and broke once the date rolled over.
github-project-automation
Bot
moved this from Review
to ✅ Done
in Refactor Coaching Platform
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Frontend for multi-organization membership: adds an existing user to an organization with an explicit role and an optional pre-assigned coach, and fixes the cross-organization leaks that became reachable once a user could belong to two organizations.
Pairs with refactor-group/refactor-platform-rs#376. They must land together: the session hooks now send an
organization_idthe backend only accepts after that PR merges.Changes
add-member-dialog.tsxbecomes two modes, gated onisAdminOrSuperAdmin: Create new member (unchanged) and Add existing member (find by email, confirm, pick a role, add). Copy states the shared-account semantics, since an org admin is less likely than a SuperAdmin to reason about them.organizationId, included in the request and in the SWR key. Without it in the key, switching organizations serves the previous one's cached list, which looks exactly like the bug being fixed.organization-switcher.tsxderives avatar initials from the selected organization instead of a hardcoded"RG", at all three render sites. Internal capitals count as word boundaries, so "BigTable" reads "BT" and "Refactor Group" reads "RG".use-todays-sessions.tspasses the selected organization, fixing the Upcoming Session and Goals Overview cards.useUserList()/UserApi.list()that called a nonexistentGET /users.Screenshots / Videos Showing UI Changes (if applicable)
If you're only 1 organization admin user
If you're an organization admin of 2 or more organizations or a super admin
...you can add existing members to your organization
They'll be able to switch between these organizations then when logged in.
Testing Strategy
npm run lint(0 errors, 9 pre-existing warnings),npx tsc --noEmitclean,npm testat 156 files / 1671 tests.Two live Playwright specs against a real backend and Chrome, both
LIVE_E2E=1gated:multi-org-live.spec.tswalks the whole driving use case. Not idempotent, needs a fresh database.multi-org-scoping-live.spec.tsis read-only and repeatable: it pins the avatar initials, that session requests carry the selected organization and refetch on switch, and that neither the Upcoming/Previous tabs nor the Upcoming Session card show another organization's sessions.Every new assertion was sabotage-checked. Two of those checks caught real gaps rather than confirming a guess:
organization_idin the request but removing it from the SWR key left the request-shape test passing while only the cache test failed.use-todays-sessionswas a separate unscoped call site. Its existing unit test was named "should fetch sessions from all organizations", pinning the old behavior as correct.Concerns
organization_idis rejected as an unknown field.GET /users/{user_id}/actionsto gain anorganization_idfirst, tracked in Scope remaining user-scoped read endpoints by organization (actions, goals, coaching-relationships) refactor-platform-rs#374.useUserListwas deleted; the members page uses the org-scoped hook of the same name fromlib/api/organizations/users.ts, which is easy to confuse when reviewing.