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

Test/aggmds counts #1064

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
2 changes: 1 addition & 1 deletion src/Discovery/Discovery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const renderFieldContent = (content: any, contentType: 'string'|'paragrap
if (Array.isArray(content)) {
return content.join(', ');
}
return content.toLocaleString();
return content ? content.toLocaleString() : 'N/A';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice I like this as this won't crash undef values

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two questions: does a missing content value need to be type matched? eg: if an int is missing the portal code should be looking for 0 as opposed to N/A? also how about __manifest? If there is none, that should be [] as opposed to N/A?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I have updated the expression

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use valueIfNotAvailable to handle missing values? Just like what we were doing in existing codes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works, however, it requires the configuration to ensure all fields are using valueIfNotAvailable
But that is cleaner solution

case 'paragraphs':
return content.split('\n').map((paragraph, i) => <p key={i}>{paragraph}</p>);
case 'link':
Expand Down
23 changes: 15 additions & 8 deletions src/Discovery/DiscoveryActionBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
import FileSaver from 'file-saver';
import { DiscoveryConfig } from './DiscoveryConfig';
import { fetchWithCreds } from '../actions';
import { loadManifestFromResources } from './aggMDSUtils';
import {
manifestServiceApiPath, hostname, jobAPIPath, externalLoginOptionsUrl,
} from '../localconf';
Expand Down Expand Up @@ -247,14 +248,16 @@ const checkDownloadStatus = (

const handleDownloadZipClick = async (
config: DiscoveryConfig,
selectedResources: any[],
selectedStudies: any[],
downloadStatus: DownloadStatus,
setDownloadStatus: (arg0: DownloadStatus) => void,
history,
location,
) => {
const { manifestFieldName } = config.features.exportToWorkspace;
const selectedResources = await loadManifestFromResources(selectedStudies, manifestFieldName);

if (config.features.exportToWorkspace.verifyExternalLogins) {
const { manifestFieldName } = config.features.exportToWorkspace;
const isLinked = await checkFederatedLoginStatus(setDownloadStatus, selectedResources, manifestFieldName, history, location);
if (!isLinked) {
return;
Expand Down Expand Up @@ -295,13 +298,15 @@ const handleDownloadZipClick = async (
).catch(() => setDownloadStatus(DOWNLOAD_FAIL_STATUS));
};

const handleDownloadManifestClick = (config: DiscoveryConfig, selectedResources: any[]) => {
const handleDownloadManifestClick = async (config: DiscoveryConfig, selectedStudies: any[]) => {
const { manifestFieldName } = config.features.exportToWorkspace;
if (!manifestFieldName) {
throw new Error('Missing required configuration field `config.features.exportToWorkspace.manifestFieldName`');
}
// combine manifests from all selected studies
const manifest = [];
// update the manifest to handle aggregated manifestFieldName
const selectedResources = await loadManifestFromResources(selectedStudies, manifestFieldName);
selectedResources.forEach((study) => {
if (study[manifestFieldName]) {
if ('commons_url' in study && !(hostname.includes(study.commons_url))) { // PlanX addition to allow hostname based DRS in manifest download clients
Expand Down Expand Up @@ -343,9 +348,11 @@ const handleExportToWorkspaceClick = async (
throw new Error('Missing required configuration field `config.features.exportToWorkspace.manifestFieldName`');
}

const selectedResourcesWithManifest = await loadManifestFromResources(selectedResources, manifestFieldName);

if (config.features.exportToWorkspace.verifyExternalLogins) {
const isLinked = await checkFederatedLoginStatus(
setDownloadStatus, selectedResources, manifestFieldName, history, location,
setDownloadStatus, selectedResourcesWithManifest, manifestFieldName, history, location,
);
if (!isLinked) {
return;
Expand All @@ -355,7 +362,7 @@ const handleExportToWorkspaceClick = async (
setExportingToWorkspace(true);
// combine manifests from all selected studies
const manifest = [];
selectedResources.forEach((study) => {
selectedResourcesWithManifest.forEach((study) => {
if (study[manifestFieldName]) {
if ('commons_url' in study && !(hostname.includes(study.commons_url))) { // PlanX addition to allow hostname based DRS in manifest download clients
// like FUSE
Expand All @@ -370,9 +377,9 @@ const handleExportToWorkspaceClick = async (
}
});

const projectNumber = selectedResources.map((study) => study.project_number);
const studyName = selectedResources.map((study) => study.study_name);
const repositoryName = selectedResources.map((study) => study.commons);
const projectNumber = selectedResourcesWithManifest.map((study) => study.project_number);
const studyName = selectedResourcesWithManifest.map((study) => study.study_name);
const repositoryName = selectedResourcesWithManifest.map((study) => study.commons);
datadogRum.addAction('exportToWorkspace', {
exportToWorkspaceProjectNumber: projectNumber,
exportToWorkspaceStudyName: studyName,
Expand Down
36 changes: 29 additions & 7 deletions src/Discovery/aggMDSUtils.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ const retrieveCommonsInfo = async (commonsName) => {
throw new Error(`Request for commons info at ${url} failed. Response: ${JSON.stringify(res, null, 2)}`);
}

const jsonResponse = await res.json();

return jsonResponse;
return res.json();
};

/**
Expand All @@ -20,8 +18,8 @@ const retrieveCommonsInfo = async (commonsName) => {
*/
const getUniqueTags = ((tags) => tags.filter((v, i, a) => a.findIndex((t) => (t.category === v.category && t.name === v.name)) === i));

const loadStudiesFromAggMDSRequests = async (offset, limit) => {
const url = `${aggMDSDataURL}?data=True&limit=${limit}&offset=${offset}`;
const loadStudiesFromAggMDSRequests = async (offset, limit, manifestFieldName) => {
const url = `${aggMDSDataURL}?data=True&limit=${limit}&offset=${offset}&counts=${manifestFieldName}`;

const res = await fetch(url);
if (res.status !== 200) {
Expand Down Expand Up @@ -63,6 +61,7 @@ const loadStudiesFromAggMDSRequests = async (offset, limit) => {
x.study_id = studyId;
x.commons = commonsName;
x.frontend_uid = `${commonsName}_${index}`;
x.guid = x._unique_id; // need to preserve for __manifest support
x._unique_id = `${commonsName}_${x._unique_id}_${index}`;
x.commons_url = commonsInfo.commons_url;
x.tags.push(Object({ category: 'Commons', name: commonsName }));
Expand Down Expand Up @@ -93,15 +92,38 @@ const loadStudiesFromAggMDSRequests = async (offset, limit) => {
return allStudies;
};

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

const offset = 0; // For pagination
const limit = 1000; // Total number of rows requested
const { manifestFieldName } = config.features.exportToWorkspace;

const studies = await loadStudiesFromAggMDSRequests(offset, limit);
const studies = await loadStudiesFromAggMDSRequests(offset, limit, manifestFieldName);

return studies;
};

/**
* Will try to get the study manifest data from the aggregateMDS and returns
* the study with the newly populated manifest field
* @param study - study to get a manifest for
* @param manifestFieldName - name of the manifest member (default __manifest)
* @returns {Promise<*>} - the updated study data
*/
const loadStudyFromMDS = async (study, manifestFieldName) => {
if (study[manifestFieldName] === 0) return { study, [manifestFieldName]: [] };

const url = `${aggMDSDataURL}/guid/${study.guid}`;
const resp = await fetch(url);
if (resp.status !== 200) {
throw new Error(`Request for study info at ${url} failed. Response: ${JSON.stringify(resp, null, 2)}`);
}
const studyWithManifest = await resp.json();
return { ...study, [manifestFieldName]: studyWithManifest[manifestFieldName] };
};

// eslint-disable-next-line max-len
export const loadManifestFromResources = async (selectedResources, manifestFieldName) => Promise.all(selectedResources.map((x) => loadStudyFromMDS(x, manifestFieldName)));

export default loadStudiesFromAggMDS;