From 1b08b756006537fa471c45922cb1b9c5176f0bad Mon Sep 17 00:00:00 2001 From: Jules Exel Date: Tue, 16 Jun 2026 14:27:06 -0400 Subject: [PATCH] fix(PROD-2187): pull linked-content model references for --models and --models-with-deps A model whose field links to another model (Content-type field with settings.ContentDefinition, e.g. FooterLinks -> FooterLinksLists) failed to push to a target that didn't already have the referenced model, with a 404 "Definition for setting X not found". Neither --models nor --models-with-deps followed model-to-model references, so the referenced definitions were never included in the push set. Add resolveReferencedModels(), a transitive, de-duplicated, cycle-safe walk over settings.ContentDefinition, and apply it on both paths: - --models (guid-data-loader): expand the requested models to include referenced models; still models-only (no content/pages/containers). - --models-with-deps (ModelDependencyTreeBuilder): new findModelsReferencedByModels step so referenced models (and their containers) join the full dependency tree. Because pushModels creates all model stubs before applying fields, the referenced model exists on the target by the time the referencing model's fields are saved. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../models/model-dependency-tree-builder.ts | 51 +++++++++ .../model-dependency-tree-builder.test.ts | 57 ++++++++++ src/lib/pushers/guid-data-loader.ts | 18 ++- .../pushers/tests/guid-data-loader.test.ts | 103 +++++++++++++++++- 4 files changed, 224 insertions(+), 5 deletions(-) diff --git a/src/lib/models/model-dependency-tree-builder.ts b/src/lib/models/model-dependency-tree-builder.ts index 6e217be..6c1911f 100644 --- a/src/lib/models/model-dependency-tree-builder.ts +++ b/src/lib/models/model-dependency-tree-builder.ts @@ -13,6 +13,43 @@ import ansiColors from "ansi-colors"; import { SitemapHierarchy } from "../pushers/page-pusher/sitemap-hierarchy"; import { AssetReferenceExtractor } from "../assets/asset-reference-extractor"; +/** + * Expand a list of model reference names to include every model they reference through + * linked-content fields. + */ +export function resolveReferencedModels(modelNames: string[], allModels: any[]): string[] { + const byLower = new Map(); + (allModels || []).forEach((m: any) => { + if (m?.referenceName) byLower.set(m.referenceName.toLowerCase(), m); + }); + + const result = new Set(); + const queue: string[] = []; + + const add = (name: string | undefined | null) => { + if (!name || typeof name !== "string") return; + const model = byLower.get(name.toLowerCase()); + const canonical = model?.referenceName ?? name; + if (!result.has(canonical)) { + result.add(canonical); + queue.push(canonical); + } + }; + + (modelNames || []).forEach(add); + + while (queue.length > 0) { + const name = queue.pop()!; + const model = byLower.get(name.toLowerCase()); + if (!model || !Array.isArray(model.fields)) continue; + for (const field of model.fields) { + add(field?.settings?.ContentDefinition); + } + } + + return Array.from(result); +} + export interface ModelDependencyTree { models: Set; // Model reference names containers: Set; @@ -75,6 +112,10 @@ export class ModelDependencyTreeBuilder { this.findTemplatesUsedByPages(tree); // 🎯 NEW: Include models for the newly discovered content this.findModelsForDiscoveredContent(tree); + // 🎯 NEW: Include models referenced by other models via linked-content fields + // (e.g. FooterLinks β†’ FooterLinksLists). Without this, a selected model whose field points at + // another model fails to save on the target with a 404 "Definition for setting X not found". + this.findModelsReferencedByModels(tree); // 🎯 NEW: Include containers for the newly discovered models this.findContainersForDiscoveredModels(tree); // 🎯 NEW: Include containers that contain discovered content items @@ -345,6 +386,16 @@ export class ModelDependencyTreeBuilder { // console.log(ansiColors.gray(` πŸ“‹ Added ${newModelCount} additional models for content dependencies`)); } + /** + * Expand the model set with models referenced by other models through linked-content fields. + * Delegates to the shared `resolveReferencedModels` walk (transitive, de-duplicated, cycle-safe). + */ + private findModelsReferencedByModels(tree: ModelDependencyTree): void { + if (!this.sourceData.models) return; + const expanded = resolveReferencedModels(Array.from(tree.models), this.sourceData.models); + expanded.forEach((name) => tree.models.add(name)); + } + /** * Find containers for newly discovered models */ diff --git a/src/lib/models/tests/model-dependency-tree-builder.test.ts b/src/lib/models/tests/model-dependency-tree-builder.test.ts index 015d095..1a8cd06 100644 --- a/src/lib/models/tests/model-dependency-tree-builder.test.ts +++ b/src/lib/models/tests/model-dependency-tree-builder.test.ts @@ -94,6 +94,63 @@ function makeSourceData(overrides: Partial = {}): any { }; } +// ─── modelβ†’model references via linked-content fields (PROD-2187) ───────────── + +function makeModelWithRefs(id: number, referenceName: string, refs: string[] = []): any { + return { + id, + referenceName, + fields: refs.map((r) => ({ type: "Content", settings: { ContentDefinition: r } })), + }; +} + +describe("ModelDependencyTreeBuilder β€” modelβ†’model references", () => { + it("includes a model referenced via a linked-content field (FooterLinks β†’ FooterLinksLists)", () => { + const builder = new ModelDependencyTreeBuilder( + makeSourceData({ + models: [makeModelWithRefs(1, "FooterLinks", ["FooterLinksLists"]), makeModelWithRefs(2, "FooterLinksLists")], + }) + ); + const tree = builder.buildDependencyTree(["FooterLinks"], "website"); + expect(tree.models.has("FooterLinks")).toBe(true); + expect(tree.models.has("FooterLinksLists")).toBe(true); + }); + + it("resolves references transitively (A β†’ B β†’ C)", () => { + const builder = new ModelDependencyTreeBuilder( + makeSourceData({ + models: [makeModelWithRefs(1, "A", ["B"]), makeModelWithRefs(2, "B", ["C"]), makeModelWithRefs(3, "C")], + }) + ); + const tree = builder.buildDependencyTree(["A"], "website"); + expect(tree.models.has("B")).toBe(true); + expect(tree.models.has("C")).toBe(true); + }); + + it("does not pull in unrelated models", () => { + const builder = new ModelDependencyTreeBuilder( + makeSourceData({ + models: [ + makeModelWithRefs(1, "FooterLinks", ["FooterLinksLists"]), + makeModelWithRefs(2, "FooterLinksLists"), + makeModelWithRefs(9, "Unrelated"), + ], + }) + ); + const tree = builder.buildDependencyTree(["FooterLinks"], "website"); + expect(tree.models.has("Unrelated")).toBe(false); + }); + + it("terminates on a reference cycle (A β†’ B β†’ A)", () => { + const builder = new ModelDependencyTreeBuilder( + makeSourceData({ models: [makeModelWithRefs(1, "A", ["B"]), makeModelWithRefs(2, "B", ["A"])] }) + ); + const tree = builder.buildDependencyTree(["A"], "website"); + expect(tree.models.has("A")).toBe(true); + expect(tree.models.has("B")).toBe(true); + }); +}); + // ─── resetLoggingFlags ──────────────────────────────────────────────────────── describe("ModelDependencyTreeBuilder.resetLoggingFlags", () => { diff --git a/src/lib/pushers/guid-data-loader.ts b/src/lib/pushers/guid-data-loader.ts index 9887065..dae1788 100644 --- a/src/lib/pushers/guid-data-loader.ts +++ b/src/lib/pushers/guid-data-loader.ts @@ -16,6 +16,11 @@ import { fileOperations } from "../../core/fileOperations"; import { getState } from "../../core/state"; import * as mgmtApi from "@agility/management-sdk"; +// Shared modelβ†’model reference resolver, defined alongside the dependency-tree builder so both the +// --models (here) and --models-with-deps (tree builder) paths use the same logic. +import { resolveReferencedModels } from "../models/model-dependency-tree-builder"; +export { resolveReferencedModels }; + export interface ModelFilterOptions { models?: string[]; // Simple model filtering modelsWithDeps?: string[]; // Model filtering with dependency tree @@ -202,13 +207,18 @@ export class GuidDataLoader { ); } - // Build dependency tree and filter all related entities using complete data - const dependencyTree = treeBuilder.buildDependencyTree(validation.valid, locale); - + // --models (simple): pull ONLY the requested models plus the models they reference through + // linked-content fields (e.g. FooterLinks β†’ FooterLinksLists), transitively. No content, pages, + // containers, or assets. Referenced models must be included or the model push fails on the target + // with a 404 "Definition for setting X not found". if (!useFullDependencyTree) { - return this.filterGuidEntitiesByModels(guidEntities, validation.valid); + const expandedModels = resolveReferencedModels(validation.valid, (completeEntities ?? guidEntities).models); + return this.filterGuidEntitiesByModels(guidEntities, expandedModels); } + // Build dependency tree and filter all related entities using complete data + const dependencyTree = treeBuilder.buildDependencyTree(validation.valid, locale); + return await this.filterGuidEntitiesByDependencyTree(completeEntities, dependencyTree, locale); } diff --git a/src/lib/pushers/tests/guid-data-loader.test.ts b/src/lib/pushers/tests/guid-data-loader.test.ts index 9b9d500..7721ce2 100644 --- a/src/lib/pushers/tests/guid-data-loader.test.ts +++ b/src/lib/pushers/tests/guid-data-loader.test.ts @@ -2,7 +2,7 @@ import * as fs from "fs"; import * as os from "os"; import * as path from "path"; import { resetState, setState, state } from "core/state"; -import { GuidDataLoader } from "../guid-data-loader"; +import { GuidDataLoader, resolveReferencedModels } from "../guid-data-loader"; let tmpDir: string; @@ -211,6 +211,64 @@ describe("GuidDataLoader.validateDataStructure", () => { }); }); +// ─── resolveReferencedModels (PROD-2187: --models pulls referenced models) ─── + +describe("resolveReferencedModels", () => { + const contentField = (refName: string) => ({ type: "Content", settings: { ContentDefinition: refName } }); + const model = (referenceName: string, refs: string[] = []) => ({ + referenceName, + fields: refs.map(contentField), + }); + + it("returns just the requested model when it references nothing", () => { + const all = [model("FooterLinksLists")]; + expect(resolveReferencedModels(["FooterLinksLists"], all)).toEqual(["FooterLinksLists"]); + }); + + it("includes a model referenced via a linked-content field (FooterLinks β†’ FooterLinksLists)", () => { + const all = [model("FooterLinks", ["FooterLinksLists"]), model("FooterLinksLists")]; + const result = resolveReferencedModels(["FooterLinks"], all); + expect(result).toEqual(expect.arrayContaining(["FooterLinks", "FooterLinksLists"])); + expect(result).toHaveLength(2); + }); + + it("resolves references transitively (A β†’ B β†’ C)", () => { + const all = [model("A", ["B"]), model("B", ["C"]), model("C")]; + const result = resolveReferencedModels(["A"], all); + expect(result).toEqual(expect.arrayContaining(["A", "B", "C"])); + expect(result).toHaveLength(3); + }); + + it("terminates on a reference cycle (A β†’ B β†’ A)", () => { + const all = [model("A", ["B"]), model("B", ["A"])]; + const result = resolveReferencedModels(["A"], all); + expect(result.sort()).toEqual(["A", "B"]); + }); + + it("matches case-insensitively but returns the canonical reference name", () => { + const all = [model("FooterLinks", ["FooterLinksLists"]), model("FooterLinksLists")]; + const result = resolveReferencedModels(["footerlinks"], all); + expect(result).toEqual(expect.arrayContaining(["FooterLinks", "FooterLinksLists"])); + }); + + it("keeps a requested model even if it is not found in the model set", () => { + expect(resolveReferencedModels(["Ghost"], [])).toEqual(["Ghost"]); + }); + + it("ignores fields with empty/absent ContentDefinition", () => { + const all = [ + { + referenceName: "M", + fields: [ + { type: "Text", settings: {} }, + { type: "Content", settings: { ContentDefinition: "" } }, + ], + }, + ]; + expect(resolveReferencedModels(["M"], all)).toEqual(["M"]); + }); +}); + // ─── loadGuidEntities β€” with prepared filesystem ───────────────────────────── describe("GuidDataLoader.loadGuidEntities", () => { @@ -291,4 +349,47 @@ describe("GuidDataLoader.loadGuidEntities", () => { /Model validation failed/ ); }); + + it("--models pulls the requested model AND its referenced models, but no content/pages/containers", async () => { + // Lay down models on disk: FooterLinks references FooterLinksLists via a linked-content field. + const guid = "models-only-refs-guid-u"; + const modelsDir = path.join(tmpDir, guid, "models"); + fs.mkdirSync(modelsDir, { recursive: true }); + fs.writeFileSync( + path.join(modelsDir, "157.json"), + JSON.stringify({ + id: 157, + referenceName: "FooterLinks", + contentDefinitionTypeID: 1, + fields: [{ name: "footerLinks", type: "Content", settings: { ContentDefinition: "FooterLinksLists" } }], + }) + ); + fs.writeFileSync( + path.join(modelsDir, "158.json"), + JSON.stringify({ id: 158, referenceName: "FooterLinksLists", contentDefinitionTypeID: 1, fields: [] }) + ); + // A model that was NOT requested and is unrelated β€” must NOT be pulled in. + fs.writeFileSync( + path.join(modelsDir, "999.json"), + JSON.stringify({ id: 999, referenceName: "Unrelated", contentDefinitionTypeID: 1, fields: [] }) + ); + + state.elements = "Models"; + state.isSync = false; + state.modelsWithDeps = ""; + + const loader = new GuidDataLoader(guid); + const entities = await loader.loadGuidEntities("en-us", { models: ["FooterLinks"] }); + + const names = entities.models.map((m: any) => m.referenceName).sort(); + expect(names).toEqual(["FooterLinks", "FooterLinksLists"]); // referenced model included, Unrelated excluded + + // Models-only: nothing else is pulled. + expect(entities.containers).toHaveLength(0); + expect(entities.content).toHaveLength(0); + expect(entities.pages).toHaveLength(0); + expect(entities.templates).toHaveLength(0); + expect(entities.assets).toHaveLength(0); + expect(entities.galleries).toHaveLength(0); + }); });