diff --git a/src/datasets/domain/models/Dataset.ts b/src/datasets/domain/models/Dataset.ts index fc6163a2..df08277c 100644 --- a/src/datasets/domain/models/Dataset.ts +++ b/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 diff --git a/src/datasets/domain/repositories/IDatasetsRepository.ts b/src/datasets/domain/repositories/IDatasetsRepository.ts index a5052dc7..da0aeffc 100644 --- a/src/datasets/domain/repositories/IDatasetsRepository.ts +++ b/src/datasets/domain/repositories/IDatasetsRepository.ts @@ -45,7 +45,8 @@ export interface IDatasetsRepository { updateDataset( datasetId: number | string, dataset: DatasetDTO, - datasetMetadataBlocks: MetadataBlock[] + datasetMetadataBlocks: MetadataBlock[], + internalVersionNumber?: number ): Promise deaccessionDataset( datasetId: number | string, diff --git a/src/datasets/domain/useCases/UpdateDataset.ts b/src/datasets/domain/useCases/UpdateDataset.ts index 58e15a59..ed90f4d1 100644 --- a/src/datasets/domain/useCases/UpdateDataset.ts +++ b/src/datasets/domain/useCases/UpdateDataset.ts @@ -18,14 +18,24 @@ export class UpdateDataset extends DatasetWriteUseCase { * * @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} - 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 { + async execute( + datasetId: number | string, + updatedDataset: DatasetDTO, + internalVersionNumber?: number + ): Promise { 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 + ) } } diff --git a/src/datasets/domain/useCases/validators/BaseMetadataFieldValidator.ts b/src/datasets/domain/useCases/validators/BaseMetadataFieldValidator.ts index 4cf4987f..146ae659 100644 --- a/src/datasets/domain/useCases/validators/BaseMetadataFieldValidator.ts +++ b/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 { diff --git a/src/datasets/domain/useCases/validators/MetadataFieldValidator.ts b/src/datasets/domain/useCases/validators/MetadataFieldValidator.ts index 32739c01..8b28131a 100644 --- a/src/datasets/domain/useCases/validators/MetadataFieldValidator.ts +++ b/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, diff --git a/src/datasets/domain/useCases/validators/SingleMetadataFieldValidator.ts b/src/datasets/domain/useCases/validators/SingleMetadataFieldValidator.ts index 4dc63888..f39f0ed4 100644 --- a/src/datasets/domain/useCases/validators/SingleMetadataFieldValidator.ts +++ b/src/datasets/domain/useCases/validators/SingleMetadataFieldValidator.ts @@ -136,6 +136,12 @@ export class SingleMetadataFieldValidator extends BaseMetadataFieldValidator { metadataFieldInfo.childMetadataFields as Record )[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 + ) + + const conditionallyRequiredChildFields: false | string[] = + !datasetMetadataFieldAndValueInfo.metadataFieldInfo.isRequired && + childMetadataFieldKeys.filter( + (childMetadataFieldKey) => + (metadataFieldInfo.childMetadataFields as Record)[ + 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]: _, ...siblingFields } = + datasetMetadataFieldAndValueInfo.metadataFieldValue as Record + + const siblingsValues = Object.values(siblingFields) as string[] + + const isAnySiblingValuePresent = siblingsValues.some( + ([, value]) => value !== undefined && value !== '' + ) + + result = !isAnySiblingValuePresent + } + + return result + } } diff --git a/src/datasets/infra/repositories/DatasetsRepository.ts b/src/datasets/infra/repositories/DatasetsRepository.ts index 4cd6467e..5d82af43 100644 --- a/src/datasets/infra/repositories/DatasetsRepository.ts +++ b/src/datasets/infra/repositories/DatasetsRepository.ts @@ -205,12 +205,18 @@ export class DatasetsRepository extends ApiRepository implements IDatasetsReposi public async updateDataset( datasetId: string | number, dataset: DatasetDTO, - datasetMetadataBlocks: MetadataBlock[] + datasetMetadataBlocks: MetadataBlock[], + internalVersionNumber?: number ): Promise { 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) => { diff --git a/src/datasets/infra/repositories/transformers/DatasetPayload.ts b/src/datasets/infra/repositories/transformers/DatasetPayload.ts index db359a4f..abbf74f8 100644 --- a/src/datasets/infra/repositories/transformers/DatasetPayload.ts +++ b/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 diff --git a/src/datasets/infra/repositories/transformers/datasetTransformers.ts b/src/datasets/infra/repositories/transformers/datasetTransformers.ts index b9e19378..a46372e0 100644 --- a/src/datasets/infra/repositories/transformers/datasetTransformers.ts +++ b/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, diff --git a/test/integration/datasets/DatasetsRepository.test.ts b/test/integration/datasets/DatasetsRepository.test.ts index d3e34bde..3b8aea38 100644 --- a/test/integration/datasets/DatasetsRepository.test.ts +++ b/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 () => { diff --git a/test/testHelpers/datasets/datasetHelper.ts b/test/testHelpers/datasets/datasetHelper.ts index dc900e83..b085eac5 100644 --- a/test/testHelpers/datasets/datasetHelper.ts +++ b/test/testHelpers/datasets/datasetHelper.ts @@ -45,6 +45,7 @@ export const createDatasetModel = ( id: 1, persistentId: 'doi:10.5072/FK2/HC6KTB', versionId: 19, + internalVersionNumber: 1, versionInfo: { majorNumber: 1, minorNumber: 0, @@ -128,6 +129,7 @@ export const createDatasetVersionPayload = ( id: 19, datasetId: 1, datasetPersistentId: 'doi:10.5072/FK2/HC6KTB', + internalVersionNumber: 1, versionNumber: 1, versionMinorNumber: 0, versionState: 'RELEASED', @@ -402,7 +404,8 @@ export const createDatasetDTO = ( timePeriodCoveredStartValue?: DatasetMetadataFieldValueDTO, contributorTypeValue?: DatasetMetadataFieldValueDTO, license?: DatasetLicense, - dateOfCreationValue?: DatasetMetadataFieldValueDTO + dateOfCreationValue?: DatasetMetadataFieldValueDTO, + contributorNameValue?: string ): DatasetDTO => { const validTitle = 'test dataset' const validAuthorFieldValue = [ @@ -434,14 +437,15 @@ export const createDatasetDTO = ( ...(dateOfCreationValue && { dateOfCreation: dateOfCreationValue }), - ...(contributorTypeValue && { - contributor: [ - { - contributorName: 'Admin, Dataverse', - contributorType: contributorTypeValue as string - } - ] - }) + ...(contributorTypeValue !== undefined && + contributorNameValue !== undefined && { + contributor: [ + { + contributorName: contributorNameValue ?? 'Admin, Dataverse', + contributorType: (contributorTypeValue as string | undefined) ?? 'Project Member' + } + ] + }) } } ] diff --git a/test/unit/datasets/DatasetResourceValidator.test.ts b/test/unit/datasets/DatasetResourceValidator.test.ts index 72113a69..91d5f10e 100644 --- a/test/unit/datasets/DatasetResourceValidator.test.ts +++ b/test/unit/datasets/DatasetResourceValidator.test.ts @@ -213,7 +213,16 @@ describe('validate', () => { }) test('should raise a controlled vocabulary error when a controlled vocabulary field has an invalid format', () => { - const testDataset = createDatasetDTO(undefined, undefined, undefined, undefined, 'Wrong Value') + const testDataset = createDatasetDTO( + undefined, + undefined, + undefined, + undefined, + 'Wrong Value', + undefined, + undefined, + 'Some Value' + ) expect.assertions(6) runValidateExpectingFieldValidationError( @@ -235,4 +244,53 @@ describe('validate', () => { ) expect(() => sut.validate(testDataset, testMetadataBlocks)).not.toThrow() }) + + test("should not raise an error when there is a compound field not required with a required childfield that has no value but siblings don't have value either", () => { + const testDataset = createDatasetDTO( + undefined, + undefined, + undefined, + undefined, + '', + undefined, + undefined, + '' + ) + expect(() => sut.validate(testDataset, testMetadataBlocks)).not.toThrow() + }) + + test('should not raise an error when there is a compound field not required with a required childfield that has value and some of the siblings has value', () => { + const testDataset = createDatasetDTO( + undefined, + undefined, + undefined, + undefined, + 'Project Member', + undefined, + undefined, + 'Foo' + ) + expect(() => sut.validate(testDataset, testMetadataBlocks)).not.toThrow() + }) + + test('should raise an error when there is a compound field not required with a required childfield that has no value but some of the siblings have', () => { + const testDataset = createDatasetDTO( + undefined, + undefined, + undefined, + undefined, + 'Project Member', + undefined, + undefined, + '' + ) + expect.assertions(6) + runValidateExpectingFieldValidationError( + testDataset, + 'contributorName', + 'There was an error when validating the field contributorName from metadata block citation with parent field contributor in position 0. Reason was: The field should not be empty.', + 'contributor', + 0 + ) + }) }) diff --git a/test/unit/datasets/UpdateDataset.test.ts b/test/unit/datasets/UpdateDataset.test.ts index bc7f3a62..4c2e7adf 100644 --- a/test/unit/datasets/UpdateDataset.test.ts +++ b/test/unit/datasets/UpdateDataset.test.ts @@ -42,7 +42,8 @@ describe('execute', () => { expect(datasetsRepositoryStub.updateDataset).toHaveBeenCalledWith( 1, testDataset, - testMetadataBlocks + testMetadataBlocks, + undefined ) }) @@ -102,7 +103,8 @@ describe('execute', () => { expect(datasetsRepositoryStub.updateDataset).toHaveBeenCalledWith( 1, testDataset, - testMetadataBlocks + testMetadataBlocks, + undefined ) })