feat(email): multipart HTML+plaintext for RI exchange pending approval (closes #296) by cristim · Pull Request #813 · LeanerCloud/CUDly

cristim · 2026-05-28T18:25:28Z

Summary

Adds riExchangePendingApprovalHTMLTemplate: inline-styled exchange table with per-row Approve/Reject CTA buttons, summary block, optional requested-by line, configurable cancellation-window note, and skipped-exchanges section -- mirrors purchaseApprovalRequestHTMLTemplate (PR feat(email): styled approve/cancel buttons + rec summary block (#287) #298).
Extends the plain-text riExchangePendingApprovalTemplate with labelled Approve: / Reject: URLs (replacing bracket notation), optional requested-by block, and configurable cancellation-window note.
Extends RIExchangeNotificationData with RequestedByName, RequestedByEmail, RequestedAt, CancellationWindowNote fields (backward-compatible zero values).
Adds sendRIExchangePendingApprovalVia private helper (shared by Sender and SMTPSender; HTML degrade on render error is non-fatal, matching sendPurchaseApprovalRequestVia's contract).
SMTPSender.SendRIExchangePendingApproval delegates to the helper (sends to s.notifyEmail).
Sender.SendRIExchangePendingApproval uses the direct SES multipart path when emailAddress + fromEmail are configured; falls back to SNS broadcast for topic-only deployments.

Test plan

go build ./... passes
go test github.com/LeanerCloud/CUDly/internal/email/... -- 322 tests pass (7 new)
TestRenderRIExchangePendingApprovalEmail_Issue296 -- plain-text carries requested-by, labelled URLs, custom note
TestRenderRIExchangePendingApprovalEmail_NoRequestedBy -- omits block + falls back to generic note
TestRenderRIExchangePendingApprovalEmailHTML_Issue296 -- approve/reject hrefs, inline green button style, table cells
TestRenderRIExchangePendingApprovalEmailHTML_DefaultCancellationNote -- generic 6-hour note when field empty
TestRenderRIExchangePendingApprovalEmailHTML_SkippedBlock -- skipped list renders

cristim · 2026-05-28T18:25:35Z

@coderabbitai review

coderabbitai · 2026-05-28T18:25:35Z

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 3 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: dacdbb6e-4b29-44f3-9ac5-096c68c6c473

📥 Commits

Reviewing files that changed from the base of the PR and between 1d9f1b8 and ec8825d.

📒 Files selected for processing (6)

internal/email/coverage_extra_test.go
internal/email/sender.go
internal/email/smtp_sender.go
internal/email/template_renderers.go
internal/email/template_renderers_test.go
internal/email/templates.go

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/296-wave6

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

coderabbitai · 2026-05-28T18:26:06Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim · 2026-05-30T10:22:32Z

@coderabbitai full review

coderabbitai · 2026-05-30T10:22:37Z

✅ Actions performed

Full review triggered.

cristim · 2026-06-04T14:04:31Z

@coderabbitai review

coderabbitai · 2026-06-04T14:09:55Z

Rate Limit Exceeded

@cristim have exceeded the limit for the number of chat messages per hour. Please wait 33 minutes and 40 seconds before sending another message.

cristim · 2026-06-07T23:43:37Z

@coderabbitai full review

Generated by Claude Code

coderabbitai · 2026-06-07T23:43:48Z

@cristim Sure, triggering a full review of PR #813 now!

🧠 Learnings used

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-06T06:43:53.792Z
Learning: In LeanerCloud/CUDly PR `#995` (issue `#950` ownership gate), the Dashboard "Upcoming purchases" widget (`dashboard.ts`) has two Cancel entry points (card-level and modal) that both call `api.deletePlannedPurchase`. The `UpcomingPurchase` response DTO must include `CreatedByUserID *string` (omitempty, mirroring `PlannedPurchase`) so the frontend can gate these buttons via a `canCancelUpcomingPurchase` helper that mirrors `canManageScheduledPurchase` on the Plans page. Without this, the backend correctly 403s non-owners but the UI has no gate and the UX is broken. Regressions: `frontend/src/__tests__/dashboard-ownership-950.test.ts` (6 tests) and `TestHandler_getUpcomingPurchases_PropagatesCreatedByUserID`. Fixed in commit 94326f6b9.

✅ Action performed

Full review finished.

cristim · 2026-06-08T04:33:53Z

Rebased onto feat/multicloud-web-frontend to resolve the merge conflict (was CONFLICTING/DIRTY, now MERGEABLE).

The conflict overlapped the multipart-HTML work in this PR with the base-branch hardening from #1015 / #1036 (stop broadcasting RI-exchange approval tokens via the SNS topic). The resolution preserves both intents:

Kept the PR's multipart/alternative send (plain-text + inline-styled HTML), the enriched RIExchangeNotificationData fields (RequestedByName, RequestedByEmail, RequestedAt, CancellationWindowNote), the HTML renderer, and the sendRIExchangePendingApprovalVia helper.
Kept the base-branch security contract: Sender.SendRIExchangePendingApproval requires RecipientEmail and routes through targeted SES (SendToEmailWithCCMultipart); it no longer falls back to the SNS broadcast topic, which would leak live approval tokens to every subscriber.
Merged both sets of struct fields (RecipientEmail/CCEmails from base + the requested-by/cancellation fields from this PR).
Made sendRIExchangePendingApprovalVia CC-aware and threaded data.CCEmails through both the SES and SMTP callers.
SMTPSender.SendRIExchangePendingApproval now resolves RecipientEmail (falling back to the static notify address) and fails loud with ErrNoRecipient when neither is configured, mirroring SendPurchaseApprovalRequest.

Test update: the base-branch SMTP no-recipient coverage test previously asserted NoError; under the hardened contract it now asserts ErrNoRecipient (renamed to TestSMTPSender_SendRIExchangePendingApproval_NoRecipient, added an ApprovalToken to make the token-leak risk explicit). The existing #1015 regression test (TestSendRIExchangePendingApproval_UsesSESNotSNS) still passes against the merged code.

Verification: go build ./... clean; go vet ./internal/email/... clean; go test ./internal/email/... 339 passed; gofmt clean.

@coderabbitai full review

coderabbitai · 2026-06-08T04:34:13Z

@cristim Thanks for the thorough rebase summary! The conflict resolution looks well-reasoned — preserving the multipart-HTML enrichment while enforcing the security contract from #1015/#1036 (targeted SES only, no SNS token broadcast) is exactly the right outcome. Triggering a full review of the rebased branch now.

🧠 Learnings used

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-05-05T15:44:54.463Z
Learning: In the LeanerCloud/CUDly repository, when a CodeRabbit review flags something as "outside-diff" that was already implemented in an earlier commit of the same PR, it is likely a stale-snapshot artifact where the review diff was generated against a pre-fix view. The inline comment convention `(CR pass on PR #<number> Finding N)` is used by the maintainer (cristim) to mark lines that were changed in direct response to a prior CodeRabbit review pass.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-06T06:43:53.792Z
Learning: In LeanerCloud/CUDly PR `#995` (issue `#950` ownership gate), the Dashboard "Upcoming purchases" widget (`dashboard.ts`) has two Cancel entry points (card-level and modal) that both call `api.deletePlannedPurchase`. The `UpcomingPurchase` response DTO must include `CreatedByUserID *string` (omitempty, mirroring `PlannedPurchase`) so the frontend can gate these buttons via a `canCancelUpcomingPurchase` helper that mirrors `canManageScheduledPurchase` on the Plans page. Without this, the backend correctly 403s non-owners but the UI has no gate and the UX is broken. Regressions: `frontend/src/__tests__/dashboard-ownership-950.test.ts` (6 tests) and `TestHandler_getUpcomingPurchases_PropagatesCreatedByUserID`. Fixed in commit 94326f6b9.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-08T04:21:35.802Z
Learning: In LeanerCloud/CUDly `frontend/src/recommendations.ts` (PR `#838`, commit d7c5764), `perRecPayments` in `FanOutBucket` must be seeded **sparsely** (override-only: only recs whose resolved payment differs from the bucket-level default). The execute path in `frontend/src/app.ts` resolves each rec's payment as `perRecPayments.get(rec.id) ?? b.payment`; if the map is fully populated the fallback is dead and changing the bucket-level dropdown has no effect on unedited rows. The per-rec change handler deletes the map entry when the user reverts to the bucket default (resuming tracking via the fallback); the bucket-level change handler re-syncs visible per-rec selects that have no override, skipping rows whose service does not support the new payment (mixed-SP buckets). Tests (i) and (j) cover the override-only contract and bucket-level propagation respectively; 322 tests pass, `tsc --noEmit` clean.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-06T06:41:06.841Z
Learning: In LeanerCloud/CUDly PR `#804`, `canRevokeCompletedRow` in `frontend/src/history.ts` must check `canAccess` (not just `getCurrentUser()` truthiness) to align with peer predicates `canCancelPendingRow`, `canApprovePendingRow`, and `canRetryFailedRow`. `revoke-own` and `revoke-any` were added to the closed `Action` union to make future UX-vs-RBAC drift a compile error.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T18:28:28.139Z
Learning: In LeanerCloud/CUDly `providers/gcp/recommendations.go`, `collectRegion` fans out to all four GCP service clients (computeengine, cloudsql, memorystore, cloudstorage) concurrently (fixed in PR `#1047` commit 95a390f, H-2 finding). `regionResult` gains `cache` and `storage` slices; merge is updated accordingly. The decision was to wire (not trim) memorystore and cloudstorage because both have complete `GetRecommendations` implementations and their no-op `PurchaseCommitment` paths are orthogonal to surfacing advisory recommendations. Regression tests: `TestRegionResult_HasCacheAndStorageFields` and `TestShouldIncludeService_Cache_Storage`.

Learnt from: cristim
Repo: LeanerCloud/CUDly PR: 0
File: :0-0
Timestamp: 2026-06-07T19:32:26.182Z
Learning: In LeanerCloud/CUDly `providers/aws/services/rds/client.go`, `findOfferingID` validates AZConfig with a switch that has a `default: error` case covering non-empty invalid values (e.g. "typo-az"), not just an empty-string check. A bad non-empty AZConfig previously fell through to `multiAZ==false` in `paginateRDSOfferings` (same mis-buy as the old default). Fixed in PR `#1085` commit 681fd9e81. Regression test added.

✅ Action performed

Full review finished.

cristim · 2026-06-08T05:31:14Z

@coderabbitai full review

coderabbitai · 2026-06-08T05:31:19Z

✅ Action performed

Full review finished.

cristim · 2026-06-19T15:41:44Z

@coderabbitai review

coderabbitai · 2026-06-19T15:41:50Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim · 2026-07-09T21:19:53Z

@coderabbitai review

coderabbitai · 2026-07-09T21:19:59Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim · 2026-07-10T14:44:55Z

@coderabbitai review

coderabbitai · 2026-07-10T14:45:01Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

cristim · 2026-07-10T22:11:53Z

@coderabbitai review

coderabbitai · 2026-07-10T22:11:58Z

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

#296) Mirror the purchaseApprovalRequestHTMLTemplate pattern for the SendRIExchangePendingApproval flow: - Add riExchangePendingApprovalHTMLTemplate: inline-styled table of exchanges with per-row Approve/Reject CTA buttons, summary block, optional requested-by line, configurable cancellation-window note, and skipped-exchanges section. - Extend riExchangePendingApprovalTemplate (plaintext): labelled Approve/Reject URLs (replacing bracket notation), optional requested-by block, configurable cancellation-window note. - Extend RIExchangeNotificationData with RequestedByName, RequestedByEmail, RequestedAt, CancellationWindowNote fields alongside the RecipientEmail/CCEmails targeting fields. - Add RenderRIExchangePendingApprovalEmailHTML renderer. - Add sendRIExchangePendingApprovalVia helper (shared by Sender and SMTPSender, non-fatal HTML degrade on render error), now CC-aware. - Sender.SendRIExchangePendingApproval: requires RecipientEmail and routes through targeted SES multipart (SendToEmailWithCCMultipart), never broadcasting approval tokens via the SNS topic. - SMTPSender.SendRIExchangePendingApproval: resolves RecipientEmail (falling back to the static notify address), fails loud with ErrNoRecipient when neither is configured, then sends multipart. - 7 new test cases covering both halves, empty/custom cancellation notes, missing requested-by, and skipped-exchanges rendering. Rebased onto feat/multicloud-web-frontend: reconciled with the #1015 / multipart send now preserves that fail-loud, targeted-SES contract instead of falling back to the SNS broadcast topic. Updated the SMTP no-recipient coverage test to assert ErrNoRecipient accordingly.

… renderer Add two tests missing from the #296 wave: - TestRenderRIExchangePendingApprovalEmailHTML_EscapesHostilePayload: feeds <script>alert('xss')</script> into RequestedByName, CancellationWindowNote, and Skipped.Reason and asserts the literal tag is absent from the rendered HTML (html/template auto-escaping). - TestRenderRIExchangePendingApprovalEmail_PlainTextVerbatim: feeds O'Brien & Co and a plus-addressed email into the plain-text renderer and asserts no ' / & entity encoding appears (text/template emits values verbatim). Also replace em-dash with double-hyphen in the HTML template comment (style rule).

Use %q for MIME boundary field (correct quoting, satisfies gocritic), replace British spellings (labelled->labeled, honoured->honored) in PR- added comments. All new lint findings introduced by the multipart feature are cleared; pre-existing issues in untouched code paths are unchanged.

cristim added triaged Item has been triaged priority/p3 Polish / idea / may never ship severity/low Minor harm urgency/this-quarter Within the quarter impact/few Limited audience effort/s Hours type/feat New capability labels May 28, 2026

cristim mentioned this pull request Jun 7, 2026

sec(email): scheduled-purchase & RI-exchange approval tokens broadcast via SNS (any subscriber can approve spend) #1015

Closed

cristim force-pushed the fix/296-wave6 branch from ae6ed04 to f6ea79a Compare June 8, 2026 04:30

cristim changed the base branch from feat/multicloud-web-frontend to main June 9, 2026 15:45

cristim force-pushed the fix/296-wave6 branch from f6ea79a to 85cb54c Compare June 19, 2026 15:41

cristim force-pushed the fix/296-wave6 branch from 85cb54c to 79da4ac Compare July 10, 2026 14:44

cristim force-pushed the fix/296-wave6 branch from 79da4ac to f228b29 Compare July 10, 2026 22:11

cristim added 3 commits July 17, 2026 21:20

cristim force-pushed the fix/296-wave6 branch from f228b29 to ec8825d Compare July 17, 2026 18:21

cristim merged commit 7b47b7c into main Jul 19, 2026
19 checks passed

cristim deleted the fix/296-wave6 branch July 27, 2026 11:09

Uh oh!

Conversation

cristim commented May 28, 2026

Summary

Test plan

Uh oh!

cristim commented May 28, 2026

Uh oh!

coderabbitai Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

coderabbitai Bot commented May 28, 2026

Uh oh!

cristim commented May 30, 2026

Uh oh!

coderabbitai Bot commented May 30, 2026

Uh oh!

cristim commented Jun 4, 2026

Uh oh!

coderabbitai Bot commented Jun 4, 2026

Rate Limit Exceeded

Uh oh!

cristim commented Jun 7, 2026

Uh oh!

coderabbitai Bot commented Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jun 8, 2026

Uh oh!

coderabbitai Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jun 8, 2026

Uh oh!

coderabbitai Bot commented Jun 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jun 19, 2026

Uh oh!

coderabbitai Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jul 9, 2026

Uh oh!

coderabbitai Bot commented Jul 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jul 10, 2026

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cristim commented Jul 10, 2026

Uh oh!

coderabbitai Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

coderabbitai Bot commented May 28, 2026 •

edited

Loading

coderabbitai Bot commented Jun 7, 2026 •

edited

Loading

coderabbitai Bot commented Jun 8, 2026 •

edited

Loading

coderabbitai Bot commented Jun 8, 2026 •

edited

Loading

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading

coderabbitai Bot commented Jul 9, 2026 •

edited

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading

coderabbitai Bot commented Jul 10, 2026 •

edited

Loading