diff --git a/src/lib/content/content-field-mapper.ts b/src/lib/content/content-field-mapper.ts index 9ffea9c..c55a45a 100644 --- a/src/lib/content/content-field-mapper.ts +++ b/src/lib/content/content-field-mapper.ts @@ -224,16 +224,35 @@ export class ContentFieldMapper { return { mappedValue: fieldValue, warnings: 1, errors: 0 }; } + // PROD-2341: a single-item linked-content selection must be emitted as the SCALAR remapped + // contentID string, not the { contentid, fulllist } object. The server-side batch engine reads + // a linked-content field value with `row[col] as string` (Agility.Shared BatchProcessing_ + // InsertContentItem.cs); an object cast yields null → the selection is stored EMPTY (the GET API + // then renders it as the SharedContent list's reference name), which is the "Draw Game arrives + // empty / component can't be edited" symptom. Emit "" when the referenced item + // is mapped; otherwise leave it untouched and warn (the PROD-2309 pre-push guard normally catches + // the unmapped case first). + if (this.isSingleItemContentSelection(fieldValue)) { + const sourceContentId = fieldValue.contentid ?? fieldValue.contentID; + const contentMapping = context.referenceMapper.getContentItemMappingByContentID(sourceContentId, "source"); + const targetContentID = this.resolveTargetContentID(contentMapping); + if (targetContentID) { + return { mappedValue: String(targetContentID), warnings, errors }; + } + return { mappedValue: fieldValue, warnings: warnings + 1, errors }; + } + // Map contentid/contentID references if (fieldValue.contentid || fieldValue.contentID) { const sourceContentId = fieldValue.contentid || fieldValue.contentID; const contentMapping = context.referenceMapper.getContentItemMappingByContentID(sourceContentId, "source"); - if (contentMapping && (contentMapping as any).contentID) { + const targetContentID = this.resolveTargetContentID(contentMapping); + if (targetContentID) { if (fieldValue.contentid !== undefined) { - mappedValue.contentid = (contentMapping as any).contentID; + mappedValue.contentid = targetContentID; } if (fieldValue.contentID !== undefined) { - mappedValue.contentID = (contentMapping as any).contentID; + mappedValue.contentID = targetContentID; } } else { warnings++; @@ -248,7 +267,7 @@ export class ContentFieldMapper { .map((id) => parseInt(id.trim())); const mappedIds = sourceIds.map((sourceId) => { const mapping = context.referenceMapper.getContentItemMappingByContentID(sourceId, "source"); - return mapping ? (mapping as any).contentID : sourceId; + return this.resolveTargetContentID(mapping) ?? sourceId; }); mappedValue.sortids = mappedIds.join(","); } @@ -256,6 +275,39 @@ export class ContentFieldMapper { return { mappedValue, warnings, errors }; } + /** + * PROD-2341: read the target contentID off a content-item mapping record. The mapper returns + * records whose target id is `targetContentID`; earlier code here read `.contentID`, which is + * always undefined on those records — so content references were never remapped and shipped with + * the SOURCE id (a dangling reference that the target stores as an empty selection). The + * `contentID` fallback keeps compatibility with any caller/test that passes that shape. + */ + private resolveTargetContentID(mapping: any): number | null { + if (!mapping) return null; + return mapping.targetContentID ?? mapping.contentID ?? null; + } + + /** + * PROD-2341: a single-item linked-content SELECTION — a linked-content dropdown where one item is + * picked. The pulled shape is `{ contentid: N, fulllist: false }` with a positive contentID, no + * `referencename`, and no `sortids`. The presence of a `fulllist` flag marks this as a linked- + * content field value (list/dropdown), distinguishing it from a bare nested `{ contentid }` + * reference (which keeps its object-form remap). This shape must be serialized as the scalar + * contentID string — see the batch-engine note in mapContentReferenceField. + */ + private isSingleItemContentSelection(fieldValue: any): boolean { + if (!fieldValue || typeof fieldValue !== "object" || Array.isArray(fieldValue)) return false; + const hasFullListKey = "fulllist" in fieldValue || "fullList" in fieldValue; + if (!hasFullListKey) return false; + const isFullList = fieldValue.fulllist === true || fieldValue.fullList === true; + if (isFullList) return false; // whole-list link — not a single-item selection + const contentId = fieldValue.contentid ?? fieldValue.contentID; + const hasPositiveContentId = typeof contentId === "number" && contentId > 0; + const hasReferenceName = "referencename" in fieldValue || "referenceName" in fieldValue; + const hasSortIds = "sortids" in fieldValue; + return hasPositiveContentId && !hasReferenceName && !hasSortIds; + } + private mapAssetUrlString( url: string, context?: ContentFieldMappingContext diff --git a/src/lib/content/tests/content-field-mapper.test.ts b/src/lib/content/tests/content-field-mapper.test.ts index 9dc7b6b..f326de0 100644 --- a/src/lib/content/tests/content-field-mapper.test.ts +++ b/src/lib/content/tests/content-field-mapper.test.ts @@ -236,6 +236,80 @@ describe("ContentFieldMapper.mapContentFields", () => { }); }); + // ─── single-item linked-content selection (PROD-2341) ─────────────────────── + describe("single-item linked-content selection", () => { + it("emits the remapped contentID as a SCALAR string for { contentid, fulllist:false } (the drawGame shape)", () => { + // Real mapper records expose `targetContentID`, NOT `contentID`; the remap must read that, + // and the batch engine reads the field with `row[col] as string`, so it must be a scalar. + const referenceMapper = makeReferenceMapper({ + getContentItemMappingByContentID: jest.fn().mockReturnValue({ targetContentID: 571 }), + }); + const context = { referenceMapper, assetMapper: makeAssetMapper() }; + const fields = { drawGame: { contentid: 11868, fulllist: false } }; + const result = mapper.mapContentFields(fields, context); + expect(result.mappedFields.drawGame).toBe("571"); + expect(result.validationErrors).toBe(0); + }); + + it("handles the capital-D contentID / fullList keys for a single-item selection", () => { + const referenceMapper = makeReferenceMapper({ + getContentItemMappingByContentID: jest.fn().mockReturnValue({ targetContentID: 109 }), + }); + const context = { referenceMapper, assetMapper: makeAssetMapper() }; + const fields = { drawGame: { contentID: 11877, fullList: false } }; + const result = mapper.mapContentFields(fields, context); + expect(result.mappedFields.drawGame).toBe("109"); + }); + + it("warns and leaves the value untouched when the referenced item is not mapped", () => { + const context = { referenceMapper: makeReferenceMapper(), assetMapper: makeAssetMapper() }; + const fields = { drawGame: { contentid: 99999, fulllist: false } }; + const result = mapper.mapContentFields(fields, context); + expect(result.mappedFields.drawGame).toEqual({ contentid: 99999, fulllist: false }); + expect(result.validationWarnings).toBeGreaterThan(0); + }); + + it("leaves an empty selection ({ fulllist:false }, no contentid) unchanged", () => { + const context = { referenceMapper: makeReferenceMapper(), assetMapper: makeAssetMapper() }; + const fields = { drawGame: { fulllist: false } }; + const result = mapper.mapContentFields(fields, context); + expect(result.mappedFields.drawGame).toEqual({ fulllist: false }); + }); + + it("does NOT scalar-ize a bare nested { contentid } reference (no fulllist key) — keeps object remap via targetContentID", () => { + const referenceMapper = makeReferenceMapper({ + getContentItemMappingByContentID: jest.fn().mockReturnValue({ targetContentID: 99 }), + }); + const context = { referenceMapper, assetMapper: makeAssetMapper() }; + const fields = { related: { contentid: 10 } }; + const result = mapper.mapContentFields(fields, context); + expect(result.mappedFields.related).toEqual({ contentid: 99 }); + }); + + it("does NOT scalar-ize a whole-list link ({ fulllist:true }) — leaves it as a list reference", () => { + const referenceMapper = makeReferenceMapper({ + getContentItemMappingByContentID: jest.fn().mockReturnValue({ targetContentID: 571 }), + }); + const context = { referenceMapper, assetMapper: makeAssetMapper() }; + const fields = { items: { referencename: "somelist", fulllist: true } }; + const result = mapper.mapContentFields(fields, context); + expect(result.mappedFields.items).toEqual({ referencename: "somelist", fulllist: true }); + }); + + it("remaps sortids (multi-select) using targetContentID as a comma string", () => { + const referenceMapper = makeReferenceMapper({ + getContentItemMappingByContentID: jest.fn().mockImplementation((id: number) => { + const map: Record = { 11868: 571, 11877: 109 }; + return map[id] ? { targetContentID: map[id] } : null; + }), + }); + const context = { referenceMapper, assetMapper: makeAssetMapper() }; + const fields = { list: { sortids: "11868,11877" } }; + const result = mapper.mapContentFields(fields, context); + expect(result.mappedFields.list.sortids).toBe("571,109"); + }); + }); + describe("cdn URL string fields", () => { it("increments validationErrors for a cdn.aglty.io string field when no context is given (mapAssetUrl throws)", () => { // mapAssetUrl unconditionally accesses context.assetMapper, so passing no context throws, diff --git a/src/lib/pushers/content-pusher/util/collect-content-id-references.ts b/src/lib/pushers/content-pusher/util/collect-content-id-references.ts new file mode 100644 index 0000000..d553228 --- /dev/null +++ b/src/lib/pushers/content-pusher/util/collect-content-id-references.ts @@ -0,0 +1,54 @@ +/** + * Recursively walks content item fields to find SINGLE-ITEM content references — + * `contentid`/`contentID` values and comma-separated `sortids` — and returns the + * referenced source contentIDs. + * + * This complements collectListReferenceNames, which only finds WHOLE-LIST references + * (a `referencename` paired with `fulllist:true`). A single-item linked-content field + * stores `{ contentid: N, fulllist: false }` with no `referencename`, so it is invisible + * to the list-reference collector — which is why the item it points at was never treated + * as a push-order dependency (PROD-2341). + * + * Only positive IDs are returned; 0 / -1 mean "no reference selected" and are ignored so + * we don't promote items with intentionally-empty linked-content fields. This matches the + * `> 0` guard used by collectUnresolvedContentReferences. + */ +export function collectContentIDReferences(fields: any): number[] { + const found: number[] = []; + + function walk(node: any): void { + if (!node) return; + + if (Array.isArray(node)) { + for (const v of node) walk(v); + return; + } + + if (typeof node === "object") { + for (const key of Object.keys(node)) { + const value = (node as any)[key]; + + // Direct single-item reference (contentid / contentID) + if ((key === "contentid" || key === "contentID") && typeof value === "number") { + if (value > 0) found.push(value); + continue; + } + + // Comma-separated content IDs in a sortids field + if (key === "sortids" && typeof value === "string") { + for (const part of value.split(",")) { + const id = parseInt(part.trim()); + if (!isNaN(id) && id > 0) found.push(id); + } + continue; + } + + // Recurse into nested objects/arrays + walk(value); + } + } + } + + walk(fields); + return found; +} diff --git a/src/lib/pushers/content-pusher/util/get-content-item-types.ts b/src/lib/pushers/content-pusher/util/get-content-item-types.ts index cafdce0..b211225 100644 --- a/src/lib/pushers/content-pusher/util/get-content-item-types.ts +++ b/src/lib/pushers/content-pusher/util/get-content-item-types.ts @@ -4,12 +4,16 @@ import { ModelMapper } from "lib/mappers/model-mapper"; import { ContentItemMapper } from "lib/mappers/content-item-mapper"; import { hasValidMappings } from "./has-valid-mappings"; import { collectListReferenceNames } from "./collect-list-reference-names"; +import { collectContentIDReferences } from "./collect-content-id-references"; /** * Classifies content items into normal, linked, and skipped categories. * * Normal items: Top-level items that are not referenced by other items - * Linked items: Items that are referenced via fullList=true in other items' fields + * Linked items: Items that are referenced by other items — either via a whole-list + * reference (fullList=true) or a single-item reference (contentid/sortids). Linked + * items are pushed FIRST so their source→target contentID mapping exists before the + * referencing item's fields are remapped (PROD-2341). * Skipped items: Items without valid container/model mappings */ export function getContentItemTypes( @@ -47,12 +51,14 @@ export function getContentItemTypes( normalSet.add(item.contentID); } - // Find all list references in this item's fields - const referenceNames = collectListReferenceNames(item.fields || {}); - if (referenceNames.length > 0) { + // Find every item this one depends on — whole-list references (by referenceName) + // AND single-item references (by contentID) — and mark them linked (pushed first). + const referencedIds = collectReferencedContentIDs(item, itemsByReferenceName, allItemsById); + if (referencedIds.length > 0) { markReferencedItems( - referenceNames, + referencedIds, itemsByReferenceName, + allItemsById, normalSet, linkedSet, skipped, @@ -95,42 +101,78 @@ function buildItemMaps(contentItems: ContentItem[]): { } /** - * Recursively marks all items referenced by the given reference names as linked. - * Uses a stack-based approach to avoid recursion limits. + * Resolves an item's direct dependency targets to concrete source contentIDs, combining + * both reference kinds: + * - whole-list references (referencename + fulllist:true) → every item sharing that + * referenceName (the full list), looked up via itemsByReferenceName; + * - single-item references (contentid / sortids) → the specific referenced item, only + * when it is present in the current content set (allItemsById). + * + * Returns the referenced items' contentIDs; the referencing item itself is never included. + */ +function collectReferencedContentIDs( + item: ContentItem, + itemsByReferenceName: Map, + allItemsById: Map +): number[] { + const ids: number[] = []; + + // Whole-list references → all items belonging to the referenced list + for (const refName of collectListReferenceNames(item.fields || {})) { + for (const target of itemsByReferenceName.get(refName) || []) { + ids.push(target.contentID); + } + } + + // Single-item references → the specific referenced item, if it is in this push set + for (const contentID of collectContentIDReferences(item.fields || {})) { + if (allItemsById.has(contentID)) { + ids.push(contentID); + } + } + + return ids; +} + +/** + * Recursively marks all items referenced (transitively) by the given contentIDs as linked, + * so they are pushed before the items that reference them. Uses a stack-based approach with + * a visited set to avoid infinite loops on circular references. */ function markReferencedItems( - referenceNames: string[], + referencedIds: number[], itemsByReferenceName: Map, + allItemsById: Map, normalSet: Set, linkedSet: Set, skipped: ContentItem[], containerMapper: ContainerMapper, modelMapper: ModelMapper ): void { - const visitedRefNames = new Set(); - const stack = [...referenceNames]; + const visited = new Set(); + const stack = [...referencedIds]; while (stack.length > 0) { - const refName = stack.pop()!; + const contentID = stack.pop()!; - if (visitedRefNames.has(refName)) continue; - visitedRefNames.add(refName); + if (visited.has(contentID)) continue; + visited.add(contentID); - const items = itemsByReferenceName.get(refName) || []; + const item = allItemsById.get(contentID); + if (!item) continue; // referenced item not in this push set — nothing to promote - for (const item of items) { - if (!hasValidMappings(item, containerMapper, modelMapper)) { - skipped.push(item); - continue; - } + if (!hasValidMappings(item, containerMapper, modelMapper)) { + skipped.push(item); + continue; + } - linkedSet.add(item.contentID); - normalSet.delete(item.contentID); // Remove from normal if it was added there + linkedSet.add(contentID); + normalSet.delete(contentID); // Remove from normal if it was added there - // Recursively process nested references - const nestedRefs = collectListReferenceNames(item.fields || {}); - for (const nestedRef of nestedRefs) { - stack.push(nestedRef); + // Recursively process this item's own dependency targets + for (const nestedId of collectReferencedContentIDs(item, itemsByReferenceName, allItemsById)) { + if (!visited.has(nestedId)) { + stack.push(nestedId); } } } diff --git a/src/lib/pushers/content-pusher/util/tests/collect-content-id-references.test.ts b/src/lib/pushers/content-pusher/util/tests/collect-content-id-references.test.ts new file mode 100644 index 0000000..b75705a --- /dev/null +++ b/src/lib/pushers/content-pusher/util/tests/collect-content-id-references.test.ts @@ -0,0 +1,53 @@ +import { collectContentIDReferences } from "../collect-content-id-references"; + +describe("collectContentIDReferences", () => { + it("returns [] for empty / non-object input", () => { + expect(collectContentIDReferences({})).toEqual([]); + expect(collectContentIDReferences(null)).toEqual([]); + expect(collectContentIDReferences(undefined)).toEqual([]); + }); + + it("collects a single-item contentid reference (the PROD-2341 drawGame shape)", () => { + const fields = { drawGame: { contentid: 11868, fulllist: false } }; + expect(collectContentIDReferences(fields)).toEqual([11868]); + }); + + it("collects the camelCase contentID key as well", () => { + expect(collectContentIDReferences({ ref: { contentID: 42 } })).toEqual([42]); + }); + + it("collects each id from a comma-separated sortids string", () => { + expect(collectContentIDReferences({ list: { sortids: "3, 7, 11" } })).toEqual([3, 7, 11]); + }); + + it("ignores 0 / negative ids (no reference selected)", () => { + expect(collectContentIDReferences({ a: { contentid: 0 }, b: { contentid: -1 } })).toEqual([]); + expect(collectContentIDReferences({ list: { sortids: "0,-1,5" } })).toEqual([5]); + }); + + it("ignores an empty linked-content field with no contentid", () => { + // item 12000 in the source: { fulllist: false } with no contentid + expect(collectContentIDReferences({ drawGame: { fulllist: false } })).toEqual([]); + }); + + it("walks nested objects and arrays", () => { + const fields = { + outer: [{ inner: { contentid: 1 } }, { inner: { contentid: 2 } }], + other: { deep: { deeper: { contentID: 3 } } }, + }; + expect(collectContentIDReferences(fields).sort((a, b) => a - b)).toEqual([1, 2, 3]); + }); + + it("collects both contentid and sortids across a single item's fields", () => { + const fields = { + single: { contentid: 100, fulllist: false }, + multi: { sortids: "200,300" }, + }; + expect(collectContentIDReferences(fields).sort((a, b) => a - b)).toEqual([100, 200, 300]); + }); + + it("does not treat a whole-list reference (referencename + fulllist) as a contentid ref", () => { + // Whole-list refs are handled by collectListReferenceNames, not here. + expect(collectContentIDReferences({ items: { referencename: "somelist", fulllist: true } })).toEqual([]); + }); +}); diff --git a/src/lib/pushers/content-pusher/util/tests/get-content-item-types.test.ts b/src/lib/pushers/content-pusher/util/tests/get-content-item-types.test.ts index 15cebbf..3d32b01 100644 --- a/src/lib/pushers/content-pusher/util/tests/get-content-item-types.test.ts +++ b/src/lib/pushers/content-pusher/util/tests/get-content-item-types.test.ts @@ -223,6 +223,85 @@ describe("getContentItemTypes — linked items", () => { }); }); +// ─── single-item content references (PROD-2341) ─────────────────────────────── + +describe("getContentItemTypes — single-item content references", () => { + it("promotes a contentid-referenced item to linked (the drawGame case) while the referencing item stays normal", () => { + // Mirrors PROD-2341: GameBanner.drawGame = { contentid: , fulllist: false } + const target = makeItem("drawgamesassets", "DrawGameAssetsSchema"); + const gameBanner = makeItem("gamebanner", "GameBanner", { + drawGame: { contentid: target.contentID, fulllist: false }, + }); + + const result = getContentItemTypes([gameBanner, target], makeValidOpts()); + + expect(result.linkedContentItems).toHaveLength(1); + expect(result.linkedContentItems[0]).toBe(target); + expect(result.normalContentItems).toHaveLength(1); + expect(result.normalContentItems[0]).toBe(gameBanner); + // The referenced item must NOT remain in the normal (pushed-second) bucket. + expect(result.normalContentItems).not.toContain(target); + }); + + it("promotes every sortids-referenced item to linked", () => { + const a = makeItem("list-a", "ModelA"); + const b = makeItem("list-b", "ModelB"); + const parent = makeItem("parent", "ModelParent", { + picks: { sortids: `${a.contentID},${b.contentID}` }, + }); + + const result = getContentItemTypes([parent, a, b], makeValidOpts()); + + const linkedIds = result.linkedContentItems.map((i) => i.contentID); + expect(linkedIds).toContain(a.contentID); + expect(linkedIds).toContain(b.contentID); + expect(result.normalContentItems).toHaveLength(1); + expect(result.normalContentItems[0]).toBe(parent); + }); + + it("does not promote anything for an empty linked field (contentid 0 / -1 / absent)", () => { + const emptyObj = makeItem("empty-obj", "ModelEmpty", { drawGame: { fulllist: false } }); + const zero = makeItem("zero-ref", "ModelZero", { drawGame: { contentid: 0, fulllist: false } }); + const neg = makeItem("neg-ref", "ModelNeg", { drawGame: { contentid: -1, fulllist: false } }); + + const result = getContentItemTypes([emptyObj, zero, neg], makeValidOpts()); + + expect(result.linkedContentItems).toHaveLength(0); + expect(result.normalContentItems).toHaveLength(3); + }); + + it("does not crash or promote when the referenced contentid is not in the push set", () => { + const orphanRef = makeItem("orphan", "ModelOrphan", { + drawGame: { contentid: 999999, fulllist: false }, + }); + + const result = getContentItemTypes([orphanRef], makeValidOpts()); + + expect(result.normalContentItems).toHaveLength(1); + expect(result.normalContentItems[0]).toBe(orphanRef); + expect(result.linkedContentItems).toHaveLength(0); + expect(result.skippedItems).toHaveLength(0); + }); + + it("follows a mixed chain: contentid ref whose target then whole-list references another item", () => { + const deep = makeItem("deep-list", "ModelDeep"); + const mid = makeItem("mid", "ModelMid", { + nested: { referenceName: "deep-list", fullList: true }, + }); + const top = makeItem("top", "ModelTop", { + drawGame: { contentid: mid.contentID, fulllist: false }, + }); + + const result = getContentItemTypes([top, mid, deep], makeValidOpts()); + + expect(result.normalContentItems).toHaveLength(1); + expect(result.normalContentItems[0]).toBe(top); + const linkedIds = result.linkedContentItems.map((i) => i.contentID); + expect(linkedIds).toContain(mid.contentID); + expect(linkedIds).toContain(deep.contentID); + }); +}); + // ─── reference to unknown item ──────────────────────────────────────────────── describe("getContentItemTypes — reference to unknown referenceName", () => {