diff --git a/app/controllers/shops_controller.rb b/app/controllers/shops_controller.rb index aa0d2f8f33..8a657990d8 100644 --- a/app/controllers/shops_controller.rb +++ b/app/controllers/shops_controller.rb @@ -4,5 +4,11 @@ class ShopsController < BaseController before_filter :enable_embedded_shopfront def index + @enterprises = Enterprise + .activated + .includes(address: :state) + .includes(:properties) + .includes(supplied_products: :properties) + .all end end diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index f4e857607e..dfcf2bb6ff 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -1,8 +1,13 @@ require 'open_food_network/enterprise_injection_data' module InjectionHelper - def inject_enterprises - inject_json_ams "enterprises", Enterprise.activated.includes(address: :state).all, Api::EnterpriseSerializer, enterprise_injection_data + def inject_enterprises(enterprises = Enterprise.activated.includes(address: :state).all) + inject_json_ams( + 'enterprises', + enterprises, + Api::EnterpriseSerializer, + enterprise_injection_data + ) end def inject_enterprise_and_relatives diff --git a/app/models/spree/property_decorator.rb b/app/models/spree/property_decorator.rb index 7349436a2b..5950212149 100644 --- a/app/models/spree/property_decorator.rb +++ b/app/models/spree/property_decorator.rb @@ -26,6 +26,9 @@ module Spree # 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 diff --git a/app/serializers/api/cached_enterprise_serializer.rb b/app/serializers/api/cached_enterprise_serializer.rb new file mode 100644 index 0000000000..4070cfa7d5 --- /dev/null +++ b/app/serializers/api/cached_enterprise_serializer.rb @@ -0,0 +1,161 @@ +require 'open_food_network/property_merge' + +module Api + class CachedEnterpriseSerializer < ActiveModel::Serializer + include SerializerHelper + + cached + + def cache_key + object.andand.cache_key + end + + attributes :name, :id, :description, :latitude, :longitude, + :long_description, :website, :instagram, :linkedin, :twitter, + :facebook, :is_primary_producer, :is_distributor, :phone, :visible, + :email_address, :hash, :logo, :promo_image, :path, :pickup, :delivery, + :icon, :icon_font, :producer_icon_font, :category, :producers, :hubs + + attributes :taxons, :supplied_taxons + + has_one :address, serializer: AddressSerializer + + has_many :supplied_properties, serializer: PropertySerializer + has_many :distributed_properties, serializer: PropertySerializer + + def pickup + services = data.shipping_method_services[object.id] + services ? services[:pickup] : false + end + + def delivery + services = data.shipping_method_services[object.id] + services ? services[:delivery] : false + end + + def email_address + object.email_address.to_s.reverse + end + + def hash + object.to_param + end + + def logo + object.logo(:medium) if object.logo? + end + + def promo_image + object.promo_image(:large) if object.promo_image? + end + + def path + enterprise_shop_path(object) + end + + def producers + relatives = data.relatives[object.id] + ids_to_objs(relatives.andand[:producers]) + end + + def hubs + relatives = data.relatives[object.id] + ids_to_objs(relatives.andand[:distributors]) + end + + def taxons + if active + ids_to_objs data.current_distributed_taxons[object.id] + else + ids_to_objs data.all_distributed_taxons[object.id] + end + end + + def supplied_taxons + ids_to_objs data.supplied_taxons[object.id] + end + + def supplied_properties + (product_properties + producer_properties).uniq do |property_object| + property_object.property.presentation + end + end + + def distributed_properties + # This results in 3 queries per enterprise + + if active + product_properties = Spree::Property.currently_sold_by(object) + producer_property_ids = ProducerProperty.currently_sold_by(object).pluck(:property_id) + + else + product_properties = Spree::Property.ever_sold_by(object) + producer_property_ids = ProducerProperty.ever_sold_by(object).pluck(:property_id) + end + + producer_properties = Spree::Property.where(id: producer_property_ids) + + OpenFoodNetwork::PropertyMerge.merge product_properties, producer_properties + end + + def active + data.active_distributors.andand.include? object + end + + # Map svg icons. + def icon + icons = { + hub: "/assets/map_005-hub.svg", + hub_profile: "/assets/map_006-hub-profile.svg", + producer_hub: "/assets/map_005-hub.svg", + producer_shop: "/assets/map_003-producer-shop.svg", + producer: "/assets/map_001-producer-only.svg", + } + icons[object.category] + end + + # Choose regular icon font for enterprises. + def icon_font + icon_fonts = { + hub: "ofn-i_063-hub", + hub_profile: "ofn-i_064-hub-reversed", + producer_hub: "ofn-i_063-hub", + producer_shop: "ofn-i_059-producer", + producer: "ofn-i_059-producer", + } + icon_fonts[object.category] + end + + # Choose producer page icon font - yes, sadly its got to be different. + # This duplicates some code but covers the producer page edge case where + # producer-hub has a producer icon without needing to duplicate the category logic in angular. + def producer_icon_font + icon_fonts = { + hub: "", + hub_profile: "", + producer_hub: "ofn-i_059-producer", + producer_shop: "ofn-i_059-producer", + producer: "ofn-i_059-producer", + } + icon_fonts[object.category] + end + + private + + def product_properties + enterprise.supplied_products.flat_map(&:properties) + end + + def producer_properties + enterprise.properties + end + + def enterprise + object + end + + def data + options[:data] + end + end +end diff --git a/app/serializers/api/enterprise_serializer.rb b/app/serializers/api/enterprise_serializer.rb index e3829174a2..fa0b09762a 100644 --- a/app/serializers/api/enterprise_serializer.rb +++ b/app/serializers/api/enterprise_serializer.rb @@ -18,161 +18,3 @@ class Api::EnterpriseSerializer < ActiveModel::Serializer Api::UncachedEnterpriseSerializer.new(object, @options).serializable_hash || {} end end - -class Api::UncachedEnterpriseSerializer < ActiveModel::Serializer - include SerializerHelper - - attributes :orders_close_at, :active - - def orders_close_at - options[:data].earliest_closing_times[object.id] - end - - def active - options[:data].active_distributors.andand.include? object - end -end - -class Api::CachedEnterpriseSerializer < ActiveModel::Serializer - include SerializerHelper - - cached - #delegate :cache_key, to: :object - - def cache_key - object.andand.cache_key - end - - - attributes :name, :id, :description, :latitude, :longitude, - :long_description, :website, :instagram, :linkedin, :twitter, - :facebook, :is_primary_producer, :is_distributor, :phone, :visible, - :email_address, :hash, :logo, :promo_image, :path, :pickup, :delivery, - :icon, :icon_font, :producer_icon_font, :category, :producers, :hubs - - attributes :taxons, :supplied_taxons - - has_one :address, serializer: Api::AddressSerializer - - has_many :supplied_properties, serializer: Api::PropertySerializer - has_many :distributed_properties, serializer: Api::PropertySerializer - - def pickup - services = options[:data].shipping_method_services[object.id] - services ? services[:pickup] : false - end - - def delivery - services = options[:data].shipping_method_services[object.id] - services ? services[:delivery] : false - end - - def email_address - object.email_address.to_s.reverse - end - - def hash - object.to_param - end - - def logo - object.logo(:medium) if object.logo? - end - - def promo_image - object.promo_image(:large) if object.promo_image? - end - - def path - enterprise_shop_path(object) - end - - def producers - relatives = options[:data].relatives[object.id] - ids_to_objs(relatives.andand[:producers]) - end - - def hubs - relatives = options[:data].relatives[object.id] - ids_to_objs(relatives.andand[:distributors]) - end - - def taxons - if active - ids_to_objs options[:data].current_distributed_taxons[object.id] - else - ids_to_objs options[:data].all_distributed_taxons[object.id] - end - end - - def supplied_taxons - ids_to_objs options[:data].supplied_taxons[object.id] - end - - def supplied_properties - # This results in 3 queries per enterprise - product_properties = Spree::Property.applied_by(object) - producer_properties = object.properties - - OpenFoodNetwork::PropertyMerge.merge product_properties, producer_properties - end - - def distributed_properties - # This results in 3 queries per enterprise - - if active - product_properties = Spree::Property.currently_sold_by(object) - producer_property_ids = ProducerProperty.currently_sold_by(object).pluck(:property_id) - - else - product_properties = Spree::Property.ever_sold_by(object) - producer_property_ids = ProducerProperty.ever_sold_by(object).pluck(:property_id) - end - - producer_properties = Spree::Property.where(id: producer_property_ids) - - OpenFoodNetwork::PropertyMerge.merge product_properties, producer_properties - end - - def active - options[:data].active_distributors.andand.include? object - end - - # Map svg icons. - def icon - icons = { - :hub => "/assets/map_005-hub.svg", - :hub_profile => "/assets/map_006-hub-profile.svg", - :producer_hub => "/assets/map_005-hub.svg", - :producer_shop => "/assets/map_003-producer-shop.svg", - :producer => "/assets/map_001-producer-only.svg", - } - icons[object.category] - end - - # Choose regular icon font for enterprises. - def icon_font - icon_fonts = { - :hub => "ofn-i_063-hub", - :hub_profile => "ofn-i_064-hub-reversed", - :producer_hub => "ofn-i_063-hub", - :producer_shop => "ofn-i_059-producer", - :producer => "ofn-i_059-producer", - } - icon_fonts[object.category] - end - - # Choose producer page icon font - yes, sadly its got to be different. - # This duplicates some code but covers the producer page edge case where - # producer-hub has a producer icon without needing to duplicate the category logic in angular. - def producer_icon_font - icon_fonts = { - :hub => "", - :hub_profile => "", - :producer_hub => "ofn-i_059-producer", - :producer_shop => "ofn-i_059-producer", - :producer => "ofn-i_059-producer", - } - icon_fonts[object.category] - end -end diff --git a/app/serializers/api/uncached_enterprise_serializer.rb b/app/serializers/api/uncached_enterprise_serializer.rb new file mode 100644 index 0000000000..bad8808533 --- /dev/null +++ b/app/serializers/api/uncached_enterprise_serializer.rb @@ -0,0 +1,15 @@ +module Api + class UncachedEnterpriseSerializer < ActiveModel::Serializer + include SerializerHelper + + attributes :orders_close_at, :active + + def orders_close_at + options[:data].earliest_closing_times[object.id] + end + + def active + options[:data].active_distributors.andand.include? object + end + end +end diff --git a/app/views/shops/_hubs.html.haml b/app/views/shops/_hubs.html.haml index 3e477b9551..aa41f3f229 100644 --- a/app/views/shops/_hubs.html.haml +++ b/app/views/shops/_hubs.html.haml @@ -1,4 +1,4 @@ -= inject_enterprises += inject_enterprises(@enterprises) #hubs.hubs{"ng-controller" => "EnterprisesCtrl", "ng-cloak" => true} .row diff --git a/lib/open_food_network/property_merge.rb b/lib/open_food_network/property_merge.rb index 9c21fb7b0d..82bf92eaee 100644 --- a/lib/open_food_network/property_merge.rb +++ b/lib/open_food_network/property_merge.rb @@ -1,18 +1,9 @@ module OpenFoodNetwork class PropertyMerge def self.merge(primary, secondary) - primary + secondary.reject do |secondary_p| - primary.any? do |primary_p| - property_of(primary_p).presentation == property_of(secondary_p).presentation - end + (primary + secondary).uniq do |property_object| + property_object.property.presentation end end - - - private - - def self.property_of(p) - p.respond_to?(:property) ? p.property : p - end end end diff --git a/spec/controllers/shops_controller_spec.rb b/spec/controllers/shops_controller_spec.rb index 400fa2f563..44f896d5f3 100644 --- a/spec/controllers/shops_controller_spec.rb +++ b/spec/controllers/shops_controller_spec.rb @@ -2,16 +2,17 @@ require 'spec_helper' describe ShopsController, type: :controller do render_views + let!(:distributor) { create(:distributor_enterprise) } let!(:invisible_distributor) { create(:distributor_enterprise, visible: false) } before do - Enterprise.stub_chain("distributors_with_active_order_cycles.ready_for_checkout") { [distributor] } + allow(Enterprise).to receive_message_chain(:distributors_with_active_order_cycles, :ready_for_checkout) { [distributor] } end # Exclusion from actual rendered view handled in features/consumer/home it "shows JSON for invisible hubs" do get :index - response.body.should have_content invisible_distributor.name + expect(response.body).to have_content(invisible_distributor.name) end end diff --git a/spec/lib/open_food_network/property_merge_spec.rb b/spec/lib/open_food_network/property_merge_spec.rb index 85f4200ab0..37568e706c 100644 --- a/spec/lib/open_food_network/property_merge_spec.rb +++ b/spec/lib/open_food_network/property_merge_spec.rb @@ -2,23 +2,31 @@ require 'spec_helper' module OpenFoodNetwork describe PropertyMerge do - let(:p1a) { create(:property, presentation: 'One') } - let(:p1b) { create(:property, presentation: 'One') } - let(:p2) { create(:property, presentation: 'Two') } + let(:property) { create(:property, presentation: 'One') } + let(:duplicate_property) { create(:property, presentation: 'One') } + let(:different_property) { create(:property, presentation: 'Two') } describe "merging Spree::Properties" do it "merges properties" do - expect(PropertyMerge.merge([p1a], [p1b, p2])).to eq [p1a, p2] + merge = PropertyMerge.merge( + [property], + [duplicate_property, different_property] + ) + expect(merge).to eq [property, different_property] end end describe "merging ProducerProperties and Spree::ProductProperties" do - let(:pp1a) { create(:product_property, property: p1a) } - let(:pp1b) { create(:producer_property, property: p1b) } - let(:pp2) { create(:producer_property, property: p2) } + let(:product_property) { create(:product_property, property: property) } + let(:duplicate_product_property) { create(:producer_property, property: duplicate_property) } + let(:producer_property) { create(:producer_property, property: different_property) } it "merges properties" do - expect(PropertyMerge.merge([pp1a], [pp1b, pp2])).to eq [pp1a, pp2] + merge = PropertyMerge.merge( + [product_property], + [duplicate_product_property, producer_property] + ) + expect(merge).to eq [product_property, producer_property] end end end diff --git a/spec/serializers/api/cached_enterprise_serializer_spec.rb b/spec/serializers/api/cached_enterprise_serializer_spec.rb new file mode 100644 index 0000000000..bea4b79bca --- /dev/null +++ b/spec/serializers/api/cached_enterprise_serializer_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Api::CachedEnterpriseSerializer do + let(:cached_enterprise_serializer) { described_class.new(enterprise) } + let(:enterprise) { create(:enterprise) } + + describe '#supplied_properties' do + let(:property) { create(:property, presentation: 'One') } + let(:duplicate_property) { create(:property, presentation: 'One') } + let(:different_property) { create(:property, presentation: 'Two') } + + let(:enterprise) do + create(:enterprise, properties: [duplicate_property, different_property]) + end + + before do + product = create(:product, properties: [property]) + enterprise.supplied_products << product + end + + it "removes duplicate product and producer properties" do + properties = cached_enterprise_serializer.supplied_properties + expect(properties).to eq([property, different_property]) + end + end +end