fix(#248): show merge success banner via PRG pattern (?merged=1 redirect)
After a successful merge, redirect 303 to /admin/tags/{targetId}?merged=1.
Load function detects the param and returns mergeSuccess:true; +page.svelte
renders the banner and cleans the URL with replaceState so refresh doesn't
re-show it.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -3,11 +3,11 @@ import type { PageServerLoad, Actions } from './$types';
|
||||
import { createApiClient } from '$lib/api.server';
|
||||
import { getErrorMessage } from '$lib/errors';
|
||||
|
||||
export const load: PageServerLoad = async ({ params, parent }) => {
|
||||
export const load: PageServerLoad = async ({ params, parent, url }) => {
|
||||
const { tags } = await parent();
|
||||
const tag = tags.find((t: { id: string }) => t.id === params.id);
|
||||
if (!tag) throw error(404, getErrorMessage('TAG_NOT_FOUND'));
|
||||
return { tag };
|
||||
return { tag, mergeSuccess: url.searchParams.has('merged') };
|
||||
};
|
||||
|
||||
export const actions: Actions = {
|
||||
@@ -47,7 +47,7 @@ export const actions: Actions = {
|
||||
return fail(result.response.status, { error: getErrorMessage(code) });
|
||||
}
|
||||
|
||||
return { mergeSuccess: true, mergeTargetId: result.data!.id };
|
||||
throw redirect(303, `/admin/tags/${result.data!.id}?merged=1`);
|
||||
},
|
||||
|
||||
delete: async ({ params, request, fetch }) => {
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
<script lang="ts">
|
||||
import { enhance } from '$app/forms';
|
||||
import { replaceState } from '$app/navigation';
|
||||
import { page } from '$app/stores';
|
||||
import { m } from '$lib/paraglide/messages.js';
|
||||
import { createUnsavedWarning } from '$lib/hooks/useUnsavedWarning.svelte';
|
||||
import UnsavedWarningBanner from '$lib/components/UnsavedWarningBanner.svelte';
|
||||
@@ -40,6 +42,12 @@ $effect(() => {
|
||||
if (form?.success) unsaved.clearOnSuccess();
|
||||
});
|
||||
|
||||
$effect(() => {
|
||||
if (data.mergeSuccess) {
|
||||
replaceState($page.url.pathname, {});
|
||||
}
|
||||
});
|
||||
|
||||
const colors = [
|
||||
'sage',
|
||||
'sienna',
|
||||
@@ -86,6 +94,15 @@ const colors = [
|
||||
{#if unsaved.showUnsavedWarning}
|
||||
<UnsavedWarningBanner onDiscard={unsaved.discard} />
|
||||
{/if}
|
||||
{#if data.mergeSuccess}
|
||||
<div
|
||||
class="mb-5 rounded border border-green-200 bg-green-50 p-3 text-sm text-green-700"
|
||||
role="status"
|
||||
aria-live="polite"
|
||||
>
|
||||
{m.admin_tag_merge_success()}
|
||||
</div>
|
||||
{/if}
|
||||
{#if form?.success}
|
||||
<div class="mb-5 rounded border border-green-200 bg-green-50 p-3 text-sm text-green-700">
|
||||
{m.admin_tag_updated()}
|
||||
|
||||
@@ -1,5 +1,4 @@
|
||||
<script lang="ts">
|
||||
import { goto } from '$app/navigation';
|
||||
import { enhance } from '$app/forms';
|
||||
import { m } from '$lib/paraglide/messages.js';
|
||||
import TagParentPicker from '$lib/components/TagParentPicker.svelte';
|
||||
@@ -15,7 +14,7 @@ type FlatTag = {
|
||||
interface Props {
|
||||
tag: { id: string; name: string; documentCount: number };
|
||||
allTags: FlatTag[];
|
||||
form: { error?: string; mergeSuccess?: boolean; mergeTargetId?: string } | null;
|
||||
form: { error?: string } | null;
|
||||
}
|
||||
|
||||
let { tag, allTags, form }: Props = $props();
|
||||
@@ -27,11 +26,6 @@ $effect(() => {
|
||||
targetId = '';
|
||||
});
|
||||
|
||||
$effect(() => {
|
||||
if (form?.mergeSuccess && form.mergeTargetId) {
|
||||
goto(`/admin/tags/${form.mergeTargetId}`);
|
||||
}
|
||||
});
|
||||
const step = $derived(targetId ? 2 : 1);
|
||||
|
||||
// All descendants of tag.id (to exclude from picker)
|
||||
@@ -125,12 +119,6 @@ const targetTag = $derived(allTags.find((t) => t.id === targetId));
|
||||
</div>
|
||||
{/if}
|
||||
|
||||
{#if form?.mergeSuccess}
|
||||
<p data-testid="merge-success-banner" class="mt-3 text-xs text-green-700">
|
||||
{m.admin_tag_merge_success()}
|
||||
</p>
|
||||
{/if}
|
||||
|
||||
{#if form?.error}
|
||||
<p class="mt-3 text-xs text-red-600">{form.error}</p>
|
||||
{/if}
|
||||
|
||||
@@ -4,7 +4,6 @@ import { page } from 'vitest/browser';
|
||||
import TagMergeZone from './TagMergeZone.svelte';
|
||||
|
||||
vi.mock('$app/forms', () => ({ enhance: () => () => {} }));
|
||||
vi.mock('$app/navigation', () => ({ goto: vi.fn() }));
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers();
|
||||
@@ -94,19 +93,3 @@ describe('TagMergeZone – stale state reset', () => {
|
||||
expect(document.querySelector('[data-testid="merge-step2"]')).toBeFalsy();
|
||||
});
|
||||
});
|
||||
|
||||
describe('TagMergeZone – success banner', () => {
|
||||
it('shows success banner when form.mergeSuccess is true', async () => {
|
||||
render(TagMergeZone, {
|
||||
tag,
|
||||
allTags,
|
||||
form: { mergeSuccess: true, mergeTargetId: 't2' }
|
||||
});
|
||||
await expect.element(page.getByTestId('merge-success-banner')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('does not show success banner when form is null', async () => {
|
||||
const { container } = render(TagMergeZone, { tag, allTags, form: null });
|
||||
expect(container.querySelector('[data-testid="merge-success-banner"]')).toBeFalsy();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { actions } from './+page.server';
|
||||
import { actions, load } from './+page.server';
|
||||
|
||||
const mockApi = {
|
||||
PUT: vi.fn(),
|
||||
@@ -13,6 +13,30 @@ vi.mock('$lib/api.server', () => ({
|
||||
|
||||
beforeEach(() => vi.clearAllMocks());
|
||||
|
||||
// ─── load function ─────────────────────────────────────────────────────────────
|
||||
|
||||
describe('tags/[id] — load function', () => {
|
||||
it('returns mergeSuccess: true when url has ?merged param', async () => {
|
||||
const url = new URL('http://localhost/admin/tags/t1?merged=1');
|
||||
const result = await load({
|
||||
params: { id: 't1' },
|
||||
parent: async () => ({ tags: [{ id: 't1', name: 'Test', documentCount: 0 }] }),
|
||||
url
|
||||
} as never);
|
||||
expect((result as { mergeSuccess: boolean }).mergeSuccess).toBe(true);
|
||||
});
|
||||
|
||||
it('returns mergeSuccess: false when url has no ?merged param', async () => {
|
||||
const url = new URL('http://localhost/admin/tags/t1');
|
||||
const result = await load({
|
||||
params: { id: 't1' },
|
||||
parent: async () => ({ tags: [{ id: 't1', name: 'Test', documentCount: 0 }] }),
|
||||
url
|
||||
} as never);
|
||||
expect((result as { mergeSuccess: boolean }).mergeSuccess).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── update action ─────────────────────────────────────────────────────────────
|
||||
|
||||
describe('tags/[id] — update action', () => {
|
||||
@@ -53,7 +77,7 @@ describe('tags/[id] — update action', () => {
|
||||
// ─── merge action ─────────────────────────────────────────────────────────────
|
||||
|
||||
describe('tags/[id] — merge action', () => {
|
||||
it('returns mergeSuccess and mergeTargetId on successful merge', async () => {
|
||||
it('redirects to target tag with ?merged=1 on successful merge', async () => {
|
||||
mockApi.POST.mockResolvedValue({
|
||||
response: { ok: true },
|
||||
data: { id: 't2', name: 'Reise' }
|
||||
@@ -62,13 +86,19 @@ describe('tags/[id] — merge action', () => {
|
||||
const formData = new FormData();
|
||||
formData.set('targetId', 't2');
|
||||
|
||||
const result = await actions.merge({
|
||||
params: { id: 't1' },
|
||||
request: { formData: async () => formData },
|
||||
fetch
|
||||
} as never);
|
||||
let redirectUrl: string | null = null;
|
||||
try {
|
||||
await actions.merge({
|
||||
params: { id: 't1' },
|
||||
request: { formData: async () => formData },
|
||||
fetch
|
||||
} as never);
|
||||
} catch (e: unknown) {
|
||||
const r = e as { location?: string };
|
||||
redirectUrl = r.location ?? null;
|
||||
}
|
||||
|
||||
expect(result).toEqual({ mergeSuccess: true, mergeTargetId: 't2' });
|
||||
expect(redirectUrl).toBe('/admin/tags/t2?merged=1');
|
||||
});
|
||||
|
||||
it('returns fail when merge API responds not ok', async () => {
|
||||
|
||||
@@ -4,7 +4,19 @@ import { page } from 'vitest/browser';
|
||||
import Page from './+page.svelte';
|
||||
|
||||
vi.mock('$app/forms', () => ({ enhance: () => () => {} }));
|
||||
vi.mock('$app/navigation', () => ({ beforeNavigate: vi.fn(), goto: vi.fn() }));
|
||||
vi.mock('$app/navigation', () => ({
|
||||
beforeNavigate: vi.fn(),
|
||||
goto: vi.fn(),
|
||||
replaceState: vi.fn()
|
||||
}));
|
||||
vi.mock('$app/stores', () => ({
|
||||
page: {
|
||||
subscribe: (fn: (v: { url: URL }) => void) => {
|
||||
fn({ url: new URL('http://localhost/admin/tags/t1') });
|
||||
return () => {};
|
||||
}
|
||||
}
|
||||
}));
|
||||
|
||||
import { beforeNavigate, goto } from '$app/navigation';
|
||||
|
||||
@@ -138,6 +150,20 @@ describe('Admin edit tag page – color picker', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─── Merge success banner ─────────────────────────────────────────────────────
|
||||
|
||||
describe('Admin edit tag page – merge success banner', () => {
|
||||
it('shows merge success banner when data.mergeSuccess is true', async () => {
|
||||
render(Page, { data: { ...baseData, mergeSuccess: true }, form: null });
|
||||
await expect.element(page.getByText(/Erfolgreich zusammengeführt/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('does not show merge success banner when data.mergeSuccess is false', async () => {
|
||||
render(Page, { data: { ...baseData, mergeSuccess: false }, form: null });
|
||||
await expect.element(page.getByText(/Erfolgreich zusammengeführt/i)).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
// ─── New components present ───────────────────────────────────────────────────
|
||||
|
||||
describe('Admin edit tag page – new components', () => {
|
||||
|
||||
Reference in New Issue
Block a user