Share one date formatter, and stop describing files that are gone

Two components each carried the same four lines turning a date into an ISO string from local parts. They now share one helper, deliberately separate from the one that formats in UTC: substituting one for the other shifts the day by one anywhere east of Greenwich, Sweden included, so a test pins the difference rather than a comment asking nicely. Creating a period ran its own copy of the sync-and-report sequence purely so it could name a wish the lookup could not yet see; it now passes that wish to the ordinary path instead. Two comments still explained their relationship to components deleted some time ago, which would send the next reader looking for files that are not there, and some styling outlived the markup it described.
2026-09-18 01:12:58 +02:00
parent 9ac67933d4
commit f2b3920cbc
8 changed files with 112 additions and 99 deletions
@@ -13,9 +13,9 @@ export interface SeasonPreset {
 }

 /**
- * The four season quick-picks, copied from `SEASON_PRESETS` in
- * `WishEditor.tsx` (PLANNING_ENGINE_SPEC.md §10.7) — deliberately NOT
- * imported from there since that component is owned by another task.
+ * The four season quick-picks (PLANNING_ENGINE_SPEC.md §10.7). This is the
+ * sole copy of `SEASON_PRESETS` — the `WishEditor.tsx` component it was
+ * originally copied from has since been deleted.
 * Every one of these is a `flexible` wish: the dates below are the bounding
 * range the allocator searches within for the best-costing placement, not
 * the final dates. A fixed (exact-dates) period is what dragging on the
@@ -1,7 +1,7 @@
 import { ReactNode, useMemo } from 'react'
 import type { PlannedLeaveDay, CalendarDayEntry } from '@/types'
 import { getColorForType, getLabelForType } from '@/utils/leaveTypes'
-import { getISOWeekNumber } from '@/utils/dates'
+import { getISOWeekNumber, formatDateLocalISO } from '@/utils/dates'
 import { useTranslation } from '@/i18n'
 import { CalendarDay } from './CalendarDay'
 import { CalendarEvent } from './CalendarEvent'
@@ -290,7 +290,7 @@ export function CalendarWeek({
            <CalendarDay
              date={date}
              isCurrentMonth={isCurrentMonth && dayIsCurrentMonth}
-              isToday={date === getTodayString()}
+              isToday={date === formatDateLocalISO(new Date())}
              isSelected={selectedDates.has(date)}
              dayType={calendarInfo?.dayType}
              holidayName={calendarInfo?.name ?? null}
@@ -392,11 +392,3 @@ export function CalendarWeek({
    </div>
  )
 }
-
-function getTodayString(): string {
-  const today = new Date()
-  const year = today.getFullYear()
-  const month = String(today.getMonth() + 1).padStart(2, '0')
-  const day = String(today.getDate()).padStart(2, '0')
-  return `${year}-${month}-${day}`
-}
@@ -1,10 +1,8 @@
 /**
 * Folds the wished-vs-funded/surplus/balance content that used to live only
- * on the Plan tab's `PlanResults.tsx` into Översikt, collapsed below the
- * fold. Read `PlanResults.tsx` for the reference implementation; this is a
- * separate, trimmed-down component (not a shared import) because
- * `PlanResults.tsx` is scheduled for deletion by another task and must not
- * be edited here.
+ * on the Plan tab's `PlanResults.tsx` (since deleted) into Översikt,
+ * collapsed below the fold. This is now the sole implementation of that
+ * content — a trimmed-down component, not a shared import.
 *
 * Read-only. The calendar's `POST /api/plan/sync` (see `Calendar.tsx`) is
 * now the only way a plan is written — it proposes and persists in one
@@ -16,14 +14,14 @@
 * like a proposal, and `/api/plan/propose` is documented read-only —
 * "persists nothing, safe to spam" (`backend/app/routers/plan.py`).
 *
- * Dropped relative to `PlanResults.tsx` (kept there / on the Plan tab
- * instead, to avoid duplicating content the Hero and `Age4Bars` already
- * cover): the warnings section, the age4Projections table, and the
- * per-window explanation/remedy detail cards. Also dropped, as of the move
- * to `/api/plan/sync`: the apply button, collision-confirmation UI, and
- * rejection rendering — applying is no longer a thing this fold does (see
- * `frontend/src/utils/rejectionMessages.ts` for where rejection rendering
- * now lives, for the sync toast in `Calendar.tsx`).
+ * Dropped relative to the old `PlanResults.tsx` (content not duplicated here
+ * since the Hero and `Age4Bars` already cover it): the warnings section, the
+ * age4Projections table, and the per-window explanation/remedy detail cards.
+ * Also dropped, as of the move to `/api/plan/sync`: the apply button,
+ * collision-confirmation UI, and rejection rendering — applying is no
+ * longer a thing this fold does (see `frontend/src/utils/rejectionMessages.ts`
+ * for where rejection rendering now lives, for the sync toast in
+ * `Calendar.tsx`).
 */
 import { ReactNode, useEffect } from 'react'
 import type { PersonalData, SurplusAdvice, WindowOutcome } from '@/types'
