fix(notifications): move onClose/goto into enhance result callback
onClose() and goto() were firing before the server responded, making it impossible for a fail() response to cancel navigation. Moved them inside the result callback behind a result.type !== 'failure' guard. Updated the $app/forms enhance mock to always invoke the returned async callback with a configurable mockFormResult, and added three tests: - success path calls onClose + goto with the correct deep-link URL - failure path skips onClose and goto - annotationId is appended to the URL when present Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -84,15 +84,17 @@ function handleViewAll() {
|
|||||||
class="contents"
|
class="contents"
|
||||||
use:enhance={() => {
|
use:enhance={() => {
|
||||||
optimisticMarkRead(notification.id);
|
optimisticMarkRead(notification.id);
|
||||||
onClose();
|
return async ({ result, update }) => {
|
||||||
goto(
|
if (result.type !== 'failure') {
|
||||||
buildCommentHref(
|
onClose();
|
||||||
notification.documentId,
|
goto(
|
||||||
notification.referenceId,
|
buildCommentHref(
|
||||||
notification.annotationId
|
notification.documentId,
|
||||||
)
|
notification.referenceId,
|
||||||
);
|
notification.annotationId
|
||||||
return async ({ update }) => {
|
)
|
||||||
|
);
|
||||||
|
}
|
||||||
await update({ reset: false, invalidateAll: false });
|
await update({ reset: false, invalidateAll: false });
|
||||||
};
|
};
|
||||||
}}
|
}}
|
||||||
|
|||||||
@@ -6,13 +6,28 @@ import NotificationDropdown from './NotificationDropdown.svelte';
|
|||||||
|
|
||||||
vi.mock('$app/navigation', () => ({ goto: vi.fn() }));
|
vi.mock('$app/navigation', () => ({ goto: vi.fn() }));
|
||||||
|
|
||||||
// Invoke the SubmitFunction synchronously on form submit so tests can assert
|
// Configurable result for the enhance mock — tests that need failure set
|
||||||
// that optimisticMarkRead / optimisticMarkAllRead are called.
|
// mockFormResult.type = 'failure' before clicking.
|
||||||
|
const mockFormResult = vi.hoisted(() => ({ type: 'success' as string }));
|
||||||
|
|
||||||
|
// Invoke the SubmitFunction and always call the returned result callback with
|
||||||
|
// mockFormResult so tests can exercise both success and failure branches.
|
||||||
vi.mock('$app/forms', () => ({
|
vi.mock('$app/forms', () => ({
|
||||||
enhance(node: HTMLFormElement, submit?: (opts: { formData: FormData }) => unknown) {
|
enhance(
|
||||||
const handler = (e: Event) => {
|
node: HTMLFormElement,
|
||||||
|
submit?: (opts: {
|
||||||
|
formData: FormData;
|
||||||
|
}) => (opts: {
|
||||||
|
result: { type: string; data?: Record<string, unknown> };
|
||||||
|
update: () => Promise<void>;
|
||||||
|
}) => Promise<void>
|
||||||
|
) {
|
||||||
|
const handler = async (e: Event) => {
|
||||||
e.preventDefault();
|
e.preventDefault();
|
||||||
submit?.({ formData: new FormData(node) } as never);
|
const cb = submit?.({ formData: new FormData(node) } as never);
|
||||||
|
if (typeof cb === 'function') {
|
||||||
|
await cb({ result: mockFormResult, update: async () => {} } as never);
|
||||||
|
}
|
||||||
};
|
};
|
||||||
node.addEventListener('submit', handler);
|
node.addEventListener('submit', handler);
|
||||||
return { destroy: () => node.removeEventListener('submit', handler) };
|
return { destroy: () => node.removeEventListener('submit', handler) };
|
||||||
@@ -22,6 +37,7 @@ vi.mock('$app/forms', () => ({
|
|||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
cleanup();
|
cleanup();
|
||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
|
mockFormResult.type = 'success'; // reset to default after each test
|
||||||
});
|
});
|
||||||
|
|
||||||
const makeNotification = (overrides: Record<string, unknown> = {}) => ({
|
const makeNotification = (overrides: Record<string, unknown> = {}) => ({
|
||||||
@@ -313,4 +329,69 @@ describe('NotificationDropdown', () => {
|
|||||||
const forms = document.querySelectorAll('form[action="/aktivitaeten?/dismiss-notification"]');
|
const forms = document.querySelectorAll('form[action="/aktivitaeten?/dismiss-notification"]');
|
||||||
expect(forms.length).toBe(2);
|
expect(forms.length).toBe(2);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('calls onClose and goto with the deep-link URL after a successful dismiss', async () => {
|
||||||
|
const onClose = vi.fn();
|
||||||
|
const n = makeNotification({
|
||||||
|
id: 'n42',
|
||||||
|
documentId: 'd1',
|
||||||
|
referenceId: 'c1',
|
||||||
|
annotationId: null,
|
||||||
|
actorName: 'Anna'
|
||||||
|
});
|
||||||
|
render(NotificationDropdown, {
|
||||||
|
props: {
|
||||||
|
notifications: [n],
|
||||||
|
optimisticMarkRead: () => {},
|
||||||
|
optimisticMarkAllRead: () => {},
|
||||||
|
onClose
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
await page.getByRole('button', { name: /Anna hat auf deinen/i }).click();
|
||||||
|
|
||||||
|
expect(onClose).toHaveBeenCalledOnce();
|
||||||
|
expect(goto).toHaveBeenCalledWith('/documents/d1?commentId=c1');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does NOT call onClose or goto when the dismiss action returns a failure', async () => {
|
||||||
|
mockFormResult.type = 'failure';
|
||||||
|
const onClose = vi.fn();
|
||||||
|
const n = makeNotification({ id: 'n99', actorName: 'Bob' });
|
||||||
|
render(NotificationDropdown, {
|
||||||
|
props: {
|
||||||
|
notifications: [n],
|
||||||
|
optimisticMarkRead: () => {},
|
||||||
|
optimisticMarkAllRead: () => {},
|
||||||
|
onClose
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
await page.getByRole('button', { name: /Bob hat auf deinen/i }).click();
|
||||||
|
|
||||||
|
expect(onClose).not.toHaveBeenCalled();
|
||||||
|
expect(goto).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('calls goto with annotationId appended when the notification has an annotationId', async () => {
|
||||||
|
const n = makeNotification({
|
||||||
|
id: 'n55',
|
||||||
|
documentId: 'd1',
|
||||||
|
referenceId: 'c1',
|
||||||
|
annotationId: 'a1',
|
||||||
|
actorName: 'Eva'
|
||||||
|
});
|
||||||
|
render(NotificationDropdown, {
|
||||||
|
props: {
|
||||||
|
notifications: [n],
|
||||||
|
optimisticMarkRead: () => {},
|
||||||
|
optimisticMarkAllRead: () => {},
|
||||||
|
onClose: () => {}
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
await page.getByRole('button', { name: /Eva hat auf deinen/i }).click();
|
||||||
|
|
||||||
|
expect(goto).toHaveBeenCalledWith('/documents/d1?commentId=c1&annotationId=a1');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user