fix(api): filter purchases by account UUID AND external id (closes #701, #498, #866)

.gitallowed

@@ -36,6 +36,9 @@
 111222333444
 555666777888
 999888777666
+# Descending-step fixture added in #956 tests (store_postgres_pgxmock_test.go,
+# handler_analytics_test.go, handler_history_test.go, handler_dashboard_test.go).
+999988887777
 
 # UUID-shaped account ID used in handler_accounts_test.go (synthetic, not a
 # real subscription/account).

internal/analytics/collector_test.go

@@ -200,7 +200,7 @@ 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) {
+func (m *mockConfigStore) GetPurchaseHistoryFiltered(ctx context.Context, filter config.PurchaseHistoryFilter) ([]config.PurchaseHistoryRecord, error) {
 	return nil, nil
 }
 

internal/api/analytics_postgres.go

@@ -4,6 +4,7 @@ package api
 import (
 	"context"
 	"fmt"
+	"strings"
 	"time"
 
 	"github.com/LeanerCloud/CUDly/internal/database"
@@ -74,13 +75,44 @@ func dimensionToColumn(dimension string) (string, error) {
 	}
 }
 
+// accountFilterClause builds the dual-column account WHERE fragment and the
+// full positional arg list for the analytics aggregate queries. baseArgs holds
+// the fixed leading binds ($1=start, $2=end); the account array binds (if any)
+// are appended after them and the returned clause references them by the right
+// positions.
+//
+// purchase_history carries two account identifiers, either of which may be the
+// only one populated on a row: account_id (the cloud-provider external number,
+// always populated) and cloud_account_id (the cloud_accounts UUID FK, NULL on
+// every direct-execute/ambient/pre-000011 row). The top-bar chip emits the
+// UUID, so matching only cloud_account_id silently dropped every NULL row
+// (issue #701/#498) and matching only account_id dropped UUID-only rows (#866).
+// We bind the UUIDs and their resolved external numbers separately and OR the
+// two ANY() predicates. Both empty -> "TRUE" (no account filter).
+func accountFilterClause(accountUUIDs, accountExternalIDs []string, baseArgs []any) (clause string, args []any) {
+	args = baseArgs
+	var ors []string
+	if len(accountUUIDs) > 0 {
+		args = append(args, accountUUIDs)
+		ors = append(ors, fmt.Sprintf("cloud_account_id::text = ANY($%d)", len(args)))
+	}
+	if len(accountExternalIDs) > 0 {
+		args = append(args, accountExternalIDs)
+		ors = append(ors, fmt.Sprintf("account_id = ANY($%d)", len(args)))
+	}
+	if len(ors) == 0 {
+		return "TRUE", args
+	}
+	return "(" + strings.Join(ors, " OR ") + ")", args
+}
+
 // QueryHistory aggregates purchase_history rows bucketed by interval.
-// accountID == "" means "all accounts accessible to the caller"; scoping
-// is enforced upstream in the handler. Returns data points in ascending
-// order and a summary covering the full window.
+// Empty accountUUIDs AND accountExternalIDs means "all accounts accessible to
+// the caller"; scoping is enforced upstream in the handler. Returns data points
+// in ascending order and a summary covering the full window.
 func (c *PostgresAnalyticsClient) QueryHistory(
 	ctx context.Context,
-	accountID string,
+	accountUUIDs, accountExternalIDs []string,
 	start, end time.Time,
 	interval string,
 ) ([]HistoryDataPoint, *HistorySummary, error) {
@@ -91,16 +123,12 @@ func (c *PostgresAnalyticsClient) QueryHistory(
 
 	// Single query grouped by (bucket, service, provider) so we can assemble
 	// both the top-line bucket totals and the by_service / by_provider
-	// breakdowns without a second trip to the DB.
-	//
-	// account_id is the legacy VARCHAR(20) external-account identifier
-	// (e.g. a 12-digit AWS account number) written by older code paths.
-	// cloud_account_id is the UUID FK to cloud_accounts added in migration
-	// 000011 and written by all current purchase flows. The topbar Account
-	// chip sends the cloud_accounts.id UUID, so we match BOTH columns so
-	// that rows written by either path are included (issue #701).
+	// breakdowns without a second trip to the DB. The account predicate is the
+	// shared dual-column clause (see accountFilterClause).
 	//
-	// #nosec G201 — `unit` is allowlisted by intervalToTruncUnit above.
+	// #nosec G201 — `unit` is allowlisted by intervalToTruncUnit above and the
+	// account clause is parameter-bound (no user input interpolated).
+	accountClause, args := accountFilterClause(accountUUIDs, accountExternalIDs, []any{start, end})
 	query := fmt.Sprintf(`
 		SELECT date_trunc('%s', timestamp) AS bucket,
 		       service,
@@ -111,12 +139,12 @@ func (c *PostgresAnalyticsClient) QueryHistory(
 		FROM purchase_history
 		WHERE timestamp >= $1
 		  AND timestamp <= $2
-		  AND ($3 = '' OR account_id = $3 OR cloud_account_id::text = $3)
+		  AND %s
 		GROUP BY bucket, service, provider
 		ORDER BY bucket ASC
-	`, unit)
+	`, unit, accountClause)
 
-	rows, err := c.db.Query(ctx, query, start, end, accountID)
+	rows, err := c.db.Query(ctx, query, args...)
 	if err != nil {
 		return nil, nil, fmt.Errorf("query purchase_history: %w", err)
 	}
@@ -184,7 +212,7 @@ func (c *PostgresAnalyticsClient) QueryHistory(
 // bucket.
 func (c *PostgresAnalyticsClient) QueryBreakdown(
 	ctx context.Context,
-	accountID string,
+	accountUUIDs, accountExternalIDs []string,
 	start, end time.Time,
 	dimension string,
 ) (map[string]BreakdownValue, error) {
@@ -193,10 +221,12 @@ func (c *PostgresAnalyticsClient) QueryBreakdown(
 		return nil, err
 	}
 
-	// account_id / cloud_account_id dual-column match: see QueryHistory
-	// comment for rationale (issue #701).
+	// Dual-column account predicate: see accountFilterClause / QueryHistory for
+	// rationale (issue #701/#498/#866).
 	//
-	// #nosec G201 — `column` is allowlisted by dimensionToColumn above.
+	// #nosec G201 — `column` is allowlisted by dimensionToColumn above and the
+	// account clause is parameter-bound (no user input interpolated).
+	accountClause, args := accountFilterClause(accountUUIDs, accountExternalIDs, []any{start, end})
 	query := fmt.Sprintf(`
 		SELECT %s AS bucket,
 		       SUM(estimated_savings)::float8 AS savings,
@@ -205,12 +235,12 @@ func (c *PostgresAnalyticsClient) QueryBreakdown(
 		FROM purchase_history
 		WHERE timestamp >= $1
 		  AND timestamp <= $2
-		  AND ($3 = '' OR account_id = $3 OR cloud_account_id::text = $3)
+		  AND %s
 		GROUP BY bucket
 		ORDER BY savings DESC
-	`, column)
+	`, column, accountClause)
 
-	rows, err := c.db.Query(ctx, query, start, end, accountID)
+	rows, err := c.db.Query(ctx, query, args...)
 	if err != nil {
 		return nil, fmt.Errorf("query breakdown: %w", err)
 	}

internal/api/analytics_postgres_test.go

@@ -64,13 +64,15 @@ func TestQueryHistory_Success(t *testing.T) {
 		AddRow(bucket1, "rds", "aws", 40.0, 10.0, 1).
 		AddRow(bucket2, "ec2", "aws", 75.0, 0.0, 1)
 
-	mock.ExpectQuery(`SELECT date_trunc\('day', timestamp\)`).
-		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), "acct-1").
+	// External-id-only filter: predicate is (account_id = ANY($3)), bound to
+	// the single external id. No cloud_account_id half since no UUIDs supplied.
+	mock.ExpectQuery(`(?s)SELECT date_trunc\('day', timestamp\).*AND \(account_id = ANY\(\$3\)\)`).
+		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), []string{"acct-1"}).
 		WillReturnRows(rows)
 
 	start := time.Date(2026, 4, 20, 0, 0, 0, 0, time.UTC)
 	end := time.Date(2026, 4, 24, 0, 0, 0, 0, time.UTC)
