Get_cram_paths external IDs fix #828
Conversation
Add a test case that exercises this code.
| @@ -431,16 +433,16 @@ async def get_sample_cram_path_map_for_seqr( | |||
| filters.append('JSON_VALUE(a.meta, "$.sequencing_type") ' + seq_check) | |||
|
|
|||
| if participant_ids: | |||
| filters.append('p.id IN :pids') | |||
| filters.append('peid.participant_id IN :pids') | |||
There was a problem hiding this comment.
This field needs to be changed to participant_id (as per the new table) here too. And a minor renaming of the table alias.
| ], | ||
| ) | ||
|
|
||
| id_map = await self.al.get_sample_cram_path_map_for_seqr(self.project_id, ['blood'], [part[0].id]) |
There was a problem hiding this comment.
Added a basic test case so that this code gets exercised. There's not enough data set up to get a non-empty id_map back, so we don't check the result very much.
| filters = [ | ||
| 'a.active', | ||
| 'a.type = "cram"', | ||
| 'a.status = "completed"', | ||
| 'p.project = :project', | ||
| 'peid.project = :project', | ||
| 'peid.name = :PRIMARY_EXTERNAL_ORG', |
There was a problem hiding this comment.
At least for now, maintained the status quo by only returning a row for the primary external id rather than several, one for each external id. This is a technique I used in several of the “higher level” queries, and will want to be revisited eventually.
|
Thanks for the help @jmarshall, appreciate your input and the inclusion of a basic test case |
|
Thanks for finding the problem… If that test case had been there previously, this wouldn't have been missed out in the multiple-ext-ids PR because even this basic test would have failed. Turns out test coverage is a good thing! |
A quick fix for the
get_sample_cram_path_map_for_seqrAnalysis table function to allow it to work with the recent external ID changes. Now joins to theparticipant_external_idtable to get the external ID.