fix(organisations): don't fetch an organisation with a non-numeric id - #8222
Draft
talissoncosta wants to merge 3 commits into
Draft
fix(organisations): don't fetch an organisation with a non-numeric id#8222talissoncosta wants to merge 3 commits into
talissoncosta wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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
Loading the onboarding page locally showed a red dev overlay reading
`[object Response]`, with two failed requests behind it:
GET projects/?organisation=account 400 {"organisation":"Must be a valid integer."}
GET organisations/account/users/ 500
Both fan out from one call, getOrganisation in organisation-store, which
validates nothing. The overlay is the same failure surfacing: _data rejects with
the raw Response, so an uncaught one prints as [object Response].
Guard it with a shared isNumericId, so a caller passing a route param or an
unloaded store value skips the request rather than triggering a 400, a 500 and an
unhandled rejection. In production the same call quietly yields an empty
organisation view, which is harder to notice than the overlay.
This is containment, not the root cause: something still passes the string
"account", and the request's initiator stack will name it.
Also unnests a pre-existing ternary in the same file, because the commit hook
lints whole staged files and refused the commit otherwise. Kept the original
comparison rather than localeCompare, which orders accents differently.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
refreshOrganisation dispatched GET_ORGANISATION without an id, so the store fell back to its own, which starts as the string 'account'. Any refresh before an organisation had been fetched sent that as the id. The id is now optional on the action, so callers that know it can pass it. Existing callers behave as before.
Number() turns values that are not ids into integers, so a truthiness check let [], ['5'] and true through, and '0' passed while 0 did not. The input is now narrowed to string or number, and an id must be a positive integer, since these are serial primary keys.
talissoncosta
force-pushed
the
fix/organisation-store-guard-invalid-id
branch
from
August 6, 2026 02:47
4244e76 to
f7982b0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
docs/if required so people know about the feature.Changes
Loading
/getting-startedlocally ran two failed requests:Both come from one call, and the id is a literal string we send ourselves:
refreshOrganisationomits the id on purpose and leans onstore.id, whose seed value is the string'account'. Onboarding calls it before anything has populated that store, so'account'goes out as the organisation id.Three changes, smallest first:
refreshOrganisationtakes an optional id. Callers that know it can pass it; the seven that don't behave exactly as before.getOrganisationskips a non-numeric id, via a sharedisNumericId. This is defence for the other unseeded-store callers:project-storecallsrefreshOrganisationfour times, andEnvironmentAsideanduseUpdateProjectWithToastonce each, none of which know an id.isNumericIdnarrows its input before converting, becauseNumber()coerces things that are not ids: a truthiness check let[],['5']andtruethrough, and'0'passed while0did not. An id now has to be a positive integer, since these are serial primary keys.The guard alone would only silence this. It needed the id fix as well, or onboarding's refresh would go from failing loudly to being skipped quietly, and the newly created project would not reach the org store.
Two things worth knowing:
[object Response]because_datarejects with the rawResponse(_data.js:97), so it names nothing._datarejecting with anErrorcarrying status, URL and body is a worthwhile follow-up, and is why this took DevTools archaeology rather than reading a message.Also in here, because the commit hook lints whole staged files and refused otherwise: a pre-existing nested ternary in the same file is unnested. It keeps the original comparison rather than
localeCompare, which orders accents differently (ätestmoves from last to second).Touches
bootstrapOnboarding.ts, which #8217 also changes. Different lines, so whichever merges second wants a rebase.How did you test this code?
isNumericIdunit tests (21), including'account'itself, and a mutation check confirming six of them fail against the old implementationtest:unit(422) andtypecheckcleanVerified the sort change is behaviour-identical, accented names included
Confirmed the seven remaining
refreshOrganisation()callers pass no id, so they keep today's behaviour/getting-startedlocally: no red overlay, and noorganisation=accountrequest in the network tabThe project onboarding creates appears in the project switcher without a reload
Organisation projects page still lists projects and members
Switching organisation still loads the new one's projects