-	points, summary, err := client.QueryHistory(ctx, "acct-1", start, end, "daily")
+	points, summary, err := client.QueryHistory(ctx, nil, []string{"acct-1"}, start, end, "daily")
 	require.NoError(t, err)
 	require.Len(t, points, 2)
 
@@ -101,16 +103,17 @@ func TestQueryHistory_Success(t *testing.T) {
 
 func TestQueryHistory_BadInterval(t *testing.T) {
 	client, _ := newMockAnalyticsClient(t)
-	_, _, err := client.QueryHistory(context.Background(), "", time.Now(), time.Now(), "yearly")
+	_, _, err := client.QueryHistory(context.Background(), nil, nil, time.Now(), time.Now(), "yearly")
 	assert.Error(t, err)
 }
 
 func TestQueryHistory_QueryError(t *testing.T) {
 	client, mock := newMockAnalyticsClient(t)
+	// No account filter: predicate degrades to TRUE, only start/end bound.
 	mock.ExpectQuery(`SELECT date_trunc`).
-		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), "").
+		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
 		WillReturnError(errors.New("db down"))
-	_, _, err := client.QueryHistory(context.Background(), "", time.Now(), time.Now(), "daily")
+	_, _, err := client.QueryHistory(context.Background(), nil, nil, time.Now(), time.Now(), "daily")
 	assert.Error(t, err)
 	assert.NoError(t, mock.ExpectationsWereMet())
 }
