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

File Restrict Use Case Extension: enable access request and edit terms of access

Author: ofahimIQSS

docs/useCases.md

@@ -1410,7 +1410,7 @@ The use case returns a number, which is the identifier of the new file.
 
 #### Restrict or Unrestrict a File
 
-Restrict or unrestrict an existing file.
+Restrict or unrestrict an existing file, given a [RestrictFileDTO](../src/users/domain/dtos/RestrictFileDTO.ts)
 
 ##### Example call:
 
@@ -1420,15 +1420,23 @@ import { restrictFile } from '@iqss/dataverse-client-javascript'
 /* ... */
 
 const fileId = 12345
+const restrictFileDTO = {
+  restrict: true,
+  enableAccessRequest: false,
+  termsOfAccess: 'terms of access'
+}
 
-restrictFile.execute(fileId, true)
+restrictFile.execute(fileId, restrictFileDTO)
 
 /* ... */
 ```
 
 _See [use case](../src/files/domain/useCases/RestrictFile.ts) implementation_.
 
 The `fileId` parameter can be a string, for persistent identifiers, or a number, for numeric identifiers.
+If restrict is false then enableAccessRequest and termsOfAccess are ignored
+If restrict is true and enableAccessRequest is false then termsOfAccess is required.
+The enableAccessRequest and termsOfAccess are applied to the Draft version of the Dataset and affect all of the restricted files in said Draft version.
 
 ## Metadata Blocks
 

src/files/domain/dtos/RestrictFileDTO.ts

@@ -0,0 +1,5 @@
+export interface RestrictFileDTO {
+  restrict: boolean
+  enableAccessRequest?: boolean
+  termsOfAccess?: string
+}

src/files/domain/repositories/IFilesRepository.ts

@@ -9,6 +9,7 @@ import { Dataset } from '../../../datasets'
 import { FileUploadDestination } from '../models/FileUploadDestination'
 import { UploadedFileDTO } from '../dtos/UploadedFileDTO'
 import { UpdateFileMetadataDTO } from '../dtos/UpdateFileMetadataDTO'
+import { RestrictFileDTO } from '../dtos/RestrictFileDTO'
 
 export interface IFilesRepository {
   getDatasetFiles(
@@ -65,7 +66,8 @@ export interface IFilesRepository {
 
   replaceFile(fileId: number | string, uploadedFileDTO: UploadedFileDTO): Promise<number>
 
-  restrictFile(fileId: number | string, restrict: boolean): Promise<undefined>
+  restrictFile(fileId: number | string, restrictFileDTO: RestrictFileDTO): Promise<void>
+
   updateFileMetadata(
     fileId: number | string,
     updateFileMetadataDTO: UpdateFileMetadataDTO

src/files/domain/useCases/RestrictFile.ts

@@ -1,5 +1,6 @@
 import { IFilesRepository } from '../repositories/IFilesRepository'
 import { UseCase } from '../../../core/domain/useCases/UseCase'
+import { RestrictFileDTO } from '../dtos/RestrictFileDTO'
 
 export class RestrictFile implements UseCase<void> {
   constructor(private readonly filesRepository: IFilesRepository) {}
@@ -9,10 +10,11 @@ export class RestrictFile implements UseCase<void> {
    * More detailed information about the file restriction behavior can be found in https://guides.dataverse.org/en/latest/api/native-api.html#restrict-files
    *
    * @param {number | string} [fileId] - The File identifier, which can be a string (for persistent identifiers), or a number (for numeric identifiers).
-   * @param {boolean} [restrict] - A boolean value that indicates whether the file should be restricted or unrestricted.
+   * @param {RestrictFileDTO} [restrictFileDTO] - The DTO containing the file restriction information.
    * @returns {Promise<void>} -This method does not return anything upon successful completion.
    */
-  async execute(fileId: number | string, restrict: boolean): Promise<void> {
-    return await this.filesRepository.restrictFile(fileId, restrict)
+
+  async execute(fileId: number | string, restrictFileDTO: RestrictFileDTO): Promise<void> {
+    return await this.filesRepository.restrictFile(fileId, restrictFileDTO)
   }
 }

src/files/infra/repositories/FilesRepository.ts

@@ -21,6 +21,7 @@ import { transformUploadDestinationsResponseToUploadDestination } from './transf
 import { UploadedFileDTO } from '../../domain/dtos/UploadedFileDTO'
 import { UpdateFileMetadataDTO } from '../../domain/dtos/UpdateFileMetadataDTO'
 import { ApiConstants } from '../../../core/infra/repositories/ApiConstants'
+import { RestrictFileDTO } from '../../domain/dtos/RestrictFileDTO'
 
 export interface GetFilesQueryParams {
   includeDeaccessioned: boolean
@@ -346,9 +347,17 @@ export class FilesRepository extends ApiRepository implements IFilesRepository {
       })
   }
 
-  public async restrictFile(fileId: number | string, restrict: boolean): Promise<undefined> {
-    return this.doPut(this.buildApiEndpoint(this.filesResourceName, 'restrict', fileId), restrict)
-      .then(() => undefined)
+  public async restrictFile(
+    fileId: number | string,
+    restrictFileDTO: RestrictFileDTO
+  ): Promise<void> {
+    return this.doPut(
+      this.buildApiEndpoint(this.filesResourceName, 'restrict', fileId),
+      restrictFileDTO
+    )
+      .then((response) => {
+        return response.data.data.message
+      })
       .catch((error) => {
         throw error
       })

test/functional/files/RestrictFile.test.ts

@@ -4,7 +4,8 @@ import {
   CreatedDatasetIdentifiers,
   restrictFile,
   getDatasetFiles,
-  WriteError
+  WriteError,
+  getDataset
 } from '../../../src'
 import { DataverseApiAuthMechanism } from '../../../src/core/infra/repositories/ApiConfig'
 import {
@@ -58,32 +59,62 @@ describe('execute', () => {
     try {
       const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId)
 
-      await restrictFile.execute(datasetFiles.files[0].id, true)
+      await restrictFile.execute(datasetFiles.files[0].id, { restrict: true })
     } catch (error) {
-      throw new Error('File should be deleted')
+      throw new Error('File should be restricted')
     } finally {
       const datasetFilesAfterRestriction = await getDatasetFiles.execute(testDatasetIds.numericId)
-
       expect(datasetFilesAfterRestriction.files[0].restricted).toEqual(true)
-
       // Unrestrict the file for the next test
-      await restrictFile.execute(datasetFilesAfterRestriction.files[0].id, false)
+      await restrictFile.execute(datasetFilesAfterRestriction.files[0].id, { restrict: false })
     }
   })
 
-  test('should succesfully unrestrict a file', async () => {
+  test('should successfully restrict a file with terms of use', async () => {
     try {
       const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId)
 
-      await restrictFile.execute(datasetFiles.files[0].id, true)
+      await restrictFile.execute(datasetFiles.files[0].id, {
+        restrict: true,
+        enableAccessRequest: false,
+        termsOfAccess: 'This file is restricted for testing purposes'
+      })
+    } catch (error) {
+      throw new Error('File should be restricted')
+    } finally {
+      const datasetFilesAfterRestriction = await getDatasetFiles.execute(testDatasetIds.numericId)
+
+      expect(datasetFilesAfterRestriction.files[0].restricted).toEqual(true)
+
+      const dataset = await getDataset.execute(testDatasetIds.numericId)
+
+      expect(dataset.termsOfUse.termsOfAccess.termsOfAccessForRestrictedFiles).toEqual(
+        'This file is restricted for testing purposes'
+      )
+
+      await restrictFile.execute(datasetFilesAfterRestriction.files[0].id, { restrict: false })
+    }
+  })
 
-      await restrictFile.execute(datasetFiles.files[0].id, false)
+  test('should succesfully unrestrict a file', async () => {
+    try {
+      const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId)
+      await restrictFile.execute(datasetFiles.files[0].id, { restrict: true })
     } catch (error) {
-      throw new Error('File should be deleted')
+      throw new Error('File should be restricted')
     } finally {
       const datasetFilesAfterRestriction = await getDatasetFiles.execute(testDatasetIds.numericId)
+      expect(datasetFilesAfterRestriction.files[0].restricted).toEqual(true)
+    }
 
-      expect(datasetFilesAfterRestriction.files[0].restricted).toEqual(false)
+    try {
+      const datasetFilesAfterRestriction = await getDatasetFiles.execute(testDatasetIds.numericId)
+      await restrictFile.execute(datasetFilesAfterRestriction.files[0].id, { restrict: false })
+    } catch (error) {
+      throw new Error('File should be unrestricted')
+    } finally {
+      const datasetFilesAfterUnrestriction = await getDatasetFiles.execute(testDatasetIds.numericId)
+      expect(datasetFilesAfterUnrestriction.files[0].restricted).toEqual(false)
     }
   })
 
@@ -93,7 +124,7 @@ describe('execute', () => {
     const nonExistentFileId = 5
 
     try {
-      await restrictFile.execute(nonExistentFileId, true)
+      await restrictFile.execute(nonExistentFileId, { restrict: true })
       throw new Error('Use case should throw an error')
     } catch (error) {
       writeError = error as WriteError
@@ -105,4 +136,23 @@ describe('execute', () => {
       )
     }
   })
+
+  test('should throw an error when the terms of use is empty while enableAccess is false', async () => {
+    let caughtError: unknown
+    try {
+      const datasetFiles = await getDatasetFiles.execute(testDatasetIds.numericId)
+      await restrictFile.execute(datasetFiles.files[0].id, {
+        restrict: true,
+        enableAccessRequest: false
+      })
+    } catch (error) {
+      caughtError = error
+    }
+
+    expect(caughtError).toBeInstanceOf(WriteError)
+    expect((caughtError as WriteError).message).toEqual(
+      new WriteError().message +
+        ' Reason was: [409] Terms of Use and Access are invalid. You must enable request access or add terms of access in datasets with restricted files.'
+    )
+  })
 })

test/integration/files/FilesRepository.test.ts

@@ -41,9 +41,12 @@ import {
   deleteCollectionViaApi,
   setStorageDriverViaApi
 } from '../../testHelpers/collections/collectionHelper'
+import { RestrictFileDTO } from '../../../src/files/domain/dtos/RestrictFileDTO'
+import { DatasetsRepository } from '../../../src/datasets/infra/repositories/DatasetsRepository'
 
 describe('FilesRepository', () => {
   const sut: FilesRepository = new FilesRepository()
+  const sutDataset: DatasetsRepository = new DatasetsRepository()
 
   let testDatasetIds: CreatedDatasetIdentifiers
 
@@ -766,13 +769,20 @@ describe('FilesRepository', () => {
   describe('restrictFile', () => {
     let restrictFileDatasetIds: CreatedDatasetIdentifiers
     const testTextFile1Name = 'test-file-1.txt'
+    const restrictFileDTO: RestrictFileDTO = {
+      restrict: true,
+      enableAccessRequest: true,
+      termsOfAccess: 'This file is restricted for testing purposes'
+    }
+
+    const unrestrictFileDTO: RestrictFileDTO = { restrict: false }
 
     const setFileToRestricted = async (fileId: number) => {
-      await sut.restrictFile(fileId, true)
+      await sut.restrictFile(fileId, restrictFileDTO)
     }
 
     const setFileToUnrestricted = async (fileId: number) => {
-      await sut.restrictFile(fileId, false)
+      await sut.restrictFile(fileId, unrestrictFileDTO)
     }
 
     beforeEach(async () => {
@@ -786,11 +796,15 @@ describe('FilesRepository', () => {
       })
     })
 
-    afterEach(async () => {
-      await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId)
-    })
+    test('should successfully restrict a file enabling access request', async () => {
+      await publishDatasetViaApi(restrictFileDatasetIds.numericId).catch(() => {
+        throw new Error('Error while publishing test Dataset')
+      })
+
+      await waitForNoLocks(restrictFileDatasetIds.numericId, 10).catch(() => {
+        throw new Error('Error while waiting for no locks')
+      })
 
-    test('should successfully restrict a file', async () => {
       const datasetFiles = await sut.getDatasetFiles(
         restrictFileDatasetIds.numericId,
         DatasetNotNumberedVersion.LATEST,
@@ -809,10 +823,22 @@ describe('FilesRepository', () => {
         FileOrderCriteria.NAME_AZ
       )
 
-      expect(datasetFilesAfterRestrict.files[0].restricted).toEqual(true)
+      expect(datasetFilesAfterRestrict.files[0].restricted).toEqual(restrictFileDTO.restrict)
 
-      // Unrestrict the file Just in case to avoid conflicts with other tests
-      await setFileToUnrestricted(datasetFiles.files[0].id)
+      const dataset = await sutDataset.getDataset(
+        restrictFileDatasetIds.numericId,
+        DatasetNotNumberedVersion.LATEST,
+        false,
+        false
+      )
+      expect(datasetFilesAfterRestrict.files[0].fileAccessRequest).toEqual(
+        restrictFileDTO.enableAccessRequest
+      )
+      expect(dataset.termsOfUse.termsOfAccess.termsOfAccessForRestrictedFiles).toEqual(
+        restrictFileDTO.termsOfAccess
+      )
+
+      await deletePublishedDatasetViaApi(restrictFileDatasetIds.persistentId)
     })
 
     test('should successfully unrestrict a file', async () => {
@@ -846,6 +872,8 @@ describe('FilesRepository', () => {
       )
 
       expect(datasetFilesAfterUnrestrict.files[0].restricted).toEqual(false)
+
+      await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId)
     })
 
     test('should return error when file was already restricted', async () => {
@@ -866,6 +894,8 @@ describe('FilesRepository', () => {
 
       // Unrestrict the file Just in case to avoid conflicts with other tests
       await setFileToUnrestricted(datasetFiles.files[0].id)
+
+      await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId)
     })
 
     test('should return error when files was already unrestricted', async () => {
@@ -881,6 +911,8 @@ describe('FilesRepository', () => {
       )
 
       await expect(setFileToUnrestricted(datasetFiles.files[0].id)).rejects.toThrow(expectedError)
+
+      await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId)
     })
 
     test('should return error when file does not exist', async () => {
@@ -889,6 +921,30 @@ describe('FilesRepository', () => {
       )
 
       await expect(setFileToRestricted(nonExistentFiledId)).rejects.toThrow(expectedError)
+
+      await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId)
+    })
+
+    test('should return error when the terms of use is empty while enableAccess is false', async () => {
+      const datasetFiles = await sut.getDatasetFiles(
+        restrictFileDatasetIds.numericId,
+        DatasetNotNumberedVersion.LATEST,
+        false,
+        FileOrderCriteria.NAME_AZ
+      )
+
+      const errorExpected = new WriteError(
+        `[409] Terms of Use and Access are invalid. You must enable request access or add terms of access in datasets with restricted files.`
+      )
+
+      await expect(
+        sut.restrictFile(datasetFiles.files[0].id, {
+          restrict: true,
+          enableAccessRequest: false
+        })
+      ).rejects.toThrow(errorExpected)
+
+      await deleteUnpublishedDatasetViaApi(restrictFileDatasetIds.numericId)
     })
   })
 })

test/unit/files/RestrictFile.test.ts

@@ -1,15 +1,22 @@
 import { WriteError } from '../../../src'
+import { RestrictFileDTO } from '../../../src/files/domain/dtos/RestrictFileDTO'
 import { IFilesRepository } from '../../../src/files/domain/repositories/IFilesRepository'
 import { RestrictFile } from '../../../src/files/domain/useCases/RestrictFile'
 
 describe('execute', () => {
+  const restrictFileDTO: RestrictFileDTO = {
+    restrict: true,
+    enableAccessRequest: true,
+    termsOfAccess: 'This file is restricted for testing purposes'
+  }
+
   test('should return undefined when repository call is successful', async () => {
     const filesRepositoryStub: IFilesRepository = {} as IFilesRepository
     filesRepositoryStub.restrictFile = jest.fn().mockResolvedValue(undefined)
 
     const sut = new RestrictFile(filesRepositoryStub)
 
-    const actual = await sut.execute(1, true)
+    const actual = await sut.execute(1, restrictFileDTO)
 
     expect(actual).toEqual(undefined)
   })
@@ -20,6 +27,6 @@ describe('execute', () => {
 
     const sut = new RestrictFile(filesRepositoryStub)
 
-    await expect(sut.execute(1, true)).rejects.toThrow(WriteError)
+    await expect(sut.execute(1, restrictFileDTO)).rejects.toThrow(WriteError)
   })
 })