From 72243dd341efd677066d8769da31675eb5f53af8 Mon Sep 17 00:00:00 2001 From: Kevin Rassool Date: Thu, 27 Aug 2026 21:15:45 +1000 Subject: [PATCH 1/2] fix: prevent custom pages overwriting each other during page sync syncPages names each local file by page.name, but every custom (page-builder) page shares the name "legacy". On campaigns with more than one custom page, all of them are written to the same legacy.json (last write wins), so most custom pages never appear locally and the surviving file's identity changes between syncs. Name files by the page's unique name when it is unique across the campaign, and fall back to the page path otherwise (cause-areas.json, terms.json, ...). Residual collisions after sanitization are disambiguated with a short uuid suffix. Deploy is unaffected: pages are PATCHed by the uuid stored inside each file. Co-Authored-By: Claude Fable 5 --- src/actions/sync.js | 49 +++++++++++++++++++++++++------ tests/sync.test.js | 71 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 9 deletions(-) create mode 100644 tests/sync.test.js diff --git a/src/actions/sync.js b/src/actions/sync.js index 67287b9..ceb9488 100644 --- a/src/actions/sync.js +++ b/src/actions/sync.js @@ -55,13 +55,43 @@ export async function syncStyles() { } } -function pageFileName(page) { - const base = - page.name || - (page.path && page.path !== '/' - ? page.path.replace(/^\//, '').replace(/\//g, '-') - : 'home'); - return `${base.replace(/[^a-zA-Z0-9._-]/g, '_')}.json`; +function pagePathBase(page) { + return page.path && page.path !== '/' + ? page.path.replace(/^\//, '').replace(/\//g, '-') + : 'home'; +} + +/** + * Compute a unique local file name for every page in a campaign. + * + * `page.name` is only unique for template pages — every custom + * (page-builder) page shares the name "legacy", so naming files by + * `page.name` alone makes all custom pages overwrite each other into a + * single legacy.json. Whenever a name is shared by more than one page, + * fall back to the page's path instead. + * + * @param {Array} pages Pages belonging to one campaign + * @returns {string[]} File name for each page, in the same order + */ +export function pageFileNames(pages) { + const nameCounts = new Map(); + for (const page of pages) { + if (page.name) { + nameCounts.set(page.name, (nameCounts.get(page.name) || 0) + 1); + } + } + + const used = new Set(); + return pages.map((page) => { + const nameIsUnique = page.name && nameCounts.get(page.name) === 1; + const base = nameIsUnique ? page.name : pagePathBase(page); + let fileName = `${base.replace(/[^a-zA-Z0-9._-]/g, '_')}.json`; + if (used.has(fileName) && page.uuid) { + fileName = fileName.replace(/\.json$/, `-${page.uuid.slice(0, 8)}.json`); + } + used.add(fileName); + return fileName; + }); } export async function syncPages() { @@ -87,7 +117,8 @@ export async function syncPages() { path: `/campaigns/${uuid}/pages?private=1&includeBody=1&limit=999`, }); - for (const page of pages.data) { + const fileNames = pageFileNames(pages.data); + for (const [index, page] of pages.data.entries()) { const out = { uuid: page.uuid, path: page.path, @@ -107,7 +138,7 @@ export async function syncPages() { }; fs.writeFileSync( - path.join(pagesDir, pageFileName(page)), + path.join(pagesDir, fileNames[index]), JSON.stringify(out, null, 4) ); } diff --git a/tests/sync.test.js b/tests/sync.test.js new file mode 100644 index 0000000..84ea3ac --- /dev/null +++ b/tests/sync.test.js @@ -0,0 +1,71 @@ +import { describe, expect, test } from 'vitest'; + +import { pageFileNames } from '../src/actions/sync.js'; + +describe('pageFileNames', () => { + test('names template pages by their unique name', () => { + const pages = [ + { uuid: 'u1', name: 'home', path: '/' }, + { uuid: 'u2', name: 'dashboard', path: '/dashboard' }, + { uuid: 'u3', name: 'profile', path: '/:id' }, + ]; + expect(pageFileNames(pages)).toEqual([ + 'home.json', + 'dashboard.json', + 'profile.json', + ]); + }); + + test('falls back to path when multiple pages share a name', () => { + // Custom (page-builder) pages all share the name "legacy" — naming + // files by name alone collapses them into a single legacy.json. + const pages = [ + { uuid: 'u1', name: 'legacy', path: '/cause-areas' }, + { uuid: 'u2', name: 'legacy', path: '/terms' }, + { uuid: 'u3', name: 'legacy', path: '/about/team' }, + { uuid: 'u4', name: 'home', path: '/' }, + ]; + expect(pageFileNames(pages)).toEqual([ + 'cause-areas.json', + 'terms.json', + 'about-team.json', + 'home.json', + ]); + }); + + test('keeps name-based file for a name used by only one page', () => { + const pages = [ + { uuid: 'u1', name: 'legacy', path: '/only-custom-page' }, + { uuid: 'u2', name: 'home', path: '/' }, + ]; + expect(pageFileNames(pages)).toEqual(['legacy.json', 'home.json']); + }); + + test('uses path for pages without a name', () => { + const pages = [ + { uuid: 'u1', name: null, path: '/register' }, + { uuid: 'u2', name: null, path: '/signup' }, + ]; + expect(pageFileNames(pages)).toEqual(['register.json', 'signup.json']); + }); + + test('sanitizes special characters in paths', () => { + const pages = [ + { uuid: 'u1', name: null, path: '/reset/:passwordResetToken?' }, + ]; + expect(pageFileNames(pages)).toEqual([ + 'reset-_passwordResetToken_.json', + ]); + }); + + test('disambiguates residual file name collisions with a uuid suffix', () => { + const pages = [ + { uuid: 'aaaaaaaa-1111', name: 'legacy', path: '/promo!' }, + { uuid: 'bbbbbbbb-2222', name: 'legacy', path: '/promo?' }, + ]; + expect(pageFileNames(pages)).toEqual([ + 'promo_.json', + 'promo_-bbbbbbbb.json', + ]); + }); +}); From 5489e7d44e6d7d7da0240d8029486cfa393439f4 Mon Sep 17 00:00:00 2001 From: Kevin Rassool Date: Thu, 27 Aug 2026 21:23:24 +1000 Subject: [PATCH 2/2] fix: reserve unique-name filenames before assigning path-based names Two-pass assignment so a custom page whose path sanitizes to a template page's name (e.g. /profile vs the profile template at /:id) can never take that template's filename, regardless of API order. The custom page gets the uuid-suffixed name instead. Addresses Bugbot review on #85. Co-Authored-By: Claude Fable 5 --- src/actions/sync.js | 38 ++++++++++++++++++++++++++++++-------- tests/sync.test.js | 16 ++++++++++++++++ 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/actions/sync.js b/src/actions/sync.js index ceb9488..ff0b5db 100644 --- a/src/actions/sync.js +++ b/src/actions/sync.js @@ -81,16 +81,38 @@ export function pageFileNames(pages) { } } + const sanitize = (base) => `${base.replace(/[^a-zA-Z0-9._-]/g, '_')}.json`; + + // First pass: pages with a unique name keep their name-based file. + // Reserving these up front keeps template filenames stable regardless + // of API order (a custom page at e.g. /profile must never take + // profile.json from the profile template page). const used = new Set(); - return pages.map((page) => { - const nameIsUnique = page.name && nameCounts.get(page.name) === 1; - const base = nameIsUnique ? page.name : pagePathBase(page); - let fileName = `${base.replace(/[^a-zA-Z0-9._-]/g, '_')}.json`; - if (used.has(fileName) && page.uuid) { - fileName = fileName.replace(/\.json$/, `-${page.uuid.slice(0, 8)}.json`); + const fileNames = pages.map((page) => { + if (page.name && nameCounts.get(page.name) === 1) { + const fileName = sanitize(page.name); + used.add(fileName); + return fileName; + } + return null; + }); + + // Second pass: pages with a shared or missing name fall back to their + // path, with a short uuid suffix on any residual collision. + return fileNames.map((fileName, index) => { + if (fileName) { + return fileName; + } + const page = pages[index]; + let candidate = sanitize(pagePathBase(page)); + if (used.has(candidate) && page.uuid) { + candidate = candidate.replace( + /\.json$/, + `-${page.uuid.slice(0, 8)}.json` + ); } - used.add(fileName); - return fileName; + used.add(candidate); + return candidate; }); } diff --git a/tests/sync.test.js b/tests/sync.test.js index 84ea3ac..384c968 100644 --- a/tests/sync.test.js +++ b/tests/sync.test.js @@ -58,6 +58,22 @@ describe('pageFileNames', () => { ]); }); + test('template filenames are reserved regardless of API order', () => { + // A custom page at the literal path /profile must not take + // profile.json from the profile template page (path /:id), even + // when the API returns the custom page first. + const pages = [ + { uuid: 'cccccccc-3333', name: 'legacy', path: '/profile' }, + { uuid: 'dddddddd-4444', name: 'legacy', path: '/terms' }, + { uuid: 'eeeeeeee-5555', name: 'profile', path: '/:id' }, + ]; + expect(pageFileNames(pages)).toEqual([ + 'profile-cccccccc.json', + 'terms.json', + 'profile.json', + ]); + }); + test('disambiguates residual file name collisions with a uuid suffix', () => { const pages = [ { uuid: 'aaaaaaaa-1111', name: 'legacy', path: '/promo!' },