fix(export): name the camera master in photo exports, not the delivered render (#1229) (#1230)

#1165 added photos.source_filename to this service's select, with a comment
saying it was there so the Lightroom round-trip could still match after a
re-upload — and then nothing read it. Every output path still used
original_filename, which is overwritten the first time an edited render is
uploaded over a proof (#745).

So after a round-trip the exports named the render. Each of these formats
exists to help a photographer find the master on disk, and the render's name
does not. The XMP case is the sharpest: the sidecar is written next to a RAW
master, so a wrongly-named one is never associated with it.

Two helpers, because the sites want different things when nothing is known:

  cameraName()             source_filename || original_filename || null
  cameraFilenameOrStored() the above, else the stored name

The dedicated `original_filename` fields (CSV column, JSON key) keep reporting
blank/null when unrecorded — echoing the sanitized stored name there would
invite a match against a file that does not exist under it. The places that
must emit some name (text list, CSV filename cell, XMP sidecar) fall back to
the stored one, as they did before.

filename_format='stored' is untouched, and rows with no source_filename still
resolve to original_filename, so nothing moves for installs that have never
run a replacement.

Co-authored-by: Paul Nothaft <[email protected]>
This commit is contained in:
Paul Nothaft
2026-08-29 11:10:05 +02:00
committed by GitHub
co-authored by Paul Nothaft
parent 4f684eb482
commit f4c054a661
2 changed files with 169 additions and 5 deletions
@@ -0,0 +1,136 @@
/**
* Photo exports name the camera master, not the delivered render (#1229).
*
* #1165 added `photos.source_filename` to this service's select and nothing
* read it — every output still used `original_filename`, which is overwritten
* the first time an edited render is uploaded over a proof (#745).
*
* So after a round-trip the exports named the render. Every format here exists
* to help a photographer find the master on disk, and the render's name does
* not. The XMP case is the sharpest: the sidecar is written next to a RAW
* master, so a wrongly-named one is never associated with it.
*/
jest.mock('../../src/database/db', () => ({
db: jest.fn(() => ({
where: () => ({ select: () => ({ first: async () => ({
event_name: 'Smith Wedding', event_date: '2026-08-29', slug: 'smith-wedding',
}) }) }),
})),
}));
const mockXmpBaseNames = [];
jest.mock('../../src/services/xmpGenerator', () => ({
XmpGenerator: class {
getXmpFilename(base) {
mockXmpBaseNames.push(base);
return `${base.replace(/\.[^.]+$/, '')}.xmp`;
}
generateXmp() { return '<x:xmpmeta/>'; }
},
}));
const { PhotoExportService } = require('../../src/services/photoExportService');
const service = new PhotoExportService();
// The post-round-trip row: the render's name landed in original_filename, the
// camera's name is still in source_filename.
const REPLACED = {
id: 1,
filename: 'wedding-smith_individual_1755892345.jpg',
original_filename: 'Smith_Wedding_1234.jpg',
source_filename: 'IMG_1234.JPG',
};
// Never replaced: source_filename holds what the camera wrote, same as original.
const UNTOUCHED = {
id: 2,
filename: 'wedding-smith_individual_1755892999.jpg',
original_filename: 'IMG_5678.JPG',
source_filename: 'IMG_5678.JPG',
};
// Predates migration 193 and its backfill — only the legacy column.
const LEGACY = {
id: 3,
filename: 'wedding-smith_individual_1755893111.jpg',
original_filename: 'DSC_9001.NEF',
source_filename: null,
};
// Nothing was ever recorded.
const BARE = {
id: 4,
filename: 'wedding-smith_individual_1755893222.jpg',
original_filename: null,
source_filename: null,
};
beforeEach(() => {
mockXmpBaseNames.length = 0;
});
describe('exportAsTxt (#1229)', () => {
it('lists the camera master after a replace, not the render', () => {
const result = service.exportAsTxt([REPLACED], { include_extension: true });
expect(result.content).toBe('IMG_1234.JPG');
});
it('still falls back to original_filename on rows with no source_filename', () => {
const result = service.exportAsTxt([LEGACY, UNTOUCHED]);
expect(result.content).toBe('DSC_9001.NEF\nIMG_5678.JPG');
});
it('falls back to the stored name when nothing was recorded', () => {
// A usable name beats an empty line in a list meant for a catalog search.
const result = service.exportAsTxt([BARE]);
expect(result.content).toBe('wedding-smith_individual_1755893222.jpg');
});
it('filename_format=stored is unaffected', () => {
const result = service.exportAsTxt([REPLACED], { filename_format: 'stored' });
expect(result.content).toBe('wedding-smith_individual_1755892345.jpg');
});
});
// Every cell is unconditionally quoted (formula-neutralized then wrapped),
// so strip the wrapper before comparing. No value under test contains a comma.
const cellsOf = (csv) => csv.split('\n')[1].split(',').map((c) => c.replace(/^"|"$/g, ''));
describe('exportAsCsv (#1229)', () => {
it('names the master in both the filename cell and the original_filename column', () => {
const cells = cellsOf(service.exportAsCsv([REPLACED]).content);
expect(cells[0]).toBe('IMG_1234.JPG');
expect(cells[1]).toBe('IMG_1234.JPG');
});
it('leaves the original_filename column blank when nothing was recorded', () => {
const cells = cellsOf(service.exportAsCsv([BARE]).content);
// Cell 0 still needs a usable name; the metadata column reports "unknown".
expect(cells[0]).toBe('wedding-smith_individual_1755893222.jpg');
expect(cells[1]).toBe('');
});
});
describe('exportAsXmpZip (#1229)', () => {
it('names the sidecar after the master so Lightroom associates it with the RAW', () => {
service.exportAsXmpZip([REPLACED, LEGACY], {});
expect(mockXmpBaseNames).toEqual(['IMG_1234.JPG', 'DSC_9001.NEF']);
});
it('filename_format=stored still uses the stored name', () => {
service.exportAsXmpZip([REPLACED], { filename_format: 'stored' });
expect(mockXmpBaseNames).toEqual(['wedding-smith_individual_1755892345.jpg']);
});
});
describe('exportAsJson (#1229)', () => {
it('reports the master as original_filename, and null when unrecorded', async () => {
const result = await service.exportAsJson([REPLACED, BARE], 7, {});
const parsed = JSON.parse(result.content);
expect(parsed.photos[0].original_filename).toBe('IMG_1234.JPG');
// The stored name stays its own field — this adds meaning, it does not swap.
expect(parsed.photos[0].filename).toBe('wedding-smith_individual_1755892345.jpg');
expect(parsed.photos[1].original_filename).toBeNull();
});
});
+33 -5
View File
@@ -14,6 +14,32 @@ const { db } = require('../database/db');
const path = require('path'); const path = require('path');
const fs = require('fs').promises; const fs = require('fs').promises;
/**
* The name the camera gave the file, or null when nothing was recorded (#1229).
*
* `source_filename` first. `original_filename` is overwritten the first time an
* edited render is uploaded over a proof (#745), so after a Lightroom
* round-trip it holds the render's name — and every export here is for finding
* the master on disk, which that name no longer does. `source_filename` is
* written once at ingest and survives a replace by design (migration 193).
*
* Null rather than the stored name: this feeds the two dedicated
* "original_filename" fields, where blank honestly reports "not recorded"
* instead of echoing a sanitized name that matches nothing.
*/
function cameraName(photo) {
return photo.source_filename || photo.original_filename || null;
}
/**
* A filename to actually write: the camera name when known, else the stored
* one. Used where the output needs *some* name — the text list, the CSV's
* filename cell, the XMP sidecar — and an empty string would be useless.
*/
function cameraFilenameOrStored(photo) {
return cameraName(photo) || photo.filename;
}
class PhotoExportService { class PhotoExportService {
constructor() { constructor() {
this.xmpGenerator = new XmpGenerator(); this.xmpGenerator = new XmpGenerator();
@@ -135,7 +161,7 @@ class PhotoExportService {
const filenames = photos.map(photo => { const filenames = photos.map(photo => {
const name = filename_format === 'original' const name = filename_format === 'original'
? (photo.original_filename || photo.filename) ? cameraFilenameOrStored(photo)
: photo.filename; : photo.filename;
return include_extension ? name : path.parse(name).name; return include_extension ? name : path.parse(name).name;
}); });
@@ -185,8 +211,8 @@ class PhotoExportService {
]; ];
const rows = photos.map(photo => [ const rows = photos.map(photo => [
filename_format === 'original' ? (photo.original_filename || photo.filename) : photo.filename, filename_format === 'original' ? cameraFilenameOrStored(photo) : photo.filename,
photo.original_filename || '', cameraName(photo) || '',
photo.average_rating ? parseFloat(photo.average_rating).toFixed(2) : '0.00', photo.average_rating ? parseFloat(photo.average_rating).toFixed(2) : '0.00',
photo.feedback_count || 0, photo.feedback_count || 0,
photo.like_count || 0, photo.like_count || 0,
@@ -239,8 +265,10 @@ class PhotoExportService {
archive.pipe(passthrough); archive.pipe(passthrough);
for (const photo of photos) { for (const photo of photos) {
// The sidecar is written next to a RAW master. Naming it after the
// edited render means Lightroom never associates the two.
const baseFilename = filename_format === 'original' const baseFilename = filename_format === 'original'
? (photo.original_filename || photo.filename) ? cameraFilenameOrStored(photo)
: photo.filename; : photo.filename;
const xmpFilename = this.xmpGenerator.getXmpFilename(baseFilename); const xmpFilename = this.xmpGenerator.getXmpFilename(baseFilename);
const xmpContent = this.xmpGenerator.generateXmp(project(photo), options); const xmpContent = this.xmpGenerator.generateXmp(project(photo), options);
@@ -279,7 +307,7 @@ class PhotoExportService {
photos: photos.map(photo => ({ photos: photos.map(photo => ({
id: photo.id, id: photo.id,
filename: photo.filename, filename: photo.filename,
original_filename: photo.original_filename || null, original_filename: cameraName(photo),
category: photo.category_name || null, category: photo.category_name || null,
rating: { rating: {
average: photo.average_rating ? parseFloat(parseFloat(photo.average_rating).toFixed(2)) : 0, average: photo.average_rating ? parseFloat(parseFloat(photo.average_rating).toFixed(2)) : 0,