OUT-3275: account dropdowns in QB settings#253
Conversation
…pers Adds the server side of the settings dashboard account-ref editor: a single /api/quickbooks/accounts route handling GET (lists income/expense/bank accounts from QBO plus the portal's currently-selected refs) and PATCH (validates each provided ref against QBO and updates qb_portal_connections scoped by portalId). - IntuitAPI.getAccountsForProductMapping runs three flat queries (no OR, parens, or IN — QBO's parser rejects all three on AccountType). - _getAnAccount SELECT now includes AccountType so PATCH validation can read it; QBAccountRowSchema tightened to require AccountType. - TokenService.updateAccountRefs validates AccountType per bucket in parallel before delegating to updateQBPortalConnection. - Response uses an allowlist (QBPortalConnectionSelectSchema.pick) so token fields can't leak. - patchFetcher helper mirrors postFetcher (throws on non-OK). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a third accordion section below Invoice Details that lets the user pick which QBO accounts the integration uses for income, expense, and bank. Skips the /accounts fetch entirely when QB isn't connected (syncFlag off or no portal connection) and surfaces a "Connect to QuickBooks" message inline instead. - useOtherSettings hook backs the section via SWR; submits only changed fields through patchFetcher and mutates the cache on success. - Custom dropdown component mirrors the QuickBooks-items dropdown pattern (button + click-outside-to-close panel) instead of a native <select>; no search box or sticky options since the lists are short. - SettingAccordion wires the new section with the existing Cancel/Update button pattern, gated on syncFlag and showButton. - useSettings opens the new "other-settings" item by default for un-enabled portals. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- IntuitAPI.getAccountsForProductMapping: asserts the three queries are built without OR / parens / IN (QBO parser limits) and that the asset bucket queries the Bank type. - AccountService.listAccountsForProductMapping: happy path + 404 when portal connection is missing; verifies SyncToken/Active are stripped. - TokenService.updateAccountRefs: per-bucket AccountType validation, rejects mismatches and missing accounts, writes scoped by portalId. - GET /api/quickbooks/accounts: 200 happy path with TEST_*_ACCOUNT_REF constants; 401 without token. - PATCH /api/quickbooks/accounts: single-field update, wrong-AccountType rejection, empty-body 422, all-three-valid, tenant isolation. - Extends QBAccountRowSchema fixtures in intuitAPI.responses.test.ts with AccountType after the schema tightening. - Pins integration mock factories on globalThis (per the documented setupFile re-entry workaround) and adds getAccountsForProductMapping to the IntuitAPI mock default so existing tests don't break. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- TokenService.updateAccountRefs: guard the post-update row destructure against undefined so a zero-row WHERE produces an explicit 500 instead of a misleading 422 from SafePortalConnectionSchema.parse(undefined). - Replace inline 'Income'/'Expense'/'Bank' magic strings with a local QBO_ACCOUNT_TYPE map. The existing AccountTypeObj enum holds lowercase routing keys, not QBO API values, so reusing it would mislead — keep the maps distinct, documented inline. - IntuitAPI.getAccountsForProductMapping: reuse QB_ACCOUNT_COLUMNS in the SELECT clauses so the schema/SQL stay in sync; drop the "no response from QBO" throw (customQuery already throws on no-response; the remaining undefined-QueryResponse case is now treated as an empty bucket so the UI shows "No matching accounts" instead of "Could not load"); rename assetOtherRaw → assetRaw. - OtherSettings dropdown: add aria-haspopup="listbox", aria-expanded, aria-labelledby on the trigger and role="listbox"/role="option"/ aria-selected on the panel so screen readers can navigate it. - Add regression test for the undefined-bucket → empty path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds an "Other Settings" accordion to the QuickBooks integration dashboard, letting portal admins choose QBO accounts for income, expense, and bank buckets via three custom dropdowns backed by a new
Confidence Score: 4/5Safe to merge — the new endpoint is correctly scoped per tenant, token fields are allowlisted out of responses, and account validation against QBO runs before any DB write. The core GET/PATCH flows, validation, and tenant isolation are solid. Two style-level concerns hold the score back slightly: AccountOption is defined identically in both common.ts and useSettings.ts (drift risk), and the accordion save/cancel button visibility is gated on index === 2 which will silently misfire if sections are ever reordered. src/components/dashboard/settings/SettingAccordion.tsx for the positional index coupling; src/hook/useSettings.ts and src/type/common.ts for the duplicated AccountOption type. Important Files Changed
Sequence DiagramsequenceDiagram
participant UI as OtherSettings (React)
participant Hook as useOtherSettings
participant GETRoute as GET /api/quickbooks/accounts
participant PATCHRoute as PATCH /api/quickbooks/accounts
participant AccountSvc as AccountService
participant TokenSvc as TokenService
participant QBO as QuickBooks Online
participant DB as qb_portal_connections
UI->>Hook: mount
Hook->>GETRoute: SWR fetch
GETRoute->>AccountSvc: listAccountsForProductMapping()
AccountSvc->>DB: getPortalTokens(portalId)
DB-->>AccountSvc: tokens + selected refs
AccountSvc->>QBO: 3x customQuery (Income/Expense/Bank)
QBO-->>AccountSvc: account lists
AccountSvc-->>GETRoute: options + selected
GETRoute-->>Hook: 200
Hook-->>UI: render dropdowns
UI->>Hook: changeSettings()
Hook-->>UI: "showButton=true"
UI->>Hook: submitOtherSettings()
Hook->>PATCHRoute: PATCH changed fields
PATCHRoute->>TokenSvc: updateAccountRefs(payload)
TokenSvc->>DB: getPortalTokens(portalId)
loop for each changed ref
TokenSvc->>QBO: getAnAccount(id)
QBO-->>TokenSvc: account row
TokenSvc->>TokenSvc: validate AccountType
end
TokenSvc->>DB: updateQBPortalConnection
DB-->>TokenSvc: updated row
TokenSvc-->>PATCHRoute: conn
PATCHRoute-->>Hook: 200 portalConnection
Hook->>GETRoute: mutate
|
AccountOption was declared identically in src/type/common.ts and src/hook/useSettings.ts. Drift risk: any addition to one shape would silently diverge from the other across the client/server boundary. Drop the local declaration in useSettings.ts; import the canonical AccountOption from @/type/common. OtherSettings.tsx imports the type directly from @/type/common too so each consumer reads the same source. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ocess Per review feedback: variable names should answer "what is this value?", not "what function ran to produce it?". Renamed in the controller + service: - `parsed` → `accountRefs` - `data` → `accountsResponse` - `payload` (local, not parameter) → `accountRefs` - `conn` → `updatedConnection` - `safe` → `safePortalConnection` No behavior change. Tests + tsc + lint clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The PATCH endpoint is dashboard-only. Every account ref the dashboard sends came from GET /api/quickbooks/accounts, which itself reads from QBO. Re-fetching each ref to validate AccountType is redundant work (up to 3 QBO API calls per save) defending against scenarios that require either a dashboard bug or direct API misuse — neither of which this layer is the right place to defend against. - TokenService.updateAccountRefs: drop the per-ref getAnAccount + AccountType check; just write the refs scoped by portal_id and return the updated row. - Lift the 404 into getPortalTokens itself (throws APIError(404) on missing connection) so callers don't need to try/catch and rewrap. - accounts.service.ts: drop the now-redundant try/catch around getPortalTokens; the APIError it throws already has the right shape. - accounts.controller.ts: inline SafePortalConnectionSchema.parse into the response. - Tests: remove the now-stale "wrong AccountType" assertions (4 unit cases + 1 integration case); simplify the survivors to drop unused getAnAccount mocks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…useSwrHelper
Four of the five useSwrHelper callers in the settings dashboard pass the
same { suspense: true, revalidateOnMount: false } overrides. Move them
into the helper's defaults; callers can still override per-call.
useOtherSettings keeps an explicit { suspense: false, revalidateOnMount:
true } override because its loading/error state must stay inline to the
accordion section. Without it, SWR's suspense bubble would hit the
page-level loading.tsx and flash a full-screen spinner over the rest of
the dashboard on first open.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
054f50b to
8d16f34
Compare
Callers can now pass useSwrHelper<MyType>(url) and `data` is typed as MyType | undefined instead of any. Default T = any keeps existing call sites working without changes. useOtherSettings adopts the generic and drops its `as ...` cast on data?.options. The other consumers in this file (product mapping, invoice detail) stay on the implicit any default and can be migrated incrementally as they're touched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Renames the third settings accordion ("Other Settings" → "Account Mapping")
end-to-end: section header, accordion id, file/folder path, component, hook,
state/props types, handlers, and destructure aliases. Also updates the
missing-option placeholder copy to "Please select <label>" instead of
surfacing the opaque QBO ref id.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Adds an Other Settings accordion section to the QuickBooks integration's settings dashboard that lets portal admins pick which QBO accounts the integration uses:
Linear: https://linear.app/assemblycom/issue/OUT-3275
Architecture
src/app/api/quickbooks/accounts/route.tsexports bothGET(returns{ options, selected }per bucket) andPATCH(validates each ref'sAccountTypeagainst QBO, then updatesqb_portal_connectionsscoped byportalId).QBPortalConnectionSelectSchema.pick({...})as an allowlist; tests assertaccessToken/refreshToken/tokenTypeare absent from the response.eq(QBPortalConnection.portalId, this.user.workspaceId); tenant-isolation integration test confirms portal A's token cannot mutate portal B's row._getAccountsForProductMappinguses three flat queries (one perAccountType);OR, parentheses, andIN-on-AccountTypeall rejected by QBO's parser. Verified empirically in sandbox.useOtherSettingshook gates the GET onsyncFlag && portalConnectionStatus; when disconnected, shows an inline "Connect to QuickBooks" message and never calls the API. Custom button-based dropdown mirrors the existing QuickBooks-items dropdown styling (no search, no sticky options).Commits
Test plan
yarn test— 211/211 pass (34 files, unit + integration via testcontainers)npx tsc --noEmit— zero errorsyarn lint:check+yarn prettier:check— clean (10 pre-existing warnings, none new)_getAnAccountSELECT extension (AccountTypeadded) doesn't surprise any existing callers (verified: all 5 existing callers only readId/Name/SyncToken/Active).AccountTypenarrowing (income subtype =SalesOfProductIncome, asset =Bank) matches product intent; "user will refine subtypes later" is captured.Testing Criteria
https://www.loom.com/share/95f84287758c4917af56ca099e948e9f
Known follow-ups (separate tickets)
checkAndUpdateAccountStatusself-heal audit — confirm it doesn't silently overwrite a user-chosen ref. Pre-existing concern flagged in the spec, not part of this PR.aria-haspopup/aria-expanded/role="listbox"/role="option"; full keyboard navigation (arrow keys, Escape) deferred.🤖 Generated with Claude Code