Skip to content

Commit 4a46024

Browse files
grigaspmergify[bot]
grigasp
authored and
mergify[bot]
committed
Presentation: Fix content traverser (createContentTraverser) creating invalid fields hierarchy when array properties are nested under NestedContentField (#9188)
(cherry picked from commit 6d5761e)
1 parent 3c336dc commit 4a46024

3 files changed

Lines changed: 177 additions & 20 deletions

File tree

‎common/changes/@itwin/presentation-common/presentation-fix-content-traverser-failing-to-handle-array-properties_2026-04-10-08-23.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@itwin/presentation-common",
5+
"comment": "Fix content traverser (`createContentTraverser`) creating invalid fields hierarchy when array properties are nested under `NestedContentField`.",
6+
"type": "none"
7+
}
8+
],
9+
"packageName": "@itwin/presentation-common"
10+
}

‎presentation/common/src/presentation-common/content/ContentTraverser.ts‎

Lines changed: 66 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,9 @@ import { LabelDefinition } from "../LabelDefinition.js";
1111
import { CategoryDescription } from "./Category.js";
1212
import { Content } from "./Content.js";
1313
import { Descriptor } from "./Descriptor.js";
14-
import { Field, NestedContentField } from "./Fields.js";
14+
import { ArrayPropertiesField, Field, NestedContentField, PropertiesField, StructPropertiesField } from "./Fields.js";
1515
import { Item } from "./Item.js";
16+
import { Property } from "./Property.js";
1617
import { PropertyValueFormat, TypeDescription } from "./TypeDescription.js";
1718
import {
1819
DisplayValue,
@@ -410,9 +411,12 @@ function traverseContentItemField(visitor: IContentVisitor, fieldHierarchy: Fiel
410411
try {
411412
const rootToThisField = createFieldPath(fieldHierarchy.field);
412413
let parentFieldName: string | undefined;
413-
const pathUpToField = rootToThisField.slice(undefined, -1);
414+
const pathUpToField: NestedContentField[] = rootToThisField.slice(undefined, -1).map((f) => {
415+
assert(f.isNestedContentField());
416+
return f;
417+
});
414418
for (let i = 0; i < pathUpToField.length; ++i) {
415-
const parentField = pathUpToField[i] as NestedContentField;
419+
const parentField = pathUpToField[i];
416420
const nextField = rootToThisField[i + 1];
417421

418422
if (item.isFieldMerged(parentField.name)) {
@@ -434,25 +438,19 @@ function traverseContentItemField(visitor: IContentVisitor, fieldHierarchy: Fiel
434438
return;
435439
}
436440

441+
let valuesAccessor = fieldHierarchy.field.name;
437442
if (fieldHierarchy.field.isNestedContentField()) {
438443
fieldHierarchy = convertNestedContentFieldHierarchyToStructArrayHierarchy(fieldHierarchy, parentFieldName);
439444
const { emptyNestedItem, convertedItem } = convertNestedContentFieldHierarchyItemToStructArrayItem(item, fieldHierarchy);
440445
if (emptyNestedItem) {
441446
return;
442447
}
443448
item = convertedItem;
449+
valuesAccessor = fieldHierarchy.field.name;
444450
} else if (pathUpToField.length > 0) {
445451
fieldHierarchy = {
446452
...fieldHierarchy,
447-
field: (function () {
448-
const clone = fieldHierarchy.field.clone();
449-
clone.type = {
450-
valueFormat: PropertyValueFormat.Array,
451-
typeName: `${fieldHierarchy.field.type.typeName}[]`,
452-
memberType: fieldHierarchy.field.type,
453-
};
454-
return clone;
455-
})(),
453+
field: new NestedContentArrayPropertiesField(fieldHierarchy.field, pathUpToField[pathUpToField.length - 1]),
456454
};
457455
}
458456

@@ -462,8 +460,8 @@ function traverseContentItemField(visitor: IContentVisitor, fieldHierarchy: Fiel
462460
item.mergedFieldNames,
463461
fieldHierarchy.field.type,
464462
parentFieldName,
465-
item.values[fieldHierarchy.field.name],
466-
item.displayValues[fieldHierarchy.field.name],
463+
item.values[valuesAccessor],
464+
item.displayValues[valuesAccessor],
467465
);
468466
} finally {
469467
visitor.finishField();
@@ -744,7 +742,13 @@ export const FIELD_NAMES_SEPARATOR = "$";
744742
* @public
745743
*/
746744
export function combineFieldNames(fieldName: string, parentFieldName?: string) {
747-
return parentFieldName ? `${parentFieldName}${FIELD_NAMES_SEPARATOR}${fieldName}` : fieldName;
745+
if (parentFieldName) {
746+
if (fieldName) {
747+
return `${parentFieldName}${FIELD_NAMES_SEPARATOR}${fieldName}`;
748+
}
749+
return parentFieldName;
750+
}
751+
return fieldName;
748752
}
749753
/**
750754
* Parses given combined field names string, constructed using [[combineFieldNames]], into a list of individual field names.
@@ -876,3 +880,50 @@ function convertNestedContentFieldHierarchyItemToStructArrayItem(item: Readonly<
876880
});
877881
return { emptyNestedItem: false, convertedItem };
878882
}
883+
884+
/**
885+
* A synthetic field that extends `NestedContentField` and adds `ArrayPropertiesField` /
886+
* `PropertiesField` surface so that consumers who narrow via `isNestedContentField()`,
887+
* `isPropertiesField()`, and `isArrayPropertiesField()` get a genuine object.
888+
*
889+
* It's needed to properly propagate properties of nested content fields when they're converted
890+
* into struct/array fields, so that consumers can use those properties when processing values.
891+
*/
892+
class NestedContentArrayPropertiesField extends NestedContentField {
893+
public readonly properties: Property[];
894+
public readonly itemsField: Field;
895+
896+
public constructor(sourceItemsField: Field, parentNestedField: NestedContentField) {
897+
super({
898+
...parentNestedField,
899+
...sourceItemsField,
900+
name: "",
901+
type: {
902+
valueFormat: PropertyValueFormat.Array,
903+
typeName: `${sourceItemsField.type.typeName}[]`,
904+
memberType: sourceItemsField.type,
905+
},
906+
});
907+
this.properties = sourceItemsField.isPropertiesField() ? sourceItemsField.properties : [];
908+
this.itemsField = sourceItemsField.clone();
909+
this.itemsField.resetParentship();
910+
}
911+
912+
public override isPropertiesField(): this is PropertiesField {
913+
return true;
914+
}
915+
916+
public isArrayPropertiesField(): this is ArrayPropertiesField {
917+
return true;
918+
}
919+
920+
public isStructPropertiesField(): this is StructPropertiesField {
921+
return false;
922+
}
923+
924+
public override clone() {
925+
const clone = new NestedContentArrayPropertiesField(this.itemsField.clone(), this);
926+
clone.rebuildParentship(this.parent);
927+
return clone;
928+
}
929+
}

‎presentation/common/src/test/content/ContentTraverser.test.ts‎

Lines changed: 101 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import {
3636
createTestSimpleContentField,
3737
createTestStructPropertiesContentField,
3838
} from "../_helpers/Content.js";
39-
import { createTestECInstanceKey } from "../_helpers/EC.js";
39+
import { createTestECInstanceKey, createTestPropertyInfo } from "../_helpers/EC.js";
4040

4141
describe("ContentTraverser", () => {
4242
class TestContentVisitor implements IContentVisitor {
@@ -543,7 +543,7 @@ describe("ContentTraverser", () => {
543543
expect(startArraySpy.firstCall.firstArg).to.containSubset({
544544
hierarchy: {
545545
field: {
546-
name: primitiveField.name,
546+
name: "",
547547
},
548548
},
549549
valueType: {
@@ -575,6 +575,102 @@ describe("ContentTraverser", () => {
575575
expect(finishArraySpy).to.be.calledOnce;
576576
});
577577

578+
it("processes array value nested under nested content item as array-of-arrays value", () => {
579+
const startArraySpy = sinon.spy(visitor, "startArray");
580+
const finishArraySpy = sinon.spy(visitor, "finishArray");
581+
const processPrimitiveValueSpy = sinon.spy(visitor, "processPrimitiveValue");
582+
const nestedContentCategory = createTestCategoryDescription({ name: "nested-content", label: "Nested content" });
583+
const arrayItemField = createTestPropertiesContentField({
584+
name: "ArrayItemField",
585+
category: nestedContentCategory,
586+
properties: [{ property: createTestPropertyInfo({ name: "ArrayProperty", type: "string" }) }],
587+
type: { valueFormat: PropertyValueFormat.Primitive, typeName: "string" },
588+
});
589+
const arrayField = createTestArrayPropertiesContentField({
590+
name: "ArrayField",
591+
category: nestedContentCategory,
592+
properties: [{ property: createTestPropertyInfo({ name: "ArrayProperty", type: "string" }) }],
593+
type: {
594+
valueFormat: PropertyValueFormat.Array,
595+
typeName: `${arrayItemField.type.typeName}[]`,
596+
memberType: arrayItemField.type,
597+
},
598+
itemsField: arrayItemField,
599+
});
600+
const rootField = createTestNestedContentField({
601+
name: "RootField",
602+
category: createTestCategoryDescription({ name: "root", label: "Root category" }),
603+
nestedFields: [arrayField],
604+
});
605+
const descriptor = createTestContentDescriptor({ fields: [rootField] });
606+
const item = createTestContentItem({
607+
values: {
608+
[rootField.name]: [
609+
{
610+
primaryKeys: [createTestECInstanceKey()],
611+
values: {
612+
[arrayField.name]: ["value1"],
613+
},
614+
displayValues: {
615+
[arrayField.name]: ["display value 1"],
616+
},
617+
mergedFieldNames: [],
618+
},
619+
],
620+
},
621+
displayValues: {
622+
[rootField.name]: undefined,
623+
},
624+
});
625+
const traverser = createContentTraverser(visitor, descriptor);
626+
traverser([item]);
627+
628+
expect(startArraySpy).to.be.calledTwice;
629+
expect(startArraySpy.firstCall.firstArg).to.containSubset({
630+
hierarchy: {
631+
field: {
632+
name: "",
633+
itemsField: {
634+
name: arrayField.name,
635+
type: arrayField.type,
636+
},
637+
type: {
638+
valueFormat: PropertyValueFormat.Array,
639+
typeName: `${arrayField.type.typeName}[]`,
640+
memberType: arrayField.type,
641+
},
642+
},
643+
},
644+
valueType: {
645+
valueFormat: PropertyValueFormat.Array,
646+
typeName: `${arrayField.type.typeName}[]`,
647+
memberType: arrayField.type,
648+
},
649+
parentFieldName: rootField.name,
650+
});
651+
expect(startArraySpy.secondCall.firstArg).to.containSubset({
652+
hierarchy: {
653+
field: {
654+
name: arrayField.name,
655+
type: arrayField.type,
656+
},
657+
},
658+
valueType: arrayField.type,
659+
parentFieldName: `${rootField.name}`,
660+
});
661+
expect(processPrimitiveValueSpy).to.be.calledOnce;
662+
expect(processPrimitiveValueSpy.firstCall.firstArg).to.containSubset({
663+
field: {
664+
name: arrayItemField.name,
665+
},
666+
valueType: arrayItemField.type,
667+
parentFieldName: `${rootField.name}${FIELD_NAMES_SEPARATOR}${arrayField.name}`,
668+
rawValue: "value1",
669+
displayValue: "display value 1",
670+
});
671+
expect(finishArraySpy).to.be.calledTwice;
672+
});
673+
578674
it("processes nested content item as struct array value", () => {
579675
const startArraySpy = sinon.spy(visitor, "startArray");
580676
const finishArraySpy = sinon.spy(visitor, "finishArray");
@@ -794,7 +890,7 @@ describe("ContentTraverser", () => {
794890
expect(startArraySpy).to.be.calledOnce;
795891
expect(startArraySpy.firstCall.firstArg).to.containSubset({
796892
hierarchy: {
797-
field: { name: primitiveField.name },
893+
field: { name: "" },
798894
childFields: [],
799895
},
800896
valueType: {
@@ -1524,15 +1620,15 @@ describe("ContentTraverser", () => {
15241620
[childPrimitiveField.name]: "ChildPrimitiveValue",
15251621
},
15261622
displayValues: {
1527-
[childNestedContentField.name]: "ChildPrimitiveDisplayValue",
1623+
[childPrimitiveField.name]: "ChildPrimitiveDisplayValue",
15281624
},
15291625
mergedFieldNames: [],
15301626
} satisfies NestedContentValue,
15311627
],
15321628
},
15331629
displayValues: {
15341630
[parentPrimitiveField.name]: "ChildPrimitiveDisplayValue",
1535-
[childNestedContentField.name]: "ChildNestedContentDisplayValue",
1631+
[childNestedContentField.name]: undefined,
15361632
},
15371633
mergedFieldNames: [],
15381634
} satisfies NestedContentValue,

0 commit comments

Comments
 (0)