-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add test to reparent subjects involved in team->shared transformations #163
base: main
Are you sure you want to change the base?
Conversation
This reverts commit d31f687.
…ceDb (#140) Co-authored-by: Michael Belousov <[email protected]>
Also documented targetScopeProvenanceJsonProps some more and added a test
throw when danglingReferencesBehavior is reject
rootElementPropagateChanges/skipBranchProvInitializeCs Skip over repositorymodel in the remap case, remove some comments
I removed the fixme about structured testing and added it to work item: #155. Removed transformerresumption API and fixmes associated with it.
…an be modified by branch B and properly propagated back to branch A (#158)
As #149 Make root subject special cased when remapping from source to target Fixed the bug on transformer side instead of native side as transformer is the only consumer of the corresponding native code
…b is not at the tip. (#157)
transformer = new IModelTransformer(teamIModelDb, sharedIModelDb, { | ||
danglingReferencesBehavior: "ignore", | ||
}); | ||
// 1) IntermediateSubject in both team and shared have parentid of root subject. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section and my large comment seem the most important to discuss. I'm not really sure what to do about this, but it doesn't really matter that much at the moment anyways since we're mostly focused on syncing in one direction only at the moment.
@@ -1526,24 +1526,24 @@ export class IModelTransformer extends IModelExportHandler { | |||
sourceElement, | |||
{ binaryGeometry: this._options.cloneUsingBinaryGeometry } | |||
); | |||
|
|||
const targetElementId = this.context.findTargetElementId(sourceElement.id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also wonder, should all of this logic live in onExportElement instead?
Right now we probably only get the targetElementId if someone explicitly remapped it (I think) but its possible that rootSubject could be remapped to a nonRootSubject by virtue of having the same fedguid due to some prior explicit remapping. Not sure if we should even allow a scenario like this though and we should probably just always expect people to setup their remappings if they are trying to do some root subject to non root subject transform.
EDIT: I could probably write a test to confirm this if you're not sure.
No description provided.