Merge pull request #273 from IQSS/edit-metadata-allow-empty-values

DatasetValidator allow empty fields for conditionally required fields.

Author: ofahimIQSS

src/datasets/domain/models/Dataset.ts

@@ -5,6 +5,7 @@ export interface Dataset {
   persistentId: string
   versionId: number
   versionInfo: DatasetVersionInfo
+  internalVersionNumber: number
   license?: DatasetLicense
   termsOfUse: TermsOfUse
   alternativePersistentId?: string
@@ -165,7 +166,7 @@ interface TopicClassification extends DatasetMetadataSubField {
   topicClassVocabURI?: string
 }
 
-interface Publication extends DatasetMetadataSubField {
+export interface Publication extends DatasetMetadataSubField {
   publicationCitation?: string
   publicationIDType?: string
   publicationIDNumber?: string

src/datasets/domain/repositories/IDatasetsRepository.ts

@@ -46,7 +46,8 @@ export interface IDatasetsRepository {
   updateDataset(
     datasetId: number | string,
     dataset: DatasetDTO,
-    datasetMetadataBlocks: MetadataBlock[]
+    datasetMetadataBlocks: MetadataBlock[],
+    internalVersionNumber?: number
   ): Promise<void>
   deaccessionDataset(
     datasetId: number | string,

src/datasets/domain/useCases/UpdateDataset.ts

@@ -18,14 +18,24 @@ export class UpdateDataset extends DatasetWriteUseCase<void> {
    *
    * @param {number | string} [datasetId] - The dataset identifier, which can be a string (for persistent identifiers), or a number (for numeric identifiers).
    * @param {DatasetDTO} [updatedDataset] - DatasetDTO object including the updated dataset metadata field values for each metadata block.
+   * @param {number} [internalVersionNumber] - The internal version number of the dataset. If another user updates the dataset version metadata before you send the update request, data inconsistencies may occur. To prevent this, you can use the optional internalVersionNumber parameter. This parameter must include the internal version number corresponding to the dataset version being updated. Note that internal version numbers increase sequentially with each version update.
    * @returns {Promise<void>} - This method does not return anything upon successful completion.
    * @throws {ResourceValidationError} - If there are validation errors related to the provided information.
    * @throws {ReadError} - If there are errors while reading data.
    * @throws {WriteError} - If there are errors while writing data.
    */
-  async execute(datasetId: number | string, updatedDataset: DatasetDTO): Promise<void> {
+  async execute(
+    datasetId: number | string,
+    updatedDataset: DatasetDTO,
+    internalVersionNumber?: number
+  ): Promise<void> {
     const metadataBlocks = await this.getNewDatasetMetadataBlocks(updatedDataset)
     this.getNewDatasetValidator().validate(updatedDataset, metadataBlocks)
-    return this.getDatasetsRepository().updateDataset(datasetId, updatedDataset, metadataBlocks)
+    return this.getDatasetsRepository().updateDataset(
+      datasetId,
+      updatedDataset,
+      metadataBlocks,
+      internalVersionNumber
+    )
   }
 }

src/datasets/domain/useCases/validators/BaseMetadataFieldValidator.ts

@@ -9,6 +9,7 @@ export interface DatasetMetadataFieldAndValueInfo {
   metadataBlockName: string
   metadataParentFieldKey?: string
   metadataFieldPosition?: number
+  allowEmptyForConditionallyRequiredField?: boolean
 }
 
 export abstract class BaseMetadataFieldValidator {

src/datasets/domain/useCases/validators/MetadataFieldValidator.ts

@@ -23,7 +23,10 @@ export class MetadataFieldValidator extends BaseMetadataFieldValidator {
       this.isEmptyString(metadataFieldValue) ||
       this.isEmptyArray(metadataFieldValue)
     ) {
-      if (metadataFieldInfo.isRequired) {
+      if (
+        metadataFieldInfo.isRequired &&
+        !datasetMetadataFieldAndValueInfo.allowEmptyForConditionallyRequiredField
+      ) {
         throw new EmptyFieldError(
           datasetMetadataFieldAndValueInfo.metadataFieldKey,
           datasetMetadataFieldAndValueInfo.metadataBlockName,

src/datasets/domain/useCases/validators/SingleMetadataFieldValidator.ts

@@ -136,6 +136,12 @@ export class SingleMetadataFieldValidator extends BaseMetadataFieldValidator {
         metadataFieldInfo.childMetadataFields as Record<string, MetadataFieldInfo>
       )[childMetadataFieldKey]
 
+      const allowEmptyForConditionallyRequiredField: boolean =
+        this.allowEmptyValueForConditionallyRequiredField(
+          datasetMetadataFieldAndValueInfo,
+          childMetadataFieldKey
+        )
+
       metadataFieldValidator.validate({
         metadataFieldInfo: childMetadataFieldInfo,
         metadataFieldKey: childMetadataFieldKey,
@@ -144,8 +150,58 @@ export class SingleMetadataFieldValidator extends BaseMetadataFieldValidator {
         )[childMetadataFieldKey],
         metadataBlockName: datasetMetadataFieldAndValueInfo.metadataBlockName,
         metadataParentFieldKey: datasetMetadataFieldAndValueInfo.metadataFieldKey,
-        metadataFieldPosition: datasetMetadataFieldAndValueInfo.metadataFieldPosition
+        metadataFieldPosition: datasetMetadataFieldAndValueInfo.metadataFieldPosition,
+        allowEmptyForConditionallyRequiredField
       })
     }
   }
+
+  /**
+   * This method allows setting empty values for conditionally required child fields.
+   * A child field is conditionally required if it is required and its parent field is not required.
+   * The child field should be required only if any of its sibling fields has a value, otherwise it should be optional.
+   */
+
+  private allowEmptyValueForConditionallyRequiredField(
+    datasetMetadataFieldAndValueInfo: DatasetMetadataFieldAndValueInfo,
+    childMetadataFieldKey: string
+  ): boolean {
+    let result = false
+    const metadataFieldInfo = datasetMetadataFieldAndValueInfo.metadataFieldInfo
+
+    const childMetadataFieldKeys = Object.keys(
+      metadataFieldInfo.childMetadataFields as Record<string, MetadataFieldInfo>
+    )
+
+    const conditionallyRequiredChildFields: false | string[] =
+      !datasetMetadataFieldAndValueInfo.metadataFieldInfo.isRequired &&
+      childMetadataFieldKeys.filter(
+        (childMetadataFieldKey) =>
+          (metadataFieldInfo.childMetadataFields as Record<string, MetadataFieldInfo>)[
+            childMetadataFieldKey
+          ].isRequired
+      )
+    const hasConditionallyRequiredChildFields = Boolean(conditionallyRequiredChildFields)
+
+    if (
+      hasConditionallyRequiredChildFields &&
+      Object.values(conditionallyRequiredChildFields as string[]).includes(childMetadataFieldKey)
+    ) {
+      // At this point we know we are standing on a child field that is required and the parent field is not required
+
+      // Get the sibling fields and check if any of them has a value
+      const { [childMetadataFieldKey as keyof Record<string, string>]: _, ...siblingFields } =
+        datasetMetadataFieldAndValueInfo.metadataFieldValue as Record<string, string>
+
+      const siblingsValues = Object.values(siblingFields) as string[]
+
+      const isAnySiblingValuePresent = siblingsValues.some(
+        ([, value]) => value !== undefined && value !== ''
+      )
+
+      result = !isAnySiblingValuePresent
+    }
+
+    return result
+  }
 }

src/datasets/infra/repositories/DatasetsRepository.ts

@@ -206,12 +206,18 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi
   public async updateDataset(
     datasetId: string | number,
     dataset: DatasetDTO,
-    datasetMetadataBlocks: MetadataBlock[]
+    datasetMetadataBlocks: MetadataBlock[],
+    internalVersionNumber?: number
   ): Promise<void> {
     return this.doPut(
       this.buildApiEndpoint(this.datasetsResourceName, `editMetadata`, datasetId),
       transformDatasetModelToUpdateDatasetRequestPayload(dataset, datasetMetadataBlocks),
-      { replace: true }
+      {
+        replace: true,
+        ...(typeof internalVersionNumber === 'number' && {
+          sourceInternalVersionNumber: internalVersionNumber
+        })
+      }
     )
       .then(() => undefined)
       .catch((error) => {

src/datasets/infra/repositories/transformers/DatasetPayload.ts

@@ -5,6 +5,7 @@ export interface DatasetPayload {
   datasetId: number
   datasetPersistentId: string
   id: number
+  internalVersionNumber: number
   versionNumber: number
   versionMinorNumber: number
   versionState: string

src/datasets/infra/repositories/transformers/datasetTransformers.ts

@@ -226,6 +226,7 @@ export const transformVersionPayloadToDataset = (
     id: versionPayload.datasetId,
     versionId: versionPayload.id,
     persistentId: versionPayload.datasetPersistentId,
+    internalVersionNumber: versionPayload.internalVersionNumber,
     versionInfo: {
       majorNumber: versionPayload.versionNumber,
       minorNumber: versionPayload.versionMinorNumber,

test/integration/datasets/DatasetsRepository.test.ts

@@ -28,7 +28,8 @@ import { MetadataBlocksRepository } from '../../../src/metadataBlocks/infra/repo
 import {
   Author,
   DatasetContact,
-  DatasetDescription
+  DatasetDescription,
+  Publication
 } from '../../../src/datasets/domain/models/Dataset'
 import {
   createCollectionViaApi,
@@ -221,6 +222,7 @@ describe('DatasetsRepository', () => {
           false
         )
         expect(actual.id).toBe(testDatasetIds.numericId)
+        expect(actual.internalVersionNumber).toBe(1)
       })
 
       test('should return dataset when it is deaccessioned and includeDeaccessioned param is set', async () => {
@@ -803,7 +805,16 @@ describe('DatasetsRepository', () => {
                   dsDescriptionValue: 'This is the description of the dataset.'
                 }
               ],
-              subject: ['Medicine, Health and Life Sciences']
+              subject: ['Medicine, Health and Life Sciences'],
+              publication: [
+                {
+                  publicationRelationType: 'Cites',
+                  publicationCitation: 'Some related publication citation',
+                  publicationIDType: 'cstr',
+                  publicationIDNumber: 'some identifier'
+                }
+              ],
+              notesText: 'This is a note for the dataset.'
             }
           }
         ]
@@ -813,6 +824,7 @@ describe('DatasetsRepository', () => {
       const citationMetadataBlock = await metadataBlocksRepository.getMetadataBlockByName(
         'citation'
       )
+
       const createdDataset = await sut.createDataset(
         testDataset,
         [citationMetadataBlock],
@@ -831,9 +843,35 @@ describe('DatasetsRepository', () => {
           .dsDescriptionValue
       ).toBe('This is the description of the dataset.')
 
+      expect(actualCreatedDataset.metadataBlocks[0].fields.notesText as string).toBe(
+        'This is a note for the dataset.'
+      )
+      expect(
+        actualCreatedDataset.metadataBlocks[0].fields.publication as Publication[]
+      ).toStrictEqual([
+        {
+          publicationRelationType: 'Cites',
+          publicationCitation: 'Some related publication citation',
+          publicationIDType: 'cstr',
+          publicationIDNumber: 'some identifier'
+        }
+      ])
+
       const updatedDsDescription = 'This is the updated description of the dataset.'
+      const updatedNotesText = ''
+      const updatedPublication = [
+        {
+          publicationRelationType: '',
+          publicationCitation: 'Some updated related publication citation',
+          publicationIDType: '',
+          publicationIDNumber: ''
+        }
+      ]
+
       testDataset.metadataBlockValues[0].fields.dsDescription[0].dsDescriptionValue =
         updatedDsDescription
+      testDataset.metadataBlockValues[0].fields.notesText = updatedNotesText
+      testDataset.metadataBlockValues[0].fields.publication = updatedPublication
 
       await sut.updateDataset(createdDataset.numericId, testDataset, [citationMetadataBlock])
 
@@ -844,6 +882,7 @@ describe('DatasetsRepository', () => {
         false
       )
 
+      expect(actualUpdatedDataset.internalVersionNumber).toBe(2)
       expect(actualUpdatedDataset.metadataBlocks[0].fields.title).toBe(
         'Dataset created using the createDataset use case'
       )
@@ -874,6 +913,104 @@ describe('DatasetsRepository', () => {
         (actualUpdatedDataset.metadataBlocks[0].fields.dsDescription[0] as DatasetDescription)
           .dsDescriptionValue
       ).toBe(updatedDsDescription)
+      expect(actualUpdatedDataset.metadataBlocks[0].fields.notesText as string).toBe(undefined)
+      expect(actualUpdatedDataset.metadataBlocks[0].fields.publication).toStrictEqual([
+        {
+          publicationCitation: 'Some updated related publication citation'
+        }
+      ])
+    })
+
+    test('should throw error if trying to update an outdated internal version dataset', async () => {
+      const testDataset = {
+        metadataBlockValues: [
+          {
+            name: 'citation',
+            fields: {
+              title: 'Dataset created using the createDataset use case',
+              author: [
+                {
+                  authorName: 'Admin, Dataverse',
+                  authorAffiliation: 'Dataverse.org'
+                },
+                {
+                  authorName: 'Owner, Dataverse',
+                  authorAffiliation: 'Dataversedemo.org'
+                }
+              ],
+              datasetContact: [
+                {
+                  datasetContactEmail: 'finch@mailinator.com',
+                  datasetContactName: 'Finch, Fiona'
+                }
+              ],
+              dsDescription: [
+                {
+                  dsDescriptionValue: 'This is the description of the dataset.'
+                }
+              ],
+              subject: ['Medicine, Health and Life Sciences']
+            }
+          }
+        ]
+      }
+
+      const metadataBlocksRepository = new MetadataBlocksRepository()
+      const citationMetadataBlock = await metadataBlocksRepository.getMetadataBlockByName(
+        'citation'
+      )
+
+      const createdDataset = await sut.createDataset(
+        testDataset,
+        [citationMetadataBlock],
+        ROOT_COLLECTION_ALIAS
+      )
+
+      const actualCreatedDataset = await sut.getDataset(
+        createdDataset.numericId,
+        DatasetNotNumberedVersion.LATEST,
+        false,
+        false
+      )
+      const actualCreatedDatasetInternalVersionNumber = actualCreatedDataset.internalVersionNumber
+
+      expect(actualCreatedDataset.internalVersionNumber).toBe(1)
+
+      // Now update the dataset and then update again with the same internal version number
+      const updatedDsDescription = 'This is the updated description of the dataset.'
+      testDataset.metadataBlockValues[0].fields.dsDescription[0].dsDescriptionValue =
+        updatedDsDescription
+
+      // First update sending the correct internal version number
+      await sut.updateDataset(
+        createdDataset.numericId,
+        testDataset,
+        [citationMetadataBlock],
+        actualCreatedDatasetInternalVersionNumber
+      )
+
+      const afterFirstUpdateDataset = await sut.getDataset(
+        createdDataset.numericId,
+        DatasetNotNumberedVersion.LATEST,
+        false,
+        false
+      )
+
+      expect(afterFirstUpdateDataset.internalVersionNumber).toBe(2)
+
+      //Now try to update again with the previous internal version number
+      const expectedError = new WriteError(
+        `[400] Dataset internal version number ${actualCreatedDatasetInternalVersionNumber} is outdated`
+      )
+
+      await expect(
+        sut.updateDataset(
+          createdDataset.numericId,
+          testDataset,
+          [citationMetadataBlock],
+          actualCreatedDatasetInternalVersionNumber
+        )
+      ).rejects.toThrow(expectedError)
     })
 
     test('should return error when dataset does not exist', async () => {