RelatedElement vs Id64String JS Props mismatch

[RohitPtnkr1996]

ViewDefinition and it's subclasses have properties like categorySelectorId, displayStyleId, modelSelectorId etc that are defined as Id64String properties.
However, the BisCore schema describes them as navigation properties.
When a user queries these properties like "SELECT $ FROM bis.Element OPTIONS USE_JS_PROP_NAMES", the ECSql row returned contains a RelatedElement for categorySelector.
When round tripping such an element, a call to constructEntity with the props returned by the query will fail as the props expect and Id64String and not a nav prop.
This necessitates a further "Js-ify" step to convert the props into acceptable values.

This PR has deprecated these "..Id" props and added an optional RelatedElement nav prop in it's place.
In the classes implementing these props, the property has been converted into a getter-setter pair to reduce the breaking change impact.
The transformer team has implemented such a function, but thay have hard-coded checks for the class names to replace and this relies on uniqueness of prop names in BIS.

Fixes: https://github.com/iTwin/itwinjs-backlog/issues/960

P.S.: There are many other places where an Id is used instead of a nav prop in a class/interface, for example: ElementProps: model, ModelProps: parentModel.
But this code is deeply embedded and will be very disruptive, so I have let it as is.

[Copilot]

Pull request overview

This PR addresses a mismatch between BIS navigation properties and the JS ...Id props for ViewDefinition (and subclasses) that breaks round-tripping when querying with OPTIONS USE_JS_PROP_NAMES, by introducing RelatedElementProps navigation props and deprecating the ...Id props.

Changes:

• Added optional navigation props (categorySelector, displayStyle, modelSelector, baseModel) to view-related props and updated frontend/backend JSON serialization to populate them.
• Updated backend ViewDefinition classes to accept either nav props or deprecated ...Id props (via getter/setter compatibility) and updated ViewStore/IModelDb to read from either.
• Updated/added tests covering ViewStore and ECSQL→JS-prop round-tripping for the affected properties.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 15 comments.

Show a summary per file

File	Description
full-stack-tests/backend/src/integration/ViewStore.test.ts	Updates integration tests to use new navigation props when constructing view definition props.
core/frontend/src/ViewState.ts	Populates categorySelector, displayStyle, and baseModel nav props in toJSON; adds fallback for baseModel.
core/frontend/src/SpatialViewState.ts	Populates modelSelector nav prop in toJSON.
core/common/src/ViewProps.ts	Adds optional nav props and deprecates ...Id props for view definitions and subclasses.
core/backend/src/test/standalone/ViewDefinition.test.ts	Updates backend tests to validate the new nav props and preserve backward compatibility expectations.
core/backend/src/test/imodel/IModel.test.ts	Updates assertions to check modelSelector?.id instead of modelSelectorId.
core/backend/src/test/element/ElementRoundTrip.test.ts	Adds ECSQL USE_JS_PROP_NAMES round-trip tests for the new nav props and deprecated ids.
core/backend/src/ViewStore.ts	Updates ViewStore validation/storage paths to accept nav props as alternatives to deprecated ids.
core/backend/src/ViewDefinition.ts	Adds nav props to backend ViewDefinition classes and updates (de)serialization/toJSON to support both forms.
core/backend/src/IModelDb.ts	Updates view state loading to prefer nav props when present (with deprecated-id fallback).

Comments suppressed due to low confidence (1)

core/backend/src/ViewDefinition.ts:803

• ViewDefinition2d.serialize writes the deprecated baseModelId onto the ECSqlRow (inst.baseModelId). If the EC schema expects only the baseModel navigation property, emitting baseModelId risks writing a non-existent property. Consider excluding baseModelId via _customHandledProps and only setting inst.baseModel (using props.baseModel ?? { id: props.baseModelId }).

  public static override serialize(props: ViewDefinition2dProps, _iModel: IModelDb): ECSqlRow {
    const inst = super.serialize(props, _iModel);
    // eslint-disable-next-line @typescript-eslint/no-deprecated
    inst.baseModel = props.baseModel ?? { id: props.baseModelId };
    // eslint-disable-next-line @typescript-eslint/no-deprecated
    inst.baseModelId = props.baseModelId;
    inst.origin = props.origin;
    inst.extents = props.delta;
    inst.rotationAngle = props.angle;
    return inst;

[markschlosseratbentley]

@RohitPtnkr1996 @aruniverse While I provided a review, I do not believe I should be the only one determining if stuff relating to BisCore schemas should merge in. I suggest getting another reviewer to also approve before merging.

[mergify]

This pull request is now in conflicts. Could you fix it @RohitPtnkr1996? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

[MichaelSwigerAtBentley]

P.S.: There are many other places where an Id is used instead of a nav prop in a class/interface, for example: ElementProps: model, ModelProps: parentModel.
But this code is deeply embedded and will be very disruptive, so I have let it as is.

We should open an issue to track this if it is something we want to change. And flag for a potential 6.0 fix