@@ -122,10 +125,10 @@ func TestQueryBreakdown_Success(t *testing.T) {
 		AddRow("rds", 100.0, 50.0, 2)
 
 	mock.ExpectQuery(`SELECT service AS bucket`).
-		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), "").
+		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
 		WillReturnRows(rows)
 
-	out, err := client.QueryBreakdown(context.Background(), "",
+	out, err := client.QueryBreakdown(context.Background(), nil, nil,
 		time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC),
 		time.Date(2026, 12, 31, 0, 0, 0, 0, time.UTC),
 		"service")
@@ -146,7 +149,7 @@ func TestQueryBreakdown_Success(t *testing.T) {
 
 func TestQueryBreakdown_BadDimension(t *testing.T) {
 	client, _ := newMockAnalyticsClient(t)
-	_, err := client.QueryBreakdown(context.Background(), "", time.Now(), time.Now(), "team")
+	_, err := client.QueryBreakdown(context.Background(), nil, nil, time.Now(), time.Now(), "team")
 	assert.Error(t, err)
 }
 
@@ -156,36 +159,38 @@ func TestQueryBreakdown_ZeroTotalYieldsZeroPct(t *testing.T) {
 		AddRow("ec2", 0.0, 0.0, 0)
 
 	mock.ExpectQuery(`SELECT service AS bucket`).
-		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), "").
+		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg()).
 		WillReturnRows(rows)
 
-	out, err := client.QueryBreakdown(context.Background(), "", time.Now(), time.Now(), "service")
+	out, err := client.QueryBreakdown(context.Background(), nil, nil, time.Now(), time.Now(), "service")
 	require.NoError(t, err)
 	assert.Equal(t, 0.0, out["ec2"].Percentage, "percentage must be 0 when total savings is 0")
 	assert.NoError(t, mock.ExpectationsWereMet())
 }
 
-// TestQueryHistory_CloudAccountIDFilter verifies that QueryHistory accepts a
-// cloud_accounts UUID as the accountID parameter (issue #701). The WHERE
-// clause matches on BOTH account_id (legacy VARCHAR(20) external ID) and
-// cloud_account_id::text (UUID FK), so rows written by either code path are
-// included. pgxmock validates the SQL shape and arg binding.
-func TestQueryHistory_CloudAccountIDFilter(t *testing.T) {
+// TestQueryHistory_DualColumnFilter verifies the dual-column account predicate
+// (issue #701/#498/#866): when a UUID and its resolved external id are both
+// supplied, the WHERE clause ORs cloud_account_id::text = ANY($3) with
+// account_id = ANY($4), so a row that carries only one of the two account
+// representations is still aggregated. pgxmock validates the SQL shape and the
+// array arg binding.
+func TestQueryHistory_DualColumnFilter(t *testing.T) {
 	client, mock := newMockAnalyticsClient(t)
 	ctx := context.Background()
 	uuid := "aabbccdd-1234-5678-abcd-aabbccddee00"
+	external := "123456789012"
 
 	bucket := time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC)
 	rows := mock.NewRows([]string{"bucket", "service", "provider", "savings", "upfront", "purchases"}).
 		AddRow(bucket, "ec2", "aws", 50.0, 20.0, 1)
 
-	mock.ExpectQuery(`(?s)SELECT date_trunc\('day', timestamp\).*AND \(\$3 = '' OR account_id = \$3 OR cloud_account_id::text = \$3\)`).
-		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), uuid).
+	mock.ExpectQuery(`(?s)SELECT date_trunc\('day', timestamp\).*AND \(cloud_account_id::text = ANY\(\$3\) OR account_id = ANY\(\$4\)\)`).
+		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), []string{uuid}, []string{external}).
 		WillReturnRows(rows)
 
 	start := time.Date(2026, 4, 25, 0, 0, 0, 0, time.UTC)
 	end := time.Date(2026, 5, 5, 0, 0, 0, 0, time.UTC)
