Skip to content

fix(email): point RI-exchange approve/reject links at the SPA deep link - #1444

Merged
IAmJSD merged 1 commit into
LeanerCloud:mainfrom
IAmJSD:fix/issue-579-rest
Oct 26, 2026
Merged

IAmJSD merged 1 commit into
LeanerCloud:mainfrom
IAmJSD:fix/issue-579-rest

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Closes #579

Why

The RI-exchange approval emails linked straight at /api/ri-exchange/{approve,reject}/:id?token=. Only POST is routed there, so clicking a link from the inbox (a GET) returned 401. PR #1353 added the SPA deep link /inventory/ri-exchange/{approve,reject}/:id?token= (confirm dialog, then POST, with a Playwright spec). The emails still pointed at the API, so users were still hitting the bug.

The "exchange history" links in the pending and completed emails pointed at /#ri-exchange. The SPA routes by path, not by hash, so they opened Home instead of the RI Exchange sub-tab.

What

  • internal/email/templates.go: the plain-text and HTML pending-approval emails now link Approve/Reject to /inventory/ri-exchange/{approve,reject}/{id}?token=.... The three history links (pending text, pending HTML footer, completed text) now go to /inventory/ri-exchange, the page the deep link itself lands on (RI_EXCHANGE_LANDING_PATH). The POST /api/ri-exchange/resolve/ instruction in the completed email is left alone: it's an API instruction, not a link.
  • internal/email/ri_exchange_links_test.go (new): renders all three emails and checks the exact approve, reject and history URLs, including a token with /+&= to prove it is query-escaped. It also checks that no /api/ri-exchange/{approve,reject}/ or /#ri-exchange link is left.
  • internal/email/template_renderers_test.go: updates the four existing fix(scripts): setup-git-secrets aborts half-configured on a second run #296 URL assertions to the new paths.

No backend route changes. The POST API stays as it is (it is what the SPA calls), and no side-effecting GET is added. internal/server/static.go already serves index.html for extensionless paths, so /inventory/ri-exchange/approve/<id> reaches the SPA.

Verification

  • Fail before: on origin/main (2a11540) with only the new test file, all three TestRIExchange*Link* tests fail. For example, the text email contains Approve: https://dashboard.example.com/api/ri-exchange/approve/rec-579?token=... and https://dashboard.example.com/#ri-exchange.
  • Pass after: go test -race -short -count=1 ./internal/email/ passes, including the 3 new tests and the updated fix(scripts): setup-git-secrets aborts half-configured on a second run #296 tests.
  • go build ./... && go vet ./... && go vet -tags integration ./... && (cd tests/e2e && go vet -tags e2e ./...): pass.
  • golangci-lint v2.10.1 on ./internal/email/..., with and without --build-tags integration: 0 issues. gocyclo -over 10 internal/email/templates.go: nothing reported.
  • Secrets scan: clean (planted key flagged). scripts/test-git-secrets-allowlist.sh: 183 PASS, 0 FAIL.
  • The SPA route side is covered by fix(frontend): add confirm-then-POST deep link for RI-exchange approval #1353's frontend/tests-e2e/riexchange-deeplink.spec.ts and frontend/src/__tests__/riexchange-deeplink.test.ts. This PR doesn't touch the frontend.
  • Not verified: sending a real email and clicking it against a deployed environment.

🤖 Generated with Claude Code

The RI-exchange approval emails linked straight at
/api/ri-exchange/{approve,reject}/:id?token=, which only routes POST, so
clicking a link from the inbox (a GET) returned 401. PR LeanerCloud#1353 added the
confirm-then-POST SPA route at /inventory/ri-exchange/{approve,reject}/:id;
the text and HTML emails now link there.

The "exchange history" links pointed at /#ri-exchange, but the SPA routes
by path, so they opened Home. They now open /inventory/ri-exchange.

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

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

@IAmJSD IAmJSD added priority/p2 Backlog-worthy severity/medium Moderate harm urgency/this-quarter Within the quarter impact/few Limited audience effort/s Hours type/bug Defect triaged Item has been triaged labels Oct 26, 2026
@IAmJSD

IAmJSD commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

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

Blocking:

  • none

Non-blocking:

  • Not a defect in this PR: frontend/src/app.ts runs handleRIExchangeDeeplink() only after the authenticated branch (the unauthenticated path shows the login modal and returns at ~line 113). So an approver who holds only the email token, and has no dashboard session, can't act from the email even though the POST route is AuthPublic and token-gated. The purchase email works the same way, so this is a fix(frontend): add confirm-then-POST deep link for RI-exchange approval #1353 design point and not a regression here. Worth confirming that the deep link survives the login round-trip.

Attacked:

  • Read the 3-dot diff against fix(email): RI-exchange approve/reject email links hit POST-only API routes and return 401 #579's acceptance criteria. The emails now point at the SPA deep link, no side-effecting GET is added, and there is a rendered-email test. The Playwright and deep-link tests for the route already exist from fix(frontend): add confirm-then-POST deep link for RI-exchange approval #1353 (riexchange-deeplink.ts, wired in app.ts:146, parser accepts exactly /inventory/ri-exchange/{approve,reject}/:id). RI_EXCHANGE_LANDING_PATH matches the new history link, and navigation.ts routes /inventory/<subtab>.
  • Grepped every .go non-test file for leftover /api/ri-exchange/{approve,reject} or /#ri-exchange links. Only the router and middleware route entries remain, which is correct. The token is still urlquery-escaped, and the new test uses /+&= to cover that in both the text and the HTML (html/template) halves.
  • Fail-before: I reverted only internal/email/templates.go in the scratch checkout. All 3 new tests and both updated fix(scripts): setup-git-secrets aborts half-configured on a second run #296 tests failed, then passed once I restored the file.
  • Ran go build ./..., go vet (with and without -tags integration) on ./internal/email/, and go test -race -short -count=1 ./internal/email/: all ok. There are no integration tests in the package.

Body-stale:

  • none

Reviewer: rD1. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.

@IAmJSD

IAmJSD commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification at 985ec82d863104b0dfb7bfe25e4eefaea9f49e2d merged onto main (529b6e38), together with #1444, #1446, #1449, #1450, #1451, #1457, #1458 in that order (combined tree 7fb727bb, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

@IAmJSD
IAmJSD marked this pull request as ready for review October 26, 2026 10:51
@IAmJSD
IAmJSD requested a review from cristim as a code owner October 26, 2026 10:51
@IAmJSD
IAmJSD merged commit e97d16c into LeanerCloud:main Oct 26, 2026
41 checks passed
@IAmJSD

IAmJSD commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on 985ec82d, green CI, and the combined-tree verification above. Squash-merged at exactly 985ec82d863104b0dfb7bfe25e4eefaea9f49e2d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/s Hours impact/few Limited audience priority/p2 Backlog-worthy severity/medium Moderate 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(email): RI-exchange approve/reject email links hit POST-only API routes and return 401

1 participant