From e6f265e72799faf18046ddfab514196d5acc83ab Mon Sep 17 00:00:00 2001 From: Daniel Reed Date: Thu, 22 Oct 2026 11:53:34 -0400 Subject: [PATCH 1/2] feat(pagination): add server-side pagination and label filters Signed-off-by: Daniel Reed --- .claude/rules/openshell-api.md | 2 +- .gitignore | 4 + README.md | 9 + backend/pkg/handlers/allworkspaces_handler.go | 25 +- backend/pkg/handlers/drafts_handler.go | 20 + .../handlers/drafts_summary_handler_test.go | 42 ++ backend/pkg/handlers/list_pagination.go | 46 ++ backend/pkg/handlers/list_pagination_test.go | 127 +++++ backend/pkg/handlers/mock_sdk_test.go | 12 + backend/pkg/handlers/sandboxes_handler.go | 28 +- backend/pkg/handlers/workspaces_handler.go | 28 +- backend/test/compat/contract_test.go | 7 +- backend/test/compat/harness_test.go | 2 +- backend/test/compat/list_pagination_test.go | 73 +++ backend/test/compat/workspace_test.go | 4 +- frontend/cypress/e2e/sandbox-lifecycle.cy.ts | 2 +- frontend/cypress/e2e/workspace-list.cy.ts | 13 +- frontend/cypress/fixtures/sandboxes.json | 193 +++---- frontend/cypress/fixtures/workspaces.json | 71 +-- frontend/cypress/support/intercepts.ts | 18 +- .../src/api/__tests__/draftSummary.spec.tsx | 32 +- .../src/api/__tests__/platform-paths.spec.ts | 68 ++- frontend/src/api/__tests__/queryKeys.spec.ts | 45 +- frontend/src/api/allWorkspaces.ts | 15 + frontend/src/api/draftSummary.ts | 26 +- frontend/src/api/index.ts | 1 + frontend/src/api/pagination.ts | 18 + frontend/src/api/queryKeys.ts | 36 +- frontend/src/api/sandboxes.ts | 28 +- frontend/src/api/workspaces.ts | 15 + frontend/src/components/AllSandboxList.tsx | 146 ++++++ frontend/src/components/AllWorkspaceCells.tsx | 17 + .../src/components/CursorPageTransition.css | 15 + .../src/components/CursorPageTransition.tsx | 35 ++ frontend/src/components/CursorPagination.tsx | 143 ++++++ .../src/components/CursorResourceTable.tsx | 285 +++++++++++ frontend/src/components/ResourceTable.tsx | 26 +- .../__tests__/CursorPagination.spec.tsx | 147 ++++++ .../__tests__/useCursorListPage.spec.tsx | 58 +++ frontend/src/hooks/useCursorListPage.ts | 92 ++++ frontend/src/hooks/useCursorSelection.ts | 62 +++ frontend/src/i18n/locales/en/common.ts | 15 + frontend/src/i18n/locales/en/workspaces.ts | 4 + frontend/src/pages/AllWorkspacesPage.tsx | 96 +--- frontend/src/pages/SandboxListPage.tsx | 475 +++++++++++------- frontend/src/pages/WorkspaceDetailPage.tsx | 11 +- frontend/src/pages/WorkspaceListPage.tsx | 357 +++++++------ .../__tests__/AllWorkspacesPage.spec.tsx | 82 ++- .../__tests__/SandboxListPage.cache.spec.tsx | 7 +- .../__tests__/SandboxListPage.list.spec.tsx | 131 ++++- .../SandboxListPage.polling.spec.tsx | 49 +- .../pages/__tests__/SandboxListPage.spec.tsx | 20 +- .../__tests__/WorkspaceListPage.spec.tsx | 155 +++++- frontend/src/pages/__tests__/pagedHookMock.ts | 21 + .../__tests__/sandboxLabelSelector.spec.ts | 24 + frontend/src/utils/sandboxLabelSelector.ts | 25 + 56 files changed, 2884 insertions(+), 624 deletions(-) create mode 100644 backend/pkg/handlers/list_pagination.go create mode 100644 backend/pkg/handlers/list_pagination_test.go create mode 100644 backend/test/compat/list_pagination_test.go create mode 100644 frontend/src/api/pagination.ts create mode 100644 frontend/src/components/AllSandboxList.tsx create mode 100644 frontend/src/components/AllWorkspaceCells.tsx create mode 100644 frontend/src/components/CursorPageTransition.css create mode 100644 frontend/src/components/CursorPageTransition.tsx create mode 100644 frontend/src/components/CursorPagination.tsx create mode 100644 frontend/src/components/CursorResourceTable.tsx create mode 100644 frontend/src/components/__tests__/CursorPagination.spec.tsx create mode 100644 frontend/src/hooks/__tests__/useCursorListPage.spec.tsx create mode 100644 frontend/src/hooks/useCursorListPage.ts create mode 100644 frontend/src/hooks/useCursorSelection.ts create mode 100644 frontend/src/pages/__tests__/pagedHookMock.ts create mode 100644 frontend/src/utils/__tests__/sandboxLabelSelector.spec.ts create mode 100644 frontend/src/utils/sandboxLabelSelector.ts diff --git a/.claude/rules/openshell-api.md b/.claude/rules/openshell-api.md index 512d8ead..fb33f77a 100644 --- a/.claude/rules/openshell-api.md +++ b/.claude/rules/openshell-api.md @@ -40,7 +40,7 @@ Skip `GatewayInterceptor`, `SupervisorMiddleware`, `ComputeDriver` — internal/ 11. **Optimistic concurrency:** `AttachSandboxProvider`, `DetachSandboxProvider`, `UpdateConfig`, `UpdateProviderProfiles` accept `expected_resource_version` — pass the ObjectMeta.resource_version from the last read. `ApproveDraftChunk` similarly takes a `review_token` (from the chunk's last `GetDraft` read) pinning the exact evaluated candidate; the BFF falls back to resolving it server-side if the client doesn't send one. 12. **Workspace scoping:** most requests carry a workspace selector (`workspace_scope`); list RPCs offer `all_workspaces`. An empty workspace name does not mean "default": gateway 0.1.x refuses a named selector with no name (`workspace_authz.rs` at v0.1.2), so a route must never forward an empty path segment as a workspace. Workspaces themselves are top-level. Exactly four lists accept the all-workspaces scope — sandboxes, sandbox templates, providers and services (`ListOptions{AllWorkspaces: true}`, which makes the SDK ignore the workspace argument) — and the gateway answers it for platform admins only. `ListServices` refuses a sandbox filter in that scope. The BFF serves them at `GET /api/v1/{sandboxes,providers,templates,services}`. 13. **Secret fields** are annotated `[(openshell.options.v1.secret) = true]` in proto — grep for `secret` when adding a wrapper and never serialize those fields to the frontend. -14. **`List` is paginated; use `ListAll` unless you are paging deliberately.** `List(...)` takes no `ctx` and returns a `*Pager[T]`. The eager form is `ListAll(ctx, ...)` returning a slice, and it is what every BFF handler uses. `ListMembers` likewise has `ListAllMembers`, and a sandbox's `ListProviders` has `ListAllProviders`. `Policy().ListAll` takes `(ctx, workspace, sandboxName, opts...)` — pass `""` for both when listing global revisions. +14. **`List` is paginated; use `ListAll` unless you are paging deliberately.** `List(...)` takes no `ctx` and returns a `*Pager[T]`. The eager form is `ListAll(ctx, ...)` returning a slice. Sandbox and workspace list handlers use `List(...).NextPage(ctx)` when `pageSize` is requested, and `ListAll` for legacy array responses. Other handlers use `ListAll`. `ListMembers` likewise has `ListAllMembers`, and a sandbox's `ListProviders` has `ListAllProviders`. `Policy().ListAll` takes `(ctx, workspace, sandboxName, opts...)` — pass `""` for both when listing global revisions. 15. **`Delete` returns `(*DeletionResult, error)`, and a nil error does not mean the resource is gone.** `DeletionOutcome` is one of Unspecified / Completed / Accepted / AlreadyAbsent. Only **Completed** and **AlreadyAbsent** establish completion; **Accepted** means the gateway queued asynchronous cleanup, and unrecognized numeric values must not be treated as completion. Convert through `models.FromSDKDeletion` rather than hardcoding `{"deleted": true}`. 16. **Network policy enum fields are proto enums, not free strings.** `access`, `enforcement` and `tls` on a policy network endpoint serialize through protojson as `NETWORK_ACCESS_PRESET_*`, `NETWORK_ENFORCEMENT_MODE_*`, `NETWORK_TLS_MODE_*`. The gateway does not accept the lowercase spellings (`read-only`, `enforce`, `verify`). diff --git a/.gitignore b/.gitignore index 3f9ebea3..13986c7d 100644 --- a/.gitignore +++ b/.gitignore @@ -28,5 +28,9 @@ frontend/cypress/screenshots/ .env.* .claude/worktrees/ +# Local agent guidance +.agents/ +AGENTS.md + # compat stack rendered config deploy/ci/.rendered/ diff --git a/README.md b/README.md index 84280e05..dc7ad490 100644 --- a/README.md +++ b/README.md @@ -87,6 +87,15 @@ This branch is built on: [docs/compatibility.md](docs/compatibility.md) says what the dashboard shows when the gateway is on another line, and how the claim is tested. +## List pagination + +`GET /api/v1/workspaces`, `GET /api/v1/workspaces/{workspace}/sandboxes`, and +`GET /api/v1/sandboxes` accept `pageSize` (1–100), an optional opaque +`pageToken`, and `labelSelector`. A token requires `pageSize`. Paged requests +return `{"items": [...], "nextPageToken": "..."}`; an empty token marks the last +page. Requests without `pageSize` retain the existing array response. The +gateway does not return a total count. + ## Documentation - [Install](docs/install.md): the full guide, with Keycloak, OpenShift and troubleshooting diff --git a/backend/pkg/handlers/allworkspaces_handler.go b/backend/pkg/handlers/allworkspaces_handler.go index 3c9bf2a3..1cb46e67 100644 --- a/backend/pkg/handlers/allworkspaces_handler.go +++ b/backend/pkg/handlers/allworkspaces_handler.go @@ -61,7 +61,30 @@ func everyWorkspace(labelSelector string) openshell.ListOptions { // ListSandboxes lists the sandboxes of every workspace. Query params: // labelSelector, as on the workspace-scoped list. func (h *AllWorkspacesHandler) ListSandboxes(w http.ResponseWriter, r *http.Request) { - sandboxes, err := h.sandboxes.ListAll(r.Context(), "", everyWorkspace(r.URL.Query().Get("labelSelector"))) + opts, paged, valid := listRequestOptions(w, r) + if !valid { + return + } + opts.AllWorkspaces = true + if paged { + pager, err := h.sandboxes.List("", opts) + if err != nil { + apiutils.WriteSDKError(w, err) + return + } + page, err := pager.NextPage(r.Context()) + if err != nil { + apiutils.WriteSDKError(w, err) + return + } + items := make([]models.Sandbox, 0, len(page.Items)) + for _, sandbox := range page.Items { + items = append(items, models.FromSDKSandbox(sandbox)) + } + apiutils.WriteJSON(w, http.StatusOK, listPageResponse[models.Sandbox]{Items: items, NextPageToken: page.NextPageToken}) + return + } + sandboxes, err := h.sandboxes.ListAll(r.Context(), "", opts) if err != nil { apiutils.WriteSDKError(w, err) return diff --git a/backend/pkg/handlers/drafts_handler.go b/backend/pkg/handlers/drafts_handler.go index 2d8b8285..3c17981d 100644 --- a/backend/pkg/handlers/drafts_handler.go +++ b/backend/pkg/handlers/drafts_handler.go @@ -64,6 +64,26 @@ const ( // GetWorkspaceDraftSummary counts the pending draft chunks of every sandbox // in a workspace. See summarizeDrafts. func (h *DraftsHandler) GetWorkspaceDraftSummary(w http.ResponseWriter, r *http.Request) { + if names, named := r.URL.Query()["sandbox"]; named { + if len(names) > 100 { + apiutils.WriteError(w, http.StatusBadRequest, apiutils.InvalidArgument, "you can request draft summaries for no more than 100 sandboxes at a time") + return + } + seen := make(map[string]bool, len(names)) + sandboxes := make([]*openshell.Sandbox, 0, len(names)) + for _, name := range names { + if !apiutils.ValidDNS1123(name) { + apiutils.WriteError(w, http.StatusBadRequest, apiutils.InvalidName, "sandbox must be a valid DNS-1123 name") + return + } + if !seen[name] { + seen[name] = true + sandboxes = append(sandboxes, &openshell.Sandbox{Name: name, Workspace: r.PathValue("workspace")}) + } + } + apiutils.WriteJSON(w, http.StatusOK, h.summarizeDrafts(r.Context(), r.PathValue("workspace"), sandboxes)) + return + } h.writeDraftSummary(w, r, r.PathValue("workspace")) } diff --git a/backend/pkg/handlers/drafts_summary_handler_test.go b/backend/pkg/handlers/drafts_summary_handler_test.go index 54cd9b1a..a124b6d1 100644 --- a/backend/pkg/handlers/drafts_summary_handler_test.go +++ b/backend/pkg/handlers/drafts_summary_handler_test.go @@ -244,6 +244,48 @@ func TestWorkspaceDraftSummaryWireShape(t *testing.T) { } } +func TestWorkspaceDraftSummaryForNamedPage(t *testing.T) { + sdk := &mockSDK{} + sdk.sandboxes.listFn = func(context.Context, string, ...openshell.ListOptions) ([]*openshell.Sandbox, error) { + t.Fatal("a named page must not list every sandbox") + return nil, nil + } + inbox := &draftInbox{chunks: map[string][]openshell.PolicyChunk{ + "team-a/busy": {{ID: "c1", Status: "pending"}}, + }} + sdk.policy.getDraftFn = inbox.get + w := serve(t, draftSummaryRoutes(sdk), http.MethodGet, "/workspaces/team-a/draft-summary?sandbox=busy&sandbox=quiet&sandbox=busy", "") + if w.Code != http.StatusOK { + t.Fatalf("status = %d; body: %s", w.Code, w.Body.String()) + } + want := map[draftRead]int{ + {workspace: "team-a", sandbox: "busy", filter: "pending"}: 1, + {workspace: "team-a", sandbox: "quiet", filter: "pending"}: 1, + } + if got := inbox.readSet(); !reflect.DeepEqual(got, want) { + t.Errorf("draft reads = %+v, want %+v", got, want) + } + var body models.DraftSummary + decodeInto(t, w, &body) + if body.TotalPending != 1 || len(body.Sandboxes) != 1 || body.Sandboxes[0].SandboxName != "busy" { + t.Errorf("summary = %+v", body) + } +} + +func TestWorkspaceDraftSummaryRejectsInvalidNamedPage(t *testing.T) { + sdk := &mockSDK{} + sdk.sandboxes.listFn = func(context.Context, string, ...openshell.ListOptions) ([]*openshell.Sandbox, error) { + t.Fatal("invalid named page must not list") + return nil, nil + } + for _, query := range []string{"sandbox=", "sandbox=bad%2Fname", strings.Repeat("sandbox=agent&", 101)} { + w := serve(t, draftSummaryRoutes(sdk), http.MethodGet, "/workspaces/team-a/draft-summary?"+query, "") + if w.Code != http.StatusBadRequest { + t.Errorf("%s: status = %d; body: %s", query, w.Code, w.Body.String()) + } + } +} + // The list of sandboxes is the one call the summary cannot do without, so // its failure is the answer. func TestDraftSummaryListFails(t *testing.T) { diff --git a/backend/pkg/handlers/list_pagination.go b/backend/pkg/handlers/list_pagination.go new file mode 100644 index 00000000..b17da995 --- /dev/null +++ b/backend/pkg/handlers/list_pagination.go @@ -0,0 +1,46 @@ +package handlers + +import ( + "net/http" + "strconv" + + openshell "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1" + + "github.com/Gkrumbach07/openshell-dashboard/backend/pkg/apiutils" +) + +// listRequestOptions keeps the existing array response unless pageSize was +// supplied. A paged request forwards the gateway's opaque continuation token. +func listRequestOptions(w http.ResponseWriter, r *http.Request) (openshell.ListOptions, bool, bool) { + query := r.URL.Query() + opts := openshell.ListOptions{LabelSelector: query.Get("labelSelector")} + sizes, paged := query["pageSize"] + if !paged { + if _, hasToken := query["pageToken"]; hasToken { + apiutils.WriteError(w, http.StatusBadRequest, apiutils.InvalidArgument, "pageToken requires pageSize") + return opts, false, false + } + return opts, false, true + } + if len(sizes) != 1 { + apiutils.WriteError(w, http.StatusBadRequest, apiutils.InvalidArgument, "pageSize must be a single integer from 1 to 100") + return opts, false, false + } + size, err := strconv.Atoi(sizes[0]) + if err != nil || size < 1 || size > 100 { + apiutils.WriteError(w, http.StatusBadRequest, apiutils.InvalidArgument, "pageSize must be an integer from 1 to 100") + return opts, false, false + } + if len(query["pageToken"]) > 1 { + apiutils.WriteError(w, http.StatusBadRequest, apiutils.InvalidArgument, "pageToken must appear once") + return opts, false, false + } + opts.PageSize = size + opts.PageToken = query.Get("pageToken") + return opts, true, true +} + +type listPageResponse[T any] struct { + NextPageToken string `json:"nextPageToken"` + Items []T `json:"items"` +} diff --git a/backend/pkg/handlers/list_pagination_test.go b/backend/pkg/handlers/list_pagination_test.go new file mode 100644 index 00000000..655cb3b9 --- /dev/null +++ b/backend/pkg/handlers/list_pagination_test.go @@ -0,0 +1,127 @@ +package handlers + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "reflect" + "testing" + + openshell "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1" + "github.com/go-chi/chi/v5" + + "github.com/Gkrumbach07/openshell-dashboard/backend/pkg/services" +) + +func TestPagedListRoutesFetchOneGatewayPage(t *testing.T) { + for _, route := range []string{"sandboxes", "workspaces", "all-sandboxes"} { + t.Run(route, func(t *testing.T) { + sdk := &mockSDK{} + calls := 0 + var gotOpts openshell.ListOptions + var gotWorkspace string + sdk.sandboxes.listFn = func(context.Context, string, ...openshell.ListOptions) ([]*openshell.Sandbox, error) { + t.Fatal("paged request must not call ListAll") + return nil, nil + } + sdk.workspaces.listFn = func(context.Context, ...openshell.ListOptions) ([]*openshell.Workspace, error) { + t.Fatal("paged request must not call ListAll") + return nil, nil + } + sdk.sandboxes.listPageFn = func(_ context.Context, workspace string, opts ...openshell.ListOptions) (*openshell.Page[*openshell.Sandbox], error) { + calls++ + gotWorkspace, gotOpts = workspace, opts[0] + return &openshell.Page[*openshell.Sandbox]{Items: []*openshell.Sandbox{{Name: "agent", Workspace: "team-a"}}, NextPageToken: "next-token"}, nil + } + sdk.workspaces.listPageFn = func(_ context.Context, opts ...openshell.ListOptions) (*openshell.Page[*openshell.Workspace], error) { + calls++ + gotOpts = opts[0] + return &openshell.Page[*openshell.Workspace]{Items: []*openshell.Workspace{{Name: "team-a"}}, NextPageToken: "next-token"}, nil + } + + r := chi.NewRouter() + path := "/workspaces/team-a/sandboxes" + switch route { + case "sandboxes": + r.Get("/workspaces/{workspace}/sandboxes", NewSandboxHandler(services.NewSandboxService(sdk.Sandboxes())).ListSandboxes) + case "workspaces": + path = "/workspaces" + r.Get(path, NewWorkspacesHandler(sdk.Workspaces()).ListWorkspaces) + case "all-sandboxes": + path = "/sandboxes" + r.Get(path, NewAllWorkspacesHandler(services.NewSandboxService(sdk.Sandboxes()), nil, nil, nil).ListSandboxes) + } + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path+"?pageSize=1&pageToken=prior-token&labelSelector=type%3Dtool", nil)) + if w.Code != http.StatusOK { + t.Fatalf("status = %d; body: %s", w.Code, w.Body.String()) + } + if calls != 1 { + t.Fatalf("SDK page calls = %d, want 1", calls) + } + want := openshell.ListOptions{PageSize: 1, PageToken: "prior-token", LabelSelector: "type=tool", AllWorkspaces: route == "all-sandboxes"} + if !reflect.DeepEqual(gotOpts, want) { + t.Errorf("options = %+v, want %+v", gotOpts, want) + } + if route == "sandboxes" && gotWorkspace != "team-a" || route == "all-sandboxes" && gotWorkspace != "" { + t.Errorf("workspace = %q", gotWorkspace) + } + var body struct { + NextPageToken string `json:"nextPageToken"` + Items []json.RawMessage `json:"items"` + } + if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { + t.Fatal(err) + } + if len(body.Items) != 1 || body.NextPageToken != "next-token" { + t.Errorf("body = %s", w.Body.String()) + } + }) + } +} + +func TestPagedListValidation(t *testing.T) { + for _, path := range []string{"/workspaces", "/workspaces/team-a/sandboxes", "/sandboxes"} { + for _, query := range []string{"pageSize=0", "pageSize=-1", "pageSize=101", "pageSize=abc", "pageSize=999999999999999999999999", "pageToken=token"} { + t.Run(path+"?"+query, func(t *testing.T) { + sdk := &mockSDK{} + r := chi.NewRouter() + r.Get("/workspaces", NewWorkspacesHandler(sdk.Workspaces()).ListWorkspaces) + r.Get("/workspaces/{workspace}/sandboxes", NewSandboxHandler(services.NewSandboxService(sdk.Sandboxes())).ListSandboxes) + r.Get("/sandboxes", NewAllWorkspacesHandler(services.NewSandboxService(sdk.Sandboxes()), nil, nil, nil).ListSandboxes) + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path+"?"+query, nil)) + if w.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400; body: %s", w.Code, w.Body.String()) + } + }) + } + } +} + +func TestPagedListEmptyAndGatewayError(t *testing.T) { + sdk := &mockSDK{} + r := chi.NewRouter() + r.Get("/workspaces", NewWorkspacesHandler(sdk.Workspaces()).ListWorkspaces) + sdk.workspaces.listPageFn = func(context.Context, ...openshell.ListOptions) (*openshell.Page[*openshell.Workspace], error) { + return &openshell.Page[*openshell.Workspace]{}, nil + } + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/workspaces?pageSize=10", nil)) + var emptyPage map[string]json.RawMessage + if err := json.Unmarshal(w.Body.Bytes(), &emptyPage); err != nil { + t.Fatal(err) + } + if w.Code != http.StatusOK || string(emptyPage["items"]) != "[]" || string(emptyPage["nextPageToken"]) != `""` { + t.Errorf("empty page: status %d, body %s", w.Code, w.Body.String()) + } + sdk.workspaces.listPageFn = func(context.Context, ...openshell.ListOptions) (*openshell.Page[*openshell.Workspace], error) { + return nil, &openshell.StatusError{Code: openshell.ErrorUnavailable, Message: "gateway down"} + } + w = httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, "/workspaces?pageSize=10", nil)) + if w.Code != http.StatusBadGateway { + t.Errorf("gateway error: status %d, body %s", w.Code, w.Body.String()) + } +} diff --git a/backend/pkg/handlers/mock_sdk_test.go b/backend/pkg/handlers/mock_sdk_test.go index 75ea11dd..e0d501a2 100644 --- a/backend/pkg/handlers/mock_sdk_test.go +++ b/backend/pkg/handlers/mock_sdk_test.go @@ -126,6 +126,7 @@ func readCreateOptions(opts []openshell.CreateOption) sentCreate { // mockSDKSandboxes provides injectable sandbox operations. type mockSDKSandboxes struct { listFn func(ctx context.Context, workspace string, opts ...openshell.ListOptions) ([]*openshell.Sandbox, error) + listPageFn func(ctx context.Context, workspace string, opts ...openshell.ListOptions) (*openshell.Page[*openshell.Sandbox], error) createFn func(ctx context.Context, workspace, name string, spec *openshell.SandboxSpec, opts ...openshell.CreateOption) (*openshell.Sandbox, error) getFn func(ctx context.Context, workspace, name string) (*openshell.Sandbox, error) deleteFn func(ctx context.Context, workspace, name string) error @@ -165,6 +166,11 @@ func (m *mockServerStream) Recv() (*pb.SandboxStreamEvent, error) { } func (m *mockSDKSandboxes) List(workspace string, opts ...openshell.ListOptions) (*openshell.Pager[*openshell.Sandbox], error) { + if m.listPageFn != nil { + return openshell.NewPager(opts[0].PageToken, func(ctx context.Context, _ string) (*openshell.Page[*openshell.Sandbox], error) { + return m.listPageFn(ctx, workspace, opts...) + }), nil + } return openshell.NewPager("", func(ctx context.Context, _ string) (*openshell.Page[*openshell.Sandbox], error) { items, err := m.ListAll(ctx, workspace, opts...) return &openshell.Page[*openshell.Sandbox]{Items: items}, err @@ -432,6 +438,7 @@ type mockSDKWorkspaces struct { createFn func(ctx context.Context, name string, labels map[string]string) (*openshell.Workspace, error) getFn func(ctx context.Context, name string) (*openshell.Workspace, error) listFn func(ctx context.Context, opts ...openshell.ListOptions) ([]*openshell.Workspace, error) + listPageFn func(ctx context.Context, opts ...openshell.ListOptions) (*openshell.Page[*openshell.Workspace], error) deleteFn func(ctx context.Context, name string) error addMemberFn func(ctx context.Context, workspace, principalSubject string, role openshell.WorkspaceRole) (*openshell.WorkspaceMember, error) removeMemberFn func(ctx context.Context, workspace, principalSubject string) error @@ -453,6 +460,11 @@ func (m *mockSDKWorkspaces) Get(ctx context.Context, name string) (*openshell.Wo } func (m *mockSDKWorkspaces) List(opts ...openshell.ListOptions) (*openshell.Pager[*openshell.Workspace], error) { + if m.listPageFn != nil { + return openshell.NewPager(opts[0].PageToken, func(ctx context.Context, _ string) (*openshell.Page[*openshell.Workspace], error) { + return m.listPageFn(ctx, opts...) + }), nil + } return openshell.NewPager("", func(ctx context.Context, _ string) (*openshell.Page[*openshell.Workspace], error) { items, err := m.ListAll(ctx, opts...) return &openshell.Page[*openshell.Workspace]{Items: items}, err diff --git a/backend/pkg/handlers/sandboxes_handler.go b/backend/pkg/handlers/sandboxes_handler.go index 553882d6..a010961f 100644 --- a/backend/pkg/handlers/sandboxes_handler.go +++ b/backend/pkg/handlers/sandboxes_handler.go @@ -5,8 +5,6 @@ import ( "log/slog" "net/http" - openshell "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1" - "github.com/Gkrumbach07/openshell-dashboard/backend/pkg/apiutils" "github.com/Gkrumbach07/openshell-dashboard/backend/pkg/models" "github.com/Gkrumbach07/openshell-dashboard/backend/pkg/services" @@ -23,11 +21,29 @@ func NewSandboxHandler(svc services.SandboxServiceInterface) *SandboxHandler { } func (h *SandboxHandler) ListSandboxes(w http.ResponseWriter, r *http.Request) { - var opts []openshell.ListOptions - if sel := r.URL.Query().Get("labelSelector"); sel != "" { - opts = append(opts, openshell.ListOptions{LabelSelector: sel}) + opts, paged, valid := listRequestOptions(w, r) + if !valid { + return + } + if paged { + pager, err := h.svc.List(r.PathValue("workspace"), opts) + if err != nil { + apiutils.WriteSDKError(w, err) + return + } + page, err := pager.NextPage(r.Context()) + if err != nil { + apiutils.WriteSDKError(w, err) + return + } + items := make([]models.Sandbox, 0, len(page.Items)) + for _, sandbox := range page.Items { + items = append(items, models.FromSDKSandbox(sandbox)) + } + apiutils.WriteJSON(w, http.StatusOK, listPageResponse[models.Sandbox]{Items: items, NextPageToken: page.NextPageToken}) + return } - sandboxes, err := h.svc.ListAll(r.Context(), r.PathValue("workspace"), opts...) + sandboxes, err := h.svc.ListAll(r.Context(), r.PathValue("workspace"), opts) if err != nil { apiutils.WriteSDKError(w, err) return diff --git a/backend/pkg/handlers/workspaces_handler.go b/backend/pkg/handlers/workspaces_handler.go index ba06c5aa..967fe733 100644 --- a/backend/pkg/handlers/workspaces_handler.go +++ b/backend/pkg/handlers/workspaces_handler.go @@ -4,8 +4,6 @@ import ( "net/http" "net/url" - openshell "github.com/NVIDIA/OpenShell/sdk/go/openshell/v1" - "github.com/Gkrumbach07/openshell-dashboard/backend/pkg/apiutils" "github.com/Gkrumbach07/openshell-dashboard/backend/pkg/models" "github.com/Gkrumbach07/openshell-dashboard/backend/pkg/services" @@ -28,11 +26,29 @@ func NewWorkspacesHandler(svc services.WorkspaceServiceInterface) *WorkspacesHan } func (h *WorkspacesHandler) ListWorkspaces(w http.ResponseWriter, r *http.Request) { - var opts []openshell.ListOptions - if sel := r.URL.Query().Get("labelSelector"); sel != "" { - opts = append(opts, openshell.ListOptions{LabelSelector: sel}) + opts, paged, valid := listRequestOptions(w, r) + if !valid { + return + } + if paged { + pager, err := h.svc.List(opts) + if err != nil { + apiutils.WriteSDKError(w, err) + return + } + page, err := pager.NextPage(r.Context()) + if err != nil { + apiutils.WriteSDKError(w, err) + return + } + items := make([]models.Workspace, 0, len(page.Items)) + for _, workspace := range page.Items { + items = append(items, models.FromSDKWorkspace(workspace)) + } + apiutils.WriteJSON(w, http.StatusOK, listPageResponse[models.Workspace]{Items: items, NextPageToken: page.NextPageToken}) + return } - workspaces, err := h.svc.ListAll(r.Context(), opts...) + workspaces, err := h.svc.ListAll(r.Context(), opts) if err != nil { apiutils.WriteSDKError(w, err) return diff --git a/backend/test/compat/contract_test.go b/backend/test/compat/contract_test.go index 1ba16286..95b01fc2 100644 --- a/backend/test/compat/contract_test.go +++ b/backend/test/compat/contract_test.go @@ -68,10 +68,9 @@ func TestDeleteEnvelopeContract(t *testing.T) { } } -// TestListEndpointsReturnArrays guards the pagination migration. The SDK's -// List now returns a *Pager[T]; handlers must use ListAll so the BFF keeps -// emitting a plain JSON array. If a handler regressed to List, this catches -// it as an object-instead-of-array decode. +// TestListEndpointsReturnArrays guards the legacy response contract. Requests +// without pageSize still emit plain arrays for existing consumers; requests +// with pageSize use the explicit page envelope tested separately. func TestListEndpointsReturnArrays(t *testing.T) { paths := []string{ "/api/v1/workspaces", diff --git a/backend/test/compat/harness_test.go b/backend/test/compat/harness_test.go index bd566744..c0142bb5 100644 --- a/backend/test/compat/harness_test.go +++ b/backend/test/compat/harness_test.go @@ -114,7 +114,7 @@ // listed across workspaces, each carrying its // own workspace; a workspace's endpoints; the // unnamed service endpoint allworkspaces_test.go -// list shapes lists are JSON arrays, not pager envelopes contract_test.go +// list shapes legacy arrays and opt-in cursor pages contract_test.go, list_pagination_test.go // // Most of these tests run in a workspace other than "default" on purpose. // Every workspace-scoped call carries a workspace selector, and only a second diff --git a/backend/test/compat/list_pagination_test.go b/backend/test/compat/list_pagination_test.go new file mode 100644 index 00000000..820f24ac --- /dev/null +++ b/backend/test/compat/list_pagination_test.go @@ -0,0 +1,73 @@ +//go:build compat + +package compat + +import ( + "net/http" + "net/url" + "sort" + "testing" +) + +type gatewayPage[T any] struct { + Items []T `json:"items"` + NextPageToken string `json:"nextPageToken"` +} + +func TestWorkspaceCursorPagination(t *testing.T) { + tag := randName("page") + names := []string{randName("pgw"), randName("pgw")} + for _, name := range names { + mustJSON(t, http.MethodPost, "/api/v1/workspaces", + map[string]any{"name": name, "labels": map[string]string{"compat-page": tag}}, nil, http.StatusCreated) + t.Cleanup(func() { removeWorkspace(name) }) + } + + path := "/api/v1/workspaces?pageSize=1&labelSelector=compat-page%3D" + tag + var first gatewayPage[workspace] + mustJSON(t, http.MethodGet, path, nil, &first, http.StatusOK) + if len(first.Items) != 1 || first.NextPageToken == "" { + t.Fatalf("first workspace page: %+v, want one item and a token", first) + } + var second gatewayPage[workspace] + mustJSON(t, http.MethodGet, path+"&pageToken="+url.QueryEscape(first.NextPageToken), nil, &second, http.StatusOK) + if len(second.Items) != 1 || second.NextPageToken != "" { + t.Fatalf("last workspace page: %+v, want one item and no token", second) + } + got := []string{first.Items[0].Metadata.Name, second.Items[0].Metadata.Name} + sort.Strings(got) + sort.Strings(names) + if got[0] != names[0] || got[1] != names[1] { + t.Errorf("paged workspace names = %v, want %v", got, names) + } +} + +func TestSandboxCursorPaginationAndScope(t *testing.T) { + requireSandboxes(t) + pair := newSandboxPair(t) + selector := "compat-tag%3D" + pair.tag + path := "/api/v1/sandboxes?pageSize=1&labelSelector=" + selector + var first gatewayPage[sandbox] + mustJSON(t, http.MethodGet, path, nil, &first, http.StatusOK) + if len(first.Items) != 1 || first.NextPageToken == "" { + t.Fatalf("first all-workspaces sandbox page: %+v, want one item and a token", first) + } + var second gatewayPage[sandbox] + mustJSON(t, http.MethodGet, path+"&pageToken="+url.QueryEscape(first.NextPageToken), nil, &second, http.StatusOK) + if len(second.Items) != 1 || second.NextPageToken != "" { + t.Fatalf("last all-workspaces sandbox page: %+v, want one item and no token", second) + } + got := []string{first.Items[0].Metadata.Workspace, second.Items[0].Metadata.Workspace} + sort.Strings(got) + want := []string{pair.wsA, pair.wsB} + sort.Strings(want) + if got[0] != want[0] || got[1] != want[1] { + t.Errorf("paged sandbox workspaces = %v, want %v", got, want) + } + + var scoped gatewayPage[sandbox] + mustJSON(t, http.MethodGet, sandboxesPath(pair.wsA)+"?pageSize=1&labelSelector="+selector, nil, &scoped, http.StatusOK) + if len(scoped.Items) != 1 || scoped.Items[0].Metadata.Workspace != pair.wsA || scoped.NextPageToken != "" { + t.Errorf("scoped sandbox page = %+v, want one sandbox in %s and no token", scoped, pair.wsA) + } +} diff --git a/backend/test/compat/workspace_test.go b/backend/test/compat/workspace_test.go index 5eced649..b30eefbe 100644 --- a/backend/test/compat/workspace_test.go +++ b/backend/test/compat/workspace_test.go @@ -65,8 +65,8 @@ func TestWorkspaceLifecycle(t *testing.T) { name := randName("cw") t.Run("list includes default", func(t *testing.T) { - // A JSON array here (not a pager envelope) is the contract the - // frontend depends on; see ListAll in .claude/rules/openshell-api.md. + // An unpaged request keeps the array contract for existing clients; + // the dashboard uses the opt-in page envelope. var list []workspace mustJSON(t, http.MethodGet, "/api/v1/workspaces", nil, &list, http.StatusOK) for _, ws := range list { diff --git a/frontend/cypress/e2e/sandbox-lifecycle.cy.ts b/frontend/cypress/e2e/sandbox-lifecycle.cy.ts index 4b62bc14..2c27c337 100644 --- a/frontend/cypress/e2e/sandbox-lifecycle.cy.ts +++ b/frontend/cypress/e2e/sandbox-lifecycle.cy.ts @@ -7,7 +7,7 @@ describe('Sandbox Lifecycle', () => { cy.visit('/workspaces/default'); cy.wait('@authConfig'); cy.wait('@whoami'); - cy.wait('@listSandboxes'); + cy.wait('@listSandboxes').its('request.query.pageSize').should('eq', '10'); // Switch to table/list view for consistent selectors cy.get('[data-testid="view-toggle-list"]').click(); }); diff --git a/frontend/cypress/e2e/workspace-list.cy.ts b/frontend/cypress/e2e/workspace-list.cy.ts index fe889a88..1a74c1f0 100644 --- a/frontend/cypress/e2e/workspace-list.cy.ts +++ b/frontend/cypress/e2e/workspace-list.cy.ts @@ -7,7 +7,7 @@ describe('Workspace List', () => { cy.visit('/workspaces'); cy.wait('@authConfig'); cy.wait('@whoami'); - cy.wait('@listWorkspaces'); + cy.wait('@listWorkspaces').its('request.query.pageSize').should('eq', '10'); }); it('displays workspace list heading', () => { @@ -40,10 +40,13 @@ describe('Workspace List', () => { }); it('shows error state when workspaces fail to load', () => { - cy.intercept('GET', '/api/v1/workspaces', { - statusCode: 500, - body: { code: 'INTERNAL', message: 'Internal error' }, - }).as('workspacesError'); + cy.intercept( + { method: 'GET', pathname: '/api/v1/workspaces' }, + { + statusCode: 500, + body: { code: 'INTERNAL', message: 'Internal error' }, + }, + ).as('workspacesError'); cy.visit('/workspaces'); cy.wait('@workspacesError'); diff --git a/frontend/cypress/fixtures/sandboxes.json b/frontend/cypress/fixtures/sandboxes.json index 3628b704..81b12324 100644 --- a/frontend/cypress/fixtures/sandboxes.json +++ b/frontend/cypress/fixtures/sandboxes.json @@ -1,99 +1,112 @@ -[ - { - "metadata": { - "id": "sbx-1-id", - "name": "my-agent", - "workspace": "default", - "labels": { "purpose": "agent" }, - "annotations": {}, - "createdAtMs": 1722700000000, - "resourceVersion": 3 - }, - "spec": { - "image": "ghcr.io/nvidia/openshell-community/sandboxes/base:latest", - "providers": ["anthropic"], - "policy": { - "filesystem": { "includeWorkdir": true }, - "networkPolicies": {} +{ + "items": [ + { + "metadata": { + "id": "sbx-1-id", + "name": "my-agent", + "workspace": "default", + "labels": { + "purpose": "agent" + }, + "annotations": {}, + "createdAtMs": 1722700000000, + "resourceVersion": 3 + }, + "spec": { + "image": "ghcr.io/nvidia/openshell-community/sandboxes/base:latest", + "providers": ["anthropic"], + "policy": { + "filesystem": { + "includeWorkdir": true + }, + "networkPolicies": {} + } + }, + "status": { + "phase": "READY", + "currentPolicyVersion": 1 } }, - "status": { - "phase": "READY", - "currentPolicyVersion": 1 - } - }, - { - "metadata": { - "id": "sbx-2-id", - "name": "data-processor", - "workspace": "default", - "labels": {}, - "annotations": {}, - "createdAtMs": 1722700100000, - "resourceVersion": 1 - }, - "spec": { - "image": "ghcr.io/nvidia/openshell-community/sandboxes/base:latest", - "policy": { - "filesystem": { "includeWorkdir": true }, - "networkPolicies": {} + { + "metadata": { + "id": "sbx-2-id", + "name": "data-processor", + "workspace": "default", + "labels": {}, + "annotations": {}, + "createdAtMs": 1722700100000, + "resourceVersion": 1 + }, + "spec": { + "image": "ghcr.io/nvidia/openshell-community/sandboxes/base:latest", + "policy": { + "filesystem": { + "includeWorkdir": true + }, + "networkPolicies": {} + } + }, + "status": { + "phase": "PROVISIONING", + "currentPolicyVersion": 1 } }, - "status": { - "phase": "PROVISIONING", - "currentPolicyVersion": 1 - } - }, - { - "metadata": { - "id": "sbx-3-id", - "name": "broken-sandbox", - "workspace": "default", - "labels": {}, - "annotations": {}, - "createdAtMs": 1722700200000, - "resourceVersion": 2 - }, - "spec": { - "image": "ghcr.io/nvidia/openshell-community/sandboxes/base:latest", - "policy": { - "filesystem": { "includeWorkdir": true }, - "networkPolicies": {} + { + "metadata": { + "id": "sbx-3-id", + "name": "broken-sandbox", + "workspace": "default", + "labels": {}, + "annotations": {}, + "createdAtMs": 1722700200000, + "resourceVersion": 2 + }, + "spec": { + "image": "ghcr.io/nvidia/openshell-community/sandboxes/base:latest", + "policy": { + "filesystem": { + "includeWorkdir": true + }, + "networkPolicies": {} + } + }, + "status": { + "phase": "ERROR", + "currentPolicyVersion": 1, + "conditions": [ + { + "type": "Ready", + "status": "False", + "reason": "ImagePullBackOff", + "message": "Failed to pull image" + } + ] } }, - "status": { - "phase": "ERROR", - "currentPolicyVersion": 1, - "conditions": [ - { - "type": "Ready", - "status": "False", - "reason": "ImagePullBackOff", - "message": "Failed to pull image" + { + "metadata": { + "id": "sbx-4-id", + "name": "cleanup-target", + "workspace": "default", + "labels": {}, + "annotations": {}, + "createdAtMs": 1722700300000, + "resourceVersion": 1 + }, + "spec": { + "image": "ghcr.io/nvidia/openshell-community/sandboxes/base:latest", + "policy": { + "filesystem": { + "includeWorkdir": true + }, + "networkPolicies": {} } - ] - } - }, - { - "metadata": { - "id": "sbx-4-id", - "name": "cleanup-target", - "workspace": "default", - "labels": {}, - "annotations": {}, - "createdAtMs": 1722700300000, - "resourceVersion": 1 - }, - "spec": { - "image": "ghcr.io/nvidia/openshell-community/sandboxes/base:latest", - "policy": { - "filesystem": { "includeWorkdir": true }, - "networkPolicies": {} + }, + "status": { + "phase": "DELETING", + "currentPolicyVersion": 1 } - }, - "status": { - "phase": "DELETING", - "currentPolicyVersion": 1 } - } -] + ], + "nextPageToken": "" +} diff --git a/frontend/cypress/fixtures/workspaces.json b/frontend/cypress/fixtures/workspaces.json index c39c3748..248b2d02 100644 --- a/frontend/cypress/fixtures/workspaces.json +++ b/frontend/cypress/fixtures/workspaces.json @@ -1,35 +1,40 @@ -[ - { - "metadata": { - "id": "ws-default-id", - "name": "default", - "labels": {}, - "annotations": {}, - "createdAtMs": 1722700000000, - "resourceVersion": 1 +{ + "items": [ + { + "metadata": { + "id": "ws-default-id", + "name": "default", + "labels": {}, + "annotations": {}, + "createdAtMs": 1722700000000, + "resourceVersion": 1 + }, + "phase": "ACTIVE" }, - "phase": "ACTIVE" - }, - { - "metadata": { - "id": "ws-dev-id", - "name": "dev-team", - "labels": { "env": "development" }, - "annotations": {}, - "createdAtMs": 1722700100000, - "resourceVersion": 2 + { + "metadata": { + "id": "ws-dev-id", + "name": "dev-team", + "labels": { + "env": "development" + }, + "annotations": {}, + "createdAtMs": 1722700100000, + "resourceVersion": 2 + }, + "phase": "ACTIVE" }, - "phase": "ACTIVE" - }, - { - "metadata": { - "id": "ws-staging-id", - "name": "staging", - "labels": {}, - "annotations": {}, - "createdAtMs": 1722700200000, - "resourceVersion": 1 - }, - "phase": "ACTIVE" - } -] + { + "metadata": { + "id": "ws-staging-id", + "name": "staging", + "labels": {}, + "annotations": {}, + "createdAtMs": 1722700200000, + "resourceVersion": 1 + }, + "phase": "ACTIVE" + } + ], + "nextPageToken": "" +} diff --git a/frontend/cypress/support/intercepts.ts b/frontend/cypress/support/intercepts.ts index 0fb6f65b..6e46e5aa 100644 --- a/frontend/cypress/support/intercepts.ts +++ b/frontend/cypress/support/intercepts.ts @@ -22,9 +22,12 @@ export const interceptGateway = () => { }; export const interceptWorkspaces = () => { - cy.intercept('GET', '/api/v1/workspaces', { - fixture: 'workspaces.json', - }).as('listWorkspaces'); + cy.intercept( + { method: 'GET', pathname: '/api/v1/workspaces' }, + { + fixture: 'workspaces.json', + }, + ).as('listWorkspaces'); cy.intercept('GET', '/api/v1/workspaces/default', { statusCode: 200, @@ -71,9 +74,12 @@ export const interceptWorkspaces = () => { }; export const interceptSandboxes = (workspace = 'default') => { - cy.intercept('GET', `/api/v1/workspaces/${workspace}/sandboxes`, { - fixture: 'sandboxes.json', - }).as('listSandboxes'); + cy.intercept( + { method: 'GET', pathname: `/api/v1/workspaces/${workspace}/sandboxes` }, + { + fixture: 'sandboxes.json', + }, + ).as('listSandboxes'); cy.intercept( 'GET', diff --git a/frontend/src/api/__tests__/draftSummary.spec.tsx b/frontend/src/api/__tests__/draftSummary.spec.tsx index 6c904763..4e61514b 100644 --- a/frontend/src/api/__tests__/draftSummary.spec.tsx +++ b/frontend/src/api/__tests__/draftSummary.spec.tsx @@ -38,16 +38,19 @@ const summary: DraftSummary = { ], }; -const setup = (workspace: string, enabled?: boolean) => { +const setup = (workspace: string, enabled?: boolean, names?: string[]) => { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } }, }); const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => ( {children} ); - const hook = renderHook(() => useWorkspaceDraftSummary(workspace, enabled), { - wrapper, - }); + const hook = renderHook( + () => useWorkspaceDraftSummary(workspace, enabled, names), + { + wrapper, + }, + ); return { ...hook, queryClient }; }; @@ -64,6 +67,27 @@ describe('workspace draft summary', () => { ); }); + it('sends only the names on the visible page', async () => { + await getWorkspaceDraftSummary('team a', ['busy', 'quiet']); + expect(mockGet).toHaveBeenCalledWith( + '/api/v1/workspaces/team%20a/draft-summary?sandbox=busy&sandbox=quiet', + ); + expect(policyKeys.workspaceDraftSummary('team-a', ['busy'])).not.toEqual( + policyKeys.workspaceDraftSummary('team-a', ['quiet']), + ); + }); + + it('makes no summary request when the page has no rows', async () => { + expect(await getWorkspaceDraftSummary('team-a', [])).toEqual({ + sandboxes: [], + totalPending: 0, + }); + const { result } = setup('team-a', true, []); + await new Promise((resolve) => setTimeout(resolve, 20)); + expect(mockGet).not.toHaveBeenCalled(); + expect(result.current.bySandbox).toEqual({}); + }); + it('keys the summary by workspace, under the key of every summary', () => { const key = policyKeys.workspaceDraftSummary('team-a'); expect(key).not.toEqual(policyKeys.workspaceDraftSummary('team-b')); diff --git a/frontend/src/api/__tests__/platform-paths.spec.ts b/frontend/src/api/__tests__/platform-paths.spec.ts index 01837e55..d367aac9 100644 --- a/frontend/src/api/__tests__/platform-paths.spec.ts +++ b/frontend/src/api/__tests__/platform-paths.spec.ts @@ -1,12 +1,17 @@ import { listAllProviders, listAllSandboxes, + listAllSandboxPage, listAllServices, listAllTemplates, } from '../allWorkspaces'; -import { allWorkspacesKeys, workspaceKeys } from '../queryKeys'; -import { deleteService, getSandboxLogs } from '../sandboxes'; -import { listWorkspaceServices, listWorkspaces } from '../workspaces'; +import { allWorkspacesKeys, sandboxKeys, workspaceKeys } from '../queryKeys'; +import { deleteService, getSandboxLogs, listSandboxPage } from '../sandboxes'; +import { + listWorkspaceServices, + listWorkspaces, + listWorkspacePage, +} from '../workspaces'; jest.mock('../client', () => ({ apiFetch: jest.fn(), @@ -42,6 +47,30 @@ describe('all-workspaces API', () => { ); }); + it('requests one sandbox page with an opaque token and selector', async () => { + await listAllSandboxPage({ + pageSize: 20, + pageToken: 'a+/=', + labelSelector: 'type=tool,team=ml', + }); + expect(mockGet).toHaveBeenCalledWith( + '/api/v1/sandboxes?pageSize=20&pageToken=a%2B%2F%3D&labelSelector=type%3Dtool%2Cteam%3Dml', + ); + expect( + allWorkspacesKeys.sandboxPage({ + pageSize: 20, + pageToken: 'a+/=', + labelSelector: 'type=tool,team=ml', + }), + ).not.toEqual( + allWorkspacesKeys.sandboxPage({ + pageSize: 20, + pageToken: '', + labelSelector: 'type=tool,team=ml', + }), + ); + }); + it('listAllProviders calls the top-level path', async () => { await listAllProviders(); expect(mockGet).toHaveBeenCalledWith('/api/v1/providers'); @@ -66,6 +95,39 @@ describe('all-workspaces API', () => { }); describe('workspaces API', () => { + it('requests one workspace page and keeps each token in its cache key', async () => { + await listWorkspacePage({ + pageSize: 10, + pageToken: 'next', + labelSelector: 'env=staging', + }); + expect(mockGet).toHaveBeenCalledWith( + '/api/v1/workspaces?pageSize=10&pageToken=next&labelSelector=env%3Dstaging', + ); + expect( + workspaceKeys.page({ + pageSize: 10, + pageToken: 'next', + labelSelector: 'env=staging', + }), + ).not.toEqual( + workspaceKeys.page({ + pageSize: 10, + pageToken: '', + labelSelector: 'env=staging', + }), + ); + }); + + it('requests one workspace-scoped sandbox page', async () => { + await listSandboxPage('team/a', { pageSize: 10 }); + expect(mockGet).toHaveBeenCalledWith( + '/api/v1/workspaces/team%2Fa/sandboxes?pageSize=10', + ); + expect(sandboxKeys.page('team/a', { pageSize: 10 })).not.toEqual( + sandboxKeys.page('team/b', { pageSize: 10 }), + ); + }); it('listWorkspaces passes a label selector, encoded', async () => { await listWorkspaces('env=staging,owner=ml'); expect(mockGet).toHaveBeenCalledWith( diff --git a/frontend/src/api/__tests__/queryKeys.spec.ts b/frontend/src/api/__tests__/queryKeys.spec.ts index dc88281d..563f623a 100644 --- a/frontend/src/api/__tests__/queryKeys.spec.ts +++ b/frontend/src/api/__tests__/queryKeys.spec.ts @@ -1,6 +1,11 @@ import { QueryClient, hashKey } from '@tanstack/react-query'; -import { sandboxKeys, templateKeys } from '../queryKeys'; +import { + allWorkspacesKeys, + sandboxKeys, + templateKeys, + workspaceKeys, +} from '../queryKeys'; // A label selector is text the user types. A list keyed by it must never be // the same cache entry as a sandbox or a template, whatever was typed. @@ -84,3 +89,41 @@ describe.each(families)('$kind query keys', ({ list, detail, scope }) => { ); }); }); + +it('keeps cursor pages distinct and under mutation invalidation prefixes', async () => { + const client = new QueryClient(); + const pages = [ + sandboxKeys.page('team-a', { + pageSize: 10, + pageToken: '', + labelSelector: 'type=tool', + }), + sandboxKeys.page('team-a', { + pageSize: 10, + pageToken: 'next', + labelSelector: 'type=tool', + }), + sandboxKeys.page('team-a', { + pageSize: 20, + pageToken: '', + labelSelector: 'type=tool', + }), + ]; + expect(new Set(pages.map(hashKey)).size).toBe(3); + pages.forEach((key) => client.setQueryData(key, 'cached')); + client.setQueryData(sandboxKeys.page('team-b', { pageSize: 10 }), 'other'); + await client.invalidateQueries({ queryKey: sandboxKeys.scope('team-a') }); + pages.forEach((key) => + expect(client.getQueryState(key)?.isInvalidated).toBe(true), + ); + expect( + client.getQueryState(sandboxKeys.page('team-b', { pageSize: 10 })) + ?.isInvalidated, + ).toBe(false); + expect(hashKey(workspaceKeys.page({ pageSize: 10 }))).not.toBe( + hashKey(workspaceKeys.detail('10')), + ); + expect(hashKey(allWorkspacesKeys.sandboxPage({ pageSize: 10 }))).not.toBe( + hashKey(allWorkspacesKeys.sandboxes('')), + ); +}); diff --git a/frontend/src/api/allWorkspaces.ts b/frontend/src/api/allWorkspaces.ts index 3a13172c..04c1f7e0 100644 --- a/frontend/src/api/allWorkspaces.ts +++ b/frontend/src/api/allWorkspaces.ts @@ -3,6 +3,8 @@ import { keepPreviousData, useQuery } from '@tanstack/react-query'; import { RESOURCE_POLL_MS, SANDBOX_POLL_MS } from '../constants'; import { get } from './client'; import { allWorkspacesKeys } from './queryKeys'; +import { pagedPath } from './pagination'; +import type { ListPage, PageRequest } from './pagination'; import type { Provider, Sandbox, @@ -24,6 +26,19 @@ const withLabelSelector = (path: string, labelSelector?: string): string => export const listAllSandboxes = (labelSelector?: string): Promise => get(withLabelSelector('/api/v1/sandboxes', labelSelector)); +export const listAllSandboxPage = ( + request: PageRequest, +): Promise> => + get>(pagedPath('/api/v1/sandboxes', request)); + +export const useAllSandboxPage = (request: PageRequest) => + useQuery({ + queryKey: allWorkspacesKeys.sandboxPage(request), + queryFn: () => listAllSandboxPage(request), + placeholderData: keepPreviousData, + refetchInterval: SANDBOX_POLL_MS, + }); + // The gateway has no filter for providers, in a workspace or across them. export const listAllProviders = (): Promise => get('/api/v1/providers'); diff --git a/frontend/src/api/draftSummary.ts b/frontend/src/api/draftSummary.ts index b42ccdac..f247b633 100644 --- a/frontend/src/api/draftSummary.ts +++ b/frontend/src/api/draftSummary.ts @@ -11,21 +11,31 @@ import type { DraftSandboxSummary, DraftSummary } from '../types'; // it from one draft read per sandbox, the way the TUI does. export const getWorkspaceDraftSummary = ( workspace: string, -): Promise => - get( - `/api/v1/workspaces/${encodeURIComponent(workspace)}/draft-summary`, - ); + names?: readonly string[], +): Promise => { + const path = `/api/v1/workspaces/${encodeURIComponent(workspace)}/draft-summary`; + if (!names) return get(path); + if (names.length === 0) + return Promise.resolve({ sandboxes: [], totalPending: 0 }); + const params = new URLSearchParams(); + names.forEach((name) => params.append('sandbox', name)); + return get(`${path}?${params.toString()}`); +}; // bySandbox holds an entry for each sandbox that has pending chunks or whose // inbox could not be read (`unavailable`). A sandbox without one has none // pending. Pass enabled=false where draft policies are switched off: nothing // is requested and every sandbox reads as having none. -export const useWorkspaceDraftSummary = (workspace: string, enabled = true) => { +export const useWorkspaceDraftSummary = ( + workspace: string, + enabled = true, + names?: readonly string[], +) => { const query = useQuery({ - queryKey: policyKeys.workspaceDraftSummary(workspace), - queryFn: () => getWorkspaceDraftSummary(workspace), + queryKey: policyKeys.workspaceDraftSummary(workspace, names), + queryFn: () => getWorkspaceDraftSummary(workspace, names), refetchInterval: DRAFT_SUMMARY_POLL_MS, - enabled, + enabled: enabled && (names === undefined || names.length > 0), }); const items = query.data?.sandboxes; diff --git a/frontend/src/api/index.ts b/frontend/src/api/index.ts index da647581..9178bc53 100644 --- a/frontend/src/api/index.ts +++ b/frontend/src/api/index.ts @@ -1,6 +1,7 @@ // The API client and hooks, re-exported from one place. Nothing in the app // imports from here: each import names the source module. export * from './client'; +export * from './pagination'; export * from './auth'; export * from './gateway'; export * from './workspaces'; diff --git a/frontend/src/api/pagination.ts b/frontend/src/api/pagination.ts new file mode 100644 index 00000000..c862014d --- /dev/null +++ b/frontend/src/api/pagination.ts @@ -0,0 +1,18 @@ +export type PageRequest = { + pageSize: number; + pageToken?: string; + labelSelector?: string; +}; + +export type ListPage = { + items: T[]; + nextPageToken: string; +}; + +export const pagedPath = (path: string, request: PageRequest): string => { + const params = new URLSearchParams(); + params.set('pageSize', String(request.pageSize)); + if (request.pageToken) params.set('pageToken', request.pageToken); + if (request.labelSelector) params.set('labelSelector', request.labelSelector); + return `${path}?${params.toString()}`; +}; diff --git a/frontend/src/api/queryKeys.ts b/frontend/src/api/queryKeys.ts index b5ed1d95..91228cc1 100644 --- a/frontend/src/api/queryKeys.ts +++ b/frontend/src/api/queryKeys.ts @@ -1,4 +1,5 @@ import type { LogFilters } from '../types'; +import type { PageRequest } from './pagination'; export const sandboxKeys = { all: ['sandboxes'] as const, @@ -9,6 +10,16 @@ export const sandboxKeys = { // start with `scope`, which is what the mutations invalidate. list: (workspace: string, labelSelector = '') => ['sandboxes', workspace, { labelSelector }] as const, + page: (workspace: string, request: PageRequest) => + [ + 'sandboxes', + workspace, + { + pageSize: request.pageSize, + pageToken: request.pageToken ?? '', + labelSelector: request.labelSelector ?? '', + }, + ] as const, detail: (workspace: string, name: string) => ['sandboxes', workspace, name] as const, // Every list of a workspace and every sandbox in it. @@ -45,6 +56,15 @@ export const workspaceKeys = { labelSelector ? (['workspaces', { labelSelector }] as const) : (['workspaces'] as const), + page: (request: PageRequest) => + [ + 'workspaces', + { + pageSize: request.pageSize, + pageToken: request.pageToken ?? '', + labelSelector: request.labelSelector ?? '', + }, + ] as const, detail: (name: string) => ['workspaces', name] as const, members: (workspace: string) => ['members', workspace] as const, services: (workspace: string) => ['workspace-services', workspace] as const, @@ -61,6 +81,16 @@ export const allWorkspacesKeys = { allSandboxes: ['all-workspaces', 'sandboxes'] as const, sandboxes: (labelSelector = '') => ['all-workspaces', 'sandboxes', labelSelector] as const, + sandboxPage: (request: PageRequest) => + [ + 'all-workspaces', + 'sandboxes', + { + pageSize: request.pageSize, + pageToken: request.pageToken ?? '', + labelSelector: request.labelSelector ?? '', + }, + ] as const, providers: ['all-workspaces', 'providers'] as const, allTemplates: ['all-workspaces', 'templates'] as const, templates: (labelSelector = '') => @@ -104,8 +134,10 @@ export const policyKeys = { draftSummary: ['draft-summary'] as const, // One workspace's summary. Prefixed by `draftSummary`, so invalidating that // covers every workspace's. - workspaceDraftSummary: (workspace: string) => - ['draft-summary', workspace] as const, + workspaceDraftSummary: (workspace: string, names?: readonly string[]) => + names + ? (['draft-summary', workspace, { sandboxNames: names }] as const) + : (['draft-summary', workspace] as const), }; export const gatewayKeys = { diff --git a/frontend/src/api/sandboxes.ts b/frontend/src/api/sandboxes.ts index b451035e..22ec2754 100644 --- a/frontend/src/api/sandboxes.ts +++ b/frontend/src/api/sandboxes.ts @@ -1,4 +1,9 @@ -import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; +import { + keepPreviousData, + useMutation, + useQuery, + useQueryClient, +} from '@tanstack/react-query'; import type { QueryClient } from '@tanstack/react-query'; import { SANDBOX_POLL_MS } from '../constants'; @@ -12,6 +17,8 @@ import { sessionExpiredError, } from './client'; import { policyKeys, sandboxKeys } from './queryKeys'; +import { pagedPath } from './pagination'; +import type { ListPage, PageRequest } from './pagination'; import { LogEvent, type CreateSandboxRequest, @@ -35,6 +42,25 @@ export const listSandboxes = ( }`, ); +export const listSandboxPage = ( + workspace: string, + request: PageRequest, +): Promise> => + get>( + pagedPath( + `/api/v1/workspaces/${encodeURIComponent(workspace)}/sandboxes`, + request, + ), + ); + +export const useSandboxPage = (workspace: string, request: PageRequest) => + useQuery({ + queryKey: sandboxKeys.page(workspace, request), + queryFn: () => listSandboxPage(workspace, request), + placeholderData: keepPreviousData, + refetchInterval: SANDBOX_POLL_MS, + }); + export const getSandbox = (workspace: string, name: string): Promise => get( `/api/v1/workspaces/${encodeURIComponent(workspace)}/sandboxes/${encodeURIComponent(name)}`, diff --git a/frontend/src/api/workspaces.ts b/frontend/src/api/workspaces.ts index 31b177e5..003e9983 100644 --- a/frontend/src/api/workspaces.ts +++ b/frontend/src/api/workspaces.ts @@ -8,6 +8,8 @@ import { import { RESOURCE_POLL_MS } from '../constants'; import { del, get, post } from './client'; import { allWorkspacesKeys, workspaceKeys } from './queryKeys'; +import { pagedPath } from './pagination'; +import type { ListPage, PageRequest } from './pagination'; import type { AddMemberRequest, CreateWorkspaceRequest, @@ -26,6 +28,19 @@ export const listWorkspaces = (labelSelector?: string): Promise => }`, ); +export const listWorkspacePage = ( + request: PageRequest, +): Promise> => + get>(pagedPath('/api/v1/workspaces', request)); + +export const useWorkspacePage = (request: PageRequest) => + useQuery({ + queryKey: workspaceKeys.page(request), + queryFn: () => listWorkspacePage(request), + placeholderData: keepPreviousData, + refetchInterval: RESOURCE_POLL_MS, + }); + export const getWorkspace = (name: string): Promise => get(`/api/v1/workspaces/${encodeURIComponent(name)}`); diff --git a/frontend/src/components/AllSandboxList.tsx b/frontend/src/components/AllSandboxList.tsx new file mode 100644 index 00000000..0f436461 --- /dev/null +++ b/frontend/src/components/AllSandboxList.tsx @@ -0,0 +1,146 @@ +import { useState } from 'react'; +import { Button } from '@patternfly/react-core'; + +import { useAllSandboxPage } from '../api/allWorkspaces'; +import { + isCursorPagePending, + useCursorListPage, + useCursorPageResult, +} from '../hooks/useCursorListPage'; +import { combineSandboxLabelSelector } from '../utils/sandboxLabelSelector'; +import { formatAge } from '../utils/formatters'; +import type { Sandbox } from '../types'; +import { ImageReference, scopedName, workspaceOf } from './AllWorkspaceCells'; +import LabelsList from './LabelsList'; +import PhaseLabel from './PhaseLabel'; +import ResourceTable from './ResourceTable'; +import type { ResourceTableColumn } from './ResourceTable'; + +type AllSandboxListProps = { + onSelectWorkspace: (workspace: string) => void; + onSelectSandbox: (workspace: string, name: string) => void; +}; + +// The sandbox tab owns its Gateway cursor, type-label filter and columns. +// Other all-workspaces resources continue to use local table pagination. +const AllSandboxList: React.FC = ({ + onSelectWorkspace, + onSelectSandbox, +}) => { + const [selector, setSelector] = useState(''); + const [typeLabel, setTypeLabel] = useState(''); + const [filterError, setFilterError] = useState(''); + const pager = useCursorListPage(`${selector}\0${typeLabel}`); + const combinedSelector = combineSandboxLabelSelector( + selector, + typeLabel, + ).selector; + const sandboxes = useAllSandboxPage({ + pageSize: pager.perPage, + pageToken: pager.pageToken, + labelSelector: combinedSelector || undefined, + }); + useCursorPageResult(pager, sandboxes); + + const applyFilters = (generic: string, type: string) => { + const result = combineSandboxLabelSelector(generic, type); + if (result.error) { + setFilterError(result.error); + return; + } + setFilterError(''); + setSelector(generic.trim()); + setTypeLabel(type.trim()); + }; + + const columns: ResourceTableColumn[] = [ + { + title: 'Workspace', + modifier: 'nowrap', + render: (sandbox) => ( + + ), + }, + { + title: 'Name', + modifier: 'nowrap', + render: (sandbox) => ( + + ), + }, + { + title: 'Status', + render: (sandbox) => ( + + ), + }, + { + title: 'Image', + modifier: 'truncate', + render: (sandbox) => , + }, + { + title: 'Labels', + render: (sandbox) => ( + + ), + }, + { + title: 'Age', + render: (sandbox) => formatAge(sandbox.metadata.createdAtMs), + }, + ]; + + return ( + applyFilters(value, typeLabel), + }} + typeLabel={{ + value: typeLabel, + onApply: (value) => applyFilters(selector, value), + error: filterError, + }} + cursorPage={{ + page: pager.page, + perPage: pager.perPage, + hasNext: Boolean(sandboxes.data?.nextPageToken), + isLoading: isCursorPagePending(sandboxes, pager.isTransitioning), + isTransitioning: pager.isTransitioning, + onPrevious: pager.previous, + onNext: () => pager.next(sandboxes.data?.nextPageToken ?? ''), + onPerPageSelect: pager.onPerPageSelect, + onReset: pager.reset, + }} + /> + ); +}; + +export default AllSandboxList; diff --git a/frontend/src/components/AllWorkspaceCells.tsx b/frontend/src/components/AllWorkspaceCells.tsx new file mode 100644 index 00000000..aab34145 --- /dev/null +++ b/frontend/src/components/AllWorkspaceCells.tsx @@ -0,0 +1,17 @@ +export const workspaceOf = (item: { + metadata: { workspace?: string }; +}): string => item.metadata.workspace ?? ''; + +// Names are unique only within a workspace. +export const scopedName = (item: { + metadata: { workspace?: string; name: string }; +}): string => `${workspaceOf(item)}/${item.metadata.name}`; + +export const ImageReference: React.FC<{ image?: string }> = ({ image }) => + image ? ( + + {image} + + ) : ( + <>- + ); diff --git a/frontend/src/components/CursorPageTransition.css b/frontend/src/components/CursorPageTransition.css new file mode 100644 index 00000000..b428b43f --- /dev/null +++ b/frontend/src/components/CursorPageTransition.css @@ -0,0 +1,15 @@ +.cursor-page-transition { + position: relative; +} + +.cursor-page-transition__content--loading { + visibility: hidden; +} + +.cursor-page-transition__loading { + position: absolute; + inset: 0; + display: flex; + align-items: center; + justify-content: center; +} diff --git a/frontend/src/components/CursorPageTransition.tsx b/frontend/src/components/CursorPageTransition.tsx new file mode 100644 index 00000000..d2b80445 --- /dev/null +++ b/frontend/src/components/CursorPageTransition.tsx @@ -0,0 +1,35 @@ +import { Spinner } from '@patternfly/react-core'; + +import './CursorPageTransition.css'; + +type CursorPageTransitionProps = { + loading: boolean; + loadingLabel: string; + children: React.ReactNode; +}; + +// Keep the table's footprint while a cursor request replaces its rows. +const CursorPageTransition: React.FC = ({ + loading, + loadingLabel, + children, +}) => ( +
+
+ {children} +
+ {loading && ( +
+ +
+ )} +
+); + +export default CursorPageTransition; diff --git a/frontend/src/components/CursorPagination.tsx b/frontend/src/components/CursorPagination.tsx new file mode 100644 index 00000000..4e631cec --- /dev/null +++ b/frontend/src/components/CursorPagination.tsx @@ -0,0 +1,143 @@ +import { useEffect, useRef, useState } from 'react'; +import type { RefObject } from 'react'; +import { + Button, + Flex, + FlexItem, + FormSelect, + FormSelectOption, +} from '@patternfly/react-core'; +import { AngleLeftIcon, AngleRightIcon } from '@patternfly/react-icons'; +import { useI18n } from '../i18n'; + +type CursorPaginationProps = { + variant: 'top' | 'bottom'; + page: number; + perPage: number; + hasNext: boolean; + isLoading?: boolean; + isError?: boolean; + resultsRef?: RefObject; + onPrevious: () => void; + onNext: () => void; + onPerPageSelect: (size: number) => void; +}; + +const CursorPagination: React.FC = ({ + variant, + page, + perPage, + hasNext, + isLoading = false, + isError = false, + resultsRef, + onPrevious, + onNext, + onPerPageSelect, +}) => { + const { t } = useI18n('common'); + const lastAnnouncedPage = useRef(page); + const pendingFocusPage = useRef(null); + const [announcement, setAnnouncement] = useState(''); + + useEffect(() => { + if (variant !== 'bottom' || isLoading || isError) return; + if (lastAnnouncedPage.current !== page) { + lastAnnouncedPage.current = page; + setAnnouncement(t('pagination.loaded', { number: page })); + } + if (pendingFocusPage.current === page) { + pendingFocusPage.current = null; + resultsRef?.current?.focus(); + } + }, [variant, page, perPage, isLoading, isError, resultsRef, t]); + + const navigate = (targetPage: number, action: () => void) => { + if (variant === 'bottom') pendingFocusPage.current = targetPage; + action(); + }; + + return ( + + {variant === 'bottom' && ( + + { + pendingFocusPage.current = 1; + onPerPageSelect(Number(value)); + }} + > + {[10, 20, 50, 100].map((size) => ( + + ))} + + + )} + + + } + > + {errorMessage} + + ); + } + + const all = query.data ?? []; + if ( + all.length === 0 && + !labelSelector?.value && + !typeLabel?.value && + !typeLabel?.error && + cursorPage.page === 1 && + !query.isLoading && + !query.isPlaceholderData && + !errorMessage + ) { + return ( + <> + {refreshNotice} + + {emptyBody} + + + ); + } + + const normalizedFilter = filter.trim().toLocaleLowerCase(); + const rows = normalizedFilter + ? all.filter((item) => + filterText(item).toLocaleLowerCase().includes(normalizedFilter), + ) + : all; + const paginationProps = { + ...cursorPage, + isLoading: cursorPage.isLoading, + isError: query.isError, + }; + + return ( + <> + {refreshNotice} + + + } breakpoint="lg"> + + setFilter(value)} + onClear={() => setFilter('')} + data-testid={`${testId}-filter`} + /> + + {labelSelector && ( + + setSelectorInput(value)} + onSearch={(_event, value) => + labelSelector.onApply(value.trim()) + } + onClear={() => { + setSelectorInput(''); + labelSelector.onApply(''); + }} + submitSearchButtonLabel="Apply label selector" + resetButtonLabel="Clear label selector" + data-testid={`${testId}-label-selector`} + /> + + )} + {typeLabel && ( + + setTypeInput(value)} + onSearch={(_event, value) => typeLabel.onApply(value.trim())} + onClear={() => { + setTypeInput(''); + typeLabel.onApply(''); + }} + submitSearchButtonLabel="Apply type label" + resetButtonLabel="Clear type label" + data-testid={`${testId}-type-label`} + /> + + )} + + + + + + + {errorMessage !== undefined && ( + + {errorMessage} + {pageFailed && ( + + )} + + )} + {typeLabel?.error && ( + + )} +

