feat(inventory+history): align Monthly Cost/Savings columns across both tables - #792
Conversation
…th tables Active Commitments now shows Monthly Savings alongside Monthly Cost; Purchase History now shows Monthly Cost alongside Monthly Savings. Users can see both numbers for each commitment without switching tabs. Closes #788.
📝 WalkthroughWalkthroughHistory and Active Commitments tables are updated to show previously unavailable columns: Monthly Cost is added to purchase history, and Monthly Savings is added to active commitments. Implementation changes add cell rendering logic, update table headers and column counts, and include corresponding test coverage for column presence and cell formatting. ChangesTable Column Enhancements for Commitment Display
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai full review |
Rate Limit Exceeded
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/src/history.ts (2)
217-217:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate skeleton column count from 11 to 12.
The history list table now has 12 columns after adding "Monthly Cost" between "Upfront Cost" and "Monthly Savings" (line 661), but the skeleton comment still references 11.
📝 Proposed fix
- // rows matches the typical first-page row count so the skeleton - // doesn't shrink dramatically when real data arrives. Column count - // (11) mirrors the rendered table headers in renderHistoryList: + // rows matches the typical first-page row count so the skeleton + // doesn't shrink dramatically when real data arrives. Column count + // (12) mirrors the rendered table headers in renderHistoryList: // Status / Date / Provider / Service / Type / Region / Count / - // Term / Upfront Cost / Monthly Savings / Plan. + // Term / Upfront Cost / Monthly Cost / Monthly Savings / Plan.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/history.ts` at line 217, The skeleton rows are being rendered with the wrong column count; update the invocation of showSkeletonRows to use 12 columns instead of 11 so the placeholder matches the updated table structure—change the call where listEl is checked (if (listEl) showSkeletonRows(listEl, 8, 11)) to pass 12 as the column count argument (i.e., showSkeletonRows(listEl, 8, 12)) so the skeleton aligns with the new "Monthly Cost" column.
220-220:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate approval queue skeleton column count from 8 to 12.
The approval queue table renders 12 columns (Date / Account / Provider / Service / Count / Term / Payment / Monthly Cost / Upfront Cost / Monthly Savings / Created by / Actions per lines 948–959), but the skeleton comment claims 8 columns.
📝 Proposed fix
- // Pending-approval queue card (issue `#340` sub-task): 3 rows x 8 - // cols matches the queue table shape (Date / Provider / Service / - // Count / Upfront / Monthly Savings / Created by / Actions). The + // Pending-approval queue card (issue `#340` sub-task): 3 rows x 12 + // cols matches the queue table shape (Date / Account / Provider / Service / + // Count / Term / Payment / Monthly Cost / Upfront Cost / Monthly Savings / Created by / Actions). The // queue is typically much shorter than the full history list, so🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/history.ts` at line 220, The skeleton comment that currently says "8 columns" is out of sync with the actual approval queue table which renders 12 columns (Date / Account / Provider / Service / Count / Term / Payment / Monthly Cost / Upfront Cost / Monthly Savings / Created by / Actions); update the comment in frontend/src/history.ts (the approval queue skeleton comment) to state "12 columns" so it matches the rendered columns referenced around lines 948–959.frontend/src/inventory.ts (1)
86-89:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate skeleton column count from 10 to 11.
The skeleton comment still references "10 cols" but
ACTIVE_COMMITMENTS_COLSis now 11 (line 72) after adding the "Monthly savings" column.📝 Proposed fix
- // 5 rows × 10 cols matches the rendered table shape (see + // 5 rows × 11 cols matches the rendered table shape (see // renderActiveCommitmentsTable). The renderer wipes the container's🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/inventory.ts` around lines 86 - 89, Update the inline comment that says "5 rows × 10 cols" to reflect the new column count (change "10 cols" to "11 cols") so it matches the actual value of ACTIVE_COMMITMENTS_COLS; keep the call to showSkeletonRows(container, 5, ACTIVE_COMMITMENTS_COLS) and the surrounding note about renderActiveCommitmentsTable unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/src/history.ts`:
- Line 217: The skeleton rows are being rendered with the wrong column count;
update the invocation of showSkeletonRows to use 12 columns instead of 11 so the
placeholder matches the updated table structure—change the call where listEl is
checked (if (listEl) showSkeletonRows(listEl, 8, 11)) to pass 12 as the column
count argument (i.e., showSkeletonRows(listEl, 8, 12)) so the skeleton aligns
with the new "Monthly Cost" column.
- Line 220: The skeleton comment that currently says "8 columns" is out of sync
with the actual approval queue table which renders 12 columns (Date / Account /
Provider / Service / Count / Term / Payment / Monthly Cost / Upfront Cost /
Monthly Savings / Created by / Actions); update the comment in
frontend/src/history.ts (the approval queue skeleton comment) to state "12
columns" so it matches the rendered columns referenced around lines 948–959.
In `@frontend/src/inventory.ts`:
- Around line 86-89: Update the inline comment that says "5 rows × 10 cols" to
reflect the new column count (change "10 cols" to "11 cols") so it matches the
actual value of ACTIVE_COMMITMENTS_COLS; keep the call to
showSkeletonRows(container, 5, ACTIVE_COMMITMENTS_COLS) and the surrounding note
about renderActiveCommitmentsTable unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4ec06756-3ae2-48a6-9438-80333b1d1796
📒 Files selected for processing (4)
frontend/src/__tests__/history.test.tsfrontend/src/__tests__/inventory.test.tsfrontend/src/history.tsfrontend/src/inventory.ts
… tables (closes #896) PR #792 added the "Monthly Cost" column to two history tables and to the active-commitments table, but the skeleton-loader call sites + their explanatory comments still referenced the pre-#792 column counts. The shimmer placeholders therefore render with one fewer cell than the real table on first paint, and the comments are out of sync with the rendered <th> lists. - history.ts:217 — purchase-history skeleton: showSkeletonRows 3rd arg 11 -> 12; comment lists the 12 rendered columns (adds "Monthly Cost"). - history.ts:224 — pending-approval queue skeleton: showSkeletonRows 3rd arg 8 -> 12; comment lists the 12 rendered columns (Date / Account / Provider / Service / Count / Term / Payment / Monthly Cost / Upfront Cost / Monthly Savings / Created by / Actions). - inventory.ts:86 — active-commitments skeleton comment: "10 cols" -> "11 cols" to match ACTIVE_COMMITMENTS_COLS (the call site already uses the constant, so this is comment-only). Verified against the rendered <th> lists in renderHistoryList (history.ts:650-663), the pending-approval queue (history.ts:946-959), and renderActiveCommitmentsTable's headers array (inventory.ts:156). Test suite (10 suites / 202 tests) and tsc --noEmit both pass.
… tables (closes #896) (#897) PR #792 added the "Monthly Cost" column to two history tables and to the active-commitments table, but the skeleton-loader call sites + their explanatory comments still referenced the pre-#792 column counts. The shimmer placeholders therefore render with one fewer cell than the real table on first paint, and the comments are out of sync with the rendered <th> lists. - history.ts:217 — purchase-history skeleton: showSkeletonRows 3rd arg 11 -> 12; comment lists the 12 rendered columns (adds "Monthly Cost"). - history.ts:224 — pending-approval queue skeleton: showSkeletonRows 3rd arg 8 -> 12; comment lists the 12 rendered columns (Date / Account / Provider / Service / Count / Term / Payment / Monthly Cost / Upfront Cost / Monthly Savings / Created by / Actions). - inventory.ts:86 — active-commitments skeleton comment: "10 cols" -> "11 cols" to match ACTIVE_COMMITMENTS_COLS (the call site already uses the constant, so this is comment-only). Verified against the rendered <th> lists in renderHistoryList (history.ts:650-663), the pending-approval queue (history.ts:946-959), and renderActiveCommitmentsTable's headers array (inventory.ts:156). Test suite (10 suites / 202 tests) and tsc --noEmit both pass.
Summary
QA Inventory 1.1: Active Commitments and Purchase History showed different subsets of per-commitment numeric columns. Users had to switch tabs to see both Monthly Cost and Monthly Savings for the same commitment.
Fix (frontend-only)
Both fields were already present in the API responses (
estimated_savingsonInventoryCommitment,monthly_costonHistoryPurchase). This was a pure rendering gap.Monthly savingsheader +estimated_savingscell.Monthly Costheader + null-guardedmonthly_costcell (mirrors the existing Approval Queue pattern).Files changed
frontend/src/inventory.tsfrontend/src/history.tsfrontend/src/__tests__/inventory.test.tsfrontend/src/__tests__/history.test.tsTest plan
td:nth-child(10)cell value assertion on new column.Closes #788.
Summary by CodeRabbit
New Features
Tests