From d31e1e67f6564b442c80894b97a754103c27d639 Mon Sep 17 00:00:00 2001 From: Madeline Collier Date: Wed, 14 Aug 2024 16:00:06 +0200 Subject: [PATCH 1/2] Remove unused load method in new admin controllers These controllers were based off an example controller in which the `load_[resource_name]` method was needed. However in these cases it looks like this method is not being used, so we should be safe to remove these unused methods from these controllers. --- .../solidus_admin/adjustment_reasons_controller.rb | 5 ----- .../solidus_admin/reimbursement_types_controller.rb | 5 ----- .../controllers/solidus_admin/return_reasons_controller.rb | 5 ----- .../solidus_admin/shipping_categories_controller.rb | 5 ----- .../controllers/solidus_admin/shipping_methods_controller.rb | 5 ----- .../controllers/solidus_admin/stock_locations_controller.rb | 5 ----- .../solidus_admin/store_credit_reasons_controller.rb | 5 ----- admin/app/controllers/solidus_admin/stores_controller.rb | 5 ----- .../controllers/solidus_admin/tax_categories_controller.rb | 5 ----- admin/app/controllers/solidus_admin/tax_rates_controller.rb | 5 ----- admin/app/controllers/solidus_admin/users_controller.rb | 5 ----- admin/app/controllers/solidus_admin/zones_controller.rb | 5 ----- 12 files changed, 60 deletions(-) diff --git a/admin/app/controllers/solidus_admin/adjustment_reasons_controller.rb b/admin/app/controllers/solidus_admin/adjustment_reasons_controller.rb index 1211530bb9d..43b59a9fac9 100644 --- a/admin/app/controllers/solidus_admin/adjustment_reasons_controller.rb +++ b/admin/app/controllers/solidus_admin/adjustment_reasons_controller.rb @@ -95,11 +95,6 @@ def destroy private - def load_adjustment_reason - @adjustment_reason = Spree::AdjustmentReason.find_by!(id: params[:id]) - authorize! action_name, @adjustment_reason - end - def find_adjustment_reason @adjustment_reason = Spree::AdjustmentReason.find(params[:id]) end diff --git a/admin/app/controllers/solidus_admin/reimbursement_types_controller.rb b/admin/app/controllers/solidus_admin/reimbursement_types_controller.rb index 67eba73f73e..530578386ee 100644 --- a/admin/app/controllers/solidus_admin/reimbursement_types_controller.rb +++ b/admin/app/controllers/solidus_admin/reimbursement_types_controller.rb @@ -19,11 +19,6 @@ def index private - def load_reimbursement_type - @reimbursement_type = Spree::ReimbursementType.find_by!(id: params[:id]) - authorize! action_name, @reimbursement_type - end - def reimbursement_type_params params.require(:reimbursement_type).permit(:reimbursement_type_id, permitted_reimbursement_type_attributes) end diff --git a/admin/app/controllers/solidus_admin/return_reasons_controller.rb b/admin/app/controllers/solidus_admin/return_reasons_controller.rb index ad3a7883218..4887f236794 100644 --- a/admin/app/controllers/solidus_admin/return_reasons_controller.rb +++ b/admin/app/controllers/solidus_admin/return_reasons_controller.rb @@ -28,11 +28,6 @@ def destroy private - def load_return_reason - @return_reason = Spree::ReturnReason.find_by!(id: params[:id]) - authorize! action_name, @return_reason - end - def return_reason_params params.require(:return_reason).permit(:return_reason_id, permitted_return_reason_attributes) end diff --git a/admin/app/controllers/solidus_admin/shipping_categories_controller.rb b/admin/app/controllers/solidus_admin/shipping_categories_controller.rb index 19fbe6687eb..c87b635ce06 100644 --- a/admin/app/controllers/solidus_admin/shipping_categories_controller.rb +++ b/admin/app/controllers/solidus_admin/shipping_categories_controller.rb @@ -97,11 +97,6 @@ def destroy private - def load_shipping_category - @shipping_category = Spree::ShippingCategory.find_by!(id: params[:id]) - authorize! action_name, @shipping_category - end - def find_shipping_category @shipping_category = Spree::ShippingCategory.find(params[:id]) end diff --git a/admin/app/controllers/solidus_admin/shipping_methods_controller.rb b/admin/app/controllers/solidus_admin/shipping_methods_controller.rb index dcf18294bbd..6b079943e29 100644 --- a/admin/app/controllers/solidus_admin/shipping_methods_controller.rb +++ b/admin/app/controllers/solidus_admin/shipping_methods_controller.rb @@ -28,11 +28,6 @@ def destroy private - def load_shipping_method - @shipping_method = Spree::ShippingMethod.find_by!(number: params[:id]) - authorize! action_name, @shipping_method - end - def shipping_method_params params.require(:shipping_method).permit(:shipping_method_id, permitted_shipping_method_attributes) end diff --git a/admin/app/controllers/solidus_admin/stock_locations_controller.rb b/admin/app/controllers/solidus_admin/stock_locations_controller.rb index 364bfb12fc3..445c4ec25b0 100644 --- a/admin/app/controllers/solidus_admin/stock_locations_controller.rb +++ b/admin/app/controllers/solidus_admin/stock_locations_controller.rb @@ -28,11 +28,6 @@ def destroy private - def load_stock_location - @stock_location = Spree::StockLocation.find_by!(number: params[:id]) - authorize! action_name, @stock_location - end - def stock_location_params params.require(:stock_location).permit(:stock_location_id, permitted_stock_location_attributes) end diff --git a/admin/app/controllers/solidus_admin/store_credit_reasons_controller.rb b/admin/app/controllers/solidus_admin/store_credit_reasons_controller.rb index 399b35781d7..08e3292fbdb 100644 --- a/admin/app/controllers/solidus_admin/store_credit_reasons_controller.rb +++ b/admin/app/controllers/solidus_admin/store_credit_reasons_controller.rb @@ -95,11 +95,6 @@ def destroy private - def load_store_credit_reason - @store_credit_reason = Spree::StoreCreditReason.find_by!(id: params[:id]) - authorize! action_name, @store_credit_reason - end - def find_store_credit_reason @store_credit_reason = Spree::StoreCreditReason.find(params[:id]) end diff --git a/admin/app/controllers/solidus_admin/stores_controller.rb b/admin/app/controllers/solidus_admin/stores_controller.rb index ef60bcf8298..c653163d5c9 100644 --- a/admin/app/controllers/solidus_admin/stores_controller.rb +++ b/admin/app/controllers/solidus_admin/stores_controller.rb @@ -28,11 +28,6 @@ def destroy private - def load_store - @store = Spree::Store.find_by!(number: params[:id]) - authorize! action_name, @store - end - def store_params params.require(:store).permit(:store_id, permitted_store_attributes) end diff --git a/admin/app/controllers/solidus_admin/tax_categories_controller.rb b/admin/app/controllers/solidus_admin/tax_categories_controller.rb index 5bea5239211..5ffa67828f5 100644 --- a/admin/app/controllers/solidus_admin/tax_categories_controller.rb +++ b/admin/app/controllers/solidus_admin/tax_categories_controller.rb @@ -99,11 +99,6 @@ def destroy private - def load_tax_category - @tax_category = Spree::TaxCategory.find_by!(number: params[:id]) - authorize! action_name, @tax_category - end - def find_tax_category @tax_category = Spree::TaxCategory.find(params[:id]) end diff --git a/admin/app/controllers/solidus_admin/tax_rates_controller.rb b/admin/app/controllers/solidus_admin/tax_rates_controller.rb index 3a313fd9826..75c2f239eb4 100644 --- a/admin/app/controllers/solidus_admin/tax_rates_controller.rb +++ b/admin/app/controllers/solidus_admin/tax_rates_controller.rb @@ -28,11 +28,6 @@ def destroy private - def load_tax_rate - @tax_rate = Spree::TaxRate.find_by!(number: params[:id]) - authorize! action_name, @tax_rate - end - def tax_rate_params params.require(:tax_rate).permit(:tax_rate_id, permitted_tax_rate_attributes) end diff --git a/admin/app/controllers/solidus_admin/users_controller.rb b/admin/app/controllers/solidus_admin/users_controller.rb index 90518339c07..b8de0af3d0e 100644 --- a/admin/app/controllers/solidus_admin/users_controller.rb +++ b/admin/app/controllers/solidus_admin/users_controller.rb @@ -34,11 +34,6 @@ def destroy private - def load_user - @user = Spree.user_class.find_by!(number: params[:id]) - authorize! action_name, @user - end - def user_params params.require(:user).permit(:user_id, permitted_user_attributes) end diff --git a/admin/app/controllers/solidus_admin/zones_controller.rb b/admin/app/controllers/solidus_admin/zones_controller.rb index 6dd3a8b3f48..11d64cd3a3a 100644 --- a/admin/app/controllers/solidus_admin/zones_controller.rb +++ b/admin/app/controllers/solidus_admin/zones_controller.rb @@ -28,11 +28,6 @@ def destroy private - def load_zone - @zone = Spree::Zone.find_by!(number: params[:id]) - authorize! action_name, @zone - end - def zone_params params.require(:zone).permit(:zone_id, permitted_zone_attributes) end From 8dc4c1f73807c9392891c2865f5da14816a43b22 Mon Sep 17 00:00:00 2001 From: Madeline Collier Date: Wed, 14 Aug 2024 16:02:56 +0200 Subject: [PATCH 2/2] Add more request spec coverage While the bulk of our coverage comes from feature tests, the Codecov percentage was sufferring as smaller portions of our controllers were going untested without these types of specs. Adding them fixes our decreases in our Codecov percentage and adds peace of mind. --- .../solidus_admin/adjustment_reasons_spec.rb | 134 ++++++++++++++++++ .../solidus_admin/shipping_categories_spec.rb | 132 +++++++++++++++++ 2 files changed, 266 insertions(+) create mode 100644 admin/spec/requests/solidus_admin/adjustment_reasons_spec.rb create mode 100644 admin/spec/requests/solidus_admin/shipping_categories_spec.rb diff --git a/admin/spec/requests/solidus_admin/adjustment_reasons_spec.rb b/admin/spec/requests/solidus_admin/adjustment_reasons_spec.rb new file mode 100644 index 00000000000..1f4a8bd7ccb --- /dev/null +++ b/admin/spec/requests/solidus_admin/adjustment_reasons_spec.rb @@ -0,0 +1,134 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "SolidusAdmin::AdjustmentReasonsController", type: :request do + let(:admin_user) { create(:admin_user) } + let(:adjustment_reason) { create(:adjustment_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.adjustment_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_adjustment_reason_path + expect(response).to have_http_status(:ok) + end + end + + describe "POST /create" do + context "with valid parameters" do + let(:valid_attributes) { { name: "Price Adjustment", code: "PRICE_ADJUST", active: true } } + + it "creates a new AdjustmentReason" do + expect { + post solidus_admin.adjustment_reasons_path, params: { adjustment_reason: valid_attributes } + }.to change(Spree::AdjustmentReason, :count).by(1) + end + + it "redirects to the index page with a 303 See Other status" do + post solidus_admin.adjustment_reasons_path, params: { adjustment_reason: valid_attributes } + expect(response).to redirect_to(solidus_admin.adjustment_reasons_path) + expect(response).to have_http_status(:see_other) + end + + it "displays a success flash message" do + post solidus_admin.adjustment_reasons_path, params: { adjustment_reason: valid_attributes } + follow_redirect! + expect(response.body).to include("Adjustment reason was successfully created.") + end + end + + context "with invalid parameters" do + let(:invalid_attributes) { { name: "", code: "", active: true } } + + it "does not create a new AdjustmentReason" do + expect { + post solidus_admin.adjustment_reasons_path, params: { adjustment_reason: invalid_attributes } + }.not_to change(Spree::AdjustmentReason, :count) + end + + it "renders the new template with unprocessable_entity status" do + post solidus_admin.adjustment_reasons_path, params: { adjustment_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_adjustment_reason_path(adjustment_reason) + expect(response).to have_http_status(:ok) + end + end + + describe "PATCH /update" do + context "with valid parameters" do + let(:valid_attributes) { { name: "Updated Adjustment Reason", code: "UPD_ADJ", active: false } } + + it "updates the adjustment reason" do + patch solidus_admin.adjustment_reason_path(adjustment_reason), params: { adjustment_reason: valid_attributes } + adjustment_reason.reload + expect(adjustment_reason.name).to eq("Updated Adjustment Reason") + expect(adjustment_reason.code).to eq("UPD_ADJ") + expect(adjustment_reason.active).to be(false) + end + + it "redirects to the index page with a 303 See Other status" do + patch solidus_admin.adjustment_reason_path(adjustment_reason), params: { adjustment_reason: valid_attributes } + expect(response).to redirect_to(solidus_admin.adjustment_reasons_path) + expect(response).to have_http_status(:see_other) + end + + it "displays a success flash message" do + patch solidus_admin.adjustment_reason_path(adjustment_reason), params: { adjustment_reason: valid_attributes } + follow_redirect! + expect(response.body).to include("Adjustment reason was successfully updated.") + end + end + + context "with invalid parameters" do + let(:invalid_attributes) { { name: "", code: "UPD_ADJ", active: false } } + + it "does not update the adjustment reason" do + original_name = adjustment_reason.name + patch solidus_admin.adjustment_reason_path(adjustment_reason), params: { adjustment_reason: invalid_attributes } + adjustment_reason.reload + expect(adjustment_reason.name).to eq(original_name) + end + + it "renders the edit template with unprocessable_entity status" do + patch solidus_admin.adjustment_reason_path(adjustment_reason), params: { adjustment_reason: invalid_attributes } + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + describe "DELETE /destroy" do + it "deletes the adjustment reason and redirects to the index page with a 303 See Other status" do + # This ensures the adjustment_reason exists prior to deletion. + adjustment_reason + + expect { + delete solidus_admin.adjustment_reason_path(adjustment_reason) + }.to change(Spree::AdjustmentReason, :count).by(-1) + + expect(response).to redirect_to(solidus_admin.adjustment_reasons_path) + expect(response).to have_http_status(:see_other) + end + + it "displays a success flash message after deletion" do + delete solidus_admin.adjustment_reason_path(adjustment_reason) + follow_redirect! + expect(response.body).to include("Adjustment reasons were successfully removed.") + end + end +end diff --git a/admin/spec/requests/solidus_admin/shipping_categories_spec.rb b/admin/spec/requests/solidus_admin/shipping_categories_spec.rb new file mode 100644 index 00000000000..ebe973cb004 --- /dev/null +++ b/admin/spec/requests/solidus_admin/shipping_categories_spec.rb @@ -0,0 +1,132 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "SolidusAdmin::ShippingCategoriesController", type: :request do + let(:admin_user) { create(:admin_user) } + let(:shipping_category) { create(:shipping_category) } + + 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.shipping_categories_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_shipping_category_path + expect(response).to have_http_status(:ok) + end + end + + describe "POST /create" do + context "with valid parameters" do + let(:valid_attributes) { { name: "Express" } } + + it "creates a new ShippingCategory" do + expect { + post solidus_admin.shipping_categories_path, params: { shipping_category: valid_attributes } + }.to change(Spree::ShippingCategory, :count).by(1) + end + + it "redirects to the index page with a 303 See Other status" do + post solidus_admin.shipping_categories_path, params: { shipping_category: valid_attributes } + expect(response).to redirect_to(solidus_admin.shipping_categories_path) + expect(response).to have_http_status(:see_other) + end + + it "displays a success flash message" do + post solidus_admin.shipping_categories_path, params: { shipping_category: valid_attributes } + follow_redirect! + expect(response.body).to include("Shipping category was successfully created.") + end + end + + context "with invalid parameters" do + let(:invalid_attributes) { { name: "" } } + + it "does not create a new ShippingCategory" do + expect { + post solidus_admin.shipping_categories_path, params: { shipping_category: invalid_attributes } + }.not_to change(Spree::ShippingCategory, :count) + end + + it "renders the new template with unprocessable_entity status" do + post solidus_admin.shipping_categories_path, params: { shipping_category: 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_shipping_category_path(shipping_category) + expect(response).to have_http_status(:ok) + end + end + + describe "PATCH /update" do + context "with valid parameters" do + let(:valid_attributes) { { name: "Updated Shipping Category" } } + + it "updates the shipping category" do + patch solidus_admin.shipping_category_path(shipping_category), params: { shipping_category: valid_attributes } + shipping_category.reload + expect(shipping_category.name).to eq("Updated Shipping Category") + end + + it "redirects to the index page with a 303 See Other status" do + patch solidus_admin.shipping_category_path(shipping_category), params: { shipping_category: valid_attributes } + expect(response).to redirect_to(solidus_admin.shipping_categories_path) + expect(response).to have_http_status(:see_other) + end + + it "displays a success flash message" do + patch solidus_admin.shipping_category_path(shipping_category), params: { shipping_category: valid_attributes } + follow_redirect! + expect(response.body).to include("Shipping category was successfully updated.") + end + end + + context "with invalid parameters" do + let(:invalid_attributes) { { name: "" } } + + it "does not update the shipping category" do + original_name = shipping_category.name + patch solidus_admin.shipping_category_path(shipping_category), params: { shipping_category: invalid_attributes } + shipping_category.reload + expect(shipping_category.name).to eq(original_name) + end + + it "renders the edit template with unprocessable_entity status" do + patch solidus_admin.shipping_category_path(shipping_category), params: { shipping_category: invalid_attributes } + expect(response).to have_http_status(:unprocessable_entity) + end + end + end + + describe "DELETE /destroy" do + it "deletes the shipping category and redirects to the index page with a 303 See Other status" do + # Ensure the shipping_category exists before deletion + shipping_category + + expect { + delete solidus_admin.shipping_category_path(shipping_category) + }.to change(Spree::ShippingCategory, :count).by(-1) + + expect(response).to redirect_to(solidus_admin.shipping_categories_path) + expect(response).to have_http_status(:see_other) + end + + it "displays a success flash message after deletion" do + delete solidus_admin.shipping_category_path(shipping_category) + follow_redirect! + expect(response.body).to include("Shipping categories were successfully removed.") + end + end +end