diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cec0b36c..3fecdc52 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,8 +18,9 @@ jobs: run: bash scripts/verify-csp-hash.sh - run: pnpm run typecheck - run: pnpm test - - name: Install Playwright browsers - run: npx playwright install chromium --with-deps + # No `playwright install` step: playwright.config.ts uses channel: "chrome", + # which resolves to ubuntu-latest's preinstalled Google Chrome — Playwright's + # own bundled Chromium download is never used and would be wasted work. - name: Run E2E tests run: pnpm test:e2e env: diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 23eb9ec5..390ea186 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -572,7 +572,9 @@ When Jira is connected, a **Jira** tab appears in the tab bar. It shows all open **Filters:** Status category (New, In Progress) and priority (Highest through Lowest) filters are available in the filter popover. -**Grouping:** Issues are grouped by Jira project key, similar to how GitHub items are grouped by repo. +**Grouping:** Issues are grouped by Jira project key, similar to how GitHub items are grouped by repo — except when Custom order is active (see below), which shows one flat list. + +**Custom order:** By default, issues show in one flat list across all projects, ranked in whatever order you've arranged them (each row shows its project as a small badge). Use the up/down arrows on each row to move it — moves work across page boundaries and the page view follows the row. Arrows are only enabled while viewing "Assigned to me" with no status/priority filter applied (your arrangement is still visible otherwise, just not editable from that view). Pick any other option from the sort dropdown to switch to grouped/sorted display instead; a small "Custom order" button next to the dropdown switches back. Project group locking (pinning a project to the top) is only available in grouped views. **Pagination:** Client-side over up to 100 fetched issues. diff --git a/src/app/components/dashboard/DashboardPage.tsx b/src/app/components/dashboard/DashboardPage.tsx index 505fbb1e..db63861a 100644 --- a/src/app/components/dashboard/DashboardPage.tsx +++ b/src/app/components/dashboard/DashboardPage.tsx @@ -9,7 +9,7 @@ import PullRequestsTab from "./PullRequestsTab"; import TrackedTab from "./TrackedTab"; import PersonalSummaryStrip from "./PersonalSummaryStrip"; import { config, setConfig, getCustomTab, isBuiltinTab, isActionsBasedTab, isTabUnscoped, updateJiraConfig, type TrackedUser } from "../../stores/config"; -import { viewState, updateViewState, setSortPreference, pruneClosedTrackedItems, removeCustomTabState, untrackJiraItem, setTabFilter, IssueFiltersSchema, PullRequestFiltersSchema, ActionsFiltersSchema } from "../../stores/view"; +import { viewState, updateViewState, setSortPreference, pruneClosedTrackedItems, removeCustomTabState, untrackJiraItem, setTabFilter, IssueFiltersSchema, PullRequestFiltersSchema, ActionsFiltersSchema, pruneJiraCustomOrder, JIRA_CUSTOM_ORDER_SCOPE } from "../../stores/view"; import DependenciesTab from "./DependenciesTab"; import { isDependencyPr, expandBotLogins, needsBodyFallback, parseRenovateBody, type VersionInfo } from "../../lib/dependency-detection"; import { isRepoExcludedFromDependencies } from "../../lib/dependency-exclusion"; @@ -479,6 +479,19 @@ export default function DashboardPage() { if (!isJiraAuthenticated()) return; setJiraIssues(result.issues); + // Only prune when the result set is complete. The Enhanced JQL Search + // endpoint (search/jql) does not return a `total` match count — it uses + // cursor-based pagination via `nextPageToken` instead. A defined + // nextPageToken means more results exist beyond this page (pagination + // truncation); its absence means this page is the complete result set. + // Treating "not in this page" as "no longer exists" on a truncated page + // would permanently destroy user-curated custom-order positions for + // issues that are simply on a later page, not actually gone (closed, + // reassigned, or resolved). + if (scope === JIRA_CUSTOM_ORDER_SCOPE && result.nextPageToken === undefined) { + pruneJiraCustomOrder(new Set(result.issues.map((i) => i.key))); + } + // Auto-prune tracked Jira items that are done or deleted (scope-independent). // Resolves status from current search results first, then bulkFetches only // keys not covered (items from a different scope than the current view). diff --git a/src/app/components/dashboard/JiraAssignedTab.tsx b/src/app/components/dashboard/JiraAssignedTab.tsx index cee4d8bd..04f46af7 100644 --- a/src/app/components/dashboard/JiraAssignedTab.tsx +++ b/src/app/components/dashboard/JiraAssignedTab.tsx @@ -1,11 +1,12 @@ -import { createEffect, createMemo, createSignal, For, Show, on } from "solid-js"; +import { createEffect, createMemo, createSignal, For, Show, on, onCleanup } from "solid-js"; import type { JiraIssue } from "../../../shared/jira-types"; -import { viewState, setTabFilter, resetAllTabFilters, JiraFiltersSchema, trackItem, untrackJiraItem, setAllExpanded } from "../../stores/view"; +import { viewState, setTabFilter, JiraFiltersSchema, trackItem, untrackJiraItem, setAllExpanded, setJiraCustomOrder, JIRA_CUSTOM_ORDER_SCOPE, JIRA_CUSTOM_SORT_FIELD } from "../../stores/view"; import { config } from "../../stores/config"; import JiraFieldValue from "./JiraFieldValue"; import { jiraStatusCategoryClass, stripParenthetical } from "../../lib/format"; import { isSafeJiraSiteUrl } from "../../lib/url"; -import { groupByRepo, computePageLayout, slicePageGroups, ensureLockedRepoGroups, orderRepoGroups } from "../../lib/grouping"; +import { groupByRepo, computePageLayout, slicePageGroups, ensureLockedRepoGroups, orderRepoGroups, applyCustomOrder } from "../../lib/grouping"; +import { withScrollLock } from "../../lib/scroll"; import PaginationControls from "../shared/PaginationControls"; import FilterPopover from "../shared/FilterPopover"; import LoadingSpinner from "../shared/LoadingSpinner"; @@ -89,6 +90,14 @@ let _jiraExpandInitialized = false; export function _resetJiraTabState() { _jiraExpandInitialized = false; + itemRefs.clear(); +} + +// Test-only accessor for the module-level itemRefs Map (see below) — lets +// tests verify the mode-exit cleanup effect actually clears stale DOM refs +// without exposing itemRefs itself. +export function _getItemRefsCount(): number { + return itemRefs.size; } const ISSUE_TYPE_ICONS: Record = Object.assign( @@ -121,6 +130,49 @@ function IssueTypeFallbackIcon(props: { name: string }) { ); } +// FLIP animation: record positions before a custom-order move, animate slide after +// DOM updates. Modeled on TrackedTab.tsx's recordPositions/animateMove/prefersReducedMotion +// trio — deliberately duplicated here rather than extracted into a +// shared utility (see plan Task 3, Step 5). +// +// Deviation from TrackedTab.tsx: TrackedTab's animateMove only guards with +// `if (prefersReducedMotion()) return;`, which skips the animation but leaves the +// preceding state mutation unprotected against scroll jump. That guard is kept here too +// (defense-in-depth), but the primary reduced-motion routing decision lives in +// handleCustomMove below, which decides whether to call animateMove at all or route the +// mutation through withScrollLock (src/app/lib/scroll.ts) instead. +const itemRefs = new Map(); +const prefersReducedMotion = () => + typeof window !== "undefined" && window.matchMedia("(prefers-reduced-motion: reduce)").matches; + +function recordPositions(keys: string[]): Map { + const snapshot = new Map(); + for (const key of keys) { + const el = itemRefs.get(key); + if (el) snapshot.set(key, el.getBoundingClientRect()); + } + return snapshot; +} + +function animateMove(before: Map, keys: string[]) { + if (prefersReducedMotion()) return; + requestAnimationFrame(() => { + for (const key of keys) { + const el = itemRefs.get(key); + if (!el) continue; + const old = before.get(key); + if (!old) continue; + const now = el.getBoundingClientRect(); + const dy = old.top - now.top; + if (Math.abs(dy) < 1) continue; + el.animate( + [{ transform: `translateY(${dy}px)` }, { transform: "translateY(0)" }], + { duration: 200, easing: "ease-in-out" } + ); + } + }); +} + export default function JiraAssignedTab(props: JiraAssignedTabProps) { const [page, setPage] = createSignal(0); @@ -172,6 +224,9 @@ export default function JiraAssignedTab(props: JiraAssignedTabProps) { }); const filteredSorted = createMemo(() => { + if (filters().sortField === JIRA_CUSTOM_SORT_FIELD) { + return applyCustomOrder(filtered(), viewState.jiraCustomOrder, (issue) => issue.key); + } const items = [...filtered()]; const field = filters().sortField; const dir = filters().sortDirection; @@ -226,12 +281,32 @@ export default function JiraAssignedTab(props: JiraAssignedTabProps) { }); type JiraItem = JiraIssue & { repoFullName: string }; - const itemsWithGroupKey = createMemo(() => - filteredSorted().map((issue): JiraItem => ({ - ...issue, - repoFullName: issue.fields.project?.key ?? "OTHER", - })) - ); + + // Cache wrapper objects by issue key so that a reorder (same JiraIssue references, + // new array order) reuses the SAME JiraItem object per issue. This preserves + // reference equality for 's keyed reconciliation, letting it move DOM nodes + // instead of tearing down / rebuilding every row on each arrow-click (Finding 4). + const itemsWithGroupKeyCache = new Map(); + const itemsWithGroupKey = createMemo(() => { + const result = filteredSorted().map((issue): JiraItem => { + const cached = itemsWithGroupKeyCache.get(issue.key); + if (cached && cached.source === issue) return cached.wrapped; + const wrapped: JiraItem = { ...issue, repoFullName: issue.fields.project?.key ?? "OTHER" }; + itemsWithGroupKeyCache.set(issue.key, { source: issue, wrapped }); + return wrapped; + }); + // Prune stale cache entries for issues that left the list (e.g. after a data + // refresh or filter change). The cache naturally self-limits to the active issue + // count (capped at ~500 elsewhere in the ecosystem), so this is a hygiene measure + // rather than a hard cap. + if (itemsWithGroupKeyCache.size > result.length) { + const activeKeys = new Set(result.map(item => item.key)); + for (const key of itemsWithGroupKeyCache.keys()) { + if (!activeKeys.has(key)) itemsWithGroupKeyCache.delete(key); + } + } + return result; + }); const repoGroups = createMemo(() => { const groups = groupByRepo(itemsWithGroupKey()); @@ -244,10 +319,35 @@ export default function JiraAssignedTab(props: JiraAssignedTabProps) { return orderRepoGroups(withLocked, lockedForTab); }); + const isCustomMode = () => filters().sortField === JIRA_CUSTOM_SORT_FIELD; + + // itemsWithGroupKey() is a 1:1, order-preserving map over filteredSorted() (adds + // repoFullName, never filters/reorders), so paginating it directly yields the same + // slices filteredSorted() would, while giving renderIssueRow's shared JiraItem rows + // the project key it needs for the Step 4 badge without re-deriving it. + // + // Both are createMemo (not plain arrow functions) because customPageItems is called + // 3+ times per render and does a real .slice() each time (Finding 1). + const customPageCount = createMemo(() => Math.max(1, Math.ceil(itemsWithGroupKey().length / ITEMS_PER_PAGE))); + const customPageItems = createMemo(() => itemsWithGroupKey().slice(page() * ITEMS_PER_PAGE, (page() + 1) * ITEMS_PER_PAGE)); + + // Prune itemRefs to current-page items in custom mode so the map does not grow + // unbounded across pages, filter changes, and data refreshes (Finding 2 & 3). + createEffect(() => { + if (!isCustomMode()) { + itemRefs.clear(); + return; + } + const pageItemKeys = new Set(customPageItems().map(item => item.key)); + for (const key of itemRefs.keys()) { + if (!pageItemKeys.has(key)) itemRefs.delete(key); + } + }); + const pageLayout = createMemo(() => computePageLayout(repoGroups(), ITEMS_PER_PAGE)); - const pageCount = createMemo(() => pageLayout().pageCount); + const pageCount = createMemo(() => (isCustomMode() ? customPageCount() : pageLayout().pageCount)); const pageGroups = createMemo(() => - slicePageGroups(repoGroups(), pageLayout().boundaries, pageCount(), page()) + slicePageGroups(repoGroups(), pageLayout().boundaries, pageLayout().pageCount, page()) ); const projectKeys = createMemo(() => repoGroups().map((g) => g.repoFullName)); @@ -266,6 +366,247 @@ export default function JiraAssignedTab(props: JiraAssignedTabProps) { setAllExpanded(TAB_KEY, keys, true); }); + // Reordering is only meaningful — and safe — against the canonical, unfiltered + // "assigned" scope: filtered() must exclude nothing so filteredSorted()'s key list + // is the complete set, matching what Task 4's prune gate guards against. + const canReorder = () => + filters().scope === JIRA_CUSTOM_ORDER_SCOPE && filters().statusCategory === "all" && filters().priority === "all"; + + const [reordering, setReordering] = createSignal(false); + let reorderTimeoutId: ReturnType | undefined; + + // Clear the module-level itemRefs map when the component unmounts (e.g. tab + // switch) so detached DOM references are not leaked across mount cycles. + // Also clear any pending reordering-lockout timeout so it doesn't fire + // setReordering on a disposed component after a tab switch mid-lockout. + onCleanup(() => { + itemRefs.clear(); + clearTimeout(reorderTimeoutId); + }); + + function handleCustomMove(jiraKey: string, direction: "up" | "down") { + if (!canReorder()) return; + if (reordering()) return; + const order = filteredSorted().map((i) => i.key); + const idx = order.indexOf(jiraKey); + if (idx === -1) return; + const targetIdx = direction === "up" ? idx - 1 : idx + 1; + if (targetIdx < 0 || targetIdx >= order.length) return; + const newPage = Math.floor(targetIdx / ITEMS_PER_PAGE); + const crossesPage = newPage !== page(); + const next = [...order]; + [next[idx], next[targetIdx]] = [next[targetIdx], next[idx]]; + const applyMove = () => { + setJiraCustomOrder(next); + if (crossesPage) setPage(newPage); + }; + setReordering(true); + if (prefersReducedMotion()) { + // Reduced motion: no animation ever, but still guard against a viewport jump + // from the mutation itself (matches withFlipAnimation's reduced-motion fallback in scroll.ts). + withScrollLock(applyMove); + } else if (crossesPage) { + // Cross-page moves must skip the FLIP animation entirely (spike pl-feas-2): the + // old page's rows become detached before animateMove's rAF callback runs, producing + // a broken/misleading animation. Jump straight to the new page instead. + applyMove(); + } else { + const pageKeys = customPageItems().map(i => i.key); + const before = recordPositions(pageKeys); + applyMove(); + animateMove(before, pageKeys); + } + // Uniform 200ms lockout across all branches — prevents rapid clicks / key-repeat + // from queuing moves in the reduced-motion and cross-page paths where the lockout + // was previously reset synchronously (Finding 5). Value matches animateMove's + // `duration: 200` — keep them in sync. + reorderTimeoutId = setTimeout(() => setReordering(false), 200); + } + + function renderIssueRow(issue: JiraItem, boundary?: { isFirst: boolean; isLast: boolean }) { + const isPinned = () => pinnedJiraKeys().has(issue.key); + const browseUrl = () => isSafeJiraSiteUrl(props.siteUrl) ? `${props.siteUrl}/browse/${issue.key}` : "#"; + const isIssueExpanded = () => expandByDefault() ? !toggledIssues().has(issue.key) : toggledIssues().has(issue.key); + const detailPanelId = `jira-detail-${issue.key}`; + const reorderTitle = () => !canReorder() ? "Switch to Assigned to me with no filters to reorder" : undefined; + return ( +
{ if (isCustomMode()) itemRefs.set(issue.key, el); }} + > + +
+ + +
+ +
+
{ + if ((e.target as HTMLElement).closest("a, button")) return; + toggleExpanded(issue.key); + }} + > +
+
+ + {(type) => { + const [imgFailed, setImgFailed] = createSignal(false); + return ( + + } + > + {type().name} setImgFailed(true)} + /> + + + ); + }} + + + {issue.key} + + + {issue.repoFullName} + + + + {issue.fields.summary} + + +
+ +

