From b3efc8a0e5c41aa8a3cdd622413040e418d20330 Mon Sep 17 00:00:00 2001 From: Joel Varty Date: Thu, 22 Oct 2026 12:01:43 -0400 Subject: [PATCH 1/3] PROD-2914: pull and sync component availability rules Component models can now carry rules about who can add them to a page, where they can be placed, and whether they're disabled. The CLI treats them by ownership: - Pull: a new "downloadModelAvailability" operation (with the Models element) writes model-availability/{modelID}.json, one file per model that has rules. The folder only exists when the instance answered: a 404/405/409 (older API, or a database without the feature yet) or an SDK without the methods pulls no rules and removes stale ones. - Push: a new "pushModelAvailability" phase runs right after templates, so the page models and zones it names exist. It sends allowedIn only, with addableBy null so the target keeps its own who rules. Targets are remapped by name (sitemap referenceName, page model name, zone page model name + referenceName), with target IDs from the template and zone mappings when they exist; source IDs are never sent. A source model with no where rules clears the target's, but only when the target's rules are known. Unchanged rules are skipped. Preflight records the plan. A target without the feature stops the phase with one warning. - Models: an update never sends isVisible, so the target keeps its own disabled state; a created model takes the source's value. - Pages: a page the API rejects with a component availability reason (HTTP 400) is reported as "Rejected by a component availability rule: ". The SDK range moves to ^0.2.0-beta.1, the expected @agility/management-sdk release with the availability methods (PROD-2913). package-lock.json is not updated: it needs that version published first. Co-Authored-By: Claude Opus 5.5 --- README.md | 21 ++ package.json | 2 +- .../download-model-availability.ts | 80 +++++ .../downloaders/download-operations-config.ts | 11 + .../tests/download-model-availability.test.ts | 123 +++++++ .../tests/download-operations-config.test.ts | 6 +- .../filesystem/get-model-availability.ts | 15 + src/lib/pushers/model-availability-pusher.ts | 208 +++++++++++ src/lib/pushers/model-pusher.ts | 8 +- src/lib/pushers/orchestrate-pushers.ts | 7 +- .../component-availability-error.ts | 40 +++ src/lib/pushers/page-pusher/process-page.ts | 8 +- .../component-availability-error.test.ts | 55 +++ .../page-pusher/tests/process-page.test.ts | 27 ++ src/lib/pushers/push-operations-config.ts | 12 + .../tests/model-availability-pusher.test.ts | 322 ++++++++++++++++++ src/lib/pushers/tests/model-pusher.test.ts | 69 ++++ .../pushers/tests/orchestrate-pushers.test.ts | 2 + .../tests/push-operations-config.test.ts | 44 ++- src/tests/sync/full-instance.test.ts | 35 +- src/tests/sync/helpers/fake-instance.ts | 35 ++ .../sync/helpers/full-instance-fixture.ts | 22 +- 22 files changed, 1124 insertions(+), 28 deletions(-) create mode 100644 src/lib/downloaders/download-model-availability.ts create mode 100644 src/lib/downloaders/tests/download-model-availability.test.ts create mode 100644 src/lib/getters/filesystem/get-model-availability.ts create mode 100644 src/lib/pushers/model-availability-pusher.ts create mode 100644 src/lib/pushers/page-pusher/component-availability-error.ts create mode 100644 src/lib/pushers/page-pusher/tests/component-availability-error.test.ts create mode 100644 src/lib/pushers/tests/model-availability-pusher.test.ts diff --git a/README.md b/README.md index ab351a03..402fcf66 100644 --- a/README.md +++ b/README.md @@ -256,6 +256,26 @@ agility sync --sourceGuid="abc123" --targetGuid="def456" --models-with-deps="Pro --- +### Component Availability Rules + +A component model can carry rules about **who** can add it to a page, **where** it can be +placed (sitemaps, page models and zones), and whether it's **disabled**. The CLI moves them +like this: + +- **Pull** saves each model's rules to `model-availability/{modelID}.json`. An instance that + doesn't support the rules yet is pulled without them. +- **Sync** sends the **where** rules with the models, after templates, so the page models and + zones they name exist on the target. They're matched on the target by name: sitemaps by + reference name, page models by name, and zones by page model name plus zone reference name. + A source model with no where rules clears the target's. +- **Who** rules and **disabled** belong to each instance and are never overwritten. A model the + sync creates starts disabled if it's disabled on the source. +- A page the target rejects because of a rule is reported with the reason, for example + `Rejected by a component availability rule: The "Hero" component isn't allowed in the "Main" + zone of the "Landing Page" page model.` + +--- + ### Selective Page Sync `--pages` narrows a sync to one or more page structures. Use it when you want to promote the @@ -576,6 +596,7 @@ agility-files/ │ │ ├── json/ # Asset metadata │ ├── galleries/ # Gallery definitions │ ├── models/ # Content models +│ ├── model-availability/ # Component availability rules, one file per model that has any │ ├── containers/ # Content containers │ ├── templates/ # Page templates │ ├── sitemap/ # Flat sitemap diff --git a/package.json b/package.json index 63196926..fbe78227 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ }, "dependencies": { "@agility/content-sync": "^1.2.0", - "@agility/management-sdk": "^0.1.38", + "@agility/management-sdk": "^0.2.0-beta.1", "ansi-colors": "^4.1.3", "axios": "^0.27.2", "cli-progress": "^3.11.2", diff --git a/src/lib/downloaders/download-model-availability.ts b/src/lib/downloaders/download-model-availability.ts new file mode 100644 index 00000000..d9da4e36 --- /dev/null +++ b/src/lib/downloaders/download-model-availability.ts @@ -0,0 +1,80 @@ +import * as fs from "fs"; +import * as path from "path"; +import * as mgmtApi from "@agility/management-sdk"; +import { fileOperations } from "core/fileOperations"; +import { getApiClient, getLoggerForGuid } from "core/state"; + +/** + * PROD-2914: component availability rules (who can add a component model, and where it can go) are + * pulled into their own folder, one file per model: `{guid}/model-availability/{modelID}.json`, the + * same layout as `models/{modelID}.json`. They live outside `models/` because everything in that + * folder is read back as a model. + * + * The folder's presence is meaningful: it exists (possibly empty) only when the instance answered, + * so "no file for a model" means "no rules". When the instance can't answer (an API or database + * without the feature yet), the folder is removed, and the push treats that side as unknown and + * leaves its rules alone. + */ +export const MODEL_AVAILABILITY_FOLDER = "model-availability"; + +/** + * Status codes that mean "this instance doesn't support availability rules yet": 404/405 from an + * API without the route, 409 from an API whose instance database hasn't been migrated. + */ +export function isAvailabilityUnsupported(error: any): boolean { + const status = error?.statusCode ?? error?.response?.status; + return status === 404 || status === 405 || status === 409; +} + +export async function downloadModelAvailability(guid: string): Promise { + const fileOps = new fileOperations(guid); + const apiClient = getApiClient(); + const logger = getLoggerForGuid(guid); + const folderPath = fileOps.getDataFolderPath(MODEL_AVAILABILITY_FOLDER); + + const removeFolder = () => { + if (fs.existsSync(folderPath)) fs.rmSync(folderPath, { recursive: true, force: true }); + }; + + // An older SDK build has no availability methods: same as an instance without the feature. + if (typeof apiClient?.modelMethods?.getAllModelAvailability !== "function") { + removeFolder(); + return; + } + + let rules: mgmtApi.ModelAvailability[]; + try { + rules = (await apiClient.modelMethods.getAllModelAvailability(guid)) || []; + } catch (error: any) { + // Never leave stale rules behind: a push would otherwise send rules the instance no longer has. + removeFolder(); + if (isAvailabilityUnsupported(error)) { + logger?.info(`Component availability rules aren't available on ${guid} yet; skipping them.`); + return; + } + logger?.error(`Error pulling component availability rules: ${error?.message || error}`); + throw error; + } + + fs.mkdirSync(folderPath, { recursive: true }); + + const pulledIds = new Set(); + for (const rule of rules) { + if (rule?.modelID === undefined || rule?.modelID === null) continue; + const fileName = rule.modelID.toString(); + pulledIds.add(fileName); + fileOps.exportFiles(MODEL_AVAILABILITY_FOLDER, fileName, rule); + } + + // A model that has lost all its rules is no longer listed: drop its file. + for (const file of fs.readdirSync(folderPath)) { + if (!file.endsWith(".json")) continue; + if (!pulledIds.has(file.replace(".json", ""))) { + fs.unlinkSync(path.join(folderPath, file)); + } + } + + if (pulledIds.size > 0) { + logger?.info(`Pulled component availability rules for ${pulledIds.size} model(s).`); + } +} diff --git a/src/lib/downloaders/download-operations-config.ts b/src/lib/downloaders/download-operations-config.ts index f8f24d17..bc3b6aa3 100644 --- a/src/lib/downloaders/download-operations-config.ts +++ b/src/lib/downloaders/download-operations-config.ts @@ -2,6 +2,7 @@ import { downloadAllGalleries } from "./download-galleries"; import { downloadAllAssets } from "./download-assets"; import { downloadAllModels } from "./download-models"; +import { downloadModelAvailability } from "./download-model-availability"; import { downloadAllTemplates } from "./download-templates"; import { downloadAllContainers } from "./download-containers"; import { downloadAllSyncSDK } from "./download-sync-sdk"; @@ -56,6 +57,16 @@ export const DOWNLOAD_OPERATIONS: Record = { }, elements: ["Models"], }, + // PROD-2914: component availability rules travel with the models. A separate operation so an + // instance without the feature (or a failed read) never fails the model download itself. + modelAvailability: { + name: "downloadModelAvailability", + description: "Download component availability rules", + handler: async (guid) => { + await downloadModelAvailability(guid); + }, + elements: ["Models"], + }, templates: { name: "downloadAllTemplates", description: "Download page templates and layouts", diff --git a/src/lib/downloaders/tests/download-model-availability.test.ts b/src/lib/downloaders/tests/download-model-availability.test.ts new file mode 100644 index 00000000..a60aa924 --- /dev/null +++ b/src/lib/downloaders/tests/download-model-availability.test.ts @@ -0,0 +1,123 @@ +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"; +import { fileOperations } from "core/fileOperations"; +import { downloadModelAvailability, isAvailabilityUnsupported } from "../download-model-availability"; +import { getModelAvailabilityFromFileSystem } from "../../getters/filesystem/get-model-availability"; + +const GUID = "avail-pull-u"; + +let tmpDir: string; +let folder: string; + +beforeAll(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "agility-availability-pull-")); + folder = path.join(tmpDir, GUID, "model-availability"); +}); + +afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +beforeEach(() => { + resetState(); + setState({ rootPath: tmpDir }); + fs.rmSync(path.join(tmpDir, GUID), { recursive: true, force: true }); + jest.spyOn(stateModule, "getLoggerForGuid").mockReturnValue({ info: jest.fn(), error: jest.fn() } as any); + jest.spyOn(console, "log").mockImplementation(() => {}); + jest.spyOn(console, "warn").mockImplementation(() => {}); + jest.spyOn(console, "error").mockImplementation(() => {}); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +function mockApi(getAll: jest.Mock | undefined) { + jest + .spyOn(stateModule, "getApiClient") + .mockReturnValue({ modelMethods: getAll ? { getAllModelAvailability: getAll } : {} } as any); +} + +function httpError(statusCode: number) { + return Object.assign(new Error(`HTTP ${statusCode}`), { statusCode }); +} + +function seedStaleFile(modelID: number) { + fs.mkdirSync(folder, { recursive: true }); + fs.writeFileSync(path.join(folder, `${modelID}.json`), JSON.stringify({ modelID })); +} + +const rule = (modelID: number) => ({ + modelID, + addableBy: null, + allowedIn: { sitemaps: [], pageModels: [], zones: [] }, +}); + +describe("downloadModelAvailability", () => { + it("writes one file per model with rules", async () => { + mockApi(jest.fn().mockResolvedValue([rule(12), rule(13)])); + + await downloadModelAvailability(GUID); + + expect(fs.readdirSync(folder).sort()).toEqual(["12.json", "13.json"]); + expect(JSON.parse(fs.readFileSync(path.join(folder, "12.json"), "utf8"))).toEqual(rule(12)); + }); + + it("creates the folder even when the instance has no rules, so the push knows there are none", async () => { + mockApi(jest.fn().mockResolvedValue([])); + + await downloadModelAvailability(GUID); + + expect(fs.existsSync(folder)).toBe(true); + expect(getModelAvailabilityFromFileSystem(new fileOperations(GUID))).toEqual([]); + }); + + it("removes the file of a model that no longer has rules", async () => { + seedStaleFile(99); + mockApi(jest.fn().mockResolvedValue([rule(12)])); + + await downloadModelAvailability(GUID); + + expect(fs.readdirSync(folder)).toEqual(["12.json"]); + }); + + it.each([404, 405, 409])("treats HTTP %i as an instance without the feature: no folder, no error", async (status) => { + seedStaleFile(12); + mockApi(jest.fn().mockRejectedValue(httpError(status))); + + await expect(downloadModelAvailability(GUID)).resolves.toBeUndefined(); + + expect(fs.existsSync(folder)).toBe(false); + expect(getModelAvailabilityFromFileSystem(new fileOperations(GUID))).toBeNull(); + }); + + it("treats an SDK without the availability methods the same way", async () => { + seedStaleFile(12); + mockApi(undefined); + + await expect(downloadModelAvailability(GUID)).resolves.toBeUndefined(); + + expect(fs.existsSync(folder)).toBe(false); + }); + + it("removes stale rules and rethrows any other error", async () => { + seedStaleFile(12); + mockApi(jest.fn().mockRejectedValue(httpError(500))); + + await expect(downloadModelAvailability(GUID)).rejects.toThrow("HTTP 500"); + + expect(fs.existsSync(folder)).toBe(false); + }); +}); + +describe("isAvailabilityUnsupported", () => { + it("reads the SDK Exception's statusCode and a raw axios error's response status", () => { + expect(isAvailabilityUnsupported({ statusCode: 409 })).toBe(true); + expect(isAvailabilityUnsupported({ response: { status: 404 } })).toBe(true); + expect(isAvailabilityUnsupported({ statusCode: 400 })).toBe(false); + expect(isAvailabilityUnsupported(new Error("network"))).toBe(false); + }); +}); diff --git a/src/lib/downloaders/tests/download-operations-config.test.ts b/src/lib/downloaders/tests/download-operations-config.test.ts index f0c1bed2..b4101d6e 100644 --- a/src/lib/downloaders/tests/download-operations-config.test.ts +++ b/src/lib/downloaders/tests/download-operations-config.test.ts @@ -87,7 +87,8 @@ describe("DownloadOperationsRegistry.getOperationsForElements", () => { setState({ elements: "Models" }); const ops = DownloadOperationsRegistry.getOperationsForElements(true); const names = ops.map((o: OperationConfig) => o.name); - expect(names).toEqual(["downloadAllModels"]); + // PROD-2914: availability rules travel with the models. + expect(names).toEqual(["downloadAllModels", "downloadModelAvailability"]); expect(names).not.toContain("downloadAllAssets"); expect(names).not.toContain("downloadAllGalleries"); }); @@ -96,7 +97,8 @@ describe("DownloadOperationsRegistry.getOperationsForElements", () => { setState({ models: "FooterLinks" }); // elements stays at the full default const ops = DownloadOperationsRegistry.getOperationsForElements(true); const names = ops.map((o: OperationConfig) => o.name); - expect(names).toEqual(["downloadAllModels"]); + // PROD-2914: availability rules travel with the models. + expect(names).toEqual(["downloadAllModels", "downloadModelAvailability"]); expect(names).not.toContain("downloadAllAssets"); }); diff --git a/src/lib/getters/filesystem/get-model-availability.ts b/src/lib/getters/filesystem/get-model-availability.ts new file mode 100644 index 00000000..d8c49d2a --- /dev/null +++ b/src/lib/getters/filesystem/get-model-availability.ts @@ -0,0 +1,15 @@ +import * as mgmtApi from "@agility/management-sdk"; +import { fileOperations } from "../../../core"; +import { MODEL_AVAILABILITY_FOLDER } from "../../downloaders/download-model-availability"; + +/** + * Get the pulled component availability rules (PROD-2914) from the filesystem. + * + * Returns `null` when the folder doesn't exist, which means the rules are unknown for this instance + * (never pulled, or the instance doesn't support them yet). An empty array means the instance was + * read and has no rules. + */ +export function getModelAvailabilityFromFileSystem(fileOps: fileOperations): mgmtApi.ModelAvailability[] | null { + if (!fileOps.folderExists(MODEL_AVAILABILITY_FOLDER)) return null; + return fileOps.readJsonFilesFromFolder(MODEL_AVAILABILITY_FOLDER) as mgmtApi.ModelAvailability[]; +} diff --git a/src/lib/pushers/model-availability-pusher.ts b/src/lib/pushers/model-availability-pusher.ts new file mode 100644 index 00000000..7d8d9027 --- /dev/null +++ b/src/lib/pushers/model-availability-pusher.ts @@ -0,0 +1,208 @@ +import * as mgmtApi from "@agility/management-sdk"; +import ansiColors from "ansi-colors"; +import { fileOperations } from "../../core/fileOperations"; +import { getApiClient, getLoggerForGuid, state } from "../../core/state"; +import { FailureDetail, PusherResult } from "../../types/sourceData"; +import { ModelMapper } from "lib/mappers/model-mapper"; +import { TemplateMapper } from "lib/mappers/template-mapper"; +import { SectionMapper } from "lib/mappers/section-mapper"; +import { getModelAvailabilityFromFileSystem } from "../getters/filesystem/get-model-availability"; +import { isAvailabilityUnsupported } from "../downloaders/download-model-availability"; +import { preflightReport } from "../preflight/preflight-report"; + +/** + * PROD-2914: push component availability rules ("where it can go") from the source to the target. + * + * Ownership, per the component availability design: + * - `allowedIn` (sitemaps, page models, zones) is modelling, so it travels with the model. The + * target ends up with the source's rules, including having them cleared when the source has none. + * - `addableBy` (who can add) belongs to the instance: it's always sent as `null`, so the API keeps + * the target's own rules. (Disabling a model is `isVisible`, handled by the model pusher.) + * + * Runs after templates, so the page models and zones it names exist on the target. Targets are + * remapped for the target instance: page models and zones by their ID mapping when there is one + * (and by name otherwise), sitemaps by `referenceName`. Source IDs are never sent, because the API + * resolves an ID before a name and a source ID can belong to a different object on the target. + */ + +const EMPTY_ALLOWED_IN: mgmtApi.ModelAvailabilityAllowedIn = { sitemaps: [], pageModels: [], zones: [] }; + +function lower(value: string | null | undefined): string { + return (value || "").toLowerCase(); +} + +function isEmptyAllowedIn(allowedIn: mgmtApi.ModelAvailabilityAllowedIn | null | undefined): boolean { + return ( + !allowedIn || + ((allowedIn.sitemaps || []).length === 0 && + (allowedIn.pageModels || []).length === 0 && + (allowedIn.zones || []).length === 0) + ); +} + +/** Name keys of an `allowedIn` group, for comparing a source group with what the target holds. */ +function allowedInKey(allowedIn: mgmtApi.ModelAvailabilityAllowedIn | null | undefined): string { + const group = allowedIn || EMPTY_ALLOWED_IN; + const sitemaps = (group.sitemaps || []).map((s) => lower(s.referenceName)).sort(); + const pageModels = (group.pageModels || []).map((p) => lower(p.name)).sort(); + const zones = (group.zones || []).map((z) => `${lower(z.pageModelName)}/${lower(z.referenceName || z.name)}`).sort(); + return JSON.stringify({ sitemaps, pageModels, zones }); +} + +/** + * Rewrite a source `allowedIn` group for the target instance. Only the portable names, and target + * IDs from the sync's own mappings, are sent. + */ +export function remapAllowedIn( + allowedIn: mgmtApi.ModelAvailabilityAllowedIn, + templateMapper: TemplateMapper, + sectionMapper: SectionMapper +): mgmtApi.ModelAvailabilityAllowedIn { + const targetPageModel = (sourcePageModelID: number | null | undefined) => + sourcePageModelID ? templateMapper.getTemplateMappingByPageTemplateID(sourcePageModelID, "source") : null; + + return { + sitemaps: (allowedIn.sitemaps || []).map((sitemap) => ({ + referenceName: sitemap.referenceName ?? null, + name: sitemap.name ?? null, + })), + pageModels: (allowedIn.pageModels || []).map((pageModel) => { + const mapping = targetPageModel(pageModel.pageModelID); + return { + ...(mapping ? { pageModelID: mapping.targetPageTemplateID } : {}), + name: mapping?.targetPageTemplateName || pageModel.name || null, + }; + }), + zones: (allowedIn.zones || []).map((zone) => { + const pageModelMapping = targetPageModel(zone.pageModelID); + const zoneMapping = zone.zoneID ? sectionMapper.getSectionMappingByID(zone.zoneID, "source") : null; + return { + ...(zoneMapping ? { zoneID: zoneMapping.targetPageItemTemplateID } : {}), + ...(pageModelMapping ? { pageModelID: pageModelMapping.targetPageTemplateID } : {}), + pageModelName: pageModelMapping?.targetPageTemplateName || zone.pageModelName || null, + referenceName: zoneMapping?.targetReferenceName || zone.referenceName || null, + name: zone.name ?? null, + }; + }), + }; +} + +export async function pushModelAvailability(sourceModels: mgmtApi.Model[]): Promise { + const { sourceGuid, targetGuid } = state; + const logger = getLoggerForGuid(sourceGuid); + + const sourceRules = getModelAvailabilityFromFileSystem(new fileOperations(sourceGuid)); + if (sourceRules === null) { + // The source didn't answer (an API or database without the feature): leave the target alone. + logger?.info("No component availability rules were pulled from the source; skipping them."); + return { status: "success", successful: 0, failed: 0, skipped: 0 }; + } + // null = unknown: without the target's rules, a source with no rules mustn't clear anything. + const targetRules = getModelAvailabilityFromFileSystem(new fileOperations(targetGuid)); + + const sourceByModelID = new Map(); + for (const rule of sourceRules) if (rule?.modelID) sourceByModelID.set(rule.modelID, rule); + const targetByModelID = new Map(); + for (const rule of targetRules || []) if (rule?.modelID) targetByModelID.set(rule.modelID, rule); + + const modelMapper = new ModelMapper(sourceGuid, targetGuid); + const templateMapper = new TemplateMapper(sourceGuid, targetGuid); + const sectionMapper = new SectionMapper(sourceGuid, targetGuid); + const apiClient = getApiClient(); + + let successful = 0; + let failed = 0; + let skipped = 0; + const failureDetails: FailureDetail[] = []; + const warningDetails: FailureDetail[] = []; + + // Only models in this sync's scope (sourceModels is already narrowed by --models and friends). + for (const sourceModel of sourceModels || []) { + if (!sourceModel?.id) continue; + const sourceAllowedIn = sourceByModelID.get(sourceModel.id)?.allowedIn ?? null; + + const mapping = modelMapper.getModelMappingByID(sourceModel.id, "source"); + if (!mapping?.targetID) { + if (isEmptyAllowedIn(sourceAllowedIn)) continue; + if (state.preflight) { + // Preflight writes no models, so a model the real run would create has no mapping yet. + preflightReport.record({ + phase: "Component availability", + action: "create", + name: sourceModel.referenceName, + detail: "set the placement rules once the model exists on the target", + }); + successful++; + } else { + // The model itself wasn't synced (failed or skipped), so there's nothing to attach rules to. + skipped++; + } + continue; + } + + const targetKnown = targetRules !== null; + const targetAllowedIn = targetByModelID.get(mapping.targetID)?.allowedIn ?? null; + + // Most models have no rules on either side: nothing to do, and nothing worth counting. + if (isEmptyAllowedIn(sourceAllowedIn) && (!targetKnown || isEmptyAllowedIn(targetAllowedIn))) continue; + + const desired = isEmptyAllowedIn(sourceAllowedIn) + ? EMPTY_ALLOWED_IN + : remapAllowedIn(sourceAllowedIn, templateMapper, sectionMapper); + + if (targetKnown && allowedInKey(desired) === allowedInKey(targetAllowedIn)) { + logger?.fileOnly(`Component availability for "${sourceModel.referenceName}" is unchanged, skipping.`); + skipped++; + continue; + } + + if (state.preflight) { + preflightReport.record({ + phase: "Component availability", + action: "update", + name: sourceModel.referenceName, + detail: isEmptyAllowedIn(desired) ? "clear the placement rules" : "replace the placement rules", + }); + successful++; + continue; + } + + try { + await apiClient.modelMethods.saveModelAvailability( + mapping.targetID, + { modelID: mapping.targetID, addableBy: null, allowedIn: desired }, + targetGuid + ); + logger?.info(`Component availability for "${sourceModel.referenceName}" updated.`); + successful++; + } catch (error: any) { + if (isAvailabilityUnsupported(error)) { + // The target can't store rules yet. Say so once and stop: every other save would fail too. + const message = + "The target instance doesn't support component availability rules yet, so placement rules weren't synced."; + console.log(ansiColors.yellow(`⚠️ ${message}`)); + logger?.warning(message); + warningDetails.push({ name: "Component availability", error: message, type: "model", guid: sourceGuid }); + break; + } + const reason = error?.message ? String(error.message) : String(error); + logger?.error(`Component availability for "${sourceModel.referenceName}" failed: ${reason}`); + failed++; + failureDetails.push({ + name: sourceModel.referenceName, + type: "model", + error: `Failed to save component availability for "${sourceModel.referenceName}" (target ID: ${mapping.targetID}): ${reason}`, + guid: sourceGuid, + }); + } + } + + return { + status: failed > 0 ? "error" : "success", + successful, + failed, + skipped, + failureDetails, + warningDetails, + }; +} diff --git a/src/lib/pushers/model-pusher.ts b/src/lib/pushers/model-pusher.ts index 354cf73e..1d65fafc 100644 --- a/src/lib/pushers/model-pusher.ts +++ b/src/lib/pushers/model-pusher.ts @@ -524,6 +524,8 @@ const createNewModel = async ( ): Promise<{ result: "created" | "failed"; error?: string }> => { try { // process the model without fields + // PROD-2914: a new model takes the source's `isVisible` (a component disabled on the source + // arrives disabled), carried by the spread. The field update that follows leaves it out. const createPayload = { ...model, id: 0, @@ -563,10 +565,14 @@ async function updateExistingModel( logger: Logs ): Promise<"updated" | "failed"> { try { - const updatePayload = { + // PROD-2914: `isVisible` (whether a component is disabled) belongs to the target instance, so an + // update never sends it: left out, the API keeps the target's value. A model created this run + // already got the source's value in its stub. + const updatePayload: mgmtApi.Model = { ...sourceModel, id: targetID, }; + delete updatePayload.isVisible; const updatedModel = await apiClient.modelMethods.saveModel(updatePayload, targetGuid); logger.model.updated(sourceModel, "updated", targetGuid); diff --git a/src/lib/pushers/orchestrate-pushers.ts b/src/lib/pushers/orchestrate-pushers.ts index 57b49a48..573c0a6a 100644 --- a/src/lib/pushers/orchestrate-pushers.ts +++ b/src/lib/pushers/orchestrate-pushers.ts @@ -198,7 +198,11 @@ export class Pushers { // URL redirections depend on nothing, so they run BEFORE templates: the template pusher // deliberately throws on mapping inconsistencies (rename protection), which aborts the rest // of the guid-level loop — redirections must not be collateral damage of that stop. - // ORDER: Models → Galleries → Assets → Containers → URL Redirections → Content → Templates → Pages + // PROD-2914: component availability rules name page models and zones, which only exist on the + // target once templates are pushed, so they run right after templates (still guid-level, so + // before the per-locale content and page phases). + // ORDER: Models → Galleries → Assets → Containers → URL Redirections → Content → Templates → + // Model availability → Pages const pusherConfig = [ PUSH_OPERATIONS.models, PUSH_OPERATIONS.galleries, @@ -207,6 +211,7 @@ export class Pushers { PUSH_OPERATIONS.urlRedirections, PUSH_OPERATIONS.content, PUSH_OPERATIONS.templates, + PUSH_OPERATIONS.modelAvailability, PUSH_OPERATIONS.pages, ]; diff --git a/src/lib/pushers/page-pusher/component-availability-error.ts b/src/lib/pushers/page-pusher/component-availability-error.ts new file mode 100644 index 00000000..2227c661 --- /dev/null +++ b/src/lib/pushers/page-pusher/component-availability-error.ts @@ -0,0 +1,40 @@ +/** + * PROD-2914: the Management API checks component availability rules before it queues a page save, + * and rejects a page that adds a component somewhere a rule doesn't allow with an HTTP 400 whose body + * is the reason. The reasons are fixed texts, the same in every Agility stack: + * + * The "{model}" component is disabled on this instance. + * You don't have permission to add the "{model}" component. + * The "{model}" component isn't allowed on the "{sitemap}" sitemap. + * The "{model}" component isn't allowed in the "{zone}" zone of the "{pageModel}" page model. + */ +const AVAILABILITY_REASON = + /The ".+" component is disabled on this instance\.|You don't have permission to add the ".+" component\.|The ".+" component isn't allowed (?:on|in) the ".+"/; + +/** The server's response body as text, from the SDK's Exception (or a raw axios error). */ +function responseText(error: any): string | null { + const data = error?.responseData ?? error?.response?.data; + if (data === undefined || data === null) return null; + if (typeof data === "string") return data; + const text = data.exceptionMessage ?? data.message ?? data.Message ?? data.detail ?? data.title; + return typeof text === "string" ? text : null; +} + +/** + * The component availability reason a page save was rejected with, or `null` when the error is + * anything else. + */ +export function componentAvailabilityReason(error: any): string | null { + const status = error?.statusCode ?? error?.response?.status; + if (status !== 400) return null; + const text = responseText(error); + if (!text || !AVAILABILITY_REASON.test(text)) return null; + return text.trim(); +} + +/** The message to report for a failed page save: the rule's reason when a rule rejected it. */ +export function describePageSaveError(error: any): string { + const reason = componentAvailabilityReason(error); + if (reason) return `Rejected by a component availability rule: ${reason}`; + return error?.message ? String(error.message) : String(error); +} diff --git a/src/lib/pushers/page-pusher/process-page.ts b/src/lib/pushers/page-pusher/process-page.ts index 927c19db..736f892b 100644 --- a/src/lib/pushers/page-pusher/process-page.ts +++ b/src/lib/pushers/page-pusher/process-page.ts @@ -7,6 +7,7 @@ import { SectionMapper } from "lib/mappers/section-mapper"; import { translateZoneNames } from "./translate-zone-names"; import { findPageInOtherLocale, OtherLocaleMapping } from "./find-page-in-other-locale"; import { refreshOtherLocaleVersions } from "./refresh-other-locale-versions"; +import { describePageSaveError } from "./component-availability-error"; import { Logs } from "core/logs"; import { state, @@ -594,14 +595,17 @@ export async function processPage({ return { status: "failure", error: errorMsg }; } } catch (error: any) { + // PROD-2914: a page the API rejected because of a component availability rule reports the + // rule's reason, not the SDK's wrapped HTTP error. + const errorMsg = describePageSaveError(error); logger.page.error( page, - `✗ Page "${page.name}" failed in locale:${locale} - ${error.message}`, + `✗ Page "${page.name}" failed in locale:${locale} - ${errorMsg}`, locale, channel, targetGuid ); - return { status: "failure", error: error.message }; + return { status: "failure", error: errorMsg }; } } diff --git a/src/lib/pushers/page-pusher/tests/component-availability-error.test.ts b/src/lib/pushers/page-pusher/tests/component-availability-error.test.ts new file mode 100644 index 00000000..1eb63490 --- /dev/null +++ b/src/lib/pushers/page-pusher/tests/component-availability-error.test.ts @@ -0,0 +1,55 @@ +import { componentAvailabilityReason, describePageSaveError } from "../component-availability-error"; + +/** An SDK Exception as the Management API 400 produces it. */ +function sdkError(statusCode: number, responseData: unknown, message = "Unable to create page. (HTTP 400): ...") { + return Object.assign(new Error(message), { statusCode, responseData }); +} + +describe("componentAvailabilityReason", () => { + it.each([ + 'The "Hero Banner" component is disabled on this instance.', + 'You don\'t have permission to add the "Hero Banner" component.', + 'The "Hero Banner" component isn\'t allowed on the "Website" sitemap.', + 'The "Hero Banner" component isn\'t allowed in the "Main" zone of the "Landing Page" page model.', + ])("returns the rule's reason for: %s", (reason) => { + expect(componentAvailabilityReason(sdkError(400, reason))).toBe(reason); + }); + + it("reads the reason from a JSON error body", () => { + const reason = 'The "Hero" component is disabled on this instance.'; + expect(componentAvailabilityReason(sdkError(400, { message: reason }))).toBe(reason); + expect(componentAvailabilityReason(sdkError(400, { Message: reason }))).toBe(reason); + }); + + it("reads a raw axios error", () => { + const reason = 'The "Hero" component is disabled on this instance.'; + expect(componentAvailabilityReason({ response: { status: 400, data: reason } })).toBe(reason); + }); + + it("ignores any other 400", () => { + expect(componentAvailabilityReason(sdkError(400, "Page name is required."))).toBeNull(); + }); + + it("ignores a rule-like text on another status", () => { + expect(componentAvailabilityReason(sdkError(500, 'The "Hero" component is disabled on this instance.'))).toBeNull(); + }); + + it("ignores errors with no response", () => { + expect(componentAvailabilityReason(new Error("socket hang up"))).toBeNull(); + expect(componentAvailabilityReason(undefined)).toBeNull(); + }); +}); + +describe("describePageSaveError", () => { + it("labels a rule rejection with the rule's reason", () => { + const reason = 'The "Hero" component isn\'t allowed in the "Main" zone of the "Landing Page" page model.'; + expect(describePageSaveError(sdkError(400, reason))).toBe(`Rejected by a component availability rule: ${reason}`); + }); + + it("keeps any other error's message unchanged", () => { + expect(describePageSaveError(sdkError(500, "boom", "Unable to create page. (HTTP 500): boom"))).toBe( + "Unable to create page. (HTTP 500): boom" + ); + expect(describePageSaveError("plain")).toBe("plain"); + }); +}); 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 d1af91bb..bddf6cad 100644 --- a/src/lib/pushers/page-pusher/tests/process-page.test.ts +++ b/src/lib/pushers/page-pusher/tests/process-page.test.ts @@ -454,6 +454,33 @@ describe("processPage — failure paths", () => { expect(result.error).toContain("network error"); }); + it("reports the reason when a component availability rule rejects the page (PROD-2914)", async () => { + const { TemplateMapper } = require("lib/mappers/template-mapper"); + TemplateMapper.mockImplementation(() => ({ + getTemplateMappingByPageTemplateName: jest.fn().mockReturnValue({ ref: "Main" }), + getMappedEntity: jest.fn().mockReturnValue({ contentSectionDefinitions: [] }), + })); + + const pageMapper = makePageMapper({ + hasSourceChanged: jest.fn().mockReturnValue(true), + }); + + const reason = 'The "Hero" component isn\'t allowed in the "Main" zone of the "Landing Page" page model.'; + const apiClient = makeApiClient(); + apiClient.pageMethods.savePage = jest + .fn() + .mockRejectedValue( + Object.assign(new Error(`Unable to create page. (HTTP 400): ${reason}`), { + statusCode: 400, + responseData: reason, + }) + ); + + const result = await processPage(makeProps({ apiClient, pageMapper })); + expect(result.status).toBe("failure"); + expect(result.error).toBe(`Rejected by a component availability rule: ${reason}`); + }); + it("returns failure with unexpected response format", async () => { const { TemplateMapper } = require("lib/mappers/template-mapper"); TemplateMapper.mockImplementation(() => ({ diff --git a/src/lib/pushers/push-operations-config.ts b/src/lib/pushers/push-operations-config.ts index c752e7b8..9aaa3732 100644 --- a/src/lib/pushers/push-operations-config.ts +++ b/src/lib/pushers/push-operations-config.ts @@ -91,6 +91,18 @@ export const PUSH_OPERATIONS: Record = { dataKey: "pages", dependencies: ["Templates", "Models", "Containers", "Content", "Galleries", "Assets"], // Pages require Templates, Models, and Containers }, + // PROD-2914: component availability rules (where a component can go). Runs after templates, + // since its rules name page models and zones; see orchestrate-pushers.ts for the order. + modelAvailability: { + name: "pushModelAvailability", + description: "Push component availability rules", + handler: async (sourceData) => { + const { pushModelAvailability } = await import("./model-availability-pusher"); + return await pushModelAvailability(sourceData["models"]); + }, + elements: ["Models"], + dataKey: "models", + }, urlRedirections: { name: "pushUrlRedirections", description: "Push URL redirections", diff --git a/src/lib/pushers/tests/model-availability-pusher.test.ts b/src/lib/pushers/tests/model-availability-pusher.test.ts new file mode 100644 index 00000000..211756fe --- /dev/null +++ b/src/lib/pushers/tests/model-availability-pusher.test.ts @@ -0,0 +1,322 @@ +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; +import { resetState, setState, initializeGuidLogger } from "core/state"; +import * as stateModule from "core/state"; +import { ModelMapper } from "lib/mappers/model-mapper"; +import { TemplateMapper } from "lib/mappers/template-mapper"; +import { SectionMapper } from "lib/mappers/section-mapper"; +import { preflightReport } from "../../preflight/preflight-report"; +import { pushModelAvailability } from "../model-availability-pusher"; + +const SOURCE = "src-avail-u"; +const TARGET = "tgt-avail-u"; + +let tmpDir: string; + +beforeAll(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "agility-availability-")); +}); + +afterAll(() => { + fs.rmSync(tmpDir, { recursive: true, force: true }); +}); + +beforeEach(() => { + resetState(); + setState({ rootPath: tmpDir, sourceGuid: SOURCE, targetGuid: TARGET, token: "test-token" }); + // Mappings and pulled rules would otherwise leak between tests (shared tmpDir, fixed guids). + fs.rmSync(path.join(tmpDir, "mappings"), { recursive: true, force: true }); + fs.rmSync(path.join(tmpDir, SOURCE), { recursive: true, force: true }); + fs.rmSync(path.join(tmpDir, TARGET), { recursive: true, force: true }); + preflightReport.reset(); + initializeGuidLogger(SOURCE, "push"); + jest.spyOn(console, "log").mockImplementation(() => {}); + jest.spyOn(console, "warn").mockImplementation(() => {}); + jest.spyOn(console, "error").mockImplementation(() => {}); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +// ─── helpers ────────────────────────────────────────────────────────────────── + +/** Write pulled rules for an instance; `null` leaves the folder out (rules unknown). */ +function writeRules(guid: string, rules: any[] | null) { + const folder = path.join(tmpDir, guid, "model-availability"); + fs.rmSync(folder, { recursive: true, force: true }); + if (rules === null) return; + fs.mkdirSync(folder, { recursive: true }); + for (const rule of rules) fs.writeFileSync(path.join(folder, `${rule.modelID}.json`), JSON.stringify(rule)); +} + +function mapModel(sourceID: number, targetID: number, referenceName = "HeroBanner") { + new ModelMapper(SOURCE, TARGET).addMapping( + { id: sourceID, referenceName, lastModifiedDate: "2026-01-01" } as any, + { id: targetID, referenceName, lastModifiedDate: "2026-01-01" } as any + ); +} + +function mapTemplateAndZone() { + new TemplateMapper(SOURCE, TARGET).addMapping( + { pageTemplateID: 3, pageTemplateName: "Landing Page" } as any, + { pageTemplateID: 30, pageTemplateName: "Landing Page" } as any + ); + new SectionMapper(SOURCE, TARGET).addMapping( + { pageItemTemplateID: 14, pageItemTemplateReferenceName: "HeroZone" } as any, + { pageItemTemplateID: 140, pageItemTemplateReferenceName: "HeroZone" } as any + ); +} + +const sourceModel = { id: 12, referenceName: "HeroBanner" } as any; + +const sourceAllowedIn = { + sitemaps: [{ sitemapID: 1, referenceName: "website", name: "Website" }], + pageModels: [{ pageModelID: 3, name: "Landing Page" }], + zones: [{ zoneID: 14, pageModelID: 3, pageModelName: "Landing Page", referenceName: "HeroZone", name: "Hero Zone" }], +}; + +function sourceRule(overrides: any = {}) { + return { + modelID: 12, + modelReferenceName: "HeroBanner", + addableBy: { permissions: ["Administrator"], users: [{ userID: 5, emailAddress: "a@example.com" }], teams: [] }, + allowedIn: sourceAllowedIn, + ...overrides, + }; +} + +function mockApi(save: jest.Mock = jest.fn().mockResolvedValue({})) { + jest.spyOn(stateModule, "getApiClient").mockReturnValue({ modelMethods: { saveModelAvailability: save } } as any); + return save; +} + +function httpError(statusCode: number, message: string) { + return Object.assign(new Error(message), { statusCode }); +} + +// ─── pushModelAvailability ──────────────────────────────────────────────────── + +describe("pushModelAvailability", () => { + it("sends allowedIn only, with addableBy null so the target keeps its own who rules", async () => { + writeRules(SOURCE, [sourceRule()]); + writeRules(TARGET, []); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(result).toMatchObject({ status: "success", successful: 1, failed: 0 }); + expect(save).toHaveBeenCalledTimes(1); + const [modelID, body, guid] = save.mock.calls[0]; + expect(modelID).toBe(120); + expect(guid).toBe(TARGET); + expect(body.modelID).toBe(120); + expect(body.addableBy).toBeNull(); + expect(body.allowedIn).toBeDefined(); + }); + + it("remaps by name and never sends a source ID", async () => { + writeRules(SOURCE, [sourceRule()]); + writeRules(TARGET, []); + mapModel(12, 120); + const save = mockApi(); + + await pushModelAvailability([sourceModel]); + + expect(save.mock.calls[0][1].allowedIn).toEqual({ + sitemaps: [{ referenceName: "website", name: "Website" }], + pageModels: [{ name: "Landing Page" }], + zones: [{ pageModelName: "Landing Page", referenceName: "HeroZone", name: "Hero Zone" }], + }); + }); + + it("uses the sync's template and zone mappings for target IDs when they exist", async () => { + writeRules(SOURCE, [sourceRule()]); + writeRules(TARGET, []); + mapModel(12, 120); + mapTemplateAndZone(); + const save = mockApi(); + + await pushModelAvailability([sourceModel]); + + const allowedIn = save.mock.calls[0][1].allowedIn; + expect(allowedIn.pageModels).toEqual([{ pageModelID: 30, name: "Landing Page" }]); + expect(allowedIn.zones).toEqual([ + { zoneID: 140, pageModelID: 30, pageModelName: "Landing Page", referenceName: "HeroZone", name: "Hero Zone" }, + ]); + // Sitemaps have no mapping: by referenceName only. + expect(allowedIn.sitemaps).toEqual([{ referenceName: "website", name: "Website" }]); + }); + + it("skips a model whose target already has the same placement rules", async () => { + writeRules(SOURCE, [sourceRule()]); + // Target has its own IDs and its own who rules; only the placement names are compared. + writeRules(TARGET, [ + { + modelID: 120, + addableBy: { permissions: ["Designer"], users: [], teams: [] }, + allowedIn: { + sitemaps: [{ sitemapID: 7, referenceName: "Website", name: "Website" }], + pageModels: [{ pageModelID: 30, name: "landing page" }], + zones: [{ zoneID: 140, pageModelID: 30, pageModelName: "Landing Page", referenceName: "HeroZone" }], + }, + }, + ]); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(save).not.toHaveBeenCalled(); + expect(result).toMatchObject({ successful: 0, skipped: 1, failed: 0 }); + }); + + it("clears the target's placement rules when the source has none", async () => { + writeRules(SOURCE, [sourceRule({ allowedIn: null })]); + writeRules(TARGET, [{ modelID: 120, addableBy: null, allowedIn: sourceAllowedIn }]); + mapModel(12, 120); + const save = mockApi(); + + await pushModelAvailability([sourceModel]); + + expect(save).toHaveBeenCalledTimes(1); + expect(save.mock.calls[0][1]).toEqual({ + modelID: 120, + addableBy: null, + allowedIn: { sitemaps: [], pageModels: [], zones: [] }, + }); + }); + + it("also clears when the source model has no rules file at all", async () => { + writeRules(SOURCE, []); + writeRules(TARGET, [{ modelID: 120, addableBy: null, allowedIn: sourceAllowedIn }]); + mapModel(12, 120); + const save = mockApi(); + + await pushModelAvailability([sourceModel]); + + expect(save.mock.calls[0][1].allowedIn).toEqual({ sitemaps: [], pageModels: [], zones: [] }); + }); + + it("never clears when the target's rules are unknown", async () => { + writeRules(SOURCE, []); + writeRules(TARGET, null); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(save).not.toHaveBeenCalled(); + expect(result).toMatchObject({ successful: 0, skipped: 0, failed: 0 }); + }); + + it("still pushes the source's rules when the target's are unknown", async () => { + writeRules(SOURCE, [sourceRule()]); + writeRules(TARGET, null); + mapModel(12, 120); + const save = mockApi(); + + await pushModelAvailability([sourceModel]); + + expect(save).toHaveBeenCalledTimes(1); + }); + + it("does nothing when the source's rules weren't pulled (an instance without the feature)", async () => { + writeRules(SOURCE, null); + writeRules(TARGET, [{ modelID: 120, addableBy: null, allowedIn: sourceAllowedIn }]); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(save).not.toHaveBeenCalled(); + expect(result).toMatchObject({ status: "success", successful: 0, skipped: 0, failed: 0 }); + }); + + it("leaves models with no rules on either side alone, and doesn't count them", async () => { + writeRules(SOURCE, []); + writeRules(TARGET, []); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(save).not.toHaveBeenCalled(); + expect(result).toMatchObject({ successful: 0, skipped: 0, failed: 0 }); + }); + + it("skips a model that has rules but no target mapping (the model itself wasn't synced)", async () => { + writeRules(SOURCE, [sourceRule()]); + writeRules(TARGET, []); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(save).not.toHaveBeenCalled(); + expect(result.skipped).toBe(1); + }); + + it("only touches models in the sync's scope", async () => { + writeRules(SOURCE, [sourceRule(), sourceRule({ modelID: 13, modelReferenceName: "Other" })]); + writeRules(TARGET, []); + mapModel(12, 120); + mapModel(13, 130, "Other"); + const save = mockApi(); + + await pushModelAvailability([sourceModel]); + + expect(save).toHaveBeenCalledTimes(1); + expect(save.mock.calls[0][0]).toBe(120); + }); + + it("reports a rejected save (e.g. an unresolved zone) as a model failure with the API's reason", async () => { + writeRules(SOURCE, [sourceRule()]); + writeRules(TARGET, []); + mapModel(12, 120); + mockApi(jest.fn().mockRejectedValue(httpError(400, 'Unknown zone "Landing Page" / "HeroZone".'))); + + const result = await pushModelAvailability([sourceModel]); + + expect(result).toMatchObject({ status: "error", successful: 0, failed: 1 }); + expect(result.failureDetails[0]).toMatchObject({ name: "HeroBanner", type: "model", guid: SOURCE }); + expect(result.failureDetails[0].error).toContain('Unknown zone "Landing Page" / "HeroZone".'); + }); + + it.each([404, 405, 409])( + "stops with one warning when the target answers %i (feature not available)", + async (status) => { + writeRules(SOURCE, [sourceRule(), sourceRule({ modelID: 13, modelReferenceName: "Other" })]); + writeRules(TARGET, []); + mapModel(12, 120); + mapModel(13, 130, "Other"); + const save = mockApi(jest.fn().mockRejectedValue(httpError(status, "not available"))); + + const result = await pushModelAvailability([sourceModel, { id: 13, referenceName: "Other" } as any]); + + expect(save).toHaveBeenCalledTimes(1); + expect(result).toMatchObject({ status: "success", failed: 0 }); + expect(result.warningDetails).toHaveLength(1); + expect(result.warningDetails[0].error).toMatch(/doesn't support component availability rules yet/); + } + ); + + it("records the planned saves in preflight and writes nothing", async () => { + writeRules(SOURCE, [sourceRule(), sourceRule({ modelID: 13, modelReferenceName: "NewModel" })]); + writeRules(TARGET, []); + mapModel(12, 120); // before preflight is on: preflight never writes mapping files + setState({ preflight: true }); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel, { id: 13, referenceName: "NewModel" } as any]); + + expect(save).not.toHaveBeenCalled(); + expect(result.successful).toBe(2); + const entries = preflightReport.getEntries(); + expect(entries).toEqual([ + expect.objectContaining({ phase: "Component availability", action: "update", name: "HeroBanner" }), + expect.objectContaining({ phase: "Component availability", action: "create", name: "NewModel" }), + ]); + }); +}); diff --git a/src/lib/pushers/tests/model-pusher.test.ts b/src/lib/pushers/tests/model-pusher.test.ts index b9cfbc84..e47480b0 100644 --- a/src/lib/pushers/tests/model-pusher.test.ts +++ b/src/lib/pushers/tests/model-pusher.test.ts @@ -983,3 +983,72 @@ describe("pushModels — effective target model registry (PROD-2542)", () => { expect(stateModule.getSavedTargetBySourceReferenceName("model", "Planned")).toBe(source); }); }); + +// ─── PROD-2914: isVisible (a disabled component) belongs to the target ─────── + +describe("pushModels — isVisible ownership (PROD-2914)", () => { + const JAN = new Date(2025, 0, 1).toISOString(); + const JUN = new Date(2025, 5, 1).toISOString(); + const fields = (label = "Title") => [{ name: "Title", label, type: "Text", itemOrder: 0, settings: {} }]; + + it("copies the source's isVisible when it creates a model", async () => { + const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 777 })); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); + const { pushModels } = await import("../model-pusher"); + + await pushModels([makeModel({ referenceName: "hidden-component", isVisible: false })], []); + + // The stub carries it; the field update that follows leaves it out (null = keep on the API). + expect(saveModel.mock.calls[0][0].isVisible).toBe(false); + expect("isVisible" in saveModel.mock.calls[1][0]).toBe(false); + }); + + it("never sends isVisible when it updates an existing model, so the target keeps its own", async () => { + const { ModelMapper } = await import("lib/mappers/model-mapper"); + new ModelMapper(state.sourceGuid, state.targetGuid).addMapping( + { id: 1, referenceName: "Hero", lastModifiedDate: JAN } as any, + { id: 10, referenceName: "Hero", lastModifiedDate: JAN } as any + ); + const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 10, referenceName: "Hero", lastModifiedDate: JUN })); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); + const { pushModels } = await import("../model-pusher"); + + const source = makeModel({ id: 1, referenceName: "Hero", lastModifiedDate: JUN, fields: fields("Headline"), isVisible: true }); + const target = makeModel({ id: 10, referenceName: "Hero", lastModifiedDate: JAN, fields: fields(), isVisible: false }); + const result = await pushModels([source], [target]); + + expect(result.successful).toBe(1); + expect(saveModel).toHaveBeenCalledTimes(1); + expect(saveModel.mock.calls[0][0].id).toBe(10); + expect("isVisible" in saveModel.mock.calls[0][0]).toBe(false); + }); + + it("doesn't treat an isVisible difference alone as a change to push", async () => { + const { ModelMapper } = await import("lib/mappers/model-mapper"); + new ModelMapper(state.sourceGuid, state.targetGuid).addMapping( + { id: 1, referenceName: "Hero", lastModifiedDate: JAN } as any, + { id: 10, referenceName: "Hero", lastModifiedDate: JAN } as any + ); + const saveModel = jest.fn(); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); + const { pushModels } = await import("../model-pusher"); + + const source = makeModel({ id: 1, referenceName: "Hero", displayName: "Hero", lastModifiedDate: JUN, fields: fields(), isVisible: true }); + const target = makeModel({ id: 10, referenceName: "Hero", displayName: "Hero", lastModifiedDate: JAN, fields: fields(), isVisible: false }); + const result = await pushModels([source], [target]); + + expect(saveModel).not.toHaveBeenCalled(); + expect(result.skipped).toBe(1); + }); + + it("works unchanged with models pulled from an API that doesn't return isVisible", async () => { + const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 778 })); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); + const { pushModels } = await import("../model-pusher"); + + const result = await pushModels([makeModel({ referenceName: "older-api-model" })], []); + + expect(result.successful).toBe(1); + expect("isVisible" in saveModel.mock.calls[0][0]).toBe(false); + }); +}); diff --git a/src/lib/pushers/tests/orchestrate-pushers.test.ts b/src/lib/pushers/tests/orchestrate-pushers.test.ts index 50d9dccc..3c33ab89 100644 --- a/src/lib/pushers/tests/orchestrate-pushers.test.ts +++ b/src/lib/pushers/tests/orchestrate-pushers.test.ts @@ -331,6 +331,8 @@ describe("Pushers.instanceOrchestrator — models-first ordering (PROD-2202)", ( "pushAssets", "pushContainers", "pushTemplates", + // PROD-2914: availability rules name page models and zones, so they follow templates. + "pushModelAvailability", "pushContent", "pushPages", ]); diff --git a/src/lib/pushers/tests/push-operations-config.test.ts b/src/lib/pushers/tests/push-operations-config.test.ts index 0b59d177..c4e6caad 100644 --- a/src/lib/pushers/tests/push-operations-config.test.ts +++ b/src/lib/pushers/tests/push-operations-config.test.ts @@ -25,21 +25,29 @@ describe("PUSH_OPERATIONS registry", () => { expect(keys).toContain("templates"); expect(keys).toContain("pages"); expect(keys).toContain("urlRedirections"); + expect(keys).toContain("modelAvailability"); + }); + + it.each([ + "galleries", + "assets", + "models", + "containers", + "content", + "templates", + "pages", + "urlRedirections", + "modelAvailability", + ])("%s operation has required fields", (key) => { + const op = PUSH_OPERATIONS[key]; + expect(op.name).toBeTruthy(); + expect(op.description).toBeTruthy(); + expect(typeof op.handler).toBe("function"); + expect(Array.isArray(op.elements)).toBe(true); + expect(op.elements.length).toBeGreaterThan(0); + expect(typeof op.dataKey).toBe("string"); }); - it.each(["galleries", "assets", "models", "containers", "content", "templates", "pages", "urlRedirections"])( - "%s operation has required fields", - (key) => { - const op = PUSH_OPERATIONS[key]; - expect(op.name).toBeTruthy(); - expect(op.description).toBeTruthy(); - expect(typeof op.handler).toBe("function"); - expect(Array.isArray(op.elements)).toBe(true); - expect(op.elements.length).toBeGreaterThan(0); - expect(typeof op.dataKey).toBe("string"); - } - ); - it("galleries operation targets Galleries element", () => { expect(PUSH_OPERATIONS.galleries.elements).toContain("Galleries"); expect(PUSH_OPERATIONS.galleries.dataKey).toBe("galleries"); @@ -75,6 +83,11 @@ describe("PUSH_OPERATIONS registry", () => { expect(PUSH_OPERATIONS.pages.dataKey).toBe("pages"); }); + it("modelAvailability operation runs with the Models element (PROD-2914)", () => { + expect(PUSH_OPERATIONS.modelAvailability.elements).toEqual(["Models"]); + expect(PUSH_OPERATIONS.modelAvailability.dataKey).toBe("models"); + }); + it("urlRedirections operation targets UrlRedirections element", () => { expect(PUSH_OPERATIONS.urlRedirections.elements).toContain("UrlRedirections"); expect(PUSH_OPERATIONS.urlRedirections.dataKey).toBe("urlRedirections"); @@ -84,9 +97,10 @@ describe("PUSH_OPERATIONS registry", () => { // ─── PushOperationsRegistry.getAllOperations ────────────────────────────────── describe("PushOperationsRegistry.getAllOperations", () => { - it("returns 8 operations total", () => { + it("returns 9 operations total", () => { const ops = PushOperationsRegistry.getAllOperations(); - expect(ops).toHaveLength(8); + // PROD-2914 added modelAvailability. + expect(ops).toHaveLength(9); }); it("each operation has a handler function", () => { diff --git a/src/tests/sync/full-instance.test.ts b/src/tests/sync/full-instance.test.ts index a545b49e..cd922e12 100644 --- a/src/tests/sync/full-instance.test.ts +++ b/src/tests/sync/full-instance.test.ts @@ -1,9 +1,9 @@ /** - * All eight push phases, driven end to end, twice. + * All nine push phases, driven end to end, twice. * * `full-sync.test.ts` covers models and containers in depth. This suite covers *breadth*: * every entity type the orchestrator pushes — models, galleries, assets, containers, URL - * redirections, templates, content and pages — created on a fresh sync and then proven inert + * redirections, templates, component availability rules, content and pages — created on a fresh sync and then proven inert * on a re-sync with no source change. * * Assertions pin **positive** counts, never just the absence of writes. A phase reporting @@ -30,7 +30,7 @@ jest.mock("lib/pushers/url-redirection-api", () => ({ saveUrlRedirections: jest.fn(), })); -describe("full instance — all eight phases", () => { +describe("full instance — all nine phases", () => { let harness: SyncHarness; let net: InstalledNetworkStubs; @@ -54,7 +54,7 @@ describe("full instance — all eight phases", () => { // ─── A. Fresh sync ───────────────────────────────────────────────────────── describe("A. fresh sync into an empty target", () => { - it("runs every one of the eight phases", async () => { + it("runs every one of the nine phases", async () => { harness = makeHarness(); const run = await harness.run(); @@ -93,6 +93,33 @@ describe("full instance — all eight phases", () => { expect(target.templates.size).toBe(1); expect(target.content.size).toBe(1); expect(target.pages.size).toBe(1); + expect(target.availability.size).toBe(1); + }); + + it("sends the placement rules remapped onto the target, and never the who rules (PROD-2914)", async () => { + harness = makeHarness(); + + await harness.run(); + const [save] = harness.fake.callsTo("modelMethods.saveModelAvailability"); + const targetModelID = harness.readMappings("models")![0].targetID; + const targetTemplate = Array.from(harness.fake.instance(harness.targetGuid).templates.values())[0]; + const targetZoneID = targetTemplate.contentSectionDefinitions[0].pageItemTemplateID; + + expect(save.payload.modelID).toBe(targetModelID); + expect(save.payload.addableBy).toBeNull(); + expect(save.payload.allowedIn).toEqual({ + sitemaps: [{ referenceName: "website", name: "Website" }], + pageModels: [], + zones: [ + { + zoneID: targetZoneID, + pageModelID: targetTemplate.pageTemplateID, + pageModelName: "Standard", + referenceName: "Main", + name: "Main", + }, + ], + }); }); it("writes a mapping for every entity type", async () => { diff --git a/src/tests/sync/helpers/fake-instance.ts b/src/tests/sync/helpers/fake-instance.ts index 4c89476d..a3684fc7 100644 --- a/src/tests/sync/helpers/fake-instance.ts +++ b/src/tests/sync/helpers/fake-instance.ts @@ -66,6 +66,8 @@ interface InstanceStore { content: Map; pages: Map; urlRedirections: Map; + /** PROD-2914: component availability rules, keyed by model ID. Only models that have any. */ + availability: Map; sitemap: any[]; locales: string[]; } @@ -80,6 +82,7 @@ function emptyStore(locales: string[]): InstanceStore { content: new Map(), pages: new Map(), urlRedirections: new Map(), + availability: new Map(), sitemap: [], locales, }; @@ -199,6 +202,13 @@ export class FakeInstance { writeJsonFile(path.join(base, locale, "page", `${page.pageID}.json`), page); }); + // PROD-2914: the folder is written even when empty. A pull only creates it when the instance + // answered, and an existing folder is how the push knows "no rules" from "unknown". + const availabilityDir = path.join(base, "model-availability"); + fs.rmSync(availabilityDir, { recursive: true, force: true }); + fs.mkdirSync(availabilityDir, { recursive: true }); + writeEach(availabilityDir, Array.from(store.availability.values()), (a) => String(a.modelID)); + // Redirections live under the LOCALE, not the guid root. The set is instance-wide, so the // same file is written for every locale the instance reports — which is what a pull does. if (store.urlRedirections.size > 0) { @@ -240,6 +250,31 @@ export class FakeInstance { this.record("modelMethods.getPageModules", guid, { includeDefaults }); return []; }, + getAllModelAvailability: async (guid: string) => { + this.record("modelMethods.getAllModelAvailability", guid); + return Array.from(this.instance(guid).availability.values()); + }, + // The API's per-group semantics: null/absent keeps the stored group, an object whose lists + // are all empty clears it, any other object replaces it. Targets are stored as sent; the + // real API resolves names to IDs, which no test here depends on. + saveModelAvailability: async (modelID: number, availability: any, guid: string) => { + this.record("modelMethods.saveModelAvailability", guid, { modelID, ...availability }); + const store = this.instance(guid); + const stored = store.availability.get(modelID) ?? { modelID, addableBy: null, allowedIn: null }; + const merge = (current: any, incoming: any) => { + if (incoming === undefined || incoming === null) return current; + const empty = Object.values(incoming).every((list: any) => !Array.isArray(list) || list.length === 0); + return empty ? null : incoming; + }; + const saved = { + ...stored, + addableBy: merge(stored.addableBy, availability?.addableBy), + allowedIn: merge(stored.allowedIn, availability?.allowedIn), + }; + if (saved.addableBy === null && saved.allowedIn === null) store.availability.delete(modelID); + else store.availability.set(modelID, saved); + return saved; + }, }; containerMethods = { diff --git a/src/tests/sync/helpers/full-instance-fixture.ts b/src/tests/sync/helpers/full-instance-fixture.ts index 1074d9c0..ae60edcd 100644 --- a/src/tests/sync/helpers/full-instance-fixture.ts +++ b/src/tests/sync/helpers/full-instance-fixture.ts @@ -1,7 +1,7 @@ import { SyncFixture } from "./run-sync"; /** - * One small instance that exercises **all eight push phases**. + * One small instance that exercises **all nine push phases**. * * Hand-authored minimal JSON on purpose. A scrubbed capture from a real pull must never be * committed here: this repo is public and git history is permanent, and scrubbing is easy to @@ -118,6 +118,22 @@ export const SITEMAP_WEBSITE = [ }, ]; +/** + * PROD-2914: placement rules for the Post model. (Real rules sit on component models; the CLI + * doesn't distinguish model kinds, and a second model would change every model count here.) The + * who rules are there to prove they never travel. + */ +export const AVAILABILITY_POST = { + modelID: 10, + modelReferenceName: "Post", + addableBy: { permissions: ["Administrator"], users: [], teams: [] }, + allowedIn: { + sitemaps: [{ sitemapID: 1, referenceName: "website", name: "Website" }], + pageModels: [], + zones: [{ zoneID: 71, pageModelID: 700, pageModelName: "Standard", referenceName: "Main", name: "Main" }], + }, +}; + export const REDIRECTIONS = { items: [{ id: 1, originUrl: "/old", destinationUrl: "/new", statusCode: 301 }], lastAccessDate: "2025-01-01T00:00:00.000", @@ -135,6 +151,7 @@ export function fullInstanceFixture(): SyncFixture { "galleries/300.json": { ...GALLERY_HERO }, "assets/json/1.json": { assetMedias: [{ ...ASSET_HERO }] }, "templates/700.json": { ...TEMPLATE_STANDARD }, + "model-availability/10.json": { ...AVAILABILITY_POST }, [`${FIXTURE_LOCALE}/item/500.json`]: { ...CONTENT_HELLO }, [`${FIXTURE_LOCALE}/page/800.json`]: { ...PAGE_HOME }, [`${FIXTURE_LOCALE}/nestedsitemap/website.json`]: SITEMAP_WEBSITE, @@ -144,7 +161,7 @@ export function fullInstanceFixture(): SyncFixture { }; } -/** The eight push operations, in the order the orchestrator runs them. */ +/** The nine push operations, in the order the orchestrator runs them. */ export const ALL_OPERATIONS = [ "pushModels", "pushGalleries", @@ -152,6 +169,7 @@ export const ALL_OPERATIONS = [ "pushContainers", "pushUrlRedirections", "pushTemplates", + "pushModelAvailability", "pushContent", "pushPages", ] as const; From 3ded77039d3dd51662ba91f62b8ae741f5dc88f1 Mon Sep 17 00:00:00 2001 From: Joel Varty Date: Thu, 22 Oct 2026 13:32:09 -0400 Subject: [PATCH 2/3] PROD-2914: sync component availability after pages; never clear from an empty source (review) - Order: the availability phase now runs last, after the pages of every locale. The API checks each page's new placements against the rules, and on a new target every placement is new, so rules (or a disabled model) pushed before the pages rejected pages that are valid on the source. A created model is now created enabled; a model disabled on the source is disabled at the end of the run (GET, then save with isVisible false), and the model mapping records that save so the next sync doesn't see a target change. - Clearing: a database without the feature reads as "no rules" (200 []), not 409, so an empty source can't be told from an unmigrated one. A model with no placement rules now clears the target's only when the source listed at least one rule. When the target's rules couldn't be read, the rules aren't pushed, with one warning. - Save errors: 405/409, or a 404 with no message (an API without the route), stop the phase as unsupported. A 404 with the API's message is that model's failure. A 403 is reported once and stops the phase. - Pull: a non-array answer (an older API serving model/{referenceName} for a model named "availability") is treated as no feature, and a failed write removes the half-written folder. - Tests: the fake only takes rules for component models and rejects new placements of disabled or disallowed components on a page save; it also allocates zone IDs. The full-instance fixture no longer puts rules on its content model, and a new component-availability suite covers a fresh sync, a re-sync and a later rejected page. Co-Authored-By: Claude Opus 5.5 --- README.md | 19 +- src/core/push.ts | 3 + src/core/state.ts | 30 +++ .../download-model-availability.ts | 54 ++-- .../tests/download-model-availability.test.ts | 22 ++ src/lib/pushers/model-availability-pusher.ts | 231 +++++++++++++++--- src/lib/pushers/model-pusher.ts | 23 +- src/lib/pushers/orchestrate-pushers.ts | 46 +++- src/lib/pushers/push-operations-config.ts | 6 +- .../tests/model-availability-pusher.test.ts | 155 +++++++++++- src/lib/pushers/tests/model-pusher.test.ts | 33 ++- .../pushers/tests/orchestrate-pushers.test.ts | 20 +- src/tests/sync/component-availability.test.ts | 216 ++++++++++++++++ src/tests/sync/full-instance.test.ts | 47 ++-- src/tests/sync/helpers/fake-instance.ts | 71 +++++- .../sync/helpers/full-instance-fixture.ts | 26 +- 16 files changed, 861 insertions(+), 141 deletions(-) create mode 100644 src/tests/sync/component-availability.test.ts diff --git a/README.md b/README.md index 402fcf66..ef717539 100644 --- a/README.md +++ b/README.md @@ -264,15 +264,22 @@ like this: - **Pull** saves each model's rules to `model-availability/{modelID}.json`. An instance that doesn't support the rules yet is pulled without them. -- **Sync** sends the **where** rules with the models, after templates, so the page models and - zones they name exist on the target. They're matched on the target by name: sitemaps by - reference name, page models by name, and zones by page model name plus zone reference name. - A source model with no where rules clears the target's. +- **Sync** sends the **where** rules with the models, as the last step, after the pages of + every locale. The target checks each page's new placements against its rules, and on a new + target every placement is new, so pages that are valid on the source are synced before the + rules arrive. The rules are matched on the target by name: sitemaps by reference name, page + models by name, and zones by page model name plus zone reference name (and by ID when the + sync has already mapped that page model or zone). +- A source model with no where rules clears the target's, but only when the source has at least + one rule on some model. An instance that hasn't been upgraded for the rules yet reads as "no + rules", so a source with none at all never clears anything. +- If the target's rules can't be read, the where rules aren't synced, and the sync says so. - **Who** rules and **disabled** belong to each instance and are never overwritten. A model the - sync creates starts disabled if it's disabled on the source. + sync creates is disabled at the end of the sync if it's disabled on the source. - A page the target rejects because of a rule is reported with the reason, for example `Rejected by a component availability rule: The "Hero" component isn't allowed in the "Main" - zone of the "Landing Page" page model.` + zone of the "Landing Page" page model.` Once the rules are on the target, a later sync that + adds a placement the rules don't allow is rejected this way. --- diff --git a/src/core/push.ts b/src/core/push.ts index 038b0cea..9f885079 100644 --- a/src/core/push.ts +++ b/src/core/push.ts @@ -6,6 +6,7 @@ import { clearFailedContentRegistry, clearHeldBackTemplateRegistry, clearSavedTargets, + clearDeferredModelDisables, clearOtherLocaleStagingRegistry, getOtherLocaleStaging, getPageCmsLink, @@ -105,6 +106,8 @@ export class Push { clearHeldBackTemplateRegistry(); // Entities saved by a previous run must not stand in for this run's pulled targets clearSavedTargets(); + // PROD-2914: models a previous run meant to disable after its pages + clearDeferredModelDisables(); clearOtherLocaleStagingRegistry(); // Preflight (PROD-2203): start each run with a clean report of planned actions diff --git a/src/core/state.ts b/src/core/state.ts index 03055355..b722f4dc 100644 --- a/src/core/state.ts +++ b/src/core/state.ts @@ -122,6 +122,11 @@ export interface State { // was not recreated (no --overwrite). Used by page pusher to skip pages that depend on them. heldBackTemplateRegistry: Map; + // Deferred model disables (PROD-2914) - source IDs of models created this run whose source is + // disabled (`isVisible: false`). They're created enabled and disabled after the pages, so the + // run's own page saves can place them (on a new target every placement counts as new). + deferredModelDisables: Set; + // Other-locale staging registry (PROD-2782) - pages that a save in one locale put in staging in // another locale, keyed by that other locale, then by target page ID. Auto-publish republishes // them; without auto-publish they are reported as warnings. @@ -215,6 +220,9 @@ export const state: State = { // Held-back template registry - templates whose mapped target is gone and was not recreated heldBackTemplateRegistry: new Map(), + // Deferred model disables - models created this run, to disable after the pages (PROD-2914) + deferredModelDisables: new Set(), + // Other-locale staging registry - pages a save in one locale put in staging in another (PROD-2782) otherLocaleStagingRegistry: new Map(), otherLocaleStagingBlocked: new Set(), @@ -522,6 +530,7 @@ export function resetState() { // Run-scoped saved target entities state.savedTargets = new Map(); + state.deferredModelDisables = new Set(); } /** @@ -881,6 +890,27 @@ export function clearSavedTargets(): void { state.savedTargets.clear(); } +/** + * PROD-2914: register a model created this run whose source is disabled (`isVisible: false`). The + * model pusher creates it enabled, and the component availability phase disables it after the + * pages, so the run's own page saves aren't rejected for placing a disabled component. + * @param sourceModelID - The source model ID + */ +export function registerDeferredModelDisable(sourceModelID: number): void { + if (sourceModelID == null) return; + state.deferredModelDisables.add(sourceModelID); +} + +/** Source IDs of the models registered by registerDeferredModelDisable this run. */ +export function getDeferredModelDisables(): number[] { + return Array.from(state.deferredModelDisables); +} + +/** Clear the deferred model disables (called at start of each sync). */ +export function clearDeferredModelDisables(): void { + state.deferredModelDisables.clear(); +} + /** * Register a source template that was held back because its mapped target template no longer * exists and --overwrite was not set diff --git a/src/lib/downloaders/download-model-availability.ts b/src/lib/downloaders/download-model-availability.ts index d9da4e36..5b87cec0 100644 --- a/src/lib/downloaders/download-model-availability.ts +++ b/src/lib/downloaders/download-model-availability.ts @@ -11,15 +11,19 @@ import { getApiClient, getLoggerForGuid } from "core/state"; * folder is read back as a model. * * The folder's presence is meaningful: it exists (possibly empty) only when the instance answered, - * so "no file for a model" means "no rules". When the instance can't answer (an API or database - * without the feature yet), the folder is removed, and the push treats that side as unknown and - * leaves its rules alone. + * so "no file for a model" means "no rules". When the instance can't answer (an API without the + * feature yet), the folder is removed, and the push treats that side as unknown. + * + * ⚠️ An instance whose DATABASE doesn't have the feature yet does answer: with an empty list, the + * same as an instance with no rules. So an empty folder doesn't prove the instance stores rules; + * the push allows for that. */ export const MODEL_AVAILABILITY_FOLDER = "model-availability"; /** - * Status codes that mean "this instance doesn't support availability rules yet": 404/405 from an - * API without the route, 409 from an API whose instance database hasn't been migrated. + * Status codes on a read that mean "this instance doesn't support availability rules yet": 404/405 + * from an API without the route. (A read on a database without the feature answers "no rules", not + * 409; a 409 is tolerated anyway, as only the save is specified to return it.) */ export function isAvailabilityUnsupported(error: any): boolean { const status = error?.statusCode ?? error?.response?.status; @@ -56,22 +60,38 @@ export async function downloadModelAvailability(guid: string): Promise { throw error; } - fs.mkdirSync(folderPath, { recursive: true }); + // An older API serves `model/availability` from its `model/{referenceName}` route: an instance + // with a model named "availability" answers with that model, not a list. That's an API without + // the feature, too. + if (!Array.isArray(rules)) { + removeFolder(); + logger?.info(`Component availability rules aren't available on ${guid} yet; skipping them.`); + return; + } const pulledIds = new Set(); - for (const rule of rules) { - if (rule?.modelID === undefined || rule?.modelID === null) continue; - const fileName = rule.modelID.toString(); - pulledIds.add(fileName); - fileOps.exportFiles(MODEL_AVAILABILITY_FOLDER, fileName, rule); - } + try { + fs.mkdirSync(folderPath, { recursive: true }); - // A model that has lost all its rules is no longer listed: drop its file. - for (const file of fs.readdirSync(folderPath)) { - if (!file.endsWith(".json")) continue; - if (!pulledIds.has(file.replace(".json", ""))) { - fs.unlinkSync(path.join(folderPath, file)); + for (const rule of rules) { + if (rule?.modelID === undefined || rule?.modelID === null) continue; + const fileName = rule.modelID.toString(); + pulledIds.add(fileName); + fileOps.exportFiles(MODEL_AVAILABILITY_FOLDER, fileName, rule); } + + // A model that has lost all its rules is no longer listed: drop its file. + for (const file of fs.readdirSync(folderPath)) { + if (!file.endsWith(".json")) continue; + if (!pulledIds.has(file.replace(".json", ""))) { + fs.unlinkSync(path.join(folderPath, file)); + } + } + } catch (error: any) { + // A half-written folder would read as the instance's current rules: remove it all. + removeFolder(); + logger?.error(`Error writing component availability rules: ${error?.message || error}`); + throw error; } if (pulledIds.size > 0) { diff --git a/src/lib/downloaders/tests/download-model-availability.test.ts b/src/lib/downloaders/tests/download-model-availability.test.ts index a60aa924..e03d8858 100644 --- a/src/lib/downloaders/tests/download-model-availability.test.ts +++ b/src/lib/downloaders/tests/download-model-availability.test.ts @@ -103,6 +103,28 @@ describe("downloadModelAvailability", () => { expect(fs.existsSync(folder)).toBe(false); }); + it("treats a single model instead of a list (an older API's model/{referenceName} route) as no feature", async () => { + seedStaleFile(12); + mockApi(jest.fn().mockResolvedValue({ id: 5, referenceName: "availability", fields: [] })); + + await expect(downloadModelAvailability(GUID)).resolves.toBeUndefined(); + + expect(fs.existsSync(folder)).toBe(false); + expect(getModelAvailabilityFromFileSystem(new fileOperations(GUID))).toBeNull(); + }); + + it("removes a half-written folder and rethrows when writing the rules fails", async () => { + seedStaleFile(99); + mockApi(jest.fn().mockResolvedValue([rule(12)])); + jest.spyOn(fileOperations.prototype, "exportFiles").mockImplementation(() => { + throw new Error("disk full"); + }); + + await expect(downloadModelAvailability(GUID)).rejects.toThrow("disk full"); + + expect(fs.existsSync(folder)).toBe(false); + }); + it("removes stale rules and rethrows any other error", async () => { seedStaleFile(12); mockApi(jest.fn().mockRejectedValue(httpError(500))); diff --git a/src/lib/pushers/model-availability-pusher.ts b/src/lib/pushers/model-availability-pusher.ts index 7d8d9027..34eb00df 100644 --- a/src/lib/pushers/model-availability-pusher.ts +++ b/src/lib/pushers/model-availability-pusher.ts @@ -1,28 +1,34 @@ import * as mgmtApi from "@agility/management-sdk"; import ansiColors from "ansi-colors"; import { fileOperations } from "../../core/fileOperations"; -import { getApiClient, getLoggerForGuid, state } from "../../core/state"; +import { getApiClient, getDeferredModelDisables, getLoggerForGuid, state } from "../../core/state"; import { FailureDetail, PusherResult } from "../../types/sourceData"; import { ModelMapper } from "lib/mappers/model-mapper"; import { TemplateMapper } from "lib/mappers/template-mapper"; import { SectionMapper } from "lib/mappers/section-mapper"; import { getModelAvailabilityFromFileSystem } from "../getters/filesystem/get-model-availability"; -import { isAvailabilityUnsupported } from "../downloaders/download-model-availability"; import { preflightReport } from "../preflight/preflight-report"; /** - * PROD-2914: push component availability rules ("where it can go") from the source to the target. + * PROD-2914: push component availability from the source to the target. * * Ownership, per the component availability design: - * - `allowedIn` (sitemaps, page models, zones) is modelling, so it travels with the model. The - * target ends up with the source's rules, including having them cleared when the source has none. + * - `allowedIn` (sitemaps, page models, zones) is modelling, so it travels with the model. * - `addableBy` (who can add) belongs to the instance: it's always sent as `null`, so the API keeps - * the target's own rules. (Disabling a model is `isVisible`, handled by the model pusher.) + * the target's own rules. + * - `isVisible` (disabled) belongs to the instance too: the model pusher never sends it on an + * update. A model created this run takes the source's value, and this phase applies it. * - * Runs after templates, so the page models and zones it names exist on the target. Targets are - * remapped for the target instance: page models and zones by their ID mapping when there is one - * (and by name otherwise), sitemaps by `referenceName`. Source IDs are never sent, because the API - * resolves an ID before a name and a source ID can belong to a different object on the target. + * This phase runs LAST, after the pages of every locale. On a new target every placement on a page + * counts as new, and the API checks new placements against the rules, so rules (or a disabled + * model) in place before the pages would reject pages that are valid on the source, where those + * placements are older than the rules. Run last, this run's pages land first; later runs only add + * placements the target doesn't have yet, and those are checked like any other edit. + * + * Targets are remapped for the target instance: page models and zones by their ID mapping when + * there is one (and by name otherwise), sitemaps by `referenceName`. Source IDs are never sent, + * because the API resolves an ID before a name and a source ID can belong to a different object on + * the target. */ const EMPTY_ALLOWED_IN: mgmtApi.ModelAvailabilityAllowedIn = { sitemaps: [], pageModels: [], zones: [] }; @@ -87,35 +93,157 @@ export function remapAllowedIn( }; } -export async function pushModelAvailability(sourceModels: mgmtApi.Model[]): Promise { +/** The server's own message in an error response, if it sent one (an unmatched route sends none). */ +function hasServerMessage(error: any): boolean { + const data = error?.responseData ?? error?.response?.data; + if (data === undefined || data === null) return false; + if (typeof data === "string") return data.trim().length > 0; + return Boolean(data.message ?? data.Message ?? data.exceptionMessage ?? data.title); +} + +/** + * How a failed availability save is handled: + * - "unsupported": the target can't store rules (405, 409 from a database without the feature, or + * a 404 with no message, which is an API without the route). Every other save would fail too. + * - "forbidden": a 403. The permission is instance-wide, so every other save would fail too. + * - "model": this model's own failure (a 400 such as an unresolved zone, or a 404 with the API's + * message, such as a model mapping that points at a model the target no longer has). + */ +function classifySaveError(error: any): "unsupported" | "forbidden" | "model" { + const status = error?.statusCode ?? error?.response?.status; + if (status === 405 || status === 409) return "unsupported"; + if (status === 404) return hasServerMessage(error) ? "model" : "unsupported"; + if (status === 403) return "forbidden"; + return "model"; +} + +function errorText(error: any): string { + return error?.message ? String(error.message) : String(error); +} + +interface PhaseCounts { + successful: number; + failed: number; + skipped: number; + failureDetails: FailureDetail[]; + warningDetails: FailureDetail[]; +} + +function emptyCounts(): PhaseCounts { + return { successful: 0, failed: 0, skipped: 0, failureDetails: [], warningDetails: [] }; +} + +/** + * Disable the models this run created whose source is disabled (`isVisible: false`). The model + * pusher created them enabled so this run's pages could place them. + */ +async function disableModelsCreatedThisRun( + sourceModels: mgmtApi.Model[], + modelMapper: ModelMapper +): Promise { + const { sourceGuid, targetGuid } = state; + const logger = getLoggerForGuid(sourceGuid); + const counts = emptyCounts(); + const deferred = new Set(getDeferredModelDisables()); + if (deferred.size === 0) return counts; + const apiClient = getApiClient(); + + for (const sourceModel of sourceModels || []) { + if (!sourceModel?.id || !deferred.has(sourceModel.id)) continue; + + if (state.preflight) { + preflightReport.record({ + phase: "Component availability", + action: "update", + name: sourceModel.referenceName, + detail: "disable the new model once the pages are synced (it's disabled on the source)", + }); + counts.successful++; + continue; + } + + const mapping = modelMapper.getModelMappingByID(sourceModel.id, "source"); + if (!mapping?.targetID) { + counts.skipped++; + continue; + } + + try { + const target = await apiClient.modelMethods.getContentModel(mapping.targetID, targetGuid); + if (!target) throw new Error(`The model with ID ${mapping.targetID} wasn't found on the target.`); + if (target.isVisible === undefined) { + // An API that doesn't return isVisible can't store it either (it saves every model enabled). + const message = + "The target instance's API can't disable models yet, so models that are disabled on the source were left enabled."; + console.log(ansiColors.yellow(`⚠️ ${message}`)); + logger?.warning(message); + counts.warningDetails.push({ name: "Component availability", error: message, type: "model", guid: sourceGuid }); + break; + } + const saved = await apiClient.modelMethods.saveModel({ ...target, isVisible: false }, targetGuid); + // The save moves the target's modified date. Record it, or the next sync reads the model as + // changed on the target and holds it back as a conflict. + modelMapper.addMapping(sourceModel, saved?.lastModifiedDate ? saved : { ...target, ...(saved || {}) }); + logger?.info(`Model "${sourceModel.referenceName}" disabled, as it is on the source.`); + counts.successful++; + } catch (error: any) { + const reason = errorText(error); + logger?.error(`Disabling model "${sourceModel.referenceName}" failed: ${reason}`); + counts.failed++; + counts.failureDetails.push({ + name: sourceModel.referenceName, + type: "model", + error: `Failed to disable model "${sourceModel.referenceName}" (target ID: ${mapping.targetID}), which is disabled on the source: ${reason}`, + guid: sourceGuid, + }); + } + } + + return counts; +} + +/** Push the source's placement rules (`allowedIn`) for the models in this sync's scope. */ +async function pushPlacementRules(sourceModels: mgmtApi.Model[], modelMapper: ModelMapper): Promise { const { sourceGuid, targetGuid } = state; const logger = getLoggerForGuid(sourceGuid); + const counts = emptyCounts(); const sourceRules = getModelAvailabilityFromFileSystem(new fileOperations(sourceGuid)); if (sourceRules === null) { - // The source didn't answer (an API or database without the feature): leave the target alone. + // The source didn't answer (an API without the feature, or a failed pull): leave the target alone. logger?.info("No component availability rules were pulled from the source; skipping them."); - return { status: "success", successful: 0, failed: 0, skipped: 0 }; + return counts; } - // null = unknown: without the target's rules, a source with no rules mustn't clear anything. const targetRules = getModelAvailabilityFromFileSystem(new fileOperations(targetGuid)); + if (targetRules === null) { + // The target didn't answer either way: its API can't store rules, or its pull failed. Saving + // without knowing what it holds would be guesswork, so say so once (when there's anything to say). + if (sourceRules.length > 0) { + const message = + "The target instance's component availability rules couldn't be read (its API may not support them yet), so placement rules weren't synced."; + console.log(ansiColors.yellow(`⚠️ ${message}`)); + logger?.warning(message); + counts.warningDetails.push({ name: "Component availability", error: message, type: "model", guid: sourceGuid }); + } + return counts; + } + + // A database without the feature reads exactly like one with no rules (an empty list), so an + // empty source can't be told from a source that hasn't been migrated yet. Only a source that + // listed at least one rule has proved it stores them, and only then is a model with no placement + // rules allowed to clear the target's. Otherwise a sync from an unmigrated source would silently + // wipe rules set on a migrated target. + const sourceStoresRules = sourceRules.length > 0; const sourceByModelID = new Map(); for (const rule of sourceRules) if (rule?.modelID) sourceByModelID.set(rule.modelID, rule); const targetByModelID = new Map(); - for (const rule of targetRules || []) if (rule?.modelID) targetByModelID.set(rule.modelID, rule); + for (const rule of targetRules) if (rule?.modelID) targetByModelID.set(rule.modelID, rule); - const modelMapper = new ModelMapper(sourceGuid, targetGuid); const templateMapper = new TemplateMapper(sourceGuid, targetGuid); const sectionMapper = new SectionMapper(sourceGuid, targetGuid); const apiClient = getApiClient(); - let successful = 0; - let failed = 0; - let skipped = 0; - const failureDetails: FailureDetail[] = []; - const warningDetails: FailureDetail[] = []; - // Only models in this sync's scope (sourceModels is already narrowed by --models and friends). for (const sourceModel of sourceModels || []) { if (!sourceModel?.id) continue; @@ -132,27 +260,27 @@ export async function pushModelAvailability(sourceModels: mgmtApi.Model[]): Prom name: sourceModel.referenceName, detail: "set the placement rules once the model exists on the target", }); - successful++; + counts.successful++; } else { // The model itself wasn't synced (failed or skipped), so there's nothing to attach rules to. - skipped++; + counts.skipped++; } continue; } - const targetKnown = targetRules !== null; const targetAllowedIn = targetByModelID.get(mapping.targetID)?.allowedIn ?? null; - // Most models have no rules on either side: nothing to do, and nothing worth counting. - if (isEmptyAllowedIn(sourceAllowedIn) && (!targetKnown || isEmptyAllowedIn(targetAllowedIn))) continue; + // Most models have no rules on either side: nothing to do, and nothing worth counting. A source + // that hasn't proved it stores rules never clears the target's. + if (isEmptyAllowedIn(sourceAllowedIn) && (!sourceStoresRules || isEmptyAllowedIn(targetAllowedIn))) continue; const desired = isEmptyAllowedIn(sourceAllowedIn) ? EMPTY_ALLOWED_IN : remapAllowedIn(sourceAllowedIn, templateMapper, sectionMapper); - if (targetKnown && allowedInKey(desired) === allowedInKey(targetAllowedIn)) { + if (allowedInKey(desired) === allowedInKey(targetAllowedIn)) { logger?.fileOnly(`Component availability for "${sourceModel.referenceName}" is unchanged, skipping.`); - skipped++; + counts.skipped++; continue; } @@ -163,7 +291,7 @@ export async function pushModelAvailability(sourceModels: mgmtApi.Model[]): Prom name: sourceModel.referenceName, detail: isEmptyAllowedIn(desired) ? "clear the placement rules" : "replace the placement rules", }); - successful++; + counts.successful++; continue; } @@ -174,21 +302,30 @@ export async function pushModelAvailability(sourceModels: mgmtApi.Model[]): Prom targetGuid ); logger?.info(`Component availability for "${sourceModel.referenceName}" updated.`); - successful++; + counts.successful++; } catch (error: any) { - if (isAvailabilityUnsupported(error)) { + const kind = classifySaveError(error); + if (kind === "unsupported") { // The target can't store rules yet. Say so once and stop: every other save would fail too. const message = "The target instance doesn't support component availability rules yet, so placement rules weren't synced."; console.log(ansiColors.yellow(`⚠️ ${message}`)); logger?.warning(message); - warningDetails.push({ name: "Component availability", error: message, type: "model", guid: sourceGuid }); + counts.warningDetails.push({ name: "Component availability", error: message, type: "model", guid: sourceGuid }); + break; + } + const reason = errorText(error); + if (kind === "forbidden") { + // One failure, not one per model: the permission is the same for all of them. + const message = `Component availability rules weren't synced: saving them needs the Manage or Design permission on the target (${reason})`; + logger?.error(message); + counts.failed++; + counts.failureDetails.push({ name: "Component availability", type: "model", error: message, guid: sourceGuid }); break; } - const reason = error?.message ? String(error.message) : String(error); logger?.error(`Component availability for "${sourceModel.referenceName}" failed: ${reason}`); - failed++; - failureDetails.push({ + counts.failed++; + counts.failureDetails.push({ name: sourceModel.referenceName, type: "model", error: `Failed to save component availability for "${sourceModel.referenceName}" (target ID: ${mapping.targetID}): ${reason}`, @@ -197,12 +334,26 @@ export async function pushModelAvailability(sourceModels: mgmtApi.Model[]): Prom } } + return counts; +} + +export async function pushModelAvailability(sourceModels: mgmtApi.Model[]): Promise { + const { sourceGuid, targetGuid } = state; + const modelMapper = new ModelMapper(sourceGuid, targetGuid); + + const parts = [ + await disableModelsCreatedThisRun(sourceModels, modelMapper), + await pushPlacementRules(sourceModels, modelMapper), + ]; + const sum = (pick: (c: PhaseCounts) => number) => parts.reduce((total, c) => total + pick(c), 0); + const failed = sum((c) => c.failed); + return { status: failed > 0 ? "error" : "success", - successful, + successful: sum((c) => c.successful), failed, - skipped, - failureDetails, - warningDetails, + skipped: sum((c) => c.skipped), + failureDetails: parts.flatMap((c) => c.failureDetails), + warningDetails: parts.flatMap((c) => c.warningDetails), }; } diff --git a/src/lib/pushers/model-pusher.ts b/src/lib/pushers/model-pusher.ts index 1d65fafc..cda36442 100644 --- a/src/lib/pushers/model-pusher.ts +++ b/src/lib/pushers/model-pusher.ts @@ -1,5 +1,11 @@ import * as mgmtApi from "@agility/management-sdk"; -import { getApiClient, state, getLoggerForGuid, recordSavedTarget } from "../../core/state"; +import { + getApiClient, + state, + getLoggerForGuid, + recordSavedTarget, + registerDeferredModelDisable, +} from "../../core/state"; import { PusherResult, FailureDetail } from "../../types/sourceData"; import { ModelMapper } from "lib/mappers/model-mapper"; import { Logs } from "core/logs"; @@ -409,6 +415,8 @@ export async function pushModels(sourceData: mgmtApi.Model[], targetData: mgmtAp for (const model of shouldCreateStub) { preflightReport.record({ phase: "Models", action: "create", name: model.referenceName }); recordSavedTarget("model", model.id, model, model.referenceName); + // PROD-2914: a real run would create it enabled and disable it after the pages. + if (model.isVisible === false) registerDeferredModelDisable(model.id); } for (const model of shouldUpdateFields) { preflightReport.record({ phase: "Models", action: "update", name: model.referenceName }); @@ -524,17 +532,21 @@ const createNewModel = async ( ): Promise<{ result: "created" | "failed"; error?: string }> => { try { // process the model without fields - // PROD-2914: a new model takes the source's `isVisible` (a component disabled on the source - // arrives disabled), carried by the spread. The field update that follows leaves it out. - const createPayload = { + const createPayload: mgmtApi.Model = { ...model, id: 0, fields: [], // no fields for a stub }; + // PROD-2914: a new model takes the source's `isVisible`, but not yet. On a new target every + // placement on a page counts as new, so a model created disabled would get this run's pages + // that use it rejected. It's created enabled (left out, the API defaults a new model to + // enabled), and the component availability phase disables it after the pages. + delete createPayload.isVisible; const newModel = await apiClient.modelMethods.saveModel(createPayload, targetGuid); logger.model.created(model, "created", targetGuid); referenceMapper.addMapping(model, newModel); + if (model.isVisible === false) registerDeferredModelDisable(model.id); return { result: "created" }; } catch (error: any) { // PROD-2211: saveModel can reject even though the API created the model server-side. Before @@ -545,6 +557,7 @@ const createNewModel = async ( if (recovered) { logger.model.created(model, "created", targetGuid); referenceMapper.addMapping(model, recovered); + if (model.isVisible === false) registerDeferredModelDisable(model.id); return { result: "created" }; } logger.model.error(model, error, targetGuid); @@ -567,7 +580,7 @@ async function updateExistingModel( try { // PROD-2914: `isVisible` (whether a component is disabled) belongs to the target instance, so an // update never sends it: left out, the API keeps the target's value. A model created this run - // already got the source's value in its stub. + // gets the source's value after the pages (see createNewModel). const updatePayload: mgmtApi.Model = { ...sourceModel, id: targetID, diff --git a/src/lib/pushers/orchestrate-pushers.ts b/src/lib/pushers/orchestrate-pushers.ts index 573c0a6a..87fd7a98 100644 --- a/src/lib/pushers/orchestrate-pushers.ts +++ b/src/lib/pushers/orchestrate-pushers.ts @@ -198,11 +198,13 @@ export class Pushers { // URL redirections depend on nothing, so they run BEFORE templates: the template pusher // deliberately throws on mapping inconsistencies (rename protection), which aborts the rest // of the guid-level loop — redirections must not be collateral damage of that stop. - // PROD-2914: component availability rules name page models and zones, which only exist on the - // target once templates are pushed, so they run right after templates (still guid-level, so - // before the per-locale content and page phases). + // PROD-2914: component availability (placement rules, and disabling models this run created) + // runs LAST, after the pages of every locale. The API checks each page's new placements + // against the rules, and on a new target every placement is new, so rules in place before the + // pages would reject pages that are valid on the source. Running after templates is also what + // the rules need: they name page models and zones. // ORDER: Models → Galleries → Assets → Containers → URL Redirections → Content → Templates → - // Model availability → Pages + // Pages → Model availability const pusherConfig = [ PUSH_OPERATIONS.models, PUSH_OPERATIONS.galleries, @@ -211,9 +213,11 @@ export class Pushers { PUSH_OPERATIONS.urlRedirections, PUSH_OPERATIONS.content, PUSH_OPERATIONS.templates, - PUSH_OPERATIONS.modelAvailability, PUSH_OPERATIONS.pages, + PUSH_OPERATIONS.modelAvailability, ]; + // Guid-level phases that must wait for the locale-level ones (see above). + const afterLocalePhases = [PUSH_OPERATIONS.modelAvailability]; // Prepare filtering options from state let filterOptions: ModelFilterOptions = {}; @@ -307,6 +311,7 @@ export class Pushers { for (const config of pusherConfig) { if (config === PUSH_OPERATIONS.pages || config === PUSH_OPERATIONS.content) continue; + if (afterLocalePhases.includes(config)) continue; // Execute guid level op const result = await this.executePushOperation({ config, @@ -391,6 +396,37 @@ export class Pushers { } } + // Guid-level phases that run after every locale's content and pages (PROD-2914). + if (afterLocalePhases.length > 0) { + const sourceData = await sourceDataLoader.loadGuidEntities( + locales[0], + Object.keys(filterOptions).length > 0 ? filterOptions : undefined + ); + const targetData = await targetDataLoader.loadGuidEntities(locales[0]); + + for (const config of afterLocalePhases) { + const result = await this.executePushOperation({ + config, + sourceData, + targetData, + locale: locales[0], + publishableContentIds, + publishablePageIds, + elements, + }); + totalSuccess += result.success; + totalFailures += result.failures; + totalSkipped += result.skipped; + phases.push(result.phase); + if (result.failureDetails) { + failureDetails.push(...result.failureDetails); + } + if (result.warningDetails) { + warningDetails.push(...result.warningDetails); + } + } + } + return { totalSuccess, totalFailures, diff --git a/src/lib/pushers/push-operations-config.ts b/src/lib/pushers/push-operations-config.ts index 9aaa3732..927bfcae 100644 --- a/src/lib/pushers/push-operations-config.ts +++ b/src/lib/pushers/push-operations-config.ts @@ -91,11 +91,11 @@ export const PUSH_OPERATIONS: Record = { dataKey: "pages", dependencies: ["Templates", "Models", "Containers", "Content", "Galleries", "Assets"], // Pages require Templates, Models, and Containers }, - // PROD-2914: component availability rules (where a component can go). Runs after templates, - // since its rules name page models and zones; see orchestrate-pushers.ts for the order. + // PROD-2914: component availability (where a component can go, and disabling models this run + // created). Runs last, after the pages of every locale; see orchestrate-pushers.ts for why. modelAvailability: { name: "pushModelAvailability", - description: "Push component availability rules", + description: "Push component availability (placement rules, disabled models)", handler: async (sourceData) => { const { pushModelAvailability } = await import("./model-availability-pusher"); return await pushModelAvailability(sourceData["models"]); diff --git a/src/lib/pushers/tests/model-availability-pusher.test.ts b/src/lib/pushers/tests/model-availability-pusher.test.ts index 211756fe..9c893af3 100644 --- a/src/lib/pushers/tests/model-availability-pusher.test.ts +++ b/src/lib/pushers/tests/model-availability-pusher.test.ts @@ -189,8 +189,9 @@ describe("pushModelAvailability", () => { }); }); - it("also clears when the source model has no rules file at all", async () => { - writeRules(SOURCE, []); + it("also clears when the source model has no rules file, once the source has listed any rule", async () => { + // Another model's rules prove the source stores them, so no file for model 12 means "no rules". + writeRules(SOURCE, [sourceRule({ modelID: 13, modelReferenceName: "Other" })]); writeRules(TARGET, [{ modelID: 120, addableBy: null, allowedIn: sourceAllowedIn }]); mapModel(12, 120); const save = mockApi(); @@ -200,6 +201,19 @@ describe("pushModelAvailability", () => { expect(save.mock.calls[0][1].allowedIn).toEqual({ sitemaps: [], pageModels: [], zones: [] }); }); + it("never clears from a source that has no rules at all (it may not have been migrated yet)", async () => { + // An unmigrated database answers with an empty list, exactly like one with no rules. + writeRules(SOURCE, []); + writeRules(TARGET, [{ modelID: 120, addableBy: null, allowedIn: sourceAllowedIn }]); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(save).not.toHaveBeenCalled(); + expect(result).toMatchObject({ status: "success", successful: 0, skipped: 0, failed: 0 }); + }); + it("never clears when the target's rules are unknown", async () => { writeRules(SOURCE, []); writeRules(TARGET, null); @@ -210,17 +224,21 @@ describe("pushModelAvailability", () => { expect(save).not.toHaveBeenCalled(); expect(result).toMatchObject({ successful: 0, skipped: 0, failed: 0 }); + expect(result.warningDetails).toEqual([]); }); - it("still pushes the source's rules when the target's are unknown", async () => { + it("pushes nothing and warns once when the source has rules but the target's couldn't be read", async () => { writeRules(SOURCE, [sourceRule()]); writeRules(TARGET, null); mapModel(12, 120); const save = mockApi(); - await pushModelAvailability([sourceModel]); + const result = await pushModelAvailability([sourceModel]); - expect(save).toHaveBeenCalledTimes(1); + expect(save).not.toHaveBeenCalled(); + expect(result).toMatchObject({ status: "success", failed: 0 }); + expect(result.warningDetails).toHaveLength(1); + expect(result.warningDetails[0].error).toMatch(/couldn't be read/); }); it("does nothing when the source's rules weren't pulled (an instance without the feature)", async () => { @@ -284,6 +302,39 @@ describe("pushModelAvailability", () => { expect(result.failureDetails[0].error).toContain('Unknown zone "Landing Page" / "HeroZone".'); }); + it("counts a 404 with the API's message (a stale model mapping) as that model's failure, and carries on", async () => { + writeRules(SOURCE, [sourceRule(), sourceRule({ modelID: 13, modelReferenceName: "Other" })]); + writeRules(TARGET, []); + mapModel(12, 120); + mapModel(13, 130, "Other"); + const notFound = Object.assign(httpError(404, "Unable to save availability for model id: 120."), { + responseData: { message: "The model with ID 120 could not be located." }, + }); + const save = mockApi(jest.fn().mockRejectedValueOnce(notFound).mockResolvedValueOnce({})); + + const result = await pushModelAvailability([sourceModel, { id: 13, referenceName: "Other" } as any]); + + expect(save).toHaveBeenCalledTimes(2); + expect(result).toMatchObject({ status: "error", successful: 1, failed: 1 }); + expect(result.failureDetails[0]).toMatchObject({ name: "HeroBanner" }); + expect(result.warningDetails).toEqual([]); + }); + + it("reports a 403 once, not once per model, and stops", async () => { + writeRules(SOURCE, [sourceRule(), sourceRule({ modelID: 13, modelReferenceName: "Other" })]); + writeRules(TARGET, []); + mapModel(12, 120); + mapModel(13, 130, "Other"); + const save = mockApi(jest.fn().mockRejectedValue(httpError(403, "Forbidden"))); + + const result = await pushModelAvailability([sourceModel, { id: 13, referenceName: "Other" } as any]); + + expect(save).toHaveBeenCalledTimes(1); + expect(result).toMatchObject({ status: "error", failed: 1 }); + expect(result.failureDetails).toHaveLength(1); + expect(result.failureDetails[0].error).toMatch(/needs the Manage or Design permission/); + }); + it.each([404, 405, 409])( "stops with one warning when the target answers %i (feature not available)", async (status) => { @@ -320,3 +371,97 @@ describe("pushModelAvailability", () => { ]); }); }); + +// ─── disabling models created this run ──────────────────────────────────────── + +describe("pushModelAvailability — models created this run that are disabled on the source", () => { + const targetModel = { id: 120, referenceName: "HeroBanner", lastModifiedDate: "2026-01-01", fields: [] }; + + function mockModelApi(overrides: { get?: jest.Mock; saveModel?: jest.Mock } = {}) { + const get = overrides.get ?? jest.fn().mockResolvedValue({ ...targetModel, isVisible: true }); + const saveModel = + overrides.saveModel ?? + jest.fn().mockImplementation(async (model: any) => ({ ...model, lastModifiedDate: "2026-05-05" })); + jest.spyOn(stateModule, "getApiClient").mockReturnValue({ + modelMethods: { getContentModel: get, saveModel, saveModelAvailability: jest.fn() }, + } as any); + return { get, saveModel }; + } + + it("disables them after the pages, and records the save in the model mapping", async () => { + writeRules(SOURCE, []); + writeRules(TARGET, []); + mapModel(12, 120); + stateModule.registerDeferredModelDisable(12); + const { get, saveModel } = mockModelApi(); + + const result = await pushModelAvailability([{ ...sourceModel, lastModifiedDate: "2026-01-01" }]); + + expect(get).toHaveBeenCalledWith(120, TARGET); + expect(saveModel).toHaveBeenCalledTimes(1); + expect(saveModel.mock.calls[0][0]).toMatchObject({ id: 120, isVisible: false }); + expect(result).toMatchObject({ status: "success", successful: 1, failed: 0 }); + // Otherwise the next sync reads the model as changed on the target. + const mapping = new ModelMapper(SOURCE, TARGET).getModelMappingByID(12, "source"); + expect(mapping?.targetLastModifiedDate).toBe("2026-05-05"); + }); + + it("leaves models that weren't created this run alone", async () => { + writeRules(SOURCE, []); + writeRules(TARGET, []); + mapModel(12, 120); + const { saveModel } = mockModelApi(); + + await pushModelAvailability([{ ...sourceModel, isVisible: false }]); + + expect(saveModel).not.toHaveBeenCalled(); + }); + + it("warns once and leaves them enabled when the target's API doesn't return isVisible", async () => { + writeRules(SOURCE, []); + writeRules(TARGET, []); + mapModel(12, 120); + mapModel(13, 130, "Other"); + stateModule.registerDeferredModelDisable(12); + stateModule.registerDeferredModelDisable(13); + const { get, saveModel } = mockModelApi({ get: jest.fn().mockResolvedValue({ ...targetModel }) }); + + const result = await pushModelAvailability([sourceModel, { id: 13, referenceName: "Other" } as any]); + + expect(get).toHaveBeenCalledTimes(1); + expect(saveModel).not.toHaveBeenCalled(); + expect(result).toMatchObject({ status: "success", failed: 0 }); + expect(result.warningDetails).toHaveLength(1); + expect(result.warningDetails[0].error).toMatch(/can't disable models yet/); + }); + + it("reports a failed disable as that model's failure", async () => { + writeRules(SOURCE, []); + writeRules(TARGET, []); + mapModel(12, 120); + stateModule.registerDeferredModelDisable(12); + mockModelApi({ saveModel: jest.fn().mockRejectedValue(new Error("boom")) }); + + const result = await pushModelAvailability([sourceModel]); + + expect(result).toMatchObject({ status: "error", successful: 0, failed: 1 }); + expect(result.failureDetails[0].error).toMatch(/Failed to disable model "HeroBanner".*boom/); + }); + + it("records the planned disable in preflight and writes nothing", async () => { + writeRules(SOURCE, []); + writeRules(TARGET, []); + setState({ preflight: true }); + stateModule.registerDeferredModelDisable(12); + const { get, saveModel } = mockModelApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(get).not.toHaveBeenCalled(); + expect(saveModel).not.toHaveBeenCalled(); + expect(result.successful).toBe(1); + expect(preflightReport.getEntries()).toEqual([ + expect.objectContaining({ phase: "Component availability", action: "update", name: "HeroBanner" }), + ]); + }); +}); diff --git a/src/lib/pushers/tests/model-pusher.test.ts b/src/lib/pushers/tests/model-pusher.test.ts index e47480b0..838740bd 100644 --- a/src/lib/pushers/tests/model-pusher.test.ts +++ b/src/lib/pushers/tests/model-pusher.test.ts @@ -991,16 +991,41 @@ describe("pushModels — isVisible ownership (PROD-2914)", () => { const JUN = new Date(2025, 5, 1).toISOString(); const fields = (label = "Title") => [{ name: "Title", label, type: "Text", itemOrder: 0, settings: {} }]; - it("copies the source's isVisible when it creates a model", async () => { + it("creates a disabled source model enabled, and defers disabling it until after the pages", async () => { const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 777 })); jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); const { pushModels } = await import("../model-pusher"); - await pushModels([makeModel({ referenceName: "hidden-component", isVisible: false })], []); + const source = makeModel({ id: 42, referenceName: "hidden-component", isVisible: false }); + await pushModels([source], []); - // The stub carries it; the field update that follows leaves it out (null = keep on the API). - expect(saveModel.mock.calls[0][0].isVisible).toBe(false); + // Neither the stub nor the field update carries it (left out, the API defaults a new model to + // enabled), so this run's page saves can place it. The availability phase disables it later. + expect("isVisible" in saveModel.mock.calls[0][0]).toBe(false); expect("isVisible" in saveModel.mock.calls[1][0]).toBe(false); + expect(stateModule.getDeferredModelDisables()).toEqual([42]); + }); + + it("defers nothing for a created model that's enabled on the source", async () => { + const saveModel = jest.fn().mockResolvedValue(makeModel({ id: 779 })); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); + const { pushModels } = await import("../model-pusher"); + + await pushModels([makeModel({ id: 43, referenceName: "shown-component", isVisible: true })], []); + + expect(stateModule.getDeferredModelDisables()).toEqual([]); + }); + + it("in preflight, records the deferred disable of a model it would create, and writes nothing", async () => { + const saveModel = jest.fn(); + jest.spyOn(stateModule, "getApiClient").mockReturnValue(makeApiClient(saveModel)); + setState({ preflight: true }); + const { pushModels } = await import("../model-pusher"); + + await pushModels([makeModel({ id: 44, referenceName: "planned-hidden", isVisible: false })], []); + + expect(saveModel).not.toHaveBeenCalled(); + expect(stateModule.getDeferredModelDisables()).toEqual([44]); }); it("never sends isVisible when it updates an existing model, so the target keeps its own", async () => { diff --git a/src/lib/pushers/tests/orchestrate-pushers.test.ts b/src/lib/pushers/tests/orchestrate-pushers.test.ts index 3c33ab89..28217f3c 100644 --- a/src/lib/pushers/tests/orchestrate-pushers.test.ts +++ b/src/lib/pushers/tests/orchestrate-pushers.test.ts @@ -331,13 +331,29 @@ describe("Pushers.instanceOrchestrator — models-first ordering (PROD-2202)", ( "pushAssets", "pushContainers", "pushTemplates", - // PROD-2914: availability rules name page models and zones, so they follow templates. - "pushModelAvailability", "pushContent", "pushPages", + // PROD-2914: component availability runs after the pages of every locale, so this run's + // pages aren't checked against rules (or a disabled model) that are older on the source. + "pushModelAvailability", ]); }); + it("runs component availability once, after the pages of every locale (PROD-2914)", async () => { + setState({ sourceGuid: "src-u", targetGuid: "tgt-u", locales: "en-us,fr-ca" }); + await stubDataLoader(); + await stubAllHandlers(); + + const order: string[] = []; + const pushers = new Pushers({ onOperationStart: (name) => order.push(name) }); + + await pushers.instanceOrchestrator(); + + expect(order.filter((name) => name === "pushPages")).toHaveLength(2); + expect(order.filter((name) => name === "pushModelAvailability")).toHaveLength(1); + expect(order[order.length - 1]).toBe("pushModelAvailability"); + }); + it("a model-validation failure aborts the sync before galleries or assets are pushed", async () => { setState({ sourceGuid: "src-u", targetGuid: "tgt-u", locales: "en-us" }); await stubDataLoader(); diff --git a/src/tests/sync/component-availability.test.ts b/src/tests/sync/component-availability.test.ts new file mode 100644 index 00000000..63da6f9a --- /dev/null +++ b/src/tests/sync/component-availability.test.ts @@ -0,0 +1,216 @@ +/** + * Component availability (PROD-2914), driven end to end through the fake instance. + * + * The fake does what the Management API does with the rules: it only takes them for a component + * model, and a page save is rejected when it adds a NEW placement of a disabled component, or one + * outside the component's page model and zone rules. Existing placements are grandfathered. + * + * That is what makes the order of the sync matter. On a new target every placement is new, so a + * source page whose placement is older than the source's rules would be rejected if the rules (or + * the disabled model) reached the target before the pages. The sync pushes them last. + */ +import { SyncHarness } from "./helpers/run-sync"; +import { installNetworkStubs, InstalledNetworkStubs } from "./helpers/network-stubs"; +import { + ALL_ELEMENTS, + FIXTURE_LOCALE, + MODEL_POST, + PAGE_HOME, + TEMPLATE_STANDARD, + fullInstanceFixture, +} from "./helpers/full-instance-fixture"; + +jest.mock("lib/pushers/batch-polling", () => ({ + pollBatchUntilComplete: jest.fn(), + extractContentBatchResults: jest.fn(), + extractPageBatchResults: jest.fn(), +})); + +jest.mock("axios", () => ({ post: jest.fn(), get: jest.fn(), default: { post: jest.fn() } })); + +jest.mock("lib/pushers/url-redirection-api", () => ({ + MAX_URL_REDIRECTION_BATCH_SIZE: 250, + saveUrlRedirections: jest.fn(), +})); + +/** The fixture's Post model as a component model (contentDefinitionTypeID 2), disabled on the source. */ +const COMPONENT_POST = { ...MODEL_POST, contentDefinitionTypeID: 2, isVisible: false }; + +/** Two zones, so a rule can allow one and not the other. */ +const TEMPLATE_TWO_ZONES = { + ...TEMPLATE_STANDARD, + contentSectionDefinitions: [ + { pageItemTemplateID: 71, pageItemTemplateReferenceName: "Main", itemOrder: 0, contentViewID: -1 }, + { pageItemTemplateID: 72, pageItemTemplateReferenceName: "Hero", itemOrder: 1, contentViewID: -1 }, + ], +}; + +const POST_MODULE = { module: "PostModule", item: { contentid: 500 } }; + +/** The page holds the component in Main, from before the source's rule said "Hero only". */ +const PAGE_WITH_POST = { ...PAGE_HOME, zones: { Main: [POST_MODULE] } }; + +/** Source rules: only the Hero zone of Standard. The who rules are there to prove they never travel. */ +const AVAILABILITY_POST = { + modelID: 10, + modelReferenceName: "Post", + addableBy: { permissions: ["Administrator"], users: [], teams: [] }, + allowedIn: { + sitemaps: [], + pageModels: [], + zones: [{ zoneID: 72, pageModelID: 700, pageModelName: "Standard", referenceName: "Hero", name: "Hero" }], + }, +}; + +function componentFixture(model: any = COMPONENT_POST) { + const fixture = fullInstanceFixture(); + fixture.source["models/10.json"] = { ...model }; + fixture.source["templates/700.json"] = { ...TEMPLATE_TWO_ZONES }; + fixture.source[`${FIXTURE_LOCALE}/page/800.json`] = { ...PAGE_WITH_POST }; + fixture.source["model-availability/10.json"] = { ...AVAILABILITY_POST }; + return fixture; +} + +function makeHarness(opts: { overwrite?: boolean; model?: any } = {}): SyncHarness { + const harness = new SyncHarness({ + elements: ALL_ELEMENTS, + fixture: componentFixture(opts.model), + overwrite: opts.overwrite ?? false, + }); + // A real sync pulls the target first; an empty, migrated target answers "no rules". + harness.fake.projectToDisk(harness.rootPath, harness.targetGuid); + return harness; +} + +describe("component availability sync (PROD-2914)", () => { + let harness: SyncHarness; + let net: InstalledNetworkStubs; + + beforeEach(() => { + net = installNetworkStubs(); + jest.spyOn(console, "log").mockImplementation(() => {}); + jest.spyOn(console, "warn").mockImplementation(() => {}); + jest.spyOn(console, "error").mockImplementation(() => {}); + }); + + afterEach(() => { + harness?.cleanup(); + net.restore(); + jest.restoreAllMocks(); + }); + + describe("fresh sync into an empty target", () => { + it.each([ + ["a disabled component outside its placement rules", COMPONENT_POST], + ["an enabled component outside its placement rules", { ...COMPONENT_POST, isVisible: true }], + ])("saves the page that holds %s", async (_label, model) => { + harness = makeHarness({ model }); + + const run = await harness.run(); + + const pages = run.phases.find((p) => p.operation === "pushPages")!; + expect({ ok: pages.successful, failed: pages.failed }).toEqual({ ok: 1, failed: 0 }); + expect(run.totals.failed).toBe(0); + const [page] = Array.from(harness.fake.instance(harness.targetGuid).pages.values()); + expect(page.zones.Main).toHaveLength(1); + }); + + it("disables the new model and sets its placement rules only after the pages", async () => { + harness = makeHarness(); + + await harness.run(); + + const methods = harness.fake.calls.map((c) => c.method); + const pageSave = methods.indexOf("pageMethods.savePage"); + const rulesSave = methods.indexOf("modelMethods.saveModelAvailability"); + const disable = harness.fake.calls.findIndex( + (c) => c.method === "modelMethods.saveModel" && c.payload?.isVisible === false + ); + expect(pageSave).toBeGreaterThanOrEqual(0); + expect(rulesSave).toBeGreaterThan(pageSave); + expect(disable).toBeGreaterThan(pageSave); + + // Created enabled: no model save before the pages carries isVisible at all. + harness.fake.calls + .slice(0, pageSave) + .filter((c) => c.method === "modelMethods.saveModel") + .forEach((c) => expect("isVisible" in c.payload).toBe(false)); + }); + + it("leaves the target with the source's disabled state and placement rules, and its own who rules", async () => { + harness = makeHarness(); + + const run = await harness.run(); + const target = harness.fake.instance(harness.targetGuid); + const targetModelID = harness.readMappings("models")![0].targetID; + const targetTemplate = Array.from(target.templates.values())[0]; + const targetHero = targetTemplate.contentSectionDefinitions.find( + (z: any) => z.pageItemTemplateReferenceName === "Hero" + ); + + const availability = run.phases.find((p) => p.operation === "pushModelAvailability")!; + expect({ ok: availability.successful, failed: availability.failed }).toEqual({ ok: 2, failed: 0 }); + expect(target.models.get(targetModelID).isVisible).toBe(false); + const saved = target.availability.get(targetModelID); + expect(saved.addableBy).toBeNull(); + expect(saved.allowedIn.sitemaps).toEqual([]); + expect(saved.allowedIn.pageModels).toEqual([]); + // Remapped onto the target: its page model ID, the names, and never a source ID. + expect(saved.allowedIn.zones).toHaveLength(1); + expect(saved.allowedIn.zones[0]).toMatchObject({ + zoneID: targetHero.pageItemTemplateID, + pageModelID: targetTemplate.pageTemplateID, + pageModelName: "Standard", + referenceName: "Hero", + name: "Hero", + }); + expect(targetHero.pageItemTemplateID).not.toBe(72); + }); + }); + + describe("re-sync with no source change", () => { + it("writes no model or rules, and the disable doesn't read as a target change", async () => { + harness = makeHarness(); + await harness.run(); + + const run2 = await harness.resync(); + + const writes = harness.fake.calls.filter((c) => /^modelMethods\.save/.test(c.method)); + expect(writes.map((c) => c.method)).toEqual([]); + expect(run2.totals.failed).toBe(0); + const models = run2.phases.find((p) => p.operation === "pushModels")!; + const availability = run2.phases.find((p) => p.operation === "pushModelAvailability")!; + expect({ ok: models.successful, skip: models.skipped }).toEqual({ ok: 0, skip: 1 }); + expect({ ok: availability.successful, skip: availability.skipped }).toEqual({ ok: 0, skip: 1 }); + // No "target model has changed" hold-back: the model phase skips as unchanged. + expect(run2.results.flatMap((r) => r.warningDetails).map((w) => w.error)).not.toContainEqual( + expect.stringMatching(/target model has changed/) + ); + }); + }); + + describe("a later sync that adds a placement", () => { + it("reports the page the target's rules reject, with the rule's reason", async () => { + harness = makeHarness({ overwrite: true }); + await harness.run(); + + // The page moves its component to Hero: a new placement, of a component the target has + // disabled since the first sync. + harness.editSourceFile(`${FIXTURE_LOCALE}/page/800.json`, { + ...PAGE_WITH_POST, + zones: { Hero: [POST_MODULE], Main: [] }, + properties: { ...PAGE_HOME.properties, versionID: 951, modified: "2026-02-02T00:00:00.000" }, + }); + const run2 = await harness.resync(); + + const pages = run2.phases.find((p) => p.operation === "pushPages")!; + expect(pages.failed).toBe(1); + const errors = run2.results.flatMap((r) => r.failureDetails).map((f) => f.error); + expect(errors).toContainEqual( + expect.stringContaining( + 'Rejected by a component availability rule: The "Post" component is disabled on this instance.' + ) + ); + }); + }); +}); diff --git a/src/tests/sync/full-instance.test.ts b/src/tests/sync/full-instance.test.ts index cd922e12..5734e1b0 100644 --- a/src/tests/sync/full-instance.test.ts +++ b/src/tests/sync/full-instance.test.ts @@ -3,8 +3,9 @@ * * `full-sync.test.ts` covers models and containers in depth. This suite covers *breadth*: * every entity type the orchestrator pushes — models, galleries, assets, containers, URL - * redirections, templates, component availability rules, content and pages — created on a fresh sync and then proven inert - * on a re-sync with no source change. + * redirections, templates, content and pages — created on a fresh sync and then proven inert + * on a re-sync with no source change. (The ninth phase, component availability, runs here with + * nothing to do; `component-availability.test.ts` covers it.) * * Assertions pin **positive** counts, never just the absence of writes. A phase reporting * `successful`/`failed`/`skipped` all zero has not "done nothing safely" — it has matched no @@ -13,7 +14,14 @@ */ import { SyncHarness } from "./helpers/run-sync"; import { installNetworkStubs, InstalledNetworkStubs } from "./helpers/network-stubs"; -import { ALL_ELEMENTS, ALL_OPERATIONS, FIXTURE_LOCALE, fullInstanceFixture, MODEL_POST } from "./helpers/full-instance-fixture"; +import { + ALL_ELEMENTS, + ALL_OPERATIONS, + FIXTURE_LOCALE, + IDLE_OPERATIONS, + fullInstanceFixture, + MODEL_POST, +} from "./helpers/full-instance-fixture"; jest.mock("lib/pushers/batch-polling", () => ({ pollBatchUntilComplete: jest.fn(), @@ -73,7 +81,7 @@ describe("full instance — all nine phases", () => { run.phases.forEach((phase) => { expect({ op: phase.operation, ok: phase.successful, failed: phase.failed }).toEqual({ op: phase.operation, - ok: 1, + ok: IDLE_OPERATIONS.includes(phase.operation) ? 0 : 1, failed: 0, }); }); @@ -93,33 +101,6 @@ describe("full instance — all nine phases", () => { expect(target.templates.size).toBe(1); expect(target.content.size).toBe(1); expect(target.pages.size).toBe(1); - expect(target.availability.size).toBe(1); - }); - - it("sends the placement rules remapped onto the target, and never the who rules (PROD-2914)", async () => { - harness = makeHarness(); - - await harness.run(); - const [save] = harness.fake.callsTo("modelMethods.saveModelAvailability"); - const targetModelID = harness.readMappings("models")![0].targetID; - const targetTemplate = Array.from(harness.fake.instance(harness.targetGuid).templates.values())[0]; - const targetZoneID = targetTemplate.contentSectionDefinitions[0].pageItemTemplateID; - - expect(save.payload.modelID).toBe(targetModelID); - expect(save.payload.addableBy).toBeNull(); - expect(save.payload.allowedIn).toEqual({ - sitemaps: [{ referenceName: "website", name: "Website" }], - pageModels: [], - zones: [ - { - zoneID: targetZoneID, - pageModelID: targetTemplate.pageTemplateID, - pageModelName: "Standard", - referenceName: "Main", - name: "Main", - }, - ], - }); }); it("writes a mapping for every entity type", async () => { @@ -170,7 +151,7 @@ describe("full instance — all nine phases", () => { expect({ op: phase.operation, ok: phase.successful, skip: phase.skipped, failed: phase.failed }).toEqual({ op: phase.operation, ok: 0, - skip: 1, + skip: IDLE_OPERATIONS.includes(phase.operation) ? 0 : 1, failed: 0, }); }); @@ -249,7 +230,7 @@ describe("full instance — all nine phases", () => { expect({ op: phase.operation, ok: phase.successful, skip: phase.skipped }).toEqual({ op: phase.operation, ok: 0, - skip: 1, + skip: IDLE_OPERATIONS.includes(phase.operation) ? 0 : 1, }); }); }); diff --git a/src/tests/sync/helpers/fake-instance.ts b/src/tests/sync/helpers/fake-instance.ts index a3684fc7..2e5115fb 100644 --- a/src/tests/sync/helpers/fake-instance.ts +++ b/src/tests/sync/helpers/fake-instance.ts @@ -88,6 +88,11 @@ function emptyStore(locales: string[]): InstanceStore { }; } +/** An error shaped like the SDK's Exception for an API error response (`{ "message": … }` body). */ +function httpError(statusCode: number, message: string): Error { + return Object.assign(new Error(`HTTP ${statusCode}: ${message}`), { statusCode, responseData: { message } }); +} + /** One recorded API call, in order. The ordered log is what the golden-file tier diffs. */ export interface RecordedCall { method: string; @@ -234,7 +239,9 @@ export class FakeInstance { // Model creation is two-pass: a shell with `fields: []` to get an ID, then the same // model again with its fields. Both arrive here; the second must update, not duplicate. const id = model?.id && model.id > 0 ? model.id : this.allocate("model"); - const saved = { ...model, id, lastModifiedDate: nextTimestamp() }; + // PROD-2914: `isVisible` left out (or null) keeps the stored value, and a new model is enabled. + const isVisible = model?.isVisible ?? store.models.get(id)?.isVisible ?? true; + const saved = { ...model, id, isVisible, lastModifiedDate: nextTimestamp() }; store.models.set(id, saved); return saved; }, @@ -256,10 +263,15 @@ export class FakeInstance { }, // The API's per-group semantics: null/absent keeps the stored group, an object whose lists // are all empty clears it, any other object replaces it. Targets are stored as sent; the - // real API resolves names to IDs, which no test here depends on. + // real API resolves names to IDs, which no test here depends on. Like the API, it only takes + // rules for a component model (contentDefinitionTypeID 2) that exists. saveModelAvailability: async (modelID: number, availability: any, guid: string) => { this.record("modelMethods.saveModelAvailability", guid, { modelID, ...availability }); const store = this.instance(guid); + const model = store.models.get(modelID); + if (!model) throw httpError(404, `The model with ID ${modelID} could not be located.`); + if (model.contentDefinitionTypeID !== 2) + throw httpError(400, "Availability can only be set on component models."); const stored = store.availability.get(modelID) ?? { modelID, addableBy: null, allowedIn: null }; const merge = (current: any, incoming: any) => { if (incoming === undefined || incoming === null) return current; @@ -309,6 +321,9 @@ export class FakeInstance { /** Same idea as `lastContentSaveIds`, for pages — they batch one page at a time. */ private lastPageSaveIds: number[] = []; + /** Zone (pageItemTemplate) IDs handed out by savePageTemplate. */ + private nextZoneId = 1001; + contentMethods = { saveContentItems: async (items: any[], guid: string, locale: string) => { this.record("contentMethods.saveContentItems", guid, items, locale); @@ -412,13 +427,21 @@ export class FakeInstance { const store = this.instance(guid); const incoming = template?.pageTemplateID; const id = incoming && incoming > 0 ? incoming : this.allocate("template"); - const saved = { ...template, pageTemplateID: id }; + // Zones get IDs as the real API gives them (PROD-2914 tests remap rules onto them). + const zones = (template?.contentSectionDefinitions ?? []).map((zone: any) => ({ + ...zone, + pageItemTemplateID: zone?.pageItemTemplateID > 0 ? zone.pageItemTemplateID : this.nextZoneId++, + })); + const saved = { ...template, pageTemplateID: id, contentSectionDefinitions: zones }; store.templates.set(id, saved); return saved; }, savePage: async (page: any, guid: string, locale: string) => { this.record("pageMethods.savePage", guid, page, locale); const incoming = page?.pageID; + // PROD-2914: the API's component availability pre-check, before anything is stored. + const rejection = this.availabilityRejection(guid, locale, page); + if (rejection) throw httpError(400, rejection); const id = incoming && incoming > 0 ? incoming : this.allocate("page"); this.instance(guid).pages.set(`${locale}:${id}`, { ...page, pageID: id }); this.lastPageSaveIds = [id]; @@ -436,6 +459,48 @@ export class FakeInstance { }, }; + /** + * PROD-2914: the Management API's component availability pre-check on a page save, reduced to + * what these tests need. Only a NEW placement (a zone + content pair the stored page doesn't + * have) is checked, so existing placements are grandfathered, as on the API. It rejects a + * disabled component, and one outside its page model and zone rules. (Sitemap rules aren't + * modelled.) Returns the API's reason text, or null when the save is allowed. + */ + private availabilityRejection(guid: string, locale: string, page: any): string | null { + const store = this.instance(guid); + const lower = (value: any) => String(value ?? "").toLowerCase(); + const placements = (p: any) => + Object.entries(p?.zones ?? {}).flatMap(([zone, modules]) => + (Array.isArray(modules) ? modules : []).map((m: any) => ({ zone, contentID: m?.item?.contentid })) + ); + const stored = page?.pageID > 0 ? store.pages.get(`${locale}:${page.pageID}`) : undefined; + const existing = new Set(placements(stored).map((p) => `${lower(p.zone)}:${p.contentID}`)); + + for (const { zone, contentID } of placements(page)) { + if (existing.has(`${lower(zone)}:${contentID}`)) continue; + const item = store.content.get(`${locale}:${contentID}`); + const model = Array.from(store.models.values()).find( + (m: any) => lower(m.referenceName) === lower(item?.properties?.definitionName) + ); + if (!model) continue; + const title = model.displayName || model.referenceName; + if (model.isVisible === false) return `The "${title}" component is disabled on this instance.`; + const allowedIn = store.availability.get(model.id)?.allowedIn; + const pageModels = allowedIn?.pageModels ?? []; + const zones = allowedIn?.zones ?? []; + if (pageModels.length === 0 && zones.length === 0) continue; + const layoutOK = + pageModels.some((pm: any) => lower(pm.name) === lower(page.templateName)) || + zones.some( + (z: any) => lower(z.pageModelName) === lower(page.templateName) && lower(z.referenceName) === lower(zone) + ); + if (!layoutOK) { + return `The "${title}" component isn't allowed in the "${zone}" zone of the "${page.templateName}" page model.`; + } + } + return null; + } + instanceMethods = { getLocales: async (guid: string) => { this.record("instanceMethods.getLocales", guid); diff --git a/src/tests/sync/helpers/full-instance-fixture.ts b/src/tests/sync/helpers/full-instance-fixture.ts index ae60edcd..646f47dd 100644 --- a/src/tests/sync/helpers/full-instance-fixture.ts +++ b/src/tests/sync/helpers/full-instance-fixture.ts @@ -118,22 +118,6 @@ export const SITEMAP_WEBSITE = [ }, ]; -/** - * PROD-2914: placement rules for the Post model. (Real rules sit on component models; the CLI - * doesn't distinguish model kinds, and a second model would change every model count here.) The - * who rules are there to prove they never travel. - */ -export const AVAILABILITY_POST = { - modelID: 10, - modelReferenceName: "Post", - addableBy: { permissions: ["Administrator"], users: [], teams: [] }, - allowedIn: { - sitemaps: [{ sitemapID: 1, referenceName: "website", name: "Website" }], - pageModels: [], - zones: [{ zoneID: 71, pageModelID: 700, pageModelName: "Standard", referenceName: "Main", name: "Main" }], - }, -}; - export const REDIRECTIONS = { items: [{ id: 1, originUrl: "/old", destinationUrl: "/new", statusCode: 301 }], lastAccessDate: "2025-01-01T00:00:00.000", @@ -151,7 +135,6 @@ export function fullInstanceFixture(): SyncFixture { "galleries/300.json": { ...GALLERY_HERO }, "assets/json/1.json": { assetMedias: [{ ...ASSET_HERO }] }, "templates/700.json": { ...TEMPLATE_STANDARD }, - "model-availability/10.json": { ...AVAILABILITY_POST }, [`${FIXTURE_LOCALE}/item/500.json`]: { ...CONTENT_HELLO }, [`${FIXTURE_LOCALE}/page/800.json`]: { ...PAGE_HOME }, [`${FIXTURE_LOCALE}/nestedsitemap/website.json`]: SITEMAP_WEBSITE, @@ -169,7 +152,14 @@ export const ALL_OPERATIONS = [ "pushContainers", "pushUrlRedirections", "pushTemplates", - "pushModelAvailability", "pushContent", "pushPages", + "pushModelAvailability", ] as const; + +/** + * Phases this fixture gives nothing to do. Component availability rules only sit on component + * models, and this fixture's one model is a content model; `component-availability.test.ts` + * covers that phase with its own fixture. The phase still runs, so it's in ALL_OPERATIONS. + */ +export const IDLE_OPERATIONS: readonly string[] = ["pushModelAvailability"]; From 1105b7d02caec89878326fe0c65c9cd810c739ba Mon Sep 17 00:00:00 2001 From: Joel Varty Date: Thu, 22 Oct 2026 14:13:29 -0400 Subject: [PATCH 3/3] PROD-2914: leave stale availability targets out of the sync; match only the four reason sentences The API returns a target deleted after the rule was saved as { id, stale: true }. remapAllowedIn and the comparison key now leave those entries out, so the save no longer fails with a nameless target and a re-sync no longer sees a change. A sitemap list, or a page-model + zone set, made up only of stale targets is still a restriction (allowed nowhere), so that model is skipped with a warning rather than sent empty, which would widen the rule to everywhere. The page-rejection check now matches a body made of exactly the four reason sentences (one or more, joined by a space), including "(unknown)" names. Co-Authored-By: Claude Opus 5.5 --- README.md | 4 + src/lib/pushers/model-availability-pusher.ts | 71 +++++++-- .../component-availability-error.ts | 19 ++- .../component-availability-error.test.ts | 29 ++++ .../tests/model-availability-pusher.test.ts | 136 ++++++++++++++++++ 5 files changed, 247 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index ef717539..e4930157 100644 --- a/README.md +++ b/README.md @@ -274,6 +274,10 @@ like this: one rule on some model. An instance that hasn't been upgraded for the rules yet reads as "no rules", so a source with none at all never clears anything. - If the target's rules can't be read, the where rules aren't synced, and the sync says so. +- A sitemap, page model or zone that was deleted on the source after a rule was saved is left + out. If that would leave a rule's sitemaps, or its page models and zones, empty, the model's + where rules aren't synced and the sync warns: sending the rule empty would allow the component + everywhere, when on the source it's allowed nowhere. - **Who** rules and **disabled** belong to each instance and are never overwritten. A model the sync creates is disabled at the end of the sync if it's disabled on the source. - A page the target rejects because of a rule is reported with the reason, for example diff --git a/src/lib/pushers/model-availability-pusher.ts b/src/lib/pushers/model-availability-pusher.ts index 34eb00df..1ad44e64 100644 --- a/src/lib/pushers/model-availability-pusher.ts +++ b/src/lib/pushers/model-availability-pusher.ts @@ -29,6 +29,12 @@ import { preflightReport } from "../preflight/preflight-report"; * there is one (and by name otherwise), sitemaps by `referenceName`. Source IDs are never sent, * because the API resolves an ID before a name and a source ID can belong to a different object on * the target. + * + * Stale targets (a sitemap, page model or zone deleted on the source after the rule was saved) come + * back from the API as `{ id, stale: true }` and are left out: they have no name to match by, and the + * API drops them on save anyway. A list made up only of stale targets still restricts the component + * (to nowhere), and sending it without them would widen the rule to "anywhere", so such a model is + * skipped with a warning instead. */ const EMPTY_ALLOWED_IN: mgmtApi.ModelAvailabilityAllowedIn = { sitemaps: [], pageModels: [], zones: [] }; @@ -46,18 +52,54 @@ function isEmptyAllowedIn(allowedIn: mgmtApi.ModelAvailabilityAllowedIn | null | ); } +function isStale(entry: { stale?: boolean | null } | null | undefined): boolean { + return entry?.stale === true; +} + +/** The entries that still exist: stale ones (deleted after the rule was saved) are left out. */ +function live(list: T[] | null | undefined): T[] { + return (list || []).filter((entry) => entry && !isStale(entry)); +} + +/** True when the list names targets, but every one of them is stale. */ +function onlyStale(list: { stale?: boolean | null }[]): boolean { + return list.length > 0 && list.every(isStale); +} + +/** + * The parts of an `allowedIn` group made up only of stale targets: the sitemap list, and the page + * model + zone set (one layout rule, so it counts as stale only when all of both are). + */ +export function staleOnlyRules(allowedIn: mgmtApi.ModelAvailabilityAllowedIn | null | undefined): string[] { + if (!allowedIn) return []; + const parts: string[] = []; + if (onlyStale(allowedIn.sitemaps || [])) parts.push("sitemap"); + if (onlyStale([...(allowedIn.pageModels || []), ...(allowedIn.zones || [])])) parts.push("page model and zone"); + return parts; +} + /** Name keys of an `allowedIn` group, for comparing a source group with what the target holds. */ function allowedInKey(allowedIn: mgmtApi.ModelAvailabilityAllowedIn | null | undefined): string { const group = allowedIn || EMPTY_ALLOWED_IN; - const sitemaps = (group.sitemaps || []).map((s) => lower(s.referenceName)).sort(); - const pageModels = (group.pageModels || []).map((p) => lower(p.name)).sort(); - const zones = (group.zones || []).map((z) => `${lower(z.pageModelName)}/${lower(z.referenceName || z.name)}`).sort(); - return JSON.stringify({ sitemaps, pageModels, zones }); + const sitemaps = live(group.sitemaps) + .map((s) => lower(s.referenceName)) + .sort(); + const pageModels = live(group.pageModels) + .map((p) => lower(p.name)) + .sort(); + const zones = live(group.zones) + .map((z) => `${lower(z.pageModelName)}/${lower(z.referenceName || z.name)}`) + .sort(); + // Stale entries are left out, but a list of nothing but stale entries still restricts the + // component (to nowhere), so it mustn't read the same as no rule at all. + const staleOnly = staleOnlyRules(group); + return JSON.stringify({ sitemaps, pageModels, zones, staleOnly }); } /** * Rewrite a source `allowedIn` group for the target instance. Only the portable names, and target - * IDs from the sync's own mappings, are sent. + * IDs from the sync's own mappings, are sent. Stale entries are left out (see `staleOnlyRules` for + * the groups where that would widen the rule). */ export function remapAllowedIn( allowedIn: mgmtApi.ModelAvailabilityAllowedIn, @@ -68,18 +110,18 @@ export function remapAllowedIn( sourcePageModelID ? templateMapper.getTemplateMappingByPageTemplateID(sourcePageModelID, "source") : null; return { - sitemaps: (allowedIn.sitemaps || []).map((sitemap) => ({ + sitemaps: live(allowedIn.sitemaps).map((sitemap) => ({ referenceName: sitemap.referenceName ?? null, name: sitemap.name ?? null, })), - pageModels: (allowedIn.pageModels || []).map((pageModel) => { + pageModels: live(allowedIn.pageModels).map((pageModel) => { const mapping = targetPageModel(pageModel.pageModelID); return { ...(mapping ? { pageModelID: mapping.targetPageTemplateID } : {}), name: mapping?.targetPageTemplateName || pageModel.name || null, }; }), - zones: (allowedIn.zones || []).map((zone) => { + zones: live(allowedIn.zones).map((zone) => { const pageModelMapping = targetPageModel(zone.pageModelID); const zoneMapping = zone.zoneID ? sectionMapper.getSectionMappingByID(zone.zoneID, "source") : null; return { @@ -249,6 +291,19 @@ async function pushPlacementRules(sourceModels: mgmtApi.Model[], modelMapper: Mo if (!sourceModel?.id) continue; const sourceAllowedIn = sourceByModelID.get(sourceModel.id)?.allowedIn ?? null; + // A rule whose sitemaps (or page models and zones) were all deleted on the source allows the + // component nowhere. Without them the rule would allow it everywhere, so leave the target alone. + const staleOnly = staleOnlyRules(sourceAllowedIn); + if (staleOnly.length > 0) { + const which = staleOnly.map((part) => `every ${part} it's limited to`).join(" and "); + const message = `Component availability for "${sourceModel.referenceName}" wasn't synced: on the source, ${which} has been deleted. Sending the rule without them would allow the component everywhere, so fix the rule on the source first.`; + console.log(ansiColors.yellow(`⚠️ ${message}`)); + logger?.warning(message); + counts.warningDetails.push({ name: sourceModel.referenceName, error: message, type: "model", guid: sourceGuid }); + counts.skipped++; + continue; + } + const mapping = modelMapper.getModelMappingByID(sourceModel.id, "source"); if (!mapping?.targetID) { if (isEmptyAllowedIn(sourceAllowedIn)) continue; diff --git a/src/lib/pushers/page-pusher/component-availability-error.ts b/src/lib/pushers/page-pusher/component-availability-error.ts index 2227c661..5db2f54d 100644 --- a/src/lib/pushers/page-pusher/component-availability-error.ts +++ b/src/lib/pushers/page-pusher/component-availability-error.ts @@ -7,9 +7,19 @@ * You don't have permission to add the "{model}" component. * The "{model}" component isn't allowed on the "{sitemap}" sitemap. * The "{model}" component isn't allowed in the "{zone}" zone of the "{pageModel}" page model. + * + * A name the server can't resolve is sent as "(unknown)". A page that breaks more than one rule + * gets each reason once, joined by a space. Any other text is not a rule rejection. */ -const AVAILABILITY_REASON = - /The ".+" component is disabled on this instance\.|You don't have permission to add the ".+" component\.|The ".+" component isn't allowed (?:on|in) the ".+"/; +const NAME = '".+?"'; +const REASON_SENTENCES = [ + `The ${NAME} component is disabled on this instance\\.`, + `You don't have permission to add the ${NAME} component\\.`, + `The ${NAME} component isn't allowed on the ${NAME} sitemap\\.`, + `The ${NAME} component isn't allowed in the ${NAME} zone of the ${NAME} page model\\.`, +]; +const ONE_REASON = `(?:${REASON_SENTENCES.join("|")})`; +const AVAILABILITY_REASON = new RegExp(`^${ONE_REASON}(?: ${ONE_REASON})*$`); /** The server's response body as text, from the SDK's Exception (or a raw axios error). */ function responseText(error: any): string | null { @@ -28,8 +38,9 @@ export function componentAvailabilityReason(error: any): string | null { const status = error?.statusCode ?? error?.response?.status; if (status !== 400) return null; const text = responseText(error); - if (!text || !AVAILABILITY_REASON.test(text)) return null; - return text.trim(); + const trimmed = text?.trim(); + if (!trimmed || !AVAILABILITY_REASON.test(trimmed)) return null; + return trimmed; } /** The message to report for a failed page save: the rule's reason when a rule rejected it. */ diff --git a/src/lib/pushers/page-pusher/tests/component-availability-error.test.ts b/src/lib/pushers/page-pusher/tests/component-availability-error.test.ts index 1eb63490..dc067ba1 100644 --- a/src/lib/pushers/page-pusher/tests/component-availability-error.test.ts +++ b/src/lib/pushers/page-pusher/tests/component-availability-error.test.ts @@ -15,10 +15,39 @@ describe("componentAvailabilityReason", () => { expect(componentAvailabilityReason(sdkError(400, reason))).toBe(reason); }); + it('accepts "(unknown)" for a name the server couldn\'t resolve', () => { + const reason = 'The "Hero" component isn\'t allowed in the "(unknown)" zone of the "(unknown)" page model.'; + expect(componentAvailabilityReason(sdkError(400, reason))).toBe(reason); + }); + + it("returns every reason when a page breaks more than one rule", () => { + const reasons = + 'The "Hero" component is disabled on this instance. The "Banner" component isn\'t allowed on the "Website" sitemap.'; + expect(componentAvailabilityReason(sdkError(400, reasons))).toBe(reasons); + }); + + it.each([ + // Fallback texts an earlier server build sent: the four sentences above are the only reasons. + "The \"Hero\" component isn't allowed on this page's sitemap.", + 'The "Hero" component isn\'t allowed in this zone.', + 'The "Hero" component can\'t be added here.', + // A reason sentence cut short, or with anything else around it. + 'The "Hero" component isn\'t allowed in the "Main" zone', + 'Page rejected: The "Hero" component is disabled on this instance.', + 'The "Hero" component is disabled on this instance. Try again later.', + ])("ignores a text that isn't exactly the reason sentences: %s", (text) => { + expect(componentAvailabilityReason(sdkError(400, text))).toBeNull(); + }); + it("reads the reason from a JSON error body", () => { const reason = 'The "Hero" component is disabled on this instance.'; expect(componentAvailabilityReason(sdkError(400, { message: reason }))).toBe(reason); expect(componentAvailabilityReason(sdkError(400, { Message: reason }))).toBe(reason); + expect( + componentAvailabilityReason( + sdkError(400, { statusCode: 400, exceptionMessage: reason, additionalParameters: null }) + ) + ).toBe(reason); }); it("reads a raw axios error", () => { diff --git a/src/lib/pushers/tests/model-availability-pusher.test.ts b/src/lib/pushers/tests/model-availability-pusher.test.ts index 9c893af3..d267c11b 100644 --- a/src/lib/pushers/tests/model-availability-pusher.test.ts +++ b/src/lib/pushers/tests/model-availability-pusher.test.ts @@ -353,6 +353,142 @@ describe("pushModelAvailability", () => { } ); + it("leaves stale (deleted) source targets out of the save", async () => { + writeRules(SOURCE, [ + sourceRule({ + allowedIn: { + sitemaps: [...sourceAllowedIn.sitemaps, { sitemapID: 2, stale: true }], + pageModels: [...sourceAllowedIn.pageModels, { pageModelID: 4, stale: true }], + zones: [...sourceAllowedIn.zones, { zoneID: 15, stale: true }], + }, + }), + ]); + writeRules(TARGET, []); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(result).toMatchObject({ status: "success", successful: 1, failed: 0 }); + expect(result.warningDetails).toEqual([]); + expect(save.mock.calls[0][1].allowedIn).toEqual({ + sitemaps: [{ referenceName: "website", name: "Website" }], + pageModels: [{ name: "Landing Page" }], + zones: [{ pageModelName: "Landing Page", referenceName: "HeroZone", name: "Hero Zone" }], + }); + }); + + it.each([ + ["sitemap", { sitemaps: [{ sitemapID: 2, stale: true }], pageModels: sourceAllowedIn.pageModels, zones: [] }], + [ + "page model and zone", + { + sitemaps: sourceAllowedIn.sitemaps, + pageModels: [{ pageModelID: 4, stale: true }], + zones: [{ zoneID: 15, stale: true }], + }, + ], + ])( + "skips a model with a warning when every %s in its rule is stale (sending it empty would widen it)", + async (part, allowedIn) => { + writeRules(SOURCE, [sourceRule({ allowedIn })]); + writeRules(TARGET, [{ modelID: 120, addableBy: null, allowedIn: sourceAllowedIn }]); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(save).not.toHaveBeenCalled(); + expect(result).toMatchObject({ status: "success", successful: 0, skipped: 1, failed: 0 }); + expect(result.warningDetails).toHaveLength(1); + expect(result.warningDetails[0]).toMatchObject({ name: "HeroBanner", type: "model" }); + expect(result.warningDetails[0].error).toContain(`every ${part} it's limited to`); + expect(result.warningDetails[0].error).toMatch(/would allow the component everywhere/); + } + ); + + it("still syncs the layout rule when only some page models and zones are stale", async () => { + // A stale page model next to a live zone: the zone keeps the rule restricted. + writeRules(SOURCE, [ + sourceRule({ + allowedIn: { sitemaps: [], pageModels: [{ pageModelID: 4, stale: true }], zones: sourceAllowedIn.zones }, + }), + ]); + writeRules(TARGET, []); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(result).toMatchObject({ successful: 1, skipped: 0 }); + expect(save.mock.calls[0][1].allowedIn).toEqual({ + sitemaps: [], + pageModels: [], + zones: [{ pageModelName: "Landing Page", referenceName: "HeroZone", name: "Hero Zone" }], + }); + }); + + it("ignores the target's stale entries when comparing, so a re-sync writes nothing", async () => { + writeRules(SOURCE, [sourceRule()]); + writeRules(TARGET, [ + { + modelID: 120, + addableBy: null, + allowedIn: { + sitemaps: [ + { sitemapID: 7, referenceName: "website" }, + { sitemapID: 8, stale: true }, + ], + pageModels: [ + { pageModelID: 30, name: "Landing Page" }, + { pageModelID: 31, stale: true }, + ], + zones: [ + { zoneID: 140, pageModelName: "Landing Page", referenceName: "HeroZone" }, + { zoneID: 141, stale: true }, + ], + }, + }, + ]); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(save).not.toHaveBeenCalled(); + expect(result).toMatchObject({ successful: 0, skipped: 1, failed: 0 }); + }); + + it("replaces a target sitemap list that holds only stale entries, which allows the component nowhere", async () => { + // The source has no sitemap rule (anywhere); the target's only sitemap was deleted (nowhere). + writeRules(SOURCE, [ + sourceRule({ allowedIn: { sitemaps: [], pageModels: sourceAllowedIn.pageModels, zones: [] } }), + ]); + writeRules(TARGET, [ + { + modelID: 120, + addableBy: null, + allowedIn: { + sitemaps: [{ sitemapID: 8, stale: true }], + pageModels: [{ pageModelID: 30, name: "Landing Page" }], + zones: [], + }, + }, + ]); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(save).toHaveBeenCalledTimes(1); + expect(result.successful).toBe(1); + expect(save.mock.calls[0][1].allowedIn).toEqual({ + sitemaps: [], + pageModels: [{ name: "Landing Page" }], + zones: [], + }); + }); + it("records the planned saves in preflight and writes nothing", async () => { writeRules(SOURCE, [sourceRule(), sourceRule({ modelID: 13, modelReferenceName: "NewModel" })]); writeRules(TARGET, []);