Skip to content

Commit aaa0b06

Browse files
Feat/discovery enhanced loading (#1599)
* feat(discoveryEnhancedLoading): Initial commit * feat(discoveryEnhancedLoading): Updated all other components * feat(discoveryEnhancedLoading): Updated CSS * feat(discoveryEnhancedLoading): added logic to conditional chnage numberOfStudiesForSmallerBatch * feat(discoveryEnhancedLoading): Updated var name to numberOfBatchesLoaded to improve clarity * feat(discoveryEnhancedLoading): added var totalNumberOfStudiesFromSmallBatches to remove magic number * feat(discoveryEnhancedLoading): started unit test for index.tsx * feat(discoveryEnhancedLoading): updated unit test for index.tsx * feat(discoveryEnhancedLoading): Committing progress on index.tsx unit test * feat(discoveryEnhancedLoading): Added MDSUtils unit test * feat(discoveryEnhancedLoading): updated MDSUtils.test.jsx * feat(discoveryEnhancedLoading): added aggMDSUtils.test.jsx and organized utils into utils folder * feat(discoveryEnhancedLoading): removed index.test.tsx because was unable to write a test to test new logic, updated MDSUtils test * feat(discoveryEnhancedLoading): Updated imports for StudyRegistration * feat(discoveryEnhancedLoading): Undid auto code formatting to avoid excessive line changes * feat(discoveryEnhancedLoading): Ran linter, resolved ESLINT err * feat(discoveryEnhancedLoading): Fixed unused var lint err * feat(discoveryEnhancedLoading): Updated aggMDSUtils to try to placate failing test that only fails in Github * feat(discoveryEnhancedLoading): Changed references from allBatchesAreLoaded to allBatchesAreReady to improve clarity * feat(discoveryEnhancedLoading): updated comment for clarity * feat(discoveryEnhancedLoading): began refactor of MDSUtils to ingest multiple params * feat(discoveryEnhancedLoading): changed variable name for clarity * feat(discoveryEnhancedLoading): reverted unintentionally changed files * feat(discoveryEnhancedLoading): formated for consistency and with linter * feat(discoveryEnhancedLoading): updated unit test to use new parameter
1 parent 079b69d commit aaa0b06

12 files changed

+336
-35
lines changed

src/Discovery/Discovery.css

+2-1
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
margin-bottom: 16px;
4545
display: flex;
4646
flex-direction: row;
47-
align-items: center;
47+
align-items: start;
4848
}
4949

5050
.discovery-header__stats-container {
@@ -104,6 +104,7 @@
104104
.discovery-header__dropdown-tags-container {
105105
flex: 3 1 60%;
106106
height: 90%;
107+
margin-top: 15px;
107108
}
108109

109110
.discovery-header__tags-header {

src/Discovery/Discovery.tsx

+22-7
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import React, {
55
import * as JsSearch from 'js-search';
66
import jsonpath from 'jsonpath';
77
import {
8-
Tag, Popover, Space, Collapse, Button, Dropdown, Pagination, Tooltip,
8+
Tag, Popover, Space, Collapse, Button, Dropdown, Pagination, Tooltip, Spin,
99
} from 'antd';
1010
import {
1111
LockOutlined,
@@ -216,6 +216,7 @@ export interface Props {
216216
onAccessSortDirectionSet: (accessSortDirection: AccessSortDirection) => any,
217217
onResourcesSelected: (resources: DiscoveryResource[]) => any,
218218
onPaginationSet: (pagination: { currentPage: number, resultsPerPage: number }) => any,
219+
allBatchesAreReady: boolean,
219220
}
220221

221222
const Discovery: React.FunctionComponent<Props> = (props: Props) => {
@@ -240,6 +241,13 @@ const Discovery: React.FunctionComponent<Props> = (props: Props) => {
240241
const [discoveryTopPadding, setDiscoveryTopPadding] = useState(30);
241242
const discoveryAccessibilityLinksRef = useRef(null);
242243

244+
const batchesAreLoading = props.allBatchesAreReady === false;
245+
const BatchLoadingSpinner = () => (
246+
<div style={{ textAlign: 'center' }}>
247+
<Spin />
248+
</div>
249+
);
250+
243251
const handleSearchChange = (ev) => {
244252
const { value } = ev.currentTarget;
245253
props.onSearchChange(value);
@@ -666,6 +674,7 @@ const Discovery: React.FunctionComponent<Props> = (props: Props) => {
666674
{/* Header with stats */}
667675
<div className='discovery-header'>
668676
<DiscoverySummary
677+
allBatchesAreReady={props.allBatchesAreReady}
669678
visibleResources={visibleResources}
670679
config={config}
671680
/>
@@ -706,12 +715,18 @@ const Discovery: React.FunctionComponent<Props> = (props: Props) => {
706715
<Collapse activeKey={(searchableTagCollapsed) ? '' : '1'} ghost>
707716
<Panel className='discovery-header__dropdown-tags-display-panel' header='' key='1'>
708717
<div className='discovery-header__dropdown-tags'>
709-
<DiscoveryDropdownTagViewer
710-
config={config}
711-
studies={props.studies}
712-
selectedTags={props.selectedTags}
713-
setSelectedTags={props.onTagsSelected}
714-
/>
718+
{ batchesAreLoading
719+
? (
720+
<BatchLoadingSpinner />
721+
)
722+
: (
723+
<DiscoveryDropdownTagViewer
724+
config={config}
725+
studies={props.studies}
726+
selectedTags={props.selectedTags}
727+
setSelectedTags={props.onTagsSelected}
728+
/>
729+
)}
715730
</div>
716731
</Panel>
717732
</Collapse>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
.discovery-loading-progress-bar {
2+
text-align: center;
3+
margin-bottom: 5px;
4+
}
5+
6+
.discovery-loading-progress-bar .discovery-header__stat-label {
7+
line-height: normal;
8+
text-transform: inherit;
9+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import React from 'react';
2+
import { render, screen } from '@testing-library/react';
3+
import '@testing-library/jest-dom';
4+
import DiscoveryLoadingProgressBar from './DiscoveryLoadingProgressBar';
5+
6+
describe('DiscoveryLoadingProgressBar', () => {
7+
it('renders the progress bar and loading text when displayProgressBar is true', () => {
8+
render(<DiscoveryLoadingProgressBar allBatchesAreReady={false} />);
9+
expect(screen.getByRole('progressbar')).toBeInTheDocument();
10+
expect(screen.getByText('Loading studies...')).toBeInTheDocument();
11+
});
12+
13+
it('sets progress to 100% when allBatchesAreReady is true', () => {
14+
render(<DiscoveryLoadingProgressBar allBatchesAreReady />);
15+
const progressBar = screen.getByRole('progressbar');
16+
expect(progressBar).toHaveAttribute('aria-valuenow', '100');
17+
});
18+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import React, { useState, useEffect } from 'react';
2+
import { Progress } from 'antd';
3+
import './DiscoveryLoadingProgress.css';
4+
5+
interface DiscoveryLoadingProgressBarProps {
6+
allBatchesAreReady: boolean;
7+
}
8+
9+
const DiscoveryLoadingProgressBar = ({
10+
allBatchesAreReady,
11+
}: DiscoveryLoadingProgressBarProps) => {
12+
const [percent, setPercent] = useState(0);
13+
const [displayProgressBar, setDisplayProgressBar] = useState(true);
14+
15+
// Auto incrementing percent logic
16+
const percentUpdateInterval = 500;
17+
const percentIncrementAmount = 5;
18+
19+
useEffect(() => {
20+
const interval = setInterval(() => {
21+
setPercent((prevPercent) => prevPercent + percentIncrementAmount);
22+
}, percentUpdateInterval);
23+
return () => clearInterval(interval);
24+
}, [percent, allBatchesAreReady]);
25+
26+
// hide the bar after a delay after the batches are ready,
27+
// giving the browser some time to process the batch
28+
const delayTimeBeforeHidingProgressBar = 2500;
29+
useEffect(() => {
30+
if (allBatchesAreReady) {
31+
setPercent(100);
32+
// Change displayProgressBar to false after delay
33+
setTimeout(() => {
34+
setDisplayProgressBar(false);
35+
}, delayTimeBeforeHidingProgressBar);
36+
}
37+
}, [allBatchesAreReady]);
38+
39+
return (
40+
<React.Fragment>
41+
{ displayProgressBar && (
42+
<div className='discovery-loading-progress-bar'>
43+
<Progress
44+
width={80}
45+
showInfo={false}
46+
percent={percent}
47+
status='success'
48+
strokeColor='#99286B'
49+
aria-valuenow={percent}
50+
/>
51+
<p className='discovery-header__stat-label'>
52+
Loading studies...
53+
</p>
54+
</div>
55+
)}
56+
</React.Fragment>
57+
);
58+
};
59+
60+
export default DiscoveryLoadingProgressBar;

src/Discovery/DiscoverySummary.tsx

+3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import uniq from 'lodash/uniq';
33
import sum from 'lodash/sum';
44
import jsonpath from 'jsonpath';
55
import { DiscoveryConfig } from './DiscoveryConfig';
6+
import DiscoveryLoadingProgressBar from './DiscoveryLoadingProgressBar/DiscoveryLoadingProgressBar';
67

78
/**
89
* Check for non-numeric items in an array and convert them to numbers.
@@ -55,6 +56,7 @@ const renderAggregation = (aggregation: AggregationConfig, studies: any[] | null
5556
interface Props {
5657
visibleResources: any[] | null;
5758
config: DiscoveryConfig;
59+
allBatchesAreReady: boolean;
5860
}
5961

6062
const DiscoverySummary = (props: Props) => (
@@ -72,6 +74,7 @@ const DiscoverySummary = (props: Props) => (
7274
{aggregation.name}
7375
</div>
7476
</div>
77+
<DiscoveryLoadingProgressBar allBatchesAreReady={props.allBatchesAreReady} />
7578
</div>
7679
</React.Fragment>
7780
))

src/Discovery/MDSUtils.jsx src/Discovery/Utils/MDSUtils/MDSUtils.jsx

+7-8
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,15 @@
1-
import { mdsURL, studyRegistrationConfig } from '../localconf';
1+
import { mdsURL, studyRegistrationConfig } from '../../../localconf';
22

3-
const LIMIT = 2000; // required or else mds defaults to returning 10 records
43
const STUDY_DATA_FIELD = 'gen3_discovery'; // field in the MDS response that contains the study data
54

6-
const loadStudiesFromMDS = async (guidType = 'discovery_metadata') => {
5+
const loadStudiesFromMDS = async (guidType = 'discovery_metadata', fetchSize = 2000, loadAllMetadata = true) => {
76
try {
87
let allStudies = [];
98
let offset = 0;
10-
// request up to LIMIT studies from MDS at a time.
9+
// request up to fetchSize number of studies from MDS at a time.
1110
let shouldContinue = true;
1211
while (shouldContinue) {
13-
const url = `${mdsURL}?data=True&_guid_type=${guidType}&limit=${LIMIT}&offset=${offset}`;
12+
const url = `${mdsURL}?data=True&_guid_type=${guidType}&limit=${fetchSize}&offset=${offset}`;
1413
// It's OK to disable no-await-in-loop rule here -- it's telling us to refactor
1514
// using Promise.all() so that we can fire multiple requests at one.
1615
// But we WANT to delay sending the next request to MDS until we know we need it.
@@ -30,12 +29,12 @@ const loadStudiesFromMDS = async (guidType = 'discovery_metadata') => {
3029
return study;
3130
});
3231
allStudies = allStudies.concat(studies);
33-
const noMoreStudiesToLoad = studies.length < LIMIT;
34-
if (noMoreStudiesToLoad) {
32+
const noMoreStudiesToLoad = studies.length < fetchSize;
33+
if (noMoreStudiesToLoad || loadAllMetadata === false) {
3534
shouldContinue = false;
3635
return allStudies;
3736
}
38-
offset += LIMIT;
37+
offset += fetchSize;
3938
}
4039
return allStudies;
4140
} catch (err) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import loadStudiesFromMDS from './MDSUtils';
2+
import { mdsURL } from '../../../localconf';
3+
4+
global.fetch = jest.fn();
5+
6+
describe('MDS Data Loading Functions', () => {
7+
afterEach(() => {
8+
jest.clearAllMocks();
9+
});
10+
11+
describe('loadStudiesFromMDS', () => {
12+
it('should load studies successfully with limit of 2000', async () => {
13+
const mockResponse = {
14+
0: { gen3_discovery: { name: 'Study 1' } },
15+
1: { gen3_discovery: { name: 'Study 2' } },
16+
};
17+
fetch.mockResolvedValueOnce({
18+
status: 200,
19+
json: jest.fn().mockResolvedValueOnce(mockResponse),
20+
});
21+
const studies = await loadStudiesFromMDS();
22+
expect(studies).toEqual([{ name: 'Study 1' }, { name: 'Study 2' }]);
23+
expect(fetch).toHaveBeenCalledTimes(1);
24+
expect(fetch).toHaveBeenCalledWith(
25+
`${mdsURL}?data=True&_guid_type=discovery_metadata&limit=2000&offset=0`,
26+
);
27+
});
28+
29+
it('should load studies successfully with limit of 3 with loadAllMetadata false', async () => {
30+
const mockResponse = {
31+
0: { gen3_discovery: { name: 'Study 1' } },
32+
1: { gen3_discovery: { name: 'Study 2' } },
33+
2: { gen3_discovery: { name: 'Study 3' } },
34+
};
35+
fetch.mockResolvedValueOnce({
36+
status: 200,
37+
json: jest.fn().mockResolvedValueOnce(mockResponse),
38+
});
39+
const studies = await loadStudiesFromMDS('discovery_metadata', 3, false);
40+
41+
expect(fetch).toHaveBeenCalledTimes(1);
42+
expect(fetch).toHaveBeenCalledWith(
43+
`${mdsURL}?data=True&_guid_type=discovery_metadata&limit=3&offset=0`,
44+
);
45+
expect(studies).toEqual([{ name: 'Study 1' },{ name: 'Study 2' },{ name: 'Study 3' }]);
46+
});
47+
48+
it('should throw an error on fetch failure', async () => {
49+
const mockResponse = {
50+
0: { gen3_discovery: { name: 'Study 1' } },
51+
1: { gen3_discovery: { name: 'Study 2' } },
52+
};
53+
fetch.mockResolvedValueOnce({
54+
status: 401,
55+
json: jest.fn().mockResolvedValueOnce(mockResponse),
56+
});
57+
const expectedErrorMsg = 'Request for study data failed: Error';
58+
let actualErrorMsg = null;
59+
try {
60+
await loadStudiesFromMDS();
61+
} catch (e) {
62+
actualErrorMsg = e.message;
63+
}
64+
expect(actualErrorMsg.toString().includes(expectedErrorMsg)).toBe(true);
65+
});
66+
67+
it('should load up to 2000 studies, then load more with a secondary request', async () => {
68+
const mockStudies = new Array(2500).fill({ mockStudy: 'info' });
69+
// Simulate first fetch (2000 studies)
70+
fetch.mockImplementationOnce(() => Promise.resolve({
71+
status: 200,
72+
json: () => Promise.resolve(mockStudies.slice(0, 2000)),
73+
}),
74+
);
75+
76+
// Simulate second fetch (500 studies)
77+
fetch.mockImplementationOnce(() => Promise.resolve({
78+
status: 200,
79+
json: () => Promise.resolve(mockStudies.slice(2000, 2500)),
80+
}),
81+
);
82+
const studies = await loadStudiesFromMDS();
83+
expect(fetch).toHaveBeenCalledTimes(2);
84+
expect(studies.length).toBe(2500);
85+
});
86+
});
87+
});

src/Discovery/aggMDSUtils.jsx src/Discovery/Utils/aggMDSUtils/aggMDSUtils.jsx

+3-8
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { discoveryConfig, aggMDSDataURL, studyRegistrationConfig } from '../localconf';
1+
import { discoveryConfig, aggMDSDataURL, studyRegistrationConfig } from '../../../localconf';
22

33
/**
44
* getUniqueTags returns a reduced subset of unique tags for the given tags.
@@ -41,7 +41,7 @@ const loadStudiesFromAggMDSRequests = async (offset, limit) => {
4141
// If the discoveryConfig has a tag with the same name as one of the fields on an entry,
4242
// add the value of that field as a tag.
4343

44-
discoveryConfig.tagCategories.forEach((tag) => {
44+
discoveryConfig?.tagCategories.forEach((tag) => {
4545
if (tag.name in entryUnpacked) {
4646
if (typeof entryUnpacked[tag.name] === 'string') {
4747
const tagValue = entryUnpacked[tag.name];
@@ -64,18 +64,13 @@ const loadStudiesFromAggMDSRequests = async (offset, limit) => {
6464

6565
allStudies = allStudies.concat(editedStudies);
6666
});
67-
6867
return allStudies;
6968
};
7069

71-
const loadStudiesFromAggMDS = async () => {
70+
const loadStudiesFromAggMDS = async (limit = 2000) => {
7271
// Retrieve from aggregate MDS
73-
7472
const offset = 0; // For pagination
75-
const limit = 2000; // Total number of rows requested
76-
7773
const studies = await loadStudiesFromAggMDSRequests(offset, limit);
78-
7974
return studies;
8075
};
8176

0 commit comments

Comments
 (0)