diff --git a/internal/api/handler_history.go b/internal/api/handler_history.go index b4371daa5..a05c2214d 100644 --- a/internal/api/handler_history.go +++ b/internal/api/handler_history.go @@ -6,6 +6,7 @@ import ( "fmt" "sort" "strconv" + "strings" "time" "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" @@ -192,7 +193,7 @@ const approvalExpiryWindow = 7 * 24 * time.Hour // the class reached its MaxListLimit cap, so older executions may have been // dropped before the filters ran, or its listing failed (issue #686). func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyFilters) ([]config.PurchaseHistoryRecord, []config.PurchaseExecution, bool) { - executions, ok := h.fetchExecutionsByClass(ctx, actionableExecutionStatuses, config.MaxListLimit) + executions, ok := h.fetchExecutionsByClass(ctx, actionableExecutionStatuses, filters.PlanID, config.MaxListLimit) incomplete := !ok || len(executions) >= config.MaxListLimit if len(executions) == 0 { return nil, nil, incomplete @@ -222,8 +223,17 @@ func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyF // the class skipped (ok=false) so the other class and completed history still // render; the caller flags the response as incomplete so the hidden rows are // not silent (issue #686). -func (h *Handler) fetchExecutionsByClass(ctx context.Context, statuses []string, limit int) ([]config.PurchaseExecution, bool) { - rows, err := h.config.GetExecutionsByStatuses(ctx, statuses, limit) +// +// A non-empty planID selects only that plan's executions in SQL, so a plan view +// is not crowded out by other plans' rows under the limit. +func (h *Handler) fetchExecutionsByClass(ctx context.Context, statuses []string, planID string, limit int) ([]config.PurchaseExecution, bool) { + var rows []config.PurchaseExecution + var err error + if planID != "" { + rows, err = h.config.ListHistoryExecutionsPage(ctx, config.HistoryExecutionsPageQuery{Statuses: statuses, PlanID: planID, Limit: limit}) + } else { + rows, err = h.config.GetExecutionsByStatuses(ctx, statuses, limit) + } if err != nil { logging.Warnf("history: failed to load executions with statuses %v: %v", statuses, err) return nil, false @@ -721,9 +731,12 @@ type historyFilters struct { ExternalIDsByProvider map[string][]string Provider string LegacyAccountID string - AccountIDs []string - Limit int - HasDate bool + // PlanID narrows the history to one purchase plan's purchases and + // executions (the plan-history view's plan_id); empty means no plan filter. + PlanID string + AccountIDs []string + Limit int + HasDate bool // Cursor is the stream position the page starts after (issue #674); nil // for the first page. Cursor *config.HistoryPageKey @@ -752,6 +765,12 @@ func parseHistoryFilters(params map[string]string) (historyFilters, error) { f.AccountIDs = accountIDs f.LegacyAccountID = params["account_id"] + planID, err := parseHistoryPlanID(params["plan_id"]) + if err != nil { + return f, err + } + f.PlanID = planID + start, end, hasDate, err := parseHistoryDateRange(params["start"], params["end"]) if err != nil { return f, err @@ -760,11 +779,29 @@ func parseHistoryFilters(params map[string]string) (historyFilters, error) { f.Start = start f.End = end + limit, err := parseHistoryLimit(params["limit"]) + if err != nil { + return f, err + } + f.Limit = limit + + cursor, pagingErr := parseHistoryPaging(params) + if pagingErr != nil { + return f, pagingErr + } + f.Cursor = cursor + + return f, nil +} + +// parseHistoryLimit parses the optional limit, clamped to [1, MaxListLimit] +// with DefaultListLimit for empty or non-positive values. +func parseHistoryLimit(raw string) (int, error) { limit := config.DefaultListLimit - if s := params["limit"]; s != "" { - n, err := strconv.Atoi(s) + if raw != "" { + n, err := strconv.Atoi(raw) if err != nil { - return f, NewClientError(400, "invalid limit: must be a positive integer") + return 0, NewClientError(400, "invalid limit: must be a positive integer") } limit = n } @@ -774,15 +811,21 @@ func parseHistoryFilters(params map[string]string) (historyFilters, error) { if limit > config.MaxListLimit { limit = config.MaxListLimit } - f.Limit = limit - - cursor, pagingErr := parseHistoryPaging(params) - if pagingErr != nil { - return f, pagingErr - } - f.Cursor = cursor + return limit, nil +} - return f, nil +// parseHistoryPlanID validates the optional plan_id filter; empty means none. +// The value is trimmed and lowercased so the SQL predicate and the in-memory +// execution check compare the same canonical text. +func parseHistoryPlanID(raw string) (string, error) { + id := strings.TrimSpace(raw) + if id == "" { + return "", nil + } + if err := validateUUID(id); err != nil { + return "", NewClientError(400, "invalid plan_id: must be a UUID") + } + return strings.ToLower(id), nil } // parseHistoryDateRange parses optional YYYY-MM-DD start/end strings. Both @@ -878,6 +921,9 @@ func (f historyFilters) matchesExecution(exec config.PurchaseExecution) bool { if !f.matchesExecutionProviderAndAccount(exec) { return false } + if f.PlanID != "" && exec.PlanID != f.PlanID { + return false + } if f.HasDate { if exec.ScheduledDate.Before(f.Start) || exec.ScheduledDate.After(f.End) { return false @@ -1016,6 +1062,7 @@ func (h *Handler) fetchPurchaseHistory(ctx context.Context, filters *historyFilt func (f historyFilters) purchaseHistoryFilter() config.PurchaseHistoryFilter { out := config.PurchaseHistoryFilter{ Provider: f.Provider, + PlanID: f.PlanID, AccountIDs: f.AccountIDs, ExternalIDsByProvider: f.ExternalIDsByProvider, Limit: f.Limit, diff --git a/internal/api/handler_history_page.go b/internal/api/handler_history_page.go index ae4076943..f5671d1d6 100644 --- a/internal/api/handler_history_page.go +++ b/internal/api/handler_history_page.go @@ -245,10 +245,10 @@ func (st *historyStream) fetchPurchases(ctx context.Context, after *config.Histo // fetchTerminalExecutions reads a batch of terminal executions and projects // the ones matching the request's filters into History rows. The date bound -// is pushed into SQL as well as checked by matchesExecution; provider and -// account are checked in Go only, like the actionable class. +// is pushed into SQL as well as checked by matchesExecution; the plan is pushed into +// SQL too; provider and account are checked in Go only, like the actionable class. func (st *historyStream) fetchTerminalExecutions(ctx context.Context, after *config.HistoryPageKey) ([]historyStreamRow, error) { - q := config.HistoryExecutionsPageQuery{Statuses: terminalExecutionStatuses, After: after, Limit: st.executions.batch} + q := config.HistoryExecutionsPageQuery{Statuses: terminalExecutionStatuses, PlanID: st.filters.PlanID, After: after, Limit: st.executions.batch} if st.filters.HasDate { start, end := st.filters.Start, st.filters.End q.Start, q.End = &start, &end diff --git a/internal/api/handler_history_plan_filter_integration_test.go b/internal/api/handler_history_plan_filter_integration_test.go new file mode 100644 index 000000000..1af99fabf --- /dev/null +++ b/internal/api/handler_history_plan_filter_integration_test.go @@ -0,0 +1,176 @@ +//go:build integration + +package api + +import ( + "context" + "encoding/json" + "strings" + "testing" + "time" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/LeanerCloud/cloud-commitments-platform/internal/database/postgres/testhelpers" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +type planFilterEnv struct { + *historySummaryEnv + container *testhelpers.PostgresContainer + planA, planB string + admin, scoped *Handler +} + +func (e *planFilterEnv) createPlan(ctx context.Context, t *testing.T, name string) string { + t.Helper() + next := time.Now().Add(time.Hour).Truncate(time.Microsecond) + plan := &config.PurchasePlan{ + Name: name, Enabled: true, + Services: map[string]config.ServiceConfig{"ec2": {Provider: "aws", Service: "ec2", Enabled: true}}, + RampSchedule: config.RampSchedule{Type: "weekly", PercentPerStep: 25, StepIntervalDays: 7, TotalSteps: 4, StartDate: time.Now().AddDate(0, 0, -7)}, + NextExecutionDate: &next, + } + require.NoError(t, plan.Validate()) + require.NoError(t, e.store.CreatePurchasePlan(ctx, plan)) + return plan.ID +} + +func (e *planFilterEnv) addPlanPurchase(ctx context.Context, t *testing.T, tenant, planID string, at time.Time) { + t.Helper() + acct := e.account(tenant) + up := 10.0 + require.NoError(t, e.store.SavePurchaseHistory(ctx, &config.PurchaseHistoryRecord{ + AccountID: acct.ExternalID, PurchaseID: uuid.NewString(), Timestamp: at, Provider: "aws", Service: "ec2", + Region: "us-east-1", ResourceType: "t3.micro", Count: 1, Term: 1, Payment: "no-upfront", + UpfrontCost: &up, EstimatedSavings: 1, PlanID: planID, CloudAccountID: &acct.ID, + })) +} + +func (e *planFilterEnv) addPlanExecution(ctx context.Context, t *testing.T, tenant, planID, status, execErr string, at time.Time) { + t.Helper() + exec := &config.PurchaseExecution{ + PlanID: planID, ExecutionID: uuid.NewString(), IdempotencyKey: uuid.NewString(), Status: status, Error: execErr, + ScheduledDate: at, TotalUpfrontCost: 500, EstimatedSavings: 50, + } + if tenant != "" { + id := e.account(tenant).ID + exec.CloudAccountID = &id + exec.Recommendations = []config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1", CloudAccountID: &id}} + } + require.NoError(t, e.store.SavePurchaseExecution(ctx, exec)) +} + +// newPlanFilterEnv seeds plan A (tenant A) and plan B (tenant B), each with two +// purchases, a failed and a pending execution, plus tenant A rows tied to no +// plan. The pending rows are recent, so the stale-expiry sweep leaves them. +func newPlanFilterEnv(ctx context.Context, t *testing.T) *planFilterEnv { + t.Helper() + base, container := setupHistorySummaryEnv(ctx, t) + e := &planFilterEnv{historySummaryEnv: base, container: container} + e.planA, e.planB = e.createPlan(ctx, t, "plan-a"), e.createPlan(ctx, t, "plan-b") + at := time.Now().UTC().Add(-3 * 24 * time.Hour).Truncate(time.Second) + for i, p := range []struct{ tenant, plan string }{{"a", e.planA}, {"b", e.planB}, {"a", ""}} { + o := time.Duration(i) * time.Minute + e.addPlanPurchase(ctx, t, p.tenant, p.plan, at.Add(o)) + e.addPlanPurchase(ctx, t, p.tenant, p.plan, at.Add(o+time.Hour)) + e.addPlanExecution(ctx, t, p.tenant, p.plan, "failed", "boom", at.Add(o+2*time.Hour)) + e.addPlanExecution(ctx, t, p.tenant, p.plan, "pending", "", at.Add(o+3*time.Hour)) + } + scopedAuth := new(MockAuthService) + scopedAuth.On("ValidateSession", mock.Anything, scopedToken).Return(&Session{UserID: scopedUserID}, nil).Maybe() + scopedAuth.On("GetUser", mock.Anything, mock.Anything).Return(&User{Email: "creator@example.com"}, nil).Maybe() + scopedAuth.grantScoped(e.tenantA.Name) + adminAuth := new(MockAuthService) + adminAuth.On("GetUser", mock.Anything, mock.Anything).Return(&User{Email: "creator@example.com"}, nil).Maybe() + adminAuth.On("ValidateSession", mock.Anything, "admin-token").Return(&Session{UserID: "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"}, nil).Maybe() + adminAuth.grantAdmin() + e.admin = &Handler{config: e.store, auth: adminAuth} + e.scoped = &Handler{config: e.store, auth: scopedAuth} + return e +} + +func historyBody(ctx context.Context, t *testing.T, h *Handler, token string, params map[string]string) (HistoryResponse, string) { + t.Helper() + resp, _ := getHistoryJSON(ctx, t, h, token, params) + raw, err := json.Marshal(resp) + require.NoError(t, err) + return resp, string(raw) +} + +func assertAllPlan(t *testing.T, resp HistoryResponse, plan string) { + t.Helper() + for i := range resp.Purchases { + assert.Equal(t, plan, resp.Purchases[i].PlanID, "row %s", resp.Purchases[i].PurchaseID) + } +} + +// TestIntegration_HistoryPlanFilter_Canonicalised: an uppercase or padded +// plan_id selects the same purchases and executions as the lowercase one, so +// the SQL predicate and the in-memory execution check agree. +func TestIntegration_HistoryPlanFilter_Canonicalised(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + e := newPlanFilterEnv(ctx, t) + + resp, want := historyBody(ctx, t, e.admin, "admin-token", map[string]string{"plan_id": e.planA}) + require.Len(t, resp.Purchases, 4, "2 purchases + failed + pending execution of plan A") + assertAllPlan(t, resp, e.planA) + assert.Equal(t, 4, resp.Summary.TotalPurchases) + for _, variant := range []string{strings.ToUpper(e.planA), " " + e.planA + " "} { + _, got := historyBody(ctx, t, e.admin, "admin-token", map[string]string{"plan_id": variant}) + assert.Equal(t, want, got, variant) + } + all, _ := historyBody(ctx, t, e.admin, "admin-token", map[string]string{}) + assert.Len(t, all.Purchases, 12, "no plan filter returns every plan and planless rows") +} + +// TestIntegration_HistoryPlanFilter_CrossTenant: a user scoped to tenant A who +// passes tenant B's plan_id gets exactly what a random plan_id gets, and +// tenant A's own plan returns only tenant A's rows. +func TestIntegration_HistoryPlanFilter_CrossTenant(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + e := newPlanFilterEnv(ctx, t) + + adminB, _ := historyBody(ctx, t, e.admin, "admin-token", map[string]string{"plan_id": e.planB}) + require.Len(t, adminB.Purchases, 4, "tenant B's plan is not empty for an unrestricted viewer") + + foreign, foreignBody := historyBody(ctx, t, e.scoped, scopedToken, map[string]string{"plan_id": e.planB}) + _, randomBody := historyBody(ctx, t, e.scoped, scopedToken, map[string]string{"plan_id": uuid.NewString()}) + assert.Empty(t, foreign.Purchases) + assertSummaryEqual(t, HistorySummary{TotalUpfront: new(float64)}, foreign.Summary) + assert.Equal(t, randomBody, foreignBody, "another tenant's plan must be indistinguishable from a missing one") + + own, _ := historyBody(ctx, t, e.scoped, scopedToken, map[string]string{"plan_id": e.planA}) + require.Len(t, own.Purchases, 4) + assertAllPlan(t, own, e.planA) + assert.Equal(t, 4, own.Summary.TotalPurchases) +} + +// TestIntegration_HistoryPlanFilter_ActionableOverCap: more than MaxListLimit +// other-plan actionable executions, all newer, must not push the plan's own +// pending execution out of the listing. +func TestIntegration_HistoryPlanFilter_ActionableOverCap(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute) + defer cancel() + e := newPlanFilterEnv(ctx, t) + _, err := e.container.DB.Pool().Exec(ctx, ` + INSERT INTO purchase_executions (plan_id, status, scheduled_date, idempotency_key) + SELECT $1, 'approved', now() - interval '1 hour' - g * interval '1 second', gen_random_uuid()::text + FROM generate_series(1, $2) g`, e.planB, config.MaxListLimit+5) + require.NoError(t, err) + + resp, _ := historyBody(ctx, t, e.admin, "admin-token", map[string]string{"plan_id": e.planA}) + require.Len(t, resp.Purchases, 4) + assertAllPlan(t, resp, e.planA) + pending := 0 + for i := range resp.Purchases { + if resp.Purchases[i].Status == "pending" { + pending++ + } + } + assert.Equal(t, 1, pending, "the plan's pending execution survives the other plan's rows") +} diff --git a/internal/api/handler_history_plan_filter_test.go b/internal/api/handler_history_plan_filter_test.go new file mode 100644 index 000000000..76328b00b --- /dev/null +++ b/internal/api/handler_history_plan_filter_test.go @@ -0,0 +1,87 @@ +package api + +import ( + "context" + "strings" + "testing" + + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +const planFilterUUID = "11111111-1111-4111-8111-111111111111" + +func TestParseHistoryFilters_PlanID(t *testing.T) { + f, err := parseHistoryFilters(map[string]string{"plan_id": planFilterUUID}) + require.NoError(t, err) + assert.Equal(t, planFilterUUID, f.PlanID) + + f, err = parseHistoryFilters(map[string]string{}) + require.NoError(t, err) + assert.Empty(t, f.PlanID, "absent plan_id means no plan filter") + + f, err = parseHistoryFilters(map[string]string{"plan_id": " " + strings.ToUpper(planFilterUUID) + "\t"}) + require.NoError(t, err) + assert.Equal(t, planFilterUUID, f.PlanID, "padded and uppercase plan_id is canonicalised") + + for _, bad := range []string{"not-a-uuid", "' OR 1=1 --", "11111111-1111-4111-8111-11111111111"} { + _, err = parseHistoryFilters(map[string]string{"plan_id": bad}) + require.Error(t, err, bad) + ce, ok := IsClientError(err) + require.True(t, ok, "got %T: %v", err, err) + assert.Equal(t, 400, ce.code) + assert.Contains(t, err.Error(), "invalid plan_id") + } +} + +func TestHistoryFilters_PlanIDMatchesExecution(t *testing.T) { + f := historyFilters{PlanID: planFilterUUID} + assert.True(t, f.matchesExecution(config.PurchaseExecution{PlanID: planFilterUUID})) + assert.False(t, f.matchesExecution(config.PurchaseExecution{PlanID: "22222222-2222-4222-8222-222222222222"})) + assert.False(t, f.matchesExecution(config.PurchaseExecution{}), "a planless execution is not in any plan's history") + assert.True(t, historyFilters{}.matchesExecution(config.PurchaseExecution{PlanID: planFilterUUID}), "no plan filter keeps a plan's execution") +} + +// TestGetHistory_PlanIDReachesStoreFilters checks that plan_id reaches both the +// row fetch and the summary aggregate, and that an absent plan_id sets neither. +func TestGetHistory_PlanIDReachesStoreFilters(t *testing.T) { + for _, tc := range []struct{ name, planID string }{{"with plan", planFilterUUID}, {"without plan", ""}} { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + var rowFilter config.PurchaseHistoryFilter + mockStore.On("ListPurchaseHistoryPage", ctx, mock.Anything, mock.Anything). + Run(func(args mock.Arguments) { rowFilter = args.Get(1).(config.PurchaseHistoryFilter) }). + Return([]config.PurchaseHistoryRecord{}, nil) + mockStore.On("GetExecutionsByStatuses", ctx, mock.Anything, mock.Anything).Return([]config.PurchaseExecution{}, nil) + var aggFilter config.HistorySummaryFilter + mockStore.On("HistorySummaryAggregate", ctx, mock.Anything). + Run(func(args mock.Arguments) { aggFilter = args.Get(1).(config.HistorySummaryFilter) }). + Return(&config.HistorySummaryGroups{}, nil) + + mockAuth, req := adminHistoryReq(ctx) + handler := &Handler{auth: mockAuth, config: mockStore} + params := map[string]string{} + if tc.planID != "" { + params["plan_id"] = tc.planID + } + _, err := handler.getHistory(ctx, req, params) + require.NoError(t, err) + assert.Equal(t, tc.planID, rowFilter.PlanID) + assert.Equal(t, tc.planID, aggFilter.History.PlanID) + }) + } +} + +func TestGetHistory_BadPlanIDIs400(t *testing.T) { + ctx := context.Background() + mockAuth, req := adminHistoryReq(ctx) + handler := &Handler{auth: mockAuth, config: new(MockConfigStore)} + _, err := handler.getHistory(ctx, req, map[string]string{"plan_id": "garbage"}) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok) + assert.Equal(t, 400, ce.code) +} diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index b94c317d6..e31f7c9c8 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -2680,6 +2680,9 @@ func purchaseHistoryFilterWhere(filter PurchaseHistoryFilter) (where string, arg if filter.Provider != "" { add("provider = $%d", filter.Provider) } + if filter.PlanID != "" { + add("plan_id = $%d", filter.PlanID) + } conds, args = appendAccountPredicate(conds, args, filter.AccountIDs, filter.ExternalIDsByProvider) if filter.Start != nil { add("timestamp >= $%d", *filter.Start) diff --git a/internal/config/store_postgres_history_page.go b/internal/config/store_postgres_history_page.go index 2f5895534..fa5092a24 100644 --- a/internal/config/store_postgres_history_page.go +++ b/internal/config/store_postgres_history_page.go @@ -68,6 +68,8 @@ type HistoryExecutionsPageQuery struct { // Statuses the rows must have. Clean completed rows (status completed, // empty error) are always excluded, as in GetExecutionsByStatuses. Statuses []string + // PlanID matches purchase_executions.plan_id exactly; empty skips the clause. + PlanID string // Start and End bound scheduled_date, inclusive; nil leaves a side open. Start *time.Time End *time.Time @@ -194,6 +196,10 @@ func (s *PostgresStore) ListHistoryExecutionsPage(ctx context.Context, q History // HistorySummaryAggregate, so the stream and the summary select the same // executions (issue #675). conds := []string{historyExecutionsPredicate} + if q.PlanID != "" { + args = append(args, q.PlanID) + conds = append(conds, fmt.Sprintf("plan_id = $%d", len(args))) + } if q.Start != nil { args = append(args, *q.Start) conds = append(conds, fmt.Sprintf("scheduled_date >= $%d", len(args))) diff --git a/internal/config/store_postgres_history_plan_filter_integration_test.go b/internal/config/store_postgres_history_plan_filter_integration_test.go new file mode 100644 index 000000000..5fe1cbfd8 --- /dev/null +++ b/internal/config/store_postgres_history_plan_filter_integration_test.go @@ -0,0 +1,113 @@ +//go:build integration + +package config + +import ( + "context" + "testing" + "time" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func createHistoryTestPlan(ctx context.Context, t *testing.T, store *PostgresStore, name string) string { + t.Helper() + next := time.Now().Add(time.Hour).Truncate(time.Microsecond) + plan := &PurchasePlan{ + Name: name, Enabled: true, + Services: map[string]ServiceConfig{"ec2": {Provider: "aws", Service: "ec2", Enabled: true}}, + RampSchedule: RampSchedule{Type: "weekly", PercentPerStep: 25, StepIntervalDays: 7, TotalSteps: 4, StartDate: time.Now().AddDate(0, 0, -7)}, + NextExecutionDate: &next, + } + require.NoError(t, plan.Validate()) + require.NoError(t, store.CreatePurchasePlan(ctx, plan)) + return plan.ID +} + +// TestIntegration_HistoryPlanIDFilter pins the plan-history view's plan_id +// filter (issue #1404): the row listings and the summary aggregate return only +// that plan's purchases and executions, it narrows (never widens) the account +// filter, and no plan filter returns everything including planless rows. +func TestIntegration_HistoryPlanIDFilter(t *testing.T) { + ctx := context.Background() + f := newHistorySummaryFixture(ctx, t) + planA := createHistoryTestPlan(ctx, t, f.store, "history-plan-a") + planB := createHistoryTestPlan(ctx, t, f.store, "history-plan-b") + a, b := f.accountA, f.accountB + + savePlanPurchase := func(day int, planID, provider, extID string, cloudAccountID *string) string { + rec := &PurchaseHistoryRecord{ + AccountID: extID, PurchaseID: uuid.NewString(), Timestamp: f.base.Add(time.Duration(day) * 24 * time.Hour), + Provider: provider, Service: "ec2", Region: "us-east-1", ResourceType: "t3.micro", Count: 1, Term: 1, + Payment: "no-upfront", EstimatedSavings: 1, PlanID: planID, CloudAccountID: cloudAccountID, + } + require.NoError(t, f.store.SavePurchaseHistory(ctx, rec)) + return rec.PurchaseID + } + pa1 := savePlanPurchase(1, planA, "aws", "ext-sum-a", &a) + pa2 := savePlanPurchase(2, planA, "azure", "ext-sum-b", &b) + savePlanPurchase(3, planB, "aws", "ext-sum-a", &a) + savePlanPurchase(4, "", "aws", "ext-sum-a", &a) // not tied to any plan + + saveExec := func(planID, status string) { + exec := &PurchaseExecution{ + PlanID: planID, ExecutionID: uuid.NewString(), IdempotencyKey: uuid.NewString(), Status: status, + Error: "boom", ScheduledDate: f.base.Add(time.Hour), CloudAccountID: &a, + } + require.NoError(t, f.store.SavePurchaseExecution(ctx, exec)) + } + saveExec(planA, "failed") + saveExec(planB, "failed") + saveExec(planB, "failed") + + purchaseIDs := func(filter PurchaseHistoryFilter) []string { + t.Helper() + filter.Limit = MaxListLimit + page, err := f.store.ListPurchaseHistoryPage(ctx, filter, nil) + require.NoError(t, err) + listed, err := f.store.GetPurchaseHistoryFiltered(ctx, filter) + require.NoError(t, err) + require.Len(t, listed, len(page), "the two listings share one predicate") + ids := make([]string, 0, len(page)) + for i := range page { + ids = append(ids, page[i].PurchaseID) + } + return ids + } + totals := func(filter PurchaseHistoryFilter) (purchases, executions int) { + t.Helper() + agg, err := f.store.HistorySummaryAggregate(ctx, HistorySummaryFilter{History: filter, ExecutionStatuses: []string{"failed"}}) + require.NoError(t, err) + for i := range agg.Purchases { + purchases += agg.Purchases[i].Count + } + for i := range agg.Executions { + executions += agg.Executions[i].Count + } + return purchases, executions + } + + assert.ElementsMatch(t, []string{pa1, pa2}, purchaseIDs(PurchaseHistoryFilter{PlanID: planA})) + p, e := totals(PurchaseHistoryFilter{PlanID: planA}) + assert.Equal(t, 2, p) + assert.Equal(t, 1, e) + p, e = totals(PurchaseHistoryFilter{PlanID: planB}) + assert.Equal(t, 1, p) + assert.Equal(t, 2, e) + + // The plan filter narrows the account filter instead of replacing it. + assert.Equal(t, []string{pa1}, purchaseIDs(PurchaseHistoryFilter{PlanID: planA, AccountIDs: []string{a}})) + p, _ = totals(PurchaseHistoryFilter{PlanID: planA, AccountIDs: []string{a}}) + assert.Equal(t, 1, p) + + // A plan with no rows matches nothing; no plan filter matches everything. + assert.Empty(t, purchaseIDs(PurchaseHistoryFilter{PlanID: uuid.NewString()})) + p, e = totals(PurchaseHistoryFilter{PlanID: uuid.NewString()}) + assert.Zero(t, p+e) + assert.Len(t, purchaseIDs(PurchaseHistoryFilter{}), 4) + p, e = totals(PurchaseHistoryFilter{}) + assert.Equal(t, 4, p) + assert.Equal(t, 3, e) +} diff --git a/internal/config/store_postgres_history_summary.go b/internal/config/store_postgres_history_summary.go index aa7571fc3..275baed74 100644 --- a/internal/config/store_postgres_history_summary.go +++ b/internal/config/store_postgres_history_summary.go @@ -105,6 +105,10 @@ const executionSummaryRecsKey = `CASE WHEN jsonb_typeof(recommendations) = 'arra func executionSummaryQuery(filter HistorySummaryFilter) (query string, args []any) { where := historyExecutionsPredicate args = []any{filter.ExecutionStatuses} + if filter.History.PlanID != "" { + args = append(args, filter.History.PlanID) + where += fmt.Sprintf(" AND plan_id = $%d", len(args)) + } if filter.ExecutionStart != nil { args = append(args, *filter.ExecutionStart) where += fmt.Sprintf(" AND scheduled_date >= $%d", len(args)) diff --git a/internal/config/types.go b/internal/config/types.go index 06b931371..5cbcb2d8e 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -955,6 +955,9 @@ type RIUtilizationCacheEntry struct { type PurchaseHistoryFilter struct { // Provider matches purchase_history.provider exactly. Empty skips the clause. Provider string + // PlanID matches purchase_history.plan_id exactly; HistorySummaryAggregate + // also applies it to purchase_executions.plan_id. Empty skips the clause. + PlanID string // AccountIDs matches purchase_history.cloud_account_id (the cloud_accounts // UUID FK) with ANY($). Empty/nil skips this half of the account predicate. AccountIDs []string