Skip to content

Commit

Permalink
Add new users admin addresses page
Browse files Browse the repository at this point in the history
This migrates the `users/:id/addresses` page to the new admin. Instead
of squashing everything into one controller action with an `if
request.put?` wrapper (like the legacy backend controller did), I chose
to break this new admin into two actions separated by their methods and
responsibilities.

The old admin page did not have anything in the way of validations or
error messages, it would simply fail to update the address and leave it
as is if the user provided invalid values. (While also showing a message
saying that the update was successful?) This page improves upon that
flow somewhat, but we are a bit limited by the lack of
`accepts_nested_attributes_for :ship_address` in terms of full form
validations when just submitting an update against the user. An
improvement to be sure, but still more could be done if desired in the
future.
  • Loading branch information
MadelineCollier committed Oct 7, 2024
1 parent 3a92a45 commit 886e4fa
Show file tree
Hide file tree
Showing 9 changed files with 312 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<%= page do %>
<%= page_header do %>
<%= page_header_back(solidus_admin.users_path) %>
<%= page_header_title(t(".title", email: @user.email)) %>
<%= page_header_actions do %>
<%= render component("ui/button").new(tag: :a, text: t(".create_order_for_user"), href: spree.new_admin_order_path(user_id: @user.id)) %>
<% end %>
<% end %>
<%= page_header do %>
<% tabs.each do |tab| %>
<%= render(component("ui/button").new(tag: :a, scheme: :ghost, text: tab[:text], 'aria-current': tab[:current], href: tab[:href])) %>
<% end %>
<% end %>
<%= page_with_sidebar do %>
<%= page_with_sidebar_main do %>
<%= render component('ui/panel').new(title: t(".billing_address")) do %>
<%= form_for @user, url: solidus_admin.update_addresses_user_path(@user), method: :put, html: { id: "#{form_id}_billing", autocomplete: "off", class: "bill_address" } do |f| %>
<%= render component('ui/forms/address').new(address: (@user.bill_address || Spree::Address.build_default), name: "user[bill_address_attributes]") %>
<div class="py-1.5 text-center">
<%= render component("ui/button").new(tag: :button, text: t(".update"), form: "#{form_id}_billing") %>
<%= render component("ui/button").new(tag: :a, text: t(".cancel"), href: solidus_admin.addresses_user_path(@user), scheme: :secondary) %>
</div>
<% end %>
<% end %>
<%= render component('ui/panel').new(title: t(".shipping_address")) do %>
<%= form_for @user, url: solidus_admin.update_addresses_user_path(@user), method: :put, html: { id: "#{form_id}_shipping", autocomplete: "off", class: "ship_address" } do |f| %>
<%= render component('ui/forms/address').new(address: (@user.ship_address || Spree::Address.build_default), name: "user[ship_address_attributes]") %>
<div class="py-1.5 text-center">
<%= render component("ui/button").new(tag: :button, text: t(".update"), form: "#{form_id}_shipping") %>
<%= render component("ui/button").new(tag: :a, text: t(".cancel"), href: solidus_admin.addresses_user_path(@user), scheme: :secondary) %>
</div>
<% end %>
<% end %>
<% end %>
<%= page_with_sidebar_aside do %>
<%= render component("ui/panel").new(title: t("spree.lifetime_stats")) do %>
<%= render component("ui/details_list").new(
items: [
{ label: t("spree.total_sales"), value: @user.display_lifetime_value.to_html },
{ label: t("spree.order_count"), value: @user.order_count.to_i },
{ label: t("spree.average_order_value"), value: @user.display_average_order_value.to_html },
{ label: t("spree.member_since"), value: @user.created_at.to_date },
{ label: t(".last_active"), value: last_login(@user) },
]
) %>
<% end %>
<% end %>
<% end %>
<% end %>
56 changes: 56 additions & 0 deletions admin/app/components/solidus_admin/users/addresses/component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# frozen_string_literal: true

class SolidusAdmin::Users::Addresses::Component < SolidusAdmin::BaseComponent
include SolidusAdmin::Layout::PageHelpers

