Merge pull request #3590 from mkllnk/3576-bulk-order-spec

3576 Fix bulk order management race condition

app/assets/javascripts/admin/line_items/controllers/line_items_controller.js.coffee

@@ -26,18 +26,21 @@ angular.module("admin.lineItems").controller 'LineItemsCtrl', ($scope, $timeout,
       $scope.startDate = moment(OrderCycles.byID[$scope.orderCycleFilter].orders_open_at).format('YYYY-MM-DD')
       $scope.endDate = moment(OrderCycles.byID[$scope.orderCycleFilter].orders_close_at).startOf('day').format('YYYY-MM-DD')
 
+    formatted_start_date = moment($scope.startDate).format()
+    formatted_end_date = moment($scope.endDate).add(1,'day').format()
+
     RequestMonitor.load $scope.orders = Orders.index(
       "q[state_not_eq]": "canceled",
       "q[completed_at_not_null]": "true",
-      "q[completed_at_gteq]": "#{moment($scope.startDate).format()}",
-      "q[completed_at_lt]": "#{moment($scope.endDate).add(1,'day').format()}"
+      "q[completed_at_gteq]": formatted_start_date,
+      "q[completed_at_lt]": formatted_end_date
     )
 
     RequestMonitor.load $scope.lineItems = LineItems.index(
       "q[order][state_not_eq]": "canceled",
       "q[order][completed_at_not_null]": "true",
-      "q[order][completed_at_gteq]": "#{moment($scope.startDate).format()}",
-      "q[order][completed_at_lt]": "#{moment($scope.endDate).add(1,'day').format()}"
+      "q[order][completed_at_gteq]": formatted_start_date,
+      "q[order][completed_at_lt]": formatted_end_date
     )
 
     unless $scope.initialized
@@ -45,11 +48,9 @@ angular.module("admin.lineItems").controller 'LineItemsCtrl', ($scope, $timeout,
       RequestMonitor.load $scope.orderCycles = OrderCycles.index(ams_prefix: "basic", as: "distributor", "q[orders_close_at_gt]": "#{moment().subtract(90,'days').format()}")
       RequestMonitor.load $scope.suppliers = Enterprises.index(action: "visible", ams_prefix: "basic", "q[is_primary_producer_eq]": "true")
 
-    RequestMonitor.load $q.all([$scope.orders.$promise, $scope.distributors.$promise, $scope.orderCycles.$promise]).then ->
+    RequestMonitor.load $q.all([$scope.orders.$promise, $scope.distributors.$promise, $scope.orderCycles.$promise, $scope.suppliers.$promise, $scope.lineItems.$promise]).then ->
       Dereferencer.dereferenceAttr $scope.orders, "distributor", Enterprises.byID
       Dereferencer.dereferenceAttr $scope.orders, "order_cycle", OrderCycles.byID
-
-    RequestMonitor.load $q.all([$scope.orders.$promise, $scope.suppliers.$promise, $scope.lineItems.$promise]).then ->
       Dereferencer.dereferenceAttr $scope.lineItems, "supplier", Enterprises.byID
       Dereferencer.dereferenceAttr $scope.lineItems, "order", Orders.byID
       $scope.bulk_order_form.$setPristine()
@@ -59,8 +60,6 @@ angular.module("admin.lineItems").controller 'LineItemsCtrl', ($scope, $timeout,
         $timeout ->
           $scope.resetSelectFilters()
 
-  $scope.refreshData()
-
   $scope.$watch 'bulk_order_form.$dirty', (newVal, oldVal) ->
     if newVal == true
       StatusMessage.display 'notice', t('js.unsaved_changes')
@@ -154,3 +153,5 @@ angular.module("admin.lineItems").controller 'LineItemsCtrl', ($scope, $timeout,
     if lineItem.quantity > 0
       lineItem.final_weight_volume = LineItems.pristineByID[lineItem.id].final_weight_volume * lineItem.quantity / LineItems.pristineByID[lineItem.id].quantity
       $scope.weightAdjustedPrice(lineItem)
+
+  $scope.refreshData()

app/assets/javascripts/admin/resources/services/orders.js.coffee

@@ -9,7 +9,7 @@ angular.module("admin.resources").factory 'Orders', ($q, OrderResource, RequestM
       request = OrderResource.index params, (data) =>
         @load(data)
         (callback || angular.noop)(data)
-      RequestMonitor.load(request.$promise)
+      @all.$promise = request.$promise
       @all
 
     load: (data) ->

app/assets/stylesheets/admin/components/ng-cloak.css.scss

@@ -0,0 +1,8 @@
+// https://docs.angularjs.org/api/ng/directive/ngCloak
+[ng-cloak],
+[data-ng-cloak],
+[x-ng-cloak],
+.ng-cloak,
+.x-ng-cloak {
+  display: none !important;
+}

app/views/spree/admin/orders/bulk_management.html.haml

@@ -49,7 +49,7 @@
 
   %hr.divider.sixteen.columns.alpha.omega{ ng: { show: 'unitsVariantSelected()' } }
 
-  %div.sixteen.columns.alpha.omega#group_buy_calculation{ ng: { show: 'unitsVariantSelected()' } }
+  %div.sixteen.columns.alpha.omega#group_buy_calculation{ ng: { show: 'unitsVariantSelected()', cloak: true } }
     %div.shared_resource{ :class => "four columns alpha" }
       %span{ :class => 'three columns alpha' }
         %input{ type: 'checkbox', :id => 'shared_resource', 'ng-model' => 'sharedResource'}
@@ -106,11 +106,11 @@
     %h1
       = t("admin.orders.bulk_management.loading")
 
-  %div{ :class => "sixteen columns alpha", 'ng-show' => '!RequestMonitor.loading && filteredLineItems.length == 0'}
+  %div{ class: "sixteen columns alpha", ng: { show: '!RequestMonitor.loading && filteredLineItems.length == 0', cloak: true } }
     %h1#no_results
       = t("admin.orders.bulk_management.no_results")
 
-  .margin-bottom-50{ 'ng-hide' => 'RequestMonitor.loading || filteredLineItems.length == 0' }
+  .margin-bottom-50{ ng: { hide: 'RequestMonitor.loading || filteredLineItems.length == 0', cloak: true } }
     %form{ name: 'bulk_order_form' }
       %table.index#listing_orders.bulk{ :class => "sixteen columns alpha", ng: { show: "initialized" } }
         %thead
@@ -157,7 +157,7 @@
               = t("admin.orders.bulk_management.ask")
               %input{ :type => 'checkbox', 'ng-model' => "confirmDelete" }
 
-        %tr.line_item{ 'ng-repeat' => "line_item in filteredLineItems = ( lineItems | filter:quickSearch | selectFilter:supplierFilter:distributorFilter:orderCycleFilter | variantFilter:selectedUnitsProduct:selectedUnitsVariant:sharedResource | orderBy:sorting.predicate:sorting.reverse )", 'ng-class-even' => "'even'", 'ng-class-odd' => "'odd'", ng: { attr: { id: "li_{{line_item.id}}" } } }
+        %tr.line_item{ ng: { repeat: "line_item in filteredLineItems = ( lineItems | filter:quickSearch | selectFilter:supplierFilter:distributorFilter:orderCycleFilter | variantFilter:selectedUnitsProduct:selectedUnitsVariant:sharedResource | orderBy:sorting.predicate:sorting.reverse )", 'class-even' => "'even'", 'class-odd' => "'odd'", attr: { id: "li_{{line_item.id}}" } } }
           %td.bulk
             %input{ :type => "checkbox", :name => 'bulk', 'ng-model' => 'line_item.checked', 'ignore-dirty' => true }
           %td.order_no{ 'ng-show' => 'columns.order_no.visible' } {{ line_item.order.number }}

spec/features/admin/bulk_order_management_spec.rb

@@ -10,11 +10,11 @@ feature %q{
 
   context "listing orders" do
     before :each do
-      login_to_admin_section
+      quick_login_as_admin
     end
 
     it "displays a message when number of line items is zero" do
-      visit '/admin/orders/bulk_management'
+      visit_bulk_order_management
       expect(page).to have_text 'No orders found.'
     end
 
@@ -27,7 +27,7 @@ feature %q{
       let!(:li3) { create(:line_item, order: o3 ) }
 
       before :each do
-        visit '/admin/orders/bulk_management'
+        visit_bulk_order_management
       end
 
       it "displays a list of line items" do
@@ -44,8 +44,7 @@ feature %q{
       let!(:li2) { create(:line_item, order: o2, product: create(:product_with_option_types) ) }
 
       before :each do
-        visit '/admin/orders/bulk_management'
-        wait_until { request_monitor_finished 'LineItemsCtrl' }
+        visit_bulk_order_management
       end
 
       it "displays a column for user's full name" do
@@ -92,8 +91,7 @@ feature %q{
       let!(:li2) { create(:line_item, order: o2) }
 
       before do
-        visit spree.admin_bulk_order_management_path
-        wait_until { request_monitor_finished 'LineItemsCtrl' }
+        visit_bulk_order_management
       end
 
       it "sorts by customer name when the customer name header is clicked" do
@@ -123,7 +121,7 @@ feature %q{
 
   context "altering line item properties" do
     before :each do
-      admin_user = quick_login_as_admin
+      quick_login_as_admin
     end
 
     context "tracking changes" do
@@ -131,7 +129,7 @@ feature %q{
       let!(:li1) { create(:line_item, order: o1, :quantity => 5 ) }
 
       before :each do
-        visit '/admin/orders/bulk_management'
+        visit_bulk_order_management
       end
 
       it "adds the class 'ng-dirty' to input elements when value is altered" do
@@ -147,7 +145,7 @@ feature %q{
 
       before :each do
         li1.variant.update_attributes(on_hand: 1, on_demand: false)
-        visit '/admin/orders/bulk_management'
+        visit_bulk_order_management
       end
 
       context "when acceptable data is sent to the server" do
@@ -179,7 +177,7 @@ feature %q{
 
   context "using page controls" do
     before :each do
-      admin_user = quick_login_as_admin
+      quick_login_as_admin
     end
 
     let!(:p1) { create(:product_with_option_types, group_buy: true, group_buy_unit_size: 5000, variant_unit: "weight", variants: [create(:variant, unit_value: 1000)] ) }
@@ -191,7 +189,7 @@ feature %q{
 
     context "modifying the weight/volume of a line item" do
       it "price is altered" do
-        visit '/admin/orders/bulk_management'
+        visit_bulk_order_management
         toggle_columns "Weight/Volume", "Price"
         within "tr#li_#{li1.id}" do
           expect(page).to have_field "price", with: "50.00"
@@ -208,7 +206,7 @@ feature %q{
 
     context "modifying the quantity of a line item" do
       it "price is altered" do
-        visit '/admin/orders/bulk_management'
+        visit_bulk_order_management
         toggle_columns "Price"
         within "tr#li_#{li1.id}" do
           expect(page).to have_field "price", with: "#{format("%.2f",li1.price * 5)}"
@@ -220,7 +218,7 @@ feature %q{
 
     context "modifying the quantity of a line item" do
       it "weight/volume is altered" do
-        visit '/admin/orders/bulk_management'
+        visit_bulk_order_management
         toggle_columns "Weight/Volume"
         within "tr#li_#{li1.id}" do
           expect(page).to have_field "final_weight_volume", with: "#{li1.final_weight_volume.round}"
@@ -232,7 +230,7 @@ feature %q{
 
     context "using column display toggle" do
       it "shows a column display toggle button, which shows a list of columns when clicked" do
-        visit '/admin/orders/bulk_management'
+        visit_bulk_order_management
 
         expect(page).to have_selector "th", :text => "NAME"
         expect(page).to have_selector "th", text: I18n.t("admin.orders.bulk_management.order_date").upcase
@@ -261,8 +259,7 @@ feature %q{
         let!(:li2) { create(:line_item, order: o1, product: create(:product, supplier: s2) ) }
 
         before :each do
-          visit '/admin/orders/bulk_management'
-          wait_until { request_monitor_finished 'LineItemsCtrl' }
+          visit_bulk_order_management
         end
 
         it "displays a select box for producers, which filters line items by the selected supplier" do
@@ -300,8 +297,7 @@ feature %q{
         let!(:li2) { create(:line_item, order: o2 ) }
 
         before :each do
-          visit '/admin/orders/bulk_management'
-          wait_until { request_monitor_finished 'LineItemsCtrl' }
+          visit_bulk_order_management
         end
 
         it "displays a select box for distributors, which filters line items by the selected distributor" do
@@ -319,10 +315,8 @@ feature %q{
         end
 
         it "displays all line items when 'All' is selected from distributor filter" do
-          expect(page).to have_selector "tr#li_#{li1.id}"
           expect(page).to have_selector "tr#li_#{li2.id}"
           select2_select d1.name, from: "distributor_filter"
-          expect(page).to have_selector "tr#li_#{li1.id}"
           expect(page).to have_no_selector "tr#li_#{li2.id}"
           select2_select "All", from: "distributor_filter"
           expect(page).to have_selector "tr#li_#{li1.id}"
@@ -340,8 +334,7 @@ feature %q{
         let!(:li2) { create(:line_item, order: o2 ) }
 
         before do
-          visit '/admin/orders/bulk_management'
-          wait_until { request_monitor_finished 'LineItemsCtrl' }
+          visit_bulk_order_management
         end
 
         it "displays a select box for order cycles, which filters line items by the selected order cycle" do
@@ -381,8 +374,7 @@ feature %q{
         let!(:li2) { create(:line_item, order: o2, product: p2 ) }
 
         before :each do
-          visit '/admin/orders/bulk_management'
-          wait_until { request_monitor_finished 'LineItemsCtrl' }
+          visit_bulk_order_management
         end
 
         it "allows filters to be used in combination" do
@@ -432,8 +424,7 @@ feature %q{
       let!(:li3) { create(:line_item, order: o3 ) }
 
       before :each do
-        visit '/admin/orders/bulk_management'
-        wait_until { request_monitor_finished 'LineItemsCtrl' }
+        visit_bulk_order_management
       end
 
       it "displays a quick search input" do
@@ -462,8 +453,7 @@ feature %q{
       let!(:li4) { create(:line_item, order: o4, :quantity => 4 ) }
 
       before :each do
-        visit '/admin/orders/bulk_management'
-        wait_until { request_monitor_finished 'LineItemsCtrl' }
+        visit_bulk_order_management
       end
 
       it "displays date fields for filtering orders, with default values set" do
@@ -535,8 +525,7 @@ feature %q{
       let!(:li2) { create(:line_item, order: o2 ) }
 
       before :each do
-        visit '/admin/orders/bulk_management'
-        wait_until { request_monitor_finished 'LineItemsCtrl' }
+        visit_bulk_order_management
       end
 
       it "displays a checkbox for each line item in the list" do
@@ -608,7 +597,7 @@ feature %q{
         let!(:li2) { create(:line_item, order: o2 ) }
 
         before :each do
-          visit '/admin/orders/bulk_management'
+          visit_bulk_order_management
         end
 
         it "shows an edit button for line_items, which takes the user to the standard edit page for the order" do
@@ -642,7 +631,7 @@ feature %q{
         let!(:li2) { create(:line_item, order: o2 ) }
 
         before :each do
-          visit '/admin/orders/bulk_management'
+          visit_bulk_order_management
         end
 
         it "removes a line item when the relevant delete button is clicked" do
@@ -652,7 +641,7 @@ feature %q{
           end
           expect(page).to have_no_selector "a.delete-line-item", :count => 2
           expect(page).to have_selector "a.delete-line-item", :count => 1
-          visit '/admin/orders/bulk_management'
+          visit_bulk_order_management
           expect(page).to have_selector "a.delete-line-item", :count => 1
         end
       end
@@ -670,7 +659,7 @@ feature %q{
       let!(:li4) { create(:line_item, order: o2, variant: v3, quantity: 1, max_quantity: 3 ) }
 
       before :each do
-        visit '/admin/orders/bulk_management'
+        visit_bulk_order_management
         within "tr#li_#{li3.id}" do
           find("a", text: li3.product.name + ": " + li3.variant.options_text).click
         end
@@ -746,13 +735,18 @@ feature %q{
     end
 
     it "shows only line item from orders that I distribute, and not those that I supply" do
-      visit '/admin/orders/bulk_management'
+      visit_bulk_order_management
 
       expect(page).to have_selector "tr#li_#{line_item_distributed.id}", :visible => true
       expect(page).to have_no_selector "tr#li_#{line_item_not_distributed.id}", :visible => true
     end
   end
 
+  def visit_bulk_order_management
+    visit spree.admin_bulk_order_management_path
+    expect(page).to have_no_text 'Loading orders'
+  end
+
   def select_date(date)
     # Wait for datepicker to open and be associated to the datepicker trigger.
     expect(page).to have_selector("#ui-datepicker-div")