+ {t('pagination.resourceResults', { title })} +

+ {query.isLoading || + (cursorPage.isTransitioning && !query.data && !query.isError) ? ( + + + + ) : ( + + + + + {columns.map((column) => ( + + ))} + + + + {rows.map((item) => ( + + {columns.map((column) => ( + + ))} + + ))} + {rows.length === 0 && errorMessage === undefined && ( + + + + )} + +
+ {column.title} +
+ {column.render(item)} +
+ No {noun} match{' '} + {all.length === 0 ? 'this label selector' : 'on this page'}. +
+ + )} + + + ); +}; + +export default CursorResourceTable; diff --git a/frontend/src/components/ResourceTable.tsx b/frontend/src/components/ResourceTable.tsx index dca71e4b..4e60b5bc 100644 --- a/frontend/src/components/ResourceTable.tsx +++ b/frontend/src/components/ResourceTable.tsx @@ -16,6 +16,7 @@ import { CubesIcon } from '@patternfly/react-icons'; import { Table, Tbody, Td, Th, Thead, Tr } from '@patternfly/react-table'; import { useTableSelection } from '../hooks/useTableSelection'; +import CursorResourceTable from './CursorResourceTable'; import RefreshErrorAlert, { isRefreshError } from './RefreshErrorAlert'; export type ResourceTableColumn = { @@ -47,7 +48,19 @@ export type ResourceTableLabelSelector = { onApply: (selector: string) => void; }; -type ResourceTableProps = { +export type ResourceTableCursorPage = { + page: number; + perPage: number; + hasNext: boolean; + isLoading?: boolean; + isTransitioning?: boolean; + onPrevious: () => void; + onNext: () => void; + onPerPageSelect: (size: number) => void; + onReset: () => void; +}; + +export type ResourceTableProps = { // Names the table, the toolbar and the test ids: "Sandboxes", "sandboxes". title: string; testId: string; @@ -59,12 +72,14 @@ type ResourceTableProps = { filterPlaceholder: string; emptyBody: string; labelSelector?: ResourceTableLabelSelector; + typeLabel?: ResourceTableLabelSelector & { error?: string }; + cursorPage?: ResourceTableCursorPage; }; // A read-only, paginated list of resources: what a list page shows when the // rows are for looking at and following, not for acting on. The lists across // workspaces and a workspace's service endpoints are built on it. -const ResourceTable = ({ +const LocalResourceTable = ({ title, testId, query, @@ -265,4 +280,11 @@ const ResourceTable = ({ ); }; +const ResourceTable = (props: ResourceTableProps): React.ReactElement => + props.cursorPage ? ( + + ) : ( + + ); + export default ResourceTable; diff --git a/frontend/src/components/__tests__/CursorPagination.spec.tsx b/frontend/src/components/__tests__/CursorPagination.spec.tsx new file mode 100644 index 00000000..660cb6f9 --- /dev/null +++ b/frontend/src/components/__tests__/CursorPagination.spec.tsx @@ -0,0 +1,147 @@ +import { useRef, useState } from 'react'; +import { fireEvent, render, screen, within } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +import CursorPagination from '../CursorPagination'; + +const PaginationPair = () => { + const [page, setPage] = useState(1); + const [perPage, setPerPage] = useState(10); + const resultsRef = useRef(null); + const props = { + page, + perPage, + hasNext: page < 3, + onPrevious: () => setPage((current) => current - 1), + onNext: () => setPage((current) => current + 1), + onPerPageSelect: (size: number) => { + setPerPage(size); + setPage(1); + }, + }; + return ( + <> + +

+ Results +

+ + + ); +}; + +const top = () => + within(screen.getByRole('navigation', { name: 'Top pagination' })); +const bottom = () => + within(screen.getByRole('navigation', { name: 'Bottom pagination' })); + +describe('CursorPagination', () => { + it('keeps both controls in sync and only offers page size below results', () => { + render(); + expect(top().queryByRole('combobox')).not.toBeInTheDocument(); + expect( + bottom().getByRole('combobox', { name: 'Items per page' }), + ).toHaveValue('10'); + expect( + top().getByRole('button', { name: 'Go to previous page' }), + ).toBeDisabled(); + + fireEvent.click(bottom().getByRole('button', { name: 'Go to next page' })); + expect(top().getByText('Page 2')).toBeInTheDocument(); + expect(bottom().getByText('Page 2')).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: 'Results' })).toHaveFocus(); + expect(screen.getAllByRole('status')).toHaveLength(1); + expect(screen.getByRole('status')).toHaveTextContent('Page 2 loaded'); + + fireEvent.click(top().getByRole('button', { name: 'Go to next page' })); + expect( + bottom().getByRole('button', { name: 'Go to next page' }), + ).toBeDisabled(); + expect(bottom().getByText('Page 3')).toBeInTheDocument(); + expect( + screen.getByRole('navigation', { name: 'Bottom pagination' }), + ).not.toHaveTextContent('of 3'); + + fireEvent.change( + bottom().getByRole('combobox', { name: 'Items per page' }), + { + target: { value: '50' }, + }, + ); + expect(top().getByText('Page 1')).toBeInTheDocument(); + expect( + bottom().getByRole('combobox', { name: 'Items per page' }), + ).toHaveValue('50'); + expect(screen.getByRole('heading', { name: 'Results' })).toHaveFocus(); + const pageSize = bottom().getByRole('combobox', { name: 'Items per page' }); + pageSize.focus(); + fireEvent.change(pageSize, { target: { value: '20' } }); + expect(top().getByText('Page 1')).toBeInTheDocument(); + expect(screen.getByRole('heading', { name: 'Results' })).toHaveFocus(); + }); + + it('waits for a completed page and does not announce background refreshes', () => { + const props = { + variant: 'bottom' as const, + page: 1, + perPage: 10, + hasNext: true, + onPrevious: jest.fn(), + onNext: jest.fn(), + onPerPageSelect: jest.fn(), + }; + const view = render(); + view.rerender(); + expect(screen.getByRole('status')).toBeEmptyDOMElement(); + expect( + screen.getByRole('button', { name: 'Go to next page' }), + ).toBeDisabled(); + view.rerender(); + expect(screen.getByRole('status')).toHaveTextContent('Page 2 loaded'); + view.rerender(); + view.rerender(); + expect(screen.getByRole('status')).toHaveTextContent('Page 2 loaded'); + }); + + it('keeps Previous available but disables Next after a page error', () => { + render( + , + ); + expect( + bottom().getByRole('button', { name: 'Go to previous page' }), + ).toBeEnabled(); + expect( + bottom().getByRole('button', { name: 'Go to next page' }), + ).toBeDisabled(); + }); + + it('keeps three-digit page labels together and supports keyboard navigation', async () => { + const user = userEvent.setup(); + const onNext = jest.fn(); + render( + , + ); + expect(bottom().getByText('Page 123')).toHaveClass('pf-v6-u-text-nowrap'); + const next = bottom().getByRole('button', { name: 'Go to next page' }); + next.focus(); + await user.keyboard('{Enter}'); + expect(onNext).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/src/hooks/__tests__/useCursorListPage.spec.tsx b/frontend/src/hooks/__tests__/useCursorListPage.spec.tsx new file mode 100644 index 00000000..4162ba0f --- /dev/null +++ b/frontend/src/hooks/__tests__/useCursorListPage.spec.tsx @@ -0,0 +1,58 @@ +import { act, renderHook } from '@testing-library/react'; +import { useCursorListPage } from '../useCursorListPage'; +import { useCursorSelection } from '../useCursorSelection'; + +describe('useCursorListPage', () => { + it('remembers opaque tokens for Previous and resets when filters change', () => { + const { result } = renderHook(() => useCursorListPage()); + expect(result.current.pageToken).toBe(''); + act(() => result.current.next('one')); + expect(result.current.page).toBe(2); + expect(result.current.pageToken).toBe('one'); + act(() => result.current.next('two')); + expect(result.current.pageToken).toBe('two'); + act(() => result.current.previous()); + expect(result.current.pageToken).toBe('one'); + act(() => result.current.reset()); + expect(result.current.page).toBe(1); + expect(result.current.pageToken).toBe(''); + }); + + it('preserves selections across pages and selects only visible names', () => { + const { result } = renderHook(() => useCursorSelection('team-a')); + act(() => result.current.toggleAll(['first'], true)); + act(() => result.current.toggleAll(['second'], true)); + expect(result.current.selected).toEqual(['first', 'second']); + act(() => result.current.toggleAll(['second'], false)); + expect(result.current.selected).toEqual(['first']); + act(() => result.current.clearSelection()); + expect(result.current.selected).toEqual([]); + }); + + it('changes page size and clears continuation history', () => { + const { result } = renderHook(() => useCursorListPage()); + act(() => result.current.next('next')); + act(() => result.current.onPerPageSelect(20)); + expect(result.current.page).toBe(1); + expect(result.current.pageToken).toBe(''); + expect(result.current.perPage).toBe(20); + }); + + it('resets tokens and selections when the workspace scope changes', () => { + const { result, rerender } = renderHook( + ({ scope }) => ({ + pager: useCursorListPage(scope), + selection: useCursorSelection(scope), + }), + { + initialProps: { scope: 'team-a' }, + }, + ); + act(() => result.current.selection.toggleOne('agent', true)); + act(() => result.current.pager.next('team-a-token')); + rerender({ scope: 'team-b' }); + expect(result.current.pager.page).toBe(1); + expect(result.current.pager.pageToken).toBe(''); + expect(result.current.selection.selected).toEqual([]); + }); +}); diff --git a/frontend/src/hooks/useCursorListPage.ts b/frontend/src/hooks/useCursorListPage.ts new file mode 100644 index 00000000..bb9f7dc4 --- /dev/null +++ b/frontend/src/hooks/useCursorListPage.ts @@ -0,0 +1,92 @@ +import { useCallback, useEffect, useState } from 'react'; + +// One token per visited page. The first page has no token, and only a +// successful response can provide the token for the next page. +export const useCursorListPage = (scope?: string) => { + const [currentScope, setCurrentScope] = useState(scope); + const [tokens, setTokens] = useState(['']); + const [page, setPage] = useState(1); + const [perPage, setPerPage] = useState(10); + const [isTransitioning, setTransitioning] = useState(false); + + if (currentScope !== scope) { + setCurrentScope(scope); + setTokens(['']); + setPage(1); + setTransitioning(true); + } + + const reset = useCallback(() => { + setTokens(['']); + setPage(1); + setTransitioning(true); + }, []); + const next = (token: string) => { + if (!token) return; + setTokens((current) => [...current.slice(0, page), token]); + setPage((current) => current + 1); + setTransitioning(true); + }; + const previous = () => { + setPage((current) => Math.max(1, current - 1)); + setTransitioning(true); + }; + const onPerPageSelect = (size: number) => { + if (size === perPage) return; + setPerPage(size); + reset(); + }; + const markLoaded = useCallback(() => setTransitioning(false), []); + + return { + page, + pageToken: tokens[page - 1] ?? '', + perPage, + onPerPageSelect, + reset, + next, + previous, + isTransitioning, + markLoaded, + }; +}; + +type PageResult = { + data?: { items: readonly unknown[]; nextPageToken: string }; + isFetching: boolean; + isError: boolean; + isPlaceholderData?: boolean; +}; + +// A background refresh already has usable rows. Only block page controls +// while the requested page has no result of its own yet. +export const isCursorPagePending = ( + query: PageResult, + isTransitioning: boolean, +): boolean => + !query.isError && + (query.isFetching || isTransitioning) && + (!query.data || Boolean(query.isPlaceholderData)); + +// A completed page settles the transition. If deletion makes a later page +// empty, return to the first page rather than displaying a stranded cursor. +export const useCursorPageResult = ( + pager: Pick< + ReturnType, + 'page' | 'markLoaded' | 'reset' + >, + query: PageResult, +) => { + const { page, markLoaded, reset } = pager; + const { data, isFetching, isError, isPlaceholderData } = query; + useEffect(() => { + if (isError) { + markLoaded(); + return; + } + if (!data || isPlaceholderData) return; + markLoaded(); + if (isFetching) return; + if (page > 1 && data.items.length === 0 && !data.nextPageToken) reset(); + }, [data, isFetching, isError, isPlaceholderData, page, markLoaded, reset]); +}; diff --git a/frontend/src/hooks/useCursorSelection.ts b/frontend/src/hooks/useCursorSelection.ts new file mode 100644 index 00000000..bc8736be --- /dev/null +++ b/frontend/src/hooks/useCursorSelection.ts @@ -0,0 +1,62 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; + +// Selection is by stable resource name, so moving a row to another page does +// not clear it. A changed workspace or selector starts a new selection scope. +export const useCursorSelection = (scope: string) => { + const [currentScope, setCurrentScope] = useState(scope); + const [selected, setSelected] = useState([]); + const observedPages = useRef(new Map>()); + + if (currentScope !== scope) { + setCurrentScope(scope); + setSelected([]); + } + + useEffect(() => { + observedPages.current.clear(); + }, [scope]); + + // A refresh can remove a selected row from a page we have already seen. + // Keep selections from other pages, whose contents are not loaded here. + const reconcilePage = useCallback((pageKey: string, pageNames: string[]) => { + const previousNames = observedPages.current.get(pageKey); + const currentNames = new Set(pageNames); + observedPages.current.set(pageKey, currentNames); + if (!previousNames) return; + + const disappeared = new Set( + [...previousNames].filter((name) => !currentNames.has(name)), + ); + if (disappeared.size === 0) return; + setSelected((current) => { + const remaining = current.filter((name) => !disappeared.has(name)); + return remaining.length === current.length ? current : remaining; + }); + }, []); + + const toggleAll = (pageNames: string[], isSelecting: boolean) => { + setSelected((current) => + isSelecting + ? [...new Set([...current, ...pageNames])] + : current.filter((name) => !pageNames.includes(name)), + ); + }; + const toggleOne = (name: string, isSelecting: boolean) => { + setSelected((current) => + isSelecting + ? [...new Set([...current, name])] + : current.filter((item) => item !== name), + ); + }; + const pageAllSelected = (pageNames: string[]) => + pageNames.length > 0 && pageNames.every((name) => selected.includes(name)); + + return { + selected, + toggleAll, + toggleOne, + pageAllSelected, + reconcilePage, + clearSelection: () => setSelected([]), + }; +}; diff --git a/frontend/src/i18n/locales/en/common.ts b/frontend/src/i18n/locales/en/common.ts index 6b5878e8..5a2c109d 100644 --- a/frontend/src/i18n/locales/en/common.ts +++ b/frontend/src/i18n/locales/en/common.ts @@ -79,4 +79,19 @@ export default { actions: { retry: 'Retry', }, + pagination: { + itemsPerPage: 'Items per page', + perPage: '{{count}} per page', + previous: 'Go to previous page', + next: 'Go to next page', + previousText: 'Previous', + nextText: 'Next', + page: 'Page {{number}}', + topNavigation: 'Top pagination', + bottomNavigation: 'Bottom pagination', + loaded: 'Page {{number}} loaded', + sandboxResults: 'Sandbox results', + workspaceResults: 'Workspace results', + resourceResults: '{{title}} results', + }, } as const; diff --git a/frontend/src/i18n/locales/en/workspaces.ts b/frontend/src/i18n/locales/en/workspaces.ts index 896df617..b4bbfa7c 100644 --- a/frontend/src/i18n/locales/en/workspaces.ts +++ b/frontend/src/i18n/locales/en/workspaces.ts @@ -8,6 +8,10 @@ export default { }, create: 'Create workspace', actionsToolbar: 'Workspace actions', + pagination: { + loadFailed: 'Could not load this page', + startAtFirst: 'Start at first page', + }, filter: { label: 'Filter workspaces by label selector', placeholder: 'Filter by label, e.g. env=staging', diff --git a/frontend/src/pages/AllWorkspacesPage.tsx b/frontend/src/pages/AllWorkspacesPage.tsx index 6ce6cc69..07ecec89 100644 --- a/frontend/src/pages/AllWorkspacesPage.tsx +++ b/frontend/src/pages/AllWorkspacesPage.tsx @@ -15,18 +15,22 @@ import { useNavigate } from 'react-router-dom'; import { useAllProviders, - useAllSandboxes, useAllServices, useAllTemplates, } from '../api/allWorkspaces'; import { useFeatureFlags } from '../api/auth'; +import AllSandboxList from '../components/AllSandboxList'; +import { + ImageReference, + scopedName, + workspaceOf, +} from '../components/AllWorkspaceCells'; import LabelsList from '../components/LabelsList'; -import PhaseLabel from '../components/PhaseLabel'; import ResourceTable from '../components/ResourceTable'; import type { ResourceTableColumn } from '../components/ResourceTable'; import ServiceEndpointsTable from '../components/ServiceEndpointsTable'; import { formatAge } from '../utils/formatters'; -import type { Provider, Sandbox, SandboxTemplate } from '../types'; +import type { Provider, SandboxTemplate } from '../types'; type AllWorkspacesPageProps = { onSelectWorkspace?: (workspace: string) => void; @@ -39,26 +43,6 @@ const TabPanel: React.FC<{ children: React.ReactNode }> = ({ children }) => (
{children}
); -const workspaceOf = (item: { metadata: { workspace?: string } }): string => - item.metadata.workspace ?? ''; - -// A name is only unique within its workspace, so rows are keyed, and -// filtered, on both. -const scopedName = (item: { - metadata: { workspace?: string; name: string }; -}): string => `${workspaceOf(item)}/${item.metadata.name}`; - -// An image reference pinned by digest is far wider than its column, which -// truncates it. The title keeps the whole reference within reach. -const ImageReference: React.FC<{ image?: string }> = ({ image }) => - image ? ( - - {image} - - ) : ( - <>- - ); - // The sandboxes, providers, templates and service endpoints of every // workspace on the gateway: `--all-workspaces` on the CLI's four lists. The // gateway answers these for platform admins only, and its refusal is shown as @@ -72,9 +56,7 @@ const AllWorkspacesPage: React.FC = ({ const navigate = useNavigate(); const features = useFeatureFlags(); const [activeTab, setActiveTab] = useState('sandboxes'); - const [sandboxSelector, setSandboxSelector] = useState(''); const [templateSelector, setTemplateSelector] = useState(''); - const sandboxes = useAllSandboxes(sandboxSelector || undefined); const providers = useAllProviders(); const templates = useAllTemplates(templateSelector || undefined); const services = useAllServices({ enabled: features.services }); @@ -109,50 +91,6 @@ const AllWorkspacesPage: React.FC = ({ ), }); - const sandboxColumns: ResourceTableColumn[] = [ - workspaceColumn(), - { - title: 'Name', - modifier: 'nowrap', - render: (sandbox) => ( - - ), - }, - { - title: 'Status', - render: (sandbox) => ( - - ), - }, - { - title: 'Image', - modifier: 'truncate', - render: (sandbox) => , - }, - { - title: 'Labels', - render: (sandbox) => ( - - ), - }, - { - title: 'Age', - render: (sandbox) => formatAge(sandbox.metadata.createdAtMs), - }, - ]; - const providerColumns: ResourceTableColumn[] = [ workspaceColumn(), { @@ -246,25 +184,13 @@ const AllWorkspacesPage: React.FC = ({ > Sandboxes {count(sandboxes.data)} - } + title={Sandboxes} data-testid="all-tab-sandboxes" > - diff --git a/frontend/src/pages/SandboxListPage.tsx b/frontend/src/pages/SandboxListPage.tsx index af85bb07..5b98b747 100644 --- a/frontend/src/pages/SandboxListPage.tsx +++ b/frontend/src/pages/SandboxListPage.tsx @@ -1,4 +1,4 @@ -import { useMemo, useState } from 'react'; +import { useEffect, useMemo, useRef, useState } from 'react'; import { Alert, Bullseye, @@ -11,7 +11,6 @@ import { EmptyStateBody, EmptyStateFooter, MenuToggle, - Pagination, SearchInput, Spinner, ToggleGroup, @@ -19,10 +18,12 @@ import { Toolbar, ToolbarContent, ToolbarItem, + ToolbarToggleGroup, } from '@patternfly/react-core'; import { CubesIcon, EllipsisVIcon, + FilterIcon, ListIcon, ThIcon, } from '@patternfly/react-icons'; @@ -37,12 +38,14 @@ import { useProviderExpiry } from '../api/providers'; import { sandboxKeys } from '../api/queryKeys'; import { deleteSandbox, - useSandboxes, + useSandboxPage, useStartSandbox, useStopSandbox, } from '../api/sandboxes'; import { useAlerts } from '../app/AlertContext'; import ConfirmDeleteModal from '../components/ConfirmDeleteModal'; +import CursorPagination from '../components/CursorPagination'; +import CursorPageTransition from '../components/CursorPageTransition'; import CreateSandboxModal from '../components/CreateSandboxModal'; import RefreshErrorAlert, { isRefreshError, @@ -50,9 +53,16 @@ import RefreshErrorAlert, { import SandboxGalleryView from '../components/sandbox/SandboxGalleryView'; import SandboxTableRow from '../components/sandbox/SandboxTableRow'; import { useBulkDelete } from '../hooks/useBulkDelete'; -import { useListPage } from '../hooks/useListPage'; +import { + isCursorPagePending, + useCursorListPage, + useCursorPageResult, +} from '../hooks/useCursorListPage'; +import { useCursorSelection } from '../hooks/useCursorSelection'; +import { useI18n } from '../i18n'; import { describeDeletions, type ResourceNoun } from '../utils/deletion'; import { hasPolicyRevision } from '../utils/sandboxLifecycle'; +import { combineSandboxLabelSelector } from '../utils/sandboxLabelSelector'; import type { Sandbox } from '../types'; const SANDBOX_NOUN: ResourceNoun = { singular: 'sandbox', plural: 'sandboxes' }; @@ -116,31 +126,74 @@ const SandboxListPage: React.FC = ({ // search is submitted, not on every key. const [selectorInput, setSelectorInput] = useState(''); const [labelSelector, setLabelSelector] = useState(''); - const sandboxes = useSandboxes(workspace, labelSelector || undefined); + const [typeInput, setTypeInput] = useState(''); + const [typeLabel, setTypeLabel] = useState(''); + const [filterError, setFilterError] = useState(''); const [isCreateOpen, setCreateOpen] = useState(false); const [nameFilter, setNameFilter] = useState(''); + const resultsRef = useRef(null); + const { t: tCommon } = useI18n('common'); const [viewMode, setViewMode] = useState(getInitialViewMode); + const scope = `${workspace}\0${labelSelector}\0${typeLabel}`; const { page, - setPage, + pageToken, + reset, + next, + previous, perPage, onPerPageSelect, + isTransitioning, + markLoaded, + } = useCursorListPage(scope); + const { selected, - numSelected, toggleAll, toggleOne, pageAllSelected, + reconcilePage, clearSelection, - isActionsOpen, - setActionsOpen, - deleteTargets, - setDeleteTargets, - closeDeleteModal, - deleteSelectedLabel, - pageOf, - } = useListPage(); + } = useCursorSelection(scope); + const [isActionsOpen, setActionsOpen] = useState(false); + const [deleteTargets, setDeleteTargets] = useState(null); + const closeDeleteModal = () => setDeleteTargets(null); + const deleteSelectedLabel = selected.length + ? `Delete selected (${selected.length})` + : 'Delete selected'; + useEffect(() => setDeleteTargets(null), [workspace]); + const combinedSelector = combineSandboxLabelSelector( + labelSelector, + typeLabel, + ).selector; + const sandboxes = useSandboxPage(workspace, { + pageSize: perPage, + pageToken, + labelSelector: combinedSelector || undefined, + }); + useCursorPageResult({ page, markLoaded, reset }, sandboxes); + useEffect(() => { + if ( + !sandboxes.data || + sandboxes.isFetching || + sandboxes.isError || + sandboxes.isPlaceholderData + ) + return; + reconcilePage( + `${perPage}\0${pageToken}`, + sandboxes.data.items.map(sandboxName), + ); + }, [ + sandboxes.data, + sandboxes.isFetching, + sandboxes.isError, + sandboxes.isPlaceholderData, + perPage, + pageToken, + reconcilePage, + ]); const filteredSandboxes = useMemo(() => { - const all = sandboxes.data ?? []; + const all = sandboxes.data?.items ?? []; const normalizedFilter = nameFilter.trim().toLocaleLowerCase(); if (!normalizedFilter) { return all; @@ -149,17 +202,19 @@ const SandboxListPage: React.FC = ({ sandbox.metadata.name.toLocaleLowerCase().includes(normalizedFilter), ); }, [sandboxes.data, nameFilter]); - // The rows of the page being shown. Taking them from pageOf keeps the page - // inside a list that got shorter, and drops from the selection a sandbox - // that a filter has hidden or that is gone: see useTableSelection. - const rows = pageOf(filteredSandboxes, sandboxName); + const rows = filteredSandboxes; + const loadedRows = sandboxes.isPlaceholderData ? [] : rows; // The sandboxes on this page whose policy can be read. One that was just // created has no policy revision until its supervisor has started, and the // gateway answers a request for it with a 404; see hasPolicyRevision. - const policyNames = rows.filter(hasPolicyRevision).map(sandboxName); + const policyNames = loadedRows.filter(hasPolicyRevision).map(sandboxName); const policyViews = useSandboxPolicies(workspace, policyNames); const providerExpiry = useProviderExpiry(workspace); - const drafts = useWorkspaceDraftSummary(workspace, features.draftPolicy); + const drafts = useWorkspaceDraftSummary( + workspace, + features.draftPolicy, + loadedRows.map(sandboxName), + ); const { addAlert, addSuccess, addDanger } = useAlerts(); const stopSandbox = useStopSandbox(workspace); const startSandbox = useStartSandbox(workspace); @@ -168,18 +223,17 @@ const SandboxListPage: React.FC = ({ sandboxKeys.scope(workspace), ); - if (sandboxes.isLoading) { - return ( - - - - ); - } - - const applySelector = (selector: string) => { + const applyFilters = (selector: string, type: string) => { setSelectorInput(selector); + setTypeInput(type); + const result = combineSandboxLabelSelector(selector, type); + if (result.error) { + setFilterError(result.error); + return; + } + setFilterError(''); setLabelSelector(selector.trim()); - setPage(1); + setTypeLabel(type.trim()); }; // The list is re-read while the page is open. A refresh that fails leaves @@ -189,7 +243,8 @@ const SandboxListPage: React.FC = ({ // caused it can be corrected. const refreshFailed = isRefreshError(sandboxes); const loadFailed = sandboxes.isError && !refreshFailed; - if (loadFailed && !labelSelector) { + const pageFailed = sandboxes.isError && page > 1; + if (loadFailed && !labelSelector && !typeLabel && page === 1) { return ( = ({ ); } - const allRows = sandboxes.data ?? []; - const totalCount = filteredSandboxes.length; + const allRows = sandboxes.data?.items ?? []; const pageNames = rows.map(sandboxName); // With a selector applied an empty list is a filter that matches nothing, // and the filter stays on screen to be changed. - const isEmpty = allRows.length === 0 && !labelSelector; + const isEmpty = + allRows.length === 0 && + !labelSelector && + !typeLabel && + page === 1 && + !sandboxes.isLoading && + !sandboxes.isPlaceholderData && + !loadFailed; const emptyState = ( @@ -232,6 +293,17 @@ const SandboxListPage: React.FC = ({ ); + const paginationProps = { + perPage, + page, + hasNext: Boolean(sandboxes.data?.nextPageToken), + isLoading: isCursorPagePending(sandboxes, isTransitioning), + isError: sandboxes.isError, + onPrevious: previous, + onNext: () => next(sandboxes.data?.nextPageToken ?? ''), + onPerPageSelect, + }; + const list = ( <> @@ -247,34 +319,55 @@ const SandboxListPage: React.FC = ({ )} {toolbarStart && {toolbarStart}} - - { - setNameFilter(value); - setPage(1); - }} - onClear={() => { - setNameFilter(''); - setPage(1); - }} - /> - - - setSelectorInput(value)} - onSearch={(_event, value) => applySelector(value)} - onClear={() => applySelector('')} - submitSearchButtonLabel="Apply label selector" - resetButtonLabel="Clear label selector" - data-testid="sandbox-label-selector" - /> - + } breakpoint="lg"> + + { + setNameFilter(value); + clearSelection(); + }} + onClear={() => { + setNameFilter(''); + clearSelection(); + }} + /> + + + setSelectorInput(value)} + onSearch={(_event, value) => applyFilters(value, typeLabel)} + onClear={() => applyFilters('', typeLabel)} + submitSearchButtonLabel="Apply label selector" + resetButtonLabel="Clear label selector" + data-testid="sandbox-label-selector" + /> + + + setTypeInput(value)} + onSearch={(_event, value) => applyFilters(labelSelector, value)} + onClear={() => applyFilters(labelSelector, '')} + submitSearchButtonLabel="Apply type label" + resetButtonLabel="Clear type label" + data-testid="sandbox-type-label" + /> + + {!compactToolbar && ( = ({ setDeleteTargets(selected)} data-testid="delete-selected-sandboxes" > @@ -335,18 +428,8 @@ const SandboxListPage: React.FC = ({ )} - - setPage(p)} - onPerPageSelect={(_event, pp) => onPerPageSelect(pp)} - isCompact - /> - {createActionPosition === 'end' && ( - + )} + + + - {loadFailed && ( + {(loadFailed || pageFailed) && ( 1 + ? 'Could not load this page' + : 'The label selector could not be applied' + } data-testid="sandbox-label-selector-error" > {(sandboxes.error as Error).message} + {page > 1 && ( + + )} )} - {compactToolbar || viewMode === 'list' ? ( - - - - - - - - - - - - - {rows.map((sandbox, rowIndex) => ( - - toggleOne(sandbox.metadata.name, isSelecting) - } - onNameClick={() => onSelect?.(sandbox.metadata.name)} - onDelete={() => setDeleteTargets([sandbox.metadata.name])} - onStop={() => - stopSandbox.mutate(sandbox.metadata.name, { - onSuccess: () => - addSuccess(`Stopping sandbox ${sandbox.metadata.name}`), - onError: (err) => - addDanger( - `Failed to stop sandbox ${sandbox.metadata.name}: ${(err as Error).message}`, - ), - }) - } - onStart={() => - startSandbox.mutate(sandbox.metadata.name, { - onSuccess: () => - addSuccess(`Starting sandbox ${sandbox.metadata.name}`), - onError: (err) => - addDanger( - `Failed to start sandbox ${sandbox.metadata.name}: ${(err as Error).message}`, - ), - }) - } - onViewLogs={() => viewSandbox(sandbox.metadata.name, 'logs')} - onOpenTerminal={ - sandbox.status.phase === 'READY' && features.terminal - ? () => viewSandbox(sandbox.metadata.name, 'terminal') - : undefined - } - policyView={policyViews[sandbox.metadata.name]} - draftSummary={drafts.bySandbox[sandbox.metadata.name]} - onReviewDrafts={ - features.draftPolicy - ? () => viewSandbox(sandbox.metadata.name, 'proposals') - : undefined - } - /> - ))} - {rows.length === 0 && !loadFailed && ( + {filterError && ( + + )} +

+ {tCommon('pagination.sandboxResults')} +

+ {sandboxes.isLoading || + (isTransitioning && !sandboxes.data && !sandboxes.isError) ? ( + + + + ) : compactToolbar || viewMode === 'list' ? ( + +
- toggleAll(pageNames, isSelecting), - isSelected: pageAllSelected(pageNames), - }} - aria-label="Select all sandboxes" - /> - NameStatusPolicyProvidersLabelsCreated -
+ - + + + + + + + - )} - -
No sandboxes match this filter. + toggleAll(pageNames, isSelecting), + isSelected: pageAllSelected(pageNames), + }} + aria-label="Select all sandboxes" + /> + NameStatusPolicyProvidersLabelsCreated
+ + + {rows.map((sandbox, rowIndex) => ( + + toggleOne(sandbox.metadata.name, isSelecting) + } + onNameClick={() => onSelect?.(sandbox.metadata.name)} + onDelete={() => setDeleteTargets([sandbox.metadata.name])} + onStop={() => + stopSandbox.mutate(sandbox.metadata.name, { + onSuccess: () => + addSuccess(`Stopping sandbox ${sandbox.metadata.name}`), + onError: (err) => + addDanger( + `Failed to stop sandbox ${sandbox.metadata.name}: ${(err as Error).message}`, + ), + }) + } + onStart={() => + startSandbox.mutate(sandbox.metadata.name, { + onSuccess: () => + addSuccess(`Starting sandbox ${sandbox.metadata.name}`), + onError: (err) => + addDanger( + `Failed to start sandbox ${sandbox.metadata.name}: ${(err as Error).message}`, + ), + }) + } + onViewLogs={() => viewSandbox(sandbox.metadata.name, 'logs')} + onOpenTerminal={ + sandbox.status.phase === 'READY' && features.terminal + ? () => viewSandbox(sandbox.metadata.name, 'terminal') + : undefined + } + policyView={policyViews[sandbox.metadata.name]} + draftSummary={drafts.bySandbox[sandbox.metadata.name]} + onReviewDrafts={ + features.draftPolicy + ? () => viewSandbox(sandbox.metadata.name, 'proposals') + : undefined + } + /> + ))} + {rows.length === 0 && !loadFailed && ( + + + {nameFilter + ? 'No sandboxes match on this page.' + : 'No sandboxes match this filter.'} + + + )} + + + ) : rows.length === 0 ? ( // The gallery's own empty state invites creating a first sandbox, // which is not what a filter that matches nothing calls for. @@ -454,27 +575,37 @@ const SandboxListPage: React.FC = ({ /> ) ) : ( - setDeleteTargets([name])} - onSelect={onSelect} - onViewLogs={(name) => viewSandbox(name, 'logs')} - onOpenTerminal={ - features.terminal - ? (name) => viewSandbox(name, 'terminal') - : undefined - } - onReviewDrafts={ - features.draftPolicy - ? (name) => viewSandbox(name, 'proposals') - : undefined - } - onCreateClick={() => setCreateOpen(true)} - /> + + setDeleteTargets([name])} + onSelect={onSelect} + onViewLogs={(name) => viewSandbox(name, 'logs')} + onOpenTerminal={ + features.terminal + ? (name) => viewSandbox(name, 'terminal') + : undefined + } + onReviewDrafts={ + features.draftPolicy + ? (name) => viewSandbox(name, 'proposals') + : undefined + } + onCreateClick={() => setCreateOpen(true)} + /> + )} + ); @@ -484,7 +615,7 @@ const SandboxListPage: React.FC = ({ // open Create sandbox form, and what was typed into it, where it is. return ( <> - {refreshFailed && ( + {refreshFailed && !pageFailed && ( = ({ const resolvedCredentialInput = renderCredentialInput ?? slots.credentialInput; const workspaceQuery = useWorkspace(workspace); - const sandboxCount = useSandboxes(workspace); const templateCount = useTemplates(workspace); const providerCount = useProviders(workspace); const memberCount = useMembers(workspace); @@ -186,14 +184,7 @@ const WorkspaceDetailPage: React.FC = ({ > - Sandboxes{' '} - {sandboxCount.data && ( - {sandboxCount.data.length} - )} - - } + title={Sandboxes} data-testid="tab-sandboxes" > diff --git a/frontend/src/pages/WorkspaceListPage.tsx b/frontend/src/pages/WorkspaceListPage.tsx index 4e962358..c38e14f9 100644 --- a/frontend/src/pages/WorkspaceListPage.tsx +++ b/frontend/src/pages/WorkspaceListPage.tsx @@ -1,4 +1,4 @@ -import { useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { useQueryClient } from '@tanstack/react-query'; import { Alert, @@ -13,15 +13,15 @@ import { EmptyStateFooter, MenuToggle, PageSection, - Pagination, SearchInput, Spinner, Title, Toolbar, ToolbarContent, ToolbarItem, + ToolbarToggleGroup, } from '@patternfly/react-core'; -import { CubesIcon, EllipsisVIcon } from '@patternfly/react-icons'; +import { CubesIcon, EllipsisVIcon, FilterIcon } from '@patternfly/react-icons'; import { ActionsColumn, Table, @@ -32,19 +32,26 @@ import { Tr, } from '@patternfly/react-table'; -import { deleteWorkspace, useWorkspaces } from '../api/workspaces'; +import { deleteWorkspace, useWorkspacePage } from '../api/workspaces'; import { allWorkspacesKeys, workspaceKeys } from '../api/queryKeys'; import { useAlerts } from '../app/AlertContext'; import { useUserRole } from '../api/rbac'; import CreateWorkspaceModal from '../components/CreateWorkspaceModal'; import ConfirmDeleteModal from '../components/ConfirmDeleteModal'; +import CursorPagination from '../components/CursorPagination'; +import CursorPageTransition from '../components/CursorPageTransition'; import LabelsList from '../components/LabelsList'; import PhaseLabel from '../components/PhaseLabel'; import RefreshErrorAlert, { isRefreshError, } from '../components/RefreshErrorAlert'; import { useBulkDelete } from '../hooks/useBulkDelete'; -import { useListPage } from '../hooks/useListPage'; +import { + isCursorPagePending, + useCursorListPage, + useCursorPageResult, +} from '../hooks/useCursorListPage'; +import { useCursorSelection } from '../hooks/useCursorSelection'; import { useI18n } from '../i18n'; import { formatAge } from '../utils/formatters'; @@ -69,29 +76,54 @@ const WorkspaceListPage: React.FC = ({ // gateway does the filtering and refuses a selector that is not key=value // pairs, so it is sent when the search is submitted, not on every key. const [selectorInput, setSelectorInput] = useState(''); + const resultsRef = useRef(null); const [labelSelector, setLabelSelector] = useState(''); - const workspaces = useWorkspaces(labelSelector || undefined); const queryClient = useQueryClient(); const { addSuccess } = useAlerts(); const { isPlatformAdmin } = useUserRole(); const [isCreateOpen, setCreateOpen] = useState(false); const { page, - setPage, + pageToken, + reset, + next, + previous, perPage, onPerPageSelect, + isTransitioning, + markLoaded, + } = useCursorListPage(labelSelector); + const { selected, toggleAll, toggleOne, pageAllSelected, + reconcilePage, clearSelection, - isActionsOpen, - setActionsOpen, - deleteTargets, - setDeleteTargets, - closeDeleteModal, - pageOf, - } = useListPage(); + } = useCursorSelection(labelSelector); + const [isActionsOpen, setActionsOpen] = useState(false); + const [deleteTargets, setDeleteTargets] = useState(null); + const closeDeleteModal = () => setDeleteTargets(null); + const workspaces = useWorkspacePage({ + pageSize: perPage, + pageToken, + labelSelector: labelSelector || undefined, + }); + useCursorPageResult({ page, markLoaded, reset }, workspaces); + useEffect(() => { + if (!workspaces.data || workspaces.isFetching || workspaces.isError) return; + reconcilePage( + `${perPage}\0${pageToken}`, + workspaces.data.items.map((workspace) => workspace.metadata.name), + ); + }, [ + workspaces.data, + workspaces.isFetching, + workspaces.isError, + perPage, + pageToken, + reconcilePage, + ]); // `openshell workspace delete `: one delete per workspace. The // reason the gateway gives for each one it refuses is kept, so that a @@ -110,30 +142,20 @@ const WorkspaceListPage: React.FC = ({ const applySelector = (selector: string) => { setSelectorInput(selector); setLabelSelector(selector.trim()); - setPage(1); // A workspace the new filter hides must not stay selected: it would be // deleted without being on screen. clearSelection(); }; - if (workspaces.isLoading) { - return ( - - - - - - ); - } - // The list is polled. A refresh that fails leaves the workspaces that // loaded before on screen, with a note above them; only a list that never // loaded takes the page. const refreshFailed = isRefreshError(workspaces); + const pageFailed = workspaces.isError && page > 1; // A failure of the filtered list is shown beside the filter, where the // selector that caused it can be corrected. - if (workspaces.isError && !labelSelector && !refreshFailed) { + if (workspaces.isError && !labelSelector && !refreshFailed && page === 1) { return ( = ({ ); } - const allRows = workspaces.data ?? []; - // The rows of the page that is shown. The list is polled and workspaces - // are deleted, here and elsewhere, so the page that was chosen can cease to - // exist: pageOf shows the last page that has rows then, not an empty - // table, and drops from the selection what is no longer listed. - const rows = pageOf(allRows, (workspace) => workspace.metadata.name); + const allRows = workspaces.data?.items ?? []; + const rows = allRows; const pageNames = rows.map((workspace) => workspace.metadata.name); - // What is selected and still listed. The list refreshes by itself, and a - // workspace somebody else deleted in the meantime is not one to delete. - const listedNames = new Set(allRows.map((w) => w.metadata.name)); - const selectedNames = selected.filter((name) => listedNames.has(name)); + // Selected names can belong to earlier pages, which are no longer loaded. + const selectedNames = selected; const targets = deleteTargets ?? []; const isBulk = targets.length > 1; @@ -225,6 +241,17 @@ const WorkspaceListPage: React.FC = ({ setDeleteTargets(failedNames); }; + const paginationProps = { + perPage, + page, + hasNext: Boolean(workspaces.data?.nextPageToken), + isLoading: isCursorPagePending(workspaces, isTransitioning), + isError: workspaces.isError, + onPrevious: previous, + onNext: () => next(workspaces.data?.nextPageToken ?? ''), + onPerPageSelect, + }; + return ( <> @@ -234,7 +261,7 @@ const WorkspaceListPage: React.FC = ({ {renderWorkspaceHeader()} )} - {refreshFailed && ( + {refreshFailed && !pageFailed && ( = ({ data-testid="workspace-refresh-error" /> )} - {allRows.length === 0 && !labelSelector ? ( + {allRows.length === 0 && + !workspaces.isLoading && + !workspaces.isPlaceholderData && + !labelSelector && + page === 1 && + !workspaces.isError ? ( = ({ openDelete(selectedNames)} data-testid="delete-selected-workspaces" > @@ -315,126 +349,155 @@ const WorkspaceListPage: React.FC = ({ )} - - setSelectorInput(value)} - onSearch={(_event, value) => applySelector(value)} - onClear={() => applySelector('')} - submitSearchButtonLabel={t('filter.apply')} - resetButtonLabel={t('filter.clear')} - data-testid="workspace-label-filter" - /> - - - setPage(p)} - onPerPageSelect={(_event, pp) => onPerPageSelect(pp)} - isCompact - /> + } breakpoint="lg"> + + setSelectorInput(value)} + onSearch={(_event, value) => applySelector(value)} + onClear={() => applySelector('')} + submitSearchButtonLabel={t('filter.apply')} + resetButtonLabel={t('filter.clear')} + data-testid="workspace-label-filter" + /> + + + + - {workspaces.isError && !refreshFailed && ( + {(workspaces.isError && !refreshFailed) || pageFailed ? ( 1 ? t('pagination.loadFailed') : t('filter.failed') + } data-testid="workspace-label-filter-error" > {(workspaces.error as Error).message} + {page > 1 && ( + + )} - )} - - - - {isPlatformAdmin && ( - - - - - {isPlatformAdmin && ( - - - - {rows.map((workspace, rowIndex) => ( - - {isPlatformAdmin && ( - - - - - {isPlatformAdmin && ( - + + + )} + +
- toggleAll(pageNames, isSelecting), - isSelected: pageAllSelected(pageNames), - }} - aria-label={t('bulkDelete.selectAll')} - /> - )} - {t('table.name')}{t('table.phase')}{t('table.labels')}{t('table.age')} - )} -
- toggleOne(workspace.metadata.name, isSelecting), - isSelected: selected.includes( - workspace.metadata.name, - ), - }} - /> - )} - - - - - - - - {formatAge(workspace.metadata.createdAtMs)} - - - openDelete([workspace.metadata.name]), - }, - ]} + ) : null} +

