From f24382912eb4dd987111ce98c8e1733f2f00ec38 Mon Sep 17 00:00:00 2001 From: Madeline Collier Date: Tue, 13 Aug 2024 17:08:16 +0200 Subject: [PATCH 1/4] Remove unused `load_refund_reason` from controller The general consensus is that this pattern persists across the new admin controllers purely by virtue of a copy/paste carry-over as it looks like it's not being used anywhere. --- .../controllers/solidus_admin/refund_reasons_controller.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/admin/app/controllers/solidus_admin/refund_reasons_controller.rb b/admin/app/controllers/solidus_admin/refund_reasons_controller.rb index c0e6ff4c6c..be3a2c1a3c 100644 --- a/admin/app/controllers/solidus_admin/refund_reasons_controller.rb +++ b/admin/app/controllers/solidus_admin/refund_reasons_controller.rb @@ -95,11 +95,6 @@ def destroy private - def load_refund_reason - @refund_reason = Spree::RefundReason.find_by!(id: params[:id]) - authorize! action_name, @refund_reason - end - def find_refund_reason @refund_reason = Spree::RefundReason.find(params[:id]) end From 479011f010b40e17ede958b18fc121fef3ede4a4 Mon Sep 17 00:00:00 2001 From: Madeline Collier Date: Tue, 13 Aug 2024 17:10:40 +0200 Subject: [PATCH 2/4] Ensure copy is consistent for admin refund reasons The copy had a few inconsistencies and copy/paste errors. --- .../components/solidus_admin/refund_reasons/edit/component.yml | 2 +- admin/config/locales/refund_reasons.en.yml | 2 +- admin/spec/features/refund_reasons_spec.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/admin/app/components/solidus_admin/refund_reasons/edit/component.yml b/admin/app/components/solidus_admin/refund_reasons/edit/component.yml index c604c1a628..045e9033cb 100644 --- a/admin/app/components/solidus_admin/refund_reasons/edit/component.yml +++ b/admin/app/components/solidus_admin/refund_reasons/edit/component.yml @@ -5,4 +5,4 @@ en: cancel: "Cancel" submit: "Update Refund Reason" hints: - active: "When checked, this adjustment reason will be available for selection when adding adjustments to orders." + active: "When checked, this refund reason will be available for selection when adding refunds to orders." diff --git a/admin/config/locales/refund_reasons.en.yml b/admin/config/locales/refund_reasons.en.yml index 97a50264e4..a748998bb9 100644 --- a/admin/config/locales/refund_reasons.en.yml +++ b/admin/config/locales/refund_reasons.en.yml @@ -3,7 +3,7 @@ en: refund_reasons: title: "Refund Reasons" destroy: - success: "Refund Reasons were successfully removed." + success: "Refund reasons were successfully removed." create: success: "Refund reason was successfully created." update: diff --git a/admin/spec/features/refund_reasons_spec.rb b/admin/spec/features/refund_reasons_spec.rb index a88099418a..ae5a52663e 100644 --- a/admin/spec/features/refund_reasons_spec.rb +++ b/admin/spec/features/refund_reasons_spec.rb @@ -14,7 +14,7 @@ select_row("Default-refund-reason") click_on "Delete" - expect(page).to have_content("Refund Reasons were successfully removed.") + expect(page).to have_content("Refund reasons were successfully removed.") expect(page).not_to have_content("Default-refund-reason") expect(Spree::RefundReason.count).to eq(0) expect(page).to be_axe_clean From 1e1de9ca9985ce3a1558834600eb3eefab8eaccd Mon Sep 17 00:00:00 2001 From: Madeline Collier Date: Tue, 13 Aug 2024 17:15:33 +0200 Subject: [PATCH 3/4] Fix attribute reference The human-readable output is the same regardless of which model we are referencing here, (always "Active") but since this is the RefundReason component, let's be consistent and reference that here. --- .../solidus_admin/refund_reasons/new/component.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/admin/app/components/solidus_admin/refund_reasons/new/component.html.erb b/admin/app/components/solidus_admin/refund_reasons/new/component.html.erb index ed82c770ca..ef1a44999d 100644 --- a/admin/app/components/solidus_admin/refund_reasons/new/component.html.erb +++ b/admin/app/components/solidus_admin/refund_reasons/new/component.html.erb @@ -10,7 +10,7 @@ value: "1", checked: f.object.active ) %> - <%= Spree::TaxCategory.human_attribute_name :active %> + <%= Spree::RefundReason.human_attribute_name :active %> <%= render component("ui/toggletip").new(text: t(".hints.active")) %> From cb3303871b8547d2132c8b591c69f16a8a68b6f5 Mon Sep 17 00:00:00 2001 From: Madeline Collier Date: Tue, 13 Aug 2024 17:22:36 +0200 Subject: [PATCH 4/4] Add request spec for new admin refund reasons --- .../solidus_admin/refund_reasons_spec.rb | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 admin/spec/requests/solidus_admin/refund_reasons_spec.rb diff --git a/admin/spec/requests/solidus_admin/refund_reasons_spec.rb b/admin/spec/requests/solidus_admin/refund_reasons_spec.rb new file mode 100644 index 0000000000..0c1a897984 --- /dev/null +++ b/admin/spec/requests/solidus_admin/refund_reasons_spec.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "SolidusAdmin::RefundReasonsController", type: :request do + let(:admin_user) { create(:admin_user) } + let(:refund_reason) { create(:refund_reason) } + + before do + allow_any_instance_of(SolidusAdmin::BaseController).to receive(:spree_current_user).and_return(admin_user) + end + + describe "GET /index" do + it "renders the index template with a 200 OK status" do + get solidus_admin.refund_reasons_path + expect(response).to have_http_status(:ok) + end + end + + describe "GET /new" do + it "renders the new template with a 200 OK status" do + get solidus_admin.new_refund_reason_path + expect(response).to have_http_status(:ok) + end + end + + describe "POST /create" do + context "with valid parameters" do + let(:valid_attributes) { { name: "Refund for Defective Item", code: "DEFECT", active: true } } + + it "creates a new RefundReason" do + expect { + post solidus_admin.refund_reasons_path, params: { refund_reason: valid_attributes } + }.to change(Spree::RefundReason, :count).by(1) + end + + it "redirects to the index page with a 303 See Other status" do + post solidus_admin.refund_reasons_path, params: { refund_reason: valid_attributes } + expect(response).to redirect_to(solidus_admin.refund_reasons_path) + expect(response).to have_http_status(:see_other) + end + + it "displays a success flash message" do + post solidus_admin.refund_reasons_path, params: { refund_reason: valid_attributes } + follow_redirect! + expect(response.body).to include("Refund reason was successfully created.") + end + end + + context "with invalid parameters" do + let(:invalid_attributes) { { name: "", code: "", active: true } } + + it "does not create a new RefundReason" do + expect { + post solidus_admin.refund_reasons_path, params: { refund_reason: invalid_attributes } + }.not_to change(Spree::RefundReason, :count) + end + + it "renders the new template with unprocessable_entity status" do + post solidus_admin.refund_reasons_path, params: { refund_reason: invalid_attributes } + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + describe "GET /edit" do + it "renders the edit template with a 200 OK status" do + get solidus_admin.edit_refund_reason_path(refund_reason) + expect(response).to have_http_status(:ok) + end + end + + describe "PATCH /update" do + context "with valid parameters" do + let(:valid_attributes) { { name: "Updated Refund Reason", code: "UPD", active: false } } + + it "updates the refund reason" do + patch solidus_admin.refund_reason_path(refund_reason), params: { refund_reason: valid_attributes } + refund_reason.reload + expect(refund_reason.name).to eq("Updated Refund Reason") + expect(refund_reason.code).to eq("UPD") + expect(refund_reason.active).to be(false) + end + + it "redirects to the index page with a 303 See Other status" do + patch solidus_admin.refund_reason_path(refund_reason), params: { refund_reason: valid_attributes } + expect(response).to redirect_to(solidus_admin.refund_reasons_path) + expect(response).to have_http_status(:see_other) + end + + it "displays a success flash message" do + patch solidus_admin.refund_reason_path(refund_reason), params: { refund_reason: valid_attributes } + follow_redirect! + expect(response.body).to include("Refund reason was successfully updated.") + end + end + + context "with invalid parameters" do + let(:invalid_attributes) { { name: "", code: "UPD", active: false } } + + it "does not update the refund reason" do + original_name = refund_reason.name + patch solidus_admin.refund_reason_path(refund_reason), params: { refund_reason: invalid_attributes } + refund_reason.reload + expect(refund_reason.name).to eq(original_name) + end + + it "renders the edit template with unprocessable_entity status" do + patch solidus_admin.refund_reason_path(refund_reason), params: { refund_reason: invalid_attributes } + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + describe "DELETE /destroy" do + it "deletes the refund reason and redirects to the index page with a 303 See Other status" do + # This ensures the refund_reason exists prior to deletion. + refund_reason + + expect { + delete solidus_admin.refund_reason_path(refund_reason) + }.to change(Spree::RefundReason, :count).by(-1) + + expect(response).to redirect_to(solidus_admin.refund_reasons_path) + expect(response).to have_http_status(:see_other) + end + + it "displays a success flash message after deletion" do + delete solidus_admin.refund_reason_path(refund_reason) + follow_redirect! + expect(response.body).to include("Refund reasons were successfully removed.") + end + end +end