def initialize(user:)
@user = user
end

def form_id
@form_id ||= "#{stimulus_id}--form-#{@user.id}"
end

def tabs
[
{
text: t('.account'),
href: solidus_admin.user_path(@user),
current: action_name == "edit",
},
{
text: t('.addresses'),
href: solidus_admin.addresses_user_path(@user),
current: action_name == "addresses",
},
{
text: t('.order_history'),
href: spree.orders_admin_user_path(@user),
# @todo: update this "current" logic once folded into new admin
current: action_name != "addresses",
},
{
text: t('.items'),
href: spree.items_admin_user_path(@user),
# @todo: update this "current" logic once folded into new admin
current: action_name != "addresses",
},
{
text: t('.store_credit'),
href: spree.admin_user_store_credits_path(@user),
# @todo: update this "current" logic once folded into new admin
current: action_name != "addresses",
},
]
end

def last_login(user)
return t('.last_login.never') if user.try(:last_sign_in_at).blank?

t(

Check warning on line 50 in admin/app/components/solidus_admin/users/addresses/component.rb

View check run for this annotation

Codecov / codecov/patch

admin/app/components/solidus_admin/users/addresses/component.rb#L50

Added line #L50 was not covered by tests
'.last_login.login_time_ago',
# @note The second `.try` is only here for the specs to work.
last_login_time: time_ago_in_words(user.try(:last_sign_in_at))
).capitalize
end
end
18 changes: 18 additions & 0 deletions admin/app/components/solidus_admin/users/addresses/component.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
en:
title: "Users / %{email} / Addresses"
account: Account
addresses: Addresses
order_history: Order History
items: Items
store_credit: Store Credit
last_active: Last Active
last_login:
login_time_ago: "%{last_login_time} ago"
never: Never
invitation_sent: Invitation sent
create_order_for_user: Create order for this user
update: Update
cancel: Cancel
back: Back
billing_address: Billing Address
shipping_address: Shipping Address
15 changes: 7 additions & 8 deletions admin/app/components/solidus_admin/users/edit/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,32 +15,31 @@ def tabs
[
{
text: t('.account'),
href: solidus_admin.users_path,
current: action_name == "show",
href: solidus_admin.user_path(@user),
current: action_name == "edit",
},
{
text: t('.addresses'),
href: spree.addresses_admin_user_path(@user),
# @todo: update this "current" logic once folded into new admin
current: action_name != "show",
href: solidus_admin.addresses_user_path(@user),
current: action_name == "addresses",
},
{
text: t('.order_history'),
href: spree.orders_admin_user_path(@user),
# @todo: update this "current" logic once folded into new admin
current: action_name != "show",
current: action_name != "edit",
},
{
text: t('.items'),
href: spree.items_admin_user_path(@user),
# @todo: update this "current" logic once folded into new admin
current: action_name != "show",
current: action_name != "edit",
},
{
text: t('.store_credit'),
href: spree.admin_user_store_credits_path(@user),
# @todo: update this "current" logic once folded into new admin
current: action_name != "show",
current: action_name != "edit",
},
]
end
Expand Down
56 changes: 53 additions & 3 deletions admin/app/controllers/solidus_admin/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
module SolidusAdmin
class UsersController < SolidusAdmin::BaseController
include SolidusAdmin::ControllerHelpers::Search
include Spree::Core::ControllerHelpers::StrongParameters

before_action :set_user, only: [:edit, :addresses, :update_addresses]

search_scope(:all, default: true)
search_scope(:customers) { _1.left_outer_joins(:role_users).where(role_users: { id: nil }) }
Expand All @@ -23,9 +26,37 @@ def index
end
end

def edit
set_user
def addresses
respond_to do |format|
format.turbo_stream { render turbo_stream: '<turbo-stream action="refresh" />' }
format.html { render component('users/addresses').new(user: @user) }
end
end

def update_addresses
set_address_from_params

if @address.valid? && @user.update(user_params)
flash[:success] = t('.success')

respond_to do |format|
format.turbo_stream { render turbo_stream: '<turbo-stream action="refresh" />' }
format.html { render component('users/addresses').new(user: @user) }
end
else
# @note It should be safe to mark the stock model validation messages as html_safe.
# rubocop:disable Rails/OutputSafety
flash[:error] = @address.errors.any? ? @address.errors.full_messages.join(", ").html_safe : t('.error')
# rubocop:enable Rails/OutputSafety

respond_to do |format|
format.turbo_stream { render turbo_stream: '<turbo-stream action="refresh" />' }
format.html { render component('users/addresses').new(user: @user), status: :unprocessable_entity }
end
end
end

def edit
respond_to do |format|
format.html { render component('users/edit').new(user: @user) }
end
Expand All @@ -47,7 +78,26 @@ def set_user
end

def user_params
params.require(:user).permit(:user_id, permitted_user_attributes)
params.require(:user).permit(
:user_id,
permitted_user_attributes,
bill_address_attributes: permitted_address_attributes,
ship_address_attributes: permitted_address_attributes
)
end

# @note This method aims to fill the gap left by not having, or not knowing
# whether we will have the following on the current Spree.user_class.
# accepts_nested_attributes_for :ship_address
# accepts_nested_attributes_for :bill_address
# Not having these strips us of the ability to leverage a lot of the
# default Rails validations.
def set_address_from_params
if user_params.key?(:bill_address_attributes)
@address = Spree::Address.new(user_params[:bill_address_attributes])
elsif user_params.key?(:ship_address_attributes)
@address = Spree::Address.new(user_params[:ship_address_attributes])
end
end

def authorization_subject
Expand Down
3 changes: 3 additions & 0 deletions admin/config/locales/users.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,6 @@ en:
title: "Users"
destroy:
success: "Users were successfully removed."
update_addresses:
success: "Address has been successfully updated."
error: "Address could not be updated."
8 changes: 7 additions & 1 deletion admin/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@
end
end

admin_resources :users, only: [:index, :edit, :destroy]
admin_resources :users, only: [:index, :edit, :destroy] do
member do
get :addresses
put :update_addresses
end
end

admin_resources :promotions, only: [:index, :destroy]
admin_resources :properties, only: [:index, :destroy]
admin_resources :option_types, only: [:index, :destroy], sortable: true
Expand Down
51 changes: 51 additions & 0 deletions admin/spec/features/users_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,55 @@
expect(page).not_to have_content("[email protected]")
end
end

context "when editing a user's addresses" do
before do
create(:user_with_addresses, email: "[email protected]")
visit "/admin/users"
find_row("[email protected]").click
click_on "Addresses"
end

it "shows the address page" do
expect(page).to have_content("Users / [email protected] / Addresses")
expect(page).to have_content("Lifetime Stats")
expect(page).to have_content("Billing Address")
expect(page).to be_axe_clean
end

it "allows editing of the existing address" do
# Valid submission
within("form.bill_address") do
fill_in "Name", with: "Galadriel"
click_on "Update"
end
expect(page).to have_content("Address has been successfully updated.")

# Valid submission
within("form.ship_address") do
fill_in "Name", with: "Elrond"
click_on "Update"
end
expect(page).to have_content("Address has been successfully updated.")

# Cancel submission
within("form.bill_address") do
fill_in "Name", with: "Smeagol"
click_on "Cancel"
end
expect(page).to have_content("Users / [email protected] / Addresses")
expect(page).not_to have_content("Smeagol")

# Invalid submission
within("form.ship_address") do
fill_in "Name", with: ""
click_on "Update"
end
expect(page).to have_content("Name can't be blank")

# The address forms weirdly only have values rather than actual text on the page.
expect(page).to have_field("user[bill_address_attributes][name]", with: "Galadriel")
expect(page).to have_field("user[ship_address_attributes][name]", with: "Elrond")
end
end
end
Loading

0 comments on commit 886e4fa

Please sign in to comment.