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

Flatencoding parsing: automatically handle date/time/date_time precision #352

Merged
merged 2 commits into from
Apr 27, 2022

Conversation

mhenke-vg
Copy link
Contributor

Make the implementation handle input that does not fit perfectly by taking the parts that can be used

e.g. DateTime input into Date field only takes the date part

@mhenke-vg mhenke-vg requested a review from stefanspiska April 8, 2022 12:36
Copy link
Contributor

@stefanspiska stefanspiska left a comment

Choose a reason for hiding this comment

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

can you please add test.

In fact there are test for this bug in org.ehrbase.conformance_test.extern.tests.SettersTest wich you could simple enable by remover the overwrite
also you have Errors in you tesst

@mhenke-vg mhenke-vg force-pushed the automatically-handle-date-time-precision branch 2 times, most recently from bdd19ec to dd653eb Compare April 13, 2022 10:57
@serefarikan
Copy link

@mhenke-vg @stefanspiska This is surely an interesting one. May I be the devil's advocate and remind that according to openEHR RM, there is not a IS_A relationship between Date and DateTime types. If I was the developer using the flat encoding, I'd expect a DateTime input into a Date field to fail because of this.

If a DateTime is provided, at least as per the RM, it means the time is significant, and I'd say it'd have to be valid or invalid as a whole. What if someone is recording the time some medication was administered at 15:00 PM during the day, and the next dose must be taken at 23:00 PM. If the conversion to Date falls back to mid day for the time component, then the patient would be given the medication at 20:00, and would risk having a stomach bleeding etc..

Apologies if I misunderstood things, just thought I'd raise the point :)

@mhenke-vg mhenke-vg force-pushed the automatically-handle-date-time-precision branch 2 times, most recently from 4cf5524 to 55b71b1 Compare April 13, 2022 11:31
@mhenke-vg mhenke-vg force-pushed the automatically-handle-date-time-precision branch from 55b71b1 to 503b2bc Compare April 21, 2022 14:24
@mhenke-vg mhenke-vg force-pushed the automatically-handle-date-time-precision branch from 503b2bc to e0bf6bb Compare April 22, 2022 08:06
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@stefanspiska stefanspiska merged commit 9b1db79 into develop Apr 27, 2022
@stefanspiska stefanspiska deleted the automatically-handle-date-time-precision branch April 27, 2022 09:40
@freshehr
Copy link

For what it is worth, I partly agree with @serefarikan here. The original archetype designer will have selected DV_DATE for a reason, and it feels as if there might be some potential risk if a full DateTime is being automatically curtailed to to Date inside the CDR. I do think it is probably valid to do so in the majority of cases but I'd have preferred for this to be checked first for clinical safety and any safe transform applied outside the CDR.

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.

4 participants