fix(frontend/test): add missing escapeHtmlAttr stub to utils mock - #1117
Conversation
plans-range-validation.test.ts used a closed mock factory for ../utils that listed every util function explicitly but omitted escapeHtmlAttr, which was added to utils.ts after the test was written. The missing stub caused a "not a function" TypeError whenever plans.ts called escapeHtmlAttr during renderPlannedPurchaseRow / renderPlanCard, making the entire plans-range-validation suite fail in CI. Added the stub with a faithful HTML-attribute escape implementation (mirrors the real utils.ts implementation) so the closed factory remains exhaustive. Closes #1115
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review Generated by Claude Code |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Get the PR diff
gh pr view 1117 --repo LeanerCloud/CUDly --json title,body,files,commits,additions,deletions,state
# Get the diff
gh pr diff 1117 --repo LeanerCloud/CUDlyLength of output: 3455 🏁 Script executed: #!/bin/bash
# Find and read the real escapeHtmlAttr implementation
rg -n 'escapeHtmlAttr' --type ts -A 10 -B 2
# Also check if there are any other closed mock factories that might be missing escapeHtmlAttr
rg -n 'escapeHtml' --type ts frontend/src/__tests__/Length of output: 34390 Review:
|
| Check | Result |
|---|---|
Real utils.ts escapeHtmlAttr delegates to escapeHtml, which encodes & < > " |
✅ Stub encodes all five: & < > " ' (superset — the ' → ' encoding is correct for attribute context) |
| Null / undefined handling | ✅ str == null guard returns '' — matches the real function's behaviour and is actually more precise than the !str guard used in the other test files (won't falsely return '' for the string '0') |
| Consistent with other closed-mock factories | ✅ Pattern matches history.test.ts, plans.test.ts, allowed-accounts.test.ts, history-approve-button.test.ts, etc. |
Any other closed-mock factories missing escapeHtmlAttr? |
✅ None — dashboard.ts and recommendations.ts do not import escapeHtmlAttr, so their test mocks don't need it |
The fix is minimal, targeted, and consistent with the established pattern across the test suite.
[approve]
🧠 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.
Problem
plans-range-validation.test.tsuses a closed mock factory for../utilsthat lists every util function explicitly. WhenescapeHtmlAttrwas added toutils.tsit was not added to this factory, so any test that exercisesrenderPlannedPurchaseRoworrenderPlanCardthrewTypeError: escapeHtmlAttr is not a function, failing the entire suite in CI.The two other XSS test files (
xss-provider-class.test.ts,xss-purchase-status.test.ts) usejest.requireActualwith...actualspread, so they inherit the real implementation automatically and were not affected.Fix
Add
escapeHtmlAttrto the closed mock factory inplans-range-validation.test.tswith a faithful HTML-attribute escape implementation (same character set the realutils.tsfunction escapes).Verification
All 42 tests in
plans-range-validation.test.tspass locally after this change.Labels (mirrored from #1115)
priority/p2|severity/medium|urgency/this-sprint|impact/internal|effort/s|type/bug|triagedCloses #1115
Generated by Claude Code