diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 643b641081..0e68fdf05e 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -139,7 +139,6 @@ Metrics/LineLength: - lib/open_food_network/payments_report.rb - lib/open_food_network/permalink_generator.rb - lib/open_food_network/products_cache.rb - - lib/open_food_network/products_renderer.rb - lib/open_food_network/reports/bulk_coop_allocation_report.rb - lib/open_food_network/reports/line_items.rb - lib/open_food_network/sales_tax_report.rb @@ -249,13 +248,10 @@ Metrics/LineLength: - spec/helpers/order_cycles_helper_spec.rb - spec/helpers/spree/admin/base_helper_spec.rb - spec/jobs/confirm_order_job_spec.rb - - spec/jobs/products_cache_integrity_checker_job_spec.rb - - spec/jobs/refresh_products_cache_job_spec.rb - spec/jobs/subscription_confirm_job_spec.rb - spec/jobs/subscription_placement_job_spec.rb - spec/lib/open_food_network/address_finder_spec.rb - spec/lib/open_food_network/bulk_coop_report_spec.rb - - spec/lib/open_food_network/cached_products_renderer_spec.rb - spec/lib/open_food_network/customers_report_spec.rb - spec/lib/open_food_network/enterprise_fee_applicator_spec.rb - spec/lib/open_food_network/enterprise_fee_calculator_spec.rb @@ -272,7 +268,6 @@ Metrics/LineLength: - spec/lib/open_food_network/permissions_spec.rb - spec/lib/open_food_network/products_and_inventory_report_spec.rb - spec/lib/open_food_network/products_cache_spec.rb - - spec/lib/open_food_network/products_renderer_spec.rb - spec/lib/open_food_network/proxy_order_syncer_spec.rb - spec/lib/open_food_network/scope_variant_to_hub_spec.rb - spec/lib/open_food_network/subscription_payment_updater_spec.rb @@ -615,7 +610,6 @@ Metrics/MethodLength: - lib/open_food_network/payments_report.rb - lib/open_food_network/permissions.rb - lib/open_food_network/products_and_inventory_report.rb - - lib/open_food_network/products_renderer.rb - lib/open_food_network/rack_request_blocker.rb - lib/open_food_network/reports/bulk_coop_allocation_report.rb - lib/open_food_network/reports/bulk_coop_supplier_report.rb @@ -670,7 +664,6 @@ Metrics/ModuleLength: - spec/controllers/api/orders_controller_spec.rb - spec/controllers/spree/api/products_controller_spec.rb - spec/lib/open_food_network/address_finder_spec.rb - - spec/lib/open_food_network/cached_products_renderer_spec.rb - spec/lib/open_food_network/customers_report_spec.rb - spec/lib/open_food_network/enterprise_fee_calculator_spec.rb - spec/lib/open_food_network/option_value_namer_spec.rb diff --git a/app/assets/javascripts/darkswarm/controllers/order_cycle_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/order_cycle_controller.js.coffee index d900fd0a27..6547237c2d 100644 --- a/app/assets/javascripts/darkswarm/controllers/order_cycle_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/order_cycle_controller.js.coffee @@ -1,4 +1,4 @@ -Darkswarm.controller "OrderCycleCtrl", ($scope, $timeout, OrderCycle) -> +Darkswarm.controller "OrderCycleCtrl", ($scope, $rootScope, $timeout, OrderCycle) -> $scope.order_cycle = OrderCycle.order_cycle $scope.OrderCycle = OrderCycle @@ -6,11 +6,12 @@ Darkswarm.controller "OrderCycleCtrl", ($scope, $timeout, OrderCycle) -> # This is a hack. We should probably write our own "popover" directive # That takes an expression instead of a trigger, and binds to that $timeout => + $rootScope.$broadcast 'orderCycleSelected' if !$scope.OrderCycle.selected() $("#order_cycle_id").trigger("openTrigger") -Darkswarm.controller "OrderCycleChangeCtrl", ($scope, $timeout, OrderCycle, Products, Variants, Cart, ChangeableOrdersAlert) -> +Darkswarm.controller "OrderCycleChangeCtrl", ($scope, $rootScope, $timeout, OrderCycle, Products, Variants, Cart, ChangeableOrdersAlert) -> # Track previous order cycle id for use with revertOrderCycle() $scope.previous_order_cycle_id = OrderCycle.order_cycle.order_cycle_id $scope.$watch 'order_cycle.order_cycle_id', (newValue, oldValue)-> @@ -32,3 +33,4 @@ Darkswarm.controller "OrderCycleChangeCtrl", ($scope, $timeout, OrderCycle, Prod Products.update() Cart.reloadFinalisedLineItems() ChangeableOrdersAlert.reload() + $rootScope.$broadcast 'orderCycleSelected' diff --git a/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee index 3c32c83875..684109f7e4 100644 --- a/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee @@ -1,41 +1,65 @@ -Darkswarm.controller "ProductsCtrl", ($scope, $filter, $rootScope, Products, OrderCycle, FilterSelectorsService, Cart, Taxons, Properties) -> +Darkswarm.controller "ProductsCtrl", ($scope, $filter, $rootScope, Products, OrderCycle, OrderCycleResource, FilterSelectorsService, Cart, Dereferencer, Taxons, Properties, currentHub, $timeout) -> $scope.Products = Products $scope.Cart = Cart $scope.query = "" $scope.taxonSelectors = FilterSelectorsService.createSelectors() $scope.propertySelectors = FilterSelectorsService.createSelectors() $scope.filtersActive = true - $scope.limit = 10 + $scope.page = 1 + $scope.per_page = 10 $scope.order_cycle = OrderCycle.order_cycle - # $scope.infiniteDisabled = true + $scope.supplied_taxons = null + $scope.supplied_properties = null - # All of this logic basically just replicates the functionality filtering an ng-repeat - # except that it allows us to filter a separate list before rendering, meaning that - # we can get much better performance when applying filters by resetting the limit on the - # number of products being rendered each time a filter is changed. + $rootScope.$on "orderCycleSelected", -> + $scope.update_filters() + $scope.clearAll() - $scope.$watch "Products.loading", (newValue, oldValue) -> - $scope.updateFilteredProducts() - $scope.$broadcast("loadFilterSelectors") if !newValue + $scope.update_filters = -> + order_cycle_id = OrderCycle.order_cycle.order_cycle_id - $scope.incrementLimit = -> - if $scope.limit < Products.products.length - $scope.limit += 10 - $scope.updateVisibleProducts() + return unless order_cycle_id - $scope.$watch 'query', -> $scope.updateFilteredProducts() - $scope.$watchCollection 'activeTaxons', -> $scope.updateFilteredProducts() - $scope.$watchCollection 'activeProperties', -> $scope.updateFilteredProducts() + params = { + id: order_cycle_id, + distributor: currentHub.id + } + OrderCycleResource.taxons params, (data)=> + $scope.supplied_taxons = {} + data.map( (taxon) -> + $scope.supplied_taxons[taxon.id] = Taxons.taxons_by_id[taxon.id] + ) + OrderCycleResource.properties params, (data)=> + $scope.supplied_properties = {} + data.map( (property) -> + $scope.supplied_properties[property.id] = Properties.properties_by_id[property.id] + ) - $scope.updateFilteredProducts = -> - $scope.limit = 10 - f1 = $filter('products')(Products.products, $scope.query) - f2 = $filter('taxons')(f1, $scope.activeTaxons) - $scope.filteredProducts = $filter('properties')(f2, $scope.activeProperties) - $scope.updateVisibleProducts() + $scope.loadMore = -> + if ($scope.page * $scope.per_page) <= Products.products.length + $scope.loadMoreProducts() - $scope.updateVisibleProducts = -> - $scope.visibleProducts = $filter('limitTo')($scope.filteredProducts, $scope.limit) + $scope.$watch 'query', (newValue, oldValue) -> $scope.loadProducts() if newValue != oldValue + $scope.$watchCollection 'activeTaxons', (newValue, oldValue) -> $scope.loadProducts() if newValue != oldValue + $scope.$watchCollection 'activeProperties', (newValue, oldValue) -> $scope.loadProducts() if newValue != oldValue + + $scope.loadProducts = -> + $scope.page = 1 + Products.update($scope.queryParams()) + + $scope.loadMoreProducts = -> + Products.update($scope.queryParams($scope.page + 1), true) + $scope.page += 1 + + $scope.queryParams = (page = null) -> + { + id: $scope.order_cycle.order_cycle_id, + page: page || $scope.page, + per_page: $scope.per_page, + 'q[name_or_meta_keywords_or_supplier_name_cont]': $scope.query, + 'q[properties_id_or_supplier_properties_id_in_any][]': $scope.activeProperties, + 'q[primary_taxon_id_in_any][]': $scope.activeTaxons + } $scope.searchKeypress = (e)-> code = e.keyCode || e.which diff --git a/app/assets/javascripts/darkswarm/services/order_cycle_resource.js.coffee b/app/assets/javascripts/darkswarm/services/order_cycle_resource.js.coffee new file mode 100644 index 0000000000..32d70d54a5 --- /dev/null +++ b/app/assets/javascripts/darkswarm/services/order_cycle_resource.js.coffee @@ -0,0 +1,21 @@ +Darkswarm.factory 'OrderCycleResource', ($resource) -> + $resource('/api/order_cycles/:id', {}, { + 'products': + method: 'GET' + isArray: true + url: '/api/order_cycles/:id/products' + params: + id: '@id' + 'taxons': + method: 'GET' + isArray: true + url: '/api/order_cycles/:id/taxons' + params: + id: '@id' + 'properties': + method: 'GET' + isArray: true + url: '/api/order_cycles/:id/properties' + params: + id: '@id' + }) diff --git a/app/assets/javascripts/darkswarm/services/products.js.coffee b/app/assets/javascripts/darkswarm/services/products.js.coffee index 0a24cbd374..59bd19efb4 100644 --- a/app/assets/javascripts/darkswarm/services/products.js.coffee +++ b/app/assets/javascripts/darkswarm/services/products.js.coffee @@ -1,26 +1,34 @@ -Darkswarm.factory 'Products', ($resource, Shopfront, Dereferencer, Taxons, Properties, Cart, Variants) -> +Darkswarm.factory 'Products', (OrderCycleResource, OrderCycle, Shopfront, currentHub, Dereferencer, Taxons, Properties, Cart, Variants) -> new class Products constructor: -> @update() - # TODO: don't need to scope this into object - # Already on object as far as controller scope is concerned - products: null + products: [] + fetched_products: [] loading: true - update: => + update: (params = {}, load_more = false) => @loading = true - @products = [] - $resource("/shop/products").query (products)=> - @products = products + order_cycle_id = OrderCycle.order_cycle.order_cycle_id + if order_cycle_id == undefined + @loading = false + return + + params['id'] = order_cycle_id + params['distributor'] = currentHub.id + + OrderCycleResource.products params, (data)=> + @products = [] unless load_more + @fetched_products = data @extend() @dereference() @registerVariants() + @products = @products.concat(@fetched_products) @loading = false extend: -> - for product in @products + for product in @fetched_products if product.variants?.length > 0 prices = (v.price for v in product.variants) product.price = Math.min.apply(null, prices) @@ -30,7 +38,7 @@ Darkswarm.factory 'Products', ($resource, Shopfront, Dereferencer, Taxons, Prope product.largeImage = product.images[0]?.large_url if product.images dereference: -> - for product in @products + for product in @fetched_products product.supplier = Shopfront.producers_by_id[product.supplier.id] Dereferencer.dereference product.taxons, Taxons.taxons_by_id @@ -40,7 +48,7 @@ Darkswarm.factory 'Products', ($resource, Shopfront, Dereferencer, Taxons, Prope # May return different objects! If the variant has already been registered # by another service, we fetch those registerVariants: -> - for product in @products + for product in @fetched_products if product.variants product.variant_names = "" product.variants = for variant in product.variants diff --git a/app/assets/javascripts/templates/shop_variant.html.haml b/app/assets/javascripts/templates/shop_variant.html.haml index 3214612e88..71ca3c33ec 100644 --- a/app/assets/javascripts/templates/shop_variant.html.haml +++ b/app/assets/javascripts/templates/shop_variant.html.haml @@ -17,7 +17,7 @@ .small-4.medium-2.large-2.columns.variant-price .table-cell.price %i.ofn-i_009-close - {{ ::variant.price_with_fees | localizeCurrency }} + {{ variant.price_with_fees | localizeCurrency }} -# Now in a template in app/assets/javascripts/templates ! %price-breakdown{"price-breakdown" => "_", variant: "variant", diff --git a/app/controllers/admin/cache_settings_controller.rb b/app/controllers/admin/cache_settings_controller.rb deleted file mode 100644 index e922031f68..0000000000 --- a/app/controllers/admin/cache_settings_controller.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'open_food_network/products_cache_integrity_checker' - -module Admin - class CacheSettingsController < Spree::Admin::BaseController - def edit - @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 - - def update - Spree::Config.set(params[:preferences]) - - respond_to do |format| - format.html { redirect_to main_app.edit_admin_cache_settings_path } - end - end - end -end diff --git a/app/controllers/api/order_cycles_controller.rb b/app/controllers/api/order_cycles_controller.rb new file mode 100644 index 0000000000..e0088d43f4 --- /dev/null +++ b/app/controllers/api/order_cycles_controller.rb @@ -0,0 +1,88 @@ +module Api + class OrderCyclesController < BaseController + include EnterprisesHelper + respond_to :json + + skip_authorization_check + + def products + products = ProductsRenderer.new( + distributor, + order_cycle, + customer, + search_params + ).products_json + + render json: products + rescue ProductsRenderer::NoProducts + render status: :not_found, json: '' + end + + def taxons + taxons = Spree::Taxon. + joins(:products). + where(spree_products: { id: distributed_products }). + select('DISTINCT spree_taxons.*') + + render json: ActiveModel::ArraySerializer.new(taxons, each_serializer: Api::TaxonSerializer) + end + + def properties + render json: ActiveModel::ArraySerializer.new( + product_properties | producer_properties, each_serializer: Api::PropertySerializer + ) + end + + private + + def product_properties + Spree::Property. + joins(:products). + where(spree_products: { id: distributed_products }). + select('DISTINCT spree_properties.*') + end + + def producer_properties + producers = Enterprise. + joins(:supplied_products). + where(spree_products: { id: distributed_products }) + + Spree::Property. + joins(:producer_properties). + where(producer_properties: { producer_id: producers }). + select('DISTINCT spree_properties.*') + end + + def search_params + permitted_search_params = params.slice :q, :page, :per_page + + if permitted_search_params.key? :q + permitted_search_params[:q].slice!(*permitted_ransack_params) + end + + permitted_search_params + end + + def permitted_ransack_params + [:name_or_meta_keywords_or_supplier_name_cont, + :properties_id_or_supplier_properties_id_in_any, + :primary_taxon_id_in_any] + end + + def distributor + Enterprise.find_by_id(params[:distributor]) + end + + def order_cycle + OrderCycle.find_by_id(params[:id]) + end + + def customer + @current_api_user.andand.customer_of(distributor) || nil + end + + def distributed_products + OrderCycleDistributedProducts.new(distributor, order_cycle, customer).products_relation + end + end +end diff --git a/app/controllers/shop_controller.rb b/app/controllers/shop_controller.rb index 1d9d727aa2..ee304d1f96 100644 --- a/app/controllers/shop_controller.rb +++ b/app/controllers/shop_controller.rb @@ -1,5 +1,3 @@ -require 'open_food_network/cached_products_renderer' - class ShopController < BaseController layout "darkswarm" before_filter :require_distributor_chosen, :set_order_cycles, except: :changeable_orders_alert @@ -9,19 +7,6 @@ class ShopController < BaseController redirect_to main_app.enterprise_shop_path(current_distributor) end - def products - renderer = OpenFoodNetwork::CachedProductsRenderer.new(current_distributor, - current_order_cycle) - - # If we add any more filtering logic, we should probably - # move it all to a lib class like 'CachedProductsFilterer' - products_json = filter(renderer.products_json) - - render json: products_json - rescue OpenFoodNetwork::CachedProductsRenderer::NoProducts - render status: :not_found, json: '' - end - def order_cycle if request.post? if oc = OrderCycle.with_distributor(@distributor).active.find_by_id(params[:order_cycle_id]) @@ -39,27 +24,4 @@ class ShopController < BaseController def changeable_orders_alert render layout: false end - - private - - def filtered_json(products_json) - if applicator.rules.any? - filter(products_json) - else - products_json - end - end - - def filter(products_json) - products_hash = JSON.parse(products_json) - applicator.filter!(products_hash) - JSON.unparse(products_hash) - end - - def applicator - return @applicator unless @applicator.nil? - @applicator = OpenFoodNetwork::TagRuleApplicator.new(current_distributor, - "FilterProducts", - current_customer.andand.tag_list) - end end diff --git a/app/jobs/products_cache_integrity_checker_job.rb b/app/jobs/products_cache_integrity_checker_job.rb deleted file mode 100644 index 1adf229c7c..0000000000 --- a/app/jobs/products_cache_integrity_checker_job.rb +++ /dev/null @@ -1,30 +0,0 @@ -require 'open_food_network/products_cache_integrity_checker' - -ProductsCacheIntegrityCheckerJob = Struct.new(:distributor_id, :order_cycle_id) do - def perform - unless checker.ok? - exception = RuntimeError.new( - "Products JSON differs from cached version for distributor: #{distributor_id}, " \ - "order cycle: #{order_cycle_id}" - ) - - Bugsnag.notify(exception) do |report| - report.add_tab(:products_cache, diff: checker.diff.to_s(:text)) - end - 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 deleted file mode 100644 index b08148b280..0000000000 --- a/app/jobs/refresh_products_cache_job.rb +++ /dev/null @@ -1,21 +0,0 @@ -require 'open_food_network/products_renderer' - -RefreshProductsCacheJob = Struct.new(:distributor_id, :order_cycle_id) do - def perform - Rails.cache.write(key, products_json) - rescue ActiveRecord::RecordNotFound - true - end - - private - - def key - "products-json-#{distributor_id}-#{order_cycle_id}" - end - - 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 index 99aefc2595..f15a194158 100644 --- a/app/models/coordinator_fee.rb +++ b/app/models/coordinator_fee.rb @@ -1,13 +1,4 @@ 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 aaeb5101d4..64c362b0fe 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -10,10 +10,6 @@ class EnterpriseFee < ActiveRecord::Base has_many :exchange_fees, dependent: :destroy has_many :exchanges, through: :exchange_fees - after_save :refresh_products_cache - # After destroy, the products cache is refreshed via the after_destroy hook for - # coordinator_fees and exchange_fees - attr_accessible :enterprise_id, :fee_type, :name, :tax_category_id, :calculator_type, :inherits_tax_category FEE_TYPES = %w(packing transport admin sales fundraising).freeze @@ -59,8 +55,4 @@ class EnterpriseFee < ActiveRecord::Base end 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 78e9f4aba7..ce28a7255d 100644 --- a/app/models/exchange.rb +++ b/app/models/exchange.rb @@ -23,9 +23,6 @@ class Exchange < ActiveRecord::Base validates :order_cycle, :sender, :receiver, presence: true validates :sender_id, uniqueness: { 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) } @@ -98,12 +95,4 @@ class Exchange < ActiveRecord::Base def participant incoming? ? sender : receiver 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 27636f2f28..ff9f12e8dd 100644 --- a/app/models/exchange_fee.rb +++ b/app/models/exchange_fee.rb @@ -1,13 +1,4 @@ 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 e7aa9dccc0..5e4bb4ef8b 100644 --- a/app/models/inventory_item.rb +++ b/app/models/inventory_item.rb @@ -1,5 +1,3 @@ -require 'open_food_network/products_cache' - class InventoryItem < ActiveRecord::Base attr_accessible :enterprise, :enterprise_id, :variant, :variant_id, :visible @@ -13,12 +11,4 @@ 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 4a37a139f9..ce931e5ea2 100644 --- a/app/models/order_cycle.rb +++ b/app/models/order_cycle.rb @@ -23,8 +23,6 @@ class OrderCycle < ActiveRecord::Base validates :name, :coordinator_id, presence: true validate :orders_close_at_after_orders_open_at? - after_save :refresh_products_cache - preference :product_selection_from_coordinator_inventory_only, :boolean, default: false scope :active, lambda { @@ -247,10 +245,6 @@ class OrderCycle < ActiveRecord::Base coordinator.users.include? user end - def refresh_products_cache - OpenFoodNetwork::ProductsCache.order_cycle_changed self - end - def items_bought_by_user(user, distributor) # The Spree::Order.complete scope only checks for completed_at date # it does not ensure state is "complete" diff --git a/app/models/producer_property.rb b/app/models/producer_property.rb index 5f299b344f..6f870b1ba9 100644 --- a/app/models/producer_property.rb +++ b/app/models/producer_property.rb @@ -4,9 +4,6 @@ class ProducerProperty < ActiveRecord::Base default_scope order("#{table_name}.position") - after_save :refresh_products_cache - after_destroy :refresh_products_cache_from_destroy - scope :ever_sold_by, ->(shop) { joins(producer: { supplied_products: { variants: { exchanges: :order_cycle } } }). merge(Exchange.outgoing). @@ -29,14 +26,4 @@ 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 8cb9f7cc66..fec270bc14 100644 --- a/app/models/spree/classification_decorator.rb +++ b/app/models/spree/classification_decorator.rb @@ -2,16 +2,9 @@ Spree::Classification.class_eval do belongs_to :product, class_name: "Spree::Product", touch: true before_destroy :dont_destroy_if_primary_taxon - after_destroy :refresh_products_cache - after_save :refresh_products_cache private - def refresh_products_cache - product = Spree::Product.with_deleted.find(product_id) if product.blank? - product.refresh_products_cache - end - def dont_destroy_if_primary_taxon if product.primary_taxon == taxon errors.add :base, I18n.t(:spree_classification_primary_taxon_error, taxon: taxon.name, product: product.name) diff --git a/app/models/spree/image_decorator.rb b/app/models/spree/image_decorator.rb index 90f62854db..7d86aae3a6 100644 --- a/app/models/spree/image_decorator.rb +++ b/app/models/spree/image_decorator.rb @@ -1,7 +1,4 @@ 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. @@ -23,10 +20,4 @@ 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 index 2ebdece88c..1ef46caa4e 100644 --- a/app/models/spree/option_type_decorator.rb +++ b/app/models/spree/option_type_decorator.rb @@ -1,12 +1,5 @@ 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 deleted file mode 100644 index 48d88ff26e..0000000000 --- a/app/models/spree/option_value_decorator.rb +++ /dev/null @@ -1,18 +0,0 @@ -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 deleted file mode 100644 index e5ad7cbda2..0000000000 --- a/app/models/spree/preference_decorator.rb +++ /dev/null @@ -1,30 +0,0 @@ -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.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 index 036daae4de..6c67ca5949 100644 --- a/app/models/spree/price_decorator.rb +++ b/app/models/spree/price_decorator.rb @@ -2,8 +2,6 @@ module Spree Price.class_eval do acts_as_paranoid without_default_scope: true - after_save :refresh_products_cache - # Allow prices to access associated soft-deleted variants. def variant Spree::Variant.unscoped { super } @@ -16,9 +14,5 @@ module Spree self.currency = Spree::Config[:currency] end end - - 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 ac7b8791a9..d6f6e31108 100644 --- a/app/models/spree/product_decorator.rb +++ b/app/models/spree/product_decorator.rb @@ -38,7 +38,6 @@ Spree::Product.class_eval do after_save :remove_previous_primary_taxon_from_taxons after_save :ensure_standard_variant after_save :update_units - after_save :refresh_products_cache # -- Joins scope :with_order_cycles_outer, -> { @@ -192,23 +191,17 @@ Spree::Product.class_eval do def destroy_with_delete_from_order_cycles transaction do - OpenFoodNetwork::ProductsCache.product_deleted(self) do - touch_distributors + touch_distributors - ExchangeVariant. - where('exchange_variants.variant_id IN (?)', variants_including_master.with_deleted. - select(:id)).destroy_all + ExchangeVariant. + where('exchange_variants.variant_id IN (?)', variants_including_master.with_deleted. + select(:id)).destroy_all - destroy_without_delete_from_order_cycles - end + destroy_without_delete_from_order_cycles end end alias_method_chain :destroy, :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 index 34432fb310..58ee8fb1ad 100644 --- a/app/models/spree/product_property_decorator.rb +++ b/app/models/spree/product_property_decorator.rb @@ -1,10 +1,5 @@ module Spree ProductProperty.class_eval do belongs_to :product, class_name: "Spree::Product", touch: true - - after_save :refresh_products_cache - after_destroy :refresh_products_cache - - delegate :refresh_products_cache, to: :product end end diff --git a/app/models/spree/property_decorator.rb b/app/models/spree/property_decorator.rb index 20088ab243..aeeb607af9 100644 --- a/app/models/spree/property_decorator.rb +++ b/app/models/spree/property_decorator.rb @@ -20,19 +20,8 @@ module Spree merge(OrderCycle.active) } - 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 - def property self end - - private - - def refresh_products_cache - product_properties(:reload).each(&:refresh_products_cache) - end end end diff --git a/app/models/spree/stock_movement_decorator.rb b/app/models/spree/stock_movement_decorator.rb deleted file mode 100644 index 40aa049943..0000000000 --- a/app/models/spree/stock_movement_decorator.rb +++ /dev/null @@ -1,10 +0,0 @@ -Spree::StockMovement.class_eval do - after_save :refresh_products_cache - - private - - def refresh_products_cache - return if stock_item.variant.blank? - OpenFoodNetwork::ProductsCache.variant_changed stock_item.variant - end -end diff --git a/app/models/spree/taxon_decorator.rb b/app/models/spree/taxon_decorator.rb index d5790b2c1d..ad2a85cfdb 100644 --- a/app/models/spree/taxon_decorator.rb +++ b/app/models/spree/taxon_decorator.rb @@ -4,8 +4,6 @@ Spree::Taxon.class_eval do attachment_definitions[:icon][:path] = 'public/images/spree/taxons/:id/:style/:basename.:extension' 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 fs = [] @@ -49,10 +47,4 @@ Spree::Taxon.class_eval do ts[t.enterprise_id.to_i] << t.id end 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 83c89122e6..4c9d27d9b9 100644 --- a/app/models/spree/variant_decorator.rb +++ b/app/models/spree/variant_decorator.rb @@ -1,7 +1,6 @@ require 'open_food_network/enterprise_fee_calculator' require 'open_food_network/variant_and_line_item_naming' require 'concerns/variant_stock' -require 'open_food_network/products_cache' Spree::Variant.class_eval do extend Spree::LocalizedNumber @@ -30,7 +29,6 @@ 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) } @@ -108,14 +106,6 @@ Spree::Variant.class_eval do OpenFoodNetwork::EnterpriseFeeCalculator.new(distributor, order_cycle).fees_by_type_for self 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 @@ -123,21 +113,7 @@ Spree::Variant.class_eval do 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 + exchange_variants(:reload).destroy_all + yield end end diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb index 4c367652fd..9f0160202b 100644 --- a/app/models/variant_override.rb +++ b/app/models/variant_override.rb @@ -12,9 +12,6 @@ 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| @@ -73,14 +70,4 @@ class VariantOverride < ActiveRecord::Base end self end - - private - - 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/serializers/api/product_serializer.rb b/app/serializers/api/product_serializer.rb index 91ff62f52a..592b67ecdb 100644 --- a/app/serializers/api/product_serializer.rb +++ b/app/serializers/api/product_serializer.rb @@ -1,43 +1,11 @@ require 'open_food_network/scope_variant_to_hub' class Api::ProductSerializer < ActiveModel::Serializer - # TODO - # Prices can't be cached? How? - def serializable_hash - cached_serializer_hash.merge(uncached_serializer_hash) - end - - private - - def cached_serializer_hash - Api::CachedProductSerializer.new(object, @options).serializable_hash - end - - def uncached_serializer_hash - Api::UncachedProductSerializer.new(object, @options).serializable_hash - end -end - -class Api::UncachedProductSerializer < ActiveModel::Serializer - attributes :price - - def price - if options[:enterprise_fee_calculator] - object.master.price + options[:enterprise_fee_calculator].indexed_fees_for(object.master) - else - object.master.price_with_fees(options[:current_distributor], options[:current_order_cycle]) - end - end -end - -class Api::CachedProductSerializer < ActiveModel::Serializer - # cached - # delegate :cache_key, to: :object include ActionView::Helpers::SanitizeHelper attributes :id, :name, :permalink, :meta_keywords attributes :group_buy, :notes, :description, :description_html - attributes :properties_with_values + attributes :properties_with_values, :price has_many :variants, serializer: Api::VariantSerializer has_one :master, serializer: Api::VariantSerializer @@ -70,4 +38,12 @@ class Api::CachedProductSerializer < ActiveModel::Serializer def master options[:master_variants][object.id].andand.first end + + def price + if options[:enterprise_fee_calculator] + object.master.price + options[:enterprise_fee_calculator].indexed_fees_for(object.master) + else + object.master.price_with_fees(options[:current_distributor], options[:current_order_cycle]) + end + end end diff --git a/app/services/order_cycle_distributed_products.rb b/app/services/order_cycle_distributed_products.rb index 77c20cde63..e85d45013d 100644 --- a/app/services/order_cycle_distributed_products.rb +++ b/app/services/order_cycle_distributed_products.rb @@ -1,9 +1,10 @@ # Returns a (paginatable) AR object for the products or variants in stock for a given shop and OC. # The stock-checking includes on_demand and stock level overrides from variant_overrides. class OrderCycleDistributedProducts - def initialize(distributor, order_cycle) + def initialize(distributor, order_cycle, customer) @distributor = distributor @order_cycle = order_cycle + @customer = customer end def products_relation @@ -11,26 +12,30 @@ class OrderCycleDistributedProducts end def variants_relation - @order_cycle. - variants_distributed_by(@distributor). + order_cycle. + variants_distributed_by(distributor). merge(stocked_variants_and_overrides) end private + attr_reader :distributor, :order_cycle, :customer + def stocked_products - @order_cycle. - variants_distributed_by(@distributor). + order_cycle. + variants_distributed_by(distributor). merge(stocked_variants_and_overrides). select("DISTINCT spree_variants.product_id") end def stocked_variants_and_overrides - Spree::Variant. + stocked_variants = Spree::Variant. joins("LEFT OUTER JOIN variant_overrides ON variant_overrides.variant_id = spree_variants.id - AND variant_overrides.hub_id = #{@distributor.id}"). + AND variant_overrides.hub_id = #{distributor.id}"). joins(:stock_items). where(query_stock_with_overrides) + + ProductTagRulesFilterer.new(distributor, customer, stocked_variants).call end def query_stock_with_overrides diff --git a/app/services/product_tag_rules_filterer.rb b/app/services/product_tag_rules_filterer.rb new file mode 100644 index 0000000000..b58cbc2c43 --- /dev/null +++ b/app/services/product_tag_rules_filterer.rb @@ -0,0 +1,111 @@ +# Takes a Spree::Variant AR object and filters results based on applicable tag rules. +# Tag rules exists in the context of enterprise, customer, and variant_overrides, +# and are applied to variant_overrides only. Returns a Spree::Variant AR object. + +class ProductTagRulesFilterer + def initialize(distributor, customer, variants_relation) + @distributor = distributor + @customer = customer + @variants_relation = variants_relation + end + + def call + return variants_relation unless distributor_rules.any? + + filter(variants_relation) + end + + private + + attr_accessor :distributor, :customer, :variants_relation + + def distributor_rules + @distributor_rules ||= TagRule::FilterProducts.prioritised.for(distributor).all + end + + def filter(variants_relation) + return variants_relation unless overrides_to_hide.any? + + variants_relation.where(query_with_tag_rules) + end + + def query_with_tag_rules + "#{variant_not_overriden} OR ( #{variant_overriden} + AND ( #{override_not_hidden_by_rule} + OR #{override_shown_by_rule} ) )" + end + + def variant_not_overriden + "variant_overrides.id IS NULL" + end + + def variant_overriden + "variant_overrides.id IS NOT NULL" + end + + def override_not_hidden_by_rule + return "FALSE" unless overrides_to_hide.any? + "variant_overrides.id NOT IN (#{overrides_to_hide.join(',')})" + end + + def override_shown_by_rule + return "FALSE" unless overrides_to_show.any? + "variant_overrides.id IN (#{overrides_to_show.join(',')})" + end + + def overrides_to_hide + @overrides_to_hide ||= VariantOverride.where(hub_id: distributor.id). + tagged_with(default_rule_tags + hide_rule_tags, any: true). + pluck(:id) + end + + def overrides_to_show + @overrides_to_show ||= VariantOverride.where(hub_id: distributor.id). + tagged_with(show_rule_tags, any: true). + pluck(:id) + end + + def default_rule_tags + default_rules.map(&:preferred_variant_tags) + end + + def hide_rule_tags + hide_rules.map(&:preferred_variant_tags) + end + + def show_rule_tags + show_rules.map(&:preferred_variant_tags) + end + + def default_rules + # These rules hide a variant_override with tag X and apply to all customers + distributor_rules.select(&:is_default?) + end + + def non_default_rules + # These rules show or hide a variant_override with tag X for customer with tag Y + distributor_rules.reject(&:is_default?) + end + + def customer_applicable_rules + # Rules which apply specifically to the current customer + @customer_applicable_rules ||= non_default_rules.select{ |rule| customer_tagged?(rule) } + end + + def hide_rules + @hide_rules ||= customer_applicable_rules. + select{ |rule| rule.preferred_matched_variants_visibility == 'hidden' } + end + + def show_rules + customer_applicable_rules - hide_rules + end + + def customer_tagged?(rule) + customer_tag_list.include? rule.preferred_customer_tags + end + + def customer_tag_list + customer.andand.tag_list || [] + end +end diff --git a/app/services/products_renderer.rb b/app/services/products_renderer.rb new file mode 100644 index 0000000000..2d64ed4471 --- /dev/null +++ b/app/services/products_renderer.rb @@ -0,0 +1,98 @@ +require 'open_food_network/scope_product_to_hub' + +class ProductsRenderer + class NoProducts < RuntimeError; end + DEFAULT_PAGE = 1 + DEFAULT_PER_PAGE = 10 + + def initialize(distributor, order_cycle, customer, args = {}) + @distributor = distributor + @order_cycle = order_cycle + @customer = customer + @args = args + end + + def products_json + raise NoProducts unless order_cycle && distributor && products + + ActiveModel::ArraySerializer.new(products, + each_serializer: Api::ProductSerializer, + current_order_cycle: order_cycle, + current_distributor: distributor, + variants: variants_for_shop_by_id, + master_variants: master_variants_for_shop_by_id, + enterprise_fee_calculator: enterprise_fee_calculator).to_json + end + + private + + attr_reader :order_cycle, :distributor, :customer, :args + + def products + return unless order_cycle + + @products ||= begin + results = distributed_products.products_relation.order(taxon_order) + + filter_and_paginate(results). + each { |product| product_scoper.scope(product) } # Scope results with variant_overrides + end + end + + def product_scoper + OpenFoodNetwork::ScopeProductToHub.new(distributor) + end + + def enterprise_fee_calculator + OpenFoodNetwork::EnterpriseFeeCalculator.new distributor, order_cycle + end + + def filter_and_paginate(query) + query. + ransack(args[:q]). + result. + page(args[:page] || DEFAULT_PAGE). + per(args[:per_page] || DEFAULT_PER_PAGE) + end + + def distributed_products + OrderCycleDistributedProducts.new(distributor, order_cycle, customer) + end + + def taxon_order + if distributor.preferred_shopfront_taxon_order.present? + distributor + .preferred_shopfront_taxon_order + .split(",").map { |id| "primary_taxon_id=#{id} DESC" } + .join(",") + ", name ASC" + else + "name ASC" + end + end + + def variants_for_shop + @variants_for_shop ||= begin + scoper = OpenFoodNetwork::ScopeVariantToHub.new(distributor) + + distributed_products.variants_relation. + includes(:default_price, :stock_locations, :product). + where(product_id: products). + each { |v| scoper.scope(v) } # Scope results with variant_overrides + end + end + + def variants_for_shop_by_id + index_by_product_id variants_for_shop.reject(&:is_master) + end + + def master_variants_for_shop_by_id + index_by_product_id variants_for_shop.select(&:is_master) + end + + def index_by_product_id(variants) + variants.each_with_object({}) do |v, vs| + vs[v.product_id] ||= [] + vs[v.product_id] << v + end + end +end diff --git a/app/views/admin/cache_settings/edit.html.haml b/app/views/admin/cache_settings/edit.html.haml deleted file mode 100644 index 222910be94..0000000000 --- a/app/views/admin/cache_settings/edit.html.haml +++ /dev/null @@ -1,31 +0,0 @@ -- content_for :page_title do - = t(:cache_settings) - -= form_tag main_app.admin_cache_settings_path, :method => :put do - .field - = hidden_field_tag 'preferences[enable_products_cache?]', '0' - = check_box_tag 'preferences[enable_products_cache?]', '1', Spree::Config[:enable_products_cache?] - = label_tag nil, t('.enable_products_cache') - .form-buttons - = button t(:update), 'icon-refresh' - -%br -%br - -%h4= t(:cache_state) -%br -%table.index - %thead - %tr - %th= t('.distributor') - %th= t('.order_cycle') - %th= t('.status') - %th= t('.diff') - %tbody - - @results.each do |result| - %tr - %td= result[:distributor].name - %td= result[:order_cycle].name - %td= result[:status] ? t(:ok) : t('.error') - %td - %pre= result[:diff].to_s(:text) diff --git a/app/views/shop/products/_filters.html.haml b/app/views/shop/products/_filters.html.haml index 10c7c20deb..100ccecc0a 100644 --- a/app/views/shop/products/_filters.html.haml +++ b/app/views/shop/products/_filters.html.haml @@ -1,5 +1,5 @@ -.filter-shopfront.taxon-selectors.text-right - %single-line-selectors{ selectors: "taxonSelectors", objects: "Products.products | products:query | properties:activeProperties | taxonsOf", "active-selectors" => "activeTaxons"} +.filter-shopfront.taxon-selectors.text-right{ng: {show: 'supplied_taxons != null'}} + %single-line-selectors{ selectors: "taxonSelectors", objects: "supplied_taxons", "active-selectors" => "activeTaxons"} -.filter-shopfront.property-selectors.text-right - %single-line-selectors{ selectors: "propertySelectors", objects: "Products.products | products:query | taxons:activeTaxons | propertiesOf", "active-selectors" => "activeProperties"} +.filter-shopfront.property-selectors.text-right{ng: {show: 'supplied_properties != null'}} + %single-line-selectors{ selectors: "propertySelectors", objects: "supplied_properties", "active-selectors" => "activeProperties"} diff --git a/app/views/shop/products/_form.html.haml b/app/views/shop/products/_form.html.haml index 3a9738a532..ed2b555f3e 100644 --- a/app/views/shop/products/_form.html.haml +++ b/app/views/shop/products/_form.html.haml @@ -22,14 +22,14 @@ .small-12.medium-6.large-5.columns %input#search.text{"ng-model" => "query", placeholder: t(:products_search), - "ng-debounce" => "100", + "ng-debounce" => "200", "ofn-disable-enter" => true} .small-12.medium-6.large-6.large-offset-1.columns = render partial: "shop/products/filters" - %div.pad-top{ "infinite-scroll" => "incrementLimit()", "infinite-scroll-distance" => "1", "infinite-scroll-disabled" => 'filteredProducts.length <= limit' } - %product.animate-repeat{"ng-controller" => "ProductNodeCtrl", "ng-repeat" => "product in visibleProducts track by product.id", "id" => "product-{{ product.id }}"} + %div.pad-top{ "infinite-scroll" => "loadMore()", "infinite-scroll-distance" => "1", "infinite-scroll-disabled" => 'Products.loading' } + %product.animate-repeat{"ng-controller" => "ProductNodeCtrl", "ng-repeat" => "product in Products.products track by product.id", "id" => "product-{{ product.id }}"} = render "shop/products/summary" %shop-variant{variant: 'variant', "ng-repeat" => "variant in product.variants | orderBy: ['name_to_display','unit_value'] track by variant.id", "id" => "variant-{{ variant.id }}", "ng-class" => "{'out-of-stock': !variant.on_demand && variant.on_hand == 0}"} @@ -41,7 +41,7 @@ .small-12.columns.text-center %img.spinner{ src: "/assets/spinning-circles.svg" } - %div{"ng-show" => "filteredProducts.length == 0 && !Products.loading"} + %div{"ng-show" => "Products.products.length == 0 && !Products.loading"} .row.summary .small-12.columns %p.no-results diff --git a/app/views/spree/admin/shared/_configuration_menu.html.haml b/app/views/spree/admin/shared/_configuration_menu.html.haml index 76bc3cdadd..7ec62c516e 100644 --- a/app/views/spree/admin/shared/_configuration_menu.html.haml +++ b/app/views/spree/admin/shared/_configuration_menu.html.haml @@ -21,7 +21,6 @@ = configurations_sidebar_menu_item Spree.t(:shipping_categories), admin_shipping_categories_path = configurations_sidebar_menu_item t(:enterprise_fees), main_app.admin_enterprise_fees_path = configurations_sidebar_menu_item Spree.t(:analytics_trackers), admin_trackers_path - = configurations_sidebar_menu_item t('admin.cache_settings.edit.title'), main_app.edit_admin_cache_settings_path = configurations_sidebar_menu_item t('admin.contents.edit.title'), main_app.edit_admin_contents_path = configurations_sidebar_menu_item t('admin.invoice_settings.edit.title'), main_app.edit_admin_invoice_settings_path = configurations_sidebar_menu_item t('admin.matomo_settings.edit.title'), main_app.edit_admin_matomo_settings_path diff --git a/config/locales/en.yml b/config/locales/en.yml index fad1407539..cbb9380ffb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -373,16 +373,6 @@ en: number_localization_settings: "Number Localization Settings" enable_localized_number: "Use the international thousand/decimal separator logic" - cache_settings: - edit: - title: "Caching" - distributor: "Distributor" - order_cycle: "Order Cycle" - status: "Status" - diff: "Diff" - error: "Error" - enable_products_cache: "Enable Products Cache?" - invoice_settings: edit: title: "Invoice Settings" diff --git a/config/routes.rb b/config/routes.rb index 6e7df6ea64..65a70165e1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -33,7 +33,6 @@ Openfoodnetwork::Application.routes.draw do end resource :shop, controller: "shop" do - get :products post :order_cycle get :order_cycle get :changeable_orders_alert diff --git a/config/routes/admin.rb b/config/routes/admin.rb index 86a0ffbaca..5b21d0baa2 100644 --- a/config/routes/admin.rb +++ b/config/routes/admin.rb @@ -75,8 +75,6 @@ Openfoodnetwork::Application.routes.draw do resource :contents - resource :cache_settings - resources :column_preferences, only: [], format: :json do put :bulk_update, on: :collection end diff --git a/config/routes/api.rb b/config/routes/api.rb index 75d3638016..19496c4e17 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -44,6 +44,10 @@ Openfoodnetwork::Application.routes.draw do resources :order_cycles do get :managed, on: :collection get :accessible, on: :collection + + get :products, on: :member + get :taxons, on: :member + get :properties, on: :member end resource :status do diff --git a/config/schedule.rb b/config/schedule.rb index 9ad169cacf..b1c5928aa9 100644 --- a/config/schedule.rb +++ b/config/schedule.rb @@ -12,10 +12,6 @@ job_type :run_file, "cd :path; :environment_variable=:environment bundle exec sc job_type :enqueue_job, "cd :path; :environment_variable=:environment bundle exec script/enqueue :task :priority :output" -every 1.day, at: '01:00am' do - rake 'ofn:cache:check_products_integrity' -end - every 1.day, at: '2:45am' do rake 'db2fog:clean' if ENV['S3_BACKUPS_BUCKET'] end diff --git a/lib/open_food_network/cached_products_renderer.rb b/lib/open_food_network/cached_products_renderer.rb deleted file mode 100644 index 6d31705cd4..0000000000 --- a/lib/open_food_network/cached_products_renderer.rb +++ /dev/null @@ -1,48 +0,0 @@ -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 < RuntimeError; end - - def initialize(distributor, order_cycle) - @distributor = distributor - @order_cycle = order_cycle - end - - def products_json - raise NoProducts, I18n.t(:no_products) if @distributor.nil? || @order_cycle.nil? - - products_json = cached_products_json - - raise NoProducts, I18n.t(:no_products) if products_json.nil? - - products_json - end - - private - - def cached_products_json - return uncached_products_json unless use_cached_products? - - Rails.cache.fetch("products-json-#{@distributor.id}-#{@order_cycle.id}") do - begin - uncached_products_json - rescue ProductsRenderer::NoProducts - nil - end - end - end - - def use_cached_products? - Spree::Config[:enable_products_cache?] - 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 deleted file mode 100644 index 089dc83030..0000000000 --- a/lib/open_food_network/products_cache.rb +++ /dev/null @@ -1,190 +0,0 @@ -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.id).each do |exchange| - refresh_cache exchange.receiver, exchange.order_cycle - end - end - - def self.variant_destroyed(variant, &block) - exchanges = exchanges_featuring_variants(variant.id).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.map(&:id)).each do |exchange| - refresh_cache exchange.receiver, exchange.order_cycle - end - end - - def self.product_deleted(product, &block) - exchanges = exchanges_featuring_variants(product.reload.variants.map(&:id)).to_a - - block.call - - exchanges.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.id, 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.select(:id)).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) - return if order_cycle.undated? - return if order_cycle.closed? - - order_cycle.exchanges.outgoing.each do |exchange| - refresh_cache exchange.receiver, order_cycle - 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.id, - distributor: inventory_item.enterprise).each do |exchange| - refresh_cache exchange.receiver, exchange.order_cycle - end - end - - def self.exchanges_featuring_variants(variant_ids, distributor: nil) - exchanges = Exchange. - outgoing. - with_any_variant(variant_ids). - joins(:order_cycle). - merge(OrderCycle.dated). - merge(OrderCycle.not_closed) - - exchanges = exchanges.to_enterprise(distributor) if distributor - - exchanges - end - private_class_method :exchanges_featuring_variants - - def self.refresh_incoming_exchanges(exchanges) - outgoing_exchanges_affected_by(exchanges).each do |outgoing_exchange| - refresh_cache(outgoing_exchange.receiver, outgoing_exchange.order_cycle) - end - end - private_class_method :refresh_incoming_exchanges - - def self.refresh_outgoing_exchange(exchange) - return if exchange.order_cycle.undated? - return if exchange.order_cycle.closed? - - refresh_cache(exchange.receiver, exchange.order_cycle) - end - private_class_method :refresh_outgoing_exchange - - def self.refresh_supplier_fee(enterprise_fee) - refresh_incoming_exchanges(enterprise_fee.exchanges) - end - private_class_method :refresh_supplier_fee - - def self.refresh_coordinator_fee(enterprise_fee) - enterprise_fee.order_cycles.each do |order_cycle| - order_cycle_changed order_cycle - end - end - private_class_method :refresh_coordinator_fee - - 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 - private_class_method :refresh_distributor_fee - - def self.incoming_exchanges(exchanges) - exchanges. - incoming. - joins(:order_cycle). - merge(OrderCycle.dated). - merge(OrderCycle.not_closed) - end - private_class_method :incoming_exchanges - - def self.outgoing_exchanges_affected_by(exchanges) - incoming = incoming_exchanges(exchanges) - order_cycle_ids = incoming.select(:order_cycle_id) - variant_ids = ExchangeVariant.where(exchange_id: incoming).select(:variant_id) - Exchange.outgoing. - in_order_cycle(order_cycle_ids). - with_any_variant(variant_ids). - except(:select).select("DISTINCT receiver_id, order_cycle_id") - end - private_class_method :outgoing_exchanges_affected_by - - 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 - private_class_method :outgoing_exchanges_with_variants - - def self.refresh_cache(distributor, order_cycle) - ProductsCacheRefreshment.refresh distributor, order_cycle - end - private_class_method :refresh_cache - end -end diff --git a/lib/open_food_network/products_cache_integrity_checker.rb b/lib/open_food_network/products_cache_integrity_checker.rb deleted file mode 100644 index a47ea804e5..0000000000 --- a/lib/open_food_network/products_cache_integrity_checker.rb +++ /dev/null @@ -1,34 +0,0 @@ -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 deleted file mode 100644 index 3bcde00f8d..0000000000 --- a/lib/open_food_network/products_cache_refreshment.rb +++ /dev/null @@ -1,42 +0,0 @@ -# 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) - job = refresh_job(distributor, order_cycle) - enqueue_job(job) unless pending_job?(job) - end - - def self.refresh_job(distributor, order_cycle) - RefreshProductsCacheJob.new(distributor.id, order_cycle.id) - end - private_class_method :refresh_job - - def self.pending_job?(job) - Delayed::Job. - where(locked_at: nil). - where(handler: job.to_yaml). - exists? - end - private_class_method :pending_job? - - def self.enqueue_job(job) - Delayed::Job.enqueue job, priority: 10 - end - private_class_method :enqueue_job - end -end diff --git a/lib/open_food_network/products_renderer.rb b/lib/open_food_network/products_renderer.rb deleted file mode 100644 index 527385b6e8..0000000000 --- a/lib/open_food_network/products_renderer.rb +++ /dev/null @@ -1,83 +0,0 @@ -require 'open_food_network/scope_product_to_hub' - -module OpenFoodNetwork - class ProductsRenderer - class NoProducts < RuntimeError; end - - def initialize(distributor, order_cycle) - @distributor = distributor - @order_cycle = order_cycle - end - - def products_json - if products - enterprise_fee_calculator = EnterpriseFeeCalculator.new @distributor, @order_cycle - - ActiveModel::ArraySerializer.new(products, - each_serializer: Api::ProductSerializer, - current_order_cycle: @order_cycle, - current_distributor: @distributor, - variants: variants_for_shop_by_id, - master_variants: master_variants_for_shop_by_id, - enterprise_fee_calculator: enterprise_fee_calculator,).to_json - else - raise NoProducts - end - end - - private - - def products - return unless @order_cycle - - @products ||= begin - scoper = ScopeProductToHub.new(@distributor) - - distributed_products.products_relation. - order(taxon_order). - each { |product| scoper.scope(product) } - end - end - - def distributed_products - OrderCycleDistributedProducts.new(@distributor, @order_cycle) - end - - def taxon_order - if @distributor.preferred_shopfront_taxon_order.present? - @distributor - .preferred_shopfront_taxon_order - .split(",").map { |id| "primary_taxon_id=#{id} DESC" } - .join(",") + ", name ASC" - else - "name ASC" - end - end - - def variants_for_shop - @variants_for_shop ||= begin - scoper = OpenFoodNetwork::ScopeVariantToHub.new(@distributor) - - distributed_products.variants_relation. - includes(:default_price, :stock_locations, :product). - where(product_id: products). - each { |v| scoper.scope(v) } - end - end - - def variants_for_shop_by_id - index_by_product_id variants_for_shop.reject(&:is_master) - end - - def master_variants_for_shop_by_id - index_by_product_id variants_for_shop.select(&:is_master) - end - - def index_by_product_id(variants) - variants.each_with_object({}) do |v, vs| - vs[v.product_id] ||= [] - vs[v.product_id] << v - end - end - end -end diff --git a/lib/tasks/cache.rake b/lib/tasks/cache.rake deleted file mode 100644 index 535f19079b..0000000000 --- a/lib/tasks/cache.rake +++ /dev/null @@ -1,19 +0,0 @@ -require 'open_food_network/products_cache_integrity_checker' - -namespace :ofn 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/api/order_cycles_controller_spec.rb b/spec/controllers/api/order_cycles_controller_spec.rb new file mode 100644 index 0000000000..1cf346277d --- /dev/null +++ b/spec/controllers/api/order_cycles_controller_spec.rb @@ -0,0 +1,196 @@ +require "spec_helper" + +module Api + describe OrderCyclesController, type: :controller do + 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!(:taxon1) { create(:taxon, name: 'Meat') } + let!(:taxon2) { create(:taxon, name: 'Vegetables') } + let!(:taxon3) { create(:taxon, name: 'Cake') } + let!(:property1) { create(:property, presentation: 'Organic') } + let!(:property2) { create(:property, presentation: 'Dairy-Free') } + let!(:property3) { create(:property, presentation: 'May Contain Nuts') } + let!(:product1) { create(:product, primary_taxon: taxon1, properties: [property1]) } + let!(:product2) { create(:product, primary_taxon: taxon2, properties: [property2]) } + let!(:product3) { create(:product, primary_taxon: taxon2) } + let!(:product4) { create(:product, primary_taxon: taxon3, properties: [property3]) } + let!(:user) { create(:user) } + let(:customer) { create(:customer, user: user, enterprise: distributor) } + + before do + exchange.variants << product1.variants.first + exchange.variants << product2.variants.first + exchange.variants << product3.variants.first + allow(controller).to receive(:spree_current_user) { user } + end + + describe "#products" do + it "loads products for distributed products in the order cycle" do + api_get :products, id: order_cycle.id, distributor: distributor.id + + expect(product_ids).to include product1.id, product2.id, product3.id + end + + context "with variant overrides" do + let!(:vo1) { + create(:variant_override, + hub: distributor, + variant: product1.variants.first, + price: 1234.56) + } + let!(:vo2) { + create(:variant_override, + hub: distributor, + variant: product2.variants.first, + count_on_hand: 0) + } + + it "returns results scoped with variant overrides" do + api_get :products, id: order_cycle.id, distributor: distributor.id + + overidden_product = json_response.select{ |product| product['id'] == product1.id } + expect(overidden_product[0]['variants'][0]['price']).to eq vo1.price.to_s + end + + it "does not return products where the variant overrides are out of stock" do + api_get :products, id: order_cycle.id, distributor: distributor.id + + expect(product_ids).to_not include product2.id + end + end + + context "with property filters" do + it "filters by product property" do + api_get :products, id: order_cycle.id, distributor: distributor.id, + q: { properties_id_or_supplier_properties_id_in_any: [property1.id, property2.id] } + + expect(product_ids).to include product1.id, product2.id + expect(product_ids).to_not include product3.id + end + end + + context "with taxon filters" do + it "filters by taxon" do + api_get :products, id: order_cycle.id, distributor: distributor.id, + q: { primary_taxon_id_in_any: [taxon2.id] } + + expect(product_ids).to include product2.id, product3.id + expect(product_ids).to_not include product1.id, product4.id + end + end + + context "when tag rules apply" do + let!(:vo1) { + create(:variant_override, + hub: distributor, + variant: product1.variants.first) + } + let!(:vo2) { + create(:variant_override, + hub: distributor, + variant: product2.variants.first) + } + let!(:vo3) { + create(:variant_override, + hub: distributor, + variant: product3.variants.first) + } + let(:default_hide_rule) { + create(:filter_products_tag_rule, + enterprise: distributor, + is_default: true, + preferred_variant_tags: "hide_these_variants_from_everyone", + preferred_matched_variants_visibility: "hidden") + } + let!(:hide_rule) { + create(:filter_products_tag_rule, + enterprise: distributor, + preferred_variant_tags: "hide_these_variants", + preferred_customer_tags: "hide_from_these_customers", + preferred_matched_variants_visibility: "hidden" ) + } + let!(:show_rule) { + create(:filter_products_tag_rule, + enterprise: distributor, + preferred_variant_tags: "show_these_variants", + preferred_customer_tags: "show_for_these_customers", + preferred_matched_variants_visibility: "visible" ) + } + + it "does not return variants hidden by general rules" do + vo1.update_attribute(:tag_list, default_hide_rule.preferred_variant_tags) + + api_get :products, id: order_cycle.id, distributor: distributor.id + + expect(product_ids).to_not include product1.id + end + + it "does not return variants hidden for this specific customer" do + vo2.update_attribute(:tag_list, hide_rule.preferred_variant_tags) + customer.update_attribute(:tag_list, hide_rule.preferred_customer_tags) + + api_get :products, id: order_cycle.id, distributor: distributor.id + + expect(product_ids).to_not include product2.id + end + + it "returns hidden variants made visible for this specific customer" do + vo1.update_attribute(:tag_list, default_hide_rule.preferred_variant_tags) + vo3.update_attribute(:tag_list, "#{show_rule.preferred_variant_tags},#{default_hide_rule.preferred_variant_tags}") + customer.update_attribute(:tag_list, show_rule.preferred_customer_tags) + + api_get :products, id: order_cycle.id, distributor: distributor.id + + expect(product_ids).to_not include product1.id + expect(product_ids).to include product3.id + end + end + end + + describe "#taxons" do + it "loads taxons for distributed products in the order cycle" do + api_get :taxons, id: order_cycle.id, distributor: distributor.id + + taxons = json_response.map{ |taxon| taxon['name'] } + + expect(json_response.length).to be 2 + expect(taxons).to include taxon1.name, taxon2.name + end + end + + describe "#properties" do + it "loads properties for distributed products in the order cycle" do + api_get :properties, id: order_cycle.id, distributor: distributor.id + + properties = json_response.map{ |property| property['name'] } + + expect(json_response.length).to be 2 + expect(properties).to include property1.presentation, property2.presentation + end + + context "with producer properties" do + let!(:property4) { create(:property) } + let!(:producer_property) { + create(:producer_property, producer_id: product1.supplier.id, property: property4) + } + + it "loads producer properties for distributed products in the order cycle" do + api_get :properties, id: order_cycle.id, distributor: distributor.id + + properties = json_response.map{ |property| property['name'] } + + expect(json_response.length).to be 3 + expect(properties).to include property1.presentation, property2.presentation, + producer_property.property.presentation + end + end + end + + private + + def product_ids + json_response.map{ |product| product['id'] } + end + end +end diff --git a/spec/controllers/api/variants_controller_spec.rb b/spec/controllers/api/variants_controller_spec.rb index 9964d40c1b..857254ca15 100644 --- a/spec/controllers/api/variants_controller_spec.rb +++ b/spec/controllers/api/variants_controller_spec.rb @@ -115,15 +115,6 @@ describe Api::VariantsController, type: :controller do expect { variant.reload }.not_to raise_error expect(variant.deleted_at).to be_nil end - - context 'when the variant is not the master' do - before { variant.update_attribute(:is_master, false) } - - it 'refreshes the cache' do - expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).with(variant) - spree_delete :soft_delete, variant_id: variant.id, product_id: variant.product.permalink, format: :json - end - end end context "as an administrator" do @@ -150,15 +141,6 @@ describe Api::VariantsController, type: :controller do expect(assigns(:variant).errors[:product]).to include "must have at least one variant" end - context 'when the variant is not the master' do - before { variant.update_attribute(:is_master, false) } - - it 'refreshes the cache' do - expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).with(variant) - spree_delete :soft_delete, variant_id: variant.id, product_id: variant.product.permalink, format: :json - end - end - context "deleted variants" do before do variant.update_column(:deleted_at, Time.zone.now) diff --git a/spec/controllers/shop_controller_spec.rb b/spec/controllers/shop_controller_spec.rb index 64b4e9281b..233fe9d36a 100644 --- a/spec/controllers/shop_controller_spec.rb +++ b/spec/controllers/shop_controller_spec.rb @@ -82,167 +82,5 @@ describe ShopController, type: :controller do expect(controller.current_order_cycle).to be_nil end end - - describe "returning products" do - 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 - allow(controller).to receive(:current_order_cycle).and_return order_cycle - xhr :get, :products - expect(response).to be_success - end - - it "does not return products if no order cycle is selected" do - allow(controller).to receive(:current_order_cycle).and_return nil - xhr :get, :products - expect(response.status).to eq(404) - expect(response.body).to be_empty - end - end - end - - describe "determining rule relevance" do - let(:products_json) { double(:products_json) } - let(:applicator) { double(:applicator) } - - before do - allow(applicator).to receive(:rules) { tag_rules } - allow(controller).to receive(:applicator) { applicator } - allow(controller).to receive(:filter) { "some filtered json" } - end - - context "when no relevant rules exist" do - let(:tag_rules) { [] } - - it "does not attempt to apply any rules" do - controller.send(:filtered_json, products_json) - expect(expect(controller).to_not(have_received(:filter))) - end - - it "returns products as JSON" do - expect(controller.send(:filtered_json, products_json)).to eq products_json - end - end - - context "when relevant rules exist" do - let(:tag_rule) { create(:filter_products_tag_rule, preferred_customer_tags: "tag1", preferred_variant_tags: "tag1", preferred_matched_variants_visibility: "hidden" ) } - let(:tag_rules) { [tag_rule] } - - it "attempts to apply any rules" do - controller.send(:filtered_json, products_json) - expect(controller).to have_received(:filter).with(products_json) - end - - it "returns filtered JSON" do - expect(controller.send(:filtered_json, products_json)).to eq "some filtered json" - end - end - end - - describe "loading available order cycles" do - let(:user) { create(:user) } - before { allow(controller).to receive(:spree_current_user) { user } } - - context "when FilterProducts tag rules are in effect" do - let(:customer) { create(:customer, user: user, enterprise: distributor) } - let!(:tag_rule) { - create(:filter_products_tag_rule, - enterprise: distributor, - preferred_customer_tags: "member", - preferred_variant_tags: "members-only") - } - let!(:default_tag_rule) { - create(:filter_products_tag_rule, - enterprise: distributor, - is_default: true, - preferred_variant_tags: "members-only") - } - let(:product1) { { "id" => 1, "name" => 'product 1', "variants" => [{ "id" => 4, "tag_list" => ["members-only"] }] } } - let(:product2) { { "id" => 2, "name" => 'product 2', "variants" => [{ "id" => 5, "tag_list" => ["members-only"] }, { "id" => 9, "tag_list" => ["something"] }] } } - let(:product3) { { "id" => 3, "name" => 'product 3', "variants" => [{ "id" => 6, "tag_list" => ["something-else"] }] } } - let(:product2_without_v5) { { "id" => 2, "name" => 'product 2', "variants" => [{ "id" => 9, "tag_list" => ["something"] }] } } - let!(:products_array) { [product1, product2, product3] } - let!(:products_json) { JSON.unparse( products_array ) } - - before do - allow(controller).to receive(:current_order) { order } - end - - context "with a preferred visiblity of 'visible', default visibility of 'hidden'" do - before { tag_rule.update_attribute(:preferred_matched_variants_visibility, 'visible') } - before { default_tag_rule.update_attribute(:preferred_matched_variants_visibility, 'hidden') } - - let(:filtered_products) { JSON.parse(controller.send(:filter, products_json)) } - - context "when the customer is nil" do - it "applies default action (hide)" do - expect(controller.current_customer).to be nil - expect(filtered_products).to include product2_without_v5, product3 - expect(filtered_products).to_not include product1, product2 - end - end - - context "when the customer's tags match" do - before { customer.update_attribute(:tag_list, 'member') } - - it "applies the action (show)" do - expect(controller.current_customer).to eq customer - expect(filtered_products).to include product1, product2, product3 - end - end - - context "when the customer's tags don't match" do - before { customer.update_attribute(:tag_list, 'something') } - - it "applies the default action (hide)" do - expect(controller.current_customer).to eq customer - expect(filtered_products).to include product2_without_v5, product3 - expect(filtered_products).to_not include product1, product2 - end - end - end - - context "with a preferred visiblity of 'hidden', default visibility of 'visible'" do - before { tag_rule.update_attribute(:preferred_matched_variants_visibility, 'hidden') } - before { default_tag_rule.update_attribute(:preferred_matched_variants_visibility, 'visible') } - - let(:filtered_products) { JSON.parse(controller.send(:filter, products_json)) } - - context "when the customer is nil" do - it "applies default action (show)" do - expect(controller.current_customer).to be nil - expect(filtered_products).to include product1, product2, product3 - end - end - - context "when the customer's tags match" do - before { customer.update_attribute(:tag_list, 'member') } - - it "applies the action (hide)" do - expect(controller.current_customer).to eq customer - expect(filtered_products).to include product2_without_v5, product3 - expect(filtered_products).to_not include product1, product2 - end - end - - context "when the customer's tags don't match" do - before { customer.update_attribute(:tag_list, 'something') } - - it "applies the default action (show)" do - expect(controller.current_customer).to eq customer - expect(filtered_products).to include product1, product2, product3 - end - end - end - end - end end end diff --git a/spec/controllers/spree/admin/variants_controller_spec.rb b/spec/controllers/spree/admin/variants_controller_spec.rb index 4d4d1049cd..e489e5364b 100644 --- a/spec/controllers/spree/admin/variants_controller_spec.rb +++ b/spec/controllers/spree/admin/variants_controller_spec.rb @@ -60,11 +60,6 @@ module Spree expect(response).to render_template('spree/admin/shared/_destroy') end - it 'refreshes the cache' do - expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).with(variant) - spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'js' - end - it 'destroys all its exchanges' do exchange = create(:exchange) variant.exchanges << exchange @@ -99,11 +94,6 @@ module Spree ) end - it 'refreshes the cache' do - expect(OpenFoodNetwork::ProductsCache).to receive(:variant_destroyed).with(variant) - spree_delete :destroy, id: variant.id, product_id: variant.product.permalink, format: 'js' - end - it 'destroys all its exchanges' do exchange = create(:exchange) variant.exchanges << exchange diff --git a/spec/features/admin/caching_spec.rb b/spec/features/admin/caching_spec.rb deleted file mode 100644 index 64d420a102..0000000000 --- a/spec/features/admin/caching_spec.rb +++ /dev/null @@ -1,46 +0,0 @@ -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" - allow(OpenFoodNetwork::ProductsRenderer).to receive(: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 - expect(page).to 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" - allow(OpenFoodNetwork::ProductsRenderer).to receive(: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 - expect(page).to have_content "Error" - end - end -end diff --git a/spec/features/admin/enterprises_spec.rb b/spec/features/admin/enterprises_spec.rb index f965dcb3ef..4244e265e7 100644 --- a/spec/features/admin/enterprises_spec.rb +++ b/spec/features/admin/enterprises_spec.rb @@ -281,35 +281,6 @@ feature ' end 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 - - # This test relies on preference persistence, so we'll turn it on for this spec only. - # It will be turned off again automatically by reset_spree_preferences in spec_helper. - Spree::Preferences::Store.instance.persistence = true - 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/consumer/shopping/checkout_spec.rb b/spec/features/consumer/shopping/checkout_spec.rb index 8f1d8e945d..61a89bc6ad 100644 --- a/spec/features/consumer/shopping/checkout_spec.rb +++ b/spec/features/consumer/shopping/checkout_spec.rb @@ -180,46 +180,6 @@ feature "As a consumer I want to check out my cart", js: true do end end - context "in the shopfront with cache enabled" do - around do |example| - original_config = Spree::Config[:enable_products_cache?] - example.run - Spree::Config[:enable_products_cache?] = original_config - end - - let(:control_product) { create(:taxed_product, supplier: supplier, price: 110, zone: zone, tax_rate_amount: 0.1) } - let(:control_variant) { control_product.variants.first } - let!(:order_cycle) { create(:simple_order_cycle, suppliers: [supplier], distributors: [distributor], coordinator: create(:distributor_enterprise), variants: [variant, control_variant]) } - - it "does not show item after all stock of an item is checked out (tesging cache update on checkout)" do - Spree::Config[:enable_products_cache?] = true - variant.update_attributes on_hand: 5 - flush_jobs - visit shop_path - - fill_in "variants[#{variant.id}]", with: '5' - wait_until { !cart_dirty } - - visit checkout_path - checkout_as_guest - - fill_out_details - fill_out_billing_address - choose free_shipping.name - choose check_without_fee.name - - place_order - expect(page).to have_content "Your order has been processed successfully" - - flush_jobs - visit shop_path - # The presence of the control product ensures the list of products is fully loaded - # before we verify the sold product is not present - expect(page).to have_content control_product.name - expect(page).not_to have_content product.name - end - end - context "on the checkout page" do before do visit checkout_path diff --git a/spec/features/consumer/shopping/products_spec.rb b/spec/features/consumer/shopping/products_spec.rb index 6282d0ba31..726346a9ed 100644 --- a/spec/features/consumer/shopping/products_spec.rb +++ b/spec/features/consumer/shopping/products_spec.rb @@ -32,7 +32,9 @@ feature "As a consumer I want to view products", js: true do visit shop_path select "monday", from: "order_cycle_id" + expect(page).to have_content product.name click_link product.name + expect(page).to have_selector '.reveal-modal' modal_should_be_open_for product @@ -48,7 +50,9 @@ feature "As a consumer I want to view products", js: true do visit shop_path select "monday", from: "order_cycle_id" + expect(page).to have_content product.name click_link product.name + expect(page).to have_selector '.reveal-modal' modal_should_be_open_for product diff --git a/spec/features/consumer/shopping/shopping_spec.rb b/spec/features/consumer/shopping/shopping_spec.rb index 2809d8f187..989eea5e4e 100644 --- a/spec/features/consumer/shopping/shopping_spec.rb +++ b/spec/features/consumer/shopping/shopping_spec.rb @@ -214,10 +214,6 @@ feature "As a consumer I want to shop with a distributor", js: true do fill_in "search", with: "Meer" # For product named "Meercats" expect(page).to have_content product2.name expect(page).not_to have_content product.name - - fill_in "search", with: "Dome" # For product with meta_keywords "Domestic" - expect(page).to have_content product.name - expect(page).not_to have_content product2.name end it "returns search results for products where the search term matches one of the product's variant names" do diff --git a/spec/javascripts/unit/darkswarm/controllers/products_controller_spec.js.coffee b/spec/javascripts/unit/darkswarm/controllers/products_controller_spec.js.coffee index 1e188f4f16..3a7dd8ba1b 100644 --- a/spec/javascripts/unit/darkswarm/controllers/products_controller_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/controllers/products_controller_spec.js.coffee @@ -27,13 +27,6 @@ describe 'ProductsCtrl', -> it 'fetches products from Products', -> expect(scope.Products.products).toEqual ['testy mctest'] - it "increments the limit up to the number of products", -> - scope.limit = 0 - scope.incrementLimit() - expect(scope.limit).toEqual 10 - scope.incrementLimit() - expect(scope.limit).toEqual 10 - it "blocks keypresses on code 13", -> event = keyCode: 13 diff --git a/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee index 0075750fba..d1b9f38bb6 100644 --- a/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee @@ -1,6 +1,7 @@ describe 'Products service', -> $httpBackend = null Products = null + OrderCycle = {} Shopfront = null Variants = null Cart = null @@ -11,6 +12,7 @@ describe 'Products service', -> properties = null taxons = null Geo = {} + endpoint = "/api/order_cycles/1/products?distributor=1" beforeEach -> product = @@ -38,6 +40,9 @@ describe 'Products service', -> producers: id: 9, name: "Test" + OrderCycle = + order_cycle: + order_cycle_id: 1 module 'Darkswarm' module ($provide)-> @@ -46,6 +51,7 @@ describe 'Products service', -> $provide.value "taxons", taxons $provide.value "properties", properties $provide.value "Geo", Geo + $provide.value "OrderCycle", OrderCycle null inject ($injector, _$httpBackend_)-> @@ -57,60 +63,60 @@ describe 'Products service', -> $httpBackend = _$httpBackend_ it "Fetches products from the backend on init", -> - $httpBackend.expectGET("/shop/products").respond([product]) + $httpBackend.expectGET(endpoint).respond([product]) $httpBackend.flush() expect(Products.products[0].test).toEqual "cats" it "dereferences suppliers", -> Shopfront.producers_by_id = {id: 9, name: "test"} - $httpBackend.expectGET("/shop/products").respond([{supplier : {id: 9}, master: {}}]) + $httpBackend.expectGET(endpoint).respond([{supplier : {id: 9}, master: {}}]) $httpBackend.flush() expect(Products.products[0].supplier).toBe Shopfront.producers_by_id["9"] it "dereferences taxons", -> product.taxons = [2] - $httpBackend.expectGET("/shop/products").respond([product]) + $httpBackend.expectGET(endpoint).respond([product]) $httpBackend.flush() expect(Products.products[0].taxons[1]).toBe taxons[0] it "dereferences properties", -> product.properties_with_values = [1] - $httpBackend.expectGET("/shop/products").respond([product]) + $httpBackend.expectGET(endpoint).respond([product]) $httpBackend.flush() expect(Products.products[0].properties[1]).toBe properties[0] it "registers variants with Variants service", -> product.variants = [{id: 1}] - $httpBackend.expectGET("/shop/products").respond([product]) + $httpBackend.expectGET(endpoint).respond([product]) $httpBackend.flush() expect(Products.products[0].variants[0]).toBe Variants.variants[1] it "stores variant names", -> product.variants = [{id: 1, name_to_display: "one"}, {id: 2, name_to_display: "two"}] - $httpBackend.expectGET("/shop/products").respond([product]) + $httpBackend.expectGET(endpoint).respond([product]) $httpBackend.flush() expect(Products.products[0].variant_names).toEqual "one two " it "sets primaryImageOrMissing when no images are provided", -> - $httpBackend.expectGET("/shop/products").respond([product]) + $httpBackend.expectGET(endpoint).respond([product]) $httpBackend.flush() expect(Products.products[0].primaryImage).toBeUndefined() expect(Products.products[0].primaryImageOrMissing).toEqual "/assets/noimage/small.png" it "sets largeImage", -> - $httpBackend.expectGET("/shop/products").respond([productWithImage]) + $httpBackend.expectGET(endpoint).respond([productWithImage]) $httpBackend.flush() expect(Products.products[0].largeImage).toEqual("foo.png") describe "determining the price to display for a product", -> it "displays the product price when the product does not have variants", -> - $httpBackend.expectGET("/shop/products").respond([product]) + $httpBackend.expectGET(endpoint).respond([product]) $httpBackend.flush() expect(Products.products[0].price).toEqual 11.00 it "displays the minimum variant price when the product has variants", -> product.variants = [{price: 22}, {price: 33}] - $httpBackend.expectGET("/shop/products").respond([product]) + $httpBackend.expectGET(endpoint).respond([product]) $httpBackend.flush() expect(Products.products[0].price).toEqual 22 diff --git a/spec/jobs/products_cache_integrity_checker_job_spec.rb b/spec/jobs/products_cache_integrity_checker_job_spec.rb deleted file mode 100644 index 19d5a79953..0000000000 --- a/spec/jobs/products_cache_integrity_checker_job_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -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 } - let(:cache_key) { "products-json-#{distributor.id}-#{order_cycle.id}" } - - before do - Rails.cache.write(cache_key, "[1, 2, 3]\n") - allow(OpenFoodNetwork::ProductsRenderer).to receive(: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.delete(cache_key) - 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 deleted file mode 100644 index c2dd66ff06..0000000000 --- a/spec/jobs/refresh_products_cache_job_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -require 'spec_helper' -require 'open_food_network/products_renderer' - -describe RefreshProductsCacheJob do - let(:distributor) { create(:distributor_enterprise) } - let(:order_cycle) { create(:simple_order_cycle) } - - context 'when the enterprise and the order cycle exist' do - before do - refresh_products_cache_job = instance_double(OpenFoodNetwork::ProductsRenderer, products_json: 'products') - allow(OpenFoodNetwork::ProductsRenderer).to receive(:new).with(distributor, order_cycle) { refresh_products_cache_job } - end - - it 'renders products and writes them to cache' do - run_job RefreshProductsCacheJob.new distributor.id, order_cycle.id - expect(Rails.cache.read("products-json-#{distributor.id}-#{order_cycle.id}")).to eq 'products' - end - end - - context 'when the order cycle does not exist' do - before do - allow(OrderCycle) - .to receive(:find) - .with(order_cycle.id) - .and_raise(ActiveRecord::RecordNotFound) - end - - it 'does not raise' do - expect { - run_job RefreshProductsCacheJob.new(distributor.id, order_cycle.id) - }.not_to raise_error - end - - it 'returns true' do - refresh_products_cache_job = RefreshProductsCacheJob.new(distributor.id, order_cycle.id) - expect(refresh_products_cache_job.perform).to eq(true) - end - end - - describe "fetching products JSON" do - let(:job) { RefreshProductsCacheJob.new(distributor.id, order_cycle.id) } - let(:products_renderer) { instance_double(OpenFoodNetwork::ProductsRenderer, products_json: nil) } - - before do - allow(OpenFoodNetwork::ProductsRenderer).to receive(:new).with(distributor, order_cycle) { products_renderer } - end - - it "fetches products JSON" do - job.perform - expect(OpenFoodNetwork::ProductsRenderer).to have_received(:new).with(distributor, order_cycle) { products_renderer } - 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 deleted file mode 100644 index 82d9302016..0000000000 --- a/spec/lib/open_food_network/cached_products_renderer_spec.rb +++ /dev/null @@ -1,134 +0,0 @@ -require 'spec_helper' -require 'open_food_network/cached_products_renderer' - -module OpenFoodNetwork - describe CachedProductsRenderer do - let(:distributor) { double(:distributor, id: 123) } - let(:order_cycle) { double(:order_cycle, id: 456) } - let(:cached_products_renderer) { CachedProductsRenderer.new(distributor, order_cycle) } - - # keeps global state unchanged - around do |example| - original_config = Spree::Config[:enable_products_cache?] - example.run - Spree::Config[:enable_products_cache?] = original_config - end - - describe "#products_json" do - let(:products_renderer) do - double(ProductsRenderer, products_json: 'uncached products') - end - - before do - allow(ProductsRenderer) - .to receive(:new) - .with(distributor, order_cycle) { products_renderer } - end - - context "products cache toggle" do - before do - Rails.cache.write "products-json-#{distributor.id}-#{order_cycle.id}", 'products' - end - - context "disabled" do - before do - Spree::Config[:enable_products_cache?] = false - end - - it "returns uncached products JSON" do - expect(cached_products_renderer.products_json).to eq 'uncached products' - end - end - - context "enabled" do - before do - Spree::Config[:enable_products_cache?] = true - end - - it "returns the cached JSON" do - expect(cached_products_renderer.products_json).to eq 'products' - end - end - end - - context "products cache enabled" do - before do - Spree::Config[:enable_products_cache?] = true - end - - describe "when the distribution is not set" do - let(:cached_products_renderer) { CachedProductsRenderer.new(nil, nil) } - - it "raises an exception and returns no products" do - expect { cached_products_renderer.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(cached_products_renderer.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 { cached_products_renderer.products_json }.to raise_error CachedProductsRenderer::NoProducts - end - end - - describe "when the products JSON is not cached" do - let(:cache_key) { "products-json-#{distributor.id}-#{order_cycle.id}" } - let(:cached_json) { Rails.cache.read(cache_key) } - let(:cache_present) { Rails.cache.exist?(cache_key) } - let(:products_renderer) do - double(ProductsRenderer, products_json: 'fresh products') - end - - before do - Rails.cache.delete(cache_key) - - allow(ProductsRenderer) - .to receive(:new) - .with(distributor, order_cycle) { products_renderer } - end - - describe "when there are products" do - it "returns products as JSON" do - expect(cached_products_renderer.products_json).to eq 'fresh products' - end - - it "caches the JSON" do - cached_products_renderer.products_json - expect(cached_json).to eq 'fresh products' - end - end - - describe "when there are no products" do - let(:products_renderer) { double(ProductsRenderer) } - - before do - allow(products_renderer).to receive(:products_json).and_raise ProductsRenderer::NoProducts - - allow(ProductsRenderer) - .to receive(:new) - .with(distributor, order_cycle) { products_renderer } - end - - it "raises an error" do - expect { cached_products_renderer.products_json }.to raise_error CachedProductsRenderer::NoProducts - end - - it "caches the products as nil" do - expect { cached_products_renderer.products_json }.to raise_error CachedProductsRenderer::NoProducts - expect(cache_present).to be - expect(cached_json).to be_nil - end - end - end - 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 deleted file mode 100644 index 74998928d5..0000000000 --- a/spec/lib/open_food_network/products_cache_refreshment_spec.rb +++ /dev/null @@ -1,69 +0,0 @@ -require 'spec_helper' -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 deleted file mode 100644 index 47d18fa8ba..0000000000 --- a/spec/lib/open_food_network/products_cache_spec.rb +++ /dev/null @@ -1,431 +0,0 @@ -require 'open_food_network/products_cache' -require 'spec_helper' - -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 product is deleted" do - let(:product) { create(:simple_product) } - let(:variant) { create(:variant, product: product) } - 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) - product.destroy - end - - it "performs the cache refresh after the product has been removed from the order cycle" do - expect(ProductsCache).to receive(:refresh_cache).with(distributor, oc) do - expect(product.reload.deleted_at).not_to be_nil - end - - product.destroy - 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) } - 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 - v_deleted.deleted_at = Time.now - v_deleted.save - 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 deleted file mode 100644 index 252f26f517..0000000000 --- a/spec/lib/open_food_network/products_renderer_spec.rb +++ /dev/null @@ -1,119 +0,0 @@ -require 'spec_helper' -require 'open_food_network/products_renderer' - -module OpenFoodNetwork - describe ProductsRenderer do - 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) } - let(:t2) { create(:taxon) } - let!(:p1) { create(:product, name: "abc", primary_taxon_id: t2.id) } - let!(:p2) { create(:product, name: "def", primary_taxon_id: t1.id) } - let!(:p3) { create(:product, name: "ghi", primary_taxon_id: t2.id) } - let!(:p4) { create(:product, name: "jkl", primary_taxon_id: t1.id) } - - before do - exchange.variants << p1.variants.first - exchange.variants << p2.variants.first - exchange.variants << p3.variants.first - exchange.variants << p4.variants.first - end - - it "sorts products by the distributor's preferred taxon list" do - allow(distributor).to receive(:preferred_shopfront_taxon_order) { "#{t1.id},#{t2.id}" } - products = pr.send(:products) - expect(products).to eq([p2, p4, p1, p3]) - end - - it "alphabetizes products by name when taxon list is not set" do - allow(distributor).to receive(:preferred_shopfront_taxon_order) { "" } - products = pr.send(:products) - expect(products).to eq([p1, p2, p3, p4]) - end - end - - context "JSON tests" do - let(:product) { create(:product) } - let(:variant) { product.variants.first } - - before do - exchange.variants << variant - end - - it "only returns products for the current order cycle" do - expect(pr.products_json).to include product.name - end - - it "doesn't return products not in stock" do - variant.update_attribute(:on_demand, false) - variant.update_attribute(:on_hand, 0) - expect(pr.products_json).not_to include product.name - end - - it "strips html from description" do - product.update_attribute(:description, "turtles frogs") - json = pr.products_json - expect(json).to include "frogs" - expect(json).not_to include "turtles frogs") + json = products_renderer.products_json + expect(json).to include "frogs" + expect(json).not_to include "