Skip to content
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

Don't add SiblingOrder when specialising the first element of an archetype #604

Merged

Conversation

VeraPrinsen
Copy link
Collaborator

@VeraPrinsen VeraPrinsen commented Jun 6, 2024

Example of the bug:

flat parent archetype:

CLUSTER
   ELEMENT[id2]
   ELEMENT[id4]

flat child archetype:

CLUSTER
   ELEMENT[id2]
   ELEMENT[id2.1]
   ELEMENT[id4]

Expected differentiated archetype:

CLUSTER
   ELEMENT[id2.1]

What comes out of the differentiator at the moment:

CLUSTER
   after[id2]
   ELEMENT[id2.1]

This gives errors when flattening the archetype, and puts the specialised id2.1 element last instead of at the place of the original id2 element.

Problem was in the 'handleDirectlyAfterSameParentNode' method where the parent id code was found, but because the for loop ended, the method still returned 'false', even though there was a parent node that this specialised element could go right after.

@VeraPrinsen VeraPrinsen requested a review from MattijsK June 6, 2024 10:45
@@ -13,7 +13,6 @@ description
definition
CLUSTER[id1.1] matches { -- Lipid studies panel
items matches {
after [id3]
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the flattener, specialised nodes will appear after their parent if there is no sibling order attacked to the specialised nodeId

Copy link

codecov bot commented Jun 11, 2024

Codecov Report

Attention: Patch coverage is 75.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 71.80%. Comparing base (e36d98e) to head (d98ac4f).

Files Patch % Lines
...in/java/com/nedap/archie/diff/LCSOrderingDiff.java 75.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #604      +/-   ##
============================================
- Coverage     71.80%   71.80%   -0.01%     
+ Complexity     6956     6955       -1     
============================================
  Files           663      663              
  Lines         22691    22690       -1     
  Branches       3676     3676              
============================================
- Hits          16294    16292       -2     
  Misses         4664     4664              
- Partials       1733     1734       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MattijsK MattijsK merged commit a40cee6 into master Jun 17, 2024
4 checks passed
@MattijsK MattijsK deleted the dont_add_siblingorder_when_specialising_first_element branch June 17, 2024 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants