Hello all… we just had a design call where we were in favor of making changes to the saveCondition API/method (stripping out a lot of business logic that we feel is incorrect).
Changes proposed:
Currently, when updating a condition it clones the condition, (ie instead of updating an existing row in the DB, it creates a new row in the DB that links back to the first row in the DB). If the the clinical status is not changed, it voids the existing row, but if the status is changed, it doesn’t void the existing row. We propose always voiding the existing row. Therefore, you have an audit trail of changes, but there’s always a single non-voided row containing the “most current” information about a condition.
Currently, when updating a condition, if you change the “clinical status”, the end date of the existing row is set to the onset state of the new version of the condition. Additionally, if the updated condition does not have an onset date, the date is set to today. We propose removing this. The saveCondition method should not alter the dates in any way.
Making these changes should help us fix a lot of bugginess in the current Ref App Condition UI, but my main concern is that this may break code others have written around Conditions. Specifically, we’d like to confirm if Bahmni would be affected by the change? This would also make the Condition logic in OpenMRS Core divergence from the Condition functionality EMR-API (where the core functionality was originally harvested from).
About Bahmni. It runs on Core 2.1.x + EMR API 1.24.x, meaning that it uses this, hence suffering from the exact same business logic.
I agree with your conclusions. Is it correct to say that basically conditions should behave as observations do? ‘Updating’ means voiding the old one and creating a new one with the updated data?
I would personally advocate for a move to OpenMRS 3.0’s conditions widget, where CRUD operations would be handled by FHIR2 with the usual update flow that you described.
Thanks @mksd… so we could change the functionality in Core 2.4 or Core 2.5 without breaking Bahmni, as long as by the time Bahmni upgrades to Core 2.4 or 2.5 it has migrated to the 3.0 condition widget and not the “Bahmni Condition UI” (which may rely on the “bad” business logic)?
Just as a small update on this: while the API does seem to do the wrong thing in terms of adding an end date and not voiding the existing row, a substantial amount of the misbehaviour actually seems to be a result of the front-end rather than solely a backend problem. Specifically, it’s the front-end which defaults the onset date to today and basically ends up requiring an onset date; the front-end also sets the end date; finally, the multiple rows is a bit hard to fix solely on the backend as the front-end is actually using the UUID of the active condition as it originally existed when the form was created.
Actually, sorry, to clarify… yeah, I had originally assumed that all of the issues would have been with the front end code, it was only after investigating that I discovered the back end issues… it doesn’t surprise me at all that there are still other front-end issues causing problems.
I raised the back end issues here particularly because they are the ones that required approval/debate as they affect not only the Condition UI but also Bahmni and whatever other new functionality may be built that uses the API.
Anyway, long story short: the Condition UI has never fully worked, so I think you should feel free to rework it as needed.
Fair enough. I only mentioned it because I more or less think I’ve fixed the backend part, but then I went to test and… everything was basically the same, except that the backend now voided the first row for a condition.
A condition - has multiple states - current active, history of etc etc
I would prefer that we don’t mix up with obs style “voided” with the active part. “voided” = false, is not same as “inactive” condition. For a condition, this means that “sleep apnea” is no longer an active condition for this patient. (not the row is voided)
For the above, IMHO, its incorrect to remove the end date. Nor should we use “voided” to mark the condition is inactive. If you want, you can create another row and update the existing row as - “history of” or so,
You may do as you please, but I would request getting the semantic meaning correct and reflect that onto the model. Also little bit consideration of how existing implementation would need to migrate would help!
@mogoodrich just ping the usual suspects on Bahmni side.
@mksd you don’t have to speak for Bahmni Coalition. But you can always bring this up to Bahmni Community!
@angshuonline Thanks for the thoughtful points. Just to clarify a few things:
We aren’t proposing to use voided to determine whether a condition is active or not. For that, we are proposing continuing to use the clinicalStatus field (which can, of course, be ACTIVE, INACTIVE or HISTORY_OF). Instead, we’re proposing to void conditions instead of using the onset and end date to determine which conditions should be displayed.
What happens in the current state is that if we have a condition of “sleep apnea” that we mark inactive, we get two database rows, one for “sleep apnea” marked as INACTIVE with an onset date of today and one for “sleep apnea” marked as ACTIVE but with an end date of today (actually, that’s not what we get, but that seems to be what it was intended we would get).
In the proposed state, instead of setting the end date for the ACTIVE version of sleep apnea, the ACTIVE row will be voided, while the INACTIVE row is left unvoided.
This allows us to free up the onset and end date to (potentially) hold a clinically-relevant time frame data (which is what the corresponding fields are for in FHIR) rather than using them to drive the UI. Since, for instance, a patient might’ve had sleep apnea at their last visit, but on their current visit the condition has cleared itself up; the “date” when that happened, though, is not the date of the visit, but some point between the last two visits.
@angshuonline Personally, I agree with that design; we didn’t want to interfere with the idea of keeping an audit log around from the original proposal. That said, if no one needs or wants the audit log of conditions, it’s certainly easier and cleaner to not do this duplicate-and-void step.
@burke@jteich or others Are there any objections to not keeping a history of conditions in the table?
Yes… @angshuonline, as @ibacher says, we were just trying to preserve auditing functionality that seems to be a requirement in the original Bahmni Condition List as referenced in the link above. If that’s no longer a requirement (or we misinterpreted it) I’m fine abandoning it.
So, in short, I agree with the design from @angshuonline as well…
On the other hand,
We can have a record of “sleep apnea” ACTIVE for a period.
so its perfectly valid to have “sleep apnea” status marked as ACTIVE during a past period. Actually there maybe multiple records indicating such condition during a life time.
One possibility: when we mark it as inactive, we do not create a new record, but the end-date is marked as current-date. so, we would record in DB - with “Sleep apnea” active for a period. Of course, the reporting or client systems would then have to decide whether the condition is active or not based on end date.
The above seems valid to me.
@mogoodrich I will verify in Bahmni.
