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

Nickakhmetov/HMP-152 protocols fix #3165

Merged
merged 19 commits into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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: 3 additions & 0 deletions CHANGELOG-hmp-152.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- Add support for multiple comma-separated protocols.io links.
- Improve parsing of protocols.io links.
- Restore display of public protocols.io links.
4 changes: 4 additions & 0 deletions context/app/default_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,7 @@ class DefaultConfig(object):
SECRET_KEY = 'should-be-overridden'
APP_CLIENT_ID = 'should-be-overridden'
APP_CLIENT_SECRET = 'should-be-overridden'

PROTOCOLS_IO_CLIENT_ID = 'should-be-overridden'
PROTOCOLS_IO_CLIENT_SECRET = 'should-be-overridden'
PROTOCOLS_IO_CLIENT_AUTH_TOKEN = 'should-be-overridden'
43 changes: 29 additions & 14 deletions context/app/static/js/components/Providers.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import React, { useMemo } from 'react';
import { SWRConfig } from 'swr';
import { FlaskDataContext, AppContext } from 'js/components/Contexts';
import { ThemeProvider } from 'styled-components';
import PropTypes from 'prop-types';
import { MuiThemeProvider, StylesProvider, createGenerateClassName } from '@material-ui/core/styles';
import CssBaseline from '@material-ui/core/CssBaseline';
import GlobalStyles from 'js/components/globalStyles';
import { ProtocolAPIContext } from 'js/components/detailPage/Protocol/ProtocolAPIContext';
import theme from '../theme';
import GlobalFonts from '../fonts';

Expand Down Expand Up @@ -37,22 +39,35 @@ function Providers({
[groupsToken, workspacesToken, isWorkspacesUser, isHubmapUser, isAuthenticated, userEmail, endpoints],
);

const protocolsContext = useMemo(
() => ({ protocolsClientId: flaskData?.protocolsClientId, clientAuthToken: flaskData?.protocolsClientToken }),
[flaskData],
);

return (
// injectFirst ensures styled-components takes priority over mui for styling
<StylesProvider generateClassName={generateClassName} injectFirst>
<GlobalFonts />
<MuiThemeProvider theme={theme}>
<ThemeProvider theme={theme}>
<AppContext.Provider value={appContext}>
<FlaskDataContext.Provider value={flaskData}>
<CssBaseline />
<GlobalStyles />
{children}
</FlaskDataContext.Provider>
</AppContext.Provider>
</ThemeProvider>
</MuiThemeProvider>
</StylesProvider>
<SWRConfig
value={{
revalidateOnFocus: false,
}}
>
<StylesProvider generateClassName={generateClassName} injectFirst>
<GlobalFonts />
<MuiThemeProvider theme={theme}>
<ThemeProvider theme={theme}>
<AppContext.Provider value={appContext}>
<FlaskDataContext.Provider value={flaskData}>
<ProtocolAPIContext.Provider value={protocolsContext}>
<CssBaseline />
<GlobalStyles />
{children}
</ProtocolAPIContext.Provider>
</FlaskDataContext.Provider>
</AppContext.Provider>
</ThemeProvider>
</MuiThemeProvider>
</StylesProvider>
</SWRConfig>
);
}

Expand Down
41 changes: 26 additions & 15 deletions context/app/static/js/components/detailPage/Protocol/Protocol.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,49 @@ import PropTypes from 'prop-types';
import Divider from '@material-ui/core/Divider';

import OutboundIconLink from 'js/shared-styles/Links/iconLinks/OutboundIconLink';
import useProtocolData from 'js/hooks/useProtocolData';
import EmailIconLink from 'js/shared-styles/Links/iconLinks/EmailIconLink';
import useProtocolData, { useFormattedProtocolUrls } from 'js/hooks/useProtocolData';
import SectionHeader from 'js/shared-styles/sections/SectionHeader';
import { DetailPageSection } from 'js/components/detailPage/style';
import { StyledPaper } from './style';
import SectionItem from '../SectionItem';

