From 1fcf797d4e470437b984ab3b4fafd30313cf44e7 Mon Sep 17 00:00:00 2001 From: Cillian O'Ruanaidh Date: Fri, 5 Feb 2021 16:08:08 +0000 Subject: [PATCH] Include sort direction parameter during bulk product update to prevent JS error causing 'Saving' text to hang Before if you did a bulk product update there was an error: > TypeError: Cannot set property 'variants' of null It only seemed to happen if pagination was required i.e. more than 15 products. It seemed to be happening because the default sort order on the products API endpoint which handles the bulk update is 'created desc' but 'name asc' on the /admin/products controller. Another fix included here is for the sorting direction arrows which were not displaying on the admin products page. The sorting arrows require the sorting expression to be on the :sorting var instead of :q.sorting. Fixes #6399 --- .../admin/bulk_product_update.js.coffee | 6 ++++-- .../index_utils/services/sort_options.js.coffee | 8 +++++--- .../admin/bulk_product_update_spec.js.coffee | 2 +- .../services/sort_options_spec.js.coffee | 16 ++++++++++++++++ 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/admin/bulk_product_update.js.coffee b/app/assets/javascripts/admin/bulk_product_update.js.coffee index a805ab67ef..4e2bd45d70 100644 --- a/app/assets/javascripts/admin/bulk_product_update.js.coffee +++ b/app/assets/javascripts/admin/bulk_product_update.js.coffee @@ -21,6 +21,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout sorting: "" } + $scope.sorting = "name asc" $scope.producers = producers $scope.taxons = Taxons.all $scope.tax_categories = tax_categories @@ -48,7 +49,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout 'q[name_cont]': $scope.q.query, 'q[supplier_id_eq]': $scope.q.producerFilter, 'q[primary_taxon_id_eq]': $scope.q.categoryFilter, - 'q[s]': $scope.q.sorting, + 'q[s]': $scope.sorting, import_date: $scope.q.importDateFilter, page: $scope.page, per_page: $scope.per_page @@ -104,7 +105,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout $scope.$watch 'sortOptions', (sort) -> return unless sort && sort.predicate != "" - $scope.q.sorting = sort.getSortingExpr() + $scope.sorting = sort.getSortingExpr(defaultDirection: "asc") $scope.fetchProducts() , true @@ -216,6 +217,7 @@ angular.module("ofn.admin").controller "AdminProductEditCtrl", ($scope, $timeout 'q[name_cont]': $scope.q.query 'q[supplier_id_eq]': $scope.q.producerFilter 'q[primary_taxon_id_eq]': $scope.q.categoryFilter + 'q[s]': $scope.sorting import_date: $scope.q.importDateFilter page: $scope.page per_page: $scope.per_page diff --git a/app/assets/javascripts/admin/index_utils/services/sort_options.js.coffee b/app/assets/javascripts/admin/index_utils/services/sort_options.js.coffee index f926dfa373..b020bda768 100644 --- a/app/assets/javascripts/admin/index_utils/services/sort_options.js.coffee +++ b/app/assets/javascripts/admin/index_utils/services/sort_options.js.coffee @@ -3,9 +3,11 @@ angular.module("admin.indexUtils").factory 'SortOptions', -> predicate: "" reverse: true - getSortingExpr: () -> - sortingExpr = this.predicate + ' desc' if this.reverse - sortingExpr = this.predicate + ' asc' if !this.reverse + getSortingExpr: (options) -> + defaultDirection = if (options && options.defaultDirection) then options.defaultDirection else "desc" + reverseDirection = if defaultDirection == "desc" then "asc" else "desc" + sortingExpr = this.predicate + ' ' + defaultDirection if this.reverse + sortingExpr = this.predicate + ' ' + reverseDirection if !this.reverse sortingExpr toggle: (predicate) -> 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 5b842bfb0a..81ea716f98 100644 --- a/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee +++ b/spec/javascripts/unit/admin/bulk_product_update_spec.js.coffee @@ -368,7 +368,7 @@ describe "AdminProductEditCtrl", -> $scope.sortOptions.toggle('name') $scope.$apply() - expect($scope.q.sorting).toEqual 'name asc' + expect($scope.sorting).toEqual 'name desc' expect($scope.fetchProducts).toHaveBeenCalled() describe "updating the product on hand count", -> diff --git a/spec/javascripts/unit/admin/index_utils/services/sort_options_spec.js.coffee b/spec/javascripts/unit/admin/index_utils/services/sort_options_spec.js.coffee index 6c4b068bc4..6beeade9d1 100644 --- a/spec/javascripts/unit/admin/index_utils/services/sort_options_spec.js.coffee +++ b/spec/javascripts/unit/admin/index_utils/services/sort_options_spec.js.coffee @@ -42,3 +42,19 @@ describe "SortOptions service", -> SortOptions.toggle("column.b") expect(SortOptions.predicate).toEqual "column.b" expect(SortOptions.reverse).toBe false + + describe "getting the sorting expression", -> + describe "when not specifying the default sort direction", -> + it "sets the direction to 'asc' after the first toggle because the default direction is 'desc'", -> + SortOptions.toggle("column.a") + expect(SortOptions.getSortingExpr()).toEqual "column.a asc" + + describe "when specifying the default sorting direction as 'desc'", -> + it "sets the direction to 'asc' after the first toggle", -> + SortOptions.toggle("column.a") + expect(SortOptions.getSortingExpr(defaultDirection: "desc")).toEqual "column.a asc" + + describe "when specifying the default sorting direction as 'asc'", -> + it "sets the direction to 'desc' after the first toggle", -> + SortOptions.toggle("column.a") + expect(SortOptions.getSortingExpr(defaultDirection: "asc")).toEqual "column.a desc"