DDP-6034-SMIDS #320
Open
DDP-6034-SMIDS #320
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # src/main/resources/master-changelog.xml
pegahtah
requested review from
gmakharat,
SimoneMaiwald and
ScalaChamp
and removed request for
gmakharat and
SimoneMaiwald
| @@ -39,7 +42,7 @@ | |||
| "oD.fax_sent_3_by, oD.fax_confirmed_3, oD.tissue_received, oD.tissue_problem_option, oD.gender, oD.destruction_policy, oD.unable_obtain_tissue, " + | |||
| "tissue_id, t.notes, count_received, tissue_type, tissue_site, tumor_type, h_e, pathology_report, collaborator_sample_id, block_sent, scrolls_received, sk_id, sm_id, " + | |||
| "sent_gp, first_sm_id, additional_tissue_value_json, expected_return, return_date, return_fedex_id, shl_work_number, tumor_percentage, tissue_sequence, " + | |||
| " scrolls_count, uss_count, h_e_count, blocks_count " + | |||
| " scrolls_count, uss_count, h_e_count, blocks_count, t.uss_sm_ids, t.scrolls_sm_ids, t.he_sm_ids " + | |||
There was a problem hiding this comment.
I think this is old version and should be removed
| @@ -30,7 +31,7 @@ | |||
| private static final String SQL_SELECT_TISSUE = "SELECT tissue_id, onc_history_detail_id, notes, count_received, tissue_type, tissue_site, tumor_type, h_e, " + | |||
| "pathology_report, collaborator_sample_id, block_sent, expected_return, return_date, return_fedex_id, scrolls_received, sk_id, sm_id, " + | |||
| "scrolls_count, uss_count, blocks_count, h_e_count, first_sm_id, sent_gp, last_changed, changed_by, additional_tissue_value_json, shl_work_number, " + | |||
| "tumor_percentage, tissue_sequence FROM ddp_tissue t WHERE NOT (deleted <=> 1) AND onc_history_detail_id = ?"; | |||
| "tumor_percentage, tissue_sequence, uss_sm_ids, scrolls_sm_ids, he_sm_ids FROM ddp_tissue t WHERE NOT (deleted <=> 1) AND onc_history_detail_id = ?"; | |||
There was a problem hiding this comment.
I think this is old version and should be removed
| return tissue; | ||
| } | ||
|
|
||
| private void setSMIds() { |
There was a problem hiding this comment.
can we get the sm ids in one call and not make 3 calls to the same table?
| @@ -278,7 +283,12 @@ public static OncHistoryDetail getOncHistoryDetail(@NonNull ResultSet rs) throws | |||
| rs.getString(DBConstants.DESTRUCTION_POLICY), | |||
| rs.getBoolean(DBConstants.UNABLE_OBTAIN_TISSUE) | |||
| ); | |||
|
|
|||
| try { | |||
| oncHistoryDetail.setParticipantId(rs.getString(DBConstants.DDP_PARTICIPANT_ID)); | |||
There was a problem hiding this comment.
participantId or ddpParticipantId?
| "tumor_percentage, tissue_sequence FROM ddp_tissue t WHERE NOT (deleted <=> 1) AND onc_history_detail_id = ?"; | ||
| "tumor_percentage, tissue_sequence, sm.sm_id_value, sm.sm_id_type_id, sm.sm_id_pk, sm.deleted, sm.tissue_id FROM ddp_tissue t " + | ||
| "LEFT JOIN sm_id sm on (sm.tissue_id = t.tissue_id AND NOT sm.deleted <=> 1 AND NOT t.deleted <=> 1) " + | ||
| "WHERE NOT (deleted <=> 1) AND onc_history_detail_id = ?"; |
There was a problem hiding this comment.
WHERE NOT (deleted <=> 1) will not work I think because of multiple columns with that name
| if(result.isEmpty()){ | ||
| response.status(404); | ||
| return null; | ||
| if(!result.isEmpty()){ |
There was a problem hiding this comment.
we are not responding with 404 if nothing is found?
SimoneMaiwald
added a commit
that referenced
this pull request
Dec 9, 2021
gmakhara
reviewed
Dec 10, 2021
| @@ -42,6 +42,7 @@ | |||
|
|
|||
There was a problem hiding this comment.
@pegahtah @SimoneMaiwald guys please pay attention to this comment: https://broadinstitute.slack.com/archives/C02BAHMHE1G/p1639065811003800?thread_ts=1639065448.003300&cid=C02BAHMHE1G
pegahtah
added a commit
that referenced
this pull request
Dec 15, 2021
* DDP-6034: - moving changes from PR into prod branch (#320) * Changed the way of making onc history detail to be better readable * added unique constraint * added unique constraint * added constant string * fixed missed part * fixed bug in tissuelist display and in isUniqueSmId * changed how we patch to avoid duplicates * fixed bug Co-authored-by: pegahtah <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
and DDP-7163