Skip to content

Commit

Permalink
Feat/discovery enhanced loading (#1599)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
jarvisraymond-uchicago authored Oct 4, 2024
1 parent 079b69d commit aaa0b06
Show file tree
Hide file tree
Showing 12 changed files with 336 additions and 35 deletions.
3 changes: 2 additions & 1 deletion src/Discovery/Discovery.css
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
margin-bottom: 16px;
display: flex;
flex-direction: row;
align-items: center;
align-items: start;
}

.discovery-header__stats-container {
Expand Down Expand Up @@ -104,6 +104,7 @@
.discovery-header__dropdown-tags-container {
flex: 3 1 60%;
height: 90%;
margin-top: 15px;
}

.discovery-header__tags-header {
Expand Down
29 changes: 22 additions & 7 deletions src/Discovery/Discovery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import React, {
import * as JsSearch from 'js-search';
import jsonpath from 'jsonpath';
import {
Tag, Popover, Space, Collapse, Button, Dropdown, Pagination, Tooltip,
Tag, Popover, Space, Collapse, Button, Dropdown, Pagination, Tooltip, Spin,
} from 'antd';
import {
LockOutlined,
Expand Down Expand Up @@ -216,6 +216,7 @@ export interface Props {
onAccessSortDirectionSet: (accessSortDirection: AccessSortDirection) => any,
onResourcesSelected: (resources: DiscoveryResource[]) => any,
onPaginationSet: (pagination: { currentPage: number, resultsPerPage: number }) => any,
allBatchesAreReady: boolean,
}

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

const batchesAreLoading = props.allBatchesAreReady === false;
const BatchLoadingSpinner = () => (
<div style={{ textAlign: 'center' }}>
<Spin />
</div>
);

const handleSearchChange = (ev) => {
const { value } = ev.currentTarget;
props.onSearchChange(value);
Expand Down Expand Up @@ -666,6 +674,7 @@ const Discovery: React.FunctionComponent<Props> = (props: Props) => {
{/* Header with stats */}
<div className='discovery-header'>
<DiscoverySummary
allBatchesAreReady={props.allBatchesAreReady}
visibleResources={visibleResources}
config={config}
/>
Expand Down Expand Up @@ -706,12 +715,18 @@ const Discovery: React.FunctionComponent<Props> = (props: Props) => {
<Collapse activeKey={(searchableTagCollapsed) ? '' : '1'} ghost>
<Panel className='discovery-header__dropdown-tags-display-panel' header='' key='1'>
<div className='discovery-header__dropdown-tags'>
<DiscoveryDropdownTagViewer
config={config}
studies={props.studies}
selectedTags={props.selectedTags}
setSelectedTags={props.onTagsSelected}
/>
{ batchesAreLoading
? (
<BatchLoadingSpinner />
)
: (
<DiscoveryDropdownTagViewer
config={config}
studies={props.studies}
selectedTags={props.selectedTags}
setSelectedTags={props.onTagsSelected}
/>
)}
</div>
</Panel>
</Collapse>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
.discovery-loading-progress-bar {
text-align: center;
margin-bottom: 5px;
}

.discovery-loading-progress-bar .discovery-header__stat-label {
line-height: normal;
text-transform: inherit;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import '@testing-library/jest-dom';
import DiscoveryLoadingProgressBar from './DiscoveryLoadingProgressBar';

describe('DiscoveryLoadingProgressBar', () => {
it('renders the progress bar and loading text when displayProgressBar is true', () => {
render(<DiscoveryLoadingProgressBar allBatchesAreReady={false} />);
expect(screen.getByRole('progressbar')).toBeInTheDocument();
expect(screen.getByText('Loading studies...')).toBeInTheDocument();
});

it('sets progress to 100% when allBatchesAreReady is true', () => {
render(<DiscoveryLoadingProgressBar allBatchesAreReady />);
const progressBar = screen.getByRole('progressbar');
expect(progressBar).toHaveAttribute('aria-valuenow', '100');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import React, { useState, useEffect } from 'react';
import { Progress } from 'antd';
import './DiscoveryLoadingProgress.css';

interface DiscoveryLoadingProgressBarProps {
allBatchesAreReady: boolean;
}

const DiscoveryLoadingProgressBar = ({
allBatchesAreReady,
}: DiscoveryLoadingProgressBarProps) => {
const [percent, setPercent] = useState(0);
const [displayProgressBar, setDisplayProgressBar] = useState(true);

// Auto incrementing percent logic
const percentUpdateInterval = 500;
const percentIncrementAmount = 5;

useEffect(() => {
const interval = setInterval(() => {
setPercent((prevPercent) => prevPercent + percentIncrementAmount);
}, percentUpdateInterval);
return () => clearInterval(interval);
}, [percent, allBatchesAreReady]);

// hide the bar after a delay after the batches are ready,
// giving the browser some time to process the batch
const delayTimeBeforeHidingProgressBar = 2500;
useEffect(() => {
if (allBatchesAreReady) {
setPercent(100);
// Change displayProgressBar to false after delay
setTimeout(() => {
setDisplayProgressBar(false);
}, delayTimeBeforeHidingProgressBar);
}
}, [allBatchesAreReady]);

return (
<React.Fragment>
{ displayProgressBar && (
<div className='discovery-loading-progress-bar'>
<Progress
width={80}
showInfo={false}
percent={percent}
status='success'
strokeColor='#99286B'
aria-valuenow={percent}
/>
<p className='discovery-header__stat-label'>
Loading studies...
</p>
</div>
)}
</React.Fragment>
);
};

export default DiscoveryLoadingProgressBar;
3 changes: 3 additions & 0 deletions src/Discovery/DiscoverySummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import uniq from 'lodash/uniq';
import sum from 'lodash/sum';
import jsonpath from 'jsonpath';
import { DiscoveryConfig } from './DiscoveryConfig';
import DiscoveryLoadingProgressBar from './DiscoveryLoadingProgressBar/DiscoveryLoadingProgressBar';

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

const DiscoverySummary = (props: Props) => (
Expand All @@ -72,6 +74,7 @@ const DiscoverySummary = (props: Props) => (
{aggregation.name}
</div>
</div>
<DiscoveryLoadingProgressBar allBatchesAreReady={props.allBatchesAreReady} />
</div>
</React.Fragment>
))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
import { mdsURL, studyRegistrationConfig } from '../localconf';
import { mdsURL, studyRegistrationConfig } from '../../../localconf';

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

const loadStudiesFromMDS = async (guidType = 'discovery_metadata') => {
const loadStudiesFromMDS = async (guidType = 'discovery_metadata', fetchSize = 2000, loadAllMetadata = true) => {
try {
let allStudies = [];
let offset = 0;
// request up to LIMIT studies from MDS at a time.
// request up to fetchSize number of studies from MDS at a time.
let shouldContinue = true;
while (shouldContinue) {
const url = `${mdsURL}?data=True&_guid_type=${guidType}&limit=${LIMIT}&offset=${offset}`;
const url = `${mdsURL}?data=True&_guid_type=${guidType}&limit=${fetchSize}&offset=${offset}`;
// It's OK to disable no-await-in-loop rule here -- it's telling us to refactor
// using Promise.all() so that we can fire multiple requests at one.
// But we WANT to delay sending the next request to MDS until we know we need it.
Expand All @@ -30,12 +29,12 @@ const loadStudiesFromMDS = async (guidType = 'discovery_metadata') => {
return study;
});
allStudies = allStudies.concat(studies);
const noMoreStudiesToLoad = studies.length < LIMIT;
if (noMoreStudiesToLoad) {
const noMoreStudiesToLoad = studies.length < fetchSize;
if (noMoreStudiesToLoad || loadAllMetadata === false) {
shouldContinue = false;
return allStudies;
}
offset += LIMIT;
offset += fetchSize;
}
return allStudies;
} catch (err) {
Expand Down
87 changes: 87 additions & 0 deletions src/Discovery/Utils/MDSUtils/MDSUtils.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
import loadStudiesFromMDS from './MDSUtils';
import { mdsURL } from '../../../localconf';

global.fetch = jest.fn();

describe('MDS Data Loading Functions', () => {
afterEach(() => {
jest.clearAllMocks();
});

describe('loadStudiesFromMDS', () => {
it('should load studies successfully with limit of 2000', async () => {
const mockResponse = {
0: { gen3_discovery: { name: 'Study 1' } },
1: { gen3_discovery: { name: 'Study 2' } },
};
fetch.mockResolvedValueOnce({
status: 200,
json: jest.fn().mockResolvedValueOnce(mockResponse),
});
const studies = await loadStudiesFromMDS();
expect(studies).toEqual([{ name: 'Study 1' }, { name: 'Study 2' }]);
expect(fetch).toHaveBeenCalledTimes(1);
expect(fetch).toHaveBeenCalledWith(
`${mdsURL}?data=True&_guid_type=discovery_metadata&limit=2000&offset=0`,
);
});

it('should load studies successfully with limit of 3 with loadAllMetadata false', async () => {
const mockResponse = {
0: { gen3_discovery: { name: 'Study 1' } },
1: { gen3_discovery: { name: 'Study 2' } },
2: { gen3_discovery: { name: 'Study 3' } },
};
fetch.mockResolvedValueOnce({
status: 200,
json: jest.fn().mockResolvedValueOnce(mockResponse),
});
const studies = await loadStudiesFromMDS('discovery_metadata', 3, false);

expect(fetch).toHaveBeenCalledTimes(1);
expect(fetch).toHaveBeenCalledWith(
`${mdsURL}?data=True&_guid_type=discovery_metadata&limit=3&offset=0`,
);
expect(studies).toEqual([{ name: 'Study 1' },{ name: 'Study 2' },{ name: 'Study 3' }]);
});

it('should throw an error on fetch failure', async () => {
const mockResponse = {
0: { gen3_discovery: { name: 'Study 1' } },
1: { gen3_discovery: { name: 'Study 2' } },
};
fetch.mockResolvedValueOnce({
status: 401,
json: jest.fn().mockResolvedValueOnce(mockResponse),
});
const expectedErrorMsg = 'Request for study data failed: Error';
let actualErrorMsg = null;
try {
await loadStudiesFromMDS();
} catch (e) {
actualErrorMsg = e.message;
}
expect(actualErrorMsg.toString().includes(expectedErrorMsg)).toBe(true);
});

it('should load up to 2000 studies, then load more with a secondary request', async () => {
const mockStudies = new Array(2500).fill({ mockStudy: 'info' });
// Simulate first fetch (2000 studies)
fetch.mockImplementationOnce(() => Promise.resolve({
status: 200,
json: () => Promise.resolve(mockStudies.slice(0, 2000)),
}),
);

// Simulate second fetch (500 studies)
fetch.mockImplementationOnce(() => Promise.resolve({
status: 200,
json: () => Promise.resolve(mockStudies.slice(2000, 2500)),
}),
);
const studies = await loadStudiesFromMDS();
expect(fetch).toHaveBeenCalledTimes(2);
expect(studies.length).toBe(2500);
});
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { discoveryConfig, aggMDSDataURL, studyRegistrationConfig } from '../localconf';
import { discoveryConfig, aggMDSDataURL, studyRegistrationConfig } from '../../../localconf';

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

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

allStudies = allStudies.concat(editedStudies);
});

return allStudies;
};

const loadStudiesFromAggMDS = async () => {
const loadStudiesFromAggMDS = async (limit = 2000) => {
// Retrieve from aggregate MDS

const offset = 0; // For pagination
const limit = 2000; // Total number of rows requested

const studies = await loadStudiesFromAggMDSRequests(offset, limit);

return studies;
};

Expand Down
Loading

0 comments on commit aaa0b06

Please sign in to comment.