Merge pull request #45 from the-luap/claude/investigate-issues-22-011CUoRMw67THYkdYBdVgG2Z

Fix Critical Bugs in Issues #22 and #30
This commit is contained in:
Paul Nothaft
2025-11-04 21:25:52 +01:00
committed by GitHub
7 changed files with 161 additions and 37 deletions
+2 -2
View File
@@ -67,7 +67,7 @@ jobs:
type=semver,pattern={{version}}
type=semver,pattern={{major}}.{{minor}}
type=semver,pattern={{major}}
type=sha,prefix={{branch}}-,format=short
type=sha,format=short
type=raw,value=latest,enable={{is_default_branch}}
- name: Build and push Backend Docker image
@@ -147,7 +147,7 @@ jobs:
type=semver,pattern={{version}}
type=semver,pattern={{major}}.{{minor}}
type=semver,pattern={{major}}
type=sha,prefix={{branch}}-,format=short
type=sha,format=short
type=raw,value=latest,enable={{is_default_branch}}
- name: Build and push Frontend Docker image
+30 -9
View File
@@ -473,21 +473,34 @@ router.patch('/:eventId/photos/:photoId', adminAuth, async (req, res) => {
try {
const { eventId, photoId } = req.params;
const { category_id } = req.body;
// Verify photo belongs to event
const photo = await db('photos')
.where({ id: photoId, event_id: eventId })
.first();
if (!photo) {
return res.status(404).json({ error: 'Photo not found' });
}
// Prepare update data
const updateData = {};
// Handle type-based categories ('individual' or 'collage')
// These are string values that map to the photo.type field
if (category_id === 'individual' || category_id === 'collage') {
updateData.type = category_id;
updateData.category_id = null; // Clear legacy category_id
} else {
// Handle legacy numeric category IDs
updateData.category_id = category_id || null;
}
// Update photo
await db('photos')
.where({ id: photoId })
.update({ category_id: category_id || null });
.update(updateData);
res.json({ message: 'Photo updated successfully' });
} catch (error) {
console.error('Error updating photo:', error);
@@ -584,17 +597,25 @@ router.post('/:eventId/photos/bulk-update', adminAuth, async (req, res) => {
return res.status(400).json({ error: 'Some photos do not belong to this event' });
}
// Update photos
// Prepare update data
const updateData = {};
if (updates.category_id !== undefined) {
updateData.category_id = updates.category_id || null;
// Handle type-based categories ('individual' or 'collage')
// These are string values that map to the photo.type field
if (updates.category_id === 'individual' || updates.category_id === 'collage') {
updateData.type = updates.category_id;
updateData.category_id = null; // Clear legacy category_id
} else {
// Handle legacy numeric category IDs
updateData.category_id = updates.category_id || null;
}
}
await db('photos')
.whereIn('id', photoIds)
.where('event_id', eventId)
.update(updateData);
res.json({ message: `${photoIds.length} photos updated successfully` });
} catch (error) {
console.error('Error bulk updating photos:', error);
+19 -6
View File
@@ -800,22 +800,35 @@ router.get('/:slug/stats', verifyGalleryAccess, async (req, res) => {
router.post('/:eventId/upload', verifyGalleryAccess, async (req, res) => {
try {
const eventId = parseInt(req.params.eventId);
// Verify the event matches the token
if (req.event.id !== eventId) {
return res.status(403).json({ error: 'Access denied' });
}
// Check if user uploads are allowed
if (!req.event.allow_user_uploads) {
return res.status(403).json({ error: 'User uploads are not allowed for this event' });
}
// Ensure temp upload directory exists
const fs = require('fs');
const tempUploadDir = '/tmp/uploads/';
if (!fs.existsSync(tempUploadDir)) {
try {
fs.mkdirSync(tempUploadDir, { recursive: true, mode: 0o755 });
logger.info('Created temp upload directory:', tempUploadDir);
} catch (mkdirErr) {
logger.error('Failed to create temp upload directory:', mkdirErr);
return res.status(500).json({ error: 'Server configuration error: unable to create upload directory' });
}
}
// Import multer and photo processing
const multer = require('multer');
const upload = multer({
dest: '/tmp/uploads/',
limits: {
const upload = multer({
dest: tempUploadDir,
limits: {
fileSize: 50 * 1024 * 1024, // 50MB
files: 10 // Max 10 files at once
},
+85 -13
View File
@@ -8,20 +8,46 @@ const { generatePhotoFilename } = require('../utils/filenameSanitizer');
const getStoragePath = () => process.env.STORAGE_PATH || path.join(__dirname, '../../../storage');
function normalizeFiles(files) {
if (!files) return [];
if (Array.isArray(files)) return files.filter(Boolean);
// Multer may expose files as an iterable object
if (typeof files[Symbol.iterator] === 'function') {
return Array.from(files).filter(Boolean);
// Handle null, undefined, or falsy values
if (!files) {
console.log('[normalizeFiles] No files provided');
return [];
}
// Handle arrays
if (Array.isArray(files)) {
const validFiles = files.filter(Boolean);
console.log(`[normalizeFiles] Normalized ${validFiles.length} files from array`);
return validFiles;
}
// Handle iterable objects (some multer configurations)
try {
if (typeof files === 'object' && typeof files[Symbol.iterator] === 'function') {
const validFiles = Array.from(files).filter(Boolean);
console.log(`[normalizeFiles] Normalized ${validFiles.length} files from iterable`);
return validFiles;
}
} catch (err) {
console.warn('[normalizeFiles] Failed to iterate files object:', err.message);
}
// Handle plain objects (multer fieldname mapping)
if (typeof files === 'object') {
return Object.values(files)
.flatMap((value) => (Array.isArray(value) ? value : [value]))
.filter(Boolean);
try {
const validFiles = Object.values(files)
.flatMap((value) => (Array.isArray(value) ? value : [value]))
.filter(Boolean);
console.log(`[normalizeFiles] Normalized ${validFiles.length} files from object`);
return validFiles;
} catch (err) {
console.warn('[normalizeFiles] Failed to process files object:', err.message);
return [];
}
}
// Unexpected type
console.warn('[normalizeFiles] Unexpected files type:', typeof files);
return [];
}
@@ -80,18 +106,46 @@ async function processUploadedPhotos(files, eventId, uploadedBy = 'admin', categ
const tempPath = file?.path || file?.filepath || file?.tempFilePath;
if (!tempPath) {
throw new Error('Uploaded file is missing a temporary path');
const fileInfo = JSON.stringify({
originalname: file?.originalname,
mimetype: file?.mimetype,
size: file?.size,
availableKeys: Object.keys(file || {})
});
throw new Error(`Uploaded file is missing a temporary path. File info: ${fileInfo}`);
}
// Verify temp file exists before copying
try {
await fs.access(tempPath);
} catch (accessErr) {
console.error(`Temp file not accessible: ${tempPath}`, {
originalname: file?.originalname,
error: accessErr.message
});
throw new Error(`Uploaded file not found at temporary location: ${tempPath}`);
}
// Use copyFile and unlink instead of rename to avoid cross-device issues
try {
await fs.copyFile(tempPath, newPath);
console.log(`Successfully copied ${file.originalname} to ${newPath}`);
} catch (copyErr) {
console.error(`Failed to copy file from ${tempPath} to ${newPath}:`, copyErr);
throw new Error(`Failed to copy uploaded file: ${copyErr.message}`);
} finally {
// Clean up temp file with better error handling
try {
await fs.unlink(tempPath);
console.log(`Cleaned up temp file: ${tempPath}`);
} catch (unlinkErr) {
// Only warn if file exists but couldn't be deleted
// ENOENT means file was already deleted, which is fine
if (unlinkErr?.code !== 'ENOENT') {
console.warn(`Failed to clean up temp upload ${tempPath}:`, unlinkErr);
console.warn(`Failed to clean up temp upload ${tempPath}:`, {
error: unlinkErr.message,
code: unlinkErr.code
});
}
}
}
@@ -154,10 +208,28 @@ async function processUploadedPhotos(files, eventId, uploadedBy = 'admin', categ
size: file.size,
type: photoType
});
console.log(`Successfully processed file ${file.originalname} (ID: ${photoId})`);
} catch (error) {
console.error(`Error processing file ${file.originalname}:`, error);
if (trx) await trx.rollback();
console.error(`Error processing file ${file.originalname}:`, {
error: error.message,
stack: error.stack,
originalname: file.originalname,
mimetype: file.mimetype,
size: file.size,
tempPath: file?.path || file?.filepath || file?.tempFilePath
});
if (trx) {
try {
await trx.rollback();
} catch (rollbackErr) {
console.error('Failed to rollback transaction:', rollbackErr);
}
}
// Continue with other files
// Note: Individual file failures don't stop the entire upload batch
}
}
+6 -2
View File
@@ -12,8 +12,12 @@ 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') {
// IMPORTANT: photo.source_origin takes precedence over event.source_mode
// This allows events in "reference" mode to have mixed sources:
// - Imported photos: source_origin = 'external'
// - Uploaded photos: source_origin = 'managed'
const mode = (photo.source_origin || event.source_mode || 'managed');
if (mode === 'reference' || mode === 'external') {
if (!photo.external_relpath) {
throw new Error('Missing external_relpath for external photo');
}
@@ -71,8 +71,9 @@ export const GalleryView: React.FC<GalleryViewProps> = ({ slug, event }) => {
setGuestId(storedGuestId);
}, []);
// Fetch photos with filter support
const { data, isLoading, error, refetch } = useGalleryPhotos(slug, filterType, guestId);
// Fetch photos WITHOUT filter (always get all photos, filter on frontend)
// This ensures counts are always calculated from the full dataset
const { data, isLoading, error, refetch } = useGalleryPhotos(slug, 'all', guestId);
// Set protection level when data is available
useEffect(() => {
@@ -162,13 +162,26 @@ export const HeroGalleryLayout: React.FC<HeroGalleryLayoutProps> = ({
</div>
{/* Scroll Indicator */}
<div className="absolute bottom-8 left-1/2 transform -translate-x-1/2 animate-bounce">
<button
onClick={() => {
// Scroll to the grid section
const gridSection = document.getElementById('gallery-grid-section');
if (gridSection) {
gridSection.scrollIntoView({ behavior: 'smooth', block: 'start' });
} else {
// Fallback: scroll down by hero section height
window.scrollBy({ top: window.innerHeight * 0.9, behavior: 'smooth' });
}
}}
className="absolute bottom-8 left-1/2 transform -translate-x-1/2 animate-bounce cursor-pointer hover:scale-110 transition-transform focus:outline-none focus:ring-2 focus:ring-white focus:ring-opacity-50 rounded-full p-2"
aria-label="Scroll to gallery"
>
<ChevronDown className="w-8 h-8 text-white drop-shadow-lg" />
</div>
</button>
</div>
{/* Grid Section */}
<div className="grid grid-cols-2 sm:grid-cols-3 lg:grid-cols-4 xl:grid-cols-5 gap-4">
<div id="gallery-grid-section" 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 (