fix(organisations): don't fetch an organisation with a non-numeric id - #8222
fix(organisations): don't fetch an organisation with a non-numeric id#8222talissoncosta wants to merge 2 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe change adds the exported Estimated code review effort: 2 (Simple) | ~10 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
front-end
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 436d5fec-84ae-43c3-9994-bf9d58c18395
📒 Files selected for processing (3)
frontend/common/stores/organisation-store.jsfrontend/common/utils/__tests__/isNumericId.test.tsfrontend/common/utils/isNumericId.ts
4244e76 to
f7982b0
Compare
organisation-store seeds its id to the string 'account', and the
dispatcher falls back to it, so any refresh before an organisation had
loaded sent 'account' as the id. That fanned out into a 400 on projects/
and a 500 on organisations/{id}/users/, which surfaced locally as an
unhandled rejection because _data rejects with the raw Response.
getOrganisation now returns early on a non-numeric id. isNumericId
narrows before converting, since Number() also accepts [], ['5'] and
true, and requires a positive integer because these are serial keys.
Onboarding, where this showed up, is left alone. Its refresh has never
worked: the project switcher reads RTK, and createProject already
invalidates the Project tag, so nothing depended on the failing call.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
f7982b0 to
bc3c15c
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f789da4c-a1e3-49d2-b6ec-8ee89a67eeea
📒 Files selected for processing (3)
frontend/common/stores/organisation-store.jsfrontend/common/utils/__tests__/isNumericId.test.tsfrontend/common/utils/isNumericId.ts
'007' is accepted on purpose, since DRF coerces it to the key 7, but nothing held that in place. Raised by CodeRabbit on #8222. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
docs/if required so people know about the feature.Reviewing this: one guard, at
organisation-store.js:120, refusing an id the API cannot accept. The judgement call is what is not here: onboarding's refresh, which triggered this, is left alone because nothing reads it. Safe to skim the tests and three lint-driven reformats in the same file (a dead import, afilterreflow, the unnested ternary).Changes
/getting-startedsentorganisation=account, giving a 400 onprojects/and a 500 onorganisations/{id}/users/, plus a red[object Response]overlay locally.We send that id ourselves. None of the code below changes in this PR, it is just where the id comes from:
getOrganisationnow returns early on a non-numeric id, via a newisNumericId.Number()on its own is not enough, since it also accepts[],['5']andtrue, so the input is narrowed to a string or a number first and has to be a positive integer.Onboarding is left alone, because that refresh has never worked and nothing reads it. The project switcher is RTK (
BreadcrumbSeparator.tsx:198) andcreateProjectalready invalidates theProjecttag. The one Flux consumer of the organisation's project list is the organisation projects page, which fetches on mount (ProjectManageWidget.tsx:63).The nested ternary in the same file is unnested because the commit hook lints whole staged files. It keeps the original comparison rather than
localeCompare, which orders accents differently.Production hits the same two failures and just renders an empty organisation view.
Screenshots
Before, the local dev overlay:
How did you test this code?
isNumericIdunit tests (21)test:unit(422) clean, and no new typecheck errors in the touched files/getting-started: no red overlay, and noorganisation=accountrequest in the network tab