diff --git a/src/core/push.ts b/src/core/push.ts index f310bf2..ecd5391 100644 --- a/src/core/push.ts +++ b/src/core/push.ts @@ -165,18 +165,25 @@ export class Push { autoPublishErrors = await this.executeAutoPublish(results, autoPublish); } + // Split auto-publish outcomes: genuine publish failures (blocking) vs + // post-publish mapping/refresh bookkeeping notices (non-blocking). PROD-2311 + const genuinePublishErrors = autoPublishErrors.filter( + ({ type }) => type === "publish" || type === "fatal" + ); + const mappingWarnings = autoPublishErrors.filter( + ({ type }) => type === "mapping" || type === "refresh" + ); + // PROD-2310: Fold auto-publish failures into the returned success signal so the - // entry point can exit non-zero. Real publish failures ("publish") and a fatal - // auto-publish crash ("fatal") are unambiguous failures that CI must detect; they - // previously never affected the exit code. "mapping"/"refresh" entries are - // post-publish bookkeeping that PROD-2311 may reclassify, so they are deliberately - // NOT treated as hard failures here (see PROD-2311 follow-up). + // entry point can exit non-zero. "mapping"/"refresh" entries are post-publish + // bookkeeping (PROD-2311) and are deliberately NOT treated as hard failures here. const success = syncSuccess && !hasBlockingAutoPublishErrors(autoPublishErrors); - // Final error summary - show if there were ANY failures (sync or auto-publish) - const hasFailures = totalSyncFailures > 0 || syncErrors.length > 0 || autoPublishErrors.length > 0; + // Final error summary - show only if there were genuine (blocking) failures + const hasBlockingFailures = + totalSyncFailures > 0 || syncErrors.length > 0 || genuinePublishErrors.length > 0; - if (hasFailures) { + if (hasBlockingFailures) { console.log(ansiColors.red("\n" + "═".repeat(50))); console.log(ansiColors.red("⚠️ ERROR SUMMARY")); console.log(ansiColors.red("═".repeat(50))); @@ -218,16 +225,26 @@ export class Push { }); } - // Show auto-publish errors - if (autoPublishErrors.length > 0) { + // Show genuine auto-publish failures (publish/fatal) + if (genuinePublishErrors.length > 0) { console.log(ansiColors.red(`\n Auto-Publish Errors:`)); - autoPublishErrors.forEach(({ locale, type, error }) => { + genuinePublishErrors.forEach(({ locale, type, error }) => { const localeDisplay = locale ? `[${locale}]` : ""; console.log(ansiColors.red(` • ${localeDisplay} ${type}: ${error}`)); }); } } + // 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) { + console.log(ansiColors.yellow(`\n Mapping Update Warnings (non-blocking):`)); + mappingWarnings.forEach(({ locale, type, error }) => { + const localeDisplay = locale ? `[${locale}]` : ""; + console.log(ansiColors.yellow(` • ${localeDisplay} ${type}: ${error}`)); + }); + } + // Show log file paths at the very end if (logFilePaths.length > 0) { console.log(ansiColors.cyan("\n📄 Log Files:")); diff --git a/src/lib/mappers/mapping-version-updater.ts b/src/lib/mappers/mapping-version-updater.ts index 886ce32..d530317 100644 --- a/src/lib/mappers/mapping-version-updater.ts +++ b/src/lib/mappers/mapping-version-updater.ts @@ -1,16 +1,17 @@ /** * Mapping Version Updater * - * After publishing, updates the mappings with the new versionIDs - * by reading the refreshed data from the filesystem using fileOperations. + * After publishing, updates the mappings with the new targetVersionIDs. + * PROD-2311: versionIDs are read directly from the Management API right after + * publish, instead of a start-of-run filesystem snapshot that misses items + * created during the same run. The Management API is strongly consistent (no + * CDN propagation delay), so a single call per item is enough — no polling. */ -import { fileOperations } from "../../core"; -import { getLogger } from "../../core/state"; +import * as mgmtApi from "@agility/management-sdk"; +import { getApiClient, getLogger } from "../../core/state"; import { ContentItemMapper } from "./content-item-mapper"; import { PageMapper } from "./page-mapper"; -import { getContentItemsFromFileSystem } from "../getters/filesystem/get-content-items"; -import { getPagesFromFileSystem } from "../getters/filesystem/get-pages"; import ansiColors from "ansi-colors"; import { MappingUpdateResult } from "../../types"; @@ -65,32 +66,21 @@ export async function updateContentMappingsAfterPublish( } try { - // Create file operations for target (we only need target data for versionID) - const targetFileOps = new fileOperations(targetGuid, locale); - - // Load content items from target filesystem (refreshed after pull) - const targetContentItems = getContentItemsFromFileSystem(targetFileOps); - - // Create content item mapper const contentMapper = new ContentItemMapper(sourceGuid, targetGuid, locale); + const apiClient = getApiClient(); - // Create lookup map for quick access - const targetContentMap = new Map(targetContentItems.map((item) => [item.contentID, item])); - - // Update targetVersionID for each published content item for (const targetContentId of uniqueContentIds) { - const targetItem = targetContentMap.get(targetContentId); - if (!targetItem) { - errors.push(`Target content item ${targetContentId} not found in filesystem`); + let targetItem: mgmtApi.ContentItem; + try { + targetItem = await apiClient.contentMethods.getContentItem(targetContentId, targetGuid, locale); + } catch (error: any) { + errors.push(`Target content item ${targetContentId} not found on target instance: ${error.message}`); continue; } - // Update only the target versionID in the mapping const result = contentMapper.updateTargetVersionID(targetContentId, targetItem.properties.versionID); - if (result.success) { updated++; - // Track all version updates with display info changes.push({ id: targetContentId, oldVersion: result.oldVersionID!, @@ -134,32 +124,21 @@ export async function updatePageMappingsAfterPublish( } try { - // Create file operations for target (we only need target data for versionID) - const targetFileOps = new fileOperations(targetGuid, locale); - - // Load pages from target filesystem (refreshed after pull) - const targetPages = getPagesFromFileSystem(targetFileOps); - - // Create page mapper const pageMapper = new PageMapper(sourceGuid, targetGuid, locale); + const apiClient = getApiClient(); - // Create lookup map for quick access - const targetPageMap = new Map(targetPages.map((page) => [page.pageID, page])); - - // Update targetVersionID for each published page for (const targetPageId of uniquePageIds) { - const targetPage = targetPageMap.get(targetPageId); - if (!targetPage) { - errors.push(`Target page ${targetPageId} not found in filesystem`); + let targetPage: mgmtApi.PageItem; + try { + targetPage = await apiClient.pageMethods.getPage(targetPageId, targetGuid, locale); + } catch (error: any) { + errors.push(`Target page ${targetPageId} not found on target instance: ${error.message}`); continue; } - // Update only the target versionID in the mapping const result = pageMapper.updateTargetVersionID(targetPageId, targetPage.properties.versionID); - if (result.success) { updated++; - // Track all version updates with display info changes.push({ id: targetPageId, oldVersion: result.oldVersionID!, diff --git a/src/lib/mappers/tests/mapping-version-updater.test.ts b/src/lib/mappers/tests/mapping-version-updater.test.ts index d606b3f..f98c3d5 100644 --- a/src/lib/mappers/tests/mapping-version-updater.test.ts +++ b/src/lib/mappers/tests/mapping-version-updater.test.ts @@ -2,30 +2,20 @@ import * as fs from "fs"; import * as os from "os"; import * as path from "path"; import { resetState, setState } from "core/state"; +import * as stateModule from "core/state"; + +// Mock the API client so no real network calls happen +const mockGetContentItem = jest.fn(); +const mockGetPage = jest.fn(); -// Mock the filesystem getters to avoid real I/O beyond what we control -jest.mock("lib/getters/filesystem/get-content-items", () => ({ - getContentItemsFromFileSystem: jest.fn(), -})); -jest.mock("lib/getters/filesystem/get-pages", () => ({ - getPagesFromFileSystem: jest.fn(), -})); - -// Import after mocks are in place -import { getContentItemsFromFileSystem } from "lib/getters/filesystem/get-content-items"; -import { getPagesFromFileSystem } from "lib/getters/filesystem/get-pages"; import { updateContentMappingsAfterPublish, updatePageMappingsAfterPublish, updateMappingsAfterPublish, - VersionChangeDetail, } from "lib/mappers/mapping-version-updater"; import { ContentItemMapper } from "lib/mappers/content-item-mapper"; import { PageMapper } from "lib/mappers/page-mapper"; -const mockGetContentItems = getContentItemsFromFileSystem as jest.MockedFunction; -const mockGetPages = getPagesFromFileSystem as jest.MockedFunction; - let tmpDir: string; beforeAll(() => { @@ -45,8 +35,12 @@ beforeEach(() => { jest.spyOn(console, "log").mockImplementation(() => {}); jest.spyOn(console, "warn").mockImplementation(() => {}); jest.spyOn(console, "error").mockImplementation(() => {}); - mockGetContentItems.mockReturnValue([]); - mockGetPages.mockReturnValue([]); + mockGetContentItem.mockReset(); + mockGetPage.mockReset(); + jest.spyOn(stateModule, "getApiClient").mockReturnValue({ + contentMethods: { getContentItem: mockGetContentItem }, + pageMethods: { getPage: mockGetPage }, + } as any); }); afterEach(() => { @@ -99,25 +93,26 @@ describe("updateContentMappingsAfterPublish", () => { const result = await updateContentMappingsAfterPublish([], SRC, TGT, LOCALE); expect(result.updated).toBe(0); expect(result.errors).toHaveLength(0); + expect(mockGetContentItem).not.toHaveBeenCalled(); }); it("deduplicates published content IDs before processing", async () => { - mockGetContentItems.mockReturnValue([]); + mockGetContentItem.mockRejectedValue(new Error("not found")); const result = await updateContentMappingsAfterPublish([100, 100, 100], SRC, TGT, LOCALE); - // 100 is not found in filesystem → one error, not three + // 100 is not found on the target → one error, not three expect(result.errors).toHaveLength(1); + expect(mockGetContentItem).toHaveBeenCalledTimes(1); }); - it("records an error when a content item is not found in target filesystem", async () => { - mockGetContentItems.mockReturnValue([]); + it("records an error when a content item is not found on the target instance", async () => { + mockGetContentItem.mockRejectedValue(new Error("404")); const result = await updateContentMappingsAfterPublish([999], SRC, TGT, LOCALE); expect(result.errors.length).toBeGreaterThan(0); expect(result.errors[0]).toMatch(/999/); }); it("records an error when no mapping exists for a target content ID", async () => { - const targetItem = makeContentItem({ contentID: 200, properties: { versionID: 7 } }); - mockGetContentItems.mockReturnValue([targetItem]); + mockGetContentItem.mockResolvedValue(makeContentItem({ contentID: 200, properties: { versionID: 7 } })); const result = await updateContentMappingsAfterPublish([200], SRC, TGT, LOCALE); expect(result.errors.length).toBeGreaterThan(0); expect(result.errors[0]).toMatch(/200/); @@ -125,18 +120,17 @@ describe("updateContentMappingsAfterPublish", () => { it("updates the mapping successfully when item and mapping exist", async () => { seedContentMapper(10, 20, 5); - const targetItem = makeContentItem({ contentID: 20, properties: { versionID: 9 } }); - mockGetContentItems.mockReturnValue([targetItem]); + mockGetContentItem.mockResolvedValue(makeContentItem({ contentID: 20, properties: { versionID: 9 } })); const result = await updateContentMappingsAfterPublish([20], SRC, TGT, LOCALE); expect(result.updated).toBe(1); expect(result.errors).toHaveLength(0); expect(result.changes[0].newVersion).toBe(9); + expect(mockGetContentItem).toHaveBeenCalledWith(20, TGT, LOCALE); }); it("tracks change.changed as false when versionID is already up to date", async () => { seedContentMapper(10, 20, 5); - const targetItem = makeContentItem({ contentID: 20, properties: { versionID: 5 } }); - mockGetContentItems.mockReturnValue([targetItem]); + mockGetContentItem.mockResolvedValue(makeContentItem({ contentID: 20, properties: { versionID: 5 } })); const result = await updateContentMappingsAfterPublish([20], SRC, TGT, LOCALE); expect(result.changes[0].changed).toBe(false); }); @@ -149,18 +143,18 @@ describe("updatePageMappingsAfterPublish", () => { const result = await updatePageMappingsAfterPublish([], SRC, TGT, LOCALE); expect(result.updated).toBe(0); expect(result.errors).toHaveLength(0); + expect(mockGetPage).not.toHaveBeenCalled(); }); - it("records an error when a page is not found in target filesystem", async () => { - mockGetPages.mockReturnValue([]); + it("records an error when a page is not found on the target instance", async () => { + mockGetPage.mockRejectedValue(new Error("404")); const result = await updatePageMappingsAfterPublish([999], SRC, TGT, LOCALE); expect(result.errors.length).toBeGreaterThan(0); expect(result.errors[0]).toMatch(/999/); }); it("records an error when no mapping exists for a target page ID", async () => { - const targetPage = makePage({ pageID: 500, properties: { versionID: 3 } }); - mockGetPages.mockReturnValue([targetPage]); + mockGetPage.mockResolvedValue(makePage({ pageID: 500, properties: { versionID: 3 } })); const result = await updatePageMappingsAfterPublish([500], SRC, TGT, LOCALE); expect(result.errors.length).toBeGreaterThan(0); expect(result.errors[0]).toMatch(/500/); @@ -168,12 +162,12 @@ describe("updatePageMappingsAfterPublish", () => { it("updates the mapping successfully when page and mapping exist", async () => { seedPageMapper(1, 50, 1); - const targetPage = makePage({ pageID: 50, properties: { versionID: 8 } }); - mockGetPages.mockReturnValue([targetPage]); + mockGetPage.mockResolvedValue(makePage({ pageID: 50, properties: { versionID: 8 } })); const result = await updatePageMappingsAfterPublish([50], SRC, TGT, LOCALE); expect(result.updated).toBe(1); expect(result.errors).toHaveLength(0); expect(result.changes[0].newVersion).toBe(8); + expect(mockGetPage).toHaveBeenCalledWith(50, TGT, LOCALE); }); }); @@ -190,12 +184,10 @@ describe("updateMappingsAfterPublish", () => { it("processes both content and page IDs", async () => { seedContentMapper(10, 20, 1); - const targetContent = makeContentItem({ contentID: 20, properties: { versionID: 2 } }); - mockGetContentItems.mockReturnValue([targetContent]); + mockGetContentItem.mockResolvedValue(makeContentItem({ contentID: 20, properties: { versionID: 2 } })); seedPageMapper(1, 50, 1); - const targetPage = makePage({ pageID: 50, properties: { versionID: 2 } }); - mockGetPages.mockReturnValue([targetPage]); + mockGetPage.mockResolvedValue(makePage({ pageID: 50, properties: { versionID: 2 } })); const { result } = await updateMappingsAfterPublish([20], [50], SRC, TGT, LOCALE); expect(result.contentMappingsUpdated).toBe(1); @@ -203,8 +195,8 @@ describe("updateMappingsAfterPublish", () => { }); it("accumulates errors from both content and page processing", async () => { - mockGetContentItems.mockReturnValue([]); - mockGetPages.mockReturnValue([]); + mockGetContentItem.mockRejectedValue(new Error("404")); + mockGetPage.mockRejectedValue(new Error("404")); const { result } = await updateMappingsAfterPublish([999], [888], SRC, TGT, LOCALE); expect(result.errors.length).toBeGreaterThanOrEqual(2); }); @@ -215,8 +207,7 @@ describe("updateMappingsAfterPublish", () => { describe("VersionChangeDetail structure", () => { it("includes expected fields in change objects", async () => { seedContentMapper(10, 20, 5); - const targetItem = makeContentItem({ contentID: 20, properties: { versionID: 9 } }); - mockGetContentItems.mockReturnValue([targetItem]); + mockGetContentItem.mockResolvedValue(makeContentItem({ contentID: 20, properties: { versionID: 9 } })); const result = await updateContentMappingsAfterPublish([20], SRC, TGT, LOCALE); const change = result.changes[0]; expect(change).toMatchObject({