diff --git a/frontend/src/App.grouping-toggle.browser.svelte.ts b/frontend/src/App.grouping-toggle.browser.svelte.ts index b87023113..3afc6d5d0 100644 --- a/frontend/src/App.grouping-toggle.browser.svelte.ts +++ b/frontend/src/App.grouping-toggle.browser.svelte.ts @@ -206,15 +206,24 @@ async function selectPullGrouping(label: string): Promise { // inline group buttons are CSS-hidden and grouping lives in the "Filters" // compact dropdown. The "Group" section is last, so pick the last item whose // label matches (the State section may also expose an "All" entry). - const filtersBtn = Array.from(document.querySelectorAll(".compact-filter-menu .filter-btn")).find((b) => - (b.textContent ?? "").includes("Filters"), - )!; + await vi.waitFor(() => expect(document.querySelector(".compact-filter-menu .filter-btn")).not.toBeNull(), WAIT); + const filtersBtn = document.querySelector(".compact-filter-menu .filter-btn")!; await page.elementLocator(filtersBtn).click(); await vi.waitFor(() => expect(document.querySelector(".compact-filter-menu .filter-dropdown")).not.toBeNull(), WAIT); + const compactLabel = compactPullGroupingLabel(label); const items = Array.from(document.querySelectorAll(".compact-filter-menu .filter-dropdown .filter-item")).filter( - (el) => (el.querySelector(".filter-label")?.textContent ?? el.textContent ?? "").trim() === label, + (el) => (el.querySelector(".filter-label")?.textContent ?? el.textContent ?? "").trim() === compactLabel, ); - await page.elementLocator(items[items.length - 1]!).click(); + const item = items[items.length - 1]; + expect(item).toBeDefined(); + await page.elementLocator(item!).click(); +} + +function compactPullGroupingLabel(label: string): string { + if (label === "Repo") return "By repo"; + if (label === "Status") return "By status"; + if (label === "All") return "Flat list"; + return label; } function activityViewBtn(): Element { diff --git a/frontend/tests/e2e-full/grouping-toggle.spec.ts b/frontend/tests/e2e-full/grouping-toggle.spec.ts index 316b2c280..c10b5f188 100644 --- a/frontend/tests/e2e-full/grouping-toggle.spec.ts +++ b/frontend/tests/e2e-full/grouping-toggle.spec.ts @@ -23,8 +23,17 @@ async function selectPullGrouping(page: Page, label: string | RegExp): Promise { diff --git a/frontend/tests/e2e-full/pull-list.spec.ts b/frontend/tests/e2e-full/pull-list.spec.ts index c4d1c9dfc..490f295a7 100644 --- a/frontend/tests/e2e-full/pull-list.spec.ts +++ b/frontend/tests/e2e-full/pull-list.spec.ts @@ -17,8 +17,10 @@ async function selectPullState(page: Page, label: string): Promise { return; } - await page.getByRole("button", { name: "Filters" }).click(); - await page.locator(".filter-dropdown .filter-item", { hasText: label }).first().click(); + const dropdown = await openCompactFilterMenu(page); + await dropdown.locator(".filter-item", { hasText: label }).first().click(); + await page.keyboard.press("Escape"); + await expect(dropdown).toBeHidden(); } async function selectPullGrouping(page: Page, label: string): Promise { @@ -28,8 +30,27 @@ async function selectPullGrouping(page: Page, label: string): Promise { return; } - await page.getByRole("button", { name: "Filters" }).click(); - await page.locator(".filter-dropdown .filter-item", { hasText: label }).last().click(); + const compactLabel = compactPullGroupingLabel(label); + const dropdown = await openCompactFilterMenu(page); + await dropdown.locator(".filter-item", { hasText: compactLabel }).click(); + await page.keyboard.press("Escape"); + await expect(dropdown).toBeHidden(); +} + +function compactPullGroupingLabel(label: string): string { + if (label === "Repo") return "By repo"; + if (label === "Status") return "By status"; + if (label === "All") return "Flat list"; + return label; +} + +async function openCompactFilterMenu(page: Page) { + const dropdown = page.locator(".filter-dropdown"); + if (!(await dropdown.isVisible())) { + await page.locator(".compact-filter-menu .filter-btn").click(); + await expect(dropdown).toBeVisible(); + } + return dropdown; } const longRepoName = "widgets-with-an-extremely-long-repository-name"; diff --git a/frontend/tests/e2e-full/sidebar-collapse.spec.ts b/frontend/tests/e2e-full/sidebar-collapse.spec.ts index abee06b51..e9a1ce13c 100644 --- a/frontend/tests/e2e-full/sidebar-collapse.spec.ts +++ b/frontend/tests/e2e-full/sidebar-collapse.spec.ts @@ -1,4 +1,15 @@ import { expect, test, type Locator, type Page } from "@playwright/test"; +import { startIsolatedE2EServer } from "./support/e2eServer"; + +type PullListRow = { + Title: string; + ReviewDecision: string; + KanbanStatus: string; +}; + +type PullDetailRow = { + merge_request: PullListRow; +}; async function waitForPRList(page: Page): Promise { await page.locator(".pull-item").first().waitFor({ state: "visible", timeout: 10_000 }); @@ -77,9 +88,7 @@ async function expectCompactFiltersAtMinimumWidth( await expect.poll(async () => sidebarWidth(sidebar)).toBe(200); const filterBar = sidebar.locator(".filter-bar").first(); - const compactFilters = filterBar.getByRole("button", { - name: "Filters", - }); + const compactFilters = filterBar.locator(".compact-filter-menu .filter-btn"); await expect(compactFilters).toBeVisible(); await expect(filterBar.locator(".state-toggle")).toBeHidden(); await expect(filterBar.locator(".group-toggle")).toBeHidden(); @@ -103,7 +112,7 @@ async function expectCompactFiltersInNarrowViewport( const sidebar = page.locator(".sidebar").first(); const filterBar = sidebar.locator(".filter-bar").first(); - await expect(filterBar.getByRole("button", { name: "Filters" })).toBeVisible(); + await expect(filterBar.locator(".compact-filter-menu .filter-btn")).toBeVisible(); await expect(filterBar.locator(".state-toggle")).toBeHidden(); await expect(filterBar.locator(".group-toggle")).toBeHidden(); } @@ -125,21 +134,22 @@ async function setPersistedSidebarWidth( } async function expectCompactFilterBar(filterBar: Locator): Promise { - await expect(filterBar.getByRole("button", { name: "Filters" })).toBeVisible(); + await expect(filterBar.locator(".compact-filter-menu .filter-btn")).toBeVisible(); + await expect(filterBar.locator(".local-filter-menu .filter-btn")).toBeHidden(); await expect(filterBar.locator(".state-toggle")).toBeHidden(); await expect(filterBar.locator(".group-toggle")).toBeHidden(); await expectFastAnimation(filterBar.locator(".compact-filter-menu")); } async function openCompactFilters(filterBar: Locator): Promise { - await filterBar.getByRole("button", { name: "Filters" }).click(); + await filterBar.locator(".compact-filter-menu .filter-btn").click(); const dropdown = filterBar.page().locator(".filter-dropdown"); await expect(dropdown).toBeVisible(); return dropdown; } async function expectExpandedFilterBar(filterBar: Locator): Promise { - await expect(filterBar.getByRole("button", { name: "Filters" })).toBeHidden(); + await expect(filterBar.locator(".compact-filter-menu .filter-btn")).toBeHidden(); await expect(filterBar.locator(".state-toggle")).toBeVisible(); await expect(filterBar.locator(".group-toggle")).toBeVisible(); await expectFastAnimation(filterBar.locator(".state-toggle")); @@ -150,6 +160,32 @@ async function expectExpandedFilterBar(filterBar: Locator): Promise { expect(filterMetrics.scrollWidth).toBeLessThanOrEqual(filterMetrics.clientWidth); } +async function expectPullLocalFilterIconOnly(filterBar: Locator): Promise { + const localFilter = filterBar.locator(".local-filter-menu"); + await expect(filterBar.locator(".state-toggle")).toBeVisible(); + await expect(filterBar.locator(".group-toggle")).toBeVisible(); + await expect(filterBar.locator(".compact-filter-menu")).toBeHidden(); + await expect(localFilter.locator(".filter-trigger-label")).toHaveCSS("display", "none"); + + const triggerWidth = await localFilter + .locator(".filter-btn") + .evaluate((node) => Math.round(node.getBoundingClientRect().width)); + expect(triggerWidth).toBe(34); + + const filterMetrics = await filterBar.evaluate((node) => ({ + clientWidth: node.clientWidth, + scrollWidth: node.scrollWidth, + })); + expect(filterMetrics.scrollWidth).toBeLessThanOrEqual(filterMetrics.clientWidth); +} + +async function expectPullLocalFilterLabeled(filterBar: Locator): Promise { + await expectExpandedFilterBar(filterBar); + const localFilter = filterBar.locator(".local-filter-menu"); + await expect(localFilter.locator(".filter-trigger-label")).toHaveText("PR filters"); + await expect(localFilter.locator(".filter-trigger-label")).not.toHaveCSS("display", "none"); +} + async function expectFastAnimation(locator: Locator): Promise { const durationMs = await locator.evaluate((node) => { const durations = getComputedStyle(node) @@ -269,9 +305,14 @@ test.describe("collapsible sidebar", () => { await expectCompactFiltersInNarrowViewport(page, "/issues", waitForIssueList); }); - test("pull filters switch at the buffered 396px fit point", async ({ page }) => { - await expectCompactFilterBar(await setPersistedSidebarWidth(page, "/pulls", 395, waitForPRList)); - await expectExpandedFilterBar(await setPersistedSidebarWidth(page, "/pulls", 396, waitForPRList)); + test("pull filters switch at the buffered 431px fit point", async ({ page }) => { + await expectCompactFilterBar(await setPersistedSidebarWidth(page, "/pulls", 430, waitForPRList)); + await expectExpandedFilterBar(await setPersistedSidebarWidth(page, "/pulls", 431, waitForPRList)); + }); + + test("pull PR filter becomes icon-only before the filter row compacts", async ({ page }) => { + await expectPullLocalFilterIconOnly(await setPersistedSidebarWidth(page, "/pulls", 520, waitForPRList)); + await expectPullLocalFilterLabeled(await setPersistedSidebarWidth(page, "/pulls", 521, waitForPRList)); }); test("issue filters switch at the buffered 373px fit point", async ({ page }) => { @@ -283,28 +324,95 @@ test.describe("collapsible sidebar", () => { const filterBar = await setPersistedSidebarWidth(page, "/pulls", 395, waitForPRList); await expectCompactFilterBar(filterBar); - let dropdown = await openCompactFilters(filterBar); + const dropdown = await openCompactFilters(filterBar); + await expect(dropdown.locator(".filter-section-title", { hasText: "PR" })).toBeVisible(); + await expect(dropdown.locator(".filter-section-title", { hasText: "Kanban" })).toBeVisible(); await dropdown.locator(".filter-item", { hasText: "Closed" }).click(); await expect(filterBar.page().locator(".state-note")).toBeVisible(); + await expect(dropdown).toBeVisible(); - dropdown = await openCompactFilters(filterBar); - await dropdown.locator(".filter-item", { hasText: "All" }).last().click(); + await dropdown.locator(".filter-item", { hasText: "Flat list" }).click(); + await expect(dropdown).toBeVisible(); await expect(page.locator(".repo-header")).toHaveCount(0, { timeout: 5_000, }); await expect(page.locator(".repo-chip").first()).toBeVisible(); }); + test("pull compact filters apply PR attributes and kanban status against the real API", async ({ page }) => { + const server = await startIsolatedE2EServer(); + try { + const stateResponse = await page.request.put(`${server.info.base_url}/api/v1/pulls/github/acme/widgets/1/state`, { + data: { status: "reviewing" }, + }); + expect(stateResponse.ok()).toBe(true); + + const pullsResponse = await page.request.get(`${server.info.base_url}/api/v1/pulls?state=open`); + expect(pullsResponse.ok()).toBe(true); + const pulls = (await pullsResponse.json()) as PullListRow[]; + const expectedTitles = pulls + .filter((pull) => pull.ReviewDecision.trim().toUpperCase() === "APPROVED" && pull.KanbanStatus === "reviewing") + .map((pull) => pull.Title); + expect(expectedTitles).toEqual(["Add widget caching layer"]); + + const filterBar = await setPersistedSidebarWidth(page, `${server.info.base_url}/pulls`, 395, waitForPRList); + await expectCompactFilterBar(filterBar); + + const dropdown = await openCompactFilters(filterBar); + await dropdown.getByRole("button", { name: "Approved", exact: true }).click(); + await dropdown.getByRole("button", { name: "Reviewing", exact: true }).click(); + + const rows = page.locator(".pull-item"); + await expect(rows).toHaveCount(expectedTitles.length); + for (const title of expectedTitles) { + await expect(page.locator(".pull-item", { hasText: title })).toBeVisible(); + } + } finally { + await server.stop(); + } + }); + + test("pull compact filters keep default-new PRs visible through the real API", async ({ page }) => { + const server = await startIsolatedE2EServer(); + try { + const detailResponse = await page.request.get(`${server.info.base_url}/api/v1/pulls/github/acme/widgets/1`); + expect(detailResponse.ok()).toBe(true); + const detail = (await detailResponse.json()) as PullDetailRow; + expect(detail.merge_request.KanbanStatus).toBe("new"); + + const pullsResponse = await page.request.get(`${server.info.base_url}/api/v1/pulls?state=open`); + expect(pullsResponse.ok()).toBe(true); + const pulls = (await pullsResponse.json()) as PullListRow[]; + const expectedTitles = pulls.filter((pull) => pull.KanbanStatus === "new").map((pull) => pull.Title); + expect(expectedTitles).toContain("Add widget caching layer"); + + const filterBar = await setPersistedSidebarWidth(page, `${server.info.base_url}/pulls`, 395, waitForPRList); + await expectCompactFilterBar(filterBar); + + const dropdown = await openCompactFilters(filterBar); + await dropdown.getByRole("button", { name: "New", exact: true }).click(); + + const rows = page.locator(".pull-item"); + await expect(rows).toHaveCount(expectedTitles.length); + for (const title of expectedTitles) { + await expect(page.locator(".pull-item", { hasText: title })).toBeVisible(); + } + } finally { + await server.stop(); + } + }); + test("issue compact filters update state and grouping", async ({ page }) => { const filterBar = await setPersistedSidebarWidth(page, "/issues", 372, waitForIssueList); await expectCompactFilterBar(filterBar); - let dropdown = await openCompactFilters(filterBar); + const dropdown = await openCompactFilters(filterBar); await dropdown.locator(".filter-item", { hasText: "Closed" }).click(); await expect(filterBar.page().locator(".state-note")).toBeVisible(); + await expect(dropdown).toBeVisible(); - dropdown = await openCompactFilters(filterBar); await dropdown.locator(".filter-item", { hasText: "All" }).last().click(); + await expect(dropdown).toBeVisible(); await expect(page.locator(".repo-header")).toHaveCount(0, { timeout: 5_000, }); diff --git a/frontend/tests/e2e/pull-list-filters.spec.ts b/frontend/tests/e2e/pull-list-filters.spec.ts new file mode 100644 index 000000000..853d251ab --- /dev/null +++ b/frontend/tests/e2e/pull-list-filters.spec.ts @@ -0,0 +1,119 @@ +import { expect, test, type Page } from "@playwright/test"; + +import { mockApi } from "./support/mockApi"; + +const repo = { + provider: "github", + platform_host: "github.com", + repo_path: "acme/widgets", + owner: "acme", + name: "widgets", + capabilities: {}, +}; + +function pull(number: number, title: string, overrides: Record = {}) { + return { + ID: number, + RepoID: 1, + GitHubID: 1000 + number, + Number: number, + URL: `https://github.com/acme/widgets/pull/${number}`, + Title: title, + Author: "marius", + State: "open", + IsDraft: false, + Body: "", + HeadBranch: `feature/${number}`, + BaseBranch: "main", + Additions: 10, + Deletions: 1, + CommentCount: 0, + ReviewDecision: "", + CIStatus: "success", + CIChecksJSON: "[]", + CreatedAt: "2026-03-29T14:00:00Z", + UpdatedAt: "2026-03-30T14:00:00Z", + LastActivityAt: "2026-03-30T14:00:00Z", + MergedAt: null, + ClosedAt: null, + MergeableState: "clean", + KanbanStatus: "new", + Starred: false, + repo_owner: "acme", + repo_name: "widgets", + platform_host: "github.com", + platform_head_sha: `${number}`.padStart(40, "a"), + repo, + worktree_links: [], + ...overrides, + }; +} + +async function mockPulls(page: Page) { + await mockApi(page); + await page.route("**/api/v1/pulls**", async (route) => { + const url = new URL(route.request().url()); + if (route.request().method() !== "GET" || url.pathname !== "/api/v1/pulls") { + await route.fallback(); + return; + } + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify([ + pull(10, "Approved review queue", { + ReviewDecision: "APPROVED", + KanbanStatus: "reviewing", + }), + pull(11, "Draft parser cleanup", { + IsDraft: true, + KanbanStatus: "waiting", + }), + pull(12, "Ready failed workflow", { + CIStatus: "failure", + KanbanStatus: "awaiting_merge", + }), + pull(13, "Ready conflict resolver", { + MergeableState: "dirty", + KanbanStatus: "new", + }), + ]), + }); + }); +} + +async function openPrFilters(page: Page): Promise { + const standaloneTrigger = page.getByTitle("PR filters"); + if (await standaloneTrigger.isVisible()) { + await standaloneTrigger.click(); + } else { + await page.locator(".compact-filter-menu .filter-btn").click(); + } + await expect(page.locator(".filter-dropdown")).toBeVisible(); +} + +test("PR filters stack attributes and allow multiple kanban statuses", async ({ page }) => { + await mockPulls(page); + await page.goto("/pulls"); + + const rows = page.locator(".pr-list-row"); + await expect(rows).toHaveCount(4); + + await openPrFilters(page); + await page.locator(".filter-dropdown").getByRole("button", { name: "Ready for review" }).click(); + await page.locator(".filter-dropdown").getByRole("button", { name: "Reviewing" }).click(); + await page.locator(".filter-dropdown").getByRole("button", { name: "Awaiting merge" }).click(); + + await expect(rows).toHaveText([/Approved review queue/, /Ready failed workflow/]); + + await page.locator(".filter-dropdown").getByRole("button", { name: "Failed CI" }).click(); + await expect(rows).toHaveText([/Ready failed workflow/]); + + await page + .locator(".filter-dropdown") + .getByRole("button", { name: /^(Clear filters|Reset view)$/ }) + .click(); + await page.locator(".filter-dropdown").getByRole("button", { name: "Merge conflicts" }).click(); + + await expect(rows).toHaveText([/Ready conflict resolver/]); +}); diff --git a/internal/db/queries.go b/internal/db/queries.go index 754d227cb..7f18c3f94 100644 --- a/internal/db/queries.go +++ b/internal/db/queries.go @@ -2499,7 +2499,11 @@ func (d *DB) ListMergeRequests(ctx context.Context, opts ListMergeRequestsOpts) args = append(args, owner, name) } if opts.KanbanState != "" { - conds = append(conds, "COALESCE(k.status, '') = ?") + if opts.KanbanState == string(KanbanStatusNew) { + conds = append(conds, "COALESCE(k.status, 'new') = ?") + } else { + conds = append(conds, "k.status = ?") + } args = append(args, opts.KanbanState) } if opts.Starred { diff --git a/internal/db/queries_test.go b/internal/db/queries_test.go index 12a9f15c9..df2b80e14 100644 --- a/internal/db/queries_test.go +++ b/internal/db/queries_test.go @@ -2597,14 +2597,13 @@ func TestListPullRequestsFilterByKanban(t *testing.T) { repoID := insertTestRepo(t, d, "owner", "repo") base := baseTime() - id1 := insertTestMR(t, d, repoID, 1, "pr 1", base) + insertTestMR(t, d, repoID, 1, "pr 1", base) id2 := insertTestMR(t, d, repoID, 2, "pr 2", base.Add(time.Hour)) id3 := insertTestMR(t, d, repoID, 3, "pr 3", base.Add(2*time.Hour)) // Set PR 2 to "reviewing". require.NoError(d.SetKanbanState(ctx, id2, "reviewing")) - // Ensure kanban for PR 1 and 3 (status = "new"). - require.NoError(d.EnsureKanbanState(ctx, id1)) + // Leave PR 1 without a kanban row; the board treats missing rows as "new". require.NoError(d.EnsureKanbanState(ctx, id3)) prs, err := d.ListMergeRequests(ctx, ListMergeRequestsOpts{KanbanState: "reviewing"}) @@ -2612,6 +2611,11 @@ func TestListPullRequestsFilterByKanban(t *testing.T) { require.Len(prs, 1) assert.Equal(2, prs[0].Number) assert.Equal(KanbanStatusReviewing, prs[0].KanbanStatus) + + prs, err = d.ListMergeRequests(ctx, ListMergeRequestsOpts{KanbanState: "new"}) + require.NoError(err) + require.Len(prs, 2) + assert.Equal([]int{3, 1}, []int{prs[0].Number, prs[1].Number}) } func TestListMergeRequests_AttachesLabels(t *testing.T) { diff --git a/internal/platform/gitlab/client.go b/internal/platform/gitlab/client.go index 14fc4840c..e24db2509 100644 --- a/internal/platform/gitlab/client.go +++ b/internal/platform/gitlab/client.go @@ -399,12 +399,19 @@ func (c *Client) projectCloneURL(ctx context.Context, projectID int64) (string, return cached, nil } - project, _, err := c.api.Projects.GetProject(projectID, nil, gitlab.WithContext(ctx)) + project, _, err := c.api.Projects.GetProject( + projectID, + nil, + gitlab.WithContext(ctx), + gitlab.WithRequestRetry(func(context.Context, *http.Response, error) (bool, error) { + return false, nil + }), + ) if err != nil || project == nil { if err == nil { err = errors.New("source project response was empty") } - return "", mapGitLabError("get_source_project", err) + return "", mapSourceProjectLookupError(err) } cloneURL := strings.TrimSpace(project.HTTPURLToRepo) c.projectCloneURLMu.Lock() @@ -892,6 +899,26 @@ func mapGitLabErrorForHost(platformHost, capability string, err error) error { } } +func mapSourceProjectLookupError(err error) error { + if err == nil { + return nil + } + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) || errors.Is(err, gitlab.ErrNotFound) { + return err + } + var gitlabErr *gitlab.ErrorResponse + if errors.As(err, &gitlabErr) { + switch { + case gitlabErr.HasStatusCode(http.StatusUnauthorized), + gitlabErr.HasStatusCode(http.StatusForbidden), + gitlabErr.HasStatusCode(http.StatusNotFound), + gitlabErr.HasStatusCode(http.StatusTooManyRequests): + return mapGitLabError("get_source_project", err) + } + } + return err +} + // isGitLabStatus reports whether err carries a typed GitLab response with the // given status. It matches only on the typed *gitlab.ErrorResponse (errors.As // unwraps platform.Error). It deliberately does NOT fall back to substring diff --git a/internal/platform/gitlab/client_test.go b/internal/platform/gitlab/client_test.go index 7059ab2ed..e4514b817 100644 --- a/internal/platform/gitlab/client_test.go +++ b/internal/platform/gitlab/client_test.go @@ -178,10 +178,10 @@ func TestClientListOpenMergeRequestsPropagatesTransientForkHeadRepoLookupFailure switch r.URL.EscapedPath() { case "/api/v4/projects/42/merge_requests": writeJSON(w, `[ - {"id": 1001, "iid": 7, "project_id": 42, "source_project_id": 77, "target_project_id": 42, "source_branch": "feature/auth", "target_branch": "main", "title": "Fork base", "state": "opened"} + {"id": 1001, "iid": 7, "project_id": 42, "source_project_id": 404, "target_project_id": 42, "source_branch": "feature/auth", "target_branch": "main", "title": "Fork base", "state": "opened"} ]`) - case "/api/v4/projects/77": - http.Error(w, "rate limited", http.StatusTooManyRequests) + case "/api/v4/projects/404": + http.Error(w, "temporary failure", http.StatusBadGateway) default: http.NotFound(w, r) } @@ -200,9 +200,8 @@ func TestClientListOpenMergeRequestsPropagatesTransientForkHeadRepoLookupFailure _, err := client.ListOpenMergeRequests(context.Background(), ref) require.Error(err) var platformErr *platform.Error - require.ErrorAs(err, &platformErr) - assert.Equal(platform.ErrCodeRateLimited, platformErr.Code) - assert.Equal("get_source_project", platformErr.Capability) + assert.NotErrorAs(err, &platformErr) + assert.Contains(err.Error(), "temporary failure") } func TestClientGetMergeRequestContinuesWhenForkHeadRepoLookupFails(t *testing.T) { diff --git a/internal/server/api_test.go b/internal/server/api_test.go index 0e841d671..b62169570 100644 --- a/internal/server/api_test.go +++ b/internal/server/api_test.go @@ -1558,6 +1558,56 @@ func TestAPIListPulls(t *testing.T) { assert.Equal("acme/widget", body[0].Repo.RepoPath) } +func TestAPIPullResponsesNormalizeMissingKanbanStateToNew(t *testing.T) { + require := require.New(t) + assert := Assert.New(t) + srv, database := setupTestServer(t) + ctx := t.Context() + + repoID, err := database.UpsertRepo(ctx, db.GitHubRepoIdentity("github.com", "acme", "widget")) + require.NoError(err) + now := time.Now().UTC().Truncate(time.Second) + mrID, err := database.UpsertMergeRequest(ctx, &db.MergeRequest{ + RepoID: repoID, + PlatformID: 7000, + Number: 7, + URL: "https://github.com/acme/widget/pull/7", + Title: "Default kanban PR", + Author: "alice", + State: "open", + HeadBranch: "feature/default-kanban", + BaseBranch: "main", + CreatedAt: now, + UpdatedAt: now, + LastActivityAt: now, + }) + require.NoError(err) + kanbanState, err := database.GetKanbanState(ctx, mrID) + require.NoError(err) + require.Nil(kanbanState) + + rawList := doJSON(t, srv, http.MethodGet, "/api/v1/pulls", nil) + require.Equal(http.StatusOK, rawList.Code) + var list []mergeRequestResponse + require.NoError(json.Unmarshal(rawList.Body.Bytes(), &list)) + require.Len(list, 1) + assert.Equal(db.KanbanStatusNew, list[0].KanbanStatus) + + rawNewList := doJSON(t, srv, http.MethodGet, "/api/v1/pulls?kanban=new", nil) + require.Equal(http.StatusOK, rawNewList.Code) + var newList []mergeRequestResponse + require.NoError(json.Unmarshal(rawNewList.Body.Bytes(), &newList)) + require.Len(newList, 1) + assert.Equal(db.KanbanStatusNew, newList[0].KanbanStatus) + + rawDetail := doJSON(t, srv, http.MethodGet, "/api/v1/pulls/gh/acme/widget/7", nil) + require.Equal(http.StatusOK, rawDetail.Code) + var detail mergeRequestDetailResponse + require.NoError(json.Unmarshal(rawDetail.Body.Bytes(), &detail)) + require.NotNil(detail.MergeRequest) + assert.Equal(db.KanbanStatusNew, detail.MergeRequest.KanbanStatus) +} + func TestAPIListItemsIncludeWorkspaceRefs(t *testing.T) { require := require.New(t) assert := Assert.New(t) @@ -24655,7 +24705,7 @@ func TestWorkspaceRuntimePlainShellAfterExitStartsFreshE2E(t *testing.T) { Command: []string{filepath.Join(t.TempDir(), "missing-tmux")}, }, Shell: config.Shell{ - Command: serverRuntimeHelperCommand("pty-close-then-sleep"), + Command: serverRuntimeHelperCommand("pty-close-on-input-then-sleep"), }, } ptyOwnerDir := filepath.Join(t.TempDir(), "pty-owner") @@ -24679,8 +24729,8 @@ func TestWorkspaceRuntimePlainShellAfterExitStartsFreshE2E(t *testing.T) { wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/ws/v1/workspaces/" + ws.Id + "/runtime/sessions/" + first.Key + "/terminal?cols=80&rows=24" - conn, _, err := websocket.Dial(ctx, wsURL, nil) - require.NoError(err) + conn := dialWebSocketForTest(t, ctx, wsURL, "plain shell after exit") + require.NoError(conn.Write(ctx, websocket.MessageBinary, []byte("exit\n"))) readCtx, cancel := context.WithTimeout(ctx, 5*time.Second) defer cancel() for { diff --git a/internal/server/e2etest/gitlab_sync_pin_test.go b/internal/server/e2etest/gitlab_sync_pin_test.go index 1a44aaaf1..d07e4d0c7 100644 --- a/internal/server/e2etest/gitlab_sync_pin_test.go +++ b/internal/server/e2etest/gitlab_sync_pin_test.go @@ -62,6 +62,99 @@ func setupGitLabCloneFixture(t *testing.T) (cloneURL, baseSHA, headSHA string) { return cloneURL, baseSHA, headSHA } +func TestGitLabSyncPRSurfacesTransientForkSourceProjectLookupAsUpstream(t *testing.T) { + require := require.New(t) + assert := assert.New(t) + ctx := t.Context() + recorder := &gitlabAPIRecorder{} + + api := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + recorder.record(r) + w.Header().Set("Content-Type", "application/json") + switch { + case r.URL.EscapedPath() == "/api/v4/projects/4242/merge_requests/7" && r.Method == http.MethodGet: + writeGitLabJSON(w, `{ + "id": 7001, "iid": 7, "project_id": 4242, "target_project_id": 4242, "source_project_id": 404, + "title": "Fork MR", "state": "opened", + "source_branch": "feature", "target_branch": "main", + "author": {"username": "author"}, + "created_at": "2026-05-01T09:00:00Z", + "updated_at": "2026-05-01T10:00:00Z" + }`) + case r.URL.EscapedPath() == "/api/v4/projects/404" && r.Method == http.MethodGet: + w.WriteHeader(http.StatusBadGateway) + writeGitLabJSON(w, `{"message": "temporary failure"}`) + default: + http.NotFound(w, r) + } + })) + t.Cleanup(api.Close) + + client, err := platformgitlab.NewClient( + "gitlab.com", + staticGitLabTokenSource("token"), + platformgitlab.WithBaseURLForTesting(api.URL+"/api/v4"), + ) + require.NoError(err) + registry, err := platform.NewRegistry(client) + require.NoError(err) + + database := dbtest.Open(t) + _, err = database.UpsertRepo(ctx, db.RepoIdentity{ + Platform: "gitlab", + PlatformHost: "gitlab.com", + PlatformRepoID: "4242", + Owner: "acme", + Name: "widget", + RepoPath: "acme/widget", + }) + require.NoError(err) + + repo := ghclient.RepoRef{ + Platform: platform.KindGitLab, + Owner: "acme", + Name: "widget", + PlatformHost: "gitlab.com", + RepoPath: "acme/widget", + PlatformRepoID: 4242, + PlatformExternalID: "4242", + CloneURL: "https://gitlab.com/acme/widget.git", + } + syncer := ghclient.NewSyncerWithRegistry( + registry, database, gitclone.New(t.TempDir(), nil), []ghclient.RepoRef{repo}, time.Minute, nil, nil, + ) + t.Cleanup(syncer.Stop) + srv := server.New(database, syncer, nil, "/", nil, server.ServerOptions{}) + + rr := doGitLabJSON(t, srv, http.MethodPost, "/api/v1/pulls/gitlab/acme/widget/7/sync", `{}`) + require.Equal(http.StatusBadGateway, rr.Code, rr.Body.String()) + + var problem struct { + Code string `json:"code"` + Detail string `json:"detail"` + Details map[string]any `json:"details"` + } + require.NoError(json.NewDecoder(rr.Body).Decode(&problem)) + assert.Equal("upstreamError", problem.Code) + assert.Contains(problem.Detail, "temporary failure") + assert.Equal("gitlab", problem.Details["provider"]) + assert.Equal("gitlab.com", problem.Details["platformHost"]) + + _, lookedUp := recorder.find(http.MethodGet, "/api/v4/projects/404") + assert.True(lookedUp, "sync should attempt the fork source project lookup") + + repoRow, err := database.GetRepoByIdentity(ctx, db.RepoIdentity{ + Platform: "gitlab", + PlatformHost: "gitlab.com", + RepoPath: "acme/widget", + }) + require.NoError(err) + require.NotNil(repoRow) + mr, err := database.GetMergeRequestByRepoIDAndNumber(ctx, repoRow.ID, 7) + require.NoError(err) + assert.Nil(mr, "failed detail sync must not persist a partial merge request row") +} + func TestGitLabNormalSyncEnablesHeadBoundMutations(t *testing.T) { require := require.New(t) assert := assert.New(t) diff --git a/internal/server/fleet_ssh_test.go b/internal/server/fleet_ssh_test.go index 409087ab5..55229225c 100644 --- a/internal/server/fleet_ssh_test.go +++ b/internal/server/fleet_ssh_test.go @@ -533,7 +533,7 @@ func TestSSHFleetWebSocketTerminalUsesAttachSpecCommand(t *testing.T) { ts := httptest.NewServer(fixture.server) t.Cleanup(ts.Close) - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() wsURL := "ws" + strings.TrimPrefix(ts.URL, "http") + "/ws/v1/fleet/hosts/epyc/workspaces/ws_1/runtime/sessions/sess-1/terminal?cols=80&rows=24" @@ -542,7 +542,7 @@ func TestSSHFleetWebSocketTerminalUsesAttachSpecCommand(t *testing.T) { defer conn.Close(websocket.StatusNormalClosure, "test done") require.NoError(conn.Write(ctx, websocket.MessageBinary, []byte("ping\n"))) - readWebSocketBinaryUntil(t, ctx, conn, 2*time.Second, "echo:ping") + readWebSocketBinaryUntil(t, ctx, conn, 5*time.Second, "echo:ping") require.Contains(fake.calls, "GET /api/v1/workspaces/ws_1/runtime/sessions/sess-1/attach-spec") } diff --git a/internal/server/huma_routes.go b/internal/server/huma_routes.go index c493e155a..e7966aab6 100644 --- a/internal/server/huma_routes.go +++ b/internal/server/huma_routes.go @@ -1404,6 +1404,7 @@ func (s *Server) listPulls(ctx context.Context, input *listPullsInput) (*listPul if stackConflictBlocked[mr.ID] { responseMR.MergeableState = "dirty" } + responseMR = mergeRequestResponseModel(responseMR) resp := mergeRequestResponse{ MergeRequest: responseMR, Repo: s.repoRefFromRepo(rp), @@ -1600,6 +1601,7 @@ func (s *Server) buildPullDetailResponse( responseMR.MergeableState = "dirty" } } + responseMR = mergeRequestResponseModel(responseMR) resp.MergeRequest = &responseMR if s.workspaces != nil { @@ -1618,6 +1620,23 @@ func (s *Server) buildPullDetailResponse( return resp, nil } +func mergeRequestResponseModel(mr db.MergeRequest) db.MergeRequest { + mr.KanbanStatus = mergeRequestResponseKanbanStatus(mr) + return mr +} + +func mergeRequestResponseKanbanStatus(mr db.MergeRequest) db.KanbanStatus { + switch mr.KanbanStatus { + case db.KanbanStatusNew, db.KanbanStatusReviewing, db.KanbanStatusWaiting, db.KanbanStatusAwaitingMerge: + return mr.KanbanStatus + case "": + return db.KanbanStatusNew + default: + slog.Warn("normalizing unexpected kanban status in merge request response", "merge_request_id", mr.ID, "status", mr.KanbanStatus) + return db.KanbanStatusNew + } +} + func verifiedReviewedHeadSHA(mr *db.MergeRequest) string { if mr == nil || mr.DiffHeadSHA == "" { return "" diff --git a/packages/ui/src/components/shared/FilterDropdown.svelte b/packages/ui/src/components/shared/FilterDropdown.svelte index 990ef7822..91fdbbb14 100644 --- a/packages/ui/src/components/shared/FilterDropdown.svelte +++ b/packages/ui/src/components/shared/FilterDropdown.svelte @@ -133,17 +133,22 @@ await openDropdown(); } - function handleSelect(item: FilterDropdownItem): void { + async function handleSelect(item: FilterDropdownItem): Promise { if (disabled || item.disabled) return; item.onSelect(); if (item.closeOnSelect) { isOpen = false; + return; } + await tick(); + positionDropdown(); } - function handleReset(): void { + async function handleReset(): Promise { if (disabled) return; onReset?.(); + await tick(); + positionDropdown(); } function itemDescriptionId(item: FilterDropdownItem): string { diff --git a/packages/ui/src/components/shared/floatingPosition.test.ts b/packages/ui/src/components/shared/floatingPosition.test.ts index 73ca1bca3..5dddfd24b 100644 --- a/packages/ui/src/components/shared/floatingPosition.test.ts +++ b/packages/ui/src/components/shared/floatingPosition.test.ts @@ -70,6 +70,17 @@ describe("floatingPopoverStyle", () => { expect(style).not.toContain("width:"); }); + it("preserves fractional trigger edges for start-aligned dropdowns", () => { + const style = floatingPopoverStyle({ + trigger: rect({ left: 58.5, bottom: 100 }), + viewportWidth: 1200, + popoverWidth: 200, + align: "start", + }); + + expect(style).toContain("left: 58.5px"); + }); + it("places measured dropdowns above the trigger when they would overflow below", () => { const style = floatingPopoverStyle({ trigger: rect({ diff --git a/packages/ui/src/components/shared/floatingPosition.ts b/packages/ui/src/components/shared/floatingPosition.ts index 11b76e990..5ec49d09e 100644 --- a/packages/ui/src/components/shared/floatingPosition.ts +++ b/packages/ui/src/components/shared/floatingPosition.ts @@ -40,7 +40,7 @@ export function floatingPopoverStyle({ triggerGap, }); - const style = [`left: ${Math.round(left)}px`, `top: ${Math.round(top)}px`]; + const style = [`left: ${formatPx(left)}px`, `top: ${Math.round(top)}px`]; if (constrainWidth) { style.push(`width: ${Math.round(width)}px`); } @@ -68,3 +68,7 @@ function floatingTop({ trigger, popoverHeight, viewportHeight, edgeGap, triggerG function clamp(value: number, min: number, max: number): number { return Math.min(Math.max(min, value), max); } + +function formatPx(value: number): string { + return Number.isInteger(value) ? String(value) : String(Number(value.toFixed(3))); +} diff --git a/packages/ui/src/components/sidebar/IssueList.svelte b/packages/ui/src/components/sidebar/IssueList.svelte index 65d40ea6c..724f956b6 100644 --- a/packages/ui/src/components/sidebar/IssueList.svelte +++ b/packages/ui/src/components/sidebar/IssueList.svelte @@ -78,7 +78,6 @@ id: `state-${state}`, label: issueStateLabel(state), active: issues.getIssueFilterState() === state, - closeOnSelect: true, onSelect: () => setIssueState(state), })), }, @@ -88,7 +87,6 @@ id: `group-${option.byRepo ? "byRepo" : "all"}`, label: option.label, active: grouping.getGroupByRepo() === option.byRepo, - closeOnSelect: true, onSelect: () => grouping.setGroupByRepo(option.byRepo), })), }, diff --git a/packages/ui/src/components/sidebar/PullList.svelte b/packages/ui/src/components/sidebar/PullList.svelte index 2649aebd5..81f9a8cb1 100644 --- a/packages/ui/src/components/sidebar/PullList.svelte +++ b/packages/ui/src/components/sidebar/PullList.svelte @@ -6,8 +6,9 @@ import Chip from "../shared/Chip.svelte"; import FilterDropdown from "../shared/FilterDropdown.svelte"; import LeftSidebarToggle from "../shared/LeftSidebarToggle.svelte"; - import type { PullRequest } from "../../api/types.js"; + import type { KanbanStatus, PullRequest } from "../../api/types.js"; import type { GroupingMode } from "../../stores/grouping.svelte.js"; + import type { PullAttributeFilter } from "../../stores/pulls.svelte.js"; import { buildPullRequestFilesRoute, buildPullRequestRoute, @@ -32,9 +33,28 @@ grouping.getGroupingMode(), ); const workflowGroups = $derived( - groupByWorkflow(pulls.getPulls()), + groupByWorkflow(pulls.getFilteredPulls()), ); const pullStateOptions = ["open", "closed", "all"] as const; + const attributeFilterOptions: { + value: PullAttributeFilter; + label: string; + }[] = [ + { value: "approved", label: "Approved" }, + { value: "draft", label: "Draft" }, + { value: "ready", label: "Ready for review" }, + { value: "merge_conflicts", label: "Merge conflicts" }, + { value: "failed_ci", label: "Failed CI" }, + ]; + const kanbanFilterOptions: { + value: KanbanStatus; + label: string; + }[] = [ + { value: "new", label: "New" }, + { value: "reviewing", label: "Reviewing" }, + { value: "waiting", label: "Waiting" }, + { value: "awaiting_merge", label: "Awaiting merge" }, + ]; const groupingOptions: { value: GroupingMode; label: string; @@ -43,9 +63,12 @@ { value: "byWorkflow", label: "Status" }, { value: "flat", label: "All" }, ]; - // Playwright-measured with a buffered "9999 PRs" count label: - // the full PR filter row first fits at 396px. - const COMPACT_FILTER_MAX_WIDTH = 395; + // Playwright-measured with the local PR filter icon and sidebar toggle: + // the full PR filter row needs 423px, so collapse with a small buffer. + const COMPACT_FILTER_MAX_WIDTH = 430; + // Keep the full filter groups visible at medium widths, but collapse the + // local PR filter trigger label before it crowds the sidebar toggle. + const LOCAL_FILTER_ICON_ONLY_MAX_WIDTH = 520; interface Props { getDetailTab?: () => string; @@ -61,6 +84,7 @@ let searchInput = $state(pulls.getSearchQuery() ?? ""); let debounceHandle: ReturnType | null = null; let refreshHandle: ReturnType | null = null; + const visiblePulls = $derived(pulls.getDisplayOrderPRs()); $effect(() => { void pulls.loadPulls(); @@ -96,19 +120,37 @@ return "All"; } + function pullStateDropdownLabel(state: string): string { + if (state === "all") return "All states"; + return pullStateLabel(state); + } + + function groupingDropdownLabel(mode: GroupingMode): string { + if (mode === "byRepo") return "By repo"; + if (mode === "byWorkflow") return "By status"; + return "Flat list"; + } + function setPullState(state: string): void { pulls.setFilterState(state); void pulls.loadPulls(); } - const compactFilterSections = $derived.by(() => [ + function resetCompactView(): void { + pulls.clearLocalFilters(); + grouping.setGroupingMode("byRepo"); + if (pulls.getFilterState() !== "open") { + setPullState("open"); + } + } + + const toolbarFilterSections = $derived.by(() => [ { title: "State", items: pullStateOptions.map((state) => ({ id: `state-${state}`, - label: pullStateLabel(state), + label: pullStateDropdownLabel(state), active: pulls.getFilterState() === state, - closeOnSelect: true, onSelect: () => setPullState(state), })), }, @@ -116,20 +158,48 @@ title: "Group", items: groupingOptions.map((option) => ({ id: `group-${option.value}`, - label: option.label, + label: groupingDropdownLabel(option.value), active: groupingMode === option.value, - closeOnSelect: true, onSelect: () => grouping.setGroupingMode(option.value), })), }, ]); + const localFilterSections = $derived.by(() => [ + { + title: "PR", + items: attributeFilterOptions.map((option) => ({ + id: `pr-${option.value}`, + label: option.label, + active: pulls.getAttributeFilters().includes(option.value), + onSelect: () => pulls.toggleAttributeFilter(option.value), + })), + }, + { + title: "Kanban", + items: kanbanFilterOptions.map((option) => ({ + id: `kanban-${option.value}`, + label: option.label, + active: pulls.getKanbanStatusFilters().includes(option.value), + onSelect: () => pulls.toggleKanbanStatusFilter(option.value), + })), + }, + ]); + const compactFilterSections = $derived.by(() => [ + ...toolbarFilterSections, + ...localFilterSections, + ]); const hasCompactFilterChanges = $derived( - pulls.getFilterState() !== "open" || groupingMode !== "byRepo", + pulls.getFilterState() !== "open" + || groupingMode !== "byRepo" + || pulls.getLocalFilterCount() > 0, ); const useCompactFilters = $derived( sidebarWidth <= COMPACT_FILTER_MAX_WIDTH, ); + const useIconOnlyLocalFilters = $derived( + sidebarWidth <= LOCAL_FILTER_ICON_ONLY_MAX_WIDTH, + ); interface PullGroup { key: string; @@ -220,7 +290,7 @@ const selectedVisiblePR = $derived.by(() => { const sel = pulls.getSelectedPR(); if (sel === null) return null; - const pr = pulls.getPulls().find( + const pr = visiblePulls.find( (p) => (p.repo_owner ?? "") === sel.owner && (p.repo_name ?? "") === sel.name && p.Number === sel.number @@ -265,7 +335,7 @@
- {pulls.getPulls().length} PRs + {visiblePulls.length} PRs
{#each pullStateOptions as s (s)} @@ -289,11 +359,27 @@ +
+
+ 0} + badgeCount={pulls.getLocalFilterCount()} + sections={localFilterSections} + resetLabel="Clear filters" + onReset={pulls.clearLocalFilters} + minWidth="190px" />
{#if isSidebarToggleEnabled()} @@ -353,14 +439,16 @@

Loading…

{:else if pulls.getError() !== null && pulls.getPulls().length === 0}

Error: {pulls.getError()}

- {:else if pulls.getPulls().length === 0 && sync.getSyncState()?.running} + {:else if visiblePulls.length === 0 && sync.getSyncState()?.running && pulls.getPulls().length === 0}
Syncing from GitHub…
- {:else if pulls.getPulls().length === 0 && !sync.getSyncState()?.last_run_at} + {:else if visiblePulls.length === 0 && !sync.getSyncState()?.last_run_at && pulls.getPulls().length === 0}

Waiting for first sync…

- {:else if pulls.getPulls().length === 0} + {:else if visiblePulls.length === 0 && pulls.getLocalFilterCount() > 0} +

No pull requests match these filters.

+ {:else if visiblePulls.length === 0}

No pull requests found.

{:else} {#if groupedPulls !== null} @@ -407,7 +495,7 @@
{/each} {:else} - {#each pulls.getPulls() as pr (pr.ID)} + {#each visiblePulls as pr (pr.ID)} {@const prRef = routeRefForPull(pr)} {@const prSelected = isSelected(prRef)} = {}): PullRequest { return { ID: id, Number: id, @@ -18,6 +18,14 @@ function pull(id: number, repoName: string, lastActivityAt: string): PullRequest name: repoName, repo_path: `acme/${repoName}`, }, + State: "open", + IsDraft: false, + ReviewDecision: "", + CIStatus: "success", + CIChecksJSON: "[]", + MergeableState: "clean", + KanbanStatus: "new", + ...overrides, } as PullRequest; } @@ -57,4 +65,65 @@ describe("pulls store display order", () => { expect(store.getDisplayOrderPRs().map((pr) => pr.ID)).toEqual([1, 3, 2]); }); + + it("filters pull requests by review state, readiness, CI, merge conflicts, and multiple kanban statuses", async () => { + const store = createPullsStore({ + client: clientWithPulls([ + pull(1, "api", "2026-05-20T15:00:00Z", { + ReviewDecision: "APPROVED", + KanbanStatus: "reviewing", + }), + pull(2, "web", "2026-05-20T14:00:00Z", { + IsDraft: true, + KanbanStatus: "waiting", + }), + pull(3, "worker", "2026-05-20T13:00:00Z", { + CIStatus: "failure", + KanbanStatus: "awaiting_merge", + }), + pull(4, "api", "2026-05-20T12:00:00Z", { + MergeableState: "dirty", + KanbanStatus: "new", + }), + ]), + }); + + await store.loadPulls(); + + store.toggleAttributeFilter("ready"); + store.toggleKanbanStatusFilter("reviewing"); + store.toggleKanbanStatusFilter("awaiting_merge"); + + expect(store.getDisplayOrderPRs().map((pr) => pr.ID)).toEqual([1, 3]); + + store.toggleAttributeFilter("failed_ci"); + + expect(store.getDisplayOrderPRs().map((pr) => pr.ID)).toEqual([3]); + + store.toggleAttributeFilter("failed_ci"); + store.toggleAttributeFilter("merge_conflicts"); + store.toggleKanbanStatusFilter("reviewing"); + store.toggleKanbanStatusFilter("awaiting_merge"); + + expect(store.getDisplayOrderPRs().map((pr) => pr.ID)).toEqual([4]); + }); + + it("matches empty, missing, and unknown kanban statuses as New", async () => { + const store = createPullsStore({ + client: clientWithPulls([ + pull(1, "api", "2026-05-20T15:00:00Z", { KanbanStatus: "" as PullRequest["KanbanStatus"] }), + pull(2, "web", "2026-05-20T14:00:00Z", { + KanbanStatus: undefined as unknown as PullRequest["KanbanStatus"], + }), + pull(3, "worker", "2026-05-20T13:00:00Z", { KanbanStatus: "later" as PullRequest["KanbanStatus"] }), + pull(4, "api", "2026-05-20T12:00:00Z", { KanbanStatus: "reviewing" }), + ]), + }); + + await store.loadPulls(); + + store.toggleKanbanStatusFilter("new"); + + expect(store.getDisplayOrderPRs().map((pr) => pr.ID)).toEqual([1, 2, 3]); + }); }); diff --git a/packages/ui/src/stores/pulls.svelte.ts b/packages/ui/src/stores/pulls.svelte.ts index db4328928..cc32b6351 100644 --- a/packages/ui/src/stores/pulls.svelte.ts +++ b/packages/ui/src/stores/pulls.svelte.ts @@ -1,6 +1,8 @@ import type { KanbanStatus, PullRequest } from "../api/types.js"; import { providerItemPath, providerRouteParams, type ProviderRouteRef } from "../api/provider-routes.js"; import type { MiddlemanClient } from "../types.js"; +import { bucketCIChecks, parseCIChecks } from "../utils/ci-buckets.js"; +import { normalizeKanbanStatus } from "./workflow.svelte.js"; export type FetchPullResult = | { status: "found"; pull: PullRequest } @@ -26,6 +28,8 @@ type PullsParams = { offset?: number; }; +export type PullAttributeFilter = "approved" | "draft" | "ready" | "merge_conflicts" | "failed_ci"; + export interface PullsStoreOptions { client: MiddlemanClient; getGlobalRepo?: () => string | undefined; @@ -49,6 +53,8 @@ export function createPullsStore(opts: PullsStoreOptions) { let loading = $state(false); let storeError = $state(null); let filterKanban = $state(undefined); + let attributeFilters = $state([]); + let kanbanStatusFilters = $state([]); let filterStarred = $state(false); let filterState = $state("open"); let searchQuery = $state(undefined); @@ -60,6 +66,10 @@ export function createPullsStore(opts: PullsStoreOptions) { return pulls; } + function getFilteredPulls(): PullRequest[] { + return pulls.filter((pr) => matchesAttributeFilters(pr) && matchesKanbanStatusFilters(pr)); + } + function isLoading(): boolean { return loading; } @@ -75,7 +85,7 @@ export function createPullsStore(opts: PullsStoreOptions) { /** Groups pulls by "owner/name" into a Map. */ function pullsByRepo(): Map { const map = new Map(); - for (const pr of pulls) { + for (const pr of getFilteredPulls()) { const key = `${pr.repo_owner ?? ""}/${pr.repo_name ?? ""}`; const existing = map.get(key); if (existing !== undefined) { @@ -91,6 +101,18 @@ export function createPullsStore(opts: PullsStoreOptions) { return filterKanban; } + function getAttributeFilters(): PullAttributeFilter[] { + return attributeFilters; + } + + function getKanbanStatusFilters(): KanbanStatus[] { + return kanbanStatusFilters; + } + + function getLocalFilterCount(): number { + return attributeFilters.length + kanbanStatusFilters.length; + } + function getFilterStarred(): boolean { return filterStarred; } @@ -121,7 +143,7 @@ export function createPullsStore(opts: PullsStoreOptions) { } return ordered; } - return pulls; + return getFilteredPulls(); } function selectNextPR(): void { @@ -172,6 +194,19 @@ export function createPullsStore(opts: PullsStoreOptions) { filterKanban = kanban; } + function toggleAttributeFilter(filter: PullAttributeFilter): void { + attributeFilters = toggleFilterValue(attributeFilters, filter); + } + + function toggleKanbanStatusFilter(status: KanbanStatus): void { + kanbanStatusFilters = toggleFilterValue(kanbanStatusFilters, status); + } + + function clearLocalFilters(): void { + attributeFilters = []; + kanbanStatusFilters = []; + } + function getSearchQuery(): string | undefined { return searchQuery; } @@ -326,13 +361,59 @@ export function createPullsStore(opts: PullsStoreOptions) { } } + function toggleFilterValue(values: T[], value: T): T[] { + if (values.includes(value)) { + return values.filter((item) => item !== value); + } + return [...values, value]; + } + + function matchesAttributeFilters(pr: PullRequest): boolean { + if (attributeFilters.length === 0) return true; + return attributeFilters.every((filter) => matchesAttributeFilter(pr, filter)); + } + + function matchesAttributeFilter(pr: PullRequest, filter: PullAttributeFilter): boolean { + if (filter === "approved") { + return pr.ReviewDecision.trim().toUpperCase() === "APPROVED"; + } + if (filter === "draft") { + return pr.IsDraft; + } + if (filter === "ready") { + return pr.State === "open" && !pr.IsDraft; + } + if (filter === "merge_conflicts") { + return pr.MergeableState === "dirty"; + } + return hasFailedCI(pr); + } + + function hasFailedCI(pr: PullRequest): boolean { + const status = pr.CIStatus.trim().toLowerCase(); + if (status === "failure" || status === "failed" || status === "error") { + return true; + } + const parsed = parseCIChecks(pr.CIChecksJSON); + if (parsed.error !== null) return false; + return bucketCIChecks(parsed.checks).failed.length > 0; + } + + function matchesKanbanStatusFilters(pr: PullRequest): boolean { + return kanbanStatusFilters.length === 0 || kanbanStatusFilters.includes(normalizeKanbanStatus(pr.KanbanStatus)); + } + return { getPulls, + getFilteredPulls, isLoading, getError, getSelectedPR, pullsByRepo, getFilterKanban, + getAttributeFilters, + getKanbanStatusFilters, + getLocalFilterCount, getFilterStarred, setFilterStarred, getFilterState, @@ -341,6 +422,9 @@ export function createPullsStore(opts: PullsStoreOptions) { selectNextPR, selectPrevPR, setFilterKanban, + toggleAttributeFilter, + toggleKanbanStatusFilter, + clearLocalFilters, getSearchQuery, setSearchQuery, selectPR, diff --git a/packages/ui/src/stores/workflow.svelte.ts b/packages/ui/src/stores/workflow.svelte.ts index aadc4be6d..015e97814 100644 --- a/packages/ui/src/stores/workflow.svelte.ts +++ b/packages/ui/src/stores/workflow.svelte.ts @@ -18,7 +18,7 @@ export interface WorkflowGroupEntry { items: PullRequest[]; } -function normalizeKanbanStatus(status: string | undefined): WorkflowGroup { +export function normalizeKanbanStatus(status: string | undefined): KanbanStatus { if (status === "new" || status === "reviewing" || status === "waiting" || status === "awaiting_merge") { return status; }