CLOUDP-57841: Delete dbusers C/OM #57
Conversation
| @@ -58,7 +59,7 @@ func (opts *atlasDBUsersCreateOpts) Run() error { | |||
|
|
|||
| func (opts *atlasDBUsersCreateOpts) newDatabaseUser() *atlas.DatabaseUser { | |||
| return &atlas.DatabaseUser{ | |||
| DatabaseName: convert.AdminDB, | |||
| DatabaseName: opts.authDB, | |||
There was a problem hiding this comment.
we missed this in the last PR
| @@ -24,7 +25,7 @@ import ( | |||
|
|
|||
| const ( | |||
| fallbackSuccessMessage = "'%s' deleted\n" | |||
| fallbackFailMessage = "'%s' not deleted\n" | |||
| fallbackFailMessage = "entry not deleted" | |||
There was a problem hiding this comment.
other fail messages don't interpolate so making the fall back compatible
| // that should perform the deletion from the store. | ||
| func (opts *deleteOpts) DeleteFromProject(d DeleterFromProject, projectID string) error { | ||
| func (opts *deleteOpts) Delete(d interface{}, a ...string) error { |
There was a problem hiding this comment.
this function can do now all three functions
| var err error | ||
| fmt.Printf("%#v", d) | ||
| switch f := d.(type) { | ||
| case func(string) error: |
There was a problem hiding this comment.
for some reason I could not use types here so getting rid of them
| func (opts *deleteOpts) SuccessMessage() string { | ||
| if opts.successMessage != "" { | ||
| return opts.successMessage | ||
| } | ||
| return fallbackSuccessMessage | ||
| } | ||
|
|
||
| // FailMessage gets the set fail message or the default value | ||
| func (opts *deleteOpts) FailMessage() string { | ||
| if opts.failMessage != "" { | ||
| return opts.failMessage | ||
| } | ||
| return fallbackFailMessage | ||
| } |
There was a problem hiding this comment.
making this just getters and leaving the printing out
| @@ -0,0 +1,99 @@ | |||
| package convert | |||
There was a problem hiding this comment.
we were missing tests for this
| return nil | ||
| var err error | ||
| fmt.Printf("%#v", d) | ||
| switch f := d.(type) { |
| if err := opts.init(); err != nil { | ||
| return err | ||
| } | ||
| opts.entry = args[0] |
There was a problem hiding this comment.
[qa] Do you think it is better to set the arg inside PreRun or Run?. I am quite sure that we are setting that arg inside Run in other commands. We should decide one place and change all the other commands in accordance with that decision. I honestly think that the preRun would be a better place 👍
There was a problem hiding this comment.
Yeah we definitely need to agree on this, I've moved it around based on need, the prompt makes sense to run on the pre run so we need the entry set before prompting, but some validations don't run before preRun, I know flags don't do it but I can't remember if the number of args validation does, if we can confirm it does then we should move them all to a preRun
|
@andreaangiolillo RFAL |
…commands (mongodb#56)" (mongodb#57) This reverts commit c0d9f1d.
Proposed changes
Jira ticket: CLOUDP-57841
Checklist
make fmtand formatted my code