Merge pull request Sage-Bionetworks#505 from nickgros/SWC-6580

packages/synapse-react-client/test/containers/entity/annotations/AnnotationEditorUtils.test.ts → packages/synapse-react-client/src/components/SchemaDrivenAnnotationEditor/AnnotationEditorUtils.test.ts

@@ -1,12 +1,28 @@
-import { AjvError } from '@rjsf/core'
+import { RJSFValidationError } from '@rjsf/utils'
 import {
   dropNullishArrayValues,
+  dropNullValues,
   getFriendlyPropertyName,
   getTransformErrors,
-} from '../../../../src/components/SchemaDrivenAnnotationEditor/AnnotationEditorUtils'
+} from './AnnotationEditorUtils'
 import { FILE_ENTITY_CONCRETE_TYPE_VALUE } from '@sage-bionetworks/synapse-types'
 
 describe('AnnotationEditorUtils tests', () => {
+  describe('dropNullValues', () => {
+    it('removes null values only', () => {
+      const formData = {
+        a: 'foo',
+        b: {},
+        c: [],
+        d: '',
+        e: undefined,
+        f: null,
+      }
+      const expected = { a: 'foo', b: {}, c: [], d: '', e: undefined }
+      expect(dropNullValues(formData)).toEqual(expected)
+    })
+  })
+
   describe('dropNullishArrayValues', () => {
     it('will remove an empty array from the formData', () => {
       const formData = {
@@ -33,7 +49,8 @@ describe('AnnotationEditorUtils tests', () => {
 
   describe('getFriendlyPropertyName', () => {
     it('strips the leading period from a schema-defined property', () => {
-      const error = {
+      const error: RJSFValidationError = {
+        stack: '.study[0] is a requiredProperty',
         property: '.study[0]',
       }
 
@@ -42,7 +59,8 @@ describe('AnnotationEditorUtils tests', () => {
       expect(getFriendlyPropertyName(error)).toEqual(expected)
     })
     it('parses additionalProperties from an error', () => {
-      const error = {
+      const error: RJSFValidationError = {
+        stack: '.study[0] is a requiredProperty',
         property: "['myProperty']",
       }
 
@@ -55,7 +73,7 @@ describe('AnnotationEditorUtils tests', () => {
   describe('transformErrors', () => {
     it('combines errors caused by an enumeration defined using anyOf', () => {
       const transformErrors = getTransformErrors()
-      const errors: AjvError[] = [
+      const errors: RJSFValidationError[] = [
         {
           name: 'type',
           property: '.study[0]',
@@ -105,7 +123,7 @@ describe('AnnotationEditorUtils tests', () => {
       const transformErrors = getTransformErrors(
         FILE_ENTITY_CONCRETE_TYPE_VALUE,
       )
-      const errors: AjvError[] = [
+      const errors: RJSFValidationError[] = [
         {
           name: 'not',
           property: "['dataFileHandleId']",

packages/synapse-react-client/src/components/SchemaDrivenAnnotationEditor/AnnotationEditorUtils.ts

@@ -27,6 +27,20 @@ export function dropNullishArrayValues(
   return newFormData
 }
 
+export function dropNullValues(
+  formData: Record<string, unknown> = {},
+): Record<string, unknown> {
+  return Object.keys(formData).reduce(
+    (acc: Record<string, unknown>, key: string) => {
+      if (formData[key] !== null) {
+        acc[key] = formData[key]
+      }
+      return acc
+    },
+    {},
+  )
+}
+
 /**
  * Inspects the property of the AjvError and modifies it to be comparable to simple key strings, like entity property keys.
  * @param error

packages/synapse-react-client/src/components/SchemaDrivenAnnotationEditor/SchemaDrivenAnnotationEditor.tsx

@@ -14,7 +14,7 @@ import {
   useGetSchemaBinding,
   useUpdateViaJson,
 } from '../../synapse-queries'
-import { SynapseClientError } from '../../utils/SynapseClientError'
+import { SynapseClientError } from '../../utils'
 import {
   ENTITY_CONCRETE_TYPE,
   EntityJson,
@@ -23,6 +23,7 @@ import { SynapseSpinner } from '../LoadingScreen/LoadingScreen'
 import { AdditionalPropertiesSchemaField } from './field/AdditionalPropertiesSchemaField'
 import {
   dropNullishArrayValues,
+  dropNullValues,
   getFriendlyPropertyName,
   getTransformErrors,
 } from './AnnotationEditorUtils'
@@ -67,7 +68,7 @@ export type SchemaDrivenAnnotationEditorProps = {
   /** If defined and formRef is not supplied, shows a 'Cancel' button and runs this effect on click */
   onCancel?: () => void
   /** Passes new form data upon each change to the form */
-  onChange?: (annotations: Record<string, any>) => void
+  onChange?: (annotations: Record<string, unknown>) => void
 }
 
 /**
@@ -91,6 +92,22 @@ function getPatternPropertiesBannedKeys(
   )
 }
 
+/**
+ * Cleans the formData to remove values that are invalid for Synapse Annotations
+ * @param formData
+ * @param dropNullishValuesInArrays
+ */
+function cleanFormData(
+  formData: Record<string, unknown> | undefined,
+  dropNullishValuesInArrays: boolean,
+) {
+  let cleanedFormData = dropNullValues(formData)
+  if (dropNullishValuesInArrays) {
+    cleanedFormData = dropNullishArrayValues(cleanedFormData)
+  }
+  return cleanedFormData
+}
+
 /**
  * Renders a form for editing an entity's annotations. The component also supports supplying just a schema ID,
  * but work to support annotation flows without an entity (i.e. before creating entities) is not yet complete.
@@ -126,12 +143,17 @@ export function SchemaDrivenAnnotationEditor(
 
   const [showConfirmation, setShowConfirmation] = React.useState(false)
 
-  const { entityMetadata: entityJson, annotations } = useGetJson(entityId!, {
-    // Metadata is being edited, so don't refetch
-    staleTime: Infinity,
-    enabled: !!entityId,
-    useErrorBoundary: true,
-  })
+  const { entityMetadata: entityJson, annotations } = useGetJson(
+    entityId!,
+    // Derived annotations will be precomputed and displayed as placeholders in the form
+    false,
+    {
+      // Metadata is being edited, so don't refetch
+      staleTime: Infinity,
+      enabled: !!entityId,
+      useErrorBoundary: true,
+    },
+  )
 
   // Annotation fields fetched and modified via the form
   const [formData, setFormData] = React.useState<
@@ -190,7 +212,7 @@ export function SchemaDrivenAnnotationEditor(
 
   function submitChangedEntity() {
     mutation.mutate({
-      ...dropNullishArrayValues(formData ?? {}),
+      ...cleanFormData(formData, true),
       ...entityJson,
     } as EntityJson)
   }
@@ -294,6 +316,17 @@ export function SchemaDrivenAnnotationEditor(
                 setFormData(formData)
                 setValidationError(undefined)
               }}
+              onBlur={() => {
+                setFormData(
+                  // Clean the formData onBlur to remove null values that we will need to strip before submission
+                  // This will ensure that the user gets accurate validation information since the data will match what the backend will receive
+                  cleanFormData(
+                    formData,
+                    // Don't remove null values in arrays--the fields will disappear, which the user probably does not want
+                    false,
+                  ),
+                )
+              }}
               onSubmit={({ formData, errors }, event) => {
                 event.preventDefault()
                 if (errors && errors.length > 0) {

packages/synapse-react-client/src/components/SchemaDrivenAnnotationEditor/SchemaDrivenAnnotationsEditor.integration.test.tsx

@@ -700,6 +700,33 @@ describe('SchemaDrivenAnnotationEditor tests', () => {
     )
   })
 
+  it('Does not submit null data for schema-defined properties', async () => {
+    server.use(
+      annotationsWithSchemaHandler,
+      ...schemaHandlers,
+      successfulUpdateHandler,
+    )
+    await renderComponent()
+    await screen.findByText('requires scientific annotations', { exact: false })
+
+    const countryField = await screen.findByLabelText('country*')
+
+    // Pick a value to ensure the property exists in the formData
+    await chooseAutocompleteOption(countryField, 'USA')
+
+    // Now clear the field
+    await userEvent.click(await screen.findByLabelText('Clear'))
+
+    // Save the form
+    await clickSaveAndConfirm()
+
+    await waitFor(() => expect(updatedJsonCaptor).toHaveBeenCalled())
+    // Because we cleared the field, country should not exist in the payload
+    expect(Object.hasOwn(updatedJsonCaptor.mock.calls[0][0], 'country')).toBe(
+      false,
+    )
+  })
+
   it('Initializes an empty array but does not submit null data', async () => {
     server.use(
       emptyArrayAnnotationsHandler, // showStringArray will be true but stringArray will have no data

packages/synapse-react-client/src/components/entity/metadata/AnnotationsTable.integration.test.tsx

@@ -75,4 +75,6 @@ describe('AnnotationsTable tests', () => {
     renderComponent()
     await screen.findByText('This Project has no annotations.')
   })
+
+  it.todo('Handles scenario where derived annotations are pending')
 })

packages/synapse-react-client/src/components/entity/metadata/AnnotationsTable.tsx

@@ -1,4 +1,4 @@
-import React from 'react'
+import React, { useCallback, useState } from 'react'
 import { isEmpty } from 'lodash-es'
 import {
   convertToEntityType,
@@ -8,32 +8,69 @@ import {
   BackendDestinationEnum,
   getEndpoint,
 } from '../../../utils/functions/getEndpoint'
-import { useGetSchemaBinding } from '../../../synapse-queries/entity/useEntityBoundSchema'
-import { useSynapseContext } from '../../../utils/context/SynapseContext'
-import { useGetJson } from '../../../synapse-queries/entity/useEntity'
-import { SkeletonTable } from '../../Skeleton/SkeletonTable'
+import {
+  useGetJson,
+  useGetSchemaBinding,
+  useGetValidationResults,
+} from '../../../synapse-queries'
+import { useSynapseContext } from '../../../utils'
+import { SkeletonTable } from '../../Skeleton'
+import dayjs from 'dayjs'
+import FullWidthAlert from '../../FullWidthAlert'
 
 export type AnnotationsTableProps = {
   readonly entityId: string
   readonly versionNumber?: number
 }
 
-export const AnnotationsTable: React.FC<AnnotationsTableProps> = ({
-  entityId,
-}) => {
+export function AnnotationsTable(props: AnnotationsTableProps) {
+  const { entityId } = props
+  const [isManuallyRefetching, setIsManuallyRefetching] = useState(false)
   /**
    * Currently, schema/validation features are only shown in experimental mode.
    */
   const { isInExperimentalMode } = useSynapseContext()
 
   // TODO: Support versioned annotations, see PLFM-7290
-  const { entityMetadata, annotations, isLoading } = useGetJson(entityId)
+  const {
+    entityMetadata,
+    annotations,
+    isLoading,
+    refetch: refetchEntityData,
+  } = useGetJson(entityId, true)
 
   const { data: boundSchema } = useGetSchemaBinding(entityId, {
     enabled: isInExperimentalMode,
   })
 
-  return isLoading ? (
+  const { data: validationResults, refetch: refetchValidationInformation } =
+    useGetValidationResults(entityId, {
+      enabled: isInExperimentalMode && Boolean(boundSchema),
+    })
+
+  const showSchemaInformation = isInExperimentalMode && Boolean(boundSchema)
+
+  // If the entity has not yet been validated since the last fetch, then derived annotations may not have been calculated.
+  const recentChangesHaveNotBeenValidated =
+    !!entityMetadata &&
+    !!validationResults &&
+    dayjs(entityMetadata.modifiedOn).diff(
+      dayjs(validationResults.validatedOn),
+    ) > 0
+
+  const onRefetch = useCallback(async () => {
+    setIsManuallyRefetching(true)
+    const promises = [
+      // Refetch the annotations, which may have changed if new derived annotations have been calculated
+      refetchEntityData(),
+      // Refetch the validation information, which we use to determine if derived annotations may still be pending
+      refetchValidationInformation(),
+    ]
+    await Promise.allSettled(promises)
+    setIsManuallyRefetching(false)
+  }, [refetchEntityData, refetchValidationInformation])
+
+  return isLoading || isManuallyRefetching ? (
     <SkeletonTable numRows={3} numCols={2} />
   ) : (
     <>
@@ -63,7 +100,7 @@ export const AnnotationsTable: React.FC<AnnotationsTableProps> = ({
                 </tr>
               )
             })}
-          {boundSchema && isInExperimentalMode ? (
+          {showSchemaInformation ? (
             <tr className="AnnotationsTable__Row">
               <td className="AnnotationsTable__Row__Key Schema">
                 Validation Schema
@@ -72,19 +109,38 @@ export const AnnotationsTable: React.FC<AnnotationsTableProps> = ({
                 <a
                   href={`${getEndpoint(
                     BackendDestinationEnum.REPO_ENDPOINT,
-                  )}repo/v1/schema/type/registered/${
-                    boundSchema.jsonSchemaVersionInfo.$id
+                  )}/repo/v1/schema/type/registered/${
+                    boundSchema!.jsonSchemaVersionInfo.$id
                   }`}
                   target="_blank"
                   rel="noopener noreferrer"
                 >
-                  {boundSchema.jsonSchemaVersionInfo.schemaName}
+                  {boundSchema!.jsonSchemaVersionInfo.schemaName}
                 </a>
               </td>
             </tr>
           ) : null}
         </tbody>
       </table>
+      {entityMetadata &&
+        showSchemaInformation &&
+        recentChangesHaveNotBeenValidated && (
+          <FullWidthAlert
+            variant="warning"
+            description={`This ${entityTypeToFriendlyName(
+              convertToEntityType(entityMetadata.concreteType),
+            )} has changed since it has been last validated. If this ${entityTypeToFriendlyName(
+              convertToEntityType(entityMetadata.concreteType),
+            )} is expected to have annotations derived from the Validation Schema, they may not yet be visible.`}
+            primaryButtonConfig={{
+              text: 'Reload Annotations',
+              onClick: () => {
+                onRefetch()
+              },
+            }}
+            isGlobal={false}
+          />
+        )}
     </>
   )
 }