-
Notifications
You must be signed in to change notification settings - Fork 17
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.
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
added new sample statuses #311
Conversation
<div class="barcode-col barcode-col-end" id="btn-view-{{ sample.sample_id }}"> | ||
<span class="sample-label">{{ _('Not Received Yet') }}</span> | ||
</div> | ||
{% if sample.latest_scan_timestamp is none %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are a couple of variable names that need to be adjusted here. In Sample.to_api() in microsetta-private-api, you added sample_latest_scan_timestamp
and sample_last_update
but this block of code is expecting latest_scan_timestamp
and last_update
.
<div class="barcode-col barcode-col-end" id="btn-view-{{ sample.sample_id }}"> | ||
<span class="sample-label">{{ _('Sample Received - Information Needed ') }} </span> | ||
</div> | ||
{% elif sample.latest_scan_timestamp > sample.last_update %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The differentiation of 'Information Needed' and 'Information Still Needed' isn't important from a UI perspective. Let's wrap this condition into the above (elif sample.last_update is none or sample.latest_scan_timestamp > sample.last_update
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A recent timestamp alone doesn't imply more information is needed? It feels like this needs to be coupled with the sample status from scanning, though perhaps that's being done implicitly upstream
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The samples are already separated based on lock status, this loop is only through the samples that are not locked.
{% elif sample.last_update is none %} | ||
<div class="barcode-col barcode-col-end" id="btn-view-{{ sample.sample_id }}"> | ||
<span class="sample-label">{{ _('Sample Received - Information Needed ') }} </span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A sample which is received, and therefore has a scan date, could be perfect in its information in which case we do not need more information, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The samples are already separated based on lock status, this loop is only through the samples that are not locked.
<div class="barcode-col barcode-col-end" id="btn-view-{{ sample.sample_id }}"> | ||
<span class="sample-label">{{ _('Sample Received - Information Still Needed ') }} </span> | ||
</div> | ||
{% else %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This block would render when sample.latest_scan_timestamp <= sample.last_update
. For the user (or admin), I think the interpretation here is "in the processing queue" or something to that effect as it still needs to be scanned again.
Note, I'm not sure if this is the correct branch to render in the (unlikely) event of a tie
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is the appropriate block in the event of a tie. Since this block of samples consists of ones that are not locked - which means they're not valid - that would imply that the wet lab admin will need to scan them again at some point, meaning an update is pending.
<div class="barcode-col barcode-col-end" id="btn-view-{{ sample.sample_id }}"> | ||
<span class="sample-label">{{ _('Sample Received - Information Needed ') }} </span> | ||
</div> | ||
{% elif sample.latest_scan_timestamp > sample.last_update %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A recent timestamp alone doesn't imply more information is needed? It feels like this needs to be coupled with the sample status from scanning, though perhaps that's being done implicitly upstream
<div class="barcode-col barcode-col-end" id="btn-view-{{ sample.sample_id }}"> | ||
<span class="sample-label">{{ _('Not Yet Received ') }} </span> | ||
</div> | ||
{% elif sample.sample_latest_sample_information_update is none or sample.sample_latest_scan_timestamp > sample.sample_latest_sample_information_update %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ayobi I just tested this on our staging server and I think we're running into a timezone issue somewhere. Here's what I did:
- Claim a sample under a user account and fill out the sample collection info (sample date/time don't really matter as long as there's a valid value there).
- Log into microsetta-admin as an admin user, hit Scan Barcode, find the sample, and mark it sample-has-inconsistencies
- Log back into the UI as the normal user.
At this point, the sample should show as 'Sample Received - Information Needed,' but it's rolling past this condition and hitting 'Sample Received - Update Pending.' When I inspected the database, the latest_sample_information_update
value in ag.ag_kit_barcodes
was 2024-02-06 17:16:48.179585-08
and the scan_timestamp
value in barcodes.barcode_scans
was 2024-02-06 09:19:35.086172-08
.
The most expedient solution is probably to switch the insert/update value for latest_sample_information_update
from utcnow()
to now()
. Can you see if changing that in the API resolves the issue, and ensure it doesn't introduce any unintended behavior?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok, I changed it to now() and is committed on the api PR. I tested it in microsetta-admin and behaved as expected.
Added last_update column to ag_kit_barcodes table to microsetta-private-api, PR here. This will enable a more descriptive sample status on the 'My Reports' tab.
The new sample statuses are as followed.