hapifhir / hapi-fhir

🔥 HAPI FHIR - Java API for HL7 FHIR Clients and Servers
http://hapifhir.io
Apache License 2.0
2.05k stars 1.34k forks source link

ValueSet.compose.include.version is not set when uploading terminology #2995

Closed jmarchionatto closed 3 years ago

jmarchionatto commented 3 years ago

Specification (https://www.hl7.org/fhir/valueset.html) indicates ValueSet.compose.include.version should indicate pointed CodeSystem version, but this is not set when uploading terminology, which could lead to errors when expanding ValueSet.

This problem become noticeable while implementing the capability of uploading a not-current terminology.

Version: HAPI FHIR 5.5.1

jamesagnew commented 3 years ago

@jmarchionatto It's worth noting that the spec does not say that ValueSet.compose.include.version should indicate the version, only that it can be used for this purpose.

Using versioned CS links from a VS is useful in some cases (e.g. a term server where multiple versions of LOINC will be used - I'm guessing that's the use case you have in mind) but I wouldn't expect it to be the default across the board.

Can you elaborate on the specific changes intended for this ticket?

jmarchionatto commented 3 years ago

@jamesagnew The changes intend to fix the fact that expansion used the include.version to pick the CodeSystem version and when it is not present it chooses the last uploaded, which I think was not correct but was less noticeable before implementing non-current versions, but will have consequences on expansion after this implementation. Actually the non-current version tests discovered this problem

from BaseTermReadSvcImpl.expandValueSetHandleIncludeOrExcludeUsingDatabase (line 899) :

        String includeOrExcludeVersion = theIncludeOrExclude.getVersion();
        TermCodeSystemVersion csv;
        if (isEmpty(includeOrExcludeVersion)) {
            csv = theCs.getCurrentVersion();
        } else {
            csv = myCodeSystemVersionDao.findByCodeSystemPidAndVersion(theCs.getPid(), includeOrExcludeVersion);
        }
jamesagnew commented 3 years ago

@jmarchionatto ah ok, yeah that makes a lot of sense

jmarchionatto commented 3 years ago

I am adding the fix to loinc handlers, as non-current versions are being implemented for loinc only.

jmarchionatto commented 3 years ago

Merged into master by https://github.com/hapifhir/hapi-fhir/pull/3001