Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions apps/backend/internal/task/service/service_resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
)

const spritesTokenEnvKey = "SPRITES_API_TOKEN"
const workspaceDeletePageSize = 500

// Workspace operations

Expand Down Expand Up @@ -90,6 +91,30 @@ func (s *Service) DeleteWorkspace(ctx context.Context, id string) error {
if err != nil {
return err
}
tasks, err := s.listAllTasksForWorkspaceDelete(ctx, id)
if err != nil {
return err
}
for _, task := range tasks {
if task == nil || task.ID == "" {
continue
}
if err := s.DeleteTask(ctx, task.ID); err != nil {
return fmt.Errorf("delete task %s: %w", task.ID, err)
}
}
workflows, err := s.workflows.ListWorkflows(ctx, id, true)
if err != nil {
return fmt.Errorf("list workspace workflows: %w", err)
}
for _, workflow := range workflows {
if workflow == nil || workflow.ID == "" {
continue
}
if err := s.DeleteWorkflow(ctx, workflow.ID); err != nil {
return fmt.Errorf("delete workflow %s: %w", workflow.ID, err)
}
}
if err := s.workspaces.DeleteWorkspace(ctx, id); err != nil {
s.logger.Error("failed to delete workspace", zap.String("workspace_id", id), zap.Error(err))
return err
Expand All @@ -99,6 +124,22 @@ func (s *Service) DeleteWorkspace(ctx context.Context, id string) error {
return nil
}

func (s *Service) listAllTasksForWorkspaceDelete(ctx context.Context, workspaceID string) ([]*models.Task, error) {
var all []*models.Task
for page := 1; ; page++ {
tasks, total, err := s.tasks.ListTasksByWorkspace(
ctx, workspaceID, "", "", "", page, workspaceDeletePageSize, true, true, false, false,
)
if err != nil {
return nil, fmt.Errorf("list workspace tasks: %w", err)
}
all = append(all, tasks...)
if len(all) >= total || len(tasks) == 0 {
return all, nil
}
}
}

func normalizeOptionalID(value *string) *string {
if value == nil {
return nil
Expand Down
40 changes: 40 additions & 0 deletions apps/backend/internal/task/service/service_resources_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,46 @@ func TestService_GetOfficeWorkflowIDs_DBError(t *testing.T) {
}
}

func TestService_DeleteWorkspaceDeletesWorkspaceOwnedTasksAndWorkflows(t *testing.T) {
svc, _, repo := createTestService(t)
ctx := context.Background()

_ = repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-delete", Name: "Delete Me"})
_ = repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-keep", Name: "Keep Me"})
_ = repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-delete", WorkspaceID: "ws-delete", Name: "Doomed"})
_ = repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-keep", WorkspaceID: "ws-keep", Name: "Keep"})
if err := repo.CreateTask(ctx, &models.Task{
ID: "task-delete",
WorkspaceID: "ws-delete",
WorkflowID: "wf-delete",
WorkflowStepID: "step-delete",
Title: "Delete task",
}); err != nil {
t.Fatalf("CreateTask: %v", err)
}

if err := svc.DeleteWorkspace(ctx, "ws-delete"); err != nil {
t.Fatalf("DeleteWorkspace: %v", err)
}

if _, err := repo.GetWorkspace(ctx, "ws-delete"); err == nil {
t.Fatalf("workspace should be deleted")
}
if _, err := repo.GetTask(ctx, "task-delete"); err == nil {
t.Fatalf("workspace task should be deleted")
}
workflows, err := repo.ListWorkflows(ctx, "ws-delete", true)
if err != nil {
t.Fatalf("ListWorkflows: %v", err)
}
if len(workflows) != 0 {
t.Fatalf("workspace workflows should be deleted, got %d", len(workflows))
}
if _, err := repo.GetWorkflow(ctx, "wf-keep"); err != nil {
t.Fatalf("unrelated workflow should remain: %v", err)
}
}

// TestService_DeleteWorkflow_ArchivesChildTasks verifies the cascade fix for
// issue #1279: workflow deletion archives any active child tasks instead of
// leaving them with a dangling workflow_id (tasks.workflow_id has no FK, so
Expand Down
2 changes: 1 addition & 1 deletion apps/web/app/actions/workspaces.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe("deleteWorkspaceAction", () => {
expect(fetchMock).toHaveBeenCalledTimes(1);

const [url, init] = fetchMock.mock.calls[0];
expect(url).toBe("http://backend.test/api/v1/office/workspaces/ws-1");
expect(url).toBe("http://backend.test/api/v1/workspaces/ws-1");
expect(init.method).toBe("DELETE");
expect(JSON.parse(init.body as string)).toEqual({ confirm_name: "My Workspace" });
});
Expand Down
2 changes: 1 addition & 1 deletion apps/web/app/actions/workspaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export async function updateWorkspaceAction(
}

export async function deleteWorkspaceAction(id: string, confirmName: string) {
await fetchJson<void>(`${apiBaseUrl}/api/v1/office/workspaces/${id}`, {
await fetchJson<void>(`${apiBaseUrl}/api/v1/workspaces/${id}`, {
method: "DELETE",
body: JSON.stringify({ confirm_name: confirmName }),
});
Expand Down
76 changes: 76 additions & 0 deletions apps/web/app/github/page.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { describe, expect, it, vi, beforeEach } from "vitest";
import { render } from "@testing-library/react";

const mocks = vi.hoisted(() => ({
listWorkspacesAction: vi.fn(),
listWorkflowsAction: vi.fn(),
listRepositoriesAction: vi.fn(),
listWorkspaceWorkflowStepsAction: vi.fn(),
fetchUserSettings: vi.fn(),
cookies: vi.fn(),
capturedInitialState: null as unknown,
}));

vi.mock("@/app/actions/workspaces", () => ({
listWorkspacesAction: mocks.listWorkspacesAction,
listWorkflowsAction: mocks.listWorkflowsAction,
listRepositoriesAction: mocks.listRepositoriesAction,
listWorkspaceWorkflowStepsAction: mocks.listWorkspaceWorkflowStepsAction,
}));

vi.mock("@/lib/api", () => ({
fetchUserSettings: mocks.fetchUserSettings,
}));

vi.mock("next/headers", () => ({
cookies: mocks.cookies,
}));

vi.mock("@/components/state-hydrator", () => ({
StateHydrator: ({ initialState }: { initialState: unknown }) => {
mocks.capturedInitialState = initialState;
return null;
},
}));

vi.mock("./github-page-client", () => ({
GitHubPageClient: () => null,
}));

import GitHubPage from "./page";

describe("GitHubPage", () => {
beforeEach(() => {
vi.clearAllMocks();
mocks.capturedInitialState = null;
mocks.listWorkspacesAction.mockResolvedValue({
workspaces: [
{ id: "ws-go", name: "GO" },
{ id: "ws-connection", name: "Connection" },
],
});
mocks.fetchUserSettings.mockResolvedValue({
settings: { workspace_id: "ws-connection" },
});
mocks.cookies.mockResolvedValue({
get: (name: string) =>
name === "office-active-workspace" ? { value: "ws-go" } : undefined,
});
mocks.listWorkflowsAction.mockResolvedValue({ workflows: [] });
mocks.listRepositoriesAction.mockResolvedValue({ repositories: [] });
mocks.listWorkspaceWorkflowStepsAction.mockResolvedValue({ steps: [] });
});

it("prefers the active workspace cookie over stale saved user settings", async () => {
const ui = await GitHubPage({});
render(ui);

expect(mocks.listWorkflowsAction).toHaveBeenCalledWith("ws-go");
expect(mocks.listRepositoriesAction).toHaveBeenCalledWith("ws-go");
expect(mocks.listWorkspaceWorkflowStepsAction).toHaveBeenCalledWith("ws-go");
expect(mocks.capturedInitialState).toMatchObject({
workspaces: { activeId: "ws-go" },
userSettings: { workspaceId: "ws-go" },
});
});
});
26 changes: 23 additions & 3 deletions apps/web/app/github/page.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { cookies } from "next/headers";
import {
listWorkspacesAction,
listWorkflowsAction,
Expand All @@ -7,6 +8,7 @@ import {
import { fetchUserSettings } from "@/lib/api";
import { StateHydrator } from "@/components/state-hydrator";
import { mapUserSettingsResponse } from "@/lib/ssr/user-settings";
import { resolveActiveId } from "@/lib/ssr/resolve-active-id";
import { GitHubPageClient } from "./github-page-client";
import type {
Workflow,
Expand All @@ -17,7 +19,15 @@ import type {
} from "@/lib/types/http";
import type { AppState } from "@/lib/state/store";

export default async function GitHubPage() {
type PageProps = {
searchParams?: Promise<Record<string, string | string[] | undefined>>;
};

function resolveParam(value: string | string[] | undefined): string | undefined {
return Array.isArray(value) ? value[0] : value;
}

export default async function GitHubPage({ searchParams }: PageProps = {}) {
let workspaces: Workspace[] = [];
let workflows: Workflow[] = [];
let steps: WorkflowStep[] = [];
Expand All @@ -27,13 +37,23 @@ export default async function GitHubPage() {
let userSettingsResponse: UserSettingsResponse | null = null;

try {
const [workspacesResponse, settingsResponse] = await Promise.all([
const resolvedParams = searchParams ? await searchParams : {};
const workspaceIdParam = resolveParam(resolvedParams.workspaceId);
const [workspacesResponse, settingsResponse, cookieStore] = await Promise.all([
listWorkspacesAction(),
fetchUserSettings({ cache: "no-store" }).catch(() => null),
cookies().catch((error) => {
console.error("Failed to read cookies on GitHub page:", error);
return null;
}),
]);
workspaces = workspacesResponse.workspaces;
userSettingsResponse = settingsResponse;
workspaceId = settingsResponse?.settings?.workspace_id || workspaces[0]?.id;
const settingsWorkspaceId = settingsResponse?.settings?.workspace_id || null;
const cookieWorkspaceId = cookieStore?.get("office-active-workspace")?.value ?? null;
workspaceId =
resolveActiveId(workspaces, workspaceIdParam, cookieWorkspaceId, settingsWorkspaceId) ??
undefined;

if (workspaceId) {
const [workflowsRes, reposRes, stepsRes] = await Promise.all([
Expand Down