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

Feat/discovery enhanced loading #1599

Merged
merged 27 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d727870
feat(discoveryEnhancedLoading): Initial commit
jarvisraymond-uchicago Sep 5, 2024
52122cb
feat(discoveryEnhancedLoading): Updated all other components
jarvisraymond-uchicago Sep 5, 2024
b22ae1b
feat(discoveryEnhancedLoading): Updated CSS
jarvisraymond-uchicago Sep 5, 2024
cfbc79a
feat(discoveryEnhancedLoading): added logic to conditional chnage num…
jarvisraymond-uchicago Sep 5, 2024
560a573
feat(discoveryEnhancedLoading): Updated var name to numberOfBatchesLo…
jarvisraymond-uchicago Sep 13, 2024
fac0c1d
feat(discoveryEnhancedLoading): added var totalNumberOfStudiesFromSma…
jarvisraymond-uchicago Sep 13, 2024
624e2ad
feat(discoveryEnhancedLoading): started unit test for index.tsx
jarvisraymond-uchicago Sep 13, 2024
1f533f4
feat(discoveryEnhancedLoading): updated unit test for index.tsx
jarvisraymond-uchicago Sep 13, 2024
dc64792
feat(discoveryEnhancedLoading): Committing progress on index.tsx unit…
jarvisraymond-uchicago Sep 20, 2024
6f43811
feat(discoveryEnhancedLoading): Added MDSUtils unit test
jarvisraymond-uchicago Sep 20, 2024
5349816
feat(discoveryEnhancedLoading): updated MDSUtils.test.jsx
jarvisraymond-uchicago Sep 20, 2024
0626248
feat(discoveryEnhancedLoading): added aggMDSUtils.test.jsx and organi…
jarvisraymond-uchicago Sep 20, 2024
b1dd85c
feat(discoveryEnhancedLoading): removed index.test.tsx because was un…
jarvisraymond-uchicago Sep 20, 2024
6a80e5e
feat(discoveryEnhancedLoading): Updated imports for StudyRegistration
jarvisraymond-uchicago Sep 26, 2024
781faf7
feat(discoveryEnhancedLoading): Undid auto code formatting to avoid e…
jarvisraymond-uchicago Sep 26, 2024
9970263
feat(discoveryEnhancedLoading): Ran linter, resolved ESLINT err
jarvisraymond-uchicago Sep 26, 2024
c1aef4d
feat(discoveryEnhancedLoading): Fixed unused var lint err
jarvisraymond-uchicago Sep 26, 2024
818cf03
Merge branch 'master' into feat/discoveryEnhancedLoading
jarvisraymond-uchicago Sep 26, 2024
09fe3b7
feat(discoveryEnhancedLoading): Updated aggMDSUtils to try to placate…
jarvisraymond-uchicago Sep 27, 2024
3c09658
feat(discoveryEnhancedLoading): Resolving merge conflict
jarvisraymond-uchicago Oct 3, 2024
6d9b9c5
feat(discoveryEnhancedLoading): Changed references from allBatchesAre…
jarvisraymond-uchicago Oct 3, 2024
790bbd9
feat(discoveryEnhancedLoading): updated comment for clarity
jarvisraymond-uchicago Oct 3, 2024
81cbd25
feat(discoveryEnhancedLoading): began refactor of MDSUtils to ingest …
jarvisraymond-uchicago Oct 4, 2024
f16e30e
feat(discoveryEnhancedLoading): changed variable name for clarity
jarvisraymond-uchicago Oct 4, 2024
2f8b7fa
feat(discoveryEnhancedLoading): reverted unintentionally changed files
jarvisraymond-uchicago Oct 4, 2024
028404c
feat(discoveryEnhancedLoading): formated for consistency and with linter
jarvisraymond-uchicago Oct 4, 2024
28382ef
feat(discoveryEnhancedLoading): updated unit test to use new parameter
jarvisraymond-uchicago Oct 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading