Improve gem login scope selection

[williantenfen]

What was the end-user or developer problem that led to this PR?

Gem login api scopes is confusing and difficult as show_dashboard permission invalidates the rest.
More details in issue #7238

What is your fix for the problem, implemented in this PR?

Enhance scope selection by prompting first exclusively scopes and message about when one of those is selected (other scopes will be disabled). updated prompt:

Enter your RubyGems.org credentials.
Don't have an account yet? Create one at https://rubygems.org/sign_up
   Email:   [redacted]
Password:   [redacted]

API Key name [redacted]:   test
The default access scope is:
  index_rubygems: y

Do you want to customise scopes? [yN]  y
show_dashboard [yN]  y
You selected show_dashboard, which must be enabled exclusively. Other scopes will be disabled.
Is this fine? (type no for selecting other scopes) [Yn]  y
...

Make sure the following tasks are checked

• Describe the problem / feature
• Write tests for features and bug fixes
• Write code to solve the problem
• Make sure you follow the current code style and write meaningful commit messages without tags

[welcome]

Thanks for opening a pull request and helping make RubyGems and Bundler better! Someone from the RubyGems team will take a look at your pull request shortly and leave any feedback. Please make sure that your pull request has tests for any changes or added functionality.

We use GitHub Actions to test and make sure your change works functionally and uses acceptable conventions, you can review the current progress of GitHub Actions in the PR status window below.

If you have any questions or concerns that you wish to ask, feel free to leave a comment in this PR or join our #rubygems or #bundler channel on Slack.

For more information about contributing to the RubyGems project feel free to review our CONTRIBUTING guide

[williantenfen]

@martinemde any feedback on this?

[martinemde]

This is great. I think we could clean up a little and then I'm happy to approve.

[williantenfen]

@martinemde thanks! updated with suggestions 😃

[deivid-rodriguez]

Thanks for working on this, left a comment on approach and implementation!

[deivid-rodriguez]

How about this alternative implementation?

diff --git a/lib/rubygems/gemcutter_utilities.rb b/lib/rubygems/gemcutter_utilities.rb
index 9b50b55ede..1236861eac 100644
--- a/lib/rubygems/gemcutter_utilities.rb
+++ b/lib/rubygems/gemcutter_utilities.rb
@@ -322,23 +322,23 @@ def get_scope_params(scope)
       say "\n"
       customise = ask_yes_no("Do you want to customise scopes?", false)
       if customise
-        scope_params = {}
-        exclusive_scope = nil
         EXCLUSIVELY_API_SCOPES.each do |s|
           selected = ask_yes_no(s.to_s, false)
-          exclusive_scope = s if selected
-          break if selected
-        end
-        if exclusive_scope
-          say "You selected #{exclusive_scope}, which must be enabled exclusively. Other scopes will be disabled."
+          next unless selected
+
+          say "You selected #{s}, which must be enabled exclusively. Other scopes will be disabled."
           keep_selection = ask_yes_no("Is this fine? (type no for selecting other scopes)", true)
-          scope_params = { exclusive_scope => true } if keep_selection
+
+          return { s => true } if keep_selection
+
+          break
         end
-        if exclusive_scope.nil? || (exclusive_scope && !keep_selection)
-          API_SCOPES.each do |s|
-            selected = ask_yes_no(s.to_s, false)
-            scope_params[s] = true if selected
-          end
+
+        scope_params = {}
+
+        API_SCOPES.each do |s|
+          selected = ask_yes_no(s.to_s, false)
+          scope_params[s] = true if selected
         end
       end
       say "\n"

Seems more straightforward to me. If an exclusive scope is selected, return it if it's fine, or directly skip to non exclusive scopes if it's not.

Alternatively, we could inform of exclusivity while asking, so we don't need yet another prompt?

The default access scope is:
  index_rubygems: y

Do you want to customise scopes? [yN]  y
show_dashboard (answering yes disables other scopes) [yN]  N
index_rubygems [yN]  N
push_rubygem [yN]  y
...

[williantenfen]

thanks!

I think the alternative makes sense as well...
I can update to that if we think its better.. let me know...

[deivid-rodriguez]

I think it's a bit simpler and usable, but let's check with @martinemde before you implement anything.

[martinemde]

I like it, @deivid-rodriguez. Saying ahead of time what will happen by choosing show_dashboard will produce less mistakes. I'm making a similar change to the web UI so that show_dashboard is presented at the top with the heading "Exclusive Scopes". On the CLI the best we can do is explain it up front.

Btw, no where do we explain why this is the case. What are the scopes? Do we need to add a ?/h option that prints a brief explanation of the scope? (this should be a follow up PR, imho)

[williantenfen]

this:

show_dashboard (answering yes disables other scopes)

or

show_dashboard (exclusive scope, answering yes will not prompt for other scopes)

?

[deivid-rodriguez]

The latter seems more clear to me 👍.

[williantenfen]

updated 🙂

[williantenfen]

sorry for the back and forwards, I should have waited for more discussions on the issue itself.. was just looking to contribute to something...

If theres something similar already implemented for adding a ? /h option, or some guidance on how to do that, I can try a PR for that as well... thanks...

[martinemde]

Yeah, this is great.

And don't worry, we always go back and forth on stuff like this. I appreciate your patience! 😀

[martinemde]

Looks ready to me. Could you squash the commits?

[martinemde]

@deivid-rodriguez ready to merge with your approval.

[deivid-rodriguez]

Awesome!