fix(admin): stop marking events expired up to 24h early (#909) (stable) (#917)

* fix(admin): stop marking events expired up to 24h early (#909) (stable) differenceInDays truncates to whole days, so an event expiring in a few hours returned 0 and three admin surfaces treated it as gone: - EventsListPage: status chip said 'Expired' (days <= 0) while the public gallery — which compares real timestamps — correctly showed 'expires in X hours'. This is the reporter's exact symptom. - EventDetailsPage: same isExpired math on the detail view. - AdminDashboard: the expiring-soon card showed '0 days left' on the final day. Expired is now gated on the actual timestamp (expires_at <= now), and the countdown chips use ceiling days so the last day reads '1 day left' instead of flipping to Expired/0. * fix(admin): drop already-expired events from the dashboard card (#909 review round) The expiring-soon card ran Math.max(1, ceil(delta)), so an event that expired while the dashboard sat open (its query isn't polled) showed '1 day left' indefinitely from the stale cached row. Expired rows are now filtered out before render; the delta is therefore always positive and the clamp is gone. * fix(admin): refresh expiry status live at the boundary (#909 review round 2) Two review findings on the admin expiry surfaces: - The dashboard 'expiring soon' card, list badges, and detail banner are all computed inline from Date.now() at render, so a page left open across an event's expiry kept showing 'active'/'1 day left' until an unrelated render — which for editor/viewer roles (no health poll) never happens. - My round-1 client-side filter on the dashboard desynced the visible list from the cached total/stat ('no events expiring' beside 'view all N'). Both are fixed by new useExpiryRefresh: it fires once at the soonest future expiry (setTimeout, overflow-guarded). The dashboard refetches its expiring + stats queries — the backend already excludes expired events, so rows/total/stats come back consistent (filter removed). The list and detail pages bump a tick so the inline badges recompute. Hooks are placed above the loading early-returns (rules-of-hooks is disabled in eslint, so this was a latent crash otherwise). * fix(admin): expiry-refresh precision + filtered refetch (#909 review round 3) Three refinements to round-2's live-expiry work: - useExpiryRefresh now re-arms past setTimeout's ~24.8-day overflow limit (capped wake-up that re-evaluates) instead of dropping the timer, so a page mounted for weeks still updates. - The dashboard requests the expiring list ordered by expires_at asc, so the five shown rows ARE the soonest to expire — the timer schedules against the true next boundary even when >5 events are expiring (getEvents gains optional sortBy/sortOrder; backend already whitelists expires_at). - EventsListPage refetches instead of only re-rendering at the boundary: under the 'expiring' filter the backend drops expired rows, so a plain tick would leave a stale 'Expired' row + total. refetch keeps rows and totals correct under every filter. --------- Co-authored-by: Paul Nothaft <[email protected]>
2026-07-30 12:14:54 +02:00
parent b32ba1ed6b
commit 6891769124
5 changed files with 117 additions and 14 deletions
@@ -0,0 +1,43 @@
+import { useEffect, useState } from 'react';
+
+// setTimeout stores its delay in a signed 32-bit int; anything larger
+// overflows and fires immediately. Events expiring weeks out don't need a
+// live tick anyway, so we simply don't schedule past this horizon.
+const MAX_TIMEOUT_MS = 2 ** 31 - 1;
+
+/**
+ * Fire `onExpiry` once, at the soonest future timestamp in `timestamps`
+ * (#909 review). Admin expiry badges are computed inline from Date.now()
+ * at render time, so without this a page left mounted across an event's
+ * expiry keeps showing the stale "active"/"1 day left" state until an
+ * unrelated render happens — which for editor/viewer roles (no health
+ * poll) may never occur. When the callback updates state/data, the next
+ * expiry reschedules automatically.
+ */
+export function useExpiryRefresh(
+  timestamps: Array<string | null | undefined>,
+  onExpiry: () => void,
+): void {
+  const next = timestamps
+    .map((t) => (t ? new Date(t).getTime() : NaN))
+    .filter((n) => Number.isFinite(n) && n > Date.now())
+    .sort((a, b) => a - b)[0];
+
+  // Bumped by a capped wake-up so the effect re-evaluates and re-arms when
+  // the target is further out than a single setTimeout can represent.
+  const [rearm, setRearm] = useState(0);
+  useEffect(() => {
+    if (next === undefined) return;
+    // +1s so the timer lands just past the boundary, not exactly on it.
+    const delay = next - Date.now() + 1000;
+    if (delay > MAX_TIMEOUT_MS) {
+      // Too far for one timer (setTimeout overflows past ~24.8 days and
+      // fires immediately). Wake at the cap and re-arm with a now-smaller
+      // remaining delay, so a page left mounted for weeks still updates.
+      const id = window.setTimeout(() => setRearm((n) => n + 1), MAX_TIMEOUT_MS);
+      return () => window.clearTimeout(id);
+    }
+    const id = window.setTimeout(onExpiry, Math.max(0, delay));
+    return () => window.clearTimeout(id);
+  }, [next, onExpiry, rearm]);
+}
@@ -1,4 +1,4 @@
-import React from 'react';
+import React, { useCallback } from 'react';
 import { useNavigate } from 'react-router-dom';
 import { 
  Calendar, 
@@ -15,7 +15,9 @@ import {
  Check,
  X
 } from 'lucide-react';
-import { differenceInDays, parseISO } from 'date-fns';
+import { parseISO } from 'date-fns';
+import { useQueryClient } from '@tanstack/react-query';
+import { useExpiryRefresh } from '../../hooks/useExpiryRefresh';
 import { useTranslation } from 'react-i18next';
 import { useLocalizedDate } from '../../hooks/useLocalizedDate';
 import { useMutationWithToast } from '../../hooks';
@@ -76,9 +78,29 @@ export const AdminDashboard: React.FC = () => {
  // which silently missed any expiring event outside the first 100 rows.
  const { data: expiringEventsData, isLoading: eventsLoading } = useQuery({
    queryKey: ['admin-events-summary', 'expiring'],
-    queryFn: () => eventsService.getEvents(1, 5, 'expiring'),
+    // Order by soonest expiry so the five shown rows ARE the earliest to
+    // expire — useExpiryRefresh then schedules against the true next boundary
+    // even when >5 events are expiring (#909 review round 3).
+    queryFn: () => eventsService.getEvents(1, 5, 'expiring', undefined, 'expires_at', 'asc'),
  });

+  // Keep the "expiring soon" card honest when a row crosses its expiry while
+  // the dashboard sits open (#909 review). Filtering client-side desynced the
+  // list from the cached total/stat; instead we refetch the whole set at the
+  // boundary — the backend returns rows/total/stats that already exclude the
+  // now-expired event, so everything stays consistent. Fixes the stale
+  // "1 day left" for roles without the health poll (editor/viewer). Placed
+  // with the other top-level hooks, above the loading early-return.
+  const queryClient = useQueryClient();
+  const refreshExpiring = useCallback(() => {
+    queryClient.invalidateQueries({ queryKey: ['admin-events-summary', 'expiring'] });
+    queryClient.invalidateQueries({ queryKey: ['admin-dashboard-stats'] });
+  }, [queryClient]);
+  useExpiryRefresh(
+    (expiringEventsData?.events ?? []).map((e: any) => e.expires_at),
+    refreshExpiring,
+  );
+
  // Pending workflow approvals — only when the workflow engine is live. These
  // are the human-in-the-loop gates (e.g. "review invoice before sending").
  const { flags } = useFeatureFlags();
@@ -237,7 +259,10 @@ export const AdminDashboard: React.FC = () => {
            ) : (
              <div className="space-y-3">
                {expiringEvents.map((event) => {
-                  const daysLeft = differenceInDays(parseISO(event.expires_at!), new Date());
+                  // Ceiling so the final partial day reads "1 day", not "0"
+                  // (#909); clamped since a row can sit at the boundary for the
+                  // instant before useExpiryRefresh refetches it away.
+                  const daysLeft = Math.max(1, Math.ceil((parseISO(event.expires_at!).getTime() - Date.now()) / 86400000));

                  return (
                    <div
@@ -1,7 +1,7 @@
-import React, { useState, useEffect, useMemo } from 'react';
+import React, { useState, useEffect, useMemo, useCallback } from 'react';
+import { useExpiryRefresh } from '../../hooks/useExpiryRefresh';
 import { useParams, useNavigate } from 'react-router-dom';
 import { useTranslation } from 'react-i18next';
-import { differenceInDays } from 'date-fns';
 import { toast } from 'react-toastify';
 import { useLocalizedDate } from '../../hooks/useLocalizedDate';

@@ -101,6 +101,13 @@ export const EventDetailsPage: React.FC = () => {
    enabled: !!id,
  });

+  // Flip the expiry banner live when the timestamp passes with the page open
+  // (#909 review) — isExpired further down is computed inline from Date.now().
+  // Kept here with the other hooks, above the loading early-return.
+  const [, setExpiryTick] = useState(0);
+  const bumpExpiryTick = useCallback(() => setExpiryTick((n) => n + 1), []);
+  useExpiryRefresh([event?.expires_at], bumpExpiryTick);
+
  // Fetch feedback settings
  const { data: eventFeedbackSettings } = useQuery({
    queryKey: ['admin-event-feedback-settings', id],
@@ -277,9 +284,14 @@ export const EventDetailsPage: React.FC = () => {
  }

  const expiresAtDate = safeParseDate(event.expires_at);
-  const daysUntilExpiration = expiresAtDate ? differenceInDays(expiresAtDate, new Date()) : null;
-  const isExpired = daysUntilExpiration !== null && daysUntilExpiration <= 0;
-  const isExpiring = daysUntilExpiration !== null && daysUntilExpiration > 0 && daysUntilExpiration <= 7;
+  // Timestamp comparison, not truncated whole days (#909): the old
+  // differenceInDays <= 0 marked events "expired" up to 24h early.
+  // Ceiling keeps the countdown at "1 day" through the final day.
+  const isExpired = expiresAtDate !== null && expiresAtDate.getTime() <= Date.now();
+  const daysUntilExpiration = expiresAtDate
+    ? Math.ceil((expiresAtDate.getTime() - Date.now()) / 86400000)
+    : null;
+  const isExpiring = !isExpired && daysUntilExpiration !== null && daysUntilExpiration > 0 && daysUntilExpiration <= 7;

  const handleStartEdit = () => {
    setEditForm({
@@ -1,4 +1,5 @@
 import React, { useState, useEffect } from 'react';
+import { useExpiryRefresh } from '../../hooks/useExpiryRefresh';
 import { useNavigate, useSearchParams } from 'react-router-dom';
 import {
  Plus,
@@ -18,7 +19,7 @@ import {
  ChevronLeft,
  ChevronRight
 } from 'lucide-react';
-import { parseISO, differenceInDays } from 'date-fns';
+import { parseISO } from 'date-fns';
 import { toast } from 'react-toastify';
 import { useModal, useMutationWithToast } from '../../hooks';
 import { useLocalizedDate } from '../../hooks/useLocalizedDate';
@@ -141,7 +142,7 @@ export const EventsListPage: React.FC = () => {

  // Fetch events — fully server-side: pagination, status filter, and search
  // (#346 — counters and search were previously bounded to the first 100 rows).
-  const { data, isLoading, error } = useQuery({
+  const { data, isLoading, error, refetch } = useQuery({
    queryKey: ['admin-events', statusFilter ?? 'all', debouncedSearchTerm, page],
    queryFn: () => eventsService.getEvents(page, PAGE_SIZE, statusFilter, debouncedSearchTerm || undefined),
    placeholderData: (prev) => prev,
@@ -230,6 +231,14 @@ export const EventsListPage: React.FC = () => {
  // Filtering and searching now happen server-side. Use the response directly,
  // ordered as the backend returned them (created_at desc by default).
  const events: Event[] = data?.events ?? [];
+
+  // Refetch when the soonest event expiry passes (#909 review): the status
+  // badge is computed inline from Date.now(), and under the "expiring" filter
+  // the backend drops the row once expires_at <= now — so a plain re-render
+  // would leave a stale "Expired" row (and total) in that filtered view.
+  // refetch() re-runs with the current page/filter/search: rows and totals
+  // both correct under every filter.
+  useExpiryRefresh(events.map((e) => e.expires_at), refetch);
  const pagination = data?.pagination;
  const totalPages = pagination?.totalPages ?? 1;
  const filteredCount = pagination?.total ?? 0;
@@ -258,8 +267,14 @@ export const EventsListPage: React.FC = () => {

    if (!event.expires_at) return { label: t('events.active'), color: 'text-green-600 dark:text-green-400 bg-green-100 dark:bg-green-900/40' };

-    const days = differenceInDays(parseISO(event.expires_at), new Date());
-    if (days <= 0) return { label: t('events.expired'), color: 'text-red-600 dark:text-red-400 bg-red-100 dark:bg-red-900/40' };
+    // Expired means the timestamp has actually passed (#909):
+    // differenceInDays truncates to whole days, so an event expiring in a
+    // few hours returned 0 and showed "Expired" while the public gallery
+    // (which compares real timestamps) correctly showed it active.
+    const expiresAt = parseISO(event.expires_at);
+    if (expiresAt.getTime() <= Date.now()) return { label: t('events.expired'), color: 'text-red-600 dark:text-red-400 bg-red-100 dark:bg-red-900/40' };
+    // Ceiling so the last day reads "1 day left", never "0 days".
+    const days = Math.ceil((expiresAt.getTime() - Date.now()) / 86400000);
    if (days <= 7) return { label: t('events.daysLeft', { count: days }), color: 'text-orange-600 dark:text-orange-400 bg-orange-100 dark:bg-orange-900/40' };

    return { label: t('events.active'), color: 'text-green-600 dark:text-green-400 bg-green-100 dark:bg-green-900/40' };
@@ -91,7 +91,9 @@ export const eventsService = {
    page: number = 1,
    limit: number = 20,
    status?: EventStatusFilter,
-    search?: string
+    search?: string,
+    sortBy?: string,
+    sortOrder?: 'asc' | 'desc'
  ): Promise<EventsListResponse> {
    const params = new URLSearchParams({
      page: page.toString(),
@@ -104,6 +106,12 @@ export const eventsService = {
    if (search) {
      params.append('search', search);
    }
+    if (sortBy) {
+      params.append('sortBy', sortBy);
+    }
+    if (sortOrder) {
+      params.append('sortOrder', sortOrder);
+    }

    const response = await api.get<EventsListResponse>(`/admin/events?${params}`);
    const data: any = response.data;