Use Case for Getting Available Dataset Types#366
Conversation
There was a problem hiding this comment.
I made this a functional test but I'd be happy to switch it to an integration test if that's more appropriate. I'm confused about which to use. They seem quite similar, both spinning up Docker containers. On my machine functional is faster, clocking in at around 20 seconds while integration takes almost 50 seconds.
There was a problem hiding this comment.
It's good to keep a functional test here! Could you also add to integration test as well to test/integration/datasets/DatasetsRepository.test.ts?
There was a problem hiding this comment.
@ChengShi-1 I know in person I said I'd delete the functional test and add an integration test, but after looking at the code I decided to keep the functional test and add an integration test, which I did in a49c004.
My reasoning was that I like how easy it is to run just the one test I added like this (because it's in its own file):
npm run test:functional -- test/functional/datasets/GetDatasetsAvailableDatasetTypes.test.ts
On the integration side, tests are all in one file (DatasetsRepository.test.ts). After adding a test, I tried to run just that test like this...
npm run test:integration DatasetsRepository.test.ts -t "getDatasetAvailableDatasetTypes"
... but it didn't work. All tests in that file ran.
ChengShi-1
left a comment
ChengShi-1
left a comment
There was a problem hiding this comment.
I left some comments, but good job overall👍
docs/useCases.md
Outdated
|
|
||
| /* ... */ | ||
|
|
||
| getDatasetAvailableDatasetTypes.execute().then((categories: String[]) => { |
There was a problem hiding this comment.
Why there is a categories (404 copy-paste error ?
There was a problem hiding this comment.
Whoops! Yes, this was a copy/paste error. Fixed in e4f09f9.
Conflicts: docs/useCases.md src/datasets/domain/repositories/IDatasetsRepository.ts src/datasets/index.ts src/datasets/infra/repositories/DatasetsRepository.ts test/integration/datasets/DatasetsRepository.test.ts
ChengShi-1
left a comment
ChengShi-1
left a comment
There was a problem hiding this comment.
Looks good to me! Great work on js-dataverse!
github-project-automation
bot
moved this from In Review 🔎
to Ready for QA ⏩
in IQSS Dataverse Project
|
tests are passing - merging. |
github-project-automation
bot
moved this from Ready for QA ⏩
to Merged 🚀
in IQSS Dataverse Project
4 participants
What this PR does / why we need it:
This PR allows for listing the available dataset types (docs) when creating a dataset. We're thinking about using a "review" dataset type in the context of Trusted Data.
Which issue(s) this PR closes:
Related Dataverse PRs:
It's a long story. I have opened a few PRs but after speaking with @ChengShi-1 it sounds like making the "get" PR (this one) first makes the most sense.
The next PR would setting the dataset type. I created the following PR for this but I'll probably create a fresh one that builds on this "get" PR:
Special notes for your reviewer:
This PR is modeled off getDatasetAvailableCategories and getAvailableStandardLicenses.
I'll leave comments under "files changed" for specific lines of code.
Suggestions on how to test this:
Run the tests.
Is there a release notes update needed for this change?:
Yes, something like this:
"You can list available dataset types."
Additional documentation:
Included.