fix(dashboard): make duplicate-warning "Open" actually open the task
The "Open" button on a possible-duplicate warning did nothing — the modal closed and no task appeared. The app has two task deep-link shapes and only one had a consumer: `?task=<id>` was handled by useDeepLink, while `#/tasks/<id>` had no `hashchange` listener anywhere in the dashboard. Five surfaces write the hash form. InlineCreateCard and NewTaskModal write it unconditionally, so their Open was always dead. QuickEntryBox, Column, and ListView try an in-memory board lookup first and fall through to the dead hash, which is why it looked intermittent: duplicate matches come from a project-wide searchTasks, so a match that is `done` or outside the loaded board slice misses the lookup and lands on the no-op. Handle the hash form inside useDeepLink so the app keeps one deep-link authority owning both shapes, rather than forking a parallel hook. The id is resolved by fetch instead of an in-memory lookup (that lookup is the dead end being removed), the hash is cleared via replaceState so re-Opening the same task fires again, and an unresolvable id toasts instead of failing silently. Regression tests assert the invariant shared by all five surfaces rather than the single reported repro: a written hash always resolves to an open or a toast, never a no-op. Verified against the pre-fix code — 5 of the 6 new tests fail without this change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/fix-duplicate-warning-open-deep-link.md
Normal file
7
.changeset/fix-duplicate-warning-open-deep-link.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Fix the "Open" button on possible-duplicate task warnings doing nothing.
|
||||
category: fix
|
||||
dev: `#/tasks/<id>` had no consumer — five surfaces wrote it (duplicate-warning Open in InlineCreateCard/NewTaskModal/QuickEntryBox, Column/ListView quick-add fallbacks) while only `?task=<id>` was implemented. `useDeepLink` now owns both shapes; unresolvable ids toast instead of no-op'ing.
|
||||
@@ -431,4 +431,90 @@ describe("useDeepLink", () => {
|
||||
expect(mockFetchTaskDetail).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
/*
|
||||
FNXC:DeepLink 2026-07-25-11:20:
|
||||
Regression coverage for the `#/tasks/<id>` hash deep link, which had NO consumer: every
|
||||
surface writing it (duplicate-warning "Open" in InlineCreateCard / NewTaskModal /
|
||||
QuickEntryBox, and the Column / ListView quick-add fallbacks) silently did nothing.
|
||||
These assert the shared invariant those five surfaces depend on rather than the single
|
||||
reported repro: a written hash always resolves to an open or a toast, never a no-op.
|
||||
*/
|
||||
describe("#/tasks/:id hash deep link (shared by all duplicate-warning Open surfaces)", () => {
|
||||
function setHash(hash: string) {
|
||||
Object.defineProperty(window, "location", {
|
||||
configurable: true,
|
||||
value: new URL(`http://localhost:3000/${hash}`),
|
||||
});
|
||||
}
|
||||
|
||||
it("opens the task when the hash is present on mount", async () => {
|
||||
setHash("#/tasks/FN-4242");
|
||||
|
||||
const { openTaskDetail } = renderUseDeepLink();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockFetchTaskDetail).toHaveBeenCalledWith("FN-4242", "proj_123");
|
||||
expect(openTaskDetail).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("opens the task when the hash is written after mount (the Open-button path)", async () => {
|
||||
const { openTaskDetail } = renderUseDeepLink();
|
||||
|
||||
setHash("#/tasks/FN-4242");
|
||||
window.dispatchEvent(new HashChangeEvent("hashchange"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockFetchTaskDetail).toHaveBeenCalledWith("FN-4242", "proj_123");
|
||||
expect(openTaskDetail).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
it("clears the hash so re-Opening the same task fires again", async () => {
|
||||
const { openTaskDetail } = renderUseDeepLink();
|
||||
|
||||
setHash("#/tasks/FN-4242");
|
||||
window.dispatchEvent(new HashChangeEvent("hashchange"));
|
||||
await waitFor(() => expect(openTaskDetail).toHaveBeenCalledTimes(1));
|
||||
|
||||
// The consumer must have stripped the hash; otherwise the second Open is a no-op.
|
||||
expect(window.location.hash).toBe("");
|
||||
|
||||
setHash("#/tasks/FN-4242");
|
||||
window.dispatchEvent(new HashChangeEvent("hashchange"));
|
||||
await waitFor(() => expect(openTaskDetail).toHaveBeenCalledTimes(2));
|
||||
});
|
||||
|
||||
it("toasts instead of failing silently when the task cannot be resolved", async () => {
|
||||
mockFetchTaskDetail.mockRejectedValueOnce(new Error("not found"));
|
||||
setHash("#/tasks/FN-9001");
|
||||
|
||||
const { addToast, openTaskDetail } = renderUseDeepLink();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(addToast).toHaveBeenCalledWith(expect.stringContaining("FN-9001"), "error");
|
||||
});
|
||||
expect(openTaskDetail).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("scopes the fetch to the active project", async () => {
|
||||
setHash("#/tasks/FN-4242");
|
||||
|
||||
renderUseDeepLink({ projectId: otherProject.id, currentProject: otherProject });
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockFetchTaskDetail).toHaveBeenCalledWith("FN-4242", "proj_456");
|
||||
});
|
||||
});
|
||||
|
||||
it("ignores hashes that are not task deep links", async () => {
|
||||
setHash("#message-abc");
|
||||
|
||||
renderUseDeepLink();
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockFetchTaskDetail).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -166,6 +166,69 @@ export function useDeepLink(options: UseDeepLinkOptions): UseDeepLinkResult {
|
||||
// deepLinkFetchedRef intentionally excluded - it's a mutable ref, not state
|
||||
]);
|
||||
|
||||
/*
|
||||
FNXC:DeepLink 2026-07-25-11:20:
|
||||
`#/tasks/<id>` is the SECOND task deep-link shape in the app, and until now it had no
|
||||
consumer at all. Five surfaces write it as their open-a-task-we-do-not-hold-in-memory
|
||||
fallback — the duplicate-warning "Open" button in InlineCreateCard, NewTaskModal, and
|
||||
QuickEntryBox, plus the Column and ListView quick-add `onOpenTask` fallbacks — so every
|
||||
one of those Opens silently did nothing and the operator saw a frozen "Possible
|
||||
duplicates" modal with an unresponsive button.
|
||||
|
||||
Invariant: writing `#/tasks/<id>` ALWAYS produces an observable outcome — the task opens,
|
||||
or a toast explains why it could not. Never a silent no-op. Handled here rather than in a
|
||||
new hook so the app keeps ONE deep-link authority owning both URL shapes.
|
||||
|
||||
Two behaviors differ deliberately from the `?task=` path above:
|
||||
- No one-shot `deepLinkFetchedRef` guard. The hash form is a repeatable in-session
|
||||
action (click Open, close detail, click Open again), not a once-per-load boot link.
|
||||
- The id is resolved by fetch, not by an in-memory board lookup, because the duplicate
|
||||
target is routinely outside the loaded slice (a `done` task, a collapsed column, a
|
||||
filtered-out row) — the exact case an in-memory lookup would silently drop.
|
||||
*/
|
||||
useEffect(() => {
|
||||
if (typeof window === "undefined") return;
|
||||
|
||||
let cancelled = false;
|
||||
|
||||
const consumeTaskHash = () => {
|
||||
const match = /^#\/tasks\/([^/?#]+)/.exec(window.location.hash);
|
||||
if (!match) return;
|
||||
|
||||
let taskId = match[1];
|
||||
try {
|
||||
taskId = decodeURIComponent(taskId);
|
||||
} catch {
|
||||
// A malformed percent-escape still names a task the caller meant to open.
|
||||
}
|
||||
|
||||
/*
|
||||
Clear the hash BEFORE awaiting. `replaceState` does not itself fire `hashchange`, so
|
||||
this is load-bearing twice: re-Opening the same task still produces a `hashchange`
|
||||
event, and a stale hash cannot re-open the task on a later mount or project switch.
|
||||
*/
|
||||
const existingState = window.history.state ?? {};
|
||||
window.history.replaceState(existingState, "", `${window.location.pathname}${window.location.search}`);
|
||||
|
||||
fetchTaskDetail(taskId, projectId)
|
||||
.then((detail) => {
|
||||
if (cancelled) return;
|
||||
openTaskDetail(detail);
|
||||
})
|
||||
.catch(() => {
|
||||
if (cancelled) return;
|
||||
addToast(t("deepLink.taskNotFound", "Task {{id}} not found", { id: taskId }), "error");
|
||||
});
|
||||
};
|
||||
|
||||
consumeTaskHash();
|
||||
window.addEventListener("hashchange", consumeTaskHash);
|
||||
return () => {
|
||||
cancelled = true;
|
||||
window.removeEventListener("hashchange", consumeTaskHash);
|
||||
};
|
||||
}, [addToast, openTaskDetail, projectId, t]);
|
||||
|
||||
const handleDetailClose = useCallback(() => {
|
||||
if (deepLinkTaskIdRef.current) {
|
||||
const params = new URLSearchParams(window.location.search);
|
||||
|
||||
Reference in New Issue
Block a user