Skip to content

Conversation

@noctillion
Copy link
Contributor

Centralizes drs fetches in ExplorerIndividualContent, adds utils.js hook for IGV-viewable files; parent dispatches all downloads and IGV subset once; removes child dispatches

Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

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

first pass

Copy link
Member

@davidlougheed davidlougheed left a comment

Choose a reason for hiding this comment

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

looking better than before! I think there's more work to be done here to eliminate duplicate code though, especially with experiments result extraction and DRS URL fetching.


useEffect(() => {
if (viewableExperimentResultsForIgv.length > 0) {
dispatch(getIgvUrlsFromDrs(viewableExperimentResultsForIgv)).catch(console.error);
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty sure this action + getFileDownloadUrlsFromDrs basically do the same thing, and so can be combined (both in action and state, by just creating a single state object called urlsByFilename)

Copy link
Member

Choose a reason for hiding this comment

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

this effect can also do everything useIndividualIgvViewableExperimentResults does instead of having a different hook, since useIndividualIgvViewableExperimentResults return value is only used here.

Copy link
Member

Choose a reason for hiding this comment

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

that way, experiments don't have to be extracted twice from the biosamples data.


export const explorerIndividualUrl = (individualID) => `/data/explorer/individuals/${individualID}`;

export const useIndividualIgvViewableExperimentResults = (individual) => {
Copy link
Member

Choose a reason for hiding this comment

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

this is only used once, so this could be put in the useEffect I commented on above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants