diff --git a/frontend/src/__tests__/plans.test.ts b/frontend/src/__tests__/plans.test.ts index 4fe55a242..46eb8e880 100644 --- a/frontend/src/__tests__/plans.test.ts +++ b/frontend/src/__tests__/plans.test.ts @@ -327,6 +327,40 @@ describe('Plans Module', () => { } }); + test('passes account_ids to api.getPlans when account filter is active (issue #705)', async () => { + // Regression test for the Account global filter being non-functional + // on the Plans page. loadPlans must forward the account selection to + // api.getPlans so the backend can JOIN plan_accounts and prune the list. + const state = await import('../state'); + const accountID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa'; + (state.getCurrentAccountIDs as jest.Mock).mockReturnValue([accountID]); + + (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + try { + await loadPlans(); + + expect(api.getPlans).toHaveBeenCalledWith({ account_ids: [accountID] }); + } finally { + (state.getCurrentAccountIDs as jest.Mock).mockReturnValue([]); + } + }); + + test('calls api.getPlans with empty object when no account is selected', async () => { + // When no account chip is active, getPlans receives {} so the backend + // returns all plans (no account_ids filter applied). + const state = await import('../state'); + (state.getCurrentAccountIDs as jest.Mock).mockReturnValue([]); + + (api.getPlans as jest.Mock).mockResolvedValue({ plans: [] }); + (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); + + await loadPlans(); + + expect(api.getPlans).toHaveBeenCalledWith({}); + }); + test('shows error on API failure', async () => { (api.getPlans as jest.Mock).mockRejectedValue(new Error('API Error')); (api.getPlannedPurchases as jest.Mock).mockResolvedValue({ purchases: [] }); diff --git a/frontend/src/api/plans.ts b/frontend/src/api/plans.ts index 23f35a5ae..44d57fafe 100644 --- a/frontend/src/api/plans.ts +++ b/frontend/src/api/plans.ts @@ -3,13 +3,23 @@ */ import { apiRequest } from './client'; -import type { Plan, CreatePlanRequest } from './types'; +import type { Plan, CreatePlanRequest, PlanFilters } from './types'; /** - * Get purchase plans + * Get purchase plans, optionally filtered by account IDs. + * + * When filters.account_ids is non-empty the backend returns only plans + * that reference at least one of those accounts via the plan_accounts + * join table. Mirrors the account_ids filtering pattern in + * getRecommendations (see recommendations.ts). */ -export async function getPlans(): Promise { - return apiRequest('/plans'); +export async function getPlans(filters: PlanFilters = {}): Promise { + const params = new URLSearchParams(); + if (filters.account_ids && filters.account_ids.length > 0) { + params.set('account_ids', filters.account_ids.join(',')); + } + const queryString = params.toString(); + return apiRequest(`/plans${queryString ? '?' + queryString : ''}`); } /** diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index 9c92c8dae..10c1ac8e0 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -132,6 +132,11 @@ export interface RecommendationFilters { account_ids?: string[]; } +// PlanFilters are the query parameters accepted by the GET /api/plans endpoint. +export interface PlanFilters { + account_ids?: string[]; +} + // Plan types export interface PlanRampSchedule { type: string; diff --git a/frontend/src/plans.ts b/frontend/src/plans.ts index e8c40e9c1..220706a94 100644 --- a/frontend/src/plans.ts +++ b/frontend/src/plans.ts @@ -46,7 +46,15 @@ export async function loadPlans(): Promise { if (newPlanBtn) newPlanBtn.hidden = !canAccess('create', 'plans'); try { - const data = await api.getPlans() as unknown as PlansResponse; + // Account filter: pass account_ids to the backend so it JOINs + // plan_accounts and returns only plans that reference one of the + // selected accounts. Empty array means "all plans" — the backend + // omits the JOIN entirely in that case. Mirrors the pattern used by + // getRecommendations (see recommendations.ts, issue #705). + const accountIDs = state.getCurrentAccountIDs(); + const data = await api.getPlans( + accountIDs.length > 0 ? { account_ids: accountIDs } : {} + ) as unknown as PlansResponse; let plans = data.plans || []; // Client-side provider filter. Backend `config.PurchasePlan` has no diff --git a/internal/analytics/collector_test.go b/internal/analytics/collector_test.go index ce12bd0a1..10d6082dd 100644 --- a/internal/analytics/collector_test.go +++ b/internal/analytics/collector_test.go @@ -153,7 +153,7 @@ func (m *mockConfigStore) DeletePurchasePlan(ctx context.Context, planID string) return nil } -func (m *mockConfigStore) ListPurchasePlans(ctx context.Context) ([]config.PurchasePlan, error) { +func (m *mockConfigStore) ListPurchasePlans(ctx context.Context, filter config.PurchasePlanFilter) ([]config.PurchasePlan, error) { return nil, nil } diff --git a/internal/api/handler_coverage_test.go b/internal/api/handler_coverage_test.go index 83cfd7346..ed74fde32 100644 --- a/internal/api/handler_coverage_test.go +++ b/internal/api/handler_coverage_test.go @@ -628,7 +628,7 @@ func TestHandler_listPlans_Error(t *testing.T) { adminSession := &Session{UserID: "admin-id", Role: "admin"} mockAuth.On("ValidateSession", ctx, "test-token").Return(adminSession, nil) - mockStore.On("ListPurchasePlans", mock.Anything).Return(nil, errors.New("db error")) + mockStore.On("ListPurchasePlans", mock.Anything, mock.Anything).Return(nil, errors.New("db error")) handler := &Handler{config: mockStore, auth: mockAuth} @@ -636,7 +636,7 @@ func TestHandler_listPlans_Error(t *testing.T) { Headers: map[string]string{"Authorization": "Bearer test-token"}, } - _, err := handler.listPlans(ctx, req) + _, err := handler.listPlans(ctx, req, map[string]string{}) assert.Error(t, err) } diff --git a/internal/api/handler_dashboard.go b/internal/api/handler_dashboard.go index 470b9ccf6..8e0ae7aeb 100644 --- a/internal/api/handler_dashboard.go +++ b/internal/api/handler_dashboard.go @@ -242,7 +242,7 @@ func (h *Handler) getUpcomingPurchases(ctx context.Context, req *events.LambdaFu return nil, fmt.Errorf("failed to get pending executions: %w", err) } - plans, err := h.config.ListPurchasePlans(ctx) + plans, err := h.config.ListPurchasePlans(ctx, config.PurchasePlanFilter{}) if err != nil { return nil, fmt.Errorf("failed to get purchase plans: %w", err) } diff --git a/internal/api/handler_dashboard_test.go b/internal/api/handler_dashboard_test.go index a7fe1b31e..53551a98d 100644 --- a/internal/api/handler_dashboard_test.go +++ b/internal/api/handler_dashboard_test.go @@ -306,7 +306,7 @@ func TestHandler_getUpcomingPurchases(t *testing.T) { } mockStore.On("GetPendingExecutions", ctx).Return(pending, nil) - mockStore.On("ListPurchasePlans", ctx).Return([]config.PurchasePlan{planA, planB}, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{planA, planB}, nil) mockAuth, req := adminDashboardReq(ctx) handler := &Handler{auth: mockAuth, config: mockStore} @@ -350,7 +350,7 @@ func TestHandler_getUpcomingPurchases_OrphanExecutionSkipped(t *testing.T) { }, } mockStore.On("GetPendingExecutions", ctx).Return(pending, nil) - mockStore.On("ListPurchasePlans", ctx).Return([]config.PurchasePlan{}, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{}, nil) mockAuth, req := adminDashboardReq(ctx) handler := &Handler{auth: mockAuth, config: mockStore} @@ -403,7 +403,7 @@ func TestHandler_getUpcomingPurchases_ScopedUser(t *testing.T) { RampSchedule: config.RampSchedule{CurrentStep: 0, TotalSteps: 5}, } - mockStore.On("ListPurchasePlans", ctx).Return([]config.PurchasePlan{planA, planB}, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{planA, planB}, nil) mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{ {ExecutionID: "exec-A", PlanID: planA.ID, Status: "pending", ScheduledDate: nextExecDate, StepNumber: 1}, {ExecutionID: "exec-B", PlanID: planB.ID, Status: "pending", ScheduledDate: nextExecDate, StepNumber: 1}, @@ -453,7 +453,7 @@ func TestHandler_getUpcomingPurchases_ScopedUser_SkipsUnattributed(t *testing.T) NextExecutionDate: &nextExecDate, RampSchedule: config.RampSchedule{CurrentStep: 0, TotalSteps: 5}, } - mockStore.On("ListPurchasePlans", ctx).Return([]config.PurchasePlan{plan}, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{plan}, nil) mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{ {ExecutionID: "exec-unattributed", PlanID: plan.ID, Status: "pending", ScheduledDate: nextExecDate, StepNumber: 1}, }, nil) @@ -793,7 +793,7 @@ func TestHandler_getUpcomingPurchases_Errors(t *testing.T) { t.Run("list plans error", func(t *testing.T) { mockStore := new(MockConfigStore) mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{}, nil) - mockStore.On("ListPurchasePlans", ctx).Return(nil, errors.New("db error")) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(nil, errors.New("db error")) mockAuth, req := adminDashboardReq(ctx) handler := &Handler{auth: mockAuth, config: mockStore} @@ -806,7 +806,7 @@ func TestHandler_getUpcomingPurchases_Errors(t *testing.T) { t.Run("no pending executions yields empty list", func(t *testing.T) { mockStore := new(MockConfigStore) mockStore.On("GetPendingExecutions", ctx).Return([]config.PurchaseExecution{}, nil) - mockStore.On("ListPurchasePlans", ctx).Return([]config.PurchasePlan{}, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{}, nil) mockAuth, req := adminDashboardReq(ctx) handler := &Handler{auth: mockAuth, config: mockStore} diff --git a/internal/api/handler_plans.go b/internal/api/handler_plans.go index 6e01984b7..6e07ba464 100644 --- a/internal/api/handler_plans.go +++ b/internal/api/handler_plans.go @@ -15,13 +15,21 @@ import ( ) // Plans handlers -func (h *Handler) listPlans(ctx context.Context, req *events.LambdaFunctionURLRequest) (*PlansResponse, error) { +func (h *Handler) listPlans(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (*PlansResponse, error) { // Require view:plans permission if _, err := h.requirePermission(ctx, req, "view", "plans"); err != nil { return nil, err } - plans, err := h.config.ListPurchasePlans(ctx) + // parseAccountIDs validates and splits the comma-separated account_ids + // query param. Returns nil (no filter) when absent or empty. + accountIDs, err := parseAccountIDs(params["account_ids"]) + if err != nil { + return nil, NewClientError(400, err.Error()) + } + + filter := config.PurchasePlanFilter{AccountIDs: accountIDs} + plans, err := h.config.ListPurchasePlans(ctx, filter) if err != nil { return nil, err } diff --git a/internal/api/handler_plans_test.go b/internal/api/handler_plans_test.go index 617d04ef0..7cd1f5877 100644 --- a/internal/api/handler_plans_test.go +++ b/internal/api/handler_plans_test.go @@ -31,7 +31,7 @@ func TestHandler_listPlans(t *testing.T) { } mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) handler := &Handler{config: mockStore, auth: mockAuth} @@ -40,12 +40,48 @@ func TestHandler_listPlans(t *testing.T) { "Authorization": "Bearer admin-token", }, } - result, err := handler.listPlans(ctx, req) + result, err := handler.listPlans(ctx, req, map[string]string{}) require.NoError(t, err) assert.Len(t, result.Plans, 2) } +func TestHandler_listPlans_AccountIDsFilter(t *testing.T) { + ctx := context.Background() + mockStore := new(MockConfigStore) + mockAuth := new(MockAuthService) + + adminSession := &Session{ + UserID: "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa", + Email: "admin@example.com", + Role: "admin", + } + + plans := []config.PurchasePlan{ + {ID: "11111111-1111-1111-1111-111111111111", Name: "Account Plan", Enabled: true}, + } + + accountID := "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" + expectedFilter := config.PurchasePlanFilter{AccountIDs: []string{accountID}} + + mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) + mockStore.On("ListPurchasePlans", ctx, expectedFilter).Return(plans, nil) + + handler := &Handler{config: mockStore, auth: mockAuth} + + req := &events.LambdaFunctionURLRequest{ + Headers: map[string]string{ + "Authorization": "Bearer admin-token", + }, + } + params := map[string]string{"account_ids": accountID} + result, err := handler.listPlans(ctx, req, params) + require.NoError(t, err) + + assert.Len(t, result.Plans, 1) + assert.Equal(t, "Account Plan", result.Plans[0].Name) +} + func TestHandler_createPlan(t *testing.T) { ctx := context.Background() mockStore := new(MockConfigStore) diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go index c00644a6f..0c82e5af4 100644 --- a/internal/api/handler_purchases.go +++ b/internal/api/handler_purchases.go @@ -95,7 +95,7 @@ func (h *Handler) getPlannedPurchases(ctx context.Context, req *events.LambdaFun return nil, fmt.Errorf("failed to get pending executions: %w", err) } - plans, err := h.config.ListPurchasePlans(ctx) + plans, err := h.config.ListPurchasePlans(ctx, config.PurchasePlanFilter{}) if err != nil { return nil, fmt.Errorf("failed to get purchase plans: %w", err) } diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go index 111c483b4..5248d7025 100644 --- a/internal/api/handler_purchases_test.go +++ b/internal/api/handler_purchases_test.go @@ -752,7 +752,7 @@ func TestHandler_getPlannedPurchases(t *testing.T) { mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) mockStore.On("GetPendingExecutions", ctx).Return(executions, nil) - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) handler := &Handler{config: mockStore, auth: mockAuth} @@ -1069,7 +1069,7 @@ func TestHandler_getPlannedPurchases_ErrorGettingPlans(t *testing.T) { mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil) mockStore.On("GetPendingExecutions", ctx).Return(executions, nil) - mockStore.On("ListPurchasePlans", ctx).Return(nil, errors.New("database error")) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(nil, errors.New("database error")) handler := &Handler{config: mockStore, auth: mockAuth} diff --git a/internal/api/handler_test.go b/internal/api/handler_test.go index d5aa51756..5f8e3b3f0 100644 --- a/internal/api/handler_test.go +++ b/internal/api/handler_test.go @@ -544,7 +544,7 @@ func TestHandler_HandleRequest_ListPlans(t *testing.T) { mockAuth.On("ValidateSession", ctx, "test-token").Return(adminSession, nil) plans := []config.PurchasePlan{{ID: "11111111-1111-1111-1111-111111111111"}} - mockStore.On("ListPurchasePlans", mock.Anything).Return(plans, nil) + mockStore.On("ListPurchasePlans", mock.Anything, mock.Anything).Return(plans, nil) handler := &Handler{config: mockStore, auth: mockAuth, apiKey: "test-key"} @@ -953,7 +953,7 @@ func TestHandler_HandleRequest_GetUpcomingPurchases(t *testing.T) { }, } - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) // New: handler now enumerates pending executions per PR #213. Fixture // supplies one pending exec for the plan above so the integration test // still observes a single upcoming row. @@ -1012,7 +1012,7 @@ func TestHandler_HandleRequest_GetPlannedPurchases(t *testing.T) { } mockStore.On("GetPendingExecutions", ctx).Return(executions, nil) - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) handler := &Handler{config: mockStore, auth: mockAuth, corsAllowedOrigin: "*", apiKey: "test-key"} @@ -1289,7 +1289,7 @@ func TestHandler_HandleRequest_ListPlans_Error(t *testing.T) { adminSession := &Session{UserID: "admin-id", Email: "admin@example.com", Role: "admin"} mockAuth.On("ValidateSession", ctx, "test-token").Return(adminSession, nil) - mockStore.On("ListPurchasePlans", mock.Anything).Return(nil, assert.AnError) + mockStore.On("ListPurchasePlans", mock.Anything, mock.Anything).Return(nil, assert.AnError) handler := &Handler{config: mockStore, auth: mockAuth, apiKey: "test-key"} diff --git a/internal/api/mocks_test.go b/internal/api/mocks_test.go index e1edcf3d3..411177862 100644 --- a/internal/api/mocks_test.go +++ b/internal/api/mocks_test.go @@ -138,8 +138,8 @@ func (m *MockConfigStore) DeletePurchasePlan(ctx context.Context, planID string) return args.Error(0) } -func (m *MockConfigStore) ListPurchasePlans(ctx context.Context) ([]config.PurchasePlan, error) { - args := m.Called(ctx) +func (m *MockConfigStore) ListPurchasePlans(ctx context.Context, filter config.PurchasePlanFilter) ([]config.PurchasePlan, error) { + args := m.Called(ctx, filter) if args.Get(0) == nil { return nil, args.Error(1) } diff --git a/internal/api/router.go b/internal/api/router.go index 5072b5377..c770b941b 100644 --- a/internal/api/router.go +++ b/internal/api/router.go @@ -432,7 +432,7 @@ func (r *Router) getRecommendationDetailHandler(ctx context.Context, req *events } func (r *Router) listPlansHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { - return r.h.listPlans(ctx, req) + return r.h.listPlans(ctx, req, req.QueryStringParameters) } func (r *Router) createPlanHandler(ctx context.Context, req *events.LambdaFunctionURLRequest, params map[string]string) (any, error) { diff --git a/internal/config/interfaces.go b/internal/config/interfaces.go index ac3cc2931..ea01e0b76 100644 --- a/internal/config/interfaces.go +++ b/internal/config/interfaces.go @@ -29,7 +29,7 @@ type StoreInterface interface { // rows and no stale plan pointer. UpdatePurchasePlanTx(ctx context.Context, tx pgx.Tx, plan *PurchasePlan) error DeletePurchasePlan(ctx context.Context, planID string) error - ListPurchasePlans(ctx context.Context) ([]PurchasePlan, error) + ListPurchasePlans(ctx context.Context, filter PurchasePlanFilter) ([]PurchasePlan, error) // Purchase executions SavePurchaseExecution(ctx context.Context, execution *PurchaseExecution) error diff --git a/internal/config/store_postgres.go b/internal/config/store_postgres.go index 869f62658..0f1a7f757 100644 --- a/internal/config/store_postgres.go +++ b/internal/config/store_postgres.go @@ -583,17 +583,44 @@ func (s *PostgresStore) DeletePurchasePlan(ctx context.Context, planID string) e return nil } -// ListPurchasePlans lists all purchase plans -func (s *PostgresStore) ListPurchasePlans(ctx context.Context) ([]PurchasePlan, error) { - query := ` - SELECT id, name, enabled, auto_purchase, notification_days_before, - services, ramp_schedule, created_at, updated_at, - next_execution_date, last_execution_date, last_notification_sent - FROM purchase_plans - ORDER BY created_at DESC - ` - - rows, err := s.db.Query(ctx, query) +// buildListPlansQuery returns the SQL query and args for ListPurchasePlans. +// When accountIDs is non-empty the query JOINs plan_accounts and filters +// on account_id IN ($1, $2, …) using parameterised placeholders so the +// result is bounded to plans that reference at least one of the given accounts. +func buildListPlansQuery(accountIDs []string) (query string, args []any) { + if len(accountIDs) == 0 { + return ` + SELECT id, name, enabled, auto_purchase, notification_days_before, + services, ramp_schedule, created_at, updated_at, + next_execution_date, last_execution_date, last_notification_sent + FROM purchase_plans + ORDER BY created_at DESC + `, nil + } + placeholders := make([]string, len(accountIDs)) + args = make([]any, len(accountIDs)) + for i, id := range accountIDs { + placeholders[i] = fmt.Sprintf("$%d", i+1) + args[i] = id + } + query = fmt.Sprintf(` + SELECT DISTINCT pp.id, pp.name, pp.enabled, pp.auto_purchase, pp.notification_days_before, + pp.services, pp.ramp_schedule, pp.created_at, pp.updated_at, + pp.next_execution_date, pp.last_execution_date, pp.last_notification_sent + FROM purchase_plans pp + JOIN plan_accounts pa ON pa.plan_id = pp.id + WHERE pa.account_id IN (%s) + ORDER BY pp.created_at DESC + `, strings.Join(placeholders, ", ")) + return query, args +} + +// ListPurchasePlans lists purchase plans, optionally filtered by account IDs. +// When filter.AccountIDs is non-empty the result is limited to plans that +// reference at least one of those accounts via the plan_accounts join table. +func (s *PostgresStore) ListPurchasePlans(ctx context.Context, filter PurchasePlanFilter) ([]PurchasePlan, error) { + query, args := buildListPlansQuery(filter.AccountIDs) + rows, err := s.db.Query(ctx, query, args...) if err != nil { return nil, fmt.Errorf("failed to list purchase plans: %w", err) } diff --git a/internal/config/store_postgres_additional_test.go b/internal/config/store_postgres_additional_test.go index 88cfce314..d02ac6008 100644 --- a/internal/config/store_postgres_additional_test.go +++ b/internal/config/store_postgres_additional_test.go @@ -522,7 +522,7 @@ func TestListPurchasePlans_ScanError(t *testing.T) { mock.ExpectQuery(`SELECT id, name, enabled, auto_purchase, notification_days_before`). WillReturnRows(rows) - plans, err := store.ListPurchasePlans(context.Background()) + plans, err := store.ListPurchasePlans(context.Background(), PurchasePlanFilter{}) assert.Error(t, err) assert.Nil(t, plans) diff --git a/internal/config/store_postgres_comprehensive_test.go b/internal/config/store_postgres_comprehensive_test.go index 8f5946b7d..de8d03341 100644 --- a/internal/config/store_postgres_comprehensive_test.go +++ b/internal/config/store_postgres_comprehensive_test.go @@ -125,7 +125,7 @@ func (s *mockablePostgresStore) UpdatePurchasePlan(ctx context.Context, plan *Pu return nil } -func (s *mockablePostgresStore) ListPurchasePlans(ctx context.Context) ([]PurchasePlan, error) { +func (s *mockablePostgresStore) ListPurchasePlans(ctx context.Context, filter PurchasePlanFilter) ([]PurchasePlan, error) { query := ` SELECT id, name, enabled, auto_purchase, notification_days_before, services, ramp_schedule, created_at, updated_at, @@ -748,7 +748,7 @@ func TestListPurchasePlans_Success(t *testing.T) { mock.ExpectQuery(`SELECT id, name, enabled, auto_purchase, notification_days_before`). WillReturnRows(rows) - plans, err := store.ListPurchasePlans(context.Background()) + plans, err := store.ListPurchasePlans(context.Background(), PurchasePlanFilter{}) require.NoError(t, err) assert.Len(t, plans, 2) @@ -782,7 +782,7 @@ func TestListPurchasePlans_Empty(t *testing.T) { mock.ExpectQuery(`SELECT id, name, enabled, auto_purchase, notification_days_before`). WillReturnRows(rows) - plans, err := store.ListPurchasePlans(context.Background()) + plans, err := store.ListPurchasePlans(context.Background(), PurchasePlanFilter{}) require.NoError(t, err) assert.NotNil(t, plans) assert.Empty(t, plans) @@ -800,7 +800,7 @@ func TestListPurchasePlans_QueryError(t *testing.T) { mock.ExpectQuery(`SELECT id, name, enabled, auto_purchase, notification_days_before`). WillReturnError(errors.New("table not found")) - plans, err := store.ListPurchasePlans(context.Background()) + plans, err := store.ListPurchasePlans(context.Background(), PurchasePlanFilter{}) assert.Error(t, err) assert.Nil(t, plans) assert.Contains(t, err.Error(), "table not found") @@ -829,7 +829,7 @@ func TestListPurchasePlans_InvalidServicesJSON(t *testing.T) { mock.ExpectQuery(`SELECT id, name, enabled, auto_purchase, notification_days_before`). WillReturnRows(rows) - plans, err := store.ListPurchasePlans(context.Background()) + plans, err := store.ListPurchasePlans(context.Background(), PurchasePlanFilter{}) assert.Error(t, err) assert.Nil(t, plans) @@ -857,7 +857,7 @@ func TestListPurchasePlans_InvalidRampScheduleJSON(t *testing.T) { mock.ExpectQuery(`SELECT id, name, enabled, auto_purchase, notification_days_before`). WillReturnRows(rows) - plans, err := store.ListPurchasePlans(context.Background()) + plans, err := store.ListPurchasePlans(context.Background(), PurchasePlanFilter{}) assert.Error(t, err) assert.Nil(t, plans) @@ -1925,7 +1925,7 @@ func TestListPurchasePlans_RowsError(t *testing.T) { mock.ExpectQuery(`SELECT id, name, enabled, auto_purchase, notification_days_before`). WillReturnRows(rows) - plans, err := store.ListPurchasePlans(context.Background()) + plans, err := store.ListPurchasePlans(context.Background(), PurchasePlanFilter{}) assert.Error(t, err) assert.Nil(t, plans) diff --git a/internal/config/store_postgres_coverage_test.go b/internal/config/store_postgres_coverage_test.go index 0311c98f1..150f448ee 100644 --- a/internal/config/store_postgres_coverage_test.go +++ b/internal/config/store_postgres_coverage_test.go @@ -342,7 +342,7 @@ func TestPostgresStore_ListPurchasePlans_NilDB(t *testing.T) { ctx := context.Background() panicked := callWithRecover(func() { - _, _ = store.ListPurchasePlans(ctx) + _, _ = store.ListPurchasePlans(ctx, PurchasePlanFilter{}) }) assert.True(t, panicked, "expected panic with nil db connection") diff --git a/internal/config/store_postgres_db_test.go b/internal/config/store_postgres_db_test.go index 7a55e2628..ecf2ba8cc 100644 --- a/internal/config/store_postgres_db_test.go +++ b/internal/config/store_postgres_db_test.go @@ -474,7 +474,7 @@ func TestPostgresStoreDB_PurchasePlans(t *testing.T) { require.NoError(t, err) } - retrieved, err := store.ListPurchasePlans(ctx) + retrieved, err := store.ListPurchasePlans(ctx, PurchasePlanFilter{}) require.NoError(t, err) assert.Len(t, retrieved, 2) }) @@ -500,7 +500,7 @@ func TestPostgresStoreDB_PurchasePlans(t *testing.T) { err := store.CreatePurchasePlan(ctx, plan) require.NoError(t, err) - plans, err := store.ListPurchasePlans(ctx) + plans, err := store.ListPurchasePlans(ctx, PurchasePlanFilter{}) require.NoError(t, err) assert.Len(t, plans, 1) assert.NotNil(t, plans[0].NextExecutionDate) diff --git a/internal/config/store_postgres_pgxmock_test.go b/internal/config/store_postgres_pgxmock_test.go index 263935624..7ec33b8a7 100644 --- a/internal/config/store_postgres_pgxmock_test.go +++ b/internal/config/store_postgres_pgxmock_test.go @@ -303,7 +303,7 @@ func TestPGXMock_ListPurchasePlans_Success(t *testing.T) { sql.NullTime{}, sql.NullTime{}, sql.NullTime{}) mock.ExpectQuery("SELECT").WillReturnRows(rows) - plans, err := store.ListPurchasePlans(ctx) + plans, err := store.ListPurchasePlans(ctx, PurchasePlanFilter{}) require.NoError(t, err) assert.Len(t, plans, 2) assert.NoError(t, mock.ExpectationsWereMet()) @@ -316,7 +316,7 @@ func TestPGXMock_ListPurchasePlans_Error(t *testing.T) { mock.ExpectQuery("SELECT").WillReturnError(errors.New("db error")) - _, err := store.ListPurchasePlans(ctx) + _, err := store.ListPurchasePlans(ctx, PurchasePlanFilter{}) require.Error(t, err) } diff --git a/internal/config/store_postgres_test.go b/internal/config/store_postgres_test.go index 16c2b64df..cd02e0cf7 100644 --- a/internal/config/store_postgres_test.go +++ b/internal/config/store_postgres_test.go @@ -265,7 +265,7 @@ func TestPostgresStore_PurchasePlans(t *testing.T) { } // List all plans - retrieved, err := store.ListPurchasePlans(ctx) + retrieved, err := store.ListPurchasePlans(ctx, PurchasePlanFilter{}) require.NoError(t, err) assert.GreaterOrEqual(t, len(retrieved), 2) }) diff --git a/internal/config/types.go b/internal/config/types.go index 7b10a1c64..b921a4b72 100644 --- a/internal/config/types.go +++ b/internal/config/types.go @@ -392,6 +392,14 @@ type RecommendationFilter struct { MinSavings float64 // 0 = no floor on monthly savings } +// PurchasePlanFilter parameterises ListPurchasePlans. Zero-value means "no +// filter" (all plans are returned). Non-empty AccountIDs restricts the result +// to plans that reference at least one of the given account IDs via the +// plan_accounts join table. +type PurchasePlanFilter struct { + AccountIDs []string // nil/empty = all plans +} + // RecommendationsFreshness describes the cache staleness state surfaced to // the frontend. LastCollectedAt is nil on a cold start. // LastCollectionError is non-nil when the most recent collect attempt diff --git a/internal/mocks/stores.go b/internal/mocks/stores.go index 89c531ea9..0413d472a 100644 --- a/internal/mocks/stores.go +++ b/internal/mocks/stores.go @@ -94,8 +94,8 @@ func (m *MockConfigStore) DeletePurchasePlan(ctx context.Context, planID string) } // ListPurchasePlans mocks the ListPurchasePlans operation -func (m *MockConfigStore) ListPurchasePlans(ctx context.Context) ([]config.PurchasePlan, error) { - args := m.Called(ctx) +func (m *MockConfigStore) ListPurchasePlans(ctx context.Context, filter config.PurchasePlanFilter) ([]config.PurchasePlan, error) { + args := m.Called(ctx, filter) if args.Get(0) == nil { return nil, args.Error(1) } diff --git a/internal/purchase/messages_test.go b/internal/purchase/messages_test.go index 146733039..3af452b08 100644 --- a/internal/purchase/messages_test.go +++ b/internal/purchase/messages_test.go @@ -83,7 +83,7 @@ func TestManager_ProcessMessage(t *testing.T) { email: mockEmail, dashboardURL: "https://dashboard.example.com", } - mockStore.On("ListPurchasePlans", ctx).Return([]config.PurchasePlan{}, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{}, nil) err := manager.ProcessMessage(ctx, `{"type": "send_notification"}`) assert.NoError(t, err) diff --git a/internal/purchase/mocks_test.go b/internal/purchase/mocks_test.go index 8b0199876..fd88ae2e3 100644 --- a/internal/purchase/mocks_test.go +++ b/internal/purchase/mocks_test.go @@ -233,8 +233,8 @@ func (m *MockConfigStore) DeletePurchasePlan(ctx context.Context, planID string) return args.Error(0) } -func (m *MockConfigStore) ListPurchasePlans(ctx context.Context) ([]config.PurchasePlan, error) { - args := m.Called(ctx) +func (m *MockConfigStore) ListPurchasePlans(ctx context.Context, filter config.PurchasePlanFilter) ([]config.PurchasePlan, error) { + args := m.Called(ctx, filter) if args.Get(0) == nil { return nil, args.Error(1) } diff --git a/internal/purchase/notifications.go b/internal/purchase/notifications.go index 63775d985..2b25b5ed0 100644 --- a/internal/purchase/notifications.go +++ b/internal/purchase/notifications.go @@ -16,7 +16,7 @@ import ( func (m *Manager) SendUpcomingPurchaseNotifications(ctx context.Context) (*NotificationResult, error) { logging.Info("Checking for upcoming purchases to notify...") - plans, err := m.config.ListPurchasePlans(ctx) + plans, err := m.config.ListPurchasePlans(ctx, config.PurchasePlanFilter{}) if err != nil { return nil, fmt.Errorf("failed to list purchase plans: %w", err) } diff --git a/internal/purchase/notifications_test.go b/internal/purchase/notifications_test.go index 6918d7a2e..57acc2fce 100644 --- a/internal/purchase/notifications_test.go +++ b/internal/purchase/notifications_test.go @@ -17,7 +17,7 @@ func TestManager_SendUpcomingPurchaseNotifications_NoPlans(t *testing.T) { mockStore := new(MockConfigStore) mockEmail := new(MockEmailSender) - mockStore.On("ListPurchasePlans", ctx).Return([]config.PurchasePlan{}, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{}, nil) manager := &Manager{ config: mockStore, @@ -48,7 +48,7 @@ func TestManager_SendUpcomingPurchaseNotifications_DisabledPlan(t *testing.T) { }, } - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) manager := &Manager{ config: mockStore, @@ -79,7 +79,7 @@ func TestManager_SendUpcomingPurchaseNotifications_NotAutoPurchase(t *testing.T) }, } - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) manager := &Manager{ config: mockStore, @@ -101,7 +101,7 @@ func TestManager_SendUpcomingPurchaseNotifications_Error(t *testing.T) { mockStore := new(MockConfigStore) mockEmail := new(MockEmailSender) - mockStore.On("ListPurchasePlans", ctx).Return(nil, errors.New("database error")) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(nil, errors.New("database error")) manager := &Manager{ config: mockStore, @@ -314,7 +314,7 @@ func TestManager_SendUpcomingPurchaseNotifications_WithNotification(t *testing.T }, } - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) // No existing execution found mockStore.On("GetExecutionByPlanAndDate", ctx, "plan-123", nextExec).Return(nil, nil) mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) @@ -354,7 +354,7 @@ func TestManager_SendUpcomingPurchaseNotifications_TooFarAway(t *testing.T) { }, } - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) manager := &Manager{ config: mockStore, @@ -390,7 +390,7 @@ func TestManager_SendUpcomingPurchaseNotifications_RecentNotification(t *testing }, } - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) manager := &Manager{ config: mockStore, @@ -423,7 +423,7 @@ func TestManager_SendUpcomingPurchaseNotifications_NoNextExecutionDate(t *testin }, } - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) manager := &Manager{ config: mockStore, @@ -458,7 +458,7 @@ func TestManager_SendUpcomingPurchaseNotifications_EmailFails(t *testing.T) { }, } - mockStore.On("ListPurchasePlans", ctx).Return(plans, nil) + mockStore.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return(plans, nil) // No existing execution found mockStore.On("GetExecutionByPlanAndDate", ctx, "plan-123", nextExec).Return(nil, nil) mockStore.On("SavePurchaseExecution", ctx, mock.AnythingOfType("*config.PurchaseExecution")).Return(nil) diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index afb7b03bd..05dc5983f 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -111,8 +111,8 @@ func (m *MockConfigStore) DeletePurchasePlan(ctx context.Context, planID string) return args.Error(0) } -func (m *MockConfigStore) ListPurchasePlans(ctx context.Context) ([]config.PurchasePlan, error) { - args := m.Called(ctx) +func (m *MockConfigStore) ListPurchasePlans(ctx context.Context, filter config.PurchasePlanFilter) ([]config.PurchasePlan, error) { + args := m.Called(ctx, filter) if args.Get(0) == nil { return nil, args.Error(1) } @@ -980,11 +980,11 @@ func TestSchedulerConfigStoreInterface(t *testing.T) { // These calls just verify the mock has the methods store.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil) store.On("ListServiceConfigs", ctx).Return([]config.ServiceConfig{}, nil) - store.On("ListPurchasePlans", ctx).Return([]config.PurchasePlan{}, nil) + store.On("ListPurchasePlans", ctx, config.PurchasePlanFilter{}).Return([]config.PurchasePlan{}, nil) _, _ = store.GetGlobalConfig(ctx) _, _ = store.ListServiceConfigs(ctx) - _, _ = store.ListPurchasePlans(ctx) + _, _ = store.ListPurchasePlans(ctx, config.PurchasePlanFilter{}) store.AssertExpectations(t) } diff --git a/internal/server/test_helpers_test.go b/internal/server/test_helpers_test.go index 033e27790..61d93120b 100644 --- a/internal/server/test_helpers_test.go +++ b/internal/server/test_helpers_test.go @@ -55,7 +55,7 @@ func (m *mockConfigStoreForHealth) DeletePurchasePlan(ctx context.Context, planI return nil } -func (m *mockConfigStoreForHealth) ListPurchasePlans(ctx context.Context) ([]config.PurchasePlan, error) { +func (m *mockConfigStoreForHealth) ListPurchasePlans(ctx context.Context, filter config.PurchasePlanFilter) ([]config.PurchasePlan, error) { return nil, nil }