fix(backend): expose externalAccountId on ExternalAccount resource#8982
fix(backend): expose externalAccountId on ExternalAccount resource#8982VihaanAgarwal wants to merge 1 commit into
Conversation
The backend ExternalAccount mapped the API's `id` field, which holds an `idn_` identification id, and never read `external_account_id` (the `eac_` resource id). Passing `getUser().externalAccounts[].id` to `deleteUserExternalAccount()` then returned a 404 because that method expects the `eac_` id. Map `external_account_id` to a new `externalAccountId` property so the resource id is reachable without a raw API call. `id` keeps its existing value to stay backwards compatible.
🦋 Changeset detectedLatest commit: 8097c8a 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 |
|
@VihaanAgarwal is attempting to deploy a commit to the Clerk Production Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthrough
ChangesExternal account ID mapping
Sequence Diagram(s)Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/backend/src/api/resources/ExternalAccount.ts (1)
10-19: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winNew required constructor arg can break existing consumers
Adding
externalAccountIdas a required positional constructor parameter is not additive for callers that donew ExternalAccount(...); it introduces a source-level breaking change. Consider making it optional with a default, or moving to an options object constructor to preserve compatibility.🤖 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/resources/ExternalAccount.ts` around lines 10 - 19, The ExternalAccount constructor change is breaking existing callers because externalAccountId is now a required positional argument. Update ExternalAccount to preserve backward compatibility by making externalAccountId optional with a safe default, or switch the constructor to accept an options object while keeping existing new ExternalAccount(...) call sites working. Make sure the change is localized in the ExternalAccount class and any related usages that rely on its constructor signature.
🤖 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.
Outside diff comments:
In `@packages/backend/src/api/resources/ExternalAccount.ts`:
- Around line 10-19: The ExternalAccount constructor change is breaking existing
callers because externalAccountId is now a required positional argument. Update
ExternalAccount to preserve backward compatibility by making externalAccountId
optional with a safe default, or switch the constructor to accept an options
object while keeping existing new ExternalAccount(...) call sites working. Make
sure the change is localized in the ExternalAccount class and any related usages
that rely on its constructor signature.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: b7af75fd-ef91-4426-a550-a55759eab66f
📒 Files selected for processing (4)
.changeset/external-account-id.mdpackages/backend/src/api/resources/ExternalAccount.tspackages/backend/src/api/resources/JSON.tspackages/backend/src/api/resources/__tests__/ExternalAccount.test.ts
Description
The backend
ExternalAccountresource maps the API'sidfield, which holds anidn_-prefixed identification id, and never readsexternal_account_id(theeac_-prefixed resource id) from the response. Theeac_id is therefore unreachable from the SDK.This bites anyone deleting an external account.
users.deleteUserExternalAccount()builds its path from theeac_id, so passinggetUser().externalAccounts[0].id(anidn_value) returns404 external_account_not_found. The only workaround today is a rawfetchto readexternal_account_idoff the JSON.The raw user response already contains both ids on each external account:
{ "id": "idn_...", "external_account_id": "eac_...", "identification_id": "idn_...", "provider": "oauth_google" }This PR maps
external_account_idto a newexternalAccountIdproperty onExternalAccount.idkeeps its current value, so the change is additive and backwards compatible. Callers can now do:Fixes #7936
Fixes #7584
How to test
ExternalAccount.fromJSONis covered by a new unit test inpackages/backend/src/api/resources/__tests__/ExternalAccount.test.tsthat assertsexternal_account_idmaps toexternalAccountIdwhileidandidentificationIdstay on theidn_value. The full@clerk/backendsuite passes.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
New Features
Bug Fixes
Tests