refactor(chronik): remove unused form actions and broken pagination UI
Two items flagged as blockers in PR #288 review: - Markus + Sara: "Mehr laden" calls GET /api/dashboard/activity?offset=N but the backend's DashboardController only accepts `limit` — `offset` was silently ignored, and every click re-fetched the same top-40 rows. Rather than add backend offset/cursor support in this PR (scope creep), remove the Load-more UI and defer pagination to a follow-up issue. 40 items covers the default case; the feature can come back with proper backend support and its own tests. - Markus + Sara: ?/dismiss and ?/mark-all form actions were dead code — the UI calls `onMarkRead` / `onMarkAllRead` callbacks (→ singleton → raw PATCH) and never submits either form. Delete both actions and their tests. Using the form-action path would require deprecating the NotificationBell's raw-PATCH as well — that's tracked separately as #286. The Dismiss markup split from the previous commit stands on its own. Part of #285, address PR #288 review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,4 +1,3 @@
|
||||
import { fail } from '@sveltejs/kit';
|
||||
import { createApiClient } from '$lib/api.server';
|
||||
import type { components } from '$lib/generated/api';
|
||||
|
||||
@@ -53,29 +52,3 @@ export async function load({ fetch, url }) {
|
||||
loadError
|
||||
};
|
||||
}
|
||||
|
||||
export const actions = {
|
||||
dismiss: async ({ request, fetch }) => {
|
||||
const api = createApiClient(fetch);
|
||||
const formData = await request.formData();
|
||||
const id = formData.get('id');
|
||||
if (typeof id !== 'string' || id.length === 0) {
|
||||
return fail(400, { error: 'missing id' });
|
||||
}
|
||||
const result = await api.PATCH('/api/notifications/{id}/read', {
|
||||
params: { path: { id } }
|
||||
});
|
||||
if (!result.response.ok) {
|
||||
return fail(result.response.status, { error: 'failed' });
|
||||
}
|
||||
return { success: true };
|
||||
},
|
||||
'mark-all': async ({ fetch }) => {
|
||||
const api = createApiClient(fetch);
|
||||
const result = await api.POST('/api/notifications/read-all');
|
||||
if (!result.response.ok) {
|
||||
return fail(result.response.status, { error: 'failed' });
|
||||
}
|
||||
return { success: true };
|
||||
}
|
||||
};
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
<script lang="ts">
|
||||
import { onMount, onDestroy, tick } from 'svelte';
|
||||
import { onMount, onDestroy } from 'svelte';
|
||||
import { goto } from '$app/navigation';
|
||||
import { page } from '$app/state';
|
||||
import * as m from '$lib/paraglide/messages.js';
|
||||
@@ -92,37 +92,9 @@ async function onMarkAllRead() {
|
||||
await notificationStore.markAllRead();
|
||||
}
|
||||
|
||||
// "Mehr laden" pagination
|
||||
let isLoadingMore = $state(false);
|
||||
let loadMoreBtn = $state<HTMLButtonElement | null>(null);
|
||||
let paginatedFeed = $state<ActivityFeedItemDTO[]>([]);
|
||||
let announcement = $state('');
|
||||
|
||||
const mergedFeed = $derived<ActivityFeedItemDTO[]>([...data.activityFeed, ...paginatedFeed]);
|
||||
|
||||
async function loadMore() {
|
||||
if (isLoadingMore) return;
|
||||
isLoadingMore = true;
|
||||
try {
|
||||
const res = await fetch(`/api/dashboard/activity?limit=40&offset=${mergedFeed.length}`, {
|
||||
credentials: 'same-origin'
|
||||
});
|
||||
if (!res.ok) throw new Error('load failed');
|
||||
const next = (await res.json()) as ActivityFeedItemDTO[];
|
||||
paginatedFeed = [...paginatedFeed, ...next];
|
||||
announcement = m.chronik_load_more_announcement({ count: next.length });
|
||||
} catch {
|
||||
// Keep silent in the unit path — the error card handles load failures.
|
||||
} finally {
|
||||
isLoadingMore = false;
|
||||
await tick();
|
||||
loadMoreBtn?.focus();
|
||||
}
|
||||
}
|
||||
|
||||
const displayFeed = $derived<ActivityFeedItemDTO[]>(
|
||||
(() => {
|
||||
const merged = mergedFeed;
|
||||
const merged = data.activityFeed;
|
||||
switch (activeFilter) {
|
||||
case 'alle':
|
||||
return merged;
|
||||
@@ -177,32 +149,6 @@ function retry() {
|
||||
</div>
|
||||
{:else}
|
||||
<ChronikTimeline items={displayFeed} />
|
||||
|
||||
<div aria-live="polite" class="sr-only">{announcement}</div>
|
||||
|
||||
<div class="mt-6 text-center">
|
||||
<button
|
||||
type="button"
|
||||
bind:this={loadMoreBtn}
|
||||
onclick={loadMore}
|
||||
aria-busy={isLoadingMore}
|
||||
disabled={isLoadingMore}
|
||||
class="rounded-sm border border-line px-4 py-3 font-sans text-sm text-ink-2 transition-colors hover:bg-muted disabled:opacity-60"
|
||||
>
|
||||
{isLoadingMore ? m.chronik_loading() : m.chronik_load_more()}
|
||||
</button>
|
||||
|
||||
{#if isLoadingMore}
|
||||
<ul aria-hidden="true" class="mt-3 flex flex-col gap-2">
|
||||
{#each [0, 1, 2] as i (i)}
|
||||
<li
|
||||
data-testid="chronik-skeleton-row"
|
||||
class="h-[72px] rounded-sm border border-line bg-muted/40"
|
||||
></li>
|
||||
{/each}
|
||||
</ul>
|
||||
{/if}
|
||||
</div>
|
||||
{/if}
|
||||
{/if}
|
||||
</main>
|
||||
|
||||
@@ -1,10 +1,8 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { actions, load } from './+page.server';
|
||||
import { load } from './+page.server';
|
||||
|
||||
const mockApi = {
|
||||
GET: vi.fn(),
|
||||
POST: vi.fn(),
|
||||
PATCH: vi.fn()
|
||||
GET: vi.fn()
|
||||
};
|
||||
|
||||
vi.mock('$lib/api.server', () => ({
|
||||
@@ -93,37 +91,3 @@ describe('chronik/load', () => {
|
||||
expect(invalidResult.filter).toBe('alle');
|
||||
});
|
||||
});
|
||||
|
||||
describe('chronik/actions', () => {
|
||||
it('dismiss: PATCHes /api/notifications/{id}/read with the form id', async () => {
|
||||
mockApi.PATCH.mockResolvedValue({ response: { ok: true } });
|
||||
const formData = new FormData();
|
||||
formData.set('id', 'n-42');
|
||||
|
||||
const result = await actions.dismiss({
|
||||
request: { formData: async () => formData },
|
||||
fetch
|
||||
} as never);
|
||||
|
||||
expect(mockApi.PATCH).toHaveBeenCalledWith('/api/notifications/{id}/read', {
|
||||
params: { path: { id: 'n-42' } }
|
||||
});
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
|
||||
it('dismiss: fails with 400 when id is missing', async () => {
|
||||
const formData = new FormData();
|
||||
const result = await actions.dismiss({
|
||||
request: { formData: async () => formData },
|
||||
fetch
|
||||
} as never);
|
||||
expect((result as { status: number }).status).toBe(400);
|
||||
});
|
||||
|
||||
it('mark-all: POSTs /api/notifications/read-all', async () => {
|
||||
mockApi.POST.mockResolvedValue({ response: { ok: true } });
|
||||
const result = await actions['mark-all']({ fetch } as never);
|
||||
expect(mockApi.POST).toHaveBeenCalledWith('/api/notifications/read-all');
|
||||
expect(result).toEqual({ success: true });
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user