fix(web): apply repository filter correctly on task board (#1215)
This commit is contained in:
@@ -24,6 +24,7 @@ import {
|
||||
} from "@/lib/types/http";
|
||||
import type { KanbanState } from "@/lib/state/slices";
|
||||
import { findTaskInSnapshots } from "@/lib/kanban/find-task";
|
||||
import { repositorySlug } from "@/lib/repository-slug";
|
||||
import { resolvePreferredSessionId } from "../task-select-helpers";
|
||||
|
||||
// Map workflow snapshot to kanban state on workspace switch.
|
||||
@@ -162,7 +163,7 @@ export function useSheetData(workspaceId: string | null) {
|
||||
const repositories = workspaceId ? (repositoriesByWorkspace[workspaceId] ?? []) : [];
|
||||
const ctx: SheetItemCtx = {
|
||||
repositoryPathsById: new Map(
|
||||
repositories.map((repo: Repository) => [repo.id, repo.local_path]),
|
||||
repositories.map((repo: Repository) => [repo.id, repositorySlug(repo)]),
|
||||
),
|
||||
workflowNameById: new Map(workflows.map((w) => [w.id, w.name])),
|
||||
stepTitleById: new Map(allSteps.map((s) => [s.id, s.title])),
|
||||
|
||||
@@ -1,6 +1,24 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import type { WorkflowSnapshotData } from "@/lib/state/slices/kanban/types";
|
||||
import { workflowStepOptions } from "./use-filter-value-options";
|
||||
import type { Repository } from "@/lib/types/http";
|
||||
import { repositorySlug } from "@/lib/repository-slug";
|
||||
import { repositoryOptions, workflowStepOptions } from "./use-filter-value-options";
|
||||
|
||||
function repo(overrides: Partial<Repository>): Repository {
|
||||
return {
|
||||
id: "r1",
|
||||
workspace_id: "w1",
|
||||
name: "",
|
||||
source_type: "local",
|
||||
local_path: "",
|
||||
provider: "",
|
||||
provider_repo_id: "",
|
||||
provider_owner: "",
|
||||
provider_name: "",
|
||||
default_branch: "main",
|
||||
...overrides,
|
||||
} as Repository;
|
||||
}
|
||||
|
||||
function snapshot(
|
||||
id: string,
|
||||
@@ -20,6 +38,25 @@ function snapshot(
|
||||
};
|
||||
}
|
||||
|
||||
describe("repositoryOptions (#1213)", () => {
|
||||
// The option value becomes the saved filter clause value. It MUST equal what
|
||||
// the task board sets as each task's repositoryPath (repositorySlug) — for a
|
||||
// local repo that is the repo name, NOT the full local_path. Using local_path
|
||||
// here was the bug: the clause matched no task and the board went empty.
|
||||
it("uses repositorySlug as the option value (local repo → name, not local_path)", () => {
|
||||
const local = repo({ name: "kandev", local_path: "/home/carlos/Projects/kandev" });
|
||||
const provider = repo({ provider_owner: "kdlbs", provider_name: "kandev-web" });
|
||||
const options = repositoryOptions({ w1: [local, provider] });
|
||||
expect(options).toEqual([
|
||||
{ value: repositorySlug(local), label: repositorySlug(local) },
|
||||
{ value: repositorySlug(provider), label: repositorySlug(provider) },
|
||||
]);
|
||||
expect(options[0].value).toBe("kandev");
|
||||
expect(options[0].value).not.toBe(local.local_path);
|
||||
expect(options[1].value).toBe("kdlbs/kandev-web");
|
||||
});
|
||||
});
|
||||
|
||||
describe("workflowStepOptions", () => {
|
||||
it("tags each step with its workflow name as the group", () => {
|
||||
const snapshots = {
|
||||
|
||||
@@ -5,6 +5,7 @@ import { useAppStore } from "@/components/state-provider";
|
||||
import type { AppState } from "@/lib/state/store";
|
||||
import type { FilterDimension } from "@/lib/state/slices/ui/sidebar-view-types";
|
||||
import { getExecutorLabel } from "@/lib/executor-icons";
|
||||
import { repositorySlug } from "@/lib/repository-slug";
|
||||
|
||||
type Option = { value: string; label: string; color?: string; group?: string };
|
||||
type Snapshots = AppState["kanbanMulti"]["snapshots"];
|
||||
@@ -45,11 +46,10 @@ function executorTypeOptions(snapshots: Snapshots): Option[] {
|
||||
return [...seen].sort().map((v) => ({ value: v, label: getExecutorLabel(v) }));
|
||||
}
|
||||
|
||||
function repositoryOptions(repositoriesByWorkspace: ReposByWorkspace): Option[] {
|
||||
export function repositoryOptions(repositoriesByWorkspace: ReposByWorkspace): Option[] {
|
||||
const repos = Object.values(repositoriesByWorkspace).flat();
|
||||
return repos.map((r) => {
|
||||
const slug =
|
||||
r.provider_owner && r.provider_name ? `${r.provider_owner}/${r.provider_name}` : r.local_path;
|
||||
const slug = repositorySlug(r);
|
||||
return { value: slug, label: slug };
|
||||
});
|
||||
}
|
||||
|
||||
@@ -24,6 +24,7 @@ import { useSidebarTaskPrefs } from "@/hooks/domains/sidebar/use-sidebar-task-pr
|
||||
import { useTaskActions, useArchiveAndSwitchTask } from "@/hooks/use-task-actions";
|
||||
import { useTaskRemoval } from "@/hooks/use-task-removal";
|
||||
import { findTaskInSnapshots } from "@/lib/kanban/find-task";
|
||||
import { repositorySlug } from "@/lib/repository-slug";
|
||||
import { buildSwitchToSession, selectTaskWithLayout } from "./task-select-helpers";
|
||||
import { getSessionInfoForTask } from "@/lib/utils/session-info";
|
||||
import { getWebSocketClient } from "@/lib/ws/connection";
|
||||
@@ -239,12 +240,7 @@ function useSidebarData(workspaceId: string | null) {
|
||||
const tasksWithRepositories = useMemo(() => {
|
||||
const repositories = workspaceId ? (repositoriesByWorkspace[workspaceId] ?? []) : [];
|
||||
const repositorySlugById = new Map(
|
||||
repositories.map((repo: Repository) => [
|
||||
repo.id,
|
||||
repo.provider_owner && repo.provider_name
|
||||
? `${repo.provider_owner}/${repo.provider_name}`
|
||||
: repo.name || repo.local_path?.split("/").filter(Boolean).pop() || repo.local_path,
|
||||
]),
|
||||
repositories.map((repo: Repository) => [repo.id, repositorySlug(repo)]),
|
||||
);
|
||||
const titleById = new Map(allTasks.map((t) => [t.id, t.title]));
|
||||
const workflowNameById = new Map(workflows.map((w) => [w.id, w.name]));
|
||||
|
||||
@@ -114,6 +114,15 @@ export class SidebarFilterPopoverPage {
|
||||
await this.clauseRow(index).getByTestId("filter-value-input").fill(value);
|
||||
}
|
||||
|
||||
/** Pick a single-select enum clause value (e.g. repository) by its option label. */
|
||||
async setClauseEnumValue(index: number, optionLabel: string): Promise<void> {
|
||||
const trigger = this.clauseRow(index).getByTestId("filter-value-select");
|
||||
await trigger.click();
|
||||
// No .first(): exact:true + unique slugs mean a single match, so Playwright's
|
||||
// strict mode surfaces a duplicate-option regression instead of masking it.
|
||||
await this.page.getByRole("option", { name: optionLabel, exact: true }).click();
|
||||
}
|
||||
|
||||
async removeClause(index: number): Promise<void> {
|
||||
await this.clauseRow(index).getByTestId("filter-clause-remove").click();
|
||||
}
|
||||
|
||||
@@ -12,7 +12,11 @@
|
||||
* - Draft semantics + discard
|
||||
* - Last-view deletion guard
|
||||
*/
|
||||
import path from "node:path";
|
||||
import fs from "node:fs";
|
||||
import { execSync } from "node:child_process";
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { makeGitEnv } from "../../helpers/git-helper";
|
||||
import { SessionPage } from "../../pages/session-page";
|
||||
import { SidebarFilterPopoverPage } from "../../pages/sidebar-filter-popover";
|
||||
|
||||
@@ -304,6 +308,67 @@ test.describe("Sidebar filter — saved views CRUD", () => {
|
||||
});
|
||||
});
|
||||
|
||||
test.describe("Sidebar filter — repository dimension (#1213)", () => {
|
||||
test("filtering by a repository keeps only that repo's tasks (not an empty board)", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
backend,
|
||||
seedData,
|
||||
}) => {
|
||||
// The default seeded repo is a LOCAL repo (no GitHub provider), named
|
||||
// "E2E Repo". Regression for #1213: a local repo's filter option value used
|
||||
// to be its full local_path while each task carried the repo *name*, so a
|
||||
// saved repository clause matched nothing and the board went empty.
|
||||
|
||||
// A second, distinct local repo so we can prove the clause keeps matches and
|
||||
// drops non-matches.
|
||||
const repoBName = "Second E2E Repo";
|
||||
const repoBDir = path.join(backend.tmpDir, "repos", "e2e-repo-filter-b");
|
||||
fs.mkdirSync(repoBDir, { recursive: true });
|
||||
const gitEnv = makeGitEnv(backend.tmpDir);
|
||||
execSync("git init -b main", { cwd: repoBDir, env: gitEnv });
|
||||
execSync('git commit --allow-empty -m "init"', { cwd: repoBDir, env: gitEnv });
|
||||
const repoB = await apiClient.createRepository(seedData.workspaceId, repoBDir, "main", {
|
||||
name: repoBName,
|
||||
});
|
||||
|
||||
await apiClient.createTask(seedData.workspaceId, "Task in repo A", {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [seedData.repositoryId],
|
||||
});
|
||||
await apiClient.createTask(seedData.workspaceId, "Task in repo B", {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
repository_ids: [repoB.id],
|
||||
});
|
||||
|
||||
const navTask = await apiClient.createTask(seedData.workspaceId, "Repo Filter Nav", {
|
||||
workflow_id: seedData.workflowId,
|
||||
workflow_step_id: seedData.startStepId,
|
||||
});
|
||||
await testPage.goto(`/t/${navTask.id}`);
|
||||
const session = new SessionPage(testPage);
|
||||
await session.waitForLoad();
|
||||
await expect(session.sidebar).toBeVisible({ timeout: 10_000 });
|
||||
const filters = new SidebarFilterPopoverPage(testPage);
|
||||
await expect(filters.bar).toBeVisible();
|
||||
|
||||
// Both tasks visible before filtering.
|
||||
await expect(session.sidebar.getByText("Task in repo A")).toBeVisible();
|
||||
await expect(session.sidebar.getByText("Task in repo B")).toBeVisible();
|
||||
|
||||
await filters.addFilterRow();
|
||||
await filters.setClauseDimension(0, "Repository");
|
||||
await filters.setClauseEnumValue(0, "E2E Repo");
|
||||
await filters.close();
|
||||
|
||||
// Repo A's task survives (this was empty before the fix); repo B's is hidden.
|
||||
await expect(session.sidebar.getByText("Task in repo A")).toBeVisible();
|
||||
await expect(session.sidebar.getByText("Task in repo B")).toHaveCount(0);
|
||||
});
|
||||
});
|
||||
|
||||
test.describe("Sidebar filter — draft semantics", () => {
|
||||
test("dirty indicator appears after edits, clears on discard", async ({
|
||||
testPage,
|
||||
|
||||
@@ -0,0 +1,45 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import type { Repository } from "@/lib/types/http";
|
||||
import { repositorySlug } from "./repository-slug";
|
||||
|
||||
function repo(overrides: Partial<Repository>): Repository {
|
||||
return {
|
||||
id: "r1",
|
||||
workspace_id: "w1",
|
||||
name: "",
|
||||
source_type: "local",
|
||||
local_path: "",
|
||||
provider: "",
|
||||
provider_repo_id: "",
|
||||
provider_owner: "",
|
||||
provider_name: "",
|
||||
default_branch: "main",
|
||||
...overrides,
|
||||
} as Repository;
|
||||
}
|
||||
|
||||
describe("repositorySlug", () => {
|
||||
it("uses owner/name for provider-backed repos", () => {
|
||||
expect(repositorySlug(repo({ provider_owner: "kdlbs", provider_name: "kandev" }))).toBe(
|
||||
"kdlbs/kandev",
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the repo name for local repos (matches board grouping identity)", () => {
|
||||
expect(
|
||||
repositorySlug(repo({ name: "kandev", local_path: "/home/carlos/Projects/kandev" })),
|
||||
).toBe("kandev");
|
||||
});
|
||||
|
||||
it("falls back to the last path segment when name is empty", () => {
|
||||
expect(repositorySlug(repo({ name: "", local_path: "/home/carlos/Projects/kandev" }))).toBe(
|
||||
"kandev",
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the full local_path when no path segment is available", () => {
|
||||
// "/" collapses to an empty array after split/filter, so the final
|
||||
// `|| repo.local_path` branch is the one actually exercised here.
|
||||
expect(repositorySlug(repo({ name: "", local_path: "/" }))).toBe("/");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,14 @@
|
||||
import type { Repository } from "@/lib/types/http";
|
||||
|
||||
/**
|
||||
* Canonical identity for a repository — the task board's per-task
|
||||
* `repositoryPath`, the sidebar filter's option value, and the grouping key.
|
||||
* All sites MUST derive it identically, or a saved repo filter compares
|
||||
* mismatched strings and silently matches nothing (#1213).
|
||||
*/
|
||||
export function repositorySlug(repo: Repository): string {
|
||||
if (repo.provider_owner && repo.provider_name) {
|
||||
return `${repo.provider_owner}/${repo.provider_name}`;
|
||||
}
|
||||
return repo.name || repo.local_path.split("/").filter(Boolean).pop() || repo.local_path;
|
||||
}
|
||||
@@ -3,6 +3,8 @@ import type { TaskSwitcherItem } from "@/components/task/task-switcher";
|
||||
import { applyFilters, applyGroup, applySort, applyView, mergeGroupOrder } from "./apply-view";
|
||||
import type { FilterClause, SidebarView } from "@/lib/state/slices/ui/sidebar-view-types";
|
||||
import { DEFAULT_VIEW } from "@/lib/state/slices/ui/sidebar-view-builtins";
|
||||
import type { Repository } from "@/lib/types/http";
|
||||
import { repositorySlug } from "@/lib/repository-slug";
|
||||
|
||||
function task(overrides: Partial<TaskSwitcherItem>): TaskSwitcherItem {
|
||||
return {
|
||||
@@ -147,6 +149,38 @@ describe("applyFilters — per-dimension", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("applyFilters — repository (#1213)", () => {
|
||||
// The saved clause value is the filter option value, and the board sets each
|
||||
// task's repositoryPath — BOTH via repositorySlug. For a local repo the slug
|
||||
// is the repo name (not the full local_path). The bug was that the option
|
||||
// value used local_path while the board used the name, so the clause matched
|
||||
// nothing and the board went empty.
|
||||
it("filters by a local repository (option value matches the board task field)", () => {
|
||||
const localRepo = {
|
||||
id: "r1",
|
||||
workspace_id: "w1",
|
||||
name: "kandev",
|
||||
source_type: "local",
|
||||
local_path: "/home/carlos/Projects/kandev",
|
||||
provider: "",
|
||||
provider_repo_id: "",
|
||||
provider_owner: "",
|
||||
provider_name: "",
|
||||
default_branch: "main",
|
||||
} as Repository;
|
||||
const optionValue = repositorySlug(localRepo);
|
||||
// The option value is the repo name, never the full filesystem path.
|
||||
expect(optionValue).toBe("kandev");
|
||||
expect(optionValue).not.toBe(localRepo.local_path);
|
||||
const tasks = [
|
||||
task({ id: "a", repositoryPath: repositorySlug(localRepo) }),
|
||||
task({ id: "b", repositoryPath: "org/other" }),
|
||||
];
|
||||
const out = applyFilters(tasks, [C({ dimension: "repository", op: "is", value: optionValue })]);
|
||||
expect(out.map((t) => t.id)).toEqual(["a"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("applyFilters — titleMatch + combos", () => {
|
||||
it("filters by titleMatch matches (case-insensitive substring)", () => {
|
||||
const tasks = [
|
||||
|
||||
Reference in New Issue
Block a user