Skip to content

Commit

Permalink
Fix merged categorized nested content building (#69)
Browse files Browse the repository at this point in the history
  • Loading branch information
@grigasp
grigasp committed Feb 14, 2023
1 parent a12de5d commit 3e24140
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 31 deletions.
7 changes: 7 additions & 0 deletions change/@itwin-presentation-components-53c9f2cb-44bc-4f40-8dbf-edb39fce6278.json
View file
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix failure to load property data when nested properties are placed in root categories different from their parent property category",
"packageName": "@itwin/presentation-components",
"email": "35135765+grigasp@users.noreply.github.com",
"dependentChangeType": "patch"
}
7 changes: 4 additions & 3 deletions packages/components/src/presentation-components/common/ContentBuilder.ts
View file
Expand Up @@ -183,17 +183,18 @@ export abstract class PropertyRecordsBuilder implements IContentVisitor {
}

public processMergedValue(props: ProcessMergedValueProps): void {
const propertyField = props.requestedField;
const value: PrimitiveValue = {
valueFormat: UiPropertyValueFormat.Primitive,
};
const record = new PropertyRecord(
value,
createPropertyDescriptionFromFieldInfo(createFieldInfo(props.mergedField, props.parentFieldName)),
createPropertyDescriptionFromFieldInfo(createFieldInfo(propertyField, props.parentFieldName)),
);
record.isMerged = true;
record.isReadonly = true;
record.autoExpand = props.mergedField.isNestedContentField() && props.mergedField.autoExpand;
this.currentPropertiesAppender.append({ record, fieldHierarchy: { field: props.mergedField, childFields: [] } });
record.autoExpand = propertyField.isNestedContentField() && propertyField.autoExpand;
this.currentPropertiesAppender.append({ record, fieldHierarchy: { field: propertyField, childFields: [] } });
}

