Skip to content
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

CLOUDP-70015: mongocli iam org(s) user(s) list|ls #393

Merged
merged 7 commits into from
Sep 2, 2020

Conversation

andreaangiolillo
Copy link
Collaborator

@andreaangiolillo andreaangiolillo commented Sep 1, 2020

Proposed changes

Jira ticket: CLOUDP-70015

Checklist

  • I have signed the MongoDB CLA
  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code

Further comments

Atlas

./bin/mongocli iam organizations users list
ID                         FIRST NAME   LAST NAME    USERNAME
5e4bc367c6b0f41bb9bbb178   Andrea       Angiolillo   andrea.angiolillo@mongodb.com
5e4c1f31e131005301e9b55c   Charts       User         charts+5e429f2e06822c6eac4d59c9@mongodb.com
5e84891139309a66a28dbc90   Charts       User         charts+5e4e593f70dfbf1010295836@mongodb.com
5eb18135de2be8555676176f   Charts       User         charts+5e57d879854180642f7844df@mongodb.com
5f4e89f73a9f5c0c8872ca8f   testss       testsss      firob92048@sanitzr.com
5e399ec9f10fab1f92b44834   Gustavo      Bazan        gustavo.bazan@mongodb.com

OM

./bin/mongocli iam organizations users list -P ops --orgId 5f4f3c760ed5b166b56b02ce
ID                         FIRST NAME   LAST NAME                     USERNAME
5f4f3c980ed5b166b56b02f8   aaaaa        aaaaa                         ssss@gmail.com
5f4f3c760ed5b166b56b02cb   Andrea       angiolillo.andrea@gmail.com   test@gmail.com

Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of small nits, thanks for some of the fixes here and there

cmd.AddCommand(CreateBuilder())
cmd.AddCommand(DeleteBuilder())
cmd.AddCommand(apikeys.Builder())
cmd.AddCommand(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thanks


const (
short = "Users operations."
long = "Create, list and manage your Atlas/Cloud Manager/Ops Manager users."
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should only mention listing, I don't think we support anything else

"github.com/spf13/cobra"
)

const atlasUserListTemplate = `ID FIRST NAME LAST NAME EMAIL{{range .Results}}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the sake of having a single template I'm ok with updating the om client to return the full object, ie including the Result field

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also could we match this to the describe command? we had username there I think, this to avoid surprises from users and offer a consistent view

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh sorry @gssbzn I didn't see you PR 😖 Thanks for that

Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andreaangiolillo andreaangiolillo merged commit 3730e69 into master Sep 2, 2020
@andreaangiolillo andreaangiolillo deleted the CLOUDP-70015 branch September 2, 2020 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants