Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Admin] New admin user edit page #5856

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MadelineCollier
Copy link
Contributor

@MadelineCollier MadelineCollier commented Sep 18, 2024

Summary

This PR is for issue: #5824

This migrates the users/:id/edit page to the new admin. It still relies on the old backend admin controller for the #update action as well as the other top level user tabs such as address, order history, and so on.

In the next couple PRs I will migrate the remaining tabs to the new interface, but for now I thought it best to keep the old interface for actions such as creating a new user, which we definitely want to still have working.

This PR does not handle the issue of permission dependent interface elements. In the old admin, some buttons would not be visible if the user lacked the permissions to action them. However it looks like that is currently standard for the entirety of the new admin as I don't see any permissions checks or anything like this anywhere in the new admin:

      <% 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 %>

If we are using a new helper or gem to manage this and I am just missing something definitely let me know which implementation I can reference! Otherwise for now I will put this up as is because handling if can? permissions checks across the whole of the admin is a bit out of scope for this PR.

In the next PRs I will migrate the other tabs of the user admin (address, order history etc) and will add the modal for user creation (although I may leave the user creation backend logic as is for now as it looks like the issue specifically states that the user invitation flow should be considered a separate task.

Again there weren't really designs for this so I just went for it and tried to keep to our existing components as much as possible. Hopefully it looks decent!

Screenshot 2024-09-18 at 10 24 18 PM

The attached video shows the functionality visually:

Screen.Recording.2024-09-18.at.9.29.12.PM.mov

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

The following are not always needed:

  • 📖 I have updated the README to account for my changes.
  • 📑 I have documented new code with YARD.
  • 🛣️ I have opened a PR to update the guides.
  • ✅ I have added automated tests to cover my changes.
  • 📸 I have attached screenshots to demo visual changes.

@github-actions github-actions bot added changelog:solidus_core Changes to the solidus_core gem changelog:solidus_admin labels Sep 18, 2024
@MadelineCollier MadelineCollier force-pushed the admin-user-creation-modification branch 2 times, most recently from 2786f52 to 0dda58d Compare September 18, 2024 20:16
This migrates the `users/:id/edit` page to the new admin. It still
relies on the old backend admin controller for the `#update` action as
well as the other top level user tabs such as address, order history,
and so on.

Co-authored-by: benjamin wil <benjamin@super.gd>
@MadelineCollier MadelineCollier marked this pull request as ready for review September 18, 2024 20:23
@MadelineCollier MadelineCollier requested a review from a team as a code owner September 18, 2024 20:23
@MadelineCollier MadelineCollier changed the title Add new users admin edit page [Admin] New admin user edit page Sep 18, 2024
@MadelineCollier
Copy link
Contributor Author

Looks like the promotions spec failures are consistent across all branches and unrelated to this PR

[Promotions GET #show when non admin should be unauthorized](https://app.circleci.com/pipelines/github/solidusio/solidus/6641/workflows/1a2097d4-4450-4d5e-974c-ceb2d39f7258/jobs/61557/tests#failed-test-0)
[Promotions GET #show when admin when finding by a promotion behaves like a JSON response should return JSON](https://app.circleci.com/pipelines/github/solidusio/solidus/6641/workflows/1a2097d4-4450-4d5e-974c-ceb2d39f7258/jobs/61557/tests#failed-test-1)
[Promotions GET #show when admin when finding by a promotion behaves like a JSON response should be ok](https://app.circleci.com/pipelines/github/solidusio/solidus/6641/workflows/1a2097d4-4450-4d5e-974c-ceb2d39f7258/jobs/61557/tests#failed-test-2)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant