Extend get guestbooks use case with stats and Download responses#449
Conversation
There was a problem hiding this comment.
Pull request overview
Adds optional guestbook usage/response statistics support to the “get guestbooks by collection” flow, aligning the client with the Dataverse API enhancement needed for the Manage Guestbooks UI.
Changes:
- Extended
getGuestbooksByCollectionIdto accept an optionalincludeStatsflag and send it as a query parameter when enabled. - Updated the
Guestbookdomain model to optionally exposeusageCountandresponseCount. - Added/updated unit + integration tests, plus documentation and changelog entries.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/unit/guestbooks/GuestbooksRepository.test.ts | New unit coverage for repository query param behavior and error handling. |
| test/unit/guestbooks/GetGuestbooksByCollectionId.test.ts | Adds unit coverage for the use case passing includeStats. |
| test/integration/guestbooks/GuestbooksRepository.test.ts | Adds integration coverage for listing stats and verifying count changes via assignment + guest response submission. |
| src/guestbooks/infra/repositories/GuestbooksRepository.ts | Adds includeStats param and forwards it as query params when true. |
| src/guestbooks/domain/useCases/GetGuestbooksByCollectionId.ts | Extends use case API to accept includeStats. |
| src/guestbooks/domain/repositories/IGuestbooksRepository.ts | Updates repository interface to include optional includeStats. |
| src/guestbooks/domain/models/Guestbook.ts | Adds optional usageCount and responseCount fields. |
| docs/useCases.md | Documents the new includeStats parameter and updates example usage. |
| CHANGELOG.md | Adds an Unreleased entry describing the new includeStats support. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ekraffmiller
left a comment
There was a problem hiding this comment.
Hi @ChengShi-1 I left some comments on the PR, hope it's not too confusing! After going back on forth between the use cases and the API, I think there is actually one more change we need in the API - which I describe in my comment in GetGuestbookResponsesOfAGuestbook. I'm happy to discuss more about it.
| /** | ||
| * Returns guestbook responses for one guestbook in a dataverse collection. | ||
| * | ||
| * @param {number | string} dataverseId - Dataverse identifier. |
There was a problem hiding this comment.
The API for getting the responses for a guestbook in JSON form is, for example:
curl -H "X-Dataverse-key:$API_TOKEN" "$SERVER_URL/api/guestbooks/$ID/responses?limit10&offset=0"
So we need to add parameters for limit and offset.
Another thing I noticed though, is that this API doesn't allow us to pass a dataverseId, so it's going to return all guestbook responses regardless of collection. So unfortunately I think we to ask for another change to the API - to be able to pass guestbookId and collectionId and get all the responses for a guestbook within a specific collection, returned in JSON format.
There was a problem hiding this comment.
I created an issue in dataverse for the api missing IQSS/dataverse#12451
| import { GuestbookResponse } from '../models/GuestbookResponse' | ||
| import { IGuestbooksRepository } from '../repositories/IGuestbooksRepository' | ||
|
|
||
| export class GetGuestbookResponsesByDataverseId implements UseCase<GuestbookResponse[]> { |
There was a problem hiding this comment.
I don't think this use case is needed, we only need to download the csv version of the responses, we don't need an array of JSON for displaying.
ekraffmiller
left a comment
There was a problem hiding this comment.
Looks good! just need to update the return type of GetGuestbookResponsesByGuestbookId to support pagination.
| const limit = 10 | ||
| const offset = 0 | ||
|
|
||
| getGuestbookResponsesByGuestbookId |
There was a problem hiding this comment.
The API will return pagination info as well, so the return type should be GustbookSubset, containing the array of Guestbook items, and the total count, similar to how we handle pagination for other objects
There was a problem hiding this comment.
Added. Thanks for catching this😃
ekraffmiller
left a comment
There was a problem hiding this comment.
thanks for the updates, looks good!
What this PR does / why we need it:
includeStatsandincludeInheritedsupport togetGuestbooksByCollectionId.Which issue(s) this PR closes:
Related Dataverse PRs:
Special notes for your reviewer:
Test failed about storage drive, so I also copy pastes a
CollectionHelper.tsfrom this PR. waiting to be mergedhttps://github.com/IQSS/dataverse-client-javascript/pull/431/changes#diff-edac108c1a43d5773116d19797d04ecdf890fc97b5cf83ce2d56d38b9a7aca18
Suggestions on how to test this:
Is there a release notes or changelog update needed for this change?:
Additional documentation: