fix(history): push filters and account scope into the actionable executions read

internal/api/handler_history.go

@@ -62,9 +62,10 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR
 	// page size, and later pages leave it out, so paging can never push a
 	// pending approval off the first page. A failure to list them must not
 	// hide completed history — log, skip, continue, and flag the response.
-	// The same filter set is applied here (in-memory against the synthesized
-	// row's recommendations and scheduled_date) so the two halves of the
-	// merged response are consistently scoped (issue #701).
+	// The same filter set and the account scope are pushed into the SQL read
+	// so the class cap counts only rows the caller can be shown (issue #632),
+	// and re-checked in memory against the synthesized row so the two halves
+	// of the merged response are consistently scoped (issue #701).
 	//
 	// Stale pending/notified executions are expired AFTER the response is
 	// assembled so the GET handler is a pure read (issue #1032): the caller
@@ -80,7 +81,7 @@ func (h *Handler) getHistory(ctx context.Context, req *events.LambdaFunctionURLR
 	var staleExecs []config.PurchaseExecution
 	actionableIncomplete := false
 	if filters.Cursor == nil {
-		extra, staleExecs, actionableIncomplete = h.fetchExecutionsAsHistory(ctx, filters)
+		extra, staleExecs, actionableIncomplete = h.fetchExecutionsAsHistory(ctx, filters, scope)
 		extra = scope.filter(extra)
 	}
 	defer h.expireStaleExecutions(staleExecs)
@@ -185,32 +186,29 @@ const approvalExpiryWindow = 7 * 24 * time.Hour
 // history must still render. Terminal executions are read by the paged
 // stream instead (fetchHistoryPage).
 //
-// 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.
+// The filter set (issue #701) and the caller's account scope are pushed into
+// the SQL read as a superset (issue #632), so the MaxListLimit cap counts only
+// executions the caller could be shown and other tenants' or filtered-out rows
+// cannot crowd a pending approval out. The exact checks still run in Go
+// against the synthetic row: provider via the recs' providers, account via
+// CloudAccountID, plan via PlanID, date via ScheduledDate here, and the scope
+// in the caller.
 //
 // The third return value reports that actionable executions may be missing:
-// 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, filters.PlanID, config.MaxListLimit)
+// the class reached its MaxListLimit cap, or its listing failed (issue #686).
+func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyFilters, scope historyScope) ([]config.PurchaseHistoryRecord, []config.PurchaseExecution, bool) {
+	q := actionableExecutionsQuery(&filters, &scope)
+	executions, ok := h.fetchExecutionsByClass(ctx, q)
 	incomplete := !ok || len(executions) >= config.MaxListLimit
+	staleExecs := h.staleExecutionsToSweep(ctx, &q, executions)
 	if len(executions) == 0 {
-		return nil, nil, incomplete
+		return nil, staleExecs, incomplete
 	}
 	approver := h.resolvePendingApproverEmail(ctx)
 	userEmailCache := h.resolveUserEmails(ctx, executions)
 	out := make([]config.PurchaseHistoryRecord, 0, len(executions))
-	var staleExecs []config.PurchaseExecution
 	for _rvc := range executions {
 		exec := executions[_rvc]
-		// Collect stale pending/notified executions for the post-assembly
-		// expire sweep. We do NOT mutate status here to keep the GET
-		// read-only: the response reflects current DB state; the sweep
-		// fires the transition so the next request sees "expired".
-		if isStaleExecution(exec) {
-			staleExecs = append(staleExecs, exec)
-		}
 		if !filters.matchesExecution(exec) {
 			continue
 		}
@@ -224,18 +222,13 @@ func (h *Handler) fetchExecutionsAsHistory(ctx context.Context, filters historyF
 // render; the caller flags the response as incomplete so the hidden rows are
 // not silent (issue #686).
 //
-// 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)
-	}
+// q carries the request filters, plan_id included, and the caller's scope, so
+// a plan view or a scoped caller is not crowded out by other rows under the
+// limit (issues #632, #1404).
+func (h *Handler) fetchExecutionsByClass(ctx context.Context, q config.ExecutionsByStatusesQuery) ([]config.PurchaseExecution, bool) {
+	rows, err := h.config.GetExecutionsByStatusesFiltered(ctx, q)
 	if err != nil {
-		logging.Warnf("history: failed to load executions with statuses %v: %v", statuses, err)
+		logging.Warnf("history: failed to load executions with statuses %v: %v", q.Statuses, err)
 		return nil, false
 	}
 	return rows, true

internal/api/handler_history_actionable.go

@@ -0,0 +1,103 @@
+package api
+
+import (
+	"context"
+	"time"
+
+	"github.com/LeanerCloud/cloud-commitments-go/pkg/logging"
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+)
+
+// actionableExecutionsQuery is the SQL read for the History view's actionable
+// class (issue #632). It carries the request filters and the caller's account
+// scope so the store applies them before the MaxListLimit cap. Each predicate
+// is a superset of the Go check the handler still runs (matchesExecution and
+// historyScope.allows), so pushing it down can only drop rows Go would drop.
+func actionableExecutionsQuery(filters *historyFilters, scope *historyScope) config.ExecutionsByStatusesQuery {
+	q := config.ExecutionsByStatusesQuery{
+		Statuses: actionableExecutionStatuses,
+		Limit:    config.MaxListLimit,
+		Provider: filters.Provider,
+		PlanID:   filters.PlanID,
+		Scope:    scope.executionScope(),
+	}
+	if filters.HasDate {
+		start, end := filters.Start, filters.End
+		q.Start, q.End = &start, &end
+	}
+	if len(filters.AccountIDs) > 0 || len(filters.ExternalIDsByProvider) > 0 {
+		q.AccountIDs = executionFilterAccountValues(filters)
+	}
+	return q
+}
+
+// executionFilterAccountValues lists every account value accountMatchesFilters
+// can match: the UUIDs and the external numbers of every provider. The
+// provider pairing of an external number is left to the Go check, so this is
+// a superset of it.
+func executionFilterAccountValues(filters *historyFilters) []string {
+	values := append([]string{}, filters.AccountIDs...)
+	for _, ids := range filters.ExternalIDsByProvider {
+		values = appendMissing(values, ids...)
+	}
+	return values
+}
+
+// executionScope returns the SQL form of the scope, or nil for an
+// unrestricted caller. It lists every account value allows can accept: the
+// scope's literal entries (allows compares the row's account against them
+// directly) plus the UUID and external number of each registered account the
+// scope allows, since allows also matches a row by the name of the account
+// its UUID or external number resolves to. An empty list keeps only the
+// caller's own unattributed rows, as allows does.
+func (s *historyScope) executionScope() *config.ExecutionScope {
+	if s.allowed.AllowsAll() {
+		return nil
+	}
+	values := appendMissing(nil, s.allowed.Accounts...)
+	for i := range s.accounts {
+		a := &s.accounts[i]
+		if s.allowed.Allows(a.ID, a.Name) {
+			values = appendMissing(values, a.ID, a.ExternalID)
+		}
+	}
+	return &config.ExecutionScope{AccountIDs: values, UserID: s.userID}
+}
+
+// isNarrowed reports whether q drops any row the unfiltered class read would
+// return. Every filter field of ExecutionsByStatusesQuery must be listed here,
+// or the stale sweep would only see the narrowed rows.
+func isNarrowed(q *config.ExecutionsByStatusesQuery) bool {
+	return q.Start != nil || q.End != nil || q.Provider != "" || q.PlanID != "" ||
+		len(q.AccountIDs) > 0 || q.Scope != nil
+}
+
+// staleExecutionsToSweep returns the stale pending/notified executions the GET
+// expires after it responds (see expireStaleExecutions). The sweep is not
+// limited to the caller's filters or scope: it never shows a row, and the
+// History load is what expires stale approvals and releases their
+// suppressions. So when q is narrowed, the stale rows are read with a
+// separate unfiltered query; otherwise they are taken from the class already
+// read. A read failure is logged and skips the sweep for this request.
+func (h *Handler) staleExecutionsToSweep(ctx context.Context, q *config.ExecutionsByStatusesQuery, executions []config.PurchaseExecution) []config.PurchaseExecution {
+	if isNarrowed(q) {
+		cutoff := time.Now().Add(-approvalExpiryWindow)
+		rows, err := h.config.GetExecutionsByStatusesFiltered(ctx, config.ExecutionsByStatusesQuery{
+			Statuses: []string{"pending", "notified"},
+			Limit:    config.DefaultListLimit,
+			End:      &cutoff,
+		})
+		if err != nil {
+			logging.Warnf("history: failed to load stale executions to expire: %v", err)
+			return nil
+		}
+		executions = rows
+	}
+	var stale []config.PurchaseExecution
+	for i := range executions {
+		if isStaleExecution(executions[i]) {
+			stale = append(stale, executions[i])
+		}
+	}
+	return stale
+}