Remove rescue_from StandardError in Api::BaseController

api/app/controllers/spree/api/base_controller.rb

@@ -16,14 +16,12 @@ class BaseController < ActionController::Base
 
       attr_accessor :current_api_user
 
-      class_attribute :error_notifier
-
       before_action :load_user
       before_action :authorize_for_order, if: proc { order_token.present? }
       before_action :authenticate_user
       before_action :load_user_roles
 
-      rescue_from StandardError, with: :error_during_processing
+      rescue_from ActionController::ParameterMissing, with: :parameter_missing_error
       rescue_from ActiveRecord::RecordNotFound, with: :not_found
       rescue_from CanCan::AccessDenied, with: :unauthorized
       rescue_from Spree::Core::GatewayError, with: :gateway_error
@@ -67,20 +65,19 @@ def unauthorized
         render "spree/api/errors/unauthorized", status: 401
       end
 
-      def error_during_processing(exception)
-        Rails.logger.error exception.message
-        Rails.logger.error exception.backtrace.join("\n")
-
-        error_notifier.call(exception, self) if error_notifier
-
-        render json: { exception: exception.message }, status: 422
-      end
-
       def gateway_error(exception)
         @order.errors.add(:base, exception.message)
         invalid_resource!(@order)
       end
 
+      def parameter_missing_error(exception)
+        render json: {
+          exception: exception.message,
+          error: exception.message,
+          missing_param: exception.param
+        }, status: 422
+      end
+
       def requires_authentication?
         Spree::Api::Config[:requires_authentication]
       end

api/app/controllers/spree/api/checkouts_controller.rb

@@ -115,11 +115,12 @@ def state_callback(before_or_after = :before)
 
       def after_update_attributes
         if params[:order] && params[:order][:coupon_code].present?
-          handler = PromotionHandler::Coupon.new(@order).apply
+          handler = PromotionHandler::Coupon.new(@order)
+          handler.apply
 
           if handler.error.present?
             @coupon_message = handler.error
-            respond_with(@order, default_template: 'spree/api/orders/could_not_apply_coupon')
+            respond_with(@order, default_template: 'spree/api/orders/could_not_apply_coupon', status: 422)
             return true
           end
         end

api/app/controllers/spree/api/resource_controller.rb

@@ -56,6 +56,8 @@ def destroy
     else
       invalid_resource!(@object)
     end
+  rescue ActiveRecord::DeleteRestrictionError
+    render "spree/api/errors/delete_restriction", status: 422
   end
 
   protected

api/app/controllers/spree/api/stock_items_controller.rb

@@ -3,6 +3,8 @@ module Api
     class StockItemsController < Spree::Api::BaseController
       before_action :load_stock_location, only: [:index, :show, :create]
 
+      rescue_from StockLocation::InvalidMovementError, with: :render_stock_items_error
+
       def index
         @stock_items = paginate(scope.ransack(params[:q]).result)
         respond_with(@stock_items)
@@ -78,6 +80,10 @@ def adjust_stock_item_count_on_hand(count_on_hand_adjustment)
         @stock_movement = @stock_location.move(@stock_item.variant, count_on_hand_adjustment, current_api_user)
         @stock_item = @stock_movement.stock_item
       end
+
+      def render_stock_items_error
+        render json: { error: Spree.t(:stock_not_below_zero) }, status: 422
+      end
     end
   end
 end

api/app/views/spree/api/errors/delete_restriction.v1.rabl

@@ -0,0 +1,2 @@
+object false
+node(:error) { I18n.t(:delete_restriction_error, scope: "spree.api") }

api/config/locales/en.yml

@@ -7,6 +7,7 @@ en:
       invalid_resource: "Invalid resource. Please fix errors and try again."
       resource_not_found: "The resource you were looking for could not be found."
       gateway_error: "There was a problem with the payment gateway: %{text}"
+      delete_restriction_error: "Cannot delete record."
       access: "API Access"
       key: "Key"
       clear_key: "Clear key"

