-
Notifications
You must be signed in to change notification settings - Fork 4
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
DD-1605 Implement several dd-dataverse-cli dataset commands #4
DD-1605 Implement several dd-dataverse-cli dataset commands #4
Conversation
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.
See my comment.
|
||
static class AssignmentCommand { | ||
static class AssignmentAction { | ||
@Option(names = "add", description = "add role assignment to specified dataset(s)'") |
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.
This hack is unnecessary. The picocli library supports subcommands. add
, remove
and list
are not options but subcommands and should be implemented that way.
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.
The Role Assignment commands are separated in sub-commands now.
See also 'Description of changes' and 'How to test'
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.
@janvanmansum If you are satisfied with the changes, you could close this conversation and accept the PR.
…ole Assignment commands separated as subcommands
…everal-dataset-commnads
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
…rged Test Units From Joke
src/main/java/nl/knaw/dans/dvcli/command/CollectionAssignRole.java
Outdated
Show resolved
Hide resolved
src/main/java/nl/knaw/dans/dvcli/command/DatasetAssignRole.java
Outdated
Show resolved
Hide resolved
src/main/java/nl/knaw/dans/dvcli/command/DatasetDeleteRole.java
Outdated
Show resolved
Hide resolved
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 like remove-role is not implemented, does that need to be fixed in this PR?
Personally I like tools that allow removal of X when they have an add X, also makes testing easier ...
Did you test all those command on the commandline, to see if the CLI handling is correct and params are passed to the API as expected.
Or maybe it is usefull to unit test this by mocking the API
remove
Role Assignment is also implemented in as sub-command of role-assignment
.
deleteRoleAssignment
is also implemented for collection and dataset in dans-dataverse-client-lib
.
|
… changes for commnets of Paul
src/main/java/nl/knaw/dans/dvcli/command/DatasetAssignRole.java
Outdated
Show resolved
Hide resolved
src/main/java/nl/knaw/dans/dvcli/command/AbstractAssignmentRole.java
Outdated
Show resolved
Hide resolved
…anges for PR reviewers
…everal-dataset-commnads
generic AbstractAssignmentRole
…et-commnads' into DD-1605-implement-several-dataset-commnads
…oleAssignmnet command with its subcommands
…everal-dataset-commnads
…subcommand remove for RoleAssignmnet command (collection and dataset)
@aliassheikh , @PaulBoon , @jo-pol : I am merging this now, but will be merging a large refactoring as well, later today. There are some good ideas in this PR, but on the other hand some unwarranted complexity. |
Fixes DD-1605 Implement several dd-dataverse-cli dataset commands
Description of changes
Most recent changes:
truncate-notifications
validate-files
deleteRoleAssignment
functions for collection and dataset indans-dataverse-client-lib
role-assignment
is now same for both collection and dataset.role-assignment
has sub-commands:list
,add
andremove
.other changes:
CollectionAssignRole
class: assignment.cvs should have the columns: PID, ASSIGNEE, ROLE like inDatasetAssignRole
.How to test
dataverse dataset pid delete-draft [-hV]
dataverse dataset pid delete-draft
dataverse dataset get-files [-hV] version
dataverse dataset pid version 01
dataverse dataset pid get-latest-version [-hV]
dataverse dataset pid get-latest-version
dataverse dataset pid get-version [-hV] (<version> | --all)
dataverse dataset pid get-version 01
dataverse dataset pid get-version --all
dataverse dataset pid publish [-hV] [[-a] [--major | --minor]]
dataverse dataset pid publish --major
dataverse dataset pid publish --minor
dataverse dataset pid publish --minor -a=true
dataverse dataset pid publish --major -a=false
dataverse dataset pid publish -a=true
Test an invalid account name:
Related PRs
(Add links)
*
Notify
@DANS-KNAW/core-systems