function ProtocolLink({ title, resolverHostnameAndDOI }) {
function ProtocolLink({ url, index }) {
const { loading, data, error } = useProtocolData(url);
if (error || loading || !data) {
if (index !== 0) {
// Only show loading message for first protocol link
return null;
}
// Extra `div` wrapper is necessary to prevent the email icon link from taking up the full width and breaking text
return (
<SectionItem>
<div>
Protocols are loading. If protocols take a significant time to load, please contact{' '}
<EmailIconLink email="[email protected]">[email protected]</EmailIconLink> about this issue
and mention the HuBMAP ID.
</div>
</SectionItem>
);
}
return (
<SectionItem label={title}>
{resolverHostnameAndDOI ? (
<OutboundIconLink href={`https://${resolverHostnameAndDOI}`}>{resolverHostnameAndDOI}</OutboundIconLink>
) : (
'Please wait...'
)}
<SectionItem label={data?.payload?.title}>
<OutboundIconLink href={data?.payload?.url}>{data?.payload?.url}</OutboundIconLink>
</SectionItem>
);
}

function Protocol({ protocol_url }) {
const matchedDoiSuffix = protocol_url.match(/\w*$/)[0];

const protocolData = useProtocolData(matchedDoiSuffix, 1);

const title = protocolData?.protocol?.title;
const resolverHostnameAndDOI = protocolData?.protocol?.doi;
const protocolUrls = useFormattedProtocolUrls(protocol_url, 1);

return (
<DetailPageSection id="protocols">
<SectionHeader>Protocols</SectionHeader>
<Divider />
<StyledPaper>
<ProtocolLink title={title} resolverHostnameAndDOI={resolverHostnameAndDOI} />
{protocolUrls.map((url, index) => (
<ProtocolLink key={url} url={url} index={index} />
))}
</StyledPaper>
</DetailPageSection>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { createContext, useContext } from 'react';

type ProtocolAPIContextType = {
clientId: string;
clientAuthToken: string;
};

export const ProtocolAPIContext = createContext<ProtocolAPIContextType | null>(null);

export const useProtocolAPIContext = () => useContext(ProtocolAPIContext);
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import useSWR from 'swr';
import { useAppContext } from 'js/components/Contexts';
import { multiFetcher } from 'js/helpers/multiFetcher';
import { multiFetcher } from 'js/helpers/swr';
import { fillUrls } from './utils';
import { PublicationVignette } from '../types';

Expand All @@ -16,10 +16,7 @@ export function usePublicationVignetteConfs({ uuid, vignetteDirName, vignette }:
const urls = vignette.figures?.map(
({ file }) => `${assetsEndpoint}/${uuid}/vignettes/${vignetteDirName}/${file}?token=${groupsToken}`,
);
const { data } = useSWR(urls, multiFetcher, {
revalidateOnFocus: false,
revalidateOnReconnect: false,
});
const { data } = useSWR(urls, multiFetcher);

if (data) {
const urlHandler = (url: string, isZarr: boolean) => {
Expand All @@ -34,8 +31,8 @@ export function usePublicationVignetteConfs({ uuid, vignetteDirName, vignette }:
};
};
// Formats the vitessce config data to replace the {{ base_url }} placeholder with the actual url.
// TODO: Improve this `unknown`; I couldn't figure out how to import the appropriate `VitessceConfig` type from Vitessce.
const formattedData: unknown[] = data.map((d) => fillUrls(d, urlHandler, requestInitHandler));
// TODO: Improve this `object` type; I couldn't figure out how to import the appropriate `VitessceConfig` type from Vitessce.
const formattedData: object[] = data.map((d) => fillUrls(d as object, urlHandler, requestInitHandler));
return formattedData;
}
return undefined;
Expand Down
5 changes: 0 additions & 5 deletions context/app/static/js/helpers/multiFetcher.ts

This file was deleted.

32 changes: 32 additions & 0 deletions context/app/static/js/helpers/swr.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* SWR fetcher which accepts an array of URLs and returns the responses as JSON
* A custom requestInit object can be passed to fetch as well.
* @example // without requestInit
* const { data } = useSWR(urls, multiFetcher);
* @example // with requestInit
* const { data } = useSWR({ urls, token }, ({ urls, token }) => multiFetcher(urls, { headers: { Authorization: `Bearer ${token}` } })
* @param urls - Array of URLs to fetch
* @param requestInit - Optional RequestInit object to pass to fetch
*/

export async function multiFetcher<T>(urls: string[], requestInit: RequestInit = {}) {
const f = (url: string) => fetch(url, requestInit).then((response) => response.json());
if (urls.length === 0) {
return Promise.resolve([] as T[]);
}
return Promise.all(urls.map((url) => f(url))) as Promise<T[]>;
}

/**
* SWR fetcher which accepts a single URL and returns the response as JSON.
* A custom requestInit object can be passed to fetch as well.
* @example // without requestInit
* const { data } = useSWR(urls, multiFetcher);
* @example // with requestInit
* const { data } = useSWR({ urls, token }, ({ urls, token }) => multiFetcher(urls, { headers: { Authorization: `Bearer ${token}` } })
* @param urls - Array of URLs to fetch
* @param requestInit - Optional RequestInit object to pass to fetch
*/
export async function fetcher<T>(url: string, requestInit: RequestInit = {}) {
return multiFetcher([url], requestInit).then((data) => data[0]) as Promise<T>;
}
53 changes: 37 additions & 16 deletions context/app/static/js/hooks/useProtocolData.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,43 @@
import React from 'react';
import { useMemo } from 'react';
import useSWR from 'swr';

function useProtocolData(doiSuffix, lastVersion = 1) {
const [protocol, setProtocol] = React.useState({});
React.useEffect(() => {
async function getAndSetProtocol() {
const url = `https://www.protocols.io/api/v3/protocols/${doiSuffix}?last_version=${lastVersion}`;
const response = await fetch(url);
if (!response.ok) {
console.error('Protocol API failed:', url, response);
return;
}
const data = await response.json();
setProtocol(data);
import { fetcher } from 'js/helpers/swr';
import { useAppContext } from 'js/components/Contexts';

export function useFormattedProtocolUrls(protocolUrls, lastVersion) {
return useMemo(() => {
if (protocolUrls.length === 0) {
return [];
}
getAndSetProtocol();
}, [doiSuffix, lastVersion]);
// Handle case with multiple URLs provided in one string and remove leading/trailing whitespace
// If only one string is provided, it will be returned as an array
// "dx.doi.org/10.17504/protocols.io.5qpvob93dl4o/v1, dx.doi.org/10.17504/protocols.io.dm6gpb7p5lzp/v1" ->
// ["dx.doi.org/10.17504/protocols.io.5qpvob93dl4o/v1", "dx.doi.org/10.17504/protocols.io.dm6gpb7p5lzp/v1"]
const protocols = protocolUrls.split(',').map((url) => url.trim());
// Strip `http://` and `https://` from the beginning of the URL if it exists
// https://dx.doi.org/10.17504/protocols.io.btnfnmbn -> dx.doi.org/10.17504/protocols.io.btnfnmbn
const noHttpPrefix = protocols.map((url) => url.replace(/^(?:https?:\/\/)?/i, ''));
// Strip `dx.doi.org/` from the beginning of the URL if it exists
// dx.doi.org/10.17504/protocols.io.btnfnmbn -> 10.17504/protocols.io.btnfnmbn
const noDomainPrefix = noHttpPrefix.map((url) => url.replace(/^dx\.doi\.org\//i, ''));
// Strip version number from end of the URL if it exists
// 10.17504/protocols.io.btnfnmbn/v1 -> 10.17504/protocols.io.btnfnmbn
const noVersionSuffix = noDomainPrefix.map((url) => url.replace(/\/v\d+$/, ''));
// Format into the API call URL
// 10.17504/protocols.io.btnfnmbn -> https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=1
const formattedUrls = noVersionSuffix.map(
(doi) => `https://www.protocols.io/api/v4/protocols/${doi}?last_version=${lastVersion}`,
);
return formattedUrls;
}, [protocolUrls, lastVersion]);
}

return protocol;
function useProtocolData(protocolUrl) {
const { protocolsClientToken } = useAppContext();
const result = useSWR([protocolUrl, protocolsClientToken], ([url, token]) =>
fetcher(url, { headers: { Authorization: `Bearer ${token}` } }),
);
return result;
}

export default useProtocolData;
83 changes: 83 additions & 0 deletions context/app/static/js/hooks/useProtocolData.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { renderHook } from '@testing-library/react-hooks';

import { useFormattedProtocolUrls } from './useProtocolData';

const getResult = (protocols, lastVersion) => {
const { result } = renderHook(({ urls, version }) => useFormattedProtocolUrls(urls, version), {
initialProps: { urls: protocols, version: lastVersion },
});
return result.current;
};

describe('useFormattedProtocolUrls', () => {
it('should format a single URL with no version number', () => {
const protocolUrls = 'https://dx.doi.org/10.17504/protocols.io.btnfnmbn';
const lastVersion = 1;
const result = getResult(protocolUrls, lastVersion);
expect(result).toEqual(['https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']);
});

it('should format multiple URLs with version numbers', () => {
const protocolUrls =
'https://dx.doi.org/10.17504/protocols.io.btnfnmbn/v1, https://dx.doi.org/10.17504/protocols.io.7d5h6en/v2';
const lastVersion = 1;
const result = getResult(protocolUrls, lastVersion);
expect(result).toEqual([
'https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=1',
'https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.7d5h6en?last_version=1',
]);
});

it('should handle URLs with http:// prefix', () => {
const protocolUrls = 'http://dx.doi.org/10.17504/protocols.io.btnfnmbn';
const lastVersion = 1;
const result = getResult(protocolUrls, lastVersion);
expect(result).toEqual(['https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']);
});

it('should handle URLs with https:// prefix', () => {
const protocolUrls = 'https://dx.doi.org/10.17504/protocols.io.btnfnmbn';
const lastVersion = 1;
const result = getResult(protocolUrls, lastVersion);
expect(result).toEqual(['https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']);
});

it('should handle URLs with dx.doi.org/ prefix', () => {
const protocolUrls = 'dx.doi.org/10.17504/protocols.io.btnfnmbn';
const lastVersion = 1;
const result = getResult(protocolUrls, lastVersion);
expect(result).toEqual(['https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']);
});

it('should handle URLs with multiple prefixes', () => {
const protocolUrls =
'https://dx.doi.org/10.17504/protocols.io.btnfnmbn/v1,http://dx.doi.org/10.17504/protocols.io.7d5h6en/v2';
const lastVersion = 1;
const result = getResult(protocolUrls, lastVersion);
expect(result).toEqual([
'https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=1',
'https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.7d5h6en?last_version=1',
]);
});

it('should handle URLs with no http or https prefix', () => {
const protocolUrls = 'dx.doi.org/10.17504/protocols.io.btnfnmbn/v1';
const lastVersion = 1;
const result = getResult(protocolUrls, lastVersion);
expect(result).toEqual(['https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']);
});

it('should handle URLs with no version number', () => {
const protocolUrls = 'https://dx.doi.org/10.17504/protocols.io.btnfnmbn';
const lastVersion = 2;
const result = getResult(protocolUrls, lastVersion);
expect(result).toEqual(['https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=2']);
});

it('should handle empty input', () => {
const protocolUrls = '';
const lastVersion = 1;
const result = getResult(protocolUrls, lastVersion);
expect(result).toEqual([]);
});
});
2 changes: 2 additions & 0 deletions context/app/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ def get_default_flask_data():
'xmodalityEndpoint': current_app.config['XMODALITY_ENDPOINT'],
'workspacesEndpoint': current_app.config['WORKSPACES_ENDPOINT'],
'workspacesWsEndpoint': current_app.config['WORKSPACES_WS_ENDPOINT'],
'protocolsClientId': current_app.config['PROTOCOLS_IO_CLIENT_ID'],
'protocolsClientToken': current_app.config['PROTOCOLS_IO_CLIENT_AUTH_TOKEN'],
},
'globalAlertMd': current_app.config.get('GLOBAL_ALERT_MD')
}
Expand Down
5 changes: 5 additions & 0 deletions example-app.conf
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ SECRET_KEY = 'abc123!'
APP_CLIENT_ID = 'TODO'
APP_CLIENT_SECRET = 'TODO'

PROTOCOLS_IO_CLIENT_ID = 'TODO'
PROTOCOLS_IO_CLIENT_SECRET = 'TODO'
PROTOCOLS_IO_CLIENT_AUTH_TOKEN = 'TODO'


# If the API is not available, uncomment "IS_MOCK";
# Restart is required for it to take effect.
# IS_MOCK = True
Expand Down
Loading