Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add use case to get all datasets previews with pagination #107

Merged
merged 15 commits into from
Dec 22, 2023

Conversation

GPortas
Copy link
Contributor

@GPortas GPortas commented Dec 19, 2023

What this PR does / why we need it:

Adds use case to get all dataset previews of the installation.

Which issue(s) this PR closes:

Special notes for your reviewer:

The integration testing environment setup has been modified, now creating the test datasets at that point, to ensure that they are correctly created and indexed in Solr when running the integration tests and that there are no race conditions between the tests, since for getAllDatasetPreviews the total number of retrieved datasets is checked.

Suggestions on how to test this:

Visual inspection and execute integration tests.

Is there a release notes update needed for this change?:

N/A

Additional documentation:

None

@GPortas GPortas changed the title Add use case to get all datasets Add use case to get all datasets previews Dec 19, 2023
@GPortas GPortas marked this pull request as ready for review December 20, 2023 13:18
@GPortas GPortas added the Size: 3 A percentage of a sprint. 2.1 hours. label Dec 20, 2023
@GPortas GPortas changed the title Add use case to get all datasets previews Add use case to get all datasets previews with pagination Dec 20, 2023
@MellyGray MellyGray self-assigned this Dec 20, 2023
Copy link
Contributor

@MellyGray MellyGray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use case looks good!

I think we're going to have everything that we need to integrate it with the frontend except that we need an use case to get the total count if we want the pagination to work

I left some comments

src/datasets/domain/useCases/GetAllDatasetPreviews.ts Outdated Show resolved Hide resolved
test/testHelpers/datasets/datasetHelper.ts Show resolved Hide resolved
test/testHelpers/datasets/datasetLockHelper.ts Outdated Show resolved Hide resolved
test/testHelpers/datasets/datasetPreviewHelper.ts Outdated Show resolved Hide resolved
test/integration/environment/setup.js Show resolved Hide resolved
@MellyGray MellyGray assigned GPortas and unassigned MellyGray Dec 20, 2023
Comment on lines +1 to +6
import { DatasetPreview } from './DatasetPreview';

export interface DatasetPreviewSubset {
datasetPreviews: DatasetPreview[];
totalDatasetCount: number;
}
Copy link
Contributor

@MellyGray MellyGray Dec 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that for the sake of consistency within the module, getDatasetFiles should also return FilesSubset, including files and the count information

This would ensure that the module's responses are predictable. Users of the module would then know that if they use some getAllItems use case, the response will be paginated and will include both the items and their count

I think that the module use cases shouldn't be coupled to the type of API used and how they are implemented, (Native API or Search API) and the module should be consistent even if each API works differently

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, but I wouldn't make this change now as I would first extend the files API to return the total files count along with the files to avoid making two API calls per use case call. We can create two new issues for this: Files API extension + files use case extension.

Copy link
Contributor Author

@GPortas GPortas Dec 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create use case to get all the datasets in a collection
2 participants