Conversation
|
Hi Ellen @ekraffmiller , this PR is ready for review. Please feel free to take a look. I’ve added two questions in the special notes for the reviewer that I’m unsure about. |
There was a problem hiding this comment.
Pull request overview
Adds Guestbook management and Access (guestbook submission for signed downloads) capabilities to the client library, supporting SPA flows that require guestbook responses prior to generating signed download URLs.
Changes:
- Introduces a new
guestbooksmodule with repository + use cases to create, fetch, list, and enable/disable guestbooks. - Introduces a new
accessmodule with repository + use cases to submit guestbook responses for signed download URLs (datafile, datafiles, dataset, dataset version). - Adds unit/integration tests plus public documentation and changelog updates for the new APIs.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/guestbooks/SetGuestbookEnabled.test.ts | Unit coverage for enabling/disabling a guestbook use case. |
| test/unit/guestbooks/GetGuestbooksByDataverseIdentifier.test.ts | Unit coverage for listing guestbooks for a collection. |
| test/unit/guestbooks/GetGuestbook.test.ts | Unit coverage for fetching a single guestbook. |
| test/unit/guestbooks/CreateGuestbook.test.ts | Unit coverage for guestbook creation use case. |
| test/unit/access/SubmitGuestbookDownloads.test.ts | Unit coverage for guestbook-submission download use cases returning signed URLs. |
| test/integration/guestbooks/GuestbooksRepository.test.ts | Integration coverage for guestbooks repository against a real API. |
| test/integration/access/AccessRepository.test.ts | Integration coverage for access repository signed-URL flows. |
| test/environment/.env | Updates Dataverse image registry/tag used by the test environment. |
| src/index.ts | Exposes guestbooks and access modules from the package root. |
| src/guestbooks/infra/repositories/GuestbooksRepository.ts | Implements Guestbooks API calls (create/get/list/enable). |
| src/guestbooks/index.ts | Exports guestbooks use case instances and DTO/model types. |
| src/guestbooks/domain/useCases/SetGuestbookEnabled.ts | Use case wrapper for enabling/disabling guestbooks. |
| src/guestbooks/domain/useCases/GetGuestbooksByCollectionId.ts | Use case wrapper for listing guestbooks in a collection. |
| src/guestbooks/domain/useCases/GetGuestbook.ts | Use case wrapper for fetching a guestbook by id. |
| src/guestbooks/domain/useCases/CreateGuestbook.ts | Use case wrapper for guestbook creation. |
| src/guestbooks/domain/repositories/IGuestbooksRepository.ts | Defines guestbooks repository interface. |
| src/guestbooks/domain/models/Guestbook.ts | Adds guestbook domain model types. |
| src/guestbooks/domain/dtos/CreateGuestbookDTO.ts | Adds DTOs for guestbook creation payloads. |
| src/access/infra/repositories/AccessRepository.ts | Implements /access/* guestbook-submission endpoints returning signed URLs. |
| src/access/index.ts | Exports access use case instances and DTO types. |
| src/access/domain/useCases/SubmitGuestbookForDatasetVersionDownload.ts | Use case wrapper for dataset-version signed download. |
| src/access/domain/useCases/SubmitGuestbookForDatasetDownload.ts | Use case wrapper for dataset signed download. |
| src/access/domain/useCases/SubmitGuestbookForDatafilesDownload.ts | Use case wrapper for multi-file signed download. |
| src/access/domain/useCases/SubmitGuestbookForDatafileDownload.ts | Use case wrapper for single-file signed download. |
| src/access/domain/repositories/IAccessRepository.ts | Defines access repository interface. |
| src/access/domain/dtos/GuestbookResponseDTO.ts | Defines DTO for guestbook response submission payload. |
| docs/useCases.md | Documents the new guestbooks/access use cases and example calls. |
| CHANGELOG.md | Announces the new guestbooks/access capabilities. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/unit/guestbooks/GetGuestbooksByDataverseIdentifier.test.ts
Outdated
Show resolved
Hide resolved
| DATAVERSE_IMAGE_REGISTRY=docker.io | ||
| DATAVERSE_IMAGE_TAG=unstable | ||
| DATAVERSE_IMAGE_REGISTRY=ghcr.io | ||
| DATAVERSE_IMAGE_TAG=12001-api-support-termofuse-guestbook |
There was a problem hiding this comment.
This change pins integration tests to a specific Dataverse image tag (12001-api-support-termofuse-guestbook) and registry. Since this looks temporary (noted in the PR description), it will make tests fragile once that tag disappears. Consider reverting to the default tag/registry (or making the tag overridable via a separate local-only env file) before merging.
| DATAVERSE_IMAGE_TAG=12001-api-support-termofuse-guestbook | |
| DATAVERSE_IMAGE_TAG=latest |
| submitGuestbookForDatasetVersionDownload | ||
| .execute(10, ':latest', { | ||
| guestbookResponse: { answers: [{ id: 123, value: 'Good' }] } | ||
| }) |
There was a problem hiding this comment.
This example also omits the required guestbookResponse nesting (GuestbookResponseDTO expects { guestbookResponse: { answers: [...] } }). Align it with the DTO and the working “Datafile Download” example above.
What this PR does / why we need it:
Adds support for Guestbook-related access flows needed for signed downloads (signed=true) via the new Access API endpoints.
Add Guestbook use cases and repository methods for creating, retrieving, listing, and enabling/disabling guestbooks.
Which issue(s) this PR closes:
Related Dataverse PRs:
Special notes for your reviewer:
I introduced an AccessRepository because these endpoints are grouped under
/api/access/*. I’m open to feedback on architecture: we could moveSubmitGuestbookForDatafileDownloaduse cases under guestbooks instead, if that is preferred.For
signed=false(which acts the same as when signed is omitted), the current frontend download flow is URL-based: it builds the download URL, the browser issues a direct GET to the backend, and the browser handles the file download automatically (see FileJSDataverseRepository.ts (line 329)). We put the browser-triggered download flow in the frontend UI repo, so this PR does not add use cases for path ifsigned=false.However, the question is whether
signed=falseshould now have use cases if it requires a guestbookResponse. If guestbook submission is required, this is no longer a pure browser-triggered download flow and we should likely model it explicitly with use case..env should be change back once the dataverse pr is ready
Is there a release notes or changelog update needed for this change?:
yes
Additional documentation: