From 5f8c8c5508d6acf9b06502e214342c2a3582eac5 Mon Sep 17 00:00:00 2001 From: paul Date: Fri, 26 Sep 2025 17:25:58 +0200 Subject: [PATCH] Fix branding asset storage path --- backend/__tests__/adminSettings.logo.test.js | 184 +++++++++++++++++++ backend/scripts/migrate-upload-assets.js | 102 ++++++++++ backend/src/routes/adminSettings.js | 16 +- 3 files changed, 296 insertions(+), 6 deletions(-) create mode 100644 backend/__tests__/adminSettings.logo.test.js create mode 100755 backend/scripts/migrate-upload-assets.js diff --git a/backend/__tests__/adminSettings.logo.test.js b/backend/__tests__/adminSettings.logo.test.js new file mode 100644 index 0000000..16eb891 --- /dev/null +++ b/backend/__tests__/adminSettings.logo.test.js @@ -0,0 +1,184 @@ +const fs = require('fs'); +const fsPromises = fs.promises; +const os = require('os'); +const path = require('path'); +const express = require('express'); +const request = require('supertest'); + +describe('Admin settings logo upload flow', () => { + let tmpDir; + let router; + let app; + let settingsStore; + + const resetModules = () => { + jest.resetModules(); + jest.clearAllMocks(); + }; + + beforeEach(async () => { + resetModules(); + + tmpDir = await fsPromises.mkdtemp(path.join(os.tmpdir(), 'picpeak-logo-')); + process.env.STORAGE_PATH = tmpDir; + + settingsStore = new Map(); + + const buildQuery = (table) => { + const filters = []; + const applyFilters = (rows) => { + if (filters.length === 0) { + return rows; + } + return rows.filter((row) => + filters.every(({ column, value }) => row[column] === value) + ); + }; + + const makeRow = (row) => ({ ...row }); + + return { + where(column, value) { + filters.push({ column, value }); + return this; + }, + first() { + if (table === 'app_settings') { + const rows = applyFilters(Array.from(settingsStore.values()).map(makeRow)); + return Promise.resolve(rows[0]); + } + return Promise.resolve(undefined); + }, + select() { + return Promise.resolve([]); + }, + sum() { + return Promise.resolve({ total: 0 }); + }, + join() { + return this; + }, + groupBy() { + return this; + }, + orderBy() { + return this; + }, + limit() { + return this; + }, + insert(payload) { + const rows = Array.isArray(payload) ? payload : [payload]; + const upsert = (row, overrides = {}) => { + if (table === 'app_settings') { + const key = row.setting_key; + const existing = settingsStore.get(key) || {}; + settingsStore.set(key, { ...existing, ...row, ...overrides }); + } + return Promise.resolve(); + }; + + return { + onConflict() { + return { + merge(overrides) { + return Promise.all(rows.map((row) => upsert(row, overrides))).then(() => undefined); + } + }; + } + }; + } + }; + }; + + const dbMock = jest.fn((table) => buildQuery(table)); + dbMock.raw = jest.fn(); + dbMock.transaction = async (handler) => handler({ + commit: async () => {}, + rollback: async () => {} + }); + + jest.doMock('../src/database/db', () => ({ + db: dbMock, + logActivity: jest.fn() + })); + + jest.doMock('../src/middleware/auth', () => ({ + adminAuth: (req, res, next) => { + req.admin = { id: 1, username: 'tester' }; + next(); + } + })); + + jest.doMock('../src/services/publicSiteService', () => ({ + clearPublicSiteCache: jest.fn(), + getDefaultPublicSitePayload: jest.fn(), + getRawPublicSiteSettings: jest.fn().mockResolvedValue({}) + })); + + jest.doMock('../src/services/rateLimitService', () => ({ + clearSettingsCache: jest.fn() + })); + + jest.doMock('../src/middleware/maintenance', () => ({ + maintenanceMiddleware: (req, res, next) => next(), + clearMaintenanceCache: jest.fn() + })); + + router = require('../src/routes/adminSettings'); + + app = express(); + app.use(express.json()); + app.use('/api/admin/settings', router); + }); + + afterEach(async () => { + resetModules(); + if (tmpDir) { + await fsPromises.rm(tmpDir, { recursive: true, force: true }); + tmpDir = null; + } + delete process.env.STORAGE_PATH; + }); + + it('stores logo uploads under STORAGE_PATH and deletes on branding reset', async () => { + const fileBuffer = Buffer.from('fake image data'); + + const uploadResponse = await request(app) + .post('/api/admin/settings/logo') + .attach('logo', fileBuffer, 'logo.png'); + + expect(uploadResponse.status).toBe(200); + expect(uploadResponse.body).toHaveProperty('logoUrl'); + const logoUrl = uploadResponse.body.logoUrl; + expect(logoUrl.startsWith('/uploads/logos/')).toBe(true); + + const storedPath = path.join(tmpDir, logoUrl.replace('/uploads/', 'uploads/')); + await expect(fsPromises.access(storedPath)).resolves.toBeUndefined(); + + await request(app) + .put('/api/admin/settings/branding') + .send({ + company_name: 'Test Co', + company_tagline: 'Tagline', + support_email: 'test@example.com', + footer_text: 'Footer', + watermark_enabled: false, + watermark_position: 'bottom-right', + watermark_opacity: 0.5, + watermark_size: 'medium', + favicon_url: null, + logo_url: '', + watermark_logo_url: null, + logo_size: 'medium', + logo_max_height: 120, + logo_position: 'left', + logo_display_header: true, + logo_display_hero: false, + logo_display_mode: 'default' + }) + .expect(200); + + await expect(fsPromises.access(storedPath)).rejects.toThrow(); + }); +}); diff --git a/backend/scripts/migrate-upload-assets.js b/backend/scripts/migrate-upload-assets.js new file mode 100755 index 0000000..b064e8f --- /dev/null +++ b/backend/scripts/migrate-upload-assets.js @@ -0,0 +1,102 @@ +#!/usr/bin/env node + +const fs = require('fs'); +const path = require('path'); + +const fsp = fs.promises; + +async function pathExists(location) { + try { + await fsp.access(location); + return true; + } catch (error) { + if (error && error.code === 'ENOENT') { + return false; + } + throw error; + } +} + +async function moveFile(source, destination) { + await fsp.mkdir(path.dirname(destination), { recursive: true }); + try { + await fsp.rename(source, destination); + } catch (error) { + if (error.code === 'EXDEV') { + await fsp.copyFile(source, destination); + await fsp.unlink(source); + } else if (error.code === 'EEXIST') { + console.warn(`Destination already exists, leaving original in place: ${destination}`); + return; + } else { + throw error; + } + } +} + +async function migrate() { + const backendRoot = path.resolve(__dirname, '..'); + const defaultStorage = path.resolve(backendRoot, '../storage'); + const targetStorage = path.resolve(process.env.STORAGE_PATH || defaultStorage); + const legacyUploadsRoot = path.resolve(backendRoot, 'storage/uploads'); + const targetUploadsRoot = path.join(targetStorage, 'uploads'); + + if (legacyUploadsRoot === targetUploadsRoot) { + console.log('Legacy uploads directory already matches target STORAGE_PATH. Nothing to migrate.'); + return; + } + + if (!fs.existsSync(legacyUploadsRoot)) { + console.log(`Legacy uploads directory not found at ${legacyUploadsRoot}. Nothing to migrate.`); + return; + } + + const categories = ['logos', 'favicons']; + let migratedCounter = 0; + + for (const category of categories) { + const legacyDir = path.join(legacyUploadsRoot, category); + if (!fs.existsSync(legacyDir)) { + continue; + } + + const targetDir = path.join(targetUploadsRoot, category); + await fsp.mkdir(targetDir, { recursive: true }); + + const entries = await fsp.readdir(legacyDir, { withFileTypes: true }); + for (const entry of entries) { + if (!entry.isFile()) { + continue; + } + + const sourcePath = path.join(legacyDir, entry.name); + const destinationPath = path.join(targetDir, entry.name); + + if (await pathExists(destinationPath)) { + console.warn(`Skipping ${sourcePath} because ${destinationPath} already exists.`); + continue; + } + + await moveFile(sourcePath, destinationPath); + migratedCounter += 1; + } + + const remaining = await fsp.readdir(legacyDir); + if (remaining.length === 0) { + await fsp.rm(legacyDir, { recursive: true, force: true }); + } + } + + if (migratedCounter === 0) { + console.log('No legacy logo or favicon files needed migration.'); + return; + } + + console.log(`Migrated ${migratedCounter} files into ${targetUploadsRoot}.`); + console.log('If the database still references legacy absolute paths, they will be cleaned up automatically on the next upload.'); +} + +migrate().catch((error) => { + console.error('Migration failed:', error); + process.exitCode = 1; +}); diff --git a/backend/src/routes/adminSettings.js b/backend/src/routes/adminSettings.js index 4d35f69..7c3a883 100644 --- a/backend/src/routes/adminSettings.js +++ b/backend/src/routes/adminSettings.js @@ -20,10 +20,12 @@ const { const { sanitizeCss } = require('../utils/cssSanitizer'); const router = express.Router(); +const getStoragePath = () => process.env.STORAGE_PATH || path.join(__dirname, '../../../storage'); + // Configure multer for logo uploads const storage = multer.diskStorage({ destination: async (req, file, cb) => { - const uploadDir = path.join(__dirname, '../../storage/uploads/logos'); + const uploadDir = path.join(getStoragePath(), 'uploads/logos'); await fs.mkdir(uploadDir, { recursive: true }); cb(null, uploadDir); }, @@ -53,7 +55,7 @@ const upload = multer({ // Configure multer for favicon uploads const faviconStorage = multer.diskStorage({ destination: async (req, file, cb) => { - const uploadDir = path.join(__dirname, '../../storage/uploads/favicons'); + const uploadDir = path.join(getStoragePath(), 'uploads/favicons'); await fs.mkdir(uploadDir, { recursive: true }); cb(null, uploadDir); }, @@ -228,7 +230,8 @@ router.put('/branding', adminAuth, async (req, res) => { if (currentFaviconUrl && typeof currentFaviconUrl === 'string' && currentFaviconUrl.startsWith('/uploads/favicons/')) { // Delete the file from filesystem - const faviconPath = path.join(__dirname, '..', '..', 'storage', currentFaviconUrl.replace('/uploads/', '')); + const relativePath = currentFaviconUrl.replace(/^\//, ''); + const faviconPath = path.join(getStoragePath(), relativePath); try { await fs.unlink(faviconPath); console.log('Deleted favicon file:', faviconPath); @@ -258,7 +261,8 @@ router.put('/branding', adminAuth, async (req, res) => { if (currentLogoUrl && typeof currentLogoUrl === 'string' && currentLogoUrl.startsWith('/uploads/logos/')) { // Delete the file from filesystem - const logoPath = path.join(__dirname, '..', '..', 'storage', currentLogoUrl.replace('/uploads/', '')); + const relativePath = currentLogoUrl.replace(/^\//, ''); + const logoPath = path.join(getStoragePath(), relativePath); try { await fs.unlink(logoPath); console.log('Deleted logo file:', logoPath); @@ -643,7 +647,7 @@ router.get('/storage/info', adminAuth, async (req, res) => { for (const archive of archives) { if (archive.archive_path) { try { - const storagePath = process.env.STORAGE_PATH || path.join(__dirname, '../../../storage'); + const storagePath = getStoragePath(); const fullArchivePath = path.join(storagePath, archive.archive_path); const stats = await fs.stat(fullArchivePath); archiveStorage += stats.size; @@ -654,7 +658,7 @@ router.get('/storage/info', adminAuth, async (req, res) => { } const DEFAULT_SOFT_LIMIT_BYTES = 10 * 1024 * 1024 * 1024; // 10GB fallback - const storagePath = process.env.STORAGE_PATH || path.join(__dirname, '../../../storage'); + const storagePath = getStoragePath(); let diskStats = null; let rawDiskTotal = null;