-	points, summary, err := client.QueryHistory(ctx, uuid, start, end, "daily")
+	points, summary, err := client.QueryHistory(ctx, []string{uuid}, []string{external}, start, end, "daily")
 	require.NoError(t, err)
 	require.Len(t, points, 1)
 	assert.InDelta(t, 50.0, points[0].TotalSavings, 1e-9)
@@ -194,21 +199,22 @@ func TestQueryHistory_CloudAccountIDFilter(t *testing.T) {
 	assert.NoError(t, mock.ExpectationsWereMet())
 }
 
-// TestQueryBreakdown_CloudAccountIDFilter verifies that QueryBreakdown accepts
-// a UUID as the accountID (issue #701). Mirrors TestQueryHistory_CloudAccountIDFilter.
-func TestQueryBreakdown_CloudAccountIDFilter(t *testing.T) {
+// TestQueryBreakdown_DualColumnFilter mirrors TestQueryHistory_DualColumnFilter
+// for the breakdown aggregate (issue #701/#498/#866).
+func TestQueryBreakdown_DualColumnFilter(t *testing.T) {
 	client, mock := newMockAnalyticsClient(t)
 	ctx := context.Background()
 	uuid := "aabbccdd-1234-5678-abcd-aabbccddee00"
+	external := "123456789012"
 
 	rows := mock.NewRows([]string{"bucket", "savings", "upfront", "purchases"}).
 		AddRow("rds", 200.0, 80.0, 3)
 
-	mock.ExpectQuery(`(?s)SELECT service AS bucket.*AND \(\$3 = '' OR account_id = \$3 OR cloud_account_id::text = \$3\)`).
-		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), uuid).
+	mock.ExpectQuery(`(?s)SELECT service AS bucket.*AND \(cloud_account_id::text = ANY\(\$3\) OR account_id = ANY\(\$4\)\)`).
+		WithArgs(pgxmock.AnyArg(), pgxmock.AnyArg(), []string{uuid}, []string{external}).
 		WillReturnRows(rows)
 
-	out, err := client.QueryBreakdown(ctx, uuid,
+	out, err := client.QueryBreakdown(ctx, []string{uuid}, []string{external},
 		time.Date(2026, 4, 1, 0, 0, 0, 0, time.UTC),
 		time.Date(2026, 5, 1, 0, 0, 0, 0, time.UTC),
 		"service")

internal/api/handler_analytics.go

@@ -67,8 +67,13 @@ func (h *Handler) getHistoryAnalytics(ctx context.Context, req *events.LambdaFun
 		return nil, err
 	}
 
+	// Resolve the requested account (a top-bar chip UUID, or "" for all) to the
+	// dual-column filter inputs so rows that carry only the external account_id
+	// (cloud_account_id NULL) are aggregated (issue #701/#498/#866).
+	accountUUIDs, accountExternalIDs := h.resolveSingleAccountFilterIDs(ctx, accountID)
+
 	// Aggregate history from the analytics client (Postgres-backed).
-	dataPoints, summary, err := h.analyticsClient.QueryHistory(ctx, accountID, start, end, interval)
+	dataPoints, summary, err := h.analyticsClient.QueryHistory(ctx, accountUUIDs, accountExternalIDs, start, end, interval)
 	if err != nil {
 		return nil, fmt.Errorf("failed to query analytics: %w", err)
 	}
@@ -109,8 +114,11 @@ func (h *Handler) getHistoryBreakdown(ctx context.Context, req *events.LambdaFun
 		return nil, err
 	}
 
+	// Resolve account UUID -> dual-column inputs; see getHistoryAnalytics.
+	accountUUIDs, accountExternalIDs := h.resolveSingleAccountFilterIDs(ctx, accountID)
+
 	// Fetch the breakdown from the analytics client (Postgres-backed).
-	data, err := h.analyticsClient.QueryBreakdown(ctx, accountID, start, end, dimension)
+	data, err := h.analyticsClient.QueryBreakdown(ctx, accountUUIDs, accountExternalIDs, start, end, dimension)
 	if err != nil {
 		return nil, fmt.Errorf("failed to query breakdown: %w", err)
 	}