diff --git a/app/assets/javascripts/admin/variant_overrides/controllers/variant_overrides_controller.js.coffee b/app/assets/javascripts/admin/variant_overrides/controllers/variant_overrides_controller.js.coffee index 9b0ed4374e..b65df80d66 100644 --- a/app/assets/javascripts/admin/variant_overrides/controllers/variant_overrides_controller.js.coffee +++ b/app/assets/javascripts/admin/variant_overrides/controllers/variant_overrides_controller.js.coffee @@ -1,4 +1,4 @@ -angular.module("admin.variantOverrides").controller "AdminVariantOverridesCtrl", ($scope, $timeout, Indexer, Columns, SpreeApiAuth, PagedFetcher, StatusMessage, hubs, producers, hubPermissions, VariantOverrides, DirtyVariantOverrides) -> +angular.module("admin.variantOverrides").controller "AdminVariantOverridesCtrl", ($scope, $http, $timeout, Indexer, Columns, SpreeApiAuth, PagedFetcher, StatusMessage, hubs, producers, hubPermissions, VariantOverrides, DirtyVariantOverrides) -> $scope.hubs = Indexer.index hubs $scope.hub = null $scope.products = [] @@ -89,8 +89,12 @@ angular.module("admin.variantOverrides").controller "AdminVariantOverridesCtrl", $scope.displayDirty() , 3000 # 3 second delay else + return unless $scope.hub_id? StatusMessage.display 'progress', 'Changing on hand stock levels...' - VariantOverrides.resetStock() + $http + method: "POST" + url: "/admin/variant_overrides/bulk_reset" + data: { hub_id: $scope.hub_id } .success (updatedVos) -> VariantOverrides.updateData updatedVos StatusMessage.display 'success', 'Stocks reset to defaults.' diff --git a/app/assets/javascripts/admin/variant_overrides/services/variant_overrides.js.coffee b/app/assets/javascripts/admin/variant_overrides/services/variant_overrides.js.coffee index 7191ceebda..4e1128572e 100644 --- a/app/assets/javascripts/admin/variant_overrides/services/variant_overrides.js.coffee +++ b/app/assets/javascripts/admin/variant_overrides/services/variant_overrides.js.coffee @@ -1,4 +1,5 @@ -angular.module("admin.variantOverrides").factory "VariantOverrides", (variantOverrides, $http) -> + +angular.module("admin.variantOverrides").factory "VariantOverrides", (variantOverrides) -> new class VariantOverrides variantOverrides: {} @@ -30,18 +31,10 @@ angular.module("admin.variantOverrides").factory "VariantOverrides", (variantOve default_stock: null resettable: false - updateIds: (updatedVos) -> for vo in updatedVos @variantOverrides[vo.hub_id][vo.variant_id].id = vo.id - resetStock: -> - $http - method: "POST" - url: "/admin/variant_overrides/bulk_reset" - data: - variant_overrides: variantOverrides - updateData: (updatedVos) -> for vo in updatedVos @variantOverrides[vo.hub_id][vo.variant_id] = vo diff --git a/app/controllers/admin/variant_overrides_controller.rb b/app/controllers/admin/variant_overrides_controller.rb index 8010a00a11..c886f80652 100644 --- a/app/controllers/admin/variant_overrides_controller.rb +++ b/app/controllers/admin/variant_overrides_controller.rb @@ -4,9 +4,10 @@ module Admin class VariantOverridesController < ResourceController include OpenFoodNetwork::SpreeApiKeyLoader + prepend_before_filter :load_data + before_filter :load_collection, only: [:bulk_update] before_filter :load_spree_api_key, only: :index - before_filter :load_data - before_filter :load_collection, only: [:bulk_update, :bulk_reset] + def index end @@ -30,14 +31,13 @@ module Admin def bulk_reset # Ensure we're authorised to update all variant overrides. - @vo_set.collection.each { |vo| authorize! :bulk_reset, vo } + @collection.each { |vo| authorize! :bulk_reset, vo } + @collection.each(&:reset_stock!) - # Changed this to use class method instead, to ensure the value in the database is used to reset and not a dirty passed-in value - #vo_set.collection.map! { |vo| vo = vo.reset_stock! } - @vo_set.collection.map! { |vo| VariantOverride.reset_stock!(vo.hub,vo.variant) } - render json: @vo_set.collection, each_serializer: Api::Admin::VariantOverrideSerializer - if @vo_set.errors.present? - render json: { errors: @vo_set.errors }, status: 400 + if collection_errors.present? + render json: { errors: collection_errors }, status: 400 + else + render json: @collection, each_serializer: Api::Admin::VariantOverrideSerializer end end @@ -54,8 +54,6 @@ module Admin @hub_permissions = OpenFoodNetwork::Permissions.new(spree_current_user). variant_override_enterprises_per_hub - - @variant_overrides = VariantOverride.for_hubs(@hubs) end def load_collection @@ -64,10 +62,20 @@ module Admin end def collection + @variant_overrides = VariantOverride.for_hubs(params[:hub_id] || @hubs) end def collection_actions [:index, :bulk_update, :bulk_reset] end + + # This has been pulled from ModelSet as it is useful for compiling a list of errors on any generic collection (not necessarily a ModelSet) + # Could be pulled down into a lower level controller if it is useful in other high level controllers + def collection_errors + errors = ActiveModel::Errors.new self + full_messages = @collection.map { |element| element.errors.full_messages }.flatten + full_messages.each { |fm| errors.add(:base, fm) } + errors + end end end diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb index ec3adebdaf..21820ce0db 100644 --- a/app/models/variant_override.rb +++ b/app/models/variant_override.rb @@ -67,15 +67,6 @@ class VariantOverride < ActiveRecord::Base self end - def self.reset_stock!(hub, variant) - vo = self.for(hub, variant) - if vo.nil? - Bugsnag.notify RuntimeError.new "Attempting to reset stock level for a variant without a VariantOverride." - else - vo.reset_stock! - end - end - private def self.for(hub, variant) diff --git a/db/schema.rb b/db/schema.rb index 08cc2bb643..465530b949 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1159,10 +1159,10 @@ ActiveRecord::Schema.define(:version => 20151128185900) do t.integer "hub_id", :null => false t.decimal "price", :precision => 8, :scale => 2 t.integer "count_on_hand" - t.string "sku" - t.boolean "on_demand" t.integer "default_stock" t.boolean "resettable" + t.string "sku" + t.boolean "on_demand" end add_index "variant_overrides", ["variant_id", "hub_id"], :name => "index_variant_overrides_on_variant_id_and_hub_id" diff --git a/spec/controllers/admin/variant_overrides_controller_spec.rb b/spec/controllers/admin/variant_overrides_controller_spec.rb index 2300c38490..d796f2d52f 100644 --- a/spec/controllers/admin/variant_overrides_controller_spec.rb +++ b/spec/controllers/admin/variant_overrides_controller_spec.rb @@ -76,37 +76,57 @@ describe Admin::VariantOverridesController, type: :controller do let!(:variant_override1) { create(:variant_override, hub: hub, variant: variant1, count_on_hand: 5, default_stock: 7, resettable: true) } let!(:variant_override2) { create(:variant_override, hub: hub, variant: variant2, count_on_hand: 2, default_stock: 1, resettable: false) } - before do - allow(controller).to receive(:spree_current_user) { hub.owner } - end + let(:params) { { format: format, hub_id: hub.id } } - context "where the producer has not granted create_variant_overrides permission to the hub" do - let(:params) { { format: format, variant_overrides: [ { id: variant_override1.id } ] } } + context "where I don't manage the variant override hub" do + before do + user = create(:user) + user.owned_enterprises << create(:enterprise) + allow(controller).to receive(:spree_current_user) { user } + end - it "restricts access" do + it "redirects to unauthorized" do spree_put :bulk_reset, params expect(response).to redirect_to spree.unauthorized_path end end - context "where the producer has granted create_variant_overrides permission to the hub" do - let!(:er1) { create(:enterprise_relationship, parent: producer, child: hub, permissions_list: [:create_variant_overrides]) } + context "where I manage the variant override hub" do + before do + allow(controller).to receive(:spree_current_user) { hub.owner } + end - context "where reset is enabled" do - let(:params) { { format: format, variant_overrides: [ { id: variant_override1.id } ] } } - - it "updates stock to default values" do + context "where the producer has not granted create_variant_overrides permission to the hub" do + it "restricts access" do spree_put :bulk_reset, params - expect(variant_override1.reload.count_on_hand).to eq 7 + expect(response).to redirect_to spree.unauthorized_path end end - context "where reset is disabled" do - let(:params) { { format: format, variant_overrides: [ { id: variant_override2.id } ] } } + context "where the producer has granted create_variant_overrides permission to the hub" do + let!(:er1) { create(:enterprise_relationship, parent: producer, child: hub, permissions_list: [:create_variant_overrides]) } - it "doesn't update on_hand" do + it "updates stock to default values where reset is enabled" do + expect(variant_override1.reload.count_on_hand).to eq 5 # reset enabled + expect(variant_override2.reload.count_on_hand).to eq 2 # reset disabled spree_put :bulk_reset, params - expect(variant_override2.reload.count_on_hand).to eq 2 + expect(variant_override1.reload.count_on_hand).to eq 7 # reset enabled + expect(variant_override2.reload.count_on_hand).to eq 2 # reset disabled + end + + context "and the producer has granted create_variant_overrides permission to another hub I manage" do + before { hub.owner.update_attribute(:enterprise_limit, 2) } + let(:hub2) { create(:distributor_enterprise, owner: hub.owner) } + let(:product) { create(:product, supplier: producer) } + let(:variant3) { create(:variant, product: product) } + let!(:variant_override3) { create(:variant_override, hub: hub2, variant: variant3, count_on_hand: 1, default_stock: 13, resettable: true) } + let!(:er2) { create(:enterprise_relationship, parent: producer, child: hub2, permissions_list: [:create_variant_overrides]) } + + it "does not reset count_on_hand for variant_overrides not in params" do + expect { + spree_put :bulk_reset, params + }.to_not change{variant_override3.reload.count_on_hand} + end end end end diff --git a/spec/javascripts/unit/admin/controllers/variant_overrides_controller_spec.js.coffee b/spec/javascripts/unit/admin/controllers/variant_overrides_controller_spec.js.coffee index 44922e4d21..699e1bc4ab 100644 --- a/spec/javascripts/unit/admin/controllers/variant_overrides_controller_spec.js.coffee +++ b/spec/javascripts/unit/admin/controllers/variant_overrides_controller_spec.js.coffee @@ -1,6 +1,6 @@ describe "VariantOverridesCtrl", -> ctrl = null - scope = null + scope = {} hubs = [{id: 1, name: 'Hub'}] producers = [{id: 2, name: 'Producer'}] products = [{id: 1, name: 'Product'}] @@ -19,7 +19,6 @@ describe "VariantOverridesCtrl", -> $provide.value 'variantOverrides', variantOverrides $provide.value 'dirtyVariantOverrides', dirtyVariantOverrides null - scope = {} inject ($controller, _VariantOverrides_, _DirtyVariantOverrides_, _StatusMessage_) -> VariantOverrides = _VariantOverrides_ @@ -69,13 +68,19 @@ describe "VariantOverridesCtrl", -> describe "setting stock to defaults", -> it "prompts to save changes if there are any pending", -> - spyOn(VariantOverrides,"resetStock") + spyOn(StatusMessage, "display") DirtyVariantOverrides.add {hub_id: 1, variant_id: 1} - scope.resetStock - #expect(scope.StatusMessage.statusMessage.text).toMatch "changes" - expect(VariantOverrides.resetStock).not.toHaveBeenCalled - it "updates and refreshes on hand value for variant overrides with a default stock level", -> - spyOn(VariantOverrides,"resetStock") - scope.resetStock - expect(VariantOverrides.resetStock).toHaveBeenCalled - #expect(scope.StatusMessage.statusMessage.text).toMatch "defaults" + scope.resetStock() + expect(StatusMessage.display).toHaveBeenCalledWith 'alert', 'Save changes first.' + + it "updates and refreshes on hand value for variant overrides with a default stock level", inject ($httpBackend) -> + scope.hub_id = 123 + variant_overrides_mock = "mock object" + spyOn(StatusMessage, "display") + spyOn(VariantOverrides, "updateData") + $httpBackend.expectPOST("/admin/variant_overrides/bulk_reset", hub_id: 123).respond 200, variant_overrides_mock + scope.resetStock() + expect(StatusMessage.display).toHaveBeenCalledWith 'progress', 'Changing on hand stock levels...' + $httpBackend.flush() + expect(VariantOverrides.updateData).toHaveBeenCalledWith variant_overrides_mock + expect(StatusMessage.display).toHaveBeenCalledWith 'success', 'Stocks reset to defaults.' diff --git a/spec/javascripts/unit/admin/services/variant_overrides_spec.js.coffee b/spec/javascripts/unit/admin/services/variant_overrides_spec.js.coffee index c596b95ae9..73dbdabee8 100644 --- a/spec/javascripts/unit/admin/services/variant_overrides_spec.js.coffee +++ b/spec/javascripts/unit/admin/services/variant_overrides_spec.js.coffee @@ -67,11 +67,6 @@ describe "VariantOverrides service", -> expect(VariantOverrides.variantOverrides[2][3].id).toEqual 1 expect(VariantOverrides.variantOverrides[2][8].id).toEqual 6 - it "sends an HTTP request to reset stock", -> - $httpBackend.expectPOST("/admin/variant_overrides/bulk_reset", variant_overrides: variantOverrides).respond 200 - VariantOverrides.resetStock variantOverrides - $httpBackend.flush() - it "updates the variant overrides on the page with new data", -> VariantOverrides.variantOverrides[1] = 3: {id: 1, hub_id: 1, variant_id: 3, price: "4.0", count_on_hand: 5, default_stock: 3, resettable: true}