Skip to content

Conversation

@gssbzn
Copy link
Contributor

@gssbzn gssbzn commented Mar 8, 2022

Proposed changes

Jira ticket: CLOUDP-114891

Checklist

  • I have added tests that prove my fix is effective or that my feature works
  • I have added any necessary documentation in document requirements section listed in CONTRIBUTING.md (if appropriate)
  • I have addressed the @mongodb/docs-cloud-team comments (if appropriate)
  • I have updated e2e/E2E-TESTS.md (if an e2e test has been added)
  • I have run make fmt and formatted my code

Further comments

global user not in VPN

./bin/mongocli login

First, copy your one-time code: AAAA-AAAA

Next, sign with your browser and enter the code.

Or go to https://account.mongodb.com/account/connect

Your code will expire after 10 minutes.
Successfully logged in as gustavo.bazan@mongodb.com.
Press Enter to continue your profile configuration
There was an error fetching your organizations: Global user is from outside whitelisted subnets.
? Do you want to enter the Org ID manually? Yes
? Default Org ID: [? for help]

More than 100 results

./bin/mongocli login

First, copy your one-time code: AAAA-AAAA

Next, sign with your browser and enter the code.

Or go to https://account.mongodb.com/account/connect

Your code will expire after 10 minutes.
Successfully logged in as gustavo.bazan@mongodb.com.
Press Enter to continue your profile configuration
You have access to more than 100 orgs
? Do you want to enter the Org ID manually? No
You have access to more than 100 projects
? Do you want to enter the Project ID manually? (y/N)

Less than 100 results

./bin/mongocli login -P gus

First, copy your one-time code: AAAA-AAAA

Next, sign with your browser and enter the code.

Or go to https://account.mongodb.com/account/connect

Your code will expire after 10 minutes.
Successfully logged in as gssbzn@gmail.com.
Press Enter to continue your profile configuration
? Choose a default organization: Test (z)
? Choose a default project: Test (z)
? Default Output Format:
? Default MongoDB Shell Path: /usr/local/bin/mongosh

Your profile is now configured.
To use this profile, you must set the flag [-P gus] for every command.
You can use [mongocli config set] to change these settings at a later time.

@gssbzn gssbzn marked this pull request as ready for review March 8, 2022 18:01
@gssbzn gssbzn requested a review from a team March 8, 2022 18:01
@boooczek
Copy link

boooczek commented Mar 9, 2022

Making some copy suggestions

global user not in VPN

./bin/mongocli login

First, copy your one-time code: AAAA-AAAA

Next, sign with your browser and enter the code.

Or go to https://account.mongodb.com/account/connect

Your code will expire after 10 minutes.
Successfully logged in as gustavo.bazan@mongodb.com.
Press Enter to continue your profile configuration
There was an error fetching your organizations: Global user is outside the Access List entries.
? Enter the default Org ID manually: [? for help]

For the below case, would it be possible to do sth like this:

More than 100 results

./bin/mongocli login

First, copy your one-time code: AAAA-AAAA

Next, sign with your browser and enter the code.

Or go to https://account.mongodb.com/account/connect

Your code will expire after 10 minutes.
Successfully logged in as gustavo.bazan@mongodb.com.
Press Enter to continue your profile configuration
? Choose a default organization or enter the Org ID: Test (z)
? Choose a default project or enter the Project ID: Test (z)

This way, it could be the same experience for any amount of orgs and projects.

@dianchenghu
Copy link

I think the proposed solution as a quick fix works well. My only concern is the number "100". I quickly asked two CLI users how many items in a selection list they think is maximum and they both told me 10, at most 20. If Jakub could get the number of how many projects in an organization for our users, the data could also be helpful. My suggestion is, if in more than 99% cases, there are less than 20 projects in an Org, then we could go with the solution Guss proposed but change "100" to "20".

Also some proposed copy changes:
Press Enter to continue your profile configuration
You have access to more than 20 organizations,
? Instead of selection from the list, do you want to enter the Org ID directly? (y/N)
You have access to more than 100 projects,
? Instead of selection from the list, do you want to enter the Project ID directly? (y/N)

blva
blva previously approved these changes Mar 9, 2022
Copy link
Collaborator

@blva blva left a comment

Choose a reason for hiding this comment

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

code review - LGTM

@gssbzn
Copy link
Contributor Author

gssbzn commented Mar 9, 2022

@boooczek we can't change the error message here that comes from the API, I can make the change separately from these changes

@boooczek @dianchenghu sorry if I missed to explain this but...

The results of fetching orgs and projects is paginated with a default page size of 100 and a hard limit of 500 per page, this is where the number 100 came from but feel free to pick any number between 1 and 500 while keeping in mind the following

Currently the org/project selects need to be created with all selectable values before hand, it can't:
a) load new pages as you scroll, nor
b) load new results from the API as you search, nor
c) be both a select and a free form input at the same time, ie allow to manually enter a value if not in the list, a feature request to do this in the lib we use here

all of the above options require investigation time to implement our own custom select and with no clear idea currently if any of those options are really possible

Based on those points is important for you both to understand that whatever number we decide, ie 20, if the user happens to want to pick the 21st value it will never be in the list nor we would be able to load it and why the current implementation totally skips the list if more than the limit we decide

Copy link
Collaborator

@andreaangiolillo andreaangiolillo left a comment

Choose a reason for hiding this comment

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

LGTM

@boooczek
Copy link

boooczek commented Mar 9, 2022

With what @gssbzn said, I'm suggesting to load the max list of 500.
Then we should log a ticket to make an investigation of the desired solution.

Co-authored-by: Andrea Angiolillo <andrea.angiolillo@mongodb.com>
@gssbzn gssbzn dismissed stale reviews from andreaangiolillo and blva via c209c54 March 9, 2022 11:09
@gssbzn gssbzn requested review from andreaangiolillo and blva March 9, 2022 12:30
@gssbzn
Copy link
Contributor Author

gssbzn commented Mar 9, 2022

@boooczek I've made the change to have a limit on 500

@dianchenghu
Copy link

LGTM

@gssbzn gssbzn merged commit 87da562 into master Mar 10, 2022
@gssbzn gssbzn deleted the CLOUDP-114891 branch March 10, 2022 10:49
@boooczek
Copy link

boooczek commented Oct 11, 2022 via email

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.

6 participants