Nickakhmetov/HMP-152 protocols fix (#3165)

* Upgrade nuka-carousel * remove weird submodule thing? * 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 * add changelog, fix regex * add protocols API values context, pull values from app conf * make authenticated requests; now seeing data, still not seeing protocol links * Restore protocol links! 🎉 Refactored to avoid requiring multifetcher for this case, but still keeping introduced multifetcher for other cases * remove console log, add explanatory comment, add changelog * undefined safety for new app conf items * Fix handling of multi-url case by removing any resulting whitespace from the split string * add space to multi-protocol spec to match the real data * expect v4 api * fix tests, fix handling of empty string input * Remove duplicate changelog * revise ProtocolApiContext import * change protocolapicontext import and add global swr config * Refactor and reorganize fetcher/multiFetcher to reduce code duplication, add documentation for usage of fetcher functions * Updated loading message
hubmapconsortium · Jul 21, 2023 · 3cc7b44 · 3cc7b44
1 parent 0cff414
commit 3cc7b44
Show file tree

Hide file tree

Showing 12 changed files with 235 additions and 57 deletions.
diff --git a/CHANGELOG-hmp-152.md 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/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
@@ -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';
 
@@ -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>
   );
 }
 

diff --git a/context/app/static/js/components/detailPage/Protocol/Protocol.jsx b/context/app/static/js/components/detailPage/Protocol/Protocol.jsx
@@ -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>
   );

diff --git a/context/app/static/js/components/detailPage/Protocol/ProtocolAPIContext.tsx 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<ProtocolAPIContextType | null>(null);
+
+export const useProtocolAPIContext = () => useContext(ProtocolAPIContext);
diff --git 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) => {
@@ -38,8 +35,8 @@ export function usePublicationVignetteConfs({ uuid, vignetteDirName, vignette }:
       return {};
     };
     // 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/multiFetcher.ts b/context/app/static/js/helpers/multiFetcher.ts
diff --git a/context/app/static/js/helpers/swr.ts 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<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>;
+}
diff --git a/context/app/static/js/hooks/useProtocolData.js b/context/app/static/js/hooks/useProtocolData.js
@@ -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;
diff --git a/context/app/static/js/hooks/useProtocolData.spec.js b/context/app/static/js/hooks/useProtocolData.spec.js
@@ -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([]);
+  });
+});
diff --git a/context/app/utils.py b/context/app/utils.py
@@ -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')
     }

diff --git 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