WIP: BOM Refactor, building order fetch and update logic with ngResource, updating rails routes

app/assets/javascripts/admin/all.js

@@ -28,6 +28,7 @@
 //= require ./enterprises/enterprises
 //= require ./enterprise_groups/enterprise_groups
 //= require ./index_utils/index_utils
+//= require ./orders/orders
 //= require ./payment_methods/payment_methods
 //= require ./products/products
 //= require ./shipping_methods/shipping_methods

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

@@ -0,0 +1 @@
+angular.module("admin.orders", ['ngResource'])

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

@@ -0,0 +1,8 @@
+angular.module("admin.orders").factory 'OrderResource', ($resource) ->
+  $resource('/admin/orders/:id/:action.json', {}, {
+    'index':
+      method: 'GET'
+      isArray: true
+    'update':
+      method: 'PUT'
+  })

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

@@ -0,0 +1,34 @@
+angular.module("admin.orders").factory 'Orders', ($q, OrderResource) ->
+  new class Orders
+    ordersByID: {}
+    pristineByID: {}
+
+    index: (params={}, callback=null) ->
+    	OrderResource.index params, (data) =>
+        for order in data
+          @ordersByID[order.id] = order
+          @pristineByID[order.id] = angular.copy(order)
+
+        (callback || angular.noop)(data)
+
+    save: (order) ->
+      deferred = $q.defer()
+      order.$update({id: order.permalink})
+      .then( (data) =>
+        @pristineByID[order.id] = angular.copy(order)
+        deferred.resolve(data)
+      ).catch (response) ->
+        deferred.reject(response)
+      deferred.promise
+
+    saved: (order) ->
+      @diff(order).length == 0
+
+    diff: (order) ->
+      changed = []
+      for attr, value of order when not angular.equals(value, @pristineByID[order.id][attr])
+        changed.push attr unless attr is "$$hashKey"
+      changed
+
+    resetAttribute: (order, attribute) ->
+      order[attribute] = @pristineByID[order.id][attribute]

app/controllers/spree/admin/orders_controller_decorator.rb

@@ -19,22 +19,22 @@ Spree::Admin::OrdersController.class_eval do
 
   before_filter :require_distributor_abn, only: :invoice
 
+  respond_to :html, :json
+
   respond_override :index => { :html =>
     { :success => lambda {
       # Filter orders to only show those distributed by current user (or all for admin user)
-      @orders = @search.result.includes([:user, :shipments, :payments]).
+      @search.result.includes([:user, :shipments, :payments]).
         distributed_by_user(spree_current_user).
         page(params[:page]).
         per(params[:per_page] || Spree::Config[:orders_per_page])
-      # Filter orders by distributor
-      if params[:distributor_ids]
-        @orders = @orders.where(distributor_id: params[:distributor_ids])
-      end
-      if params[:order_cycle_ids]
-        @orders = @orders.where(order_cycle_id: params[:order_cycle_ids])
-      end
     } } }
 
+  respond_override index: { :json => { :success => lambda {
+    search = OpenFoodNetwork::Permissions.new(spree_current_user).editable_orders.ransack(params[:q])
+    render_as_json search.result.sort_by(&:id)
+  } } }
+
   # Overwrite to use confirm_email_for_customer instead of confirm_email.
   # This uses a new template. See mailers/spree/order_mailer_decorator.rb.
   def resend
@@ -60,12 +60,6 @@ Spree::Admin::OrdersController.class_eval do
     @order.update_distribution_charge!
   end
 
-  def managed
-    permissions = OpenFoodNetwork::Permissions.new(spree_current_user)
-    @orders = permissions.editable_orders.order(:id).ransack(params[:q]).result.page(params[:page]).per(params[:per_page])
-    render json: @orders, each_serializer: Api::Admin::OrderSerializer
-  end
-
   private
 
   def require_distributor_abn

app/serializers/api/admin/id_serializer.rb

@@ -0,0 +1,3 @@
+class Api::Admin::IdSerializer < ActiveModel::Serializer
+  attributes :id
+end

app/serializers/api/admin/order_serializer.rb

@@ -1,8 +1,8 @@
 class Api::Admin::OrderSerializer < ActiveModel::Serializer
-  attributes :id, :number, :full_name, :email, :phone, :completed_at, :line_items
+  attributes :id, :number, :full_name, :email, :phone, :completed_at
 
-  has_one :distributor, serializer: Api::Admin::IdNameSerializer
-  has_one :order_cycle, serializer: Api::Admin::BasicOrderCycleSerializer
+  has_one :distributor, serializer: Api::Admin::IdSerializer
+  has_one :order_cycle, serializer: Api::Admin::IdSerializer
 
   def full_name
     object.billing_address.nil? ? "" : ( object.billing_address.full_name || "" )
@@ -19,13 +19,4 @@ class Api::Admin::OrderSerializer < ActiveModel::Serializer
   def completed_at
     object.completed_at.blank? ? "" : object.completed_at.strftime("%F %T")
   end
-
-  def line_items
-    # we used to have a scope here, but we are at the point where a user which can edit an order
-    # should be able to edit all of the line_items as well, making the scope redundant
-    ActiveModel::ArraySerializer.new(
-      object.line_items.order('id ASC'),
-      {each_serializer: Api::Admin::LineItemSerializer}
-    )
-  end
 end

spec/controllers/spree/admin/orders_controller_spec.rb

@@ -29,10 +29,10 @@ describe Spree::Admin::OrdersController do
     end
   end
 
-  describe "#managed" do
+  describe "#index" do
     render_views
 
-    let(:order_attributes) { [:id, :full_name, :email, :phone, :completed_at, :line_items, :distributor, :order_cycle, :number] }
+    let(:order_attributes) { [:id, :full_name, :email, :phone, :completed_at, :distributor, :order_cycle, :number] }
 
     def self.make_simple_data!
       let!(:dist1) { FactoryGirl.create(:distributor_enterprise) }
@@ -51,8 +51,8 @@ describe Spree::Admin::OrdersController do
 
       make_simple_data!
 
-      it "should deny me access to managed orders" do
-        spree_get :managed, { :template => 'bulk_index', :format => :json }
+      it "should deny me access to the index action" do
+        spree_get :index, :format => :json
         expect(response).to redirect_to spree.unauthorized_path
       end
     end
@@ -62,7 +62,7 @@ describe Spree::Admin::OrdersController do
 
       before do
         controller.stub spree_current_user: quick_login_as_admin
-        spree_get :managed, { :template => 'bulk_index', :format => :json }
+        spree_get :index, :format => :json
       end
 
       it "retrieves a list of orders with appropriate attributes, including line items with appropriate attributes" do
@@ -70,11 +70,6 @@ describe Spree::Admin::OrdersController do
         order_attributes.all?{ |attr| keys.include? attr }.should == true
       end
 
-      it "retrieves a list of line items with appropriate attributes" do
-        li_keys = json_response.first['line_items'].first.keys.map{ |key| key.to_sym }
-        line_item_attributes.all?{ |attr| li_keys.include? attr }.should == true
-      end
-
       it "sorts orders in ascending id order" do
         ids = json_response.map{ |order| order['id'] }
         ids[0].should < ids[1]
@@ -85,21 +80,8 @@ describe Spree::Admin::OrdersController do
         json_response.map{ |order| order['completed_at'] }.all?{ |a| a.match("^\\d{4}-\\d{2}-\\d{2} \\d{2}:\\d{2}:\\d{2}$") }.should == true
       end
 
-      it "returns an array for line_items" do
-        json_response.map{ |order| order['line_items'] }.all?{ |a| a.is_a? Array }.should == true
-      end
-
-      it "returns quantity and max quantity at integers" do
-        json_response.map{ |order| order['line_items'] }.flatten.map{ |li| li['quantity'] }.all?{ |q| q.is_a? Fixnum }.should == true
-        json_response.map{ |order| order['line_items'] }.flatten.map{ |li| li['max_quantity'] }.all?{ |mq| mq.nil? || mq.is_a?( Fixnum ) }.should == true
-      end
-
-      it "returns supplier object with id and name keys" do
-        json_response.map{ |order| order['line_items'] }.flatten.map{ |li| li['supplier'] }.all?{ |s| s.has_key?('id') && s.has_key?('name') }.should == true
-      end
-
-      it "returns distributor object with id and name keys" do
-        json_response.map{ |order| order['distributor'] }.all?{ |d| d.has_key?('id') && d.has_key?('name') }.should == true
+      it "returns distributor object with id key" do
+        json_response.map{ |order| order['distributor'] }.all?{ |d| d.has_key?('id') }.should == true
       end
 
       it "retrieves the order number" do
@@ -123,7 +105,7 @@ describe Spree::Admin::OrdersController do
 
         before do
           controller.stub spree_current_user: supplier.owner
-          spree_get :managed, { :format => :json }
+          spree_get :index, :format => :json
         end
 
         it "does not display line items for which my enterprise is a supplier" do
@@ -134,33 +116,25 @@ describe Spree::Admin::OrdersController do
       context "coordinator enterprise" do
         before do
           controller.stub spree_current_user: coordinator.owner
-          spree_get :managed, { :format => :json }
+          spree_get :index, :format => :json
         end
 
         it "retrieves a list of orders" do
           keys = json_response.first.keys.map{ |key| key.to_sym }
           order_attributes.all?{ |attr| keys.include? attr }.should == true
         end
-
-        it "only displays line items from orders for which my enterprise is the order_cycle coorinator" do
-          json_response.map{ |order| order['line_items'] }.flatten.map{ |line_item| line_item["id"] }.should match_array [line_item1.id, line_item2.id, line_item3.id]
-        end
       end
 
       context "hub enterprise" do
         before do
           controller.stub spree_current_user: distributor1.owner
-          spree_get :managed, { :format => :json }
+          spree_get :index, :format => :json
         end
 
         it "retrieves a list of orders" do
           keys = json_response.first.keys.map{ |key| key.to_sym }
           order_attributes.all?{ |attr| keys.include? attr }.should == true
         end
-
-        it "only displays line items from orders for which my enterprise is a distributor" do
-          json_response.map{ |order| order['line_items'] }.flatten.map{ |line_item| line_item["id"] }.should match_array [line_item1.id, line_item2.id]
-        end
       end
     end
   end

spec/javascripts/unit/admin/orders/services/orders_spec.js.coffee

@@ -0,0 +1,106 @@
+describe "Orders service", ->
+  Orders = OrderResource = orders = $httpBackend = null
+
+  beforeEach ->
+    module 'admin.orders'
+
+    this.addMatchers
+      toDeepEqual: (expected) ->
+        return angular.equals(this.actual, expected)
+
+    inject ($q, _$httpBackend_, _Orders_, _OrderResource_) ->
+      Orders = _Orders_
+      OrderResource = _OrderResource_
+      $httpBackend = _$httpBackend_
+
+  describe "#index", ->
+    result = response = null
+
+    beforeEach ->
+      response = [{ id: 5, name: 'Order 1'}]
+      $httpBackend.expectGET('/admin/orders.json').respond 200, response
+      result = Orders.index()
+      $httpBackend.flush()
+
+    it "stores returned data in @ordersByID, with ids as keys", ->
+      # OrderResource returns instances of Resource rather than raw objects
+      expect(Orders.ordersByID).toDeepEqual { 5: response[0] }
+
+    it "stores returned data in @pristineByID, with ids as keys", ->
+      expect(Orders.pristineByID).toDeepEqual { 5: response[0] }
+
+    it "returns an array of orders", ->
+      expect(result).toDeepEqual response
+
+
+  describe "#save", ->
+    result = null
+
+    describe "success", ->
+      order = null
+      resolved = false
+
+      beforeEach ->
+        order = new OrderResource({ id: 15, permalink: 'order1', name: 'Order 1' })
+        $httpBackend.expectPUT('/admin/orders/order1.json').respond 200, { id: 15, name: 'Order 1'}
+        Orders.save(order).then( -> resolved = true)
+        $httpBackend.flush()
+
+      it "updates the pristine copy of the order", ->
+        # Resource results have extra properties ($then, $promise) that cause them to not
+        # be exactly equal to the response object provided to the expectPUT clause above.
+        expect(Orders.pristineByID[15]).toEqual order
+
+      it "resolves the promise", ->
+        expect(resolved).toBe(true);
+
+
+    describe "failure", ->
+      order = null
+      rejected = false
+
+      beforeEach ->
+        order = new OrderResource( { id: 15, permalink: 'permalink', name: 'Order 1' } )
+        $httpBackend.expectPUT('/admin/orders/permalink.json').respond 422, { error: 'obj' }
+        Orders.save(order).catch( -> rejected = true)
+        $httpBackend.flush()
+
+      it "does not update the pristine copy of the order", ->
+        expect(Orders.pristineByID[15]).toBeUndefined()
+
+      it "rejects the promise", ->
+        expect(rejected).toBe(true);
+
+  describe "#saved", ->
+    describe "when attributes of the object have been altered", ->
+      beforeEach ->
+        spyOn(Orders, "diff").andReturn ["attr1", "attr2"]
+
+      it "returns false", ->
+        expect(Orders.saved({})).toBe false
+
+    describe "when attributes of the object have not been altered", ->
+      beforeEach ->
+        spyOn(Orders, "diff").andReturn []
+
+      it "returns false", ->
+        expect(Orders.saved({})).toBe true
+
+
+  describe "diff", ->
+    beforeEach ->
+      Orders.pristineByID = { 23: { id: 23, name: "ent1", is_primary_producer: true } }
+
+    it "returns a list of properties that have been altered", ->
+      expect(Orders.diff({ id: 23, name: "order123", is_primary_producer: true })).toEqual ["name"]
+
+
+  describe "resetAttribute", ->
+    order = { id: 23, name: "ent1", is_primary_producer: true }
+
+    beforeEach ->
+      Orders.pristineByID = { 23: { id: 23, name: "order1", is_primary_producer: true } }
+
+    it "resets the specified value according to the pristine record", ->
+      Orders.resetAttribute(order, "name")
+      expect(order.name).toEqual "order1"