fix(fhir): SAV-901: Set published_date on a Lab Request when published via the FHIR API #7244
Conversation
…d via the FHIR API
rohan-bes
force-pushed
the
fix/sav-901-set-published-date-when-publish-from-fhir
branch
from
fc28b8a to
4ff8392
Compare
| @@ -114,6 +114,9 @@ export class FhirDiagnosticReport extends FhirResource { | |||
|
|
|||
| if (this.shouldUpdateLabRequestStatus(labRequest, newStatus)) { | |||
| labRequest.set({ status: newStatus }); | |||
| if (newStatus === LAB_REQUEST_STATUSES.PUBLISHED) { | |||
| labRequest.set({ publishedDate: new Date() }); | |||
There was a problem hiding this comment.
Hmmmm but publishedDate seems to be a string, so we probably want the getCurrentDateTimeString() thing? Also assuming it's datetime and not just date here Yeah okay it is datetime string by looking at the model heh
There was a problem hiding this comment.
Good call! I swapped this out and the tests still passed so maybe it would get handled anyway but best to be explicit that it's a string 👍
… setting published_date
| expect(labRequest.status).toBe(LAB_REQUEST_STATUSES.PUBLISHED); | ||
| expect(labRequest.publishedDate).toBe(dateFnsTz.format(fakeDate, 'yyyy-MM-dd HH:mm:ss')); | ||
| expect((await labRequest.getLatestAttachment()).title).toBe(testAttachment.title); | ||
| jest.useRealTimers(); |
There was a problem hiding this comment.
Hmmmm is this line really evaluated? I don't know when, but I think I recall that logs past the last expect get ignored? Just double check that it's working I think otherwise we could move it after we get the response hey?
There was a problem hiding this comment.
The jest.useRealTimers()? Yep it does, it clears mock on Date.now(). Without it the tests don't work.
I did it this way (mocking inside just the one test) because it seems like our setup/teardown code for these tests relies on Date.now() so this was a simple way to work around that.
However it's a good point if the expect fails for some reason then maybe it won't evaluate 🤔 so I'll move it after getting the response to be safe 👍
There was a problem hiding this comment.
Okay now I'm confused, I removed the line and it seems like the tests work regardless. I was sure I had tested that previously and it was required... 🤷
There was a problem hiding this comment.
Oh well! Let's keep it for good measure? IDK?
|
Android builds 📱
|
Changes
Previously when a Lab Request is transitioned to
Publishedby an incomingDiagnosticReportwe weren't setting thepublished_date. We should be doing this.Deploys
Remember to...