Allow element to change its parent or model or both#9303
Conversation
|
Didn't we already deal with this in a past PR? I distinctly remember telling whomever was working on it then to NOT call this "move" - that is the term we use for geometric location. It will confuse everyone! At the time, i remember discussing why "move tree" is a bad idea. What brings this up again? |
|
khanaffan
changed the title
|
This pull request is now in conflicts. Could you fix it @khanaffan? 🙏 |
|
@copilot resolve the merge conflicts in this pull request |
Merge conflicts resolved in commit eee59c2. The two conflicts were:
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
This pull request is now in conflicts. Could you fix it @khanaffan? 🙏 |
|
@copilot resolve the merge conflicts in this pull request |
Merge conflicts resolved. The only conflict was in |
|
This pull request is now in conflicts. Could you fix it @khanaffan? 🙏 |
khanaffan
requested review from
a team,
aruniverse,
ben-polinsky,
dassaf4,
saeeedtorabi and
wgoehrig
as code owners
Add @beta EditTxn.changeElementParent and EditTxn.changeElementModel to move an element to a different parent or model without deleting and re-inserting it. Only the target element is moved; its children are unaffected. Reparenting is restricted to within the same model, and changeElementModel requires a root element whose target model is of the same class. Add the @beta ElementError namespace in core-common for reporting element-operation failures. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
khanaffan
force-pushed
the
affank/move-el
branch
from
8f12b27 to
49033bc
Compare
rschili
left a comment
rschili
left a comment
There was a problem hiding this comment.
Pr description is stale, plus I'm re-reviewing this, withdrawing my earlier approve until I'm finished, a lot has changed.
- Updated comments and documentation to clarify that moving an element's model relocates its entire subtree, preserving the parent-child hierarchy. - Improved validation logic to ensure that any descendant with a Model-scoped code blocks the move, maintaining atomicity. - Added tests to verify the behavior of subtree moves and cache invalidation for affected elements.
|
This pull request is now in conflicts. Could you fix it @khanaffan? 🙏 |
changeElementModel moved an element's subtree between two models but only invalidated the element caches, leaving stale source/target ModelProps in models[_cache] (e.g. model geometry-derived state). Element insert/update/ delete invalidate models[_cache] via Element.onInserted/onUpdated/onDeleted, but the native changeElementModel does not fire those element callbacks, so the wrapper must evict both affected models itself. Adds a regression test verifying both the source and target model cache entries are invalidated after the move. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove redundant blocked/allowed-cases docs from ChangeElementParentProps and ChangeElementModelProps; point to the authoritative EditTxn method docs. - Remove unused _verifyChannel import from IModelDb.ts. - Fix deprecated IModelDb.Elements.changeElementParent/changeElementModel forwarders: @throws ITwinError (not IModelError) and @deprecated in 5.11.0 (these methods are new in this release, not 5.9.0). - Document ElementError.scope and ElementError.Key, and rename the scope from "itwin-element" to "itwin-Element" to match the title-case convention used by other recent ITwinError scopes (itwin-EditTxn, itwin-ServerBasedLocks, ...). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update backend docs and changelog wording so changeElementModel is described as a root-element/subtree move, not a replacement for cross-model reparenting of parented elements. The changelog example now uses separate child/root ids and narrows ElementError wording to the explicit validation cases. Because the native model move updates the whole subtree in bulk, the EditTxn wrapper now verifies exclusive locks for every element in the moved subtree before invoking native. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add targeted tests for: - rejecting self-parenting before it can write a corrupt parent id, - requiring exclusive locks for every element in a subtree model move, - updating source and target geometric model geometryGuids when moving a geometric element between models. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
imodel-native: iTwin/imodel-native#1428
Add
changeElementParentandchangeElementModelmethods to@itwin/core-backendAdds new
@betaAPIs for moving elements between models and/or parents without deleting and re-inserting them. The native implementation automatically handles moving element subtrees (assemblies) — all descendants are moved along with the parent element.Changes
EditTxn.changeElementParent— changes the parent of an element. If the new parent is in a different model, the element's model changes as well. When called on an element with children, all descendants are automatically moved as well (handled by the native implementation). Enforces model type matching, channel verification, and lock checks.EditTxn.changeElementModel— changes the model of an element, making it a root element (no parent) in the new model. When called on an element with children, all descendants are automatically moved as well (handled by the native implementation). Enforces model type matching, channel verification, and lock checks.IModelDb.Elements.changeElementParent— deprecated convenience wrapper that delegates to the implicit transaction. UseEditTxn.changeElementParentinstead within an explicit EditTxn scope.IModelDb.Elements.changeElementModel— deprecated convenience wrapper that delegates to the implicit transaction. UseEditTxn.changeElementModelinstead within an explicit EditTxn scope.ChangeElementParentProps/ChangeElementModelProps— interfaces specifying move parameters (element id, target parent/model).ElementError— new error namespace inITwinCoreErrors.ts(@itwin/core-common) for element operation errors (e.g. model type mismatch).Code scope restrictions
Blocked scopes (will throw):
Model-scoped code — code uniqueness is tied to the source model; moving across models would break uniqueness guarantees.ParentElement-scoped code — code scope is invalidated by reparenting; the scope reference becomes stale.Allowed scopes (move proceeds):
Repository-scoped code — unique across the entire iModel, unaffected by model or parent changes.RelatedElement-scoped code — scope element is independent of the parent/model hierarchy.For elements with blocked code scopes, use delete+insert instead of move.
Validation
ChangeElementParentModel.test.ts) covering single-element moves, subtree moves (assemblies), model type mismatch, circular dependency rejection, self-parenting rejection, blocked code scopes (Model-scoped, ParentElement-scoped), allowed code scopes (Repository-scoped, RelatedElement-scoped, empty), and channel verification.