Skip to content

fix: hide granular project access behind flag#3073

Merged
HarshMN2345 merged 1 commit into
mainfrom
fix-hide-granular-project-access-flag
May 30, 2026
Merged

fix: hide granular project access behind flag#3073
HarshMN2345 merged 1 commit into
mainfrom
fix-hide-granular-project-access-flag

Conversation

@HarshMN2345
Copy link
Copy Markdown
Member

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.)

@HarshMN2345 HarshMN2345 requested a review from Meldiron May 30, 2026 18:30
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 30, 2026

Greptile Summary

This PR gates the granular project access UI (project-specific roles in member creation, editing, and listing) behind a new granular-project-access feature flag, and skips the listProjects API call in the page load function when the flag is off.

  • Adds granularProjectAccess to flags.ts using the existing isFlagEnabled pattern, and threads it through the load function (via parent()) and all three member management components.
  • The conditional listProjects fetch in +page.ts correctly avoids the network call when the flag is disabled, and null for orgProjects was already a handled case before this PR.

Confidence Score: 4/5

Safe 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 multiDb flag pattern exactly. The two style observations — missing !! coercion in +page.svelte and depends() being called after await parent() — are cosmetic and have no runtime impact.

+page.ts and +page.svelte have small style deviations worth a second look.

Important Files Changed

Filename Overview
src/lib/flags.ts Adds granularProjectAccess flag using the same isFlagEnabled pattern as the existing multiDb flag; no issues.
src/routes/(console)/organization-[organization]/members/+page.ts Calls await parent() before depends(), inverting the original ordering; benign in practice since parent layouts already register those deps, but depends() calls won't execute if parent throws a redirect.
src/routes/(console)/organization-[organization]/members/+page.svelte New supportsProjectRoles reactive statement doesn't coerce to boolean (missing !!) unlike the equivalent logic in createMember.svelte and edit.svelte; functionally safe in Svelte {#if} blocks but inconsistent.
src/routes/(console)/organization-[organization]/createMember.svelte Adds flag check to supportsProjectRoles using $derived and !! coercion, consistent with edit.svelte; no issues.
src/routes/(console)/organization-[organization]/members/edit.svelte Adds flag check to supportsProjectRoles using $derived and !! coercion, mirroring createMember.svelte; no issues.

Reviews (1): Last reviewed commit: "fix: hide granular project access behind..." | Re-trigger Greptile

Comment on lines +65 to +68
$: supportsProjectRoles =
isCloud &&
flags.granularProjectAccess({ account: $user, organization: $organization }) &&
$currentPlan?.supportsOrganizationRoles;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 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.

Suggested change
$: 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!

Comment on lines +8 to 11
export const load: PageLoad = async ({ url, params, route, depends, parent }) => {
const { account, organization } = await parent();
depends(Dependencies.ORGANIZATION);
depends(Dependencies.MEMBERS);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 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.

Suggested change
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();

@HarshMN2345 HarshMN2345 merged commit aef6589 into main May 30, 2026
4 checks passed
@HarshMN2345 HarshMN2345 deleted the fix-hide-granular-project-access-flag branch May 30, 2026 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants