diff --git a/.travis.yml b/.travis.yml index 0ba99dc9e1..56d94e5952 100644 --- a/.travis.yml +++ b/.travis.yml @@ -38,7 +38,7 @@ before_script: script: - 'if [ "$KARMA" = "true" ]; then bundle exec rake karma:run; else echo "Skipping karma run"; fi' #- "KNAPSACK_GENERATE_REPORT=true bundle exec rspec spec" - - "bundle exec rake knapsack:rspec" + - "bundle exec rake 'knapsack:rspec[--tag ~performance]'" after_success: - > diff --git a/Gemfile b/Gemfile index 300b17ffa3..3690b194b8 100644 --- a/Gemfile +++ b/Gemfile @@ -55,6 +55,7 @@ gem 'figaro' gem 'blockenspiel' gem 'acts-as-taggable-on', '~> 3.4' gem 'paper_trail', '~> 3.0.8' +gem 'diffy' gem 'wicked_pdf' gem 'wkhtmltopdf-binary' diff --git a/Gemfile.lock b/Gemfile.lock index 6dd9888f8c..7d0022ccca 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -248,6 +248,7 @@ GEM devise-encryptable (0.1.2) devise (>= 2.1.0) diff-lcs (1.2.4) + diffy (3.1.0) em-websocket (0.5.0) eventmachine (>= 0.12.9) http_parser.rb (~> 0.5.3) @@ -668,6 +669,7 @@ DEPENDENCIES debugger-linecache deface! delayed_job_active_record + diffy factory_girl_rails figaro foreigner diff --git a/app/assets/javascripts/darkswarm/services/map.js.coffee b/app/assets/javascripts/darkswarm/services/map.js.coffee index 96768e8379..f4bad04acc 100644 --- a/app/assets/javascripts/darkswarm/services/map.js.coffee +++ b/app/assets/javascripts/darkswarm/services/map.js.coffee @@ -2,6 +2,8 @@ Darkswarm.factory "OfnMap", (Enterprises, EnterpriseModal, visibleFilter) -> new class OfnMap constructor: -> @enterprises = @enterprise_markers(Enterprises.enterprises) + @enterprises = @enterprises.filter (enterprise) -> + enterprise.latitude != null || enterprise.longitude != null # Remove enterprises w/o lat or long enterprise_markers: (enterprises) -> @extend(enterprise) for enterprise in visibleFilter(enterprises) diff --git a/app/controllers/admin/cache_settings_controller.rb b/app/controllers/admin/cache_settings_controller.rb new file mode 100644 index 0000000000..138bbc7b18 --- /dev/null +++ b/app/controllers/admin/cache_settings_controller.rb @@ -0,0 +1,13 @@ +require 'open_food_network/products_cache_integrity_checker' + +class Admin::CacheSettingsController < Spree::Admin::BaseController + + def show + @results = Exchange.cachable.map do |exchange| + checker = OpenFoodNetwork::ProductsCacheIntegrityChecker.new(exchange.receiver, exchange.order_cycle) + + {distributor: exchange.receiver, order_cycle: exchange.order_cycle, status: checker.ok?, diff: checker.diff} + end + end + +end diff --git a/app/controllers/shop_controller.rb b/app/controllers/shop_controller.rb index 8d7b9ab274..55b6bcc02b 100644 --- a/app/controllers/shop_controller.rb +++ b/app/controllers/shop_controller.rb @@ -1,4 +1,4 @@ -require 'open_food_network/products_renderer' +require 'open_food_network/cached_products_renderer' class ShopController < BaseController layout "darkswarm" @@ -11,11 +11,11 @@ class ShopController < BaseController def products begin - products_json = OpenFoodNetwork::ProductsRenderer.new(current_distributor, current_order_cycle).products + products_json = OpenFoodNetwork::CachedProductsRenderer.new(current_distributor, current_order_cycle).products_json render json: products_json - rescue OpenFoodNetwork::ProductsRenderer::NoProducts + rescue OpenFoodNetwork::CachedProductsRenderer::NoProducts render status: 404, json: '' end end diff --git a/app/jobs/products_cache_integrity_checker_job.rb b/app/jobs/products_cache_integrity_checker_job.rb new file mode 100644 index 0000000000..78feb3a1b0 --- /dev/null +++ b/app/jobs/products_cache_integrity_checker_job.rb @@ -0,0 +1,24 @@ +require 'open_food_network/products_cache_integrity_checker' + +ProductsCacheIntegrityCheckerJob = Struct.new(:distributor_id, :order_cycle_id) do + def perform + unless checker.ok? + Bugsnag.notify RuntimeError.new("Products JSON differs from cached version for distributor: #{distributor_id}, order cycle: #{order_cycle_id}"), diff: checker.diff.to_s(:text) + end + end + + + private + + def checker + OpenFoodNetwork::ProductsCacheIntegrityChecker.new(distributor, order_cycle) + end + + def distributor + Enterprise.find distributor_id + end + + def order_cycle + OrderCycle.find order_cycle_id + end +end diff --git a/app/jobs/refresh_products_cache_job.rb b/app/jobs/refresh_products_cache_job.rb new file mode 100644 index 0000000000..0c66925be8 --- /dev/null +++ b/app/jobs/refresh_products_cache_job.rb @@ -0,0 +1,16 @@ +require 'open_food_network/products_renderer' + +RefreshProductsCacheJob = Struct.new(:distributor_id, :order_cycle_id) do + def perform + Rails.cache.write "products-json-#{distributor_id}-#{order_cycle_id}", products_json + end + + + private + + def products_json + distributor = Enterprise.find distributor_id + order_cycle = OrderCycle.find order_cycle_id + OpenFoodNetwork::ProductsRenderer.new(distributor, order_cycle).products_json + end +end diff --git a/app/models/coordinator_fee.rb b/app/models/coordinator_fee.rb new file mode 100644 index 0000000000..135ee821a9 --- /dev/null +++ b/app/models/coordinator_fee.rb @@ -0,0 +1,15 @@ +class CoordinatorFee < ActiveRecord::Base + belongs_to :order_cycle + belongs_to :enterprise_fee + + after_save :refresh_products_cache + after_destroy :refresh_products_cache + + + private + + def refresh_products_cache + order_cycle.refresh_products_cache + end + +end diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index 59b2648d30..d1d0816340 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -1,11 +1,18 @@ class EnterpriseFee < ActiveRecord::Base belongs_to :enterprise belongs_to :tax_category, class_name: 'Spree::TaxCategory', foreign_key: 'tax_category_id' - has_and_belongs_to_many :order_cycles, join_table: 'coordinator_fees' + + has_many :coordinator_fees, dependent: :destroy + has_many :order_cycles, through: :coordinator_fees + has_many :exchange_fees, dependent: :destroy has_many :exchanges, through: :exchange_fees - before_destroy { order_cycles.clear } + + after_save :refresh_products_cache + # After destroy, the products cache is refreshed via the after_destroy hook for + # coordinator_fees and exchange_fees + calculated_adjustments @@ -59,6 +66,7 @@ class EnterpriseFee < ActiveRecord::Base :locked => true}, :without_protection => true) end + private def ensure_valid_tax_category_settings @@ -72,4 +80,8 @@ class EnterpriseFee < ActiveRecord::Base end return true end + + def refresh_products_cache + OpenFoodNetwork::ProductsCache.enterprise_fee_changed self + end end diff --git a/app/models/exchange.rb b/app/models/exchange.rb index 137924f7ac..ad290f6c26 100644 --- a/app/models/exchange.rb +++ b/app/models/exchange.rb @@ -13,6 +13,9 @@ class Exchange < ActiveRecord::Base validates_presence_of :order_cycle, :sender, :receiver validates_uniqueness_of :sender_id, :scope => [:order_cycle_id, :receiver_id, :incoming] + after_save :refresh_products_cache + after_destroy :refresh_products_cache_from_destroy + accepts_nested_attributes_for :variants scope :in_order_cycle, lambda { |order_cycle| where(order_cycle_id: order_cycle) } @@ -31,6 +34,12 @@ class Exchange < ActiveRecord::Base joins('INNER JOIN enterprises AS receiver ON (receiver.id = exchanges.receiver_id)'). order("CASE WHEN exchanges.incoming='t' THEN sender.name ELSE receiver.name END") + # Exchanges on order cycles that are dated and are upcoming or open are cached + scope :cachable, outgoing. + joins(:order_cycle). + merge(OrderCycle.dated). + merge(OrderCycle.not_closed) + scope :managed_by, lambda { |user| if user.has_spree_role?('admin') scoped @@ -75,4 +84,11 @@ class Exchange < ActiveRecord::Base end end + def refresh_products_cache + OpenFoodNetwork::ProductsCache.exchange_changed self + end + + def refresh_products_cache_from_destroy + OpenFoodNetwork::ProductsCache.exchange_destroyed self + end end diff --git a/app/models/exchange_fee.rb b/app/models/exchange_fee.rb index ff9f12e8dd..03fce8ce04 100644 --- a/app/models/exchange_fee.rb +++ b/app/models/exchange_fee.rb @@ -1,4 +1,15 @@ class ExchangeFee < ActiveRecord::Base belongs_to :exchange belongs_to :enterprise_fee + + + after_save :refresh_products_cache + after_destroy :refresh_products_cache + + + private + + def refresh_products_cache + exchange.refresh_products_cache + end end diff --git a/app/models/inventory_item.rb b/app/models/inventory_item.rb index 2648e38341..60e1713612 100644 --- a/app/models/inventory_item.rb +++ b/app/models/inventory_item.rb @@ -1,3 +1,5 @@ +require 'open_food_network/products_cache' + class InventoryItem < ActiveRecord::Base attr_accessible :enterprise, :enterprise_id, :variant, :variant_id, :visible @@ -11,4 +13,13 @@ class InventoryItem < ActiveRecord::Base scope :visible, where(visible: true) scope :hidden, where(visible: false) + + after_save :refresh_products_cache + + + private + + def refresh_products_cache + OpenFoodNetwork::ProductsCache.inventory_item_changed self + end end diff --git a/app/models/order_cycle.rb b/app/models/order_cycle.rb index 31a68faa37..894cbac7f7 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -1,6 +1,8 @@ class OrderCycle < ActiveRecord::Base belongs_to :coordinator, :class_name => 'Enterprise' - has_and_belongs_to_many :coordinator_fees, :class_name => 'EnterpriseFee', :join_table => 'coordinator_fees' + + has_many :coordinator_fee_refs, class_name: 'CoordinatorFee' + has_many :coordinator_fees, through: :coordinator_fee_refs, source: :enterprise_fee has_many :exchanges, :dependent => :destroy @@ -11,14 +13,18 @@ class OrderCycle < ActiveRecord::Base validates_presence_of :name, :coordinator_id + after_save :refresh_products_cache + preference :product_selection_from_coordinator_inventory_only, :boolean, default: false scope :active, lambda { where('order_cycles.orders_open_at <= ? AND order_cycles.orders_close_at >= ?', Time.zone.now, Time.zone.now) } scope :active_or_complete, lambda { where('order_cycles.orders_open_at <= ?', Time.zone.now) } scope :inactive, lambda { where('order_cycles.orders_open_at > ? OR order_cycles.orders_close_at < ?', Time.zone.now, Time.zone.now) } scope :upcoming, lambda { where('order_cycles.orders_open_at > ?', Time.zone.now) } + scope :not_closed, lambda { where('order_cycles.orders_close_at > ? OR order_cycles.orders_close_at IS NULL', Time.zone.now) } scope :closed, lambda { where('order_cycles.orders_close_at < ?', Time.zone.now).order("order_cycles.orders_close_at DESC") } scope :undated, where('order_cycles.orders_open_at IS NULL OR orders_close_at IS NULL') + scope :dated, where('orders_open_at IS NOT NULL AND orders_close_at IS NOT NULL') scope :soonest_closing, lambda { active.order('order_cycles.orders_close_at ASC') } # TODO This method returns all the closed orders. So maybe we can replace it with :recently_closed. @@ -187,6 +193,10 @@ class OrderCycle < ActiveRecord::Base self.variants.include? variant end + def dated? + !undated? + end + def undated? self.orders_open_at.nil? || self.orders_close_at.nil? end @@ -236,6 +246,10 @@ class OrderCycle < ActiveRecord::Base coordinator.users.include? user end + def refresh_products_cache + OpenFoodNetwork::ProductsCache.order_cycle_changed self + end + private diff --git a/app/models/producer_property.rb b/app/models/producer_property.rb index 178e7646ff..5b9fdd6d1c 100644 --- a/app/models/producer_property.rb +++ b/app/models/producer_property.rb @@ -1,8 +1,12 @@ class ProducerProperty < ActiveRecord::Base + belongs_to :producer, class_name: 'Enterprise' belongs_to :property, class_name: 'Spree::Property' default_scope order("#{self.table_name}.position") + after_save :refresh_products_cache + after_destroy :refresh_products_cache_from_destroy + def property_name property.name if property @@ -14,4 +18,16 @@ class ProducerProperty < ActiveRecord::Base Spree::Property.create(name: name, presentation: name) end end + + + private + + def refresh_products_cache + OpenFoodNetwork::ProductsCache.producer_property_changed self + end + + def refresh_products_cache_from_destroy + OpenFoodNetwork::ProductsCache.producer_property_destroyed self + end + end diff --git a/app/models/spree/classification_decorator.rb b/app/models/spree/classification_decorator.rb index c69eb01fcf..5e9655a907 100644 --- a/app/models/spree/classification_decorator.rb +++ b/app/models/spree/classification_decorator.rb @@ -1,6 +1,15 @@ Spree::Classification.class_eval do belongs_to :product, :class_name => "Spree::Product", touch: true + after_save :refresh_products_cache before_destroy :dont_destroy_if_primary_taxon + after_destroy :refresh_products_cache + + + private + + def refresh_products_cache + product.refresh_products_cache + end def dont_destroy_if_primary_taxon if product.primary_taxon == taxon diff --git a/app/models/spree/image_decorator.rb b/app/models/spree/image_decorator.rb index 7d86aae3a6..24056926b7 100644 --- a/app/models/spree/image_decorator.rb +++ b/app/models/spree/image_decorator.rb @@ -1,4 +1,7 @@ Spree::Image.class_eval do + after_save :refresh_products_cache + after_destroy :refresh_products_cache + # Spree stores attachent definitions in JSON. This converts the style name and format to # strings. However, when paperclip encounters these, it doesn't recognise the format. # Here we solve that problem by converting format and style name to symbols. @@ -20,4 +23,11 @@ Spree::Image.class_eval do end reformat_styles + + + private + + def refresh_products_cache + viewable.try :refresh_products_cache + end end diff --git a/app/models/spree/option_type_decorator.rb b/app/models/spree/option_type_decorator.rb new file mode 100644 index 0000000000..aea706c847 --- /dev/null +++ b/app/models/spree/option_type_decorator.rb @@ -0,0 +1,13 @@ +module Spree + OptionType.class_eval do + has_many :products, through: :product_option_types + after_save :refresh_products_cache + + + private + + def refresh_products_cache + products(:reload).each &:refresh_products_cache + end + end +end diff --git a/app/models/spree/option_value_decorator.rb b/app/models/spree/option_value_decorator.rb new file mode 100644 index 0000000000..cfe9c23cca --- /dev/null +++ b/app/models/spree/option_value_decorator.rb @@ -0,0 +1,20 @@ +module Spree + OptionValue.class_eval do + after_save :refresh_products_cache + around_destroy :refresh_products_cache_from_destroy + + + private + + def refresh_products_cache + variants(:reload).each &:refresh_products_cache + end + + def refresh_products_cache_from_destroy + vs = variants(:reload).to_a + yield + vs.each &:refresh_products_cache + end + + end +end diff --git a/app/models/spree/preference_decorator.rb b/app/models/spree/preference_decorator.rb new file mode 100644 index 0000000000..c3ab3f9a94 --- /dev/null +++ b/app/models/spree/preference_decorator.rb @@ -0,0 +1,31 @@ +require 'open_food_network/products_cache' + +module Spree + Preference.class_eval do + after_save :refresh_products_cache + + # When the setting preferred_product_selection_from_inventory_only has changed, we want to + # refresh all active exchanges for this enterprise. + def refresh_products_cache + if product_selection_from_inventory_only_changed? + OpenFoodNetwork::ProductsCache.distributor_changed(enterprise) + end + end + + + private + + def product_selection_from_inventory_only_changed? + key =~ product_selection_from_inventory_only_regex + end + + def enterprise + enterprise_id = key.match(product_selection_from_inventory_only_regex)[1] + enterprise = Enterprise.find enterprise_id + end + + def product_selection_from_inventory_only_regex + /^enterprise\/product_selection_from_inventory_only\/(\d+)$/ + end + end +end diff --git a/app/models/spree/price_decorator.rb b/app/models/spree/price_decorator.rb new file mode 100644 index 0000000000..f50a86066b --- /dev/null +++ b/app/models/spree/price_decorator.rb @@ -0,0 +1,12 @@ +module Spree + Price.class_eval do + after_save :refresh_products_cache + + + private + + def refresh_products_cache + variant.andand.refresh_products_cache + end + end +end diff --git a/app/models/spree/product_decorator.rb b/app/models/spree/product_decorator.rb index 07b9eafd8e..e6acd440c7 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -22,8 +22,6 @@ Spree::Product.class_eval do attr_accessible :variant_unit, :variant_unit_scale, :variant_unit_name, :unit_value attr_accessible :inherits_properties, :sku - before_validation :sanitize_permalink - # validates_presence_of :variants, unless: :new_record?, message: "Product must have at least one variant" validates_presence_of :supplier validates :primary_taxon, presence: { message: I18n.t("validation_msg_product_category_cant_be_blank") } @@ -35,11 +33,13 @@ Spree::Product.class_eval do validates_presence_of :variant_unit_name, if: -> p { p.variant_unit == 'items' } - after_save :ensure_standard_variant after_initialize :set_available_on_to_now, :if => :new_record? - after_save :update_units - after_touch :touch_distributors + before_validation :sanitize_permalink before_save :add_primary_taxon_to_taxons + after_touch :touch_distributors + after_save :ensure_standard_variant + after_save :update_units + after_save :refresh_products_cache # -- Joins @@ -198,6 +198,11 @@ Spree::Product.class_eval do alias_method_chain :delete, :delete_from_order_cycles + def refresh_products_cache + OpenFoodNetwork::ProductsCache.product_changed self + end + + private def set_available_on_to_now diff --git a/app/models/spree/product_property_decorator.rb b/app/models/spree/product_property_decorator.rb new file mode 100644 index 0000000000..0f3329c03d --- /dev/null +++ b/app/models/spree/product_property_decorator.rb @@ -0,0 +1,10 @@ +module Spree + ProductProperty.class_eval do + after_save :refresh_products_cache + after_destroy :refresh_products_cache + + def refresh_products_cache + product.refresh_products_cache + end + end +end diff --git a/app/models/spree/property_decorator.rb b/app/models/spree/property_decorator.rb new file mode 100644 index 0000000000..5b8e53338c --- /dev/null +++ b/app/models/spree/property_decorator.rb @@ -0,0 +1,15 @@ +module Spree + Property.class_eval do + after_save :refresh_products_cache + + # When a Property is destroyed, dependent-destroy will destroy all ProductProperties, + # which will take care of refreshing the products cache + + + private + + def refresh_products_cache + product_properties(:reload).each &:refresh_products_cache + end + end +end diff --git a/app/models/spree/taxon_decorator.rb b/app/models/spree/taxon_decorator.rb index 1a26ce73a8..a1e07cc53e 100644 --- a/app/models/spree/taxon_decorator.rb +++ b/app/models/spree/taxon_decorator.rb @@ -1,7 +1,12 @@ Spree::Taxon.class_eval do + has_many :classifications, :dependent => :destroy + + self.attachment_definitions[:icon][:path] = 'public/images/spree/taxons/:id/:style/:basename.:extension' self.attachment_definitions[:icon][:url] = '/images/spree/taxons/:id/:style/:basename.:extension' + after_save :refresh_products_cache + # Indicate which filters should be used for this taxon def applicable_filters @@ -45,4 +50,11 @@ Spree::Taxon.class_eval do taxons end + + + private + + def refresh_products_cache + products(:reload).each &:refresh_products_cache + end end diff --git a/app/models/spree/variant_decorator.rb b/app/models/spree/variant_decorator.rb index ea04a1a5ac..21a228de19 100644 --- a/app/models/spree/variant_decorator.rb +++ b/app/models/spree/variant_decorator.rb @@ -1,5 +1,6 @@ require 'open_food_network/enterprise_fee_calculator' require 'open_food_network/variant_and_line_item_naming' +require 'open_food_network/products_cache' Spree::Variant.class_eval do # Remove method From Spree, so method from the naming module is used instead @@ -9,7 +10,7 @@ Spree::Variant.class_eval do include OpenFoodNetwork::VariantAndLineItemNaming - has_many :exchange_variants, dependent: :destroy + has_many :exchange_variants has_many :exchanges, through: :exchange_variants has_many :variant_overrides has_many :inventory_items @@ -25,6 +26,9 @@ Spree::Variant.class_eval do before_validation :update_weight_from_unit_value, if: -> v { v.product.present? } after_save :update_units + after_save :refresh_products_cache + around_destroy :destruction + scope :with_order_cycles_inner, joins(exchanges: :order_cycle) @@ -92,9 +96,37 @@ Spree::Variant.class_eval do end end + def refresh_products_cache + if is_master? + product.refresh_products_cache + else + OpenFoodNetwork::ProductsCache.variant_changed self + end + end + + private def update_weight_from_unit_value self.weight = weight_from_unit_value if self.product.variant_unit == 'weight' && unit_value.present? end + + def destruction + if is_master? + exchange_variants(:reload).destroy_all + yield + product.refresh_products_cache + + else + OpenFoodNetwork::ProductsCache.variant_destroyed(self) do + # Remove this association here instead of using dependent: :destroy because + # dependent-destroy acts before this around_filter is called, so ProductsCache + # has no way of knowing which exchanges the variant was a member of. + exchange_variants(:reload).destroy_all + + # Destroy the variant + yield + end + end + end end diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb index baede3e62d..b373900e0c 100644 --- a/app/models/variant_override.rb +++ b/app/models/variant_override.rb @@ -6,6 +6,9 @@ class VariantOverride < ActiveRecord::Base # 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_or_equal_to: 0 }, allow_nil: true + after_save :refresh_products_cache_from_save + after_destroy :refresh_products_cache_from_destroy + default_scope where(permission_revoked_at: nil) scope :for_hubs, lambda { |hubs| @@ -75,4 +78,11 @@ class VariantOverride < ActiveRecord::Base VariantOverride.where(variant_id: variant, hub_id: hub).first end + def refresh_products_cache_from_save + OpenFoodNetwork::ProductsCache.variant_override_changed self + end + + def refresh_products_cache_from_destroy + OpenFoodNetwork::ProductsCache.variant_override_destroyed self + end end diff --git a/app/overrides/spree/admin/shared/_configuration_menu/add_caching.html.haml.deface b/app/overrides/spree/admin/shared/_configuration_menu/add_caching.html.haml.deface new file mode 100644 index 0000000000..1eb416e72a --- /dev/null +++ b/app/overrides/spree/admin/shared/_configuration_menu/add_caching.html.haml.deface @@ -0,0 +1,4 @@ +/ insert_bottom "[data-hook='admin_configurations_sidebar_menu']" + +%li + = link_to 'Caching', main_app.admin_cache_settings_path diff --git a/app/serializers/api/product_serializer.rb b/app/serializers/api/product_serializer.rb index 03a66afe89..aa353173dc 100644 --- a/app/serializers/api/product_serializer.rb +++ b/app/serializers/api/product_serializer.rb @@ -36,7 +36,7 @@ class Api::CachedProductSerializer < ActiveModel::Serializer #delegate :cache_key, to: :object include ActionView::Helpers::SanitizeHelper - attributes :id, :name, :permalink, :count_on_hand + attributes :id, :name, :permalink attributes :on_demand, :group_buy, :notes, :description attributes :properties_with_values diff --git a/app/views/admin/cache_settings/show.html.haml b/app/views/admin/cache_settings/show.html.haml new file mode 100644 index 0000000000..79b3e5acaf --- /dev/null +++ b/app/views/admin/cache_settings/show.html.haml @@ -0,0 +1,18 @@ +- content_for :page_title do + = t(:cache_settings) + +%table.index + %thead + %tr + %th Distributor + %th Order Cycle + %th Status + %th Diff + %tbody + - @results.each do |result| + %tr + %td= result[:distributor].name + %td= result[:order_cycle].name + %td= result[:status] ? 'OK' : 'Error' + %td + %pre= result[:diff].to_s(:text) diff --git a/config/routes.rb b/config/routes.rb index 9821a8b4bf..c314747786 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -126,6 +126,8 @@ Openfoodnetwork::Application.routes.draw do resource :business_model_configuration, only: [:edit, :update], controller: 'business_model_configuration' + resource :cache_settings + resource :account, only: [:show], controller: 'account' end diff --git a/config/schedule.rb b/config/schedule.rb index a09ca55dce..023382330a 100644 --- a/config/schedule.rb +++ b/config/schedule.rb @@ -7,6 +7,10 @@ env "MAILTO", "rohan@rohanmitchell.com" # If we use -e with a file containing specs, rspec interprets it and filters out our examples job_type :run_file, "cd :path; :environment_variable=:environment bundle exec script/rails runner :task :output" +every 1.hour do + rake 'openfoodnetwork:cache:check_products_integrity' +end + every 1.day, at: '12:05am' do run_file "lib/open_food_network/integrity_checker.rb" end diff --git a/db/migrate/20160204013914_add_id_to_coordinator_fees.rb b/db/migrate/20160204013914_add_id_to_coordinator_fees.rb new file mode 100644 index 0000000000..74326a6006 --- /dev/null +++ b/db/migrate/20160204013914_add_id_to_coordinator_fees.rb @@ -0,0 +1,5 @@ +class AddIdToCoordinatorFees < ActiveRecord::Migration + def change + add_column :coordinator_fees, :id, :primary_key + end +end diff --git a/db/schema.rb b/db/schema.rb index c84337565a..cdab93834c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -176,7 +176,7 @@ ActiveRecord::Schema.define(:version => 20160302044850) do add_index "cms_snippets", ["site_id", "identifier"], :name => "index_cms_snippets_on_site_id_and_identifier", :unique => true add_index "cms_snippets", ["site_id", "position"], :name => "index_cms_snippets_on_site_id_and_position" - create_table "coordinator_fees", :id => false, :force => true do |t| + create_table "coordinator_fees", :force => true do |t| t.integer "order_cycle_id" t.integer "enterprise_fee_id" end @@ -682,9 +682,9 @@ ActiveRecord::Schema.define(:version => 20160302044850) do t.string "email" t.text "special_instructions" t.integer "distributor_id" - t.integer "order_cycle_id" t.string "currency" t.string "last_ip_address" + t.integer "order_cycle_id" t.integer "cart_id" t.integer "customer_id" end diff --git a/lib/open_food_network/cached_products_renderer.rb b/lib/open_food_network/cached_products_renderer.rb new file mode 100644 index 0000000000..2174bf4094 --- /dev/null +++ b/lib/open_food_network/cached_products_renderer.rb @@ -0,0 +1,47 @@ +require 'open_food_network/products_renderer' + +# Wrapper for ProductsRenderer that caches the JSON output. +# ProductsRenderer::NoProducts is represented in the cache as nil, +# but re-raised to provide the same interface as ProductsRenderer. + +module OpenFoodNetwork + class CachedProductsRenderer + class NoProducts < Exception; end + + def initialize(distributor, order_cycle) + @distributor = distributor + @order_cycle = order_cycle + end + + def products_json + raise NoProducts.new if @distributor.nil? || @order_cycle.nil? + + products_json = Rails.cache.fetch("products-json-#{@distributor.id}-#{@order_cycle.id}") do + log_warning + + begin + uncached_products_json + rescue ProductsRenderer::NoProducts + nil + end + end + + raise NoProducts.new if products_json.nil? + + products_json + end + + + private + + def log_warning + if Rails.env.production? || Rails.env.staging? + Bugsnag.notify RuntimeError.new("Live server MISS on products cache for distributor: #{@distributor.id}, order cycle: #{@order_cycle.id}") + end + end + + def uncached_products_json + ProductsRenderer.new(@distributor, @order_cycle).products_json + end + end +end diff --git a/lib/open_food_network/products_cache.rb b/lib/open_food_network/products_cache.rb new file mode 100644 index 0000000000..f6ef15829f --- /dev/null +++ b/lib/open_food_network/products_cache.rb @@ -0,0 +1,182 @@ +require 'open_food_network/products_cache_refreshment' + +# When elements of the data model change, refresh the appropriate parts of the products cache. + +module OpenFoodNetwork + class ProductsCache + def self.variant_changed(variant) + exchanges_featuring_variants(variant).each do |exchange| + refresh_cache exchange.receiver, exchange.order_cycle + end + end + + + def self.variant_destroyed(variant, &block) + exchanges = exchanges_featuring_variants(variant).to_a + + block.call + + exchanges.each do |exchange| + refresh_cache exchange.receiver, exchange.order_cycle + end + end + + + def self.product_changed(product) + exchanges_featuring_variants(product.variants).each do |exchange| + refresh_cache exchange.receiver, exchange.order_cycle + end + end + + + def self.variant_override_changed(variant_override) + exchanges_featuring_variants(variant_override.variant, distributor: variant_override.hub).each do |exchange| + refresh_cache exchange.receiver, exchange.order_cycle + end + end + + + def self.variant_override_destroyed(variant_override) + variant_override_changed variant_override + end + + + def self.producer_property_changed(producer_property) + products = producer_property.producer.supplied_products + variants = Spree::Variant. + where(is_master: false, deleted_at: nil). + where(product_id: products) + + exchanges_featuring_variants(variants).each do |exchange| + refresh_cache exchange.receiver, exchange.order_cycle + end + end + + + def self.producer_property_destroyed(producer_property) + producer_property_changed producer_property + end + + + def self.order_cycle_changed(order_cycle) + if order_cycle.dated? && !order_cycle.closed? + order_cycle.exchanges.outgoing.each do |exchange| + refresh_cache exchange.receiver, order_cycle + end + end + end + + + def self.exchange_changed(exchange) + if exchange.incoming + refresh_incoming_exchanges(Exchange.where(id: exchange)) + else + refresh_outgoing_exchange(exchange) + end + end + + + def self.exchange_destroyed(exchange) + exchange_changed exchange + end + + + def self.enterprise_fee_changed(enterprise_fee) + refresh_supplier_fee enterprise_fee + refresh_coordinator_fee enterprise_fee + refresh_distributor_fee enterprise_fee + end + + + def self.distributor_changed(enterprise) + Exchange.cachable.where(receiver_id: enterprise).each do |exchange| + refresh_cache exchange.receiver, exchange.order_cycle + end + end + + + def self.inventory_item_changed(inventory_item) + exchanges_featuring_variants(inventory_item.variant, distributor: inventory_item.enterprise).each do |exchange| + refresh_cache exchange.receiver, exchange.order_cycle + end + end + + + private + + def self.exchanges_featuring_variants(variants, distributor: nil) + exchanges = Exchange. + outgoing. + with_any_variant(variants). + joins(:order_cycle). + merge(OrderCycle.dated). + merge(OrderCycle.not_closed) + + exchanges = exchanges.to_enterprise(distributor) if distributor + + exchanges + end + + + def self.refresh_incoming_exchanges(exchanges) + incoming_exchanges(exchanges).map do |exchange| + outgoing_exchanges_with_variants(exchange.order_cycle, exchange.variant_ids) + end.flatten.uniq.each do |exchange| + refresh_cache exchange.receiver, exchange.order_cycle + end + end + + + def self.refresh_outgoing_exchange(exchange) + if exchange.order_cycle.dated? && !exchange.order_cycle.closed? + refresh_cache exchange.receiver, exchange.order_cycle + end + end + + + def self.refresh_supplier_fee(enterprise_fee) + refresh_incoming_exchanges(enterprise_fee.exchanges) + end + + + def self.refresh_coordinator_fee(enterprise_fee) + enterprise_fee.order_cycles.each do |order_cycle| + order_cycle_changed order_cycle + end + end + + + def self.refresh_distributor_fee(enterprise_fee) + enterprise_fee.exchange_fees. + joins(:exchange => :order_cycle). + merge(Exchange.outgoing). + merge(OrderCycle.dated). + merge(OrderCycle.not_closed). + each do |exf| + + refresh_cache exf.exchange.receiver, exf.exchange.order_cycle + end + end + + + def self.incoming_exchanges(exchanges) + exchanges. + incoming. + joins(:order_cycle). + merge(OrderCycle.dated). + merge(OrderCycle.not_closed) + end + + + def self.outgoing_exchanges_with_variants(order_cycle, variant_ids) + order_cycle.exchanges.outgoing. + joins(:exchange_variants). + where('exchange_variants.variant_id IN (?)', variant_ids) + end + + + def self.refresh_cache(distributor, order_cycle) + ProductsCacheRefreshment.refresh distributor, order_cycle + end + end +end diff --git a/lib/open_food_network/products_cache_integrity_checker.rb b/lib/open_food_network/products_cache_integrity_checker.rb new file mode 100644 index 0000000000..8098124feb --- /dev/null +++ b/lib/open_food_network/products_cache_integrity_checker.rb @@ -0,0 +1,35 @@ +require 'open_food_network/products_renderer' + +module OpenFoodNetwork + class ProductsCacheIntegrityChecker + def initialize(distributor, order_cycle) + @distributor = distributor + @order_cycle = order_cycle + end + + def ok? + diff.none? + end + + def diff + @diff ||= Diffy::Diff.new pretty(cached_json), pretty(rendered_json) + end + + + private + + def cached_json + Rails.cache.read("products-json-#{@distributor.id}-#{@order_cycle.id}") || {}.to_json + end + + def rendered_json + OpenFoodNetwork::ProductsRenderer.new(@distributor, @order_cycle).products_json + rescue OpenFoodNetwork::ProductsRenderer::NoProducts + nil + end + + def pretty(json) + JSON.pretty_generate JSON.parse json + end + end +end diff --git a/lib/open_food_network/products_cache_refreshment.rb b/lib/open_food_network/products_cache_refreshment.rb new file mode 100644 index 0000000000..276734570f --- /dev/null +++ b/lib/open_food_network/products_cache_refreshment.rb @@ -0,0 +1,47 @@ +# When enqueuing a job to refresh the products cache for a particular distribution, there +# is no benefit in having more than one job waiting in the queue to be run. + +# Imagine that an admin updates a product. This calls for the products cache to be +# updated, otherwise customers will see stale data. + +# Now while that update is running, the admin makes another change to the product. Since this change +# has been made after the previous update started running, the already-running update will not +# include that change - we need another job. So we enqueue another one. + +# Before that job starts running, our zealous admin makes yet another change. This time, there +# is a job running *and* there is a job that has not yet started to run. In this case, there's no +# benefit in enqueuing another job. When the previously enqueued job starts running, it will pick up +# our admin's update and include it. So we ignore this change (from a cache refreshment perspective) +# and go home happy to have saved our job worker's time. + +module OpenFoodNetwork + class ProductsCacheRefreshment + def self.refresh(distributor, order_cycle) + unless pending_job? distributor, order_cycle + enqueue_job distributor, order_cycle + end + end + + + private + + def self.pending_job?(distributor, order_cycle) + # To inspect each job, we need to deserialize the payload. + # This is slow, and if it's a problem in practice, we could pre-filter in SQL + # for handlers matching the class name, distributor id and order cycle id. + + Delayed::Job. + where(locked_at: nil). + map(&:payload_object). + select { |j| + j.class == RefreshProductsCacheJob && + j.distributor_id == distributor.id && + j.order_cycle_id == order_cycle.id + }.any? + end + + def self.enqueue_job(distributor, order_cycle) + Delayed::Job.enqueue RefreshProductsCacheJob.new(distributor.id, order_cycle.id), priority: 10 + end + end +end diff --git a/lib/open_food_network/products_renderer.rb b/lib/open_food_network/products_renderer.rb index d745b1b794..6dd0d5f5ed 100644 --- a/lib/open_food_network/products_renderer.rb +++ b/lib/open_food_network/products_renderer.rb @@ -9,8 +9,8 @@ module OpenFoodNetwork @order_cycle = order_cycle end - def products - products = products_for_shop + def products_json + products = load_products if products enterprise_fee_calculator = EnterpriseFeeCalculator.new @distributor, @order_cycle @@ -31,7 +31,7 @@ module OpenFoodNetwork private - def products_for_shop + def load_products if @order_cycle scoper = ScopeProductToHub.new(@distributor) diff --git a/lib/tasks/cache.rake b/lib/tasks/cache.rake new file mode 100644 index 0000000000..64a5d3909a --- /dev/null +++ b/lib/tasks/cache.rake @@ -0,0 +1,20 @@ +require 'open_food_network/products_cache_integrity_checker' + +namespace :openfoodnetwork do + namespace :cache do + desc 'check the integrity of the products cache' + task :check_products_integrity => :environment do + Exchange.cachable.each do |exchange| + Delayed::Job.enqueue ProductsCacheIntegrityCheckerJob.new(exchange.receiver_id, exchange.order_cycle_id), priority: 20 + end + end + + + desc 'warm the products cache' + task :warm_products => :environment do + Exchange.cachable.each do |exchange| + Delayed::Job.enqueue RefreshProductsCacheJob.new(exchange.receiver_id, exchange.order_cycle_id), priority: 10 + end + end + end +end diff --git a/spec/controllers/shop_controller_spec.rb b/spec/controllers/shop_controller_spec.rb index 09ea85dd44..73231bc86c 100644 --- a/spec/controllers/shop_controller_spec.rb +++ b/spec/controllers/shop_controller_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe ShopController do - let(:d) { create(:distributor_enterprise) } + let(:distributor) { create(:distributor_enterprise) } it "redirects to the home page if no distributor is selected" do spree_get :show @@ -11,26 +11,26 @@ describe ShopController do describe "with a distributor in place" do before do - controller.stub(:current_distributor).and_return d + controller.stub(:current_distributor).and_return distributor end - describe "Selecting order cycles" do + describe "selecting an order cycle" do it "should select an order cycle when only one order cycle is open" do - oc1 = create(:simple_order_cycle, distributors: [d]) + oc1 = create(:simple_order_cycle, distributors: [distributor]) spree_get :show controller.current_order_cycle.should == oc1 end it "should not set an order cycle when multiple order cycles are open" do - oc1 = create(:simple_order_cycle, distributors: [d]) - oc2 = create(:simple_order_cycle, distributors: [d]) + oc1 = create(:simple_order_cycle, distributors: [distributor]) + oc2 = create(:simple_order_cycle, distributors: [distributor]) spree_get :show - controller.current_order_cycle.should == nil + controller.current_order_cycle.should be_nil end it "should allow the user to post to select the current order cycle" do - oc1 = create(:simple_order_cycle, distributors: [d]) - oc2 = create(:simple_order_cycle, distributors: [d]) + oc1 = create(:simple_order_cycle, distributors: [distributor]) + oc2 = create(:simple_order_cycle, distributors: [distributor]) spree_post :order_cycle, order_cycle_id: oc2.id response.should be_success @@ -39,9 +39,10 @@ describe ShopController do context "JSON tests" do render_views - it "should return the order cycle details when the oc is selected" do - oc1 = create(:simple_order_cycle, distributors: [d]) - oc2 = create(:simple_order_cycle, distributors: [d]) + + it "should return the order cycle details when the OC is selected" do + oc1 = create(:simple_order_cycle, distributors: [distributor]) + oc2 = create(:simple_order_cycle, distributors: [distributor]) spree_post :order_cycle, order_cycle_id: oc2.id response.should be_success @@ -49,7 +50,7 @@ describe ShopController do end it "should return the current order cycle when hit with GET" do - oc1 = create(:simple_order_cycle, distributors: [d]) + oc1 = create(:simple_order_cycle, distributors: [distributor]) controller.stub(:current_order_cycle).and_return oc1 spree_get :order_cycle response.body.should have_content oc1.id @@ -57,13 +58,13 @@ describe ShopController do end it "should not allow the user to select an invalid order cycle" do - oc1 = create(:simple_order_cycle, distributors: [d]) - oc2 = create(:simple_order_cycle, distributors: [d]) + oc1 = create(:simple_order_cycle, distributors: [distributor]) + oc2 = create(:simple_order_cycle, distributors: [distributor]) oc3 = create(:simple_order_cycle, distributors: [create(:distributor_enterprise)]) spree_post :order_cycle, order_cycle_id: oc3.id response.status.should == 404 - controller.current_order_cycle.should == nil + controller.current_order_cycle.should be_nil end end @@ -71,31 +72,32 @@ describe ShopController do describe "producers/suppliers" do let(:supplier) { create(:supplier_enterprise) } let(:product) { create(:product, supplier: supplier) } - let(:order_cycle) { create(:simple_order_cycle, distributors: [d], coordinator: create(:distributor_enterprise)) } + let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor]) } before do - exchange = Exchange.find(order_cycle.exchanges.to_enterprises(d).outgoing.first.id) + exchange = order_cycle.exchanges.to_enterprises(distributor).outgoing.first exchange.variants << product.master end end describe "returning products" do - let(:order_cycle) { create(:simple_order_cycle, distributors: [d], coordinator: create(:distributor_enterprise)) } - let(:exchange) { Exchange.find(order_cycle.exchanges.to_enterprises(d).outgoing.first.id) } + let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor]) } + let(:exchange) { order_cycle.exchanges.to_enterprises(distributor).outgoing.first } describe "requests and responses" do let(:product) { create(:product) } + before do exchange.variants << product.variants.first end - it "returns products via json" do + it "returns products via JSON" do controller.stub(:current_order_cycle).and_return order_cycle xhr :get, :products response.should be_success end - it "does not return products if no order_cycle is selected" do + it "does not return products if no order cycle is selected" do controller.stub(:current_order_cycle).and_return nil xhr :get, :products response.status.should == 404 diff --git a/spec/factories.rb b/spec/factories.rb index 21c6cecd15..2e479696b4 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -60,8 +60,8 @@ FactoryGirl.define do factory :simple_order_cycle, :class => OrderCycle do sequence(:name) { |n| "Order Cycle #{n}" } - orders_open_at { Time.zone.now - 1.day } - orders_close_at { Time.zone.now + 1.week } + orders_open_at { 1.day.ago } + orders_close_at { 1.week.from_now } coordinator { Enterprise.is_distributor.first || FactoryGirl.create(:distributor_enterprise) } @@ -84,6 +84,26 @@ FactoryGirl.define do end end + factory :undated_order_cycle, parent: :simple_order_cycle do + orders_open_at nil + orders_close_at nil + end + + factory :upcoming_order_cycle, parent: :simple_order_cycle do + orders_open_at { 1.week.from_now } + orders_close_at { 2.weeks.from_now } + end + + factory :open_order_cycle, parent: :simple_order_cycle do + orders_open_at { 1.week.ago } + orders_close_at { 1.week.from_now } + end + + factory :closed_order_cycle, parent: :simple_order_cycle do + orders_open_at { 2.weeks.ago } + orders_close_at { 1.week.ago } + end + factory :exchange, :class => Exchange do order_cycle { OrderCycle.first || FactoryGirl.create(:simple_order_cycle) } sender { FactoryGirl.create(:enterprise) } diff --git a/spec/features/admin/caching_spec.rb b/spec/features/admin/caching_spec.rb new file mode 100644 index 0000000000..22d372e19b --- /dev/null +++ b/spec/features/admin/caching_spec.rb @@ -0,0 +1,43 @@ +require 'spec_helper' +require 'open_food_network/products_renderer' + +feature 'Caching' do + include AuthenticationWorkflow + include WebHelper + + before { quick_login_as_admin } + + describe "displaying integrity checker results" do + let(:distributor) { create(:distributor_enterprise) } + let(:order_cycle) { create(:open_order_cycle, distributors: [distributor]) } + + it "displays results when things are good" do + # Given matching data + Rails.cache.write "products-json-#{distributor.id}-#{order_cycle.id}", "[1, 2, 3]\n" + OpenFoodNetwork::ProductsRenderer.stub(:new) { double(:pr, products_json: "[1, 2, 3]\n") } + + # When I visit the cache status page + visit spree.admin_path + click_link 'Configuration' + click_link 'Caching' + + # Then I should see some status information + page.should have_content "OK" + end + + it "displays results when there are errors" do + # Given matching data + Rails.cache.write "products-json-#{distributor.id}-#{order_cycle.id}", "[1, 2, 3]\n" + OpenFoodNetwork::ProductsRenderer.stub(:new) { double(:pr, products_json: "[1, 3]\n") } + + # When I visit the cache status page + visit spree.admin_path + click_link 'Configuration' + click_link 'Caching' + + # Then I should see some status information + page.should have_content "Error" + end + + end +end diff --git a/spec/features/admin/enterprises_spec.rb b/spec/features/admin/enterprises_spec.rb index 46a0105bb4..92ac89015e 100644 --- a/spec/features/admin/enterprises_spec.rb +++ b/spec/features/admin/enterprises_spec.rb @@ -251,6 +251,32 @@ feature %q{ end + describe "inventory settings", js: true do + let!(:enterprise) { create(:distributor_enterprise) } + let!(:product) { create(:simple_product) } + let!(:order_cycle) { create(:simple_order_cycle, distributors: [enterprise], variants: [product.variants.first]) } + + before do + Delayed::Job.destroy_all + quick_login_as_admin + end + + it "refreshes the cache when I change what products appear on my shopfront" do + # Given a product that's not in my inventory, but is in an active order cycle + + # When I change which products appear on the shopfront + visit edit_admin_enterprise_path(enterprise) + within(".side_menu") { click_link 'Inventory Settings' } + choose 'enterprise_preferred_product_selection_from_inventory_only_1' + + # Then a job should have been enqueued to refresh the cache + expect do + click_button 'Update' + end.to enqueue_job RefreshProductsCacheJob, distributor_id: enterprise.id, order_cycle_id: order_cycle.id + end + end + + context "as an Enterprise user", js: true do let(:supplier1) { create(:supplier_enterprise, name: 'First Supplier') } let(:supplier2) { create(:supplier_enterprise, name: 'Another Supplier') } diff --git a/spec/features/admin/payment_method_spec.rb b/spec/features/admin/payment_method_spec.rb index 5c16f886e5..caf52c08f8 100644 --- a/spec/features/admin/payment_method_spec.rb +++ b/spec/features/admin/payment_method_spec.rb @@ -30,7 +30,7 @@ feature %q{ payment_method.distributors.should == [@distributors[0]] end - scenario "updating a payment method" do + scenario "updating a payment method", retry: 3 do pm = create(:payment_method, distributors: [@distributors[0]]) login_to_admin_section diff --git a/spec/features/admin/variant_overrides_spec.rb b/spec/features/admin/variant_overrides_spec.rb index 6f59b78aec..a3a08c2466 100644 --- a/spec/features/admin/variant_overrides_spec.rb +++ b/spec/features/admin/variant_overrides_spec.rb @@ -318,7 +318,7 @@ feature %q{ select2_select hub.name, from: 'hub_id' end - it "alerts the user to the presence of new products, and allows them to be added or hidden" do + it "alerts the user to the presence of new products, and allows them to be added or hidden", retry: 3 do expect(page).to have_no_selector "table#variant-overrides tr#v_#{variant1.id}" expect(page).to have_no_selector "table#variant-overrides tr#v_#{variant2.id}" diff --git a/spec/javascripts/unit/darkswarm/services/map_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/map_spec.js.coffee index 4252000460..669f0ca74d 100644 --- a/spec/javascripts/unit/darkswarm/services/map_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/map_spec.js.coffee @@ -9,6 +9,17 @@ describe "Hubs service", -> orders_close_at: new Date() type: "hub" visible: true + latitude: 0 + longitude: 0 + } + { + id: 3 + active: false + orders_close_at: new Date() + type: "hub" + visible: true + latitude: null + longitude: null } ] @@ -24,3 +35,6 @@ describe "Hubs service", -> it "builds MapMarkers from enterprises", -> expect(OfnMap.enterprises[0].id).toBe enterprises[0].id + + it "excludes enterprises without latitude or longitude", -> + expect(OfnMap.enterprises.map (e) -> e.id).not.toContain enterprises[1].id diff --git a/spec/jobs/products_cache_integrity_checker_job_spec.rb b/spec/jobs/products_cache_integrity_checker_job_spec.rb new file mode 100644 index 0000000000..1770f9bf84 --- /dev/null +++ b/spec/jobs/products_cache_integrity_checker_job_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' +require 'open_food_network/products_renderer' + +describe ProductsCacheIntegrityCheckerJob do + describe "reporting on differences between the products cache and the current products" do + let(:distributor) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle) } + let(:job) { ProductsCacheIntegrityCheckerJob.new distributor.id, order_cycle.id } + + before do + Rails.cache.write "products-json-#{distributor.id}-#{order_cycle.id}", "[1, 2, 3]\n" + OpenFoodNetwork::ProductsRenderer.stub(:new) { double(:pr, products_json: "[1, 3]\n") } + end + + it "reports errors" do + expect(Bugsnag).to receive(:notify) + run_job job + end + + it "deals with nil cached_json" do + Rails.cache.clear + expect(Bugsnag).to receive(:notify) + run_job job + end + end +end diff --git a/spec/jobs/refresh_products_cache_job_spec.rb b/spec/jobs/refresh_products_cache_job_spec.rb new file mode 100644 index 0000000000..d20efe5fad --- /dev/null +++ b/spec/jobs/refresh_products_cache_job_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' +require 'open_food_network/products_renderer' + +describe RefreshProductsCacheJob do + let(:distributor) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle) } + + it "renders products and writes them to cache" do + RefreshProductsCacheJob.any_instance.stub(:products_json) { 'products' } + + run_job RefreshProductsCacheJob.new distributor.id, order_cycle.id + + expect(Rails.cache.read("products-json-#{distributor.id}-#{order_cycle.id}")).to eq 'products' + end + + describe "fetching products JSON" do + let(:job) { RefreshProductsCacheJob.new distributor.id, order_cycle.id } + let(:pr) { double(:products_renderer, products_json: nil) } + + it "fetches products JSON" do + expect(OpenFoodNetwork::ProductsRenderer).to receive(:new).with(distributor, order_cycle) { pr } + job.send(:products_json) + end + end +end diff --git a/spec/lib/open_food_network/cached_products_renderer_spec.rb b/spec/lib/open_food_network/cached_products_renderer_spec.rb new file mode 100644 index 0000000000..03b0ab05d5 --- /dev/null +++ b/spec/lib/open_food_network/cached_products_renderer_spec.rb @@ -0,0 +1,111 @@ +require 'spec_helper' +require 'open_food_network/cached_products_renderer' +require 'open_food_network/products_renderer' + +module OpenFoodNetwork + describe CachedProductsRenderer do + let(:distributor) { double(:distributor, id: 123) } + let(:order_cycle) { double(:order_cycle, id: 456) } + let(:cpr) { CachedProductsRenderer.new(distributor, order_cycle) } + + describe "when the distribution is not set" do + let(:cpr) { CachedProductsRenderer.new(nil, nil) } + + it "raises an exception and returns no products" do + expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts + end + end + + describe "when the products JSON is already cached" do + before do + Rails.cache.write "products-json-#{distributor.id}-#{order_cycle.id}", 'products' + end + + it "returns the cached JSON" do + expect(cpr.products_json).to eq 'products' + end + + it "raises an exception when there are no products" do + Rails.cache.write "products-json-#{distributor.id}-#{order_cycle.id}", nil + expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts + end + end + + describe "when the products JSON is not cached" do + let(:cached_json) { Rails.cache.read "products-json-#{distributor.id}-#{order_cycle.id}" } + let(:cache_present) { Rails.cache.exist? "products-json-#{distributor.id}-#{order_cycle.id}" } + + before do + Rails.cache.clear + cpr.stub(:uncached_products_json) { 'fresh products' } + end + + describe "when there are products" do + it "returns products as JSON" do + expect(cpr.products_json).to eq 'fresh products' + end + + it "caches the JSON" do + cpr.products_json + expect(cached_json).to eq 'fresh products' + end + + it "logs a warning" do + cpr.should_receive :log_warning + cpr.products_json + end + end + + describe "when there are no products" do + before { cpr.stub(:uncached_products_json).and_raise ProductsRenderer::NoProducts } + + it "raises an error" do + expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts + end + + it "caches the products as nil" do + expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts + expect(cache_present).to be + expect(cached_json).to be_nil + end + + it "logs a warning" do + cpr.should_receive :log_warning + expect { cpr.products_json }.to raise_error CachedProductsRenderer::NoProducts + end + end + end + + describe "logging a warning" do + it "logs a warning when in production" do + Rails.env.stub(:production?) { true } + expect(Bugsnag).to receive(:notify) + cpr.send(:log_warning) + end + + it "logs a warning when in staging" do + Rails.env.stub(:production?) { false } + Rails.env.stub(:staging?) { true } + expect(Bugsnag).to receive(:notify) + cpr.send(:log_warning) + end + + it "does not log a warning in development or test" do + expect(Bugsnag).to receive(:notify).never + cpr.send(:log_warning) + end + end + + describe "fetching uncached products from ProductsRenderer" do + let(:pr) { double(:products_renderer, products_json: 'uncached products') } + + before do + ProductsRenderer.stub(:new) { pr } + end + + it "returns the uncached products" do + expect(cpr.send(:uncached_products_json)).to eq 'uncached products' + end + end + end +end diff --git a/spec/lib/open_food_network/products_cache_refreshment_spec.rb b/spec/lib/open_food_network/products_cache_refreshment_spec.rb new file mode 100644 index 0000000000..f65b81346e --- /dev/null +++ b/spec/lib/open_food_network/products_cache_refreshment_spec.rb @@ -0,0 +1,68 @@ +require 'open_food_network/products_cache_refreshment' + +module OpenFoodNetwork + describe ProductsCacheRefreshment do + let(:distributor) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle) } + + before { Delayed::Job.destroy_all } + + describe "when there are no tasks enqueued" do + it "enqueues the task" do + expect do + ProductsCacheRefreshment.refresh distributor, order_cycle + end.to enqueue_job RefreshProductsCacheJob + end + + it "enqueues the job with a lower than default priority" do + ProductsCacheRefreshment.refresh distributor, order_cycle + job = Delayed::Job.last + expect(job.priority).to be > Delayed::Worker.default_priority + end + end + + describe "when there is an enqueued task, and it is running" do + before do + job = Delayed::Job.enqueue RefreshProductsCacheJob.new distributor.id, order_cycle.id + job.update_attributes! locked_by: 'asdf', locked_at: Time.now + end + + it "enqueues another task" do + expect do + ProductsCacheRefreshment.refresh distributor, order_cycle + end.to enqueue_job RefreshProductsCacheJob + end + end + + describe "when there are two enqueued tasks, and one is running" do + before do + job1 = Delayed::Job.enqueue RefreshProductsCacheJob.new distributor.id, order_cycle.id + job1.update_attributes! locked_by: 'asdf', locked_at: Time.now + job2 = Delayed::Job.enqueue RefreshProductsCacheJob.new distributor.id, order_cycle.id + end + + it "does not enqueue another task" do + expect do + ProductsCacheRefreshment.refresh distributor, order_cycle + end.not_to enqueue_job RefreshProductsCacheJob + end + end + + describe "enqueuing tasks with different distributions" do + let(:distributor2) { create(:distributor_enterprise) } + let(:order_cycle2) { create(:simple_order_cycle) } + + before do + job1 = Delayed::Job.enqueue RefreshProductsCacheJob.new distributor.id, order_cycle.id + job1.update_attributes! locked_by: 'asdf', locked_at: Time.now + job2 = Delayed::Job.enqueue RefreshProductsCacheJob.new distributor.id, order_cycle.id + end + + it "ignores tasks with differing distributions when choosing whether to enqueue a job" do + expect do + ProductsCacheRefreshment.refresh distributor2, order_cycle2 + end.to enqueue_job RefreshProductsCacheJob + end + end + end +end diff --git a/spec/lib/open_food_network/products_cache_spec.rb b/spec/lib/open_food_network/products_cache_spec.rb new file mode 100644 index 0000000000..a395873f45 --- /dev/null +++ b/spec/lib/open_food_network/products_cache_spec.rb @@ -0,0 +1,418 @@ +require 'open_food_network/products_cache' + +module OpenFoodNetwork + describe ProductsCache do + describe "when a variant changes" do + let(:variant) { create(:variant) } + let(:variant_undistributed) { create(:variant) } + let(:supplier) { create(:supplier_enterprise) } + let(:coordinator) { create(:distributor_enterprise) } + let(:distributor) { create(:distributor_enterprise) } + let(:oc_undated) { create(:undated_order_cycle, distributors: [distributor], variants: [variant]) } + let(:oc_upcoming) { create(:upcoming_order_cycle, suppliers: [supplier], coordinator: coordinator, distributors: [distributor], variants: [variant]) } + let(:oc_open) { create(:open_order_cycle, distributors: [distributor], variants: [variant]) } + let(:oc_closed) { create(:closed_order_cycle, distributors: [distributor], variants: [variant]) } + + it "refreshes distributions with upcoming order cycles" do + oc_upcoming + expect(ProductsCache).to receive(:refresh_cache).with(distributor, oc_upcoming) + ProductsCache.variant_changed variant + end + + it "refreshes distributions with open order cycles" do + oc_open + expect(ProductsCache).to receive(:refresh_cache).with(distributor, oc_open) + ProductsCache.variant_changed variant + end + + it "does not refresh distributions with undated order cycles" do + oc_undated + expect(ProductsCache).not_to receive(:refresh_cache).with(distributor, oc_undated) + ProductsCache.variant_changed variant + end + + it "does not refresh distributions with closed order cycles" do + oc_closed + expect(ProductsCache).not_to receive(:refresh_cache).with(distributor, oc_closed) + ProductsCache.variant_changed variant + end + + it "limits refresh to outgoing exchanges" do + oc_upcoming + expect(ProductsCache).not_to receive(:refresh_cache).with(coordinator, oc_upcoming) + ProductsCache.variant_changed variant + end + + it "does not refresh distributions where the variant does not appear" do + oc_undated; oc_upcoming; oc_open; oc_closed + variant_undistributed + expect(ProductsCache).not_to receive(:refresh_cache) + ProductsCache.variant_changed variant_undistributed + end + end + + describe "when a variant is destroyed" do + let(:variant) { create(:variant) } + let(:distributor) { create(:distributor_enterprise) } + let!(:oc) { create(:open_order_cycle, distributors: [distributor], variants: [variant]) } + + it "refreshes the cache based on exchanges the variant was in before destruction" do + expect(ProductsCache).to receive(:refresh_cache).with(distributor, oc) + variant.destroy + end + + it "performs the cache refresh after the variant has been destroyed" do + expect(ProductsCache).to receive(:refresh_cache).with(distributor, oc) do + expect(Spree::Variant.where(id: variant.id)).to be_empty + end + + variant.destroy + end + end + + describe "when a product changes" do + let(:product) { create(:simple_product) } + let(:v1) { create(:variant, product: product) } + let(:v2) { create(:variant, product: product) } + let(:d1) { create(:distributor_enterprise) } + let(:d2) { create(:distributor_enterprise) } + let(:oc) { create(:open_order_cycle) } + let!(:ex1) { create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d1, variants: [v1]) } + let!(:ex2) { create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d2, variants: [v1, v2]) } + + before { product.reload } + + it "refreshes the distribution each variant appears in, once each" do + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).once + expect(ProductsCache).to receive(:refresh_cache).with(d2, oc).once + ProductsCache.product_changed product + end + end + + describe "when a variant override changes" do + let(:variant) { create(:variant) } + let(:d1) { create(:distributor_enterprise) } + let(:d2) { create(:distributor_enterprise) } + let!(:vo) { create(:variant_override, variant: variant, hub: d1) } + let!(:oc) { create(:open_order_cycle, distributors: [d1, d2], variants: [variant]) } + + it "refreshes the distributions that the variant override affects" do + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).once + ProductsCache.variant_override_changed vo + end + + it "does not refresh other distributors of the variant" do + expect(ProductsCache).to receive(:refresh_cache).with(d2, oc).never + ProductsCache.variant_override_changed vo + end + end + + + describe "when a variant override is destroyed" do + let(:vo) { double(:variant_override) } + + it "performs the same refresh as a variant override change" do + expect(ProductsCache).to receive(:variant_override_changed).with(vo) + ProductsCache.variant_override_destroyed vo + end + end + + + describe "when a producer property is changed" do + let(:s) { create(:supplier_enterprise) } + let(:pp) { s.producer_properties.last } + let(:product) { create(:simple_product, supplier: s) } + let(:v1) { create(:variant, product: product) } + let(:v2) { create(:variant, product: product) } + let(:v_deleted) { create(:variant, product: product, deleted_at: Time.now) } + let(:d1) { create(:distributor_enterprise) } + let(:d2) { create(:distributor_enterprise) } + let(:d3) { create(:distributor_enterprise) } + let(:oc) { create(:open_order_cycle) } + let!(:ex1) { create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d1, variants: [v1]) } + let!(:ex2) { create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d2, variants: [v1, v2]) } + let!(:ex3) { create(:exchange, order_cycle: oc, sender: oc.coordinator, receiver: d3, variants: [product.master, v_deleted]) } + + before do + s.set_producer_property :organic, 'NASAA 12345' + end + + it "refreshes the distributions the supplied variants appear in" do + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).once + expect(ProductsCache).to receive(:refresh_cache).with(d2, oc).once + ProductsCache.producer_property_changed pp + end + + it "doesn't respond to master or deleted variants" do + expect(ProductsCache).to receive(:refresh_cache).with(d3, oc).never + ProductsCache.producer_property_changed pp + end + end + + + describe "when a producer property is destroyed" do + let(:producer_property) { double(:producer_property) } + + it "triggers the same update as a change to the producer property" do + expect(ProductsCache).to receive(:producer_property_changed).with(producer_property) + ProductsCache.producer_property_destroyed producer_property + end + end + + + describe "when an order cycle is changed" do + let(:variant) { create(:variant) } + let(:s) { create(:supplier_enterprise) } + let(:c) { create(:distributor_enterprise) } + let(:d1) { create(:distributor_enterprise) } + let(:d2) { create(:distributor_enterprise) } + let!(:oc_open) { create(:open_order_cycle, suppliers: [s], coordinator: c, distributors: [d1, d2], variants: [variant]) } + let!(:oc_upcoming) { create(:upcoming_order_cycle, suppliers: [s], coordinator: c, distributors: [d1, d2], variants: [variant]) } + + before do + oc_open.reload + oc_upcoming.reload + end + + it "updates each outgoing distribution in an upcoming order cycle" do + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc_upcoming).once + expect(ProductsCache).to receive(:refresh_cache).with(d2, oc_upcoming).once + ProductsCache.order_cycle_changed oc_upcoming + end + + it "updates each outgoing distribution in an open order cycle" do + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc_open).once + expect(ProductsCache).to receive(:refresh_cache).with(d2, oc_open).once + ProductsCache.order_cycle_changed oc_open + end + + it "does nothing when the order cycle has been made undated" do + expect(ProductsCache).to receive(:refresh_cache).never + oc_open.orders_open_at = oc_open.orders_close_at = nil + oc_open.save! + end + + it "does nothing when the order cycle has been closed" do + expect(ProductsCache).to receive(:refresh_cache).never + oc_open.orders_open_at = 2.weeks.ago + oc_open.orders_close_at = 1.week.ago + oc_open.save! + end + + it "does not update incoming exchanges" do + expect(ProductsCache).to receive(:refresh_cache).with(c, oc_open).never + ProductsCache.order_cycle_changed oc_open + end + end + + + describe "when an exchange is changed" do + let(:s) { create(:supplier_enterprise) } + let(:c) { create(:distributor_enterprise) } + let(:d1) { create(:distributor_enterprise) } + let(:d2) { create(:distributor_enterprise) } + let(:v) { create(:variant) } + let(:oc) { create(:open_order_cycle, coordinator: c) } + + describe "incoming exchanges" do + let!(:ex1) { create(:exchange, order_cycle: oc, sender: s, receiver: c, incoming: true, variants: [v]) } + let!(:ex2) { create(:exchange, order_cycle: oc, sender: c, receiver: d1, incoming: false, variants: [v]) } + let!(:ex3) { create(:exchange, order_cycle: oc, sender: c, receiver: d2, incoming: false, variants: []) } + + before { oc.reload } + + it "updates distributions that include one of the supplier's variants" do + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).once + ProductsCache.exchange_changed ex1 + end + + it "doesn't update distributions that don't include any of the supplier's variants" do + expect(ProductsCache).to receive(:refresh_cache).with(d2, oc).never + ProductsCache.exchange_changed ex1 + end + end + + describe "outgoing exchanges" do + let!(:ex) { create(:exchange, order_cycle: oc, sender: c, receiver: d1, incoming: false) } + + it "does not update for undated order cycles" do + oc.update_attributes! orders_open_at: nil, orders_close_at: nil + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).never + ProductsCache.exchange_changed ex + end + + it "updates for upcoming order cycles" do + oc.update_attributes! orders_open_at: 1.week.from_now, orders_close_at: 2.weeks.from_now + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).once + ProductsCache.exchange_changed ex + end + + it "updates for open order cycles" do + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).once + ProductsCache.exchange_changed ex + end + + it "does not update for closed order cycles" do + oc.update_attributes! orders_open_at: 2.weeks.ago, orders_close_at: 1.week.ago + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).never + ProductsCache.exchange_changed ex + end + end + end + + + describe "when an exchange is destroyed" do + let(:exchange) { double(:exchange) } + + it "triggers the same update as a change to the exchange" do + expect(ProductsCache).to receive(:exchange_changed).with(exchange) + ProductsCache.exchange_destroyed exchange + end + end + + + describe "when an enterprise fee is changed" do + let(:s) { create(:supplier_enterprise) } + let(:c) { create(:distributor_enterprise) } + let(:d1) { create(:distributor_enterprise) } + let(:d2) { create(:distributor_enterprise) } + let(:ef) { create(:enterprise_fee) } + let(:ef_coord) { create(:enterprise_fee, order_cycles: [oc]) } + let(:oc) { create(:open_order_cycle, coordinator: c) } + + + describe "updating exchanges when it's a supplier fee" do + let(:v) { create(:variant) } + let!(:ex1) { create(:exchange, order_cycle: oc, sender: s, receiver: c, incoming: true, variants: [v], enterprise_fees: [ef]) } + let!(:ex2) { create(:exchange, order_cycle: oc, sender: c, receiver: d1, incoming: false, variants: [v]) } + let!(:ex3) { create(:exchange, order_cycle: oc, sender: c, receiver: d2, incoming: false, variants: []) } + + before { ef.reload } + + describe "updating distributions that include one of the supplier's variants" do + it "does not update undated order cycles" do + oc.update_attributes! orders_open_at: nil, orders_close_at: nil + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).never + ProductsCache.enterprise_fee_changed ef + end + + it "updates upcoming order cycles" do + oc.update_attributes! orders_open_at: 1.week.from_now, orders_close_at: 2.weeks.from_now + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).once + ProductsCache.enterprise_fee_changed ef + end + + it "updates open order cycles" do + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).once + ProductsCache.enterprise_fee_changed ef + end + + it "does not update closed order cycles" do + oc.update_attributes! orders_open_at: 2.weeks.ago, orders_close_at: 1.week.ago + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).never + ProductsCache.enterprise_fee_changed ef + end + end + + it "doesn't update distributions that don't include any of the supplier's variants" do + expect(ProductsCache).to receive(:refresh_cache).with(d2, oc).never + ProductsCache.enterprise_fee_changed ef + end + end + + + it "updates order cycles when it's a coordinator fee" do + ef_coord + expect(ProductsCache).to receive(:order_cycle_changed).with(oc).once + ProductsCache.enterprise_fee_changed ef_coord + end + + + describe "updating exchanges when it's a distributor fee" do + let(:ex0) { create(:exchange, order_cycle: oc, sender: s, receiver: c, incoming: true, enterprise_fees: [ef]) } + let(:ex1) { create(:exchange, order_cycle: oc, sender: c, receiver: d1, incoming: false, enterprise_fees: [ef]) } + let(:ex2) { create(:exchange, order_cycle: oc, sender: c, receiver: d2, incoming: false, enterprise_fees: []) } + + describe "updating distributions that include the fee" do + it "does not update undated order cycles" do + oc.update_attributes! orders_open_at: nil, orders_close_at: nil + ex1 + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).never + ProductsCache.enterprise_fee_changed ef + end + + it "updates upcoming order cycles" do + oc.update_attributes! orders_open_at: 1.week.from_now, orders_close_at: 2.weeks.from_now + ex1 + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).once + ProductsCache.enterprise_fee_changed ef + end + + it "updates open order cycles" do + ex1 + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).once + ProductsCache.enterprise_fee_changed ef + end + + it "does not update closed order cycles" do + oc.update_attributes! orders_open_at: 2.weeks.ago, orders_close_at: 1.week.ago + ex1 + expect(ProductsCache).to receive(:refresh_cache).with(d1, oc).never + ProductsCache.enterprise_fee_changed ef + end + end + + it "doesn't update exchanges that don't include the fee" do + ex1; ex2 + expect(ProductsCache).to receive(:refresh_cache).with(d2, oc).never + ProductsCache.enterprise_fee_changed ef + end + + it "doesn't update incoming exchanges" do + ex0 + expect(ProductsCache).to receive(:refresh_cache).with(c, oc).never + ProductsCache.enterprise_fee_changed ef + end + end + end + + describe "when a distributor enterprise is changed" do + let(:d) { create(:distributor_enterprise) } + let(:oc) { create(:open_order_cycle, distributors: [d]) } + + it "updates each distribution the enterprise is active in" do + expect(ProductsCache).to receive(:refresh_cache).with(d, oc) + ProductsCache.distributor_changed d + end + end + + describe "when an inventory item is changed" do + let!(:d) { create(:distributor_enterprise) } + let!(:v) { create(:variant) } + let!(:oc1) { create(:open_order_cycle, distributors: [d], variants: [v]) } + let(:oc2) { create(:open_order_cycle, distributors: [d], variants: []) } + let!(:ii) { create(:inventory_item, enterprise: d, variant: v) } + + it "updates each distribution for that enterprise+variant" do + expect(ProductsCache).to receive(:refresh_cache).with(d, oc1) + ProductsCache.inventory_item_changed ii + end + + it "doesn't update distributions that don't feature the variant" do + oc2 + expect(ProductsCache).to receive(:refresh_cache).with(d, oc2).never + ProductsCache.inventory_item_changed ii + end + end + + describe "refreshing the cache" do + let(:distributor) { double(:distributor) } + let(:order_cycle) { double(:order_cycle) } + + it "notifies ProductsCacheRefreshment" do + expect(ProductsCacheRefreshment).to receive(:refresh).with(distributor, order_cycle) + ProductsCache.send(:refresh_cache, distributor, order_cycle) + end + end + end +end diff --git a/spec/lib/open_food_network/products_renderer_spec.rb b/spec/lib/open_food_network/products_renderer_spec.rb index a231fd3688..1069c9946c 100644 --- a/spec/lib/open_food_network/products_renderer_spec.rb +++ b/spec/lib/open_food_network/products_renderer_spec.rb @@ -3,10 +3,10 @@ require 'open_food_network/products_renderer' module OpenFoodNetwork describe ProductsRenderer do - let(:d) { create(:distributor_enterprise) } - let(:order_cycle) { create(:simple_order_cycle, distributors: [d], coordinator: create(:distributor_enterprise)) } - let(:exchange) { Exchange.find(order_cycle.exchanges.to_enterprises(d).outgoing.first.id) } - let(:pr) { ProductsRenderer.new(d, order_cycle) } + let(:distributor) { create(:distributor_enterprise) } + let(:order_cycle) { create(:simple_order_cycle, distributors: [distributor]) } + let(:exchange) { order_cycle.exchanges.to_enterprises(distributor).outgoing.first } + let(:pr) { ProductsRenderer.new(distributor, order_cycle) } describe "sorting" do let(:t1) { create(:taxon) } @@ -24,14 +24,14 @@ module OpenFoodNetwork end it "sorts products by the distributor's preferred taxon list" do - d.stub(:preferred_shopfront_taxon_order) {"#{t1.id},#{t2.id}"} - products = pr.send(:products_for_shop) + distributor.stub(:preferred_shopfront_taxon_order) {"#{t1.id},#{t2.id}"} + products = pr.send(:load_products) products.should == [p2, p4, p1, p3] end it "alphabetizes products by name when taxon list is not set" do - d.stub(:preferred_shopfront_taxon_order) {""} - products = pr.send(:products_for_shop) + distributor.stub(:preferred_shopfront_taxon_order) {""} + products = pr.send(:load_products) products.should == [p1, p2, p3, p4] end end @@ -45,17 +45,17 @@ module OpenFoodNetwork end it "only returns products for the current order cycle" do - pr.products.should include product.name + pr.products_json.should include product.name end it "doesn't return products not in stock" do variant.update_attribute(:count_on_hand, 0) - pr.products.should_not include product.name + pr.products_json.should_not include product.name end it "strips html from description" do product.update_attribute(:description, "turtles frogs") - json = pr.products + json = pr.products_json json.should include "frogs" json.should_not include " ["48x48>", :png]} end end + + describe "callbacks" do + let!(:product) { create(:simple_product) } + + let!(:image_file) { File.open("#{Rails.root}/app/assets/images/logo-white.png") } + let!(:image) { Image.create(viewable_id: product.master.id, viewable_type: 'Spree::Variant', alt: "image", attachment: image_file) } + + it "refreshes the products cache when changed" do + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + image.alt = 'asdf' + image.save + end + + it "refreshes the products cache when destroyed" do + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + image.destroy + end + end end end diff --git a/spec/models/spree/option_type_spec.rb b/spec/models/spree/option_type_spec.rb new file mode 100644 index 0000000000..cfc0654733 --- /dev/null +++ b/spec/models/spree/option_type_spec.rb @@ -0,0 +1,28 @@ +require 'spec_helper' + +module Spree + describe OptionType do + describe "products cache" do + let!(:product) { create(:simple_product, option_types: [option_type]) } + let(:variant) { product.variants.first } + let(:option_type) { create(:option_type) } + let(:option_value) { create(:option_value, option_type: option_type) } + + before do + option_type.reload + variant.option_values << option_value + end + + it "refreshes the products cache on change, via product" do + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + option_type.name = 'foo' + option_type.save! + end + + it "refreshes the products cache on destruction, via option value destruction" do + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_changed).with(variant) + option_type.destroy + end + end + end +end diff --git a/spec/models/spree/option_value_spec.rb b/spec/models/spree/option_value_spec.rb new file mode 100644 index 0000000000..f784e08af2 --- /dev/null +++ b/spec/models/spree/option_value_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +module Spree + describe OptionValue do + describe "products cache" do + let(:variant) { create(:variant) } + let(:option_value) { create(:option_value) } + + before do + variant.option_values << option_value + option_value.reload + end + + it "refreshes the products cache on change, via variant" do + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_changed).with(variant) + option_value.name = 'foo' + option_value.save! + end + + it "refreshes the products cache on destruction, via variant" do + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_changed).with(variant) + option_value.destroy + end + end + end +end diff --git a/spec/models/spree/preference_spec.rb b/spec/models/spree/preference_spec.rb new file mode 100644 index 0000000000..ff71c9c9c6 --- /dev/null +++ b/spec/models/spree/preference_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +module Spree + describe Preference do + describe "refreshing the products cache" do + it "reports when product_selection_from_inventory_only has changed" do + p = Preference.new(key: 'enterprise/product_selection_from_inventory_only/123') + expect(p.send(:product_selection_from_inventory_only_changed?)).to be_true + end + + it "reports when product_selection_from_inventory_only has not changed" do + p = Preference.new(key: 'enterprise/shopfront_message/123') + expect(p.send(:product_selection_from_inventory_only_changed?)).to be_false + end + + it "looks up the referenced enterprise" do + e = create(:distributor_enterprise) + p = Preference.new(key: "enterprise/product_selection_from_inventory_only/#{e.id}") + expect(p.send(:enterprise)).to eql e + end + end + end +end diff --git a/spec/models/spree/price_spec.rb b/spec/models/spree/price_spec.rb new file mode 100644 index 0000000000..5d48afd95b --- /dev/null +++ b/spec/models/spree/price_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +module Spree + describe Price do + describe "callbacks" do + let(:variant) { create(:variant) } + let(:price) { variant.default_price } + + it "refreshes the products cache on change" do + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_changed).with(variant) + price.amount = 123 + price.save + end + + # Do not refresh on price destruction - this (only?) happens when variant is destroyed, + # and in that case the variant will take responsibility for refreshing the cache + + it "does not refresh the cache when variant is not set" do + # Creates a price without the back link to variant + create(:product, master: create(:variant)) + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_changed).never + end + end + end +end diff --git a/spec/models/spree/product_property_spec.rb b/spec/models/spree/product_property_spec.rb new file mode 100644 index 0000000000..1c9799ab29 --- /dev/null +++ b/spec/models/spree/product_property_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +module Spree + describe ProductProperty do + describe "callbacks" do + let(:product) { product_property.product } + let(:product_property) { create(:product_property) } + + it "refreshes the products cache on save, via Product" do + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + product_property.value = 123 + product_property.save + end + + it "refreshes the products cache on destroy" do + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + product_property.destroy + end + end + end +end diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index c37b49bc23..9c2ee6b664 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -161,6 +161,19 @@ module Spree end end + describe "callbacks" do + let(:product) { create(:simple_product) } + + it "refreshes the products cache on save" do + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + product.name = 'asdf' + product.save + end + + # On destroy, all distributed variants are refreshed by a Variant around_destroy + # callback, so we don't need to do anything on the product model. + end + describe "scopes" do describe "in_supplier" do it "shows products in supplier" do diff --git a/spec/models/spree/property_spec.rb b/spec/models/spree/property_spec.rb new file mode 100644 index 0000000000..a86513b5a9 --- /dev/null +++ b/spec/models/spree/property_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +module Spree + describe Property do + describe "callbacks" do + let(:property) { product_property.property } + let(:product) { product_property.product } + let(:product_property) { create(:product_property) } + + it "refreshes the products cache on save" do + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + property.name = 'asdf' + property.save + end + end + end +end diff --git a/spec/models/spree/shipping_method_spec.rb b/spec/models/spree/shipping_method_spec.rb index 0e738470dd..33c44b8b6d 100644 --- a/spec/models/spree/shipping_method_spec.rb +++ b/spec/models/spree/shipping_method_spec.rb @@ -37,21 +37,28 @@ module Spree describe "availability" do - let(:sm) { build(:shipping_method) } + let(:sm) { create(:shipping_method) } + let(:currency) { 'AUD' } + + before do + sm.calculator.preferred_currency = currency + end it "is available to orders that match its distributor" do - o = build(:order, ship_address: build(:address), distributor: sm.distributors.first) + o = create(:order, ship_address: create(:address), + distributor: sm.distributors.first, currency: currency) sm.should be_available_to_order o end it "is not available to orders that do not match its distributor" do - o = build(:order, ship_address: build(:address), - distributor: build(:distributor_enterprise)) + o = create(:order, ship_address: create(:address), + distributor: create(:distributor_enterprise), currency: currency) sm.should_not be_available_to_order o end it "is available to orders with no shipping address" do - o = build(:order, ship_address: nil, distributor: sm.distributors.first) + o = create(:order, ship_address: nil, + distributor: sm.distributors.first, currency: currency) sm.should be_available_to_order o end end diff --git a/spec/models/spree/taxon_spec.rb b/spec/models/spree/taxon_spec.rb index a0d729c054..926e5b3c5f 100644 --- a/spec/models/spree/taxon_spec.rb +++ b/spec/models/spree/taxon_spec.rb @@ -7,6 +7,21 @@ module Spree let(:t1) { create(:taxon) } let(:t2) { create(:taxon) } + describe "callbacks" do + let(:product) { create(:simple_product, taxons: [t1]) } + + it "refreshes the products cache on save" do + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + t1.name = 'asdf' + t1.save + end + + it "refreshes the products cache on destroy" do + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + t1.destroy + end + end + describe "finding all supplied taxons" do let!(:p1) { create(:simple_product, supplier: e, taxons: [t1, t2]) } diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 82eed6ba63..a6c24bd3d3 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -1,5 +1,6 @@ require 'spec_helper' require 'open_food_network/option_value_namer' +require 'open_food_network/products_cache' module Spree describe Variant do @@ -163,6 +164,40 @@ module Spree end end + describe "callbacks" do + let(:variant) { create(:variant) } + + it "refreshes the products cache on save" do + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_changed).with(variant) + variant.sku = 'abc123' + variant.save + end + + it "refreshes the products cache on destroy" do + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).with(variant) + variant.destroy + end + + context "when it is the master variant" do + let(:product) { create(:simple_product) } + let(:master) { product.master } + + it "refreshes the products cache for the entire product on save" do + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_changed).never + master.sku = 'abc123' + master.save + end + + it "refreshes the products cache for the entire product on destroy" do + # Does this ever happen? + expect(OpenFoodNetwork::ProductsCache).to receive(:product_changed).with(product) + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).never + master.destroy + end + end + end + describe "indexing variants by id" do let!(:v1) { create(:variant) } let!(:v2) { create(:variant) } diff --git a/spec/models/variant_override_spec.rb b/spec/models/variant_override_spec.rb index 9efc53b521..a24a921f43 100644 --- a/spec/models/variant_override_spec.rb +++ b/spec/models/variant_override_spec.rb @@ -30,6 +30,22 @@ describe VariantOverride do end + describe "callbacks" do + let!(:vo) { create(:variant_override, hub: hub, variant: variant) } + + it "refreshes the products cache on save" do + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_override_changed).with(vo) + vo.price = 123.45 + vo.save + end + + it "refreshes the products cache on destroy" do + expect(OpenFoodNetwork::ProductsCache).to receive(:variant_override_destroyed).with(vo) + vo.destroy + end + end + + describe "looking up prices" do it "returns the numeric price when present" do VariantOverride.create!(variant: variant, hub: hub, price: 12.34) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 80d7c2eeb7..a477ad31e1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -4,6 +4,7 @@ require 'rubygems' require 'pry' unless ENV['CI'] require 'knapsack' +Knapsack.tracker.config({enable_time_offset_warning: false}) unless ENV['CI'] Knapsack::Adapters::RSpecAdapter.bind ENV["RAILS_ENV"] ||= 'test'