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

Paginated clinical table #10211

Merged
merged 10 commits into from
Jun 16, 2023

Conversation

BasLee
Copy link
Contributor

@BasLee BasLee commented Jun 7, 2023

Paginate the clinical table using original (lazy mobx) table components.

The backend /clinical-data-table/fetch already contained most elements needed for pagination, like page size and page number. This PR also adds a header containing the total number of samples, needed for pagination in the frontend.
To be able to sort properly, the sortBy parameter also had to be modified.

See frontend PR for more information.

Changes

  • Add total-count response header to /clinical-data-table/fetch
  • Allow sorting by sample ID, patient ID, and clinical attributes with the getSampleClinicalDataClinicalTable query and new count query

@BasLee BasLee requested a review from alisman June 7, 2023 09:25
@BasLee BasLee changed the title POC: paginated clinical table by re-implementing data store POC: paginated clinical table Jun 7, 2023
@BasLee BasLee changed the title POC: paginated clinical table Paginated clinical table Jun 7, 2023
@JREastonMarks
Copy link
Collaborator

@BasLee Is this error in the automated tests, something that is of concern?
Invalid parameter count for "ISNULL", expected count: "2"; SQL statement: line 216

@BasLee
Copy link
Contributor Author

BasLee commented Jun 12, 2023

@BasLee Is this error in the automated tests, something that is of concern?

@JREastonMarks Seemed to be a difference between mysql and h2 db used in tests. Fixed in 5ba9828

Copy link
Member

@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.

@BasLee It looks good to me, I think if you rebase your pr, the core tests will pass. I have few minor comments.

@BasLee BasLee force-pushed the poc-paginated-clinical-table branch from 5ba9828 to 9d7c2b3 Compare June 14, 2023 12:48
@BasLee BasLee requested a review from dippindots June 14, 2023 12:49
Copy link
Member

@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.

LGTM

@BasLee BasLee force-pushed the poc-paginated-clinical-table branch from 18626e2 to 654a5d6 Compare June 15, 2023 15:11
@sonarcloud
Copy link

sonarcloud bot commented Jun 15, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 2 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@dippindots dippindots merged commit 15cf8ce into cBioPortal:master Jun 16, 2023
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants