diff --git a/README.md b/README.md index ab351a0..e493015 100644 --- a/README.md +++ b/README.md @@ -256,6 +256,37 @@ 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, 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. +- 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 + `Rejected by a component availability rule: The "Hero" component isn't allowed in the "Main" + 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. + +--- + ### Selective Page Sync `--pages` narrows a sync to one or more page structures. Use it when you want to promote the @@ -576,6 +607,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 6319692..fbe7822 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/core/push.ts b/src/core/push.ts index 038b0ce..9f88507 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 0305535..b722f4d 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 new file mode 100644 index 0000000..5b87cec --- /dev/null +++ b/src/lib/downloaders/download-model-availability.ts @@ -0,0 +1,100 @@ +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 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 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; + 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; + } + + // 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(); + try { + fs.mkdirSync(folderPath, { recursive: true }); + + 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) { + 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 f8f24d1..bc3b6aa 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 0000000..e03d885 --- /dev/null +++ b/src/lib/downloaders/tests/download-model-availability.test.ts @@ -0,0 +1,145 @@ +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("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))); + + 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 f0c1bed..b4101d6 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 0000000..d8c49d2 --- /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 0000000..1ad44e6 --- /dev/null +++ b/src/lib/pushers/model-availability-pusher.ts @@ -0,0 +1,414 @@ +import * as mgmtApi from "@agility/management-sdk"; +import ansiColors from "ansi-colors"; +import { fileOperations } from "../../core/fileOperations"; +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 { preflightReport } from "../preflight/preflight-report"; + +/** + * 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. + * - `addableBy` (who can add) belongs to the instance: it's always sent as `null`, so the API keeps + * 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. + * + * 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. + * + * 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: [] }; + +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) + ); +} + +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 = 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. Stale entries are left out (see `staleOnlyRules` for + * the groups where that would widen the rule). + */ +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: live(allowedIn.sitemaps).map((sitemap) => ({ + referenceName: sitemap.referenceName ?? null, + name: sitemap.name ?? null, + })), + pageModels: live(allowedIn.pageModels).map((pageModel) => { + const mapping = targetPageModel(pageModel.pageModelID); + return { + ...(mapping ? { pageModelID: mapping.targetPageTemplateID } : {}), + name: mapping?.targetPageTemplateName || pageModel.name || null, + }; + }), + zones: live(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, + }; + }), + }; +} + +/** 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 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 counts; + } + 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); + + const templateMapper = new TemplateMapper(sourceGuid, targetGuid); + const sectionMapper = new SectionMapper(sourceGuid, targetGuid); + const apiClient = getApiClient(); + + // 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; + + // 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; + 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", + }); + counts.successful++; + } else { + // The model itself wasn't synced (failed or skipped), so there's nothing to attach rules to. + counts.skipped++; + } + continue; + } + + const targetAllowedIn = targetByModelID.get(mapping.targetID)?.allowedIn ?? null; + + // 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 (allowedInKey(desired) === allowedInKey(targetAllowedIn)) { + logger?.fileOnly(`Component availability for "${sourceModel.referenceName}" is unchanged, skipping.`); + counts.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", + }); + counts.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.`); + counts.successful++; + } catch (error: any) { + 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); + 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; + } + logger?.error(`Component availability for "${sourceModel.referenceName}" failed: ${reason}`); + counts.failed++; + counts.failureDetails.push({ + name: sourceModel.referenceName, + type: "model", + error: `Failed to save component availability for "${sourceModel.referenceName}" (target ID: ${mapping.targetID}): ${reason}`, + guid: sourceGuid, + }); + } + } + + 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: sum((c) => c.successful), + failed, + 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 354cf73..cda3644 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,15 +532,21 @@ const createNewModel = async ( ): Promise<{ result: "created" | "failed"; error?: string }> => { try { // process the model without fields - 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 @@ -543,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); @@ -563,10 +578,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 + // gets the source's value after the pages (see createNewModel). + 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 57b49a4..87fd7a9 100644 --- a/src/lib/pushers/orchestrate-pushers.ts +++ b/src/lib/pushers/orchestrate-pushers.ts @@ -198,7 +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. - // ORDER: Models → Galleries → Assets → Containers → URL Redirections → Content → Templates → Pages + // 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 → + // Pages → Model availability const pusherConfig = [ PUSH_OPERATIONS.models, PUSH_OPERATIONS.galleries, @@ -208,7 +214,10 @@ export class Pushers { PUSH_OPERATIONS.content, PUSH_OPERATIONS.templates, 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 = {}; @@ -302,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, @@ -386,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/page-pusher/component-availability-error.ts b/src/lib/pushers/page-pusher/component-availability-error.ts new file mode 100644 index 0000000..5db2f54 --- /dev/null +++ b/src/lib/pushers/page-pusher/component-availability-error.ts @@ -0,0 +1,51 @@ +/** + * 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. + * + * 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 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 { + 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); + 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. */ +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 927c19d..736f892 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 0000000..dc067ba --- /dev/null +++ b/src/lib/pushers/page-pusher/tests/component-availability-error.test.ts @@ -0,0 +1,84 @@ +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('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", () => { + 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 d1af91b..bddf6ca 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 c752e7b..927bfca 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 (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 (placement rules, disabled models)", + 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 0000000..d267c11 --- /dev/null +++ b/src/lib/pushers/tests/model-availability-pusher.test.ts @@ -0,0 +1,603 @@ +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, 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(); + + await pushModelAvailability([sourceModel]); + + 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); + mapModel(12, 120); + const save = mockApi(); + + const result = await pushModelAvailability([sourceModel]); + + expect(save).not.toHaveBeenCalled(); + expect(result).toMatchObject({ successful: 0, skipped: 0, failed: 0 }); + expect(result.warningDetails).toEqual([]); + }); + + 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(); + + const result = await pushModelAvailability([sourceModel]); + + 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 () => { + 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("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) => { + 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("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, []); + 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" }), + ]); + }); +}); + +// ─── 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 b9cfbc8..838740b 100644 --- a/src/lib/pushers/tests/model-pusher.test.ts +++ b/src/lib/pushers/tests/model-pusher.test.ts @@ -983,3 +983,97 @@ 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("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"); + + const source = makeModel({ id: 42, referenceName: "hidden-component", isVisible: false }); + await pushModels([source], []); + + // 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 () => { + 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 50d9dcc..28217f3 100644 --- a/src/lib/pushers/tests/orchestrate-pushers.test.ts +++ b/src/lib/pushers/tests/orchestrate-pushers.test.ts @@ -333,9 +333,27 @@ describe("Pushers.instanceOrchestrator — models-first ordering (PROD-2202)", ( "pushTemplates", "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/lib/pushers/tests/push-operations-config.test.ts b/src/lib/pushers/tests/push-operations-config.test.ts index 0b59d17..c4e6caa 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/component-availability.test.ts b/src/tests/sync/component-availability.test.ts new file mode 100644 index 0000000..63da6f9 --- /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 a545b49..5734e1b 100644 --- a/src/tests/sync/full-instance.test.ts +++ b/src/tests/sync/full-instance.test.ts @@ -1,10 +1,11 @@ /** - * 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 - * on a re-sync with no source change. + * 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(), @@ -30,7 +38,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 +62,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(); @@ -73,7 +81,7 @@ describe("full instance — all eight 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, }); }); @@ -143,7 +151,7 @@ describe("full instance — all eight 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, }); }); @@ -222,7 +230,7 @@ describe("full instance — all eight 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 4c89476..2e5115f 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,11 +82,17 @@ function emptyStore(locales: string[]): InstanceStore { content: new Map(), pages: new Map(), urlRedirections: new Map(), + availability: new Map(), sitemap: [], locales, }; } +/** 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; @@ -199,6 +207,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) { @@ -224,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; }, @@ -240,6 +257,36 @@ 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. 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; + 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 = { @@ -274,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); @@ -377,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]; @@ -401,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 1074d9c..646f47d 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 @@ -144,7 +144,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", @@ -154,4 +154,12 @@ export const ALL_OPERATIONS = [ "pushTemplates", "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"];