@@ -34,9 +32,9 @@ import { resolveWishName } from '@/utils/wishName'

 type Translate = (key: TranslationKey, params?: TranslationParams) => string

-// §9.2 surplus-advice kinds — mirrors `PlanResults.tsx`'s `SURPLUS_ADVICE_KEYS`/
-// `SURPLUS_ADVICE_BODY_KEYS` (the two catalogues are independent copies, not
-// a shared import — see the file banner above).
+// §9.2 surplus-advice kinds — originally mirrored the deleted `PlanResults.tsx`'s
+// `SURPLUS_ADVICE_KEYS`/`SURPLUS_ADVICE_BODY_KEYS`; this is now the sole copy
+// (see the file banner above).
 const SURPLUS_ADVICE_KEYS: Record<string, TranslationKey> = {
  wish_more_leave: 'planResults.surplusAdviceWishMoreLeave',
  shift_mix_to_fp: 'planResults.surplusAdviceShiftMixToFp',
@@ -30,7 +30,7 @@ import { PeriodPopover, type PeriodPopoverSavePayload } from '@/components/calen
 import { PresetRail, type SeasonPreset } from '@/components/calendar/PresetRail'
 import { getColorForType } from '@/utils/leaveTypes'
 import { resolveWishName, resolveWishNameFromData } from '@/utils/wishName'
-import { datesInRange, formatDateForDisplay } from '@/utils/dates'
+import { datesInRange, formatDateForDisplay, formatDateLocalISO } from '@/utils/dates'
 import { renderRejection, type Translate } from '@/utils/rejectionMessages'
 import { useTranslation } from '@/i18n'
 import './calendar.css'
@@ -285,7 +285,17 @@ export function Calendar(): ReactNode {
  const runSync = useCallback(
    async (
      workspaceId: string,
-      options?: { silent?: boolean; undo?: CalendarToast['undo']; watchWishId?: string },
+      options?: {
+        silent?: boolean
+        undo?: CalendarToast['undo']
+        watchWishId?: string
+        // Used right after creating a wish, where `wishesHook.data` is
+        // whatever it was as of this closure's own render (captured before
+        // the create's background refetch lands) and so never actually
+        // contains a wish created moments ago. Passing the just-created
+        // wish object directly sidesteps that — see `buildSyncToastMessage`.
+        watchWish?: LeaveWish
+      },
    ): Promise<PlanSyncResponse | undefined> => {
      try {
        const result = await syncHook.sync(workspaceId)
@@ -297,6 +307,7 @@ export function Calendar(): ReactNode {
            membersHook.data,
            options?.watchWishId,
            t,
+            options?.watchWish,
          )
          if (message) showToast(message, options?.undo ?? null)
        }
@@ -319,46 +330,6 @@ export function Calendar(): ReactNode {
    void runSync(workspace.id, { silent: true })
  }, [workspace?.id, runSync])

-  // Runs the sync that persists a just-created wish, then reports it and
-  // wires up undo — split out from a plain `runSync(..., { undo })` call
-  // because undo here needs to pass the just-created wish's own fields
-  // (`created`) to `buildSyncToastMessage` rather than looking `wishId` up
-  // in `wishesHook.data`, which is whatever it was as of this closure's own
-  // render (captured before the create's background refetch lands) and so
-  // never actually contains a wish created moments ago.
-  //
-  // Undo itself is just "delete the wish, re-sync": the backend deletes
-  // every `planned_leave_day` row the wish produced as part of deleting the
-  // wish (`LeaveWishService.delete()`), so there is nothing left here for
-  // the frontend to individually track or clean up.
-  const runSyncAfterWishCreate = useCallback(
-    async (workspaceId: string, created: LeaveWish): Promise<PlanSyncResponse | undefined> => {
-      const wishId = created.id
-      const result = await runSync(workspaceId, { silent: true })
-      if (!result) return result
-
-      const message = buildSyncToastMessage(
-        result,
-        wishesHook.data,
-        membersHook.data,
-        wishId,
-        t,
-        created,
-      )
-      if (message) {
-        showToast(message, {
-          label: t('calendar.undoButton'),
-          run: async () => {
-            await wishesHook.remove(wishId)
-            await runSync(workspaceId, { silent: true })
-          },
-        })
-      }
-      return result
-    },
-    [runSync, wishesHook, membersHook.data, t, showToast],
-  )
-
  const handleCreatePeriodFromDrag = useCallback(
    async (dates: string[]) => {
      setSelectedDates(new Set())
@@ -393,7 +364,21 @@ export function Calendar(): ReactNode {
          parentPersonalDataUuid: personalDataUuid,
          windowMode: 'fixed',
        })
-        await runSyncAfterWishCreate(workspace.id, created)
+        // Undo is just "delete the wish, re-sync": the backend deletes every
+        // `planned_leave_day` row the wish produced as part of deleting the
+        // wish (`LeaveWishService.delete()`), so there is nothing left here
+        // for the frontend to individually track or clean up.
+        await runSync(workspace.id, {
+          watchWishId: created.id,
+          watchWish: created,
+          undo: {
+            label: t('calendar.undoButton'),
+            run: async () => {
+              await wishesHook.remove(created.id)
+              await runSync(workspace.id, { silent: true })
+            },
+          },
+        })
      } catch (err) {
        showToast(err instanceof Error ? err.message : t('calendar.periodCreateError'), null)
      } finally {
@@ -406,7 +391,7 @@ export function Calendar(): ReactNode {
      personalDataUuid,
      wishesHook,
      t,
-      runSyncAfterWishCreate,
+      runSync,
      showToast,
    ],
  )
@@ -461,11 +446,11 @@ export function Calendar(): ReactNode {

  for (let i = 0; i < startOffset; i++) {
    const prevDate = new Date(year, month, -startOffset + i + 1)
-    currentWeek.push(formatDate(prevDate))
+    currentWeek.push(formatDateLocalISO(prevDate))
  }

  for (let day = 1; day <= daysInMonth; day++) {
-    currentWeek.push(formatDate(new Date(year, month, day)))
+    currentWeek.push(formatDateLocalISO(new Date(year, month, day)))
    if (currentWeek.length === 7) {
      weeks.push(currentWeek)
      currentWeek = []
@@ -474,7 +459,7 @@ export function Calendar(): ReactNode {

  const remaining = 7 - currentWeek.length
  for (let i = 0; i < remaining; i++) {
-    currentWeek.push(formatDate(new Date(year, month + 1, i + 1)))
+    currentWeek.push(formatDateLocalISO(new Date(year, month + 1, i + 1)))
  }
  weeks.push(currentWeek)

@@ -764,7 +749,17 @@ export function Calendar(): ReactNode {
        windowMode: 'flexible',
      })

-      const result = await runSyncAfterWishCreate(workspace.id, created)
+      const result = await runSync(workspace.id, {
+        watchWishId: created.id,
+        watchWish: created,
+        undo: {
+          label: t('calendar.undoButton'),
+          run: async () => {
+            await wishesHook.remove(created.id)
+            await runSync(workspace.id, { silent: true })
+          },
+        },
+      })

      if (result) {
        const placedDates = result.changes
@@ -1018,10 +1013,3 @@ export function Calendar(): ReactNode {
    </Layout>
  )
 }
-
-function formatDate(date: Date): string {
-  const year = date.getFullYear()
-  const month = String(date.getMonth() + 1).padStart(2, '0')
-  const day = String(date.getDate()).padStart(2, '0')
-  return `${year}-${month}-${day}`
-}
@@ -304,10 +304,6 @@
  color: var(--c-danger);
 }

-.plan-fold__success {
-  color: var(--c-success);
-}
-
 /* Button Styles */
 button {
  background-color: var(--c-accent);
@@ -142,12 +142,6 @@
    font-size: 36px;
  }

-  /* Progress chart */
-  .progress-chart__stats {
-    flex-direction: column;
-    gap: var(--sp-2);
-  }
-
  /* Calendar responsiveness */
  .calendar__grid {
    font-size: var(--fs-sm);
@@ -193,10 +187,6 @@
    animation: none;
  }

-  .progress-chart__fill {
-    transition: none;
-  }
-
  .header__nav-link,
  .header__logout-button,
  .header__child-select,
@@ -45,6 +45,25 @@ export function formatDateForInput(date: string | Date): string {
  return `${year}-${month}-${day}`
 }

+/**
+ * Format a Date's *local* calendar date as YYYY-MM-DD, using
+ * getFullYear/getMonth/getDate rather than the UTC getters.
+ *
+ * This is deliberately NOT `formatDateForInput`, which uses UTC getters and
+ * assumes the Date was built from an ISO string at UTC midnight. Callers
+ * here (the calendar grid) build Dates from local year/month/day components
+ * (`new Date(year, month, day)`), so reading them back with UTC getters
+ * would shift the date by one day in any timezone ahead of UTC (Sweden
+ * included). Do not "consolidate" this with `formatDateForInput` — they
+ * serve different Date constructions and are not interchangeable.
+ */
+export function formatDateLocalISO(date: Date): string {
+  const year = date.getFullYear()
+  const month = String(date.getMonth() + 1).padStart(2, '0')
+  const day = String(date.getDate()).padStart(2, '0')
+  return `${year}-${month}-${day}`
+}
+
 /**
 * Parse an ISO date string (YYYY-MM-DD) to a Date object.
 */
@@ -1,7 +1,8 @@
-import { describe, it, expect } from 'vitest'
+import { describe, it, expect, afterEach, vi } from 'vitest'
 import {
  formatDateForDisplay,
  formatDateForInput,
+  formatDateLocalISO,
  parseInputDate,
  isValidDate,
  getDayName,
@@ -81,6 +82,35 @@ describe('dates.ts utilities', () => {
    })
  })

+  describe('formatDateLocalISO', () => {
+    afterEach(() => {
+      vi.unstubAllEnvs()
+    })
+
+    it('formats a Date built from local year/month/day components', () => {
+      const date = new Date(2026, 5, 15) // local June 15, 2026
+      expect(formatDateLocalISO(date)).toBe('2026-06-15')
+    })
+
+    it('pads single-digit month and day', () => {
+      const date = new Date(2026, 0, 5) // local January 5, 2026
+      expect(formatDateLocalISO(date)).toBe('2026-01-05')
+    })
+
+    it('reads the local calendar date, not the UTC one — this is the whole point of the function', () => {
+      // Pacific/Kiritimati is UTC+14. A moment at 23:30 UTC on Jan 1 is
+      // already local Jan 2 there. `formatDateForInput` (UTC getters) must
+      // report Jan 1; `formatDateLocalISO` (local getters) must report
+      // Jan 2. If this ever collapses to the same value, someone has
+      // swapped in the UTC variant and reintroduced the off-by-one-day bug
+      // this function exists to avoid.
+      vi.stubEnv('TZ', 'Pacific/Kiritimati')
+      const date = new Date(Date.UTC(2026, 0, 1, 23, 30))
+      expect(formatDateForInput(date)).toBe('2026-01-01')
+      expect(formatDateLocalISO(date)).toBe('2026-01-02')
+    })
+  })
+
  describe('parseInputDate', () => {
    it('parses ISO date string to Date object', () => {
      const result = parseInputDate('2026-06-15')