Fixes for consistent handling of resource errors on admin

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

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

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))

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

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

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.

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

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

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

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

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

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) }

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

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

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

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

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

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

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

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

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