fix journey bugs reported by roel-de-vries (#722-#736)

Mobile UI:
- #722 timeline carousel no longer cut off by BottomNav (uses --bottom-nav-h var)
- #723 scroll-snap-type relaxed to proximity so small swipes no longer skip entries
- #724 defensive padding-bottom fix in JourneySettingsDialog for iOS PWA
- #725 add back/settings buttons + journey title subtitle to mobile activity view
- #726 active entry re-centers after scroll settle; tap inactive card activates
  it (does not jump straight into editor)

Entry editor flow:
- #727 photo uploads queue locally until Save for existing entries too
  (previously fired upload immediately; Cancel silently kept the new photo)
- #728 Cancel/Close with unsaved changes now requires confirm (window.confirm)
- #729 linking a Gallery photo into an entry now copies the row (old MOVE
  behavior meant Remove-from-Entry also nuked the Gallery original)
- #731 addPhoto / addProviderPhoto / linkPhotoToEntry promote skeleton
  entries to concrete 'entry' type when content is added

Permissions:
- #732 updateJourney switched from canEdit to isOwner — editors can still
  edit entries and photos, just not the journey shell (title, cover, status)
- #733 Contributors list gains a per-row remove (X) control with confirm
- #734 my_role is computed server-side and returned with the journey; UI
  gates Settings/Add/Edit/Delete controls based on role
- #736 createOrUpdateJourneyShareLink + deleteJourneyShareLink now require
  isOwner (previously NO permission check at all — anyone authenticated
  could publish or unpublish a journey)

