From b43049af4753419cd7992aaf40eba2412c28684e Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 4 Nov 2015 18:16:17 +1100 Subject: [PATCH] WIP: BOM Refactor, building line_item fetch and update logic with ngResource --- app/assets/javascripts/admin/all.js | 1 + .../directives/confirm_change.js.coffee | 19 ++++ .../directives/confirm_model_change.js.coffee | 6 + .../filters/select_filter.js.coffee | 7 ++ .../filters/variant_filter.js.coffee | 6 + .../admin/line_items/line_items.js.coffee | 1 + .../services/line_item_resource.js.coffee | 12 ++ .../line_items/services/line_items.js.coffee | 48 ++++++++ .../admin/line_items_controller_decorator.rb | 13 +++ app/models/spree/ability_decorator.rb | 3 +- .../api/admin/line_item_serializer.rb | 6 +- config/routes.rb | 4 + .../spree/admin/line_items_controller_spec.rb | 104 +++++++++++++++++ .../services/line_items_spec.js.coffee | 107 ++++++++++++++++++ 14 files changed, 334 insertions(+), 3 deletions(-) create mode 100644 app/assets/javascripts/admin/line_items/directives/confirm_change.js.coffee create mode 100644 app/assets/javascripts/admin/line_items/directives/confirm_model_change.js.coffee create mode 100644 app/assets/javascripts/admin/line_items/filters/select_filter.js.coffee create mode 100644 app/assets/javascripts/admin/line_items/filters/variant_filter.js.coffee create mode 100644 app/assets/javascripts/admin/line_items/line_items.js.coffee create mode 100644 app/assets/javascripts/admin/line_items/services/line_item_resource.js.coffee create mode 100644 app/assets/javascripts/admin/line_items/services/line_items.js.coffee create mode 100644 spec/controllers/spree/admin/line_items_controller_spec.rb create mode 100644 spec/javascripts/unit/admin/line_items/services/line_items_spec.js.coffee diff --git a/app/assets/javascripts/admin/all.js b/app/assets/javascripts/admin/all.js index b1455c4584..831fc3db4b 100644 --- a/app/assets/javascripts/admin/all.js +++ b/app/assets/javascripts/admin/all.js @@ -28,6 +28,7 @@ //= require ./enterprises/enterprises //= require ./enterprise_groups/enterprise_groups //= require ./index_utils/index_utils +//= require ./line_items/line_items //= require ./orders/orders //= require ./payment_methods/payment_methods //= require ./products/products diff --git a/app/assets/javascripts/admin/line_items/directives/confirm_change.js.coffee b/app/assets/javascripts/admin/line_items/directives/confirm_change.js.coffee new file mode 100644 index 0000000000..706b1c0ad7 --- /dev/null +++ b/app/assets/javascripts/admin/line_items/directives/confirm_change.js.coffee @@ -0,0 +1,19 @@ +# Used with the ngChange directive to prevent updates to the relevant model unless a callback returns true +angular.module("admin.lineItems").directive "confirmChange", -> + restrict: "A" + require: 'ngModel' + scope: + confirmChange: "&" + link: (scope, element, attrs, ngModel) -> + valid = null + + ngModel.$parsers.push (val) => + return val if val == valid + if scope.confirmChange() + # ngModel is changed, triggers ngChange callback + return valid = val + else + valid = ngModel.$modelValue + ngModel.$setViewValue(valid) + ngModel.$render() + return valid diff --git a/app/assets/javascripts/admin/line_items/directives/confirm_model_change.js.coffee b/app/assets/javascripts/admin/line_items/directives/confirm_model_change.js.coffee new file mode 100644 index 0000000000..a0b5272981 --- /dev/null +++ b/app/assets/javascripts/admin/line_items/directives/confirm_model_change.js.coffee @@ -0,0 +1,6 @@ +angular.module("ofn.admin").directive "ofnConfirmModelChange", (ofnConfirmHandler,$timeout) -> + restrict: "A" + link: (scope, element, attrs) -> + handler = ofnConfirmHandler scope, -> scope.fetchOrders() + scope.$watch attrs.ngModel, (oldValue,newValue) -> + handler() unless oldValue == undefined || newValue == oldValue \ No newline at end of file diff --git a/app/assets/javascripts/admin/line_items/filters/select_filter.js.coffee b/app/assets/javascripts/admin/line_items/filters/select_filter.js.coffee new file mode 100644 index 0000000000..1c9742f283 --- /dev/null +++ b/app/assets/javascripts/admin/line_items/filters/select_filter.js.coffee @@ -0,0 +1,7 @@ +angular.module("admin.lineItems").filter "selectFilter", (blankOption) -> + return (lineItems,selectedSupplier,selectedDistributor,selectedOrderCycle) -> + filtered = [] + filtered.push lineItem for lineItem in lineItems when (angular.equals(selectedSupplier,"0") || lineItem.supplier.id == selectedSupplier) && + (angular.equals(selectedDistributor,"0") || lineItem.order.distributor.id == selectedDistributor) && + (angular.equals(selectedOrderCycle,"0") || lineItem.order.order_cycle.id == selectedOrderCycle) + filtered diff --git a/app/assets/javascripts/admin/line_items/filters/variant_filter.js.coffee b/app/assets/javascripts/admin/line_items/filters/variant_filter.js.coffee new file mode 100644 index 0000000000..8ddcf667e7 --- /dev/null +++ b/app/assets/javascripts/admin/line_items/filters/variant_filter.js.coffee @@ -0,0 +1,6 @@ +angular.module("admin.lineItems").filter "variantFilter", -> + return (lineItems,selectedUnitsProduct,selectedUnitsVariant,sharedResource) -> + filtered = [] + filtered.push lineItem for lineItem in lineItems when (angular.equals(selectedUnitsProduct,{}) || + (lineItem.units_product.id == selectedUnitsProduct.id && (sharedResource || lineItem.units_variant.id == selectedUnitsVariant.id ) ) ) + filtered diff --git a/app/assets/javascripts/admin/line_items/line_items.js.coffee b/app/assets/javascripts/admin/line_items/line_items.js.coffee new file mode 100644 index 0000000000..afac42b5c3 --- /dev/null +++ b/app/assets/javascripts/admin/line_items/line_items.js.coffee @@ -0,0 +1 @@ +angular.module("admin.lineItems", ["admin.indexUtils", "admin.lineItems", "admin.enterprises", "admin.order_cycles"]) diff --git a/app/assets/javascripts/admin/line_items/services/line_item_resource.js.coffee b/app/assets/javascripts/admin/line_items/services/line_item_resource.js.coffee new file mode 100644 index 0000000000..f5a4983b21 --- /dev/null +++ b/app/assets/javascripts/admin/line_items/services/line_item_resource.js.coffee @@ -0,0 +1,12 @@ +angular.module("admin.lineItems").factory 'LineItemResource', ($resource) -> + $resource('/admin/:orders/:order_number/line_items/:id.json', {}, { + 'index': + method: 'GET' + isArray: true + 'update': + method: 'PUT' + transformRequest: (data, headersGetter) => + requestData = {} + requestData[attr] = data[attr] for attr in ["price", "quantity", "final_weight_volume"] + angular.toJson(requestData) + }) diff --git a/app/assets/javascripts/admin/line_items/services/line_items.js.coffee b/app/assets/javascripts/admin/line_items/services/line_items.js.coffee new file mode 100644 index 0000000000..38dd3ffe93 --- /dev/null +++ b/app/assets/javascripts/admin/line_items/services/line_items.js.coffee @@ -0,0 +1,48 @@ +angular.module("admin.lineItems").factory 'LineItems', ($q, LineItemResource) -> + new class LineItems + lineItemsByID: {} + pristineByID: {} + + index: (params={}, callback=null) -> + LineItemResource.index params, (data) => + @resetData() + for lineItem in data + @lineItemsByID[lineItem.id] = lineItem + @pristineByID[lineItem.id] = angular.copy(lineItem) + + (callback || angular.noop)(data) + + resetData: -> + @lineItemsByID = {} + @pristineByID = {} + + saveAll: -> + for id, lineItem of @lineItemsByID when !@isSaved(lineItem) + @save(lineItem) + + save: (lineItem) -> + deferred = $q.defer() + lineItem.$update({id: lineItem.id, orders: "orders", order_number: lineItem.order.number}) + .then( (data) => + @pristineByID[lineItem.id] = angular.copy(lineItem) + deferred.resolve(data) + ).catch (response) -> + deferred.reject(response) + deferred.promise + + allSaved: -> + for id, lineItem of @lineItemsByID + return false unless @isSaved(lineItem) + true + + isSaved: (lineItem) -> + @diff(lineItem).length == 0 + + diff: (lineItem) -> + changed = [] + for attr, value of lineItem when not angular.equals(value, @pristineByID[lineItem.id][attr]) + changed.push attr if attr in ["price", "quantity", "final_weight_volume"] + changed + + resetAttribute: (lineItem, attribute) -> + lineItem[attribute] = @pristineByID[lineItem.id][attribute] diff --git a/app/controllers/spree/admin/line_items_controller_decorator.rb b/app/controllers/spree/admin/line_items_controller_decorator.rb index ca83baa00b..6b62d0b4ea 100644 --- a/app/controllers/spree/admin/line_items_controller_decorator.rb +++ b/app/controllers/spree/admin/line_items_controller_decorator.rb @@ -1,4 +1,17 @@ Spree::Admin::LineItemsController.class_eval do + prepend_before_filter :load_order, except: :index + + respond_to :json + + def index + respond_to do |format| + format.json do + search = OpenFoodNetwork::Permissions.new(spree_current_user).editable_line_items.ransack(params[:q]) + render_as_json search.result.sort_by(&:order_id) + end + end + end + private def load_order diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index abf4024fd8..4dbe434a66 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -149,7 +149,8 @@ class AbilityDecorator end can [:admin, :bulk_management, :managed], Spree::Order if user.admin? || user.enterprises.any?(&:is_distributor) can [:admin , :for_line_items], Enterprise - can [:destroy], Spree::LineItem do |item| + can [:admin, :index, :create], Spree::LineItem + can [:destroy, :update], Spree::LineItem do |item| user.admin? || user.enterprises.include?(order.distributor) || user == order.order_cycle.manager end diff --git a/app/serializers/api/admin/line_item_serializer.rb b/app/serializers/api/admin/line_item_serializer.rb index d12e3b291a..53a4ae8274 100644 --- a/app/serializers/api/admin/line_item_serializer.rb +++ b/app/serializers/api/admin/line_item_serializer.rb @@ -1,8 +1,10 @@ class Api::Admin::LineItemSerializer < ActiveModel::Serializer - attributes :id, :quantity, :max_quantity, :supplier, :price, :final_weight_volume, :units_product, :units_variant + attributes :id, :quantity, :max_quantity, :price, :supplier, :final_weight_volume, :units_product, :units_variant + + has_one :order, serializer: Api::Admin::IdSerializer def supplier - Api::Admin::IdNameSerializer.new(object.product.supplier).serializable_hash + { id: object.product.supplier_id } end def units_product diff --git a/config/routes.rb b/config/routes.rb index 1d26a469b9..79e846d542 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -209,6 +209,10 @@ Spree::Core::Engine.routes.prepend do get :print, on: :member get :managed, on: :collection end + + resources :line_items do + get :index, on: :collection, format: :json + end end resources :orders do diff --git a/spec/controllers/spree/admin/line_items_controller_spec.rb b/spec/controllers/spree/admin/line_items_controller_spec.rb new file mode 100644 index 0000000000..2ce4018982 --- /dev/null +++ b/spec/controllers/spree/admin/line_items_controller_spec.rb @@ -0,0 +1,104 @@ +require 'spec_helper' + +describe Spree::Admin::LineItemsController do + include AuthenticationWorkflow + + describe "#index" do + render_views + + let(:line_item_attributes) { [:id, :quantity, :max_quantity, :price, :supplier, :final_weight_volume, :units_product, :units_variant, :order] } + let!(:dist1) { FactoryGirl.create(:distributor_enterprise) } + let!(:order1) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } + let!(:order2) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } + let!(:order3) { FactoryGirl.create(:order, state: 'complete', completed_at: Time.now, distributor: dist1, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item1) { FactoryGirl.create(:line_item, order: order1) } + let!(:line_item2) { FactoryGirl.create(:line_item, order: order2) } + let!(:line_item3) { FactoryGirl.create(:line_item, order: order2) } + let!(:line_item4) { FactoryGirl.create(:line_item, order: order3) } + + context "as a normal user" do + before { controller.stub spree_current_user: create_enterprise_user } + + 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 + + context "as an administrator" do + + before do + controller.stub spree_current_user: quick_login_as_admin + spree_get :index, :format => :json + end + + it "retrieves a list of line_items with appropriate attributes, including line items with appropriate attributes" do + keys = json_response.first.keys.map{ |key| key.to_sym } + line_item_attributes.all?{ |attr| keys.include? attr }.should == true + end + + it "sorts line_items in ascending id line_item" do + ids = json_response.map{ |line_item| line_item['id'] } + ids[0].should < ids[1] + ids[1].should < ids[2] + end + + it "formats final_weight_volume as a float" do + json_response.map{ |line_item| line_item['final_weight_volume'] }.all?{ |fwv| fwv.is_a?(Float) }.should == true + end + + it "returns distributor object with id key" do + json_response.map{ |line_item| line_item['supplier'] }.all?{ |d| d.has_key?('id') }.should == true + end + end + + context "as an enterprise user" do + let(:supplier) { create(:supplier_enterprise) } + let(:distributor1) { create(:distributor_enterprise) } + let(:distributor2) { create(:distributor_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle, coordinator: coordinator) } + let!(:order1) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.now, distributor: distributor1, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item1) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } + let!(:line_item2) { FactoryGirl.create(:line_item, order: order1, product: FactoryGirl.create(:product, supplier: supplier)) } + let!(:order2) { FactoryGirl.create(:order, order_cycle: order_cycle, state: 'complete', completed_at: Time.now, distributor: distributor2, billing_address: FactoryGirl.create(:address) ) } + let!(:line_item3) { FactoryGirl.create(:line_item, order: order2, product: FactoryGirl.create(:product, supplier: supplier)) } + + context "producer enterprise" do + + before do + controller.stub spree_current_user: supplier.owner + spree_get :index, :format => :json + end + + it "does not display line items for which my enterprise is a supplier" do + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "coordinator enterprise" do + before do + controller.stub spree_current_user: coordinator.owner + spree_get :index, :format => :json + end + + it "retrieves a list of line_items" do + keys = json_response.first.keys.map{ |key| key.to_sym } + line_item_attributes.all?{ |attr| keys.include? attr }.should == true + end + end + + context "hub enterprise" do + before do + controller.stub spree_current_user: distributor1.owner + spree_get :index, :format => :json + end + + it "retrieves a list of line_items" do + keys = json_response.first.keys.map{ |key| key.to_sym } + line_item_attributes.all?{ |attr| keys.include? attr }.should == true + end + end + end + end +end diff --git a/spec/javascripts/unit/admin/line_items/services/line_items_spec.js.coffee b/spec/javascripts/unit/admin/line_items/services/line_items_spec.js.coffee new file mode 100644 index 0000000000..2796a2dd8a --- /dev/null +++ b/spec/javascripts/unit/admin/line_items/services/line_items_spec.js.coffee @@ -0,0 +1,107 @@ +describe "LineItems service", -> + LineItems = LineItemResource = lineItems = $httpBackend = null + + beforeEach -> + module 'admin.lineItems' + + this.addMatchers + toDeepEqual: (expected) -> + return angular.equals(this.actual, expected) + + inject ($q, _$httpBackend_, _LineItems_, _LineItemResource_) -> + LineItems = _LineItems_ + LineItemResource = _LineItemResource_ + $httpBackend = _$httpBackend_ + + describe "#index", -> + result = response = null + + beforeEach -> + response = [{ id: 5, name: 'LineItem 1'}] + $httpBackend.expectGET('/admin/line_items.json').respond 200, response + result = LineItems.index() + $httpBackend.flush() + + it "stores returned data in @lineItemsByID, with ids as keys", -> + # LineItemResource returns instances of Resource rather than raw objects + expect(LineItems.lineItemsByID).toDeepEqual { 5: response[0] } + + it "stores returned data in @pristineByID, with ids as keys", -> + expect(LineItems.pristineByID).toDeepEqual { 5: response[0] } + + it "returns an array of line items", -> + expect(result).toDeepEqual response + + + describe "#save", -> + result = null + + describe "success", -> + lineItem = null + resolved = false + + beforeEach -> + lineItem = new LineItemResource({ id: 15, order: { number: '12345678'} }) + $httpBackend.expectPUT('/admin/orders/12345678/line_items/15.json').respond 200, { id: 15, name: 'LineItem 1'} + LineItems.save(lineItem).then( -> resolved = true) + $httpBackend.flush() + + it "updates the pristine copy of the lineItem", -> + # 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(LineItems.pristineByID[15]).toEqual lineItem + + it "resolves the promise", -> + expect(resolved).toBe(true); + + + describe "failure", -> + lineItem = null + rejected = false + + beforeEach -> + lineItem = new LineItemResource( { id: 15, order: { number: '12345678'} } ) + $httpBackend.expectPUT('/admin/orders/12345678/line_items/15.json').respond 422, { error: 'obj' } + LineItems.save(lineItem).catch( -> rejected = true) + $httpBackend.flush() + + it "does not update the pristine copy of the lineItem", -> + expect(LineItems.pristineByID[15]).toBeUndefined() + + it "rejects the promise", -> + expect(rejected).toBe(true); + + describe "#isSaved", -> + describe "when attributes of the object have been altered", -> + beforeEach -> + spyOn(LineItems, "diff").andReturn ["attr1", "attr2"] + + it "returns false", -> + expect(LineItems.isSaved({})).toBe false + + describe "when attributes of the object have not been altered", -> + beforeEach -> + spyOn(LineItems, "diff").andReturn [] + + it "returns false", -> + expect(LineItems.isSaved({})).toBe true + + + describe "diff", -> + beforeEach -> + LineItems.pristineByID = { 23: { id: 23, price: 15, quantity: 3, something: 3 } } + + it "returns a list of properties that have been altered and are in the list of updateable attrs", -> + expect(LineItems.diff({ id: 23, price: 12, quantity: 3 })).toEqual ["price"] + expect(LineItems.diff({ id: 23, price: 15, something: 1 })).toEqual [] + + + describe "resetAttribute", -> + lineItem = { id: 23, price: 15 } + + beforeEach -> + LineItems.pristineByID = { 23: { id: 23, price: 12, quantity: 3 } } + + it "resets the specified value according to the pristine record", -> + LineItems.resetAttribute(lineItem, "price") + expect(lineItem.price).toEqual 12