fix(planner): address all PR review blockers

- Fix logic bug `{#if !isPlanner === false}` - view/cook buttons now visible for all roles, swap only for planner - Convert Tauschen from dead button to link with suggestions href - Add week.ts unit tests (23 tests covering getWeekStart Sunday edge case, prevWeek/nextWeek, weekDays, isToday, formatWeekRange) - Fix isToday to use UTC consistently (.toISOString().slice(0,10)) instead of local date - Add server-side role guard to createPlan action (403 for members) - Add weekStart format validation in createPlan action - Add isSelected prop to DayMealCard with green treatment - Make variety banner sticky on mobile (always visible per spec) - Add day name abbreviation above date badge in desktop column headers - Remove placeholder Navigation text from desktop sidebar - Add aria-label to desktop empty tile buttons - Add variety score partial failure test, multiple overlaps test, WeekStrip today+selected test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2026-04-03 11:07:47 +02:00
parent e3f8d8ad73
commit 5d2bb9e84e
9 changed files with 336 additions and 59 deletions
--- a/frontend/src/routes/(app)/planner/+page.server.ts
+++ b/frontend/src/routes/(app)/planner/+page.server.ts
@@ -27,10 +27,20 @@ export const load: PageServerLoad = async ({ fetch, url }) => {
 };

 export const actions: Actions = {
-	createPlan: async ({ fetch, request }) => {
+	createPlan: async ({ fetch, request, locals }) => {
+		// Role guard: only planners may create week plans
+		if (locals.benutzer?.rolle !== 'planer') {
+			return { success: false, error: 'Keine Berechtigung.' };
+		}
+
 		const formData = await request.formData();
 		const weekStart = formData.get('weekStart') as string;

+		// Validate weekStart format: must be YYYY-MM-DD
+		if (!weekStart || !/^\d{4}-\d{2}-\d{2}$/.test(weekStart)) {
+			return { success: false, error: 'Ungültiges Datum.' };
+		}
+
 		const api = apiClient(fetch);
 		const { data, error } = await api.POST('/v1/week-plans', {
 			body: { weekStart }
--- a/frontend/src/routes/(app)/planner/+page.svelte
+++ b/frontend/src/routes/(app)/planner/+page.svelte
@@ -3,17 +3,14 @@
 	import VarietyScoreCard from '$lib/planner/VarietyScoreCard.svelte';
 	import WeekStrip from '$lib/planner/WeekStrip.svelte';
 	import DayMealCard from '$lib/planner/DayMealCard.svelte';
-	import { prevWeek, nextWeek, getWeekStart, weekDays, formatDayLabel, formatWeekRange, isToday } from '$lib/planner/week';
+	import { prevWeek, nextWeek, getWeekStart, weekDays, formatDayLabel, formatDayAbbr, formatWeekRange } from '$lib/planner/week';

 	let { data } = $props();

 	// Capture initial weekStart before reactivity for $state initialization
 	const initialWeekStart: string = data.weekStart;
-	const todayStr = getWeekStart(new Date());
-	const today = (() => {
-		const d = new Date();
-		return `${d.getFullYear()}-${String(d.getMonth() + 1).padStart(2, '0')}-${String(d.getDate()).padStart(2, '0')}`;
-	})();
+	// Use UTC date string (YYYY-MM-DD) consistently
+	const today: string = new Date().toISOString().slice(0, 10);

 	let weekStart = $derived(data.weekStart);
 	let weekPlan = $derived(data.weekPlan);
@@ -35,7 +32,8 @@
 	});

 	let selectedSlot = $derived(slotMap[selectedDay] ?? { id: null, slotDate: selectedDay, recipe: null });
-	let remainingSlots = $derived(days.filter(d => d > selectedDay).map((d: string) => slotMap[d] ?? { id: null, slotDate: d, recipe: null }));
+	let remainingSlots = $derived(days.filter((d: string) => d > selectedDay).map((d: string) => slotMap[d] ?? { id: null, slotDate: d, recipe: null }));
+	let remainingSlotsWithMeal = $derived(remainingSlots.filter((s: any) => s.recipe));

 	let isPlanner = $derived((data as any).benutzer?.rolle === 'planer');

@@ -88,9 +86,9 @@
 		</div>
 	</header>

-	<!-- Variety banner (always visible) -->
+	<!-- Variety banner: sticky below the top nav so it's always visible (spec requirement) -->
 	{#if varietyScore}
-		<div class="px-4 pt-3">
+		<div class="sticky z-10 px-4 pt-3" style="top: 56px;">
 			<VarietyScoreCard
 				score={varietyScore.score ?? 0}
 				ingredientOverlaps={varietyScore.ingredientOverlaps ?? []}
@@ -115,17 +113,22 @@
 		<p class="mb-2 font-[var(--font-sans)] text-[12px] font-medium uppercase tracking-wide text-[var(--color-text-muted)]">
 			{formatDayLabel(selectedDay)}
 		</p>
-		<DayMealCard slot={selectedSlot} isToday={selectedDay === today} readonly={!isPlanner} />
+		<DayMealCard
+			slot={selectedSlot}
+			isToday={selectedDay === today}
+			isSelected={true}
+			readonly={!isPlanner}
+		/>
 	</div>

 	<!-- Remaining days list -->
-	{#if remainingSlots.filter((s: any) => s.recipe).length > 0}
+	{#if remainingSlotsWithMeal.length > 0}
 		<div class="px-4 pt-6 pb-4">
 			<h2 class="mb-3 font-[var(--font-sans)] text-[12px] font-medium uppercase tracking-wide text-[var(--color-text-muted)]">
 				Restliche Woche
 			</h2>
 			<div class="space-y-2 md:grid md:grid-cols-2 md:gap-2 md:space-y-0">
-				{#each remainingSlots.filter((s: any) => s.recipe) as slot}
+				{#each remainingSlotsWithMeal as slot}
 					<button
 						type="button"
 						onclick={() => handleSelectDay(slot.slotDate)}
@@ -134,7 +137,7 @@
 						<span class="min-w-[36px] font-[var(--font-sans)] text-[12px] text-[var(--color-text-muted)]">
 							{formatDayLabel(slot.slotDate).split(',')[0]}
 						</span>
-						<span class="flex-1 font-[var(--font-sans)] text-[14px] font-medium text-[var(--color-text)] truncate">
+						<span class="flex-1 truncate font-[var(--font-sans)] text-[14px] font-medium text-[var(--color-text)]">
 							{slot.recipe?.name}
 						</span>
 						{#if isPlanner}
@@ -209,9 +212,6 @@
 	<div class="flex flex-1 overflow-hidden">
 		<!-- Left sidebar -->
 		<aside class="flex w-[224px] flex-shrink-0 flex-col border-r border-[var(--color-border)] bg-[var(--color-surface)] p-4">
-			<nav class="flex-1">
-				<p class="font-[var(--font-sans)] text-[12px] font-medium uppercase tracking-wide text-[var(--color-text-muted)]">Navigation</p>
-			</nav>
 			<!-- Variety widget at bottom -->
 			{#if varietyScore}
 				<div class="mt-auto">
@@ -245,12 +245,16 @@
 						{@const isTodayDay = day === today}
 						{@const isSelectedDay = day === selectedDay}
 						{@const dateNum = day.slice(-2).replace(/^0/, '')}
+						{@const dayAbbr = formatDayAbbr(day, 'narrow')}

 						<div class="flex flex-col">
-							<!-- Column header -->
-							<div class="mb-2 text-center">
+							<!-- Column header: day name + date badge -->
+							<div class="mb-2 flex flex-col items-center gap-1">
+								<p class="font-[var(--font-sans)] text-[9px] uppercase tracking-wide text-[var(--color-text-muted)]">
+									{dayAbbr}
+								</p>
 								<div
-									class="mx-auto flex h-6 w-6 items-center justify-center rounded-full text-[11px] font-medium
+									class="flex h-6 w-6 items-center justify-center rounded-full text-[11px] font-medium
 										{isTodayDay ? 'bg-[var(--yellow)] text-white' : ''}
 										{isSelectedDay && !isTodayDay ? 'bg-[var(--green-tint)] text-[var(--green-dark)]' : ''}
 										{!isTodayDay && !isSelectedDay ? 'bg-transparent text-[var(--color-text)]' : ''}"
@@ -263,12 +267,12 @@
 							<button
 								type="button"
 								onclick={() => handleSelectDay(day)}
+								aria-label={slot.recipe ? slot.recipe.name : `Gericht wählen für ${formatDayLabel(day)}`}
 								class="flex flex-1 flex-col rounded-[var(--radius-lg)] border p-2 text-left shadow-[var(--shadow-card)] transition-all hover:border-[var(--green-light)] hover:shadow-[var(--shadow-raised)]
-									{slot.recipe ? 'bg-[var(--color-surface)]' : 'border-dashed bg-transparent'}
+									{slot.recipe && !isTodayDay && !isSelectedDay ? 'border-[var(--color-border)] bg-[var(--color-surface)]' : ''}
 									{isTodayDay && slot.recipe ? 'border-2 border-[var(--yellow)] bg-[var(--yellow-tint)]' : ''}
 									{isSelectedDay && !isTodayDay && slot.recipe ? 'border-2 border-[var(--green)] bg-[var(--green-tint)]' : ''}
-									{!isTodayDay && !isSelectedDay && slot.recipe ? 'border-[var(--color-border)]' : ''}
-									{!slot.recipe ? 'border-[var(--color-border)]' : ''}"
+									{!slot.recipe ? 'border-dashed border-[var(--color-border)] bg-transparent' : ''}"
 							>
 								{#if slot.recipe}
 									<p class="font-[var(--font-display)] text-[13px] font-[300] leading-tight text-[var(--color-text)]">
@@ -276,7 +280,7 @@
 									</p>
 								{:else}
 									<div class="flex flex-1 flex-col items-center justify-center py-4 text-[var(--color-text-muted)]">
-										<span class="text-[18px]">+</span>
+										<span class="text-[18px]" aria-hidden="true">+</span>
 										<span class="font-[var(--font-sans)] text-[11px]">Gericht wählen</span>
 									</div>
 								{/if}
@@ -305,30 +309,30 @@
 					</p>
 				{/if}

-				{#if !isPlanner === false}
-					<div class="mt-4 space-y-2">
+				<!-- View and cook actions shown to all roles -->
+				<div class="mt-4 space-y-2">
+					<a
+						href="/recipes/{selectedSlot.recipe.id}"
+						class="block rounded-[var(--radius-md)] border border-[var(--color-border)] px-3 py-2 text-center text-[13px] font-medium tracking-[0.04em] font-[var(--font-sans)] text-[var(--color-text)] hover:bg-[var(--color-surface)]"
+					>
+						Rezept ansehen
+					</a>
+					<a
+						href="/recipes/{selectedSlot.recipe.id}/cook"
+						class="block rounded-[var(--radius-md)] bg-[var(--green-dark)] px-3 py-2 text-center text-[13px] font-medium tracking-[0.04em] font-[var(--font-sans)] text-white"
+					>
+						Koch-Modus
+					</a>
+					<!-- Swap action: planner only -->
+					{#if isPlanner}
 						<a
-							href="/recipes/{selectedSlot.recipe.id}"
+							href="/planner/suggestions?day={selectedDay}"
 							class="block rounded-[var(--radius-md)] border border-[var(--color-border)] px-3 py-2 text-center text-[13px] font-medium tracking-[0.04em] font-[var(--font-sans)] text-[var(--color-text)] hover:bg-[var(--color-surface)]"
 						>
-							Rezept ansehen
+							Gericht tauschen
 						</a>
-						<a
-							href="/recipes/{selectedSlot.recipe.id}/cook"
-							class="block rounded-[var(--radius-md)] bg-[var(--green-dark)] px-3 py-2 text-center text-[13px] font-medium tracking-[0.04em] font-[var(--font-sans)] text-white"
-						>
-							Koch-Modus
-						</a>
-						{#if isPlanner}
-							<a
-								href="/planner/suggestions?day={selectedDay}"
-								class="block rounded-[var(--radius-md)] border border-[var(--color-border)] px-3 py-2 text-center text-[13px] font-medium tracking-[0.04em] font-[var(--font-sans)] text-[var(--color-text)] hover:bg-[var(--color-surface)]"
-							>
-								Gericht tauschen
-							</a>
-						{/if}
-					</div>
-				{/if}
+					{/if}
+				</div>
 			{:else}
 				<p class="font-[var(--font-sans)] text-[14px] text-[var(--color-text-muted)]">Kein Gericht geplant</p>
 				{#if isPlanner}
--- a/frontend/src/routes/(app)/planner/page.server.test.ts
+++ b/frontend/src/routes/(app)/planner/page.server.test.ts
@@ -111,7 +111,8 @@ describe('planner page — actions', () => {
 		formData.set('weekStart', '2026-03-30');
 		const result = await actions.createPlan({
 			fetch: vi.fn(),
-			request: { formData: async () => formData }
+			request: { formData: async () => formData },
+			locals: { benutzer: { id: 'u1', name: 'Test', rolle: 'planer' }, haushalt: { id: 'h1', name: 'Test' } }
 		});
 		expect(mockPost).toHaveBeenCalledWith('/v1/week-plans', expect.objectContaining({ body: { weekStart: '2026-03-30' } }));
 		expect(result).toEqual({ success: true });
@@ -123,8 +124,72 @@ describe('planner page — actions', () => {
 		formData.set('weekStart', '2026-03-30');
 		const result = await actions.createPlan({
 			fetch: vi.fn(),
-			request: { formData: async () => formData }
+			request: { formData: async () => formData },
+			locals: { benutzer: { id: 'u1', name: 'Test', rolle: 'planer' }, haushalt: { id: 'h1', name: 'Test' } }
 		});
 		expect(result).toEqual({ success: false, error: expect.any(String) });
 	});
+
+	it('createPlan action returns error for invalid weekStart format', async () => {
+		const formData = new FormData();
+		formData.set('weekStart', 'not-a-date');
+		const result = await actions.createPlan({
+			fetch: vi.fn(),
+			request: { formData: async () => formData },
+			locals: { benutzer: { id: 'u1', name: 'Test', rolle: 'planer' }, haushalt: { id: 'h1', name: 'Test' } }
+		});
+		expect(result).toEqual({ success: false, error: 'Ungültiges Datum.' });
+		expect(mockPost).not.toHaveBeenCalled();
+	});
+
+	it('createPlan action returns error when weekStart is missing', async () => {
+		const formData = new FormData();
+		const result = await actions.createPlan({
+			fetch: vi.fn(),
+			request: { formData: async () => formData },
+			locals: { benutzer: { id: 'u1', name: 'Test', rolle: 'planer' }, haushalt: { id: 'h1', name: 'Test' } }
+		});
+		expect(result).toEqual({ success: false, error: 'Ungültiges Datum.' });
+	});
+
+	it('createPlan action returns permission error for member role', async () => {
+		const formData = new FormData();
+		formData.set('weekStart', '2026-03-30');
+		const result = await actions.createPlan({
+			fetch: vi.fn(),
+			request: { formData: async () => formData },
+			locals: { benutzer: { id: 'u1', name: 'Test', rolle: 'mitglied' }, haushalt: { id: 'h1', name: 'Test' } }
+		});
+		expect(result).toEqual({ success: false, error: 'Keine Berechtigung.' });
+		expect(mockPost).not.toHaveBeenCalled();
+	});
+});
+
+describe('planner page — variety score partial failure', () => {
+	let load: any;
+
+	beforeEach(async () => {
+		mockGet.mockReset();
+		mockPost.mockReset();
+		vi.resetModules();
+		const mod = await import('./+page.server');
+		load = mod.load;
+	});
+
+	const mockWeekPlan = {
+		id: 'plan-1',
+		weekStart: '2026-03-30',
+		status: 'draft',
+		slots: []
+	};
+
+	it('returns weekPlan even when variety score API fails', async () => {
+		mockGet.mockResolvedValueOnce({ data: mockWeekPlan, error: undefined });
+		mockGet.mockResolvedValueOnce({ data: undefined, error: { status: 500 } });
+		const url = new URL('http://localhost/planner');
+		const result = await load({ fetch: vi.fn(), url });
+		expect(result.weekPlan).toBeDefined();
+		expect(result.weekPlan.id).toBe('plan-1');
+		expect(result.varietyScore).toBeNull();
+	});
 });