From 7639e9a38d523adc055ca55e45302aee4aa8e5b3 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 10 Feb 2020 13:51:44 +0000 Subject: [PATCH 1/4] Extrac ErrorsParser to separate class and make it handle the rails error structure with keys --- .../admin/bulk_product_update.js.coffee | 11 ++++----- .../utils/services/errors_parser.js.coffee | 17 ++++++++++++++ .../admin/bulk_product_update_spec.js.coffee | 23 +++++++++++++------ 3 files changed, 38 insertions(+), 13 deletions(-) create mode 100644 app/assets/javascripts/admin/utils/services/errors_parser.js.coffee diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index 6d51f2e597..d92bc73566 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -1,4 +1,4 @@ -angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout, $filter, $http, $window, BulkProducts, DisplayProperties, DirtyProducts, VariantUnitManager, StatusMessage, producers, Taxons, Columns, tax_categories, RequestMonitor, SortOptions) -> +angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout, $filter, $http, $window, BulkProducts, DisplayProperties, DirtyProducts, VariantUnitManager, StatusMessage, producers, Taxons, Columns, tax_categories, RequestMonitor, SortOptions, ErrorsParser) -> $scope.StatusMessage = StatusMessage $scope.columns = Columns.columns @@ -230,10 +230,9 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout BulkProducts.updateVariantLists(data.products || []) $timeout -> $scope.displaySuccess() ).error (data, status) -> - if status == 400 && data.errors? && data.errors.length > 0 - errors = error + "\n" for error in data.errors - alert t("products_update_error") + "\n" + errors - $scope.displayFailure t("products_update_error") + if status == 400 && data.errors? + errorsString = ErrorsParser.toString(data.errors, status) + $scope.displayFailure t("products_update_error") + "\n" + errorsString else $scope.displayFailure t("products_update_error_data") + status @@ -284,7 +283,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout $scope.displayFailure = (failMessage) -> - StatusMessage.display 'failure', t("products_update_error_msg") + "#{failMessage}" + StatusMessage.display 'failure', t("products_update_error_msg") + " #{failMessage}" $scope.displayDirtyProducts = -> diff --git a/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee b/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee new file mode 100644 index 0000000000..12c1682d77 --- /dev/null +++ b/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee @@ -0,0 +1,17 @@ +# Parses a structure of errors that came from the server +angular.module("admin.utils").factory "ErrorsParser", -> + new class ErrorsParser + toString: (errors, defaultContent = "") => + if errors.length > 0 + # it is an array of errors + errorsString = error + "\n" for error in errors + else + # it is a hash of errors + keys = Object.keys(errors) + errorsString = "" + for key in keys + errorsString += error for error in errors[key] + + errorsString = defaultContent if errorsString == "" + + errorsString diff --git a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee index 40233684ea..600e0c16e4 100644 --- a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee @@ -710,13 +710,22 @@ describe "AdminProductEditCtrl", -> $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 "displaying the error information when post returns 400", -> + beforeEach -> + spyOn $scope, "displayFailure" + $scope.products = "updated list of products" + + it "displays errors in an array", -> + $httpBackend.expectPOST("/admin/products/bulk_update").respond 400, { "errors": ["an error"] } + $scope.updateProducts "updated list of products" + $httpBackend.flush() + expect($scope.displayFailure).toHaveBeenCalledWith("Saving failed with the following error(s):\nan error\n") + + it "displays errors in a hash", -> + $httpBackend.expectPOST("/admin/products/bulk_update").respond 400, { "errors": { "base": ["a basic error"] } } + $scope.updateProducts "updated list of products" + $httpBackend.flush() + expect($scope.displayFailure).toHaveBeenCalledWith("Saving failed with the following error(s):\na basic error") describe "adding variants", -> From 0aaa04295bcb61e0b3f7e6752daf3285adbace28 Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Mon, 10 Feb 2020 15:15:26 +0000 Subject: [PATCH 2/4] Improve and unit test errorsParser --- .../utils/services/errors_parser.js.coffee | 19 +++++++++++++----- .../admin/bulk_product_update_spec.js.coffee | 2 +- .../services/errors_parser_spec.js.coffee | 20 +++++++++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 spec/javascripts/unit/admin/utils/services/errors_parser_spec.js.coffee diff --git a/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee b/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee index 12c1682d77..87a83360a1 100644 --- a/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee +++ b/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee @@ -2,16 +2,25 @@ angular.module("admin.utils").factory "ErrorsParser", -> new class ErrorsParser toString: (errors, defaultContent = "") => + return defaultContent unless errors? + + errorsString = "" if errors.length > 0 # it is an array of errors - errorsString = error + "\n" for error in errors + errorsString = this.arrayToString(errors) else # it is a hash of errors keys = Object.keys(errors) - errorsString = "" for key in keys - errorsString += error for error in errors[key] + errorsString += this.arrayToString(errors[key]) - errorsString = defaultContent if errorsString == "" + this.defaultIfEmpty(errorsString, defaultContent) - errorsString + arrayToString: (array) => + string = "" + string += entry + "\n" for entry in array + string + + defaultIfEmpty: (content, defaultContent) => + return defaultContent if content == "" + content diff --git a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee index 600e0c16e4..4feff89868 100644 --- a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee @@ -725,7 +725,7 @@ describe "AdminProductEditCtrl", -> $httpBackend.expectPOST("/admin/products/bulk_update").respond 400, { "errors": { "base": ["a basic error"] } } $scope.updateProducts "updated list of products" $httpBackend.flush() - expect($scope.displayFailure).toHaveBeenCalledWith("Saving failed with the following error(s):\na basic error") + expect($scope.displayFailure).toHaveBeenCalledWith("Saving failed with the following error(s):\na basic error\n") describe "adding variants", -> diff --git a/spec/javascripts/unit/admin/utils/services/errors_parser_spec.js.coffee b/spec/javascripts/unit/admin/utils/services/errors_parser_spec.js.coffee new file mode 100644 index 0000000000..9ae47b201a --- /dev/null +++ b/spec/javascripts/unit/admin/utils/services/errors_parser_spec.js.coffee @@ -0,0 +1,20 @@ +describe "ErrorsParser service", -> + errorsParser = null + + beforeEach -> + module('admin.utils') + inject (ErrorsParser) -> + errorsParser = ErrorsParser + + describe "toString", -> + it "returns empty string for nil errors", -> + expect(errorsParser.toString(null)).toEqual "" + + it "returns the elements in the array if an array is provided", -> + expect(errorsParser.toString(["1", "2"])).toEqual "1\n2\n" + + it "returns the elements in the hash if a hash is provided", -> + expect(errorsParser.toString({ "keyname": ["1", "2"] })).toEqual "1\n2\n" + + it "returns all elements in all hash keys provided", -> + expect(errorsParser.toString({ "keyname1": ["1", "2"], "keyname2": ["3", "4"] })).toEqual "1\n2\n3\n4\n" From 2711736004257201f63fa3c52217f952573e288c Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Tue, 18 Feb 2020 10:49:41 +0000 Subject: [PATCH 3/4] Use Array#join and make code simpler --- .../admin/utils/services/errors_parser.js.coffee | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee b/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee index 87a83360a1..f03ce54467 100644 --- a/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee +++ b/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee @@ -7,20 +7,15 @@ angular.module("admin.utils").factory "ErrorsParser", -> errorsString = "" if errors.length > 0 # it is an array of errors - errorsString = this.arrayToString(errors) + errorsString = errors.join("\n") else # it is a hash of errors keys = Object.keys(errors) for key in keys - errorsString += this.arrayToString(errors[key]) + errorsString += errors[key].join("\n") + "\n" this.defaultIfEmpty(errorsString, defaultContent) - arrayToString: (array) => - string = "" - string += entry + "\n" for entry in array - string - defaultIfEmpty: (content, defaultContent) => return defaultContent if content == "" content From 1803ea3c38f454413f4d040ece3eefec0fd56fcd Mon Sep 17 00:00:00 2001 From: Luis Ramos Date: Thu, 20 Feb 2020 10:06:10 +0000 Subject: [PATCH 4/4] Add traling breakline to case where errors come in a array --- .../javascripts/admin/utils/services/errors_parser.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee b/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee index f03ce54467..0e667a0fc0 100644 --- a/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee +++ b/app/assets/javascripts/admin/utils/services/errors_parser.js.coffee @@ -7,7 +7,7 @@ angular.module("admin.utils").factory "ErrorsParser", -> errorsString = "" if errors.length > 0 # it is an array of errors - errorsString = errors.join("\n") + errorsString = errors.join("\n") + "\n" else # it is a hash of errors keys = Object.keys(errors)