From cfa1729d07d59c2e113fbed44190ce304d4ec563 Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 4 Oct 2026 16:12:42 +0800 Subject: [PATCH] fix(accounts): bulk-by-filter honours the group filter the account page sends The fork's account list reads its group filter as group_id, and the account page sends that name in the filters of bulk-by-filter requests as well. The bulk filter struct only bound the older name, group, so the group was dropped when the targets were resolved: a bulk edit of "all filtered results" could change more accounts than the list showed (with only a group selected, every account), and a bulk taxonomy update failed its match-count check. BulkUpdateAccountFilters now binds group_id next to group. The request uses group_id, or group when group_id is empty; two different values are rejected with INVALID_GROUP_FILTER instead of one being picked. Both filter shapes (scalar and console) and both handlers resolve through the same value. Co-Authored-By: Claude Opus 5.5 --- .../admin/account_bulk_job_scope_test.go | 7 ++- .../internal/handler/admin/account_handler.go | 28 +++++++-- .../account_handler_mixed_channel_test.go | 62 +++++++++++++++++++ 3 files changed, 90 insertions(+), 7 deletions(-) diff --git a/backend/internal/handler/admin/account_bulk_job_scope_test.go b/backend/internal/handler/admin/account_bulk_job_scope_test.go index bb5bb739af99..a0f5e59da192 100644 --- a/backend/internal/handler/admin/account_bulk_job_scope_test.go +++ b/backend/internal/handler/admin/account_bulk_job_scope_test.go @@ -215,6 +215,7 @@ type bulkJobScopeAdmin struct { accountsByID map[int64]*service.Account matches []int64 filterCalls int + filterGroups []int64 // the group each target-resolving list call filtered by filterErr error updatedIDs []int64 bulkInputs []*service.BulkUpdateAccountsInput @@ -242,11 +243,13 @@ func (s *bulkJobScopeAdmin) listMatches() ([]service.Account, int64, error) { return accounts, int64(len(accounts)), nil } -func (s *bulkJobScopeAdmin) ListAccounts(context.Context, int, int, string, string, string, string, int64, string, string, string) ([]service.Account, int64, error) { +func (s *bulkJobScopeAdmin) ListAccounts(_ context.Context, _ int, _ int, _ string, _ string, _ string, _ string, groupID int64, _ string, _ string, _ string) ([]service.Account, int64, error) { + s.filterGroups = append(s.filterGroups, groupID) return s.listMatches() } -func (s *bulkJobScopeAdmin) ListAccountsConsole(context.Context, int, int, service.AccountConsoleFilters) ([]service.Account, int64, error) { +func (s *bulkJobScopeAdmin) ListAccountsConsole(_ context.Context, _ int, _ int, filters service.AccountConsoleFilters) ([]service.Account, int64, error) { + s.filterGroups = append(s.filterGroups, filters.GroupID) return s.listMatches() } diff --git a/backend/internal/handler/admin/account_handler.go b/backend/internal/handler/admin/account_handler.go index e70f9ffbc695..6f9cd777de3b 100644 --- a/backend/internal/handler/admin/account_handler.go +++ b/backend/internal/handler/admin/account_handler.go @@ -224,7 +224,8 @@ type BulkUpdateAccountFilters struct { Folders string `json:"folders"` Tags string `json:"tags"` AccountIDs string `json:"account_ids"` - Group string `json:"group"` + GroupID string `json:"group_id"` // the group filter under the name the account list reads + Group string `json:"group"` // the same filter under its older name Search string `json:"search"` PrivacyMode string `json:"privacy_mode"` } @@ -2272,15 +2273,33 @@ func splitBulkAccountFilterValues(values ...string) []string { return out } +// bulkAccountFilterGroup returns the group a bulk filter selects: group_id, the name the account list and the +// account page use, or the older group. Two different values are rejected rather than one of them being picked, +// since the filter decides which accounts a bulk operation changes. +func bulkAccountFilterGroup(filters *BulkUpdateAccountFilters) (string, error) { + groupID, group := strings.TrimSpace(filters.GroupID), strings.TrimSpace(filters.Group) + if groupID == "" { + return group, nil + } + if group != "" && group != groupID { + return "", infraerrors.BadRequest("INVALID_GROUP_FILTER", "group and group_id filters differ") + } + return groupID, nil +} + func toServiceBulkUpdateAccountFilters(filters *BulkUpdateAccountFilters) (*service.BulkUpdateAccountFilters, error) { if filters == nil { return nil, nil } + group, err := bulkAccountFilterGroup(filters) + if err != nil { + return nil, err + } out := &service.BulkUpdateAccountFilters{ Platform: filters.Platform, Type: filters.Type, Status: filters.Status, - Group: filters.Group, + Group: group, Search: filters.Search, PrivacyMode: filters.PrivacyMode, } @@ -2309,18 +2328,17 @@ func toServiceBulkUpdateAccountFilters(filters *BulkUpdateAccountFilters) (*serv Plans: splitBulkAccountFilterValues(filters.Plans), Search: strings.TrimSpace(filters.Search), GroupID: 0, PrivacyMode: strings.TrimSpace(filters.PrivacyMode), SortBy: "id", SortOrder: "asc", } - switch strings.TrimSpace(filters.Group) { + switch group { case "": case accountListGroupUngroupedQueryValue: console.GroupID = service.AccountListGroupUngrouped default: - groupID, err := strconv.ParseInt(strings.TrimSpace(filters.Group), 10, 64) + groupID, err := strconv.ParseInt(group, 10, 64) if err != nil || groupID <= 0 { return nil, infraerrors.BadRequest("INVALID_GROUP_FILTER", "invalid group filter") } console.GroupID = groupID } - var err error console.ProxyIDs, console.IncludeDirect, err = parseIDQueryValues(splitBulkAccountFilterValues(filters.Proxies), "direct") if err != nil { return nil, err diff --git a/backend/internal/handler/admin/account_handler_mixed_channel_test.go b/backend/internal/handler/admin/account_handler_mixed_channel_test.go index edf78a79d3a1..646df8b4df51 100644 --- a/backend/internal/handler/admin/account_handler_mixed_channel_test.go +++ b/backend/internal/handler/admin/account_handler_mixed_channel_test.go @@ -266,6 +266,68 @@ func TestBulkUpdateAcceptsFilterTargetRequest(t *testing.T) { require.Nil(t, filters.Console) } +// The account page sends the group filter as group_id, the name the account list reads. A bulk request must +// resolve its targets within that group, whichever of the two filter shapes it uses. +func TestBulkUpdateFilterTargetResolvesWithinTheGroupFilter(t *testing.T) { + for _, test := range []struct { + name string + filters map[string]any + wantGroup int64 + }{ + {name: "group_id_with_scalar_filters", filters: map[string]any{"status": "active", "group_id": "12"}, wantGroup: 12}, + {name: "group_id_with_console_filters", filters: map[string]any{"statuses": "active", "group_id": "12"}, wantGroup: 12}, + {name: "group_id_alone", filters: map[string]any{"group_id": "12"}, wantGroup: 12}, + {name: "ungrouped_with_scalar_filters", filters: map[string]any{"status": "active", "group_id": "ungrouped"}, wantGroup: service.AccountListGroupUngrouped}, + {name: "ungrouped_with_console_filters", filters: map[string]any{"statuses": "active", "group_id": "ungrouped"}, wantGroup: service.AccountListGroupUngrouped}, + {name: "older_name_with_scalar_filters", filters: map[string]any{"status": "active", "group": "12"}, wantGroup: 12}, + {name: "older_name_with_console_filters", filters: map[string]any{"statuses": "active", "group": "12"}, wantGroup: 12}, + {name: "both_names_agreeing", filters: map[string]any{"statuses": "active", "group_id": "12", "group": " 12 "}, wantGroup: 12}, + {name: "no_group", filters: map[string]any{"statuses": "active"}, wantGroup: 0}, + } { + t.Run(test.name, func(t *testing.T) { + adminSvc := newBulkJobScopeAdmin(7) + adminSvc.matches = []int64{7} + router, _, _ := newBulkJobScopeRouter(adminSvc) + + body, _ := json.Marshal(map[string]any{"filters": test.filters, "schedulable": true}) + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/bulk", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + setAccountJobTestIdempotencyKey(req) + router.ServeHTTP(rec, req) + + require.Equal(t, http.StatusAccepted, rec.Code, rec.Body.String()) + require.Equal(t, []int64{test.wantGroup}, adminSvc.filterGroups) + }) + } + + for _, test := range []struct { + name string + filters map[string]any + }{ + {name: "names_disagreeing", filters: map[string]any{"statuses": "active", "group_id": "12", "group": "13"}}, + {name: "names_disagreeing_with_scalar_filters", filters: map[string]any{"status": "active", "group_id": "12", "group": "ungrouped"}}, + {name: "group_id_not_an_id", filters: map[string]any{"statuses": "active", "group_id": "twelve"}}, + } { + t.Run(test.name, func(t *testing.T) { + adminSvc := newBulkJobScopeAdmin(7) + adminSvc.matches = []int64{7} + router, _, jobs := newBulkJobScopeRouter(adminSvc) + + body, _ := json.Marshal(map[string]any{"filters": test.filters, "schedulable": true}) + rec := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodPost, "/bulk", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + setAccountJobTestIdempotencyKey(req) + router.ServeHTTP(rec, req) + + require.Equal(t, http.StatusBadRequest, rec.Code, rec.Body.String()) + require.Empty(t, adminSvc.filterGroups, "an unusable group filter must not resolve any target") + require.Empty(t, jobs.created) + }) + } +} + func TestBulkUpdateAcceptsCockpitConsoleFilterTargetRequest(t *testing.T) { adminSvc := newBulkJobScopeAdmin(7) adminSvc.matches = []int64{7}