Skip to content
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

validate sample and patient before render the patient view #2728

Merged
merged 2 commits into from Nov 6, 2019

Conversation

dippindots
Copy link
Member

@dippindots dippindots commented Sep 23, 2019

validate sample and patient before render the patient view
Fix # cBioPortal/cbioportal#5357.

Changes proposed in this pull request:

  • a show error when sample or patient not exists
    image

@dippindots dippindots self-assigned this Sep 23, 2019
@jjgao jjgao temporarily deployed to cbioportal-frontend-pr-2728 September 23, 2019 19:46 Inactive
@dippindots
Copy link
Member Author

dippindots commented Sep 24, 2019

test links:

  1. sample
  2. patient

@alisman
Copy link
Collaborator

alisman commented Oct 17, 2019

@dippindots
For this case, we should not say "URL invalid." The URL form is valid, it's just that the ID doesn't exist. So, error message should be that Patient doesn't exist.


if ('caseId' in query) {
patientViewPageStore.setPatientId(query.caseId as string);
const validationPatient = await validatePatient(query.caseId, query.studyId);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this check belongs in the patientViewStore, not here.

@alisman
Copy link
Collaborator

alisman commented Oct 17, 2019

When a user submits a request with invalid sample/patient, we already get 404s. So we can accomplish this ticket by properly handling those errors states for the existing mobx promises. In other words, we should just show a message when we detect isError=true

here is example
https://www.cbioportal.org/patient?studyId=ucec_tcga_pub&caseId=TCG-BK-A0CC

@jjgao jjgao temporarily deployed to cbioportal-frontend-pr-2728 October 23, 2019 15:21 Inactive
@jjgao jjgao temporarily deployed to cbioportal-frontend-pr-2728 October 23, 2019 15:25 Inactive
@jjgao jjgao temporarily deployed to cbioportal-f-patient-vi-xhdy7a October 23, 2019 18:58 Inactive
@jjgao jjgao temporarily deployed to cbioportal-f-patient-vi-xhdy7a October 24, 2019 16:33 Inactive
Copy link
Member Author

@dippindots dippindots left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alisman solution updated, test links:

  1. sample
  2. patient

@jjgao jjgao temporarily deployed to cbioportal-f-patient-vi-xhdy7a November 1, 2019 21:50 Inactive
@jjgao jjgao temporarily deployed to cbioportal-f-patient-vi-xhdy7a November 4, 2019 18:51 Inactive
@jjgao jjgao temporarily deployed to cbioportal-f-patient-vi-xhdy7a November 4, 2019 19:24 Inactive
@jjgao jjgao temporarily deployed to cbioportal-f-patient-vi-xhdy7a November 4, 2019 19:35 Inactive
@jjgao jjgao temporarily deployed to cbioportal-f-patient-vi-xhdy7a November 6, 2019 16:52 Inactive
@jjgao jjgao temporarily deployed to cbioportal-f-patient-vi-xhdy7a November 6, 2019 19:54 Inactive
@jjgao jjgao temporarily deployed to cbioportal-f-patient-vi-xhdy7a November 6, 2019 21:09 Inactive
@jjgao jjgao temporarily deployed to cbioportal-f-patient-vi-xhdy7a November 6, 2019 21:41 Inactive
@alisman alisman merged commit 621dfa4 into cBioPortal:master Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants