Refactoring AngularJS Shop Variant filtering logic for improved speed

app/assets/javascripts/darkswarm/controllers/cart_controller.js.coffee

@@ -1,10 +1,2 @@
 Darkswarm.controller "CartCtrl", ($scope, Cart, $timeout) ->
   $scope.Cart = Cart
-  initializing = true
-
-  $scope.$watchCollection "Cart.line_items_present()", ->
-    if initializing
-      $timeout ->
-        initializing = false
-    else
-      $scope.Cart.orderChanged()

app/assets/javascripts/darkswarm/controllers/line_item_controller.js.coffee

@@ -1,5 +0,0 @@
-Darkswarm.controller "LineItemCtrl", ($scope)->
-  $scope.$watch '[line_item.quantity, line_item.max_quantity]', (newValue, oldValue)->
-    if newValue != oldValue
-      $scope.Cart.orderChanged()
-  , true

app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee

@@ -7,15 +7,35 @@ Darkswarm.controller "ProductsCtrl", ($scope, $filter, $rootScope, Products, Ord
   $scope.filtersActive = true
   $scope.limit = 10
   $scope.order_cycle = OrderCycle.order_cycle
+  # $scope.infiniteDisabled = true
+
+  # All of this logic basically just replicates the functionality filtering an ng-repeat
+  # except that it allows us to filter a separate list before rendering, meaning that
+  # we can get much better performance when applying filters by resetting the limit on the
+  # number of products being rendered each time a filter is changed.
 
   $scope.$watch "Products.loading", (newValue, oldValue) ->
+    $scope.updateFilteredProducts()
     $scope.$broadcast("loadFilterSelectors") if !newValue
 
   $scope.incrementLimit = ->
-    $scope.limit += 10 if $scope.limit < Products.products.length
+    if $scope.limit < Products.products.length
+      $scope.limit += 10
+      $scope.updateVisibleProducts()
 
-  $scope.$watchGroup ['query','taxonSelectors','propertySelectors'], ->
+  $scope.$watch 'query', -> $scope.updateFilteredProducts()
+  $scope.$watchCollection 'activeTaxons', -> $scope.updateFilteredProducts()
+  $scope.$watchCollection 'activeProperties', -> $scope.updateFilteredProducts()
+
+  $scope.updateFilteredProducts = ->
     $scope.limit = 10
+    f1 = $filter('products')(Products.products, $scope.query)
+    f2 = $filter('taxons')(f1, $scope.activeTaxons)
+    $scope.filteredProducts = $filter('properties')(f2, $scope.activeProperties)
+    $scope.updateVisibleProducts()
+
+  $scope.updateVisibleProducts = ->
+    $scope.visibleProducts = $filter('limitTo')($scope.filteredProducts, $scope.limit)
 
   $scope.searchKeypress = (e)->
     code = e.keyCode || e.which

app/assets/javascripts/darkswarm/directives/shop_variant.js.coffee

@@ -1,6 +1,9 @@
-Darkswarm.directive "shopVariant",  ->
+Darkswarm.directive "shopVariant", ->
   restrict: 'E'
   replace: true
   templateUrl: 'shop_variant.html'
   scope:
     variant: '='
+  controller: ($scope, Cart) ->
+    $scope.$watchGroup ['variant.line_item.quantity', 'variant.line_item.max_quantity'], ->
+      Cart.adjust($scope.variant.line_item)

app/assets/javascripts/darkswarm/services/cart.js.coffee

@@ -11,7 +11,15 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, $modal, $roo
       for line_item in @line_items
         line_item.variant.line_item = line_item
         Variants.register line_item.variant
-        line_item.variant.extended_name = @extendedVariantName(line_item.variant)
+
+    adjust: (line_item) =>
+      line_item.total_price = line_item.variant.price_with_fees * line_item.quantity
+      if line_item.quantity > 0
+        @line_items.push line_item unless line_item in @line_items
+      else
+        index = @line_items.indexOf(line_item)
+        @line_items.splice(index, 1) if index >= 0
+      @orderChanged()
 
     orderChanged: =>
       @unsaved()
@@ -48,7 +56,7 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, $modal, $roo
       # TODO: These changes to quantity/max_quantity trigger another cart update, which
       #       is unnecessary.
 
-      for li in @line_items_present()
+      for li in @line_items when li.quantity > 0
         if stockLevels[li.variant.id]?
           li.variant.count_on_hand = stockLevels[li.variant.id].on_hand
           if li.quantity > li.variant.count_on_hand
@@ -67,7 +75,7 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, $modal, $roo
 
     data: =>
       variants = {}
-      for li in @line_items_present()
+      for li in @line_items when li.quantity > 0
         variants[li.variant.id] =
           quantity: li.quantity
           max_quantity: li.max_quantity
@@ -89,45 +97,21 @@ Darkswarm.factory 'Cart', (CurrentOrder, Variants, $timeout, $http, $modal, $roo
       $(window).bind "beforeunload", ->
         t 'order_not_saved_yet'
 
-    line_items_present: =>
-      @line_items.filter (li)->
-        li.quantity > 0
-
     total_item_count: =>
-      @line_items_present().reduce (sum,li) ->
+      @line_items.reduce (sum,li) ->
         sum = sum + li.quantity
       , 0
 
     empty: =>
-      @line_items_present().length == 0
+      @line_items.length == 0
 
     total: =>
-      @line_items_present().map (li)->
-        li.variant.totalPrice()
+      @line_items.map (li)->
+        li.total_price
       .reduce (t, price)->
         t + price
       , 0
 
-    register_variant: (variant)=>
-      exists = @line_items.some (li)-> li.variant == variant
-      @create_line_item(variant) unless exists
-
     clear: ->
       @line_items = []
       storage.clearAll() # One day this will have to be moar GRANULAR
-
-    create_line_item: (variant)->
-      variant.extended_name = @extendedVariantName(variant)
-      variant.line_item =
-        variant: variant
-        quantity: null
-        max_quantity: null
-      @line_items.push variant.line_item
-
-    extendedVariantName: (variant) =>
-      if variant.product_name == variant.name_to_display
-        variant.product_name
-      else
-        name =  "#{variant.product_name} - #{variant.name_to_display}"
-        name += " (#{variant.options_text})" if variant.options_text
-        name

app/assets/javascripts/darkswarm/services/products.js.coffee

@@ -17,7 +17,6 @@ Darkswarm.factory 'Products', ($resource, Enterprises, Dereferencer, Taxons, Pro
         @extend()
         @dereference()
         @registerVariants()
-        @registerVariantsWithCart()
         @loading = false
 
     extend: ->
@@ -44,15 +43,7 @@ Darkswarm.factory 'Products', ($resource, Enterprises, Dereferencer, Taxons, Pro
     registerVariants: ->
       for product in @products
         if product.variants
-          product.variants = (Variants.register variant for variant in product.variants)
-          variant.product = product for variant in product.variants
-        if product.master
-          product.master.product = product
-          product.master = Variants.register product.master
-
-    registerVariantsWithCart: ->
-      for product in @products
-        if product.variants
-          for variant in product.variants
-            Cart.register_variant variant
-        Cart.register_variant product.master if product.master
+          product.variants = for variant in product.variants
+            variant = Variants.register variant
+            variant.product = product
+            variant

app/assets/javascripts/darkswarm/services/variants.js.coffee

@@ -9,8 +9,21 @@ Darkswarm.factory 'Variants', ->
       @variants[variant.id] ||= @extend variant
 
     extend: (variant)->
-      # Add totalPrice method to calculate line item total. This should be on a line item!
-      variant.totalPrice = ->
-        variant.price_with_fees * variant.line_item.quantity
-      variant.basePricePercentage = Math.round(variant.price / variant.price_with_fees * 100)
+      variant.extended_name = @extendedVariantName(variant)
+      variant.base_price_percentage = Math.round(variant.price / variant.price_with_fees * 100)
+      variant.line_item ||= @lineItemFor(variant) # line_item may have been initialised in Cart#constructor
+      variant.line_item.total_price = variant.price_with_fees * variant.line_item.quantity
       variant
+
+    extendedVariantName: (variant) =>
+      if variant.product_name == variant.name_to_display
+        variant.product_name
+      else
+        name =  "#{variant.product_name} - #{variant.name_to_display}"
+        name += " (#{variant.options_text})" if variant.options_text
+        name
+
+    lineItemFor: (variant) ->
+      variant: variant
+      quantity: null
+      max_quantity: null

app/assets/javascripts/templates/price_breakdown.html.haml

@@ -2,7 +2,7 @@
   %span.joyride-nub.right
   .joyride-content-wrapper
     .collapsed{"ng-show" => "!expanded"}
-      %price-percentage{percentage: 'variant.basePricePercentage'}
+      %price-percentage{percentage: 'variant.base_price_percentage'}
       %a{"ng-click" => "expanded = !expanded"}
         %span{"ng-bind" => "::'price_breakdown' | t"}
         %i.ofn-i_005-caret-down
@@ -10,26 +10,26 @@
     .expanded{"ng-show" => "expanded"}
       %ul
         %li.cost
-          .right {{ variant.price | localizeCurrency }}
+          .right {{ ::variant.price | localizeCurrency }}
           %span{"ng-bind" => "::'item_cost' | t"}
         %li.admin-fee{"ng-if" => "::variant.fees.admin"}
-          .right {{ variant.fees.admin | localizeCurrency }}
+          .right {{ ::variant.fees.admin | localizeCurrency }}
           %span{"ng-bind" => "::'admin_fee' | t"}
         %li.sales-fee{"ng-if" => "::variant.fees.sales"}
-          .right {{ variant.fees.sales | localizeCurrency }}
+          .right {{ ::variant.fees.sales | localizeCurrency }}
           %span{"ng-bind" => "::'sales_fee' | t"}
         %li.packing-fee{"ng-if" => "::variant.fees.packing"}
-          .right {{ variant.fees.packing | localizeCurrency }}
+          .right {{ ::variant.fees.packing | localizeCurrency }}
           %span{"ng-bind" => "::'packing_fee' | t"}
         %li.transport-fee{"ng-if" => "::variant.fees.transport"}
-          .right {{ variant.fees.transport | localizeCurrency }}
+          .right {{ ::variant.fees.transport | localizeCurrency }}
           %span{"ng-bind" => "::'transport_fee' | t"}
         %li.fundraising-fee{"ng-if" => "::variant.fees.fundraising"}
-          .right {{ variant.fees.fundraising | localizeCurrency }}
+          .right {{ ::variant.fees.fundraising | localizeCurrency }}
           %span{"ng-bind" => "::'fundraising_fee' | t"}
         %li.total
           %strong
-            .right = {{ variant.price_with_fees | localizeCurrency }}
+            .right = {{ ::variant.price_with_fees | localizeCurrency }}
              
 
       %a{"ng-click" => "expanded = !expanded"}

app/assets/javascripts/templates/price_percentage.html.haml

@@ -1,5 +1,4 @@
 .progress
-  .right {{'fees' | t}}
+  .right {{::'fees' | t}}
   .meter
-    {{'item_cost' | t}}
-  
+    {{::'item_cost' | t}}

app/assets/javascripts/templates/shop_variant.html.haml

@@ -27,5 +27,5 @@
 
   .small-12.medium-2.large-2.columns.total-price.text-right
     .table-cell
-      %strong{"ng-class" => "{filled: variant.totalPrice()}"}
-        {{ variant.totalPrice() | localizeCurrency }}
+      %strong{"ng-class" => "{filled: variant.line_item.total_price}"}
+        {{ variant.line_item.total_price | localizeCurrency }}

app/views/shared/menu/_cart.html.haml

@@ -17,8 +17,7 @@
         %a.button.primary.tiny{href: checkout_path, "ng-disabled" => "Cart.dirty || Cart.empty()"}
           = t 'checkout'
       %table
-        %tr.product-cart{"ng-repeat" => "line_item in Cart.line_items_present()",
-        "ng-controller" => "LineItemCtrl", "id" => "cart-variant-{{ line_item.variant.id }}"}
+        %tr.product-cart{"ng-repeat" => "line_item in Cart.line_items", "id" => "cart-variant-{{ line_item.variant.id }}"}
           %td
             %small
               %strong
@@ -33,9 +32,9 @@
             %small
               \=
               %strong
-                .total-price.right {{ line_item.variant.totalPrice() | localizeCurrency }}
+                .total-price.right {{ line_item.total_price | localizeCurrency }}
 
-      %table{"ng-show" => "Cart.line_items_present().length > 0"}
+      %table{"ng-show" => "Cart.line_items.length > 0"}
         %tr.total-cart
           %td
             %em

app/views/shop/products/_form.html.haml

@@ -28,12 +28,18 @@
       .small-12.medium-6.large-6.large-offset-1.columns
         = render partial: "shop/products/filters"
 
-    %div.pad-top{ "infinite-scroll" => "incrementLimit()", "infinite-scroll-distance" => "1", "inifinite-scroll-disabled" => '{{filteredProducts.length <= limit}}' }
-      %product.animate-repeat{"ng-controller" => "ProductNodeCtrl", "ng-repeat" => "product in filteredProducts = (Products.products | products:query | taxons:activeTaxons | properties: activeProperties) | limitTo:limit track by product.id ", "id" => "product-{{ product.id }}"}
+    %div.pad-top{ "infinite-scroll" => "incrementLimit()", "infinite-scroll-distance" => "1", "infinite-scroll-disabled" => 'filteredProducts.length <= limit' }
+      %product.animate-repeat{"ng-controller" => "ProductNodeCtrl", "ng-repeat" => "product in visibleProducts track by product.id", "id" => "product-{{ product.id }}"}
         = render "shop/products/summary"
-        -# %shop-variant{variant: 'product.master', "ng-if" => "::!product.hasVariants", "id" => "variant-{{ product.master.id }}"}
         %shop-variant{variant: 'variant', "ng-repeat" => "variant in product.variants track by variant.id", "id" => "variant-{{ variant.id }}", "ng-class" => "{'out-of-stock': !variant.on_demand && variant.count_on_hand == 0}"}
 
+      -# Load more button, which can be used to initiate infinite scrolling.
+      -# %product{ "ng-hide" => "Products.loading || !infiniteDisabled || limit >= filteredProducts.length" }
+      -#   .row.summary
+      -#     .small-12.columns.text-center
+      -#       %a.button{ ng: { click: 'infiniteDisabled = false; incrementLimit();' } }
+      -#         Load More Products
+
       %product{"ng-show" => "Products.loading"}
         .row.summary
           .small-12.columns.text-center

spec/javascripts/unit/darkswarm/services/cart_spec.js.coffee

@@ -33,18 +33,19 @@ describe 'Cart service', ->
   it "generates extended variant names", ->
     expect(Cart.line_items[0].variant.extended_name).toEqual "name"
 
-  it "creates and backreferences new line items if necessary", ->
-    Cart.register_variant(v2 = {id: 2})
-    expect(Cart.line_items[1].variant).toBe v2
-    expect(Cart.line_items[1].variant.line_item).toBe Cart.line_items[1]
-
-  it "returns a list of items actually in the cart", ->
-    expect(Cart.line_items_present()).toEqual []
+  it "adds item to and removes items from the cart", ->
+    Cart.line_items = []
+    expect(Cart.line_items.length).toEqual 0
     order.line_items[0].quantity = 1
-    expect(Cart.line_items_present().length).toEqual
+    expect(Cart.line_items.length).toEqual 0
+    Cart.adjust(order.line_items[0])
+    expect(Cart.line_items.length).toEqual 1
+    order.line_items[0].quantity = 0
+    expect(Cart.line_items.length).toEqual 1
+    Cart.adjust(order.line_items[0])
+    expect(Cart.line_items.length).toEqual 0
 
   it "sums the quantity of each line item for cart total", ->
-    expect(Cart.line_items_present()).toEqual []
     order.line_items[0].quantity = 2
     expect(Cart.total_item_count()).toEqual 2
 
@@ -130,62 +131,62 @@ describe 'Cart service', ->
     describe "when an item is out of stock", ->
       it "reduces the quantity in the cart", ->
         li = {variant: {id: 1}, quantity: 5}
+        Cart.line_items = [li]
         stockLevels = {1: {quantity: 0, max_quantity: 0, on_hand: 0}}
-        spyOn(Cart, 'line_items_present').and.returnValue [li]
         Cart.compareAndNotifyStockLevels stockLevels
         expect(li.quantity).toEqual 0
         expect(li.max_quantity).toBeUndefined()
 
       it "reduces the max_quantity in the cart", ->
         li = {variant: {id: 1}, quantity: 5, max_quantity: 6}
+        Cart.line_items = [li]
         stockLevels = {1: {quantity: 0, max_quantity: 0, on_hand: 0}}
-        spyOn(Cart, 'line_items_present').and.returnValue [li]
         Cart.compareAndNotifyStockLevels stockLevels
         expect(li.max_quantity).toEqual 0
 
       it "resets the count on hand available", ->
         li = {variant: {id: 1, count_on_hand: 10}, quantity: 5}
+        Cart.line_items = [li]
         stockLevels = {1: {quantity: 0, max_quantity: 0, on_hand: 0}}
-        spyOn(Cart, 'line_items_present').and.returnValue [li]
         Cart.compareAndNotifyStockLevels stockLevels
         expect(li.variant.count_on_hand).toEqual 0
 
     describe "when the quantity available is less than that requested", ->
       it "reduces the quantity in the cart", ->
         li = {variant: {id: 1}, quantity: 6}
+        Cart.line_items = [li]
         stockLevels = {1: {quantity: 5, on_hand: 5}}
-        spyOn(Cart, 'line_items_present').and.returnValue [li]
         Cart.compareAndNotifyStockLevels stockLevels
         expect(li.quantity).toEqual 5
         expect(li.max_quantity).toBeUndefined()
 
       it "does not reduce the max_quantity in the cart", ->
         li = {variant: {id: 1}, quantity: 6, max_quantity: 7}
+        Cart.line_items = [li]
         stockLevels = {1: {quantity: 5, max_quantity: 5, on_hand: 5}}
-        spyOn(Cart, 'line_items_present').and.returnValue [li]
         Cart.compareAndNotifyStockLevels stockLevels
         expect(li.max_quantity).toEqual 7
 
       it "resets the count on hand available", ->
         li = {variant: {id: 1}, quantity: 6}
+        Cart.line_items = [li]
         stockLevels = {1: {quantity: 5, on_hand: 6}}
-        spyOn(Cart, 'line_items_present').and.returnValue [li]
         Cart.compareAndNotifyStockLevels stockLevels
         expect(li.variant.count_on_hand).toEqual 6
 
     describe "when the client-side quantity has been increased during the request", ->
       it "does not reset the quantity", ->
         li = {variant: {id: 1}, quantity: 6}
+        Cart.line_items = [li]
         stockLevels = {1: {quantity: 5, on_hand: 6}}
-        spyOn(Cart, 'line_items_present').and.returnValue [li]
         Cart.compareAndNotifyStockLevels stockLevels
         expect(li.quantity).toEqual 6
         expect(li.max_quantity).toBeUndefined()
 
       it "does not reset the max_quantity", ->
         li = {variant: {id: 1}, quantity: 5, max_quantity: 7}
+        Cart.line_items = [li]
         stockLevels = {1: {quantity: 5, max_quantity: 6, on_hand: 7}}
-        spyOn(Cart, 'line_items_present').and.returnValue [li]
         Cart.compareAndNotifyStockLevels stockLevels
         expect(li.quantity).toEqual 5
         expect(li.max_quantity).toEqual 7
@@ -193,14 +194,14 @@ describe 'Cart service', ->
     describe "when the client-side quantity has been changed from 0 to 1 during the request", ->
       it "does not reset the quantity", ->
         li = {variant: {id: 1}, quantity: 1}
-        spyOn(Cart, 'line_items_present').and.returnValue [li]
+        Cart.line_items = [li]
         Cart.compareAndNotifyStockLevels {}
         expect(li.quantity).toEqual 1
         expect(li.max_quantity).toBeUndefined()
 
       it "does not reset the max_quantity", ->
         li = {variant: {id: 1}, quantity: 1, max_quantity: 1}
-        spyOn(Cart, 'line_items_present').and.returnValue [li]
+        Cart.line_items = [li]
         Cart.compareAndNotifyStockLevels {}
         expect(li.quantity).toEqual 1
         expect(li.max_quantity).toEqual 1
@@ -222,23 +223,3 @@ describe 'Cart service', ->
     expect(Cart.line_items).not.toEqual []
     Cart.clear()
     expect(Cart.line_items).toEqual []
-
-  describe "generating an extended variant name", ->
-    it "returns the product name when it is the same as the variant name", ->
-      variant = {product_name: 'product_name', name_to_display: 'product_name'}
-      expect(Cart.extendedVariantName(variant)).toEqual "product_name"
-
-    describe "when the product name and the variant name differ", ->
-      it "returns a combined name when there is no options text", ->
-        variant =
-          product_name: 'product_name'
-          name_to_display: 'name_to_display'
-        expect(Cart.extendedVariantName(variant)).toEqual "product_name - name_to_display"
-
-      it "returns a combined name when there is some options text", ->
-        variant =
-          product_name: 'product_name'
-          name_to_display: 'name_to_display'
-          options_text: 'options_text'
-
-        expect(Cart.extendedVariantName(variant)).toEqual "product_name - name_to_display (options_text)"

spec/javascripts/unit/darkswarm/services/products_spec.js.coffee

@@ -82,12 +82,6 @@ describe 'Products service', ->
     $httpBackend.flush()
     expect(Products.products[0].variants[0]).toBe Variants.variants[1]
 
-  it "registers variants with the Cart", ->
-    product.variants = [{id: 8}]
-    $httpBackend.expectGET("/shop/products").respond([product])
-    $httpBackend.flush()
-    expect(Cart.line_items[0].variant).toBe Products.products[0].variants[0]
-
   it "sets primaryImageOrMissing when no images are provided", ->
     $httpBackend.expectGET("/shop/products").respond([product])
     $httpBackend.flush()

spec/javascripts/unit/darkswarm/services/variants_spec.js.coffee

@@ -21,11 +21,45 @@ describe 'Variants service', ->
   it "will return the same object as passed", ->
     expect(Variants.register(variant)).toBe variant
 
+  describe "initialising the line_item", ->
+    describe "when variant.line_item does not exist", ->
+      it "creates it", ->
+        line_item = Variants.register(variant).line_item
+        expect(line_item).toBeDefined()
+        expect(line_item.total_price).toEqual 0
+
+    describe "when variant.line_item already exists", ->
+      beforeEach ->
+        variant.line_item = { quantity: 4 }
+
+      it "initialises the total_price", ->
+        expect(Variants.register(variant).line_item.total_price).toEqual 400
+
   it "initialises base price percentage", ->
-    expect(Variants.register(variant).basePricePercentage).toEqual 81
+    expect(Variants.register(variant).base_price_percentage).toEqual 81
 
   it "clears registered variants", ->
     Variants.register(variant)
     expect(Variants.variants[variant.id]).toBe variant
     Variants.clear()
-    expect(Variants.variants[variant.id]).toBeUndefined()
+    expect(Variants.variants[variant.id]).toBeUndefined()
+
+  describe "generating an extended variant name", ->
+    it "returns the product name when it is the same as the variant name", ->
+      variant = {product_name: 'product_name', name_to_display: 'product_name'}
+      expect(Variants.extendedVariantName(variant)).toEqual "product_name"
+
+    describe "when the product name and the variant name differ", ->
+      it "returns a combined name when there is no options text", ->
+        variant =
+          product_name: 'product_name'
+          name_to_display: 'name_to_display'
+        expect(Variants.extendedVariantName(variant)).toEqual "product_name - name_to_display"
+
+      it "returns a combined name when there is some options text", ->
+        variant =
+          product_name: 'product_name'
+          name_to_display: 'name_to_display'
+          options_text: 'options_text'
+
+        expect(Variants.extendedVariantName(variant)).toEqual "product_name - name_to_display (options_text)"