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

Changed iif to work with the correct context #2842

Merged
merged 2 commits into from
Sep 2, 2024

Conversation

Kasdejong
Copy link
Contributor

@Kasdejong Kasdejong commented Aug 22, 2024

Description

the FP iif function should now work with the correct focus.

Related issues

Resolves #2625

Testing

Extra iif tests.

@Kasdejong
Copy link
Contributor Author

The expected (in hapi and fplab) behaviour seems to be strange here. It is also not well documented and it relies on reflection. For example, on fp-test-patient, where Patient.contained[0].name is an array of 2 names:

  • "Patient.contained[0].name.iif(exists(), 'named', 'unnamed')" is expected to run exists() on the name array and return a single string.
  • "Patient.contained[0].name.iif(use = 'official', given, 'not official')" is expected to run use = 'official' on every individual name element (as if there is a hidden select there), and so it returns an array of strings.

We cannot determine at compile time if name is enumerable, so instead, we would have to mark a method as a method which works on collections on or single elements, and add an overload to iif. I feel like this was not the intention, and I would like to discuss with eg @brianpos about what truly is expected of us.

It is worth noting that we pass the official tests by accident! "Patient.contained[0].name.iif(exists(), 'named', 'unnamed')" throws an error (iif not supported on lists), but still manages to return the correct result, since it only runs on the first element on the list :)

@Kasdejong Kasdejong marked this pull request as draft August 22, 2024 11:44
@Kasdejong Kasdejong marked this pull request as ready for review August 28, 2024 09:59
@Kasdejong Kasdejong changed the title Changed iif to work on collections Changed iif to work with the correct context Aug 28, 2024
@ewoutkramer ewoutkramer merged commit f859b8d into develop Sep 2, 2024
16 checks passed
@ewoutkramer ewoutkramer deleted the feature/FP-iif-context branch September 2, 2024 12:19
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.

Fhirpath iif function context...
2 participants