CMS-247 Apply new Studio design to Users page#160
Conversation
📝 WalkthroughWalkthroughThis PR implements the CMS-247 Users page redesign by adding authorization contract specs, centralizing grant-handling logic into a reusable model module, and refactoring the component to use typed grant helpers and design-system-compliant UI patterns while preserving existing user-management contracts and owner safeguards. ChangesUsers Page Redesign with Authorization Contracts
Sequence DiagramsequenceDiagram
participant User
participant InviteDialog as InviteUserDialog
participant Model as users-page-model
participant API as Backend
participant UsersTable
User->>InviteDialog: Click "Invite user"
User->>InviteDialog: Enter email, select role, optionally select folder
InviteDialog->>Model: Call createGrantInput(role, pathPrefix, environment, project)
Model->>Model: Determine scopeKind (global/folder_prefix/project)
Model->>InviteDialog: Return typed grant payload
InviteDialog->>API: POST /api/v1/auth/users/invite with grants
API->>API: Validate owner-role restrictions per spec
API->>User: Invite sent / error
API->>UsersTable: Pending invite appears with derived role + scope label
User->>UsersTable: Click "Edit role" on existing user
UsersTable->>InviteDialog: Open EditRoleDialog with currentRole
InviteDialog->>Model: Call createUpdatedGrants(newRole, pathPrefix, existingGrants)
Model->>InviteDialog: Return updated grants array
InviteDialog->>API: POST /api/v1/auth/users/{id}/grants with updated grants
API->>API: Validate update does not demote final owner
API->>UsersTable: User row role/scope updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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 |
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 (1)
docs/specs/SPEC-005-auth-authorization-and-request-routing.md (1)
597-604:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
Invite grant inputmarksenvironmentoptional, but folder scope requires itLine 602 currently says
environmentis optional, but invitefolder_prefixgrants are validated server-side as requiringproject,environment, andpathPrefix. Please tighten this contract text so clients don’t implement an invalid payload shape.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/specs/SPEC-005-auth-authorization-and-request-routing.md` around lines 597 - 604, The spec currently marks the environment field as optional in the Invite grant input but server validation requires environment for project and folder_prefix scopes; update the contract so environment is required when scopeKind is "project" or "folder_prefix" (or simply required for non-global scopes). Modify the Invite grant input block (fields role, scopeKind, project, environment, pathPrefix) to reflect that environment is mandatory for project and folder_prefix grants and include a short comment indicating when project and pathPrefix are required.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/studio/src/lib/runtime-ui/app/admin/users-page-model.ts`:
- Line 1: The PR changed source under the published package "packages/studio"
but lacks a changeset; run the changeset generator (bun run changeset) in the
repo root, select the "packages/studio" package, choose the appropriate bump
type, provide a short summary, then commit the newly created changeset file to
the PR so that packages/studio/src/** changes are recorded; ensure the changeset
is included in the same branch before merging.
- Around line 83-100: The code returns a project-scoped grant without a project
when both fallbackProject and activeProject are null: detect when project
(computed from fallbackProject ?? activeProject ?? undefined) is undefined
before returning a scopeKind: "project" object and either throw a user-facing
error or return a validation result; update the branch in users-page-model.ts
around the project/scopeKind construction (the variables project,
trimmedPathPrefix, activeEnvironment, role) so that if trimmedPathPrefix is
falsy and project is undefined you do not emit { scopeKind: "project" } —
instead return an error/validation or require the caller to select a project.
Ensure the change preserves the existing folder_prefix branch that uses project
and activeEnvironment.
- Around line 146-159: The code currently assumes the edited grant is
currentGrants[0] (used in fallbackProject and when returning the updated list),
which is brittle; instead locate the specific grant being edited by its unique
id from the UI (e.g., selectedGrantId or the grant id passed into
createUpdatedGrants), use that matchedGrant.project for fallbackProject, and
return a new array that replaces only the matched grant: map currentGrants and
for each grant return editedGrant if grant.id === selectedGrantId, otherwise
return toGrantInput(grant). Update references in this block (createGrantInput
call, fallbackProject, and the return) to use the deterministically found
matchedGrant rather than currentGrants[0].
---
Outside diff comments:
In `@docs/specs/SPEC-005-auth-authorization-and-request-routing.md`:
- Around line 597-604: The spec currently marks the environment field as
optional in the Invite grant input but server validation requires environment
for project and folder_prefix scopes; update the contract so environment is
required when scopeKind is "project" or "folder_prefix" (or simply required for
non-global scopes). Modify the Invite grant input block (fields role, scopeKind,
project, environment, pathPrefix) to reflect that environment is mandatory for
project and folder_prefix grants and include a short comment indicating when
project and pathPrefix are required.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a2ac0923-6119-4f96-8693-99fe71ef598b
📒 Files selected for processing (6)
.ai/plans/2026-05-28-cms-247-users-page-redesign.mddocs/specs/SPEC-005-auth-authorization-and-request-routing.mddocs/specs/SPEC-006-studio-runtime-and-ui.mdpackages/studio/src/lib/runtime-ui/app/admin/users-page-model.tspackages/studio/src/lib/runtime-ui/app/admin/users-page.test.tsxpackages/studio/src/lib/runtime-ui/app/admin/users-page.tsx
| @@ -0,0 +1,159 @@ | |||
| import type { InviteUserInput, UserWithGrants } from "../../../users-api.js"; | |||
There was a problem hiding this comment.
Missing changeset for published package source changes
This PR modifies packages/studio/src/** TypeScript files; please include a changeset generated via bun run changeset before merge/update.
As per coding guidelines, **/packages/*/src/**/*.ts: “If a change touches src/** or package.json in a published package, create a changeset with bun run changeset before opening or updating the PR”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/studio/src/lib/runtime-ui/app/admin/users-page-model.ts` at line 1,
The PR changed source under the published package "packages/studio" but lacks a
changeset; run the changeset generator (bun run changeset) in the repo root,
select the "packages/studio" package, choose the appropriate bump type, provide
a short summary, then commit the newly created changeset file to the PR so that
packages/studio/src/** changes are recorded; ensure the changeset is included in
the same branch before merging.
| const project = fallbackProject ?? activeProject ?? undefined; | ||
| const trimmedPathPrefix = pathPrefix.trim(); | ||
|
|
||
| if (trimmedPathPrefix && activeEnvironment && project) { | ||
| return { | ||
| role, | ||
| scopeKind: "folder_prefix", | ||
| project, | ||
| environment: activeEnvironment, | ||
| pathPrefix: trimmedPathPrefix, | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| role, | ||
| scopeKind: "project", | ||
| ...(project ? { project } : {}), | ||
| }; |
There was a problem hiding this comment.
Project-scope grant can be emitted without project
If activeProject and fallbackProject are both null, Line 96-100 returns { scopeKind: "project" } without project, which is invalid for the backend contract and guarantees a failed mutation. Guard this case before returning or throw early with a user-facing error.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/studio/src/lib/runtime-ui/app/admin/users-page-model.ts` around
lines 83 - 100, The code returns a project-scoped grant without a project when
both fallbackProject and activeProject are null: detect when project (computed
from fallbackProject ?? activeProject ?? undefined) is undefined before
returning a scopeKind: "project" object and either throw a user-facing error or
return a validation result; update the branch in users-page-model.ts around the
project/scopeKind construction (the variables project, trimmedPathPrefix,
activeEnvironment, role) so that if trimmedPathPrefix is falsy and project is
undefined you do not emit { scopeKind: "project" } — instead return an
error/validation or require the caller to select a project. Ensure the change
preserves the existing folder_prefix branch that uses project and
activeEnvironment.
| const editedGrant = createGrantInput({ | ||
| role, | ||
| pathPrefix, | ||
| activeProject, | ||
| activeEnvironment, | ||
| fallbackProject: currentGrants[0]?.project ?? null, | ||
| }); | ||
|
|
||
| if (currentGrants.length <= 1) { | ||
| return [editedGrant]; | ||
| } | ||
|
|
||
| return [editedGrant, ...currentGrants.slice(1).map(toGrantInput)]; | ||
| } |
There was a problem hiding this comment.
createUpdatedGrants updates currentGrants[0] instead of the edited effective role grant
Line 151/158 assumes the first grant is the one being edited. If grant order differs from effective-role order, edits can preserve the actual highest/effective grant and appear to “not apply.” Please target the grant being edited deterministically (e.g., by matching selected role/scope source grant id from UI), not index 0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/studio/src/lib/runtime-ui/app/admin/users-page-model.ts` around
lines 146 - 159, The code currently assumes the edited grant is currentGrants[0]
(used in fallbackProject and when returning the updated list), which is brittle;
instead locate the specific grant being edited by its unique id from the UI
(e.g., selectedGrantId or the grant id passed into createUpdatedGrants), use
that matchedGrant.project for fallbackProject, and return a new array that
replaces only the matched grant: map currentGrants and for each grant return
editedGrant if grant.id === selectedGrantId, otherwise return
toGrantInput(grant). Update references in this block (createGrantInput call,
fallbackProject, and the return) to use the deterministically found matchedGrant
rather than currentGrants[0].
Summary
Verification
Summary by CodeRabbit
Release Notes
New Features
Documentation