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 c2fab4e7c7..bb4a89d7f6 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 @@ -59,11 +59,11 @@ angular.module("admin.variantOverrides").controller "AdminVariantOverridesCtrl", StatusMessage.display 'progress', 'Saving...' DirtyVariantOverrides.save() .success (updatedVos) -> - console.log DirtyVariantOverrides.all() DirtyVariantOverrides.clear() VariantOverrides.updateIds updatedVos - StatusMessage.display 'success', 'Changes saved.' $scope.variant_overrides_form.$setPristine() + StatusMessage.display 'success', 'Changes saved.' + VariantOverrides.updateData updatedVos # Refresh page data .error (data, status) -> StatusMessage.display 'failure', $scope.updateError(data, status) @@ -78,12 +78,18 @@ angular.module("admin.variantOverrides").controller "AdminVariantOverridesCtrl", errors = errors.concat field_errors errors = errors.join ', ' "I had some trouble saving: #{errors}" - else "Oh no! I was unable to save your changes." $scope.resetStock = -> - VariantOverrides.resetStock() - .success (updatedVos) -> - VariantOverrides.updateData updatedVos - $timeout -> StatusMessage.display 'success', 'Stocks reset to defaults.' + if DirtyVariantOverrides.count() > 0 + StatusMessage.display 'alert', 'Save changes first.' + $timeout -> + $scope.displayDirty() + , 3000 # 3 second delay + else + StatusMessage.display 'progress', 'Changing on hand stock levels...' + VariantOverrides.resetStock() + .success (updatedVos) -> + VariantOverrides.updateData updatedVos + $timeout -> 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 559305b1a3..72caecbcc1 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 @@ -20,6 +20,7 @@ angular.module("admin.variantOverrides").factory "VariantOverrides", (variantOve count_on_hand: null on_demand: null default_stock: null + enable_reset: false updateIds: (updatedVos) -> for vo in updatedVos diff --git a/app/controllers/admin/variant_overrides_controller.rb b/app/controllers/admin/variant_overrides_controller.rb index 9edf82a82a..e187b8a085 100644 --- a/app/controllers/admin/variant_overrides_controller.rb +++ b/app/controllers/admin/variant_overrides_controller.rb @@ -33,10 +33,12 @@ module Admin collection_hash = Hash[params[:variant_overrides].each_with_index.map { |vo, i| [i, vo] }] vo_set = VariantOverrideSet.new @variant_overrides, collection_attributes: collection_hash - # Ensure we're authorised to update all variant overrides + # Ensure we're authorised to update all variant overrides. vo_set.collection.each { |vo| authorize! :update, vo } - vo_set.collection.map! { |vo| vo = vo.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 diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb index c7d111fefc..efac7e5bd6 100644 --- a/app/models/variant_override.rb +++ b/app/models/variant_override.rb @@ -4,7 +4,7 @@ class VariantOverride < ActiveRecord::Base validates_presence_of :hub_id, :variant_id # Default stock can be nil, indicating stock should not be reset or zero, meaning reset to zero. Need to ensure this can be set by the user. - validates :default_stock, numericality: { greater_than: 0 }, allow_nil: true + validates :default_stock, numericality: { greater_than_or_equal_to: 0 }, allow_nil: true scope :for_hubs, lambda { |hubs| where(hub_id: hubs) @@ -50,22 +50,25 @@ class VariantOverride < ActiveRecord::Base Bugsnag.notify RuntimeError.new "Attempting to decrement stock level on a VariantOverride without a count_on_hand specified." end end - + def default_stock? default_stock.present? end - + def reset_stock! - if default_stock? - self.attributes = { count_on_hand: default_stock } - self.save + if enable_reset + if default_stock? + self.attributes = { count_on_hand: default_stock } + self.save + else + Bugsnag.notify RuntimeError.new "Attempting to reset stock level for a variant with no default stock level." + end end - self + 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 diff --git a/app/models/variant_override_set.rb b/app/models/variant_override_set.rb index 0fc0d170e9..f1aaf1074c 100644 --- a/app/models/variant_override_set.rb +++ b/app/models/variant_override_set.rb @@ -1,12 +1,13 @@ class VariantOverrideSet < ModelSet def initialize(collection, attributes={}) - super(VariantOverride, collection, attributes, nil, - proc { |attrs| deletable?(attrs) } ) + super(VariantOverride, collection, attributes, nil, proc { |attrs| deletable?(attrs) } ) end def deletable?(attrs) attrs['price'].blank? && attrs['count_on_hand'].blank? && + attrs['default_stock'].blank? && + attrs['enable_reset'].blank? && attrs['sku'].nil? && attrs['on_demand'].nil? end diff --git a/app/serializers/api/admin/variant_override_serializer.rb b/app/serializers/api/admin/variant_override_serializer.rb index 54b4419fb7..fa989af96e 100644 --- a/app/serializers/api/admin/variant_override_serializer.rb +++ b/app/serializers/api/admin/variant_override_serializer.rb @@ -1,3 +1,3 @@ class Api::Admin::VariantOverrideSerializer < ActiveModel::Serializer - attributes :id, :hub_id, :variant_id, :sku, :price, :count_on_hand, :on_demand, :default_stock + attributes :id, :hub_id, :variant_id, :sku, :price, :count_on_hand, :on_demand, :default_stock, :enable_reset end diff --git a/app/views/admin/variant_overrides/_products.html.haml b/app/views/admin/variant_overrides/_products.html.haml index f268c90130..b88febeab3 100644 --- a/app/views/admin/variant_overrides/_products.html.haml +++ b/app/views/admin/variant_overrides/_products.html.haml @@ -7,7 +7,7 @@ %th.price{ ng: { show: 'columns.price.visible' } } Price %th.on_hand{ ng: { show: 'columns.on_hand.visible' } } On hand %th.on_demand{ ng: { show: 'columns.on_demand.visible' } } On Demand? - %th Default stock + %th{colspan: 2} Enable stock level reset? %tbody{bindonce: true, ng: {repeat: 'product in products | hubPermissions:hubPermissions:hub.id | attrFilter:{producer_id:producerFilter} | filter:query' } } = render 'admin/variant_overrides/products_product' = render 'admin/variant_overrides/products_variants' diff --git a/app/views/admin/variant_overrides/_products_product.html.haml b/app/views/admin/variant_overrides/_products_product.html.haml index 6f58c9bd75..1bcf4cf016 100644 --- a/app/views/admin/variant_overrides/_products_product.html.haml +++ b/app/views/admin/variant_overrides/_products_product.html.haml @@ -5,4 +5,4 @@ %td.price{ ng: { show: 'columns.price.visible' } } %td.on_hand{ ng: { show: 'columns.on_hand.visible' } } %td.on_demand{ ng: { show: 'columns.on_demand.visible' } } - %td + %td{colspan: 2} diff --git a/app/views/admin/variant_overrides/_products_variants.html.haml b/app/views/admin/variant_overrides/_products_variants.html.haml index 510689b83d..9e5bf1f356 100644 --- a/app/views/admin/variant_overrides/_products_variants.html.haml +++ b/app/views/admin/variant_overrides/_products_variants.html.haml @@ -15,4 +15,6 @@ %input{name: 'variant-overrides-{{ variant.id }}-count-on-hand', type: 'text', ng: {model: 'variantOverrides[hub.id][variant.id].count_on_hand'}, placeholder: '{{ variant.on_hand }}', 'ofn-track-variant-override' => 'count_on_hand'} %td - %input{name: 'variant-overrides-{{ variant.id }}-default-stock', type: 'text', ng: {model: 'variantOverrides[hub.id][variant.id].default_stock'}, placeholder: '{{ variant.default_stock }}', 'ofn-track-variant-override' => 'default_stock'} + %input{name: 'variant-overrides-{{ variant.id }}-enable_reset', type: 'checkbox', ng: {model: 'variantOverrides[hub.id][variant.id].enable_reset'}, placeholder: '{{ variant.enable_reset }}', 'ofn-track-variant-override' => 'enable_reset'} + %td + %input{name: 'variant-overrides-{{ variant.id }}-default-stock', type: 'text', ng: {model: 'variantOverrides[hub.id][variant.id].default_stock'}, placeholder: '{{ variant.default_stock ? variant.default_stock : "Default stock"}}', 'ofn-track-variant-override' => 'default_stock'} diff --git a/db/migrate/20150827194622_add_enable_reset_to_variant_overrides.rb b/db/migrate/20150827194622_add_enable_reset_to_variant_overrides.rb new file mode 100644 index 0000000000..172cce6588 --- /dev/null +++ b/db/migrate/20150827194622_add_enable_reset_to_variant_overrides.rb @@ -0,0 +1,5 @@ +class AddEnableResetToVariantOverrides < ActiveRecord::Migration + def change + add_column :variant_overrides, :enable_reset, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 9adf1c5391..60f3819562 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1162,6 +1162,7 @@ ActiveRecord::Schema.define(:version => 20151126235409) do t.string "sku" t.boolean "on_demand" t.integer "default_stock" + t.boolean "enable_reset" 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_spec.rb b/spec/controllers/admin/variant_overrides_spec.rb index f726bd8869..aa2596ab34 100644 --- a/spec/controllers/admin/variant_overrides_spec.rb +++ b/spec/controllers/admin/variant_overrides_spec.rb @@ -34,16 +34,26 @@ module Admin end context "when a reset request is received" do it "updates stock to default values" do + v1 = create(:variant) + v2 = create(:variant) + vo1 = create(:variant_override, hub: hub, variant: v1, price: "6.0", count_on_hand: 5, default_stock: 7, enable_reset: true) + vo2 = create(:variant_override, hub: hub, variant: v2, price: "6.0", count_on_hand: 2, default_stock: 1, enable_reset: false) + params = {"variant_overrides" => [vo1.attributes, vo2.attributes]} + spree_put :bulk_reset, params + + vo1.reload + expect(vo1.count_on_hand).to eq 7 + end + it "doesn't update where reset is disabled" do v1 = create(:variant) v2 = create(:variant) - vo1 = create(:variant_override, hub: hub, variant: v1, price: "6.0", count_on_hand: 5, default_stock: 7) - vo2 = create(:variant_override, hub: hub, variant: v2, price: "6.0", count_on_hand: 2, default_stock: 1) + vo1 = create(:variant_override, hub: hub, variant: v1, price: "6.0", count_on_hand: 5, default_stock: 7, enable_reset: true) + vo2 = create(:variant_override, hub: hub, variant: v2, price: "6.0", count_on_hand: 2, default_stock: 1, enable_reset: false) params = {"variant_overrides" => [vo1.attributes, vo2.attributes]} spree_put :bulk_reset, params - vo1.reload - expect(vo1.count_on_hand).to eq 7 + vo2.reload - expect(vo2.count_on_hand).to eq 1 + expect(vo2.count_on_hand).to eq 2 end end end diff --git a/spec/factories.rb b/spec/factories.rb index 8ae6a094c2..f6eeff4680 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -94,6 +94,8 @@ FactoryGirl.define do factory :variant_override, :class => VariantOverride do price 77.77 count_on_hand 11111 + default_stock 2000 + enable_reset false end factory :enterprise, :class => Enterprise do 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 22b8300409..e75cf51692 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 @@ -7,18 +7,25 @@ describe "VariantOverridesCtrl", -> hubPermissions = {} VariantOverrides = null variantOverrides = {} + DirtyVariantOverrides = null + dirtyVariantOverrides = {} + StatusMessage = null + statusMessage = {} beforeEach -> module 'admin.variantOverrides' module ($provide) -> $provide.value 'SpreeApiKey', 'API_KEY' $provide.value 'variantOverrides', variantOverrides + $provide.value 'dirtyVariantOverrides', dirtyVariantOverrides null scope = {} - inject ($controller, _VariantOverrides_) -> + inject ($controller, Indexer, _VariantOverrides_, _DirtyVariantOverrides_, _StatusMessage_) -> VariantOverrides = _VariantOverrides_ - ctrl = $controller 'AdminVariantOverridesCtrl', {$scope: scope, hubs: hubs, producers: producers, products: products, hubPermissions: hubPermissions, VariantOverrides: _VariantOverrides_} + DirtyVariantOverrides = _DirtyVariantOverrides_ + StatusMessage = _StatusMessage_ + ctrl = $controller 'AdminVariantOverridesCtrl', {$scope: scope, hubs: hubs, producers: producers, products: products, hubPermissions: hubPermissions, VariantOverrides: _VariantOverrides_, DirtyVariantOverrides: _DirtyVariantOverrides_, StatusMessage: _StatusMessage_} it "initialises the hub list and the chosen hub", -> expect(scope.hubs).toEqual { 1: {id: 1, name: 'Hub'} } @@ -59,3 +66,16 @@ describe "VariantOverridesCtrl", -> it "returns a generic message otherwise", -> expect(scope.updateError({}, 500)).toEqual "Oh no! I was unable to save your changes." + + describe "setting stock to defaults", -> + it "prompts to save changes if there are any pending", -> + spyOn(VariantOverrides,"resetStock") + 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" diff --git a/spec/models/variant_override_spec.rb b/spec/models/variant_override_spec.rb index 2cd422e01d..d1b7f0f84a 100644 --- a/spec/models/variant_override_spec.rb +++ b/spec/models/variant_override_spec.rb @@ -49,16 +49,16 @@ describe VariantOverride do describe "checking if stock levels have been overriden" do it "returns true when stock level has been overridden" do create(:variant_override, variant: variant, hub: hub, count_on_hand: 12) - VariantOverride.stock_overridden?(hub, variant).should be_true + VariantOverride.stock_overridden?(hub, variant).should be true end it "returns false when the override has no stock level" do create(:variant_override, variant: variant, hub: hub, count_on_hand: nil) - VariantOverride.stock_overridden?(hub, variant).should be_false + VariantOverride.stock_overridden?(hub, variant).should be false end it "returns false when there is no override for the hub/variant" do - VariantOverride.stock_overridden?(hub, variant).should be_false + VariantOverride.stock_overridden?(hub, variant).should be false end end @@ -69,43 +69,40 @@ describe VariantOverride do vo.reload.count_on_hand.should == 10 end - it "silently logs an error if the variant override doesn't have a stock level" do - vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: nil) - Bugsnag.should_receive(:notify) - VariantOverride.decrement_stock! hub, variant, 2 - vo.reload.count_on_hand.should be_nil - end - it "silently logs an error if the variant override does not exist" do Bugsnag.should_receive(:notify) VariantOverride.decrement_stock! hub, variant, 2 end end - + describe "checking default stock value is present" do it "returns true when a default stock level has been set" do vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock: 20) - vo.default_stock?.should be_true + vo.default_stock?.should be true end it "returns false when the override has no default stock level" do vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock:nil) - vo.default_stock.should be_false + vo.default_stock?.should be false end end describe "resetting stock levels" do it "resets the on hand level to the value in the default_stock field" do - vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock: 20) + vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock: 20, enable_reset: true) vo.reset_stock! vo.reload.count_on_hand.should == 20 end it "silently logs an error if the variant override doesn't have a default stock level" do - vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock:nil) + vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock:nil, enable_reset: true) Bugsnag.should_receive(:notify) vo.reset_stock! vo.reload.count_on_hand.should == 12 - vo.reload.default_stock.should be_nil + end + it "doesn't reset the level if the behaviour is disabled" do + vo = create(:variant_override, variant: variant, hub: hub, count_on_hand: 12, default_stock: 10, enable_reset: false) + vo.reset_stock! + vo.reload.count_on_hand.should == 12 end end end