From 260a2ea7236eb652f485ecaf2899b98692770d62 Mon Sep 17 00:00:00 2001 From: Kevin Date: Wed, 29 Jul 2026 09:52:45 -0400 Subject: [PATCH] PROD-2350: match page zones and template sections by ID, not position/name Page zone content was translated to target template sections by array position, so pages silently landed content in the wrong zone whenever source and target templates defined their sections in a different order. The same reference-name matching also affected template-pusher's section payload construction and its up-to-date change detection, risking duplicate sections or a missed update whenever a section was renamed. Adds SectionMapper (mirroring ModelMapper/ContainerMapper) to persist source/target pageItemTemplateID pairs, seeded from the confirmed savePageTemplate response and backfilled when an up-to-date template is skipped. translateZoneNames, the template payload builder, and hasTemplateChanged now prefer this ID mapping, falling back to a direct name match (and finally position) only when no mapping exists yet. Co-Authored-By: Claude Sonnet 5 --- src/lib/mappers/section-mapper.ts | 117 ++++++++ src/lib/mappers/template-mapper.ts | 76 +++-- src/lib/mappers/tests/section-mapper.test.ts | 158 ++++++++++ src/lib/mappers/tests/template-mapper.test.ts | 92 ++++++ src/lib/pushers/page-pusher/process-page.ts | 10 +- .../tests/translate-zone-names.test.ts | 75 +++++ .../page-pusher/translate-zone-names.ts | 81 +++++- src/lib/pushers/template-pusher.ts | 89 +++++- src/lib/pushers/tests/template-pusher.test.ts | 270 ++++++++++++++++++ 9 files changed, 924 insertions(+), 44 deletions(-) create mode 100644 src/lib/mappers/section-mapper.ts create mode 100644 src/lib/mappers/tests/section-mapper.test.ts diff --git a/src/lib/mappers/section-mapper.ts b/src/lib/mappers/section-mapper.ts new file mode 100644 index 00000000..ac253f44 --- /dev/null +++ b/src/lib/mappers/section-mapper.ts @@ -0,0 +1,117 @@ +import { fileOperations } from "../../core"; +import * as mgmtApi from "@agility/management-sdk"; + +interface SectionMapping { + sourceGuid: string; + targetGuid: string; + sourcePageItemTemplateID: number; + targetPageItemTemplateID: number; + sourceReferenceName?: string; + targetReferenceName?: string; +} + +// PROD-2350: content section definitions (page zones) were being matched between source and +// target templates by reference name (or worse, by array position) every time a page or template +// was pushed. That breaks whenever a section is renamed or reordered on either side. This mapper +// persists the source/target pageItemTemplateID pairing once (established in template-pusher.ts +// from the confirmed API response), so later lookups are a stable ID join instead of a repeated +// guess against possibly-stale names. +export class SectionMapper { + private fileOps: fileOperations; + private sourceGuid: string; + private targetGuid: string; + private mappings: SectionMapping[]; + private directory: string; + + constructor(sourceGuid: string, targetGuid: string) { + this.sourceGuid = sourceGuid; + this.targetGuid = targetGuid; + this.directory = "sections"; + // this will provide access to the /agility-files/{GUID} folder + this.fileOps = new fileOperations(targetGuid); + this.mappings = this.loadMapping(); + } + + getSectionMapping( + section: mgmtApi.ContentSectionDefinition, + type: "source" | "target" + ): SectionMapping | null { + if (!section) return null; + const mapping = this.mappings.find((m: SectionMapping) => + type === "source" + ? m.sourcePageItemTemplateID === section.pageItemTemplateID + : m.targetPageItemTemplateID === section.pageItemTemplateID + ); + if (!mapping) return null; + return mapping; + } + + getSectionMappingByID(pageItemTemplateID: number, type: "source" | "target"): SectionMapping | null { + const mapping = this.mappings.find((m: SectionMapping) => + type === "source" + ? m.sourcePageItemTemplateID === pageItemTemplateID + : m.targetPageItemTemplateID === pageItemTemplateID + ); + if (!mapping) return null; + return mapping; + } + + addMapping( + sourceSection: mgmtApi.ContentSectionDefinition, + targetSection: mgmtApi.ContentSectionDefinition + ) { + const targetMapping = this.getSectionMapping(targetSection, "target"); + const sourceMapping = this.getSectionMapping(sourceSection, "source"); + + if (targetMapping && sourceMapping && targetMapping !== sourceMapping) { + throw new Error( + `Invalid Mappings detected! Source pageItemTemplateID: ${sourceSection.pageItemTemplateID}, Target pageItemTemplateID: ${targetSection.pageItemTemplateID}` + ); + } + + if (targetMapping) { + this.updateMapping(sourceSection, targetSection, targetMapping); + } else { + const newMapping: SectionMapping = { + sourceGuid: this.sourceGuid, + targetGuid: this.targetGuid, + sourcePageItemTemplateID: sourceSection.pageItemTemplateID, + targetPageItemTemplateID: targetSection.pageItemTemplateID, + sourceReferenceName: sourceSection.pageItemTemplateReferenceName, + targetReferenceName: targetSection.pageItemTemplateReferenceName, + }; + + this.mappings.push(newMapping); + } + + this.saveMapping(); + } + + updateMapping( + sourceSection: mgmtApi.ContentSectionDefinition, + targetSection: mgmtApi.ContentSectionDefinition, + mapping: SectionMapping + ) { + if (targetSection.pageItemTemplateID !== mapping.targetPageItemTemplateID) { + throw new Error( + `Invalid items trying to be mapped! Source pageItemTemplateID: ${sourceSection.pageItemTemplateID}, Target pageItemTemplateID: ${targetSection.pageItemTemplateID}` + ); + } + mapping.sourceGuid = this.sourceGuid; + mapping.targetGuid = this.targetGuid; + mapping.sourcePageItemTemplateID = sourceSection.pageItemTemplateID; + mapping.targetPageItemTemplateID = targetSection.pageItemTemplateID; + mapping.sourceReferenceName = sourceSection.pageItemTemplateReferenceName; + mapping.targetReferenceName = targetSection.pageItemTemplateReferenceName; + this.saveMapping(); + } + + loadMapping() { + const mapping = this.fileOps.getMappingFile(this.directory, this.sourceGuid, this.targetGuid); + return mapping; + } + + saveMapping() { + this.fileOps.saveMappingFile(this.mappings, this.directory, this.sourceGuid, this.targetGuid); + } +} diff --git a/src/lib/mappers/template-mapper.ts b/src/lib/mappers/template-mapper.ts index bdc25b25..f19dadf4 100644 --- a/src/lib/mappers/template-mapper.ts +++ b/src/lib/mappers/template-mapper.ts @@ -1,5 +1,6 @@ import { fileOperations } from "../../core"; import * as mgmtApi from "@agility/management-sdk"; +import { SectionMapper } from "./section-mapper"; interface TemplateMapping { sourceGuid: string; targetGuid: string; @@ -9,24 +10,6 @@ interface TemplateMapping { targetPageTemplateName: string; } -// Templates have no lastModifiedDate, so change detection compares the source and -// target structure directly. Per-instance IDs (pageItemTemplateID, pageTemplateID, -// contentViewID, contentDefinitionID, itemContainerID) are excluded — they always -// differ between instances. Sections are sorted by reference name so payload -// ordering doesn't matter; itemOrder still captures real reordering. -function normalizeTemplate(template: mgmtApi.PageModel): string { - const sections = (template.contentSectionDefinitions || []) - .map((def) => ({ - name: def.pageItemTemplateName ?? null, - referenceName: def.pageItemTemplateReferenceName ?? null, - type: def.pageItemTemplateType ?? null, - itemOrder: def.itemOrder ?? null, - })) - .sort((a, b) => String(a.referenceName).localeCompare(String(b.referenceName))); - - return JSON.stringify({ pageTemplateName: template.pageTemplateName ?? null, sections }); -} - export class TemplateMapper { private fileOps: fileOperations; private sourceGuid: string; @@ -133,8 +116,61 @@ export class TemplateMapper { this.fileOps.saveMappingFile(this.mappings, this.directory, this.sourceGuid, this.targetGuid); } - hasTemplateChanged(sourceTemplate: mgmtApi.PageModel | null, targetTemplate: mgmtApi.PageModel | null): boolean { + // Templates have no lastModifiedDate, so change detection compares the source and target + // structure directly. Per-instance IDs (pageItemTemplateID, pageTemplateID, contentViewID, + // contentDefinitionID, itemContainerID) are excluded — they always differ between instances. + // + // PROD-2350: pair each source section with its target counterpart by the persisted + // pageItemTemplateID mapping when one is available — that's the actual identity, not a + // name-based guess — and only fall back to a reference-name match when no mapping exists yet + // (bootstrap, or sectionMapper not supplied). Matching purely by reference name can silently + // miss a real change (e.g. two sections trade reference names and item orders at once) since + // it re-derives the pairing from a value that isn't guaranteed to still identify the section. + hasTemplateChanged( + sourceTemplate: mgmtApi.PageModel | null, + targetTemplate: mgmtApi.PageModel | null, + sectionMapper?: SectionMapper | null + ): boolean { if (!sourceTemplate || !targetTemplate) return false; - return normalizeTemplate(sourceTemplate) !== normalizeTemplate(targetTemplate); + + if ((sourceTemplate.pageTemplateName ?? null) !== (targetTemplate.pageTemplateName ?? null)) return true; + + const sourceSections = sourceTemplate.contentSectionDefinitions || []; + const targetSections = targetTemplate.contentSectionDefinitions || []; + + if (sourceSections.length !== targetSections.length) return true; + + for (const sourceSection of sourceSections) { + if (!sourceSection) return true; + + let targetSection: mgmtApi.ContentSectionDefinition | null = null; + + if (sectionMapper && sourceSection.pageItemTemplateID != null) { + const sectionMapping = sectionMapper.getSectionMappingByID(sourceSection.pageItemTemplateID, "source"); + if (sectionMapping) { + targetSection = + targetSections.find((t) => t?.pageItemTemplateID === sectionMapping.targetPageItemTemplateID) ?? null; + } + } + + if (!targetSection) { + targetSection = + targetSections.find( + (t) => t?.pageItemTemplateReferenceName === sourceSection.pageItemTemplateReferenceName + ) ?? null; + } + + if (!targetSection) return true; + + const sectionChanged = + (sourceSection.pageItemTemplateName ?? null) !== (targetSection.pageItemTemplateName ?? null) || + (sourceSection.pageItemTemplateReferenceName ?? null) !== (targetSection.pageItemTemplateReferenceName ?? null) || + (sourceSection.pageItemTemplateType ?? null) !== (targetSection.pageItemTemplateType ?? null) || + (sourceSection.itemOrder ?? null) !== (targetSection.itemOrder ?? null); + + if (sectionChanged) return true; + } + + return false; } } diff --git a/src/lib/mappers/tests/section-mapper.test.ts b/src/lib/mappers/tests/section-mapper.test.ts new file mode 100644 index 00000000..9a408c50 --- /dev/null +++ b/src/lib/mappers/tests/section-mapper.test.ts @@ -0,0 +1,158 @@ +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { resetState, setState } from "core/state"; +import { SectionMapper } from "lib/mappers/section-mapper"; + +let tmpDir: string; + +beforeAll(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "agility-section-mapper-")); +}); + +afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +beforeEach(() => { + resetState(); + setState({ rootPath: tmpDir }); + jest.spyOn(console, "log").mockImplementation(() => {}); + jest.spyOn(console, "warn").mockImplementation(() => {}); + jest.spyOn(console, "error").mockImplementation(() => {}); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +let testCounter = 0; +let currentSrc: string; +let currentTgt: string; + +function makeMapper(): SectionMapper { + testCounter++; + currentSrc = `src-${testCounter}`; + currentTgt = `tgt-${testCounter}`; + return new SectionMapper(currentSrc, currentTgt); +} + +function makeSection(overrides: Record = {}): any { + return { + pageItemTemplateID: 1, + pageItemTemplateReferenceName: "Main", + itemOrder: 0, + contentViewID: 100, + ...overrides, + }; +} + +// ─── constructor ────────────────────────────────────────────────────────────── + +describe("SectionMapper constructor", () => { + it("constructs without throwing", () => { + expect(() => makeMapper()).not.toThrow(); + }); +}); + +// ─── getSectionMapping ──────────────────────────────────────────────────────── + +describe("SectionMapper.getSectionMapping", () => { + it("returns null when section is null", () => { + const mapper = makeMapper(); + expect(mapper.getSectionMapping(null as any, "source")).toBeNull(); + }); + + it("returns null when no mapping exists", () => { + const mapper = makeMapper(); + expect(mapper.getSectionMapping(makeSection({ pageItemTemplateID: 999 }), "source")).toBeNull(); + }); + + it("finds mapping by source pageItemTemplateID after addMapping", () => { + const mapper = makeMapper(); + mapper.addMapping( + makeSection({ pageItemTemplateID: 10, pageItemTemplateReferenceName: "SourceMain" }), + makeSection({ pageItemTemplateID: 20, pageItemTemplateReferenceName: "TargetMain" }) + ); + const found = mapper.getSectionMapping(makeSection({ pageItemTemplateID: 10 }), "source"); + expect(found).not.toBeNull(); + expect(found!.targetPageItemTemplateID).toBe(20); + }); + + it("finds mapping by target pageItemTemplateID after addMapping", () => { + const mapper = makeMapper(); + mapper.addMapping(makeSection({ pageItemTemplateID: 10 }), makeSection({ pageItemTemplateID: 20 })); + const found = mapper.getSectionMapping(makeSection({ pageItemTemplateID: 20 }), "target"); + expect(found!.sourcePageItemTemplateID).toBe(10); + }); +}); + +// ─── getSectionMappingByID ──────────────────────────────────────────────────── + +describe("SectionMapper.getSectionMappingByID", () => { + it("returns null for unknown ID", () => { + const mapper = makeMapper(); + expect(mapper.getSectionMappingByID(999, "source")).toBeNull(); + }); + + it("returns mapping by source ID", () => { + const mapper = makeMapper(); + mapper.addMapping(makeSection({ pageItemTemplateID: 5 }), makeSection({ pageItemTemplateID: 6 })); + expect(mapper.getSectionMappingByID(5, "source")).not.toBeNull(); + }); + + it("returns mapping by target ID", () => { + const mapper = makeMapper(); + mapper.addMapping(makeSection({ pageItemTemplateID: 5 }), makeSection({ pageItemTemplateID: 6 })); + expect(mapper.getSectionMappingByID(6, "target")).not.toBeNull(); + }); + + // PROD-2350: the whole point of the ID mapping — a rename on either side shouldn't break + // the lookup once the mapping has been established. + it("still resolves after the source section is renamed", () => { + const mapper = makeMapper(); + mapper.addMapping( + makeSection({ pageItemTemplateID: 5, pageItemTemplateReferenceName: "OldName" }), + makeSection({ pageItemTemplateID: 6, pageItemTemplateReferenceName: "Main" }) + ); + const found = mapper.getSectionMappingByID(5, "source"); + expect(found!.targetPageItemTemplateID).toBe(6); + }); +}); + +// ─── addMapping / updateMapping ─────────────────────────────────────────────── + +describe("SectionMapper.addMapping", () => { + it("adds a new mapping", () => { + const mapper = makeMapper(); + mapper.addMapping(makeSection({ pageItemTemplateID: 10 }), makeSection({ pageItemTemplateID: 20 })); + expect(mapper.getSectionMappingByID(20, "target")).not.toBeNull(); + }); + + it("updates existing mapping when target already exists", () => { + const mapper = makeMapper(); + const tgt = makeSection({ pageItemTemplateID: 20 }); + mapper.addMapping(makeSection({ pageItemTemplateID: 10, pageItemTemplateReferenceName: "Old" }), tgt); + mapper.addMapping(makeSection({ pageItemTemplateID: 11, pageItemTemplateReferenceName: "New" }), tgt); + const found = mapper.getSectionMappingByID(20, "target")!; + expect(found.sourcePageItemTemplateID).toBe(11); + expect(found.sourceReferenceName).toBe("New"); + }); + + it("throws when source and target already point at different mappings", () => { + const mapper = makeMapper(); + mapper.addMapping(makeSection({ pageItemTemplateID: 1 }), makeSection({ pageItemTemplateID: 2 })); + mapper.addMapping(makeSection({ pageItemTemplateID: 3 }), makeSection({ pageItemTemplateID: 4 })); + expect(() => + mapper.addMapping(makeSection({ pageItemTemplateID: 1 }), makeSection({ pageItemTemplateID: 4 })) + ).toThrow(/Invalid Mappings detected/); + }); + + it("persists across mapper instances for the same guid pair", () => { + const mapper = makeMapper(); + mapper.addMapping(makeSection({ pageItemTemplateID: 10 }), makeSection({ pageItemTemplateID: 20 })); + + const reloaded = new SectionMapper(currentSrc, currentTgt); + expect(reloaded.getSectionMappingByID(10, "source")).not.toBeNull(); + }); +}); diff --git a/src/lib/mappers/tests/template-mapper.test.ts b/src/lib/mappers/tests/template-mapper.test.ts index c0a631d2..0620e8b3 100644 --- a/src/lib/mappers/tests/template-mapper.test.ts +++ b/src/lib/mappers/tests/template-mapper.test.ts @@ -3,6 +3,7 @@ import * as os from "os"; import * as path from "path"; import { resetState, setState } from "core/state"; import { TemplateMapper } from "lib/mappers/template-mapper"; +import { SectionMapper } from "lib/mappers/section-mapper"; let tmpDir: string; @@ -202,6 +203,13 @@ function makeSection(overrides: Record = {}): any { }; } +let sectionMapperCounter = 0; + +function makeSectionMapper(): SectionMapper { + sectionMapperCounter++; + return new SectionMapper(`sm-src-${sectionMapperCounter}`, `sm-tgt-${sectionMapperCounter}`); +} + describe("TemplateMapper.hasTemplateChanged", () => { it("returns false when either template is null", () => { const mapper = makeMapper(); @@ -271,3 +279,87 @@ describe("TemplateMapper.hasTemplateChanged", () => { expect(mapper.hasTemplateChanged(makeTemplate({ pageTemplateName: "New" }), makeTemplate())).toBe(true); }); }); + +// ─── hasTemplateChanged — with SectionMapper (ID-based matching, PROD-2350) ── + +describe("TemplateMapper.hasTemplateChanged — with SectionMapper", () => { + it("detects a change that pure reference-name matching would miss", () => { + // Two sections trade reference names AND item orders at the same time, so once both + // arrays are sorted by name, they serialize to the exact same JSON — a false "unchanged" + // if matching purely by name. Section id 1 is mapped to target id 10 (currently "main"/1); + // source id 1 has actually changed to "footer"/2, which the ID-based pairing catches + // immediately since it never re-derives identity from the (now swapped) names. + const mapper = makeMapper(); + const sectionMapper = makeSectionMapper(); + sectionMapper.addMapping( + makeSection({ pageItemTemplateID: 1, pageItemTemplateReferenceName: "main", itemOrder: 1 }), + makeSection({ pageItemTemplateID: 10, pageItemTemplateReferenceName: "main", itemOrder: 1 }) + ); + sectionMapper.addMapping( + makeSection({ pageItemTemplateID: 2, pageItemTemplateReferenceName: "footer", itemOrder: 2 }), + makeSection({ pageItemTemplateID: 20, pageItemTemplateReferenceName: "footer", itemOrder: 2 }) + ); + + const source = makeTemplate({ + contentSectionDefinitions: [ + makeSection({ pageItemTemplateID: 1, pageItemTemplateReferenceName: "footer", itemOrder: 2 }), + makeSection({ pageItemTemplateID: 2, pageItemTemplateReferenceName: "main", itemOrder: 1 }), + ], + }); + const target = makeTemplate({ + contentSectionDefinitions: [ + makeSection({ pageItemTemplateID: 10, pageItemTemplateReferenceName: "main", itemOrder: 1 }), + makeSection({ pageItemTemplateID: 20, pageItemTemplateReferenceName: "footer", itemOrder: 2 }), + ], + }); + + // Without the section mapping, this looks unchanged (false negative). + expect(mapper.hasTemplateChanged(source, target)).toBe(false); + // With it, the swap is correctly detected. + expect(mapper.hasTemplateChanged(source, target, sectionMapper)).toBe(true); + }); + + it("returns false when sections are correctly matched by ID and nothing actually changed", () => { + const mapper = makeMapper(); + const sectionMapper = makeSectionMapper(); + sectionMapper.addMapping( + makeSection({ pageItemTemplateID: 1, pageItemTemplateReferenceName: "main" }), + makeSection({ pageItemTemplateID: 10, pageItemTemplateReferenceName: "main" }) + ); + + const source = makeTemplate({ contentSectionDefinitions: [makeSection({ pageItemTemplateID: 1 })] }); + const target = makeTemplate({ contentSectionDefinitions: [makeSection({ pageItemTemplateID: 10 })] }); + + expect(mapper.hasTemplateChanged(source, target, sectionMapper)).toBe(false); + }); + + it("detects a rename via the ID mapping even when the target still has the old name", () => { + const mapper = makeMapper(); + const sectionMapper = makeSectionMapper(); + sectionMapper.addMapping( + makeSection({ pageItemTemplateID: 1, pageItemTemplateReferenceName: "OldName" }), + makeSection({ pageItemTemplateID: 10, pageItemTemplateReferenceName: "OldName" }) + ); + + const source = makeTemplate({ + contentSectionDefinitions: [ + makeSection({ pageItemTemplateID: 1, pageItemTemplateReferenceName: "NewName" }), + ], + }); + const target = makeTemplate({ + contentSectionDefinitions: [makeSection({ pageItemTemplateID: 10, pageItemTemplateReferenceName: "OldName" })], + }); + + expect(mapper.hasTemplateChanged(source, target, sectionMapper)).toBe(true); + }); + + it("falls back to reference-name matching for sections with no recorded ID mapping", () => { + const mapper = makeMapper(); + const sectionMapper = makeSectionMapper(); // empty — no mappings recorded yet + + const source = makeTemplate({ contentSectionDefinitions: [makeSection()] }); + const target = makeTemplate({ contentSectionDefinitions: [makeSection()] }); + + expect(mapper.hasTemplateChanged(source, target, sectionMapper)).toBe(false); + }); +}); diff --git a/src/lib/pushers/page-pusher/process-page.ts b/src/lib/pushers/page-pusher/process-page.ts index 5dcb6f0c..6eae1e82 100644 --- a/src/lib/pushers/page-pusher/process-page.ts +++ b/src/lib/pushers/page-pusher/process-page.ts @@ -3,6 +3,7 @@ import ansiColors from "ansi-colors"; import { PageMapper } from "../../mappers/page-mapper"; import { ContentItemMapper } from "lib/mappers/content-item-mapper"; import { TemplateMapper } from "lib/mappers/template-mapper"; // Internal helper function to process a single page +import { SectionMapper } from "lib/mappers/section-mapper"; import { translateZoneNames } from "./translate-zone-names"; import { findPageInOtherLocale, OtherLocaleMapping } from "./find-page-in-other-locale"; import { Logs } from "core/logs"; @@ -48,6 +49,7 @@ export async function processPage({ try { let targetTemplate: mgmtApi.PageModel | null = null; + let sourceTemplate: mgmtApi.PageModel | null = null; // Only try to find template mapping for non-folder pages if (page.pageType !== "folder" && page.templateName) { // Find the template mapping @@ -70,6 +72,9 @@ export async function processPage({ return { status: "skip" }; } targetTemplate = templateMapper.getMappedEntity(templateRef, "target") as mgmtApi.PageModel; + // PROD-2350: needed alongside targetTemplate so translateZoneNames can resolve each zone's + // current pageItemTemplateID on the source side before translating it through SectionMapper. + sourceTemplate = templateMapper.getMappedEntity(templateRef, "source") as mgmtApi.PageModel; } //get the existing page from the target instance @@ -172,7 +177,10 @@ export async function processPage({ let sourceZones = page.zones ? { ...page.zones } : {}; // Clone zones or use empty object // CRITICAL: Translate zone names to match template expectations BEFORE content mapping - let mappedZones = translateZoneNames(sourceZones, targetTemplate) as { [key: string]: PageModuleExtended[] }; + const sectionMapper = new SectionMapper(sourceGuid, targetGuid); + let mappedZones = translateZoneNames(sourceZones, targetTemplate, { sourceTemplate, sectionMapper }) as { + [key: string]: PageModuleExtended[]; + }; // Content mapping validation - collect all content IDs that need mapping const contentIdsToValidate: number[] = []; diff --git a/src/lib/pushers/page-pusher/tests/translate-zone-names.test.ts b/src/lib/pushers/page-pusher/tests/translate-zone-names.test.ts index 457d0864..8eb10915 100644 --- a/src/lib/pushers/page-pusher/tests/translate-zone-names.test.ts +++ b/src/lib/pushers/page-pusher/tests/translate-zone-names.test.ts @@ -201,3 +201,78 @@ describe("translateZoneNames — edge cases", () => { expect(() => translateZoneNames(original, makeTemplate(["NewZone"]))).not.toThrow(); }); }); + +// ─── PROD-2350: matching by name/ID instead of array position ───────────────── + +describe("translateZoneNames — PROD-2350 regression: reordered sections with matching names", () => { + it("keeps each zone's content with its own name even when target section order differs", () => { + // Source template sections in order [Main, Sidebar]; target in order [Sidebar, Main]. + const zones = { Main: [module1], Sidebar: [module2] }; + const template: any = { + contentSectionDefinitions: [ + { pageItemTemplateReferenceName: "Sidebar", itemOrder: 0 }, + { pageItemTemplateReferenceName: "Main", itemOrder: 1 }, + ], + }; + const result = translateZoneNames(zones, template); + expect(result["Main"]).toEqual([module1]); + expect(result["Sidebar"]).toEqual([module2]); + }); + + it("matches by name even when only one of several zones is reordered", () => { + const zones = { A: [module1], B: [module2], C: [module3] }; + const template = makeTemplate(["C", "A", "B"]); // reordered vs. source insertion order + const result = translateZoneNames(zones, template); + expect(result["A"]).toEqual([module1]); + expect(result["B"]).toEqual([module2]); + expect(result["C"]).toEqual([module3]); + }); +}); + +// ─── ID-based resolution via SectionMapper ───────────────────────────────────── + +describe("translateZoneNames — ID-based resolution via SectionMapper", () => { + it("resolves a renamed zone through the persisted section ID mapping", () => { + // Page was pulled while the source section was still called "OldName". The source + // template has since been renamed to "NewName" (same pageItemTemplateID: 5), and the + // target section was already mapped to pageItemTemplateID 50 under its own current name. + const zones = { OldName: [module1] }; + const sourceTemplate: any = { + contentSectionDefinitions: [{ pageItemTemplateID: 5, pageItemTemplateReferenceName: "OldName" }], + }; + const targetTemplate: any = { + contentSectionDefinitions: [{ pageItemTemplateID: 50, pageItemTemplateReferenceName: "TargetCurrentName" }], + }; + const sectionMapper: any = { + getSectionMappingByID: jest.fn().mockImplementation((id: number, type: string) => + type === "source" && id === 5 ? { sourcePageItemTemplateID: 5, targetPageItemTemplateID: 50 } : null + ), + }; + + const result = translateZoneNames(zones, targetTemplate, { sourceTemplate, sectionMapper }); + expect(result["TargetCurrentName"]).toEqual([module1]); + expect(result).not.toHaveProperty("OldName"); + }); + + it("falls back to name/positional matching when no section mapping is recorded yet", () => { + const zones = { Main: [module1] }; + const sourceTemplate: any = { + contentSectionDefinitions: [{ pageItemTemplateID: 5, pageItemTemplateReferenceName: "Main" }], + }; + const targetTemplate = makeTemplate(["Main"]); + const sectionMapper: any = { getSectionMappingByID: jest.fn().mockReturnValue(null) }; + + const result = translateZoneNames(zones, targetTemplate, { sourceTemplate, sectionMapper }); + expect(result["Main"]).toEqual([module1]); + }); + + it("falls back to name/positional matching when sourceTemplate is not provided", () => { + const zones = { Main: [module1] }; + const targetTemplate = makeTemplate(["Main"]); + const sectionMapper: any = { getSectionMappingByID: jest.fn() }; + + const result = translateZoneNames(zones, targetTemplate, { sectionMapper }); + expect(result["Main"]).toEqual([module1]); + expect(sectionMapper.getSectionMappingByID).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/pushers/page-pusher/translate-zone-names.ts b/src/lib/pushers/page-pusher/translate-zone-names.ts index bf9e3aa2..c086c4c1 100644 --- a/src/lib/pushers/page-pusher/translate-zone-names.ts +++ b/src/lib/pushers/page-pusher/translate-zone-names.ts @@ -1,31 +1,82 @@ import * as mgmtApi from "@agility/management-sdk"; +import { SectionMapper } from "../../mappers/section-mapper"; -export function translateZoneNames(sourceZones: any, targetTemplate: mgmtApi.PageModel | null): any { +interface TranslateZoneNamesOptions { + sourceTemplate?: mgmtApi.PageModel | null; + sectionMapper?: SectionMapper | null; +} + +export function translateZoneNames( + sourceZones: any, + targetTemplate: mgmtApi.PageModel | null, + options: TranslateZoneNamesOptions = {} +): any { if (!sourceZones || !targetTemplate?.contentSectionDefinitions) { return sourceZones || {}; // No template or sections, return as-is } + const { sourceTemplate, sectionMapper } = options; + + const targetSections = (targetTemplate.contentSectionDefinitions || []) + .slice() + .sort((a, b) => (a.itemOrder || 0) - (b.itemOrder || 0)); // Sort by item order + const targetNames = targetSections.map((def) => def.pageItemTemplateReferenceName); + const translatedZones: any = {}; - const sectionNames = targetTemplate.contentSectionDefinitions - .sort((a, b) => (a.itemOrder || 0) - (b.itemOrder || 0)) // Sort by item order - .map((def) => def.pageItemTemplateReferenceName); + const matchedTargetNames = new Set(); + const remainingSourceEntries: [string, any][] = []; + + // Pass 1: resolve each source zone against a target section, preferring the persisted + // source->target pageItemTemplateID mapping (PROD-2350) since it's stable even when a + // section has been renamed or reordered on either side. Falls back to a direct name match + // when no ID mapping is available yet (first push after upgrading, or a genuinely new zone). + for (const [sourceZoneName, zoneContent] of Object.entries(sourceZones)) { + let targetZoneName: string | null = null; + + if (sourceTemplate && sectionMapper) { + const sourceSection = sourceTemplate.contentSectionDefinitions?.find( + (s) => s?.pageItemTemplateReferenceName === sourceZoneName + ); + if (sourceSection?.pageItemTemplateID != null) { + const sectionMapping = sectionMapper.getSectionMappingByID(sourceSection.pageItemTemplateID, "source"); + const targetSection = sectionMapping + ? targetSections.find((t) => t?.pageItemTemplateID === sectionMapping.targetPageItemTemplateID) + : null; + if (targetSection?.pageItemTemplateReferenceName) { + targetZoneName = targetSection.pageItemTemplateReferenceName; + } + } + } + + if (!targetZoneName && targetNames.indexOf(sourceZoneName) !== -1) { + targetZoneName = sourceZoneName; + } + + if (targetZoneName && !matchedTargetNames.has(targetZoneName)) { + translatedZones[targetZoneName] = zoneContent; + matchedTargetNames.add(targetZoneName); + } else { + remainingSourceEntries.push([sourceZoneName, zoneContent]); + } + } - // Map source zones to template section names in order - const sourceZoneEntries = Object.entries(sourceZones); + // Pass 2: positional fallback for whatever's left over — legacy behavior for zones that + // can't be resolved by ID or name (e.g. a section renamed on both sides between pulls). + const remainingTargetNames = targetNames.filter((name) => !matchedTargetNames.has(name)); - for (let i = 0; i < sourceZoneEntries.length && i < sectionNames.length; i++) { - const [sourceZoneName, zoneContent] = sourceZoneEntries[i]; - const targetZoneName = sectionNames[i]; - translatedZones[targetZoneName] = zoneContent; + for (let i = 0; i < remainingSourceEntries.length && i < remainingTargetNames.length; i++) { + const [, zoneContent] = remainingSourceEntries[i]; + translatedZones[remainingTargetNames[i]] = zoneContent; } - // CRITICAL FIX: Instead of dropping extra zones, combine them into the main zone - if (sourceZoneEntries.length > sectionNames.length && sectionNames.length > 0) { - const mainZoneName = sectionNames[0]; // Use first (main) zone as target + // Overflow: more leftover source zones than leftover target sections — combine the extras + // into the main (first, by itemOrder) target zone instead of dropping them. + if (remainingSourceEntries.length > remainingTargetNames.length && targetNames.length > 0) { + const mainZoneName = targetNames[0]; const mainZoneModules = Array.isArray(translatedZones[mainZoneName]) ? [...translatedZones[mainZoneName]] : []; - for (let i = sectionNames.length; i < sourceZoneEntries.length; i++) { - const [sourceZoneName, zoneContent] = sourceZoneEntries[i]; + for (let i = remainingTargetNames.length; i < remainingSourceEntries.length; i++) { + const [, zoneContent] = remainingSourceEntries[i]; if (Array.isArray(zoneContent) && zoneContent.length > 0) { mainZoneModules.push(...zoneContent); } diff --git a/src/lib/pushers/template-pusher.ts b/src/lib/pushers/template-pusher.ts index d5d9b984..88b408e4 100644 --- a/src/lib/pushers/template-pusher.ts +++ b/src/lib/pushers/template-pusher.ts @@ -3,6 +3,7 @@ import { state, getLoggerForGuid } from "../../core/state"; import { TemplateMapper } from "lib/mappers/template-mapper"; import { ModelMapper } from "lib/mappers/model-mapper"; import { ContainerMapper } from "lib/mappers/container-mapper"; +import { SectionMapper } from "lib/mappers/section-mapper"; import { FailureDetail, PusherResult } from "types/sourceData"; import { preflightReport } from "../preflight/preflight-report"; @@ -32,9 +33,10 @@ export async function pushTemplates( let sourceTemplate = sourceTemplates[i]; const { sourceGuid, targetGuid } = state; - const referenceMapper = new TemplateMapper(sourceGuid[0], targetGuid[0]); + const templateMapper = new TemplateMapper(sourceGuid[0], targetGuid[0]); + const sectionMapper = new SectionMapper(sourceGuid[0], targetGuid[0]); - let existingMapping = referenceMapper.getTemplateMapping(sourceTemplate, "source"); + let existingMapping = templateMapper.getTemplateMapping(sourceTemplate, "source"); let targetTemplate: mgmtApi.PageModel | null = null; // If we have a mapping, try to get the target template via the target template id from the mapping @@ -64,12 +66,48 @@ export async function pushTemplates( // Templates have no lastModifiedDate, so compare the source and target // structure directly: identical -> skip, different -> update (source wins), // mapped but missing on target -> fall through and recreate. - const templateChanged = referenceMapper.hasTemplateChanged(sourceTemplate, targetTemplate); + const templateChanged = templateMapper.hasTemplateChanged(sourceTemplate, targetTemplate, sectionMapper); const shouldUpdate = existingMapping !== null && targetTemplate !== null && templateChanged; const shouldSkip = existingMapping !== null && targetTemplate !== null && !templateChanged; if (shouldSkip) { + // Backfill the section ID mapping for templates that are already in sync but predate + // SectionMapper (PROD-2350) — otherwise a template that never changes again would never + // get its sections seeded, leaving translateZoneNames on the name-match fallback forever. + // "Up to date" means hasTemplateChanged already found the source and target structurally + // identical, so source/target section counts and names should always line up here; if + // they don't, something is inconsistent and we hard-stop rather than guess. + const sourceSections = sourceTemplate.contentSectionDefinitions || []; + const targetSections = targetTemplate?.contentSectionDefinitions || []; + + if (sourceSections.length !== targetSections.length) { + throw new Error( + `Page template validation failed: template "${sourceTemplate.pageTemplateName}" (ID: ${sourceTemplate.pageTemplateID}) is marked up to date, but has ${sourceSections.length} section(s) on the source and ${targetSections.length} on the target. ` + + `This indicates a mapping inconsistency; review the template mappings and re-run. Please contact AgilityCMS Support to resolve this issue` + ); + } + + for (const sourceSection of sourceSections) { + const targetSection = targetSections.find( + (t) => t.pageItemTemplateReferenceName === sourceSection.pageItemTemplateReferenceName + ); + + if (!targetSection) { + throw new Error( + `Page template validation failed: template "${sourceTemplate.pageTemplateName}" (ID: ${sourceTemplate.pageTemplateID}) is marked up to date, but source section "${sourceSection.pageItemTemplateReferenceName}" has no matching section on the target. ` + + `This indicates a mapping inconsistency; review the template mappings and re-run. Please contact AgilityCMS Support to resolve this issue` + ); + } + + if (sourceSection.pageItemTemplateID == null || targetSection.pageItemTemplateID == null) continue; + + const existingSectionMapping = sectionMapper.getSectionMappingByID(sourceSection.pageItemTemplateID, "source"); + if (!existingSectionMapping) { + sectionMapper.addMapping(sourceSection, targetSection); + } + } + logger.template.skipped(sourceTemplate, "Up to date, skipping", targetGuid[0]); preflightReport.record({ phase: "Templates", @@ -91,16 +129,38 @@ export async function pushTemplates( else { let targetId = shouldUpdate ? targetTemplate?.pageTemplateID : -1; + // Prepare payload const mappedSections = sourceTemplate.contentSectionDefinitions.map((sourceContentSecDef) => { const mappedDef = { ...sourceContentSecDef }; - // Find the target section data to use for updates - const targetSection = shouldUpdate ? targetTemplate?.contentSectionDefinitions?.find((targetContentSecDef) => targetContentSecDef.pageItemTemplateReferenceName === sourceContentSecDef.pageItemTemplateReferenceName) : null; + // Find the target section data to use for updates. Prefer the persisted ID mapping — + // PROD-2350: matching by reference name breaks whenever a section has been renamed on + // either side. Fall back to a name match only when no mapping exists yet (first push + // after upgrading, or a section that's genuinely new). + let targetSection: mgmtApi.ContentSectionDefinition | null = null; + if (shouldUpdate) { + const sectionMapping = sourceContentSecDef.pageItemTemplateID + ? sectionMapper.getSectionMappingByID(sourceContentSecDef.pageItemTemplateID, "source") + : null; + + targetSection = sectionMapping + ? targetTemplate?.contentSectionDefinitions?.find( + (t) => t.pageItemTemplateID === sectionMapping.targetPageItemTemplateID + ) ?? null + : null; + + if (!targetSection) { + targetSection = + targetTemplate?.contentSectionDefinitions?.find( + (t) => t.pageItemTemplateReferenceName === sourceContentSecDef.pageItemTemplateReferenceName + ) ?? null; + } + } - mappedDef.pageItemTemplateID = shouldUpdate ? targetSection?.pageItemTemplateID ?? -1 : -1; + mappedDef.pageItemTemplateID = targetSection?.pageItemTemplateID ?? -1; mappedDef.pageTemplateID = targetId; - mappedDef.contentViewID = shouldUpdate ? targetSection?.contentViewID ?? -1 : -1; + mappedDef.contentViewID = targetSection?.contentViewID ?? -1; // should have the models by now if (sourceContentSecDef.contentDefinitionID) { @@ -127,7 +187,20 @@ export async function pushTemplates( try { const savedTemplate = await apiClient.pageMethods.savePageTemplate(targetGuid[0], locale, payload); - referenceMapper.addMapping(sourceTemplate, savedTemplate); + templateMapper.addMapping(sourceTemplate, savedTemplate); + + // Refresh section-level ID mappings from the confirmed response — this both seeds the + // mapping the first time a section is seen and keeps it correct going forward, regardless + // of any later renames on either side. + for (const sourceSection of sourceTemplate.contentSectionDefinitions || []) { + const savedSection = savedTemplate.contentSectionDefinitions?.find( + (s) => s.pageItemTemplateReferenceName === sourceSection.pageItemTemplateReferenceName + ); + if (savedSection?.pageItemTemplateID != null && sourceSection.pageItemTemplateID != null) { + sectionMapper.addMapping(sourceSection, savedSection); + } + } + const action = shouldUpdate ? "updated" : "created"; logger.template[action](sourceTemplate, action, targetGuid[0]); successful++; diff --git a/src/lib/pushers/tests/template-pusher.test.ts b/src/lib/pushers/tests/template-pusher.test.ts index 1d1c3e20..f6af4bb2 100644 --- a/src/lib/pushers/tests/template-pusher.test.ts +++ b/src/lib/pushers/tests/template-pusher.test.ts @@ -2,6 +2,8 @@ import * as fs from "fs"; import * as os from "os"; import * as path from "path"; import { resetState, setState, state, initializeGuidLogger } from "core/state"; +import { TemplateMapper } from "lib/mappers/template-mapper"; +import { SectionMapper } from "lib/mappers/section-mapper"; let tmpDir: string; @@ -185,3 +187,271 @@ describe("pushTemplates — overwrite mode", () => { expect(result.successful).toBe(1); }); }); + +// ─── pushTemplates — PROD-2350: section ID mapping ─────────────────────────── + +describe("pushTemplates — section ID mapping", () => { + it("seeds a section mapping from the confirmed API response on create", async () => { + const sourceTpl = makeTemplate({ + pageTemplateID: 5001, + pageTemplateName: "SectionSeedTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 7, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: -1 }, + ], + }); + + const savedTpl = makeTemplate({ + pageTemplateID: 5002, + pageTemplateName: "SectionSeedTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 70, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: 999 }, + ], + }); + const savePageTemplate = jest.fn().mockResolvedValue(savedTpl); + state.cachedApiClient = { pageMethods: { savePageTemplate } } as any; + + const { pushTemplates } = await import("../template-pusher"); + const result = await pushTemplates([sourceTpl], [], "en-us"); + + expect(result.successful).toBe(1); + + const sectionMapper = new SectionMapper("src-tpl-u", "tgt-tpl-u"); + const mapping = sectionMapper.getSectionMappingByID(7, "source"); + expect(mapping).not.toBeNull(); + expect(mapping!.targetPageItemTemplateID).toBe(70); + }); + + it("falls back to reference-name matching when no section mapping exists yet (bootstrap)", async () => { + const sourceTpl = makeTemplate({ + pageTemplateID: 5003, + pageTemplateName: "BootstrapTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 8, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: -1 }, + // Extra section vs. target so hasTemplateChanged detects a real structural diff and + // takes the update path instead of skipping (source and target are otherwise identical). + { pageItemTemplateID: 9, pageItemTemplateReferenceName: "Extra", itemOrder: 1, contentViewID: -1 }, + ], + }); + const targetTpl = makeTemplate({ + pageTemplateID: 5004, + pageTemplateName: "BootstrapTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 80, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: 444 }, + ], + }); + + // Establish the template-level mapping (source<->target) but no section mapping yet — + // simulates a sync that predates the SectionMapper being introduced. + new TemplateMapper("src-tpl-u", "tgt-tpl-u").addMapping(sourceTpl, targetTpl); + + const savedTpl = { ...targetTpl }; + const savePageTemplate = jest.fn().mockResolvedValue(savedTpl); + state.cachedApiClient = { pageMethods: { savePageTemplate } } as any; + + const { pushTemplates } = await import("../template-pusher"); + await pushTemplates([sourceTpl], [targetTpl], "en-us"); + + const payload = savePageTemplate.mock.calls[0][2]; + const mainSection = payload.contentSectionDefinitions.find( + (s: any) => s.pageItemTemplateReferenceName === "Main" + ); + // Bootstrap fallback (name match) preserved the target's existing section identity — + // it did NOT send -1, which would have caused the API to create a duplicate section. + expect(mainSection.pageItemTemplateID).toBe(80); + expect(mainSection.contentViewID).toBe(444); + }); + + it("uses the persisted section mapping instead of name matching after a source rename", async () => { + const sourceTplBefore = makeTemplate({ + pageTemplateID: 5005, + pageTemplateName: "RenameTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 9, pageItemTemplateReferenceName: "OldName", itemOrder: 0, contentViewID: -1 }, + ], + }); + const targetTplBefore = makeTemplate({ + pageTemplateID: 5006, + pageTemplateName: "RenameTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 90, pageItemTemplateReferenceName: "OldName", itemOrder: 0, contentViewID: 777 }, + ], + }); + + // Establish both the template-level and section-level mappings as they'd exist after an + // earlier successful push (before the rename happened). + new TemplateMapper("src-tpl-u", "tgt-tpl-u").addMapping(sourceTplBefore, targetTplBefore); + new SectionMapper("src-tpl-u", "tgt-tpl-u").addMapping( + sourceTplBefore.contentSectionDefinitions[0], + targetTplBefore.contentSectionDefinitions[0] + ); + + // Source renames the section (same pageItemTemplateID: 9); target hasn't been pushed yet, + // so the live target template still reports the old name. + const sourceTplRenamed = { + ...sourceTplBefore, + contentSectionDefinitions: [ + { pageItemTemplateID: 9, pageItemTemplateReferenceName: "NewName", itemOrder: 0, contentViewID: -1 }, + ], + }; + const targetTplCurrent = targetTplBefore; + + const savedTpl = { + ...targetTplCurrent, + contentSectionDefinitions: [ + { pageItemTemplateID: 90, pageItemTemplateReferenceName: "NewName", itemOrder: 0, contentViewID: 777 }, + ], + }; + const savePageTemplate = jest.fn().mockResolvedValue(savedTpl); + state.cachedApiClient = { pageMethods: { savePageTemplate } } as any; + + const { pushTemplates } = await import("../template-pusher"); + await pushTemplates([sourceTplRenamed], [targetTplCurrent], "en-us"); + + const payload = savePageTemplate.mock.calls[0][2]; + // A name match against the stale target ("OldName") would have missed and sent -1, + // creating a duplicate section instead of updating the existing one. + expect(payload.contentSectionDefinitions[0].pageItemTemplateID).toBe(90); + expect(payload.contentSectionDefinitions[0].contentViewID).toBe(777); + + // The mapping is refreshed with the new name so future lookups stay correct. + const refreshed = new SectionMapper("src-tpl-u", "tgt-tpl-u").getSectionMappingByID(9, "source"); + expect(refreshed!.targetPageItemTemplateID).toBe(90); + expect(refreshed!.sourceReferenceName).toBe("NewName"); + }); +}); + +// ─── pushTemplates — section ID mapping backfill on skip ───────────────────── + +describe("pushTemplates — section ID mapping backfill when skipping up-to-date templates", () => { + it("backfills a section mapping on skip when none exists yet", async () => { + const sourceTpl = makeTemplate({ + pageTemplateID: 5007, + pageTemplateName: "SkipBackfillTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 11, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: -1 }, + ], + }); + const targetTpl = makeTemplate({ + pageTemplateID: 5008, + pageTemplateName: "SkipBackfillTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 110, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: 555 }, + ], + }); + + // Structurally identical -> hasTemplateChanged is false -> shouldSkip path, no section + // mapping pre-populated (simulates a sync predating SectionMapper). + new TemplateMapper("src-tpl-u", "tgt-tpl-u").addMapping(sourceTpl, targetTpl); + + const savePageTemplate = jest.fn(); + state.cachedApiClient = { pageMethods: { savePageTemplate } } as any; + + const { pushTemplates } = await import("../template-pusher"); + const result = await pushTemplates([sourceTpl], [targetTpl], "en-us"); + + expect(result.skipped).toBe(1); + expect(savePageTemplate).not.toHaveBeenCalled(); + + const mapping = new SectionMapper("src-tpl-u", "tgt-tpl-u").getSectionMappingByID(11, "source"); + expect(mapping).not.toBeNull(); + expect(mapping!.targetPageItemTemplateID).toBe(110); + }); + + it("does not overwrite an existing section mapping on skip", async () => { + const sourceTpl = makeTemplate({ + pageTemplateID: 5009, + pageTemplateName: "SkipNoOverwriteTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 12, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: -1 }, + ], + }); + const targetTpl = makeTemplate({ + pageTemplateID: 5010, + pageTemplateName: "SkipNoOverwriteTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 120, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: 666 }, + ], + }); + + new TemplateMapper("src-tpl-u", "tgt-tpl-u").addMapping(sourceTpl, targetTpl); + // Pre-seed a stale mapping pointing at a different target ID than the current name match + // would find — proves the skip path only creates a mapping when one is missing, it never + // refreshes an existing one. + new SectionMapper("src-tpl-u", "tgt-tpl-u").addMapping(sourceTpl.contentSectionDefinitions[0], { + pageItemTemplateID: 999, + pageItemTemplateReferenceName: "Main", + }); + + const savePageTemplate = jest.fn(); + state.cachedApiClient = { pageMethods: { savePageTemplate } } as any; + + const { pushTemplates } = await import("../template-pusher"); + await pushTemplates([sourceTpl], [targetTpl], "en-us"); + + const mapping = new SectionMapper("src-tpl-u", "tgt-tpl-u").getSectionMappingByID(12, "source"); + expect(mapping!.targetPageItemTemplateID).toBe(999); + }); + + it("throws when the skip path finds mismatched section counts", async () => { + const sourceTpl = makeTemplate({ + pageTemplateID: 5011, + pageTemplateName: "SkipMismatchTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 13, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: -1 }, + { pageItemTemplateID: 14, pageItemTemplateReferenceName: "Sidebar", itemOrder: 1, contentViewID: -1 }, + ], + }); + const targetTpl = makeTemplate({ + pageTemplateID: 5012, + pageTemplateName: "SkipMismatchTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 130, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: 555 }, + ], + }); + + new TemplateMapper("src-tpl-u", "tgt-tpl-u").addMapping(sourceTpl, targetTpl); + + // Force the "up to date" path even though the section arrays don't actually match, to + // exercise the defensive validation (this shouldn't happen in practice since + // hasTemplateChanged would normally catch this itself). + jest.spyOn(TemplateMapper.prototype, "hasTemplateChanged").mockReturnValue(false); + + const savePageTemplate = jest.fn(); + state.cachedApiClient = { pageMethods: { savePageTemplate } } as any; + + const { pushTemplates } = await import("../template-pusher"); + + await expect(pushTemplates([sourceTpl], [targetTpl], "en-us")).rejects.toThrow( + /marked up to date, but has 2 section\(s\) on the source and 1 on the target/ + ); + }); + + it("throws when the skip path finds a source section with no matching target name", async () => { + const sourceTpl = makeTemplate({ + pageTemplateID: 5013, + pageTemplateName: "SkipNameMismatchTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 15, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: -1 }, + ], + }); + const targetTpl = makeTemplate({ + pageTemplateID: 5014, + pageTemplateName: "SkipNameMismatchTemplate", + contentSectionDefinitions: [ + { pageItemTemplateID: 150, pageItemTemplateReferenceName: "SomethingElse", itemOrder: 0, contentViewID: 555 }, + ], + }); + + new TemplateMapper("src-tpl-u", "tgt-tpl-u").addMapping(sourceTpl, targetTpl); + jest.spyOn(TemplateMapper.prototype, "hasTemplateChanged").mockReturnValue(false); + + const savePageTemplate = jest.fn(); + state.cachedApiClient = { pageMethods: { savePageTemplate } } as any; + + const { pushTemplates } = await import("../template-pusher"); + + await expect(pushTemplates([sourceTpl], [targetTpl], "en-us")).rejects.toThrow( + /source section "Main" has no matching section on the target/ + ); + }); +});