+ {tCommon('pagination.workspaceResults')} +

+ {workspaces.isLoading || + (isTransitioning && !workspaces.data && !workspaces.isError) ? ( + + + + ) : ( + + + + + {isPlatformAdmin && ( + - ))} - {/* Only a selector that was applied can have matched + )} + + + + + {isPlatformAdmin && ( + + + + {rows.map((workspace, rowIndex) => ( + + {isPlatformAdmin && ( + + + + + {isPlatformAdmin && ( + + )} + + ))} + {/* Only a selector that was applied can have matched nothing. */} - {rows.length === 0 && !workspaces.isError && labelSelector && ( - - - - )} - -
+ toggleAll(pageNames, isSelecting), + isSelected: pageAllSelected(pageNames), + }} + aria-label={t('bulkDelete.selectAll')} /> - - )} -
{t('table.name')}{t('table.phase')}{t('table.labels')}{t('table.age')} + )} +
+ toggleOne(workspace.metadata.name, isSelecting), + isSelected: selected.includes( + workspace.metadata.name, + ), + }} + /> + )} + + + + + + + + {formatAge(workspace.metadata.createdAtMs)} + + + openDelete([workspace.metadata.name]), + }, + ]} + /> +
- {t('filter.noMatch')} -
+ {rows.length === 0 && + !workspaces.isError && + labelSelector && ( +
+ {t('filter.noMatch')} +
+ + )} + )} diff --git a/frontend/src/pages/__tests__/AllWorkspacesPage.spec.tsx b/frontend/src/pages/__tests__/AllWorkspacesPage.spec.tsx index cc298ec5..c5b515e4 100644 --- a/frontend/src/pages/__tests__/AllWorkspacesPage.spec.tsx +++ b/frontend/src/pages/__tests__/AllWorkspacesPage.spec.tsx @@ -11,6 +11,7 @@ import type { jest.mock('../../api/allWorkspaces', () => ({ useAllSandboxes: jest.fn(), + useAllSandboxPage: jest.fn(), useAllProviders: jest.fn(), useAllTemplates: jest.fn(), useAllServices: jest.fn(), @@ -24,10 +25,19 @@ jest.mock('../../api/auth', () => ({ import { useAllProviders, useAllSandboxes, + useAllSandboxPage, useAllServices, useAllTemplates, } from '../../api/allWorkspaces'; const mockUseAllSandboxes = useAllSandboxes as jest.Mock; +const mockUseAllSandboxPage = useAllSandboxPage as jest.Mock; +import { pagedResult } from './pagedHookMock'; + +beforeEach(() => { + mockUseAllSandboxPage.mockImplementation((request) => + pagedResult(mockUseAllSandboxes(request.labelSelector), request), + ); +}); const mockUseAllProviders = useAllProviders as jest.Mock; const mockUseAllTemplates = useAllTemplates as jest.Mock; const mockUseAllServices = useAllServices as jest.Mock; @@ -196,11 +206,14 @@ describe('AllWorkspacesPage', () => { expect(image.closest('td')).toHaveClass('pf-m-truncate'); }); - it('counts each list on its tab', () => { + it('does not show an inaccurate sandbox total', () => { renderPage(); expect(screen.getByRole('tab', { name: /Sandboxes/ })).toHaveTextContent( - 'Sandboxes 3', + 'Sandboxes', ); + expect( + screen.getByRole('tab', { name: /Sandboxes/ }), + ).not.toHaveTextContent('3'); expect(screen.getByRole('tab', { name: /Providers/ })).toHaveTextContent( 'Providers 2', ); @@ -270,7 +283,9 @@ describe('AllWorkspacesPage', () => { it('filters the rows by workspace or name as it is typed', () => { renderPage(); - const filter = screen.getByRole('textbox', { name: 'Filter sandboxes' }); + const filter = screen.getByRole('textbox', { + name: 'Filter current page of sandboxes', + }); fireEvent.change(filter, { target: { value: 'team-b' } }); expect(rowsOf('all-sandboxes-table')).toHaveLength(2); @@ -281,7 +296,7 @@ describe('AllWorkspacesPage', () => { expect(rows[0]).toHaveTextContent('builder'); fireEvent.change(filter, { target: { value: 'nothing-like-this' } }); - expect(screen.getByText('No sandboxes match this filter.')).toBeVisible(); + expect(screen.getByText('No sandboxes match on this page.')).toBeVisible(); }); // `--selector`: the gateway applies it, so it goes to the request when it @@ -305,6 +320,65 @@ describe('AllWorkspacesPage', () => { expect(mockUseAllTemplates).toHaveBeenLastCalledWith(undefined); }); + it('adds the type label to the all-workspaces sandbox selector', () => { + renderPage(); + const selector = screen.getByRole('textbox', { + name: 'Filter sandboxes by label selector', + }); + fireEvent.change(selector, { target: { value: 'team=ml' } }); + fireEvent.keyDown(selector, { key: 'Enter' }); + const type = screen.getByRole('textbox', { + name: 'Filter sandboxes by type label', + }); + fireEvent.change(type, { target: { value: 'type=tool' } }); + fireEvent.keyDown(type, { key: 'Enter' }); + expect(mockUseAllSandboxPage).toHaveBeenLastCalledWith({ + pageSize: 10, + pageToken: '', + labelSelector: 'team=ml,type=tool', + }); + expect( + screen.getByRole('tab', { name: 'Sandboxes' }), + ).not.toHaveTextContent('3'); + }); + + it('keeps the type field visible when its selector matches no sandboxes', () => { + mockUseAllSandboxes.mockImplementation((selector?: string) => + ok(selector === 'type=tool' ? [] : sandboxes), + ); + renderPage(); + const type = screen.getByRole('textbox', { + name: 'Filter sandboxes by type label', + }); + fireEvent.change(type, { target: { value: 'type=tool' } }); + fireEvent.keyDown(type, { key: 'Enter' }); + expect(screen.getByTestId('all-sandboxes-table')).toBeInTheDocument(); + expect(type).toHaveValue('type=tool'); + expect(screen.queryByTestId('all-sandboxes-empty')).not.toBeInTheDocument(); + }); + + it('rejects a conflicting type key without changing the request', () => { + renderPage(); + const selector = screen.getByRole('textbox', { + name: 'Filter sandboxes by label selector', + }); + fireEvent.change(selector, { target: { value: 'type=agent' } }); + fireEvent.keyDown(selector, { key: 'Enter' }); + const type = screen.getByRole('textbox', { + name: 'Filter sandboxes by type label', + }); + fireEvent.change(type, { target: { value: 'type=tool' } }); + fireEvent.keyDown(type, { key: 'Enter' }); + expect( + screen.getByTestId('all-sandboxes-type-label-error'), + ).toHaveTextContent('already uses type'); + expect(mockUseAllSandboxPage).toHaveBeenLastCalledWith({ + pageSize: 10, + pageToken: '', + labelSelector: 'type=agent', + }); + }); + it('shows why a label selector was refused and keeps the box', () => { mockUseAllSandboxes.mockImplementation((selector?: string) => selector diff --git a/frontend/src/pages/__tests__/SandboxListPage.cache.spec.tsx b/frontend/src/pages/__tests__/SandboxListPage.cache.spec.tsx index 90314b17..613c0eb6 100644 --- a/frontend/src/pages/__tests__/SandboxListPage.cache.spec.tsx +++ b/frontend/src/pages/__tests__/SandboxListPage.cache.spec.tsx @@ -93,7 +93,10 @@ describe('SandboxListPage and the sandbox cache', () => { : { ok: true, status: 200, - json: async () => [sandbox('web'), sandbox('db')], + json: async () => ({ + items: [sandbox('web'), sandbox('db')], + nextPageToken: '', + }), }, ); global.fetch = fetchMock as unknown as typeof fetch; @@ -127,7 +130,7 @@ describe('SandboxListPage and the sandbox cache', () => { ).toHaveTextContent("invalid label selector 'web'"); expect(screen.queryByTestId('crashed')).not.toBeInTheDocument(); expect(fetchMock).toHaveBeenCalledWith( - '/api/v1/workspaces/team-a/sandboxes?labelSelector=web', + '/api/v1/workspaces/team-a/sandboxes?pageSize=10&labelSelector=web', expect.anything(), ); // And the sandbox that was looked at is still what its own key holds. diff --git a/frontend/src/pages/__tests__/SandboxListPage.list.spec.tsx b/frontend/src/pages/__tests__/SandboxListPage.list.spec.tsx index 96a86b78..81cb4013 100644 --- a/frontend/src/pages/__tests__/SandboxListPage.list.spec.tsx +++ b/frontend/src/pages/__tests__/SandboxListPage.list.spec.tsx @@ -68,6 +68,7 @@ jest.mock('../../api/auth', () => ({ jest.mock('../../api/sandboxes', () => ({ useSandboxes: jest.fn(), + useSandboxPage: jest.fn(), deleteSandbox: jest.fn(), useStopSandbox: jest.fn(() => ({ mutate: jest.fn(), isPending: false })), useStartSandbox: jest.fn(() => ({ mutate: jest.fn(), isPending: false })), @@ -126,9 +127,20 @@ jest.mock('../../slots', () => ({ useSlots: () => ({}) })); import { useWorkspaceDraftSummary } from '../../api/draftSummary'; import { useSandboxPolicies } from '../../api/policy'; -import { useSandboxes } from '../../api/sandboxes'; +import { useSandboxes, useSandboxPage } from '../../api/sandboxes'; +import { pagedResult } from './pagedHookMock'; const mockUseSandboxes = useSandboxes as jest.Mock; +const mockUseSandboxPage = useSandboxPage as jest.Mock; + +const bottomPagination = () => + within(screen.getByRole('navigation', { name: 'Bottom pagination' })); + +beforeEach(() => { + mockUseSandboxPage.mockImplementation((workspace, request) => + pagedResult(mockUseSandboxes(workspace, request.labelSelector), request), + ); +}); const mockUseSandboxPolicies = useSandboxPolicies as jest.Mock; const mockUseWorkspaceDraftSummary = useWorkspaceDraftSummary as jest.Mock; @@ -195,6 +207,116 @@ describe('SandboxListPage list', () => { }); describe('label selector', () => { + it('combines an applied type label with the generic selector', () => { + renderPage(); + applySelector('team=ml'); + const type = within(screen.getByTestId('sandbox-type-label')).getByRole( + 'textbox', + ); + fireEvent.change(type, { target: { value: 'type=tool' } }); + fireEvent.click(screen.getByRole('button', { name: 'Apply type label' })); + expect(mockUseSandboxPage).toHaveBeenLastCalledWith('team-a', { + pageSize: 10, + pageToken: '', + labelSelector: 'team=ml,type=tool', + }); + }); + + it('rejects a type key already used in the generic selector', () => { + renderPage(); + applySelector('type=agent'); + const type = within(screen.getByTestId('sandbox-type-label')).getByRole( + 'textbox', + ); + fireEvent.change(type, { target: { value: 'type=tool' } }); + fireEvent.click(screen.getByRole('button', { name: 'Apply type label' })); + expect(screen.getByTestId('sandbox-type-label-error')).toHaveTextContent( + 'already uses type', + ); + expect(mockUseSandboxPage).toHaveBeenLastCalledWith('team-a', { + pageSize: 10, + pageToken: '', + labelSelector: 'type=agent', + }); + }); + + it('requests only the next page and retains earlier selections', () => { + mockUseSandboxes.mockReturnValue( + listOf( + ...Array.from({ length: 11 }, (_, i) => + makeSandbox(`agent-${i + 1}`), + ), + ), + ); + renderPage(); + const firstRow = screen + .getByTestId('sandbox-table') + .querySelector('tbody tr'); + fireEvent.click(within(firstRow as HTMLElement).getByRole('checkbox')); + fireEvent.click( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ); + expect(mockUseSandboxPage).toHaveBeenLastCalledWith('team-a', { + pageSize: 10, + pageToken: '10', + labelSelector: undefined, + }); + expect(mockUseWorkspaceDraftSummary).toHaveBeenLastCalledWith( + 'team-a', + true, + ['agent-11'], + ); + const secondRow = screen + .getByTestId('sandbox-table') + .querySelector('tbody tr'); + fireEvent.click(within(secondRow as HTMLElement).getByRole('checkbox')); + fireEvent.click(screen.getByTestId('sandbox-actions-kebab')); + expect(screen.getByTestId('delete-selected-sandboxes')).toHaveTextContent( + 'Delete selected (2)', + ); + expect( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ).toBeDisabled(); + fireEvent.click( + bottomPagination().getByRole('button', { name: 'Go to previous page' }), + ); + expect(mockUseSandboxPage).toHaveBeenLastCalledWith('team-a', { + pageSize: 10, + pageToken: '', + labelSelector: undefined, + }); + }); + + it('offers a first-page reset when a continuation token is rejected', () => { + mockUseSandboxPage.mockImplementation((_workspace, request) => + request.pageToken + ? { + isLoading: false, + isError: true, + error: new Error('token expired'), + data: undefined, + } + : { + isLoading: false, + isError: false, + data: { items: [makeSandbox('agent-1')], nextPageToken: 'stale' }, + }, + ); + renderPage(); + fireEvent.click( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ); + expect(screen.getByText('token expired')).toBeInTheDocument(); + fireEvent.click( + screen.getByRole('button', { name: 'Start at first page' }), + ); + expect(mockUseSandboxPage).toHaveBeenLastCalledWith('team-a', { + pageSize: 10, + pageToken: '', + labelSelector: undefined, + }); + expect(bottomPagination().getByText('Page 1')).toBeInTheDocument(); + }); it('lists every sandbox of the workspace until a selector is applied', () => { renderPage(); expect(mockUseSandboxes).toHaveBeenCalledWith('team-a', undefined); @@ -353,7 +475,11 @@ describe('SandboxListPage list', () => { it('reads the summary of this workspace', () => { renderPage(); - expect(mockUseWorkspaceDraftSummary).toHaveBeenCalledWith('team-a', true); + expect(mockUseWorkspaceDraftSummary).toHaveBeenCalledWith( + 'team-a', + true, + ['agent-1', 'agent-2'], + ); }); it('badges the sandboxes that have rules pending, in the table', () => { @@ -418,6 +544,7 @@ describe('SandboxListPage list', () => { expect(mockUseWorkspaceDraftSummary).toHaveBeenCalledWith( 'team-a', false, + ['agent-1', 'agent-2'], ); expect(screen.queryByText(/pending rule/)).not.toBeInTheDocument(); }); diff --git a/frontend/src/pages/__tests__/SandboxListPage.polling.spec.tsx b/frontend/src/pages/__tests__/SandboxListPage.polling.spec.tsx index 6f5a06f6..25536ef8 100644 --- a/frontend/src/pages/__tests__/SandboxListPage.polling.spec.tsx +++ b/frontend/src/pages/__tests__/SandboxListPage.polling.spec.tsx @@ -13,6 +13,7 @@ jest.mock('../../api/auth', () => ({ })); jest.mock('../../api/sandboxes', () => ({ useSandboxes: jest.fn(), + useSandboxPage: jest.fn(), deleteSandbox: jest.fn(), useStopSandbox: jest.fn(() => ({ mutate: jest.fn(), isPending: false })), useStartSandbox: jest.fn(() => ({ mutate: jest.fn(), isPending: false })), @@ -62,8 +63,19 @@ jest.mock('../../components/CreateSandboxModal', () => ({ }, })); -import { useSandboxes } from '../../api/sandboxes'; +import { useSandboxes, useSandboxPage } from '../../api/sandboxes'; +import { pagedResult } from './pagedHookMock'; const mockUseSandboxes = useSandboxes as jest.Mock; +const mockUseSandboxPage = useSandboxPage as jest.Mock; + +const bottomPagination = () => + within(screen.getByRole('navigation', { name: 'Bottom pagination' })); + +beforeEach(() => { + mockUseSandboxPage.mockImplementation((workspace, request) => + pagedResult(mockUseSandboxes(workspace, request.labelSelector), request), + ); +}); const sandbox = (index: number): Sandbox => ({ metadata: { @@ -247,7 +259,9 @@ describe('SandboxListPage while it is being used', () => { it('shows the last page that has rows, not an empty table', () => { mockUseSandboxes.mockReturnValue(loaded(sandboxes(11))); const view = render(page()); - fireEvent.click(screen.getByRole('button', { name: 'Go to next page' })); + fireEvent.click( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ); expect(shownNames()).toEqual(['sb-11']); // The eleventh sandbox is deleted, here or elsewhere. @@ -264,11 +278,13 @@ describe('SandboxListPage while it is being used', () => { describe('the selection', () => { const filterByName = (text: string) => fireEvent.change( - screen.getByRole('textbox', { name: 'Filter sandboxes by name' }), + screen.getByRole('textbox', { + name: 'Filter current page of sandboxes by name', + }), { target: { value: text } }, ); - it('lets go of a sandbox that the name filter hides, for good', () => { + it('clears selection when the current-page name filter changes', () => { mockUseSandboxes.mockReturnValue(loaded(sandboxes(3))); render(page()); selectRow('sb-01'); @@ -280,17 +296,17 @@ describe('SandboxListPage while it is being used', () => { filterByName('sb-01'); expect(screen.getByTestId('delete-selected-sandboxes')).toHaveTextContent( - 'Delete selected (1)', + 'Delete selected', ); // Clearing the filter does not select it again behind the user's back. filterByName(''); expect(screen.getByTestId('delete-selected-sandboxes')).toHaveTextContent( - 'Delete selected (1)', + 'Delete selected', ); }); - it('lets go of everything a label selector leaves out', () => { + it('clears selection when the label selector changes', () => { mockUseSandboxes.mockImplementation((_workspace, selector) => loaded(selector ? [sandbox(2)] : sandboxes(3)), ); @@ -309,14 +325,7 @@ describe('SandboxListPage while it is being used', () => { ); openActions(); - expect(screen.getByTestId('delete-selected-sandboxes')).toHaveTextContent( - 'Delete selected (1)', - ); - fireEvent.click(deleteSelected()); - // One left: the question names it and asks for it to be typed. - expect( - screen.getByText(/Sandbox "sb-02" will be permanently deleted/), - ).toBeInTheDocument(); + expect(deleteSelected()).toBeDisabled(); }); it('lets go of a sandbox that is gone from the list', () => { @@ -338,7 +347,9 @@ describe('SandboxListPage while it is being used', () => { mockUseSandboxes.mockReturnValue(loaded(sandboxes(11))); render(page()); selectRow('sb-01'); - fireEvent.click(screen.getByRole('button', { name: 'Go to next page' })); + fireEvent.click( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ); selectRow('sb-11'); openActions(); @@ -359,8 +370,10 @@ describe('SandboxListPage while it is being used', () => { it('counts the sandboxes it does not spell out in a long bulk delete', () => { mockUseSandboxes.mockReturnValue(loaded(sandboxes(12))); render(page()); - fireEvent.click(screen.getByRole('button', { name: /1 - 10 of 12/ })); - fireEvent.click(screen.getByRole('menuitem', { name: '20 per page' })); + fireEvent.change( + screen.getByRole('combobox', { name: 'Items per page' }), + { target: { value: '20' } }, + ); fireEvent.click( screen.getByRole('checkbox', { name: 'Select all rows' }), ); diff --git a/frontend/src/pages/__tests__/SandboxListPage.spec.tsx b/frontend/src/pages/__tests__/SandboxListPage.spec.tsx index 397f784f..dcdb5507 100644 --- a/frontend/src/pages/__tests__/SandboxListPage.spec.tsx +++ b/frontend/src/pages/__tests__/SandboxListPage.spec.tsx @@ -26,6 +26,7 @@ const mockSandbox: Sandbox = { jest.mock('../../api/sandboxes', () => ({ useSandboxes: jest.fn(), + useSandboxPage: jest.fn(), deleteSandbox: jest.fn(), useStopSandbox: jest.fn(() => ({ mutate: jest.fn(), isPending: false })), useStartSandbox: jest.fn(() => ({ mutate: jest.fn(), isPending: false })), @@ -96,8 +97,16 @@ jest.mock('../../hooks/useJsonValidation', () => ({ }), })); -import { useSandboxes } from '../../api/sandboxes'; +import { useSandboxes, useSandboxPage } from '../../api/sandboxes'; +import { pagedResult } from './pagedHookMock'; const mockUseSandboxes = useSandboxes as jest.Mock; +const mockUseSandboxPage = useSandboxPage as jest.Mock; + +beforeEach(() => { + mockUseSandboxPage.mockImplementation((workspace, request) => + pagedResult(mockUseSandboxes(workspace, request.labelSelector), request), + ); +}); const renderPage = () => render( @@ -184,9 +193,12 @@ describe('SandboxListPage', () => { ); expect(screen.getByText('Workspace selector')).toBeInTheDocument(); - fireEvent.change(screen.getByPlaceholderText('Filter by name'), { - target: { value: 'research' }, - }); + fireEvent.change( + screen.getByPlaceholderText('Filter current page by name'), + { + target: { value: 'research' }, + }, + ); expect(screen.getByText('research-runner')).toBeInTheDocument(); expect(screen.queryByText('test-sandbox')).not.toBeInTheDocument(); }); diff --git a/frontend/src/pages/__tests__/WorkspaceListPage.spec.tsx b/frontend/src/pages/__tests__/WorkspaceListPage.spec.tsx index 22e90d14..7dc1a6bb 100644 --- a/frontend/src/pages/__tests__/WorkspaceListPage.spec.tsx +++ b/frontend/src/pages/__tests__/WorkspaceListPage.spec.tsx @@ -12,6 +12,7 @@ import type { Workspace } from '../../types'; jest.mock('../../api/workspaces', () => ({ useWorkspaces: jest.fn(), + useWorkspacePage: jest.fn(), deleteWorkspace: jest.fn(), useDeleteWorkspace: jest.fn(() => ({ mutate: jest.fn(), @@ -39,10 +40,25 @@ jest.mock('../../app/AlertContext', () => ({ useAlerts: jest.fn(() => ({ addSuccess: mockAddSuccess })), })); -import { deleteWorkspace, useWorkspaces } from '../../api/workspaces'; +import { + deleteWorkspace, + useWorkspaces, + useWorkspacePage, +} from '../../api/workspaces'; +import { pagedResult } from './pagedHookMock'; const mockUseWorkspaces = useWorkspaces as jest.Mock; +const mockUseWorkspacePage = useWorkspacePage as jest.Mock; const mockDeleteWorkspace = deleteWorkspace as jest.Mock; +const bottomPagination = () => + within(screen.getByRole('navigation', { name: 'Bottom pagination' })); + +beforeEach(() => { + mockUseWorkspacePage.mockImplementation((request) => + pagedResult(mockUseWorkspaces(request.labelSelector), request), + ); +}); + // The page deletes through the query client (it invalidates the lists a // deleted workspace was in), so it is rendered inside one. The list itself is // served by the mock above and never reaches it. @@ -106,6 +122,135 @@ describe('WorkspaceListPage', () => { mockIsPlatformAdmin = true; }); + it('uses the continuation token and resets when page size changes', () => { + serve({ '': Array.from({ length: 11 }, (_, i) => workspace(`ws-${i}`)) }); + render(); + fireEvent.click( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ); + expect(mockUseWorkspacePage).toHaveBeenLastCalledWith({ + pageSize: 10, + pageToken: '10', + labelSelector: undefined, + }); + fireEvent.change(screen.getByRole('combobox', { name: 'Items per page' }), { + target: { value: '20' }, + }); + expect(mockUseWorkspacePage).toHaveBeenLastCalledWith({ + pageSize: 20, + pageToken: '', + labelSelector: undefined, + }); + expect(bottomPagination().getByText('Page 1')).toBeInTheDocument(); + }); + + it('holds table space behind a spinner while the next page loads', () => { + const first = { + items: [workspace('first')], + nextPageToken: 'next', + }; + const second = { + items: [workspace('second')], + nextPageToken: 'third', + }; + let pending = true; + let fetching = true; + mockUseWorkspacePage.mockImplementation((request) => ({ + isLoading: false, + isFetching: Boolean(request.pageToken && fetching), + isError: false, + isPlaceholderData: Boolean(request.pageToken && pending), + data: request.pageToken && !pending ? second : first, + })); + const view = render(); + fireEvent.click( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ); + + const table = screen.getByTestId('workspace-table'); + const content = table.parentElement as HTMLElement; + expect(content).toHaveAttribute('aria-hidden', 'true'); + expect(content).toHaveAttribute('inert'); + expect(content).toHaveClass('cursor-page-transition__content--loading'); + expect(screen.getByLabelText('Loading workspaces')).toBeInTheDocument(); + expect( + bottomPagination().getByRole('button', { name: 'Go to previous page' }), + ).toBeDisabled(); + + pending = false; + view.rerender(); + expect(listedNames()).toEqual(['second']); + expect(content).toHaveAttribute('aria-hidden', 'false'); + expect(content).not.toHaveAttribute('inert'); + expect(screen.queryByLabelText('Loading workspaces')).toBeNull(); + expect( + bottomPagination().getByRole('button', { name: 'Go to previous page' }), + ).toBeEnabled(); + expect( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ).toBeEnabled(); + expect( + screen.getByRole('combobox', { name: 'Items per page' }), + ).toBeEnabled(); + + fetching = false; + view.rerender(); + expect(listedNames()).toEqual(['second']); + }); + + it('keeps pagination controls enabled during a background refresh', () => { + let fetching = false; + mockUseWorkspacePage.mockImplementation(() => ({ + isLoading: false, + isFetching: fetching, + isError: false, + isPlaceholderData: false, + data: { items: [workspace('first')], nextPageToken: 'next' }, + })); + const view = render(); + fetching = true; + view.rerender(); + + expect(listedNames()).toEqual(['first']); + expect( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ).toBeEnabled(); + expect( + screen.getByRole('combobox', { name: 'Items per page' }), + ).toBeEnabled(); + }); + + it('offers a first-page reset when a continuation token is rejected', () => { + mockUseWorkspacePage.mockImplementation((request) => + request.pageToken + ? { + isLoading: false, + isError: true, + error: new Error('token expired'), + data: undefined, + } + : { + isLoading: false, + isError: false, + data: { items: [workspace('team-a')], nextPageToken: 'stale' }, + }, + ); + render(); + fireEvent.click( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ); + expect(screen.getByText('token expired')).toBeInTheDocument(); + fireEvent.click( + screen.getByRole('button', { name: 'Start at first page' }), + ); + expect(mockUseWorkspacePage).toHaveBeenLastCalledWith({ + pageSize: 10, + pageToken: '', + labelSelector: undefined, + }); + expect(bottomPagination().getByText('Page 1')).toBeInTheDocument(); + }); + it('lists every workspace with its labels until a filter is applied', () => { serve({ '': all }); render(); @@ -658,7 +803,9 @@ describe('WorkspaceListPage when the list shrinks', () => { let listed = many(11); serveList(() => listed); const view = render(); - fireEvent.click(screen.getByRole('button', { name: 'Go to next page' })); + fireEvent.click( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ); expect(listedNames()).toEqual(['ws-11']); // The one workspace on the second page is deleted, here or elsewhere. @@ -674,7 +821,9 @@ describe('WorkspaceListPage when the list shrinks', () => { let listed = many(11); serveList(() => listed); const view = render(); - fireEvent.click(screen.getByRole('button', { name: 'Go to next page' })); + fireEvent.click( + bottomPagination().getByRole('button', { name: 'Go to next page' }), + ); listed = many(10); view.rerender(); diff --git a/frontend/src/pages/__tests__/pagedHookMock.ts b/frontend/src/pages/__tests__/pagedHookMock.ts new file mode 100644 index 00000000..db417acb --- /dev/null +++ b/frontend/src/pages/__tests__/pagedHookMock.ts @@ -0,0 +1,21 @@ +import type { PageRequest } from '../../api/pagination'; + +// Adapts fixtures written for the legacy array hook to the one-page contract. +export const pagedResult = ( + result: { data?: T[]; [key: string]: unknown }, + request: PageRequest, +) => { + if (!result.data) return result; + const offset = Number(request.pageToken || '0'); + const items = result.data.slice(offset, offset + request.pageSize); + return { + ...result, + data: { + items, + nextPageToken: + offset + request.pageSize < result.data.length + ? String(offset + request.pageSize) + : '', + }, + }; +}; diff --git a/frontend/src/utils/__tests__/sandboxLabelSelector.spec.ts b/frontend/src/utils/__tests__/sandboxLabelSelector.spec.ts new file mode 100644 index 00000000..d932268c --- /dev/null +++ b/frontend/src/utils/__tests__/sandboxLabelSelector.spec.ts @@ -0,0 +1,24 @@ +import { combineSandboxLabelSelector } from '../sandboxLabelSelector'; + +describe('combineSandboxLabelSelector', () => { + it('combines a type label and generic selector', () => { + expect(combineSandboxLabelSelector('team=ml', 'type=tool')).toEqual({ + selector: 'team=ml,type=tool', + }); + }); + + it('rejects invalid type entries and duplicate keys', () => { + expect(combineSandboxLabelSelector('team=ml', 'type')).toHaveProperty( + 'error', + ); + expect( + combineSandboxLabelSelector('type=agent,team=ml', 'type=tool'), + ).toHaveProperty('error'); + }); + + it('leaves the generic selector alone without a type label', () => { + expect(combineSandboxLabelSelector('team=ml', '')).toEqual({ + selector: 'team=ml', + }); + }); +}); diff --git a/frontend/src/utils/sandboxLabelSelector.ts b/frontend/src/utils/sandboxLabelSelector.ts new file mode 100644 index 00000000..d816444c --- /dev/null +++ b/frontend/src/utils/sandboxLabelSelector.ts @@ -0,0 +1,25 @@ +export const combineSandboxLabelSelector = ( + generic: string, + typeLabel: string, +): { selector: string; error?: string } => { + const selector = generic.trim(); + const type = typeLabel.trim(); + if (!type) return { selector }; + + const parts = type.split('='); + if ( + parts.length !== 2 || + !parts[0].trim() || + !parts[1].trim() || + type.includes(',') || + parts[0] !== parts[0].trim() || + parts[1] !== parts[1].trim() + ) { + return { selector, error: 'Type label must be one key=value pair.' }; + } + const key = parts[0]; + if (selector.split(',').some((entry) => entry.split('=')[0].trim() === key)) { + return { selector, error: `The label selector already uses ${key}.` }; + } + return { selector: selector ? `${selector},${type}` : type }; +}; From 5b95b7f50313d90cf33dfa7f1b21cc004b3e8de3 Mon Sep 17 00:00:00 2001 From: Gage Krumbach Date: Thu, 22 Oct 2026 17:25:00 -0500 Subject: [PATCH 2/2] test(handlers): pass the raw exec client to NewSandboxService #124 added a second NewSandboxService parameter (SandboxRawExec). The pagination tests still made the one-argument call, so the handlers test package stopped compiling after the rebase onto main. Pass nil, as the other handler tests do. Co-Authored-By: Claude Opus 5.5 --- backend/pkg/handlers/list_pagination_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/backend/pkg/handlers/list_pagination_test.go b/backend/pkg/handlers/list_pagination_test.go index 655cb3b9..312c610f 100644 --- a/backend/pkg/handlers/list_pagination_test.go +++ b/backend/pkg/handlers/list_pagination_test.go @@ -44,13 +44,13 @@ func TestPagedListRoutesFetchOneGatewayPage(t *testing.T) { path := "/workspaces/team-a/sandboxes" switch route { case "sandboxes": - r.Get("/workspaces/{workspace}/sandboxes", NewSandboxHandler(services.NewSandboxService(sdk.Sandboxes())).ListSandboxes) + r.Get("/workspaces/{workspace}/sandboxes", NewSandboxHandler(services.NewSandboxService(sdk.Sandboxes(), nil)).ListSandboxes) case "workspaces": path = "/workspaces" r.Get(path, NewWorkspacesHandler(sdk.Workspaces()).ListWorkspaces) case "all-sandboxes": path = "/sandboxes" - r.Get(path, NewAllWorkspacesHandler(services.NewSandboxService(sdk.Sandboxes()), nil, nil, nil).ListSandboxes) + r.Get(path, NewAllWorkspacesHandler(services.NewSandboxService(sdk.Sandboxes(), nil), nil, nil, nil).ListSandboxes) } w := httptest.NewRecorder() r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path+"?pageSize=1&pageToken=prior-token&labelSelector=type%3Dtool", nil)) @@ -88,8 +88,8 @@ func TestPagedListValidation(t *testing.T) { sdk := &mockSDK{} r := chi.NewRouter() r.Get("/workspaces", NewWorkspacesHandler(sdk.Workspaces()).ListWorkspaces) - r.Get("/workspaces/{workspace}/sandboxes", NewSandboxHandler(services.NewSandboxService(sdk.Sandboxes())).ListSandboxes) - r.Get("/sandboxes", NewAllWorkspacesHandler(services.NewSandboxService(sdk.Sandboxes()), nil, nil, nil).ListSandboxes) + r.Get("/workspaces/{workspace}/sandboxes", NewSandboxHandler(services.NewSandboxService(sdk.Sandboxes(), nil)).ListSandboxes) + r.Get("/sandboxes", NewAllWorkspacesHandler(services.NewSandboxService(sdk.Sandboxes(), nil), nil, nil, nil).ListSandboxes) w := httptest.NewRecorder() r.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path+"?"+query, nil)) if w.Code != http.StatusBadRequest {