From 382e6ebbd93f3b478c2782efe70e5e133ea8a2f6 Mon Sep 17 00:00:00 2001 From: Kevin Date: Thu, 6 Aug 2026 13:49:27 -0400 Subject: [PATCH] PROD-2316: drop unresolvable page modules instead of failing the whole page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A page referencing content the CLI couldn't resolve (not pulled, failed earlier, other-locale-only, deleted) previously hard-failed in process-page.ts even though the per-module mapping loop had already excluded the unresolvable module from the rebuilt zones. Now the page pushes with those modules dropped, and each drop is reported as a non-blocking "Page Module Warnings" summary section (PROD-2311's mapping-warnings pattern) that never affects failure counts or the exit code. Server-side SavePage (BatchInsertPageItem's toBeDeletedList diff) removes payload-absent modules from an existing target page, so a dropped module is genuinely removed on target — matching the decision that source-side unpublished/removed modules must propagate downstream. Two safeguards: - Self-healing: when a push drops modules, the page mapping is recorded with sourceVersionID 0 ("never cleanly synced"), so the page re-pushes on every sync until a push completes with no drops — restoring modules automatically once their content becomes resolvable. targetVersionID is still recorded accurately, so this never surfaces as a conflict. - Total-loss guard now also covers the UPDATE path (was create-only): if EVERY module fails to resolve, the page fails instead of wiping all modules off the existing target page. Genuine source-side emptiness is unaffected (originalModuleCount is 0). All drops are collected (previously only the first missing mapping was tracked), threaded process-page → process-sitemap → push-pages → orchestrate-pushers → core/push as warningDetails. Co-Authored-By: Claude Fable 5 --- src/core/push.ts | 24 +++ src/lib/pushers/orchestrate-pushers.ts | 25 ++- src/lib/pushers/page-pusher/process-page.ts | 139 +++++++------- .../pushers/page-pusher/process-sitemap.ts | 11 +- src/lib/pushers/page-pusher/push-pages.ts | 11 +- .../page-pusher/tests/process-page.test.ts | 173 +++++++++++++++++- src/types/sourceData.ts | 5 + 7 files changed, 311 insertions(+), 77 deletions(-) diff --git a/src/core/push.ts b/src/core/push.ts index 96d41c21..6feb6578 100644 --- a/src/core/push.ts +++ b/src/core/push.ts @@ -119,6 +119,9 @@ export class Push { guid?: string; locale?: string; }> = []; + // PROD-2316: non-blocking notices (e.g. page modules dropped because their content + // couldn't be resolved). Shown in their own section; never affect success/exit code. + const syncWarningDetails: typeof syncFailureDetails = []; results.forEach((result: PushResults) => { // Track item-level failures from totalFailures @@ -129,6 +132,9 @@ export class Push { if (result.failureDetails && result.failureDetails.length > 0) { syncFailureDetails.push(...result.failureDetails); } + if (result.warningDetails && result.warningDetails.length > 0) { + syncWarningDetails.push(...result.warningDetails); + } // Track operation-level failures if (result.failed && result.failed.length > 0) { result.failed.forEach((f) => { @@ -230,6 +236,24 @@ export class Push { } } + // PROD-2316: dropped-module notices — a page pushed successfully, but one or more of + // its modules referenced content that couldn't be resolved on the target, so those + // modules were left off the pushed page. Non-blocking: the modules return automatically + // on a future push of the page once the content syncs. + if (syncWarningDetails.length > 0) { + console.log(ansiColors.yellow(`\n Page Module Warnings (non-blocking, ${syncWarningDetails.length}):`)); + syncWarningDetails.forEach(({ name, error, pageID, contentID, guid, locale }) => { + const prefix = guid && locale ? `[${guid}][${locale}]` : guid ? `[${guid}]` : ""; + console.log(ansiColors.yellow(` ${prefix} • ${name}: ${error}`)); + if (pageID && guid && locale) { + console.log(ansiColors.gray(` ${getPageCmsLink(guid, locale, pageID)}`)); + } + if (contentID && guid && locale) { + console.log(ansiColors.gray(` ${getContentCmsLink(guid, locale, contentID)}`)); + } + }); + } + // PROD-2311: post-publish mapping/version bookkeeping notices are NOT publish // failures — show them under a non-blocking header so they don't read as broken. if (mappingWarnings.length > 0) { diff --git a/src/lib/pushers/orchestrate-pushers.ts b/src/lib/pushers/orchestrate-pushers.ts index a6a03ca3..e76d8508 100644 --- a/src/lib/pushers/orchestrate-pushers.ts +++ b/src/lib/pushers/orchestrate-pushers.ts @@ -24,6 +24,8 @@ export interface PushResults { publishablePageIdsByLocale: Map; // Individual failure details for error summary failureDetails: FailureDetail[]; + // PROD-2316: non-blocking notices (e.g. page modules dropped due to unresolvable content) + warningDetails: FailureDetail[]; } export interface PusherConfig { @@ -66,6 +68,7 @@ export class Pushers { publishableContentIdsByLocale: new Map(), publishablePageIdsByLocale: new Map(), failureDetails: [], + warningDetails: [], }; try { @@ -84,6 +87,7 @@ export class Pushers { results.publishableContentIdsByLocale = pushResults.publishableContentIdsByLocale; results.publishablePageIdsByLocale = pushResults.publishablePageIdsByLocale; results.failureDetails = pushResults.failureDetails; + results.warningDetails = pushResults.warningDetails; // Calculate final duration results.totalDuration = Date.now() - startTime; @@ -155,6 +159,7 @@ export class Pushers { publishableContentIdsByLocale: Map; publishablePageIdsByLocale: Map; failureDetails: FailureDetail[]; + warningDetails: FailureDetail[]; }> { const { locale: locales, elements: stateElements } = state; const elements = stateElements.split(","); @@ -170,6 +175,8 @@ export class Pushers { const publishablePageIdsByLocale = new Map(); // Collect individual failure details const failureDetails: FailureDetail[] = []; + // PROD-2316: collect non-blocking warning details + const warningDetails: FailureDetail[] = []; // PROD-2202: Models run FIRST so the model-mapping validation (the rename/reassignment // mismatch detection in pushModels) fails the sync before any galleries or assets are @@ -240,6 +247,9 @@ export class Pushers { if (result.failureDetails) { failureDetails.push(...result.failureDetails); } + if (result.warningDetails) { + warningDetails.push(...result.warningDetails); + } } } catch (error: any) { // Re-throw validation errors immediately to stop sync. @@ -284,6 +294,9 @@ export class Pushers { if (result.failureDetails) { failureDetails.push(...result.failureDetails); } + if (result.warningDetails) { + warningDetails.push(...result.warningDetails); + } // Store per-locale IDs and also add to combined list if (config === PUSH_OPERATIONS.content && localeContentIds.length > 0) { @@ -308,6 +321,7 @@ export class Pushers { publishableContentIdsByLocale, publishablePageIdsByLocale, failureDetails, + warningDetails, }; } catch (error) { console.error(ansiColors.red("Error during pusher execution:"), error); @@ -331,7 +345,13 @@ export class Pushers { publishableContentIds?: number[]; publishablePageIds?: number[]; elements: string[]; - }): Promise<{ success: number; failures: number; skipped: number; failureDetails?: FailureDetail[] }> { + }): Promise<{ + success: number; + failures: number; + skipped: number; + failureDetails?: FailureDetail[]; + warningDetails?: FailureDetail[]; + }> { const elementData = sourceData[config.dataKey as keyof GuidEntities] || []; // Skip if no data for this element type or element not requested @@ -342,7 +362,7 @@ export class Pushers { console.log( ansiColors.yellow(`⚠️ Skipping ${config.description} for locale ${locale} - no data or filtered by --locales`) ); - return { success: 0, failures: 0, skipped: 0, failureDetails: [] }; + return { success: 0, failures: 0, skipped: 0, failureDetails: [], warningDetails: [] }; } this.config.onOperationStart?.(config.name, state.sourceGuid, state.targetGuid); @@ -383,6 +403,7 @@ export class Pushers { failures: pusherResult.failed || 0, skipped: pusherResult.skipped || 0, failureDetails: pusherResult.failureDetails || [], + warningDetails: pusherResult.warningDetails || [], }; } diff --git a/src/lib/pushers/page-pusher/process-page.ts b/src/lib/pushers/page-pusher/process-page.ts index 6eae1e82..0f64d9dc 100644 --- a/src/lib/pushers/page-pusher/process-page.ts +++ b/src/lib/pushers/page-pusher/process-page.ts @@ -8,7 +8,7 @@ import { translateZoneNames } from "./translate-zone-names"; import { findPageInOtherLocale, OtherLocaleMapping } from "./find-page-in-other-locale"; import { Logs } from "core/logs"; import { state, getFailedContent, contentExistsInSourceData, contentExistsInOtherLocale } from "core/state"; -import { PageModuleExtended } from "types/sourceData"; +import { FailureDetail, PageModuleExtended } from "types/sourceData"; import { preflightReport } from "../../preflight/preflight-report"; interface Props { @@ -25,7 +25,14 @@ interface Props { logger: Logs; } -export type PageProcessResult = { status: "success" | "skip" | "failure"; error?: string; contentID?: number }; +export type PageProcessResult = { + status: "success" | "skip" | "failure"; + error?: string; + contentID?: number; + // PROD-2316: non-blocking notices for modules dropped from this page because their content + // couldn't be resolved on the target. The page itself still pushes. + warnings?: FailureDetail[]; +}; export async function processPage({ channel, @@ -182,27 +189,11 @@ export async function processPage({ [key: string]: PageModuleExtended[]; }; - // Content mapping validation - collect all content IDs that need mapping - const contentIdsToValidate: number[] = []; - for (const [zoneName, zoneModules] of Object.entries(mappedZones)) { - if (Array.isArray(zoneModules)) { - for (const module of zoneModules) { - if (module.item && typeof module.item === "object") { - const sourceContentId = module.item.contentid || module.item.contentId; - if (sourceContentId && sourceContentId > 0) { - contentIdsToValidate.push(sourceContentId); - } - } - } - } - } - - // Content mapping validation (silent unless errors) - const contentMapper = new ContentItemMapper(sourceGuid, targetGuid, locale); - // Track first missing content mapping for error summary - let firstMissingContentError: string | null = null; - let firstMissingContentID: number | null = null; + // PROD-2316: modules whose content can't be resolved are DROPPED from the pushed page rather + // than failing the whole page. Collect every drop (not just the first) as a non-blocking + // warning so the run summary can report exactly what was left out and why. + const droppedModules: FailureDetail[] = []; for (const [zoneName, zoneModules] of Object.entries(mappedZones)) { const newZoneContent = []; @@ -258,12 +249,29 @@ export async function processPage({ mappingError = `No content mapping for ${module.module} (contentID ${sourceContentId}) - content has never been synced or model may have changed`; } - // Don't log individual errors inline - they'll appear in the final summary - // Capture first error and contentID for summary - if (!firstMissingContentError) { - firstMissingContentError = mappingError; - firstMissingContentID = sourceContentId; - } + // PROD-2316: drop this module from the pushed page (it is NOT added to + // newZoneContent) and record a non-blocking warning. The rest of the page still + // pushes. NOTE: the server's SavePage diff (BatchInsertPageItem's toBeDeletedList) + // REMOVES payload-absent modules from an existing target page, so a dropped module + // is genuinely removed on the target — the mapping below is therefore recorded as + // dirty (sourceVersionID 0) so the page re-pushes each sync and the module is + // restored automatically once its content becomes resolvable. + droppedModules.push({ + name: page.name || `Page ${page.pageID}`, + error: `Dropped module ${module.module} — ${mappingError}`, + type: "page", + pageID: page.pageID, + contentID: sourceContentId, + guid: sourceGuid, + locale, + }); + logger.page.skipped( + page, + `dropped module ${module.module} (contentID ${sourceContentId}) — ${mappingError}`, + locale, + channel, + targetGuid + ); } } else { // Module without content reference - keep it @@ -278,43 +286,10 @@ export async function processPage({ mappedZones[zoneName] = newZoneContent; } - // Content mapping validation - check which mappings were successful - if (contentIdsToValidate.length > 0) { - const mappingResults: { [contentId: number]: { found: boolean; targetId?: number; error?: string } } = {}; - let foundMappings = 0; - let missingMappings = 0; - - contentIdsToValidate.forEach((sourceContentId) => { - const contentMapping = contentMapper.getContentItemMappingByContentID(sourceContentId, "source"); - const targetContentID = contentMapping?.targetContentID; - if (targetContentID) { - mappingResults[sourceContentId] = { - found: true, - targetId: targetContentID, - }; - foundMappings++; - } else { - mappingResults[sourceContentId] = { - found: false, - error: targetContentID ? "Invalid target ID" : "No mapping found", - }; - missingMappings++; - } - }); - - if (missingMappings > 0) { - console.error( - ansiColors.bgRed( - `✗ Page "${page.name}" failed - ${missingMappings}/${contentIdsToValidate.length} missing content mappings` - ) - ); - return { - status: "failure", - error: firstMissingContentError || `${missingMappings} missing content mappings`, - contentID: firstMissingContentID || undefined, - }; - } - } + // PROD-2316: the hard-fail that previously aborted the whole page here whenever ANY module's + // content mapping was missing has been removed. Unresolvable modules are dropped above (with + // per-module warnings in `droppedModules`); the page itself still pushes with the modules + // that DID resolve. // Check if page has any content left after filtering const totalModules = Object.values(mappedZones).reduce((sum: number, zone) => { @@ -365,10 +340,20 @@ export async function processPage({ // If the page originally had modules but now has none, that's a problem // If it never had modules, that's fine (folder pages, etc.) - if (originalModuleCount > 0 && !existingPage && !isLegitimateEmptyPage(page)) { + // PROD-2316: this guard now also covers the UPDATE path (previously create-only). With + // partial drops allowed, a page whose EVERY module failed to resolve signals a systemic + // problem (e.g. the whole content phase failed) — pushing it would wipe all modules off the + // existing target page. Genuine source-side emptiness is unaffected: if the author removed + // every module in source, originalModuleCount is 0 and this never triggers. + if (originalModuleCount > 0 && droppedModules.length > 0 && !isLegitimateEmptyPage(page)) { const lostModulesError = `Lost all ${originalModuleCount} modules during content mapping`; console.error(`✗ Page "${page.name}" ${lostModulesError}`); - return { status: "failure", error: lostModulesError }; + return { + status: "failure", + error: lostModulesError, + contentID: droppedModules[0]?.contentID, + warnings: droppedModules, + }; } } @@ -504,7 +489,20 @@ export async function processPage({ createdPageData.properties.versionID = savedPageVersionID; // Set version ID from batch result } - pageMapper.addMapping(page, createdPageData); // Use original page for source key + if (droppedModules.length > 0) { + // PROD-2316: modules were dropped, so record the mapping with sourceVersionID 0 + // ("never cleanly synced"). hasSourceChanged() then stays true on every subsequent + // sync, so the page keeps re-pushing until a push completes with no drops — which + // restores the dropped modules automatically once their content becomes resolvable. + // targetVersionID is still recorded accurately, so this never reads as a conflict. + const dirtySourcePage = { + ...page, + properties: { ...page.properties, versionID: 0 }, + } as mgmtApi.PageItem; + pageMapper.addMapping(dirtySourcePage, createdPageData); + } else { + pageMapper.addMapping(page, createdPageData); // Use original page for source key + } const pageTypeDisplay = { @@ -518,7 +516,8 @@ export async function processPage({ } else { logger.page.created(page, "created", locale, channel, targetGuid); } - return { status: "success" }; // Success + // PROD-2316: surface any dropped-module notices alongside the success. + return { status: "success", warnings: droppedModules.length > 0 ? droppedModules : undefined }; } else { let errorMsg: string; if (batchFailedItems.length > 0 && batchFailedItems[0].error) { @@ -540,7 +539,7 @@ export async function processPage({ channel, targetGuid ); - return { status: "failure", error: errorMsg }; + return { status: "failure", error: errorMsg, warnings: droppedModules.length > 0 ? droppedModules : undefined }; } } else { const errorMsg = "Unexpected response format"; diff --git a/src/lib/pushers/page-pusher/process-sitemap.ts b/src/lib/pushers/page-pusher/process-sitemap.ts index 5ce0815a..d95cd109 100644 --- a/src/lib/pushers/page-pusher/process-sitemap.ts +++ b/src/lib/pushers/page-pusher/process-sitemap.ts @@ -1,7 +1,7 @@ import * as mgmtApi from "@agility/management-sdk"; import ansiColors from "ansi-colors"; import { state, getApiClient } from "../../../core/state"; -import { PusherResult } from "../../../types/sourceData"; +import { FailureDetail, PusherResult } from "../../../types/sourceData"; import { SitemapHierarchy } from "./sitemap-hierarchy"; import { PageMapper } from "../../mappers/page-mapper"; import { processPage } from "./process-page"; @@ -22,6 +22,8 @@ interface ReturnType { guid?: string; locale?: string; }>; + // PROD-2316: non-blocking dropped-module notices collected from processPage results. + warningDetails: FailureDetail[]; } interface Props { @@ -72,6 +74,7 @@ export async function processSitemap({ skipped: 0, publishableIds: [], failureDetails: [], + warningDetails: [], }; // Reverse the sitemap nodes to process them in the correct order @@ -124,6 +127,11 @@ export async function processSitemap({ logger, }); + // PROD-2316: collect dropped-module notices regardless of the page's final status. + if (pageRes.warnings && pageRes.warnings.length > 0) { + returnData.warningDetails.push(...pageRes.warnings); + } + if (pageRes.status === "success") { returnData.successful++; @@ -180,6 +188,7 @@ export async function processSitemap({ returnData.skipped += childRes.skipped; returnData.publishableIds.push(...childRes.publishableIds); returnData.failureDetails.push(...childRes.failureDetails); + returnData.warningDetails.push(...childRes.warningDetails); // Update previousPageID for next iteration previousPageID = node.pageID; diff --git a/src/lib/pushers/page-pusher/push-pages.ts b/src/lib/pushers/page-pusher/push-pages.ts index 6ffd68f0..456bbc14 100644 --- a/src/lib/pushers/page-pusher/push-pages.ts +++ b/src/lib/pushers/page-pusher/push-pages.ts @@ -1,6 +1,6 @@ import * as mgmtApi from "@agility/management-sdk"; import { state, getApiClient, getLoggerForGuid } from "core/state"; -import { PusherResult } from "../../../types/sourceData"; +import { FailureDetail, PusherResult } from "../../../types/sourceData"; import { SitemapHierarchy } from "lib/pushers/page-pusher/sitemap-hierarchy"; import { PageMapper } from "lib/mappers/page-mapper"; import { processSitemap, resetProcessedPageIDs } from "./process-sitemap"; @@ -16,7 +16,7 @@ export async function pushPages(sourceData: mgmtApi.PageItem[], locale: string): if (!pages || pages.length === 0) { console.log("No pages found to process."); - return { status: "success", successful: 0, failed: 0, skipped: 0, failureDetails: [] }; + return { status: "success", successful: 0, failed: 0, skipped: 0, failureDetails: [], warningDetails: [] }; } const sitemapHierarchy = new SitemapHierarchy(); @@ -43,6 +43,8 @@ export async function pushPages(sourceData: mgmtApi.PageItem[], locale: string): guid?: string; locale?: string; }> = []; + // PROD-2316: non-blocking dropped-module notices from processPage + let warningDetails: FailureDetail[] = []; //loop all the channels for (const channel of channels) { @@ -82,6 +84,9 @@ export async function pushPages(sourceData: mgmtApi.PageItem[], locale: string): if (res.failureDetails && res.failureDetails.length > 0) { failureDetails.push(...res.failureDetails); } + if (res.warningDetails && res.warningDetails.length > 0) { + warningDetails.push(...res.warningDetails); + } if (res.failed > 0) { status = "error"; @@ -122,5 +127,5 @@ export async function pushPages(sourceData: mgmtApi.PageItem[], locale: string): ); } - return { status, successful, failed, skipped, publishableIds: uniquePublishableIds, failureDetails }; + return { status, successful, failed, skipped, publishableIds: uniquePublishableIds, failureDetails, warningDetails }; } diff --git a/src/lib/pushers/page-pusher/tests/process-page.test.ts b/src/lib/pushers/page-pusher/tests/process-page.test.ts index 3eb1887e..0420dd09 100644 --- a/src/lib/pushers/page-pusher/tests/process-page.test.ts +++ b/src/lib/pushers/page-pusher/tests/process-page.test.ts @@ -354,7 +354,7 @@ describe("processPage — failure paths", () => { // ─── missing content mapping ────────────────────────────────────────────────── describe("processPage — missing content mappings", () => { - it("returns failure when a zone module has no content mapping", async () => { + it("returns failure when the page's ONLY module has no content mapping (total-loss guard)", async () => { const { TemplateMapper } = require("lib/mappers/template-mapper"); // Template with a section definition so the zone name is mapped through correctly TemplateMapper.mockImplementation(() => ({ @@ -387,6 +387,177 @@ describe("processPage — missing content mappings", () => { }); }); +// ─── PROD-2316: unresolvable modules are dropped, page still pushes ─────────── + +describe("processPage — dropped modules (PROD-2316)", () => { + function setupTemplateWithMainZone() { + const { TemplateMapper } = require("lib/mappers/template-mapper"); + TemplateMapper.mockImplementation(() => ({ + getTemplateMappingByPageTemplateName: jest.fn().mockReturnValue({ ref: "Main" }), + getMappedEntity: jest.fn().mockReturnValue({ + contentSectionDefinitions: [{ pageItemTemplateReferenceName: "Main", itemOrder: 0 }], + }), + })); + } + + function setupContentMapperResolvingOnly(resolvableIds: Record) { + const { ContentItemMapper } = require("lib/mappers/content-item-mapper"); + ContentItemMapper.mockImplementation(() => ({ + getContentItemMappingByContentID: jest.fn((id: number) => + resolvableIds[id] ? { targetContentID: resolvableIds[id] } : null + ), + })); + } + + it("pushes the page successfully with the unresolvable module dropped, and reports a warning", async () => { + setupTemplateWithMainZone(); + setupContentMapperResolvingOnly({ 55: 955 }); // 55 resolves; 66 does not + + const pageWithContent = makePage({ + zones: { + Main: [ + { module: "Hero", item: { contentid: 55 } }, + { module: "Broken", item: { contentid: 66 } }, + ], + }, + }); + + const pageMapper = makePageMapper({ hasSourceChanged: jest.fn().mockReturnValue(true) }); + const apiClient = makeApiClient(); + + mockExtract.mockReturnValue({ + successfulItems: [{ newId: 400, newItem: { processedItemVersionID: 7 } }], + failedItems: [], + }); + + const result = await processPage(makeProps({ page: pageWithContent, pageMapper, apiClient })); + + expect(result.status).toBe("success"); + expect(result.warnings).toHaveLength(1); + expect(result.warnings![0].contentID).toBe(66); + expect(result.warnings![0].error).toContain("Dropped module Broken"); + + // The pushed payload contains only the resolvable module, remapped to its target ID. + const savedPayload = (apiClient.pageMethods.savePage as jest.Mock).mock.calls[0][0]; + expect(savedPayload.zones.Main).toHaveLength(1); + expect(savedPayload.zones.Main[0].item.contentid).toBe(955); + }); + + it("records the page mapping as dirty (sourceVersionID 0) when modules were dropped, so it re-pushes next sync", async () => { + setupTemplateWithMainZone(); + setupContentMapperResolvingOnly({ 55: 955 }); + + const pageWithContent = makePage({ + properties: { state: 2, versionID: 42 }, + zones: { + Main: [ + { module: "Hero", item: { contentid: 55 } }, + { module: "Broken", item: { contentid: 66 } }, + ], + }, + }); + + const pageMapper = makePageMapper({ hasSourceChanged: jest.fn().mockReturnValue(true) }); + + mockExtract.mockReturnValue({ + successfulItems: [{ newId: 400, newItem: { processedItemVersionID: 7 } }], + failedItems: [], + }); + + await processPage(makeProps({ page: pageWithContent, pageMapper })); + + expect(pageMapper.addMapping).toHaveBeenCalledTimes(1); + const [sourceArg] = (pageMapper.addMapping as jest.Mock).mock.calls[0]; + expect(sourceArg.properties.versionID).toBe(0); + }); + + it("records the page mapping normally (real versionID) when nothing was dropped", async () => { + setupTemplateWithMainZone(); + setupContentMapperResolvingOnly({ 55: 955 }); + + const pageWithContent = makePage({ + properties: { state: 2, versionID: 42 }, + zones: { Main: [{ module: "Hero", item: { contentid: 55 } }] }, + }); + + const pageMapper = makePageMapper({ hasSourceChanged: jest.fn().mockReturnValue(true) }); + + mockExtract.mockReturnValue({ + successfulItems: [{ newId: 401, newItem: { processedItemVersionID: 8 } }], + failedItems: [], + }); + + const result = await processPage(makeProps({ page: pageWithContent, pageMapper })); + + expect(result.status).toBe("success"); + expect(result.warnings).toBeUndefined(); + const [sourceArg] = (pageMapper.addMapping as jest.Mock).mock.calls[0]; + expect(sourceArg.properties.versionID).toBe(42); + }); + + it("fails an UPDATE (not just a create) when EVERY module is unresolvable, instead of wiping the target page", async () => { + setupTemplateWithMainZone(); + setupContentMapperResolvingOnly({}); // nothing resolves + + const pageWithContent = makePage({ + zones: { + Main: [ + { module: "Hero", item: { contentid: 55 } }, + { module: "Promo", item: { contentid: 66 } }, + ], + }, + }); + + // Existing page on target → update path (the old guard only covered creates) + const existingTargetPage = makePage({ pageID: 99 }); + const pageMapper = makePageMapper({ + getPageMapping: jest.fn().mockReturnValue({ targetPageID: 99, sourcePageID: 1 }), + getMappedEntity: jest.fn().mockReturnValue(existingTargetPage), + hasSourceChanged: jest.fn().mockReturnValue(true), + hasTargetChanged: jest.fn().mockReturnValue(null), + }); + + const apiClient = makeApiClient(); + const result = await processPage(makeProps({ page: pageWithContent, pageMapper, apiClient })); + + expect(result.status).toBe("failure"); + expect(result.error).toContain("Lost all 2 modules"); + expect(result.warnings).toHaveLength(2); + // The destructive save is never attempted. + expect(apiClient.pageMethods.savePage).not.toHaveBeenCalled(); + }); + + it("keeps modules without any content reference while dropping unresolvable ones", async () => { + setupTemplateWithMainZone(); + setupContentMapperResolvingOnly({}); + + const pageWithContent = makePage({ + zones: { + Main: [ + { module: "StaticBanner", item: null }, // no content reference — always kept + { module: "Broken", item: { contentid: 66 } }, + ], + }, + }); + + const pageMapper = makePageMapper({ hasSourceChanged: jest.fn().mockReturnValue(true) }); + const apiClient = makeApiClient(); + + mockExtract.mockReturnValue({ + successfulItems: [{ newId: 402, newItem: { processedItemVersionID: 9 } }], + failedItems: [], + }); + + const result = await processPage(makeProps({ page: pageWithContent, pageMapper, apiClient })); + + expect(result.status).toBe("success"); + expect(result.warnings).toHaveLength(1); + const savedPayload = (apiClient.pageMethods.savePage as jest.Mock).mock.calls[0][0]; + expect(savedPayload.zones.Main).toHaveLength(1); + expect(savedPayload.zones.Main[0].module).toBe("StaticBanner"); + }); +}); + // ─── channel fallback ───────────────────────────────────────────────────────── describe("processPage — channel resolution", () => { diff --git a/src/types/sourceData.ts b/src/types/sourceData.ts index e9278838..973de7e0 100644 --- a/src/types/sourceData.ts +++ b/src/types/sourceData.ts @@ -54,6 +54,11 @@ export interface PusherResult { status: "success" | "error"; publishableIds?: number[]; // Optional: target instance IDs for workflow operations (content items and pages only) failureDetails?: FailureDetail[]; // Individual failure details for error summary + // PROD-2316: non-blocking notices (e.g. a page module dropped because its content couldn't be + // resolved). Reuses the FailureDetail shape — `error` carries the warning text — so the same + // CMS-link helpers work. These are surfaced in their own summary section and never affect the + // failure counts or the exit code. + warningDetails?: FailureDetail[]; } /**