-
Notifications
You must be signed in to change notification settings - Fork 8
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
GetCollectionItems use case, for returning dataset, collection and file item types #181
Conversation
… into 170-file-collection-results
Waiting for IQSS/dataverse#10810 |
Waiting for IQSS/dataverse#10810 to be approved. |
Thanks @ekraffmiller. We need to extend the API to add those missing properties. Not a big issue, but something to work on before updating this PR: IQSS/dataverse#10846 Adding the waiting tag. |
@ekraffmiller I have already implemented the remaining changes. I thought it was necessary to extend the API, but the properties were already being returned. |
@GPortas sorry for not realizing this before, something crucial; we are going to need in each preview object (Collection, Dataset and File), the type of collection item it belongs to, so we can easily know in the SPA what type of preview object we are reading and display the correct card component. |
No problem. I just added it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just one comment about the integration test.
expect(actualDatasetPreview.versionInfo.lastUpdateTime).not.toBeUndefined() | ||
expect(actualDatasetPreview.versionInfo.majorNumber).toBeUndefined() | ||
expect(actualDatasetPreview.versionInfo.minorNumber).toBeUndefined() | ||
expect(actualDatasetPreview.versionInfo.state).toBe('DRAFT') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GPortas I think a test of actualDatasetPreview
parentName
and parentAlias
can be added here?
Edit: Also need a test of type
for the Preview objects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! I just added those assertions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me!
What this PR does / why we need it:
Adds GetCollectionItems use case, for returning dataset, collection and file item types. Includes search text and type filtering option.
Which issue(s) this PR closes:
Related Dataverse PRs:
Special notes for your reviewer:
I have reduced branch test coverage from 95% to 90%.
The pre-commit hook was failing because branch coverage was not being reached in classes where there should be branch coverage, and it was showing 0%. I have created an issue to address this problem separately: #186
In a future iteration we will include collection type results.
Suggestions on how to test this:
Visual inspection of the code and github actions.