Skip to content

feat(FR-2209): add useCurrentUserProjectRoles hook with myRoles RBAC query#6655

Closed
yomybaby wants to merge 3 commits intomainfrom
04-13-feat_fr-2209_scope_admin_sessions_by_project_for_project_admins
Closed

feat(FR-2209): add useCurrentUserProjectRoles hook with myRoles RBAC query#6655
yomybaby wants to merge 3 commits intomainfrom
04-13-feat_fr-2209_scope_admin_sessions_by_project_for_project_admins

Conversation

@yomybaby
Copy link
Copy Markdown
Member

resolves #NNN (FR-MMM)

Checklist: (if applicable)

  • Documentation
  • Minium required manager version
  • Specific setting for review (eg., KB link, endpoint or how to setup)
  • Minimum requirements to check during review
  • Test case(s) to demonstrate the difference of before/after

Copy link
Copy Markdown
Member Author

yomybaby commented Apr 13, 2026


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • flow:merge-queue - adds this PR to the back of the merge queue
  • flow:hotfix - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has required the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 13, 2026

Coverage report for ./react

St.
Category Percentage Covered / Total
🔴 Statements
9.01% (+1.34% 🔼)
1770/19653
🔴 Branches
8.13% (+0.73% 🔼)
1115/13712
🔴 Functions
5.36% (+0.63% 🔼)
286/5337
🔴 Lines
8.7% (+1.35% 🔼)
1660/19082

Test suite run success

859 tests passing in 39 suites.

Report generated by 🧪jest coverage report action from ad9c279

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the admin compute session list to support RBAC-based scoping by introducing an optional scopeId GraphQL variable and applying it to all compute_session_nodes queries so project-level admins can be restricted to the currently selected project while superadmins remain unscoped.

Changes:

  • Derive a scopeId based on the user’s effective admin role and current project.
  • Add $scopeId: ScopeField to AdminComputeSessionListPageQuery and pass it to all compute_session_nodes calls.
  • Include scopeId in the query variables used by useLazyLoadQuery.

Comment on lines +74 to +77
// TODO(needs-backend): FR-2313 — domain scope for compute_session_nodes
const scopeId: string | undefined =
effectiveAdminRole === 'projectAdmin' && currentProject.id
? `project:${currentProject.id}`
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scopeId becomes undefined when effectiveAdminRole === 'projectAdmin' but currentProject.id is null/undefined, which contradicts the comment that projectAdmin should be scoped to the current project. This can unintentionally fall back to an unscoped (global) query; consider blocking render until currentProject.id is available or using a safe scoped value that cannot broaden access.

Suggested change
// TODO(needs-backend): FR-2313 — domain scope for compute_session_nodes
const scopeId: string | undefined =
effectiveAdminRole === 'projectAdmin' && currentProject.id
? `project:${currentProject.id}`
// If the current project is not yet available for a projectAdmin, use a
// non-matching sentinel scope instead of `undefined` so we never fall back
// to an unscoped/global query by accident.
// TODO(needs-backend): FR-2313 — domain scope for compute_session_nodes
const scopeId: string | undefined =
effectiveAdminRole === 'projectAdmin'
? currentProject.id
? `project:${currentProject.id}`
: 'project:__missing_current_project__'

Copilot uses AI. Check for mistakes.
@yomybaby yomybaby force-pushed the 04-13-feat_fr-2209_scope_admin_sessions_by_project_for_project_admins branch from ffdeb1d to 69b23bf Compare April 13, 2026 14:49
@github-actions github-actions Bot added size:M 30~100 LoC and removed size:S 10~30 LoC labels Apr 13, 2026
@yomybaby yomybaby changed the base branch from 04-13-feat_fr-2209_add_usecurrentuserprojectroles_hook_with_myroles_rbac_query to graphite-base/6655 April 14, 2026 08:07
@yomybaby yomybaby force-pushed the 04-13-feat_fr-2209_scope_admin_sessions_by_project_for_project_admins branch from 69b23bf to e23f4a1 Compare April 14, 2026 08:07
@yomybaby yomybaby force-pushed the graphite-base/6655 branch from 4181d2d to db18202 Compare April 14, 2026 08:07
@graphite-app graphite-app Bot force-pushed the graphite-base/6655 branch from db18202 to 9dc530a Compare April 14, 2026 08:43
@graphite-app graphite-app Bot force-pushed the 04-13-feat_fr-2209_scope_admin_sessions_by_project_for_project_admins branch from e23f4a1 to 6d49de6 Compare April 14, 2026 08:43
@github-actions github-actions Bot added area:ux UI / UX issue. area:i18n Localization size:L 100~500 LoC and removed size:M 30~100 LoC labels Apr 14, 2026
@graphite-app graphite-app Bot changed the base branch from graphite-base/6655 to main April 14, 2026 08:44
@graphite-app graphite-app Bot force-pushed the 04-13-feat_fr-2209_scope_admin_sessions_by_project_for_project_admins branch from 6d49de6 to ad9c279 Compare April 14, 2026 08:44
@yomybaby
Copy link
Copy Markdown
Member Author

Closing: dev-plan.md is already merged to main via stacked PRs; code changes in this PR are superseded.

@yomybaby yomybaby closed this Apr 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:i18n Localization area:ux UI / UX issue. size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants