Arvados

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.

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.

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.

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?

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?

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.

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?

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.

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.

reviewed 160aa31a20a754b165f0184d712fba8b65519125

looks good to me.