Immich upload (#730):
- migration 111: add users.immich_auto_upload (default 0)
- migration 112: seed provider_field for the toggle (idempotent, FK-safe)
- journey photo upload only mirrors to Immich when the user has opted in
- Settings UI gets a "Mirror journey photos to Immich on upload" checkbox

Test updates:
- JOURNEY-SVC-019 inverted to assert editor cannot update journey settings
- JOURNEY-SHARE-007 now passes userId (owner) to deleteJourneyShareLink
- FE-PAGE-JOURNEYDETAIL-148 inverted to assert photos stay pending until Save
- client/tests still green (2676/2676)

Also fixed en route: gallery entry title is now the literal 'Gallery' on the
wire (used to send the translated label, which broke server-side title === 'Gallery'
checks in non-English locales); confirm interpolation uses {username} single
braces matching the existing i18n runtime; Settings footer uses icon-only
delete/archive buttons on mobile so the row doesn't wrap.
This commit is contained in:
Maurice
2026-04-18 19:11:16 +02:00
parent bc192d3106
commit 4974013995
27 changed files with 448 additions and 169 deletions
+29
View File
@@ -1738,6 +1738,35 @@ function runMigrations(db: Database.Database): void {
AND substr(reservations.reservation_end_time, 1, 10) != substr(reservations.reservation_time, 1, 10)
`);
},
// Migration 111: opt-in Immich auto-upload — users column only (#730)
// Default is off — uploading to Immich must be an explicit choice, not a
// side effect of having a writable API key.
() => {
try { db.exec('ALTER TABLE users ADD COLUMN immich_auto_upload INTEGER NOT NULL DEFAULT 0'); }
catch (err: any) { if (!err.message?.includes('duplicate column name')) throw err; }
},
// Migration 112: expose immich auto-upload toggle in the Settings UI (#730)
// Runs after Immich provider seeding so the FK to photo_providers holds.
() => {
try {
const hasTable = db.prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name IN ('photo_providers', 'photo_provider_fields')").all() as Array<{ name: string }>;
const hasProviders = hasTable.some(t => t.name === 'photo_providers');
const hasFields = hasTable.some(t => t.name === 'photo_provider_fields');
if (hasProviders && hasFields) {
const immichRow = db.prepare("SELECT 1 FROM photo_providers WHERE id = 'immich' LIMIT 1").get();
if (immichRow) {
db.prepare(`
INSERT OR IGNORE INTO photo_provider_fields
(provider_id, field_key, label, input_type, placeholder, required, secret, settings_key, payload_key, sort_order)
VALUES
('immich', 'immich_auto_upload', 'immichAutoUpload', 'checkbox', NULL, 0, 0, 'auto_upload', 'auto_upload', 5)
`).run();
}
}
} catch (err: any) {
if (!err.message?.includes('no such table') && !err.message?.includes('FOREIGN KEY')) throw err;
}
},
];
if (currentVersion < migrations.length) {
+21 -11
View File
@@ -6,6 +6,7 @@ import crypto from 'node:crypto';
import { authenticate } from '../middleware/auth';
import { AuthRequest } from '../types';
import * as svc from '../services/journeyService';
import { db } from '../db/database';
import { createOrUpdateJourneyShareLink, getJourneyShareLink, deleteJourneyShareLink, getPublicJourney } from '../services/journeyShareService';
import { uploadToImmich } from '../services/memories/immichService';
@@ -95,16 +96,21 @@ router.post('/entries/:entryId/photos', authenticate, upload.array('photos', 10)
req.body?.caption
);
if (photo) {
// sync to Immich if connected — update the same photo record
try {
const immichId = await uploadToImmich(authReq.user.id, relativePath, file.originalname);
if (immichId) {
svc.setPhotoProvider(photo.id, 'immich', immichId, authReq.user.id);
photo.provider = 'immich' as any;
photo.asset_id = immichId;
photo.owner_id = authReq.user.id;
}
} catch {}
// Mirror to Immich only when the user has explicitly opted in via the
// Immich integration settings. Avoids the "surprise upload" in #730
// where a write-capable API key implicitly enabled mirroring.
const prefs = db.prepare('SELECT immich_auto_upload FROM users WHERE id = ?').get(authReq.user.id) as { immich_auto_upload?: number } | undefined;
if (prefs?.immich_auto_upload) {
try {
const immichId = await uploadToImmich(authReq.user.id, relativePath, file.originalname);
if (immichId) {
svc.setPhotoProvider(photo.id, 'immich', immichId, authReq.user.id);
photo.provider = 'immich' as any;
photo.asset_id = immichId;
photo.owner_id = authReq.user.id;
}
} catch {}
}
results.push(photo);
}
}
@@ -301,11 +307,15 @@ router.post('/:id/share-link', authenticate, (req: Request, res: Response) => {
const authReq = req as AuthRequest;
const { share_timeline, share_gallery, share_map } = req.body || {};
const result = createOrUpdateJourneyShareLink(Number(req.params.id), authReq.user.id, { share_timeline, share_gallery, share_map });
if (!result) return res.status(403).json({ error: 'Not allowed' });
res.json(result);
});
router.delete('/:id/share-link', authenticate, (req: Request, res: Response) => {
deleteJourneyShareLink(Number(req.params.id));
const authReq = req as AuthRequest;
if (!deleteJourneyShareLink(Number(req.params.id), authReq.user.id)) {
return res.status(403).json({ error: 'Not allowed' });
}
res.json({ success: true });
});
+5 -1
View File
@@ -7,6 +7,7 @@ import { getClientIp } from '../../services/auditLog';
import {
getConnectionSettings,
saveImmichSettings,
setImmichAutoUpload,
testConnection,
getConnectionStatus,
browseTimeline,
@@ -31,9 +32,12 @@ router.get('/settings', authenticate, (req: Request, res: Response) => {
router.put('/settings', authenticate, async (req: Request, res: Response) => {
const authReq = req as AuthRequest;
const { immich_url, immich_api_key } = req.body;
const { immich_url, immich_api_key, auto_upload } = req.body;
const result = await saveImmichSettings(authReq.user.id, immich_url, immich_api_key, getClientIp(req));
if (!result.success) return res.status(400).json({ error: result.error });
if (typeof auto_upload === 'boolean') {
setImmichAutoUpload(authReq.user.id, auto_upload);
}
if (result.warning) return res.json({ success: true, warning: result.warning });
res.json({ success: true });
});
+58 -10
View File
@@ -167,6 +167,19 @@ export function getJourneyFull(journeyId: number, userId: number) {
'SELECT hide_skeletons FROM journey_contributors WHERE journey_id = ? AND user_id = ?'
).get(journeyId, userId) as { hide_skeletons: number } | undefined;
// Determine the viewer's role on this journey so the UI can gate edit/settings
// actions. 'owner' = creator, 'editor' | 'viewer' = from journey_contributors.
const journeyRow = journey as unknown as { user_id?: number };
let myRole: 'owner' | 'editor' | 'viewer' | null = null;
if (journeyRow.user_id === userId) {
myRole = 'owner';
} else {
const contribRow = db.prepare(
'SELECT role FROM journey_contributors WHERE journey_id = ? AND user_id = ?'
).get(journeyId, userId) as { role: 'editor' | 'viewer' } | undefined;
myRole = contribRow?.role ?? null;
}
return {
...journey,
entries: enrichedEntries,
@@ -174,6 +187,7 @@ export function getJourneyFull(journeyId: number, userId: number) {
contributors,
stats: { entries: entryCount, photos: photoCount, places: places.length },
hide_skeletons: !!(userPrefs?.hide_skeletons),
my_role: myRole,
};
}
@@ -184,7 +198,9 @@ export function updateJourney(journeyId: number, userId: number, data: Partial<{
cover_image: string;
status: string;
}>): Journey | null {
if (!canEdit(journeyId, userId)) return null;
// Journey-level settings (title, cover, status) are owner-only — editors
// may only edit entries and photos, not reshape the journey itself.
if (!isOwner(journeyId, userId)) return null;
const ALLOWED_STATUSES = ['draft', 'active', 'completed', 'archived'];
const allowed = ['title', 'subtitle', 'cover_gradient', 'cover_image', 'status'];
@@ -615,6 +631,14 @@ export function deleteEntry(entryId: number, userId: number, sid?: string): bool
// ── Photos ───────────────────────────────────────────────────────────────
// Promote a skeleton suggestion to a concrete entry. Called whenever the user
// adds content (photo upload, provider photo, gallery link) — a suggestion
// with photos is no longer just a suggestion.
function promoteSkeletonIfNeeded(entry: JourneyEntry): void {
if (entry.type !== 'skeleton') return;
db.prepare('UPDATE journey_entries SET type = ?, updated_at = ? WHERE id = ?').run('entry', ts(), entry.id);
}
export function addPhoto(entryId: number, userId: number, filePath: string, thumbnailPath?: string, caption?: string): JourneyPhoto | null {
const entry = db.prepare('SELECT * FROM journey_entries WHERE id = ?').get(entryId) as JourneyEntry | undefined;
if (!entry) return null;
@@ -629,6 +653,8 @@ export function addPhoto(entryId: number, userId: number, filePath: string, thum
VALUES (?, ?, ?, ?, ?)
`).run(entryId, trekPhotoId, caption || null, (maxOrder?.m ?? -1) + 1, now);
promoteSkeletonIfNeeded(entry);
return db.prepare(`SELECT ${JP_SELECT} FROM ${JP_JOIN} WHERE jp.id = ?`).get(Number(res.lastInsertRowid)) as JourneyPhoto;
}
@@ -651,6 +677,8 @@ export function addProviderPhoto(entryId: number, userId: number, provider: stri
VALUES (?, ?, ?, ?, ?)
`).run(entryId, trekPhotoId, caption || null, (maxOrder?.m ?? -1) + 1, now);
promoteSkeletonIfNeeded(entry);
return db.prepare(`SELECT ${JP_SELECT} FROM ${JP_JOIN} WHERE jp.id = ?`).get(Number(res.lastInsertRowid)) as JourneyPhoto;
}
@@ -664,21 +692,41 @@ export function linkPhotoToEntry(entryId: number, photoId: number, userId: numbe
if (source.entry_id === entryId) return source;
const oldEntryId = source.entry_id;
const oldEntry = db.prepare('SELECT * FROM journey_entries WHERE id = ?').get(source.entry_id) as JourneyEntry | undefined;
const sourceIsGallery = oldEntry?.title === 'Gallery';
// move photo to the target entry
db.prepare('UPDATE journey_photos SET entry_id = ? WHERE id = ?').run(entryId, photoId);
// skip if target already has this photo (by trek_photo_id)
const dupe = db.prepare('SELECT id FROM journey_photos WHERE entry_id = ? AND photo_id = ?').get(entryId, source.photo_id) as { id: number } | undefined;
if (dupe) return db.prepare(`SELECT ${JP_SELECT} FROM ${JP_JOIN} WHERE jp.id = ?`).get(dupe.id) as JourneyPhoto;
// clean up: if old entry was a "Gallery" entry and is now empty, delete it
const oldEntry = db.prepare('SELECT * FROM journey_entries WHERE id = ?').get(oldEntryId) as JourneyEntry | undefined;
if (oldEntry && oldEntry.title === 'Gallery') {
const remaining = db.prepare('SELECT COUNT(*) as c FROM journey_photos WHERE entry_id = ?').get(oldEntryId) as { c: number };
const maxOrder = db.prepare('SELECT MAX(sort_order) as m FROM journey_photos WHERE entry_id = ?').get(entryId) as { m: number | null };
let resultId: number;
if (sourceIsGallery) {
// Copy so the photo stays in the gallery even after being used in an entry.
const res = db.prepare(`
INSERT INTO journey_photos (entry_id, photo_id, caption, sort_order, created_at)
VALUES (?, ?, ?, ?, ?)
`).run(entryId, source.photo_id, source.caption || null, (maxOrder?.m ?? -1) + 1, ts());
resultId = Number(res.lastInsertRowid);
} else {
// Non-gallery source: keep existing move behavior.
db.prepare('UPDATE journey_photos SET entry_id = ? WHERE id = ?').run(entryId, photoId);
resultId = photoId;
}
promoteSkeletonIfNeeded(entry);
// If we moved out of a Gallery entry (shouldn't happen with the guard above,
// but kept for any legacy data), clean up the Gallery wrapper if emptied.
if (!sourceIsGallery && oldEntry && oldEntry.title === 'Gallery') {
const remaining = db.prepare('SELECT COUNT(*) as c FROM journey_photos WHERE entry_id = ?').get(source.entry_id) as { c: number };
if (remaining.c === 0) {
db.prepare('DELETE FROM journey_entries WHERE id = ?').run(oldEntryId);
db.prepare('DELETE FROM journey_entries WHERE id = ?').run(source.entry_id);
}
}
return db.prepare(`SELECT ${JP_SELECT} FROM ${JP_JOIN} WHERE jp.id = ?`).get(photoId) as JourneyPhoto;
return db.prepare(`SELECT ${JP_SELECT} FROM ${JP_JOIN} WHERE jp.id = ?`).get(resultId) as JourneyPhoto;
}
export function setPhotoProvider(photoId: number, provider: string, assetId: string, ownerId: number) {
+9 -2
View File
@@ -1,5 +1,6 @@
import { db } from '../db/database';
import crypto from 'crypto';
import { isOwner } from './journeyService';
interface JourneySharePermissions {
share_timeline?: boolean;
@@ -19,7 +20,11 @@ export function createOrUpdateJourneyShareLink(
journeyId: number,
createdBy: number,
permissions: JourneySharePermissions
): { token: string; created: boolean } {
): { token: string; created: boolean } | null {
// Public sharing is an owner-only action — editors/viewers must not be
// able to publish the journey or change which screens are shared.
if (!isOwner(journeyId, createdBy)) return null;
const {
share_timeline = true,
share_gallery = true,
@@ -51,8 +56,10 @@ export function getJourneyShareLink(journeyId: number): JourneyShareTokenInfo |
};
}
export function deleteJourneyShareLink(journeyId: number): void {
export function deleteJourneyShareLink(journeyId: number, userId: number): boolean {
if (!isOwner(journeyId, userId)) return false;
db.prepare('DELETE FROM journey_share_tokens WHERE journey_id = ?').run(journeyId);
return true;
}
export function validateShareTokenForPhoto(token: string, photoId: number): { journeyId: number; ownerId: number } | null {
@@ -25,12 +25,18 @@ export function isValidAssetId(id: string): boolean {
export function getConnectionSettings(userId: number) {
const creds = getImmichCredentials(userId);
const prefs = db.prepare('SELECT immich_auto_upload FROM users WHERE id = ?').get(userId) as { immich_auto_upload?: number } | undefined;
return {
immich_url: creds?.immich_url || '',
connected: !!(creds?.immich_url && creds?.immich_api_key),
auto_upload: !!(prefs?.immich_auto_upload),
};
}
export function setImmichAutoUpload(userId: number, enabled: boolean): void {
db.prepare('UPDATE users SET immich_auto_upload = ? WHERE id = ?').run(enabled ? 1 : 0, userId);
}
export async function saveImmichSettings(
userId: number,
immichUrl: string | undefined,
@@ -318,7 +318,9 @@ describe('updateJourney', () => {
expect(updated!.subtitle).toBe('New Sub');
});
it('JOURNEY-SVC-019: editor contributor can update', () => {
it('JOURNEY-SVC-019: editor contributor cannot update journey settings (#732)', () => {
// Post-#732: journey-level settings (title/cover/status) are owner-only.
// Editors keep access to entries and photos, but not the journey shell.
const { user: owner } = createUser(testDb);
const { user: editor } = createUser(testDb);
const journey = createJourney(testDb, owner.id, { title: 'Original' });
@@ -326,8 +328,7 @@ describe('updateJourney', () => {
const updated = updateJourney(journey.id, editor.id, { title: 'Edited' });
expect(updated).not.toBeNull();
expect(updated!.title).toBe('Edited');
expect(updated).toBeNull();
});
it('JOURNEY-SVC-020: viewer cannot update', () => {
@@ -176,13 +176,14 @@ describe('getJourneyShareLink', () => {
});
describe('deleteJourneyShareLink', () => {
it('JOURNEY-SHARE-007: removes an existing share link', () => {
it('JOURNEY-SHARE-007: owner can remove an existing share link', () => {
const { user } = createUser(testDb);
const journey = createJourney(testDb, user.id);
createOrUpdateJourneyShareLink(journey.id, user.id, {});
deleteJourneyShareLink(journey.id);
const ok = deleteJourneyShareLink(journey.id, user.id);
expect(ok).toBe(true);
expect(getJourneyShareLink(journey.id)).toBeNull();
});
@@ -190,7 +191,7 @@ describe('deleteJourneyShareLink', () => {
const { user } = createUser(testDb);
const journey = createJourney(testDb, user.id);
expect(() => deleteJourneyShareLink(journey.id)).not.toThrow();
expect(() => deleteJourneyShareLink(journey.id, user.id)).not.toThrow();
});
});