fix: hide granular project access behind flag#3073
Conversation
Greptile SummaryThis PR gates the granular project access UI (project-specific roles in member creation, editing, and listing) behind a new
Confidence Score: 4/5Safe to merge; the flag correctly gates all UI surfaces and the conditional API call, with no data loss or functional regression risk. The implementation is straightforward and follows the existing
Important Files Changed
Reviews (1): Last reviewed commit: "fix: hide granular project access behind..." | Re-trigger Greptile |
| $: supportsProjectRoles = | ||
| isCloud && | ||
| flags.granularProjectAccess({ account: $user, organization: $organization }) && | ||
| $currentPlan?.supportsOrganizationRoles; |
There was a problem hiding this comment.
The
supportsProjectRoles reactive statement does not coerce the last operand to boolean, so its type is boolean | undefined rather than boolean. While Svelte's {#if} treats undefined as falsy, the equivalent derived values in createMember.svelte and edit.svelte both use !! for consistency. Keeping the same style avoids subtle surprises if this value is ever passed to a function that checks strict type equality.
| $: supportsProjectRoles = | |
| isCloud && | |
| flags.granularProjectAccess({ account: $user, organization: $organization }) && | |
| $currentPlan?.supportsOrganizationRoles; | |
| $: supportsProjectRoles = | |
| isCloud && | |
| flags.granularProjectAccess({ account: $user, organization: $organization }) && | |
| !!$currentPlan?.supportsOrganizationRoles; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| export const load: PageLoad = async ({ url, params, route, depends, parent }) => { | ||
| const { account, organization } = await parent(); | ||
| depends(Dependencies.ORGANIZATION); | ||
| depends(Dependencies.MEMBERS); |
There was a problem hiding this comment.
depends() is called after await parent(), reversing the original order. If parent() throws (e.g., triggers a SvelteKit redirect), the depends() registrations are never reached. In practice the parent layout already registers both dependencies, so this is safe today, but moving the calls before the await matches the pre-existing convention and is more defensive.
| export const load: PageLoad = async ({ url, params, route, depends, parent }) => { | |
| const { account, organization } = await parent(); | |
| depends(Dependencies.ORGANIZATION); | |
| depends(Dependencies.MEMBERS); | |
| export const load: PageLoad = async ({ url, params, route, depends, parent }) => { | |
| depends(Dependencies.ORGANIZATION); | |
| depends(Dependencies.MEMBERS); | |
| const { account, organization } = await parent(); |
What does this PR do?
(Provide a description of what this PR does.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)