fix(approvals): populate Account/Payment/MonthlyCost on Approval queue rows (refs #704)

[cristim]

Summary

Fixes #704 follow-up (PR #713 added the columns but data wasn't populated; verification on QA row 274 still failed with all cells showing -).

Root cause: executionToHistoryRow and projectRecommendationFields in internal/api/handler_history.go never wrote Account or Payment onto Approval queue rows for web-initiated bulk purchases. buildPendingExecution sets account IDs at the per-recommendation level only, never on the execution. Similarly, r.Payment was never copied onto row.Payment in either the single-rec or multi-rec branch. Frontend accessors were already wired correctly; they just had nothing to read.

Changes

1. executionToHistoryRow: fall back to collapseRecommendationAccount(exec.Recommendations) when exec.CloudAccountID is nil.
2. projectRecommendationFields single-rec branch: copy r.Payment to row.Payment.
3. projectRecommendationFields multi-rec branch: row.Payment = collapseRecommendationPayment(recs) and row.MonthlyCost = sumRecommendationMonthlyCost(recs).
4. New helpers: collapseRecommendationPayment, collapseRecommendationAccount, sumRecommendationMonthlyCost.

Test plan

• TestHandler_getHistory_ApprovalQueueColumnsPopulated covers 3 shapes (single-rec, multi-rec uniform payment, multi-rec heterogeneous payment)
• TestHandler_getHistory_InProgressRowMapsRecFields extended with Payment assertion
• New frontend test asserts Account/Term/Payment/Monthly Cost cells render real values, not -
• Backend internal/api: 1277 pass
• Frontend: 1989 pass, 1 skipped (63 suites)
• go vet clean
• Manual verification: open Approval queue with web-initiated bulk Azure purchase; columns should now populate

Closes #733 referenced in commit; verifies QA row 274.

Summary by CodeRabbit

• Bug Fixes

  • Improved account filtering accuracy in history requests by using effective account ID resolution, ensuring correct handling of edge cases where account information is incomplete.

• Tests

  • Enhanced test coverage for account filtering scenarios with various account configuration states.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 923f1973-95d8-4222-b09b-4ca4cf067146

📥 Commits

Reviewing files that changed from the base of the PR and between e68a7c9 and 933fdb2.

📒 Files selected for processing (2)

• internal/api/handler_history.go
• internal/api/handler_history_test.go

---

📝 Walkthrough

Walkthrough

This PR refines the account_ids filtering predicate in historyFilters.matchesExecution to resolve the execution's effective account ID by preferring exec.CloudAccountID and falling back to collapsing per-recommendation accounts via collapseRecommendationAccount. Executions whose effective account resolves to empty strings are excluded when filtering is active. Tests are updated to cover the nil execution account with missing recommendation account scenario and the new rec-level fallback behavior for pending executions.

Changes

Account ID Filtering with Effective Account Resolution

Layer / File(s)	Summary
Filter logic and documentation update internal/api/handler_history.go	Updated documentation in matchesExecution describes effective-account resolution: use exec.CloudAccountID when present, fall back to collapseRecommendationAccount, and exclude executions that resolve to "" when account_ids is specified. Corresponding predicate implementation changed to compute accountID via the same fallback chain and filter by membership in AccountIDs.
Test coverage for effective account fallback internal/api/handler_history_test.go	Updated TestHandler_getHistory_FilterParams to clarify that executions with nil exec.CloudAccountID and missing recommendation CloudAccountID are still excluded when filtering. Added new sub-test in TestHandler_getHistory_ApprovalQueueColumnsPopulated that verifies account_ids filtering matches pending executions by falling back to recommendation-level CloudAccountID when exec.CloudAccountID is nil, and asserts the resulting row's AccountID is resolved from the recommendation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• LeanerCloud/CUDly#734: Adds collapseRecommendationAccount helper to synthesize AccountID for pending Approval Queue rows by falling back to per-recommendation accounts; this PR applies the same effective-account resolution logic to the account_ids filtering predicate.

Suggested labels

effort/m, type/bug

Poem

A filter now sees with discerning eyes,
Effective accounts through each compromise—
When clouds don't call, recs whisper their name,
Empty sums fade, but matching ones claim! 🐰✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: populating Account/Payment/MonthlyCost on Approval queue rows, addressing the core issue in this changeset.
Linked Issues check	✅ Passed	The PR addresses both #704 and #733 requirements by populating Account, Payment, and MonthlyCost fields via backend helpers and tests, matching the proposed fixes.
Out of Scope Changes check	✅ Passed	All changes are scoped to fixing backend data population and adding regression tests; no unrelated modifications to other features or systems.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/704-approval-columns-v2

---

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

[cristim]

@coderabbitai review

[coderabbitai]

✅ 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.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/api/handler_history_test.go (1)

1117-1249: ⚡ Quick win

Add a regression case for account_ids filtering with rec-level AccountID fallback

This suite is strong, but it misses the account-filter path for web-bulk rows (exec.CloudAccountID=nil, rec-level CloudAccountID set). A targeted sub-test here would lock the behavior and prevent silent drops when account filters are active.

🤖 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 `@internal/api/handler_history_test.go` around lines 1117 - 1249, Add a new
sub-test inside TestHandler_getHistory_ApprovalQueueColumnsPopulated that
verifies account filtering when exec.CloudAccountID is nil but recommendations
carry CloudAccountID: create a pending execution with exec.CloudAccountID unset
and a RecommendationRecord whose CloudAccountID is set, call handler.getHistory
(using MockConfigStore stubs for GetExecutionsByStatuses, GetAllPurchaseHistory
and GetGlobalConfig and adminHistoryReq for auth/req), pass an account_id filter
in the request map, and assert that the returned row is present and
row.AccountID equals the rec.CloudAccountID; this ensures the account filter
path uses the rec-level fallback in getHistory.

🤖 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.

Inline comments:
In `@internal/api/handler_history.go`:
- Around line 228-237: historyFilters.matchesExecution currently checks
exec.CloudAccountID directly and thus rejects web bulk-purchase executions where
CloudAccountID is nil even though the UI fallback uses
collapseRecommendationAccount(exec.Recommendations) for rendering; update
matchesExecution to use the same fallback: when exec.CloudAccountID is
empty/nil, call collapseRecommendationAccount(exec.Recommendations) and use that
value for account_id comparisons (treat empty string from
collapseRecommendationAccount as a non-match as before). Ensure you reference
and update the logic inside historyFilters.matchesExecution (and any helper it
calls) so filtering behavior matches the display fallback.

---

Nitpick comments:
In `@internal/api/handler_history_test.go`:
- Around line 1117-1249: Add a new sub-test inside
TestHandler_getHistory_ApprovalQueueColumnsPopulated that verifies account
filtering when exec.CloudAccountID is nil but recommendations carry
CloudAccountID: create a pending execution with exec.CloudAccountID unset and a
RecommendationRecord whose CloudAccountID is set, call handler.getHistory (using
MockConfigStore stubs for GetExecutionsByStatuses, GetAllPurchaseHistory and
GetGlobalConfig and adminHistoryReq for auth/req), pass an account_id filter in
the request map, and assert that the returned row is present and row.AccountID
equals the rec.CloudAccountID; this ensures the account filter path uses the
rec-level fallback in getHistory.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 712adc07-03fc-43a6-b6f7-e7fab7907bbb

📥 Commits

Reviewing files that changed from the base of the PR and between 6fcf6a6 and e68a7c9.

📒 Files selected for processing (3)

• frontend/src/__tests__/history-approval-queue.test.ts
• internal/api/handler_history.go
• internal/api/handler_history_test.go

[cristim]

Addressed the nitpick in commit 8704e49 - added a sub-test in TestHandler_getHistory_ApprovalQueueColumnsPopulated that exercises the rec-level CloudAccountID fallback when exec.CloudAccountID is nil.

@coderabbitai review

[coderabbitai]

@cristim Thanks for the follow-up! Adding a dedicated sub-test for the exec.CloudAccountID == nil fallback path is exactly the right coverage — it ensures collapseRecommendationAccount is exercised on the critical code path. Triggering a fresh review now.

✅ 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]

Rebased onto current base branch (resolved conflicts with #729, #737, #740, #734 merged in). @coderabbitai review

[coderabbitai]

@cristim Thanks for the rebase and conflict resolution with #729, #737, #740, and #734! Triggering a fresh review now.

✅ 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.