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

Fix double conversion of assumed values in ADL1.4 conversion #305

Merged
merged 1 commit into from
May 17, 2021

Conversation

J3173
Copy link
Collaborator

@J3173 J3173 commented May 12, 2021

Assumed values were sometimes double converted in the ADL1.4 -> ADL2 conversion, resulting in invalid archetypes. This PR removes one of those conversions and adds a test.

@codecov
Copy link

codecov bot commented May 12, 2021

Codecov Report

Merging #305 (9642982) into master (f8880c2) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #305      +/-   ##
============================================
+ Coverage     70.01%   70.02%   +0.01%     
- Complexity     6286     6287       +1     
============================================
  Files           627      627              
  Lines         21222    21217       -5     
  Branches       3404     3403       -1     
============================================
- Hits          14858    14857       -1     
+ Misses         4763     4761       -2     
+ Partials       1601     1599       -2     
Impacted Files Coverage Δ Complexity Δ
...dap/archie/adl14/ADL14TermConstraintConverter.java 83.57% <ø> (-0.39%) 53.00 <0.00> (-1.00)
.../nedap/archie/aom/primitives/CTerminologyCode.java 73.33% <0.00%> (+4.44%) 30.00% <0.00%> (+2.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8880c2...9642982. Read the comment docs.

@J3173 J3173 marked this pull request as ready for review May 12, 2021 10:48
@J3173 J3173 requested a review from pieterbos May 12, 2021 10:49
Copy link
Collaborator

@pieterbos pieterbos left a comment

Choose a reason for hiding this comment

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

Looks good, see separate comment for another issue I found.

CAttribute cAttribute = archetype.itemAtPath("/data[id2]/events[id3]/state[id14]/items[id15]/value[id9004]/defining_code");
CTerminologyCode cTerminologyCode = (CTerminologyCode) cAttribute.getChildren().get(0);

assertNull(cTerminologyCode.getAssumedValue().getTerminologyId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

according to https://specifications.openehr.org/releases/BASE/latest/foundation_types.html#_terminology_code_class this must have a value. The correct one would be "local".

I created a separate issue for that, #309, since I do not think it's a good idea to solve this in this PR.

@J3173 J3173 merged commit 860715f into master May 17, 2021
@J3173 J3173 deleted the adl14-assumed-value branch May 18, 2021 09:09
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