Feature #6013

[Workbench] clean up 'setup user' behavior in the admin interface

Added by Ward Vandewege over 6 years ago. Updated over 6 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Radhika Chippada
Category:
Workbench
Target version:
Start date:
06/10/2015
Due date:
% Done:

100%

Estimated time:
(Total: 2.00 h)
Story points:
1.0

Description

Problem.

The user admin page says:

As an admin, you can setup this user. Please input a VM and repository for the user. If you had previously provided any of these items, they are pre-filled for you and you can leave them as is if you would like to reuse them.

which is followed by the 'Setup user' button.

That button shows a form that has (amongst other things) a 'Repository Name and Shell Login' field.

This field is actually ignored now when the shell account is created - we always use the part of the e-mail address before the @ sign to create the shell account (with some extra logic to avoid collisions).

In addition, users can now create their own repositories through the manage account page.

Finally, having one form that does both things at once is confusing/annoying, because I usually only want to create a shell account or create a repository.

Solution

a) remove the 'Repository Name and Shell Login' field
b) rename the form to 'Set up shell account'
c) rename the button to 'Set up shell account'
d) change the leading text to

As an admin, you can set up a shell account for this user. The login name is automatically generated from the e-mail address.

e) change the logic of the form so it no longer creates a repository and repository link.
f) the shell groups input and logic can stay as it is
g) the text of the e-mail that is kicked off automatically when this form is submitted should be changed to:

=============================================================================
Subject: Welcome to Curoverse - shell account enabled

<username>,

Your Arvados shell account has been set up. Please visit the manage account page at

&lt;workbench_url&gt;/manage_account

for connection instructions.

Thanks,
The Arvados team. =============================================================================


Subtasks

Task #6298: Review branch: 6013-cleanup-setup-implResolvedRadhika Chippada

Associated revisions

Revision 09fcf10f
Added by Radhika Chippada over 6 years ago

closes #6013
Merge branch '6013-cleanup-setup-impl'

History

#1 Updated by Ward Vandewege over 6 years ago

  • Description updated (diff)

#2 Updated by Ward Vandewege over 6 years ago

  • Description updated (diff)

#3 Updated by Radhika Chippada over 6 years ago

The same setup popup is also used by the "Add new user" button in /users page. Hence, the changes need to agree with this flow as well.

#4 Updated by Brett Smith over 6 years ago

  • Category set to Workbench
  • Target version set to 2015-07-08 sprint
  • Story points set to 1.0

#5 Updated by Radhika Chippada over 6 years ago

  • Assigned To set to Radhika Chippada

#6 Updated by Radhika Chippada over 6 years ago

Some comments to clarify the requirements.

  • The following: I think it is better to leave these as "Setup user" since the form is used not only to create shell account, but also to add groups. In addition, an admin may update groups more than once but leave the VM alone. In fact, selecting VM is not "mandatory" either in the form. Also, in case of new user, email also will be entered. So, I think leaving it alone is simpler and cleaner, while still fulfuling the main goal of "not doing repositories at this time". We may add more (like groups) to the form in future, and leaving it generic seems desirable.

b) rename the form to 'Create shell account'
c) rename the button to 'Create shell account'

  • f) the text of the e-mail that is kicked off automatically when this form is submitted should be changed to ...
    • I think this also not really required. I think we can leave the title alone, and update the body of the message instead.

#7 Updated by Ward Vandewege over 6 years ago

The goal of this ticket is to get away from the 'one form rules them all' behavior.

Admin functions should be very simple and very straightforward. Think principle of least surprise. The admin should never have to wonder if clicking a button is also going to have some strange side effect (like creating a git repo when all they want to do is create a shell acccount...).

No more than one function per form. If the groups function has been added to the same form, it needs to go.

The description above effectively amounts to removing the 'create a repository' functionality because it can be done by the user themselves, or by an admin cloaking as the user.

#8 Updated by Brett Smith over 6 years ago

Ward Vandewege wrote:

Admin functions should be very simple and very straightforward. Think principle of least surprise. The admin should never have to wonder if clicking a button is also going to have some strange side effect (like creating a git repo when all they want to do is create a shell acccount...).

No more than one function per form. If the groups function has been added to the same form, it needs to go.

I think we're talking past each other here. The "groups" Radhika is referring to are group memberships for the shell account, which just got added earlier this week. Surely that's an appropriate input for a form to set up a shell account generally?

#9 Updated by Ward Vandewege over 6 years ago

Brett Smith wrote:

Ward Vandewege wrote:

Admin functions should be very simple and very straightforward. Think principle of least surprise. The admin should never have to wonder if clicking a button is also going to have some strange side effect (like creating a git repo when all they want to do is create a shell acccount...).

No more than one function per form. If the groups function has been added to the same form, it needs to go.

I think we're talking past each other here. The "groups" Radhika is referring to are group memberships for the shell account, which just got added earlier this week. Surely that's an appropriate input for a form to set up a shell account generally?

Okay, that's fair. I looked at the version on 4xphq and that does seem fine. I assume that if I want to add a group to an existing account, I can do that with that form and it won't blow up because the account already exists?

#10 Updated by Brett Smith over 6 years ago

Ward Vandewege wrote:

Okay, that's fair. I looked at the version on 4xphq and that does seem fine. I assume that if I want to add a group to an existing account, I can do that with that form and it won't blow up because the account already exists?

It won't blow up, but it won't do anything either, because of the way the logic works in the shell account creation script. We'll need to make that script smarter if you want that to work.

#11 Updated by Ward Vandewege over 6 years ago

Brett Smith wrote:

Ward Vandewege wrote:

Okay, that's fair. I looked at the version on 4xphq and that does seem fine. I assume that if I want to add a group to an existing account, I can do that with that form and it won't blow up because the account already exists?

It won't blow up, but it won't do anything either, because of the way the logic works in the shell account creation script. We'll need to make that script smarter if you want that to work.

Ah. Yeah, we should probably make that work. Where do you want me to put that ticket? Just future sprints?

#12 Updated by Brett Smith over 6 years ago

Ward Vandewege wrote:

Ah. Yeah, we should probably make that work. Where do you want me to put that ticket? Just future sprints?

This is now #6403.

#13 Updated by Brett Smith over 6 years ago

  • Description updated (diff)

Updating the description to reflect the recent conversation on this ticket. The current description should be implementable and satisfying to all parties.

#14 Updated by Brett Smith over 6 years ago

  • Description updated (diff)

#15 Updated by Radhika Chippada over 6 years ago

  • Status changed from New to In Progress

#16 Updated by Radhika Chippada over 6 years ago

Notes about updates in 6013-cleanup-setup-impl branch:

  • All the items listed in the description are addressed.
  • On the server side: left the "setup_repo_vm_links" method as is on the admin side. Since workbench will no longer be sending repo_name, changing this method implementation on server side is not required and this logic will not be used. I can remove repo handling from this method and update all tests if we indeed want to "cleanup" the code. Thanks.

#17 Updated by Nico César over 6 years ago

reviewed 160aa31a20a754b165f0184d712fba8b65519125

looks good to me.

#18 Updated by Radhika Chippada over 6 years ago

  • Status changed from In Progress to Resolved

Applied in changeset arvados|commit:09fcf10ff841e5032145936385b406412674a368.

Also available in: Atom PDF