public processPrimitiveValue(props: ProcessPrimitiveValueProps): void {
Expand Down
20 changes: 5 additions & 15 deletions packages/components/src/presentation-components/propertygrid/DataProvider.ts
View file
Expand Up @@ -15,8 +15,7 @@ import { IModelConnection } from "@itwin/core-frontend";
import {
addFieldHierarchy, CategoryDescription, ContentFlags, DefaultContentDisplayTypes, Descriptor, DescriptorOverrides, Field, FieldHierarchy,
InstanceKey, NestedContentValue, PropertyValueFormat as PresentationPropertyValueFormat, ProcessFieldHierarchiesProps, ProcessPrimitiveValueProps,
RelationshipMeaning, Ruleset, StartArrayProps, StartCategoryProps, StartContentProps, StartStructProps, traverseContentItem, traverseFieldHierarchy,
Value, ValuesMap,
RelationshipMeaning, Ruleset, StartArrayProps, StartContentProps, StartStructProps, traverseContentItem, traverseFieldHierarchy, Value, ValuesMap,
} from "@itwin/presentation-common";
import { FavoritePropertiesScope, Presentation } from "@itwin/presentation-frontend";
import { FieldHierarchyRecord, IPropertiesAppender, PropertyRecordsBuilder } from "../common/ContentBuilder";
Expand Down Expand Up @@ -309,7 +308,6 @@ class PropertyDataBuilder extends PropertyRecordsBuilder {
private _categoriesCache: PropertyCategoriesCache;
private _categorizedRecords = new Map<string, FieldHierarchyRecord[]>();
private _favoriteFieldHierarchies: FieldHierarchy[] = [];
private _categoriesStack: CategoryDescription[] = [];

constructor(props: PropertyDataBuilderProps) {
super();
Expand All @@ -325,12 +323,7 @@ class PropertyDataBuilder extends PropertyRecordsBuilder {
protected createRootPropertiesAppender(): IPropertiesAppender {
return {
append: (record: FieldHierarchyRecord): void => {
// Note: usually the last category on the stack should be what we want, but in some cases,
// when record's parent is merged, we need another category. In any case it's always expected
// to be on the stack.
const category = this._categoriesStack.find((c) => c.name === record.fieldHierarchy.field.category.name);
assert(category !== undefined);

const category = record.fieldHierarchy.field.category;
let records = this._categorizedRecords.get(category.name);
if (!records) {
records = [];
Expand Down Expand Up @@ -485,12 +478,6 @@ class PropertyDataBuilder extends PropertyRecordsBuilder {
props.hierarchies.push(...this._favoriteFieldHierarchies);
}

public override startCategory(props: StartCategoryProps): boolean {
this._categoriesStack.push(props.category);
return true;
}
public override finishCategory(): void { this._categoriesStack.pop(); }

public override startStruct(props: StartStructProps): boolean {
if (this.shouldSkipField(props.hierarchy.field, () => !Object.keys(props.rawValues).length))
return false;
Expand Down Expand Up @@ -538,6 +525,9 @@ class PropertyCategoriesCache {
constructor(private _enableCategoryNesting: boolean) { }

public initFromDescriptor(descriptor: Descriptor) {
descriptor.categories.forEach((category) => {
this.cache(category);
});
this.initFromFields(descriptor.fields);
}

Expand Down
94 changes: 90 additions & 4 deletions packages/components/src/test/propertygrid/DataProvider.test.snap
View file
Expand Up @@ -6,7 +6,7 @@ Object {
Object {
"expand": false,
"label": "Test Category",
"name": "test-category",
"name": "my-category",
"renderer": Object {
"name": "test",
},
Expand All @@ -26,7 +26,7 @@ Object {
},
},
"records": Object {
"test-category": Array [
"my-category": Array [
PropertyRecord {
"property": Object {
"displayLabel": "Simple Field",
Expand Down Expand Up @@ -87,6 +87,49 @@ Object {
}
`;

exports[`PropertyDataProvider getData with flat categories nested content handling merges parent field when child field's category is different and parent is merged 1`] = `
Object {
"categories": Array [
Object {
"expand": false,
"label": "Test Category",
"name": "Category2",
},
],
"description": undefined,
"label": PropertyRecord {
"property": Object {
"displayLabel": "Label",
"name": "label",
"typename": "string",
},
"value": Object {
"displayValue": "",
"value": "",
"valueFormat": 0,
},
},
"records": Object {
"Category2": Array [
PropertyRecord {
"autoExpand": false,
"isMerged": true,
"isReadonly": true,
"property": Object {
"displayLabel": "Simple Field",
"name": "nested-field-1",
"typename": "string",
},
"value": Object {
"valueFormat": 0,
},
},
],
},
"reusePropertyDataState": true,
}
`;

exports[`PropertyDataProvider getData with flat categories nested content handling moves all nested fields into separate category and hides nested content field when all nested fields are categorized 1`] = `
Object {
"categories": Array [
Expand Down Expand Up @@ -1301,7 +1344,7 @@ Object {
Object {
"expand": false,
"label": "Test Category",
"name": "test-category",
"name": "my-category",
"renderer": Object {
"name": "test",
},
Expand All @@ -1321,7 +1364,7 @@ Object {
},
},
"records": Object {
"test-category": Array [
"my-category": Array [
PropertyRecord {
"property": Object {
"displayLabel": "Simple Field",
Expand Down Expand Up @@ -1382,6 +1425,49 @@ Object {
}
`;

exports[`PropertyDataProvider getData with nested categories nested content handling merges parent field when child field's category is different and parent is merged 1`] = `
Object {
"categories": Array [
Object {
"expand": false,
"label": "Test Category",
"name": "Category2",
},
],
"description": undefined,
"label": PropertyRecord {
"property": Object {
"displayLabel": "Label",
"name": "label",
"typename": "string",
},
"value": Object {
"displayValue": "",
"value": "",
"valueFormat": 0,
},
},
"records": Object {
"Category2": Array [
PropertyRecord {
"autoExpand": false,
"isMerged": true,
"isReadonly": true,
"property": Object {
"displayLabel": "Simple Field",
"name": "nested-field-1",
"typename": "string",
},
"value": Object {
"valueFormat": 0,
},
},
],
},
"reusePropertyDataState": true,
}
`;

exports[`PropertyDataProvider getData with nested categories nested content handling moves all nested fields into separate category and hides nested content field when all nested fields are categorized 1`] = `
Object {
"categories": Array [
Expand Down
40 changes: 31 additions & 9 deletions packages/components/src/test/propertygrid/DataProvider.test.ts
View file
Expand Up @@ -310,6 +310,7 @@ describe("PropertyDataProvider", () => {
it("assigns category renderer", async () => {
const field = createPrimitiveField({
category: createTestCategoryDescription({
name: "my-category",
renderer: { name: "test" },
}),
});
Expand Down Expand Up @@ -797,6 +798,23 @@ describe("PropertyDataProvider", () => {
expect(await provider.getData()).to.matchSnapshot();
});

it("merges parent field when child field's category is different and parent is merged", async () => {
const category1 = createTestCategoryDescription({ name: "Category1" });
const category2 = createTestCategoryDescription({ name: "Category2" });
const nestedField1 = createPrimitiveField({ name: "nested-field-1", category: category2 });
const field = createTestNestedContentField({ name: "root-field", category: category1, nestedFields: [nestedField1] });
const descriptor = createTestContentDescriptor({ categories: [category1, category2], fields: [field] });
const values = {
[field.name]: undefined,
};
const displayValues = {
[field.name]: "*** Varies ***",
};
const record = createTestContentItem({ values, displayValues, mergedFieldNames: [field.name] });
(provider as any).getContent = async () => new Content(descriptor, [record]);
expect(await provider.getData()).to.matchSnapshot();
});

it("moves all nested fields into separate category and hides nested content field when all nested fields are categorized", async () => {
const category1 = createTestCategoryDescription({ name: "Category1" });
const category2 = createTestCategoryDescription({ name: "Category2" });
Expand Down Expand Up @@ -1340,7 +1358,7 @@ describe("PropertyDataProvider", () => {
}]);
});

it("puts properties field parent record into favorites category if property is merged", async () => {
it("puts nested properties field into favorites category when parent field is merged", async () => {
const parentCategory = createTestCategoryDescription({ name: "parent-category", label: "Parent" });
const childCategory = createTestCategoryDescription({ name: "child-category", label: "Child", parent: parentCategory });
const propertiesField = createTestSimpleContentField({ name: "primitive-property", label: "Primitive", category: childCategory });
Expand All @@ -1363,17 +1381,21 @@ describe("PropertyDataProvider", () => {

if (provider.isNestedPropertyCategoryGroupingEnabled) {
const favoritesCategory = data.categories.find((c) => c.name === FAVORITES_CATEGORY_NAME)!;
expect(favoritesCategory.childCategories!.length).to.eq(1);
expect(favoritesCategory.childCategories).to.containSubset([{
label: "Parent",
}]);
expect(data.records[`${FAVORITES_CATEGORY_NAME}-${parentCategory.name}`].length).to.eq(1);
expect(data.records[`${FAVORITES_CATEGORY_NAME}-${parentCategory.name}`]).to.containSubset([{
property: { displayLabel: "Nested Content" },
expect(favoritesCategory).to.containSubset({
childCategories: [{
label: parentCategory.label,
childCategories: [{
label: childCategory.label,
}],
}],
});
expect(data.records[`${FAVORITES_CATEGORY_NAME}-${childCategory.name}`].length).to.eq(1);
expect(data.records[`${FAVORITES_CATEGORY_NAME}-${childCategory.name}`]).to.containSubset([{
property: { displayLabel: propertiesField.label },
}]);
} else {
expect(data.records[FAVORITES_CATEGORY_NAME].length).to.eq(1);
expect(data.records[FAVORITES_CATEGORY_NAME][0].property.displayLabel).to.be.eq(nestedContentField.label);
expect(data.records[FAVORITES_CATEGORY_NAME][0].property.displayLabel).to.eq(propertiesField.label);
}
});

Expand Down

0 comments on commit 3e24140

Please sign in to comment.