From 521f227f742252e2ab168e1eacf8bafec4f440eb Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 18 Dec 2015 09:56:56 +1100 Subject: [PATCH] Adding sku and on_demand to VariantOverride --- .../variant_overrides_controller.js.coffee | 2 + .../services/variant_overrides.js.coffee | 6 +- app/models/spree/ability_decorator.rb | 2 + app/models/variant_override_set.rb | 9 ++- .../api/admin/variant_override_serializer.rb | 2 +- .../api/admin/variant_serializer.rb | 2 +- .../variant_overrides/_products.html.haml | 2 + .../_products_product.html.haml | 2 + .../_products_variants.html.haml | 5 +- ..._on_demand_and_sku_to_variant_overrides.rb | 6 ++ db/schema.rb | 4 +- lib/open_food_network/scope_variant_to_hub.rb | 19 +++-- .../variant_overrides_controller_spec.rb | 57 +++++++++++++++ .../scope_variant_to_hub_spec.rb | 73 ++++++++++++++++++- 14 files changed, 176 insertions(+), 15 deletions(-) create mode 100644 db/migrate/20151126235409_add_on_demand_and_sku_to_variant_overrides.rb create mode 100644 spec/controllers/admin/variant_overrides_controller_spec.rb 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 290e2b4afc..f4c819f14a 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 @@ -11,8 +11,10 @@ angular.module("admin.variantOverrides").controller "AdminVariantOverridesCtrl", $scope.columns = Columns.setColumns producer: { name: "Producer", visible: true } product: { name: "Product", visible: true } + sku: { name: "SKU", visible: false } price: { name: "Price", visible: true } on_hand: { name: "On Hand", visible: true } + on_demand: { name: "On Demand", visible: false } $scope.resetSelectFilters = -> $scope.producerFilter = 0 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 697d459b75..524dc9056a 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 @@ -15,8 +15,10 @@ angular.module("admin.variantOverrides").factory "VariantOverrides", (variantOve @variantOverrides[hub.id][variant.id] ||= variant_id: variant.id hub_id: hub.id - price: '' - count_on_hand: '' + sku: null + price: null + count_on_hand: null + on_demand: null updateIds: (updatedVos) -> for vo in updatedVos diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 25e0ced2cc..b9716e2eb3 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -112,6 +112,8 @@ class AbilityDecorator end can [:admin, :index, :read, :update, :bulk_update], VariantOverride do |vo| + next false unless vo.hub.present? && vo.variant.andand.product.andand.supplier.present? + hub_auth = OpenFoodNetwork::Permissions.new(user). variant_override_hubs. include? vo.hub diff --git a/app/models/variant_override_set.rb b/app/models/variant_override_set.rb index 985190095b..0fc0d170e9 100644 --- a/app/models/variant_override_set.rb +++ b/app/models/variant_override_set.rb @@ -1,6 +1,13 @@ class VariantOverrideSet < ModelSet def initialize(collection, attributes={}) super(VariantOverride, collection, attributes, nil, - proc { |attrs| attrs['price'].blank? && attrs['count_on_hand'].blank? } ) + proc { |attrs| deletable?(attrs) } ) + end + + def deletable?(attrs) + attrs['price'].blank? && + attrs['count_on_hand'].blank? && + attrs['sku'].nil? && + attrs['on_demand'].nil? end end diff --git a/app/serializers/api/admin/variant_override_serializer.rb b/app/serializers/api/admin/variant_override_serializer.rb index ebe76a1049..d4f584d9ea 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, :price, :count_on_hand + attributes :id, :hub_id, :variant_id, :sku, :price, :count_on_hand, :on_demand end diff --git a/app/serializers/api/admin/variant_serializer.rb b/app/serializers/api/admin/variant_serializer.rb index 510f7af333..66acfe8ece 100644 --- a/app/serializers/api/admin/variant_serializer.rb +++ b/app/serializers/api/admin/variant_serializer.rb @@ -1,5 +1,5 @@ class Api::Admin::VariantSerializer < ActiveModel::Serializer - attributes :id, :options_text, :unit_value, :unit_description, :unit_to_display, :on_demand, :display_as, :display_name, :name_to_display + attributes :id, :options_text, :unit_value, :unit_description, :unit_to_display, :on_demand, :display_as, :display_name, :name_to_display, :sku attributes :on_hand, :price has_many :variant_overrides diff --git a/app/views/admin/variant_overrides/_products.html.haml b/app/views/admin/variant_overrides/_products.html.haml index 34125f4b05..f4b692c06b 100644 --- a/app/views/admin/variant_overrides/_products.html.haml +++ b/app/views/admin/variant_overrides/_products.html.haml @@ -3,8 +3,10 @@ %tr{ ng: { controller: "ColumnsCtrl" } } %th.producer{ ng: { show: 'columns.producer.visible' } } Producer %th.product{ ng: { show: 'columns.product.visible' } } Product + %th.sku{ ng: { show: 'columns.sku.visible' } } SKU %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? %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 a7e9989c70..825be1a6f6 100644 --- a/app/views/admin/variant_overrides/_products_product.html.haml +++ b/app/views/admin/variant_overrides/_products_product.html.haml @@ -1,5 +1,7 @@ %tr.product.even %td.producer{ ng: { show: 'columns.producer.visible' }, bo: { bind: 'producersByID[product.producer_id].name'} } %td.product{ ng: { show: 'columns.product.visible' }, bo: { bind: 'product.name'} } + %td.sku{ ng: { show: 'columns.sku.visible' } } %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' } } diff --git a/app/views/admin/variant_overrides/_products_variants.html.haml b/app/views/admin/variant_overrides/_products_variants.html.haml index 324ef9070f..687197f09a 100644 --- a/app/views/admin/variant_overrides/_products_variants.html.haml +++ b/app/views/admin/variant_overrides/_products_variants.html.haml @@ -3,8 +3,11 @@ %td.product{ ng: { show: 'columns.product.visible' } } %span{ bo: { bind: 'variant.display_name || ""'} } .variant-override-unit{ bo: { bind: 'variant.unit_to_display'} } + %td.sku{ ng: { show: 'columns.sku.visible' } } + %input{name: 'variant-overrides-{{ variant.id }}-sku', type: 'text', ng: {model: 'variantOverrides[hub.id][variant.id].sku'}, placeholder: '{{ variant.sku }}', 'ofn-track-variant-override' => 'sku'} %td.price{ ng: { show: 'columns.price.visible' } } %input{name: 'variant-overrides-{{ variant.id }}-price', type: 'text', ng: {model: 'variantOverrides[hub.id][variant.id].price'}, placeholder: '{{ variant.price }}', 'ofn-track-variant-override' => 'price'} - %td.on_hand{ ng: { show: 'columns.on_hand.visible' } } %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' => 'price'} + %td.on_demand{ ng: { show: 'columns.on_demand.visible' } } + %input.field{ :type => 'checkbox', name: 'variant-overrides-{{ variant.id }}-on_demand', ng: { model: 'variantOverrides[hub.id][variant.id].on_demand' }, 'ofn-track-variant-override' => 'on_demand' } diff --git a/db/migrate/20151126235409_add_on_demand_and_sku_to_variant_overrides.rb b/db/migrate/20151126235409_add_on_demand_and_sku_to_variant_overrides.rb new file mode 100644 index 0000000000..9c47bfbc27 --- /dev/null +++ b/db/migrate/20151126235409_add_on_demand_and_sku_to_variant_overrides.rb @@ -0,0 +1,6 @@ +class AddOnDemandAndSkuToVariantOverrides < ActiveRecord::Migration + def change + add_column :variant_overrides, :sku, :string, :default => nil, :after => :hub_id + add_column :variant_overrides, :on_demand, :boolean, :default => nil, :after => :count_on_hand + end +end diff --git a/db/schema.rb b/db/schema.rb index fb50bec0a1..91895e993d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended to check this file into your version control system. -ActiveRecord::Schema.define(:version => 20151125051510) do +ActiveRecord::Schema.define(:version => 20151126235409) do create_table "account_invoices", :force => true do |t| t.integer "user_id", :null => false @@ -1159,6 +1159,8 @@ ActiveRecord::Schema.define(:version => 20151125051510) 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" end add_index "variant_overrides", ["variant_id", "hub_id"], :name => "index_variant_overrides_on_variant_id_and_hub_id" diff --git a/lib/open_food_network/scope_variant_to_hub.rb b/lib/open_food_network/scope_variant_to_hub.rb index 80396ffa17..37a2455616 100644 --- a/lib/open_food_network/scope_variant_to_hub.rb +++ b/lib/open_food_network/scope_variant_to_hub.rb @@ -26,12 +26,16 @@ module OpenFoodNetwork end def on_demand - if @variant_override.andand.count_on_hand.present? - # If we're overriding the stock level of an on_demand variant, show it as not - # on_demand, so our stock control can take effect. - false + if @variant_override.andand.on_demand.nil? + if @variant_override.andand.count_on_hand.present? + # If we're overriding the stock level of an on_demand variant, show it as not + # on_demand, so our stock control can take effect. + false + else + super + end else - super + @variant_override.andand.on_demand end end @@ -42,7 +46,10 @@ module OpenFoodNetwork super end end - end + def sku + @variant_override.andand.sku || super + end + end end end diff --git a/spec/controllers/admin/variant_overrides_controller_spec.rb b/spec/controllers/admin/variant_overrides_controller_spec.rb new file mode 100644 index 0000000000..7e58e7fd42 --- /dev/null +++ b/spec/controllers/admin/variant_overrides_controller_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe Admin::VariantOverridesController, type: :controller do + # include AuthenticationWorkflow + + describe "bulk_update" do + context "json" do + let(:format) { :json } + + let(:hub) { create(:distributor_enterprise) } + let(:variant) { create(:variant) } + let!(:variant_override) { create(:variant_override, hub: hub, variant: variant) } + let(:variant_override_params) { [ { id: variant_override.id, price: 123.45, count_on_hand: 321, sku: "MySKU", on_demand: false } ] } + + context "where I don't manage the variant override hub" do + before do + user = create(:user) + user.owned_enterprises << create(:enterprise) + controller.stub spree_current_user: user + end + + it "redirects to unauthorized" do + spree_put :bulk_update, format: format, variant_overrides: variant_override_params + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "where I manage the variant override hub" do + before do + controller.stub spree_current_user: hub.owner + end + + context "but the producer has not granted VO permission" do + it "redirects to unauthorized" do + spree_put :bulk_update, format: format, variant_overrides: variant_override_params + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "and the producer has granted VO permission" do + before do + create(:enterprise_relationship, parent: variant.product.supplier, child: hub, permissions_list: [:create_variant_overrides]) + end + + it "allows me to update the variant override" do + spree_put :bulk_update, format: format, variant_overrides: variant_override_params + variant_override.reload + expect(variant_override.price).to eq 123.45 + expect(variant_override.count_on_hand).to eq 321 + expect(variant_override.sku).to eq "MySKU" + expect(variant_override.on_demand).to eq false + end + end + end + end + end +end diff --git a/spec/lib/open_food_network/scope_variant_to_hub_spec.rb b/spec/lib/open_food_network/scope_variant_to_hub_spec.rb index 2a249f9c7a..82b24c7e02 100644 --- a/spec/lib/open_food_network/scope_variant_to_hub_spec.rb +++ b/spec/lib/open_food_network/scope_variant_to_hub_spec.rb @@ -3,8 +3,8 @@ require 'open_food_network/scope_variant_to_hub' module OpenFoodNetwork describe ScopeVariantToHub do let(:hub) { create(:distributor_enterprise) } - let(:v) { create(:variant, price: 11.11, count_on_hand: 1) } - let(:vo) { create(:variant_override, hub: hub, variant: v, price: 22.22, count_on_hand: 2) } + let(:v) { create(:variant, price: 11.11, count_on_hand: 1, on_demand: true, sku: "VARIANTSKU") } + let(:vo) { create(:variant_override, hub: hub, variant: v, price: 22.22, count_on_hand: 2, on_demand: false, sku: "VOSKU") } let(:vo_price_only) { create(:variant_override, hub: hub, variant: v, price: 22.22, count_on_hand: nil) } let(:scoper) { ScopeVariantToHub.new(hub) } @@ -66,6 +66,75 @@ module OpenFoodNetwork v.on_demand.should be_true end end + + describe "overriding on_demand" do + context "when an override exists" do + before { vo } + + context "with an on_demand set" do + it "returns the overridden on_demand" do + scoper.scope v + expect(v.on_demand).to be_false + end + end + + context "without an on_demand set" do + before { vo.update_column(:on_demand, nil) } + + context "when count_on_hand is set" do + it "returns false" do + scoper.scope v + expect(v.on_demand).to be_false + end + end + + context "when count_on_hand is not set" do + before { vo.update_column(:count_on_hand, nil) } + + it "returns the variant's on_demand" do + scoper.scope v + expect(v.on_demand).to be_true + end + end + end + end + + context "when no override exists" do + it "returns the variant's on_demand" do + scoper.scope v + expect(v.on_demand).to be_true + end + end + end + + describe "overriding sku" do + context "when an override exists" do + before { vo } + + context "with an sku set" do + it "returns the overridden sku" do + scoper.scope v + expect(v.sku).to eq "VOSKU" + end + end + + context "without an sku set" do + before { vo.update_column(:sku, nil) } + + it "returns the variant's sku" do + scoper.scope v + expect(v.sku).to eq "VARIANTSKU" + end + end + end + + context "when no override exists" do + it "returns the variant's sku" do + scoper.scope v + expect(v.sku).to eq "VARIANTSKU" + end + end + end end end end