fix(export): deleting a comment mid-build no longer strands the ZIP forever
Found while auditing the test suite: the ViewerOnly carry-forward (added by me in
9666d74) permanently breaks the ZIP download if a comment is deleted while the ZIP is
still being built.
A comment-only change doesn't alter the ZIP's media, so `Affects::ViewerOnly` carries the
finished archive into the new epoch rather than rebuilding it. But "finished" is a
PRECONDITION, and between `release_gallery` and the ZIP worker completing it is false —
for a real multi-GB gallery, for MINUTES. Deleting a comment in that window is an utterly
ordinary thing to do.
The carry-forward UPDATE is guarded on `status = 'done'`, so in that window it matched
nothing — and ViewerOnly re-armed only the viewer. The ZIP row was left at the retired
epoch; the in-flight worker finished and wrote `done` at an epoch `export_current` no
longer matches. `GET /export/zip` then 404s FOREVER: recovery only runs at boot, and
`release_gallery` refuses an already-released event. The guests' keepsake is simply gone,
and the only escape is the rebuild button added one commit ago.
Fix: the carry-forward's own `rows_affected()` decides. It matched => there is a current,
finished ZIP and only the viewer needs rebuilding. It didn't => there is nothing to
preserve, so rebuild the ZIP too, like any other invalidation. Never assume the
precondition; ask the UPDATE.
Proven non-vacuous: with the fix reverted the new test fails with the ZIP stranded at the
retired epoch (job epoch 1, event epoch 2, export_current holding only "html", download
404). Backend 40 tests, e2e 163 passed / 1 skipped.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -421,6 +421,60 @@ test.describe('Flow re-review — reopen→re-release export integrity + stale-k
|
||||
expect(listing.some((n) => n.includes(keep)), 'the kept photo survives').toBe(true);
|
||||
});
|
||||
|
||||
test('a comment deleted while the ZIP is still BUILDING does not strand the ZIP', async ({
|
||||
host,
|
||||
guest,
|
||||
}) => {
|
||||
// A comment-only change (`Affects::ViewerOnly`) carries the finished ZIP into the new epoch
|
||||
// rather than spending minutes rebuilding an archive whose media didn't change. But "finished"
|
||||
// is a PRECONDITION, and between `release_gallery` and the ZIP worker completing it is false —
|
||||
// for a real multi-GB gallery, for MINUTES. Deleting a comment in that window is an utterly
|
||||
// ordinary thing to do.
|
||||
//
|
||||
// The carry-forward UPDATE is guarded on `status = 'done'`, so in that window it matched
|
||||
// NOTHING — and ViewerOnly re-armed only the viewer. The ZIP row was left at the retired epoch,
|
||||
// the in-flight worker finished and wrote `done` at an epoch `export_current` no longer matches,
|
||||
// and `GET /export/zip` 404'd FOREVER. Nothing re-armed it: recovery only runs at boot, and
|
||||
// `release_gallery` refuses an already-released event.
|
||||
//
|
||||
// Fix: the carry-forward's own `rows_affected()` decides. Nothing carried ⇒ rebuild the ZIP too.
|
||||
const g = await guest('Commenter');
|
||||
const uploadId = await seedUpload(host.jwt, { caption: 'pic' });
|
||||
const cRes = await fetch(BASE + `/api/v1/upload/${uploadId}/comments`, {
|
||||
method: 'POST',
|
||||
headers: { Authorization: `Bearer ${g.jwt}`, 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ body: 'schoenes foto' }),
|
||||
});
|
||||
expect(cRes.status).toBe(201);
|
||||
const commentId = (await cRes.json()).id;
|
||||
|
||||
expect((await post('/api/v1/host/gallery/release', host.jwt)).status).toBe(204);
|
||||
await waitExportDone(host.jwt);
|
||||
|
||||
// Put the ZIP back into the state it occupies for most of a real release: still building.
|
||||
// Everything else in this test is the real product.
|
||||
await pgQuery(
|
||||
`UPDATE export_job ej SET status = 'running', progress_pct = 50
|
||||
FROM event e WHERE e.id = ej.event_id AND e.slug = '${SLUG}' AND ej.type = 'zip'`
|
||||
);
|
||||
|
||||
// A guest deletes their own comment → Affects::ViewerOnly.
|
||||
const del = await fetch(BASE + `/api/v1/comment/${commentId}`, {
|
||||
method: 'DELETE',
|
||||
headers: { Authorization: `Bearer ${g.jwt}` },
|
||||
});
|
||||
expect(del.status).toBe(204);
|
||||
|
||||
// The ZIP must have been RE-ARMED at the live epoch, not abandoned at the retired one.
|
||||
expect(await zipJobEpoch(), 'the ZIP must not be stranded at a retired epoch').toBe(
|
||||
await eventEpoch()
|
||||
);
|
||||
|
||||
// And it must actually become downloadable again, with its media intact.
|
||||
await waitExportDone(host.jwt);
|
||||
expect(await downloadZipEntries(host.jwt)).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('a release broadcasts export-progress 100 for both types and one export-available', async ({
|
||||
host,
|
||||
}) => {
|
||||
|
||||
Reference in New Issue
Block a user