Skip to content

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

Merged
talissoncosta merged 2 commits intoFlagsmith:mainfrom
saudademjj:saudademjj/fix-6553-permissions-tabs-loading
Apr 6, 2026
Merged

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

Conversation

@saudademjj
Copy link
Copy Markdown
Contributor

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
Copy Markdown

@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
Copy Markdown

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
Copy Markdown
Contributor 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
Copy Markdown
Contributor Author

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

@talissoncosta
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

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

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 3, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
flagsmith-frontend-preview Ready Ready Preview, Comment Apr 3, 2026 5:47pm
flagsmith-frontend-staging Ready Ready Preview, Comment Apr 3, 2026 5:47pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Ignored Ignored Preview Apr 3, 2026 5:47pm

Request 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