From f123ba1ad1edd243063554ecda37e3b9684f9733 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 25 May 2026 20:09:58 +0200 Subject: [PATCH] fix(api/history): honor provider/account_ids/start/end query params in Purchase History handler The /api/history handler accepted provider, account_ids, start, and end query params from the frontend (Purchase History date filter row 3.4 and the Purchases-page Global filter rows 4.2/4.4/4.5) but silently dropped them: fetchPurchaseHistory only forwarded account_id and limit to the store, and fetchExecutionsAsHistory ignored every filter. Visible filter affordances were no-ops. Parse the four query params in a shared historyFilters struct so both halves of the merged /api/history response (the SQL purchase_history rows and the synthesised execution rows) apply the same scope: - provider validated via the existing whitelist (aws/azure/gcp/"all"/"") - account_ids parsed via parseAccountIDs (UUIDs, MaxAccountIDsPerRequest) - start/end as YYYY-MM-DD with a 366-day cap mirroring PR #529 (issue #414) to prevent a full-table-scan DoS New store method GetPurchaseHistoryFiltered pushes the filter set into a WHERE clause built like buildRecommendationFilter; each predicate is applied only when its filter is non-empty. The legacy GetPurchaseHistory / GetAllPurchaseHistory methods are preserved for the no-filter fast path (dashboard, inventory, analytics) and for the legacy singular account_id param, which targets a different (VARCHAR(20)) column than the cloud_account_id UUIDs the new filter consumes and must not be coerced into the new filter's clause. Tests cover each filter independently and combined, the legacy account_id fast path, malformed/inverted/oversized dates returning 400, the 366-day boundary (366 accepted, 367 rejected), and the pgxmock SQL shape for the new store method including the no-filter / partial-filter / limit-clamp variants. Closes #701 --- internal/analytics/collector_test.go | 4 + internal/api/handler_history.go | 279 +++++++++++- internal/api/handler_history_test.go | 418 ++++++++++++++++++ internal/api/mocks_test.go | 8 + internal/config/interfaces.go | 18 + internal/config/store_postgres.go | 62 +++ .../config/store_postgres_pgxmock_test.go | 120 +++++ internal/mocks/stores.go | 9 + internal/purchase/mocks_test.go | 8 + internal/scheduler/scheduler_test.go | 8 + internal/server/test_helpers_test.go | 4 + 11 files changed, 918 insertions(+), 20 deletions(-) diff --git a/internal/analytics/collector_test.go b/internal/analytics/collector_test.go index ce12bd0a1..f551470f0 100644 --- a/internal/analytics/collector_test.go +++ b/internal/analytics/collector_test.go @@ -196,6 +196,10 @@ func (m *mockConfigStore) GetAllPurchaseHistory(ctx context.Context, limit int) return nil, nil } +func (m *mockConfigStore) GetPurchaseHistoryFiltered(ctx context.Context, providerFilter string, accountIDs []string, start, end *time.Time, limit int) ([]config.PurchaseHistoryRecord, error) { + return nil, nil +} + func (m *mockConfigStore) CleanupOldExecutions(ctx context.Context, retentionDays int) (int64, error) { return 0, nil } diff --git a/internal/api/handler_history.go b/internal/api/handler_history.go index 0d7e74d13..33a460a4a 100644 --- a/internal/api/handler_history.go +++ b/internal/api/handler_history.go @@ -22,11 +22,12 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR return nil, err } - if err := logMultiAccountFilter(params); err != nil { + filters, err := parseHistoryFilters(params) + if err != nil { return nil, err } - completed, err := h.fetchPurchaseHistory(ctx, params) + completed, err := h.fetchPurchaseHistory(ctx, filters) if err != nil { return nil, err } @@ -40,8 +41,11 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR // 7-day cutoff (expired). Executions live in a separate table // (purchase_executions) from the completed purchase_history rows, so we // merge after the fact. A failure to list executions must not hide - // completed history — log, skip, continue. - extra := h.fetchExecutionsAsHistory(ctx) + // completed history — log, skip, continue. The same filter set is applied + // here (in-memory against the synthesised row's recommendations and + // scheduled_date) so the two halves of the merged response are + // consistently scoped (issue #701). + extra := h.fetchExecutionsAsHistory(ctx, filters) all := append(completed, extra...) //nolint:gocritic // intentional new slice @@ -108,7 +112,13 @@ const approvalExpiryWindow = 7 * 24 * time.Hour // lazily transitioned to "expired" in the store so a stale approval link // can't be clicked and the History badge reflects reality. A listing error // is logged and skipped — completed history must still render. -func (h *Handler) fetchExecutionsAsHistory(ctx context.Context) []config.PurchaseHistoryRecord { +// +// The filter set (issue #701) is applied in Go against the synthetic row: +// provider via the recs' collapsed provider, account via CloudAccountID, +// date via ScheduledDate. Filtering after expireIfStale so a row that +// transitioned to expired during this request still gets evaluated against +// the same predicates as a DB row. +func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyFilters) []config.PurchaseHistoryRecord { executions, err := h.config.GetExecutionsByStatuses(ctx, historyExecutionStatuses, config.DefaultListLimit) if err != nil { logging.Warnf("history: failed to load non-completed executions: %v", err) @@ -129,6 +139,9 @@ func (h *Handler) fetchExecutionsAsHistory(ctx context.Context) []config.Purchas continue } exec = h.expireIfStale(ctx, exec) + if !filters.matchesExecution(exec) { + continue + } out = append(out, executionToHistoryRow(exec, approver)) } return out @@ -387,35 +400,261 @@ func collapseRecommendationProvider(recs []config.RecommendationRecord) string { return p } -// logMultiAccountFilter validates and logs the multi-account filter param. -// GetPurchaseHistory/GetAllPurchaseHistory do not yet support multi-account -// filtering at the store layer; the param is accepted for observability. -func logMultiAccountFilter(params map[string]string) error { +// MaxHistoryDateRangeDays caps the inclusive start/end window the History +// handler accepts on a single request. Mirrors the analytics cap (issue +// #414 / PR #529): an unbounded range turns the WHERE-on-timestamp into a +// full-table scan over purchase_history, which the dashboard frontend +// neither needs nor renders coherently past a year. 366 admits a full leap +// year and rejects anything larger with 400. +const MaxHistoryDateRangeDays = 366 + +// historyFilters carries the shared filter set used by both halves of the +// merged /api/history response: the SQL path (purchase_history rows in +// fetchPurchaseHistory) and the in-memory path (synthesised execution rows +// in fetchExecutionsAsHistory). Keeping them in one struct guarantees the +// two halves stay scoped consistently — the bug behind issue #701 was that +// the executions path ignored the filters the SQL path was supposed to apply. +// +// Two account inputs are accepted but they are NOT interchangeable: +// - LegacyAccountID is the singular `account_id` query param. It is the +// cloud-provider account number (VARCHAR(20) — e.g. "123456789012"), +// matched against purchase_history.account_id by GetPurchaseHistory. +// Only the no-other-filters fast path consumes it; the filtered SQL +// path ignores it because cloud_account_id is a UUID column. +// - AccountIDs is the plural `account_ids` query param, a list of UUIDs +// matched against purchase_history.cloud_account_id (the +// cloud_accounts FK). This is what the frontend sends. +// +// HasDate is true iff start OR end was supplied. When false the Start/End +// times are zero-valued and the SQL/in-memory date predicates are skipped +// entirely (so legacy clients that don't send dates keep working). +type historyFilters struct { + Provider string + LegacyAccountID string + AccountIDs []string + HasDate bool + Start time.Time + End time.Time + Limit int +} + +// parseHistoryFilters validates and normalises the /api/history query string. +// Returns a 400 ClientError for malformed provider, non-UUID account_ids, a +// start/end that doesn't parse as YYYY-MM-DD, an inverted range +// (start > end), or a range exceeding MaxHistoryDateRangeDays. +func parseHistoryFilters(params map[string]string) (historyFilters, error) { + var f historyFilters + + provider := params["provider"] + if provider == "all" { + provider = "" // "all" is the explicit "no filter" sentinel + } + if err := validateProvider(provider); err != nil { + return f, err + } + f.Provider = provider + accountIDs, err := parseAccountIDs(params["account_ids"]) if err != nil { - return NewClientError(400, err.Error()) + return f, NewClientError(400, err.Error()) } - if len(accountIDs) > 0 { - logging.Infof("history: account_ids filter received (%d accounts); per-account filtering not yet implemented", len(accountIDs)) + f.AccountIDs = accountIDs + f.LegacyAccountID = params["account_id"] + + start, end, hasDate, err := parseHistoryDateRange(params["start"], params["end"]) + if err != nil { + return f, err } - return nil -} + f.HasDate = hasDate + f.Start = start + f.End = end -// fetchPurchaseHistory reads purchases from the store, honouring the legacy -// singular account_id query param and the limit cap. -func (h *Handler) fetchPurchaseHistory(ctx context.Context, params map[string]string) ([]config.PurchaseHistoryRecord, error) { limit := config.DefaultListLimit if s := params["limit"]; s != "" { fmt.Sscanf(s, "%d", &limit) } + if limit <= 0 { + limit = config.DefaultListLimit + } if limit > config.MaxListLimit { limit = config.MaxListLimit } + f.Limit = limit + + return f, nil +} + +// parseHistoryDateRange parses optional YYYY-MM-DD start/end strings. Both +// empty -> no date filter (hasDate=false, returned times are zero). One or +// both populated -> hasDate=true; an unset bound defaults to span the +// maximum allowed window from the supplied side. The window is capped at +// MaxHistoryDateRangeDays to mirror PR #529 (issue #414) and prevent a full- +// table-scan DoS via start=1970-01-01&end=2100-12-31. +// +// YYYY-MM-DD only (per issue #701 spec): the frontend's +// fields emit exactly that format, so accepting RFC 3339 here would be a +// surface area not exercised in production and a divergence from the +// analytics handler's broader format set. +func parseHistoryDateRange(startStr, endStr string) (time.Time, time.Time, bool, error) { + if startStr == "" && endStr == "" { + return time.Time{}, time.Time{}, false, nil + } + start, end, err := parseHistoryDateBounds(startStr, endStr) + if err != nil { + return time.Time{}, time.Time{}, false, err + } + const maxWindow = MaxHistoryDateRangeDays * 24 * time.Hour + // Fill in the open side, if any, so the absent bound spans the maximum + // allowed window from the supplied side. This keeps a "start only" or + // "end only" call meaningful AND still bounded by the same DoS cap. + switch { + case startStr == "" && endStr != "": + start = end.Add(-maxWindow) + case startStr != "" && endStr == "": + end = start.Add(maxWindow) + } + if start.After(end) { + return time.Time{}, time.Time{}, false, NewClientError(400, + "start date must be before or equal to end date") + } + if end.Sub(start) > maxWindow { + return time.Time{}, time.Time{}, false, NewClientError(400, + fmt.Sprintf("date range too large: maximum allowed range is %d days", MaxHistoryDateRangeDays)) + } + return start, end, true, nil +} + +// parseHistoryDateBounds parses each YYYY-MM-DD side independently. An empty +// string returns the zero value for that side; the caller is responsible for +// substituting a sensible default. The end side is rolled forward to end-of- +// day so a date input is inclusive of the chosen day. +func parseHistoryDateBounds(startStr, endStr string) (time.Time, time.Time, error) { + const layout = "2006-01-02" + var start, end time.Time + if startStr != "" { + s, err := time.ParseInLocation(layout, startStr, time.UTC) + if err != nil { + return time.Time{}, time.Time{}, NewClientError(400, + "invalid start date format: expected YYYY-MM-DD") + } + start = s + } + if endStr != "" { + e, err := time.ParseInLocation(layout, endStr, time.UTC) + if err != nil { + return time.Time{}, time.Time{}, NewClientError(400, + "invalid end date format: expected YYYY-MM-DD") + } + // Make end inclusive of the requested day (frontend semantics: a + // user picking 2024-01-31 expects rows from that day to render). + end = e.Add(24*time.Hour - time.Second) + } + return start, end, nil +} - if accountID := params["account_id"]; accountID != "" { - return h.config.GetPurchaseHistory(ctx, accountID, limit) +// matchesExecution reports whether a PurchaseExecution should be retained +// after applying the request filters in Go (the SQL path is on +// purchase_history; this in-memory equivalent keeps the two halves of the +// merged response consistently scoped, issue #701). +// +// - Provider: matches when ANY recommendation in the execution carries +// the filter value. A single-rec execution collapses to one provider; a +// multi-rec basket that spans providers (e.g. aws+azure) matches any of +// them — dropping such an execution because its collapsed display label +// is "multiple" would hide real activity the user owns. +// - Account: matches when the execution's CloudAccountID is one of the +// filtered IDs. Executions with a NULL CloudAccountID are excluded once +// account_ids is non-empty, mirroring the SQL semantics on +// purchase_history.cloud_account_id (issue #211). +// - Date: matches when ScheduledDate is within [Start, End]. Inclusive +// both sides; End is the end-of-day for YYYY-MM-DD inputs. +func (f historyFilters) matchesExecution(exec config.PurchaseExecution) bool { + if f.Provider != "" { + if !executionHasProvider(exec, f.Provider) { + return false + } } - return h.config.GetAllPurchaseHistory(ctx, limit) + if len(f.AccountIDs) > 0 { + // AccountIDs are UUIDs against cloud_account_id; legacy + // LegacyAccountID is intentionally NOT folded here because it is a + // different (VARCHAR(20)) cloud-provider account number that the + // fast path applies on the SQL side. + if exec.CloudAccountID == nil { + return false + } + if !stringInSlice(*exec.CloudAccountID, f.AccountIDs) { + return false + } + } + if f.HasDate { + if exec.ScheduledDate.Before(f.Start) || exec.ScheduledDate.After(f.End) { + return false + } + } + return true +} + +// executionHasProvider reports whether any of the execution's +// recommendations carries the given provider value. +func executionHasProvider(exec config.PurchaseExecution, provider string) bool { + for _, r := range exec.Recommendations { + if r.Provider == provider { + return true + } + } + return false +} + +// stringInSlice is a tiny linear-search helper for the per-execution +// account filter. The slice is bounded by MaxAccountIDsPerRequest (200) so +// a linear scan is comfortably cheaper than the map allocation a set would +// require, and the call is hot only for executions (DB rows go through SQL). +func stringInSlice(needle string, haystack []string) bool { + for _, h := range haystack { + if h == needle { + return true + } + } + return false +} + +// fetchPurchaseHistory reads purchases from the store, applying the parsed +// filter set. The store-level filtered method (added with issue #701) +// pushes provider / cloud_account_id / timestamp range into SQL; the +// fast-path legacy methods (no-filter, single-account-only) are kept for +// the dashboard/inventory/analytics callers that don't speak the new shape. +// +// LegacyAccountID is honoured only on the fast path (it filters the +// VARCHAR(20) purchase_history.account_id column). When combined with any +// new filter it is ignored: the new filtered method's account predicate is +// on cloud_account_id (UUID), and silently coercing one column's identifier +// into the other would either match nothing or, worse, match the wrong +// rows. Callers using the new filter set should send `account_ids` +// (UUIDs); the legacy singular param is preserved for the dashboard's +// historical, single-cloud-account view. +func (h *Handler) fetchPurchaseHistory(ctx context.Context, filters historyFilters) ([]config.PurchaseHistoryRecord, error) { + noNewFilters := !filters.HasDate && filters.Provider == "" && len(filters.AccountIDs) == 0 + if noNewFilters { + if filters.LegacyAccountID != "" { + return h.config.GetPurchaseHistory(ctx, filters.LegacyAccountID, filters.Limit) + } + return h.config.GetAllPurchaseHistory(ctx, filters.Limit) + } + + var startPtr, endPtr *time.Time + if filters.HasDate { + s, e := filters.Start, filters.End + startPtr = &s + endPtr = &e + } + return h.config.GetPurchaseHistoryFiltered( + ctx, + filters.Provider, + filters.AccountIDs, + startPtr, + endPtr, + filters.Limit, + ) } // filterPurchaseHistoryByAllowedAccounts drops records whose AccountID/Name diff --git a/internal/api/handler_history_test.go b/internal/api/handler_history_test.go index 4e1c14281..0a70556fd 100644 --- a/internal/api/handler_history_test.go +++ b/internal/api/handler_history_test.go @@ -622,6 +622,424 @@ func TestHandler_getHistory_CompletedDBRowWithDescriptionStillCounts(t *testing. assert.Equal(t, 70.0, resp.Summary.TotalMonthlySavings) } +// TestHandler_getHistory_FilterParams is the issue #701 primary regression +// guard. /api/history must honour the provider / account_ids / start / end +// query params the frontend sends — both on the SQL path (purchase_history +// rows in fetchPurchaseHistory) and on the in-memory path (synthesised +// execution rows in fetchExecutionsAsHistory). The filters were previously +// dropped silently; visible filter affordances were no-ops. +// +// Each sub-test seeds enough rows on both halves that an unfiltered +// response would include extras, then asserts the filter prunes both halves +// consistently and that the SQL path receives the right store call. +// +// Each filter is exercised independently first, then combined. +func TestHandler_getHistory_FilterParams(t *testing.T) { + approver := "ops@example.com" + + // Helper to build a fresh handler/store/req triple per sub-test so mock + // expectations don't bleed across cases. + newHandler := func(t *testing.T) (*MockConfigStore, *Handler, *events.LambdaFunctionURLRequest, context.Context) { + t.Helper() + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth, req := adminHistoryReq(ctx) + mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{NotificationEmail: &approver}, nil).Maybe() + return mockStore, &Handler{auth: mockAuth, config: mockStore}, req, ctx + } + + t.Run("provider filter is pushed to SQL and prunes executions", func(t *testing.T) { + mockStore, handler, req, ctx := newHandler(t) + + // SQL path: must be called via GetPurchaseHistoryFiltered with + // provider="aws" and no other filters set. Return only the aws row. + filtered := []config.PurchaseHistoryRecord{ + {AccountID: "acc-aws", PurchaseID: "p-aws", Provider: "aws", UpfrontCost: 100.0}, + } + mockStore.On("GetPurchaseHistoryFiltered", ctx, "aws", []string(nil), (*time.Time)(nil), (*time.Time)(nil), config.DefaultListLimit). + Return(filtered, nil).Once() + + // Executions path: two execs, one aws-rec and one azure-rec. Only the + // aws-rec exec must survive the in-memory provider filter. + execs := []config.PurchaseExecution{ + { + ExecutionID: "exec-aws", + Status: "pending", + ScheduledDate: time.Now(), + Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1"}}, + }, + { + ExecutionID: "exec-azure", + Status: "pending", + ScheduledDate: time.Now(), + Recommendations: []config.RecommendationRecord{{Provider: "azure", Service: "vm", Region: "westeurope"}}, + }, + } + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(execs, nil) + + result, err := handler.getHistory(ctx, req, map[string]string{"provider": "aws"}) + require.NoError(t, err) + resp := result.(HistoryResponse) + + // 1 DB row + 1 surviving exec. + require.Len(t, resp.Purchases, 2) + for _, p := range resp.Purchases { + assert.NotEqual(t, "exec-azure", p.PurchaseID, "azure-only execution must be filtered out when provider=aws") + } + mockStore.AssertCalled(t, "GetPurchaseHistoryFiltered", ctx, "aws", []string(nil), (*time.Time)(nil), (*time.Time)(nil), config.DefaultListLimit) + }) + + t.Run("provider=all is treated as no filter (legacy SQL path)", func(t *testing.T) { + mockStore, handler, req, ctx := newHandler(t) + mockStore.On("GetAllPurchaseHistory", ctx, config.DefaultListLimit).Return([]config.PurchaseHistoryRecord{}, nil).Once() + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return([]config.PurchaseExecution{}, nil) + + _, err := handler.getHistory(ctx, req, map[string]string{"provider": "all"}) + require.NoError(t, err) + mockStore.AssertCalled(t, "GetAllPurchaseHistory", ctx, config.DefaultListLimit) + mockStore.AssertNotCalled(t, "GetPurchaseHistoryFiltered", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + t.Run("account_ids filter is pushed to SQL and prunes executions", func(t *testing.T) { + mockStore, handler, req, ctx := newHandler(t) + + uuidA := "11111111-1111-1111-1111-111111111111" + uuidB := "22222222-2222-2222-2222-222222222222" + + mockStore.On("GetPurchaseHistoryFiltered", ctx, "", []string{uuidA, uuidB}, (*time.Time)(nil), (*time.Time)(nil), config.DefaultListLimit). + Return([]config.PurchaseHistoryRecord{{AccountID: "acc-A", PurchaseID: "p-A"}}, nil).Once() + + execs := []config.PurchaseExecution{ + { + ExecutionID: "exec-in-A", + Status: "pending", + ScheduledDate: time.Now(), + CloudAccountID: strPtr(uuidA), + Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}, + }, + { + ExecutionID: "exec-in-C", + Status: "pending", + ScheduledDate: time.Now(), + CloudAccountID: strPtr("33333333-3333-3333-3333-333333333333"), + Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}, + }, + { + ExecutionID: "exec-nil-account", + Status: "pending", + ScheduledDate: time.Now(), + Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}, + }, + } + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(execs, nil) + + result, err := handler.getHistory(ctx, req, map[string]string{"account_ids": uuidA + "," + uuidB}) + require.NoError(t, err) + resp := result.(HistoryResponse) + + // 1 DB row + only the exec whose CloudAccountID is in the filter list. + // NULL CloudAccountID execs must be excluded (mirrors SQL semantics). + require.Len(t, resp.Purchases, 2) + ids := map[string]bool{} + for _, p := range resp.Purchases { + ids[p.PurchaseID] = true + } + assert.True(t, ids["exec-in-A"], "execution in account A must survive the filter") + assert.False(t, ids["exec-in-C"], "execution in unrelated account C must be filtered out") + assert.False(t, ids["exec-nil-account"], "execution with NULL CloudAccountID must be excluded once account_ids is non-empty") + }) + + t.Run("legacy singular account_id is ignored when combined with new filters", func(t *testing.T) { + // account_id (singular) is the AWS-style VARCHAR(20) account number + // matched against purchase_history.account_id by the legacy + // GetPurchaseHistory; it is NOT a UUID and therefore must not be + // silently coerced into the cloud_account_id (UUID) WHERE clause of + // the new GetPurchaseHistoryFiltered method when other filters are + // also present. The frontend uses `account_ids` (plural, UUIDs) for + // the new shape; the singular param survives only for backward + // compatibility with the no-other-filters fast path. + mockStore, handler, req, ctx := newHandler(t) + + mockStore.On("GetPurchaseHistoryFiltered", ctx, "aws", []string(nil), (*time.Time)(nil), (*time.Time)(nil), config.DefaultListLimit). + Return([]config.PurchaseHistoryRecord{}, nil).Once() + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return([]config.PurchaseExecution{}, nil) + + _, err := handler.getHistory(ctx, req, map[string]string{"provider": "aws", "account_id": "123456789012"}) + require.NoError(t, err) + mockStore.AssertExpectations(t) + mockStore.AssertNotCalled(t, "GetPurchaseHistory", mock.Anything, mock.Anything, mock.Anything) + }) + + t.Run("legacy singular account_id alone still hits the fast path", func(t *testing.T) { + mockStore, handler, req, ctx := newHandler(t) + mockStore.On("GetPurchaseHistory", ctx, "123456789012", config.DefaultListLimit). + Return([]config.PurchaseHistoryRecord{{AccountID: "123456789012", PurchaseID: "p-legacy"}}, nil).Once() + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return([]config.PurchaseExecution{}, nil) + + _, err := handler.getHistory(ctx, req, map[string]string{"account_id": "123456789012"}) + require.NoError(t, err) + mockStore.AssertExpectations(t) + mockStore.AssertNotCalled(t, "GetPurchaseHistoryFiltered", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) + + t.Run("start/end date filter is pushed to SQL and prunes executions", func(t *testing.T) { + mockStore, handler, req, ctx := newHandler(t) + + // Build start/end relative to "now" so the in-range execution's + // ScheduledDate stays younger than the approvalExpiryWindow (7 days). + // Older ScheduledDates would trigger the lazy expireIfStale path and + // require mocking TransitionExecutionStatus — orthogonal to what + // this test covers. + now := time.Now().UTC() + startDay := now.AddDate(0, 0, -3).Format("2006-01-02") + endDay := now.Format("2006-01-02") + outOfRangeDay := now.AddDate(0, 0, -10) // older than the window AND outside the requested range + + mockStore.On("GetPurchaseHistoryFiltered", + ctx, "", []string(nil), + mock.MatchedBy(func(p *time.Time) bool { return p != nil && p.Format("2006-01-02") == startDay }), + mock.MatchedBy(func(p *time.Time) bool { return p != nil && p.Format("2006-01-02") == endDay }), + config.DefaultListLimit, + ).Return([]config.PurchaseHistoryRecord{{AccountID: "in-range", PurchaseID: "p-in-range"}}, nil).Once() + + execs := []config.PurchaseExecution{ + { + ExecutionID: "exec-in-range", + Status: "approved", // approved -> expireIfStale short-circuits before transition + ScheduledDate: now.Add(-12 * time.Hour), + Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}, + }, + { + ExecutionID: "exec-out-of-range", + Status: "approved", + ScheduledDate: outOfRangeDay, + Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}, + }, + } + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(execs, nil) + + result, err := handler.getHistory(ctx, req, map[string]string{"start": startDay, "end": endDay}) + require.NoError(t, err) + resp := result.(HistoryResponse) + + require.Len(t, resp.Purchases, 2, "1 DB row + 1 in-range exec") + for _, p := range resp.Purchases { + assert.NotEqual(t, "exec-out-of-range", p.PurchaseID, "out-of-range execution must be filtered out") + } + }) + + t.Run("combined provider+account_ids+date filter is applied on both halves", func(t *testing.T) { + mockStore, handler, req, ctx := newHandler(t) + + uuidA := "55555555-5555-5555-5555-555555555555" + mockStore.On("GetPurchaseHistoryFiltered", + ctx, "aws", []string{uuidA}, + mock.MatchedBy(func(p *time.Time) bool { return p != nil }), + mock.MatchedBy(func(p *time.Time) bool { return p != nil }), + config.DefaultListLimit, + ).Return([]config.PurchaseHistoryRecord{{AccountID: "match", PurchaseID: "p-match"}}, nil).Once() + + now := time.Now().UTC() + startDay := now.AddDate(0, 0, -3).Format("2006-01-02") + endDay := now.Format("2006-01-02") + // Use "approved" status so expireIfStale short-circuits regardless of + // ScheduledDate (the date predicate itself is what we're asserting). + inRange := now.Add(-12 * time.Hour) + execs := []config.PurchaseExecution{ + { + ExecutionID: "exec-match", + Status: "approved", + ScheduledDate: inRange, + CloudAccountID: strPtr(uuidA), + Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}, + }, + { + ExecutionID: "exec-wrong-provider", + Status: "approved", + ScheduledDate: inRange, + CloudAccountID: strPtr(uuidA), + Recommendations: []config.RecommendationRecord{{Provider: "azure", Service: "vm"}}, + }, + { + ExecutionID: "exec-wrong-account", + Status: "approved", + ScheduledDate: inRange, + CloudAccountID: strPtr("66666666-6666-6666-6666-666666666666"), + Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2"}}, + }, + } + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return(execs, nil) + + result, err := handler.getHistory(ctx, req, map[string]string{ + "provider": "aws", + "account_ids": uuidA, + "start": startDay, + "end": endDay, + }) + require.NoError(t, err) + resp := result.(HistoryResponse) + + require.Len(t, resp.Purchases, 2, "1 DB row + only the exec that matches ALL three filters") + ids := map[string]bool{} + for _, p := range resp.Purchases { + ids[p.PurchaseID] = true + } + assert.True(t, ids["exec-match"]) + assert.False(t, ids["exec-wrong-provider"]) + assert.False(t, ids["exec-wrong-account"]) + }) +} + +// TestHandler_getHistory_FilterValidation covers the 400-on-malformed-input +// paths for issue #701: an invalid provider, a non-UUID account_id, a date +// that doesn't parse as YYYY-MM-DD, an inverted range, and a range that +// exceeds MaxHistoryDateRangeDays (the DoS guard mirroring PR #529 / issue +// #414). Each must return a 400 ClientError; none must reach the store. +func TestHandler_getHistory_FilterValidation(t *testing.T) { + cases := []struct { + name string + params map[string]string + wantCode int + wantContain string + }{ + { + name: "invalid provider", + params: map[string]string{"provider": "unknown"}, + wantCode: 400, + wantContain: "invalid provider", + }, + { + name: "non-UUID account_ids", + params: map[string]string{"account_ids": "not-a-uuid"}, + wantCode: 400, + wantContain: "invalid account_ids", + }, + { + name: "malformed start date (not YYYY-MM-DD)", + params: map[string]string{"start": "01/02/2024"}, + wantCode: 400, + wantContain: "invalid start date format", + }, + { + name: "malformed end date", + params: map[string]string{"end": "2024-13-40"}, + wantCode: 400, + wantContain: "invalid end date format", + }, + { + name: "inverted range (start after end)", + params: map[string]string{"start": "2024-12-31", "end": "2024-01-01"}, + wantCode: 400, + wantContain: "before or equal to end", + }, + { + name: "range exceeds 366 days", + params: map[string]string{"start": "2024-01-01", "end": "2025-12-31"}, + wantCode: 400, + wantContain: "date range too large", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth, req := adminHistoryReq(ctx) + handler := &Handler{auth: mockAuth, config: mockStore} + + _, err := handler.getHistory(ctx, req, tc.params) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "validation failure must surface as a ClientError, got %T: %v", err, err) + assert.Equal(t, tc.wantCode, ce.code) + assert.Contains(t, err.Error(), tc.wantContain) + + // Sanity guards: no store calls leaked through on a 400. + mockStore.AssertNotCalled(t, "GetPurchaseHistory", mock.Anything, mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "GetAllPurchaseHistory", mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "GetPurchaseHistoryFiltered", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) + mockStore.AssertNotCalled(t, "GetExecutionsByStatuses", mock.Anything, mock.Anything, mock.Anything) + }) + } +} + +// TestHandler_getHistory_DateRangeBoundary asserts the inclusive boundaries +// of the 366-day window: a range of exactly 366 days is accepted; 367 is +// rejected. Mirrors the analytics handler's range cap (PR #529). +func TestHandler_getHistory_DateRangeBoundary(t *testing.T) { + ctx := context.Background() + + t.Run("range of exactly 366 days is accepted", func(t *testing.T) { + mockStore := new(MockConfigStore) + mockAuth, req := adminHistoryReq(ctx) + handler := &Handler{auth: mockAuth, config: mockStore} + mockStore.On("GetPurchaseHistoryFiltered", ctx, "", []string(nil), mock.Anything, mock.Anything, config.DefaultListLimit). + Return([]config.PurchaseHistoryRecord{}, nil) + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return([]config.PurchaseExecution{}, nil) + + // 2024 is a leap year — Jan 1 to Dec 31 inclusive is 366 days. + _, err := handler.getHistory(ctx, req, map[string]string{"start": "2024-01-01", "end": "2024-12-31"}) + require.NoError(t, err, "366-day range must be accepted (boundary)") + }) + + t.Run("range of 367 days is rejected", func(t *testing.T) { + mockStore := new(MockConfigStore) + mockAuth, req := adminHistoryReq(ctx) + handler := &Handler{auth: mockAuth, config: mockStore} + + _, err := handler.getHistory(ctx, req, map[string]string{"start": "2024-01-01", "end": "2025-01-02"}) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 400, ce.code) + }) +} + +// TestParseHistoryDateRange covers the YYYY-MM-DD parser in isolation: empty +// inputs disable the filter (hasDate=false), one-sided inputs default the +// other bound open, and end is rolled forward to end-of-day so date inputs +// behave inclusively on the user's chosen day. +func TestParseHistoryDateRange(t *testing.T) { + t.Run("both empty -> no date filter", func(t *testing.T) { + s, e, has, err := parseHistoryDateRange("", "") + require.NoError(t, err) + assert.False(t, has, "empty inputs must yield hasDate=false so the SQL clause is skipped") + assert.True(t, s.IsZero()) + assert.True(t, e.IsZero()) + }) + + t.Run("only end -> start defaults to end - MaxHistoryDateRangeDays", func(t *testing.T) { + s, e, has, err := parseHistoryDateRange("", "2024-06-15") + require.NoError(t, err) + assert.True(t, has) + assert.Equal(t, "2024-06-15", e.Format("2006-01-02")) + // The absent lower bound must be MaxHistoryDateRangeDays before end + // (not epoch) so the open-side default still satisfies the DoS cap. + expectedStart := e.Add(-MaxHistoryDateRangeDays * 24 * time.Hour) + assert.Equal(t, expectedStart.Format("2006-01-02"), s.Format("2006-01-02")) + }) + + t.Run("only start -> end defaults to start + MaxHistoryDateRangeDays", func(t *testing.T) { + s, e, has, err := parseHistoryDateRange("2024-06-15", "") + require.NoError(t, err) + assert.True(t, has) + assert.Equal(t, "2024-06-15", s.Format("2006-01-02")) + expectedEnd := s.Add(MaxHistoryDateRangeDays * 24 * time.Hour) + assert.Equal(t, expectedEnd.Format("2006-01-02"), e.Format("2006-01-02")) + }) + + t.Run("end is inclusive of the requested day", func(t *testing.T) { + _, e, _, err := parseHistoryDateRange("2024-01-01", "2024-01-15") + require.NoError(t, err) + // End must be 2024-01-15 23:59:59 UTC so a row stamped at any time on + // that day is included. + assert.Equal(t, 2024, e.Year()) + assert.Equal(t, time.January, e.Month()) + assert.Equal(t, 15, e.Day()) + assert.Equal(t, 23, e.Hour()) + }) +} + // TestHandler_getHistory_CompletedExecutionNotDuplicated guards the dedup path. // The store loads "completed" executions now (so audit-gap rows can surface), // but a NORMAL completed execution (Error=="") is already represented by its diff --git a/internal/api/mocks_test.go b/internal/api/mocks_test.go index e1edcf3d3..b351f2ea3 100644 --- a/internal/api/mocks_test.go +++ b/internal/api/mocks_test.go @@ -196,6 +196,14 @@ func (m *MockConfigStore) GetAllPurchaseHistory(ctx context.Context, limit int) return args.Get(0).([]config.PurchaseHistoryRecord), args.Error(1) } +func (m *MockConfigStore) GetPurchaseHistoryFiltered(ctx context.Context, providerFilter string, accountIDs []string, start, end *time.Time, limit int) ([]config.PurchaseHistoryRecord, error) { + args := m.Called(ctx, providerFilter, accountIDs, start, end, limit) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).([]config.PurchaseHistoryRecord), args.Error(1) +} + func (m *MockConfigStore) GetExecutionByID(ctx context.Context, executionID string) (*config.PurchaseExecution, error) { args := m.Called(ctx, executionID) if args.Get(0) == nil { diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go index ac3cc2931..2fd14a2c2 100644 --- a/internal/config/interfaces.go +++ b/internal/config/interfaces.go @@ -84,6 +84,24 @@ type StoreInterface interface { SavePurchaseHistory(ctx context.Context, record *PurchaseHistoryRecord) error GetPurchaseHistory(ctx context.Context, accountID string, limit int) ([]PurchaseHistoryRecord, error) GetAllPurchaseHistory(ctx context.Context, limit int) ([]PurchaseHistoryRecord, error) + // GetPurchaseHistoryFiltered reads purchase_history rows matching the + // supplied filter set, newest-first, capped at limit. Each filter is + // applied independently and only when non-empty: + // - providerFilter: matches purchase_history.provider exactly. Empty + // skips the clause. + // - accountIDs: matches purchase_history.cloud_account_id (UUID) with + // ANY($). Empty/nil skips the clause; non-empty excludes legacy + // ambient rows whose cloud_account_id IS NULL (mirrors the + // recommendations filter semantics on issue #211). + // - start/end: bounds purchase_history.timestamp with a BETWEEN. nil + // for both skips the clause; nil for either sets that side open + // (caller is responsible for any range cap, see + // api.MaxHistoryDateRangeDays). + // Added for issue #701: the legacy GetPurchaseHistory / + // GetAllPurchaseHistory pair only accepted a single account_id and the + // limit, so the /api/history handler silently dropped the + // provider/account_ids/start/end query params the frontend was sending. + GetPurchaseHistoryFiltered(ctx context.Context, providerFilter string, accountIDs []string, start, end *time.Time, limit int) ([]PurchaseHistoryRecord, error) // RI Exchange history SaveRIExchangeRecord(ctx context.Context, record *RIExchangeRecord) error diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index 869f62658..3f8cc3f77 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -1304,6 +1304,68 @@ func (s *PostgresStore) GetAllPurchaseHistory(ctx context.Context, limit int) ([ return s.queryPurchaseHistory(ctx, query, limit) } +// GetPurchaseHistoryFiltered reads purchase_history rows matching the +// supplied filter set, newest-first, capped at limit. See the StoreInterface +// docstring for the per-filter semantics. Each WHERE clause is appended +// only when its filter is populated, so callers that pass an empty +// providerFilter / nil accountIDs / nil dates get the same plan-shape as +// GetAllPurchaseHistory. Implementation mirrors buildRecommendationFilter +// (store_postgres_recommendations.go). +func (s *PostgresStore) GetPurchaseHistoryFiltered( + ctx context.Context, + providerFilter string, + accountIDs []string, + start, end *time.Time, + limit int, +) ([]PurchaseHistoryRecord, error) { + if limit <= 0 { + limit = DefaultListLimit + } + if limit > MaxListLimit { + limit = MaxListLimit + } + + var ( + conds []string + args []any + ) + add := func(cond string, val any) { + conds = append(conds, fmt.Sprintf(cond, len(args)+1)) + args = append(args, val) + } + if providerFilter != "" { + add("provider = $%d", providerFilter) + } + if len(accountIDs) > 0 { + // NULL cloud_account_id rows are excluded once a list is supplied, + // same as buildRecommendationFilter (issue #211 semantics). + add("cloud_account_id = ANY($%d)", accountIDs) + } + if start != nil { + add("timestamp >= $%d", *start) + } + if end != nil { + add("timestamp <= $%d", *end) + } + + where := "" + if len(conds) > 0 { + where = " WHERE " + strings.Join(conds, " AND ") + } + args = append(args, limit) + + query := fmt.Sprintf(` + SELECT account_id, purchase_id, timestamp, provider, service, region, + resource_type, count, term, payment, upfront_cost, monthly_cost, + estimated_savings, plan_id, plan_name, ramp_step, cloud_account_id + FROM purchase_history%s + ORDER BY timestamp DESC + LIMIT $%d + `, where, len(args)) + + return s.queryPurchaseHistory(ctx, query, args...) +} + // queryPurchaseHistory is a helper to query and scan purchase history func (s *PostgresStore) queryPurchaseHistory(ctx context.Context, query string, args ...any) ([]PurchaseHistoryRecord, error) { rows, err := s.db.Query(ctx, query, args...) diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go index 263935624..bd95d844e 100644 --- a/internal/config/store_postgres_pgxmock_test.go +++ b/internal/config/store_postgres_pgxmock_test.go @@ -491,6 +491,126 @@ func TestPGXMock_GetPurchaseHistory_Success(t *testing.T) { assert.NoError(t, mock.ExpectationsWereMet()) } +// ─── GetPurchaseHistoryFiltered (issue #701) ───────────────────────────────── + +// purchaseHistoryCols lists the SELECT columns for purchase_history rows in +// the order GetPurchaseHistoryFiltered scans them. +var purchaseHistoryCols = []string{ + "account_id", "purchase_id", "timestamp", "provider", "service", "region", + "resource_type", "count", "term", "payment", "upfront_cost", "monthly_cost", + "estimated_savings", "plan_id", "plan_name", "ramp_step", "cloud_account_id", +} + +// purchaseHistoryRow builds a single AddRow tuple matching purchaseHistoryCols. +func purchaseHistoryRow(now time.Time, provider, acct string) []interface{} { + return []interface{}{ + acct, "pur-1", now, provider, "ec2", "us-east-1", + "m5.large", 1, 1, "no-upfront", 100.0, 50.0, 200.0, + sql.NullString{}, sql.NullString{}, 0, sql.NullString{}, + } +} + +// TestPGXMock_GetPurchaseHistoryFiltered_AllFilters asserts the SQL emitted +// when every filter is set: WHERE provider = $1 AND cloud_account_id = +// ANY($2) AND timestamp >= $3 AND timestamp <= $4, ORDER BY timestamp DESC, +// LIMIT $5. Each filter's positional argument is bound in declaration order +// (provider, accountIDs, start, end, limit). +func TestPGXMock_GetPurchaseHistoryFiltered_AllFilters(t *testing.T) { + mock := newMock(t) + store := storeWith(mock) + ctx := context.Background() + + now := time.Now().Truncate(time.Second) + start := now.Add(-24 * time.Hour) + end := now + + rows := pgxmock.NewRows(purchaseHistoryCols).AddRow(purchaseHistoryRow(now, "aws", "acct-1")...) + mock.ExpectQuery( + `SELECT account_id, purchase_id, timestamp, provider, service, region.*FROM purchase_history WHERE provider = \$1 AND cloud_account_id = ANY\(\$2\) AND timestamp >= \$3 AND timestamp <= \$4.*ORDER BY timestamp DESC.*LIMIT \$5`, + ).WithArgs("aws", []string{"acct-uuid-1"}, start, end, 50).WillReturnRows(rows) + + records, err := store.GetPurchaseHistoryFiltered(ctx, "aws", []string{"acct-uuid-1"}, &start, &end, 50) + require.NoError(t, err) + require.Len(t, records, 1) + assert.Equal(t, "aws", records[0].Provider) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +// TestPGXMock_GetPurchaseHistoryFiltered_NoFilters asserts that an +// all-defaults call (empty provider, nil accountIDs, nil dates) emits a +// WHERE-less query identical in shape to GetAllPurchaseHistory. This is +// what proves the handler's fast-path call into the legacy methods stays a +// valid alternative — the filtered variant degrades gracefully. +func TestPGXMock_GetPurchaseHistoryFiltered_NoFilters(t *testing.T) { + mock := newMock(t) + store := storeWith(mock) + ctx := context.Background() + + now := time.Now().Truncate(time.Second) + rows := pgxmock.NewRows(purchaseHistoryCols).AddRow(purchaseHistoryRow(now, "aws", "acct-1")...) + // No WHERE clause; the only argument bound is the LIMIT. + mock.ExpectQuery(`SELECT.*FROM purchase_history\s+ORDER BY timestamp DESC.*LIMIT \$1`). + WithArgs(100).WillReturnRows(rows) + + records, err := store.GetPurchaseHistoryFiltered(ctx, "", nil, nil, nil, 100) + require.NoError(t, err) + assert.Len(t, records, 1) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +// TestPGXMock_GetPurchaseHistoryFiltered_PartialFilters asserts that +// supplying only a subset of filters (here: provider + start, no +// accountIDs, no end) emits exactly two AND clauses and binds the right +// positional arguments. Guards against an off-by-one in the placeholder +// counter (the bug shape mirroring buildRecommendationFilter's add helper). +func TestPGXMock_GetPurchaseHistoryFiltered_PartialFilters(t *testing.T) { + mock := newMock(t) + store := storeWith(mock) + ctx := context.Background() + + now := time.Now().Truncate(time.Second) + start := now.Add(-7 * 24 * time.Hour) + + rows := pgxmock.NewRows(purchaseHistoryCols) + mock.ExpectQuery( + `FROM purchase_history WHERE provider = \$1 AND timestamp >= \$2.*ORDER BY timestamp DESC.*LIMIT \$3`, + ).WithArgs("azure", start, 25).WillReturnRows(rows) + + _, err := store.GetPurchaseHistoryFiltered(ctx, "azure", nil, &start, nil, 25) + require.NoError(t, err) + assert.NoError(t, mock.ExpectationsWereMet()) +} + +// TestPGXMock_GetPurchaseHistoryFiltered_LimitClamp asserts the store-side +// limit clamp: an out-of-range limit (negative or above MaxListLimit) is +// normalised before the query runs so a malicious or buggy caller can't +// exfiltrate the entire table by passing limit=2_000_000. +func TestPGXMock_GetPurchaseHistoryFiltered_LimitClamp(t *testing.T) { + mock := newMock(t) + store := storeWith(mock) + ctx := context.Background() + + rows := pgxmock.NewRows(purchaseHistoryCols) + // Negative limit must be replaced with DefaultListLimit. + mock.ExpectQuery(`FROM purchase_history\s+ORDER BY timestamp DESC.*LIMIT \$1`). + WithArgs(DefaultListLimit).WillReturnRows(rows) + + _, err := store.GetPurchaseHistoryFiltered(ctx, "", nil, nil, nil, -5) + require.NoError(t, err) + assert.NoError(t, mock.ExpectationsWereMet()) + + // Over-MaxListLimit must be clamped to MaxListLimit. + mock2 := newMock(t) + store2 := storeWith(mock2) + rows2 := pgxmock.NewRows(purchaseHistoryCols) + mock2.ExpectQuery(`FROM purchase_history\s+ORDER BY timestamp DESC.*LIMIT \$1`). + WithArgs(MaxListLimit).WillReturnRows(rows2) + + _, err = store2.GetPurchaseHistoryFiltered(ctx, "", nil, nil, nil, MaxListLimit+1) + require.NoError(t, err) + assert.NoError(t, mock2.ExpectationsWereMet()) +} + // ─── GetRIExchangeRecord / queryRIExchangeRecords ───────────────────────────── func riExchangeRow(now time.Time) []interface{} { diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go index 89c531ea9..d9931b7bf 100644 --- a/internal/mocks/stores.go +++ b/internal/mocks/stores.go @@ -189,6 +189,15 @@ func (m *MockConfigStore) GetAllPurchaseHistory(ctx context.Context, limit int) return args.Get(0).([]config.PurchaseHistoryRecord), args.Error(1) } +// GetPurchaseHistoryFiltered mocks the GetPurchaseHistoryFiltered operation (issue #701). +func (m *MockConfigStore) GetPurchaseHistoryFiltered(ctx context.Context, providerFilter string, accountIDs []string, start, end *time.Time, limit int) ([]config.PurchaseHistoryRecord, error) { + args := m.Called(ctx, providerFilter, accountIDs, start, end, limit) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).([]config.PurchaseHistoryRecord), args.Error(1) +} + func (m *MockConfigStore) SaveRIExchangeRecord(ctx context.Context, record *config.RIExchangeRecord) error { args := m.Called(ctx, record) return args.Error(0) diff --git a/internal/purchase/mocks_test.go b/internal/purchase/mocks_test.go index 8b0199876..db460f3d2 100644 --- a/internal/purchase/mocks_test.go +++ b/internal/purchase/mocks_test.go @@ -344,6 +344,14 @@ func (m *MockConfigStore) GetAllPurchaseHistory(ctx context.Context, limit int) return args.Get(0).([]config.PurchaseHistoryRecord), args.Error(1) } +func (m *MockConfigStore) GetPurchaseHistoryFiltered(ctx context.Context, providerFilter string, accountIDs []string, start, end *time.Time, limit int) ([]config.PurchaseHistoryRecord, error) { + args := m.Called(ctx, providerFilter, accountIDs, start, end, limit) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).([]config.PurchaseHistoryRecord), args.Error(1) +} + func (m *MockConfigStore) SaveRIExchangeRecord(ctx context.Context, record *config.RIExchangeRecord) error { args := m.Called(ctx, record) return args.Error(0) diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index afb7b03bd..90d7fd7b2 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -169,6 +169,14 @@ func (m *MockConfigStore) GetAllPurchaseHistory(ctx context.Context, limit int) return args.Get(0).([]config.PurchaseHistoryRecord), args.Error(1) } +func (m *MockConfigStore) GetPurchaseHistoryFiltered(ctx context.Context, providerFilter string, accountIDs []string, start, end *time.Time, limit int) ([]config.PurchaseHistoryRecord, error) { + args := m.Called(ctx, providerFilter, accountIDs, start, end, limit) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).([]config.PurchaseHistoryRecord), args.Error(1) +} + func (m *MockConfigStore) GetExecutionByID(ctx context.Context, executionID string) (*config.PurchaseExecution, error) { args := m.Called(ctx, executionID) if args.Get(0) == nil { diff --git a/internal/server/test_helpers_test.go b/internal/server/test_helpers_test.go index 033e27790..7369acb3f 100644 --- a/internal/server/test_helpers_test.go +++ b/internal/server/test_helpers_test.go @@ -95,6 +95,10 @@ func (m *mockConfigStoreForHealth) GetAllPurchaseHistory(ctx context.Context, li return nil, nil } +func (m *mockConfigStoreForHealth) GetPurchaseHistoryFiltered(ctx context.Context, providerFilter string, accountIDs []string, start, end *time.Time, limit int) ([]config.PurchaseHistoryRecord, error) { + return nil, nil +} + func (m *mockConfigStoreForHealth) CleanupOldExecutions(ctx context.Context, retentionDays int) (int64, error) { return 0, nil }