sample summary report expansion #585
Conversation
| if 'sample_barcodes' in body: | ||
| project_id = None | ||
| barcodes = body["sample_barcodes"] | ||
| elif 'kit_ids' in body: | ||
| project_id = None |
There was a problem hiding this comment.
Since all but one condition set project_id = None, it would be more efficient to move that statement to above the if block and only change it for the project_id condition.
| if project_id is None: | ||
| raise ValueError("project_id must be defined.") | ||
|
|
||
| with Transaction() as t: | ||
| return AdminRepo(t).get_project_barcodes(project_id) | ||
|
|
||
|
|
||
| def get_barcodes_by_kit_ids(kit_ids): | ||
| if kit_ids is None: | ||
| raise ValueError("kit_id must be defined.") |
There was a problem hiding this comment.
It would be helpful for the API layer to catch these ValueErrors and return a useful error to microsetta-admin
| kit_id_name = kit_repo.get_kit_id_name_by_barcode(barcode) | ||
| outbound_fedex_tracking = \ | ||
| admin_repo.get_outbound_tracking_by_barcodes(barcode) | ||
| inbound_fedex_tracking = \ | ||
| admin_repo.get_inbound_tracking_by_barcodes(barcode) | ||
| first_scan_timestamp_status = \ | ||
| admin_repo.get_first_scan_timestamp_by_barcodes(barcode) | ||
| last_scan_timestamp_status = \ | ||
| admin_repo.get_last_scan_timestamp_by_barcodes(barcode) | ||
|
|
There was a problem hiding this comment.
Many of these values are already being exposed here from admin_repo.get_diagnostics_by_barcode() - is there a reason you're re-retrieving them?
| cur.execute(query_kit_ids, [tuple(kit_ids)]) | ||
| ag_kit_ids = [row[0] for row in cur.fetchall()] | ||
|
|
||
| if not ag_kit_ids: | ||
| return [] | ||
|
|
||
| query_barcodes = """ | ||
| SELECT barcode | ||
| FROM ag.ag_kit_barcodes | ||
| WHERE ag_kit_id IN %s | ||
| """ | ||
| cur.execute(query_barcodes, [tuple(ag_kit_ids)]) |
There was a problem hiding this comment.
Is there a reason this can't be done in one query using a join statement?
| The list of observed barcodes | ||
| """ | ||
| query_emails = """ | ||
| SELECT created_with_kit_id |
There was a problem hiding this comment.
This will not work as intended. created_with_kit_id is only populated if the account was created using a Kit ID, which is only a subset of historical accounts and is impossible for newer accounts. You need to leverage the account->source->sample relationship to find samples by email.
| query_outbound_tracking = """ | ||
| SELECT kit_id | ||
| FROM barcodes.kit | ||
| WHERE outbound_fedex_tracking IN %s | ||
| """ | ||
|
|
||
| with self._transaction.cursor() as cur: | ||
| cur.execute(query_outbound_tracking, | ||
| [tuple(outbound_tracking_numbers)]) | ||
| kit_ids = [row[0] for row in cur.fetchall()] | ||
|
|
||
| if not kit_ids: | ||
| return [] | ||
|
|
||
| query_barcodes = """ | ||
| SELECT barcode | ||
| FROM barcodes.barcode | ||
| WHERE kit_id IN %s | ||
| """ | ||
| cur.execute(query_barcodes, [tuple(kit_ids)]) |
There was a problem hiding this comment.
Is there a reason this isn't done as one query using a join statement?
| query_inbound_tracking = """ | ||
| SELECT kit_id | ||
| FROM barcodes.kit | ||
| WHERE inbound_fedex_tracking IN %s | ||
| """ | ||
|
|
||
| with self._transaction.cursor() as cur: | ||
| cur.execute(query_inbound_tracking, | ||
| [tuple(inbound_tracking_numbers)]) | ||
| kit_ids = [row[0] for row in cur.fetchall()] | ||
|
|
||
| if not kit_ids: | ||
| return [] | ||
|
|
||
| query_barcodes = """ | ||
| SELECT barcode | ||
| FROM barcodes.barcode | ||
| WHERE kit_id IN %s | ||
| """ | ||
| cur.execute(query_barcodes, [tuple(kit_ids)]) |
There was a problem hiding this comment.
Is there a reason this isn't done as one query using a join statement?
| @@ -67,3 +67,18 @@ def get_kit_unused_samples(self, supplied_kit_id): | |||
| else: | |||
| samples = [sample_repo._get_sample_by_id(r[1]) for r in rows] | |||
| return Kit(rows[0][0], samples) | |||
|
|
|||
| def get_kit_id_name_by_barcode(self, barcode): | |||
There was a problem hiding this comment.
Is there a reason you're not leveraging existing code that does this?
At the moment, to get a sample summary report, 2 of the 3 ways we do this is by searching for a single barcode or searching through a csv file of barcodes. With this expansion, we are also going to be able to search via Kit ID, Email, Outbound Tracking Number, and Inbound Tracking Number and also see these fields in the output of the report.