diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index c6b10c689b..319d764dd3 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -275,6 +275,15 @@ These browser suites are intended for targeted local verification and CI, not th For normal issue work, start with the smallest targeted check that proves the change. Reserve repo-wide typecheck/build/test runs for PR-ready handoff or changes broad enough that narrow checks do not cover the risk. +### Recent task ordering + +The streamlined sidebar keeps five recent tasks per company and account in browser +storage. It sorts by the newest observed task or comment activity, not by live-run +state. Older detail responses cannot move the stored activity time backward. +Activity-only reorderings wait for one second without further activity changes; +new and removed tasks appear immediately. Titles, status, and live indicators stay +current during that delay. + ## One-Command Local Run For a first-time local install, you can bootstrap and run in one command: diff --git a/ui/src/components/SidebarRecentTasks.test.tsx b/ui/src/components/SidebarRecentTasks.test.tsx index 0b3ba98700..30f6d654bb 100644 --- a/ui/src/components/SidebarRecentTasks.test.tsx +++ b/ui/src/components/SidebarRecentTasks.test.tsx @@ -10,6 +10,7 @@ import { getRecentTasksStorageKey, readRecentTasks, recordRecentTask, + pruneRecentTasks, } from "@/lib/recent-tasks"; import { queryKeys } from "@/lib/queryKeys"; @@ -75,6 +76,7 @@ describe("SidebarRecentTasks", () => { act(() => root.unmount()); container.remove(); vi.clearAllMocks(); + vi.useRealTimers(); }); async function render() { @@ -169,6 +171,69 @@ describe("SidebarRecentTasks", () => { expect(actions?.className).toContain("opacity-0"); }); + it("keeps rows stable during alternating activity and applies the final order after quiet", async () => { + const tasks = [1, 2, 3].map((id) => ({ + id: `issue-${id}`, + companyId: "company-1", + title: `Task ${id}`, + identifier: `PAP-${id}`, + status: "done" as const, + hiddenAt: null, + updatedAt: new Date(id), + })); + tasks.forEach((task) => recordRecentTask(task, "user-1")); + mockIssuesApi.get.mockImplementation(async (id: string) => tasks.find((task) => task.id === id)); + const queryClient = await render(); + const order = () => Array.from(container.querySelectorAll("a")).map((link) => link.getAttribute("href")); + const originalOrder = ["/issues/issue-3", "/issues/issue-2", "/issues/issue-1"]; + expect(order()).toEqual(originalOrder); + vi.useFakeTimers(); + + for (let index = 0; index < 6; index += 1) { + const task = tasks[index % 2]!; + await act(async () => { + queryClient.setQueryData(queryKeys.issues.detail(task.id), { + ...task, title: `Updated ${task.id}`, updatedAt: new Date(100 + index), + }); + await vi.advanceTimersByTimeAsync(0); + }); + await act(async () => { await vi.advanceTimersByTimeAsync(500); }); + expect(order()).toEqual(originalOrder); + expect(container.textContent).toContain(`Updated ${task.id}`); + } + + await act(async () => { await vi.advanceTimersByTimeAsync(499); }); + expect(order()).toEqual(originalOrder); + await act(async () => { await vi.advanceTimersByTimeAsync(1); }); + expect(order()).toEqual(["/issues/issue-2", "/issues/issue-1", "/issues/issue-3"]); + expect(mockIssuesApi.get).toHaveBeenCalledTimes(3); + queryClient.clear(); + }); + + it("adds and removes tasks immediately while an activity reorder is pending", async () => { + const task = (id: string, recordedAt: number) => ({ + id, companyId: "company-1", title: id, identifier: id, + status: "todo" as const, updatedAt: new Date(recordedAt), + }); + recordRecentTask(task("first", 1), "user-1"); + recordRecentTask(task("second", 2), "user-1"); + mockIssuesApi.get.mockImplementation(async (id: string) => task(id, 0)); + const queryClient = await render(); + vi.useFakeTimers(); + act(() => recordRecentTask(task("first", 3), "user-1")); + expect(container.querySelector("a")?.getAttribute("href")).toBe("/issues/second"); + await act(async () => { + recordRecentTask(task("new", 4), "user-1"); + await vi.advanceTimersByTimeAsync(0); + }); + expect(container.querySelector("a")?.getAttribute("href")).toBe("/issues/new"); + act(() => pruneRecentTasks(getRecentTasksStorageKey("company-1", "user-1"), "company-1", new Set(["second"]))); + expect(container.querySelector('a[href="/issues/second"]')).toBeNull(); + await act(async () => { await vi.advanceTimersByTimeAsync(2_000); }); + expect(container.querySelector('a[href="/issues/second"]')).toBeNull(); + queryClient.clear(); + }); + it("opens the compact task actions menu from the ellipsis button", async () => { recordRecentTask({ id: "issue-1", diff --git a/ui/src/hooks/useRecentTasks.ts b/ui/src/hooks/useRecentTasks.ts index 5fe3156498..01d0a6e28e 100644 --- a/ui/src/hooks/useRecentTasks.ts +++ b/ui/src/hooks/useRecentTasks.ts @@ -18,6 +18,8 @@ type RecentTasksUpdatedDetail = { entries: RecentTaskEntry[]; }; +const RECENT_TASK_ORDER_DEBOUNCE_MS = 1_000; + export function useRecentTasks({ companyId, userId, @@ -57,8 +59,10 @@ export function useRecentTasks({ }; }, [companyId, storageKey]); + // Keep query observers in a fixed order when activity changes the display order. + const queryEntries = [...entries].sort((left, right) => left.id.localeCompare(right.id)); const detailQueries = useQueries({ - queries: entries.map((entry) => ({ + queries: queryEntries.map((entry) => ({ queryKey: queryKeys.issues.detail(entry.id), queryFn: () => issuesApi.get(entry.id), retry: false, @@ -66,8 +70,9 @@ export function useRecentTasks({ })), }); - const refreshedEntries = entries.map((entry, index) => { - const issue = detailQueries[index]?.data; + const issueById = new Map(detailQueries.flatMap((query) => query.data ? [[query.data.id, query.data] as const] : [])); + const refreshedEntries = entries.map((entry) => { + const issue = issueById.get(entry.id); if (!issue || issue.companyId !== companyId || issue.hiddenAt) return entry; return { ...entry, @@ -86,7 +91,7 @@ export function useRecentTasks({ const resolvedIssues: Issue[] = []; const removeIds = new Set(); detailQueries.forEach((query, index) => { - const entry = entries[index]; + const entry = queryEntries[index]; if (!entry) return; if (query.data) { if (query.data.companyId !== companyId || query.data.hiddenAt) { @@ -105,8 +110,26 @@ export function useRecentTasks({ // eslint-disable-next-line react-hooks/exhaustive-deps }, [companyId, entries, queryRevision, storageKey]); + const [settledOrder, setSettledOrder] = useState(() => entries.map((entry) => entry.id)); + const membership = JSON.stringify(queryEntries.map((entry) => entry.id)); + const activityRevision = JSON.stringify(entries.map((entry) => [entry.id, entry.recordedAt])); + useEffect(() => { + const latestOrder = (JSON.parse(activityRevision) as Array<[string, number]>).map(([id]) => id); + // Additions and removals are immediate. Only activity-driven moves wait for quiet. + if (JSON.stringify([...settledOrder].sort((a, b) => a.localeCompare(b))) !== membership) { + setSettledOrder(latestOrder); + return; + } + if (latestOrder.every((id, index) => id === settledOrder[index])) return; + const timeout = window.setTimeout(() => setSettledOrder(latestOrder), RECENT_TASK_ORDER_DEBOUNCE_MS); + return () => window.clearTimeout(timeout); + }, [activityRevision, membership, settledOrder, storageKey]); + + const entryById = new Map(refreshedEntries.map((entry) => [entry.id, entry])); + const hasSameMembership = settledOrder.length === entries.length && settledOrder.every((id) => entryById.has(id)); + return { - entries: refreshedEntries, + entries: hasSameMembership ? settledOrder.map((id) => entryById.get(id)!) : refreshedEntries, storageKey, }; } diff --git a/ui/src/lib/recent-tasks.test.ts b/ui/src/lib/recent-tasks.test.ts index ae64c38d75..05466a5f63 100644 --- a/ui/src/lib/recent-tasks.test.ts +++ b/ui/src/lib/recent-tasks.test.ts @@ -83,6 +83,36 @@ describe("recent task persistence", () => { ]); }); + it("does not demote recent comment activity when older task details arrive", () => { + const storageKey = getRecentTasksStorageKey("company-1", "user-1"); + recordRecentTask(issue("1"), "user-1", 100); + recordRecentTask(issue("2"), "user-1", 90); + const listener = vi.fn(); + window.addEventListener(RECENT_TASKS_UPDATED_EVENT, listener); + + for (let index = 0; index < 10; index += 1) { + recordRecentTask({ ...issue("1"), updatedAt: new Date(50) }, "user-1"); + updateRecentTaskSnapshots(storageKey, "company-1", [ + { ...issue("1"), updatedAt: new Date(50) }, + { ...issue("2"), updatedAt: new Date(80) }, + ]); + } + + expect(readRecentTasks(storageKey, "company-1").map(({ id, recordedAt }) => ({ id, recordedAt }))).toEqual([ + { id: "1", recordedAt: 100 }, + { id: "2", recordedAt: 90 }, + ]); + expect(listener).not.toHaveBeenCalled(); + window.removeEventListener(RECENT_TASKS_UPDATED_EVENT, listener); + + updateRecentTaskSnapshots(storageKey, "company-1", [{ + ...issue("1"), title: "Renamed task", status: "done", updatedAt: new Date(60), + }]); + expect(readRecentTasks(storageKey, "company-1")[0]).toMatchObject({ + id: "1", title: "Renamed task", status: "done", recordedAt: 100, + }); + }); + it("ignores malformed and cross-company entries", () => { const storageKey = getRecentTasksStorageKey("company-1", "user-1"); window.localStorage.setItem(storageKey, JSON.stringify([ diff --git a/ui/src/lib/recent-tasks.ts b/ui/src/lib/recent-tasks.ts index 6308eeb2ac..9958dd9d25 100644 --- a/ui/src/lib/recent-tasks.ts +++ b/ui/src/lib/recent-tasks.ts @@ -94,7 +94,8 @@ export function recordRecentTask( title: issue.title, identifier: issue.identifier, status: issue.status, - recordedAt: activityAt, + // A stale detail query must not undo a newer comment or activity update. + recordedAt: Math.max(activityAt, existing?.recordedAt ?? activityAt), }; if ( existing @@ -135,7 +136,9 @@ export function updateRecentTaskSnapshots( const issue = issueById.get(entry.id); if (!issue || issue.companyId !== companyId) return entry; const activityAt = new Date(issue.updatedAt).getTime(); - const nextRecordedAt = Number.isFinite(activityAt) ? activityAt : entry.recordedAt; + const nextRecordedAt = Number.isFinite(activityAt) + ? Math.max(activityAt, entry.recordedAt) + : entry.recordedAt; if ( issue.title === entry.title && issue.identifier === entry.identifier