From 090e754077e54d6ec39dff859766de29d8967cf7 Mon Sep 17 00:00:00 2001 From: Maurice Date: Sat, 24 Oct 2026 04:05:59 +0000 Subject: [PATCH] fix(studio): a restore brings in no photo from outside the journey's gallery, letter fills counted restore_book_layout took any layout the caller sent, so a layout naming a photo of another journey was kept, and laying a page out on it put that photo into this book while place_photo refused the same id. The layout is now held to the rule a page restore states: every photo it names, in a frame, on a map marker or in the letters of a text, is in the gallery or on a page of the book already, or the call is refused with photo_not_in_gallery. restore_book_page and the design apply read the photos of a page in one place now (book-photo-refs.ts), which counts the photo a text is filled with. A page put back with letters filled with a photo from outside the gallery is refused as a frame of it would be, and a designed page whose letter fill photo left the gallery since is skipped as photo_gone; on a new book's cover the letters keep their colour instead, as its frame is emptied. A saved layout keeps no photo of the page it was taken from in its letters, as it keeps none in its frames, so laying another page out on it no longer carries that photo along. A layout kept before this fills such letters with the first photo of the page it is laid on, or leaves them in their colour. The style operation checks a letter fill photo only when the change sets the fill, so a title whose photo left the gallery can still be resized or recoloured, as the text operation already allowed. --- server/src/nest/journey/book-design-apply.ts | 24 ++++------ .../nest/journey/journey-book-edit.service.ts | 5 +- .../nest/journey-book-edit.book-level.test.ts | 35 +++++++++++++- .../nest/journey/book-design-apply.test.ts | 45 +++++++++++++++++- shared/src/book/book-doc-ops.spec.ts | 40 ++++++++++++++++ shared/src/book/book-doc-ops.ts | 13 ++++-- shared/src/book/book-layouts.spec.ts | 46 ++++++++++++++++++- shared/src/book/book-ops-elements.ts | 3 +- shared/src/book/book-page-ops.spec.ts | 29 ++++++++++++ shared/src/book/book-page-ops.ts | 23 ++-------- shared/src/book/book-photo-refs.ts | 39 ++++++++++++++++ shared/src/book/book-saved-layouts.ts | 26 +++++++++-- shared/src/book/book-text-ops.spec.ts | 14 ++++++ shared/src/index.ts | 1 + 14 files changed, 294 insertions(+), 49 deletions(-) create mode 100644 shared/src/book/book-photo-refs.ts diff --git a/server/src/nest/journey/book-design-apply.ts b/server/src/nest/journey/book-design-apply.ts index b6d8215bd..3eb332588 100644 --- a/server/src/nest/journey/book-design-apply.ts +++ b/server/src/nest/journey/book-design-apply.ts @@ -1,4 +1,5 @@ import { + bookSpreadPhotoIds, compareCodeUnits, MAX_SPREADS, type BookDesignPlanRow, @@ -53,15 +54,6 @@ export function canonical(value: unknown): string { const samePage = (a: BookSpread | undefined, b: BookSpread | undefined) => a !== undefined && b !== undefined && canonical(a) === canonical(b); -/** The photos a page shows: its frames, placed or parked, and the pictures its maps mark stops with. */ -function photosOf(spread: BookSpread): number[] { - return [...spread.elements, ...spread.parked].flatMap((el) => { - if (el.kind === 'photo') return el.photoId === null ? [] : [el.photoId]; - if (el.kind === 'map') return el.points.flatMap((p) => (p.photoId === null ? [] : [p.photoId])); - return []; - }); -} - /** Where a new page goes in `spreads`: after the nearest plan page before it that is there, inside the covers. */ function insertAt(spreads: readonly BookSpread[], plan: readonly BookDesignPlanRow[], row: number, page: BookSpread) { /** Where the first page of one of these roles stands; the end of the book when there is none. */ @@ -86,17 +78,21 @@ function insertAt(spreads: readonly BookSpread[], plan: readonly BookDesignPlanR /** * A framing page of a new book without the photos that left the gallery: the - * book needs its cover and its back, so their frames are emptied (and a map's - * stops lose their picture) rather than the page left out. + * book needs its cover and its back, so their frames are emptied (a map's + * stops lose their picture, and letters filled with one keep their colour) + * rather than the page left out. */ function withoutGonePhotos(spread: BookSpread, gallery: ReadonlySet): BookSpread { const gone = (id: number | null) => id !== null && !gallery.has(id); - if (!photosOf(spread).some(gone)) return spread; + if (!bookSpreadPhotoIds(spread).some(gone)) return spread; const clean = (el: BookSpread['elements'][number]): BookSpread['elements'][number] => { if (el.kind === 'photo') return gone(el.photoId) ? { ...el, photoId: null } : el; if (el.kind === 'map') { return { ...el, points: el.points.map((p) => (gone(p.photoId) ? { ...p, photoId: null } : p)) }; } + if (el.kind === 'text' && el.fillPaint?.type === 'photo' && gone(el.fillPaint.photoId)) { + return { ...el, fillPaint: null }; + } return el; }; return { ...spread, elements: spread.elements.map(clean), parked: spread.parked.map(clean) }; @@ -116,7 +112,7 @@ function newBook(draft: DraftToApply, gallery: ReadonlySet): AppliedDesi skipped.push({ spreadId: spread.id, reason: 'excluded' }); return []; } - if (!photosOf(spread).every((id) => gallery.has(id))) { + if (!bookSpreadPhotoIds(spread).every((id) => gallery.has(id))) { skipped.push({ spreadId: spread.id, reason: 'photo_gone' }); return []; } @@ -175,7 +171,7 @@ export function applyDesignDraft( } const page = designed.get(row.spreadId); if (!page) return; - if (!photosOf(page).every((id) => gallery.has(id))) return skip(row.spreadId, 'photo_gone'); + if (!bookSpreadPhotoIds(page).every((id) => gallery.has(id))) return skip(row.spreadId, 'photo_gone'); if (row.action === 'replace') { if (at < 0 || !samePage(spreads[at], base.get(row.spreadId))) return skip(row.spreadId, 'changed_since'); spreads[at] = page; diff --git a/server/src/nest/journey/journey-book-edit.service.ts b/server/src/nest/journey/journey-book-edit.service.ts index c86a9bb00..ffb1484e1 100644 --- a/server/src/nest/journey/journey-book-edit.service.ts +++ b/server/src/nest/journey/journey-book-edit.service.ts @@ -381,10 +381,11 @@ export class JourneyBookEditService { /** * `restore_book_layout`, the assistant's undo of `remove_book_layout`: the * layout it answered put back, only while the book is at `expectedVersion`. - * The layout comes as JSON and is parsed again like any argument. + * The layout comes as JSON and is parsed again like any argument; a photo it + * names is the gallery's or the book's already, as for a page put back. */ async restoreLayout(journeyId: number, userId: number, args: RestoreBookLayoutRequest): Promise { - return this.write(journeyId, userId, (doc) => restoreBookLayout(stored(doc), parsedJson(args.layout)), { + return this.write(journeyId, userId, (doc, ctx) => restoreBookLayout(stored(doc), parsedJson(args.layout), ctx), { expectedVersion: args.expectedVersion, }); } diff --git a/server/tests/unit/nest/journey-book-edit.book-level.test.ts b/server/tests/unit/nest/journey-book-edit.book-level.test.ts index 9726ec333..09c651358 100644 --- a/server/tests/unit/nest/journey-book-edit.book-level.test.ts +++ b/server/tests/unit/nest/journey-book-edit.book-level.test.ts @@ -1,7 +1,8 @@ /** * JourneyBookEditService, the book-level tools and what the engine now knows * (JBLVL-*): the page format and numbers set and set back at a version, a - * page copied, a layout kept, taken away and put back; a page of a design + * page copied, a layout kept, taken away and put back, with no photo from + * outside the journey's gallery; a page of a design * draft read instead of the book's (410 once the draft is gone); the print * check reading the original's size from the pixel signals; and the travel * context in the user's language. Taken from JourneyBookModule over a @@ -149,6 +150,38 @@ describe('the book-level tools', () => { }); }); +describe('what a restore may bring back', () => { + it('JBLVL-007: restore_book_layout takes no photo from outside the gallery or the book, letters too', async () => { + const { journey, owner, inner } = await crew(); + const mine = await galleryPhoto(journey.id, owner.id); + const elsewhere = await makeJourney(orm, owner.id, { title: 'Porto' }); + const foreign = await galleryPhoto(elsewhere.id, owner.id); + const saved = await edit.saveLayout(journey.id, owner.id, { spreadId: inner, name: 'Two photos' }); + const layoutId = saved.layout_id ?? ''; + const removed = await edit.removeLayout(journey.id, owner.id, { layoutId, expectedVersion: saved.version }); + const layout = removed.previous_layout; + if (!layout) throw new Error('no layout answered'); + const frame = { x: 10, y: 10, w: 80, h: 80 }; + const framed = (photoId: number) => + JSON.stringify({ ...layout, elements: [...layout.elements, { id: 'p-x', kind: 'photo', frame, photoId }] }); + const lettered = (photoId: number) => + JSON.stringify({ + ...layout, + elements: [{ id: 't-x', kind: 'text', frame, text: 'Porto', fillPaint: { type: 'photo', photoId } }], + }); + const at = removed.version; + const restore = (json: string) => + refusal(() => edit.restoreLayout(journey.id, owner.id, { layout: json, expectedVersion: at })); + const stray = { status: 422, code: 'book_photo_not_in_gallery' }; + + expect(await restore(framed(foreign))).toMatchObject(stray); + expect(await restore(lettered(foreign))).toMatchObject(stray); + const back = await edit.restoreLayout(journey.id, owner.id, { layout: framed(mine), expectedVersion: at }); + expect(back.version).toBe(at + 1); + expect((await edit.overview(journey.id, owner.id)).layouts.map((l) => l.id)).toEqual([layoutId]); + }); +}); + describe('what the engine reads', () => { it('JBLVL-004: describe_book_page and a trial read a page of a design draft, its maker only, 410 once it is gone', async () => { const { journey, owner, viewer } = await crew(); diff --git a/server/tests/unit/nest/journey/book-design-apply.test.ts b/server/tests/unit/nest/journey/book-design-apply.test.ts index 8ae500c5c..f742bebee 100644 --- a/server/tests/unit/nest/journey/book-design-apply.test.ts +++ b/server/tests/unit/nest/journey/book-design-apply.test.ts @@ -7,8 +7,8 @@ * format and title come along unless someone set others since, and a page * size moved since takes nothing. A journey that had no book gets the chosen * pages as a new book (a cover photo gone since leaves an empty frame, a map - * stop's picture gone since leaves a dot), one that lost its book since, or - * got one since, gets nothing. A plan row the draft holds no page for writes + * stop's picture gone since leaves a dot, letters filled with it keep their + * colour), one that lost its book since, or got one since, gets nothing. A plan row the draft holds no page for writes * nothing, and a book at the page limit takes no new page. */ import { applyDesignDraft, type DraftToApply } from '../../../../src/nest/journey/book-design-apply'; @@ -18,6 +18,7 @@ import { bookPageSetupForPreset, bookPhotoElementSchema, bookShapeElementSchema, + bookTextElementSchema, MAX_SPREADS, type BookDesignPlanRow, type BookDocument, @@ -319,4 +320,44 @@ describe('applyDesignDraft', () => { expect(out.skipped).toEqual([{ spreadId: 'n', reason: 'changed_since' }]); expect(out.doc.spreads).toHaveLength(MAX_SPREADS); }); + + it('JBAPPLY-015: a page whose letters are filled with a photo gone since stays out, as a frame of it would', () => { + const lettered = (id: string, photoId: number) => + bookTextElementSchema.parse({ + id, + kind: 'text', + frame: { x: 0, y: 0, w: 80, h: 30 }, + text: 'Lisbon', + fillPaint: { type: 'photo', photoId }, + }); + const base = stored(); + const filled = { ...page('a', 'inner'), elements: [lettered('t-a', 42)] }; + const draft: DraftToApply = { base, doc: book([filled]), plan: [row('a', 'replace')], selection: ['a'] }; + + const out = applyDesignDraft(base, draft, new Set([7])); + + expect(out.applied).toEqual([]); + expect(out.skipped).toEqual([{ spreadId: 'a', reason: 'photo_gone' }]); + expect(applyDesignDraft(base, draft, new Set([42])).applied).toEqual(['a']); + }); + + it('JBAPPLY-016: a new book keeps its cover title when its fill photo left the gallery, in its own colour', () => { + const framed = page('c', 'cover', [42]); + const title = bookTextElementSchema.parse({ + id: 't-c', + kind: 'text', + frame: { x: 0, y: 0, w: 80, h: 30 }, + text: 'Lisbon', + fillPaint: { type: 'photo', photoId: 42 }, + }); + const doc = book([{ ...framed, elements: [...framed.elements, title] }, ...pages('f:first', 'l:last', 'k:back')]); + const draft: DraftToApply = { base: null, doc, plan: doc.spreads.map((s) => row(s.id, 'insert')), selection: [] }; + + const out = applyDesignDraft(null, draft, new Set()); + + const [frame, words] = out.doc.spreads[0]?.elements ?? []; + expect(frame).toMatchObject({ kind: 'photo', photoId: null }); + expect(words).toMatchObject({ kind: 'text', text: 'Lisbon', fillPaint: null }); + expect(out.applied).toEqual(['c', 'f', 'l', 'k']); + }); }); diff --git a/shared/src/book/book-doc-ops.spec.ts b/shared/src/book/book-doc-ops.spec.ts index a6c8b775a..c3cdfd786 100644 --- a/shared/src/book/book-doc-ops.spec.ts +++ b/shared/src/book/book-doc-ops.spec.ts @@ -3,9 +3,11 @@ import { blankBook } from './book-pages'; import { MAX_BOOK_LAYOUTS, bookDocumentSchema, + bookPhotoElementSchema, bookTextElementSchema, type BookDocument, type BookElement, + type BookLayout, type BookSpread, } from './book.schema'; @@ -100,4 +102,42 @@ describe('pages and layouts', () => { expect(broken).toMatchObject({ ok: false, error: { code: 'invalid_op' } }); expect(removeBookLayout(saved, { layoutId: 'ly-nope' })).toMatchObject({ error: { code: 'layout_not_found' } }); }); + + it('BOOKDOC-007: puts a layout back only with photos of the gallery or of the book, a letter fill counted', () => { + const saved = ok(saveBookLayout(book(), { spreadId: 'sp-in' }, { newId })).doc; + const layout = saved.layouts?.[0] as BookLayout; + const removed = ok(removeBookLayout(saved, { layoutId: layout.id })).doc; + const ctx = { photos: [{ photoId: 5 }] }; + const framed = (photoId: number): BookLayout => ({ + ...layout, + elements: [...layout.elements, bookPhotoElementSchema.parse({ id: 'p-1', kind: 'photo', frame, photoId })], + }); + const lettered = (photoId: number): BookLayout => ({ + ...layout, + elements: [ + bookTextElementSchema.parse({ + id: 't-2', + kind: 'text', + frame, + text: 'Fjord', + fillPaint: { type: 'photo', photoId }, + }), + ], + }); + const stray = { ok: false, error: { code: 'photo_not_in_gallery' } }; + + expect(restoreBookLayout(removed, framed(999), ctx)).toMatchObject(stray); + expect(restoreBookLayout(removed, lettered(999), ctx)).toMatchObject(stray); + expect(ok(restoreBookLayout(removed, framed(5), ctx)).doc.layouts).toHaveLength(1); + expect(ok(restoreBookLayout(removed, lettered(5), ctx)).doc.layouts).toHaveLength(1); + + // A photo on a page of the book already may come back, and without a gallery nothing is checked. + const pictured = (removed.spreads[2] as BookSpread).elements.concat(framed(999).elements.slice(-1)); + const holding = { + ...removed, + spreads: removed.spreads.map((sp, i) => (i === 2 ? { ...sp, elements: pictured } : sp)), + }; + expect(ok(restoreBookLayout(holding, lettered(999), ctx)).doc.layouts).toHaveLength(1); + expect(ok(restoreBookLayout(removed, framed(999))).doc.layouts).toHaveLength(1); + }); }); diff --git a/shared/src/book/book-doc-ops.ts b/shared/src/book/book-doc-ops.ts index fd7fe5135..c9d6189cd 100644 --- a/shared/src/book/book-doc-ops.ts +++ b/shared/src/book/book-doc-ops.ts @@ -17,6 +17,7 @@ import { setBookSetupRequestSchema, } from './book-ops.schema'; import { BOOK_PAGE_PRESET_SIZES, canEditSpread } from './book-pages'; +import { bookElementPhotoIds, checkRestorablePhotos } from './book-photo-refs'; import { bookLayoutFromSpread, nextBookLayoutName } from './book-saved-layouts'; import { MAX_BOOK_LAYOUTS, @@ -149,9 +150,12 @@ export function removeBookLayout(doc: BookDocument, args: unknown): BookDocOpRes /** * A removed layout put back as it was, for the assistant's undo of * `remove_book_layout`. The layout comes from a stored undo record, so it is - * parsed again like every other argument. + * parsed again like every other argument, and held to a page restore's rule: + * with a gallery in the context, a photo it names (a frame's, a marker's, a + * letter fill's) is in the gallery or on a page of the book already, since a + * layout laid on a page brings its photos along. */ -export function restoreBookLayout(doc: BookDocument, raw: unknown): BookDocOpResult { +export function restoreBookLayout(doc: BookDocument, raw: unknown, ctx: BookOpsContext = {}): BookDocOpResult { const parsed = bookLayoutSchema.safeParse(raw); if (!parsed.success) return { ok: false, error: invalidOp(parsed.error, null) }; const layout = parsed.data; @@ -160,5 +164,8 @@ export function restoreBookLayout(doc: BookDocument, raw: unknown): BookDocOpRes if (layouts.length >= MAX_BOOK_LAYOUTS) { return refused('layouts_full', `a book keeps at most ${MAX_BOOK_LAYOUTS} layouts; remove one first`); } - return done({ ...doc, layouts: [...layouts, layout] }, { created: [layout.id] }); + return caught(() => { + checkRestorablePhotos(bookElementPhotoIds(layout.elements), doc, ctx); + return done({ ...doc, layouts: [...layouts, layout] }, { created: [layout.id] }); + }); } diff --git a/shared/src/book/book-layouts.spec.ts b/shared/src/book/book-layouts.spec.ts index b998fe6a0..a59f2e3f5 100644 --- a/shared/src/book/book-layouts.spec.ts +++ b/shared/src/book/book-layouts.spec.ts @@ -6,13 +6,17 @@ import { findBookTemplate, isBookOrnamentText, } from './book-layouts'; -import { applySavedBookLayout, bookLayoutFromSpread } from './book-saved-layouts'; +import { bookElementPhotoIds, bookSpreadPhotoIds } from './book-photo-refs'; +import { applySavedBookLayout, bookLayoutFromSpread, bookLayoutPreview } from './book-saved-layouts'; import { bookPageSetupSchema, + bookPhotoElementSchema, bookSpreadSchema, bookTextElementSchema, type BookElement, + type BookLayout, type BookPageSetup, + type BookSpread, } from './book.schema'; import { describe, expect, it } from 'vitest'; @@ -259,3 +263,43 @@ describe('a mark set as text on a page laid out again', () => { expect(next.parked).toEqual([]); }); }); + +describe('letters filled with a photo in a saved layout', () => { + const frame = { x: 10, y: 10, w: 80, h: 40 }; + const filledWith = (photoId: number) => ({ type: 'photo' as const, photoId, focalX: 0.5, focalY: 0.5, zoom: 1 }); + const title = (photoId: number) => + bookTextElementSchema.parse({ + id: 'h', + kind: 'text', + frame, + text: 'Sintra', + size: 40, + fillPaint: filledWith(photoId), + }); + const pictured = (id: string, photoId: number) => + bookPhotoElementSchema.parse({ id, kind: 'photo', frame, photoId, focalX: 0.2, focalY: 0.7 }); + const spread = (id: string, elements: BookElement[]): BookSpread => + bookSpreadSchema.parse({ id, role: 'inner', elements }); + const fills = (sp: BookSpread) => sp.elements.flatMap((e) => (e.kind === 'text' ? [e.fillPaint ?? null] : [])); + + it('BOOKLAY-021: keeps no photo of the page it was saved from, in its frames or in its letters', () => { + const layout = bookLayoutFromSpread(spread('a', [pictured('p', 42), title(42)]), page, 'Title'); + expect(layout.elements.map((e) => e.kind)).toEqual(['photo', 'text']); + expect(bookElementPhotoIds(layout.elements)).toEqual([]); + const next = applySavedBookLayout(spread('b', [pictured('q', 7), title(7)]), layout, page); + expect(fills(next)).toEqual([null]); + expect(bookSpreadPhotoIds(next)).toEqual([7]); + }); + + it("BOOKLAY-022: an older layout's filled title takes the page's own photo, or its colour without one", () => { + const saved: BookLayout = { + ...bookLayoutFromSpread(spread('a', [pictured('p', 42)]), page, 'Old'), + elements: [bookPhotoElementSchema.parse({ id: 'p', kind: 'photo', frame }), title(42)], + }; + const onPhotos = applySavedBookLayout(spread('b', [pictured('q', 7), title(7)]), saved, page); + expect(fills(onPhotos)).toEqual([{ type: 'photo', photoId: 7, focalX: 0.2, focalY: 0.7, zoom: 1 }]); + const bare = applySavedBookLayout(spread('c', [title(7)]), saved, page); + expect(fills(bare)).toEqual([null]); + expect(fills(bookLayoutPreview(saved, page))).toEqual([null]); + }); +}); diff --git a/shared/src/book/book-ops-elements.ts b/shared/src/book/book-ops-elements.ts index 99c072b99..4e99c0567 100644 --- a/shared/src/book/book-ops-elements.ts +++ b/shared/src/book/book-ops-elements.ts @@ -445,7 +445,8 @@ function opStyle(spread: BookSpread, op: Op<'style'>, env: ElementOpEnv): BookSp throw failOp('wrong_kind', `${el.id} is a ${el.kind}; set.kind says ${op.set.kind}`, env.opIndex, el.id); } const styled = withProps(el, op.set, env.doc.theme); - checkTextFillPhoto(styled, env); + // Only a fill this style sets is checked: a text whose fill photo left the gallery can still be restyled. + if (op.set.kind === 'text' && op.set.fillPaint !== undefined) checkTextFillPhoto(styled, env); return replaceAll(spread, [styled]); } diff --git a/shared/src/book/book-page-ops.spec.ts b/shared/src/book/book-page-ops.spec.ts index 850cb4a62..e3aa2ab42 100644 --- a/shared/src/book/book-page-ops.spec.ts +++ b/shared/src/book/book-page-ops.spec.ts @@ -234,6 +234,35 @@ describe('restore', () => { expect(ok(restoreBookPage(withPhoto, args(stray))).spreads[3]?.elements[0]?.id).toBe('p-stray'); }); + it('BOOKPG-028: counts the photo the letters of a text are filled with, coming back and in the book', () => { + const base = book(2); + const box = { x: 10, y: 10, w: 80, h: 30 }; + const lettered = (id: string, photoId: number) => + bookTextElementSchema.parse({ + id, + kind: 'text', + frame: box, + text: 'Fjord', + fillPaint: { type: 'photo', photoId }, + }); + const withFill = { + ...base, + spreads: base.spreads.map((s) => (s.id === 'sp-in1' ? { ...s, elements: [lettered('t-kept', 7)] } : s)), + }; + const page = withFill.spreads[3] as BookSpread; + const photos = [{ photoId: 1 }]; + const args = (spread: BookSpread) => ({ pages: [{ spread, index: 3 }], expectedVersion: 1 }); + const stray = { ...page, elements: [lettered('t-stray', 99)] }; + expect(refused(restoreBookPage(withFill, args(stray), { photos }))).toBe('photo_not_in_gallery'); + // A photo the book holds only as a fill is in the book already. + const framed = bookPhotoElementSchema.parse({ id: 'p-again', kind: 'photo', frame: box, photoId: 7 }); + expect( + ok(restoreBookPage(withFill, args({ ...page, elements: [framed] }), { photos })).spreads[3]?.elements[0]?.id, + ).toBe('p-again'); + const fromGallery = { ...page, elements: [lettered('t-new', 1)] }; + expect(ok(restoreBookPage(withFill, args(fromGallery), { photos })).spreads[3]?.elements[0]?.id).toBe('t-new'); + }); + it('BOOKPG-026: keeps a locked element as it is on a page put back in place, and adds no locked one', () => { const base = book(1); const lockedText = bookTextElementSchema.parse({ diff --git a/shared/src/book/book-page-ops.ts b/shared/src/book/book-page-ops.ts index 5c62180bb..d741b8336 100644 --- a/shared/src/book/book-page-ops.ts +++ b/shared/src/book/book-page-ops.ts @@ -21,6 +21,7 @@ import { type BookPageOpInput, } from './book-ops.schema'; import { blankSpread, canEditSpread, innerSpreadBounds } from './book-pages'; +import { bookSpreadPhotoIds, checkRestorablePhotos } from './book-photo-refs'; import { MAX_SPREADS, bookSpreadSchema, @@ -148,7 +149,8 @@ export function restoreBookPage(doc: BookDocument, args: unknown, ctx: BookOpsCo .sort((a, b) => a.index - b.index); let cur = doc; for (const { spread, index } of pages) { - restorablePhotos(spread, cur, ctx); + // A letter fill counts as a photo, as a frame does (book-photo-refs.ts). + checkRestorablePhotos(bookSpreadPhotoIds(spread), cur, ctx); cur = putBack(cur, spread, index); } const restored = [...new Set(pages.map((p) => p.spread.id))]; @@ -184,25 +186,6 @@ function putBack(doc: BookDocument, spread: BookSpread, index: number): BookDocu return { ...doc, spreads: [...doc.spreads.slice(0, to), spread, ...doc.spreads.slice(to)] }; } -/** The photos a page shows: its photo frames, placed or parked, and the pictures on its map markers. */ -function photoIdsOf(spread: BookSpread): number[] { - return [...spread.elements, ...spread.parked].flatMap((e) => { - if (e.kind === 'photo') return e.photoId == null ? [] : [e.photoId]; - if (e.kind === 'map') return e.points.flatMap((p) => (p.photoId == null ? [] : [p.photoId])); - return []; - }); -} - -/** With a gallery known, every photo of a page coming back is in it or somewhere in the book already. */ -function restorablePhotos(spread: BookSpread, doc: BookDocument, ctx: BookOpsContext): void { - if (!ctx.photos) return; - const known = new Set([...ctx.photos.map((p) => p.photoId), ...doc.spreads.flatMap(photoIdsOf)]); - const stray = photoIdsOf(spread).find((id) => !known.has(id)); - if (stray !== undefined) { - throw failOp('photo_not_in_gallery', `photo ${stray} is neither in this journey's gallery nor in the book`, null); - } -} - /** Key order aside, the same value: a parsed and a stored element list their fields in different orders. */ function canonical(value: unknown): string { if (Array.isArray(value)) return `[${value.map(canonical).join(',')}]`; diff --git a/shared/src/book/book-photo-refs.ts b/shared/src/book/book-photo-refs.ts new file mode 100644 index 000000000..f7b0eba9a --- /dev/null +++ b/shared/src/book/book-photo-refs.ts @@ -0,0 +1,39 @@ +import { failOp, type BookOpsContext } from './book-ops-core'; +import type { BookDocument, BookElement, BookSpread } from './book.schema'; + +/** + * The photos a book refers to by id, read in one place: a photo frame's + * picture, the pictures on a map's markers, and the photo the letters of a + * text are filled with. Every check that a photo belongs to the journey + * reads this list (a restore, a design applied), so a kind of reference is + * never checked in one of them and missed in another. + */ + +/** The photo ids these elements show, in their order. */ +export function bookElementPhotoIds(elements: readonly BookElement[]): number[] { + return elements.flatMap((e) => { + if (e.kind === 'photo') return e.photoId == null ? [] : [e.photoId]; + if (e.kind === 'map') return e.points.flatMap((p) => (p.photoId == null ? [] : [p.photoId])); + if (e.kind === 'text' && e.fillPaint?.type === 'photo') return [e.fillPaint.photoId]; + return []; + }); +} + +/** The photo ids a page shows, placed or parked. */ +export function bookSpreadPhotoIds(spread: BookSpread): number[] { + return bookElementPhotoIds([...spread.elements, ...spread.parked]); +} + +/** + * With a gallery known, each of these photos is in it or on a page of the + * book already: what a restore may bring back. The first that is neither is + * refused, as an edit refuses a photo from outside the gallery. + */ +export function checkRestorablePhotos(photoIds: readonly number[], doc: BookDocument, ctx: BookOpsContext): void { + if (!ctx.photos) return; + const known = new Set([...ctx.photos.map((p) => p.photoId), ...doc.spreads.flatMap(bookSpreadPhotoIds)]); + const stray = photoIds.find((id) => !known.has(id)); + if (stray !== undefined) { + throw failOp('photo_not_in_gallery', `photo ${stray} is neither in this journey's gallery nor in the book`, null); + } +} diff --git a/shared/src/book/book-saved-layouts.ts b/shared/src/book/book-saved-layouts.ts index 1eaf9f66a..3ce3b804a 100644 --- a/shared/src/book/book-saved-layouts.ts +++ b/shared/src/book/book-saved-layouts.ts @@ -6,6 +6,7 @@ import { type BookLayout, type BookPageSetup, type BookSpread, + type BookTextElement, } from './book.schema'; /** @@ -25,6 +26,11 @@ import { export const bookLayoutRole = (spread: BookSpread): BookLayout['role'] => spread.role === 'inner' ? 'inner' : 'single'; +/** A text with letters filled with a photo, as a layout keeps it: in its colour, the picture being the page's. */ +function withoutFillPhoto(el: BookElement): BookElement { + return el.kind === 'text' && el.fillPaint?.type === 'photo' ? { ...el, fillPaint: null } : el; +} + /** The arrangement of a spread, as a layout to keep. */ export function bookLayoutFromSpread(spread: BookSpread, page: BookPageSetup, name: string): BookLayout { return { @@ -35,8 +41,8 @@ export function bookLayoutFromSpread(spread: BookSpread, page: BookPageSetup, na pageHeight: page.pageHeight, background: spread.background, ...(spread.backgroundPaint ? { backgroundPaint: spread.backgroundPaint } : {}), - // The design, not the pictures: every frame comes out empty. - elements: spread.elements.map((el) => (el.kind === 'photo' ? { ...el, photoId: null } : el)), + // The design, not the pictures: every frame comes out empty, and no letters keep the page's photo. + elements: spread.elements.map((el) => (el.kind === 'photo' ? { ...el, photoId: null } : withoutFillPhoto(el))), }; } @@ -50,6 +56,14 @@ export function nextBookLayoutName(existing: readonly BookLayout[], base: string const textSize = (el: BookElement) => (el.kind === 'text' ? el.size : 0); +/** Letters a layout fills with a photo take this page's first picture, or keep their colour on a page without one. */ +function photoFilled(el: BookTextElement, photo: BookElement | undefined): BookTextElement { + if (el.fillPaint?.type !== 'photo') return el; + if (photo?.kind !== 'photo' || photo.photoId == null) return { ...el, fillPaint: null }; + const fill = { type: 'photo' as const, photoId: photo.photoId, focalX: photo.focalX, focalY: photo.focalY, zoom: 1 }; + return { ...el, fillPaint: fill }; +} + /** * Lay a spread out on a saved layout, keeping what the spread holds. * @@ -105,11 +119,13 @@ export function applySavedBookLayout(spread: BookSpread, layout: BookLayout, pag continue; } if (base.kind === 'text' && el.kind === 'text') { + // A layout kept before its letters lost their photo still names the photo of the page it was saved from. + const own = photoFilled(base, photos[0]); const src = wordsFor.get(el); if (src && src.kind === 'text') { - out.push({ ...base, text: src.text, binding: src.binding, overridden: src.overridden }); + out.push({ ...own, text: src.text, binding: src.binding, overridden: src.overridden }); } else if (!el.binding) { - out.push(base); + out.push(own); } continue; } @@ -146,7 +162,7 @@ export function bookLayoutPreview(layout: BookLayout, page: BookPageSetup): Book elements: layout.elements.map( (el) => ({ - ...el, + ...withoutFillPhoto(el), frame: { x: el.frame.x * sx, y: el.frame.y * sy, w: el.frame.w * sx, h: el.frame.h * sy }, }) as BookElement, ), diff --git a/shared/src/book/book-text-ops.spec.ts b/shared/src/book/book-text-ops.spec.ts index 659785184..f2ba0f860 100644 --- a/shared/src/book/book-text-ops.spec.ts +++ b/shared/src/book/book-text-ops.spec.ts @@ -102,6 +102,20 @@ describe('style sets the effects of a text', () => { ); }); + it('BKTOP-013: styles a text whose fill photo left the gallery since, and checks only a fill it sets', () => { + const gone = text('t1', { fillPaint: { type: 'photo', photoId: 42 } }); + const resized = ok(run(book([gone]), [{ op: 'style', elementId: 't1', set: { kind: 'text', size: 40 } }])); + expect(only(resized)).toMatchObject({ size: 40, fillPaint: { type: 'photo', photoId: 42 } }); + const tinted = ok(run(book([gone]), [{ op: 'style', elementId: 't1', set: { kind: 'text', color: '#c0392b' } }])); + expect(only(tinted).color).toBe('#c0392b'); + const cleared = ok(run(book([gone]), [{ op: 'style', elementId: 't1', set: { kind: 'text', fillPaint: null } }])); + expect(only(cleared).fillPaint).toBeNull(); + const again = { type: 'photo' as const, photoId: 42 }; + expect( + refusal(run(book([gone]), [{ op: 'style', elementId: 't1', set: { kind: 'text', fillPaint: again } }])), + ).toBe('photo_not_in_gallery'); + }); + it('BKTOP-003: refuses an effect value the document cannot hold', () => { const bad = run(book([text('t1')]), [ { op: 'style', elementId: 't1', set: { kind: 'text', outline: { color: '#ffffff', width: 9 } } }, diff --git a/shared/src/index.ts b/shared/src/index.ts index fb0c713a2..3f5ea58b8 100644 --- a/shared/src/index.ts +++ b/shared/src/index.ts @@ -133,6 +133,7 @@ export * from './book/book-line.schema'; export * from './book/book-line'; export * from './book/book-line-ops.schema'; export { bookNewLine, bookLineStyled } from './book/book-ops-line'; +export { bookSpreadPhotoIds } from './book/book-photo-refs'; export * from './book/book-normalize'; export * from './book/book-schema-rev'; export * from './book/journey-stats.schema';