classIdPresentInChange should be true when ecClassId is present #6341
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@khanaffan Could you review this change? |
khanaffan
approved these changes
Jan 4, 2024
pmconne
approved these changes
Jan 5, 2024
There was a problem hiding this comment.
Keep an eye on apparent test flakery. I requeud the builds - all three had failed, each differently, one in a previously-flaky test that was recently fixed.
EDIT: The failure below recurred on both Windows and Linux (iOS failed too, but its XML output is too tedious to parse, didn't check).
1) Changeset Reader API
Changeset reader / EC adaptor:
AssertionError: expected 6 to equal 4
+ expected - actual
-6
+4
at Context.<anonymous> (src\test\standalone\ChangesetReader.test.ts:229:14)
|
This test isn't flaky and is a direct result of my change it seems |
|
@Mergifyio backport release/4.3.x |
✅ Backports have been created
|
mergify bot
pushed a commit
that referenced
this pull request
Jan 9, 2024
Co-authored-by: Affan Khan <khanaffan@users.noreply.github.com> Co-authored-by: affank <khanaffan@gmail.com> (cherry picked from commit cc73874)
nick4598
added a commit
to iTwin/imodel-transformer
that referenced
this pull request
Jan 12, 2024
…changed (#135) Keeping the PR as draft at the moment, because waiting on a change from Affan to the changesetECAdaptor to not error out when there is no ECClassId and instead fall back to the ecclassid of the root class. The above scenario occurs when the element is no longer present in the database (because it was deleted in some future changeset that isn't currently being processed) so it can't be queried for its ecclassid. EDIT: In 4.3.3 there is a different issue with classIdPresentInChange. See below PR iTwin/itwinjs-core#6341 --------- Co-authored-by: Michael Belousov <mike.belousov@bentley.com>
nick4598
added a commit
to iTwin/imodel-transformer
that referenced
this pull request
Nov 20, 2024
…changed (#135) Keeping the PR as draft at the moment, because waiting on a change from Affan to the changesetECAdaptor to not error out when there is no ECClassId and instead fall back to the ecclassid of the root class. The above scenario occurs when the element is no longer present in the database (because it was deleted in some future changeset that isn't currently being processed) so it can't be queried for its ecclassid. EDIT: In 4.3.3 there is a different issue with classIdPresentInChange. See below PR iTwin/itwinjs-core#6341 --------- Co-authored-by: Michael Belousov <mike.belousov@bentley.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Without this change I get a ChangedECInstance which has no ECClassId or fallbackClassId
With the change a classId is present:
