From 07a2d3a48e96d36939676e427797ee21fff3b7e5 Mon Sep 17 00:00:00 2001 From: Madeline Collier Date: Mon, 2 Dec 2024 15:21:06 +0100 Subject: [PATCH 1/3] Move store_credits actions to their own controller The admin store credit flow is complicated enough and has enough of its own actions that it should really be in its own controller rather than squished into the users controller. --- .../solidus_admin/store_credits_controller.rb | 21 +++++++++++++++++++ .../solidus_admin/users_controller.rb | 10 +-------- admin/config/routes.rb | 2 +- .../solidus_admin/store_credits_spec.rb | 21 +++++++++++++++++++ .../spec/requests/solidus_admin/users_spec.rb | 7 ------- 5 files changed, 44 insertions(+), 17 deletions(-) create mode 100644 admin/app/controllers/solidus_admin/store_credits_controller.rb create mode 100644 admin/spec/requests/solidus_admin/store_credits_spec.rb diff --git a/admin/app/controllers/solidus_admin/store_credits_controller.rb b/admin/app/controllers/solidus_admin/store_credits_controller.rb new file mode 100644 index 00000000000..449d1d67e2c --- /dev/null +++ b/admin/app/controllers/solidus_admin/store_credits_controller.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module SolidusAdmin + class StoreCreditsController < SolidusAdmin::BaseController + before_action :set_user + + def index + @store_credits = Spree::StoreCredit.where(user_id: @user.id).order(id: :desc) + + respond_to do |format| + format.html { render component("users/store_credits/index").new(user: @user, store_credits: @store_credits) } + end + end + + private + + def set_user + @user = Spree.user_class.find(params[:id]) + end + end +end diff --git a/admin/app/controllers/solidus_admin/users_controller.rb b/admin/app/controllers/solidus_admin/users_controller.rb index 21626459457..7e7d181a1bd 100644 --- a/admin/app/controllers/solidus_admin/users_controller.rb +++ b/admin/app/controllers/solidus_admin/users_controller.rb @@ -5,7 +5,7 @@ class UsersController < SolidusAdmin::BaseController include SolidusAdmin::ControllerHelpers::Search include Spree::Core::ControllerHelpers::StrongParameters - before_action :set_user, only: [:edit, :addresses, :update_addresses, :orders, :items, :store_credits] + before_action :set_user, only: [:edit, :addresses, :update_addresses, :orders, :items] search_scope(:all, default: true) search_scope(:customers) { _1.left_outer_joins(:role_users).where(role_users: { id: nil }) } @@ -81,14 +81,6 @@ def destroy redirect_back_or_to users_path, status: :see_other end - def store_credits - @store_credits = Spree::StoreCredit.where(user_id: @user.id).order(id: :desc) - - respond_to do |format| - format.html { render component("users/store_credits/index").new(user: @user, store_credits: @store_credits) } - end - end - private def set_user diff --git a/admin/config/routes.rb b/admin/config/routes.rb index c42101b7720..80a2ce7cd6e 100644 --- a/admin/config/routes.rb +++ b/admin/config/routes.rb @@ -51,7 +51,7 @@ put :update_addresses get :orders get :items - get :store_credits + get :store_credits, controller: "store_credits", action: "index" end end diff --git a/admin/spec/requests/solidus_admin/store_credits_spec.rb b/admin/spec/requests/solidus_admin/store_credits_spec.rb new file mode 100644 index 00000000000..b15a42ac4a0 --- /dev/null +++ b/admin/spec/requests/solidus_admin/store_credits_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe SolidusAdmin::StoreCreditsController, type: :request do + let(:admin_user) { create(:admin_user) } + let(:user) { create(:user) } + let!(:store_credit) { create(:store_credit, user: user) } + + before do + allow_any_instance_of(SolidusAdmin::BaseController).to receive(:spree_current_user).and_return(admin_user) + end + + describe "GET /store_credits" do + it "renders the store credits template with a 200 OK status" do + get solidus_admin.store_credits_user_path(user) + expect(response).to have_http_status(:ok) + expect(response.body).to include(store_credit.amount.to_s) + end + end +end diff --git a/admin/spec/requests/solidus_admin/users_spec.rb b/admin/spec/requests/solidus_admin/users_spec.rb index 21fcaa99b8e..c2ca7c88e2b 100644 --- a/admin/spec/requests/solidus_admin/users_spec.rb +++ b/admin/spec/requests/solidus_admin/users_spec.rb @@ -99,13 +99,6 @@ end end - describe "GET /store_credits" do - it "renders the store credits template with a 200 OK status" do - get solidus_admin.store_credits_user_path(user) - expect(response).to have_http_status(:ok) - end - end - describe "GET /addresses" do it "renders the addresses template with a 200 OK status" do get solidus_admin.addresses_user_path(user) From bfa373941c077e596531125964e8fce63d3a7903 Mon Sep 17 00:00:00 2001 From: Madeline Collier Date: Mon, 2 Dec 2024 17:10:33 +0100 Subject: [PATCH 2/3] Clarify store_credit invalidated styles If a store credit has been invalidated it will show a red badge, if it has not, it will show a green badge. This is more in line with expectations than the previous colour scheme. --- .../solidus_admin/users/store_credits/index/component.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admin/app/components/solidus_admin/users/store_credits/index/component.rb b/admin/app/components/solidus_admin/users/store_credits/index/component.rb index e039fd8cc6d..ebfb0d663e5 100644 --- a/admin/app/components/solidus_admin/users/store_credits/index/component.rb +++ b/admin/app/components/solidus_admin/users/store_credits/index/component.rb @@ -98,7 +98,7 @@ def columns header: :invalidated, col: { class: "w-[15%]" }, data: ->(store_credit) do - store_credit.invalidated? ? component('ui/badge').yes : component('ui/badge').no + store_credit.invalidated? ? component('ui/badge').new(name: :yes, color: :red, size: :m) : component('ui/badge').new(name: :no, color: :green, size: :m) end }, ] From 97919a8dcdd9beab87a28f700da8cc426003bcf0 Mon Sep 17 00:00:00 2001 From: Madeline Collier Date: Mon, 2 Dec 2024 17:33:02 +0100 Subject: [PATCH 3/3] Add new admin store_credits show page The store credits flow is in the process of being significantly refactored so this will eventually be a page which both displays the store credit info and allows editing (when appropriate). It will match the flow of the new user addresses page. --- .../users/store_credits/index/component.rb | 2 +- .../store_credits/show/component.html.erb | 59 +++++++++ .../users/store_credits/show/component.rb | 112 ++++++++++++++++++ .../users/store_credits/show/component.yml | 17 +++ .../solidus_admin/store_credits_controller.rb | 15 ++- admin/config/routes.rb | 3 +- admin/spec/features/users_spec.rb | 26 ++++ .../solidus_admin/store_credits_spec.rb | 11 +- 8 files changed, 241 insertions(+), 4 deletions(-) create mode 100644 admin/app/components/solidus_admin/users/store_credits/show/component.html.erb create mode 100644 admin/app/components/solidus_admin/users/store_credits/show/component.rb create mode 100644 admin/app/components/solidus_admin/users/store_credits/show/component.yml diff --git a/admin/app/components/solidus_admin/users/store_credits/index/component.rb b/admin/app/components/solidus_admin/users/store_credits/index/component.rb index ebfb0d663e5..f3036e4cd50 100644 --- a/admin/app/components/solidus_admin/users/store_credits/index/component.rb +++ b/admin/app/components/solidus_admin/users/store_credits/index/component.rb @@ -36,7 +36,7 @@ def tabs }, { text: t('.store_credit'), - href: solidus_admin.store_credits_user_path(@user), + href: solidus_admin.user_store_credits_path(@user), current: true, }, ] diff --git a/admin/app/components/solidus_admin/users/store_credits/show/component.html.erb b/admin/app/components/solidus_admin/users/store_credits/show/component.html.erb new file mode 100644 index 00000000000..ca89b32b5fd --- /dev/null +++ b/admin/app/components/solidus_admin/users/store_credits/show/component.html.erb @@ -0,0 +1,59 @@ +<%= page do %> + <%= page_header do %> + <%= page_header_back(solidus_admin.users_path) %> + <%= page_header_title(t(".title", email: @user.email, amount: @store_credit.display_amount)) %> + + <%= page_header_actions do %> + <%# TODO: can? actions in admin %> + <%# if @store_credit.editable? && can?(:edit, @store_credit) %> + <% if @store_credit.editable? %> + <%= render component("ui/button").new(tag: :a, text: t(".change_amount"), href: spree.edit_amount_admin_user_store_credit_path(@user, @store_credit)) %> + <% end %> + <%# TODO: can? actions in admin %> + <%# if @store_credit.invalidateable? && can?(:invalidate, @store_credit) %> + <% if @store_credit.invalidateable? %> + <%= render component("ui/button").new(scheme: :danger, tag: :a, text: t(".invalidate"), href: spree.edit_validity_admin_user_store_credit_path(@user, @store_credit)) %> + <% end %> + + <% 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(".store_credit")) do %> + <%= render component('ui/details_list').new( + items: [ + { label: t('.credited'), value: @store_credit.display_amount }, + { label: t('.created_by'), value: @store_credit.created_by_email }, + { label: t('.type'), value: @store_credit.category_name }, + { label: t('.memo'), value: @store_credit.memo } + ] + ) %> + <% end %> + + + <% if @events.present? %> +

