-
Notifications
You must be signed in to change notification settings - Fork 10
Edit Terms: Use Case Update Terms of Access #389
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
Conversation
This reverts commit cfe34ea.
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.
Pull Request Overview
This PR adds functionality to update the terms of access for restricted files in a dataset, enabling users to modify fields such as fileAccessRequest, termsOfAccessForRestrictedFiles, and other access-related properties.
Key changes:
- Implemented
UpdateTermsOfAccessuse case with supporting repository methods and transformers - Added comprehensive test coverage (unit, integration, and functional tests)
- Updated documentation with usage examples and important notes about behavior with published datasets
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/datasets/domain/useCases/UpdateTermsOfAccess.ts | New use case implementing the update terms of access functionality |
| src/datasets/domain/repositories/IDatasetsRepository.ts | Added interface method for updateTermsOfAccess |
| src/datasets/infra/repositories/DatasetsRepository.ts | Implemented repository method with API endpoint integration |
| src/datasets/infra/repositories/transformers/termsOfAccessTransformers.ts | New transformer to convert TermsOfAccess model to API payload format |
| src/datasets/domain/models/Dataset.ts | Added blank line for formatting |
| src/datasets/index.ts | Exported new updateTermsOfAccess use case |
| test/unit/datasets/UpdateTermsOfAccess.test.ts | Unit tests for the use case |
| test/integration/datasets/DatasetsRepository.test.ts | Integration tests covering various scenarios including published datasets |
| test/functional/datasets/UpdateTermsOfAccess.test.ts | Functional tests validating end-to-end behavior |
| docs/useCases.md | Added documentation with usage examples and behavioral notes |
| src/datasets/domain/useCases/SetAvailableLicensesForDatasetType.ts | Added JSDoc parameters documentation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…se-client-javascript into updateTermsOfAccess
ekraffmiller
left a comment
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.
Hi @ChengShi-1 it looks good, I just have a question about the TermsOfAccessInputType
|
|
||
| type TermsOfAccessInput = TermsOfAccess & { termsOfAccess?: string } | ||
|
|
||
| export const transformTermsOfAccessToUpdatePayload = (terms: TermsOfAccessInput) => { |
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.
Why does this method need TermsOfAccessInput type, since it is only passed a TermsOfAccess type? Is it meant to be called somewhere else?
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.
Good catch. It is not used somewhere else, so I remove it.
ekraffmiller
left a comment
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, approved
|
test integration failing |
test fixed. |
|
@ekraffmiller Could you help to merge this? Thanks! |
|
@ChengShi-1 I'm sorry I didn't see your message! Thanks for doing the merge 👍 |
What this PR does / why we need it:
Inspired from Edit terms page, related to 2025 Q3 features.
Adds an api to allow the user to update the terms of access for restricted files of a given dataset
Which issue(s) this PR closes:
Related Dataverse PRs:
Special notes for your reviewer:
(included them in usecases.md)
fileAccessRequestwill update that field and leave all other terms absent (undefined). In practice, the new values you send fully replace the previous set of terms — so if you omit a field, you are effectively clearing it unless you include its original value in the new input.Suggestions on how to test this:
Is there a release notes or changelog update needed for this change?:
yes