Skip to content

Commit

Permalink
fix: useFiles to prevent unnecessary calls to the use case
Browse files Browse the repository at this point in the history
  • Loading branch information
MellyGray committed Aug 30, 2023
1 parent 769b8b2 commit cfdbd29
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 24 deletions.
1 change: 0 additions & 1 deletion src/files/infrastructure/FileJSDataverseRepository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ 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'
import {
getDatasetFiles,
getFileDownloadCount,
Expand Down
2 changes: 1 addition & 1 deletion src/sections/dataset/dataset-files/useFiles.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function useFiles(
useEffect(() => {
setIsLoading(true)

if (filesCountInfo.total !== 0) {
if (filesCountInfo.total !== 0 && paginationInfo?.total !== 0) {
getFilesByDatasetPersistentId(
filesRepository,
datasetPersistentId,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { FileInfoHeader } from '../../../../../../../src/sections/dataset/dataset-files/files-table/file-info/FileInfoHeader'

describe('FileInfoHeader', () => {
it('renders the file info header', () => {
cy.customMount(<FileInfoHeader pageCount={1} pageIndex={0} pageSize={100} />)

cy.findByText('1 to 100 of 100 Files').should('exist')
})

it('does not render the file info header when there are no files', () => {
cy.customMount(<FileInfoHeader pageCount={0} pageIndex={0} pageSize={100} />)

cy.findByText('1 to 100 of 100 Files').should('not.exist')
})
})
39 changes: 32 additions & 7 deletions tests/component/sections/dataset/dataset-files/useFiles.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,31 @@ import { FileRepository } from '../../../../../src/files/domain/repositories/Fil
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'
import { DatasetMother } from '../../../dataset/domain/models/DatasetMother'
import { useEffect, useState } from 'react'
import { FilePaginationInfo } from '../../../../../src/files/domain/models/FilePaginationInfo'
import {
DatasetPublishingStatus,
DatasetVersion
} from '../../../../../src/dataset/domain/models/Dataset'

const files = FileMother.createMany(100)
const filesCountInfo = FilesCountInfoMother.create({ total: 100 })
const fileRepository: FileRepository = {} as FileRepository
const datasetVersion = DatasetMother.create().version
const datasetVersion = new DatasetVersion(1, DatasetPublishingStatus.RELEASED, 1, 0)

const FilesTableTestComponent = ({ datasetPersistentId }: { datasetPersistentId: string }) => {
const [paginationInfo, setPaginationInfo] = useState<FilePaginationInfo>(new FilePaginationInfo())
const { isLoading, files, filesCountInfo } = useFiles(
fileRepository,
datasetPersistentId,
datasetVersion
datasetVersion,
paginationInfo
)

useEffect(() => {
setPaginationInfo(paginationInfo.withTotal(filesCountInfo.total))
}, [filesCountInfo])

if (isLoading) {
return <span>Loading...</span>
}
Expand Down Expand Up @@ -67,20 +79,33 @@ describe('useFiles', () => {
it('calls the file repository', () => {
cy.customMount(<FilesTableTestComponent datasetPersistentId="persistentId" />)

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.wrap(fileRepository.getAllByDatasetPersistentId).should(
'be.calledOnceWith',
'persistentId',
datasetVersion,
new FilePaginationInfo(1, 10, 100),
undefined
)

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', () => {
const files = FileMother.createMany(5)
fileRepository.getAllByDatasetPersistentId = cy.stub().resolves(files)
fileRepository.getFileUserPermissionsById = cy.stub().resolves(
new Promise((resolve) => {
setTimeout(() => {
resolve(FileUserPermissionsMother.create({ fileId: files[0].id }))
}, 1000)
})
)

cy.customMount(
<FilePermissionsProvider repository={fileRepository}>
<FilesTableTestComponent datasetPersistentId="persistentId" />
Expand Down
28 changes: 14 additions & 14 deletions tests/e2e-integration/e2e/sections/dataset/Dataset.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('Dataset', () => {
it('successfully loads a dataset in draft mode', () => {
cy.wrap(DatasetHelper.create())
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.visit(`/spa/datasets?persistentId=${persistentId}`)

cy.fixture('dataset-finch1.json').then((dataset: Dataset) => {
Expand All @@ -38,7 +38,7 @@ describe('Dataset', () => {
it('loads page not found when the user is not authenticated and tries to access a draft', () => {
cy.wrap(DatasetHelper.create())
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.wrap(TestsUtils.logout())
cy.visit(`/spa/datasets?persistentId=${persistentId}`)

Expand All @@ -49,7 +49,7 @@ describe('Dataset', () => {
it('successfully loads a dataset when passing the id and version', () => {
cy.wrap(DatasetHelper.create().then((dataset) => DatasetHelper.publish(dataset.persistentId)))
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.wait(1500)
cy.visit(`/spa/datasets?persistentId=${persistentId}&version=1.0`)

Expand All @@ -67,7 +67,7 @@ describe('Dataset', () => {
it('loads the latest version of the dataset when passing a wrong version', () => {
cy.wrap(DatasetHelper.create().then((dataset) => DatasetHelper.publish(dataset.persistentId)))
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.visit(`/spa/datasets?persistentId=${persistentId}&version=2.0`)

cy.fixture('dataset-finch1.json').then((dataset: Dataset) => {
Expand All @@ -89,7 +89,7 @@ describe('Dataset', () => {
it('successfully loads a dataset using a privateUrlToken', () => {
cy.wrap(DatasetHelper.create().then((dataset) => DatasetHelper.createPrivateUrl(dataset.id)))
.its('token')
.then((token) => {
.then((token: string) => {
cy.visit(`/spa/datasets?privateUrlToken=${token}`)

cy.fixture('dataset-finch1.json').then((dataset: Dataset) => {
Expand All @@ -109,7 +109,7 @@ describe('Dataset', () => {
)
)
.its('token')
.then((token) => {
.then((token: string) => {
cy.visit(`/spa/datasets?privateUrlToken=${token}`)

cy.fixture('dataset-finch1.json').then((dataset: Dataset) => {
Expand All @@ -133,7 +133,7 @@ describe('Dataset', () => {
it('successfully loads the files tab', () => {
cy.wrap(DatasetHelper.create())
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.visit(`/spa/datasets?persistentId=${persistentId}`)

cy.findByText('Files').should('exist')
Expand All @@ -145,7 +145,7 @@ describe('Dataset', () => {
it('successfully loads the files tab with files', () => {
cy.wrap(DatasetHelper.createWithFiles(3))
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.visit(`/spa/datasets?persistentId=${persistentId}`)

cy.findByText('Files').should('exist')
Expand All @@ -161,7 +161,7 @@ describe('Dataset', () => {
// TODO - Connect files count implementation to the pagination
cy.wrap(DatasetHelper.createWithFiles(20), { timeout: 10000 })
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.visit(`/spa/datasets?persistentId=${persistentId}`)

cy.findByText('Files').should('exist')
Expand All @@ -181,7 +181,7 @@ describe('Dataset', () => {
it('successfully loads the action buttons when the user is logged in as owner', () => {
cy.wrap(DatasetHelper.createWithFiles(3))
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.visit(`/spa/datasets?persistentId=${persistentId}`)

cy.findByText('Files').should('exist')
Expand All @@ -200,7 +200,7 @@ describe('Dataset', () => {
)
)
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.wrap(TestsUtils.logout())

cy.visit(`/spa/datasets?persistentId=${persistentId}`)
Expand All @@ -217,7 +217,7 @@ describe('Dataset', () => {
it('loads the restricted files when the user is logged in as owner', () => {
cy.wrap(DatasetHelper.createWithFilesRestricted(1))
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.visit(`/spa/datasets?persistentId=${persistentId}`)

cy.findByText('Files').should('exist')
Expand All @@ -240,7 +240,7 @@ describe('Dataset', () => {
)
)
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.wrap(TestsUtils.logout())

cy.visit(`/spa/datasets?persistentId=${persistentId}`)
Expand All @@ -266,7 +266,7 @@ describe('Dataset', () => {
)
)
.its('persistentId')
.then((persistentId) => {
.then((persistentId: string) => {
cy.visit(`/spa/datasets?persistentId=${persistentId}`)

cy.findByText('Files').should('exist')
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e-integration/shared/datasets/DatasetHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class DatasetHelper extends DataverseApiHelper {
filesIds: number[],
embargoDate: string
): Promise<DatasetResponse> {
const response = this.request<DatasetResponse>(
const response = await this.request<DatasetResponse>(
`/datasets/:persistentId/files/actions/:set-embargo?persistentId=${persistentId}`,
'POST',
{ fileIds: filesIds, dateAvailable: embargoDate, reason: 'Standard project embargo' }
Expand Down

0 comments on commit cfdbd29

Please sign in to comment.