+ {issue.fields.summary} +

+ +
+
+ + + {stripParenthetical(issue.fields.priority!.name)} + + + + {issue.fields.status.name} + +
+ + + + +
+ +
+ 0} + fallback={ +

+ No custom fields configured — add them in Settings. +

+ } + > +
+ + {(field) => { + const val = () => (issue.fields as Record)[field.id]; + return ( + + + {field.name}: + + + + ); + }} + +
+ +
+ +
+
+ ); + } + return (
{/* Filter + sort toolbar */} @@ -315,7 +656,11 @@ export default function JiraAssignedTab(props: JiraAssignedTabProps) { @@ -324,20 +669,35 @@ export default function JiraAssignedTab(props: JiraAssignedTabProps) { {filtered().length} issue{filtered().length !== 1 ? "s" : ""} + + + { setTabFilter("jiraAssigned", "sortField", field); setTabFilter("jiraAssigned", "sortDirection", dir); setPage(0); }} /> - setAllExpanded(TAB_KEY, projectKeys(), true)} - onCollapseAll={() => setAllExpanded(TAB_KEY, projectKeys(), false)} - /> + + setAllExpanded(TAB_KEY, projectKeys(), true)} + onCollapseAll={() => setAllExpanded(TAB_KEY, projectKeys(), false)} + /> +
@@ -347,193 +707,65 @@ export default function JiraAssignedTab(props: JiraAssignedTabProps) { - {/* Jira project groups + locked stubs */} - 0}> -
- - {(group) => { - const isEmpty = () => group.items.length === 0; - const isExpanded = () => !isEmpty() && !!(viewState.expandedRepos[TAB_KEY] ?? {})[group.repoFullName]; - - return ( -
-
- + +
+ +
+ + {(issue) => renderIssueRow(issue)} + +
+ + +
+ No matching issues in {group.repoFullName} +
- - -
- -
- - {(issue) => { - const isPinned = () => pinnedJiraKeys().has(issue.key); - const browseUrl = () => isSafeJiraSiteUrl(props.siteUrl) ? `${props.siteUrl}/browse/${issue.key}` : "#"; - const isIssueExpanded = () => expandByDefault() ? !toggledIssues().has(issue.key) : toggledIssues().has(issue.key); - const detailPanelId = `jira-detail-${issue.key}`; - return ( -
-
{ - if ((e.target as HTMLElement).closest("a, button")) return; - toggleExpanded(issue.key); - }} - > -
-
- - {(type) => { - const [imgFailed, setImgFailed] = createSignal(false); - return ( - - } - > - {type().name} setImgFailed(true)} - /> - - - ); - }} - - - {issue.key} - - - - {issue.fields.summary} - - -
- -

- {issue.fields.summary} -

- -
-
- - - {stripParenthetical(issue.fields.priority!.name)} - - - - {issue.fields.status.name} - -
- - - - -
- -
- 0} - fallback={ -

- No custom fields configured — add them in Settings. -

- } - > -
- - {(field) => { - const val = () => (issue.fields as Record)[field.id]; - return ( - - - {field.name}: - - - - ); - }} - -
- -
- -
- ); - }} - -
- - -
- No matching issues in {group.repoFullName}
- -
- ); - }} - - + ); + }} + + + } + > +
+ + {(issue, index) => + renderIssueRow(issue, { + isFirst: page() === 0 && index() === 0, + isLast: page() === pageCount() - 1 && index() === customPageItems().length - 1, + }) + } + +
+ 1}>
void; + placeholder?: string; } interface FlatOption { @@ -57,6 +58,7 @@ export default function SortDropdown(props: SortDropdownProps) { optionTextValue="label" value={flatOptions().find((o) => o.value === selected()) ?? null} onChange={(opt) => handleChange(opt?.value ?? null)} + placeholder={props.placeholder ?? "Sort by"} itemComponent={(itemProps) => ( - >{(state) => state.selectedOption()?.label ?? "Sort by"} + >{(state) => state.selectedOption()?.label ?? (props.placeholder ?? "Sort by")} ▾ diff --git a/src/app/lib/grouping.ts b/src/app/lib/grouping.ts index 3cfc0d82..ea4e9a9a 100644 --- a/src/app/lib/grouping.ts +++ b/src/app/lib/grouping.ts @@ -90,6 +90,41 @@ export function orderRepoGroups( return [...locked, ...unlocked]; } +/** + * Orders `items` by `order` (a list of keys produced by `keyFn`), appending any + * items not referenced in `order` at the end, in their original relative order. + * If two items share the same key, the later one in `items` wins. `order` is + * walked as-is and is not itself deduplicated — a key appearing twice in `order` + * will pull the matching item twice into the result. + * Returns `items` unchanged (same reference) when `order` is empty. + */ +export function applyCustomOrder(items: T[], order: string[], keyFn: (item: T) => string): T[] { + if (order.length === 0) return items; + + const map = new Map(); + for (const item of items) { + map.set(keyFn(item), item); + } + + const referenced = new Set(); + const result: T[] = []; + for (const key of order) { + const item = map.get(key); + if (item !== undefined) { + result.push(item); + referenced.add(key); + } + } + + for (const [key, item] of map) { + if (!referenced.has(key)) { + result.push(item); + } + } + + return result; +} + /** * Three-tier involvement check for scope filtering. * Shared by IssuesTab and PullRequestsTab — keep both call sites in sync. diff --git a/src/app/stores/view.ts b/src/app/stores/view.ts index d35c7f7c..ed7cdb67 100644 --- a/src/app/stores/view.ts +++ b/src/app/stores/view.ts @@ -7,6 +7,10 @@ export const VIEW_STORAGE_KEY = "github-tracker:view"; const IGNORED_ITEMS_CAP = 500; const TRACKED_ITEMS_CAP = 200; export const LOCKED_REPOS_CAP = 50; +export const JIRA_CUSTOM_ORDER_CAP = 500; +export const JIRA_CUSTOM_ORDER_KEY_MAX_LENGTH = 50; +export const JIRA_CUSTOM_ORDER_SCOPE = "assigned" as const; +export const JIRA_CUSTOM_SORT_FIELD = "custom" as const; export const TrackedItemSchema = z.object({ id: z.number(), @@ -59,7 +63,7 @@ export const JiraFiltersSchema = z.object({ scope: z.enum(["assigned", "reported", "watching"]).or(z.string().regex(/^[a-zA-Z0-9_\-]+$/).max(100)).default("assigned"), statusCategory: z.enum(["all", "new", "indeterminate"]).default("all"), priority: z.enum(["all", "Highest", "High", "Medium", "Low", "Lowest"]).default("all"), - sortField: z.string().default("status"), + sortField: z.string().default(JIRA_CUSTOM_SORT_FIELD), sortDirection: z.enum(["asc", "desc"]).default("asc"), }); @@ -100,13 +104,13 @@ export const ViewStateSchema = z.object({ issues: IssueFiltersSchema.default({ scope: "involves_me", role: "all", comments: "all", user: "all" }), pullRequests: PullRequestFiltersSchema.default({ scope: "involves_me", role: "all", reviewDecision: "all", draft: "all", checkStatus: "all", sizeCategory: "all", user: "all" }), actions: ActionsFiltersSchema.default({ conclusion: "all", event: "all" }), - jiraAssigned: JiraFiltersSchema.default({ scope: "assigned", statusCategory: "all", priority: "all", sortField: "status", sortDirection: "asc" }), + jiraAssigned: JiraFiltersSchema.default({ scope: "assigned", statusCategory: "all", priority: "all", sortField: JIRA_CUSTOM_SORT_FIELD, sortDirection: "asc" }), dependencies: DependencyFiltersSchema.default({ updateType: "all", bot: "all" }), }).default({ issues: { scope: "involves_me", role: "all", comments: "all", user: "all" }, pullRequests: { scope: "involves_me", role: "all", reviewDecision: "all", draft: "all", checkStatus: "all", sizeCategory: "all", user: "all" }, actions: { conclusion: "all", event: "all" }, - jiraAssigned: { scope: "assigned", statusCategory: "all", priority: "all", sortField: "status", sortDirection: "asc" }, + jiraAssigned: { scope: "assigned", statusCategory: "all", priority: "all", sortField: JIRA_CUSTOM_SORT_FIELD, sortDirection: "asc" }, dependencies: { updateType: "all", bot: "all" }, }), showPrRuns: z.boolean().default(false), @@ -127,12 +131,15 @@ export const ViewStateSchema = z.object({ lockedRepos: z.record(z.string(), z.array(z.string().max(200)).max(LOCKED_REPOS_CAP)).default({ issues: [], pullRequests: [], actions: [], jiraAssigned: [] }), trackedItems: z.array(TrackedItemSchema).max(TRACKED_ITEMS_CAP).default([]), dependencyExpandedGroups: z.array(z.string()).default(["mergeable"]), + jiraCustomOrder: z.array(z.string().max(JIRA_CUSTOM_ORDER_KEY_MAX_LENGTH)).max(JIRA_CUSTOM_ORDER_CAP).default([]) + .transform((arr) => [...new Set(arr)]), }); export type ViewState = z.infer; export type IgnoredItem = ViewState["ignoredItems"][number]; const REPO_STATE_TAB_IDS = ["issues", "pullRequests", "actions", "jiraAssigned"] as const; +const VIEW_STATE_KEYS = new Set(Object.keys(ViewStateSchema.shape)); export function migrateLockedRepos(raw: unknown): unknown { if (raw == null) return { issues: [], pullRequests: [], actions: [], jiraAssigned: [] }; @@ -171,6 +178,13 @@ function loadViewState(): ViewState { } } } + if (Array.isArray(obj.jiraCustomOrder)) { + obj.jiraCustomOrder = obj.jiraCustomOrder + .filter((item): item is string => typeof item === "string" && item.length <= JIRA_CUSTOM_ORDER_KEY_MAX_LENGTH) + .slice(0, JIRA_CUSTOM_ORDER_CAP); + } else if (obj.jiraCustomOrder !== undefined) { + delete obj.jiraCustomOrder; + } const result = ViewStateSchema.safeParse(parsed); if (result.success) return result.data; return ViewStateSchema.parse({}); @@ -209,7 +223,7 @@ export function resetViewState(): void { issues: { scope: "involves_me", role: "all", comments: "all", user: "all" }, pullRequests: { scope: "involves_me", role: "all", reviewDecision: "all", draft: "all", checkStatus: "all", sizeCategory: "all", user: "all" }, actions: { conclusion: "all", event: "all" }, - jiraAssigned: { scope: "assigned", statusCategory: "all", priority: "all", sortField: "status", sortDirection: "asc" }, + jiraAssigned: { scope: "assigned", statusCategory: "all", priority: "all", sortField: JIRA_CUSTOM_SORT_FIELD, sortDirection: "asc" }, dependencies: { updateType: "all", bot: "all" }, }, showPrRuns: false, @@ -219,6 +233,7 @@ export function resetViewState(): void { lockedRepos: { issues: [], pullRequests: [], actions: [], jiraAssigned: [] }, trackedItems: [], dependencyExpandedGroups: ["mergeable"], + jiraCustomOrder: [], }); }) ); @@ -333,6 +348,10 @@ export function resetAllTabFilters( } else if (tab === "pullRequests") { draft.tabFilters.pullRequests = PullRequestFiltersSchema.parse({}); } else if (tab === "jiraAssigned") { + // WARNING: This resets sortField and scope as a side effect (back to defaults + // "custom" and "assigned"). JiraAssignedTab.tsx's Clear button uses two targeted + // setTabFilter calls instead of this function specifically to avoid that. + // Do not casually wire this back in for Jira without accounting for those resets. draft.tabFilters.jiraAssigned = JiraFiltersSchema.parse({}); } else if (tab === "actions") { draft.tabFilters.actions = ActionsFiltersSchema.parse({}); @@ -565,6 +584,34 @@ export function moveTrackedItem( ); } +export function setJiraCustomOrder(order: string[]): void { + // Drop entries exceeding per-item length cap (consistent with loadViewState's guard), + // then dedup (schema transform only fires on parse, not on direct produce() mutations) + const sanitized = order.filter( + (k): k is string => typeof k === "string" && k.length <= JIRA_CUSTOM_ORDER_KEY_MAX_LENGTH + ); + const deduped = [...new Set(sanitized)]; + setViewState( + produce((draft) => { + draft.jiraCustomOrder = deduped.length > JIRA_CUSTOM_ORDER_CAP + ? deduped.slice(0, JIRA_CUSTOM_ORDER_CAP) + : deduped; + }) + ); +} + +export function pruneJiraCustomOrder(activeJiraKeys: Set): void { + const current = untrack(() => viewState.jiraCustomOrder); + if (current.length === 0) return; + const filtered = current.filter((key) => activeJiraKeys.has(key)); + if (filtered.length === current.length) return; + setViewState( + produce((draft) => { + draft.jiraCustomOrder = filtered; + }) + ); +} + export function pruneClosedTrackedItems(pruneKeys: Set): void { setViewState( produce((draft) => { @@ -576,27 +623,120 @@ export function pruneClosedTrackedItems(pruneKeys: Set): void { } export function initViewPersistence(): void { + if (typeof window !== "undefined") { + const handleViewStorage = (e: StorageEvent) => { + if (e.key !== VIEW_STORAGE_KEY || e.newValue === null) return; + let parsed: unknown; + try { + parsed = JSON.parse(e.newValue); + } catch { + return; + } + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return; + // Guard: if the incoming blob doesn't contain jiraCustomOrder at all, skip sync. + // Without this, Zod's .default([]) would backfill the missing key with an empty + // array and silently wipe the current tab's real order. + if (!("jiraCustomOrder" in (parsed as Record))) return; + const result = ViewStateSchema.pick({ jiraCustomOrder: true }).safeParse(parsed); + if (!result.success) return; + const incoming = result.data.jiraCustomOrder; + if (JSON.stringify(incoming) === JSON.stringify(untrack(() => viewState.jiraCustomOrder))) return; + setViewState(produce((draft) => { draft.jiraCustomOrder = incoming; })); + }; + window.addEventListener("storage", handleViewStorage); + onCleanup(() => window.removeEventListener("storage", handleViewStorage)); + } + + // Baseline snapshot of what this tab last knew to be persisted to localStorage + // (starts at the state this tab loaded at boot). Used by commitSnapshot() to work + // out which top-level fields THIS tab actually changed, so a debounced write only + // overlays those fields onto whatever is CURRENTLY on disk instead of blindly + // overwriting with this tab's full (possibly stale) in-memory snapshot. Without + // this, a full-object overwrite from Tab A could silently revert a newer value + // Tab B wrote for some other field (e.g. jiraCustomOrder) in the gap between when + // Tab A's debounce timer started and when it actually fires. + let lastSyncedSnapshot: Record = JSON.parse(JSON.stringify(untrack(() => viewState))); + + function readOnDiskState(): Record | undefined { + try { + const raw = localStorage.getItem(VIEW_STORAGE_KEY); + if (raw === null) return undefined; + const parsed = JSON.parse(raw) as unknown; + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) return undefined; + // Only trust keys that are part of the current ViewState schema AND whose value + // passes that field's own Zod validation. localStorage is writable by any + // same-origin script (extensions, a stale/incompatible schema version, manual + // tampering), and commitSnapshot() below folds this content into every future + // write for fields this tab doesn't happen to touch itself — without both the + // key-name allowlist and per-field value validation, unrecognized or malformed + // on-disk content would be perpetually re-persisted instead of self-healing on + // the next write, as it did before the merge-on-write logic existed. Validated + // one key at a time (not a single ViewStateSchema.pick(...).safeParse() over the + // whole object) so one malformed field can't cause every other valid field to be + // dropped too. + const filtered: Record = {}; + for (const [key, value] of Object.entries(parsed as Record)) { + if (!VIEW_STATE_KEYS.has(key)) continue; + const result = ViewStateSchema.pick({ [key]: true } as Partial>).safeParse({ [key]: value }); + if (result.success) filtered[key] = (result.data as Record)[key]; + } + return filtered; + } catch { + return undefined; + } + } + + function commitSnapshot(snapshot: Record): void { + const onDisk = readOnDiskState(); + // Start from whatever is currently on disk (may include newer values another tab + // wrote); fall back to this tab's own snapshot if disk is unreadable/absent. + const merged: Record = { ...(onDisk ?? snapshot) }; + for (const key of Object.keys(snapshot)) { + // Always include a key this tab knows about but that's missing from the on-disk + // blob entirely (stale/corrupted/version-skewed data) — no legitimate flow ever + // produces a ViewState with a top-level key truly absent, so treat that as + // recoverable staleness rather than silently dropping this tab's known value + // (which Zod's .default() would otherwise backfill on the next load). + if (!(key in merged) || JSON.stringify(snapshot[key]) !== JSON.stringify(lastSyncedSnapshot[key])) { + merged[key] = snapshot[key]; + } + } + try { + localStorage.setItem(VIEW_STORAGE_KEY, JSON.stringify(merged)); + lastSyncedSnapshot = merged; + } catch { + pushNotification("localStorage:view", "View state write failed — storage may be full", "warning"); + } + } + let debounceTimer: ReturnType | undefined; - let pendingJson: string | undefined; + let pendingSnapshot: Record | undefined; createEffect(() => { const json = JSON.stringify(viewState); // synchronous read → tracked by SolidJS - pendingJson = json; + const snapshot = JSON.parse(json) as Record; + pendingSnapshot = snapshot; clearTimeout(debounceTimer); debounceTimer = setTimeout(() => { - pendingJson = undefined; - try { - localStorage.setItem(VIEW_STORAGE_KEY, json); - } catch { - pushNotification("localStorage:view", "View state write failed — storage may be full", "warning"); - } + pendingSnapshot = undefined; + commitSnapshot(snapshot); }, 200); - onCleanup(() => { - clearTimeout(debounceTimer); - // Flush pending write synchronously so HMR doesn't lose state - if (pendingJson !== undefined) { - try { localStorage.setItem(VIEW_STORAGE_KEY, pendingJson); } catch { /* best-effort */ } - pendingJson = undefined; - } - }); + }); + + // Registered on the OUTER owner, as a sibling of createEffect rather than + // nested inside it (matching the storage-listener cleanup above). Solid + // invokes a computation's onCleanup on BOTH disposal AND recomputation — + // nesting this inside createEffect would flush the stale pending snapshot + // synchronously on every dependency change (any viewState mutation), + // defeating the 200ms debounce for every change except the trailing one. + // Registered here instead, it only fires once, when this owner (the App + // component) is truly disposed — the unmount/HMR case the comment below + // describes. + onCleanup(() => { + clearTimeout(debounceTimer); + // Flush pending write synchronously so HMR doesn't lose state + if (pendingSnapshot !== undefined) { + commitSnapshot(pendingSnapshot); + pendingSnapshot = undefined; + } }); } diff --git a/tests/components/DashboardPage.test.tsx b/tests/components/DashboardPage.test.tsx index c3e01de8..cdd5c445 100644 --- a/tests/components/DashboardPage.test.tsx +++ b/tests/components/DashboardPage.test.tsx @@ -2498,6 +2498,227 @@ describe("DashboardPage — dependency exclusions", () => { }); }); +describe("DashboardPage — pruneJiraCustomOrder on refresh", () => { + // Draining the microtask queue deterministically (no real-time sleep) — + // same pattern as flushPromises() in tests/services/poll.test.ts and + // tests/services/events-poll.test.ts. fetchJiraAssigned() has no + // intervening `await` between the searchJql resolution and the + // scope/truncation-gated prune call, so a handful of microtask ticks is + // enough to guarantee that code has run before we assert on the result. + async function flushPromises(): Promise { + for (let i = 0; i < 10; i++) await Promise.resolve(); + } + + // Shared mock scaffolding for all three tests below: each needs jiraAuth, + // isJiraAuthenticated, and JiraClient to behave differently from the + // default mocks configured in the outer beforeEach, which requires a + // fully isolated module reload (vi.resetModules() + vi.doMock() + fresh + // dynamic imports). The only thing that varies between tests is the + // searchJql mock's resolved value. + async function setupPruneTestMocks(mockSearchJql: ReturnType) { + vi.resetModules(); + authClearCallbacks.length = 0; + + vi.doMock("../../src/app/stores/auth", () => ({ + clearAuth: vi.fn(), + expireToken: vi.fn(), + token: () => "fake-token", + user: () => ({ login: "testuser", avatar_url: "", name: "Test User" }), + isAuthenticated: () => true, + onAuthCleared: vi.fn((cb: () => void) => { authClearCallbacks.push(cb); }), + DASHBOARD_STORAGE_KEY: "github-tracker:dashboard", + DEP_META_STORAGE_KEY: "github-tracker:dep-meta", + jiraAuth: vi.fn(() => ({ + cloudId: "cloud-123", + accessToken: "tok", + siteUrl: "https://test.atlassian.net", + siteName: "Test Site", + })), + isJiraAuthenticated: vi.fn(() => true), + setJiraAuth: vi.fn(), + clearJiraAuth: vi.fn(), + ensureJiraTokenValid: vi.fn().mockResolvedValue(true), + })); + + const MockJiraClient = vi.fn(function (this: Record) { + this.searchJql = mockSearchJql; + this.bulkFetch = vi.fn().mockResolvedValue({ issues: [], errors: [] }); + }); + vi.doMock("../../src/app/services/jira-client", () => ({ + JiraClient: MockJiraClient, + JiraProxyClient: vi.fn(), + JiraApiError: class JiraApiError extends Error { + status: number; + constructor(status: number, _body: unknown, message: string) { + super(message); + this.status = status; + } + }, + DEFAULT_FIELDS: ["summary", "status", "priority", "assignee", "project", "updated", "issuetype", "created"], + })); + + vi.doMock("../../src/app/services/poll", () => ({ + fetchAllData: vi.fn().mockResolvedValue({ + issues: [], pullRequests: [], workflowRuns: [], errors: [], + }), + createPollCoordinator: vi.fn().mockImplementation( + (_getInterval: unknown, fetchAll: () => Promise) => { + void fetchAll().catch(() => {}); + return { isRefreshing: () => false, lastRefreshAt: () => null, manualRefresh: vi.fn(), destroy: vi.fn() }; + } + ), + createHotPollCoordinator: vi.fn().mockImplementation(() => ({ destroy: vi.fn() })), + createEventsPollCoordinator: vi.fn().mockImplementation(() => ({ destroy: vi.fn() })), + rebuildHotSets: vi.fn(), + seedHotSetsFromTargeted: vi.fn(), + clearHotSets: vi.fn(), + getHotPollGeneration: vi.fn().mockReturnValue(0), + })); + + // Fresh imports after mock registration + const freshView = await import("../../src/app/stores/view"); + const freshConfig = await import("../../src/app/stores/config"); + const freshDash = await import("../../src/app/components/dashboard/DashboardPage"); + + freshView.resetViewState(); + freshConfig.resetConfig(); + freshConfig.updateJiraConfig({ enabled: true, siteUrl: "https://test.atlassian.net", siteName: "Test Site", authMethod: "oauth" }); + + return { freshView, freshConfig, freshDash }; + } + + it("prunes jiraCustomOrder entries absent from assigned-scope results", async () => { + const mockSearchJql = vi.fn().mockResolvedValue({ + issues: [ + { + id: "1001", key: "PROJ-1", self: "https://test.atlassian.net/rest/api/3/issue/1001", + fields: { + summary: "First issue", status: { id: "1", name: "To Do", statusCategory: { id: 2, key: "new" as const, name: "To Do" } }, + priority: { id: "3", name: "Medium" }, assignee: null, + project: { id: "10000", key: "PROJ", name: "Project" }, + }, + }, + { + id: "1003", key: "PROJ-3", self: "https://test.atlassian.net/rest/api/3/issue/1003", + fields: { + summary: "Third issue", status: { id: "1", name: "To Do", statusCategory: { id: 2, key: "new" as const, name: "To Do" } }, + priority: { id: "3", name: "Medium" }, assignee: null, + project: { id: "10000", key: "PROJ", name: "Project" }, + }, + }, + ], + // No nextPageToken: this is the complete result set — safe to prune. + maxResults: 100, startAt: 0, + }); + + const { freshView, freshDash } = await setupPruneTestMocks(mockSearchJql); + freshView.setJiraCustomOrder(["PROJ-1", "PROJ-2", "PROJ-3"]); + + render(() => ); + + await waitFor(() => { + expect(freshView.viewState.jiraCustomOrder).toEqual(["PROJ-1", "PROJ-3"]); + }, { timeout: 3000 }); + }); + + it("does NOT prune jiraCustomOrder when a non-assigned scope is active", async () => { + // This is the primary regression test for the scope gate at + // DashboardPage.tsx (`if (scope === JIRA_CUSTOM_ORDER_SCOPE)`). It mounts + // the real and lets the real fetchJiraAssigned() run, + // activating scope "reported" before mount and returning Jira results + // that deliberately share NO keys with the existing jiraCustomOrder. If + // the gate were ever inverted or removed, the real prune call would wipe + // jiraCustomOrder down to [] here. + const mockSearchJql = vi.fn().mockResolvedValue({ + issues: [ + { + id: "2001", key: "OTHER-1", self: "https://test.atlassian.net/rest/api/3/issue/2001", + fields: { + summary: "Reported issue", status: { id: "1", name: "To Do", statusCategory: { id: 2, key: "new" as const, name: "To Do" } }, + priority: { id: "3", name: "Medium" }, assignee: null, + project: { id: "20000", key: "OTHER", name: "Other Project" }, + }, + }, + ], + maxResults: 100, startAt: 0, + }); + + const { freshView, freshDash } = await setupPruneTestMocks(mockSearchJql); + freshView.setJiraCustomOrder(["PROJ-1", "PROJ-2", "PROJ-3"]); + // Activate a non-"assigned" scope BEFORE mount, so the immediate + // on-mount fetchJiraAssigned() call reads scope "reported" from + // viewState.tabFilters.jiraAssigned — exactly the branch the gate at + // DashboardPage.tsx is supposed to skip pruning for. + freshView.setTabFilter("jiraAssigned", "scope", "reported"); + + render(() => ); + + // Wait for the real fetchJiraAssigned() to actually invoke searchJql... + await waitFor(() => { + expect(mockSearchJql).toHaveBeenCalled(); + }, { timeout: 3000 }); + // ...then deterministically flush the microtask queue so the code after + // the awaited searchJql call (setJiraIssues + the scope-gated prune + // call) has actually finished running before we assert on the result. + await flushPromises(); + + expect(freshView.viewState.jiraCustomOrder).toEqual(["PROJ-1", "PROJ-2", "PROJ-3"]); + }); + + it("does NOT prune jiraCustomOrder when the assigned-scope result is truncated by pagination", async () => { + // Regression test for the truncation guard at DashboardPage.tsx + // (`result.nextPageToken === undefined`). searchJql returns only 2 + // issues and a defined nextPageToken — simulating a user with more than + // maxResults assigned non-done issues, where the real API paginates via + // an opaque cursor rather than reporting a total match count. + // pruneJiraCustomOrder must NOT run here: treating "not in this page" as + // "no longer exists" would permanently delete the custom-order position + // for issues that are simply on a later page, not actually gone. + const mockSearchJql = vi.fn().mockResolvedValue({ + issues: [ + { + id: "1001", key: "PROJ-1", self: "https://test.atlassian.net/rest/api/3/issue/1001", + fields: { + summary: "First issue", status: { id: "1", name: "To Do", statusCategory: { id: 2, key: "new" as const, name: "To Do" } }, + priority: { id: "3", name: "Medium" }, assignee: null, + project: { id: "10000", key: "PROJ", name: "Project" }, + }, + }, + { + id: "1002", key: "PROJ-2", self: "https://test.atlassian.net/rest/api/3/issue/1002", + fields: { + summary: "Second issue", status: { id: "1", name: "To Do", statusCategory: { id: 2, key: "new" as const, name: "To Do" } }, + priority: { id: "3", name: "Medium" }, assignee: null, + project: { id: "10000", key: "PROJ", name: "Project" }, + }, + }, + ], + // A defined nextPageToken means more results exist beyond this page: + // the API result is truncated. + nextPageToken: "some-token-value", maxResults: 100, startAt: 0, + }); + + const { freshView, freshDash } = await setupPruneTestMocks(mockSearchJql); + // PROJ-3 is not present in the (truncated) search results but must + // survive because the result set is incomplete. + freshView.setJiraCustomOrder(["PROJ-1", "PROJ-2", "PROJ-3"]); + + render(() => ); + + // Wait for the real fetchJiraAssigned() to actually invoke searchJql... + await waitFor(() => { + expect(mockSearchJql).toHaveBeenCalled(); + }, { timeout: 3000 }); + // ...then deterministically flush the microtask queue so the code after + // the awaited searchJql call (setJiraIssues + the truncation-gated + // prune call) has actually finished running before we assert on the + // result. + await flushPromises(); + + expect(freshView.viewState.jiraCustomOrder).toEqual(["PROJ-1", "PROJ-2", "PROJ-3"]); + }); +}); + // ── Dependencies tab — abandonedDepsMap + dashboardIssueUrls reset on auth clear ─ describe("DashboardPage — abandonedDepsMap and dashboardIssueUrls on auth clear", () => { diff --git a/tests/components/dashboard/JiraAssignedTab.test.tsx b/tests/components/dashboard/JiraAssignedTab.test.tsx index 7541f9a1..6c6d98e2 100644 --- a/tests/components/dashboard/JiraAssignedTab.test.tsx +++ b/tests/components/dashboard/JiraAssignedTab.test.tsx @@ -1,18 +1,43 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { render, screen } from "@solidjs/testing-library"; +import { createSignal } from "solid-js"; // ── Module mocks ────────────────────────────────────────────────────────────── let mockTrackedItems: Array<{ source: string; jiraKey?: string }> = []; let mockJiraFilters: { scope: string; statusCategory: string; priority: string; sortField: string; sortDirection: string } = { scope: "assigned", statusCategory: "all", priority: "all", sortField: "status", sortDirection: "asc" }; +let mockJiraCustomOrder: string[] = []; + +// Lazily-created trigger signal (NOT created at vi.mock hoist time — solid-js's +// createSignal isn't safely callable there, since vi.mock factories may run +// before this file's `import { createSignal } from "solid-js"` has resolved). +// Reading it inside the tabFilters Proxy trap below registers a genuine Solid +// dependency, so bumpMockJiraFilters() can force filters() (a createMemo in +// JiraAssignedTab.tsx) to re-evaluate AFTER mount. Plain `mockJiraFilters = {...}` +// reassignment (used by ~90 pre-existing tests that only need to set state +// BEFORE render()) is untouched and still works exactly as before — this signal +// is purely an opt-in for tests that need a LIVE, post-mount filter change. +let _jiraFiltersVersion: [() => number, (v: number | ((p: number) => number)) => void] | undefined; +function jiraFiltersVersionSignal() { + if (!_jiraFiltersVersion) _jiraFiltersVersion = createSignal(0); + return _jiraFiltersVersion; +} +function bumpMockJiraFilters(next: typeof mockJiraFilters): void { + mockJiraFilters = next; + jiraFiltersVersionSignal()[1]((v) => v + 1); +} vi.mock("../../../src/app/stores/view", () => ({ viewState: new Proxy({} as Record, { get(_t, key: string) { if (key === "trackedItems") return mockTrackedItems; - if (key === "tabFilters") return { jiraAssigned: mockJiraFilters }; + if (key === "tabFilters") { + jiraFiltersVersionSignal()[0](); // register reactive dependency + return { jiraAssigned: mockJiraFilters }; + } if (key === "lockedRepos") return {}; if (key === "expandedRepos") return { jiraAssigned: new Proxy({}, { get: () => true }) }; + if (key === "jiraCustomOrder") return mockJiraCustomOrder; return undefined; }, }), @@ -22,16 +47,19 @@ vi.mock("../../../src/app/stores/view", () => ({ trackItem: vi.fn(), untrackJiraItem: vi.fn(), setAllExpanded: vi.fn(), + setJiraCustomOrder: vi.fn(), + JIRA_CUSTOM_ORDER_SCOPE: "assigned", + JIRA_CUSTOM_SORT_FIELD: "custom", })); vi.mock("../../../src/app/stores/config", () => ({ config: { enableTracking: false }, })); -import JiraAssignedTab, { _resetJiraTabState } from "../../../src/app/components/dashboard/JiraAssignedTab"; +import JiraAssignedTab, { _resetJiraTabState, _getItemRefsCount } from "../../../src/app/components/dashboard/JiraAssignedTab"; import type { JiraIssue } from "../../../src/shared/jira-types"; import { config } from "../../../src/app/stores/config"; -import { trackItem, untrackJiraItem, setAllExpanded, setTabFilter } from "../../../src/app/stores/view"; +import { trackItem, untrackJiraItem, setAllExpanded, setTabFilter, setJiraCustomOrder } from "../../../src/app/stores/view"; // ── Fixtures ────────────────────────────────────────────────────────────────── @@ -66,12 +94,19 @@ function makeIssue( const SITE_URL = "https://mysite.atlassian.net"; +// Builds a jiraAssigned filter object defaulting to the canonical, unfiltered +// "assigned" + "custom" state that gates reordering (canReorder() in the component). +function customFilters(overrides: Partial = {}): typeof mockJiraFilters { + return { scope: "assigned", statusCategory: "all", priority: "all", sortField: "custom", sortDirection: "asc", ...overrides }; +} + // ── Tests ───────────────────────────────────────────────────────────────────── describe("JiraAssignedTab", () => { beforeEach(() => { mockTrackedItems = []; mockJiraFilters = { scope: "assigned", statusCategory: "all", priority: "all", sortField: "status", sortDirection: "asc" }; + mockJiraCustomOrder = []; _resetJiraTabState(); vi.clearAllMocks(); }); @@ -552,4 +587,462 @@ describe("JiraAssignedTab", () => { (config as Record).jira = { customScopes: [] }; }); }); + + // ── Custom order (Task 3) ────────────────────────────────────────────────── + + describe("custom order — flat rendering", () => { + it("renders a flat list with no group headers in custom mode (fresh/default state)", () => { + mockJiraFilters = customFilters(); + const issues = [ + makeIssue("ALPHA-1", "ALPHA"), + makeIssue("BETA-1", "BETA"), + ]; + const { container } = render(() => ); + + // Group headers are wrapped in a "group/repo-header" div — none should render. + // (Per-row expand/collapse chevrons also carry aria-expanded, so that alone + // isn't a reliable signal that a *group* header is absent. classList.contains + // is used instead of a CSS class selector because happy-dom's querySelector + // rejects the unescaped "/" in "group/repo-header" as an invalid selector.) + const hasGroupHeader = Array.from(container.querySelectorAll("div")).some((el) => + el.classList.contains("group/repo-header") + ); + expect(hasGroupHeader).toBe(false); + // Rows still render directly + const rows = screen.getAllByRole("listitem"); + expect(rows).toHaveLength(2); + expect(screen.getByText("ALPHA-1")).toBeTruthy(); + expect(screen.getByText("BETA-1")).toBeTruthy(); + }); + + it("shows a per-row project badge for each issue in custom mode", () => { + mockJiraFilters = customFilters(); + const issues = [makeIssue("ALPHA-1", "ALPHA")]; + render(() => ); + + const row = screen.getAllByRole("listitem")[0]; + const badge = row.querySelector(".badge-ghost"); + expect(badge?.textContent).toBe("ALPHA"); + }); + + it("hides ExpandCollapseButtons in custom mode", () => { + mockJiraFilters = customFilters(); + const issues = [makeIssue("ALPHA-1", "ALPHA")]; + render(() => ); + + expect(screen.queryByRole("button", { name: /expand all/i })).toBeNull(); + expect(screen.queryByRole("button", { name: /collapse all/i })).toBeNull(); + }); + + it("shows 'Custom order' placeholder text in the SortDropdown trigger while in custom mode", () => { + mockJiraFilters = customFilters(); + const issues = [makeIssue("PROJ-1")]; + render(() => ); + + expect(screen.getByText("Custom order")).toBeTruthy(); + }); + + it("restores grouped display with RepoLockControls and hides per-row project badges when sorted by Priority", () => { + mockJiraFilters = { scope: "assigned", statusCategory: "all", priority: "all", sortField: "priority", sortDirection: "asc" }; + const issues = [makeIssue("ALPHA-1", "ALPHA")]; + render(() => ); + + // Group header present + expect(screen.getByRole("button", { expanded: true })).toBeTruthy(); + // RepoLockControls present (pin button) + expect(screen.getByRole("button", { name: /pin alpha to top of list/i })).toBeTruthy(); + // No per-row project badge + const row = screen.getAllByRole("listitem")[0]; + expect(row.querySelector(".badge-ghost")).toBeNull(); + // No arrow buttons + expect(screen.queryByRole("button", { name: /move up:/i })).toBeNull(); + expect(screen.queryByRole("button", { name: /move down:/i })).toBeNull(); + }); + + it("does not show RepoLockControls or group headers in custom mode", () => { + mockJiraFilters = customFilters(); + const issues = [makeIssue("ALPHA-1", "ALPHA")]; + render(() => ); + + expect(screen.queryByRole("button", { name: /pin alpha to top of list/i })).toBeNull(); + }); + + it("auto-expands all project groups on first entry to a grouped sort", () => { + mockJiraFilters = { scope: "assigned", statusCategory: "all", priority: "all", sortField: "priority", sortDirection: "asc" }; + const issues = [ + makeIssue("ALPHA-1", "ALPHA"), + makeIssue("BETA-1", "BETA"), + ]; + render(() => ); + + expect(vi.mocked(setAllExpanded)).toHaveBeenCalledWith("jiraAssigned", ["ALPHA", "BETA"], true); + }); + + it("clears itemRefs when the sortField changes away from custom while still mounted (live mode switch)", () => { + mockJiraFilters = customFilters(); + const issues = [makeIssue("PROJ-1"), makeIssue("PROJ-2")]; + render(() => ); + + // Rows are mounted in custom mode, so their ref callbacks have populated + // the module-level itemRefs map. + expect(_getItemRefsCount()).toBeGreaterThan(0); + + // Switch to a grouped sort WITHOUT unmounting — exercises the itemRefs + // pruning effect's early-return branch (itemRefs.clear() instead of a + // bare `return`), not the separate onCleanup-on-unmount path. + bumpMockJiraFilters({ scope: "assigned", statusCategory: "all", priority: "all", sortField: "priority", sortDirection: "asc" }); + + expect(_getItemRefsCount()).toBe(0); + }); + }); + + describe("custom order — arrow button interactions", () => { + it("clicking move-down swaps a row with the next and persists the full order via setJiraCustomOrder", () => { + mockJiraFilters = customFilters(); + const issues = [makeIssue("PROJ-1"), makeIssue("PROJ-2")]; + render(() => ); + + const downBtn = screen.getByRole("button", { name: "Move down: PROJ-1" }); + downBtn.click(); + + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledTimes(1); + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledWith(["PROJ-2", "PROJ-1"]); + }); + + it("clicking move-up swaps a row with the previous and persists the full order via setJiraCustomOrder", () => { + mockJiraFilters = customFilters(); + const issues = [makeIssue("PROJ-1"), makeIssue("PROJ-2")]; + render(() => ); + + const upBtn = screen.getByRole("button", { name: "Move up: PROJ-2" }); + upBtn.click(); + + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledTimes(1); + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledWith(["PROJ-2", "PROJ-1"]); + }); + + it("disables the up arrow on the true first item and does not disable the down arrow merely for being the current page's last item", () => { + mockJiraFilters = customFilters(); + const issues = Array.from({ length: 27 }, (_, i) => makeIssue(`PROJ-${i + 1}`)); + render(() => ); + + const upFirst = screen.getByRole("button", { name: "Move up: PROJ-1" }) as HTMLButtonElement; + const downPageEdge = screen.getByRole("button", { name: "Move down: PROJ-25" }) as HTMLButtonElement; + expect(upFirst.disabled).toBe(true); + expect(downPageEdge.disabled).toBe(false); + }); + + it("disables the down arrow only on the true last item across pages, and enables the up arrow on the first item of a non-first page", () => { + mockJiraFilters = customFilters(); + const issues = Array.from({ length: 27 }, (_, i) => makeIssue(`PROJ-${i + 1}`)); + render(() => ); + + screen.getByRole("button", { name: /next page/i }).click(); + + const upPageStart = screen.getByRole("button", { name: "Move up: PROJ-26" }) as HTMLButtonElement; + const downLast = screen.getByRole("button", { name: "Move down: PROJ-27" }) as HTMLButtonElement; + expect(upPageStart.disabled).toBe(false); + expect(downLast.disabled).toBe(true); + }); + + it("disables arrow buttons when a status filter is active", () => { + mockJiraFilters = customFilters({ statusCategory: "new" }); + const issues = [makeIssue("PROJ-1", "PROJ", "new"), makeIssue("PROJ-2", "PROJ", "new")]; + render(() => ); + + const downBtn = screen.getByRole("button", { name: "Move down: PROJ-1" }) as HTMLButtonElement; + expect(downBtn.disabled).toBe(true); + expect(downBtn.getAttribute("title")).toBe("Switch to Assigned to me with no filters to reorder"); + }); + + it("disables arrow buttons when a priority filter is active", () => { + mockJiraFilters = customFilters({ priority: "High" }); + const issues = [makeIssue("PROJ-1", "PROJ", "indeterminate", "High"), makeIssue("PROJ-2", "PROJ", "indeterminate", "High")]; + render(() => ); + + const downBtn = screen.getByRole("button", { name: "Move down: PROJ-1" }) as HTMLButtonElement; + expect(downBtn.disabled).toBe(true); + }); + + it("disables arrow buttons when scope is not 'assigned'", () => { + mockJiraFilters = customFilters({ scope: "reported" }); + const issues = [makeIssue("PROJ-1"), makeIssue("PROJ-2")]; + render(() => ); + + const downBtn = screen.getByRole("button", { name: "Move down: PROJ-1" }) as HTMLButtonElement; + expect(downBtn.disabled).toBe(true); + }); + + it("does not call setJiraCustomOrder when clicking a disabled arrow button while a filter is active (no data loss)", () => { + mockJiraCustomOrder = ["PROJ-1", "PROJ-2"]; + mockJiraFilters = customFilters({ priority: "High" }); + const issues = [makeIssue("PROJ-1", "PROJ", "indeterminate", "High"), makeIssue("PROJ-2", "PROJ", "indeterminate", "Medium")]; + render(() => ); + + const downBtn = screen.getByRole("button", { name: "Move down: PROJ-1" }) as HTMLButtonElement; + expect(downBtn.disabled).toBe(true); + downBtn.click(); + + // The disabled attribute blocks the click entirely — setJiraCustomOrder is never + // called, so the full recorded order (including PROJ-2, which the active priority + // filter excludes from filteredSorted()) can never be silently truncated. + expect(vi.mocked(setJiraCustomOrder)).not.toHaveBeenCalled(); + }); + + it("disables both arrow buttons immediately after a click, before the animation/timeout settles", () => { + mockJiraFilters = customFilters(); + const issues = [makeIssue("PROJ-1"), makeIssue("PROJ-2"), makeIssue("PROJ-3")]; + render(() => ); + + const downBtn = screen.getByRole("button", { name: "Move down: PROJ-2" }) as HTMLButtonElement; + const upBtn = screen.getByRole("button", { name: "Move up: PROJ-2" }) as HTMLButtonElement; + expect(downBtn.disabled).toBe(false); + + downBtn.click(); + + expect(downBtn.disabled).toBe(true); + expect(upBtn.disabled).toBe(true); + }); + + it("clicking an arrow twice in immediate succession only applies the first move", () => { + mockJiraFilters = customFilters(); + const issues = [makeIssue("PROJ-1"), makeIssue("PROJ-2"), makeIssue("PROJ-3")]; + render(() => ); + + const downBtn = screen.getByRole("button", { name: "Move down: PROJ-1" }) as HTMLButtonElement; + downBtn.click(); + downBtn.click(); + + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledTimes(1); + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledWith(["PROJ-2", "PROJ-1", "PROJ-3"]); + }); + + it("renders a newly-appearing issue (not yet in jiraCustomOrder) at the bottom of the list with functional arrow buttons", () => { + mockJiraCustomOrder = ["PROJ-2", "PROJ-1"]; + mockJiraFilters = customFilters(); + const issues = [makeIssue("PROJ-1"), makeIssue("PROJ-2"), makeIssue("PROJ-3")]; + render(() => ); + + const rows = screen.getAllByRole("listitem"); + const keys = rows.map((el) => el.querySelector(".font-mono")?.textContent); + expect(keys).toEqual(["PROJ-2", "PROJ-1", "PROJ-3"]); + + const upBtn = screen.getByRole("button", { name: "Move up: PROJ-3" }) as HTMLButtonElement; + expect(upBtn.disabled).toBe(false); + upBtn.click(); + + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledWith(["PROJ-2", "PROJ-3", "PROJ-1"]); + }); + }); + + describe("custom order — FLIP animation / reduced motion", () => { + afterEach(() => { + delete (Element.prototype as unknown as { animate?: unknown }).animate; + vi.restoreAllMocks(); + }); + + it("invokes Element.prototype.animate for a same-page move", () => { + vi.spyOn(window, "matchMedia").mockReturnValue({ matches: false } as MediaQueryList); + let rectCallCount = 0; + vi.spyOn(Element.prototype, "getBoundingClientRect").mockImplementation(() => { + rectCallCount += 1; + return { top: rectCallCount * 10, left: 0, right: 0, bottom: 0, width: 0, height: 0, x: 0, y: 0, toJSON: () => ({}) } as DOMRect; + }); + vi.spyOn(window, "requestAnimationFrame").mockImplementation((cb) => { cb(0); return 0; }); + const animateSpy = vi.fn(); + Element.prototype.animate = animateSpy; + + mockJiraFilters = customFilters(); + const issues = [makeIssue("PROJ-1"), makeIssue("PROJ-2"), makeIssue("PROJ-3")]; + render(() => ); + + screen.getByRole("button", { name: "Move down: PROJ-1" }).click(); + + expect(animateSpy).toHaveBeenCalled(); + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledWith(["PROJ-2", "PROJ-1", "PROJ-3"]); + }); + + it("does not invoke Element.prototype.animate and advances the page for a move crossing forward into the next page", () => { + vi.spyOn(window, "matchMedia").mockReturnValue({ matches: false } as MediaQueryList); + const animateSpy = vi.fn(); + Element.prototype.animate = animateSpy; + + mockJiraFilters = customFilters(); + const issues = Array.from({ length: 27 }, (_, i) => makeIssue(`PROJ-${i + 1}`)); + render(() => ); + + screen.getByRole("button", { name: "Move down: PROJ-25" }).click(); + + expect(animateSpy).not.toHaveBeenCalled(); + expect(screen.getByText(/page 2 of 2/i)).toBeTruthy(); + const expected = issues.map((i) => i.key); + [expected[24], expected[25]] = [expected[25], expected[24]]; + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledWith(expected); + }); + + it("does not invoke Element.prototype.animate and returns to the previous page for a move crossing backward", () => { + vi.spyOn(window, "matchMedia").mockReturnValue({ matches: false } as MediaQueryList); + mockJiraFilters = customFilters(); + const issues = Array.from({ length: 27 }, (_, i) => makeIssue(`PROJ-${i + 1}`)); + render(() => ); + + screen.getByRole("button", { name: /next page/i }).click(); + expect(screen.getByText(/page 2 of 2/i)).toBeTruthy(); + + const animateSpy = vi.fn(); + Element.prototype.animate = animateSpy; + screen.getByRole("button", { name: "Move up: PROJ-26" }).click(); + + expect(animateSpy).not.toHaveBeenCalled(); + expect(screen.getByText(/page 1 of 2/i)).toBeTruthy(); + const expected = issues.map((i) => i.key); + [expected[24], expected[25]] = [expected[25], expected[24]]; + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledWith(expected); + }); + + it("with prefers-reduced-motion, calls window.scrollTo instead of animating for a same-page move", () => { + vi.spyOn(window, "matchMedia").mockReturnValue({ matches: true } as MediaQueryList); + vi.spyOn(window, "scrollTo").mockImplementation(() => {}); + const animateSpy = vi.fn(); + Element.prototype.animate = animateSpy; + + mockJiraFilters = customFilters(); + const issues = [makeIssue("PROJ-1"), makeIssue("PROJ-2")]; + render(() => ); + + screen.getByRole("button", { name: "Move down: PROJ-1" }).click(); + + expect(window.scrollTo).toHaveBeenCalled(); + expect(animateSpy).not.toHaveBeenCalled(); + expect(vi.mocked(setJiraCustomOrder)).toHaveBeenCalledWith(["PROJ-2", "PROJ-1"]); + }); + + it("with prefers-reduced-motion, calls window.scrollTo instead of animating for a cross-page move", () => { + vi.spyOn(window, "matchMedia").mockReturnValue({ matches: true } as MediaQueryList); + vi.spyOn(window, "scrollTo").mockImplementation(() => {}); + const animateSpy = vi.fn(); + Element.prototype.animate = animateSpy; + + mockJiraFilters = customFilters(); + const issues = Array.from({ length: 27 }, (_, i) => makeIssue(`PROJ-${i + 1}`)); + render(() => ); + + screen.getByRole("button", { name: "Move down: PROJ-25" }).click(); + + expect(window.scrollTo).toHaveBeenCalled(); + expect(animateSpy).not.toHaveBeenCalled(); + expect(screen.getByText(/page 2 of 2/i)).toBeTruthy(); + }); + + it("preserves DOM element identity (same node, not recreated) after a same-page reorder", () => { + // The mock store's setJiraCustomOrder is a no-op, so clicking an arrow + // button does not reorder the DOM. To test actual DOM identity we drive + // the reorder through the reactive prop layer: pass a signal-backed + // issues array, change mockJiraCustomOrder, and trigger a re-render by + // updating the signal (same JiraIssue objects, new array reference). + // The itemsWithGroupKeyCache inside the component should keep the same + // wrapped JiraItem objects for unchanged source issues, so 's keyed + // reconciliation moves the DOM nodes instead of tearing down / rebuilding. + mockJiraFilters = customFilters(); + mockJiraCustomOrder = ["PROJ-1", "PROJ-2", "PROJ-3"]; + + const issueA = makeIssue("PROJ-1"); + const issueB = makeIssue("PROJ-2"); + const issueC = makeIssue("PROJ-3"); + + const [issues, setIssues] = createSignal([issueA, issueB, issueC]); + render(() => ); + + // Capture the actual DOM node for PROJ-2's row before the reorder + const rowsBefore = screen.getAllByRole("listitem"); + expect(rowsBefore[1].querySelector(".font-mono")?.textContent).toBe("PROJ-2"); + const proj2NodeBefore = rowsBefore[1]; + + // Swap PROJ-1 and PROJ-2 in the custom order, then poke the signal to + // trigger a reactive re-evaluation of filtered → filteredSorted → + // itemsWithGroupKey → customPageItems → . + mockJiraCustomOrder = ["PROJ-2", "PROJ-1", "PROJ-3"]; + setIssues([issueA, issueB, issueC]); + + // After the reactive update, PROJ-2 should be at index 0. The critical + // assertion: the DOM node is the SAME object reference — moved by , + // not torn down and rebuilt. + const rowsAfter = screen.getAllByRole("listitem"); + expect(rowsAfter[0].querySelector(".font-mono")?.textContent).toBe("PROJ-2"); + expect(rowsAfter[0]).toBe(proj2NodeBefore); + }); + }); + + describe("custom order — Clear button / reset button interplay", () => { + it("clicking Clear while sorted by Priority with a filter active only clears status/priority filters, not sortField or scope", () => { + mockJiraFilters = { scope: "assigned", statusCategory: "all", priority: "High", sortField: "priority", sortDirection: "asc" }; + const issues = [makeIssue("PROJ-1", "PROJ", "indeterminate", "High")]; + render(() => ); + + screen.getByRole("button", { name: /clear/i }).click(); + + expect(vi.mocked(setTabFilter)).toHaveBeenCalledWith("jiraAssigned", "statusCategory", "all"); + expect(vi.mocked(setTabFilter)).toHaveBeenCalledWith("jiraAssigned", "priority", "all"); + const fieldsChanged = vi.mocked(setTabFilter).mock.calls.map((call) => call[1]); + expect(fieldsChanged).not.toContain("sortField"); + expect(fieldsChanged).not.toContain("scope"); + }); + + it("shows the '↺ Custom order' reset button when sortField is not custom, and hides it in custom mode", () => { + mockJiraFilters = { scope: "assigned", statusCategory: "all", priority: "all", sortField: "priority", sortDirection: "asc" }; + const issues = [makeIssue("PROJ-1")]; + const { unmount } = render(() => ); + // Matched by its exact "↺ Custom order" text, which disambiguates it from + // the SortDropdown trigger — in custom mode the trigger's own accessible + // name also contains "Custom order" (via its placeholder prop), so a bare + // /custom order/i regex would match both buttons. + expect(screen.getByRole("button", { name: /↺\s*custom order/i })).toBeTruthy(); + unmount(); + + mockJiraFilters = customFilters(); + render(() => ); + expect(screen.queryByRole("button", { name: /↺\s*custom order/i })).toBeNull(); + }); + + it("clicking the reset button switches sortField to custom and resets the page to 0", () => { + mockJiraFilters = { scope: "assigned", statusCategory: "all", priority: "all", sortField: "priority", sortDirection: "asc" }; + const issues = [ + ...Array.from({ length: 15 }, (_, i) => makeIssue(`ALPHA-${i + 1}`, "ALPHA")), + ...Array.from({ length: 15 }, (_, i) => makeIssue(`BETA-${i + 1}`, "BETA")), + ]; + render(() => ); + + screen.getByRole("button", { name: /next page/i }).click(); + expect(screen.getByText(/page 2 of 2/i)).toBeTruthy(); + + screen.getByRole("button", { name: /custom order/i }).click(); + + expect(vi.mocked(setTabFilter)).toHaveBeenCalledWith("jiraAssigned", "sortField", "custom"); + expect(screen.getByText(/page 1 of 2/i)).toBeTruthy(); + }); + }); + + describe("custom order — cleanup on unmount", () => { + afterEach(() => { + vi.useRealTimers(); + }); + + it("clears the pending reorder-lockout timeout on unmount so it cannot fire after disposal", () => { + vi.useFakeTimers(); + const clearTimeoutSpy = vi.spyOn(globalThis, "clearTimeout"); + + mockJiraFilters = customFilters(); + const issues = [makeIssue("PROJ-1"), makeIssue("PROJ-2")]; + const { unmount } = render(() => ); + + screen.getByRole("button", { name: "Move down: PROJ-1" }).click(); + // A 200ms reorder-lockout timeout is now pending. + clearTimeoutSpy.mockClear(); + unmount(); + + // onCleanup must clear the pending timeout so it cannot fire setReordering + // on a disposed component after a tab switch mid-lockout. + expect(clearTimeoutSpy).toHaveBeenCalled(); + }); + }); }); diff --git a/tests/components/shared/SortDropdown.test.tsx b/tests/components/shared/SortDropdown.test.tsx index 65620261..4b598af2 100644 --- a/tests/components/shared/SortDropdown.test.tsx +++ b/tests/components/shared/SortDropdown.test.tsx @@ -139,4 +139,44 @@ describe("SortDropdown", () => { expect(opts.some((t) => t.includes("(most)"))).toBe(true); expect(opts.some((t) => t.includes("(fewest)"))).toBe(true); }); + + it("renders default 'Sort by' placeholder text when no placeholder prop is given and value doesn't match any option", () => { + render(() => ( + + )); + expect(screen.getByText("Sort by")).toBeTruthy(); + }); + + it("renders custom placeholder text when placeholder prop is given and value doesn't match any option", () => { + render(() => ( + + )); + expect(screen.getByText("Custom order")).toBeTruthy(); + expect(screen.queryByText("Sort by")).toBeNull(); + }); + + it("renders the selected option's label instead of the placeholder when value matches a real option", () => { + render(() => ( + + )); + expect(screen.getByText("Title (A-Z)")).toBeTruthy(); + expect(screen.queryByText("Custom order")).toBeNull(); + }); }); diff --git a/tests/lib/grouping.test.ts b/tests/lib/grouping.test.ts index e0b01ecb..5f84bd05 100644 --- a/tests/lib/grouping.test.ts +++ b/tests/lib/grouping.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "vitest"; -import { groupByRepo, computePageLayout, slicePageGroups, isUserInvolved, ensureLockedRepoGroups, type RepoGroup } from "../../src/app/lib/grouping"; +import { groupByRepo, computePageLayout, slicePageGroups, isUserInvolved, ensureLockedRepoGroups, applyCustomOrder, type RepoGroup } from "../../src/app/lib/grouping"; interface Item { repoFullName: string; @@ -287,3 +287,55 @@ describe("ensureLockedRepoGroups", () => { expect(result[1].workflows).toEqual([]); }); }); + +describe("applyCustomOrder", () => { + const keyFn = (item: Item) => String(item.id); + + it("returns items in the exact order when order fully matches", () => { + const items = [makeItem("org/a", 1), makeItem("org/a", 2), makeItem("org/a", 3)]; + const result = applyCustomOrder(items, ["3", "1", "2"], keyFn); + expect(result.map((i) => i.id)).toEqual([3, 1, 2]); + }); + + it("appends unlisted items at the end in original relative order", () => { + const items = [makeItem("org/a", 1), makeItem("org/a", 2), makeItem("org/a", 3), makeItem("org/a", 4)]; + const result = applyCustomOrder(items, ["3", "1"], keyFn); + expect(result.map((i) => i.id)).toEqual([3, 1, 2, 4]); + }); + + it("returns items unchanged by reference when order is empty", () => { + const items = [makeItem("org/a", 1), makeItem("org/a", 2)]; + const result = applyCustomOrder(items, [], keyFn); + expect(result).toBe(items); // same reference — no copy + }); + + it("silently ignores order keys not present in items", () => { + const items = [makeItem("org/a", 1), makeItem("org/a", 2)]; + const result = applyCustomOrder(items, ["1", "99"], keyFn); + expect(result.map((i) => i.id)).toEqual([1, 2]); + expect(result).toHaveLength(2); // no phantom entry for the unmatched "99" key + }); + + it("keeps only the later item when two items share the same key", () => { + const first = makeItem("org/a", 1); + const second = makeItem("org/b", 1); // same keyFn() result ("1") as `first` + // Route both through the "unreferenced" append path so the assertion + // exercises dedup at map-build time, independent of the `order` walk. + const result = applyCustomOrder([first, second], ["nonexistent"], keyFn); + expect(result).toHaveLength(1); + expect(result[0].repoFullName).toBe("org/b"); + }); + + it("documents duplicate-entry behavior when a key appears twice in order", () => { + // applyCustomOrder does not deduplicate `order` itself — a repeated key + // pulls the same item into the result once per occurrence, per the + // function's own doc comment ("order is walked as-is and is not itself + // deduplicated"). This test pins that behavior rather than leaving it + // unspecified. + const item = makeItem("org/a", 1); + const result = applyCustomOrder([item], ["1", "1"], keyFn); + expect(result).toHaveLength(2); + expect(result[0]).toBe(item); + expect(result[1]).toBe(item); + }); +}); diff --git a/tests/stores/view-jira-order.test.ts b/tests/stores/view-jira-order.test.ts new file mode 100644 index 00000000..c0e48736 --- /dev/null +++ b/tests/stores/view-jira-order.test.ts @@ -0,0 +1,279 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { createRoot } from "solid-js"; +import { + viewState, + resetViewState, + setJiraCustomOrder, + pruneJiraCustomOrder, + initViewPersistence, + ViewStateSchema, + VIEW_STORAGE_KEY, + JIRA_CUSTOM_ORDER_CAP, + JIRA_CUSTOM_ORDER_KEY_MAX_LENGTH, +} from "../../src/app/stores/view"; + +describe("jira custom order store actions", () => { + beforeEach(() => { + resetViewState(); + }); + + describe("setJiraCustomOrder", () => { + it("replaces the array wholesale", () => { + setJiraCustomOrder(["PROJ-1", "PROJ-2", "PROJ-3"]); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1", "PROJ-2", "PROJ-3"]); + }); + + it("overwrites a previous order", () => { + setJiraCustomOrder(["PROJ-1", "PROJ-2"]); + setJiraCustomOrder(["PROJ-3", "PROJ-1"]); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-3", "PROJ-1"]); + }); + + it("truncates to JIRA_CUSTOM_ORDER_CAP when longer", () => { + const oversized = Array.from({ length: JIRA_CUSTOM_ORDER_CAP + 50 }, (_, i) => `KEY-${i}`); + setJiraCustomOrder(oversized); + expect(viewState.jiraCustomOrder.length).toBe(JIRA_CUSTOM_ORDER_CAP); + expect(viewState.jiraCustomOrder[0]).toBe("KEY-0"); + expect(viewState.jiraCustomOrder[JIRA_CUSTOM_ORDER_CAP - 1]).toBe(`KEY-${JIRA_CUSTOM_ORDER_CAP - 1}`); + }); + + it("accepts an empty array", () => { + setJiraCustomOrder(["PROJ-1"]); + setJiraCustomOrder([]); + expect(viewState.jiraCustomOrder).toEqual([]); + }); + + it("does not truncate when exactly at cap", () => { + const exact = Array.from({ length: JIRA_CUSTOM_ORDER_CAP }, (_, i) => `KEY-${i}`); + setJiraCustomOrder(exact); + expect(viewState.jiraCustomOrder.length).toBe(JIRA_CUSTOM_ORDER_CAP); + }); + + it("deduplicates keys, keeping first occurrence and relative order", () => { + setJiraCustomOrder(["A", "B", "A", "C"]); + expect(viewState.jiraCustomOrder).toEqual(["A", "B", "C"]); + }); + + it("drops entries exceeding per-item max length", () => { + const tooLong = "K".repeat(JIRA_CUSTOM_ORDER_KEY_MAX_LENGTH + 1); + const atLimit = "K".repeat(JIRA_CUSTOM_ORDER_KEY_MAX_LENGTH); + setJiraCustomOrder(["PROJ-1", tooLong, atLimit, "PROJ-2"]); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1", atLimit, "PROJ-2"]); + }); + + it("drops non-string entries instead of throwing when given malformed input", () => { + // order is typed string[], but the only production caller builds it from + // live, unvalidated Jira API data — guard against a non-string slipping + // through at runtime despite the type signature. + const malformed = ["PROJ-1", 42, null, undefined, "PROJ-2"] as unknown as string[]; + expect(() => setJiraCustomOrder(malformed)).not.toThrow(); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1", "PROJ-2"]); + }); + }); + + describe("pruneJiraCustomOrder", () => { + it("drops stale keys and preserves order of active ones", () => { + setJiraCustomOrder(["PROJ-1", "PROJ-2", "PROJ-3", "PROJ-4"]); + pruneJiraCustomOrder(new Set(["PROJ-1", "PROJ-3"])); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1", "PROJ-3"]); + }); + + it("is a no-op when the order is empty", () => { + pruneJiraCustomOrder(new Set(["PROJ-1"])); + expect(viewState.jiraCustomOrder).toEqual([]); + }); + + it("is a no-op when all keys are active", () => { + setJiraCustomOrder(["PROJ-1", "PROJ-2"]); + pruneJiraCustomOrder(new Set(["PROJ-1", "PROJ-2", "PROJ-3"])); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1", "PROJ-2"]); + }); + + it("clears the order when no keys are active", () => { + setJiraCustomOrder(["PROJ-1", "PROJ-2"]); + pruneJiraCustomOrder(new Set(["PROJ-99"])); + expect(viewState.jiraCustomOrder).toEqual([]); + }); + }); + + describe("resetViewState", () => { + it("clears jiraCustomOrder", () => { + setJiraCustomOrder(["PROJ-1", "PROJ-2"]); + resetViewState(); + expect(viewState.jiraCustomOrder).toEqual([]); + }); + + it("resets sortField to 'custom'", () => { + resetViewState(); + expect(viewState.tabFilters.jiraAssigned.sortField).toBe("custom"); + }); + }); + + describe("schema defaults", () => { + it("defaults jiraAssigned.sortField to 'custom' on fresh parse", () => { + const result = ViewStateSchema.parse({}); + expect(result.tabFilters.jiraAssigned.sortField).toBe("custom"); + }); + + it("defaults jiraCustomOrder to empty array on fresh parse", () => { + const result = ViewStateSchema.parse({}); + expect(result.jiraCustomOrder).toEqual([]); + }); + + it("preserves existing sortField from persisted state", () => { + const result = ViewStateSchema.parse({ + tabFilters: { + jiraAssigned: { sortField: "priority", sortDirection: "asc", scope: "assigned", statusCategory: "all", priority: "all" }, + }, + }); + expect(result.tabFilters.jiraAssigned.sortField).toBe("priority"); + }); + + it("deduplicates jiraCustomOrder at schema level during parse", () => { + const result = ViewStateSchema.parse({ + jiraCustomOrder: ["A", "B", "A", "C", "B"], + }); + expect(result.jiraCustomOrder).toEqual(["A", "B", "C"]); + }); + + it("deduplicates jiraCustomOrder at schema level during safeParse", () => { + const result = ViewStateSchema.safeParse({ + jiraCustomOrder: ["X", "Y", "X"], + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.jiraCustomOrder).toEqual(["X", "Y"]); + } + }); + + it("schema dedup shrinks array — cannot violate max cap", () => { + // An array at exactly the cap with all duplicates should parse fine and dedup + const atCap = Array.from({ length: JIRA_CUSTOM_ORDER_CAP }, () => "SAME-KEY"); + const result = ViewStateSchema.safeParse({ jiraCustomOrder: atCap }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.jiraCustomOrder).toEqual(["SAME-KEY"]); + } + }); + + it("schema pick + safeParse deduplicates for cross-tab sync path", () => { + const result = ViewStateSchema.pick({ jiraCustomOrder: true }).safeParse({ + jiraCustomOrder: ["P-1", "P-2", "P-1", "P-3"], + }); + expect(result.success).toBe(true); + if (result.success) { + expect(result.data.jiraCustomOrder).toEqual(["P-1", "P-2", "P-3"]); + } + }); + }); +}); + +describe("cross-tab sync for jiraCustomOrder", () => { + let dispose: (() => void) | undefined; + + beforeEach(() => { + resetViewState(); + }); + + afterEach(() => { + dispose?.(); + dispose = undefined; + }); + + function setupPersistence() { + createRoot((d) => { + dispose = d; + initViewPersistence(); + }); + } + + function dispatchStorageEvent(newValue: string | null) { + window.dispatchEvent(new StorageEvent("storage", { + key: VIEW_STORAGE_KEY, + newValue, + })); + } + + it("updates jiraCustomOrder from another tab's storage write", () => { + setupPersistence(); + const blob = JSON.stringify({ jiraCustomOrder: ["PROJ-A", "PROJ-B"] }); + dispatchStorageEvent(blob); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-A", "PROJ-B"]); + }); + + it("ignores storage events for unrelated keys", () => { + setupPersistence(); + setJiraCustomOrder(["PROJ-1"]); + window.dispatchEvent(new StorageEvent("storage", { + key: "some-other-key", + newValue: JSON.stringify({ jiraCustomOrder: ["PROJ-X"] }), + })); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1"]); + }); + + it("rejects incoming blob exceeding JIRA_CUSTOM_ORDER_CAP (safeParse fails)", () => { + setupPersistence(); + setJiraCustomOrder(["PROJ-1"]); + const oversized = Array.from({ length: JIRA_CUSTOM_ORDER_CAP + 100 }, (_, i) => `KEY-${i}`); + dispatchStorageEvent(JSON.stringify({ jiraCustomOrder: oversized })); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1"]); + }); + + it("does not touch other viewState fields when syncing jiraCustomOrder", () => { + setupPersistence(); + const blob = JSON.stringify({ + lastActiveTab: "actions", + jiraCustomOrder: ["PROJ-Z"], + }); + dispatchStorageEvent(blob); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-Z"]); + expect(viewState.lastActiveTab).toBe("issues"); + }); + + it("handles malformed JSON in newValue without throwing", () => { + setupPersistence(); + setJiraCustomOrder(["PROJ-1"]); + expect(() => dispatchStorageEvent("not valid json")).not.toThrow(); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1"]); + }); + + it("ignores null newValue", () => { + setupPersistence(); + setJiraCustomOrder(["PROJ-1"]); + dispatchStorageEvent(null); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1"]); + }); + + it("is a no-op when incoming order matches current (dedup guard)", () => { + setupPersistence(); + setJiraCustomOrder(["PROJ-1", "PROJ-2"]); + const blob = JSON.stringify({ jiraCustomOrder: ["PROJ-1", "PROJ-2"] }); + dispatchStorageEvent(blob); + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1", "PROJ-2"]); + }); + + it("removes listener after dispose so subsequent events have no effect", () => { + setupPersistence(); + dispose?.(); + dispose = undefined; + dispatchStorageEvent(JSON.stringify({ jiraCustomOrder: ["PROJ-NEW"] })); + expect(viewState.jiraCustomOrder).toEqual([]); + }); + + it("does not wipe jiraCustomOrder when incoming blob is missing the key entirely", () => { + setupPersistence(); + setJiraCustomOrder(["PROJ-1", "PROJ-2", "PROJ-3"]); + // Simulate a stale blob from a pre-feature tab that doesn't have jiraCustomOrder + const staleBlob = JSON.stringify({ lastActiveTab: "issues" }); + dispatchStorageEvent(staleBlob); + // Current order must be preserved — NOT wiped to [] + expect(viewState.jiraCustomOrder).toEqual(["PROJ-1", "PROJ-2", "PROJ-3"]); + }); + + it("deduplicates incoming jiraCustomOrder from cross-tab sync", () => { + setupPersistence(); + const blob = JSON.stringify({ jiraCustomOrder: ["A", "B", "A", "C"] }); + dispatchStorageEvent(blob); + expect(viewState.jiraCustomOrder).toEqual(["A", "B", "C"]); + }); +}); diff --git a/tests/stores/view.test.ts b/tests/stores/view.test.ts index d679d01b..092b60c9 100644 --- a/tests/stores/view.test.ts +++ b/tests/stores/view.test.ts @@ -27,6 +27,7 @@ import { lockRepo, untrackJiraItem, moveJiraItem, + setJiraCustomOrder, } from "../../src/app/stores/view"; import type { IgnoredItem, TrackedItem, ViewState } from "../../src/app/stores/view"; import { getNotifications, clearNotifications } from "../../src/app/lib/errors"; @@ -271,26 +272,399 @@ describe("pruneStaleIgnoredItems", () => { }); describe("initViewPersistence", () => { - it("persists state changes to localStorage via createEffect", async () => { + // Shared harness: every test in this block needs fake timers, a createRoot + // to host initViewPersistence()'s effect, and real-timer restoration + // afterward. `fn` receives `dispose` so tests can end the root whenever + // their scenario requires (immediately, to test flush-on-disposal; or at + // the end, after asserting the debounced write). `dispose()` is called + // unconditionally in `finally` (idempotent in Solid — a no-op if the test + // already called it) so a thrown assertion mid-test can't leave this + // root's createEffect/storage-listener live against the shared, module-level + // `viewState`, where it would keep firing (with real timers already + // restored) against later, unrelated tests. + async function withViewPersistence(fn: (dispose: () => void) => Promise | void): Promise { vi.useFakeTimers(); let dispose!: () => void; createRoot((d) => { dispose = d; initViewPersistence(); + }); + try { + await fn(dispose); + } finally { + dispose(); + vi.useRealTimers(); + } + } + + // Reads whatever's currently on disk, or falls back to the live in-memory + // viewState on the very first write of a test. + function currentOnDisk(): Record { + return JSON.parse(localStorageMock.getItem(VIEW_KEY) ?? JSON.stringify(viewState)); + } + + // Simulates another tab writing directly to localStorage — merges `overrides` + // onto whatever's already on disk. + function seedOtherTabWrite(overrides: Record): void { + localStorageMock.setItem(VIEW_KEY, JSON.stringify({ ...currentOnDisk(), ...overrides })); + } + + it("persists state changes to localStorage via createEffect", async () => { + await withViewPersistence(async (dispose) => { setGlobalFilter("testorg", "testrepo"); + // SolidJS effects are scheduled as microtasks — flush with a tick + await Promise.resolve(); + // Persistence is debounced by 200ms + vi.advanceTimersByTime(200); + + const raw = localStorageMock.getItem(VIEW_KEY); + expect(raw).not.toBeNull(); + const parsed = JSON.parse(raw!); + expect(parsed.globalFilter.org).toBe("testorg"); + expect(parsed.globalFilter.repo).toBe("testrepo"); + dispose(); + }); + }); + + it("coalesces two separate changes within the debounce window into a single write of the latest state", async () => { + // Regression test: the debounce effect's flush-on-cleanup must be + // registered on the OUTER owner, not nested inside createEffect. Solid + // invokes onCleanup on both disposal AND recomputation — if nested + // inside the effect, a second change arriving before the first change's + // 200ms timer fires would flush the FIRST (stale) snapshot immediately + // instead of coalescing into one trailing write of the latest state. + await withViewPersistence(async (dispose) => { + const setItemSpy = vi.spyOn(localStorageMock, "setItem"); + + setGlobalFilter("org1", "repo1"); + await Promise.resolve(); + vi.advanceTimersByTime(50); // well within the 200ms window + + setGlobalFilter("org2", "repo2"); + await Promise.resolve(); + // No write should have happened yet from either change. + expect(setItemSpy).not.toHaveBeenCalled(); + + vi.advanceTimersByTime(200); + + // Exactly one write, reflecting the latest (second) state. + expect(setItemSpy).toHaveBeenCalledTimes(1); + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.globalFilter.org).toBe("org2"); + expect(parsed.globalFilter.repo).toBe("repo2"); + + setItemSpy.mockRestore(); + dispose(); + }); + }); + + it("preserves another tab's concurrent write to a field this tab didn't touch (merge-on-write)", async () => { + await withViewPersistence(async (dispose) => { + // Simulate another tab writing directly to localStorage — a full + // ViewState-shaped blob differing only in jiraCustomOrder, a field this + // tab has never touched (still at its boot-time baseline of []). + seedOtherTabWrite({ jiraCustomOrder: ["OTHER-1"] }); + + // This tab changes an unrelated field, triggering its own debounced write. + setGlobalFilter("org1", "repo1"); + await Promise.resolve(); + vi.advanceTimersByTime(200); + + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.globalFilter.org).toBe("org1"); + // The other tab's jiraCustomOrder write must survive — this tab never + // touched that field, so its own stale in-memory copy (still []) must + // NOT clobber what's already on disk. + expect(parsed.jiraCustomOrder).toEqual(["OTHER-1"]); + + dispose(); + }); + }); + + it("this tab's own concurrent change to a field wins over a divergent on-disk value for that same field", async () => { + await withViewPersistence(async (dispose) => { + seedOtherTabWrite({ jiraCustomOrder: ["OTHER-1"] }); + + // This tab ALSO changes jiraCustomOrder itself — its own value should win + // over whatever the other tab wrote, since this tab's change is more recent. + setJiraCustomOrder(["MINE-1", "MINE-2"]); + await Promise.resolve(); + vi.advanceTimersByTime(200); + + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.jiraCustomOrder).toEqual(["MINE-1", "MINE-2"]); + + dispose(); + }); + }); + + it("drops unknown/unrecognized keys from the on-disk blob before merging (schema allowlist)", async () => { + // Regression test: localStorage is writable by any same-origin script + // (extensions, a stale/incompatible schema version, manual tampering). + // readOnDiskState() must filter the parsed blob down to known + // ViewStateSchema top-level keys before commitSnapshot() folds it into + // the merged write — otherwise unrecognized content would be perpetually + // re-persisted instead of self-healing on the next write. + await withViewPersistence(async (dispose) => { + const onDiskBeforeTamper = JSON.parse(localStorageMock.getItem(VIEW_KEY) ?? JSON.stringify(viewState)); + const taintedJson = JSON.stringify({ ...onDiskBeforeTamper, maliciousInjectedField: "should not survive" }); + // Splice in a literal "__proto__" key at the JSON-text level — `{...x, __proto__: y}` + // as an object-literal would set the object's actual prototype (a language special + // case) rather than produce JSON text containing a "__proto__" key. JSON.parse, by + // contrast, does NOT apply that special-case magic: parsing `{"__proto__":{...}}` + // creates a plain OWN property literally named "__proto__" — which is the actual + // shape a tampered localStorage blob would take, and what readOnDiskState() must + // filter out via the schema allowlist rather than relying on spread's inertness alone. + const tainted = taintedJson.slice(0, -1) + ',"__proto__":{"polluted":true}}'; + localStorageMock.setItem(VIEW_KEY, tainted); + + setGlobalFilter("org1", "repo1"); + await Promise.resolve(); + vi.advanceTimersByTime(200); + + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.globalFilter.org).toBe("org1"); + expect(parsed.maliciousInjectedField).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(parsed, "__proto__")).toBe(false); + expect(Object.getPrototypeOf({})).not.toHaveProperty("polluted"); + + dispose(); + }); + }); + + it("drops a known key's value from the on-disk blob when it fails that field's own schema validation", async () => { + // Regression test: the allowlist in readOnDiskState() must validate each + // known key's VALUE against ViewStateSchema (not just its NAME) before + // folding it into the merge — a structurally-valid-key-but-malformed-value + // (e.g. jiraCustomOrder holding an object instead of a string array) + // would otherwise survive the {...onDisk} spread untouched and be + // re-persisted forever, never passing through Zod validation on write. + await withViewPersistence(async (dispose) => { + const onDiskBeforeTamper = JSON.parse(localStorageMock.getItem(VIEW_KEY) ?? JSON.stringify(viewState)); + localStorageMock.setItem(VIEW_KEY, JSON.stringify({ + ...onDiskBeforeTamper, + jiraCustomOrder: { foo: "bar" }, // wrong shape: object instead of string[] + })); + + setGlobalFilter("org1", "repo1"); + await Promise.resolve(); + vi.advanceTimersByTime(200); + + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.globalFilter.org).toBe("org1"); + // The malformed jiraCustomOrder must be dropped, not echoed back — this + // tab's own (valid, empty) value fills the gap instead. + expect(parsed.jiraCustomOrder).toEqual([]); + + dispose(); + }); + }); + + it("preserves this tab's known value for a key missing entirely from a corrupted/stale on-disk blob", async () => { + await withViewPersistence(async (dispose) => { + setJiraCustomOrder(["MINE-1"]); + await Promise.resolve(); + vi.advanceTimersByTime(200); + expect(JSON.parse(localStorageMock.getItem(VIEW_KEY)!).jiraCustomOrder).toEqual(["MINE-1"]); + + // Simulate a stale/corrupted on-disk blob missing jiraCustomOrder entirely + // (version skew, manual tampering) — this tab's own last-known value for + // that key must survive even though it hasn't changed since baseline. + const onDisk = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + delete onDisk.jiraCustomOrder; + localStorageMock.setItem(VIEW_KEY, JSON.stringify(onDisk)); + + setGlobalFilter("org1", "repo1"); + await Promise.resolve(); + vi.advanceTimersByTime(200); + + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.globalFilter.org).toBe("org1"); + expect(parsed.jiraCustomOrder).toEqual(["MINE-1"]); + + dispose(); + }); + }); + + it("falls back to this tab's own full snapshot when the on-disk blob is malformed JSON", async () => { + await withViewPersistence(async (dispose) => { + localStorageMock.setItem(VIEW_KEY, "{not valid json"); + + expect(() => { + setGlobalFilter("org1", "repo1"); + }).not.toThrow(); + await Promise.resolve(); + expect(() => vi.advanceTimersByTime(200)).not.toThrow(); + + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.globalFilter.org).toBe("org1"); + // Must be the FULL snapshot, not just the one field this test changed — + // an untouched default field must also be present, ruling out a + // regression that wrote only a partial object on this fallback path. + expect(parsed.lastActiveTab).toBe("issues"); + expect(parsed.jiraCustomOrder).toEqual([]); + + dispose(); + }); + }); + + it("falls back to this tab's own full snapshot when the on-disk blob is a JSON array or primitive", async () => { + await withViewPersistence(async (dispose) => { + localStorageMock.setItem(VIEW_KEY, "[1,2,3]"); + + setGlobalFilter("org1", "repo1"); + await Promise.resolve(); + vi.advanceTimersByTime(200); + + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.globalFilter.org).toBe("org1"); + expect(parsed.lastActiveTab).toBe("issues"); + + dispose(); + }); + }); + + it("pushes a warning notification and does not throw when localStorage.setItem fails (e.g. quota exceeded)", async () => { + await withViewPersistence(async (dispose) => { + clearNotifications(); + vi.spyOn(localStorageMock, "setItem").mockImplementation(() => { + throw new Error("QuotaExceededError"); + }); + + setGlobalFilter("org1", "repo1"); + await Promise.resolve(); + expect(() => vi.advanceTimersByTime(200)).not.toThrow(); + + const notifications = getNotifications(); + expect(notifications.some((n) => n.source === "localStorage:view")).toBe(true); + + vi.mocked(localStorageMock.setItem).mockRestore(); + dispose(); + }); + }); + + it("flushes a pending debounced write synchronously on disposal (unmount/HMR)", async () => { + await withViewPersistence(async (dispose) => { + setGlobalFilter("unmount-org", "unmount-repo"); + await Promise.resolve(); + // Dispose before the 200ms debounce timer ever fires. + dispose(); + + const raw = localStorageMock.getItem(VIEW_KEY); + expect(raw).not.toBeNull(); + const parsed = JSON.parse(raw!); + expect(parsed.globalFilter.org).toBe("unmount-org"); + expect(parsed.globalFilter.repo).toBe("unmount-repo"); }); + }); +}); - // SolidJS effects are scheduled as microtasks — flush with a tick +describe("initViewPersistence — genuine two-tab concurrency", () => { + // Unlike the "preserves another tab's concurrent write" tests above (which + // simulate "tab B" via a single synchronous localStorage.setItem call + // representing an already-completed write), this describe block spins up + // TWO fully independent module instances of stores/view.ts via + // vi.resetModules() + fresh dynamic imports — each with its own in-memory + // viewState, its own commitSnapshot()/lastSyncedSnapshot closure, and its + // own live createEffect/debounce timer — sharing only the same + // localStorageMock (the one durable channel two real browser tabs would + // actually share). This exercises the hardest case for the merge-on-write + // algorithm: two tabs changing the SAME field around the same time. + beforeEach(() => { + localStorageMock.clear(); + }); + + async function importFreshViewModule() { + vi.resetModules(); + return import("../../src/app/stores/view"); + } + + it("whichever tab's commitSnapshot() actually runs last wins when both change the same field at the same tick", async () => { + vi.useFakeTimers(); + const tabA = await importFreshViewModule(); + const tabB = await importFreshViewModule(); + + let disposeA!: () => void; + let disposeB!: () => void; + createRoot((d) => { disposeA = d; tabA.initViewPersistence(); }); + createRoot((d) => { disposeB = d; tabB.initViewPersistence(); }); + + // A changes first, B changes second, both at essentially the same fake-timer + // tick — both 200ms timers are scheduled for the identical target time, so + // JS timer FIFO ordering (registration order) fires A's callback before B's. + tabA.setJiraCustomOrder(["FROM-A"]); await Promise.resolve(); - // Persistence is debounced by 200ms + tabB.setJiraCustomOrder(["FROM-B"]); + await Promise.resolve(); + vi.advanceTimersByTime(200); - const raw = localStorageMock.getItem(VIEW_KEY); - expect(raw).not.toBeNull(); - const parsed = JSON.parse(raw!); - expect(parsed.globalFilter.org).toBe("testorg"); - expect(parsed.globalFilter.repo).toBe("testrepo"); - dispose(); + // B's commit runs strictly after A's (registered later, same target time), + // so B's write is the one that lands last on disk. + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.jiraCustomOrder).toEqual(["FROM-B"]); + + disposeA(); + disposeB(); + vi.useRealTimers(); + }); + + it("reversing which tab changes the field last reverses which value wins", async () => { + vi.useFakeTimers(); + const tabA = await importFreshViewModule(); + const tabB = await importFreshViewModule(); + + let disposeA!: () => void; + let disposeB!: () => void; + createRoot((d) => { disposeA = d; tabA.initViewPersistence(); }); + createRoot((d) => { disposeB = d; tabB.initViewPersistence(); }); + + // B changes first this time, A changes second — A's commit should now + // run last and win. + tabB.setJiraCustomOrder(["FROM-B"]); + await Promise.resolve(); + tabA.setJiraCustomOrder(["FROM-A"]); + await Promise.resolve(); + + vi.advanceTimersByTime(200); + + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.jiraCustomOrder).toEqual(["FROM-A"]); + + disposeA(); + disposeB(); + vi.useRealTimers(); + }); + + it("a tab's unrelated field change does not clobber the other tab's more recent same-field write", async () => { + vi.useFakeTimers(); + const tabA = await importFreshViewModule(); + const tabB = await importFreshViewModule(); + + let disposeA!: () => void; + let disposeB!: () => void; + createRoot((d) => { disposeA = d; tabA.initViewPersistence(); }); + createRoot((d) => { disposeB = d; tabB.initViewPersistence(); }); + + // B writes jiraCustomOrder and its debounced commit fully completes first. + tabB.setJiraCustomOrder(["FROM-B"]); + await Promise.resolve(); + vi.advanceTimersByTime(200); + expect(JSON.parse(localStorageMock.getItem(VIEW_KEY)!).jiraCustomOrder).toEqual(["FROM-B"]); + + // A, which never touched jiraCustomOrder, now changes an unrelated field. + // A's own stale in-memory jiraCustomOrder ([]) must NOT overwrite B's + // already-committed value merely because A is writing at all. + tabA.setGlobalFilter("org1", "repo1"); + await Promise.resolve(); + vi.advanceTimersByTime(200); + + const parsed = JSON.parse(localStorageMock.getItem(VIEW_KEY)!); + expect(parsed.globalFilter.org).toBe("org1"); + expect(parsed.jiraCustomOrder).toEqual(["FROM-B"]); + + disposeA(); + disposeB(); vi.useRealTimers(); }); }); @@ -1051,4 +1425,85 @@ describe("loadViewState — cap-guard integration", () => { expect(mod.viewState.lockedRepos["issues"]).toEqual(["org/repo", "org/other"]); }); + + it("truncates oversized jiraCustomOrder arrays to JIRA_CUSTOM_ORDER_CAP", async () => { + const bigArray = Array.from({ length: 600 }, (_, i) => `KEY-${i}`); + localStorageMock.setItem(VIEW_KEY, JSON.stringify({ + jiraCustomOrder: bigArray, + })); + + vi.resetModules(); + const mod = await import("../../src/app/stores/view"); + + expect(mod.viewState.jiraCustomOrder.length).toBe(mod.JIRA_CUSTOM_ORDER_CAP); + expect(mod.viewState.jiraCustomOrder[0]).toBe("KEY-0"); + expect(mod.viewState.jiraCustomOrder[mod.JIRA_CUSTOM_ORDER_CAP - 1]).toBe( + `KEY-${mod.JIRA_CUSTOM_ORDER_CAP - 1}` + ); + }); + + it("filters non-string elements from jiraCustomOrder arrays", async () => { + localStorageMock.setItem(VIEW_KEY, JSON.stringify({ + jiraCustomOrder: [42, "PROJ-1", null, true, "PROJ-2"], + })); + + vi.resetModules(); + const mod = await import("../../src/app/stores/view"); + + expect(mod.viewState.jiraCustomOrder).toEqual(["PROJ-1", "PROJ-2"]); + }); + + it("drops (not truncates) jiraCustomOrder entries longer than JIRA_CUSTOM_ORDER_KEY_MAX_LENGTH", async () => { + const maxLen = 50; + const tooLong = "X".repeat(maxLen + 1); // 51 chars — over the limit + localStorageMock.setItem(VIEW_KEY, JSON.stringify({ + jiraCustomOrder: ["PROJ-1", tooLong], + })); + + vi.resetModules(); + const mod = await import("../../src/app/stores/view"); + + // Sanity: confirm the constant matches what this test assumes before trusting the assertions below. + expect(mod.JIRA_CUSTOM_ORDER_KEY_MAX_LENGTH).toBe(maxLen); + // The oversized entry is dropped entirely, NOT truncated to a 50-char prefix. + // If the guard truncated instead of dropping, the array would be + // ["PROJ-1", "X".repeat(50)] with length 2 — assert both the exact + // array and the absence of a truncated variant to distinguish the two behaviors. + expect(mod.viewState.jiraCustomOrder).toEqual(["PROJ-1"]); + expect(mod.viewState.jiraCustomOrder).not.toContain("X".repeat(maxLen)); + }); + + it("deletes a non-array (string) jiraCustomOrder value so ViewStateSchema's default applies", async () => { + localStorageMock.setItem(VIEW_KEY, JSON.stringify({ + jiraCustomOrder: "not-an-array", + })); + + vi.resetModules(); + const mod = await import("../../src/app/stores/view"); + + expect(mod.viewState.jiraCustomOrder).toEqual([]); + }); + + it("deletes a non-array (number) jiraCustomOrder value so ViewStateSchema's default applies", async () => { + localStorageMock.setItem(VIEW_KEY, JSON.stringify({ + jiraCustomOrder: 42, + })); + + vi.resetModules(); + const mod = await import("../../src/app/stores/view"); + + expect(mod.viewState.jiraCustomOrder).toEqual([]); + }); + + it("defaults jiraCustomOrder to [] when the key is missing from the raw blob", async () => { + localStorageMock.setItem(VIEW_KEY, JSON.stringify({ + lastActiveTab: "jiraAssigned", + })); + + vi.resetModules(); + const mod = await import("../../src/app/stores/view"); + + expect(mod.viewState.jiraCustomOrder).toEqual([]); + expect(mod.viewState.lastActiveTab).toBe("jiraAssigned"); + }); });