diff --git a/backend/app/controllers/spree/admin/base_controller.rb b/backend/app/controllers/spree/admin/base_controller.rb index d87d62b19b1..f44ac0f894e 100644 --- a/backend/app/controllers/spree/admin/base_controller.rb +++ b/backend/app/controllers/spree/admin/base_controller.rb @@ -62,6 +62,12 @@ def lock_order def order_mutex_redirect_path edit_admin_order_path(@order) end + + def resource_not_found(flash_class:, redirect_url:) + flash[:error] = flash_message_for(flash_class.new, :not_found) + redirect_to redirect_url + nil + end end end end diff --git a/backend/app/controllers/spree/admin/cancellations_controller.rb b/backend/app/controllers/spree/admin/cancellations_controller.rb index 3e7ac13e19b..b6fe42f8c27 100644 --- a/backend/app/controllers/spree/admin/cancellations_controller.rb +++ b/backend/app/controllers/spree/admin/cancellations_controller.rb @@ -36,6 +36,8 @@ def created_by def load_order @order = Spree::Order.find_by!(number: params[:order_id]) authorize! action, @order + rescue ActiveRecord::RecordNotFound + resource_not_found(flash_class: Spree::Order, redirect_url: admin_orders_path) end def model_class diff --git a/backend/app/controllers/spree/admin/customer_returns_controller.rb b/backend/app/controllers/spree/admin/customer_returns_controller.rb index a1201399f68..cb6c204ef91 100644 --- a/backend/app/controllers/spree/admin/customer_returns_controller.rb +++ b/backend/app/controllers/spree/admin/customer_returns_controller.rb @@ -42,6 +42,8 @@ def find_resource def collection parent # trigger loading the order + return unless @order + @collection ||= Spree::ReturnItem .accessible_by(current_ability, :read) .where(inventory_unit_id: @order.inventory_units.pluck(:id)) diff --git a/backend/app/controllers/spree/admin/images_controller.rb b/backend/app/controllers/spree/admin/images_controller.rb index 13a2df3d64e..f281da56ac2 100644 --- a/backend/app/controllers/spree/admin/images_controller.rb +++ b/backend/app/controllers/spree/admin/images_controller.rb @@ -4,7 +4,6 @@ module Spree module Admin class ImagesController < ResourceController before_action :load_data - create.before :set_viewable update.before :set_viewable @@ -24,6 +23,8 @@ def load_data [variant.sku_and_options_text, variant.id] end @variants.insert(0, [t('spree.all'), @product.master.id]) + rescue ActiveRecord::RecordNotFound + resource_not_found(flash_class: Spree::Product, redirect_url: admin_products_path) end def set_viewable diff --git a/backend/app/controllers/spree/admin/orders/customer_details_controller.rb b/backend/app/controllers/spree/admin/orders/customer_details_controller.rb index 37f74502cc0..734afa57809 100644 --- a/backend/app/controllers/spree/admin/orders/customer_details_controller.rb +++ b/backend/app/controllers/spree/admin/orders/customer_details_controller.rb @@ -61,6 +61,8 @@ def order_params def load_order @order = Spree::Order.includes(:adjustments).find_by!(number: params[:order_id]) + rescue ActiveRecord::RecordNotFound + resource_not_found(flash_class: Spree::Order, redirect_url: admin_orders_path) end def model_class diff --git a/backend/app/controllers/spree/admin/orders_controller.rb b/backend/app/controllers/spree/admin/orders_controller.rb index 08fafabeb45..f8787479f8a 100644 --- a/backend/app/controllers/spree/admin/orders_controller.rb +++ b/backend/app/controllers/spree/admin/orders_controller.rb @@ -10,7 +10,6 @@ class OrdersController < Spree::Admin::BaseController around_action :lock_order, only: [:update, :advance, :complete, :confirm, :cancel, :resume, :approve, :resend] rescue_from Spree::Order::InsufficientStock, with: :insufficient_stock_error - respond_to :html def index @@ -168,6 +167,8 @@ def order_params def load_order @order = Spree::Order.includes(:adjustments).find_by!(number: params[:id]) authorize! action, @order + rescue ActiveRecord::RecordNotFound + resource_not_found(flash_class: Spree::Order, redirect_url: admin_orders_path) end # Used for extensions which need to provide their own custom event links on the order details view. diff --git a/backend/app/controllers/spree/admin/payments_controller.rb b/backend/app/controllers/spree/admin/payments_controller.rb index 0a60ee9ea2a..eda6ca65665 100644 --- a/backend/app/controllers/spree/admin/payments_controller.rb +++ b/backend/app/controllers/spree/admin/payments_controller.rb @@ -92,6 +92,8 @@ def load_order @order = Spree::Order.find_by!(number: params[:order_id]) authorize! action, @order @order + rescue ActiveRecord::RecordNotFound + resource_not_found(flash_class: Spree::Order, redirect_url: admin_orders_path) end def load_payment diff --git a/backend/app/controllers/spree/admin/resource_controller.rb b/backend/app/controllers/spree/admin/resource_controller.rb index 8406cf97d50..dbdcbd467b5 100644 --- a/backend/app/controllers/spree/admin/resource_controller.rb +++ b/backend/app/controllers/spree/admin/resource_controller.rb @@ -5,7 +5,9 @@ class Spree::Admin::ResourceController < Spree::Admin::BaseController helper_method :new_object_url, :edit_object_url, :object_url, :collection_url before_action :load_resource, except: :update_positions - rescue_from ActiveRecord::RecordNotFound, with: :resource_not_found + rescue_from ActiveRecord::RecordNotFound do + resource_not_found + end rescue_from ActiveRecord::RecordInvalid, with: :resource_invalid respond_to :html @@ -134,9 +136,8 @@ def belongs_to(model_name, options = {}) end end - def resource_not_found - flash[:error] = flash_message_for(model_class.new, :not_found) - redirect_to collection_url + def resource_not_found(flash_class: model_class, redirect_url: collection_url) + super end def model_class @@ -193,12 +194,14 @@ def parent if parent? @parent ||= self.class.parent_data[:model_class] .includes(self.class.parent_data[:includes]) - .find_by(self.class.parent_data[:find_by] => params["#{parent_model_name}_id"]) + .find_by!(self.class.parent_data[:find_by] => params["#{parent_model_name}_id"]) instance_variable_set("@#{parent_model_name}", @parent) else Spree::Deprecation.warn "Calling #parent is deprecated on a ResourceController which has not defined a belongs_to" nil end + rescue ActiveRecord::RecordNotFound => e + resource_not_found(flash_class: e.model.constantize, redirect_url: spree.polymorphic_url([:admin, parent_model_name.pluralize])) end def parent? @@ -222,7 +225,8 @@ def build_resource end def collection - return parent.send(controller_name) if parent? + return parent.send(controller_name) if parent? && parent + if model_class.respond_to?(:accessible_by) && !current_ability.has_block?(params[:action], model_class) model_class.accessible_by(current_ability, action) else diff --git a/backend/app/controllers/spree/admin/taxons_controller.rb b/backend/app/controllers/spree/admin/taxons_controller.rb index 51005b3604e..1271b3af123 100644 --- a/backend/app/controllers/spree/admin/taxons_controller.rb +++ b/backend/app/controllers/spree/admin/taxons_controller.rb @@ -3,6 +3,7 @@ module Spree module Admin class TaxonsController < Spree::Admin::BaseController + rescue_from ActiveRecord::RecordNotFound, with: :resource_not_found respond_to :html, :json, :js def index @@ -83,6 +84,10 @@ def destroy def taxon_params params.require(:taxon).permit(permitted_taxon_attributes) end + + def resource_not_found + super(flash_class: Taxon, redirect_url: admin_taxonomies_path) + end end end end diff --git a/backend/app/controllers/spree/admin/variants_controller.rb b/backend/app/controllers/spree/admin/variants_controller.rb index 002187290ee..8c1ff0861ce 100644 --- a/backend/app/controllers/spree/admin/variants_controller.rb +++ b/backend/app/controllers/spree/admin/variants_controller.rb @@ -43,8 +43,10 @@ def redirect_on_empty_option_values end def parent - @parent ||= Spree::Product.with_discarded.find_by(slug: params[:product_id]) + @parent ||= Spree::Product.with_discarded.find_by!(slug: params[:product_id]) @product = @parent + rescue ActiveRecord::RecordNotFound + resource_not_found(flash_class: Spree::Product, redirect_url: admin_products_path) end end end diff --git a/backend/spec/controllers/spree/admin/cancellations_controller_spec.rb b/backend/spec/controllers/spree/admin/cancellations_controller_spec.rb index a0a5c9a85f7..88dc509ca08 100644 --- a/backend/spec/controllers/spree/admin/cancellations_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/cancellations_controller_spec.rb @@ -17,6 +17,14 @@ expect(response).to be_ok end end + + context "existent order id not given" do + it "redirects and flashes about the non-existent order" do + get :index, params: { order_id: 'non-existent-order' } + expect(response).to redirect_to(spree.admin_orders_path) + expect(flash[:error]).to eql("Order is not found") + end + end end describe "#cancel" do diff --git a/backend/spec/controllers/spree/admin/customer_returns_controller_spec.rb b/backend/spec/controllers/spree/admin/customer_returns_controller_spec.rb index c1f67bb9b94..ddbacb73e93 100644 --- a/backend/spec/controllers/spree/admin/customer_returns_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/customer_returns_controller_spec.rb @@ -26,6 +26,14 @@ module Admin end end + context "existent order id not given" do + it "redirects and flashes about the non-existent order" do + get :index, params: { order_id: 'non-existent-order' } + expect(response).to redirect_to(spree.admin_orders_path) + expect(flash[:error]).to eql("Order is not found") + end + end + describe "#new" do let(:order) { create(:shipped_order, line_items_count: 1) } let!(:inactive_reimbursement_type) { create(:reimbursement_type, active: false) } diff --git a/backend/spec/controllers/spree/admin/images_controller_spec.rb b/backend/spec/controllers/spree/admin/images_controller_spec.rb new file mode 100644 index 00000000000..629d32da62a --- /dev/null +++ b/backend/spec/controllers/spree/admin/images_controller_spec.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Spree::Admin::ImagesController, type: :controller do + stub_authorization! + + it "cannot find a non-existent product" do + get :index, params: { product_id: "non-existent-product" } + expect(response).to redirect_to(spree.admin_products_path) + expect(flash[:error]).to eql("Product is not found") + end +end diff --git a/backend/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb b/backend/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb index 6710d7a0962..e78bf168c20 100644 --- a/backend/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/orders/customer_details_controller_spec.rb @@ -53,6 +53,13 @@ end end end + context "existent order id not given" do + it "redirects and flashes about the non-existent order" do + get :edit, params: { order_id: 'non-existent-order' } + expect(response).to redirect_to(spree.admin_orders_path) + expect(flash[:error]).to eql("Order is not found") + end + end end context "#update" do diff --git a/backend/spec/controllers/spree/admin/orders_controller_spec.rb b/backend/spec/controllers/spree/admin/orders_controller_spec.rb index de57fb1cf65..899821611be 100644 --- a/backend/spec/controllers/spree/admin/orders_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/orders_controller_spec.rb @@ -396,13 +396,13 @@ end end - context "order number not given" do + context "existent order number not given" do stub_authorization! - it "raise active record not found" do - expect { - get :edit, params: { id: 0 } - }.to raise_error ActiveRecord::RecordNotFound + it "cannot find non-existent order" do + get :edit, params: { id: 0 } + expect(response).to redirect_to(spree.admin_orders_path) + expect(flash[:error]).to eql("Order is not found") end end end diff --git a/backend/spec/controllers/spree/admin/payments_controller_spec.rb b/backend/spec/controllers/spree/admin/payments_controller_spec.rb index fabae0d7374..1cc20671f5e 100644 --- a/backend/spec/controllers/spree/admin/payments_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/payments_controller_spec.rb @@ -152,6 +152,14 @@ module Admin expect(response).to redirect_to(spree.edit_admin_order_customer_path(order)) end end + + context "existent order id not given" do + it "redirects and flashes about the non-existent order" do + get :index, params: { order_id: 'non-existent-order' } + expect(response).to redirect_to(spree.admin_orders_path) + expect(flash[:error]).to eql("Order is not found") + end + end end describe '#fire' do diff --git a/backend/spec/controllers/spree/admin/prices_controller_spec.rb b/backend/spec/controllers/spree/admin/prices_controller_spec.rb index 0a865c22f77..21471b0143b 100644 --- a/backend/spec/controllers/spree/admin/prices_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/prices_controller_spec.rb @@ -41,5 +41,14 @@ expect(assigns(:product)).to eq(product) end end + context "existent product id not given" do + subject { get :index, params: { product_id: 'non-existent-product' } } + + it "cannot find non-existent product" do + subject + expect(response).to redirect_to(spree.admin_products_path) + expect(flash[:error]).to eql("Product is not found") + end + end end end diff --git a/backend/spec/controllers/spree/admin/product_properties_controller_spec.rb b/backend/spec/controllers/spree/admin/product_properties_controller_spec.rb index 69b4e248034..b7eab913634 100644 --- a/backend/spec/controllers/spree/admin/product_properties_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/product_properties_controller_spec.rb @@ -64,6 +64,21 @@ module Admin expect(assigns(:variant_property_rule)).to eq first_rule end end + + context "existent product id not given" do + let(:parameters) do + { + product_id: 'non-existent-product' + } + end + + before { subject } + + it "cannot find non-existent product" do + expect(response).to redirect_to(spree.admin_products_path) + expect(flash[:error]).to eql("Product is not found") + end + end end end end diff --git a/backend/spec/controllers/spree/admin/store_credits_controller_spec.rb b/backend/spec/controllers/spree/admin/store_credits_controller_spec.rb index 66e0d6efe01..aa3b775795e 100644 --- a/backend/spec/controllers/spree/admin/store_credits_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/store_credits_controller_spec.rb @@ -308,4 +308,12 @@ end end end + context 'User does not exist' do + let(:store_credit) { create(:store_credit, user: user, category: a_credit_category) } + it "cannot find a store-credit for non-existent user" do + get :index, params: { user_id: 'non-existent-user', id: store_credit.id } + expect(response).to redirect_to(spree.admin_users_path) + expect(flash[:error]).to eql("User is not found") + end + end end diff --git a/backend/spec/controllers/spree/admin/taxons_controller_spec.rb b/backend/spec/controllers/spree/admin/taxons_controller_spec.rb new file mode 100644 index 00000000000..41d84efe002 --- /dev/null +++ b/backend/spec/controllers/spree/admin/taxons_controller_spec.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Spree::Admin::TaxonsController, type: :controller do + stub_authorization! + + describe "#edit" do + let!(:taxon) { create(:taxon) } + + it "finds existing taxon" do + get :edit, params: { taxonomy_id: taxon.taxonomy, id: taxon.id } + expect(response.status).to eq(200) + end + + it "cannot find a non-existing taxon with existent taxonomy" do + get :edit, params: { taxonomy_id: taxon.taxonomy, id: 'non-existent-taxon' } + expect(response).to redirect_to(spree.admin_taxonomies_path) + expect(flash[:error]).to eql("Taxon is not found") + end + + it "cannot find a existing taxon with non-existent taxonomy" do + get :edit, params: { taxonomy_id: 'non-existent-taxonomy', id: taxon.id } + expect(response).to redirect_to(spree.admin_taxonomies_path) + expect(flash[:error]).to eql("Taxon is not found") + end + end +end diff --git a/backend/spec/controllers/spree/admin/variants_controller_spec.rb b/backend/spec/controllers/spree/admin/variants_controller_spec.rb index 3b971940a8a..92b217dec9a 100644 --- a/backend/spec/controllers/spree/admin/variants_controller_spec.rb +++ b/backend/spec/controllers/spree/admin/variants_controller_spec.rb @@ -53,6 +53,15 @@ module Admin expect(assigns(:collection)).to include deleted_variant end end + context "existent product id not given" do + let(:params) { { product_id: 'non-existent-product' } } + + it "cannot find non-existent product" do + subject + expect(response).to redirect_to(spree.admin_products_path) + expect(flash[:error]).to eql("Product is not found") + end + end end end end