From d6ac29e40f274723d950989721b222e7b4169a0b Mon Sep 17 00:00:00 2001 From: MellyGray Date: Mon, 21 Aug 2023 18:51:43 +0200 Subject: [PATCH 01/13] feat(FileUserPermissions): add FilePermissionsProvider to check if user can download the file --- src/files/domain/models/File.ts | 25 ++-- .../domain/models/FileUserPermissions.ts | 9 ++ .../domain/repositories/FileRepository.ts | 2 + .../useCases/checkFileDownloadPermission.ts | 21 +++ .../FileJSDataverseRepository.ts | 10 ++ .../file-info-data/FileEmbargoDate.tsx | 2 +- .../FilePermissionsContext.ts | 13 ++ .../FilePermissionsProvider.tsx | 46 +++++++ .../files/FileMockLoadingRepository.ts | 10 ++ src/stories/files/FileMockNoDataRepository.ts | 10 ++ .../files/FileMockNoFiltersRepository.ts | 10 ++ src/stories/files/FileMockRepository.ts | 10 ++ .../files/domain/models/FileMother.ts | 37 ++++-- .../models/FileUserPermissionsMother.ts | 13 ++ .../file-info-data/FileEmbargoDate.spec.tsx | 30 ++--- .../useFilePermissions.spec.tsx | 121 ++++++++++++++++++ 16 files changed, 330 insertions(+), 39 deletions(-) create mode 100644 src/files/domain/models/FileUserPermissions.ts create mode 100644 src/files/domain/useCases/checkFileDownloadPermission.ts create mode 100644 src/sections/file/file-permissions/FilePermissionsContext.ts create mode 100644 src/sections/file/file-permissions/FilePermissionsProvider.tsx create mode 100644 tests/component/files/domain/models/FileUserPermissionsMother.ts create mode 100644 tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx diff --git a/src/files/domain/models/File.ts b/src/files/domain/models/File.ts index f49cb5947..d943199b7 100644 --- a/src/files/domain/models/File.ts +++ b/src/files/domain/models/File.ts @@ -76,9 +76,12 @@ export interface FileDate { date: string } -export interface FileEmbargo { - active: boolean - date: string +export class FileEmbargo { + constructor(readonly dateAvailable: Date) {} + + get isActive(): boolean { + return this.dateAvailable > new Date() + } } export interface FileTabularData { @@ -144,24 +147,23 @@ export class File { } get accessStatus(): FileAccessStatus { - if (!this.access.restricted && !this.embargo?.active) { + if (!this.access.restricted && !this.isActivelyEmbargoed) { return FileAccessStatus.PUBLIC } if (!this.permissions.canDownload) { - if (!this.embargo?.active) { + if (!this.isActivelyEmbargoed) { return FileAccessStatus.RESTRICTED } return FileAccessStatus.EMBARGOED_RESTRICTED } - if (!this.embargo?.active) { + if (!this.isActivelyEmbargoed) { return FileAccessStatus.RESTRICTED_WITH_ACCESS } return FileAccessStatus.EMBARGOED } - // TODO - Use this attribute for the FilesThumbnail components get lockStatus(): FileLockStatus { - if (!this.access.restricted && !this.embargo?.active) { + if (!this.access.restricted && !this.isActivelyEmbargoed) { return FileLockStatus.OPEN } if (!this.permissions.canDownload) { @@ -169,4 +171,11 @@ export class File { } return FileLockStatus.UNLOCKED } + + get isActivelyEmbargoed(): boolean { + if (this.embargo) { + return this.embargo.isActive + } + return false + } } diff --git a/src/files/domain/models/FileUserPermissions.ts b/src/files/domain/models/FileUserPermissions.ts new file mode 100644 index 000000000..715a20220 --- /dev/null +++ b/src/files/domain/models/FileUserPermissions.ts @@ -0,0 +1,9 @@ +export interface FileUserPermissions { + fileId: string + canDownloadFile: boolean + canEditDataset: boolean +} + +export enum FilePermission { + DOWNLOAD_FILE = 'downloadFile' +} diff --git a/src/files/domain/repositories/FileRepository.ts b/src/files/domain/repositories/FileRepository.ts index eda64b94c..e178f3ede 100644 --- a/src/files/domain/repositories/FileRepository.ts +++ b/src/files/domain/repositories/FileRepository.ts @@ -2,6 +2,7 @@ import { File } from '../models/File' import { FileCriteria } from '../models/FileCriteria' import { FilesCountInfo } from '../models/FilesCountInfo' import { FilePaginationInfo } from '../models/FilePaginationInfo' +import { FileUserPermissions } from '../models/FileUserPermissions' export interface FileRepository { getAllByDatasetPersistentId: ( @@ -14,4 +15,5 @@ export interface FileRepository { datasetPersistentId: string, version?: string ) => Promise + getFileUserPermissionsById: (id: string) => Promise } diff --git a/src/files/domain/useCases/checkFileDownloadPermission.ts b/src/files/domain/useCases/checkFileDownloadPermission.ts new file mode 100644 index 000000000..9dbd6818f --- /dev/null +++ b/src/files/domain/useCases/checkFileDownloadPermission.ts @@ -0,0 +1,21 @@ +import { FileRepository } from '../repositories/FileRepository' +import { File, FileStatus } from '../models/File' + +export async function checkFileDownloadPermission( + fileRepository: FileRepository, + file: File +): Promise { + if (file.version.status === FileStatus.DEACCESSIONED) { + return fileRepository.getFileUserPermissionsById(file.id).then((permissions) => { + return permissions.canEditDataset + }) + } + + if (!file.access.restricted && !file.isActivelyEmbargoed) { + return true + } + + return fileRepository.getFileUserPermissionsById(file.id).then((permissions) => { + return permissions.canDownloadFile + }) +} diff --git a/src/files/infrastructure/FileJSDataverseRepository.ts b/src/files/infrastructure/FileJSDataverseRepository.ts index fbbb5511d..5656984a6 100644 --- a/src/files/infrastructure/FileJSDataverseRepository.ts +++ b/src/files/infrastructure/FileJSDataverseRepository.ts @@ -4,6 +4,8 @@ import { FilesMockData } from '../../stories/files/FileMockData' import { FilesCountInfo } from '../domain/models/FilesCountInfo' import { FilesCountInfoMother } from '../../../tests/component/files/domain/models/FilesCountInfoMother' import { FilePaginationInfo } from '../domain/models/FilePaginationInfo' +import { FileUserPermissions } from '../domain/models/FileUserPermissions' +import { FileUserPermissionsMother } from '../../../tests/component/files/domain/models/FileUserPermissionsMother' export class FileJSDataverseRepository implements FileRepository { getAllByDatasetPersistentId( @@ -34,4 +36,12 @@ export class FileJSDataverseRepository implements FileRepository { }, 1000) }) } + getFileUserPermissionsById(id: string): Promise { + // TODO - implement using js-dataverse + return new Promise((resolve) => { + setTimeout(() => { + resolve(FileUserPermissionsMother.create()) + }, 1000) + }) + } } diff --git a/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/FileEmbargoDate.tsx b/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/FileEmbargoDate.tsx index d9fa4fb24..dcba0cada 100644 --- a/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/FileEmbargoDate.tsx +++ b/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/FileEmbargoDate.tsx @@ -16,7 +16,7 @@ export function FileEmbargoDate({ embargo, status }: FileEmbargoDateProps) { return (
- {t(embargoTypeOfDate(embargo.active, status))} {embargo.date} + {t(embargoTypeOfDate(embargo.isActive, status))} {embargo.dateAvailable.toDateString()}
) diff --git a/src/sections/file/file-permissions/FilePermissionsContext.ts b/src/sections/file/file-permissions/FilePermissionsContext.ts new file mode 100644 index 000000000..930ba2770 --- /dev/null +++ b/src/sections/file/file-permissions/FilePermissionsContext.ts @@ -0,0 +1,13 @@ +import { createContext, useContext } from 'react' +import { FilePermission } from '../../../files/domain/models/FileUserPermissions' +import { File } from '../../../files/domain/models/File' + +interface FilePermissionsContextProps { + checkSessionUserHasFilePermission: (permission: FilePermission, file: File) => Promise +} + +export const FilePermissionsContext = createContext({ + checkSessionUserHasFilePermission: () => Promise.reject('Not implemented') +}) + +export const useFilePermissions = () => useContext(FilePermissionsContext) diff --git a/src/sections/file/file-permissions/FilePermissionsProvider.tsx b/src/sections/file/file-permissions/FilePermissionsProvider.tsx new file mode 100644 index 000000000..589735f06 --- /dev/null +++ b/src/sections/file/file-permissions/FilePermissionsProvider.tsx @@ -0,0 +1,46 @@ +import { PropsWithChildren } from 'react' +import { FileRepository } from '../../../files/domain/repositories/FileRepository' +import { FilePermission } from '../../../files/domain/models/FileUserPermissions' +import { FilePermissionsContext } from './FilePermissionsContext' +import { File } from '../../../files/domain/models/File' +import { checkFileDownloadPermission } from '../../../files/domain/useCases/checkFileDownloadPermission' + +interface SessionUserFilePermissionsProviderProps { + repository: FileRepository +} + +export function FilePermissionsProvider({ + repository, + children +}: PropsWithChildren) { + const checkSessionUserHasFileDownloadPermission = (file: File): Promise => { + return checkFileDownloadPermission(repository, file) + .then((canDownloadFile) => { + // TODO - Cache the result + return canDownloadFile + }) + .catch((error) => { + console.error('There was an error getting the file download permission', error) + return false + }) + } + + function checkSessionUserHasFilePermission( + permission: FilePermission, + file: File + ): Promise { + switch (permission) { + case FilePermission.DOWNLOAD_FILE: + return checkSessionUserHasFileDownloadPermission(file) + default: + throw new Error(`Unknown file permission`) + } + } + + return ( + + {children} + + ) +} diff --git a/src/stories/files/FileMockLoadingRepository.ts b/src/stories/files/FileMockLoadingRepository.ts index 86fb02192..eeea77cc7 100644 --- a/src/stories/files/FileMockLoadingRepository.ts +++ b/src/stories/files/FileMockLoadingRepository.ts @@ -1,6 +1,8 @@ import { FileRepository } from '../../files/domain/repositories/FileRepository' import { File } from '../../files/domain/models/File' import { FilesCountInfo } from '../../files/domain/models/FilesCountInfo' +import { FileUserPermissions } from '../../files/domain/models/FileUserPermissions' +import { FileUserPermissionsMother } from '../../../tests/component/files/domain/models/FileUserPermissionsMother' export class FileMockLoadingRepository implements FileRepository { // eslint-disable-next-line unused-imports/no-unused-vars @@ -23,4 +25,12 @@ export class FileMockLoadingRepository implements FileRepository { }, 1000) }) } + // eslint-disable-next-line unused-imports/no-unused-vars + getFileUserPermissionsById(id: string): Promise { + return new Promise((resolve) => { + setTimeout(() => { + resolve(FileUserPermissionsMother.create()) + }, 1000) + }) + } } diff --git a/src/stories/files/FileMockNoDataRepository.ts b/src/stories/files/FileMockNoDataRepository.ts index 60cead095..018c02ddf 100644 --- a/src/stories/files/FileMockNoDataRepository.ts +++ b/src/stories/files/FileMockNoDataRepository.ts @@ -2,6 +2,8 @@ import { FileRepository } from '../../files/domain/repositories/FileRepository' import { File } from '../../files/domain/models/File' import { FilesCountInfo } from '../../files/domain/models/FilesCountInfo' import { FilesCountInfoMother } from '../../../tests/component/files/domain/models/FilesCountInfoMother' +import { FileUserPermissions } from '../../files/domain/models/FileUserPermissions' +import { FileUserPermissionsMother } from '../../../tests/component/files/domain/models/FileUserPermissionsMother' export class FileMockNoDataRepository implements FileRepository { // eslint-disable-next-line unused-imports/no-unused-vars @@ -24,4 +26,12 @@ export class FileMockNoDataRepository implements FileRepository { }, 1000) }) } + // eslint-disable-next-line unused-imports/no-unused-vars + getFileUserPermissionsById(id: string): Promise { + return new Promise((resolve) => { + setTimeout(() => { + resolve(FileUserPermissionsMother.create()) + }, 1000) + }) + } } diff --git a/src/stories/files/FileMockNoFiltersRepository.ts b/src/stories/files/FileMockNoFiltersRepository.ts index 345f1e6de..aac1b62de 100644 --- a/src/stories/files/FileMockNoFiltersRepository.ts +++ b/src/stories/files/FileMockNoFiltersRepository.ts @@ -3,6 +3,8 @@ import { File } from '../../files/domain/models/File' import { FilesCountInfo } from '../../files/domain/models/FilesCountInfo' import { FilesCountInfoMother } from '../../../tests/component/files/domain/models/FilesCountInfoMother' import { FilesMockData } from './FileMockData' +import { FileUserPermissions } from '../../files/domain/models/FileUserPermissions' +import { FileUserPermissionsMother } from '../../../tests/component/files/domain/models/FileUserPermissionsMother' export class FileMockNoFiltersRepository implements FileRepository { // eslint-disable-next-line unused-imports/no-unused-vars @@ -25,4 +27,12 @@ export class FileMockNoFiltersRepository implements FileRepository { }, 1000) }) } + // eslint-disable-next-line unused-imports/no-unused-vars + getFileUserPermissionsById(id: string): Promise { + return new Promise((resolve) => { + setTimeout(() => { + resolve(FileUserPermissionsMother.create()) + }, 1000) + }) + } } diff --git a/src/stories/files/FileMockRepository.ts b/src/stories/files/FileMockRepository.ts index 90bb32613..4a30be3c8 100644 --- a/src/stories/files/FileMockRepository.ts +++ b/src/stories/files/FileMockRepository.ts @@ -4,6 +4,8 @@ import { File } from '../../files/domain/models/File' import { FilesCountInfo } from '../../files/domain/models/FilesCountInfo' import { FilesCountInfoMother } from '../../../tests/component/files/domain/models/FilesCountInfoMother' import { FilePaginationInfo } from '../../files/domain/models/FilePaginationInfo' +import { FileUserPermissionsMother } from '../../../tests/component/files/domain/models/FileUserPermissionsMother' +import { FileUserPermissions } from '../../files/domain/models/FileUserPermissions' export class FileMockRepository implements FileRepository { // eslint-disable-next-line unused-imports/no-unused-vars @@ -30,4 +32,12 @@ export class FileMockRepository implements FileRepository { }, 1000) }) } + // eslint-disable-next-line unused-imports/no-unused-vars + getFileUserPermissionsById(id: string): Promise { + return new Promise((resolve) => { + setTimeout(() => { + resolve(FileUserPermissionsMother.create()) + }, 1000) + }) + } } diff --git a/tests/component/files/domain/models/FileMother.ts b/tests/component/files/domain/models/FileMother.ts index a7986ee09..db56db34e 100644 --- a/tests/component/files/domain/models/FileMother.ts +++ b/tests/component/files/domain/models/FileMother.ts @@ -22,6 +22,18 @@ const createFakeFileLabel = (): FileLabel => ({ value: faker.lorem.word() }) +export class FileEmbargoMother { + static create(): FileEmbargo { + const dateAvailable = faker.date.future() + return new FileEmbargo(dateAvailable) + } + + static createNotActive(): FileEmbargo { + const dateAvailable = faker.date.past() + return new FileEmbargo(dateAvailable) + } +} + export class FileMother { static create(props?: Partial): File { const thumbnail = valueOrUndefined(faker.image.imageUrl()) @@ -64,10 +76,7 @@ export class FileMother { checksum: checksum, thumbnail: thumbnail, directory: valueOrUndefined(faker.system.directoryPath()), - embargo: valueOrUndefined({ - active: faker.datatype.boolean(), - date: faker.date.recent().toDateString() - }), + embargo: valueOrUndefined(FileEmbargoMother.create()), tabularData: fileType === 'tabular data' && !checksum ? { @@ -147,10 +156,7 @@ export class FileMother { static createWithEmbargo(): File { return this.createDefault({ - embargo: { - active: true, - date: faker.date.future().toDateString() - } + embargo: FileEmbargoMother.create() }) } @@ -164,10 +170,7 @@ export class FileMother { permissions: { canDownload: false }, - embargo: { - active: true, - date: faker.date.future().toDateString() - } + embargo: FileEmbargoMother.create() }) } @@ -299,4 +302,14 @@ export class FileMother { type: new FileType('image') }) } + + static createDeaccessioned(): File { + return this.createDefault({ + version: { + majorNumber: 1, + minorNumber: 0, + status: FileStatus.DEACCESSIONED + } + }) + } } diff --git a/tests/component/files/domain/models/FileUserPermissionsMother.ts b/tests/component/files/domain/models/FileUserPermissionsMother.ts new file mode 100644 index 000000000..3a34e6b09 --- /dev/null +++ b/tests/component/files/domain/models/FileUserPermissionsMother.ts @@ -0,0 +1,13 @@ +import { FileUserPermissions } from '../../../../../src/files/domain/models/FileUserPermissions' +import { faker } from '@faker-js/faker' + +export class FileUserPermissionsMother { + static create(props?: Partial): FileUserPermissions { + return { + fileId: faker.datatype.uuid(), + canDownloadFile: faker.datatype.boolean(), + canEditDataset: faker.datatype.boolean(), + ...props + } + } +} diff --git a/tests/component/sections/dataset/dataset-files/files-table/files-info/file-info-cell/file-info-data/FileEmbargoDate.spec.tsx b/tests/component/sections/dataset/dataset-files/files-table/files-info/file-info-cell/file-info-data/FileEmbargoDate.spec.tsx index a56b683b5..0a6ee49f5 100644 --- a/tests/component/sections/dataset/dataset-files/files-table/files-info/file-info-cell/file-info-data/FileEmbargoDate.spec.tsx +++ b/tests/component/sections/dataset/dataset-files/files-table/files-info/file-info-cell/file-info-data/FileEmbargoDate.spec.tsx @@ -1,40 +1,34 @@ import { FileEmbargoDate } from '../../../../../../../../../src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/FileEmbargoDate' import { FileStatus } from '../../../../../../../../../src/files/domain/models/File' +import { FileEmbargoMother } from '../../../../../../../files/domain/models/FileMother' describe('FileEmbargoDate', () => { it('renders the embargo date when embargo exists', () => { - const embargo = { - active: true, - date: '2023-06-14' - } + const embargo = FileEmbargoMother.create() const status = FileStatus.RELEASED cy.customMount() - cy.findByText('Embargoed until 2023-06-14').should('exist') + cy.findByText(`Embargoed until ${embargo.dateAvailable.toDateString()}`).should('exist') }) it('renders the until embargo date when embargo is not active and the file is not released', () => { - const embargo = { - active: false, - date: '2023-06-14' - } + const embargo = FileEmbargoMother.createNotActive() const status = FileStatus.RELEASED cy.customMount() - cy.findByText('Was embargoed until 2023-06-14').should('exist') + cy.findByText(`Was embargoed until ${embargo.dateAvailable.toDateString()}`).should('exist') }) it('renders the draft until embargo date when embargo is active and the file is not released', () => { - const embargo = { - active: true, - date: '2023-06-14' - } + const embargo = FileEmbargoMother.create() const status = FileStatus.DRAFT cy.customMount() - cy.findByText('Draft: will be embargoed until 2023-06-14').should('exist') + cy.findByText(`Draft: will be embargoed until ${embargo.dateAvailable.toDateString()}`).should( + 'exist' + ) }) it('renders an empty fragment when embargo is undefined', () => { @@ -43,8 +37,8 @@ describe('FileEmbargoDate', () => { cy.customMount() - cy.findByText('Draft: will be embargoed until 2023-06-14').should('not.exist') - cy.findByText('Embargoed until 2023-06-14').should('not.exist') - cy.findByText('Was embargoed until 2023-06-14').should('not.exist') + cy.findByText(/Draft: will be embargoed until/).should('not.exist') + cy.findByText(/Embargoed until/).should('not.exist') + cy.findByText(/Was embargoed until/).should('not.exist') }) }) diff --git a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx new file mode 100644 index 000000000..c08840b1b --- /dev/null +++ b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx @@ -0,0 +1,121 @@ +import { useFilePermissions } from '../../../../../src/sections/file/file-permissions/FilePermissionsContext' +import { FilePermission } from '../../../../../src/files/domain/models/FileUserPermissions' +import { FileMother } from '../../../files/domain/models/FileMother' +import { useEffect, useState } from 'react' +import { FilePermissionsProvider } from '../../../../../src/sections/file/file-permissions/FilePermissionsProvider' +import { FileRepository } from '../../../../../src/files/domain/repositories/FileRepository' +import { File } from '../../../../../src/files/domain/models/File' +import { FileUserPermissionsMother } from '../../../files/domain/models/FileUserPermissionsMother' +import { FilesCountInfoMother } from '../../../files/domain/models/FilesCountInfoMother' + +const fileRepository: FileRepository = {} as FileRepository +function TestComponent({ file }: { file: File }) { + const { checkSessionUserHasFilePermission } = useFilePermissions() + const [hasFileDownloadPermission, setHasFileDownloadPermission] = useState(false) + useEffect(() => { + checkSessionUserHasFilePermission(FilePermission.DOWNLOAD_FILE, file) + .then((hasPermission) => { + setHasFileDownloadPermission(hasPermission) + }) + .catch((error) => { + console.error('There was an error getting the file download permission', error) + }) + }, [file]) + + return ( +
+ {hasFileDownloadPermission ? ( + Has download permission + ) : ( + Does not have download permission + )} +
+ ) +} +describe('useFilePermissions', () => { + beforeEach(() => { + fileRepository.getAllByDatasetPersistentId = cy.stub().resolves([]) + fileRepository.getCountInfoByDatasetPersistentId = cy + .stub() + .resolves(FilesCountInfoMother.create()) + }) + + it('should not call getFileUserPermissionsById when the file is not deaccessioned nor restricted nor embargoed', () => { + const file = FileMother.createDefault() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id })) + + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('not.be.called') + cy.findByText('Has download permission').should('exist') + }) + + it('should call getFileUserPermissionsById when the file is deaccessioned', () => { + const file = FileMother.createDeaccessioned() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canEditDataset: true })) + + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has download permission').should('exist') + }) + + it('should call getFileUserPermissionsById when the file is restricted', () => { + const file = FileMother.createWithRestrictedAccess() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has download permission').should('exist') + }) + + it('should call getFileUserPermissionsById when the file is embargoed', () => { + const file = FileMother.createWithEmbargo() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has download permission').should('exist') + }) + + it.skip('should use the cached state canDownloadPermissionByFileId the second time the file is being consulted', () => { + // TODO - Implement cache + const file = FileMother.createWithRestrictedAccess() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) + cy.mount( + + + + + ) + + cy.findAllByText('Has download permission').should('exist') + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledOnce') + }) +}) From 3769c732230758738cd719da08463cbe58853b2c Mon Sep 17 00:00:00 2001 From: MellyGray Date: Tue, 22 Aug 2023 10:13:20 +0200 Subject: [PATCH 02/13] feat(FileUserPermissions): add useFileDownloadPermission hook to get the download permissions --- .../FilePermissionsProvider.tsx | 2 - .../useFileDownloadPermission.tsx | 23 +++++++ .../useFileDownloadPermission.spec.tsx | 63 +++++++++++++++++++ .../useFilePermissions.spec.tsx | 15 +++++ 4 files changed, 101 insertions(+), 2 deletions(-) create mode 100644 src/sections/file/file-permissions/useFileDownloadPermission.tsx create mode 100644 tests/component/sections/file/file-permissions/useFileDownloadPermission.spec.tsx diff --git a/src/sections/file/file-permissions/FilePermissionsProvider.tsx b/src/sections/file/file-permissions/FilePermissionsProvider.tsx index 589735f06..d8c3c8c6d 100644 --- a/src/sections/file/file-permissions/FilePermissionsProvider.tsx +++ b/src/sections/file/file-permissions/FilePermissionsProvider.tsx @@ -32,8 +32,6 @@ export function FilePermissionsProvider({ switch (permission) { case FilePermission.DOWNLOAD_FILE: return checkSessionUserHasFileDownloadPermission(file) - default: - throw new Error(`Unknown file permission`) } } diff --git a/src/sections/file/file-permissions/useFileDownloadPermission.tsx b/src/sections/file/file-permissions/useFileDownloadPermission.tsx new file mode 100644 index 000000000..61e02bff7 --- /dev/null +++ b/src/sections/file/file-permissions/useFileDownloadPermission.tsx @@ -0,0 +1,23 @@ +import { useEffect, useState } from 'react' +import { FilePermission } from '../../../files/domain/models/FileUserPermissions' +import { useFilePermissions } from './FilePermissionsContext' +import { File } from '../../../files/domain/models/File' + +export function useFileDownloadPermission(file: File) { + const { checkSessionUserHasFilePermission } = useFilePermissions() + const [sessionUserHasFileDownloadPermission, setSessionUserHasFileDownloadPermission] = + useState(false) + + useEffect(() => { + checkSessionUserHasFilePermission(FilePermission.DOWNLOAD_FILE, file) + .then((hasPermission) => { + setSessionUserHasFileDownloadPermission(hasPermission) + }) + .catch((error) => { + setSessionUserHasFileDownloadPermission(false) + console.error('There was an error getting the file download permission', error) + }) + }, [file]) + + return { sessionUserHasFileDownloadPermission } +} diff --git a/tests/component/sections/file/file-permissions/useFileDownloadPermission.spec.tsx b/tests/component/sections/file/file-permissions/useFileDownloadPermission.spec.tsx new file mode 100644 index 000000000..90e465f6d --- /dev/null +++ b/tests/component/sections/file/file-permissions/useFileDownloadPermission.spec.tsx @@ -0,0 +1,63 @@ +import { FileMother } from '../../../files/domain/models/FileMother' +import { FilePermissionsProvider } from '../../../../../src/sections/file/file-permissions/FilePermissionsProvider' +import { FileRepository } from '../../../../../src/files/domain/repositories/FileRepository' +import { File } from '../../../../../src/files/domain/models/File' +import { FileUserPermissionsMother } from '../../../files/domain/models/FileUserPermissionsMother' +import { FilesCountInfoMother } from '../../../files/domain/models/FilesCountInfoMother' +import { useFileDownloadPermission } from '../../../../../src/sections/file/file-permissions/useFileDownloadPermission' + +const fileRepository: FileRepository = {} as FileRepository +function TestComponent({ file }: { file: File }) { + const { sessionUserHasFileDownloadPermission } = useFileDownloadPermission(file) + + return ( +
+ {sessionUserHasFileDownloadPermission ? ( + Has download permission + ) : ( + Does not have download permission + )} +
+ ) +} + +describe('useFileDownloadPermission', () => { + beforeEach(() => { + fileRepository.getAllByDatasetPersistentId = cy.stub().resolves([]) + fileRepository.getCountInfoByDatasetPersistentId = cy + .stub() + .resolves(FilesCountInfoMother.create()) + }) + + it('should return file download permission', () => { + const file = FileMother.createDeaccessioned() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canEditDataset: true })) + + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has download permission').should('exist') + }) + + it('should return false for file download permission if there is an error', () => { + const file = FileMother.createDeaccessioned() + fileRepository.getFileUserPermissionsById = cy + .stub() + .rejects(new Error('Error getting file user permissions')) + + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Does not have download permission').should('exist') + }) +}) diff --git a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx index c08840b1b..5b44bf70f 100644 --- a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx +++ b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx @@ -102,6 +102,21 @@ describe('useFilePermissions', () => { cy.findByText('Has download permission').should('exist') }) + it('should return false if there is an error in the use case request', () => { + const file = FileMother.createWithEmbargo() + fileRepository.getFileUserPermissionsById = cy + .stub() + .rejects(new Error('There was an error getting the file user permissions')) + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Does not have download permission').should('exist') + }) + it.skip('should use the cached state canDownloadPermissionByFileId the second time the file is being consulted', () => { // TODO - Implement cache const file = FileMother.createWithRestrictedAccess() From 0e0265d43c9c0732588041aa6fcc570aaf8509d1 Mon Sep 17 00:00:00 2001 From: MellyGray Date: Tue, 22 Aug 2023 13:59:55 +0200 Subject: [PATCH 03/13] feat(FileUserPermissions): refactor Files Table to read new permissions --- public/locales/en/files.json | 7 +- src/files/domain/models/File.ts | 45 ------- .../access-file-menu/AccessFileMenu.tsx | 11 +- .../access-file-menu/AccessStatus.tsx | 95 ++++++++++++++ .../access-file-menu/AccessStatusText.tsx | 39 ------ .../access-file-menu/RequestAccessOption.tsx | 47 +++---- .../file-info/file-info-cell/FileInfoCell.tsx | 7 +- .../file-thumbnail/FileThumbnail.module.scss | 2 +- .../file-thumbnail/FileThumbnail.tsx | 19 ++- .../FileThumbnailRestrictedIcon.tsx | 34 +++-- .../file-thumbnail/FileThumbnail.stories.tsx | 36 +----- .../files/domain/models/FileMother.ts | 28 ---- .../access-file-menu/AccessStatus.spec.tsx | 94 ++++++++++++++ .../AccessStatusText.spec.tsx | 68 ---------- .../RequestAccessOption.spec.tsx | 121 ++++++------------ .../file-thumbnail/FileThumbnail.spec.tsx | 71 ++++++---- 16 files changed, 338 insertions(+), 386 deletions(-) create mode 100644 src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatus.tsx delete mode 100644 src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatusText.tsx create mode 100644 tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatus.spec.tsx delete mode 100644 tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatusText.spec.tsx diff --git a/public/locales/en/files.json b/public/locales/en/files.json index 125829fb3..d2def2522 100644 --- a/public/locales/en/files.json +++ b/public/locales/en/files.json @@ -32,16 +32,13 @@ "name": "Restricted", "icon": "Restricted File Icon" }, - "restrictedAccess": { + "restrictedWithAccess": { "name": "Restricted with Access Granted", "icon": "Restricted with access Icon" }, "embargoed": { "name": "Embargoed" }, - "embargoedRestricted": { - "name": "Embargoed" - }, "public": { "name": "Public", "icon": "Public File Icon" @@ -128,7 +125,7 @@ "cancel": "Cancel", "close": "Close", "embargoed": "Files are unavailable during the specified embargo", - "embargoedRestricted": "Files are unavailable during the specified embargo and restricted after that", + "embargoedThenRestricted": "Files are unavailable during the specified embargo and restricted after that", "requestNotAllowed": "Users may not request access to files", "accessRequested": "Access Requested" } diff --git a/src/files/domain/models/File.ts b/src/files/domain/models/File.ts index d943199b7..7d5ef0294 100644 --- a/src/files/domain/models/File.ts +++ b/src/files/domain/models/File.ts @@ -34,14 +34,6 @@ export interface FileAccess { requested: boolean } -export enum FileAccessStatus { - PUBLIC = 'public', - RESTRICTED = 'restricted', - RESTRICTED_WITH_ACCESS = 'restrictedAccess', - EMBARGOED = 'embargoed', - EMBARGOED_RESTRICTED = 'embargoedRestricted' -} - export enum FileStatus { DRAFT = 'draft', RELEASED = 'released', @@ -112,23 +104,12 @@ export class FileType { } } -export enum FileLockStatus { - LOCKED = 'locked', - UNLOCKED = 'unlocked', - OPEN = 'open' -} - -export interface FilePermissions { - canDownload: boolean -} - export class File { constructor( readonly id: string, readonly version: FileVersion, readonly name: string, readonly access: FileAccess, - readonly permissions: FilePermissions, readonly type: FileType, readonly size: FileSize, readonly date: FileDate, @@ -146,32 +127,6 @@ export class File { return `/file?id=${this.id}&version=${this.version.toString()}` } - get accessStatus(): FileAccessStatus { - if (!this.access.restricted && !this.isActivelyEmbargoed) { - return FileAccessStatus.PUBLIC - } - if (!this.permissions.canDownload) { - if (!this.isActivelyEmbargoed) { - return FileAccessStatus.RESTRICTED - } - return FileAccessStatus.EMBARGOED_RESTRICTED - } - if (!this.isActivelyEmbargoed) { - return FileAccessStatus.RESTRICTED_WITH_ACCESS - } - return FileAccessStatus.EMBARGOED - } - - get lockStatus(): FileLockStatus { - if (!this.access.restricted && !this.isActivelyEmbargoed) { - return FileLockStatus.OPEN - } - if (!this.permissions.canDownload) { - return FileLockStatus.LOCKED - } - return FileLockStatus.UNLOCKED - } - get isActivelyEmbargoed(): boolean { if (this.embargo) { return this.embargo.isActive diff --git a/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessFileMenu.tsx b/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessFileMenu.tsx index 60f78210b..72a53597a 100644 --- a/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessFileMenu.tsx +++ b/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessFileMenu.tsx @@ -1,6 +1,6 @@ import { File } from '../../../../../../../../files/domain/models/File' import { Download, FileEarmark } from 'react-bootstrap-icons' -import { AccessStatusText } from './AccessStatusText' +import { AccessStatus } from './AccessStatus' import { RequestAccessOption } from './RequestAccessOption' import { DropdownButton, DropdownHeader, Tooltip } from '@iqss/dataverse-design-system' import { useTranslation } from 'react-i18next' @@ -21,13 +21,8 @@ export function AccessFileMenu({ file }: FileActionButtonAccessFileProps) { {t('actions.accessFileMenu.headers.fileAccess')} - - + + ) diff --git a/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatus.tsx b/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatus.tsx new file mode 100644 index 000000000..2cff5b898 --- /dev/null +++ b/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatus.tsx @@ -0,0 +1,95 @@ +import { File } from '../../../../../../../../files/domain/models/File' +import { Globe, LockFill, UnlockFill } from 'react-bootstrap-icons' +import { useTranslation } from 'react-i18next' +import styles from './AccessFileMenu.module.scss' +import { DropdownButtonItem } from '@iqss/dataverse-design-system' +import { useFileDownloadPermission } from '../../../../../../../file/file-permissions/useFileDownloadPermission' + +interface AccessStatusProps { + file: File +} + +export function AccessStatus({ file }: AccessStatusProps) { + const { sessionUserHasFileDownloadPermission } = useFileDownloadPermission(file) + + return ( + + + {' '} + + + + ) +} + +function AccessStatusIcon({ + sessionUserHasFileDownloadPermission, + restricted +}: { + sessionUserHasFileDownloadPermission: boolean + restricted: boolean +}) { + const { t } = useTranslation('files') + if (restricted) { + if (sessionUserHasFileDownloadPermission) { + return ( + + ) + } + return ( + + ) + } + return +} + +function AccessStatusText({ + file, + sessionUserHasFileDownloadPermission +}: { + file: File + sessionUserHasFileDownloadPermission: boolean +}) { + const { t } = useTranslation('files') + const getAccessStatus = () => { + if (file.isActivelyEmbargoed) { + return 'embargoed' + } + + if (file.access.restricted) { + if (!sessionUserHasFileDownloadPermission) { + return 'restricted' + } + + return 'restrictedWithAccess' + } + + return 'public' + } + + return ( + + {t(`table.fileAccess.${getAccessStatus()}.name`)} + + ) +} diff --git a/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatusText.tsx b/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatusText.tsx deleted file mode 100644 index 8746cb1af..000000000 --- a/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatusText.tsx +++ /dev/null @@ -1,39 +0,0 @@ -import { FileAccessStatus, FileLockStatus } from '../../../../../../../../files/domain/models/File' -import { Globe, LockFill, UnlockFill } from 'react-bootstrap-icons' -import { useTranslation } from 'react-i18next' -import styles from './AccessFileMenu.module.scss' -import { DropdownButtonItem } from '@iqss/dataverse-design-system' - -interface AccessStatusProps { - accessStatus: FileAccessStatus - lockStatus: FileLockStatus -} - -export function AccessStatusText({ accessStatus, lockStatus }: AccessStatusProps) { - const { t } = useTranslation('files') - - return ( - - - {t(`table.fileAccess.${accessStatus}.name`)} - - - ) -} - -function LockStatusIcon({ lockStatus }: { lockStatus: FileLockStatus }) { - const { t } = useTranslation('files') - if (lockStatus === FileLockStatus.UNLOCKED) { - return - } - if (lockStatus === FileLockStatus.LOCKED) { - return - } - return -} - -const semanticMeaningByLockStatus = { - [FileLockStatus.OPEN]: 'success', - [FileLockStatus.UNLOCKED]: 'success', - [FileLockStatus.LOCKED]: 'danger' -} diff --git a/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/RequestAccessOption.tsx b/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/RequestAccessOption.tsx index f417c1f4f..5fae44539 100644 --- a/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/RequestAccessOption.tsx +++ b/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/RequestAccessOption.tsx @@ -1,50 +1,39 @@ import { DropdownButtonItem } from '@iqss/dataverse-design-system' import styles from './AccessFileMenu.module.scss' import { RequestAccessModal } from './RequestAccessModal' -import { - FileAccess, - FileAccessStatus, - FileStatus -} from '../../../../../../../../files/domain/models/File' +import { File, FileStatus } from '../../../../../../../../files/domain/models/File' import { useTranslation } from 'react-i18next' +import { useFileDownloadPermission } from '../../../../../../../file/file-permissions/useFileDownloadPermission' interface RequestAccessButtonProps { - fileId: string - versionStatus: FileStatus - accessStatus: FileAccessStatus - access: FileAccess + file: File } -export function RequestAccessOption({ - fileId, - versionStatus, - accessStatus, - access -}: RequestAccessButtonProps) { +export function RequestAccessOption({ file }: RequestAccessButtonProps) { const { t } = useTranslation('files') - if ( - versionStatus === FileStatus.DEACCESSIONED || - accessStatus === FileAccessStatus.PUBLIC || - accessStatus === FileAccessStatus.RESTRICTED_WITH_ACCESS - ) { + const { sessionUserHasFileDownloadPermission } = useFileDownloadPermission(file) + + if (file.version.status === FileStatus.DEACCESSIONED || sessionUserHasFileDownloadPermission) { return <> } - if (accessStatus === FileAccessStatus.EMBARGOED) { + if (file.isActivelyEmbargoed) { + if (file.access.restricted) { + return ( + + {t('requestAccess.embargoedThenRestricted')}. + + ) + } return {t('requestAccess.embargoed')}. } - if (accessStatus === FileAccessStatus.EMBARGOED_RESTRICTED) { - return ( - {t('requestAccess.embargoedRestricted')}. - ) - } - if (!access.canBeRequested) { + if (!file.access.canBeRequested) { return {t('requestAccess.requestNotAllowed')}. } - if (access.requested) { + if (file.access.requested) { return ( {t('requestAccess.accessRequested')} ) } - return + return } diff --git a/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/FileInfoCell.tsx b/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/FileInfoCell.tsx index cfae0bd2f..bd7d428fc 100644 --- a/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/FileInfoCell.tsx +++ b/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/FileInfoCell.tsx @@ -16,12 +16,7 @@ export function FileInfoCell({ file }: { file: File }) { return (
- +
diff --git a/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.module.scss b/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.module.scss index bd87ec255..aaaafe437 100644 --- a/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.module.scss +++ b/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.module.scss @@ -51,7 +51,7 @@ color: $dv-danger-color; } -.restricted-icon-restrictedAccess { +.restricted-icon-restrictedWithAccess { @extend %restricted-icon; right: -16px; diff --git a/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.tsx b/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.tsx index b84dc043c..403de00e1 100644 --- a/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.tsx +++ b/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.tsx @@ -1,18 +1,17 @@ import { FileThumbnailIcon } from './FileThumbnailIcon' import { FileThumbnailPreviewImage } from './FileThumbnailPreviewImage' -import { FileLockStatus, FileType } from '../../../../../../../../files/domain/models/File' +import { File } from '../../../../../../../../files/domain/models/File' import { FileThumbnailRestrictedIcon } from './FileThumbnailRestrictedIcon' import styles from './FileThumbnail.module.scss' +import { useFileDownloadPermission } from '../../../../../../../file/file-permissions/useFileDownloadPermission' interface FileThumbnailProps { - thumbnail?: string | undefined - name: string - type: FileType - lockStatus: FileLockStatus + file: File } -export function FileThumbnail({ thumbnail, name, type, lockStatus }: FileThumbnailProps) { - const showPreviewImage = thumbnail && lockStatus !== FileLockStatus.LOCKED +export function FileThumbnail({ file }: FileThumbnailProps) { + const { sessionUserHasFileDownloadPermission } = useFileDownloadPermission(file) + const showPreviewImage = file.thumbnail && sessionUserHasFileDownloadPermission return (
{showPreviewImage ? ( - + ) : ( - + )} - +
) } diff --git a/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnailRestrictedIcon.tsx b/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnailRestrictedIcon.tsx index 10efcdda6..167a14545 100644 --- a/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnailRestrictedIcon.tsx +++ b/src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnailRestrictedIcon.tsx @@ -2,24 +2,36 @@ import { LockFill, UnlockFill } from 'react-bootstrap-icons' import styles from './FileThumbnail.module.scss' import { useTranslation } from 'react-i18next' import { Tooltip } from '@iqss/dataverse-design-system' -import { FileLockStatus } from '../../../../../../../../files/domain/models/File' +import { File } from '../../../../../../../../files/domain/models/File' +import { useFileDownloadPermission } from '../../../../../../../file/file-permissions/useFileDownloadPermission' -export function FileThumbnailRestrictedIcon({ lockStatus }: { lockStatus: FileLockStatus }) { - const { t } = useTranslation('files') - const restrictedType = lockStatus === FileLockStatus.LOCKED ? 'restricted' : 'restrictedAccess' - - if (lockStatus === FileLockStatus.OPEN) { +export function FileThumbnailRestrictedIcon({ file }: { file: File }) { + if (!file.access.restricted) { return <> } + + const { t } = useTranslation('files') + const { sessionUserHasFileDownloadPermission } = useFileDownloadPermission(file) return ( - + - {lockStatus === FileLockStatus.LOCKED ? ( - + {sessionUserHasFileDownloadPermission ? ( + ) : ( - + )} diff --git a/src/stories/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.stories.tsx b/src/stories/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.stories.tsx index 5e08b9c35..0f94b0b67 100644 --- a/src/stories/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.stories.tsx +++ b/src/stories/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.stories.tsx @@ -15,55 +15,27 @@ type Story = StoryObj export const WithIcon: Story = { render: () => { const file = FileMother.createDefault() - return ( - - ) + return } } export const WithThumbnailPreview: Story = { render: () => { const file = FileMother.createWithThumbnail() - return ( - - ) + return } } export const WithThumbnailRestrictedLockedIcon: Story = { render: () => { const file = FileMother.createWithRestrictedAccess() - return ( - - ) + return } } export const WithThumbnailRestrictedUnlockedIcon: Story = { render: () => { const file = FileMother.createWithRestrictedAccessWithAccessGranted() - return ( - - ) + return } } diff --git a/tests/component/files/domain/models/FileMother.ts b/tests/component/files/domain/models/FileMother.ts index db56db34e..021eff5f7 100644 --- a/tests/component/files/domain/models/FileMother.ts +++ b/tests/component/files/domain/models/FileMother.ts @@ -47,9 +47,6 @@ export class FileMother { canBeRequested: faker.datatype.boolean(), requested: faker.datatype.boolean() }, - permissions: { - canDownload: faker.datatype.boolean() - }, version: { majorNumber: faker.datatype.number(), minorNumber: faker.datatype.number(), @@ -98,7 +95,6 @@ export class FileMother { ), fileMockedData.name, fileMockedData.access, - fileMockedData.permissions, fileMockedData.type, new FileSize(fileMockedData.size.value, fileMockedData.size.unit), fileMockedData.date, @@ -167,9 +163,6 @@ export class FileMother { canBeRequested: false, requested: false }, - permissions: { - canDownload: false - }, embargo: FileEmbargoMother.create() }) } @@ -204,9 +197,6 @@ export class FileMother { canBeRequested: false, requested: false }, - permissions: { - canDownload: true - }, embargo: undefined }) } @@ -218,9 +208,6 @@ export class FileMother { canBeRequested: false, requested: false }, - permissions: { - canDownload: false - }, embargo: undefined }) } @@ -232,9 +219,6 @@ export class FileMother { canBeRequested: true, requested: false }, - permissions: { - canDownload: true - }, embargo: undefined }) } @@ -246,9 +230,6 @@ export class FileMother { canBeRequested: true, requested: false }, - permissions: { - canDownload: false - }, embargo: undefined }) } @@ -260,9 +241,6 @@ export class FileMother { canBeRequested: true, requested: true }, - permissions: { - canDownload: false - }, embargo: undefined }) } @@ -280,9 +258,6 @@ export class FileMother { canBeRequested: true, requested: false }, - permissions: { - canDownload: true - }, thumbnail: faker.image.imageUrl(), type: new FileType('image') }) @@ -295,9 +270,6 @@ export class FileMother { canBeRequested: false, requested: false }, - permissions: { - canDownload: false - }, thumbnail: faker.image.imageUrl(), type: new FileType('image') }) diff --git a/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatus.spec.tsx b/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatus.spec.tsx new file mode 100644 index 000000000..79e591067 --- /dev/null +++ b/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatus.spec.tsx @@ -0,0 +1,94 @@ +import { FileMother } from '../../../../../../../../files/domain/models/FileMother' +import { AccessStatus } from '../../../../../../../../../../src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatus' +import styles from '../../../../../../../../../../src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessFileMenu.module.scss' +import { FileRepository } from '../../../../../../../../../../src/files/domain/repositories/FileRepository' +import { FileUserPermissionsMother } from '../../../../../../../../files/domain/models/FileUserPermissionsMother' +import { FilePermissionsProvider } from '../../../../../../../../../../src/sections/file/file-permissions/FilePermissionsProvider' + +describe('AccessStatus', () => { + it('renders the access status public', () => { + const filePublic = FileMother.createWithPublicAccess() + cy.customMount() + + cy.findByText('Public').should('exist').should('have.class', styles.success) + cy.findByText('Public File Icon').should('exist') + }) + + it('renders the access status restricted', () => { + const fileRestricted = FileMother.createWithRestrictedAccess() + cy.customMount() + + cy.findByText('Restricted').should('exist').should('have.class', styles.danger) + cy.findByText('Restricted File Icon').should('exist') + }) + + it('renders the access status restricted with access', () => { + const fileRestrictedWithAccess = FileMother.createWithRestrictedAccessWithAccessGranted() + const fileRepository: FileRepository = {} as FileRepository + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: fileRestrictedWithAccess.id, + canDownloadFile: true + }) + ) + + cy.customMount( + + + + ) + + cy.findByText('Restricted with Access Granted') + .should('exist') + .should('have.class', styles.success) + cy.findByText('Restricted with access Icon').should('exist') + }) + + it('renders the access status embargoed', () => { + const fileRestrictedWithAccess = FileMother.createWithRestrictedAccessWithAccessGranted() + const fileRepository: FileRepository = {} as FileRepository + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: fileRestrictedWithAccess.id, + canDownloadFile: true + }) + ) + const fileEmbargoed = FileMother.createWithEmbargo() + cy.customMount( + + {' '} + + ) + + cy.findByText('Embargoed').should('exist').should('have.class', styles.success) + cy.findByText('Public File Icon').should('exist') + }) + + it('renders the access status embargoed restricted', () => { + const fileEmbargoedRestricted = FileMother.createWithEmbargoRestricted() + cy.customMount() + + cy.findByText('Embargoed').should('exist').should('have.class', styles.danger) + cy.findByText('Restricted File Icon').should('exist') + }) + + it('renders the access status embargoed restricted with access', () => { + const fileRestrictedWithAccess = FileMother.createWithRestrictedAccessWithAccessGranted() + const fileRepository: FileRepository = {} as FileRepository + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: fileRestrictedWithAccess.id, + canDownloadFile: true + }) + ) + const fileEmbargoedRestricted = FileMother.createWithEmbargoRestricted() + cy.customMount( + + {' '} + + ) + + cy.findByText('Embargoed').should('exist').should('have.class', styles.success) + cy.findByText('Restricted with access Icon').should('exist') + }) +}) diff --git a/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatusText.spec.tsx b/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatusText.spec.tsx deleted file mode 100644 index 77e06e7cd..000000000 --- a/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatusText.spec.tsx +++ /dev/null @@ -1,68 +0,0 @@ -import { FileMother } from '../../../../../../../../files/domain/models/FileMother' -import { AccessStatusText } from '../../../../../../../../../../src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessStatusText' -import styles from '../../../../../../../../../../src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessFileMenu.module.scss' -describe('AccessStatus', () => { - it('renders the access status public', () => { - const filePublic = FileMother.createWithPublicAccess() - cy.customMount( - - ) - - cy.findByText('Public').should('exist').should('have.class', styles.success) - cy.findByText('Public File Icon').should('exist') - }) - - it('renders the access status restricted', () => { - const fileRestricted = FileMother.createWithRestrictedAccess() - cy.customMount( - - ) - - cy.findByText('Restricted').should('exist').should('have.class', styles.danger) - cy.findByText('Restricted File Icon').should('exist') - }) - - it('renders the access status restricted with access', () => { - const fileRestricted = FileMother.createWithRestrictedAccessWithAccessGranted() - cy.customMount( - - ) - - cy.findByText('Restricted with Access Granted') - .should('exist') - .should('have.class', styles.success) - cy.findByText('Restricted with access Icon').should('exist') - }) - - it('renders the access status embargoed', () => { - const fileEmbargoed = FileMother.createWithEmbargo() - cy.customMount( - - ) - - cy.findByText('Embargoed').should('exist').should('have.class', styles.success) - cy.findByText('Restricted with access Icon').should('exist') - }) - - it('renders the access status embargoed restricted', () => { - const fileEmbargoed = FileMother.createWithEmbargoRestricted() - cy.customMount( - - ) - - cy.findByText('Embargoed').should('exist').should('have.class', styles.danger) - cy.findByText('Restricted File Icon').should('exist') - }) -}) diff --git a/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/RequestAccessOption.spec.tsx b/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/RequestAccessOption.spec.tsx index 86fbb6146..dbeaec172 100644 --- a/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/RequestAccessOption.spec.tsx +++ b/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/RequestAccessOption.spec.tsx @@ -1,30 +1,13 @@ import { RequestAccessOption } from '../../../../../../../../../../src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/RequestAccessOption' -import { - FileAccessStatus, - FileStatus -} from '../../../../../../../../../../src/files/domain/models/File' - -const accessCannotBeRequested = { - restricted: true, - canBeRequested: false, - requested: false -} -const accessCanBeRequested = { - restricted: true, - canBeRequested: true, - requested: false -} -const fileId = 'file-id' +import { FileMother } from '../../../../../../../../files/domain/models/FileMother' +import { FileRepository } from '../../../../../../../../../../src/files/domain/repositories/FileRepository' +import { FileUserPermissionsMother } from '../../../../../../../../files/domain/models/FileUserPermissionsMother' +import { FilePermissionsProvider } from '../../../../../../../../../../src/sections/file/file-permissions/FilePermissionsProvider' + describe('RequestAccessOption', () => { it('renders the embargoed message when the file is embargoed', () => { - cy.customMount( - - ) + const fileEmbargoed = FileMother.createWithEmbargo() + cy.customMount() cy.findByRole('button', { name: 'Files are unavailable during the specified embargo.' }) .should('exist') @@ -32,14 +15,8 @@ describe('RequestAccessOption', () => { }) it('renders the embargo then restricted message when the file is embargoed and restricted', () => { - cy.customMount( - - ) + const fileEmbargoedRestricted = FileMother.createWithEmbargoRestricted() + cy.customMount() cy.findByRole('button', { name: 'Files are unavailable during the specified embargo and restricted after that.' @@ -49,14 +26,8 @@ describe('RequestAccessOption', () => { }) it('renders the Users may not request access to files. message when the file is restricted and access request is not allowed', () => { - cy.customMount( - - ) + const fileRestricted = FileMother.createWithRestrictedAccess() + cy.customMount() cy.findByRole('button', { name: 'Users may not request access to files.' }) .should('exist') @@ -64,32 +35,16 @@ describe('RequestAccessOption', () => { }) it('renders the request access button when the file is restricted and can be requested', () => { - cy.customMount( - - ) + const fileRestrictedCanBeRequested = FileMother.createWithAccessRequestAllowed() + cy.customMount() cy.findByRole('button', { name: 'Request Access' }).should('exist') }) it('renders the access requested message when hen the file is restricted and the access has already been requested', () => { - const accessRequested = { - ...accessCanBeRequested, - requested: true - } + const fileAlreadyRequested = FileMother.createWithAccessRequestPending() - cy.customMount( - - ) + cy.customMount() cy.findByRole('button', { name: 'Access Requested' }) .should('exist') @@ -97,14 +52,8 @@ describe('RequestAccessOption', () => { }) it('does not render the request access button when the file is deaccessioned', () => { - cy.customMount( - - ) + const fileDeaccessioned = FileMother.createDeaccessioned() + cy.customMount() cy.findByRole('button', { name: 'Users may not request access to files.' }).should('not.exist') cy.findByRole('button', { name: 'Request Access' }).should('not.exist') @@ -118,13 +67,19 @@ describe('RequestAccessOption', () => { }) it('does not render the request access button when the file status is public', () => { + const filePublic = FileMother.createWithPublicAccess() + const fileRepository: FileRepository = {} as FileRepository + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: filePublic.id, + canDownloadFile: true + }) + ) + cy.customMount( - + + + ) cy.findByRole('button', { name: 'Users may not request access to files.' }).should('not.exist') @@ -139,13 +94,19 @@ describe('RequestAccessOption', () => { }) it('does not render the request access button when the file status is restricted with access granted', () => { + const fileRestrictedWithAccess = FileMother.createWithRestrictedAccessWithAccessGranted() + const fileRepository: FileRepository = {} as FileRepository + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: fileRestrictedWithAccess.id, + canDownloadFile: true + }) + ) + cy.customMount( - + + + ) cy.findByRole('button', { name: 'Users may not request access to files.' }).should('not.exist') diff --git a/tests/component/sections/dataset/dataset-files/files-table/files-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.spec.tsx b/tests/component/sections/dataset/dataset-files/files-table/files-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.spec.tsx index 42b771692..3c8949ec2 100644 --- a/tests/component/sections/dataset/dataset-files/files-table/files-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.spec.tsx +++ b/tests/component/sections/dataset/dataset-files/files-table/files-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.spec.tsx @@ -1,16 +1,24 @@ import { FileThumbnail } from '../../../../../../../../../../src/sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail' import { FileMother } from '../../../../../../../../files/domain/models/FileMother' +import { FileRepository } from '../../../../../../../../../../src/files/domain/repositories/FileRepository' +import { FileUserPermissionsMother } from '../../../../../../../../files/domain/models/FileUserPermissionsMother' +import { FilePermissionsProvider } from '../../../../../../../../../../src/sections/file/file-permissions/FilePermissionsProvider' +const fileRepository: FileRepository = {} as FileRepository describe('FileThumbnail', () => { - it('renders FileThumbnailPreviewImage when thumbnail is provided', () => { + it('renders FileThumbnailPreviewImage when thumbnail is provided and file can be downloaded', () => { const file = FileMother.createWithThumbnail() + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: file.id, + canDownloadFile: true + }) + ) + cy.customMount( - + + + ) cy.findByAltText(file.name).should('exist') @@ -21,16 +29,28 @@ describe('FileThumbnail', () => { cy.findByText('Restricted with access Icon').should('not.exist') }) + it('does not render FileThumbnailPreviewImage when thumbnail is provided and file cannot be downloaded', () => { + const file = FileMother.createWithThumbnail() + cy.customMount() + + cy.findByAltText(file.name).should('not.exist') + cy.findByText('Restricted File Icon').should('not.exist') + cy.findByText('Restricted with access Icon').should('not.exist') + }) + it('renders FileThumbnailPreviewImage when thumbnail is provided with unlocked icon if restricted with access', () => { const file = FileMother.createWithThumbnailRestrictedWithAccessGranted() + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: file.id, + canDownloadFile: true + }) + ) cy.customMount( - + + + ) cy.findByAltText(file.name).should('exist') @@ -45,14 +65,7 @@ describe('FileThumbnail', () => { it('does not render FileThumbnailPreviewImage when thumbnail is provided if restricted with no access', () => { const file = FileMother.createWithThumbnailRestricted() - cy.customMount( - - ) + cy.customMount() cy.findByAltText(file.name).should('not.exist') cy.findByText('icon-image').should('exist') @@ -65,7 +78,7 @@ describe('FileThumbnail', () => { it('renders FileThumbnailIcon when thumbnail is not provided', () => { const file = FileMother.createDefault() - cy.customMount() + cy.customMount() cy.findByText('icon-file').should('exist') @@ -76,7 +89,7 @@ describe('FileThumbnail', () => { it('renders FileThumbnailIcon when thumbnail is not provided with lock icon when restricted with no access', () => { const file = FileMother.createWithRestrictedAccess() - cy.customMount() + cy.customMount() cy.findByText('icon-file').should('exist') @@ -88,8 +101,18 @@ describe('FileThumbnail', () => { it('renders FileThumbnailIcon when thumbnail is not provided with unlock icon when restricted with access', () => { const file = FileMother.createWithRestrictedAccessWithAccessGranted() + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: file.id, + canDownloadFile: true + }) + ) - cy.customMount() + cy.customMount( + + + + ) cy.findByText('icon-file').should('exist') From 6dcfe6f1dfb59e0e61deaa3a979b8abe7da5dd64 Mon Sep 17 00:00:00 2001 From: MellyGray Date: Tue, 22 Aug 2023 14:18:05 +0200 Subject: [PATCH 04/13] feat(FileUserPermissions): add WithFilePermissions decorator to stories --- src/stories/dataset/Dataset.stories.tsx | 3 ++- .../files/file-permission/WithFilePermissions.tsx | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) create mode 100644 src/stories/files/file-permission/WithFilePermissions.tsx diff --git a/src/stories/dataset/Dataset.stories.tsx b/src/stories/dataset/Dataset.stories.tsx index 747d1adaa..7c4bc629f 100644 --- a/src/stories/dataset/Dataset.stories.tsx +++ b/src/stories/dataset/Dataset.stories.tsx @@ -11,11 +11,12 @@ import { WithCitationMetadataBlockInfo } from './WithCitationMetadataBlockInfo' import { FileMockNoDataRepository } from '../files/FileMockNoDataRepository' import { WithSettings } from '../WithSettings' import { WithLoggedInUser } from '../WithLoggedInUser' +import { WithFilePermissions } from '../files/file-permission/WithFilePermissions' const meta: Meta = { title: 'Pages/Dataset', component: Dataset, - decorators: [WithI18next, WithCitationMetadataBlockInfo, WithSettings], + decorators: [WithI18next, WithCitationMetadataBlockInfo, WithSettings, WithFilePermissions], parameters: { // Sets the delay for all stories. chromatic: { delay: 15000, pauseAnimationAtEnd: true } diff --git a/src/stories/files/file-permission/WithFilePermissions.tsx b/src/stories/files/file-permission/WithFilePermissions.tsx new file mode 100644 index 000000000..35fc8d15c --- /dev/null +++ b/src/stories/files/file-permission/WithFilePermissions.tsx @@ -0,0 +1,11 @@ +import { StoryFn } from '@storybook/react' +import { FilePermissionsProvider } from '../../../sections/file/file-permissions/FilePermissionsProvider' +import { FileMockRepository } from '../FileMockRepository' + +export const WithFilePermissions = (Story: StoryFn) => { + return ( + + + + ) +} From 98c60c4b58aeb4adff8399ff6c0d9372e7c53f2d Mon Sep 17 00:00:00 2001 From: MellyGray Date: Tue, 22 Aug 2023 15:40:33 +0200 Subject: [PATCH 05/13] feat(FileUserPermissions): add latestVersionRestricted to File model --- src/files/domain/models/File.ts | 1 + .../useCases/checkFileDownloadPermission.ts | 3 +- .../files/domain/models/FileMother.ts | 28 ++++++++++++++++++- .../useFilePermissions.spec.tsx | 16 ++++++++++- 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/files/domain/models/File.ts b/src/files/domain/models/File.ts index 7d5ef0294..77e4ed71b 100644 --- a/src/files/domain/models/File.ts +++ b/src/files/domain/models/File.ts @@ -30,6 +30,7 @@ export class FileSize { export interface FileAccess { restricted: boolean + latestVersionRestricted: boolean canBeRequested: boolean requested: boolean } diff --git a/src/files/domain/useCases/checkFileDownloadPermission.ts b/src/files/domain/useCases/checkFileDownloadPermission.ts index 9dbd6818f..5d99a4bc9 100644 --- a/src/files/domain/useCases/checkFileDownloadPermission.ts +++ b/src/files/domain/useCases/checkFileDownloadPermission.ts @@ -11,7 +11,8 @@ export async function checkFileDownloadPermission( }) } - if (!file.access.restricted && !file.isActivelyEmbargoed) { + const isRestricted = file.access.restricted || file.access.latestVersionRestricted + if (!isRestricted && !file.isActivelyEmbargoed) { return true } diff --git a/tests/component/files/domain/models/FileMother.ts b/tests/component/files/domain/models/FileMother.ts index 021eff5f7..eb1edbc34 100644 --- a/tests/component/files/domain/models/FileMother.ts +++ b/tests/component/files/domain/models/FileMother.ts @@ -44,6 +44,7 @@ export class FileMother { name: faker.system.fileName(), access: { restricted: faker.datatype.boolean(), + latestVersionRestricted: faker.datatype.boolean(), canBeRequested: faker.datatype.boolean(), requested: faker.datatype.boolean() }, @@ -121,7 +122,12 @@ export class FileMother { minorNumber: 0, status: FileStatus.RELEASED }, - access: { restricted: false, canBeRequested: false, requested: false }, + access: { + restricted: false, + latestVersionRestricted: false, + canBeRequested: false, + requested: false + }, permissions: { canDownload: true }, labels: [], checksum: undefined, @@ -160,6 +166,7 @@ export class FileMother { return this.createDefault({ access: { restricted: true, + latestVersionRestricted: true, canBeRequested: false, requested: false }, @@ -194,6 +201,19 @@ export class FileMother { return this.createDefault({ access: { restricted: false, + latestVersionRestricted: false, + canBeRequested: false, + requested: false + }, + embargo: undefined + }) + } + + static createWithPublicAccessButLatestVersionRestricted(): File { + return this.createDefault({ + access: { + restricted: false, + latestVersionRestricted: true, canBeRequested: false, requested: false }, @@ -205,6 +225,7 @@ export class FileMother { return this.createDefault({ access: { restricted: true, + latestVersionRestricted: true, canBeRequested: false, requested: false }, @@ -216,6 +237,7 @@ export class FileMother { return this.createDefault({ access: { restricted: true, + latestVersionRestricted: true, canBeRequested: true, requested: false }, @@ -227,6 +249,7 @@ export class FileMother { return this.createDefault({ access: { restricted: true, + latestVersionRestricted: true, canBeRequested: true, requested: false }, @@ -238,6 +261,7 @@ export class FileMother { return this.createDefault({ access: { restricted: true, + latestVersionRestricted: true, canBeRequested: true, requested: true }, @@ -255,6 +279,7 @@ export class FileMother { return this.createDefault({ access: { restricted: true, + latestVersionRestricted: true, canBeRequested: true, requested: false }, @@ -267,6 +292,7 @@ export class FileMother { return this.createDefault({ access: { restricted: true, + latestVersionRestricted: true, canBeRequested: false, requested: false }, diff --git a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx index 5b44bf70f..73db02b26 100644 --- a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx +++ b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx @@ -51,7 +51,6 @@ describe('useFilePermissions', () => { ) - cy.wrap(fileRepository.getFileUserPermissionsById).should('not.be.called') cy.findByText('Has download permission').should('exist') }) @@ -87,6 +86,21 @@ describe('useFilePermissions', () => { cy.findByText('Has download permission').should('exist') }) + it('should call getFileUserPermissionsById when the file is public but latest version is restricted', () => { + const file = FileMother.createWithPublicAccessButLatestVersionRestricted() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has download permission').should('exist') + }) + it('should call getFileUserPermissionsById when the file is embargoed', () => { const file = FileMother.createWithEmbargo() fileRepository.getFileUserPermissionsById = cy From 911e4c4c51dac98e47d7cf194eaf578093133be6 Mon Sep 17 00:00:00 2001 From: MellyGray Date: Tue, 22 Aug 2023 16:05:22 +0200 Subject: [PATCH 06/13] feat(FileUserPermissions): add hook to manage edit dataset permission --- .../domain/models/FileUserPermissions.ts | 3 +- .../checkFileEditDatasetPermission.ts | 11 + .../FilePermissionsProvider.tsx | 13 + .../useFileEditDatasetPermission.tsx | 23 ++ .../useFileEditDatasetPermission.spec.tsx | 63 ++++ .../useFilePermissions.spec.tsx | 270 +++++++++++------- 6 files changed, 277 insertions(+), 106 deletions(-) create mode 100644 src/files/domain/useCases/checkFileEditDatasetPermission.ts create mode 100644 src/sections/file/file-permissions/useFileEditDatasetPermission.tsx create mode 100644 tests/component/sections/file/file-permissions/useFileEditDatasetPermission.spec.tsx diff --git a/src/files/domain/models/FileUserPermissions.ts b/src/files/domain/models/FileUserPermissions.ts index 715a20220..cc72f79c5 100644 --- a/src/files/domain/models/FileUserPermissions.ts +++ b/src/files/domain/models/FileUserPermissions.ts @@ -5,5 +5,6 @@ export interface FileUserPermissions { } export enum FilePermission { - DOWNLOAD_FILE = 'downloadFile' + DOWNLOAD_FILE = 'downloadFile', + EDIT_DATASET = 'editDataset' } diff --git a/src/files/domain/useCases/checkFileEditDatasetPermission.ts b/src/files/domain/useCases/checkFileEditDatasetPermission.ts new file mode 100644 index 000000000..d7ea9d5fd --- /dev/null +++ b/src/files/domain/useCases/checkFileEditDatasetPermission.ts @@ -0,0 +1,11 @@ +import { FileRepository } from '../repositories/FileRepository' +import { File } from '../models/File' + +export async function checkFileEditDatasetPermission( + fileRepository: FileRepository, + file: File +): Promise { + return fileRepository.getFileUserPermissionsById(file.id).then((permissions) => { + return permissions.canEditDataset + }) +} diff --git a/src/sections/file/file-permissions/FilePermissionsProvider.tsx b/src/sections/file/file-permissions/FilePermissionsProvider.tsx index d8c3c8c6d..d8df5dec4 100644 --- a/src/sections/file/file-permissions/FilePermissionsProvider.tsx +++ b/src/sections/file/file-permissions/FilePermissionsProvider.tsx @@ -4,6 +4,7 @@ import { FilePermission } from '../../../files/domain/models/FileUserPermissions import { FilePermissionsContext } from './FilePermissionsContext' import { File } from '../../../files/domain/models/File' import { checkFileDownloadPermission } from '../../../files/domain/useCases/checkFileDownloadPermission' +import { checkFileEditDatasetPermission } from '../../../files/domain/useCases/checkFileEditDatasetPermission' interface SessionUserFilePermissionsProviderProps { repository: FileRepository @@ -24,6 +25,16 @@ export function FilePermissionsProvider({ return false }) } + const checkSessionUserHasEditDatasetPermission = (file: File): Promise => { + return checkFileEditDatasetPermission(repository, file) + .then((canEditDataset) => { + return canEditDataset + }) + .catch((error) => { + console.error('There was an error getting the edit dataset permission', error) + return false + }) + } function checkSessionUserHasFilePermission( permission: FilePermission, @@ -32,6 +43,8 @@ export function FilePermissionsProvider({ switch (permission) { case FilePermission.DOWNLOAD_FILE: return checkSessionUserHasFileDownloadPermission(file) + case FilePermission.EDIT_DATASET: + return checkSessionUserHasEditDatasetPermission(file) } } diff --git a/src/sections/file/file-permissions/useFileEditDatasetPermission.tsx b/src/sections/file/file-permissions/useFileEditDatasetPermission.tsx new file mode 100644 index 000000000..04d2143e3 --- /dev/null +++ b/src/sections/file/file-permissions/useFileEditDatasetPermission.tsx @@ -0,0 +1,23 @@ +import { useEffect, useState } from 'react' +import { FilePermission } from '../../../files/domain/models/FileUserPermissions' +import { useFilePermissions } from './FilePermissionsContext' +import { File } from '../../../files/domain/models/File' + +export function useFileEditDatasetPermission(file: File) { + const { checkSessionUserHasFilePermission } = useFilePermissions() + const [sessionUserHasEditDatasetPermission, setSessionUserHasEditDatasetPermission] = + useState(false) + + useEffect(() => { + checkSessionUserHasFilePermission(FilePermission.EDIT_DATASET, file) + .then((hasPermission) => { + setSessionUserHasEditDatasetPermission(hasPermission) + }) + .catch((error) => { + setSessionUserHasEditDatasetPermission(false) + console.error('There was an error getting the edit dataset permission', error) + }) + }, [file]) + + return { sessionUserHasEditDatasetPermission } +} diff --git a/tests/component/sections/file/file-permissions/useFileEditDatasetPermission.spec.tsx b/tests/component/sections/file/file-permissions/useFileEditDatasetPermission.spec.tsx new file mode 100644 index 000000000..870dc4ef1 --- /dev/null +++ b/tests/component/sections/file/file-permissions/useFileEditDatasetPermission.spec.tsx @@ -0,0 +1,63 @@ +import { FileMother } from '../../../files/domain/models/FileMother' +import { FilePermissionsProvider } from '../../../../../src/sections/file/file-permissions/FilePermissionsProvider' +import { FileRepository } from '../../../../../src/files/domain/repositories/FileRepository' +import { File } from '../../../../../src/files/domain/models/File' +import { FileUserPermissionsMother } from '../../../files/domain/models/FileUserPermissionsMother' +import { FilesCountInfoMother } from '../../../files/domain/models/FilesCountInfoMother' +import { useFileEditDatasetPermission } from '../../../../../src/sections/file/file-permissions/useFileEditDatasetPermission' + +const fileRepository: FileRepository = {} as FileRepository +function TestComponent({ file }: { file: File }) { + const { sessionUserHasEditDatasetPermission } = useFileEditDatasetPermission(file) + + return ( +
+ {sessionUserHasEditDatasetPermission ? ( + Has edit dataset permission + ) : ( + Does not have edit dataset permission + )} +
+ ) +} + +describe('useFileEditDatasetPermission', () => { + beforeEach(() => { + fileRepository.getAllByDatasetPersistentId = cy.stub().resolves([]) + fileRepository.getCountInfoByDatasetPersistentId = cy + .stub() + .resolves(FilesCountInfoMother.create()) + }) + + it('should return edit dataset permission', () => { + const file = FileMother.createDefault() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canEditDataset: true })) + + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has edit dataset permission').should('exist') + }) + + it('should return false for edit dataset permission if there is an error', () => { + const file = FileMother.createDefault() + fileRepository.getFileUserPermissionsById = cy + .stub() + .rejects(new Error('Error getting file user permissions')) + + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Does not have edit dataset permission').should('exist') + }) +}) diff --git a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx index 73db02b26..b6cf80e58 100644 --- a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx +++ b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx @@ -9,7 +9,7 @@ import { FileUserPermissionsMother } from '../../../files/domain/models/FileUser import { FilesCountInfoMother } from '../../../files/domain/models/FilesCountInfoMother' const fileRepository: FileRepository = {} as FileRepository -function TestComponent({ file }: { file: File }) { +function DownloadFileTestComponent({ file }: { file: File }) { const { checkSessionUserHasFilePermission } = useFilePermissions() const [hasFileDownloadPermission, setHasFileDownloadPermission] = useState(false) useEffect(() => { @@ -32,6 +32,31 @@ function TestComponent({ file }: { file: File }) {
) } + +function EditDatasetTestComponent({ file }: { file: File }) { + const { checkSessionUserHasFilePermission } = useFilePermissions() + const [hasEditDatasetPermission, setHasEditDatasetPermission] = useState(false) + useEffect(() => { + checkSessionUserHasFilePermission(FilePermission.EDIT_DATASET, file) + .then((hasPermission) => { + setHasEditDatasetPermission(hasPermission) + }) + .catch((error) => { + console.error('There was an error getting the edit dataset permission', error) + }) + }, [file]) + + return ( +
+ {hasEditDatasetPermission ? ( + Has edit dataset permission + ) : ( + Does not have edit dataset permission + )} +
+ ) +} + describe('useFilePermissions', () => { beforeEach(() => { fileRepository.getAllByDatasetPersistentId = cy.stub().resolves([]) @@ -40,111 +65,146 @@ describe('useFilePermissions', () => { .resolves(FilesCountInfoMother.create()) }) - it('should not call getFileUserPermissionsById when the file is not deaccessioned nor restricted nor embargoed', () => { - const file = FileMother.createDefault() - fileRepository.getFileUserPermissionsById = cy - .stub() - .resolves(FileUserPermissionsMother.create({ fileId: file.id })) - - cy.mount( - - - - ) - cy.wrap(fileRepository.getFileUserPermissionsById).should('not.be.called') - cy.findByText('Has download permission').should('exist') - }) - - it('should call getFileUserPermissionsById when the file is deaccessioned', () => { - const file = FileMother.createDeaccessioned() - fileRepository.getFileUserPermissionsById = cy - .stub() - .resolves(FileUserPermissionsMother.create({ fileId: file.id, canEditDataset: true })) - - cy.mount( - - - - ) - - cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Has download permission').should('exist') - }) - - it('should call getFileUserPermissionsById when the file is restricted', () => { - const file = FileMother.createWithRestrictedAccess() - fileRepository.getFileUserPermissionsById = cy - .stub() - .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) - cy.mount( - - - - ) - - cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Has download permission').should('exist') - }) - - it('should call getFileUserPermissionsById when the file is public but latest version is restricted', () => { - const file = FileMother.createWithPublicAccessButLatestVersionRestricted() - fileRepository.getFileUserPermissionsById = cy - .stub() - .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) - cy.mount( - - - - ) - - cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Has download permission').should('exist') - }) - - it('should call getFileUserPermissionsById when the file is embargoed', () => { - const file = FileMother.createWithEmbargo() - fileRepository.getFileUserPermissionsById = cy - .stub() - .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) - cy.mount( - - - - ) - - cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Has download permission').should('exist') + describe('Download permission', () => { + it('should not call getFileUserPermissionsById when the file is not deaccessioned nor restricted nor embargoed', () => { + const file = FileMother.createDefault() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id })) + + cy.mount( + + + + ) + cy.wrap(fileRepository.getFileUserPermissionsById).should('not.be.called') + cy.findByText('Has download permission').should('exist') + }) + + it('should call getFileUserPermissionsById when the file is deaccessioned', () => { + const file = FileMother.createDeaccessioned() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canEditDataset: true })) + + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has download permission').should('exist') + }) + + it('should call getFileUserPermissionsById when the file is restricted', () => { + const file = FileMother.createWithRestrictedAccess() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has download permission').should('exist') + }) + + it('should call getFileUserPermissionsById when the file is public but latest version is restricted', () => { + const file = FileMother.createWithPublicAccessButLatestVersionRestricted() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has download permission').should('exist') + }) + + it('should call getFileUserPermissionsById when the file is embargoed', () => { + const file = FileMother.createWithEmbargo() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has download permission').should('exist') + }) + + it('should return false if there is an error in the use case request', () => { + const file = FileMother.createWithEmbargo() + fileRepository.getFileUserPermissionsById = cy + .stub() + .rejects(new Error('There was an error getting the file user permissions')) + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Does not have download permission').should('exist') + }) + + it.skip('should use the cached state canDownloadPermissionByFileId the second time the file is being consulted', () => { + // TODO - Implement cache + const file = FileMother.createWithRestrictedAccess() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) + cy.mount( + + + + + ) + + cy.findAllByText('Has download permission').should('exist') + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledOnce') + }) }) - it('should return false if there is an error in the use case request', () => { - const file = FileMother.createWithEmbargo() - fileRepository.getFileUserPermissionsById = cy - .stub() - .rejects(new Error('There was an error getting the file user permissions')) - cy.mount( - - - - ) - - cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Does not have download permission').should('exist') - }) - - it.skip('should use the cached state canDownloadPermissionByFileId the second time the file is being consulted', () => { - // TODO - Implement cache - const file = FileMother.createWithRestrictedAccess() - fileRepository.getFileUserPermissionsById = cy - .stub() - .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) - cy.mount( - - - - - ) - - cy.findAllByText('Has download permission').should('exist') - cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledOnce') + describe('Edit dataset permission', () => { + it('should call getFileUserPermissionsById when asking for edit dataset permission', () => { + const file = FileMother.createDefault() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canEditDataset: true })) + + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Has edit dataset permission').should('exist') + }) + + it('should return false if there is an error in the use case request', () => { + const file = FileMother.createDefault() + fileRepository.getFileUserPermissionsById = cy + .stub() + .rejects(new Error('There was an error getting the file user permissions')) + cy.mount( + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) + cy.findByText('Does not have edit dataset permission').should('exist') + }) }) }) From fa2e348fe420389378b1b9aee0773ea659ab0b7e Mon Sep 17 00:00:00 2001 From: MellyGray Date: Tue, 22 Aug 2023 16:25:59 +0200 Subject: [PATCH 07/13] feat(FileUserPermissions): replace TODO's by the real permissions hook to get editDataset permission --- .../edit-files-menu/EditFilesMenu.tsx | 5 +- .../file-options-menu/FileOptionsMenu.tsx | 5 +- .../useFileEditDatasetPermission.tsx | 1 + .../edit-files-menu/EditFilesMenu.spec.tsx | 59 ++++++++++++---- .../FileOptionsMenu.spec.tsx | 70 ++++++++++++++----- 5 files changed, 107 insertions(+), 33 deletions(-) diff --git a/src/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.tsx b/src/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.tsx index 3b8a16e08..e5c664274 100644 --- a/src/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.tsx +++ b/src/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.tsx @@ -5,6 +5,7 @@ import styles from './EditFilesMenu.module.scss' import { EditFilesOptions } from './EditFilesOptions' import { File } from '../../../../../../files/domain/models/File' import { useTranslation } from 'react-i18next' +import { useFileEditDatasetPermission } from '../../../../../file/file-permissions/useFileEditDatasetPermission' interface EditFilesMenuProps { files: File[] @@ -13,14 +14,14 @@ const MINIMUM_FILES_COUNT_TO_SHOW_EDIT_FILES_BUTTON = 1 export function EditFilesMenu({ files }: EditFilesMenuProps) { const { t } = useTranslation('files') const { user } = useSession() - const userHasDatasetUpdatePermissions = true // TODO - Implement permissions + const { sessionUserHasEditDatasetPermission } = useFileEditDatasetPermission(files[0]) const datasetHasValidTermsOfAccess = true // TODO - Implement terms of access validation const datasetLockedFromEdits = false // TODO - Ask Guillermo if this a dataset property coming from the api if ( files.length < MINIMUM_FILES_COUNT_TO_SHOW_EDIT_FILES_BUTTON || !user || - !userHasDatasetUpdatePermissions + !sessionUserHasEditDatasetPermission ) { return <> } diff --git a/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/file-options-menu/FileOptionsMenu.tsx b/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/file-options-menu/FileOptionsMenu.tsx index 56d11a735..630c9e8f5 100644 --- a/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/file-options-menu/FileOptionsMenu.tsx +++ b/src/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/file-options-menu/FileOptionsMenu.tsx @@ -4,16 +4,17 @@ import { PencilFill, ThreeDotsVertical } from 'react-bootstrap-icons' import { useSession } from '../../../../../../../session/SessionContext' import { EditFilesOptions } from '../../../edit-files-menu/EditFilesOptions' import { useTranslation } from 'react-i18next' +import { useFileEditDatasetPermission } from '../../../../../../../file/file-permissions/useFileEditDatasetPermission' export function FileOptionsMenu({ file }: { file: File }) { const { t } = useTranslation('files') const { user } = useSession() - const userHasDatasetUpdatePermissions = true // TODO - Implement permissions + const { sessionUserHasEditDatasetPermission } = useFileEditDatasetPermission(file) const datasetHasValidTermsOfAccess = true // TODO - Implement terms of access validation const datasetLockedFromEdits = false // TODO - Ask Guillermo if this a dataset property coming from the api // const isDeleted = false // TODO - Ask Guillermo if this is a file property coming from the api - if (!user || !userHasDatasetUpdatePermissions || !datasetHasValidTermsOfAccess) { + if (!user || !sessionUserHasEditDatasetPermission || !datasetHasValidTermsOfAccess) { return <> } diff --git a/src/sections/file/file-permissions/useFileEditDatasetPermission.tsx b/src/sections/file/file-permissions/useFileEditDatasetPermission.tsx index 04d2143e3..f593dea77 100644 --- a/src/sections/file/file-permissions/useFileEditDatasetPermission.tsx +++ b/src/sections/file/file-permissions/useFileEditDatasetPermission.tsx @@ -3,6 +3,7 @@ import { FilePermission } from '../../../files/domain/models/FileUserPermissions import { useFilePermissions } from './FilePermissionsContext' import { File } from '../../../files/domain/models/File' +// TODO: Consider getting the permission from the dataset instead of from the file export function useFileEditDatasetPermission(file: File) { const { checkSessionUserHasFilePermission } = useFilePermissions() const [sessionUserHasEditDatasetPermission, setSessionUserHasEditDatasetPermission] = diff --git a/tests/component/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.spec.tsx b/tests/component/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.spec.tsx index b546238b7..dd7aac757 100644 --- a/tests/component/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.spec.tsx +++ b/tests/component/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.spec.tsx @@ -3,20 +3,32 @@ import { UserMother } from '../../../../../../users/domain/models/UserMother' import { UserRepository } from '../../../../../../../../src/users/domain/repositories/UserRepository' import { SessionProvider } from '../../../../../../../../src/sections/session/SessionProvider' import { FileMother } from '../../../../../../files/domain/models/FileMother' +import { FileRepository } from '../../../../../../../../src/files/domain/repositories/FileRepository' +import { FileUserPermissionsMother } from '../../../../../../files/domain/models/FileUserPermissionsMother' +import { FilePermissionsProvider } from '../../../../../../../../src/sections/file/file-permissions/FilePermissionsProvider' const user = UserMother.create() const userRepository = {} as UserRepository const files = FileMother.createMany(2) +const fileRepository: FileRepository = {} as FileRepository describe('EditFilesMenu', () => { beforeEach(() => { userRepository.getAuthenticated = cy.stub().resolves(user) userRepository.removeAuthenticated = cy.stub().resolves() + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: files[0].id, + canEditDataset: true + }) + ) }) it('renders the Edit Files menu', () => { cy.customMount( - - - + + + + + ) cy.findByRole('button', { name: 'Edit Files' }).should('exist') @@ -26,9 +38,11 @@ describe('EditFilesMenu', () => { userRepository.getAuthenticated = cy.stub().resolves(null) cy.customMount( - - - + + + + + ) cy.findByRole('button', { name: 'Edit Files' }).should('not.exist') @@ -36,9 +50,11 @@ describe('EditFilesMenu', () => { it('does not render the Edit Files menu when there are no files in the dataset', () => { cy.customMount( - - - + + + + + ) cy.findByRole('button', { name: 'Edit Files' }).should('not.exist') @@ -46,9 +62,11 @@ describe('EditFilesMenu', () => { it('renders the Edit Files options', () => { cy.customMount( - - - + + + + + ) cy.findByRole('button', { name: 'Edit Files' }).click() @@ -56,7 +74,22 @@ describe('EditFilesMenu', () => { }) it.skip('does not render the Edit Files menu when the user does not have update dataset permissions', () => { - // TODO: Implement this test + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: files[0].id, + canEditDataset: false + }) + ) + + cy.customMount( + + + + + + ) + + cy.findByRole('button', { name: 'Edit Files' }).should('not.exist') }) it.skip('renders the disabled Edit Files menu when the dataset is locked from edits', () => { diff --git a/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/file-options-menu/FileOptionsMenu.spec.tsx b/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/file-options-menu/FileOptionsMenu.spec.tsx index 4fabacca9..f4b961ddf 100644 --- a/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/file-options-menu/FileOptionsMenu.spec.tsx +++ b/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/file-options-menu/FileOptionsMenu.spec.tsx @@ -3,27 +3,44 @@ import { FileMother } from '../../../../../../../../files/domain/models/FileMoth import { UserMother } from '../../../../../../../../users/domain/models/UserMother' import { UserRepository } from '../../../../../../../../../../src/users/domain/repositories/UserRepository' import { SessionProvider } from '../../../../../../../../../../src/sections/session/SessionProvider' +import { FileRepository } from '../../../../../../../../../../src/files/domain/repositories/FileRepository' +import { FileUserPermissionsMother } from '../../../../../../../../files/domain/models/FileUserPermissionsMother' +import { FilePermissionsProvider } from '../../../../../../../../../../src/sections/file/file-permissions/FilePermissionsProvider' const file = FileMother.createDefault() const user = UserMother.create() const userRepository = {} as UserRepository +const fileRepository: FileRepository = {} as FileRepository describe('FileOptionsMenu', () => { - it('renders the FileOptionsMenu', () => { + beforeEach(() => { userRepository.getAuthenticated = cy.stub().resolves(user) userRepository.removeAuthenticated = cy.stub().resolves() + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: file.id, + canEditDataset: true + }) + ) + }) + + it('renders the FileOptionsMenu', () => { cy.customMount( - - - + + + + + ) cy.findByRole('button', { name: 'File Options' }).should('exist') }) it('renders the file options menu with tooltip', () => { cy.customMount( - - - + + + + + ) cy.findByRole('button', { name: 'File Options' }).trigger('mouseover') @@ -32,9 +49,11 @@ describe('FileOptionsMenu', () => { it('renders the dropdown header', () => { cy.customMount( - - - + + + + + ) cy.findByRole('button', { name: 'File Options' }).should('exist').click() @@ -42,13 +61,30 @@ describe('FileOptionsMenu', () => { }) it('does not render is the user is not authenticated', () => { - cy.customMount() + cy.customMount( + + + + ) cy.findByRole('button', { name: 'File Options' }).should('not.exist') }) - it.skip('does not render is the user do not have permissions to update the dataset', () => { - // TODO: Implement this test + it('does not render is the user do not have permissions to update the dataset', () => { + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: file.id, + canEditDataset: false + }) + ) + cy.customMount( + + + + + + ) + cy.findByRole('button', { name: 'File Options' }).should('not.exist') }) it.skip('does not render if there are not valid terms of access', () => { @@ -65,9 +101,11 @@ describe('FileOptionsMenu', () => { it('renders the menu options', () => { cy.customMount( - - - + + + + + ) cy.findByRole('button', { name: 'File Options' }).click() From 81dfb57264bdfe3d84527e5bb3d890c68b3d16d0 Mon Sep 17 00:00:00 2001 From: MellyGray Date: Tue, 22 Aug 2023 16:43:02 +0200 Subject: [PATCH 08/13] feat(FileUserPermission): add TODO about the privateUrlUser --- src/files/domain/useCases/checkFileDownloadPermission.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/files/domain/useCases/checkFileDownloadPermission.ts b/src/files/domain/useCases/checkFileDownloadPermission.ts index 5d99a4bc9..a5475eb1e 100644 --- a/src/files/domain/useCases/checkFileDownloadPermission.ts +++ b/src/files/domain/useCases/checkFileDownloadPermission.ts @@ -5,6 +5,8 @@ export async function checkFileDownloadPermission( fileRepository: FileRepository, file: File ): Promise { + // TODO: ask Guillermo if we want to check the privateUrlUser with userRepository.getAuthenticated() + if (file.version.status === FileStatus.DEACCESSIONED) { return fileRepository.getFileUserPermissionsById(file.id).then((permissions) => { return permissions.canEditDataset From e565a6810cde959ca55c9a412544d56e5366b7e8 Mon Sep 17 00:00:00 2001 From: MellyGray Date: Tue, 22 Aug 2023 17:24:04 +0200 Subject: [PATCH 09/13] feat(FileUserPerission): manage downloadFile permission for privateUrl --- .../useCases/checkFileDownloadPermission.ts | 2 -- .../FilePermissionsProvider.tsx | 5 +++++ .../useFilePermissions.spec.tsx | 22 +++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/src/files/domain/useCases/checkFileDownloadPermission.ts b/src/files/domain/useCases/checkFileDownloadPermission.ts index a5475eb1e..5d99a4bc9 100644 --- a/src/files/domain/useCases/checkFileDownloadPermission.ts +++ b/src/files/domain/useCases/checkFileDownloadPermission.ts @@ -5,8 +5,6 @@ export async function checkFileDownloadPermission( fileRepository: FileRepository, file: File ): Promise { - // TODO: ask Guillermo if we want to check the privateUrlUser with userRepository.getAuthenticated() - if (file.version.status === FileStatus.DEACCESSIONED) { return fileRepository.getFileUserPermissionsById(file.id).then((permissions) => { return permissions.canEditDataset diff --git a/src/sections/file/file-permissions/FilePermissionsProvider.tsx b/src/sections/file/file-permissions/FilePermissionsProvider.tsx index d8df5dec4..940734505 100644 --- a/src/sections/file/file-permissions/FilePermissionsProvider.tsx +++ b/src/sections/file/file-permissions/FilePermissionsProvider.tsx @@ -5,6 +5,7 @@ import { FilePermissionsContext } from './FilePermissionsContext' import { File } from '../../../files/domain/models/File' import { checkFileDownloadPermission } from '../../../files/domain/useCases/checkFileDownloadPermission' import { checkFileEditDatasetPermission } from '../../../files/domain/useCases/checkFileEditDatasetPermission' +import { useAnonymized } from '../../dataset/anonymized/AnonymizedContext' interface SessionUserFilePermissionsProviderProps { repository: FileRepository @@ -14,7 +15,11 @@ export function FilePermissionsProvider({ repository, children }: PropsWithChildren) { + const { anonymizedView } = useAnonymized() const checkSessionUserHasFileDownloadPermission = (file: File): Promise => { + if (anonymizedView) { + return Promise.resolve(true) // If the user is in anonymized view, they can always download the file + } return checkFileDownloadPermission(repository, file) .then((canDownloadFile) => { // TODO - Cache the result diff --git a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx index b6cf80e58..08488453d 100644 --- a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx +++ b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx @@ -7,6 +7,7 @@ import { FileRepository } from '../../../../../src/files/domain/repositories/Fil import { File } from '../../../../../src/files/domain/models/File' import { FileUserPermissionsMother } from '../../../files/domain/models/FileUserPermissionsMother' import { FilesCountInfoMother } from '../../../files/domain/models/FilesCountInfoMother' +import { AnonymizedContext } from '../../../../../src/sections/dataset/anonymized/AnonymizedContext' const fileRepository: FileRepository = {} as FileRepository function DownloadFileTestComponent({ file }: { file: File }) { @@ -173,6 +174,27 @@ describe('useFilePermissions', () => { cy.findAllByText('Has download permission').should('exist') cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledOnce') }) + + it('should always allow to download if the user is in anonymized view (privateUrl)', () => { + const file = FileMother.createWithRestrictedAccess() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: false })) + + const anonymizedView = true + const setAnonymizedView = () => {} + cy.mount( + + + + + + + ) + + cy.wrap(fileRepository.getFileUserPermissionsById).should('not.be.called') + cy.findAllByText('Has download permission').should('exist') + }) }) describe('Edit dataset permission', () => { From 147478d024b7a1d19cd67bb7dc3b27449c97c2a5 Mon Sep 17 00:00:00 2001 From: MellyGray Date: Tue, 22 Aug 2023 17:30:56 +0200 Subject: [PATCH 10/13] fix: tests failing because of missing permissions provider in the tests --- .../file-actions/FileActionsHeader.spec.tsx | 19 +++++++++++++++--- .../FileActionButtons.spec.tsx | 20 +++++++++++++++---- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/tests/component/sections/dataset/dataset-files/files-table/file-actions/FileActionsHeader.spec.tsx b/tests/component/sections/dataset/dataset-files/files-table/file-actions/FileActionsHeader.spec.tsx index 259c05fdf..9d22c9601 100644 --- a/tests/component/sections/dataset/dataset-files/files-table/file-actions/FileActionsHeader.spec.tsx +++ b/tests/component/sections/dataset/dataset-files/files-table/file-actions/FileActionsHeader.spec.tsx @@ -3,6 +3,9 @@ import { UserMother } from '../../../../../users/domain/models/UserMother' import { UserRepository } from '../../../../../../../src/users/domain/repositories/UserRepository' import { SessionProvider } from '../../../../../../../src/sections/session/SessionProvider' import { FileMother } from '../../../../../files/domain/models/FileMother' +import { FileRepository } from '../../../../../../../src/files/domain/repositories/FileRepository' +import { FileUserPermissionsMother } from '../../../../../files/domain/models/FileUserPermissionsMother' +import { FilePermissionsProvider } from '../../../../../../../src/sections/file/file-permissions/FilePermissionsProvider' describe('FileActionsHeader', () => { it('renders the file actions header', () => { @@ -10,11 +13,21 @@ describe('FileActionsHeader', () => { const userRepository = {} as UserRepository userRepository.getAuthenticated = cy.stub().resolves(user) userRepository.removeAuthenticated = cy.stub().resolves() + const files = FileMother.createMany(2) + const fileRepository: FileRepository = {} as FileRepository + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: files[0].id, + canEditDataset: true + }) + ) cy.customMount( - - - + + + + + ) cy.findByRole('button', { name: 'Edit Files' }).should('exist') diff --git a/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/FileActionButtons.spec.tsx b/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/FileActionButtons.spec.tsx index d78277a63..beef29fba 100644 --- a/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/FileActionButtons.spec.tsx +++ b/tests/component/sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/FileActionButtons.spec.tsx @@ -3,6 +3,9 @@ import { FileMother } from '../../../../../../../files/domain/models/FileMother' import { UserMother } from '../../../../../../../users/domain/models/UserMother' import { UserRepository } from '../../../../../../../../../src/users/domain/repositories/UserRepository' import { SessionProvider } from '../../../../../../../../../src/sections/session/SessionProvider' +import { FileRepository } from '../../../../../../../../../src/files/domain/repositories/FileRepository' +import { FileUserPermissionsMother } from '../../../../../../../files/domain/models/FileUserPermissionsMother' +import { FilePermissionsProvider } from '../../../../../../../../../src/sections/file/file-permissions/FilePermissionsProvider' const file = FileMother.createDefault() describe('FileActionButtons', () => { @@ -13,16 +16,25 @@ describe('FileActionButtons', () => { cy.findByRole('button', { name: 'Access File' }).should('exist') }) - it('renders the file action buttons with user logged in', () => { + it('renders the file action buttons with user logged in and edit dataset permissions', () => { const user = UserMother.create() const userRepository = {} as UserRepository userRepository.getAuthenticated = cy.stub().resolves(user) userRepository.removeAuthenticated = cy.stub().resolves() + const fileRepository: FileRepository = {} as FileRepository + fileRepository.getFileUserPermissionsById = cy.stub().resolves( + FileUserPermissionsMother.create({ + fileId: file.id, + canEditDataset: true + }) + ) cy.customMount( - - - + + + + + ) cy.findByRole('group', { name: 'File Action Buttons' }).should('exist') From 1d01d54e698722a7ee88fbdcd3466400785e4372 Mon Sep 17 00:00:00 2001 From: MellyGray Date: Wed, 23 Aug 2023 10:20:23 +0200 Subject: [PATCH 11/13] fix(stories): add WithFilePermissionsGranted decorator --- .../AccessFileMenu.stories.tsx | 4 ++++ .../edit-files-menu/EditFilesMenu.stories.tsx | 3 ++- .../FileOptionsMenu.stories.tsx | 3 ++- .../file-thumbnail/FileThumbnail.stories.tsx | 12 ++++++++++++ .../FileWithGrantedPermissionsRepository.ts | 18 ++++++++++++++++++ .../WithFilePermissionsGranted.tsx | 11 +++++++++++ .../domain/models/FileUserPermissionsMother.ts | 7 +++++++ 7 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 src/stories/files/FileWithGrantedPermissionsRepository.ts create mode 100644 src/stories/files/file-permission/WithFilePermissionsGranted.tsx diff --git a/src/stories/dataset/dataset-files/files-table/file-actions/access-file-menu/AccessFileMenu.stories.tsx b/src/stories/dataset/dataset-files/files-table/file-actions/access-file-menu/AccessFileMenu.stories.tsx index d8a89a1de..a1e4b668d 100644 --- a/src/stories/dataset/dataset-files/files-table/file-actions/access-file-menu/AccessFileMenu.stories.tsx +++ b/src/stories/dataset/dataset-files/files-table/file-actions/access-file-menu/AccessFileMenu.stories.tsx @@ -3,6 +3,7 @@ import { WithI18next } from '../../../../../WithI18next' import { WithSettings } from '../../../../../WithSettings' import { AccessFileMenu } from '../../../../../../sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/access-file-menu/AccessFileMenu' import { FileMother } from '../../../../../../../tests/component/files/domain/models/FileMother' +import { WithFilePermissionsGranted } from '../../../../../files/file-permission/WithFilePermissionsGranted' const meta: Meta = { title: @@ -15,6 +16,7 @@ export default meta type Story = StoryObj export const Default: Story = { + decorators: [WithFilePermissionsGranted], render: () => } @@ -31,10 +33,12 @@ export const RestrictedWithAccessRequestPending: Story = { } export const RestrictedWithAccessGranted: Story = { + decorators: [WithFilePermissionsGranted], render: () => } export const WithEmbargo: Story = { + decorators: [WithFilePermissionsGranted], render: () => } diff --git a/src/stories/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.stories.tsx b/src/stories/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.stories.tsx index b5c58b01e..e85af7831 100644 --- a/src/stories/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.stories.tsx +++ b/src/stories/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.stories.tsx @@ -4,11 +4,12 @@ import { WithSettings } from '../../../../../WithSettings' import { FileMother } from '../../../../../../../tests/component/files/domain/models/FileMother' import { EditFilesMenu } from '../../../../../../sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu' import { WithLoggedInUser } from '../../../../../WithLoggedInUser' +import { WithFilePermissionsGranted } from '../../../../../files/file-permission/WithFilePermissionsGranted' const meta: Meta = { title: 'Sections/Dataset Page/DatasetFiles/FilesTable/EditFilesMenu', component: EditFilesMenu, - decorators: [WithI18next, WithSettings, WithLoggedInUser] + decorators: [WithI18next, WithSettings, WithLoggedInUser, WithFilePermissionsGranted] } export default meta diff --git a/src/stories/dataset/dataset-files/files-table/file-actions/file-options-menu/FileOptionsMenu.stories.tsx b/src/stories/dataset/dataset-files/files-table/file-actions/file-options-menu/FileOptionsMenu.stories.tsx index 5bb6b146d..fe0a53970 100644 --- a/src/stories/dataset/dataset-files/files-table/file-actions/file-options-menu/FileOptionsMenu.stories.tsx +++ b/src/stories/dataset/dataset-files/files-table/file-actions/file-options-menu/FileOptionsMenu.stories.tsx @@ -4,12 +4,13 @@ import { WithSettings } from '../../../../../WithSettings' import { FileMother } from '../../../../../../../tests/component/files/domain/models/FileMother' import { FileOptionsMenu } from '../../../../../../sections/dataset/dataset-files/files-table/file-actions/file-actions-cell/file-action-buttons/file-options-menu/FileOptionsMenu' import { WithLoggedInUser } from '../../../../../WithLoggedInUser' +import { WithFilePermissionsGranted } from '../../../../../files/file-permission/WithFilePermissionsGranted' const meta: Meta = { title: 'Sections/Dataset Page/DatasetFiles/FilesTable/FileActionsCell/FileActionButtons/FileOptionsMenu', component: FileOptionsMenu, - decorators: [WithI18next, WithSettings, WithLoggedInUser] + decorators: [WithI18next, WithSettings, WithLoggedInUser, WithFilePermissionsGranted] } export default meta diff --git a/src/stories/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.stories.tsx b/src/stories/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.stories.tsx index 0f94b0b67..fdecf889a 100644 --- a/src/stories/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.stories.tsx +++ b/src/stories/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail.stories.tsx @@ -2,6 +2,7 @@ import { Meta, StoryObj } from '@storybook/react' import { FileThumbnail } from '../../../../../../../../sections/dataset/dataset-files/files-table/file-info/file-info-cell/file-info-data/file-thumbnail/FileThumbnail' import { WithI18next } from '../../../../../../../WithI18next' import { FileMother } from '../../../../../../../../../tests/component/files/domain/models/FileMother' +import { WithFilePermissionsGranted } from '../../../../../../../files/file-permission/WithFilePermissionsGranted' const meta: Meta = { title: 'Sections/Dataset Page/DatasetFiles/FilesTable/FileInfoCell/FileThumbnail', @@ -13,6 +14,7 @@ export default meta type Story = StoryObj export const WithIcon: Story = { + decorators: [WithFilePermissionsGranted], render: () => { const file = FileMother.createDefault() return @@ -20,6 +22,7 @@ export const WithIcon: Story = { } export const WithThumbnailPreview: Story = { + decorators: [WithFilePermissionsGranted], render: () => { const file = FileMother.createWithThumbnail() return @@ -34,8 +37,17 @@ export const WithThumbnailRestrictedLockedIcon: Story = { } export const WithThumbnailRestrictedUnlockedIcon: Story = { + decorators: [WithFilePermissionsGranted], render: () => { const file = FileMother.createWithRestrictedAccessWithAccessGranted() return } } + +export const WithThumbnailPreviewRestrictedUnlockedIcon: Story = { + decorators: [WithFilePermissionsGranted], + render: () => { + const file = FileMother.createWithThumbnailRestrictedWithAccessGranted() + return + } +} diff --git a/src/stories/files/FileWithGrantedPermissionsRepository.ts b/src/stories/files/FileWithGrantedPermissionsRepository.ts new file mode 100644 index 000000000..6bc472d5b --- /dev/null +++ b/src/stories/files/FileWithGrantedPermissionsRepository.ts @@ -0,0 +1,18 @@ +import { FileRepository } from '../../files/domain/repositories/FileRepository' +import { FileUserPermissionsMother } from '../../../tests/component/files/domain/models/FileUserPermissionsMother' +import { FileUserPermissions } from '../../files/domain/models/FileUserPermissions' +import { FileMockRepository } from './FileMockRepository' + +export class FileWithGrantedPermissionsRepository + extends FileMockRepository + implements FileRepository +{ + // eslint-disable-next-line unused-imports/no-unused-vars + getFileUserPermissionsById(id: string): Promise { + return new Promise((resolve) => { + setTimeout(() => { + resolve(FileUserPermissionsMother.createWithGrantedPermissions()) + }, 1000) + }) + } +} diff --git a/src/stories/files/file-permission/WithFilePermissionsGranted.tsx b/src/stories/files/file-permission/WithFilePermissionsGranted.tsx new file mode 100644 index 000000000..1a21fbd97 --- /dev/null +++ b/src/stories/files/file-permission/WithFilePermissionsGranted.tsx @@ -0,0 +1,11 @@ +import { StoryFn } from '@storybook/react' +import { FilePermissionsProvider } from '../../../sections/file/file-permissions/FilePermissionsProvider' +import { FileWithGrantedPermissionsRepository } from '../FileWithGrantedPermissionsRepository' + +export const WithFilePermissionsGranted = (Story: StoryFn) => { + return ( + + + + ) +} diff --git a/tests/component/files/domain/models/FileUserPermissionsMother.ts b/tests/component/files/domain/models/FileUserPermissionsMother.ts index 3a34e6b09..c1f33cd64 100644 --- a/tests/component/files/domain/models/FileUserPermissionsMother.ts +++ b/tests/component/files/domain/models/FileUserPermissionsMother.ts @@ -10,4 +10,11 @@ export class FileUserPermissionsMother { ...props } } + + static createWithGrantedPermissions(): FileUserPermissions { + return FileUserPermissionsMother.create({ + canDownloadFile: true, + canEditDataset: true + }) + } } From fee1543a3893eec1978a122f593d0091824f13e8 Mon Sep 17 00:00:00 2001 From: MellyGray Date: Wed, 23 Aug 2023 18:48:55 +0200 Subject: [PATCH 12/13] feat(FileUserPermissions): fetch the permissions and save them before showing the files table --- .../FileJSDataverseRepository.ts | 1 + .../edit-files-menu/EditFilesMenu.tsx | 2 +- .../dataset/dataset-files/useFiles.tsx | 8 +- .../FilePermissionsContext.ts | 4 +- .../FilePermissionsProvider.tsx | 46 ++++++- src/stories/dataset/Dataset.stories.tsx | 17 +-- .../FileWithDeniedPermissionsRepository.ts | 18 +++ .../file-permission/WithFilePermissions.tsx | 11 -- .../WithFilePermissionsDenied.tsx | 11 ++ .../models/FileUserPermissionsMother.ts | 7 ++ .../dataset/dataset-files/useFiles.spec.tsx | 93 +++++++++++++++ .../useFilePermissions.spec.tsx | 112 +++++++++++------- 12 files changed, 266 insertions(+), 64 deletions(-) create mode 100644 src/stories/files/FileWithDeniedPermissionsRepository.ts delete mode 100644 src/stories/files/file-permission/WithFilePermissions.tsx create mode 100644 src/stories/files/file-permission/WithFilePermissionsDenied.tsx create mode 100644 tests/component/sections/dataset/dataset-files/useFiles.spec.tsx diff --git a/src/files/infrastructure/FileJSDataverseRepository.ts b/src/files/infrastructure/FileJSDataverseRepository.ts index 5656984a6..e36d29913 100644 --- a/src/files/infrastructure/FileJSDataverseRepository.ts +++ b/src/files/infrastructure/FileJSDataverseRepository.ts @@ -36,6 +36,7 @@ export class FileJSDataverseRepository implements FileRepository { }, 1000) }) } + // eslint-disable-next-line unused-imports/no-unused-vars getFileUserPermissionsById(id: string): Promise { // TODO - implement using js-dataverse return new Promise((resolve) => { diff --git a/src/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.tsx b/src/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.tsx index e5c664274..2ab92b860 100644 --- a/src/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.tsx +++ b/src/sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu.tsx @@ -14,7 +14,7 @@ const MINIMUM_FILES_COUNT_TO_SHOW_EDIT_FILES_BUTTON = 1 export function EditFilesMenu({ files }: EditFilesMenuProps) { const { t } = useTranslation('files') const { user } = useSession() - const { sessionUserHasEditDatasetPermission } = useFileEditDatasetPermission(files[0]) + const { sessionUserHasEditDatasetPermission } = useFileEditDatasetPermission(files[0] || {}) const datasetHasValidTermsOfAccess = true // TODO - Implement terms of access validation const datasetLockedFromEdits = false // TODO - Ask Guillermo if this a dataset property coming from the api diff --git a/src/sections/dataset/dataset-files/useFiles.tsx b/src/sections/dataset/dataset-files/useFiles.tsx index a4def71b4..2c8059e77 100644 --- a/src/sections/dataset/dataset-files/useFiles.tsx +++ b/src/sections/dataset/dataset-files/useFiles.tsx @@ -6,6 +6,8 @@ import { FileCriteria } from '../../../files/domain/models/FileCriteria' import { FilesCountInfo } from '../../../files/domain/models/FilesCountInfo' import { getFilesCountInfoByDatasetPersistentId } from '../../../files/domain/useCases/getFilesCountInfoByDatasetPersistentId' import { FilePaginationInfo } from '../../../files/domain/models/FilePaginationInfo' +import { useFilePermissions } from '../../file/file-permissions/FilePermissionsContext' +import { FilePermission } from '../../../files/domain/models/FileUserPermissions' export function useFiles( filesRepository: FileRepository, @@ -14,6 +16,7 @@ export function useFiles( paginationInfo?: FilePaginationInfo, criteria?: FileCriteria ) { + const { fetchFilesPermission } = useFilePermissions() const [files, setFiles] = useState([]) const [isLoading, setIsLoading] = useState(true) const [filesCountInfo, setFilesCountInfo] = useState({ @@ -44,8 +47,11 @@ export function useFiles( ) .then((files: File[]) => { setFiles(files) - setIsLoading(false) + return files }) + .then((files: File[]) => + fetchFilesPermission(FilePermission.DOWNLOAD_FILE, files).then(() => setIsLoading(false)) + ) .catch((error) => { console.error('There was an error getting the files', error) setIsLoading(false) diff --git a/src/sections/file/file-permissions/FilePermissionsContext.ts b/src/sections/file/file-permissions/FilePermissionsContext.ts index 930ba2770..244d5ea52 100644 --- a/src/sections/file/file-permissions/FilePermissionsContext.ts +++ b/src/sections/file/file-permissions/FilePermissionsContext.ts @@ -4,10 +4,12 @@ import { File } from '../../../files/domain/models/File' interface FilePermissionsContextProps { checkSessionUserHasFilePermission: (permission: FilePermission, file: File) => Promise + fetchFilesPermission: (permission: FilePermission, files: File[]) => Promise } export const FilePermissionsContext = createContext({ - checkSessionUserHasFilePermission: () => Promise.reject('Not implemented') + checkSessionUserHasFilePermission: () => Promise.reject('Not implemented'), + fetchFilesPermission: () => Promise.reject('Not implemented') }) export const useFilePermissions = () => useContext(FilePermissionsContext) diff --git a/src/sections/file/file-permissions/FilePermissionsProvider.tsx b/src/sections/file/file-permissions/FilePermissionsProvider.tsx index 940734505..efdea374e 100644 --- a/src/sections/file/file-permissions/FilePermissionsProvider.tsx +++ b/src/sections/file/file-permissions/FilePermissionsProvider.tsx @@ -15,14 +15,33 @@ export function FilePermissionsProvider({ repository, children }: PropsWithChildren) { + const filePermissionsMap = new Map() const { anonymizedView } = useAnonymized() + + const updateFilePermissionsMap = ( + fileIdToUpdate: string, + newPermissionKey: string, + newPermissionValue: boolean + ) => { + if (filePermissionsMap.has(fileIdToUpdate)) { + const existingValue = filePermissionsMap.get(fileIdToUpdate) + if (existingValue) { + existingValue[newPermissionKey] = newPermissionValue + } + } else { + const newValue = { + [newPermissionKey]: newPermissionValue + } + filePermissionsMap.set(fileIdToUpdate, newValue) + } + } const checkSessionUserHasFileDownloadPermission = (file: File): Promise => { if (anonymizedView) { return Promise.resolve(true) // If the user is in anonymized view, they can always download the file } return checkFileDownloadPermission(repository, file) .then((canDownloadFile) => { - // TODO - Cache the result + updateFilePermissionsMap(file.id, FilePermission.DOWNLOAD_FILE, canDownloadFile) return canDownloadFile }) .catch((error) => { @@ -33,6 +52,7 @@ export function FilePermissionsProvider({ const checkSessionUserHasEditDatasetPermission = (file: File): Promise => { return checkFileEditDatasetPermission(repository, file) .then((canEditDataset) => { + updateFilePermissionsMap(file.id, FilePermission.EDIT_DATASET, canEditDataset) return canEditDataset }) .catch((error) => { @@ -45,6 +65,13 @@ export function FilePermissionsProvider({ permission: FilePermission, file: File ): Promise { + if (filePermissionsMap.has(file.id)) { + const savedPermission = filePermissionsMap.get(file.id)?.[permission] + if (savedPermission !== undefined) { + return Promise.resolve(savedPermission) + } + } + switch (permission) { case FilePermission.DOWNLOAD_FILE: return checkSessionUserHasFileDownloadPermission(file) @@ -53,9 +80,24 @@ export function FilePermissionsProvider({ } } + function fetchFilesPermission(permission: FilePermission, files: File[]): Promise { + return Promise.all( + files.map((file) => + checkSessionUserHasFilePermission(permission, file) + .then((hasPermission) => { + return hasPermission + }) + .catch((error) => { + console.error('There was an error getting the file permission', error) + return false + }) + ) + ) + } + return ( + value={{ checkSessionUserHasFilePermission, fetchFilesPermission }}> {children} ) diff --git a/src/stories/dataset/Dataset.stories.tsx b/src/stories/dataset/Dataset.stories.tsx index 7c4bc629f..d0da00035 100644 --- a/src/stories/dataset/Dataset.stories.tsx +++ b/src/stories/dataset/Dataset.stories.tsx @@ -11,12 +11,13 @@ import { WithCitationMetadataBlockInfo } from './WithCitationMetadataBlockInfo' import { FileMockNoDataRepository } from '../files/FileMockNoDataRepository' import { WithSettings } from '../WithSettings' import { WithLoggedInUser } from '../WithLoggedInUser' -import { WithFilePermissions } from '../files/file-permission/WithFilePermissions' +import { WithFilePermissionsDenied } from '../files/file-permission/WithFilePermissionsDenied' +import { WithFilePermissionsGranted } from '../files/file-permission/WithFilePermissionsGranted' const meta: Meta = { title: 'Pages/Dataset', component: Dataset, - decorators: [WithI18next, WithCitationMetadataBlockInfo, WithSettings, WithFilePermissions], + decorators: [WithI18next, WithCitationMetadataBlockInfo, WithSettings], parameters: { // Sets the delay for all stories. chromatic: { delay: 15000, pauseAnimationAtEnd: true } @@ -27,7 +28,7 @@ export default meta type Story = StoryObj export const Default: Story = { - decorators: [WithLayout], + decorators: [WithLayout, WithFilePermissionsDenied], render: () => ( ( ( ( ( ( { + return new Promise((resolve) => { + setTimeout(() => { + resolve(FileUserPermissionsMother.createWithDeniedPermissions()) + }, 1000) + }) + } +} diff --git a/src/stories/files/file-permission/WithFilePermissions.tsx b/src/stories/files/file-permission/WithFilePermissions.tsx deleted file mode 100644 index 35fc8d15c..000000000 --- a/src/stories/files/file-permission/WithFilePermissions.tsx +++ /dev/null @@ -1,11 +0,0 @@ -import { StoryFn } from '@storybook/react' -import { FilePermissionsProvider } from '../../../sections/file/file-permissions/FilePermissionsProvider' -import { FileMockRepository } from '../FileMockRepository' - -export const WithFilePermissions = (Story: StoryFn) => { - return ( - - - - ) -} diff --git a/src/stories/files/file-permission/WithFilePermissionsDenied.tsx b/src/stories/files/file-permission/WithFilePermissionsDenied.tsx new file mode 100644 index 000000000..13d14b870 --- /dev/null +++ b/src/stories/files/file-permission/WithFilePermissionsDenied.tsx @@ -0,0 +1,11 @@ +import { StoryFn } from '@storybook/react' +import { FilePermissionsProvider } from '../../../sections/file/file-permissions/FilePermissionsProvider' +import { FileWithDeniedPermissionsRepository } from '../FileWithDeniedPermissionsRepository' + +export const WithFilePermissionsDenied = (Story: StoryFn) => { + return ( + + + + ) +} diff --git a/tests/component/files/domain/models/FileUserPermissionsMother.ts b/tests/component/files/domain/models/FileUserPermissionsMother.ts index c1f33cd64..0bf6086bd 100644 --- a/tests/component/files/domain/models/FileUserPermissionsMother.ts +++ b/tests/component/files/domain/models/FileUserPermissionsMother.ts @@ -17,4 +17,11 @@ export class FileUserPermissionsMother { canEditDataset: true }) } + + static createWithDeniedPermissions(): FileUserPermissions { + return FileUserPermissionsMother.create({ + canDownloadFile: false, + canEditDataset: false + }) + } } diff --git a/tests/component/sections/dataset/dataset-files/useFiles.spec.tsx b/tests/component/sections/dataset/dataset-files/useFiles.spec.tsx new file mode 100644 index 000000000..50ef7978b --- /dev/null +++ b/tests/component/sections/dataset/dataset-files/useFiles.spec.tsx @@ -0,0 +1,93 @@ +import { FilesCountInfoMother } from '../../../files/domain/models/FilesCountInfoMother' +import { FileMother } from '../../../files/domain/models/FileMother' +import { FileRepository } from '../../../../../src/files/domain/repositories/FileRepository' +import { useFiles } from '../../../../../src/sections/dataset/dataset-files/useFiles' +import { FileUserPermissionsMother } from '../../../files/domain/models/FileUserPermissionsMother' +import { FilePermissionsProvider } from '../../../../../src/sections/file/file-permissions/FilePermissionsProvider' + +const files = FileMother.createMany(100) +const filesCountInfo = FilesCountInfoMother.create({ total: 100 }) +const fileRepository: FileRepository = {} as FileRepository + +const FilesTableTestComponent = ({ datasetPersistentId }: { datasetPersistentId: string }) => { + const { isLoading, files, filesCountInfo } = useFiles(fileRepository, datasetPersistentId) + if (isLoading) { + return Loading... + } + return ( + <> +
Files count: {filesCountInfo.total}
+ + + {files.map((file) => ( + + + + ))} + +
{file.name}
+ + ) +} + +describe('useFiles', () => { + beforeEach(() => { + fileRepository.getAllByDatasetPersistentId = cy.stub().resolves(files) + fileRepository.getCountInfoByDatasetPersistentId = cy.stub().resolves(filesCountInfo) + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: files[0].id })) + }) + + it('returns the files', () => { + cy.customMount() + + cy.findByText('Loading...').should('exist') + cy.findByText('Loading...').should('not.exist') + + cy.findAllByRole('row').should('have.length', 100) + cy.findByText(files[0].name).should('exist') + cy.findByText(files[99].name).should('exist') + }) + + it('returns the files count info', () => { + cy.customMount() + + cy.findByText('Loading...').should('exist') + cy.findByText('Files count: 100').should('exist') + cy.findByText('Loading...').should('not.exist') + }) + + it('calls the file repository', () => { + cy.customMount() + + cy.findByText('Loading...').should('exist') + cy.wrap(fileRepository.getAllByDatasetPersistentId).should('be.calledOnceWith', 'persistentId') + + cy.findByText('Loading...').should('exist') + cy.wrap(fileRepository.getCountInfoByDatasetPersistentId).should( + 'be.calledOnceWith', + 'persistentId' + ) + + cy.findByText('Loading...').should('exist') + cy.findByText('Files count: 100').should('exist') + }) + + it('calls the file repository to get the permissions before removing the loading', () => { + cy.customMount( + + + + ) + + cy.findByText('Loading...').should('exist') + cy.wrap(fileRepository.getAllByDatasetPersistentId).should('be.calledOnceWith', 'persistentId') + + cy.findByText('Loading...').should('exist') + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', files[0].id) + + cy.findByText('Loading...').should('exist') + cy.findByText('Files count: 100').should('exist') + }) +}) diff --git a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx index 08488453d..7f2d2b9f1 100644 --- a/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx +++ b/tests/component/sections/file/file-permissions/useFilePermissions.spec.tsx @@ -10,49 +10,67 @@ import { FilesCountInfoMother } from '../../../files/domain/models/FilesCountInf import { AnonymizedContext } from '../../../../../src/sections/dataset/anonymized/AnonymizedContext' const fileRepository: FileRepository = {} as FileRepository -function DownloadFileTestComponent({ file }: { file: File }) { + +function SavedPermissionsTestComponent({ + file, + permission +}: { + file: File + permission: FilePermission +}) { const { checkSessionUserHasFilePermission } = useFilePermissions() - const [hasFileDownloadPermission, setHasFileDownloadPermission] = useState(false) + const [hasFilePermission, setHasFilePermission] = useState(false) + const [hasFilePermissionAgain, setHasFilePermissionAgain] = useState(false) useEffect(() => { - checkSessionUserHasFilePermission(FilePermission.DOWNLOAD_FILE, file) + checkSessionUserHasFilePermission(permission, file) .then((hasPermission) => { - setHasFileDownloadPermission(hasPermission) + setHasFilePermission(hasPermission) + }) + .then(() => { + return checkSessionUserHasFilePermission(permission, file).then((hasPermission) => { + setHasFilePermissionAgain(hasPermission) + }) }) .catch((error) => { - console.error('There was an error getting the file download permission', error) + console.error('There was an error getting the file permission', error) }) }, [file]) return (
- {hasFileDownloadPermission ? ( - Has download permission + {hasFilePermission ? ( + Has file permission + ) : ( + Does not have file permission + )} + {hasFilePermissionAgain ? ( + Has file permission again ) : ( - Does not have download permission + Does not have file permission again )}
) } -function EditDatasetTestComponent({ file }: { file: File }) { +function TestComponent({ file, permission }: { file: File; permission: FilePermission }) { const { checkSessionUserHasFilePermission } = useFilePermissions() - const [hasEditDatasetPermission, setHasEditDatasetPermission] = useState(false) + const [hasFilePermission, setHasFilePermission] = useState(false) useEffect(() => { - checkSessionUserHasFilePermission(FilePermission.EDIT_DATASET, file) + checkSessionUserHasFilePermission(permission, file) .then((hasPermission) => { - setHasEditDatasetPermission(hasPermission) + setHasFilePermission(hasPermission) }) .catch((error) => { - console.error('There was an error getting the edit dataset permission', error) + console.error('There was an error getting the file permission', error) }) }, [file]) return (
- {hasEditDatasetPermission ? ( - Has edit dataset permission + {hasFilePermission ? ( + Has file permission ) : ( - Does not have edit dataset permission + Does not have file permission )}
) @@ -75,11 +93,11 @@ describe('useFilePermissions', () => { cy.mount( - + ) cy.wrap(fileRepository.getFileUserPermissionsById).should('not.be.called') - cy.findByText('Has download permission').should('exist') + cy.findByText('Has file permission').should('exist') }) it('should call getFileUserPermissionsById when the file is deaccessioned', () => { @@ -90,12 +108,12 @@ describe('useFilePermissions', () => { cy.mount( - + ) cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Has download permission').should('exist') + cy.findByText('Has file permission').should('exist') }) it('should call getFileUserPermissionsById when the file is restricted', () => { @@ -105,12 +123,12 @@ describe('useFilePermissions', () => { .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) cy.mount( - + ) cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Has download permission').should('exist') + cy.findByText('Has file permission').should('exist') }) it('should call getFileUserPermissionsById when the file is public but latest version is restricted', () => { @@ -120,12 +138,12 @@ describe('useFilePermissions', () => { .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) cy.mount( - + ) cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Has download permission').should('exist') + cy.findByText('Has file permission').should('exist') }) it('should call getFileUserPermissionsById when the file is embargoed', () => { @@ -135,12 +153,12 @@ describe('useFilePermissions', () => { .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) cy.mount( - + ) cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Has download permission').should('exist') + cy.findByText('Has file permission').should('exist') }) it('should return false if there is an error in the use case request', () => { @@ -150,28 +168,27 @@ describe('useFilePermissions', () => { .rejects(new Error('There was an error getting the file user permissions')) cy.mount( - + ) cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Does not have download permission').should('exist') + cy.findByText('Does not have file permission').should('exist') }) - it.skip('should use the cached state canDownloadPermissionByFileId the second time the file is being consulted', () => { - // TODO - Implement cache + it('should use the saved state of the permission the second time the file is being consulted', () => { const file = FileMother.createWithRestrictedAccess() fileRepository.getFileUserPermissionsById = cy .stub() .resolves(FileUserPermissionsMother.create({ fileId: file.id, canDownloadFile: true })) cy.mount( - - + ) - cy.findAllByText('Has download permission').should('exist') + cy.findAllByText('Has file permission').should('exist') + cy.findAllByText('Has file permission again').should('exist') cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledOnce') }) @@ -186,14 +203,13 @@ describe('useFilePermissions', () => { cy.mount( - - + ) cy.wrap(fileRepository.getFileUserPermissionsById).should('not.be.called') - cy.findAllByText('Has download permission').should('exist') + cy.findAllByText('Has file permission').should('exist') }) }) @@ -206,12 +222,12 @@ describe('useFilePermissions', () => { cy.mount( - + ) cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Has edit dataset permission').should('exist') + cy.findByText('Has file permission').should('exist') }) it('should return false if there is an error in the use case request', () => { @@ -221,12 +237,28 @@ describe('useFilePermissions', () => { .rejects(new Error('There was an error getting the file user permissions')) cy.mount( - + ) cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledWith', file.id) - cy.findByText('Does not have edit dataset permission').should('exist') + cy.findByText('Does not have file permission').should('exist') + }) + + it('should use the saved state of the edit dataset permission the second time the file is being consulted', () => { + const file = FileMother.createDefault() + fileRepository.getFileUserPermissionsById = cy + .stub() + .resolves(FileUserPermissionsMother.create({ fileId: file.id, canEditDataset: true })) + cy.mount( + + + + ) + + cy.findAllByText('Has file permission').should('exist') + cy.findAllByText('Has file permission again').should('exist') + cy.wrap(fileRepository.getFileUserPermissionsById).should('be.calledOnce') }) }) }) From ee46d318c88faf6ab240fe55f6e1e95e5af388bc Mon Sep 17 00:00:00 2001 From: MellyGray Date: Tue, 19 Sep 2023 10:52:31 +0200 Subject: [PATCH 13/13] fix(Storybook): remove duplicated story after merge --- .../edit-files-menu/EditFilesMenu.stories.tsx | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 src/stories/dataset/dataset-files/files-table/file-actions/file-action-buttons/edit-files-menu/EditFilesMenu.stories.tsx diff --git a/src/stories/dataset/dataset-files/files-table/file-actions/file-action-buttons/edit-files-menu/EditFilesMenu.stories.tsx b/src/stories/dataset/dataset-files/files-table/file-actions/file-action-buttons/edit-files-menu/EditFilesMenu.stories.tsx deleted file mode 100644 index d633dc6e3..000000000 --- a/src/stories/dataset/dataset-files/files-table/file-actions/file-action-buttons/edit-files-menu/EditFilesMenu.stories.tsx +++ /dev/null @@ -1,19 +0,0 @@ -import { Meta, StoryObj } from '@storybook/react' -import { WithI18next } from '../../../../../../WithI18next' -import { WithSettings } from '../../../../../../WithSettings' -import { FileMother } from '../../../../../../../../tests/component/files/domain/models/FileMother' -import { EditFilesMenu } from '../../../../../../../sections/dataset/dataset-files/files-table/file-actions/edit-files-menu/EditFilesMenu' -import { WithLoggedInUser } from '../../../../../../WithLoggedInUser' - -const meta: Meta = { - title: 'Sections/Dataset Page/DatasetFiles/FilesTable/EditFilesMenu', - component: EditFilesMenu, - decorators: [WithI18next, WithSettings, WithLoggedInUser] -} - -export default meta -type Story = StoryObj - -export const Default: Story = { - render: () => -}