From 5fea15e8a93e1ad29d6610b5fd14b6fc4497093a Mon Sep 17 00:00:00 2001 From: Rob H Date: Fri, 8 Aug 2014 14:02:15 +1000 Subject: [PATCH] Better error reporting for failed save on BPE --- .../javascripts/admin/bulk_product_update.js.coffee | 9 +++++++-- .../spree/admin/products_controller_decorator.rb | 6 +++++- .../unit/bulk_product_update_spec.js.coffee | 12 ++++++++++-- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 1b0ec47698..35ec6e694c 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -271,7 +271,12 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", [ $scope.updateVariantLists(data.products) $timeout -> $scope.displaySuccess() ).error (data, status) -> - $scope.displayFailure "Server returned with error status: " + status + if status == 400 && data.errors? && data.errors.length > 0 + errors = error + "\n" for error in data.errors + alert "Saving failed with the following error(s):\n" + errors + $scope.displayFailure "Save failed due to invalid data" + else + $scope.displayFailure "Server returned with error status: " + status $scope.packProduct = (product) -> @@ -336,7 +341,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", [ $scope.displayFailure = (failMessage) -> $scope.setMessage $scope.updateStatusMessage, "Saving failed. " + failMessage, color: "#DA5354" - , 10000 + , false $scope.displayDirtyProducts = -> diff --git a/app/controllers/spree/admin/products_controller_decorator.rb b/app/controllers/spree/admin/products_controller_decorator.rb index 8b3b42feea..9f4fdbe46b 100644 --- a/app/controllers/spree/admin/products_controller_decorator.rb +++ b/app/controllers/spree/admin/products_controller_decorator.rb @@ -33,7 +33,11 @@ Spree::Admin::ProductsController.class_eval do if product_set.save redirect_to "/api/products/bulk_products?page=1;per_page=500;#{bulk_index_query}" else - render :nothing => true, :status => 418 + if product_set.errors.present? + render json: { errors: product_set.errors }, status: 400 + else + render :nothing => true, :status => 500 + end end end diff --git a/spec/javascripts/unit/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/bulk_product_update_spec.js.coffee index 270af2aaeb..b9081a1882 100644 --- a/spec/javascripts/unit/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/bulk_product_update_spec.js.coffee @@ -842,14 +842,22 @@ describe "AdminProductEditCtrl", -> expect(DirtyProducts.clear).toHaveBeenCalled() expect($scope.updateVariantLists).toHaveBeenCalled() - it "runs displayFailure() when post returns error", -> + it "runs displayFailure() when post returns an error", -> spyOn $scope, "displayFailure" $scope.products = "updated list of products" - $httpBackend.expectPOST("/admin/products/bulk_update").respond 404, "updated list of products" + $httpBackend.expectPOST("/admin/products/bulk_update").respond 500, "updated list of products" $scope.updateProducts "updated list of products" $httpBackend.flush() expect($scope.displayFailure).toHaveBeenCalled() + it "shows an alert with error information when post returns 400 with an errors array", -> + spyOn(window, "alert") + $scope.products = "updated list of products" + $httpBackend.expectPOST("/admin/products/bulk_update").respond 400, { "errors": ["an error"] } + $scope.updateProducts "updated list of products" + $httpBackend.flush() + expect(window.alert).toHaveBeenCalledWith("Saving failed with the following error(s):\nan error\n") + describe "fetching a product by id", -> it "returns the product when it is present", -> product = {id: 123}