api/spec/controllers/spree/api/base_controller_spec.rb

@@ -62,66 +62,10 @@ def index
     end
   end
 
-  it 'chatches StandardError' do
-    expect(subject).to receive(:authenticate_user).and_return(true)
-    expect(subject).to receive(:load_user_roles).and_return(true)
-    expect(subject).to receive(:index).and_raise("no joy")
-    get :index, params: { token: "fake_key" }
-    expect(json_response).to eq({ "exception" => "no joy" })
-    expect(response.content_type).to eq("application/json")
-  end
-
-  it 'raises Exception' do
-    expect(subject).to receive(:authenticate_user).and_return(true)
-    expect(subject).to receive(:load_user_roles).and_return(true)
-    expect(subject).to receive(:index).and_raise(Exception.new("no joy"))
-    expect {
-      get :index, params: { token: "fake_key" }
-    }.to raise_error(Exception, "no joy")
-  end
-
   it "lets a subclass override the product associations that are eager-loaded" do
     expect(controller.respond_to?(:product_includes, true)).to be
   end
 
-  describe '#error_during_processing' do
-    controller(FakesController) do
-      # GET /foo
-      # Simulates a failed API call.
-      def foo
-        raise StandardError
-      end
-    end
-
-    # What would be placed in config/initializers/spree.rb
-    Spree::Api::BaseController.error_notifier = proc do |e, controller|
-      MockHoneybadger.notify_or_ignore(e, rack_env: controller.request.env)
-    end
-
-    ##
-    # Fake HB alert class
-    class MockHoneybadger
-      # https://github.com/honeybadger-io/honeybadger-ruby/blob/master/lib/honeybadger.rb#L136
-      def self.notify_or_ignore(_exception, _opts = {})
-      end
-    end
-
-    before do
-      user = double(email: "[email protected]")
-      allow(user).to receive_message_chain :spree_roles, pluck: []
-      allow(Spree.user_class).to receive_messages find_by: user
-      @routes = ActionDispatch::Routing::RouteSet.new.tap do |r|
-        r.draw { get 'foo' => 'fakes#foo' }
-      end
-    end
-
-    it 'should notify notify_error_during_processing' do
-      expect(MockHoneybadger).to receive(:notify_or_ignore).once.with(kind_of(Exception), rack_env: kind_of(Hash))
-      get :foo, params: { token: 123 }
-      expect(response.status).to eq(422)
-    end
-  end
-
   context 'insufficient stock' do
     before do
       expect(subject).to receive(:authenticate_user).and_return(true)

api/spec/requests/spree/api/checkouts_controller_spec.rb

@@ -349,6 +349,14 @@ module Spree
         expect(PromotionHandler::Coupon).to receive(:new).with(order).and_call_original
         expect_any_instance_of(PromotionHandler::Coupon).to receive(:apply).and_return({ coupon_applied?: true })
         put spree.api_checkout_path(order.to_param), params: { order_token: order.guest_token, order: { coupon_code: "foobar" } }
+        expect(response.status).to eq(200)
+      end
+
+      it "renders error failing to apply coupon" do
+        order.update_column(:state, "payment")
+        put spree.api_checkout_path(order.to_param), params: { order_token: order.guest_token, order: { coupon_code: "foobar" } }
+        expect(response.status).to eq(422)
+        expect(json_response).to eq({ "error" => "The coupon code you entered doesn't exist. Please try again." })
       end
     end
 

api/spec/requests/spree/api/orders_controller_spec.rb

@@ -277,13 +277,6 @@ module Spree
       expect(json_response["checkout_steps"]).to eq(%w[address delivery confirm complete])
     end
 
-    # Regression test for https://github.com/spree/spree/issues/1992
-    it "can view an order not in a standard state" do
-      allow_any_instance_of(Order).to receive_messages user: current_api_user
-      order.update_column(:state, 'shipped')
-      get spree.api_order_path(order)
-    end
-
     it "can not view someone else's order" do
       allow_any_instance_of(Order).to receive_messages user: stub_model(Spree::LegacyUser)
       get spree.api_order_path(order)

api/spec/requests/spree/api/users_controller_spec.rb

@@ -142,8 +142,8 @@ module Spree
       it "cannot destroy user with orders" do
         create(:completed_order_with_totals, user: user)
         delete spree.api_user_path(user)
-        expect(json_response["exception"]).to eq "Spree::Core::DestroyWithOrdersError"
         expect(response.status).to eq(422)
+        expect(json_response).to eq({ "error" => "Cannot delete record." })
       end
     end
   end

backend/app/controllers/spree/admin/users_controller.rb

@@ -1,7 +1,7 @@
 module Spree
   module Admin
     class UsersController < ResourceController
-      rescue_from Spree::Core::DestroyWithOrdersError, with: :user_destroy_with_orders_error
+      rescue_from ActiveRecord::DeleteRestrictionError, with: :user_destroy_with_orders_error
 
       after_action :sign_in_if_change_own_password, only: :update
 

core/app/models/concerns/spree/user_methods.rb

@@ -17,7 +17,7 @@ module UserMethods
       has_many :stock_locations, through: :user_stock_locations
 
       has_many :spree_orders, foreign_key: "user_id", class_name: "Spree::Order"
-      has_many :orders, foreign_key: "user_id", class_name: "Spree::Order"
+      has_many :orders, foreign_key: "user_id", class_name: "Spree::Order", dependent: :restrict_with_exception
 
       has_many :store_credits, -> { includes(:credit_type) }, foreign_key: "user_id", class_name: "Spree::StoreCredit"
       has_many :store_credit_events, through: :store_credits

core/app/models/spree/legacy_user.rb

@@ -8,19 +8,11 @@ class LegacyUser < Spree::Base
 
     self.table_name = 'spree_users'
 
-    before_destroy :check_completed_orders
-
     def self.model_name
       ActiveModel::Name.new Spree::LegacyUser, Spree, 'user'
     end
 
     attr_accessor :password
     attr_accessor :password_confirmation
-
-    private
-
-    def check_completed_orders
-      raise Spree::Core::DestroyWithOrdersError if orders.complete.present?
-    end
   end
 end

core/lib/spree/core.rb

@@ -12,6 +12,8 @@
 require 'ransack'
 require 'state_machines-activerecord'
 
+require 'spree/deprecation'
+
 # This is required because ActiveModel::Validations#invalid? conflicts with the
 # invalid state of a Payment. In the future this should be removed.
 StateMachines::Machine.ignore_method_conflicts = true
@@ -45,7 +47,9 @@ module Core
     autoload :ProductFilters, "spree/core/product_filters"
 
     class GatewayError < RuntimeError; end
-    class DestroyWithOrdersError < StandardError; end
+
+    include ActiveSupport::Deprecation::DeprecatedConstantAccessor
+    deprecate_constant 'DestroyWithOrdersError', ActiveRecord::DeleteRestrictionError, deprecator: Spree::Deprecation
   end
 end
 
@@ -80,6 +84,5 @@ class DestroyWithOrdersError < StandardError; end
 require 'spree/core/role_configuration'
 require 'spree/core/stock_configuration'
 require 'spree/permission_sets'
-require 'spree/deprecation'
 
 require 'spree/core/price_migrator'

core/spec/models/spree/concerns/user_methods_spec.rb

@@ -38,4 +38,23 @@
       it { is_expected.to be_nil }
     end
   end
+
+  describe "deleting user" do
+    context "with no orders" do
+      it "fails validation" do
+        test_user.destroy!
+        expect(test_user).to be_destroyed
+      end
+    end
+
+    context "with an order" do
+      let!(:order) { create(:order, user: test_user) }
+
+      it "fails validation" do
+        expect {
+          test_user.destroy!
+        }.to raise_error(ActiveRecord::DeleteRestrictionError)
+      end
+    end
+  end
 end