Skip to content

fix bug around handling inactive codes #303

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.

Sign up for GitHub

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

Merged
grahamegrieve merged 1 commit into from
Oct 5, 2026
Merged

fix bug around handling inactive codes #303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions tests/tx/inactive-codeableconcept.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/**
* $validate-code on a CodeableConcept: the INACTIVE_CONCEPT_FOUND warning must name the coding
* that is actually inactive. It used to name the first coding's code whichever coding was
* inactive - e.g. CVX 197 reported as inactive because a second, inactive coding sat beside it.
*/

const request = require('supertest');
const { getTestApp, shutdownTestApp } = require('./setup');

const SYS = 'http://example.org/fhir/CodeSystem/inactive-cc';
const OTHER = 'http://example.org/fhir/CodeSystem/inactive-cc-other';

const CS = {
resourceType: 'CodeSystem', url: SYS, version: '1.0.0', status: 'active', content: 'complete',
concept: [
{ code: 'codeActive', display: 'Display Active' },
{ code: 'codeInactive', display: 'Display Inactive', property: [{ code: 'inactive', valueBoolean: true }] }
]
};
// a second code system, not in the value set, whose only concept is inactive
const CS_OTHER = {
resourceType: 'CodeSystem', url: OTHER, version: '1.0.0', status: 'active', content: 'complete',
concept: [{ code: 'otherInactive', display: 'Other Inactive', property: [{ code: 'inactive', valueBoolean: true }] }]
};
const VS = {
resourceType: 'ValueSet', url: 'http://example.org/fhir/ValueSet/inactive-cc', version: '1.0.0',
status: 'active', compose: { include: [{ system: SYS }] }
};

describe('INACTIVE_CONCEPT_FOUND names the inactive coding in a CodeableConcept', () => {
let app;

beforeAll(async () => {
app = await getTestApp();
}, 60000);

afterAll(async () => {
await shutdownTestApp();
});

const validate = (coding) => request(app)
.post('/tx/r5/ValueSet/$validate-code')
.set('Content-Type', 'application/json')
.send({ resourceType: 'Parameters', parameter: [
{ name: 'valueSet', resource: VS },
{ name: 'codeableConcept', valueCodeableConcept: { coding } },
{ name: 'tx-resource', resource: CS },
{ name: 'tx-resource', resource: CS_OTHER }
] });

const inactiveIssues = (body) => {
const oo = (body.parameter.find(p => p.name === 'issues') || {}).resource || { issue: [] };
return oo.issue.filter(i => /has a status of/.test(i.details.text));
};
// the inactive parameter describes the returned code only, not whichever coding was inactive
const param = (body, name) => body.parameter.find(p => p.name === name);

test('first coding active, second inactive (same system): names the second', async () => {
const res = await validate([{ system: SYS, code: 'codeActive' }, { system: SYS, code: 'codeInactive' }]);
expect(res.status).toBe(200);
const iss = inactiveIssues(res.body);
expect(iss.length).toBe(1);
expect(iss[0].details.text).toBe("The concept 'codeInactive' has a status of inactive and its use should be reviewed");
expect(iss[0].expression).toEqual(['CodeableConcept.coding[1]']);
expect(param(res.body, 'code').valueCode).toBe('codeActive');
expect(param(res.body, 'inactive')).toBeUndefined();
});

// the later, active coding used to reset the inactive flag, so no warning was produced at all
test('first coding inactive, second active: still warns, naming the first', async () => {
const res = await validate([{ system: SYS, code: 'codeInactive' }, { system: SYS, code: 'codeActive' }]);
expect(res.status).toBe(200);
const iss = inactiveIssues(res.body);
expect(iss.length).toBe(1);
expect(iss[0].details.text).toBe("The concept 'codeInactive' has a status of inactive and its use should be reviewed");
expect(iss[0].expression).toEqual(['CodeableConcept.coding[0]']);
expect(param(res.body, 'code').valueCode).toBe('codeInactive');
expect(param(res.body, 'inactive').valueBoolean).toBe(true);
});

test('second coding inactive and not in the value set: names it, not the first coding', async () => {
const res = await validate([{ system: SYS, code: 'codeActive' }, { system: OTHER, code: 'otherInactive' }]);
expect(res.status).toBe(200);
const iss = inactiveIssues(res.body);
expect(iss.length).toBe(1);
expect(iss[0].details.text).toBe("The concept 'otherInactive' has a status of inactive and its use should be reviewed");
expect(iss[0].expression).toEqual(['CodeableConcept.coding[1]']);
expect(param(res.body, 'inactive')).toBeUndefined();
});
});
61 changes: 45 additions & 16 deletions tx/workers/validate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,9 +586,14 @@ class ValueSetChecker {
if (msg) {
op.addIssue(new Issue('information', 'informational', addToPath(path, 'code'), null, msg, 'process-note'));
}
inactive.value = await cs.isInactive(ctxt.context);
inactive.path = path;
vstatus.value = await cs.getStatus(ctxt.context);
// in a CodeableConcept a later coding must not clear the inactive status an earlier
// coding found, or the warning silently disappears when the inactive coding comes first
if (!inactive.value) {
inactive.value = await cs.isInactive(ctxt.context);
inactive.path = path;
inactive.code = code;
vstatus.value = await cs.getStatus(ctxt.context);
}
}
if (displays !== null) {
await this.worker.listDisplaysFromCodeSystem(displays, cs, ctxt.context);
Expand Down Expand Up @@ -942,6 +947,7 @@ class ValueSetChecker {
let pdisp;
let psys;
let pcode;
let ppath; // the path of the coding that is returned
let pver;


Expand Down Expand Up @@ -1080,6 +1086,7 @@ class ValueSetChecker {
if (pcode === undefined) {
psys = c.system;
pcode = c.code;
ppath = path;
if (ver.value) {
pver = ver.value;
}
Expand Down Expand Up @@ -1202,6 +1209,7 @@ class ValueSetChecker {
if (!this.params.membershipOnly && !inactive.value && await prov.isInactive(ctxt.context)) {
inactive.value = true;
inactive.path = path;
inactive.code = c.code;
const st = await prov.getStatus(ctxt.context);
if (st) {
vstatus.value = st;
Expand Down Expand Up @@ -1357,9 +1365,14 @@ class ValueSetChecker {
}

if (inactive.value) {
result.addParamBool('inactive', inactive.value);
if (vstatus.value && vstatus.value !== 'inactive') {
result.addParamCode('status', vstatus.value);
// the inactive and status parameters describe the code being returned. In a CodeableConcept
// the inactive coding may be a different one (or none was returned at all): it still gets
// its warning below, but the parameters are not claimed for the returned code
if (mode !== 'codeableConcept' || (pcode && inactive.path === ppath)) {
result.addParamBool('inactive', inactive.value);
if (vstatus.value && vstatus.value !== 'inactive') {
result.addParamCode('status', vstatus.value);
}
}
let mpath = inactive.path;
if (!mpath) {
Expand All @@ -1371,7 +1384,9 @@ class ValueSetChecker {
mid1 = 'INACTIVE_CONCEPT_FOUND_ADD';
mm1 = 'inactive';
}
let m = this.worker.i18n.translate(mid1, this.params.HTTPLanguages, [vstatus.value, tcode, mm1]);
// report the code that is actually inactive - in a CodeableConcept that isn't necessarily
// the first coding (tcode), which is what this message used to name
let m = this.worker.i18n.translate(mid1, this.params.HTTPLanguages, [vstatus.value, inactive.code || tcode, mm1]);
msg(m);
op.addIssue(new Issue('warning', 'business-rule', mpath, 'INACTIVE_CONCEPT_FOUND', m, 'code-comment'));
} else if (vstatus.value && vstatus.value.toLowerCase() === 'deprecated') {
Expand Down Expand Up @@ -1565,6 +1580,7 @@ class ValueSetChecker {
if (!this.params.membershipOnly && role !== 'not in') {
inactive.value = true;
inactive.path = path;
inactive.code = code;
if (inactive.value) {
vstatus.value = await cs.getStatus(loc.context);
}
Expand All @@ -1575,6 +1591,7 @@ class ValueSetChecker {
if (role !== 'not in') {
inactive.value = true;
inactive.path = path;
inactive.code = code;
vstatus.value = await cs.getStatus(loc.context);
let msg = this.worker.i18n.translate('STATUS_CODE_WARNING_CODE', this.params.HTTPLanguages, ['not active', code]);
messages.push(msg);
Expand All @@ -1583,9 +1600,12 @@ class ValueSetChecker {
} else {
result = true;
if (role !== 'not in') {
inactive.value = await cs.isInactive(loc.context);
inactive.path = path;
vstatus.value = await cs.getStatus(loc.context);
if (!inactive.value) {
inactive.value = await cs.isInactive(loc.context);
inactive.path = path;
inactive.code = code;
vstatus.value = await cs.getStatus(loc.context);
}
if (vcc !== null) {
if (!vcc.coding) {
vcc.coding = [];
Expand Down Expand Up @@ -1622,6 +1642,7 @@ class ValueSetChecker {
if (!this.params.membershipOnly) {
inactive.value = true;
inactive.path = path;
inactive.code = code;
vstatus.value = await cs.getStatus(loc);
}
} else {
Expand All @@ -1635,9 +1656,12 @@ class ValueSetChecker {
} else if (Extensions.has(cc,'http://hl7.org/fhir/StructureDefinition/valueset-deprecated')) {
op.addIssue(new Issue('warning', 'business-rule', addToPath(path, 'code'), 'CONCEPT_DEPRECATED_IN_VALUESET', this.worker.i18n.translate('CONCEPT_DEPRECATED_IN_VALUESET', this.params.HTTPLanguages, [cs.system(), code, 'deprecated', vs.vurl]), 'code-comment'));
}
inactive.value = await cs.isInactive(loc);
inactive.path = path;
vstatus.value = await cs.getStatus(loc);
if (!inactive.value) {
inactive.value = await cs.isInactive(loc);
inactive.path = path;
inactive.code = code;
vstatus.value = await cs.getStatus(loc);
}
result = true;
return result;
}
Expand Down Expand Up @@ -1686,6 +1710,7 @@ class ValueSetChecker {
if (!this.params.membershipOnly) {
inactive.value = true;
inactive.path = path;
inactive.code = code;
vstatus.value = await cs.getStatus(loc);
}
} else {
Expand All @@ -1698,6 +1723,7 @@ class ValueSetChecker {
if (await cs.isInactive(loc)) {
inactive.value = true;
inactive.path = path;
inactive.code = code;
// only replace a status we already have with a real one - a filter context
// doesn't always carry the status (SNOMED, LOINC), and the assembly reads
// vstatus for the message wording as well as for the status parameter
Expand Down Expand Up @@ -1735,9 +1761,12 @@ class ValueSetChecker {
}
} else {
result = true;
inactive.value = await cs.isInactive(loc.context);
inactive.path = path;
vstatus.value = await cs.getStatus(loc.context);
if (!inactive.value) {
inactive.value = await cs.isInactive(loc.context);
inactive.path = path;
inactive.code = code;
vstatus.value = await cs.getStatus(loc.context);
}
await this.worker.listDisplaysFromCodeSystem(displays, cs, loc.context);
return result;
}
Expand Down
Loading