From 2fde159b55bfa29c6c2ff31b46410e9df1079ab2 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Fri, 7 Jul 2023 15:50:52 -0400 Subject: [PATCH 01/18] Upgrade nuka-carousel --- vitessce | 1 + 1 file changed, 1 insertion(+) create mode 160000 vitessce diff --git a/vitessce b/vitessce new file mode 160000 index 0000000000..cf48564ff1 --- /dev/null +++ b/vitessce @@ -0,0 +1 @@ +Subproject commit cf48564ff115f1260b20a5dfd1958aaa7048ac3e From 2157b14359cde99bc0cf001df2273b689fdb5106 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Fri, 7 Jul 2023 15:58:37 -0400 Subject: [PATCH 02/18] remove weird submodule thing? --- vitessce | 1 - 1 file changed, 1 deletion(-) delete mode 160000 vitessce diff --git a/vitessce b/vitessce deleted file mode 160000 index cf48564ff1..0000000000 --- a/vitessce +++ /dev/null @@ -1 +0,0 @@ -Subproject commit cf48564ff115f1260b20a5dfd1958aaa7048ac3e From aab15dd693ab12d048e66d25c1be5da839c805fc Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Tue, 11 Jul 2023 16:43:26 -0400 Subject: [PATCH 03/18] WIP Logic for trimming/revising the links works Corresponding specs are failing (`Cannot read properties of null (reading 'useMemo'`) Also need to set up protocols API auth flow --- .../detailPage/Protocol/Protocol.jsx | 11 +-- .../app/static/js/hooks/useProtocolData.js | 52 ++++++++----- .../static/js/hooks/useProtocolData.spec.js | 76 +++++++++++++++++++ 3 files changed, 115 insertions(+), 24 deletions(-) create mode 100644 context/app/static/js/hooks/useProtocolData.spec.js diff --git a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx index 2616cee9ac..c37ae815be 100644 --- a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx +++ b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx @@ -22,19 +22,16 @@ function ProtocolLink({ title, resolverHostnameAndDOI }) { } 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 protocolData = useProtocolData(protocol_url, 1); return ( Protocols - + {protocolData.map(({ title, resolverHostnameAndDOI }) => ( + + ))} ); diff --git a/context/app/static/js/hooks/useProtocolData.js b/context/app/static/js/hooks/useProtocolData.js index cd3f592af2..43616960db 100644 --- a/context/app/static/js/hooks/useProtocolData.js +++ b/context/app/static/js/hooks/useProtocolData.js @@ -1,22 +1,40 @@ -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); - } - getAndSetProtocol(); - }, [doiSuffix, lastVersion]); +import { multiFetcher } from 'js/helpers/multiFetcher'; - return protocol; +export function useFormattedProtocolUrls(protocolUrls, lastVersion) { + return useMemo(() => { + // Handle case with multiple URLs provided in one string + // If only one string is provided, it will be returned as an array + const protocols = protocolUrls.split(','); + // 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/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=1 + const formattedUrls = noVersionSuffix.map( + // TODO: Update to v4 API (see HMP-254) + (doi) => `https://www.protocols.io/api/v3/protocols/${doi}?last_version=${lastVersion}`, + ); + return formattedUrls; + }, [protocolUrls, lastVersion]); +} + +function useProtocolData(protocolUrls, lastVersion = 1) { + const urls = useFormattedProtocolUrls(protocolUrls, lastVersion); + + const protocols = useSWR(urls, multiFetcher, { + revalidateOnFocus: false, + }); + + return protocols.data ?? []; } export default useProtocolData; diff --git a/context/app/static/js/hooks/useProtocolData.spec.js b/context/app/static/js/hooks/useProtocolData.spec.js new file mode 100644 index 0000000000..ec7285c984 --- /dev/null +++ b/context/app/static/js/hooks/useProtocolData.spec.js @@ -0,0 +1,76 @@ +import { renderHook } from '@testing-library/react-hooks'; + +import { useFormattedProtocolUrls } from './useProtocolData'; + +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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + expect(result).toEqual(['https://www.protocols.io/api/v3/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 = 2; + const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + expect(result).toEqual([ + 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=2', + 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.7d5h6en?last_version=2', + ]); + }); + + it('should handle URLs with http:// prefix', () => { + const protocolUrls = 'http://dx.doi.org/10.17504/protocols.io.btnfnmbn'; + const lastVersion = 1; + const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + expect(result).toEqual(['https://www.protocols.io/api/v3/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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + expect(result).toEqual(['https://www.protocols.io/api/v3/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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + expect(result).toEqual(['https://www.protocols.io/api/v3/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 = 2; + const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + expect(result).toEqual([ + 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=2', + 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.7d5h6en?last_version=2', + ]); + }); + + 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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + expect(result).toEqual(['https://www.protocols.io/api/v3/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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + expect(result).toEqual(['https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=2']); + }); + + it('should handle empty input', () => { + const protocolUrls = ''; + const lastVersion = 1; + const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + expect(result).toEqual([]); + }); +}); From 70f6ff3601c01936319fd418e8d58a7735b05dc2 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Wed, 12 Jul 2023 12:31:57 -0400 Subject: [PATCH 04/18] add changelog, fix regex --- CHANGELOG-hmp-152.md | 3 +++ context/app/static/js/hooks/useProtocolData.js | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 CHANGELOG-hmp-152.md diff --git a/CHANGELOG-hmp-152.md b/CHANGELOG-hmp-152.md new file mode 100644 index 0000000000..c233d492a9 --- /dev/null +++ b/CHANGELOG-hmp-152.md @@ -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. diff --git a/context/app/static/js/hooks/useProtocolData.js b/context/app/static/js/hooks/useProtocolData.js index 43616960db..ee77e209b4 100644 --- a/context/app/static/js/hooks/useProtocolData.js +++ b/context/app/static/js/hooks/useProtocolData.js @@ -13,7 +13,7 @@ export function useFormattedProtocolUrls(protocolUrls, lastVersion) { 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, '')); + 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+$/, '')); From 64f7b3bb7a2256df9a04f5e3588289341da6e430 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Thu, 13 Jul 2023 13:35:04 -0400 Subject: [PATCH 05/18] add protocols API values context, pull values from app conf --- context/app/default_config.py | 4 ++++ context/app/static/js/components/Providers.jsx | 14 +++++++++++--- .../detailPage/Protocol/ProtocolAPIContext.tsx | 10 ++++++++++ context/app/utils.py | 2 ++ example-app.conf | 5 +++++ 5 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 context/app/static/js/components/detailPage/Protocol/ProtocolAPIContext.tsx diff --git a/context/app/default_config.py b/context/app/default_config.py index 6699c92f35..763b8cd01f 100644 --- a/context/app/default_config.py +++ b/context/app/default_config.py @@ -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' diff --git a/context/app/static/js/components/Providers.jsx b/context/app/static/js/components/Providers.jsx index a0e63f9514..66d727e7c1 100644 --- a/context/app/static/js/components/Providers.jsx +++ b/context/app/static/js/components/Providers.jsx @@ -7,6 +7,7 @@ import CssBaseline from '@material-ui/core/CssBaseline'; import GlobalStyles from 'js/components/globalStyles'; import theme from '../theme'; import GlobalFonts from '../fonts'; +import { ProtocolAPIContext } from './detailPage/Protocol/ProtocolAPIContext'; const generateClassName = createGenerateClassName({ disableGlobal: true, @@ -35,6 +36,11 @@ function Providers({ [groupsToken, workspacesToken, isWorkspacesUser, isAuthenticated, userEmail, endpoints], ); + const protocolsContext = useMemo( + () => ({ protocolsClientId: flaskData.protocolsClientId, clientAuthToken: flaskData.protocolsClientToken }), + [flaskData], + ); + return ( // injectFirst ensures styled-components takes priority over mui for styling @@ -43,9 +49,11 @@ function Providers({ - - - {children} + + + + {children} + diff --git a/context/app/static/js/components/detailPage/Protocol/ProtocolAPIContext.tsx b/context/app/static/js/components/detailPage/Protocol/ProtocolAPIContext.tsx new file mode 100644 index 0000000000..03d6c52d28 --- /dev/null +++ b/context/app/static/js/components/detailPage/Protocol/ProtocolAPIContext.tsx @@ -0,0 +1,10 @@ +import { createContext, useContext } from 'react'; + +type ProtocolAPIContextType = { + clientId: string; + clientAuthToken: string; +}; + +export const ProtocolAPIContext = createContext(null); + +export const useProtocolAPIContext = () => useContext(ProtocolAPIContext); diff --git a/context/app/utils.py b/context/app/utils.py index af02818dfe..3280d4cc95 100644 --- a/context/app/utils.py +++ b/context/app/utils.py @@ -33,6 +33,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') } diff --git a/example-app.conf b/example-app.conf index 27ef790580..2f27374d84 100644 --- a/example-app.conf +++ b/example-app.conf @@ -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 From 8c8bd1b2dadcbe4e189f7099f4fc408d2ab3554e Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Mon, 17 Jul 2023 17:06:25 -0400 Subject: [PATCH 06/18] make authenticated requests; now seeing data, still not seeing protocol links --- .../components/detailPage/Protocol/Protocol.jsx | 2 +- context/app/static/js/helpers/multiFetcher.ts | 16 ++++++++++++++++ context/app/static/js/hooks/useProtocolData.js | 11 ++++++----- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx index c37ae815be..60b24a5b31 100644 --- a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx +++ b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx @@ -29,7 +29,7 @@ function Protocol({ protocol_url }) { Protocols - {protocolData.map(({ title, resolverHostnameAndDOI }) => ( + {protocolData?.map(({ title, resolverHostnameAndDOI }) => ( ))} diff --git a/context/app/static/js/helpers/multiFetcher.ts b/context/app/static/js/helpers/multiFetcher.ts index e12f94db6a..43d058bf9a 100644 --- a/context/app/static/js/helpers/multiFetcher.ts +++ b/context/app/static/js/helpers/multiFetcher.ts @@ -1,5 +1,21 @@ // Fetcher function that lets SWR fetch multiple urls at once export const multiFetcher = (...urls: string[]) => { const f = (url: string) => fetch(url).then((response) => response.json()); + if (urls.length === 0) { + return Promise.resolve([]); + } + return Promise.all(urls.map((url) => f(url))); +}; + +export const multiFetcherWithToken = (token: string, ...urls: string[]) => { + const f = (url: string) => + fetch(url, { + headers: { + Authorization: `Bearer ${token}`, + }, + }).then((response) => response.json()); + if (urls.length === 0) { + return Promise.resolve([]); + } return Promise.all(urls.map((url) => f(url))); }; diff --git a/context/app/static/js/hooks/useProtocolData.js b/context/app/static/js/hooks/useProtocolData.js index ee77e209b4..b3b0bb3749 100644 --- a/context/app/static/js/hooks/useProtocolData.js +++ b/context/app/static/js/hooks/useProtocolData.js @@ -1,7 +1,8 @@ import { useMemo } from 'react'; import useSWR from 'swr'; -import { multiFetcher } from 'js/helpers/multiFetcher'; +import { multiFetcherWithToken } from 'js/helpers/multiFetcher'; +import { useAppContext } from 'js/components/Contexts'; export function useFormattedProtocolUrls(protocolUrls, lastVersion) { return useMemo(() => { @@ -20,21 +21,21 @@ export function useFormattedProtocolUrls(protocolUrls, lastVersion) { // Format into the API call URL // 10.17504/protocols.io.btnfnmbn -> https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=1 const formattedUrls = noVersionSuffix.map( - // TODO: Update to v4 API (see HMP-254) - (doi) => `https://www.protocols.io/api/v3/protocols/${doi}?last_version=${lastVersion}`, + (doi) => `https://www.protocols.io/api/v4/protocols/${doi}?last_version=${lastVersion}`, ); return formattedUrls; }, [protocolUrls, lastVersion]); } function useProtocolData(protocolUrls, lastVersion = 1) { + const { protocolsClientToken } = useAppContext(); const urls = useFormattedProtocolUrls(protocolUrls, lastVersion); - const protocols = useSWR(urls, multiFetcher, { + const protocols = useSWR([protocolsClientToken, urls], ([token, apiUrls]) => multiFetcherWithToken(token, apiUrls), { revalidateOnFocus: false, }); - return protocols.data ?? []; + return protocols.data; } export default useProtocolData; From cfa962eacaa7ff36c495defa7190463642dfb6c0 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Tue, 18 Jul 2023 11:12:20 -0400 Subject: [PATCH 07/18] =?UTF-8?q?Restore=20protocol=20links!=20?= =?UTF-8?q?=F0=9F=8E=89=20Refactored=20to=20avoid=20requiring=20multifetch?= =?UTF-8?q?er=20for=20this=20case,=20but=20still=20keeping=20introduced=20?= =?UTF-8?q?multifetcher=20for=20other=20cases?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../detailPage/Protocol/Protocol.jsx | 28 +++++++++++-------- context/app/static/js/helpers/fetcher.ts | 11 ++++++++ .../app/static/js/hooks/useProtocolData.js | 11 +++----- 3 files changed, 32 insertions(+), 18 deletions(-) create mode 100644 context/app/static/js/helpers/fetcher.ts diff --git a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx index 60b24a5b31..dea64807fd 100644 --- a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx +++ b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx @@ -3,34 +3,40 @@ 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 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 }) { + const { loading, data, error } = useProtocolData(url); + if (error) { + + This protocol may be private or otherwise inaccessible. + ; + } + if (loading || !data) { + Please wait...; + } + console.log(data?.payload); return ( - - {resolverHostnameAndDOI ? ( - {resolverHostnameAndDOI} - ) : ( - 'Please wait...' - )} + + {data?.payload?.url} ); } function Protocol({ protocol_url }) { - const protocolData = useProtocolData(protocol_url, 1); + const protocolUrls = useFormattedProtocolUrls(protocol_url, 1); return ( Protocols - {protocolData?.map(({ title, resolverHostnameAndDOI }) => ( - + {protocolUrls.map((url) => ( + ))} diff --git a/context/app/static/js/helpers/fetcher.ts b/context/app/static/js/helpers/fetcher.ts new file mode 100644 index 0000000000..5afa51ae66 --- /dev/null +++ b/context/app/static/js/helpers/fetcher.ts @@ -0,0 +1,11 @@ +// basic SWR fetcher + +export async function fetcher(url: string) { + return fetch(url).then((response) => response.json()); +} + +// SWR fetcher with token + +export async function fetcherWithToken(token: string, url: string) { + return fetch(url, { headers: { Authorization: `Bearer ${token}` } }).then((response) => response.json()); +} diff --git a/context/app/static/js/hooks/useProtocolData.js b/context/app/static/js/hooks/useProtocolData.js index b3b0bb3749..acffc91d04 100644 --- a/context/app/static/js/hooks/useProtocolData.js +++ b/context/app/static/js/hooks/useProtocolData.js @@ -1,7 +1,7 @@ import { useMemo } from 'react'; import useSWR from 'swr'; -import { multiFetcherWithToken } from 'js/helpers/multiFetcher'; +import { fetcherWithToken } from 'js/helpers/fetcher'; import { useAppContext } from 'js/components/Contexts'; export function useFormattedProtocolUrls(protocolUrls, lastVersion) { @@ -27,15 +27,12 @@ export function useFormattedProtocolUrls(protocolUrls, lastVersion) { }, [protocolUrls, lastVersion]); } -function useProtocolData(protocolUrls, lastVersion = 1) { +function useProtocolData(protocolUrl) { const { protocolsClientToken } = useAppContext(); - const urls = useFormattedProtocolUrls(protocolUrls, lastVersion); - - const protocols = useSWR([protocolsClientToken, urls], ([token, apiUrls]) => multiFetcherWithToken(token, apiUrls), { + const result = useSWR([protocolsClientToken, protocolUrl], ([token, url]) => fetcherWithToken(token, url), { revalidateOnFocus: false, }); - - return protocols.data; + return result; } export default useProtocolData; From 7e8d852870276d832231efdf7c94e3d3ccb8d9a1 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Tue, 18 Jul 2023 11:26:12 -0400 Subject: [PATCH 08/18] remove console log, add explanatory comment, add changelog --- CHANGELOG-fix-protocol-links.md | 2 ++ .../app/static/js/components/detailPage/Protocol/Protocol.jsx | 1 - context/app/static/js/hooks/useProtocolData.js | 2 ++ 3 files changed, 4 insertions(+), 1 deletion(-) create mode 100644 CHANGELOG-fix-protocol-links.md diff --git a/CHANGELOG-fix-protocol-links.md b/CHANGELOG-fix-protocol-links.md new file mode 100644 index 0000000000..c853b81a20 --- /dev/null +++ b/CHANGELOG-fix-protocol-links.md @@ -0,0 +1,2 @@ +- Restore protocol links on donor pages. +- Handle multiple protocol links per dataset. \ No newline at end of file diff --git a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx index dea64807fd..d0282888d4 100644 --- a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx +++ b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx @@ -19,7 +19,6 @@ function ProtocolLink({ url }) { if (loading || !data) { Please wait...; } - console.log(data?.payload); return ( {data?.payload?.url} diff --git a/context/app/static/js/hooks/useProtocolData.js b/context/app/static/js/hooks/useProtocolData.js index acffc91d04..fc4f2b68af 100644 --- a/context/app/static/js/hooks/useProtocolData.js +++ b/context/app/static/js/hooks/useProtocolData.js @@ -8,6 +8,8 @@ export function useFormattedProtocolUrls(protocolUrls, lastVersion) { return useMemo(() => { // Handle case with multiple URLs provided in one string // 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(','); // 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 From d218794b2a1cdd326bacf4b39874d16b4d663edc Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Tue, 18 Jul 2023 11:31:13 -0400 Subject: [PATCH 09/18] undefined safety for new app conf items --- context/app/static/js/components/Providers.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/context/app/static/js/components/Providers.jsx b/context/app/static/js/components/Providers.jsx index 8069df0c02..b26fe473bb 100644 --- a/context/app/static/js/components/Providers.jsx +++ b/context/app/static/js/components/Providers.jsx @@ -39,7 +39,7 @@ function Providers({ ); const protocolsContext = useMemo( - () => ({ protocolsClientId: flaskData.protocolsClientId, clientAuthToken: flaskData.protocolsClientToken }), + () => ({ protocolsClientId: flaskData?.protocolsClientId, clientAuthToken: flaskData?.protocolsClientToken }), [flaskData], ); From f4c9bdeb1279c593800843d2afa040cad7f65c32 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Tue, 18 Jul 2023 11:43:58 -0400 Subject: [PATCH 10/18] Fix handling of multi-url case by removing any resulting whitespace from the split string --- context/app/static/js/hooks/useProtocolData.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/context/app/static/js/hooks/useProtocolData.js b/context/app/static/js/hooks/useProtocolData.js index fc4f2b68af..687ff4e545 100644 --- a/context/app/static/js/hooks/useProtocolData.js +++ b/context/app/static/js/hooks/useProtocolData.js @@ -6,11 +6,11 @@ import { useAppContext } from 'js/components/Contexts'; export function useFormattedProtocolUrls(protocolUrls, lastVersion) { return useMemo(() => { - // Handle case with multiple URLs provided in one string + // 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(','); + 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, '')); From 5b849acf36c4c1f7750779223df3a6251bc993bd Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Tue, 18 Jul 2023 11:45:14 -0400 Subject: [PATCH 11/18] add space to multi-protocol spec to match the real data --- context/app/static/js/hooks/useProtocolData.spec.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/context/app/static/js/hooks/useProtocolData.spec.js b/context/app/static/js/hooks/useProtocolData.spec.js index ec7285c984..2c3e86288d 100644 --- a/context/app/static/js/hooks/useProtocolData.spec.js +++ b/context/app/static/js/hooks/useProtocolData.spec.js @@ -12,12 +12,12 @@ describe('useFormattedProtocolUrls', () => { 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 = 2; + '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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); expect(result).toEqual([ - 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=2', - 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.7d5h6en?last_version=2', + 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=1', + 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.7d5h6en?last_version=1', ]); }); From 8bc9cffe07d35f69fc54fa302e9c29b65057af49 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Tue, 18 Jul 2023 11:51:08 -0400 Subject: [PATCH 12/18] expect v4 api --- .../app/static/js/hooks/useProtocolData.js | 2 +- .../static/js/hooks/useProtocolData.spec.js | 20 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/context/app/static/js/hooks/useProtocolData.js b/context/app/static/js/hooks/useProtocolData.js index 687ff4e545..da1bb5ba16 100644 --- a/context/app/static/js/hooks/useProtocolData.js +++ b/context/app/static/js/hooks/useProtocolData.js @@ -21,7 +21,7 @@ export function useFormattedProtocolUrls(protocolUrls, lastVersion) { // 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/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=1 + // 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}`, ); diff --git a/context/app/static/js/hooks/useProtocolData.spec.js b/context/app/static/js/hooks/useProtocolData.spec.js index 2c3e86288d..a466185e6e 100644 --- a/context/app/static/js/hooks/useProtocolData.spec.js +++ b/context/app/static/js/hooks/useProtocolData.spec.js @@ -7,7 +7,7 @@ describe('useFormattedProtocolUrls', () => { const protocolUrls = 'https://dx.doi.org/10.17504/protocols.io.btnfnmbn'; const lastVersion = 1; const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); - expect(result).toEqual(['https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']); + 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', () => { @@ -16,8 +16,8 @@ describe('useFormattedProtocolUrls', () => { const lastVersion = 1; const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); expect(result).toEqual([ - 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=1', - 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.7d5h6en?last_version=1', + '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', ]); }); @@ -25,21 +25,21 @@ describe('useFormattedProtocolUrls', () => { const protocolUrls = 'http://dx.doi.org/10.17504/protocols.io.btnfnmbn'; const lastVersion = 1; const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); - expect(result).toEqual(['https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']); + 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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); - expect(result).toEqual(['https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']); + 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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); - expect(result).toEqual(['https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']); + 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', () => { @@ -48,8 +48,8 @@ describe('useFormattedProtocolUrls', () => { const lastVersion = 2; const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); expect(result).toEqual([ - 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=2', - 'https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.7d5h6en?last_version=2', + 'https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=2', + 'https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.7d5h6en?last_version=2', ]); }); @@ -57,14 +57,14 @@ describe('useFormattedProtocolUrls', () => { const protocolUrls = 'dx.doi.org/10.17504/protocols.io.btnfnmbn/v1'; const lastVersion = 1; const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); - expect(result).toEqual(['https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']); + 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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); - expect(result).toEqual(['https://www.protocols.io/api/v3/protocols/10.17504/protocols.io.btnfnmbn?last_version=2']); + expect(result).toEqual(['https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=2']); }); it('should handle empty input', () => { From 0b62fec4261466fa9b142c24d9874a7d2a330f80 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Tue, 18 Jul 2023 12:32:19 -0400 Subject: [PATCH 13/18] fix tests, fix handling of empty string input --- .../app/static/js/hooks/useProtocolData.js | 3 ++ .../static/js/hooks/useProtocolData.spec.js | 31 ++++++++++++------- 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/context/app/static/js/hooks/useProtocolData.js b/context/app/static/js/hooks/useProtocolData.js index da1bb5ba16..073bf47bf6 100644 --- a/context/app/static/js/hooks/useProtocolData.js +++ b/context/app/static/js/hooks/useProtocolData.js @@ -6,6 +6,9 @@ import { useAppContext } from 'js/components/Contexts'; export function useFormattedProtocolUrls(protocolUrls, lastVersion) { return useMemo(() => { + if (protocolUrls.length === 0) { + return []; + } // 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" -> diff --git a/context/app/static/js/hooks/useProtocolData.spec.js b/context/app/static/js/hooks/useProtocolData.spec.js index a466185e6e..d0a8d8720e 100644 --- a/context/app/static/js/hooks/useProtocolData.spec.js +++ b/context/app/static/js/hooks/useProtocolData.spec.js @@ -2,11 +2,18 @@ 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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + const result = getResult(protocolUrls, lastVersion); expect(result).toEqual(['https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.btnfnmbn?last_version=1']); }); @@ -14,7 +21,7 @@ describe('useFormattedProtocolUrls', () => { 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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + 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', @@ -24,53 +31,53 @@ describe('useFormattedProtocolUrls', () => { it('should handle URLs with http:// prefix', () => { const protocolUrls = 'http://dx.doi.org/10.17504/protocols.io.btnfnmbn'; const lastVersion = 1; - const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + 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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + 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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + 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 = 2; - const { result } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + 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=2', - 'https://www.protocols.io/api/v4/protocols/10.17504/protocols.io.7d5h6en?last_version=2', + '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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + 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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + 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 } = renderHook(useFormattedProtocolUrls(protocolUrls, lastVersion)); + const result = getResult(protocolUrls, lastVersion); expect(result).toEqual([]); }); }); From 756d038d8a3d2d893729ae7b011feac8f7717e83 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Fri, 21 Jul 2023 11:51:42 -0400 Subject: [PATCH 14/18] Remove duplicate changelog --- CHANGELOG-fix-protocol-links.md | 2 -- 1 file changed, 2 deletions(-) delete mode 100644 CHANGELOG-fix-protocol-links.md diff --git a/CHANGELOG-fix-protocol-links.md b/CHANGELOG-fix-protocol-links.md deleted file mode 100644 index c853b81a20..0000000000 --- a/CHANGELOG-fix-protocol-links.md +++ /dev/null @@ -1,2 +0,0 @@ -- Restore protocol links on donor pages. -- Handle multiple protocol links per dataset. \ No newline at end of file From dd31b34cc5ab5b99d42aa4ff356ee12041f56977 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Fri, 21 Jul 2023 11:52:51 -0400 Subject: [PATCH 15/18] revise ProtocolApiContext import --- context/app/static/js/components/Providers.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/context/app/static/js/components/Providers.jsx b/context/app/static/js/components/Providers.jsx index b26fe473bb..d8faf97eae 100644 --- a/context/app/static/js/components/Providers.jsx +++ b/context/app/static/js/components/Providers.jsx @@ -5,9 +5,9 @@ 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'; -import { ProtocolAPIContext } from './detailPage/Protocol/ProtocolAPIContext'; const generateClassName = createGenerateClassName({ disableGlobal: true, From b16c1643ffd1c214bae1fe11036be97a8d6f9e2b Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Fri, 21 Jul 2023 11:58:48 -0400 Subject: [PATCH 16/18] change protocolapicontext import and add global swr config --- .../app/static/js/components/Providers.jsx | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/context/app/static/js/components/Providers.jsx b/context/app/static/js/components/Providers.jsx index d8faf97eae..0e56f2ee6d 100644 --- a/context/app/static/js/components/Providers.jsx +++ b/context/app/static/js/components/Providers.jsx @@ -1,4 +1,5 @@ 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'; @@ -45,22 +46,28 @@ function Providers({ return ( // injectFirst ensures styled-components takes priority over mui for styling - - - - - - - - - - {children} - - - - - - + + + + + + + + + + + {children} + + + + + + + ); } From d13330b9116740ec196a3371546b160778d65fef Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Fri, 21 Jul 2023 12:23:35 -0400 Subject: [PATCH 17/18] Refactor and reorganize fetcher/multiFetcher to reduce code duplication, add documentation for usage of fetcher functions --- .../publications/PublicationVignette/hooks.ts | 11 +++---- context/app/static/js/helpers/fetcher.ts | 11 ------- context/app/static/js/helpers/multiFetcher.ts | 21 ------------ context/app/static/js/helpers/swr.ts | 32 +++++++++++++++++++ .../app/static/js/hooks/useProtocolData.js | 8 ++--- 5 files changed, 40 insertions(+), 43 deletions(-) delete mode 100644 context/app/static/js/helpers/fetcher.ts delete mode 100644 context/app/static/js/helpers/multiFetcher.ts create mode 100644 context/app/static/js/helpers/swr.ts diff --git a/context/app/static/js/components/publications/PublicationVignette/hooks.ts b/context/app/static/js/components/publications/PublicationVignette/hooks.ts index 67ac367266..4244d572c0 100644 --- a/context/app/static/js/components/publications/PublicationVignette/hooks.ts +++ b/context/app/static/js/components/publications/PublicationVignette/hooks.ts @@ -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'; @@ -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) => { @@ -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; diff --git a/context/app/static/js/helpers/fetcher.ts b/context/app/static/js/helpers/fetcher.ts deleted file mode 100644 index 5afa51ae66..0000000000 --- a/context/app/static/js/helpers/fetcher.ts +++ /dev/null @@ -1,11 +0,0 @@ -// basic SWR fetcher - -export async function fetcher(url: string) { - return fetch(url).then((response) => response.json()); -} - -// SWR fetcher with token - -export async function fetcherWithToken(token: string, url: string) { - return fetch(url, { headers: { Authorization: `Bearer ${token}` } }).then((response) => response.json()); -} diff --git a/context/app/static/js/helpers/multiFetcher.ts b/context/app/static/js/helpers/multiFetcher.ts deleted file mode 100644 index 43d058bf9a..0000000000 --- a/context/app/static/js/helpers/multiFetcher.ts +++ /dev/null @@ -1,21 +0,0 @@ -// Fetcher function that lets SWR fetch multiple urls at once -export const multiFetcher = (...urls: string[]) => { - const f = (url: string) => fetch(url).then((response) => response.json()); - if (urls.length === 0) { - return Promise.resolve([]); - } - return Promise.all(urls.map((url) => f(url))); -}; - -export const multiFetcherWithToken = (token: string, ...urls: string[]) => { - const f = (url: string) => - fetch(url, { - headers: { - Authorization: `Bearer ${token}`, - }, - }).then((response) => response.json()); - if (urls.length === 0) { - return Promise.resolve([]); - } - return Promise.all(urls.map((url) => f(url))); -}; diff --git a/context/app/static/js/helpers/swr.ts b/context/app/static/js/helpers/swr.ts new file mode 100644 index 0000000000..da171bba08 --- /dev/null +++ b/context/app/static/js/helpers/swr.ts @@ -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(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; +} + +/** + * 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(url: string, requestInit: RequestInit = {}) { + return multiFetcher([url], requestInit).then((data) => data[0]) as Promise; +} diff --git a/context/app/static/js/hooks/useProtocolData.js b/context/app/static/js/hooks/useProtocolData.js index 073bf47bf6..958a96db98 100644 --- a/context/app/static/js/hooks/useProtocolData.js +++ b/context/app/static/js/hooks/useProtocolData.js @@ -1,7 +1,7 @@ import { useMemo } from 'react'; import useSWR from 'swr'; -import { fetcherWithToken } from 'js/helpers/fetcher'; +import { fetcher } from 'js/helpers/swr'; import { useAppContext } from 'js/components/Contexts'; export function useFormattedProtocolUrls(protocolUrls, lastVersion) { @@ -34,9 +34,9 @@ export function useFormattedProtocolUrls(protocolUrls, lastVersion) { function useProtocolData(protocolUrl) { const { protocolsClientToken } = useAppContext(); - const result = useSWR([protocolsClientToken, protocolUrl], ([token, url]) => fetcherWithToken(token, url), { - revalidateOnFocus: false, - }); + const result = useSWR([protocolUrl, protocolsClientToken], ([url, token]) => + fetcher(url, { headers: { Authorization: `Bearer ${token}` } }), + ); return result; } From 4f8f269a1fc5016c381b6700ae73b5a5bde9df09 Mon Sep 17 00:00:00 2001 From: Nikolay Akhmetov Date: Fri, 21 Jul 2023 12:50:38 -0400 Subject: [PATCH 18/18] Updated loading message --- .../detailPage/Protocol/Protocol.jsx | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx index d0282888d4..46f335d2e6 100644 --- a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx +++ b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx @@ -3,21 +3,30 @@ import PropTypes from 'prop-types'; import Divider from '@material-ui/core/Divider'; import OutboundIconLink from 'js/shared-styles/Links/iconLinks/OutboundIconLink'; +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({ url }) { +function ProtocolLink({ url, index }) { const { loading, data, error } = useProtocolData(url); - if (error) { - - This protocol may be private or otherwise inaccessible. - ; - } - if (loading || !data) { - Please wait...; + 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 ( + +
+ Protocols are loading. If protocols take a significant time to load, please contact{' '} + help@hubmapconsortium.org about this issue + and mention the HuBMAP ID. +
+
+ ); } return ( @@ -34,8 +43,8 @@ function Protocol({ protocol_url }) { Protocols - {protocolUrls.map((url) => ( - + {protocolUrls.map((url, index) => ( + ))}