diff --git a/e2e/dependencies.spec.ts b/e2e/dependencies.spec.ts index 421407fb..c5467c03 100644 --- a/e2e/dependencies.spec.ts +++ b/e2e/dependencies.spec.ts @@ -26,10 +26,10 @@ function makeDepPR(overrides: Record = {}) { }; } -function graphqlWithDepPRs(prs: Record[]) { +function graphqlWithDepPRs(prs: Record[], issues: Record[] = []) { return { data: { - issues: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + issues: { issueCount: issues.length, pageInfo: { hasNextPage: false, endCursor: null }, nodes: issues }, prInvolves: { issueCount: prs.length, pageInfo: { hasNextPage: false, endCursor: null }, @@ -41,6 +41,69 @@ function graphqlWithDepPRs(prs: Record[]) { }; } +function makeDashboardIssue(overrides: Record = {}) { + return { + id: "issue_dashboard_1", + databaseId: 5001, + number: 1, + title: "Dependency Dashboard", + state: "OPEN", + url: "https://github.com/testorg/testrepo/issues/1", + createdAt: recentDate, + updatedAt: recentDate, + author: { login: "renovate[bot]", avatarUrl: "https://avatars.githubusercontent.com/in/2740" }, + labels: { nodes: [] }, + assignees: { nodes: [] }, + comments: { totalCount: 0 }, + repository: { nameWithOwner: "testorg/testrepo", stargazerCount: 10 }, + ...overrides, + }; +} + +const ABANDONED_LODASH_BODY = `## Abandoned + +| Datasource | Package | Last Updated | +| --- | --- | --- | +| npm | lodash | 2025-01-01 | +`; + +/** + * Registers a single /graphql handler that serves the light combined search response for + * every call EXCEPT the dashboard-issue-body follow-up query, which it detects by inspecting + * the GraphQL query text itself — not `variables.request?.apiSource`. That field is consumed + * internally by @octokit/graphql as reserved request-config (used only for the app's own + * analytics hook, see github.ts) and never reaches the serialized wire body. Both + * DASHBOARD_ISSUE_BODIES_QUERY and DEP_PR_BODIES_QUERY (and the heavy PR backfill/enrichment + * query) share the same `$ids` variable name, so `"ids" in variables` alone can't disambiguate + * them either — the query string's selection set (`... on Issue { id body }`, unique to the + * dashboard-body query) is the only reliable signal, confirmed by inspecting the actual + * serialized request bodies at runtime. + */ +async function mockGraphqlWithDashboardBody( + page: import("@playwright/test").Page, + prs: Record[], + issues: Record[], + dashboardIssueId: string, + dashboardBody: string +) { + await page.route("https://api.github.com/graphql", (route) => { + const parsed = route.request().postDataJSON() as { query?: string } | null; + const query = parsed?.query ?? ""; + if (query.includes("on Issue { id body }")) { + return route.fulfill({ + status: 200, + json: { + data: { + nodes: [{ id: dashboardIssueId, body: dashboardBody }], + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2099-01-01T00:00:00Z" }, + }, + }, + }); + } + return route.fulfill({ status: 200, json: graphqlWithDepPRs(prs, issues) }); + }); +} + // ── Dependencies tab visibility ───────────────────────────────────────────── test("dependencies tab auto-appears when dep bot PRs exist", async ({ page }) => { @@ -71,6 +134,83 @@ test("settings toggle hides the dependencies tab", async ({ page }) => { await expect(page.getByRole("tab", { name: /dependencies/i })).toHaveCount(0); }); +// ── Bot-detection signal isolation ────────────────────────────────────────── + +test("author-login-only detection: dependabot[bot] PR appears in Dependencies, not Pull Requests", async ({ page }) => { + const pr = makeDepPR({ + author: { login: "dependabot[bot]", avatarUrl: "https://avatars.githubusercontent.com/in/2740" }, + headRefName: "some-unrelated-branch", + title: "Fix something unrelated", + labels: { nodes: [] }, + }); + + await setupAuth(page); + await page.route("https://api.github.com/graphql", (route) => + route.fulfill({ status: 200, json: graphqlWithDepPRs([pr]) }) + ); + await page.goto("/dashboard"); + + await expect(page.getByRole("tab", { name: /dependencies/i })).toBeVisible({ timeout: 10_000 }); + await page.getByRole("tab", { name: /dependencies/i }).click(); + // Only the "mergeable" status group is expanded by default; these light, recent + // PRs land in "Needs Action" (collapsed), so expand all groups before asserting. + await page.getByLabel("Expand all repos").click(); + await expect(page.getByText(pr.title)).toBeVisible(); + + await page.getByRole("tab", { name: /pull requests/i }).click(); + await expect(page.getByText(pr.title)).toHaveCount(0); +}); + +test("branch-prefix-only detection: renovate/ branch PR appears in Dependencies, not Pull Requests", async ({ page }) => { + const pr = makeDepPR({ + author: { login: "some-human" }, + headRefName: "renovate/foo-1.x", + title: "Fix something unrelated", + labels: { nodes: [] }, + }); + + await setupAuth(page); + await page.route("https://api.github.com/graphql", (route) => + route.fulfill({ status: 200, json: graphqlWithDepPRs([pr]) }) + ); + await page.goto("/dashboard"); + + await expect(page.getByRole("tab", { name: /dependencies/i })).toBeVisible({ timeout: 10_000 }); + await page.getByRole("tab", { name: /dependencies/i }).click(); + // Only the "mergeable" status group is expanded by default; these light, recent + // PRs land in "Needs Action" (collapsed), so expand all groups before asserting. + await page.getByLabel("Expand all repos").click(); + await expect(page.getByText(pr.title)).toBeVisible(); + + await page.getByRole("tab", { name: /pull requests/i }).click(); + await expect(page.getByText(pr.title)).toHaveCount(0); +}); + +test("label-only detection: dependencies-labeled PR appears in Dependencies, not Pull Requests", async ({ page }) => { + const pr = makeDepPR({ + author: { login: "some-human" }, + headRefName: "some-feature", + title: "Fix something unrelated", + labels: { nodes: [{ name: "dependencies" }] }, + }); + + await setupAuth(page); + await page.route("https://api.github.com/graphql", (route) => + route.fulfill({ status: 200, json: graphqlWithDepPRs([pr]) }) + ); + await page.goto("/dashboard"); + + await expect(page.getByRole("tab", { name: /dependencies/i })).toBeVisible({ timeout: 10_000 }); + await page.getByRole("tab", { name: /dependencies/i }).click(); + // Only the "mergeable" status group is expanded by default; these light, recent + // PRs land in "Needs Action" (collapsed), so expand all groups before asserting. + await page.getByLabel("Expand all repos").click(); + await expect(page.getByText(pr.title)).toBeVisible(); + + await page.getByRole("tab", { name: /pull requests/i }).click(); + await expect(page.getByText(pr.title)).toHaveCount(0); +}); + // ── Status groups ─────────────────────────────────────────────────────────── test("status groups render correctly", async ({ page }) => { @@ -103,3 +243,45 @@ test("status groups render correctly", async ({ page }) => { await expect(page.getByText("Needs Action")).toBeVisible(); await expect(page.getByText("Stale")).toBeVisible(); }); + +// ── Abandoned-dependency pill ──────────────────────────────────────────────── + +test("abandoned dep badge links to the Dependency Dashboard issue when matched", async ({ page }) => { + const dashboardIssue = makeDashboardIssue(); + // makeDepPR()'s default title is "Bump lodash from 4.17.20 to 4.17.21" — matches + // the "lodash" row in ABANDONED_LODASH_BODY via matchAbandonedToPr()'s substring match. + const pr = makeDepPR(); + + await setupAuth(page); + await mockGraphqlWithDashboardBody(page, [pr], [dashboardIssue], dashboardIssue.id, ABANDONED_LODASH_BODY); + await page.goto("/dashboard"); + + await expect(page.getByRole("tab", { name: /dependencies/i })).toBeVisible({ timeout: 10_000 }); + await page.getByRole("tab", { name: /dependencies/i }).click(); + await page.getByLabel("Expand all repos").click(); + + // The dashboard-issue-body follow-up query only fires after the first full poll + // completes, so the badge starts as absent/plain-text and becomes a link asynchronously. + const abandonedLink = page.getByRole("link", { name: /abandoned/i }); + await expect(abandonedLink).toBeVisible({ timeout: 10_000 }); + await expect(abandonedLink).toHaveAttribute("href", dashboardIssue.url); +}); + +test("abandoned dep badge renders as plain text when title-suffix heuristic matches with no dashboard issue", async ({ page }) => { + const pr = makeDepPR({ + title: "Bump left-pad from 1.0.0 to 1.0.1 - abandoned", + }); + + await setupAuth(page); + await page.route("https://api.github.com/graphql", (route) => + route.fulfill({ status: 200, json: graphqlWithDepPRs([pr]) }) + ); + await page.goto("/dashboard"); + + await expect(page.getByRole("tab", { name: /dependencies/i })).toBeVisible({ timeout: 10_000 }); + await page.getByRole("tab", { name: /dependencies/i }).click(); + await page.getByLabel("Expand all repos").click(); + + await expect(page.getByText("Abandoned")).toBeVisible(); + await expect(page.getByRole("link", { name: /abandoned/i })).toHaveCount(0); +}); diff --git a/e2e/notifications.spec.ts b/e2e/notifications.spec.ts new file mode 100644 index 00000000..eb5caf84 --- /dev/null +++ b/e2e/notifications.spec.ts @@ -0,0 +1,100 @@ +import { test, expect, type Page } from "@playwright/test"; +import { setupAuth } from "./helpers"; + +const emptyGraphqlFixture = { + data: { + issues: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prInvolves: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + prReviewReq: { issueCount: 0, pageInfo: { hasNextPage: false, endCursor: null }, nodes: [] }, + rateLimit: { limit: 5000, remaining: 4999, resetAt: "2099-01-01T00:00:00Z" }, + }, +}; + +/** + * Fakes a real GitHub primary-rate-limit response on the FIRST /graphql POST so + * @octokit/plugin-throttling's onRateLimit handler fires (github.ts:86-98), which calls + * pushNotification("rate-limit", ..., "warning", true). The plugin then auto-retries + * once internally (retryCount < 1 => true); the SECOND POST returns normal fixture data + * so the dashboard loads successfully afterward. + */ +async function triggerRateLimitOnFirstGraphqlCall(page: Page) { + let callCount = 0; + await page.route("https://api.github.com/graphql", (route) => { + callCount += 1; + if (callCount === 1) { + const resetEpochSeconds = Math.floor(Date.now() / 1000) + 1; + return route.fulfill({ + status: 403, + headers: { + "x-ratelimit-limit": "5000", + "x-ratelimit-remaining": "0", + "x-ratelimit-reset": String(resetEpochSeconds), + // Cross-origin fetch() only exposes safelisted response headers to JS unless the + // server opts in via Access-Control-Expose-Headers (the real GitHub API does this) — + // without it, @octokit/plugin-throttling can't see x-ratelimit-remaining and never + // fires onRateLimit; the request just fails and surfaces as a generic api error instead. + "access-control-expose-headers": "x-ratelimit-limit, x-ratelimit-remaining, x-ratelimit-reset", + }, + json: { message: "API rate limit exceeded for token." }, + }); + } + return route.fulfill({ status: 200, json: emptyGraphqlFixture }); + }); +} + +async function openDrawerFromBell(page: Page) { + await page.goto("/dashboard"); + + const bell = page.getByRole("button", { name: /Notifications, \d+ unread/i }); + await expect(bell).toBeVisible({ timeout: 15_000 }); + await bell.click(); + + const overlay = page.getByTestId("notification-overlay"); + await expect(overlay).toBeVisible(); + const dialog = page.getByRole("dialog"); + await expect(dialog).toBeVisible(); + // ToastContainer renders the same notification as a toast concurrently (expected, + // per Task 7 plan) — scope to the dialog to avoid a strict-mode double match. + await expect(dialog.getByText(/Rate limit hit/i)).toBeVisible(); + return overlay; +} + +// ── Open/close animation via real rate-limit-triggered notification ───────── + +test("rate-limit notification opens the drawer and Escape closes it", async ({ page }) => { + await setupAuth(page); + await triggerRateLimitOnFirstGraphqlCall(page); + + const overlay = await openDrawerFromBell(page); + + await page.keyboard.press("Escape"); + await expect(overlay).toBeHidden(); +}); + +test("rate-limit notification opens the drawer and the close button closes it", async ({ page }) => { + await setupAuth(page); + await triggerRateLimitOnFirstGraphqlCall(page); + + const overlay = await openDrawerFromBell(page); + + await page.getByLabel("Close notifications").click(); + await expect(overlay).toBeHidden(); +}); + +// ── prefers-reduced-motion ─────────────────────────────────────────────────── + +test("drawer still opens and closes correctly under prefers-reduced-motion", async ({ page }) => { + await page.emulateMedia({ reducedMotion: "reduce" }); + await setupAuth(page); + await triggerRateLimitOnFirstGraphqlCall(page); + + const overlay = await openDrawerFromBell(page); + + // Under prefers-reduced-motion, index.css disables the drawer keyframe animation + // entirely (`.drawer-overlay[data-expanded] { animation: none }`), unlike the + // normal `overlay-fade-in` animation applied without the media query match. + await expect(overlay).toHaveCSS("animation-name", "none"); + + await page.getByLabel("Close notifications").click(); + await expect(overlay).toBeHidden(); +}); diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 9b36f203..83120415 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -1,4 +1,9 @@ packages: - "mcp" +allowBuilds: + esbuild: true + sharp: true + workerd: true + strictDepBuilds: false diff --git a/src/app/components/dashboard/IgnoreBadge.tsx b/src/app/components/dashboard/IgnoreBadge.tsx index 48b16d06..ecf3acbd 100644 --- a/src/app/components/dashboard/IgnoreBadge.tsx +++ b/src/app/components/dashboard/IgnoreBadge.tsx @@ -1,4 +1,5 @@ import { createSignal, For, Show } from "solid-js"; +import { Popover } from "@kobalte/core/popover"; import type { IgnoredItem } from "../../stores/view"; import { Tooltip } from "../shared/Tooltip"; @@ -28,12 +29,6 @@ function formatDate(ts: number): string { export default function IgnoreBadge(props: IgnoreBadgeProps) { const [open, setOpen] = createSignal(false); - function handleBackdropClick(e: MouseEvent) { - if (e.target === e.currentTarget) { - setOpen(false); - } - } - function handleUnignoreAll() { for (const item of props.items) { props.onUnignore(item.id); @@ -43,13 +38,11 @@ export default function IgnoreBadge(props: IgnoreBadgeProps) { return ( 0}> -
+ - + - - {/* Backdrop */} - + + + ); } diff --git a/src/app/components/settings/SettingsPage.tsx b/src/app/components/settings/SettingsPage.tsx index e7d3cd8c..0cad0b09 100644 --- a/src/app/components/settings/SettingsPage.tsx +++ b/src/app/components/settings/SettingsPage.tsx @@ -1,10 +1,11 @@ import { createSignal, createMemo, Show, For, onCleanup, onMount } from "solid-js"; +import { Select } from "@kobalte/core/select"; import * as Sentry from "@sentry/solid"; import { getRelayStatus } from "../../lib/mcp-relay"; import { useNavigate } from "@solidjs/router"; import { config, updateConfig, updateJiraConfig, updateJiraCustomFields, updateJiraCustomScopes, setMonitoredRepo, isActionsBasedTab } from "../../stores/config"; -import type { Config } from "../../stores/config"; -import { viewState, updateViewState } from "../../stores/view"; +import type { JiraCustomField } from "../../../shared/schemas"; +import { viewState, updateViewState, setTabFilter } from "../../stores/view"; import { clearAuth, jiraAuth, setJiraAuth, clearJiraConfigFull, isJiraAuthenticated, token, setAuthFromPat } from "../../stores/auth"; import type { GitHubUser } from "../../stores/auth"; import { isValidPatFormat } from "../../lib/pat"; @@ -34,6 +35,16 @@ import type { RepoRef } from "../../services/api"; const VALID_JIRA_CLIENT_ID_RE = /^[A-Za-z0-9_-]+$/; +interface NumberSelectOption { + value: number; + label: string; +} + +interface StringSelectOption { + value: string; + label: string; +} + export const SETTINGS_PAGE_SECTION_IDS = [ "orgs-repos", "tracked-users", @@ -50,6 +61,21 @@ export const SETTINGS_PAGE_SECTION_IDS = [ "data", ] as const; +// Built-in Jira scope values, duplicated from JiraAssignedTab.tsx's BUILTIN_SCOPE_OPTIONS +// (not exported there to avoid a cross-component coupling for 3 literal strings). +const BUILTIN_JIRA_SCOPES = ["assigned", "reported", "watching"]; + +/** + * Returns the scope value to fall back to if `activeScope` no longer exists in + * `scopes` (e.g. the custom scope backing it was just deleted, or Jira was + * disconnected and `scopes` is empty) — otherwise null. Guards against firing + * an invalid JQL query with a stale custom scope ID (BUG-008). + */ +export function staleJiraScopeReset(activeScope: string, scopes: JiraCustomField[]): string | null { + const validScopeIds = new Set([...BUILTIN_JIRA_SCOPES, ...scopes.map((s) => s.id)]); + return validScopeIds.has(activeScope) ? null : "assigned"; +} + export default function SettingsPage() { const navigate = useNavigate(); @@ -423,11 +449,16 @@ export default function SettingsPage() { if (viewState.lastActiveTab === "jiraAssigned") { updateViewState({ lastActiveTab: "issues" }); } + // Stale scope guard: clearJiraConfigFull() wipes customScopes, so a tab + // filter pointing at a custom scope ID would otherwise survive reconnect + // and fire an invalid JQL query against the new Jira instance (BUG-008). + const reset = staleJiraScopeReset(viewState.tabFilters.jiraAssigned.scope, []); + if (reset) setTabFilter("jiraAssigned", "scope", reset); } // ── Refresh interval options ────────────────────────────────────────────── - const refreshOptions = [ + const refreshOptions: NumberSelectOption[] = [ { value: 60, label: "1 minute" }, { value: 120, label: "2 minutes" }, { value: 300, label: "5 minutes (default)" }, @@ -437,7 +468,7 @@ export default function SettingsPage() { { value: 0, label: "Off" }, ]; - const tabOptions = createMemo(() => [ + const tabOptions = createMemo(() => [ { value: "issues", label: "Issues" }, { value: "pullRequests", label: "Pull Requests" }, ...(config.enableActions ? [{ value: "actions", label: "GitHub Actions" }] : []), @@ -448,7 +479,7 @@ export default function SettingsPage() { ]); - const itemsPerPageOptions = [ + const itemsPerPageOptions: NumberSelectOption[] = [ { value: 10, label: "10" }, { value: 25, label: "25" }, { value: 50, label: "50" }, @@ -608,17 +639,32 @@ export default function SettingsPage() { label="Refresh interval" description="How often to poll GitHub for new data" > - opt.value === config.refreshInterval) ?? null} + onChange={(opt) => opt && saveWithFeedback({ refreshInterval: opt.value })} + itemComponent={(itemProps) => ( + + {itemProps.item.rawValue.label} + + )} > - {refreshOptions.map((opt) => ( - - ))} - + + > + {(state) => state.selectedOption()?.label ?? ""} + + + + + + + + - opt.value === config.itemsPerPage) ?? null} + onChange={(opt) => opt && saveWithFeedback({ itemsPerPage: opt.value })} + itemComponent={(itemProps) => ( + + {itemProps.item.rawValue.label} + + )} > - {itemsPerPageOptions.map((opt) => ( - - ))} - + + > + {(state) => state.selectedOption()?.label ?? ""} + + + + + + + + @@ -742,17 +803,32 @@ export default function SettingsPage() { label="Default tab" description="Tab shown when opening the dashboard fresh" > - opt.value === config.defaultTab) ?? null} + onChange={(opt) => opt && saveWithFeedback({ defaultTab: opt.value })} + itemComponent={(itemProps) => ( + + {itemProps.item.rawValue.label} + + )} > - {tabOptions().map((opt) => ( - - ))} - + + > + {(state) => state.selectedOption()?.label ?? ""} + + + + + + + + { updateJiraCustomScopes(scopes); setShowScopePicker(false); }} + onSave={(scopes) => { + updateJiraCustomScopes(scopes); + setShowScopePicker(false); + // Stale scope guard: if the active Jira tab filter pointed at a + // custom scope that was just removed, reset it before it can fire + // an invalid JQL query (BUG-008). + const reset = staleJiraScopeReset(viewState.tabFilters.jiraAssigned.scope, scopes); + if (reset) setTabFilter("jiraAssigned", "scope", reset); + }} onCancel={() => setShowScopePicker(false)} />
@@ -1246,7 +1330,16 @@ export default function SettingsPage() { class="toggle toggle-primary" aria-label="Enable dependencies tab" checked={config.dependencies?.enabled ?? true} - onChange={() => saveWithFeedback({ dependencies: { ...config.dependencies, enabled: !(config.dependencies?.enabled ?? true) } })} + onChange={() => { + const val = !(config.dependencies?.enabled ?? true); + saveWithFeedback({ + dependencies: { ...config.dependencies, enabled: val }, + ...(!val && config.defaultTab === "dependencies" ? { defaultTab: "issues" as const } : {}), + }); + if (!val && viewState.lastActiveTab === "dependencies") { + updateViewState({ lastActiveTab: "issues" }); + } + }} /> void; @@ -17,9 +19,11 @@ export default function ScopeToggle(props: ScopeToggleProps) { props.onChange("scope", e.currentTarget.checked ? "involves_me" : "all") } /> - - {checked() ? "Involves me" : "All activity"} - + + + {checked() ? "Involves me" : "All activity"} + + ); } diff --git a/src/app/stores/view.ts b/src/app/stores/view.ts index 0c94a14a..b04cabf1 100644 --- a/src/app/stores/view.ts +++ b/src/app/stores/view.ts @@ -224,12 +224,25 @@ export function resetViewState(): void { ); } +// KNOWN LIMITATION: ViewStateSchema.pick() only prevents inflation of TOP-LEVEL fields +// absent from `partial`. It does NOT protect a nested field's own missing sub-keys from +// being backfilled with schema defaults if a caller ever passes a partially-populated +// nested object (e.g. `{ globalFilter: { org: "x" } }` without `repo`) — Zod's per-field +// `.default()` still fires within a nested object's own schema regardless of `.pick()` at +// the parent level. Not a regression: no current call site passes anything but flat scalar +// fields, and `updateConfig()` in config.ts has this same limitation today. export function updateViewState(partial: Partial): void { - setViewState( - produce((draft) => { - Object.assign(draft, partial); - }) - ); + const keys = Object.keys(partial) as (keyof ViewState)[]; + if (keys.length === 0) return; + const shape = Object.fromEntries(keys.map((k) => [k, true])) as Partial>; + const validated = ViewStateSchema.pick(shape).safeParse(partial); + if (!validated.success) { + pushNotification("view:updateViewState", "Some view settings could not be saved due to invalid data.", "warning"); + return; + } + setViewState(produce((draft) => { + Object.assign(draft, validated.data); + })); } export function ignoreItem(item: IgnoredItem): void { diff --git a/tests/components/IgnoreBadge.test.tsx b/tests/components/IgnoreBadge.test.tsx index ba534eea..1f8ac011 100644 --- a/tests/components/IgnoreBadge.test.tsx +++ b/tests/components/IgnoreBadge.test.tsx @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from "vitest"; -import { render, screen } from "@solidjs/testing-library"; +import { render, screen, fireEvent } from "@solidjs/testing-library"; import userEvent from "@testing-library/user-event"; import IgnoreBadge from "../../src/app/components/dashboard/IgnoreBadge"; import type { IgnoredItem } from "../../src/app/stores/view"; @@ -49,6 +49,11 @@ describe("IgnoreBadge", () => { // Now open expect(button.getAttribute("aria-expanded")).toBe("true"); + + // Regression guard: popover content must retain z-50 so it stacks above + // surrounding dashboard content (fix(ui): adds missing z-50 to IgnoreBadge popover content). + const content = document.querySelector("[aria-label='Ignored items']"); + expect(content?.className).toContain("z-50"); }); it("clicking badge again closes popover", async () => { @@ -84,14 +89,20 @@ describe("IgnoreBadge", () => { const onUnignore = vi.fn(); const items = [ makeIgnoredItem({ id: 123, title: "My Issue" }), + makeIgnoredItem({ id: 456, title: "Another Issue" }), ]; render(() => ); - await user.click(getTrigger(1)); + const button = getTrigger(2); + await user.click(button); - const unignoreBtn = screen.getByText("Unignore"); - await user.click(unignoreBtn); + const unignoreBtns = screen.getAllByText("Unignore"); + await user.click(unignoreBtns[0]); expect(onUnignore).toHaveBeenCalledWith(123); + // Deliberate behavior: individual "Unignore" clicks must NOT close the + // popover — only "Unignore All" does. Verify with 2+ items remaining so + // the click doesn't trivially empty the list. + expect(button.getAttribute("aria-expanded")).toBe("true"); }); it("'Unignore All' calls onUnignore for every item", async () => { @@ -114,7 +125,7 @@ describe("IgnoreBadge", () => { expect(onUnignore).toHaveBeenCalledWith(3); }); - it("clicking backdrop closes popover", async () => { + it("clicking outside closes popover", async () => { const user = userEvent.setup(); const items = [makeIgnoredItem()]; render(() => {}} />); @@ -122,11 +133,13 @@ describe("IgnoreBadge", () => { await user.click(button); expect(button.getAttribute("aria-expanded")).toBe("true"); - // The backdrop is aria-hidden div with fixed positioning - const backdrop = document.querySelector('[aria-hidden="true"]') as HTMLElement; - expect(backdrop).toBeDefined(); - // Simulate clicking the backdrop itself (target === currentTarget) - await user.click(backdrop); + // Kobalte's createInteractOutside registers its document pointerdown listener + // inside a setTimeout(0) (to avoid catching the click that opened the popover). + // This file uses userEvent's real-timer async model throughout, so wait a real + // tick rather than introducing fake timers. + await new Promise((resolve) => setTimeout(resolve, 0)); + fireEvent.pointerDown(document.body); + expect(button.getAttribute("aria-expanded")).toBe("false"); }); }); diff --git a/tests/components/settings/JiraSection.test.tsx b/tests/components/settings/JiraSection.test.tsx index 081bf22e..a014f7b7 100644 --- a/tests/components/settings/JiraSection.test.tsx +++ b/tests/components/settings/JiraSection.test.tsx @@ -103,9 +103,10 @@ vi.mock("../../../src/app/stores/config", () => ({ })); vi.mock("../../../src/app/stores/view", () => ({ - viewState: { lastActiveTab: "issues", tabFilters: {}, expandedRepos: {}, lockedRepos: {}, trackedItems: [], activeScopeTab: "involved" }, + viewState: { lastActiveTab: "issues", tabFilters: { jiraAssigned: { scope: "assigned" } }, expandedRepos: {}, lockedRepos: {}, trackedItems: [], activeScopeTab: "involved" }, updateViewState: vi.fn(), resetViewState: vi.fn(), + setTabFilter: vi.fn(), ViewStateSchema: { parse: vi.fn((x: unknown) => x) }, })); @@ -163,7 +164,8 @@ vi.mock("../../../src/app/services/jira-keys", () => ({ })); // Component imports after all mocks -import SettingsPage from "../../../src/app/components/settings/SettingsPage"; +import SettingsPage, { staleJiraScopeReset } from "../../../src/app/components/settings/SettingsPage"; +import { viewState, setTabFilter } from "../../../src/app/stores/view"; // ── Helpers ─────────────────────────────────────────────────────────────────── @@ -525,6 +527,31 @@ describe("SettingsPage Jira section — connected state", () => { expect(mockClearJiraConfigFull).toHaveBeenCalled(); }); + it("Disconnect resets a stale custom jiraAssigned scope to 'assigned' (BUG-008)", async () => { + viewState.tabFilters.jiraAssigned.scope = "customfield_10001"; + renderSettings(); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Disconnect/i })).toBeTruthy(); + }); + + fireEvent.click(screen.getByRole("button", { name: /Disconnect/i })); + + expect(setTabFilter).toHaveBeenCalledWith("jiraAssigned", "scope", "assigned"); + }); + + it("Disconnect does not touch jiraAssigned scope when it's already 'assigned'", async () => { + viewState.tabFilters.jiraAssigned.scope = "assigned"; + renderSettings(); + await waitFor(() => { + expect(screen.getByRole("button", { name: /Disconnect/i })).toBeTruthy(); + }); + + vi.mocked(setTabFilter).mockClear(); + fireEvent.click(screen.getByRole("button", { name: /Disconnect/i })); + + expect(setTabFilter).not.toHaveBeenCalled(); + }); + it("Disconnect does not show OAuth connect buttons (only when disconnected)", async () => { renderSettings(); await waitFor(() => { @@ -535,3 +562,24 @@ describe("SettingsPage Jira section — connected state", () => { expect(screen.queryByText(/Use API token/i)).toBeNull(); }); }); + +describe("staleJiraScopeReset (BUG-008)", () => { + it("returns null when the active scope is a built-in value", () => { + expect(staleJiraScopeReset("assigned", [])).toBeNull(); + expect(staleJiraScopeReset("reported", [{ id: "customfield_1", name: "Foo" }])).toBeNull(); + expect(staleJiraScopeReset("watching", [])).toBeNull(); + }); + + it("returns null when the active scope matches a surviving custom scope id", () => { + expect( + staleJiraScopeReset("customfield_10001", [{ id: "customfield_10001", name: "Epic Filter" }]) + ).toBeNull(); + }); + + it("returns 'assigned' when the active scope is a custom id no longer present", () => { + expect(staleJiraScopeReset("customfield_10001", [])).toBe("assigned"); + expect( + staleJiraScopeReset("customfield_10001", [{ id: "customfield_9999", name: "Other" }]) + ).toBe("assigned"); + }); +}); diff --git a/tests/components/settings/SettingsPage.test.tsx b/tests/components/settings/SettingsPage.test.tsx index 249db2b7..ac5240da 100644 --- a/tests/components/settings/SettingsPage.test.tsx +++ b/tests/components/settings/SettingsPage.test.tsx @@ -219,14 +219,15 @@ describe("SettingsPage — rendering", () => { describe("SettingsPage — Refresh interval", () => { it("shows current refresh interval value", () => { renderSettings(); - screen.getByDisplayValue("5 minutes (default)"); + screen.getByRole("button", { name: "5 minutes (default)" }); }); it("changing refresh interval calls updateConfig", async () => { const user = userEvent.setup(); renderSettings(); - const select = screen.getByDisplayValue("5 minutes (default)"); - await user.selectOptions(select, "60"); + const trigger = screen.getByRole("button", { name: "5 minutes (default)" }); + await user.click(trigger); + await user.click(screen.getByRole("option", { name: "1 minute" })); expect(config.refreshInterval).toBe(60); }); }); @@ -261,14 +262,15 @@ describe("SettingsPage — Appearance", () => { it("shows current items per page value", () => { renderSettings(); - screen.getByDisplayValue("25"); + screen.getByRole("button", { name: "25" }); }); it("changing items per page updates config", async () => { const user = userEvent.setup(); renderSettings(); - const ippSelect = screen.getByDisplayValue("25"); - await user.selectOptions(ippSelect, "50"); + const trigger = screen.getByRole("button", { name: "25" }); + await user.click(trigger); + await user.click(screen.getByRole("option", { name: "50" })); expect(config.itemsPerPage).toBe(50); }); }); @@ -276,14 +278,15 @@ describe("SettingsPage — Appearance", () => { describe("SettingsPage — Tabs", () => { it("shows current default tab value", () => { renderSettings(); - screen.getByDisplayValue("Issues"); + screen.getByRole("button", { name: "Issues" }); }); it("changing default tab updates config", async () => { const user = userEvent.setup(); renderSettings(); - const tabSelect = screen.getByDisplayValue("Issues"); - await user.selectOptions(tabSelect, "pullRequests"); + const trigger = screen.getByRole("button", { name: "Issues" }); + await user.click(trigger); + await user.click(screen.getByRole("option", { name: "Pull Requests" })); expect(config.defaultTab).toBe("pullRequests"); }); @@ -978,15 +981,21 @@ describe("SettingsPage — enableTracking toggle", () => { expect(config.defaultTab).toBe("pullRequests"); }); - it("shows 'Tracked Items' option in defaultTab select when enableTracking is true", () => { + it("shows 'Tracked Items' option in defaultTab select when enableTracking is true", async () => { + const user = userEvent.setup(); updateConfig({ enableTracking: true }); renderSettings(); + await user.click(screen.getByRole("button", { name: "Issues" })); screen.getByRole("option", { name: "Tracked Items" }); }); - it("hides 'Tracked Items' option in defaultTab select when enableTracking is false", () => { + it("hides 'Tracked Items' option in defaultTab select when enableTracking is false", async () => { + const user = userEvent.setup(); updateConfig({ enableTracking: false }); renderSettings(); + // Open the trigger first — Kobalte doesn't mount role="option" elements until the + // listbox opens, so querying for absence without opening would pass vacuously. + await user.click(screen.getByRole("button", { name: "Issues" })); expect(screen.queryByRole("option", { name: "Tracked Items" })).toBeNull(); }); @@ -1027,7 +1036,8 @@ describe("SettingsPage — custom tabs in default tab dropdown", () => { updateConfig({ customTabs: [] }); }); - it("custom tab name appears as an option in the default tab select", () => { + it("custom tab name appears as an option in the default tab select", async () => { + const user = userEvent.setup(); updateConfig({ customTabs: [{ id: "custom-tab-1", @@ -1040,10 +1050,12 @@ describe("SettingsPage — custom tabs in default tab dropdown", () => { }], }); renderSettings(); + await user.click(screen.getByRole("button", { name: "Issues" })); screen.getByRole("option", { name: "My Custom Tab" }); }); - it("multiple custom tabs all appear in the default tab select", () => { + it("multiple custom tabs all appear in the default tab select", async () => { + const user = userEvent.setup(); updateConfig({ customTabs: [ { @@ -1067,6 +1079,7 @@ describe("SettingsPage — custom tabs in default tab dropdown", () => { ], }); renderSettings(); + await user.click(screen.getByRole("button", { name: "Issues" })); screen.getByRole("option", { name: "Alpha Tab" }); screen.getByRole("option", { name: "Beta Tab" }); }); @@ -1085,8 +1098,9 @@ describe("SettingsPage — custom tabs in default tab dropdown", () => { }], }); renderSettings(); - const tabSelect = screen.getByDisplayValue("Issues"); - await user.selectOptions(tabSelect, "my-tab"); + const trigger = screen.getByRole("button", { name: "Issues" }); + await user.click(trigger); + await user.click(screen.getByRole("option", { name: "My Tab" })); expect(config.defaultTab).toBe("my-tab"); }); }); @@ -1174,6 +1188,34 @@ describe("Dependencies settings section", () => { expect(config.dependencies.enabled).toBe(false); }); + it("disabling dependencies resets defaultTab to 'issues' when it was 'dependencies'", () => { + updateConfig({ dependencies: { enabled: true, rebaseLabel: "rebase" }, defaultTab: "dependencies" }); + renderSettings(); + const toggle = screen.getByRole("checkbox", { name: /Enable dependencies tab/i }); + fireEvent.click(toggle); + expect(config.dependencies.enabled).toBe(false); + expect(config.defaultTab).toBe("issues"); + }); + + it("disabling dependencies preserves defaultTab when it was not 'dependencies'", () => { + updateConfig({ dependencies: { enabled: true, rebaseLabel: "rebase" }, defaultTab: "pullRequests" }); + renderSettings(); + const toggle = screen.getByRole("checkbox", { name: /Enable dependencies tab/i }); + fireEvent.click(toggle); + expect(config.dependencies.enabled).toBe(false); + expect(config.defaultTab).toBe("pullRequests"); + }); + + it("disabling dependencies resets lastActiveTab to 'issues' when it was 'dependencies'", () => { + updateConfig({ dependencies: { enabled: true, rebaseLabel: "rebase" } }); + updateViewState({ lastActiveTab: "dependencies" }); + renderSettings(); + const toggle = screen.getByRole("checkbox", { name: /Enable dependencies tab/i }); + fireEvent.click(toggle); + expect(config.dependencies.enabled).toBe(false); + expect(viewState.lastActiveTab).toBe("issues"); + }); + it("renders rebase label input with current value", () => { updateConfig({ dependencies: { enabled: true, rebaseLabel: "rebase-please" } }); renderSettings(); diff --git a/tests/components/settings/actions-toggle.test.tsx b/tests/components/settings/actions-toggle.test.tsx index cefaae2c..4270bd46 100644 --- a/tests/components/settings/actions-toggle.test.tsx +++ b/tests/components/settings/actions-toggle.test.tsx @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { screen, fireEvent } from "@solidjs/testing-library"; +import userEvent from "@testing-library/user-event"; // ── localStorage mock ──────────────────────────────────────────────────────── @@ -277,38 +278,25 @@ describe("Actions toggle — negative cases", () => { }); describe("Actions toggle — default tab dropdown filtering", () => { - it("excludes GitHub Actions option from default tab select when enableActions is false", () => { + // The default tab select is a Kobalte Select — options only mount in the DOM once + // the trigger is opened, so each test must open it before asserting presence/absence. + it("excludes GitHub Actions option from default tab select when enableActions is false", async () => { + const user = userEvent.setup(); updateConfig({ enableActions: false }); renderSettings(); - const selects = document.querySelectorAll("select"); - let actionsOptionFound = false; - for (const sel of selects) { - for (const opt of sel.options) { - if (opt.value === "actions") { - actionsOptionFound = true; - break; - } - } - } - expect(actionsOptionFound).toBe(false); + await user.click(screen.getByRole("button", { name: "Issues" })); + expect(screen.queryByRole("option", { name: "GitHub Actions" })).toBeNull(); }); - it("includes GitHub Actions option in default tab select when enableActions is true", () => { + it("includes GitHub Actions option in default tab select when enableActions is true", async () => { + const user = userEvent.setup(); renderSettings(); - const selects = document.querySelectorAll("select"); - let actionsOptionFound = false; - for (const sel of selects) { - for (const opt of sel.options) { - if (opt.value === "actions") { - actionsOptionFound = true; - break; - } - } - } - expect(actionsOptionFound).toBe(true); + await user.click(screen.getByRole("button", { name: "Issues" })); + screen.getByRole("option", { name: "GitHub Actions" }); }); - it("excludes actions-based custom tabs from default tab select when enableActions is false", () => { + it("excludes actions-based custom tabs from default tab select when enableActions is false", async () => { + const user = userEvent.setup(); updateConfig({ enableActions: false, customTabs: [ @@ -317,16 +305,8 @@ describe("Actions toggle — default tab dropdown filtering", () => { ], }); renderSettings(); - const selects = document.querySelectorAll("select"); - let actionsCustomFound = false; - let issuesCustomFound = false; - for (const sel of selects) { - for (const opt of sel.options) { - if (opt.value === "my-actions") actionsCustomFound = true; - if (opt.value === "my-issues") issuesCustomFound = true; - } - } - expect(actionsCustomFound).toBe(false); - expect(issuesCustomFound).toBe(true); + await user.click(screen.getByRole("button", { name: "Issues" })); + expect(screen.queryByRole("option", { name: "My Actions" })).toBeNull(); + screen.getByRole("option", { name: "My Issues" }); }); }); diff --git a/tests/components/shared/ScopeToggle.test.tsx b/tests/components/shared/ScopeToggle.test.tsx index ecb48581..3ee5b150 100644 --- a/tests/components/shared/ScopeToggle.test.tsx +++ b/tests/components/shared/ScopeToggle.test.tsx @@ -1,8 +1,16 @@ -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { render, screen, fireEvent } from "@solidjs/testing-library"; import ScopeToggle from "../../../src/app/components/shared/ScopeToggle"; describe("ScopeToggle", () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + it("renders checkbox checked when value is 'involves_me'", () => { render(() => {}} />); const checkbox = screen.getByRole("checkbox", { name: /Scope filter/i }); @@ -46,4 +54,19 @@ describe("ScopeToggle", () => { const checkbox = screen.getByRole("checkbox", { name: "Scope filter" }); expect(checkbox).toBeDefined(); }); + + it("shows a tooltip explaining the toggle after hovering the label", () => { + const { container } = render(() => ( + {}} /> + )); + const trigger = container.querySelector("span.inline-flex")!; + expect(document.body.textContent).not.toContain( + "Toggle between items involving you and all tracked activity" + ); + fireEvent.pointerEnter(trigger); + vi.advanceTimersByTime(300); + expect(document.body.textContent).toContain( + "Toggle between items involving you and all tracked activity" + ); + }); }); diff --git a/tests/stores/view.test.ts b/tests/stores/view.test.ts index a1981a09..ba5710e0 100644 --- a/tests/stores/view.test.ts +++ b/tests/stores/view.test.ts @@ -28,7 +28,8 @@ import { untrackJiraItem, moveJiraItem, } from "../../src/app/stores/view"; -import type { IgnoredItem, TrackedItem } from "../../src/app/stores/view"; +import type { IgnoredItem, TrackedItem, ViewState } from "../../src/app/stores/view"; +import { getNotifications, clearNotifications } from "../../src/app/lib/errors"; // view.ts uses createStore — setters work outside reactive context. // We use createRoot only for initViewPersistence (which calls createEffect). @@ -79,6 +80,67 @@ describe("updateViewState", () => { expect(viewState.globalFilter).toEqual({ org: null, repo: null }); expect(viewState.ignoredItems).toEqual([]); }); + + it("is a safe no-op when called with an empty object", () => { + updateViewState({ lastActiveTab: "pullRequests" }); + updateViewState({}); + expect(viewState.lastActiveTab).toBe("pullRequests"); + }); + + // Real call sites (DashboardPage, SettingsPage, config.ts, IssuesTab) only ever pass + // these two field/type shapes — verify both validate and apply correctly. + it("applies a real call-site shape: { lastActiveTab: string }", () => { + updateViewState({ lastActiveTab: "jiraAssigned" }); + expect(viewState.lastActiveTab).toBe("jiraAssigned"); + }); + + it("applies a real call-site shape: { hideDepDashboard: boolean }", () => { + expect(viewState.hideDepDashboard).toBe(true); + updateViewState({ hideDepDashboard: false }); + expect(viewState.hideDepDashboard).toBe(false); + updateViewState({ hideDepDashboard: true }); + expect(viewState.hideDepDashboard).toBe(true); + }); + + // Structural guard: updating ANY single field must never wipe other fields. + // Catches Zod v4 .partial().safeParse() default inflation (BUG-001 class, mirrors config.test.ts). + it.each([ + ["lastActiveTab", { lastActiveTab: "actions" }], + ["showPrRuns", { showPrRuns: true }], + ["hideDepDashboard", { hideDepDashboard: false }], + ["dependencyExpandedGroups", { dependencyExpandedGroups: ["patch"] }], + ] satisfies [string, Partial][])("updating only %s preserves other non-default fields", (_fieldName, patch) => { + // Seed an unrelated field with a non-default value first. + updateViewState({ lastActiveTab: "pullRequests" }); + expect(viewState.lastActiveTab).toBe("pullRequests"); + + // Now update a (possibly different) single field. + updateViewState(patch); + + // The previously-seeded field must still hold its non-default value + // unless this iteration's patch targeted it directly. + if (!("lastActiveTab" in patch)) { + expect(viewState.lastActiveTab).toBe("pullRequests"); + } + }); + + describe("invalid data", () => { + beforeEach(() => { + clearNotifications(); + }); + + it("rejects a single field with the wrong type, pushes a warning notification, and leaves the store unchanged", () => { + const before = viewState.hideDepDashboard; + updateViewState({ hideDepDashboard: "not-a-boolean" as unknown as boolean }); + + expect(viewState.hideDepDashboard).toBe(before); + + const notifications = getNotifications(); + expect(notifications).toHaveLength(1); + expect(notifications[0].source).toBe("view:updateViewState"); + expect(notifications[0].severity).toBe("warning"); + }); + }); }); describe("setGlobalFilter", () => {