solidus icon indicating copy to clipboard operation
solidus copied to clipboard

[Admin][Users] Introduce user creation and modification capability

Open MadelineCollier opened this issue 1 year ago • 7 comments

Update

This issue has grown in scope to encompass the entirety of the user admin's sub-menus since those didn't have their own standalone tickets. Image

Description

Develop the functionality to add and edit users through the admin panel, providing fields for key user details and the ability to assign roles.

Note: The user invitation feature will be implemented in a separated task

Features

  • Form for entering user details (name, email, role).
  • Validation and user creation logic

Figma Design

Storyboard


Design details

Image

MadelineCollier avatar Aug 13 '24 16:08 MadelineCollier

Image

@kennyadsl @adammathys do we want to add a name column to the spree_users table via a migration in core to accommodate this design? Or should we just leave it at email for now? I know it's a pretty common customization for stores to just add firstname/lastname/name to Spree::User themselves.

Same deal with the profile picture? Add it as an attachment on the model in core? I just want to make sure I'm not making too many assumptions based on the design, but if we want to 100% match the designs regardless of what we currently have for columns in core I can keep that in mind for future issues and just add whatever we need to adhere to them.

MadelineCollier avatar Aug 22 '24 10:08 MadelineCollier

Good questions, someone has to make a decision here.

It's pretty common to have this info on the user table indeed. So I guess we should do it. On the other side, it's not strictly related to this feature. I mean, I imagine the name will be initially populated with a migration that copy the name from the last order's shipping address, and we could also add this later in isolation.

For the attachment, my preference is to start without it, it has been never requested to us if I recall correctly. Some other options might be using gravatar for now? I can definitely live without it and I'm sure there's other stuff with more priority.

That said, I'm ok with having @MadelineCollier having a final word here. One way or the other it will work and it will be an improvement.

kennyadsl avatar Aug 22 '24 11:08 kennyadsl

Progress update from this week, still some kinks to iron out before it can go to PR. There weren't any designs aside from the "invite new user" flow so I just freestyled it with our existing page layouts and components. Hope it looks okay!

https://github.com/user-attachments/assets/7d1d7a8f-888b-44cc-8f82-85afe90b3cd6

I am noticing that there are a lot of spots in the old admin that look like this:

      <% if can?(:addresses, @user) %>
        <li class="<%= 'active' if current == :address %>">
          <%= link_to t("spree.admin.user.addresses"), spree.addresses_admin_user_path(@user) %>
        </li>
      <% end %>

I don't see any similar occurrences in the new admin, and I am wondering if we have an existing plan for how we want to handle CanCan stuff in our new components. For now I am just # @todo do this later -ing. Any thoughts or suggestions?

MadelineCollier avatar Sep 13 '24 16:09 MadelineCollier

@MadelineCollier nice! We should definitely address the missing CanCan conditional code. @rainerdema @elia do you have any context why this is missing in the new admin? Were we planning to add it later?

kennyadsl avatar Sep 13 '24 17:09 kennyadsl

Hey @rainerdema (separate issue from the above) I have been wondering about re-using the component('ui/forms/address') in the new admin users/addresses page.

The old admin version looks like this:

Screenshot 2024-10-01 at 4 20 58 PM

The only roadblock for reusing the component('ui/forms/address') seems to be that the component is maybe a bit old/possibly only functional within the context of the orders admin? I am not totally sure what's up with it but it doesn't pass a form object to the text_field. Screenshot 2024-10-01 at 4 21 39 PM

Is there a reason it's not passing the form here? Or is this code just a bit older and missing out on the new options that text_field has now?

Every other occurrence that I could find of component("ui/forms/field").text_field seems pass f as the form object.

Screenshot 2024-10-01 at 4 22 26 PM

Hoping to get some context from you before I make any changes that might break something in the orders admin :)

MadelineCollier avatar Oct 01 '24 14:10 MadelineCollier

@MadelineCollier nice! We should definitely address the missing CanCan conditional code. @rainerdema @elia do you have any context why this is missing in the new admin? Were we planning to add it later?

@kennyadsl @MadelineCollier I think we were planning to add them later in some way, there were a few changes that we wanted to make to make permissions more understandable and manageable from the UI. I guess that's an unsolved problem, and we should find the best trade off between the components being self contained and also know about permissions.

I think probably the easiest separation would be to avoid any permissions checks within /ui components, and only allow the checks when building a whole page.

elia avatar Oct 03 '24 12:10 elia

The only roadblock for reusing the component('ui/forms/address') seems to be that the component is maybe a bit old/possibly only functional within the context of the orders admin? I am not totally sure what's up with it but it doesn't pass a form object to the text_field.

@MadelineCollier I think you're right that's an older version, but should be easy enough to pass the form object name prefix to it and make it work outside of the order. The order surely needs to control that prefix to separate billing and shipping addresses.

elia avatar Oct 03 '24 12:10 elia