from code review

jay-hodgson · Jun 22, 2023 · fdaf192 · fdaf192
1 parent 2c7ee43
commit fdaf192
Show file tree

Hide file tree

Showing 7 changed files with 91 additions and 62 deletions.
diff --git a/packages/synapse-react-client/src/components/TermsAndConditions/TermsAndConditions.tsx b/packages/synapse-react-client/src/components/TermsAndConditions/TermsAndConditions.tsx
@@ -8,28 +8,32 @@ import {
 } from '../../utils/SynapseConstants'
 import { useGetFullTableQueryResults } from '../../synapse-queries'
 
-export const termsAndConditionsTableID = 'syn51718002'
-
 export type TermsAndConditionsProps = {
+  termsAndConditionsTableID: string
+  termsAndConditionsTableVersion: string
   onFormChange: (formComplete: boolean) => void
   hideLinkToFullTC?: boolean
 }
 
 const TermsAndConditions: React.FunctionComponent<TermsAndConditionsProps> = ({
+  termsAndConditionsTableID,
+  termsAndConditionsTableVersion,
   onFormChange,
   hideLinkToFullTC = false,
 }) => {
   const [tcList, setTcList] = useState<tcItem[]>([])
   // Fetch the table data
-  const tableQuery = useGetFullTableQueryResults({
-    entityId: termsAndConditionsTableID,
-    query: {
-      sql: `SELECT * FROM ${termsAndConditionsTableID} ORDER BY order asc`,
+  const { data, isLoading } = useGetFullTableQueryResults(
+    {
+      entityId: termsAndConditionsTableID,
+      query: {
+        sql: `SELECT * FROM ${termsAndConditionsTableID}.${termsAndConditionsTableVersion} ORDER BY order asc`,
+      },
+      partMask: BUNDLE_MASK_QUERY_RESULTS,
+      concreteType: 'org.sagebionetworks.repo.model.table.QueryBundleRequest',
     },
-    partMask: BUNDLE_MASK_QUERY_RESULTS,
-    concreteType: 'org.sagebionetworks.repo.model.table.QueryBundleRequest',
-  })
-  const { data, isLoading } = tableQuery
+    { staleTime: Infinity },
+  )
 
   // update tcList when data changes (transform)
   useEffect(() => {
@@ -131,6 +135,7 @@ const TermsAndConditions: React.FunctionComponent<TermsAndConditionsProps> = ({
                   id={i}
                   checked={checkboxChecked[i]}
                   enabled={checkboxEnabled[i]}
+                  termsAndConditionsTableID={termsAndConditionsTableID}
                   onChange={updateCheckboxState}
                 />
               )

diff --git a/packages/synapse-react-client/src/components/TermsAndConditions/TermsAndConditionsItem.tsx b/packages/synapse-react-client/src/components/TermsAndConditions/TermsAndConditionsItem.tsx
@@ -1,14 +1,11 @@
 import React, { useEffect, useState } from 'react'
 import { CheckIcon } from '../../assets/icons/terms/CheckIcon'
-import { termsAndConditionsTableID } from './TermsAndConditions'
-import {
-  FileHandleAssociateType,
-  FileResult,
-} from '@sage-bionetworks/synapse-types'
-import SynapseClient from '../../synapse-client'
+import { FileHandleAssociateType } from '@sage-bionetworks/synapse-types'
 import { Link, Skeleton } from '@mui/material'
 import { SkeletonParagraph } from '../Skeleton'
 import { times } from 'lodash-es'
+import { useGetPresignedUrlContentFromFHA } from '../../synapse-queries/file/useFiles'
+import { MarkdownSynapse } from '../Markdown'
 
 export type tcItem = {
   iconFileHandleId: string
@@ -21,48 +18,22 @@ export type TermsAndConditionsItemProps = {
   enabled: boolean
   checked: boolean
   item: tcItem
+  termsAndConditionsTableID: string
   onChange: (id: number) => void
 }
 
 const TermsAndConditionsItem: React.FunctionComponent<
   TermsAndConditionsItemProps
 > = props => {
-  const { id, item, enabled, checked, onChange } = props
+  const { id, item, enabled, checked, onChange, termsAndConditionsTableID } =
+    props
   const { iconFileHandleId, label, description } = item
-  const [iconFileResult, setIconFileResult] = useState<FileResult | undefined>(
-    undefined,
-  )
-  const [iconFileContent, setIconFileContent] = useState<string>('')
-
-  useEffect(() => {
-    if (iconFileHandleId) {
-      SynapseClient.getFiles({
-        requestedFiles: [
-          {
-            associateObjectId: termsAndConditionsTableID,
-            associateObjectType: FileHandleAssociateType.TableEntity,
-            fileHandleId: iconFileHandleId,
-          },
-        ],
-        includeFileHandles: true,
-        includePreSignedURLs: true,
-        includePreviewPreSignedURLs: false,
-      }).then(fileResults => {
-        setIconFileResult(fileResults.requestedFiles[0])
-      })
-    }
-  }, [iconFileHandleId])
+  const { data: iconFileContent } = useGetPresignedUrlContentFromFHA({
+    associateObjectId: termsAndConditionsTableID,
+    associateObjectType: FileHandleAssociateType.TableEntity,
+    fileHandleId: iconFileHandleId,
+  })
 
-  useEffect(() => {
-    if (iconFileResult) {
-      SynapseClient.getFileHandleContent(
-        iconFileResult.fileHandle!,
-        iconFileResult.preSignedURL!,
-      ).then(content => {
-        setIconFileContent(content)
-      })
-    }
-  }, [iconFileResult])
   const [showDesc, setShowDes] = useState<boolean>(false)
   const [isChecked, setIsChecked] = useState<boolean>(false)
   let mounted = true
@@ -93,18 +64,16 @@ const TermsAndConditionsItem: React.FunctionComponent<
     <li className={enabled ? 'terms-enabled' : ''}>
       <span
         className="terms-icon"
-        dangerouslySetInnerHTML={{ __html: iconFileContent }}
+        dangerouslySetInnerHTML={{ __html: iconFileContent ?? '' }}
       />
       <span className="terms-desc">
-        <label
-          id={`toc-item-${id}`}
-          dangerouslySetInnerHTML={{ __html: label }}
-        />
+        <label id={`toc-item-${id}`}>
+          <MarkdownSynapse markdown={label} />
+        </label>
         {showDesc && description && (
-          <div
-            className="terms-desc-content"
-            dangerouslySetInnerHTML={{ __html: description }}
-          />
+          <div className="terms-desc-content">
+            <MarkdownSynapse markdown={description} />
+          </div>
         )}
         {description && (
           <div>

diff --git a/packages/synapse-react-client/src/style/components/_terms-and-conditions.scss b/packages/synapse-react-client/src/style/components/_terms-and-conditions.scss
@@ -73,6 +73,7 @@ $signatureLine-width: 420px;
         margin-right: 14px;
         display: inline-block;
         line-height: $terms-checkbox-dimension;
+        cursor: pointer;
       }
       .terms-checked {
         background-color: #28a745;

diff --git a/packages/synapse-react-client/src/synapse-queries/KeyFactory.ts b/packages/synapse-react-client/src/synapse-queries/KeyFactory.ts
@@ -11,6 +11,7 @@ import {
   FavoriteSortBy,
   FavoriteSortDirection,
   FileHandle,
+  FileHandleAssociation,
   GetProjectsParameters,
   PrincipalAliasRequest,
   QueryBundleRequest,
@@ -432,6 +433,12 @@ export class KeyFactory {
     return this.getKey('presignedUrlContent', fileHandle, request, maxSizeBytes)
   }
 
+  public getPresignedUrlFromFHAContentQueryKey(
+    fileHandleAssociation: FileHandleAssociation,
+  ) {
+    return this.getKey('presignedUrlContentFromFHA', fileHandleAssociation)
+  }
+
   public getProfileImageQueryKey(userId: string) {
     return this.getKey('profileImageData', userId)
   }

diff --git a/packages/synapse-react-client/src/synapse-queries/file/useFiles.ts b/packages/synapse-react-client/src/synapse-queries/file/useFiles.ts
@@ -6,6 +6,7 @@ import {
   BatchFileRequest,
   BatchFileResult,
   FileHandle,
+  FileHandleAssociation,
 } from '@sage-bionetworks/synapse-types'
 
 export function useGetPresignedUrlContent(
@@ -42,6 +43,38 @@ export function useGetPresignedUrlContent(
   )
 }
 
+export function useGetPresignedUrlContentFromFHA(
+  fileHandleAssociation: FileHandleAssociation,
+  options?: Omit<UseQueryOptions<string, SynapseClientError>, 'staleTime'>,
+) {
+  const { accessToken, keyFactory } = useSynapseContext()
+  const queryFn = async () => {
+    const batchFileResult = await SynapseClient.getFiles(
+      {
+        requestedFiles: [fileHandleAssociation],
+        includeFileHandles: true,
+        includePreSignedURLs: true,
+        includePreviewPreSignedURLs: false,
+      },
+      accessToken,
+    )
+    const data = await SynapseClient.getFileHandleContent(
+      batchFileResult.requestedFiles[0].fileHandle!,
+      batchFileResult.requestedFiles[0].preSignedURL!,
+    )
+    return data
+  }
+  return useQuery<string, SynapseClientError>(
+    keyFactory.getPresignedUrlFromFHAContentQueryKey(fileHandleAssociation),
+
+    queryFn,
+    {
+      staleTime: Infinity,
+      ...options,
+    },
+  )
+}
+
 /**
  * Get a blob containing the image data for the avatar of a Synapse user. Returns null if the user does not have a profile image.
  */

diff --git a/packages/synapse-react-client/stories/TermsAndConditions.stories.ts b/packages/synapse-react-client/stories/TermsAndConditions.stories.ts
@@ -1,16 +1,30 @@
-import { Meta, StoryObj } from '@storybook/react'
+// import { Meta, StoryObj } from '@storybook/react'
 import TermsAndConditions from '../src/components/TermsAndConditions/TermsAndConditions'
 import { displayToast } from '../src/components/ToastMessage/ToastMessage'
+// import { getHandlersForTableQuery } from '../mocks/msw/handlers/tableQueryHandlers'
+// import { MOCK_REPO_ORIGIN } from '../src/utils/functions/getEndpoint'
+// import mockSyn51718002 from '../mocks/query/syn51718002.json'
+// import { QueryResultBundle } from '@sage-bionetworks/synapse-types'
 
 const meta = {
   title: 'Synapse/TermsAndConditions',
+  // parameters: {
+  //   stack: 'mock',
+  //   msw: {
+  //     handlers: [
+  //       ...getHandlersForTableQuery(mockSyn51718002 as QueryResultBundle, MOCK_REPO_ORIGIN)
+  //     ]
+  //   }
+  // },
   component: TermsAndConditions,
 } satisfies Meta
 export default meta
 type Story = StoryObj<typeof meta>
 
 export const Demo: Story = {
   args: {
+    termsAndConditionsTableID: 'syn51718002',
+    termsAndConditionsTableVersion: '4',
     onFormChange: formComplete => {
       if (formComplete) {
         displayToast('All items accepted!')

diff --git a/packages/synapse-react-client/test/containers/TermsAndConditions.test.tsx b/packages/synapse-react-client/test/containers/TermsAndConditions.test.tsx
@@ -2,7 +2,7 @@ import React from 'react'
 import TermsAndConditions from '../../src/components/TermsAndConditions/TermsAndConditions'
 import { SynapseClient, SynapseContextType } from '../../src'
 import { QueryResultBundle } from '@sage-bionetworks/synapse-types'
-import termsAndConditionsResponse from '../../mocks/query/syn51718002.json'
+import mockSyn51718002 from '../../mocks/query/syn51718002.json'
 import { createWrapper } from '../testutils/TestingLibraryUtils'
 import { render, screen, waitFor } from '@testing-library/react'
 
@@ -25,7 +25,7 @@ describe('Terms And Conditions: basic functionality', () => {
     jest.clearAllMocks()
     jest
       .spyOn(SynapseClient, 'getFullQueryTableResults')
-      .mockResolvedValue(termsAndConditionsResponse as QueryResultBundle)
+      .mockResolvedValue(mockSyn51718002 as QueryResultBundle)
   })
 
   const checkboxCount = 8