Fix admin reference mode regressions

2025-10-02 23:39:17 +02:00
parent fc1bf53412
commit 775e417e55
12 changed files with 2608 additions and 30 deletions
@@ -0,0 +1,207 @@
+const fs = require('fs');
+const path = require('path');
+const os = require('os');
+const express = require('express');
+const request = require('supertest');
+
+describe('Admin photos in reference mode', () => {
+  let tmpDir;
+  let storagePath;
+  let db;
+  let app;
+  let categoryId;
+
+  const resetModules = () => {
+    jest.resetModules();
+    jest.clearAllMocks();
+  };
+
+  beforeAll(async () => {
+    tmpDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'picpeak-admin-photos-'));
+    storagePath = path.join(tmpDir, 'storage');
+    await fs.promises.mkdir(storagePath, { recursive: true });
+
+    process.env.NODE_ENV = 'test';
+    process.env.TEST_DATABASE_PATH = path.join(tmpDir, 'data', 'photo_sharing_test.db');
+    await fs.promises.mkdir(path.dirname(process.env.TEST_DATABASE_PATH), { recursive: true });
+    try {
+      await fs.promises.unlink(process.env.TEST_DATABASE_PATH);
+    } catch (_) {
+      /* ignore */
+    }
+    process.env.STORAGE_PATH = storagePath;
+
+    resetModules();
+
+    jest.doMock('../../src/middleware/auth', () => ({
+      adminAuth: (req, _res, next) => {
+        req.admin = { id: 1, username: 'tester' };
+        next();
+      }
+    }));
+
+    jest.doMock('../../src/services/imageProcessor', () => ({
+      generateThumbnail: jest.fn().mockResolvedValue('thumbnails/mock-thumb.jpg'),
+      ensureThumbnail: jest.fn()
+    }));
+
+    jest.doMock('../../src/middleware/uploadValidation', () => ({
+      validateUploadedFiles: (_req, _res, next) => next()
+    }));
+
+    jest.doMock('../../src/utils/fileSecurityUtils', () => {
+      const actual = jest.requireActual('../../src/utils/fileSecurityUtils');
+      return {
+        ...actual,
+        validateFileType: () => true,
+        createFileUploadValidator: () => (_req, _res, next) => next()
+      };
+    });
+
+    jest.doMock('../../src/utils/logger', () => ({
+      debug: jest.fn(),
+      info: jest.fn(),
+      warn: jest.fn(),
+      error: jest.fn()
+    }));
+
+    const dbModule = require('../../src/database/db');
+    db = dbModule.db;
+
+    await db.schema.dropTableIfExists('photo_feedback');
+    await db.schema.dropTableIfExists('photos');
+    await db.schema.dropTableIfExists('photo_categories');
+    await db.schema.dropTableIfExists('events');
+
+    await db.schema.createTable('events', (table) => {
+      table.increments('id').primary();
+      table.string('slug').notNullable();
+      table.string('event_name').notNullable();
+      table.string('source_mode').notNullable();
+      table.string('external_path');
+    });
+
+    await db.schema.createTable('photo_categories', (table) => {
+      table.increments('id').primary();
+      table.string('name').notNullable();
+      table.string('slug').notNullable();
+      table.boolean('is_global').defaultTo(true);
+      table.integer('event_id');
+    });
+
+    await db.schema.createTable('photos', (table) => {
+      table.increments('id').primary();
+      table.integer('event_id').notNullable();
+      table.string('filename').notNullable();
+      table.string('path').notNullable();
+      table.string('thumbnail_path');
+      table.string('type').notNullable();
+      table.integer('size_bytes');
+      table.integer('category_id');
+      table.string('source_origin');
+      table.string('external_relpath');
+      table.datetime('uploaded_at').defaultTo(db.fn.now());
+      table.float('average_rating').defaultTo(0);
+      table.integer('like_count').defaultTo(0);
+      table.integer('favorite_count').defaultTo(0);
+    });
+
+    await db.schema.createTable('photo_feedback', (table) => {
+      table.increments('id');
+      table.integer('photo_id');
+      table.string('feedback_type');
+      table.boolean('is_approved');
+      table.boolean('is_hidden');
+    });
+
+    await db('events').insert({
+      id: 1,
+      slug: 'test-event',
+      event_name: 'Test Event',
+      source_mode: 'reference',
+      external_path: 'external/library'
+    });
+
+    const insertedCategory = await db('photo_categories').insert({
+      name: 'Highlights',
+      slug: 'highlights',
+      is_global: true
+    });
+    categoryId = Array.isArray(insertedCategory) ? insertedCategory[0] : insertedCategory;
+
+    const router = require('../../src/routes/adminPhotos');
+    app = express();
+    app.use(express.json());
+    app.use('/api/admin/events', router);
+  });
+
+  afterAll(async () => {
+    if (db) {
+      await db.destroy();
+    }
+    resetModules();
+    delete process.env.TEST_DATABASE_PATH;
+    delete process.env.STORAGE_PATH;
+    if (tmpDir) {
+      await fs.promises.rm(tmpDir, { recursive: true, force: true });
+    }
+  });
+
+  it('stores managed uploads with category information and managed origin', async () => {
+    const uploadResponse = await request(app)
+      .post(`/api/admin/events/1/upload`)
+      .field('category_id', String(categoryId))
+      .attach('photos', Buffer.from('fake image data'), 'photo.jpg');
+
+    expect(uploadResponse.status).toBe(200);
+    expect(uploadResponse.body).toHaveProperty('photos');
+    expect(Array.isArray(uploadResponse.body.photos)).toBe(true);
+
+    const photo = await db('photos').first();
+    expect(photo).toBeTruthy();
+    expect(photo.category_id).toBe(categoryId);
+    expect(photo.source_origin).toBe('managed');
+    expect(photo.external_relpath).toBeNull();
+  });
+
+  it('returns numeric category metadata when listing photos', async () => {
+    await db('photos').insert({
+      event_id: 1,
+      filename: 'external.jpg',
+      path: 'test-event/external.jpg',
+      thumbnail_path: null,
+      type: 'individual',
+      size_bytes: 123,
+      source_origin: 'external',
+      external_relpath: 'individual/external.jpg'
+    });
+
+    const response = await request(app)
+      .get(`/api/admin/events/1/photos`)
+      .expect(200);
+
+    expect(Array.isArray(response.body.photos)).toBe(true);
+    const managedPhoto = response.body.photos.find((p) => p.category_id === categoryId);
+    expect(managedPhoto).toBeTruthy();
+    expect(managedPhoto.category_name).toBe('Highlights');
+
+    const filtered = await request(app)
+      .get(`/api/admin/events/1/photos`)
+      .query({ category_id: String(categoryId) })
+      .expect(200);
+
+    expect(filtered.body.photos.every((p) => p.category_id === categoryId)).toBe(true);
+  });
+
+  it('normalizes category updates', async () => {
+    const photo = await db('photos').first();
+
+    await request(app)
+      .patch(`/api/admin/events/1/photos/${photo.id}`)
+      .send({ category_id: '0' })
+      .expect(200);
+
+    const updated = await db('photos').where({ id: photo.id }).first();
+    expect(updated.category_id).toBeNull();
+  });
+});
@@ -66,6 +66,16 @@ describe('resolvePhotoFilePath', () => {
    expect(result).toBe(path.join('/mock/external', 'picsum-demo/individual', 'look-02.jpg'));
  });

