Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 56 additions & 4 deletions src/lib/content/content-field-mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 "<targetContentID>" 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++;
Expand All @@ -248,14 +267,47 @@ 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(",");
}

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
Expand Down
74 changes: 74 additions & 0 deletions src/lib/content/tests/content-field-mapper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<number, number> = { 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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
92 changes: 67 additions & 25 deletions src/lib/pushers/content-pusher/util/get-content-item-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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<string, ContentItem[]>,
allItemsById: Map<number, ContentItem>
): 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<string, ContentItem[]>,
allItemsById: Map<number, ContentItem>,
normalSet: Set<number>,
linkedSet: Set<number>,
skipped: ContentItem[],
containerMapper: ContainerMapper,
modelMapper: ModelMapper
): void {
const visitedRefNames = new Set<string>();
const stack = [...referenceNames];
const visited = new Set<number>();
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);
}
}
}
Expand Down
Loading
Loading