Skip to content

fix(frontend): avoid PermissionsTabs crash before projects load#6993

Open
saudademjj wants to merge 2 commits intoFlagsmith:mainfrom
saudademjj:saudademjj/fix-6553-permissions-tabs-loading
Open

fix(frontend): avoid PermissionsTabs crash before projects load#6993
saudademjj wants to merge 2 commits intoFlagsmith:mainfrom
saudademjj:saudademjj/fix-6553-permissions-tabs-loading

Conversation

@saudademjj
Copy link

Thanks for submitting a PR! Please check the boxes below:

  • I have read the Contributing Guide.
  • I have added information to docs/ if required so people know about the feature.
  • I have filled in the "Changes" section below.
  • I have filled in the "How did you test this code" section below.

Changes

Closes #6553

  • guard the project permissions list against OrganisationStore.getProjects() returning undefined before the organisation data has loaded
  • add a regression test for the loading case so PermissionsTabs keeps rendering with an empty project list instead of throwing

How did you test this code?

  • on 100.92.89.12 via Docker node:22, ran npm run test:unit -- --runInBand web/components/__tests__/PermissionsTabs.test.ts
  • on 100.92.89.12 via Docker node:22, ran npx eslint web/components/PermissionsTabs.tsx web/components/__tests__/PermissionsTabs.test.ts
  • npm run typecheck currently fails on the repository baseline with many unrelated frontend errors on main; I did not change those files and did not treat them as blockers for this focused fix

@saudademjj saudademjj requested a review from a team as a code owner March 19, 2026 10:53
@saudademjj saudademjj requested review from Zaimwa9 and removed request for a team March 19, 2026 10:53
Copy link

@claude claude bot left a comment

Choose a reason for hiding this comment

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

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit in Settings → Usage.

Once credits are available, reopen this pull request to trigger a review.

@vercel
Copy link

vercel bot commented Mar 19, 2026

@saudademjj is attempting to deploy a commit to the Flagsmith Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions bot added the front-end Issue related to the React Front End Dashboard label Mar 19, 2026
@saudademjj
Copy link
Author

Code-side checks look healthy on my side: apply-labels, pre-commit.ci, and the targeted local validation for this fix all passed. The remaining red statuses appear to be Vercel team authorization gates rather than code failures.

@saudademjj
Copy link
Author

The failing checks are all Vercel deployments, each reporting "Authorization required to deploy". The code checks themselves are passing.

@talissoncosta
Copy link
Contributor

talissoncosta commented Mar 20, 2026

Hey @saudademjj, thanks for picking this up and for the thorough testing writeup! The fix is spot on 🎯

A couple of small suggestions:

1. Use ?? instead of ||

getProjects() returns store.model && store.model.projects, which can evaluate to false (not just undefined). Nullish coalescing is more precise here — it only kicks in for null/undefined, not falsy values:

- mainItems={(projectData || []).map((v) => ({
+ mainItems={(projectData ?? []).map((v) => ({

2. Drop the test file for now

We really appreciate the effort of adding a regression test — that's the right instinct! In this case though, the component is tightly coupled to OrganisationStore, which means testing it in isolation requires a lot of mocking (9 jest.mock() calls + a global Row stub). That makes the test brittle and expensive to maintain for what it covers.

We're planning to decouple the store dependency down the line (e.g. passing projectData as a prop), and that's when proper tests will become straightforward and valuable. For now, the one-line fix speaks for itself.

- Replace || with ?? for projectData fallback
- Remove PermissionsTabs.test.ts per review feedback
@saudademjj
Copy link
Author

Good points — updated: swapped to ?? and removed the test file. Thanks for the review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

front-end Issue related to the React Front End Dashboard

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeError when loading PermissionsTabs - projectData is undefined

2 participants