+  it('falls back to managed storage when external metadata is missing', () => {
+    const event = { slug: 'fashion-show', source_mode: 'reference', external_path: 'picsum-demo' };
+    const photo = { path: 'fashion-show/new-upload.jpg' };
+
+    const result = resolvePhotoFilePath(event, photo);
+
+    expect(resolveExternalPath).not.toHaveBeenCalled();
+    expect(result).toBe(path.join(backendRoot, 'storage', 'events/active', 'fashion-show', 'new-upload.jpg'));
+  });
+
  it('throws when external photo is missing relative path data', () => {
    const event = { slug: 'fashion-show', source_mode: 'reference', external_path: 'picsum-demo' };
    const photo = { source_origin: 'external' };
@@ -13,6 +13,24 @@ const router = express.Router();
 // Get storage path from environment or default
 const getStoragePath = () => process.env.STORAGE_PATH || path.join(__dirname, '../../../storage');

+const parseCategoryId = (value) => {
+  if (value === undefined || value === null) return null;
+  if (typeof value === 'number' && Number.isInteger(value)) {
+    return value === 0 ? null : value;
+  }
+  if (typeof value === 'string') {
+    const trimmed = value.trim();
+    if (!trimmed || trimmed === 'null') return null;
+    if (/^\d+$/.test(trimmed)) {
+      const parsed = parseInt(trimmed, 10);
+      if (!Number.isNaN(parsed)) {
+        return parsed === 0 ? null : parsed;
+      }
+    }
+  }
+  return null;
+};
+
 // Configure multer for file uploads
 // IMPORTANT: Using synchronous functions to prevent file corruption
 const storage = multer.diskStorage({
@@ -158,16 +176,16 @@ router.post('/:eventId/upload', adminAuth, uploadTimeout(600000), (req, res, nex
    }
    
    // Parse category_id to number if provided
-    const parsedCategoryId = category_id ? parseInt(category_id, 10) : null;
+    const numericCategoryId = parseCategoryId(category_id);
    
    // Determine photo type from category_id parameter (for backwards compatibility)
    let photoType = 'individual'; // default
    let categoryName = 'individual';
    
-    if (parsedCategoryId === 1 || category_id === 'collage') {
+    if (numericCategoryId === 1 || category_id === 'collage') {
      photoType = 'collage';
      categoryName = 'collages';
-    } else if (parsedCategoryId === 2 || category_id === 'individual') {
+    } else if (numericCategoryId === 2 || category_id === 'individual') {
      photoType = 'individual';
      categoryName = 'individual';
    }
@@ -239,7 +257,9 @@ router.post('/:eventId/upload', adminAuth, uploadTimeout(600000), (req, res, nex
              path: relativePath,
              thumbnail_path: null, // Will generate after successful commit
              type: photoType,
-              size_bytes: tempStats.size // Use actual file size from stat
+              size_bytes: tempStats.size, // Use actual file size from stat
+              category_id: numericCategoryId,
+              source_origin: 'managed'
            };
            
            batchPhotos.push(photoData);
@@ -475,9 +495,11 @@ router.patch('/:eventId/photos/:photoId', adminAuth, async (req, res) => {
    }
    
    // Update photo
+    const normalizedCategoryId = parseCategoryId(category_id);
+
    await db('photos')
      .where({ id: photoId })
-      .update({ category_id: category_id || null });
+      .update({ category_id: normalizedCategoryId });
    
    res.json({ message: 'Photo updated successfully' });
  } catch (error) {
@@ -578,7 +600,7 @@ router.post('/:eventId/photos/bulk-update', adminAuth, async (req, res) => {
    // Update photos
    const updateData = {};
    if (updates.category_id !== undefined) {
-      updateData.category_id = updates.category_id || null;
+      updateData.category_id = parseCategoryId(updates.category_id);
    }
    
    await db('photos')
@@ -632,14 +654,22 @@ router.get('/:eventId/photos', adminAuth, async (req, res) => {
    const { category_id, type, search, sort = 'date', order = 'desc' } = req.query;
    
    let query = db('photos')
+      .leftJoin('photo_categories as pc', 'pc.id', 'photos.category_id')
      .where({ 'photos.event_id': eventId })
-      .select('photos.*');
+      .select(
+        'photos.*',
+        'pc.name as category_display_name',
+        'pc.slug as category_display_slug'
+      );
    
    // Filter by type (individual/collage) - category_id maps to type
    if (category_id !== undefined) {
-      if (category_id === '' || category_id === '0') {
-        // For backwards compatibility, empty category means no filter
-        // Don't filter anything
+      if (category_id === '') {
+        // No filter when empty string is provided
+      } else if (category_id === '0') {
+        query = query.whereNull('photos.category_id');
+      } else if (/^\d+$/.test(category_id)) {
+        query = query.where('photos.category_id', parseInt(category_id, 10));
      } else if (category_id === 'individual' || category_id === 'collage') {
        query = query.where({ 'photos.type': category_id });
      }
@@ -666,6 +696,10 @@ router.get('/:eventId/photos', adminAuth, async (req, res) => {
    
    const photos = await query.orderBy(orderByColumn, order);

+    if (photos.length === 0) {
+      return res.json({ photos: [] });
+    }
+
    // Get comment counts separately
    const commentCounts = await db('photo_feedback')
      .whereIn('photo_id', photos.map(p => p.id))
@@ -690,9 +724,11 @@ router.get('/:eventId/photos', adminAuth, async (req, res) => {
        // Always expose a thumbnail URL; backend will generate on demand if missing
        thumbnail_url: `/admin/photos/${eventId}/thumbnail/${photo.id}`,
        type: photo.type,
-        category_id: photo.type,
-        category_name: photo.type === 'individual' ? 'Individual Photos' : 'Collages',
-        category_slug: photo.type,
+        category_id: photo.category_id !== null && photo.category_id !== undefined
+          ? Number(photo.category_id)
+          : null,
+        category_name: photo.category_display_name || (photo.type === 'individual' ? 'Individual Photos' : 'Collages'),
+        category_slug: photo.category_display_slug || photo.type,
        size: photo.size_bytes,
        uploaded_at: photo.uploaded_at,
        // Feedback data
@@ -12,8 +12,10 @@ const getStoragePath = () => process.env.STORAGE_PATH || path.join(__dirname, '.
 function resolvePhotoFilePath(event, photo) {
  if (!event || !photo) throw new Error('resolvePhotoFilePath requires event and photo');

-  const mode = (event.source_mode || photo.source_origin || 'managed');
-  if (mode === 'reference' || photo.source_origin === 'external') {
+  const isExternal = photo.source_origin === 'external' ||
+    (!!photo.external_relpath && (event.source_mode === 'reference' || event.source_mode === 'external'));
+
+  if (isExternal) {
    if (!photo.external_relpath) {
      throw new Error('Missing external_relpath for external photo');
    }
@@ -8,7 +8,8 @@
    "build": "vite build",
    "build:check": "tsc -b && vite build",
    "lint": "eslint .",
-    "preview": "vite preview"
+    "preview": "vite preview",
+    "test": "vitest run src/components/admin/__tests__/ThemeCustomizerEnhanced.test.tsx"
  },
  "dependencies": {
    "@tanstack/react-query": "^5.0.0",
@@ -47,6 +48,9 @@
  },
  "devDependencies": {
    "@eslint/js": "^9.29.0",
+    "@testing-library/jest-dom": "^6.6.3",
+    "@testing-library/react": "^16.1.0",
+    "@testing-library/user-event": "^14.6.1",
    "@types/react": "^18.3.12",
    "@types/react-dom": "^18.3.1",
    "@vitejs/plugin-react": "^4.5.2",
@@ -55,10 +59,12 @@
    "eslint-plugin-react-hooks": "^5.2.0",
    "eslint-plugin-react-refresh": "^0.4.20",
    "globals": "^16.2.0",
+    "jsdom": "^25.0.1",
    "postcss": "^8.4.21",
    "tailwindcss": "^3.3.0",
    "typescript": "~5.8.3",
    "typescript-eslint": "^8.34.1",
-    "vite": "^7.1.6"
+    "vite": "^7.1.6",
+    "vitest": "^2.1.5"
  }
 }
@@ -14,6 +14,8 @@ interface ThemeCustomizerEnhancedProps {
  isPreviewMode?: boolean;
  showGalleryLayouts?: boolean;
  hideActions?: boolean;
+  onApply?: (theme: ThemeConfig, metadata: { presetName: string }) => Promise<void> | void;
+  isApplying?: boolean;
 }

 const layoutIcons: Record<GalleryLayoutType, React.ReactNode> = {
@@ -34,7 +36,9 @@ export const ThemeCustomizerEnhanced: React.FC<ThemeCustomizerEnhancedProps> = (
  onPresetChange,
  isPreviewMode = false,
  showGalleryLayouts = true,
-  hideActions = false
+  hideActions = false,
+  onApply,
+  isApplying = false
 }) => {
  const { t } = useTranslation();
  const [localTheme, setLocalTheme] = useState<ThemeConfig>(value);
@@ -80,8 +84,13 @@ export const ThemeCustomizerEnhanced: React.FC<ThemeCustomizerEnhancedProps> = (
    }
  };

-  const handleApply = () => {
-    onChange({ ...localTheme, customCss });
+  const handleApply = async () => {
+    const themeWithCss = { ...localTheme, customCss };
+    onChange(themeWithCss);
+
+    if (onApply) {
+      await onApply(themeWithCss, { presetName: selectedPreset });
+    }
  };

  const handleReset = () => {
@@ -587,8 +596,9 @@ export const ThemeCustomizerEnhanced: React.FC<ThemeCustomizerEnhancedProps> = (
            variant="primary"
            leftIcon={<Palette className="w-4 h-4" />}
            onClick={handleApply}
+            disabled={isApplying}
          >
-            {t('branding.applyTheme')}
+            {isApplying ? t('common.applying', 'Applying...') : t('branding.applyTheme')}
          </Button>
        </div>
      )}
@@ -0,0 +1,70 @@
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { vi } from 'vitest';
+
+import { ThemeCustomizerEnhanced } from '../ThemeCustomizerEnhanced';
+import type { ThemeConfig } from '../../../types/theme.types';
+
+vi.mock('react-i18next', async () => {
+  const actual = await vi.importActual<typeof import('react-i18next')>('react-i18next');
+  return {
+    ...actual,
+    useTranslation: () => ({
+      t: (_key: string, fallback?: string) => fallback ?? _key
+    })
+  };
+});
+
+describe('ThemeCustomizerEnhanced', () => {
+  const baseTheme: ThemeConfig = {
+    primaryColor: '#000000',
+    accentColor: '#ffffff',
+    backgroundColor: '#eeeeee',
+    textColor: '#111111',
+    galleryLayout: 'grid',
+    gallerySettings: {
+      spacing: 'normal'
+    }
+  };
+
+  it('invokes onApply when Apply Theme is clicked', async () => {
+    const user = userEvent.setup();
+    const handleChange = vi.fn();
+    const handleApply = vi.fn().mockResolvedValue(undefined);
+
+    render(
+      <ThemeCustomizerEnhanced
+        value={baseTheme}
+        onChange={handleChange}
+        presetName="default"
+        onApply={handleApply}
+      />
+    );
+
+    const applyButton = screen.getByRole('button', { name: /branding\.applyTheme/i });
+    await user.click(applyButton);
+
+    expect(handleChange).toHaveBeenCalled();
+    expect(handleApply).toHaveBeenCalledTimes(1);
+    expect(handleApply).toHaveBeenCalledWith(
+      expect.objectContaining({ primaryColor: '#000000' }),
+      expect.objectContaining({ presetName: 'default' })
+    );
+  });
+
+  it('disables the Apply button while applying', () => {
+    const handleChange = vi.fn();
+
+    render(
+      <ThemeCustomizerEnhanced
+        value={baseTheme}
+        onChange={handleChange}
+        presetName="default"
+        isApplying={true}
+      />
+    );
+
+    const applyButton = screen.getByRole('button', { name: /applying/i });
+    expect(applyButton).toBeDisabled();
+  });
+});
@@ -1,4 +1,4 @@
-import React, { useState, useEffect } from 'react';
+import React, { useState, useEffect, useRef, useCallback } from 'react';
 import { Download, Maximize2, Check, ChevronDown, Calendar, Clock, Heart, MessageSquare } from 'lucide-react';
 import { parseISO } from 'date-fns';
 import { useTranslation } from 'react-i18next';
@@ -163,12 +163,22 @@ export const HeroGalleryLayout: React.FC<HeroGalleryLayoutProps> = ({
        
        {/* Scroll Indicator */}
        <div className="absolute bottom-8 left-1/2 transform -translate-x-1/2 animate-bounce">
-          <ChevronDown className="w-8 h-8 text-white drop-shadow-lg" />
+          <button
+            type="button"
+            onClick={handleScrollToGrid}
+            className="rounded-full border border-white/30 bg-white/10 p-3 text-white transition focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-white focus-visible:ring-offset-2 hover:bg-white/20"
+            aria-label={t('gallery.scrollToGallery', 'Scroll to gallery')}
+          >
+            <ChevronDown className="w-8 h-8 drop-shadow-lg" />
+          </button>
        </div>
      </div>

      {/* Grid Section */}
-      <div className="grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-4 xl:grid-cols-5 gap-4">
+      <div
+        ref={gridRef}
+        className="grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-4 xl:grid-cols-5 gap-4"
+      >
        {remainingPhotos.map((photo) => {
          const actualIndex = photos.findIndex(p => p.id === photo.id);
          return (
@@ -313,3 +323,9 @@ export const HeroGalleryLayout: React.FC<HeroGalleryLayoutProps> = ({
    </>
  );
 };
+  const gridRef = useRef<HTMLDivElement | null>(null);
+  const handleScrollToGrid = useCallback(() => {
+    if (gridRef.current) {
+      gridRef.current.scrollIntoView({ behavior: 'smooth', block: 'start' });
+    }
+  }, []);
@@ -236,6 +236,28 @@ export const EventDetailsPage: React.FC = () => {
    },
  });

+  const applyThemeMutation = useMutation({
+    mutationFn: async ({ theme, presetName }: { theme: ThemeConfig; presetName: string }) => {
+      if (!id) {
+        throw new Error('Missing event identifier');
+      }
+
+      const colorThemeValue = presetName && presetName !== 'custom'
+        ? presetName
+        : JSON.stringify(theme);
+
+      return eventsService.updateEvent(parseInt(id), { color_theme: colorThemeValue });
+    },
+    onSuccess: () => {
+      queryClient.invalidateQueries({ queryKey: ['admin-event', id] });
+      toast.success(t('branding.themeApplied', 'Theme updated'));
+    },
+    onError: (error: any) => {
+      const message = error?.response?.data?.error || t('branding.themeApplyError', 'Failed to apply theme');
+      toast.error(message);
+    }
+  });
+
  // Archive mutation
  const archiveMutation = useMutation({
    mutationFn: () => eventsService.archiveEvent(parseInt(id!)),
@@ -1124,6 +1146,20 @@ export const EventDetailsPage: React.FC = () => {
                }}
                isPreviewMode={false}
                showGalleryLayouts={true}
+                onApply={async (theme, { presetName }) => {
+                  const resolvedPreset = presetName || 'custom';
+                  setCurrentTheme(theme);
+                  setCurrentPresetName(resolvedPreset);
+
+                  const themeValue = resolvedPreset !== 'custom'
+                    ? resolvedPreset
+                    : JSON.stringify(theme);
+
+                  setEditForm(prev => ({ ...prev, color_theme: themeValue }));
+
+                  await applyThemeMutation.mutateAsync({ theme, presetName: resolvedPreset });
+                }}
+                isApplying={applyThemeMutation.isPending}
              />
            </Card>
          )}
@@ -15,6 +15,11 @@ export default defineConfig({
    },
    sourcemap: true,
  },
+  test: {
+    environment: 'jsdom',
+    setupFiles: './vitest.setup.ts',
+    globals: true
+  },
  server: {
    port: 5173,
    host: true,
@@ -0,0 +1,7 @@
+import { expect, vi } from 'vitest';
+import * as matchers from '@testing-library/jest-dom/matchers';
+
+expect.extend(matchers);
+
+// Provide Jest-compatible globals for existing tests that rely on jest.fn
+(globalThis as any).jest = vi;