Uh oh!
There was an error while loading. Please reload this page.
fix(backend): guard deleteOrganization against empty organization ID - #9036
Conversation
deleteOrganization() was the only ID-based method on OrganizationAPI
without a requireId() guard. Because joinPaths() drops falsy segments,
deleteOrganization('') built the path /organizations and issued a DELETE
to the collection route instead of failing fast locally. Add the guard so
an empty ID throws 'A valid resource ID is required.' like its siblings.The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: cd53787 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthrough
ChangesdeleteOrganization ID validation
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
@clerk/astro@clerk/backend@clerk/chrome-extension@clerk/clerk-js@clerk/electron@clerk/electron-passkeys@clerk/eslint-plugin@clerk/expo@clerk/expo-passkeys@clerk/express@clerk/fastify@clerk/hono@clerk/localizations@clerk/nextjs@clerk/nuxt@clerk/react@clerk/react-router@clerk/shared@clerk/tanstack-react-start@clerk/testing@clerk/ui@clerk/upgrade@clerk/vuecommit: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/backend/src/api/endpoints/OrganizationApi.ts (1)
363-365: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the new fail-fast behavior on this public API.
deleteOrganization()now throws locally for empty IDs, but that contract still isn’t reflected in JSDoc on this reference-facing method. Please document the validation/throw behavior here, and loop in Docs if this surface is rendered in generated reference docs.As per coding guidelines, "All public APIs must be documented with JSDoc"; as per path instructions, "If a PR adds or changes public/reference-facing API surface area, check whether the corresponding JSDoc is present, accurate, and aligned with the implementation" and "leave a review note reminding the contributor that the Docs team may need to review the change."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/backend/src/api/endpoints/OrganizationApi.ts` around lines 363 - 365, The public OrganizationApi.deleteOrganization method now fail-fasts on empty IDs via requireId(organizationId), but its JSDoc does not document this validation or thrown behavior. Update the JSDoc on deleteOrganization to describe the required non-empty organizationId and that it throws locally when the ID is missing/invalid, and note that the Docs team should review this reference-facing API if it is included in generated docs.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/backend/src/api/endpoints/OrganizationApi.ts`:
- Around line 363-365: The public OrganizationApi.deleteOrganization method now
fail-fasts on empty IDs via requireId(organizationId), but its JSDoc does not
document this validation or thrown behavior. Update the JSDoc on
deleteOrganization to describe the required non-empty organizationId and that it
throws locally when the ID is missing/invalid, and note that the Docs team
should review this reference-facing API if it is included in generated docs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: fb335c10-7f8d-496c-9149-d56fad063b44
📒 Files selected for processing (3)
.changeset/delete-organization-require-id.mdpackages/backend/src/api/__tests__/OrganizationApi.test.tspackages/backend/src/api/endpoints/OrganizationApi.ts
API Changes Report
Summary
No API Changes DetectedAll packages have stable APIs with no detected changes. Report generated by Break Check Last ran on |
OrganizationAPI.deleteOrganization()was the lone ID-based method on that client without arequireId()guard. SincejoinPaths()filters out falsy segments,deleteOrganization('')quietly built/organizationsand fired aDELETEat the collection route instead of throwing locally. This adds the guard so an empty ID fails fast withA valid resource ID is required., matching every sibling method.Pre-existing on
main(spotted by CodeRabbit on #8853); pulling it out as its own one-liner rather than folding a behavior change into that docs-only PR. The added test confirms the empty-ID call now rejects locally (it fails without the guard).Summary by CodeRabbit
Bug Fixes
Tests
Chores