-
Notifications
You must be signed in to change notification settings - Fork 4
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
[DT-777] Filter by participant count #2733
Changes from 21 commits
2cbedca
45612b9
9b2edc9
e2d3e06
37606ba
9f89de8
0f08b2c
8dd3cc4
d03c0bf
e1383f1
d0556f8
c033344
4cc1ac8
44e9f1c
2417d78
af1a501
24ca3b1
c4ed6ee
2d3b3ab
851879e
455abe3
ff293e5
79eaba9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,9 @@ const datasets = [ | |
datasetId: 123456, | ||
datasetIdentifier: `DUOS-123456`, | ||
datasetName: 'Some Dataset 1', | ||
participantCount: 100, | ||
study: { | ||
studyName: 'Some Study 1', | ||
studyId: 1, | ||
dataCustodianEmail: ['Some Data Custodian Email 1'], | ||
} | ||
|
@@ -23,9 +25,11 @@ const props = { | |
|
||
describe('Dataset Search Table tests', () => { | ||
|
||
describe('Data library with three datasets', () => { | ||
describe('Data library with one dataset footer tests', () => { | ||
beforeEach(() => { | ||
cy.initApplicationConfig(); | ||
cy.stub(TerraDataRepo, 'listSnapshotsByDatasetIds').returns({}); | ||
cy.clock(); | ||
mount(<DatasetSearchTable {...props} />); | ||
}); | ||
|
||
|
@@ -38,6 +42,56 @@ describe('Dataset Search Table tests', () => { | |
cy.get('#header-checkbox').click(); | ||
cy.contains('1 dataset selected from 1 study'); | ||
}); | ||
}); | ||
|
||
|
||
describe('Data library filter by participant count tests', () => { | ||
|
||
beforeEach(() => { | ||
cy.initApplicationConfig(); | ||
cy.stub(TerraDataRepo, 'listSnapshotsByDatasetIds').returns({}); | ||
cy.clock(); | ||
}); | ||
|
||
function handler(request, searchText) { | ||
if (JSON.stringify(request.body).includes(searchText)) { | ||
request.reply(['filtered']); | ||
} else { | ||
request.reply([]); | ||
} | ||
} | ||
|
||
|
||
it('When a participant count filter is applied the query is updated', () => { | ||
cy.intercept( | ||
{method: 'POST', url: '**/search/index'}, (req) => { | ||
return handler(req, '{"range":{"participantCount":{"gte":null,"lte":50}}}'); | ||
}).as('searchIndex1'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it required that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good question, no I don't think so I just changed them to detangle as part of the debugging effort but I can change them back now |
||
mount(<DatasetSearchTable {...props} />); | ||
// first clear the default value (100), without clearing first, type('50') would result in input of 10050 | ||
cy.get('#participantCountMax-range-input').clear().type('50'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cypress recommends not chaining calls after clear. It's a little more verbose, but safer to:
|
||
cy.tick(150); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cypress recommends that there be a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I use the clock command in the before each method, but I could move it into each individual test There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you, I did miss that. No need if it's there, that should cover it. |
||
// this api call should have had a request that contained the searchText | ||
cy.wait('@searchIndex1').then((response) => { | ||
expect(response.response.body[0]).to.equal('filtered'); | ||
}); | ||
cy.get('@searchIndex1.all').should('have.length', 1); | ||
|
||
}); | ||
|
||
it('When an invalid participant count filter is applied the query represents the default value', () => { | ||
cy.intercept({method: 'POST', url: '**/search/index'}, (req) => { | ||
// when non-numeric input is entered, the default value (in this case, 100) is used | ||
return handler(req, '{"range":{"participantCount":{"gte":100,"lte":null}}}'); | ||
}).as('searchIndex2'); | ||
mount(<DatasetSearchTable {...props} />); | ||
cy.get('#participantCountMin-range-input').type('test'); | ||
cy.tick(150); | ||
cy.wait('@searchIndex2').then((response) => { | ||
expect(response.response.body[0]).to.equal('filtered'); | ||
}); | ||
cy.get('@searchIndex2.all').should('have.length', 1); | ||
}); | ||
|
||
}); | ||
}); |
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -3,8 +3,8 @@ import Tabs from '@mui/material/Tabs'; | |||||||||
import useOnMount from '@mui/utils/useOnMount'; | ||||||||||
import * as React from 'react'; | ||||||||||
import { Box, Button } from '@mui/material'; | ||||||||||
import { useEffect, useRef, useState } from 'react'; | ||||||||||
import { isEmpty } from 'lodash'; | ||||||||||
import {useEffect, useRef, useState} from 'react'; | ||||||||||
import { isArray, isEmpty } from 'lodash'; | ||||||||||
import { TerraDataRepo } from '../../libs/ajax/TerraDataRepo'; | ||||||||||
import { DatasetSearchTableDisplay } from './DatasetSearchTableDisplay'; | ||||||||||
import { datasetSearchTableTabs } from './DatasetSearchTableConstants'; | ||||||||||
|
@@ -37,7 +37,9 @@ const defaultFilters = { | |||||||||
accessManagement: [], | ||||||||||
dataUse: [], | ||||||||||
dataType: [], | ||||||||||
dac: [] | ||||||||||
dac: [], | ||||||||||
participantCountMin: null, | ||||||||||
participantCountMax: null, | ||||||||||
}; | ||||||||||
|
||||||||||
export const DatasetSearchTable = (props) => { | ||||||||||
|
@@ -49,8 +51,17 @@ export const DatasetSearchTable = (props) => { | |||||||||
const [selectedTable, setSelectedTable] = useState(datasetSearchTableTabs.study); | ||||||||||
const [searchTerm, setSearchTerm] = useState(''); | ||||||||||
|
||||||||||
const isFiltered = (filter, category) => (filters[category]).indexOf(filter) > -1; | ||||||||||
const numSelectedFilters = (filters) => Object.values(filters).reduce((sum, array) => sum + array.length, 0); | ||||||||||
const isFilteredArray = (filter, category) => (filters[category]).indexOf(filter) > -1; | ||||||||||
|
||||||||||
const anyFiltersSelected = (filters) => { | ||||||||||
return Object.values(filters).some((filter) => { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For a single line lambda, you can omit the
Suggested change
I know in terra-ui this was the standard, not sure if duos-ui has an opinion about this. |
||||||||||
if (isArray(filter)) { | ||||||||||
return filter.length > 0; | ||||||||||
} else { | ||||||||||
return filter !== null; | ||||||||||
} | ||||||||||
pshapiro4broad marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
}); | ||||||||||
}; | ||||||||||
|
||||||||||
const getExportableDatasets = async (datasets) => { | ||||||||||
// Note the dataset identifier is in each sub-table row. | ||||||||||
|
@@ -108,7 +119,7 @@ export const DatasetSearchTable = (props) => { | |||||||||
} | ||||||||||
|
||||||||||
let filterQuery = {}; | ||||||||||
if (numSelectedFilters(filters) > 0) { | ||||||||||
if (anyFiltersSelected(filters)) { | ||||||||||
const filterTerms = []; | ||||||||||
|
||||||||||
filterTerms.push({ | ||||||||||
|
@@ -155,6 +166,15 @@ export const DatasetSearchTable = (props) => { | |||||||||
} | ||||||||||
}); | ||||||||||
|
||||||||||
filterTerms.push({ | ||||||||||
'range': { | ||||||||||
'participantCount': { | ||||||||||
'gte': filters.participantCountMin, | ||||||||||
'lte': filters.participantCountMax, | ||||||||||
rjohanek marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||
} | ||||||||||
} | ||||||||||
}); | ||||||||||
|
||||||||||
if (filterTerms.length > 0) { | ||||||||||
filterQuery = [ | ||||||||||
{ | ||||||||||
|
@@ -179,13 +199,19 @@ export const DatasetSearchTable = (props) => { | |||||||||
}; | ||||||||||
}; | ||||||||||
|
||||||||||
const filterHandler = (event, data, category, filter) => { | ||||||||||
var newFilters = _.clone(filters); | ||||||||||
if (!isFiltered(filter, category) && filter !== '') { | ||||||||||
newFilters[category] = filters[category].concat(filter); | ||||||||||
const filterHandler = (category, filter) => { | ||||||||||
let newFilter; | ||||||||||
if (isArray(filters[category])) { | ||||||||||
if (!isFilteredArray(filter, category) && filter !== '') { | ||||||||||
newFilter = filters[category].concat(filter); | ||||||||||
} else { | ||||||||||
newFilter = filters[category].filter((f) => f !== filter); | ||||||||||
} | ||||||||||
} else { | ||||||||||
newFilters[category] = filters[category].filter((f) => f !== filter); | ||||||||||
newFilter = filter; | ||||||||||
} | ||||||||||
const newFilters = _.clone(filters); | ||||||||||
newFilters[category] = newFilter; | ||||||||||
setFilters(newFilters); | ||||||||||
}; | ||||||||||
|
||||||||||
|
@@ -280,7 +306,7 @@ export const DatasetSearchTable = (props) => { | |||||||||
</Box> | ||||||||||
<Box sx={{display: 'flex', flexDirection: 'row', paddingTop: '2em'}}> | ||||||||||
<Box sx={{width: '14%', padding: '0 1em'}}> | ||||||||||
<DatasetFilterList datasets={datasets} filters={filters} filterHandler={filterHandler} isFiltered={isFiltered} onClear={() => setFilters(defaultFilters)}/> | ||||||||||
<DatasetFilterList datasets={datasets} filterHandler={filterHandler} isFiltered={isFilteredArray} onClear={() => setFilters(defaultFilters)}/> | ||||||||||
</Box> | ||||||||||
<Box sx={{width: '85%', padding: '0 1em'}}> | ||||||||||
{(() => { | ||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unused