From 4642088f57e33aff6a7a1395c9ed5eb8ef05bcbf Mon Sep 17 00:00:00 2001 From: Pim van Nierop Date: Tue, 7 May 2024 16:22:16 +0200 Subject: [PATCH] Paginated download of data for Clinical Data tab in Study View (#4786) * Implement server side filtering and sorting mode and limit record count * implement data fetcher * Update clinical data collection data type * Pass attribute id for sort Instead of display name. * Fix sort direction passed by table component * Fix total item count for paginated results * Allow override of the total count label * Do not render the clinical data table unutil visible attributes is populated to avoid flash of two column table (premature) * Point e2e at beta * run against custom backend * Fix LazyMobxTable unit tests * Update api client * Address duplicate imports --------- Co-authored-by: lismana Co-authored-by: Zhaoyuan (Ryan) Fu --- env/master.sh | 3 +- .../generated/CBioPortalAPIInternal-docs.json | 35 +-- .../src/generated/CBioPortalAPIInternal.ts | 16 +- .../cbioportal-ts-api-client/src/index.tsx | 2 +- src/pages/studyView/StudyViewPageStore.ts | 61 ++-- src/pages/studyView/StudyViewUtils.spec.tsx | 82 ------ src/pages/studyView/StudyViewUtils.tsx | 108 ++----- src/pages/studyView/tabs/ClinicalDataTab.tsx | 277 ++++++++++++++---- .../lazyMobXTable/LazyMobXTable.spec.tsx | 68 +++-- .../lazyMobXTable/LazyMobXTable.tsx | 125 ++++++-- .../mutationTable/MutationTable.tsx | 5 +- 11 files changed, 428 insertions(+), 354 deletions(-) diff --git a/env/master.sh b/env/master.sh index d84e56dda80..49f303e188b 100644 --- a/env/master.sh +++ b/env/master.sh @@ -1,2 +1,3 @@ -export CBIOPORTAL_URL="${CBIOPORTAL_URL:-https://www.cbioportal.org}" +export CBIOPORTAL_URL="${CBIOPORTAL_URL:-https://beta.cbioportal.org}" export GENOME_NEXUS_URL="${GENOME_NEXUS_URL:-https://www.genomenexus.org}" +export BACKEND=cbioportal:demo-fix-clinical-table-sorting \ No newline at end of file diff --git a/packages/cbioportal-ts-api-client/src/generated/CBioPortalAPIInternal-docs.json b/packages/cbioportal-ts-api-client/src/generated/CBioPortalAPIInternal-docs.json index 400ac77a974..cbdd0fbf7fc 100644 --- a/packages/cbioportal-ts-api-client/src/generated/CBioPortalAPIInternal-docs.json +++ b/packages/cbioportal-ts-api-client/src/generated/CBioPortalAPIInternal-docs.json @@ -381,7 +381,7 @@ }, { "default": 0, - "description": "Page number of the result list", + "description": "Page number of the result list. Zero represents the first page.", "format": "int32", "in": "query", "minimum": 0, @@ -428,7 +428,7 @@ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/ClinicalDataCollection" + "$ref": "#/definitions/SampleClinicalDataCollection" } } }, @@ -3639,23 +3639,6 @@ }, "type": "object" }, - "ClinicalDataCollection": { - "properties": { - "patientClinicalData": { - "items": { - "$ref": "#/definitions/ClinicalData" - }, - "type": "array" - }, - "sampleClinicalData": { - "items": { - "$ref": "#/definitions/ClinicalData" - }, - "type": "array" - } - }, - "type": "object" - }, "ClinicalDataCount": { "properties": { "count": { @@ -5417,6 +5400,20 @@ ], "type": "object" }, + "SampleClinicalDataCollection": { + "properties": { + "byUniqueSampleKey": { + "additionalProperties": { + "items": { + "$ref": "#/definitions/ClinicalData" + }, + "type": "array" + }, + "type": "object" + } + }, + "type": "object" + }, "SampleIdentifier": { "properties": { "sampleId": { diff --git a/packages/cbioportal-ts-api-client/src/generated/CBioPortalAPIInternal.ts b/packages/cbioportal-ts-api-client/src/generated/CBioPortalAPIInternal.ts index 4202dd5db8f..43e68255e06 100644 --- a/packages/cbioportal-ts-api-client/src/generated/CBioPortalAPIInternal.ts +++ b/packages/cbioportal-ts-api-client/src/generated/CBioPortalAPIInternal.ts @@ -186,12 +186,6 @@ export type ClinicalDataBinFilter = { 'start': number -}; -export type ClinicalDataCollection = { - 'patientClinicalData': Array < ClinicalData > - - 'sampleClinicalData': Array < ClinicalData > - }; export type ClinicalDataCount = { 'count': number @@ -964,6 +958,10 @@ export type Sample = { 'uniqueSampleKey': string +}; +export type SampleClinicalDataCollection = { + 'byUniqueSampleKey': {} + }; export type SampleIdentifier = { 'sampleId': string @@ -1981,7 +1979,7 @@ export default class CBioPortalAPIInternal { * @method * @name CBioPortalAPIInternal#fetchClinicalDataClinicalTableUsingPOST * @param {integer} pageSize - Page size of the result list - * @param {integer} pageNumber - Page number of the result list + * @param {integer} pageNumber - Page number of the result list. Zero represents the first page. * @param {string} searchTerm - Search term to filter sample rows. Samples are returned with a partial match to the search term for any sample clinical attribute. * @param {string} sortBy - sampleId, patientId, or the ATTR_ID to sorted by * @param {string} direction - Direction of the sort @@ -2050,7 +2048,7 @@ export default class CBioPortalAPIInternal { * @method * @name CBioPortalAPIInternal#fetchClinicalDataClinicalTableUsingPOST * @param {integer} pageSize - Page size of the result list - * @param {integer} pageNumber - Page number of the result list + * @param {integer} pageNumber - Page number of the result list. Zero represents the first page. * @param {string} searchTerm - Search term to filter sample rows. Samples are returned with a partial match to the search term for any sample clinical attribute. * @param {string} sortBy - sampleId, patientId, or the ATTR_ID to sorted by * @param {string} direction - Direction of the sort @@ -2065,7 +2063,7 @@ export default class CBioPortalAPIInternal { 'studyViewFilter' ? : StudyViewFilter, $queryParameters ? : any, $domain ? : string - }): Promise < ClinicalDataCollection > { + }): Promise < SampleClinicalDataCollection > { return this.fetchClinicalDataClinicalTableUsingPOSTWithHttpInfo(parameters).then(function(response: request.Response) { return response.body; }); diff --git a/packages/cbioportal-ts-api-client/src/index.tsx b/packages/cbioportal-ts-api-client/src/index.tsx index 3237ca62647..304c48e8295 100644 --- a/packages/cbioportal-ts-api-client/src/index.tsx +++ b/packages/cbioportal-ts-api-client/src/index.tsx @@ -13,7 +13,7 @@ export { BinsGeneratorConfig, ClinicalDataBinFilter, ClinicalDataBinCountFilter, - ClinicalDataCollection, + SampleClinicalDataCollection, ClinicalDataCount, ClinicalDataCountFilter, ClinicalDataCountItem, diff --git a/src/pages/studyView/StudyViewPageStore.ts b/src/pages/studyView/StudyViewPageStore.ts index 44e28488c05..7ccfe01d7bf 100644 --- a/src/pages/studyView/StudyViewPageStore.ts +++ b/src/pages/studyView/StudyViewPageStore.ts @@ -371,6 +371,7 @@ import { PlotsColoringParam, PlotsSelectionParam, } from 'pages/resultsView/ResultsViewURLWrapper'; +import { SortDirection } from 'shared/components/lazyMobXTable/LazyMobXTable'; export const STUDY_VIEW_FILTER_AUTOSUBMIT = 'study_view_filter_autosubmit'; @@ -5864,6 +5865,13 @@ export class StudyViewPageStore }, }); + readonly clinicalAttributeDisplayNameToClinicalAttribute = remoteData({ + await: () => [this.clinicalAttributes], + invoke: async () => { + return _.keyBy(this.clinicalAttributes.result!, 'displayName'); + }, + }); + readonly clinicalAttributeIdToDataType = remoteData({ await: () => [this.clinicalAttributes], invoke: async () => { @@ -9241,39 +9249,6 @@ export class StudyViewPageStore default: {}, }); - readonly getDataForClinicalDataTab = remoteData({ - await: () => [ - this.clinicalAttributes, - this.selectedSamples, - this.sampleSetByKey, - ], - onError: () => {}, - invoke: async () => { - if (this.selectedSamples.result.length === 0) { - return Promise.resolve([]); - } - let sampleClinicalDataMap = await getAllClinicalDataByStudyViewFilter( - this.filters - ); - - const sampleClinicalDataArray = _.mapValues( - sampleClinicalDataMap, - (attrs, uniqueSampleId) => { - const sample = this.sampleSetByKey.result![uniqueSampleId]; - return { - studyId: sample.studyId, - patientId: sample.patientId, - sampleId: sample.sampleId, - ...attrs, - }; - } - ); - - return _.values(sampleClinicalDataArray); - }, - default: [], - }); - readonly clinicalAttributeProduct = remoteData({ await: () => [this.clinicalAttributes, this.selectedSamples], invoke: async () => { @@ -9815,8 +9790,10 @@ export class StudyViewPageStore if (this.selectedSamples.result.length === 0) { return Promise.resolve(''); } - let sampleClinicalDataMap = await getAllClinicalDataByStudyViewFilter( - this.filters + let sampleClinicalDataResponse = await getAllClinicalDataByStudyViewFilter( + this.filters, + undefined, + undefined ); let clinicalAttributesNameSet = _.reduce( @@ -9836,13 +9813,19 @@ export class StudyViewPageStore let dataRows = _.reduce( this.selectedSamples.result, (acc, next) => { - let sampleData: { [attributeId: string]: string } = { + const sampleData = { studyId: next.studyId, patientId: next.patientId, sampleId: next.sampleId, - ...(sampleClinicalDataMap[next.uniqueSampleKey] || {}), - }; - + } as { [attributeId: string]: string }; + const clinicalData = + sampleClinicalDataResponse.data[next.uniqueSampleKey]; + _.forEach( + clinicalData, + (attr: ClinicalData) => + (sampleData[attr['clinicalAttributeId']] = + attr['value']) + ); acc.push( _.map( Object.keys(clinicalAttributesNameSet), diff --git a/src/pages/studyView/StudyViewUtils.spec.tsx b/src/pages/studyView/StudyViewUtils.spec.tsx index a500f9a62ed..4e731b9a51c 100644 --- a/src/pages/studyView/StudyViewUtils.spec.tsx +++ b/src/pages/studyView/StudyViewUtils.spec.tsx @@ -55,7 +55,6 @@ import { isLogScaleByValues, isOccupied, makePatientToClinicalAnalysisGroup, - mergeClinicalDataCollection, needAdditionShiftForLogScaleBarChart, pickClinicalDataColors, shouldShowChart, @@ -72,7 +71,6 @@ import { CancerStudy, ClinicalAttribute, ClinicalData, - ClinicalDataCollection, DataFilterValue, Sample, StudyViewFilter, @@ -2755,86 +2753,6 @@ describe('StudyViewUtils', () => { }); }); - describe('mergeClinicalDataCollection', () => { - it('handles merge when patient key defined in sample data', () => { - const input = ({ - sampleClinicalData: [ - { - sampleId: 's1', - patientId: 'p1', - studyId: 'study1', - uniqueSampleKey: 's1key', - uniquePatientKey: 'p1key', - clinicalAttributeId: 'attr1', - value: 'value1', - }, - ], - patientClinicalData: [ - { - sampleId: undefined, - patientId: 'p1', - studyId: 'study1', - uniqueSampleKey: undefined, - uniquePatientKey: 'p1key', - clinicalAttributeId: 'attr2', - value: 'value2', - }, - ], - } as unknown) as ClinicalDataCollection; - const expected = { - s1key: { - attr1: 'value1', - attr2: 'value2', - }, - }; - assert.deepEqual( - expected, - mergeClinicalDataCollection( - (input as unknown) as ClinicalDataCollection - ) - ); - }); - - it('handles merge when patient key absent in sample data', () => { - const input = ({ - sampleClinicalData: [ - { - sampleId: 's1', - patientId: 'p1', - studyId: 'study1', - uniqueSampleKey: 's1key', - uniquePatientKey: 'p1key', - clinicalAttributeId: 'attr1', - value: 'value1', - }, - ], - patientClinicalData: [ - { - sampleId: undefined, - patientId: 'p2', - studyId: 'study1', - uniqueSampleKey: undefined, - // note the patient key that does not line-up with the sample - uniquePatientKey: 'p2key', - clinicalAttributeId: 'attr2', - value: 'value2', - }, - ], - } as unknown) as ClinicalDataCollection; - const expected = { - s1key: { - attr1: 'value1', - }, - }; - assert.deepEqual( - expected, - mergeClinicalDataCollection( - (input as unknown) as ClinicalDataCollection - ) - ); - }); - }); - describe('toFixedDigit', () => { const negativeValues = [ -666.666, diff --git a/src/pages/studyView/StudyViewUtils.tsx b/src/pages/studyView/StudyViewUtils.tsx index cc2318113b2..23831fd2f74 100644 --- a/src/pages/studyView/StudyViewUtils.tsx +++ b/src/pages/studyView/StudyViewUtils.tsx @@ -7,7 +7,6 @@ import { ClinicalData, ClinicalDataBin, ClinicalDataBinFilter, - ClinicalDataCollection, ClinicalDataCount, ClinicalDataMultiStudyFilter, DataFilterValue, @@ -28,6 +27,7 @@ import { PatientIdentifier, PatientTreatmentRow, Sample, + SampleClinicalDataCollection, SampleIdentifier, SampleTreatmentRow, StructuralVariantFilterQuery, @@ -3154,82 +3154,38 @@ export function updateSavedUserPreferenceChartIds( } export async function getAllClinicalDataByStudyViewFilter( - studyViewFilter: StudyViewFilter -): Promise<{ [sampleId: string]: { [attributeId: string]: string } }> { - const localClinicalDataCollection: ClinicalDataCollection = { - sampleClinicalData: [], - patientClinicalData: [], - }; - let remoteClinicalDataCollection: ClinicalDataCollection = { - sampleClinicalData: [], - patientClinicalData: [], - }; - const maxPageSize = 500000; - let pageNumber = 0; - do { - const remoteClinicalDataCollection = await internalClient.fetchClinicalDataClinicalTableUsingPOST( - { - studyViewFilter, - pageSize: maxPageSize, - pageNumber: pageNumber, - searchTerm: undefined, - sortBy: undefined, - direction: 'ASC', - } - ); - localClinicalDataCollection.sampleClinicalData = localClinicalDataCollection.sampleClinicalData.concat( - remoteClinicalDataCollection.sampleClinicalData - ); - localClinicalDataCollection.patientClinicalData = localClinicalDataCollection.patientClinicalData.concat( - remoteClinicalDataCollection.patientClinicalData - ); - pageNumber++; - } while (remoteClinicalDataCollection.sampleClinicalData.length > 0); - - return mergeClinicalDataCollection(localClinicalDataCollection); -} + studyViewFilter: StudyViewFilter, + searchTerm: string | undefined, + sortAttributeId: any, + sortDirection: any = 'asc', + pageSize: number = 500 +): Promise<{ + totalItems: number; + data: { [uniqueSampleKey: string]: ClinicalData[] }; +}> { + const [remoteClinicalDataCollection, totalItems]: [ + SampleClinicalDataCollection, + number + ] = await internalClient + .fetchClinicalDataClinicalTableUsingPOSTWithHttpInfo({ + studyViewFilter, + pageSize, + pageNumber: 0, + searchTerm: searchTerm, + sortBy: sortAttributeId, + direction: sortDirection?.toUpperCase(), + }) + .then(response => { + return [ + response.body, + parseInt(response.header['total-count'] || 0), + ]; + }); -export function mergeClinicalDataCollection( - clinicalDataCollection: ClinicalDataCollection -): { [sampleId: string]: { [attributeId: string]: string } } { - const patientKeyedSampleData = _.groupBy( - clinicalDataCollection.sampleClinicalData, - d => d.uniquePatientKey - ); - let patientKeyedSampleKeyedData = _.mapValues( - patientKeyedSampleData, - sampleData => _.groupBy(sampleData, d => d.uniqueSampleKey) - ); - const patientKeyedPatientData = _.groupBy( - clinicalDataCollection.patientClinicalData, - d => d.uniquePatientKey - ); - // Add patient level clinical data to sample clinical data. - patientKeyedSampleKeyedData = _.mapValues( - patientKeyedSampleKeyedData, - (sampleKeyedData, patientId) => - _.mapValues(sampleKeyedData, (attrs, sampleId) => - attrs.concat(patientKeyedPatientData[patientId] || []) - ) - ); - // Remove patient id levels (only keep sample id keys). - const sampleKeyedData = _.assign( - {}, - ..._.values(patientKeyedSampleKeyedData) - ); - // Put all clinical attributes in one object. - const sampleCollapsedAttributes = _.mapValues( - sampleKeyedData, - clinicalData => { - const data = _.map(clinicalData, (datum: ClinicalData) => { - const obj: { [attrId: string]: string } = {}; - obj[datum.clinicalAttributeId] = datum.value; - return obj; - }); - return _.assign({}, ...data); - } - ); - return sampleCollapsedAttributes; + return { + totalItems, + data: remoteClinicalDataCollection.byUniqueSampleKey, + }; } export function convertClinicalDataBinsToDataBins( diff --git a/src/pages/studyView/tabs/ClinicalDataTab.tsx b/src/pages/studyView/tabs/ClinicalDataTab.tsx index 8857d7b6f14..954c05cfb40 100644 --- a/src/pages/studyView/tabs/ClinicalDataTab.tsx +++ b/src/pages/studyView/tabs/ClinicalDataTab.tsx @@ -2,6 +2,7 @@ import * as React from 'react'; import { Column, default as LazyMobXTable, + SortDirection, } from 'shared/components/lazyMobXTable/LazyMobXTable'; import { observer } from 'mobx-react'; import _ from 'lodash'; @@ -13,6 +14,7 @@ import { ChartMeta, SpecialChartsUniqueKeyEnum, DataType, + getAllClinicalDataByStudyViewFilter, } from '../StudyViewUtils'; import LoadingIndicator from 'shared/components/loadingIndicator/LoadingIndicator'; import { StudyViewPageStore } from 'pages/studyView/StudyViewPageStore'; @@ -22,13 +24,17 @@ import { remoteData, } from 'cbioportal-frontend-commons'; import { Else, If, Then } from 'react-if'; -import ProgressIndicator, { - IProgressIndicatorItem, -} from '../../../shared/components/progressIndicator/ProgressIndicator'; +import { IProgressIndicatorItem } from '../../../shared/components/progressIndicator/ProgressIndicator'; import autobind from 'autobind-decorator'; import { WindowWidthBox } from '../../../shared/components/WindowWidthBox/WindowWidthBox'; import { getServerConfig } from 'config/config'; import { StudyViewPageTabKeyEnum } from '../StudyViewPageTabs'; +import { computed, makeObservable, observable } from 'mobx'; +import { + ClinicalData, + Sample, + StudyViewFilter, +} from 'cbioportal-ts-api-client'; export interface IClinicalDataTabTable { store: StudyViewPageStore; @@ -38,11 +44,62 @@ class ClinicalDataTabTableComponent extends LazyMobXTable<{ [id: string]: string; }> {} +const CLINICAL_DATA_RECORD_LIMIT = 500; + +type SortCriteria = { + field: string | undefined; + direction: SortDirection | undefined; +}; + +async function fetchClinicalDataForStudyViewClinicalDataTab( + filters: StudyViewFilter, + sampleSetByKey: { [sampleId: string]: Sample }, + searchTerm: string | undefined, + sortAttributeId: string | undefined, + sortDirection: 'asc' | 'desc' | undefined, + recordLimit: number +) { + let sampleClinicalDataResponse = await getAllClinicalDataByStudyViewFilter( + filters, + searchTerm, + sortAttributeId, + sortDirection, + recordLimit + ); + + const aggregatedSampleClinicalData = _.mapValues( + sampleClinicalDataResponse.data, + (attrs, uniqueSampleId) => { + const sample = sampleSetByKey[uniqueSampleId]; + const sampleData = { + studyId: sample.studyId, + patientId: sample.patientId, + sampleId: sample.sampleId, + } as { [attributeId: string]: string }; + attrs.forEach( + attr => + (sampleData[attr['clinicalAttributeId']] = attr['value']) + ); + return sampleData; + } + ); + + return { + totalItems: sampleClinicalDataResponse.totalItems, + data: _.values(aggregatedSampleClinicalData), + }; +} + @observer export class ClinicalDataTab extends React.Component< IClinicalDataTabTable, {} > { + constructor(props: IClinicalDataTabTable) { + super(props); + makeObservable(this); + } + getDefaultColumnConfig( key: string, columnName: string, @@ -82,6 +139,56 @@ export class ClinicalDataTab extends React.Component< }; } + @observable clinicalDataTabSearchTerm: string | undefined = undefined; + + @observable clinicalDataSortCriteria: SortCriteria = { + field: undefined, + direction: undefined, + }; + + @computed + get clinicalDataSortAttributeId(): string | undefined { + return this.clinicalDataSortCriteria?.field + ? this.props.store.clinicalAttributeDisplayNameToClinicalAttribute + .result![this.clinicalDataSortCriteria.field][ + 'clinicalAttributeId' + ] + : undefined; + } + + @computed + get clinicalDataSortDirection(): 'asc' | 'desc' | undefined { + return this.clinicalDataSortCriteria?.direction; + } + + readonly getDataForClinicalDataTab = remoteData({ + await: () => [ + this.props.store.clinicalAttributes, + this.props.store.selectedSamples, + this.props.store.sampleSetByKey, + this.props.store.clinicalAttributeDisplayNameToClinicalAttribute, + ], + onError: () => {}, + invoke: async () => { + if (this.props.store.selectedSamples.result.length === 0) { + return Promise.resolve({ totalItems: 0, data: [] }); + } + + const sampleClinicalData = await fetchClinicalDataForStudyViewClinicalDataTab( + this.props.store.filters, + this.props.store.sampleSetByKey.result!, + this.clinicalDataTabSearchTerm, + this.clinicalDataSortAttributeId, + this.clinicalDataSortDirection, + CLINICAL_DATA_RECORD_LIMIT + ); + + return Promise.resolve(sampleClinicalData); + }, + }); + + // this problem is that the visible attributes are not yet populated. + readonly columns = remoteData({ invoke: async () => { let defaultColumns: Column<{ [id: string]: string }>[] = [ @@ -173,12 +280,17 @@ export class ClinicalDataTab extends React.Component< label: 'Loading clinical data' + (elapsedSecs > 2 ? ' - this can take several seconds' : ''), - promises: [this.props.store.getDataForClinicalDataTab], + promises: [this.getDataForClinicalDataTab], }, ]; } public render() { + // not that the columns which are showing in the table + // are dependent on visible attributes. + // for this reason we need to wait for visible attributes to be populated + // this simplest way to await this is just no avoid rendering the table when there are + // no visibleAttributes return ( @@ -188,18 +300,13 @@ export class ClinicalDataTab extends React.Component< .isPending || this.props.store.maxSamplesForClinicalTab .isPending || - this.props.store.selectedSamples.isPending + this.props.store.selectedSamples.isPending || + this.props.store.visibleAttributes.length < 1 } > @@ -242,63 +349,105 @@ export class ClinicalDataTab extends React.Component< .{' '} - + + { + this + .getDataForClinicalDataTab + .result?.totalItems + }{' '} + results + + } - > - + showCopyDownload={ + getServerConfig() + .skin_hide_download_controls === + DownloadControlOption.SHOW_ALL + } + showCountHeader={false} + showColumnVisibility={false} + onFilterTextChange={searchTerm => + (this.clinicalDataTabSearchTerm = searchTerm) + } + onSortDirectionChange={( + field, + sortDirection + ) => { + this.clinicalDataSortCriteria = { + field: field, + direction: sortDirection, + }; + }} + data={ + this.getDataForClinicalDataTab + .result?.data || [] + } + showLoading={ + this.getDataForClinicalDataTab + .isPending || + this.columns.isPending + } + loadingComponent={ - - - - - - - + } + columns={this.columns.result} + copyDownloadProps={{ + showCopy: false, + downloadFilename: this.props.store + .clinicalDataDownloadFilename, + }} + initialFilterString={ + this.clinicalDataTabSearchTerm + } + initialSortDirection={ + this.clinicalDataSortCriteria + ?.direction + } + initialSortColumn={ + this.clinicalDataSortCriteria?.field + } + downloadDataFetcher={() => { + return fetchClinicalDataForStudyViewClinicalDataTab( + this.props.store.filters, + this.props.store.sampleSetByKey + .result!, + this.clinicalDataTabSearchTerm, + this + .clinicalDataSortAttributeId, + this.clinicalDataSortDirection, + 500 + ).then(data => { + return data.data; + }); + }} + // result limited mode will show a message when user reaches maximum + // allowed result and explain to them they can use filtering or sorting + // to find more specific results + // this should only engage when the total matching items reported by server + // exceeds the allowed limit + // this allows us to limit the number of results without introducing the complication + // of server side pagination + isResultLimited={ + !!this.getDataForClinicalDataTab + .result?.totalItems + ? this.getDataForClinicalDataTab + .result?.totalItems > + CLINICAL_DATA_RECORD_LIMIT + : false + } + resultCountOverride={ + this.getDataForClinicalDataTab + .result?.totalItems + } + /> diff --git a/src/shared/components/lazyMobXTable/LazyMobXTable.spec.tsx b/src/shared/components/lazyMobXTable/LazyMobXTable.spec.tsx index 73522a4ad62..a8ce6050ea9 100644 --- a/src/shared/components/lazyMobXTable/LazyMobXTable.spec.tsx +++ b/src/shared/components/lazyMobXTable/LazyMobXTable.spec.tsx @@ -2398,16 +2398,20 @@ describe('LazyMobXTable', () => { }); describe('pagination', () => { it('starts with 50 items per page', () => { - let table = mount(); - assert.equal(getItemsPerPage(table), 50, 'with no data'); - table.setProps({ - columns: simpleColumns, - data: data.slice(0, 100), - }); + let table = mount( +
+ ); assert.equal(getItemsPerPage(table), 50, 'with data'); }); it('limits page accessing appropriately, depending on how many items total and how many per page, and changes page numbers correctly', () => { - let table = mount(
); + let table = mount( +
+ ); + assert.equal(getCurrentPage(table), 0, 'starts on page 0'); assert.isFalse( clickPrevPage(table), @@ -2510,46 +2514,40 @@ describe('LazyMobXTable', () => { }); it('properly handles changing items per page', () => { - let table = mount(
); - selectItemsPerPage(table, 25); - assert.equal( - getItemsPerPage(table), - 25, - 'case: no data; it successfully changes the items per page to 25' + let table = mount( +
); + + console.log('row count'); + console.log(data.length); + + selectItemsPerPage(table, 2); assert.equal( - getCurrentPage(table), - 0, - 'case: no data; still on page 0' + getItemsPerPage(table), + 2, + 'it successfully changes the items per page to 25' ); + assert.equal(getCurrentPage(table), 0, "we're on page 1"); assert.equal( - getNumVisibleRows(table), - 0, - 'case: no data; no rows visible with 25/page, because no data' + getVisibleRows(table).length, + 2, + 'there are two items on the page' ); selectItemsPerPage(table, -1); assert.equal( getItemsPerPage(table), -1, - 'case: no data; it successfully changes the items per page to show all' - ); - assert.equal( - getCurrentPage(table), - 0, - 'case: no data; still on page 0' - ); - assert.equal( - getNumVisibleRows(table), - 0, - 'case: no data; no rows visible with show all, because no data' + 'it successfully changes the items per page to show all' ); + assert.equal(getCurrentPage(table), 0, 'still on page 1'); + assert.equal(getNumVisibleRows(table), 5, 'shows all rows now'); table.setProps({ columns: columns, data: [simpleData[0]] }); selectItemsPerPage(table, 25); assert.equal( getItemsPerPage(table), 25, - 'case: 1 data; it successfully changes the items per page to 25' + 'it successfully changes the items per page to 25' ); assert.equal( getCurrentPage(table), @@ -2747,12 +2745,18 @@ describe('LazyMobXTable', () => { }); it('shows the right text before the paging buttons', () => { let table = mount(
); + + table.setProps({ + columns: simpleColumns, + data: data.slice(0, 100), + }); + assert.equal( getItemsPerPage(table), 50, 'confirm 50 items per page' ); - assert.equal(getTextBeforeButtons(table), 'Showing 0-0 of 0'); + assert.equal(getTextBeforeButtons(table), 'Showing 1-5 of 5'); table.setProps({ columns: simpleColumns, data: [simpleData[0]] }); assert.equal(getTextBeforeButtons(table), 'Showing 1-1 of 1'); diff --git a/src/shared/components/lazyMobXTable/LazyMobXTable.tsx b/src/shared/components/lazyMobXTable/LazyMobXTable.tsx index dc14e1668de..2dbedd4ae7e 100644 --- a/src/shared/components/lazyMobXTable/LazyMobXTable.tsx +++ b/src/shared/components/lazyMobXTable/LazyMobXTable.tsx @@ -81,7 +81,10 @@ type LazyMobXTableProps = { columns: Column[]; data?: T[]; dataStore?: ILazyMobXTableApplicationDataStore; - downloadDataFetcher?: ILazyMobXTableApplicationLazyDownloadDataFetcher; + downloadDataFetcher?: + | ILazyMobXTableApplicationLazyDownloadDataFetcher + | (() => Promise) + | undefined; initialSortColumn?: string; initialSortDirection?: SortDirection; initialItemsPerPage?: number; @@ -120,6 +123,12 @@ type LazyMobXTableProps = { ) => JSX.Element | undefined; deactivateColumnFilter?: (columnId: string) => void; customControls?: JSX.Element; + onFilterTextChange?: (filterString: string) => void; + onSortDirectionChange?: (field: string, direction: SortDirection) => void; + isResultLimited?: boolean; + resultCountOverride?: number; + showLoading?: boolean; + loadingComponent?: JSX.Element; }; function compareValues( @@ -279,11 +288,16 @@ export class LazyMobXTableStore { @observable public headerRefs: React.RefObject[]; @observable public downloadDataFetcher: | ILazyMobXTableApplicationLazyDownloadDataFetcher + | undefined + | (() => Promise) | undefined; @observable private onRowClick: ((d: T) => void) | undefined; @observable private onRowMouseEnter: ((d: T) => void) | undefined; @observable private onRowMouseLeave: ((d: T) => void) | undefined; + @observable resultCountOverride: number | undefined; + // now we need to use this in control if passed in. maybe call it override + // this observable is intended to always refer to props.columnToHeaderFilterIconModal @observable private _columnToHeaderFilterIconModal: | ((column: Column) => JSX.Element | undefined) @@ -298,6 +312,8 @@ export class LazyMobXTableStore { | { [columnId: string]: boolean } | undefined; + @observable private onSortDirectionChange: any; + @computed public get itemsPerPage() { return this.dataStore.itemsPerPage; } @@ -440,12 +456,26 @@ export class LazyMobXTableStore { @action public defaultHeaderClick(column: Column) { - this.sortAscending = this.getNextSortAscending(column); - this.dataStore.sortAscending = this.sortAscending; + const sortAscending = this.getNextSortAscending(column); + + if (this.onSortDirectionChange) { + // even though are tracking sort direction in parent store + // we unfortunately still need keep these properties in sync + // a larger refactor would be necessary + this.sortAscending = !this.sortAscending; + this.sortColumn = column.name; + this.onSortDirectionChange( + column.name, + this.sortAscending ? 'asc' : 'desc' + ); + } else { + this.sortAscending = sortAscending; + this.dataStore.sortAscending = sortAscending; - this.sortColumn = column.name; - this.dataStore.sortMetric = this.sortMetric; - this.page = 0; + this.sortColumn = column.name; + this.dataStore.sortMetric = this.sortMetric; + this.page = 0; + } } @computed @@ -618,7 +648,10 @@ export class LazyMobXTableStore { itemsLabel = ` ${itemsLabel}`; } - return `Showing ${firstVisibleItemDisp}-${lastVisibleItemDisp} of ${this.displayData.length}${itemsLabel}`; + // this allows us override the count for display purposes when we are in the + // result limited scenario + const total = this.resultCountOverride || this.displayData.length; + return `Showing ${firstVisibleItemDisp}-${lastVisibleItemDisp} of ${total}${itemsLabel}`; } @computed get tds(): JSX.Element[][] { @@ -765,6 +798,9 @@ export class LazyMobXTableStore { this.onRowClick = props.onRowClick; this.onRowMouseEnter = props.onRowMouseEnter; this.onRowMouseLeave = props.onRowMouseLeave; + this.onSortDirectionChange = props.onSortDirectionChange; + + this.resultCountOverride = props.resultCountOverride; if (props.dataStore) { this.dataStore = props.dataStore; @@ -889,23 +925,32 @@ export default class LazyMobXTable extends React.Component< // we need to download all the lazy data before initiating the download process. if (this.store.downloadDataFetcher) { // populate the cache instances with all available data for the lazy loaded columns - this.store.downloadDataFetcher - .fetchAndCacheAllLazyData() - .then(() => { - // we don't use allData directly, - // we rely on the data cached by the download data fetcher + if (typeof this.store.downloadDataFetcher === 'function') { + this.store.downloadDataFetcher().then(data => { resolve({ status: 'complete', - text: this.getDownloadData(), - }); - }) - .catch(() => { - // even if loading of all lazy data fails, resolve with partial data - resolve({ - status: 'incomplete', - text: this.getDownloadData(), + text: JSON.stringify(data), }); }); + } else { + this.store.downloadDataFetcher + .fetchAndCacheAllLazyData() + .then(() => { + // we don't use allData directly, + // we rely on the data cached by the download data fetcher + resolve({ + status: 'complete', + text: this.getDownloadData(), + }); + }) + .catch(() => { + // even if loading of all lazy data fails, resolve with partial data + resolve({ + status: 'incomplete', + text: this.getDownloadData(), + }); + }); + } } // no lazy data to preload, just return the current download data else { @@ -929,7 +974,9 @@ export default class LazyMobXTable extends React.Component< this.handlers = { onFilterTextChange: (() => { return inputBoxChangeTimeoutEvent(filterValue => { - this.store.setFilterString(filterValue); + props.onFilterTextChange + ? props.onFilterTextChange(filterValue) + : this.store.setFilterString(filterValue); }, 400); })(), clearFilterText: () => { @@ -1097,10 +1144,12 @@ export default class LazyMobXTable extends React.Component< this.props.paginationProps ); } - return ; - // } else { - // return null; - // } + + return this.store.displayData.length > 0 ? ( + + ) : ( + <> + ); } private get countHeader() { @@ -1259,11 +1308,16 @@ export default class LazyMobXTable extends React.Component< : 'visible', }} > - + {(this.props.showLoading && this.props.loadingComponent) || + null} + + + + ); } @@ -1273,6 +1327,17 @@ export default class LazyMobXTable extends React.Component<
{this.getTopToolbar} {this.getTable} + {this.props.isResultLimited && + this.store.maxPage > 0 && + this.store.page === this.store.maxPage && ( +
+ + You've reached the maximum viewable records.{' '} +
+ Please use filter or sort to refine result set. + +
+ )} {!this.props.showPaginationAtTop && ( {this.getBottomToolbar} )} diff --git a/src/shared/components/mutationTable/MutationTable.tsx b/src/shared/components/mutationTable/MutationTable.tsx index 0b6d8966f10..ffe8ecfb5bb 100644 --- a/src/shared/components/mutationTable/MutationTable.tsx +++ b/src/shared/components/mutationTable/MutationTable.tsx @@ -137,7 +137,10 @@ export interface IMutationTableProps { namespaceColumns?: NamespaceColumnConfig; data?: Mutation[][]; dataStore?: ILazyMobXTableApplicationDataStore; - downloadDataFetcher?: ILazyMobXTableApplicationLazyDownloadDataFetcher; + downloadDataFetcher?: + | ILazyMobXTableApplicationLazyDownloadDataFetcher + | (() => Promise) + | undefined; initialItemsPerPage?: number; itemsLabel?: string; itemsLabelPlural?: string;