Skip to content

fix(frontend): type getUpcomingPurchases as the { purchases } envelope - #1397

Merged
IAmJSD merged 2 commits into
LeanerCloud:mainfrom
IAmJSD:ui/issue-1189
Oct 24, 2026
Merged

IAmJSD merged 2 commits into
LeanerCloud:mainfrom
IAmJSD:ui/issue-1189

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

getUpcomingPurchases() declared Promise<UpcomingPurchase[]>, but GET /api/dashboard/upcoming returns { purchases: [...] }, and both dashboard call sites cast around the mismatch, so tsc never checked how the response was used. There is no user-visible bug today; this PR types the real envelope and removes the casts so a future refactor can't silently break the Home upcoming-purchases widget.

Closes #1189

What changed

  • frontend/src/api/types.ts: new UpcomingPurchasesResponse { purchases: UpcomingPurchase[] | null } (nullable because the Go handler serializes an empty nil slice as null).
  • frontend/src/api/dashboard.ts: getUpcomingPurchases(): Promise<UpcomingPurchasesResponse>; the type is exported from frontend/src/api/index.ts.
  • frontend/src/dashboard.ts: both casts removed (loadDashboard and retryUpcomingPurchases).
  • frontend/src/types.ts: the duplicated local UpcomingPurchase interface (with provider narrowed to Provider) is now export type UpcomingPurchase = api.UpcomingPurchase, since that narrowing was what forced a cast.
  • Tests: api.test.ts "resolves the { purchases } envelope" (with a typed annotation, so typecheck also guards it); a parametrized dashboard.test.ts case for purchases: null and a null response, both showing the empty state and no error.

Commits: cdc5bf35, and the merge with main e7956be9.

Why this approach

It matches the existing PlannedPurchasesResponse pattern. Typing purchases as nullable reflects the real wire format and makes tsc require the existing || [] guard. Aliasing the local type instead of keeping two hand-synced copies removes the drift that caused the cast; the widget only uses provider as a string (badge class whitelist, uppercase label), so widening it loses nothing. Temporarily reverting the return type to an array produced 4 tsc errors (both dashboard call sites, the new api test and the then-unused import), so the type change is load-bearing.

Screenshots

No visible change: this is a types-only change; the dashboard renders the same.

Verification

Run on this branch's head e7956be9 (level with main 51aa5b13), from frontend/:

  • npm run lint: 0 errors. Typecheck and build pass.
  • Jest: 3917 passed, 1 skipped.
  • Playwright (e2e-local.sh): 208 passed.
  • Secrets scan clean (planted key flagged); scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.
  • Merged with main at 51aa5b1; conflicts in frontend/src/__tests__/api.test.ts resolved, see the merge commit.

Notes for review

Aliasing the local types.ts UpcomingPurchase to api.UpcomingPurchase widens provider from Provider to string for the dashboard. Keeping a separate view type plus a mapping function is the alternative if the narrower type is preferred.

Overlaps

#713 changes renderUpcomingPurchases in the same file (frontend/src/dashboard.ts). git merge-tree reports a clean merge and the types stay compatible, but both touch the same widget, so they are best reviewed and landed together.

Note for the UI owner

This changes UI code that @cristim owns. IAmJSD reviewed the before/after screenshots and asked for it to be merged; any design point you disagree with can be a follow-up.

🤖 Generated with Claude Code

getUpcomingPurchases() declared Promise<UpcomingPurchase[]>, but
GET /api/dashboard/upcoming returns { purchases: [...] }, so both
dashboard call sites cast the result to an ad-hoc object type and tsc
never checked them. The function now returns UpcomingPurchasesResponse,
whose purchases field is UpcomingPurchase[] | null because the Go
handler serializes an empty result from a nil slice as null.

The casts are gone. The dashboard's hand-kept UpcomingPurchase copy in
types.ts (provider narrowed to Provider) is now an alias of the API
type, since that narrowing was what forced a cast; the widget already
treats provider as an arbitrary string. Reverting the return type to an
array now fails typecheck at both call sites and in the new api test.

Refs LeanerCloud#1189

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD IAmJSD added impact/internal Team-internal only triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/this-quarter Within the quarter effort/xs Trivial / one-liner type/bug Defect labels Oct 24, 2026
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

  • Run on-demand review

This review includes 7 billable files and costs up to $1.75.

Or wait 59 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
Commits

Reviewing files that changed from the base of the PR and between 51aa5b1 and e7956be.

Files selected for processing: 7
  • frontend/src/__tests__/api.test.ts
  • frontend/src/__tests__/dashboard.test.ts
  • frontend/src/api/dashboard.ts
  • frontend/src/api/index.ts
  • frontend/src/api/types.ts
  • frontend/src/dashboard.ts
  • frontend/src/types.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Conflicts:
- frontend/src/__tests__/api.test.ts: import block; kept main's new
  base64Encode import from ../api/client and added this branch's
  UpcomingPurchasesResponse to the shared type import.

dashboard.ts merged cleanly: main's rework keeps both
api.getUpcomingPurchases() call sites, which still read the typed
{ purchases } envelope without casts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE e7956be9a7177976f94aea57a6982ad00d30c2c9

Blocking:

@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification at e7956be9 merged onto current main (8fda7e51; merge-tree 9ca50b85; Linux, Node, headless Chromium):

  • This branch's own checks passed at this head: lint, typecheck, build, the full jest suite, the full Playwright e2e suite, and the git-secrets checks (see Verification in the PR body).
  • The combined tree of every UI PR in this merge wave (tree dc090cc2, main plus 17 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 4097 passed, 4098 total), and the full Playwright suite (232 passed).
  • CI is green on e7956be9 (CI Success, Run pre-commit hooks).
  • The independent review above approves this exact head.
  • Nothing was applied to any cloud.

@IAmJSD
IAmJSD marked this pull request as ready for review October 24, 2026 19:27
@IAmJSD
IAmJSD merged commit 6122fa2 into LeanerCloud:main Oct 24, 2026
28 of 29 checks passed
@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Marked ready and merged by IAmJSD's agent, at IAmJSD's request (IAmJSD reviewed the before/after screenshots; cristim is traveling), after the independent review and local verification above, all at e7956be9.

IAmJSD added a commit to IAmJSD/cloud-commitments-platform that referenced this pull request Oct 24, 2026
No textual conflicts. Checked main's LeanerCloud#1397 (getUpcomingPurchases now
typed as the { purchases } envelope; dashboard UpcomingPurchase is an
alias of the API type): lib/upcoming-scope.ts and renderUpcomingPurchases
still typecheck against it, and the provider scope applies to
data?.purchases on both the initial load and the Retry path.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
IAmJSD added a commit to IAmJSD/cloud-commitments-platform that referenced this pull request Oct 24, 2026
Conflicts:
- frontend/src/app.ts: import block only. This branch imports
  setOpenAccountsHandler (first-run card's Admin > Accounts navigation);
  main's LeanerCloud#1336 imports initPurchasingNav. Kept both imports; both call
  sites merged cleanly and are unchanged.

Also checked: main's LeanerCloud#1397 envelope typing for getUpcomingPurchases sits
next to this branch's 4th allSettled entry (listAccountsMinimal) in
loadDashboard and merged cleanly. LeanerCloud#1368's "Select none" rename is the
per-column popover button, separate from this branch's "Clear filters"
(clearAllColumnFilters) action.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/xs Trivial / one-liner impact/internal Team-internal only priority/p3 Polish / idea / may never ship severity/low Minor harm triaged Item has been triaged type/bug Defect urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(frontend): getUpcomingPurchases declares an array but the API returns { purchases }

1 participant