sc-17719 change backfill source for existing patient diagnosis dates #5752
Conversation
| SET diagnosed_confirmed_at = recorded_at | ||
| WHERE diagnosed_confirmed_at IS NULL | ||
| AND recorded_at IS NOT NULL; | ||
| SQL |
There was a problem hiding this comment.
All the existing patients in production have recorded_at present. So, this updating diagnosed_confirmed_at = recorded_at is correct. For existing patients we want to capture when the patient was diagnosed and patient's registration time is the only timestamp present in the current flow, which we can use of.
My opinion - patients registered in the new flow had missing diagnosed_confirmed_at (which is now fixed), after this migration ran, all patients had a valid diagnosed_confirmed_at. So we don't have to remove this part.
There was a problem hiding this comment.
My only concern is around long-term clarity. If existing patients have diagnosed_confirmed_at derived from patient.recorded_at, while newer patients derive it from medical_history.device_created_at, the same field would represent different source semantics across cohorts. This could be confusing for future analysis or debugging unless clearly documented.
There was a problem hiding this comment.
Ok, but for ET, we would have to manually revert this migration, and then re run it again.
I am not sure whether it would cause any more issues or not.
There was a problem hiding this comment.
I think reverting and re-running this migration on ET could be risky. To avoid any issues with migration history, I am thinking is to keep this migration unchanged and handle ET via a one-time, scoped backfill through a script executed safely via the Rails console for existing records. Since the updated code is currently deployed only on ET, we can apply this fix specifically there. For other environments where the code is not yet deployed, the migration will run as intended and handle the backfill correctly when deployed.
| ) | ||
|
|
||
| earliest = [htn_time, dm_time].compact.min | ||
| mh.patient.update_columns(diagnosed_confirmed_at: earliest) if earliest |
There was a problem hiding this comment.
This will not work for all the patients because for some medical_histories hypertension/dm = unknown (in production). For these patients diagnosed_confirmed_at would still remain null.
In my opinion - we don't want that. Instead we want all the existing patients to be treated as a regular diagnosed patient.
Story card: sc-17719
Refer to this Slack thread for more context: Slack-thread
Because
diagnosed_confirmed_atwas remaining nil when medical histories were created whilepatient.recorded_atwas still unset. The old backfill depended onpatient.recorded_at, so any missingrecorded_atcaused diagnosis timestamps (htn/dm) to stay nil.This addresses
Updates the backfill source to use
medical_history.device_created_atto populate diagnosis dates consistently for existing patients.Test instructions
Run the migration and verify diagnosis dates are populated for existing records.