search

IQSS / dataverse-client-javascript

A Dataverse client for JavaScript and TypeScript
MIT License
16 stars 8 forks source link

109 - Add totalFilesCount to GetDatasetFiles use case #116

Closed MellyGray closed 9 months ago

MellyGray commented 9 months ago

What this PR does / why we need it:

This PR introduces totalFilesCount to the GetDatasetFiles use case, which is essential for pagination. This ensures consistency with the pagination implementation, as seen in DatasetPreviewSubset.

Which issue(s) this PR closes:

Related Dataverse PRs:

Special notes for your reviewer:

The implementation should be consistent with GetAllDatasetPreviews implementation, so just compare the code with that use case

Suggestions on how to test this:

Run integration tests or review GitHub action execution

MellyGray commented 9 months ago

Looks good.

We need also to export the FilesSubset model in the index.ts file of the files folder, just as we do with the DatasetPreviewSubset

I addressed the change. But I'm worried about what would had happened if you hadn't noticed. I think that somehow we need some tests to test the package from the outside. To force the check on these index exports, and also ensure that the package functions properly when used as a library

GPortas commented 9 months ago

Looks good. We need also to export the FilesSubset model in the index.ts file of the files folder, just as we do with the DatasetPreviewSubset

I addressed the change. But I'm worried about what would had happened if you hadn't noticed. I think that somehow we need some tests to test the package from the outside. To force the check on these index exports, and also ensure that the package functions properly when used as a library

Yes. There is an open issue related to functional tests that would be useful for what you mention: https://github.com/IQSS/dataverse-client-javascript/issues/81

I just updated the issue to link to this comment.