<%= t(".store_credit_history") %>

+ <%= render component('ui/table').new( + id: stimulus_id, + data: { + class: event_model_class, + rows: @events, + columns: columns, + url: -> { row_url(_1) }, + }, + )%> + <% end %> + <% end %> + + <%= page_with_sidebar_aside do %> + <%= render component("users/stats").new(user: @user) %> + <% end %> + <% end %> +<% end %> diff --git a/admin/app/components/solidus_admin/users/store_credits/show/component.rb b/admin/app/components/solidus_admin/users/store_credits/show/component.rb new file mode 100644 index 00000000000..1101ec04eb8 --- /dev/null +++ b/admin/app/components/solidus_admin/users/store_credits/show/component.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +class SolidusAdmin::Users::StoreCredits::Show::Component < SolidusAdmin::BaseComponent + include SolidusAdmin::Layout::PageHelpers + include Spree::Admin::StoreCreditEventsHelper + + def initialize(user:, store_credit:, events:) + @user = user + @store_credit = store_credit + @events = events + end + + def model_class + Spree::StoreCredit + end + + def event_model_class + Spree::StoreCreditEvent + end + + def tabs + [ + { + text: t('.account'), + href: solidus_admin.user_path(@user), + current: false, + }, + { + text: t('.addresses'), + href: solidus_admin.addresses_user_path(@user), + current: false, + }, + { + text: t('.order_history'), + href: solidus_admin.orders_user_path(@user), + current: false, + }, + { + text: t('.items'), + href: spree.items_admin_user_path(@user), + current: false, + }, + { + text: t('.store_credit'), + href: solidus_admin.user_store_credits_path(@user), + current: true, + }, + ] + end + + def form_id + @form_id ||= "#{stimulus_id}--form-#{@store_credit.id}" + end + + def row_url(_) + "#" + end + + def columns + [ + { + header: :date, + col: { class: "w-[15%]" }, + data: ->(event) do + content_tag :div, event.created_at.strftime("%b %d '%y %l:%M%P"), class: "text-sm" + end + }, + { + header: :action, + col: { class: "w-[10%]" }, + data: ->(event) do + content_tag :div, store_credit_event_admin_action_name(event), class: "text-sm" + end + }, + { + header: :credited, + col: { class: "w-[10%]" }, + data: ->(event) do + content_tag :div, event.display_amount, class: "text-sm" + end + }, + { + header: :created_by, + col: { class: "w-[20%]" }, + data: ->(event) do + content_tag :div, store_credit_event_originator_link(event), class: "underline cursor-pointer text-sm" + end + }, + { + header: :total_amount, + col: { class: "w-[10%]" }, + data: ->(event) do + content_tag :div, event.display_user_total_amount, class: "text-sm" + end + }, + { + header: :total_unused, + col: { class: "w-[10%]" }, + data: ->(event) do + content_tag :div, event.display_remaining_amount, class: "text-sm" + end + }, + { + header: :reason_for_updating, + col: { class: "w-[25%]" }, + data: ->(event) do + content_tag :div, event.store_credit_reason&.name, class: "text-sm" + end + }, + ] + end +end diff --git a/admin/app/components/solidus_admin/users/store_credits/show/component.yml b/admin/app/components/solidus_admin/users/store_credits/show/component.yml new file mode 100644 index 00000000000..d0a4ec2d378 --- /dev/null +++ b/admin/app/components/solidus_admin/users/store_credits/show/component.yml @@ -0,0 +1,17 @@ +en: + title: "Users / %{email} / Store Credit / %{amount}" + account: Account + addresses: Addresses + order_history: Order History + items: Items + store_credit: Store Credit + last_active: Last Active + add_store_credit: Add Store Credit + change_amount: Change Amount + invalidate: Invalidate + store_credit_history: Store Credit History + credited: Credited + created_by: Created By + type: Type + memo: Memo + back: Back diff --git a/admin/app/controllers/solidus_admin/store_credits_controller.rb b/admin/app/controllers/solidus_admin/store_credits_controller.rb index 449d1d67e2c..c7032de4907 100644 --- a/admin/app/controllers/solidus_admin/store_credits_controller.rb +++ b/admin/app/controllers/solidus_admin/store_credits_controller.rb @@ -3,6 +3,7 @@ module SolidusAdmin class StoreCreditsController < SolidusAdmin::BaseController before_action :set_user + before_action :set_store_credit, only: [:show] def index @store_credits = Spree::StoreCredit.where(user_id: @user.id).order(id: :desc) @@ -12,10 +13,22 @@ def index end end + def show + @store_credit_events = @store_credit.store_credit_events.chronological + + respond_to do |format| + format.html { render component("users/store_credits/show").new(user: @user, store_credit: @store_credit, events: @store_credit_events) } + end + end + private + def set_store_credit + @store_credit = Spree::StoreCredit.find(params[:id]) + end + def set_user - @user = Spree.user_class.find(params[:id]) + @user = Spree.user_class.find(params[:user_id]) end end end diff --git a/admin/config/routes.rb b/admin/config/routes.rb index 80a2ce7cd6e..eb820febbf2 100644 --- a/admin/config/routes.rb +++ b/admin/config/routes.rb @@ -51,8 +51,9 @@ put :update_addresses get :orders get :items - get :store_credits, controller: "store_credits", action: "index" end + + resources :store_credits, only: [:index, :show], controller: "store_credits" end admin_resources :promotions, only: [:index, :destroy] diff --git a/admin/spec/features/users_spec.rb b/admin/spec/features/users_spec.rb index b19e31b1e9c..fcfd0b632c4 100644 --- a/admin/spec/features/users_spec.rb +++ b/admin/spec/features/users_spec.rb @@ -276,6 +276,7 @@ context "when a user has store credits" do let!(:store_credit) { create(:store_credit, amount: 199.00, currency: "USD") } + let!(:store_credit_reason) { create(:store_credit_reason, name: "credit given in error") } before do store_credit.user.update(email: "customer@example.com") @@ -303,6 +304,31 @@ expect(page).to have_content("Invalidated") expect(page).not_to have_content("No Store Credits found.") end + + context "when clicking through to a single store credit" do + let!(:store_credit_reason) { create(:store_credit_reason, name: "credit given in error") } + + before do + stub_authorization!(admin) + end + + it "shows individual store credit details" do + find_row("$199.00").click + expect(page).to have_content("Users / customer@example.com / Store Credit / $199.00") + expect(page).to have_content("Store Credit History") + expect(page).to have_content("Action") + expect(page).to have_content("Added") + click_on "Invalidate" + select "credit given in error", from: "store_credit_reason_id" + click_on "Invalidate" + expect(page).to have_content("Store Credit History") + expect(page).to have_content("Action") + expect(page).to have_content("Added") + expect(page).to have_content("Invalidated") + expect(page).to have_content("Reason for updating") + expect(page).to have_content("credit given in error") + end + end end end end diff --git a/admin/spec/requests/solidus_admin/store_credits_spec.rb b/admin/spec/requests/solidus_admin/store_credits_spec.rb index b15a42ac4a0..75de4192748 100644 --- a/admin/spec/requests/solidus_admin/store_credits_spec.rb +++ b/admin/spec/requests/solidus_admin/store_credits_spec.rb @@ -6,6 +6,7 @@ let(:admin_user) { create(:admin_user) } let(:user) { create(:user) } let!(:store_credit) { create(:store_credit, user: user) } + let!(:store_credit_event) { create(:store_credit_adjustment_event) } before do allow_any_instance_of(SolidusAdmin::BaseController).to receive(:spree_current_user).and_return(admin_user) @@ -13,7 +14,15 @@ describe "GET /store_credits" do it "renders the store credits template with a 200 OK status" do - get solidus_admin.store_credits_user_path(user) + get solidus_admin.user_store_credits_path(user) + expect(response).to have_http_status(:ok) + expect(response.body).to include(store_credit.amount.to_s) + end + end + + describe "GET /show" do + it "renders the store credit show page with a 200 OK status" do + get solidus_admin.user_store_credit_path(user, store_credit) expect(response).to have_http_status(:ok) expect(response.body).to include(store_credit.amount.to_s) end