-
Notifications
You must be signed in to change notification settings - Fork 50
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
SM-417: Add project create, update, and delete commands #53
Conversation
…d changes from master
Looks like a great start, thanks @coltonhurst! A small thing missing, we have to add the new create, update, delete commands to the big command struct here: https://github.com/bitwarden/sdk/blob/9e2e04c1b3a81505c1daeaab85486df1359bad9d/crates/bitwarden/src/sdk/request/command.rs#L120-L127 And then use it here: That way the new commands can be used both directly with the rust crate but also from the JSON wrapper and all the language bindings. |
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.
Two minor comments, looks good otherwise!
|
||
#[derive(Serialize, Deserialize, Debug, JsonSchema)] | ||
#[serde(rename_all = "camelCase", deny_unknown_fields)] | ||
pub struct ProjectPutRequest { |
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.
Should this be called updateRequest? Put
feels like an implementation detail.
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 was to follow the convention in secrets_request.rs which uses SecretPutRequest
. We definitely can refactor out Put
in multiple locations and use Update
though. This could be done in a different ticket, this one, or we take a different approach?
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.
Discussed this, we will address this in a separate PR
let enc = client | ||
.get_encryption_settings() | ||
.as_ref() | ||
.ok_or(Error::VaultLocked)?; |
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.
Isn't enc
already defined above? I think we can reuse it.
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.
Hmm, any recommendations on a better way to do this? By removing it we break one of the borrowing rules. With it removed we have the following error, the compiler delineates the following:
- immutable borrow on line 35/36
- mutable borrow on line 46
- immutable borrow on line 54
cannot borrow *client as mutable because it is also borrowed as immutable
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.
Discussed with @dani-garcia, addressing this in this PR: #70
let enc = client | ||
.get_encryption_settings() | ||
.as_ref() | ||
.ok_or(Error::VaultLocked)?; |
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.
Same
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.
Same as here
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.
Update here
Type of change
Objective
Adds the ability to create, update, and delete projects via the Bitwarden SDK.
This was originally under the SM-417 internal ticket, but is now more specifically represented by the child SM-783 internal ticket.
Code changes
create
,update
, anddelete
commands for matching in the run functioncreate
,update
, anddelete
functionscreate_project
,update_project
, anddelete_project
project commandscreate
,update
, anddelete
commands to the ProjectsCommand enumProjectCreateRequest
,ProjectPutRequest
, and theProjectsDeleteRequest
structs that are the request modelsProjectsDeleteResponse
andProjectDeleteResponse
structs that are the response modelsAll other schema update file changes were generated by
npm run schemas
.Before you submit