From e329b4bfb03a281fbd4062c73bd6cfaa9ed6887c Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Tue, 7 May 2019 12:23:52 +0100 Subject: [PATCH 01/16] Improve queries for serializers --- app/helpers/injection_helper.rb | 6 +++++- app/helpers/serializer_helper.rb | 9 +++++++++ .../enterprise_shopfront_list_serializer.rb | 3 ++- spec/helpers/serializer_helper_spec.rb | 18 ++++++++++++++++++ 4 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 spec/helpers/serializer_helper_spec.rb diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index d3e3989d95..87f3efe1d7 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -1,6 +1,8 @@ require 'open_food_network/enterprise_injection_data' module InjectionHelper + include SerializerHelper + def inject_enterprises(enterprises = Enterprise.activated.includes(address: :state).all) inject_json_ams( 'enterprises', @@ -11,9 +13,11 @@ module InjectionHelper end def inject_enterprise_shopfront_list + select_only = required_attributes Enterprise, Api::EnterpriseShopfrontListSerializer + inject_json_ams( 'enterprises', - Enterprise.activated.includes(address: :state).all, + Enterprise.activated.select(select_only).includes(address: :state).all, Api::EnterpriseShopfrontListSerializer ) end diff --git a/app/helpers/serializer_helper.rb b/app/helpers/serializer_helper.rb index ab39e8c252..539e826bfc 100644 --- a/app/helpers/serializer_helper.rb +++ b/app/helpers/serializer_helper.rb @@ -3,4 +3,13 @@ module SerializerHelper return [] if ids.blank? ids.map { |id| { id: id } } end + + # Returns an array of the fields a serializer needs from it's object + # so we can #select only what the serializer will actually use + def required_attributes(model, serializer) + model_attributes = model.attribute_names + serializer_attributes = serializer._attributes.keys.map(&:to_s) + + (serializer_attributes & model_attributes).map { |attr| "#{model.table_name}.#{attr}" } + end end diff --git a/app/serializers/api/enterprise_shopfront_list_serializer.rb b/app/serializers/api/enterprise_shopfront_list_serializer.rb index 30b5ed3590..a8ded21bf1 100644 --- a/app/serializers/api/enterprise_shopfront_list_serializer.rb +++ b/app/serializers/api/enterprise_shopfront_list_serializer.rb @@ -2,7 +2,8 @@ module Api class EnterpriseShopfrontListSerializer < ActiveModel::Serializer attributes :name, :id, :latitude, :longitude, :is_primary_producer, :is_distributor, - :visible, :path, :icon, :icon_font, :producer_icon_font + :visible, :path, :icon, :icon_font, :producer_icon_font, :address_id, :sells, + :permalink has_one :address, serializer: Api::AddressSerializer diff --git a/spec/helpers/serializer_helper_spec.rb b/spec/helpers/serializer_helper_spec.rb new file mode 100644 index 0000000000..2cc66073f3 --- /dev/null +++ b/spec/helpers/serializer_helper_spec.rb @@ -0,0 +1,18 @@ +require 'spec_helper' + +describe SerializerHelper, type: :helper do + let(:serializer) do + class ExampleEnterpriseSerializer < ActiveModel::Serializer + attributes :id, :name + end + ExampleEnterpriseSerializer + end + + describe "#required_attributes" do + it "returns only the attributes from the model that the serializer needs to be queried" do + required_attributes = helper.required_attributes Enterprise, serializer + + expect(required_attributes).to eq ['enterprises.id', 'enterprises.name'] + end + end +end From 45d65baf8e03ec6f50adc6fe520b622a46f4df9e Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 9 May 2019 15:06:43 +0100 Subject: [PATCH 02/16] Improve shops performance --- .../controllers/hub_node_controller.js.coffee | 36 ++++++++++++++++--- .../directives/enterprise_modal.js.coffee | 15 +++----- .../darkswarm/services/enterprises.js.coffee | 6 ++-- app/controllers/shops_controller.rb | 1 + .../api/cached_enterprise_serializer.rb | 12 +------ app/views/shops/_fat.html.haml | 10 ++++-- app/views/shops/_hubs.html.haml | 2 -- app/views/shops/index.html.haml | 2 ++ .../enterprise_injection_data.rb | 4 --- 9 files changed, 52 insertions(+), 36 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee index 922dc8abb4..e587dcfb62 100644 --- a/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee @@ -1,9 +1,37 @@ -Darkswarm.controller "HubNodeCtrl", ($scope, HashNavigation, Navigation, $location, $templateCache, CurrentHub) -> - $scope.toggle = (e) -> - HashNavigation.toggle $scope.hub.hash if !angular.element(e.target).inheritedData('is-link') +Darkswarm.controller "HubNodeCtrl", ($scope, HashNavigation, Navigation, $location, $templateCache, CurrentHub, $http) -> + $scope.shopfront_loading = false + $scope.enterprise_details = [] + # Toggles shopfront tabs open/closed. Fetches enterprise details from the api, diplays them and adds them + # to $scope.enterprise_details, or simply displays the details again if previously fetched + $scope.toggle = (event) -> + if $scope.open() + $scope.toggle_tab(event) + return + + if $scope.enterprise_details[$scope.hub.id] + $scope.hub = $scope.enterprise_details[$scope.hub.id] + $scope.toggle_tab(event) + return + + $scope.shopfront_loading = true + $scope.toggle_tab(event) + + $http.get("/api/enterprises/" + $scope.hub.id + "/shopfront") + .success (data) -> + $scope.shopfront_loading = false + $scope.hub = data + $scope.enterprise_details[$scope.hub.id] = $scope.hub + .error (data) -> + console.error(data) + + $scope.toggle_tab = (event) -> + HashNavigation.toggle $scope.hub.hash if !angular.element(event.target).inheritedData('is-link') + + # Returns boolean: pulldown tab is currently open/closed $scope.open = -> HashNavigation.active $scope.hub.hash - + + # Returns boolean: is this hub the hub that the user is currently "shopping" in? $scope.current = -> $scope.hub.id is CurrentHub.hub.id diff --git a/app/assets/javascripts/darkswarm/directives/enterprise_modal.js.coffee b/app/assets/javascripts/darkswarm/directives/enterprise_modal.js.coffee index 099f2a39e4..ad0d93a1b6 100644 --- a/app/assets/javascripts/darkswarm/directives/enterprise_modal.js.coffee +++ b/app/assets/javascripts/darkswarm/directives/enterprise_modal.js.coffee @@ -1,15 +1,10 @@ -Darkswarm.directive "enterpriseModal", ($modal, Enterprises, EnterpriseResource) -> +Darkswarm.directive "enterpriseModal", (EnterpriseModal) -> restrict: 'E' replace: true template: "" transclude: true link: (scope, elem, attrs, ctrl) -> - elem.on "click", (ev) => - ev.stopPropagation() - params = - id: scope.enterprise.id - EnterpriseResource.relatives params, (data) => - Enterprises.addEnterprises data - scope.enterprise = Enterprises.enterprises_by_id[scope.enterprise.id] - Enterprises.dereferenceEnterprise scope.enterprise - scope.modalInstance = $modal.open(controller: ctrl, templateUrl: 'enterprise_modal.html', scope: scope) + elem.on "click", (event) => + event.stopPropagation() + + scope.modalInstance = EnterpriseModal.open scope.enterprise \ No newline at end of file diff --git a/app/assets/javascripts/darkswarm/services/enterprises.js.coffee b/app/assets/javascripts/darkswarm/services/enterprises.js.coffee index 73dbd1627b..6d667e0001 100644 --- a/app/assets/javascripts/darkswarm/services/enterprises.js.coffee +++ b/app/assets/javascripts/darkswarm/services/enterprises.js.coffee @@ -1,14 +1,18 @@ Darkswarm.factory 'Enterprises', (enterprises, CurrentHub, Taxons, Dereferencer, visibleFilter, Matcher, Geo, $rootScope) -> new class Enterprises enterprises_by_id: {} + constructor: -> # Populate Enterprises.enterprises from json in page. @enterprises = enterprises + # Map enterprises to id/object pairs for lookup. for enterprise in enterprises @enterprises_by_id[enterprise.id] = enterprise + # Replace enterprise and taxons ids with actual objects. @dereferenceEnterprises() + @visible_enterprises = visibleFilter @enterprises @producers = @visible_enterprises.filter (enterprise)-> enterprise.category in ["producer_hub", "producer_shop", "producer"] @@ -22,8 +26,6 @@ Darkswarm.factory 'Enterprises', (enterprises, CurrentHub, Taxons, Dereferencer, @dereferenceEnterprise enterprise dereferenceEnterprise: (enterprise) -> - @dereferenceProperty(enterprise, 'hubs', @enterprises_by_id) - @dereferenceProperty(enterprise, 'producers', @enterprises_by_id) @dereferenceProperty(enterprise, 'taxons', Taxons.taxons_by_id) @dereferenceProperty(enterprise, 'supplied_taxons', Taxons.taxons_by_id) diff --git a/app/controllers/shops_controller.rb b/app/controllers/shops_controller.rb index 8a657990d8..25c5995170 100644 --- a/app/controllers/shops_controller.rb +++ b/app/controllers/shops_controller.rb @@ -6,6 +6,7 @@ class ShopsController < BaseController def index @enterprises = Enterprise .activated + .is_distributor .includes(address: :state) .includes(:properties) .includes(supplied_products: :properties) diff --git a/app/serializers/api/cached_enterprise_serializer.rb b/app/serializers/api/cached_enterprise_serializer.rb index ebb56dfb0e..4e421ff48e 100644 --- a/app/serializers/api/cached_enterprise_serializer.rb +++ b/app/serializers/api/cached_enterprise_serializer.rb @@ -14,7 +14,7 @@ module Api :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 + :icon, :icon_font, :producer_icon_font, :category attributes :taxons, :supplied_taxons @@ -53,16 +53,6 @@ module Api enterprise_shop_path(enterprise) end - def producers - relatives = data.relatives[enterprise.id] - ids_to_objs(relatives.andand[:producers]) - end - - def hubs - relatives = data.relatives[enterprise.id] - ids_to_objs(relatives.andand[:distributors]) - end - def taxons if active ids_to_objs data.current_distributed_taxons[enterprise.id] diff --git a/app/views/shops/_fat.html.haml b/app/views/shops/_fat.html.haml index 66752acefd..da9c579454 100644 --- a/app/views/shops/_fat.html.haml +++ b/app/views/shops/_fat.html.haml @@ -1,5 +1,9 @@ .row.active_table_row{"ng-show" => "open()", "ng-click" => "toggle($event)", "ng-class" => "{'open' : open()}"} - .columns.small-12.medium-6.large-5.fat + .columns.small-12.fat.text-center{"ng-show" => "open() && shopfront_loading"} + %p + %img.spinner.text-center{ src: "/assets/spinning-circles.svg" } + + .columns.small-12.medium-6.large-5.fat{"ng-show" => "open() && !shopfront_loading"} %div{"ng-if" => "::hub.taxons"} %label = t :hubs_buy @@ -13,7 +17,7 @@ %span{"ng-bind" => "property.presentation"} %div.show-for-medium-up{"ng-if" => "::hub.taxons.length==0"}   - .columns.small-12.medium-3.large-2.fat + .columns.small-12.medium-3.large-2.fat{"ng-show" => "open() && !shopfront_loading"} %div{"ng-if" => "::(hub.pickup || hub.delivery)"} %label = t :hubs_delivery_options @@ -24,7 +28,7 @@ %li.delivery{"ng-if" => "::hub.delivery"} %i.ofn-i_039-delivery = t :hubs_delivery - .columns.small-12.medium-3.large-5.fat + .columns.small-12.medium-3.large-5.fat{"ng-show" => "open() && !shopfront_loading"} %div{"ng-if" => "::hub.producers"} %label = t :hubs_producers diff --git a/app/views/shops/_hubs.html.haml b/app/views/shops/_hubs.html.haml index aa41f3f229..3d83d28923 100644 --- a/app/views/shops/_hubs.html.haml +++ b/app/views/shops/_hubs.html.haml @@ -1,5 +1,3 @@ -= inject_enterprises(@enterprises) - #hubs.hubs{"ng-controller" => "EnterprisesCtrl", "ng-cloak" => true} .row .small-12.columns diff --git a/app/views/shops/index.html.haml b/app/views/shops/index.html.haml index 7e288f4157..f38a1c6797 100644 --- a/app/views/shops/index.html.haml +++ b/app/views/shops/index.html.haml @@ -1,6 +1,8 @@ - content_for(:title) do = t :shops_title += inject_enterprises(@enterprises) + #panes #shops.pane .row diff --git a/lib/open_food_network/enterprise_injection_data.rb b/lib/open_food_network/enterprise_injection_data.rb index 3d15344472..3f23ff7a0a 100644 --- a/lib/open_food_network/enterprise_injection_data.rb +++ b/lib/open_food_network/enterprise_injection_data.rb @@ -12,10 +12,6 @@ module OpenFoodNetwork @shipping_method_services ||= Spree::ShippingMethod.services end - def relatives - @relatives ||= EnterpriseRelationship.relatives(true) - end - def supplied_taxons @supplied_taxons ||= Spree::Taxon.supplied_taxons end From ed97400a23738bb7f51b019f3c03f2975a9f861d Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 9 May 2019 20:27:41 +0100 Subject: [PATCH 03/16] Improve producers performance --- .../producer_node_controller.js.coffee | 32 +++++++++++++++++-- app/controllers/producers_controller.rb | 10 +++++- .../api/enterprise_thin_serializer.rb | 2 +- app/views/producers/_fat.html.haml | 13 +++++--- app/views/producers/_skinny.html.haml | 18 ++++++----- app/views/producers/index.html.haml | 2 +- 6 files changed, 58 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/producer_node_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/producer_node_controller.js.coffee index d46106a830..e8aeb58af2 100644 --- a/app/assets/javascripts/darkswarm/controllers/producer_node_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/producer_node_controller.js.coffee @@ -1,6 +1,32 @@ -Darkswarm.controller "ProducerNodeCtrl", ($scope, HashNavigation, $anchorScroll) -> - $scope.toggle = -> - HashNavigation.toggle $scope.producer.hash +Darkswarm.controller "ProducerNodeCtrl", ($scope, HashNavigation, $anchorScroll, $http) -> + $scope.shopfront_loading = false + $scope.enterprise_details = [] + + # Toggles shopfront tabs open/closed. Fetches enterprise details from the api, diplays them and adds them + # to $scope.enterprise_details, or simply displays the details again if previously fetched + $scope.toggle = (event) -> + if $scope.open() + $scope.toggle_tab(event) + return + + if $scope.enterprise_details[$scope.producer.id] + $scope.producer = $scope.enterprise_details[$scope.producer.id] + $scope.toggle_tab(event) + return + + $scope.shopfront_loading = true + $scope.toggle_tab(event) + + $http.get("/api/enterprises/" + $scope.producer.id + "/shopfront") + .success (data) -> + $scope.shopfront_loading = false + $scope.producer = data + $scope.enterprise_details[$scope.producer.id] = $scope.producer + .error (data) -> + console.error(data) + + $scope.toggle_tab = (event) -> + HashNavigation.toggle $scope.producer.hash if !angular.element(event.target).inheritedData('is-link') $scope.open = -> HashNavigation.active($scope.producer.hash) diff --git a/app/controllers/producers_controller.rb b/app/controllers/producers_controller.rb index efdd889aed..4dd7800003 100644 --- a/app/controllers/producers_controller.rb +++ b/app/controllers/producers_controller.rb @@ -3,5 +3,13 @@ class ProducersController < BaseController before_filter :enable_embedded_shopfront - def index; end + def index + @enterprises = Enterprise + .activated + .is_primary_producer + .includes(address: :state) + .includes(:properties) + .includes(supplied_products: :properties) + .all + end end diff --git a/app/serializers/api/enterprise_thin_serializer.rb b/app/serializers/api/enterprise_thin_serializer.rb index cdc4d6b9eb..7127ab12bd 100644 --- a/app/serializers/api/enterprise_thin_serializer.rb +++ b/app/serializers/api/enterprise_thin_serializer.rb @@ -1,6 +1,6 @@ module Api class EnterpriseThinSerializer < ActiveModel::Serializer - attributes :name, :id, :active, :path + attributes :name, :id, :active, :path, :visible has_one :address, serializer: Api::AddressSerializer diff --git a/app/views/producers/_fat.html.haml b/app/views/producers/_fat.html.haml index 190e244bb5..2dc7454671 100644 --- a/app/views/producers/_fat.html.haml +++ b/app/views/producers/_fat.html.haml @@ -1,6 +1,9 @@ .row.active_table_row{"ng-if" => "open()", "ng-click" => "toggle($event)", "ng-class" => "{'open' : open()}"} + .columns.small-12.fat.text-center{"ng-show" => "open() && shopfront_loading"} + %p + %img.spinner.text-center{ src: "/assets/spinning-circles.svg" } - .columns.small-12.medium-7.large-7.fat + .columns.small-12.medium-7.large-7.fat{"ng-show" => "open() && !shopfront_loading"} / Will add in long description available once clean up HTML formatting producer.long_description %div{"ng-if" => "::producer.description"} %label @@ -11,7 +14,7 @@ %label   %img.right.show-for-medium-up{"ng-src" => "{{::producer.logo}}" } - .columns.small-12.medium-5.large-5.fat + .columns.small-12.medium-5.large-5.fat{"ng-show" => "open() && !shopfront_loading"} %div{"ng-if" => "::producer.supplied_taxons"} %label @@ -64,8 +67,8 @@ %a{"ng-href" => "http://instagram.com/{{::producer.instagram}}", target: "_blank"} %i.ofn-i_043-instagram -.row.active_table_row.pad-top{"ng-if" => "open() && producer.hubs"} - .columns.small-12 +.row.active_table_row.pad-top{"ng-if" => "open() && producer.hubs && !shopfront_loading"} + .columns.small-12{"ng-if" => "producer.hubs.length > 0"} .row .columns.small-12.fat %div{"ng-if" => "::producer.name"} @@ -76,7 +79,7 @@ .row.cta-container .columns.small-12 %a.cta-hub{"ng-repeat" => "hub in producer.hubs | visible | orderBy:'-active'", - "ng-href" => "{{::hub.path}}", "ng-attr-target" => "{{ embedded_layout ? '_blank' : undefined }}", "ofn-change-hub" => "hub", + "ng-href" => "{{::hub.path}}", "ng-attr-target" => "{{ embedded_layout ? '_blank' : undefined }}", "ng-class" => "::{primary: hub.active, secondary: !hub.active}"} %i.ofn-i_068-shop-reversed{"ng-if" => "::hub.active"} %i.ofn-i_068-shop-reversed{"ng-if" => "::!hub.active"} diff --git a/app/views/producers/_skinny.html.haml b/app/views/producers/_skinny.html.haml index 9771576842..6e05619d07 100644 --- a/app/views/producers/_skinny.html.haml +++ b/app/views/producers/_skinny.html.haml @@ -1,24 +1,26 @@ .row.active_table_row{"ng-click" => "toggle($event)", "ng-class" => "{'closed' : !open(), 'is_distributor' : producer.is_distributor}"} - .columns.small-12.medium-8.large-8.skinny-head + .columns.small-12.medium-7.large-7.skinny-head %span{"ng-if" => "::producer.is_distributor" } - %a.is_distributor{"ng-href" => "{{::producer.path}}", "ng-attr-target" => "{{ embedded_layout ? '_blank' : undefined}}"} - .row.vertical-align-middle - .columns.small-2.medium-2.large-2 + .row.vertical-align-middle + .columns.small-12 + %a.is_distributor{"ng-href" => "{{::producer.path}}", "ng-attr-target" => "{{ embedded_layout ? '_blank' : undefined}}", "data-is-link" => "true"} %i{ng: {class: "::producer.producer_icon_font"}} - .columns.small-10.medium-10.large-10 %span.margin-top %strong{"ng-bind" => "::producer.name"} + %span.producer-name{"ng-if" => "::!producer.is_distributor" } .row.vertical-align-middle - .columns.small-2.medium-2.large-2 + .columns.small-12 %i{ng: {class: "::producer.producer_icon_font"}} - .columns.small-10.medium-10.large-10 %span.margin-top %strong{"ng-bind" => "::producer.name"} - .columns.small-6.medium-2.large-2 + + .columns.small-6.medium-3.large-3 %span.margin-top{"ng-bind" => "::producer.address.city"} + .columns.small-4.medium-1.large-1 %span.margin-top{"ng-bind" => "::producer.address.state_name"} + .columns.small-2.medium-1.large-1.text-right %span.margin-top %i{"ng-class" => "{'ofn-i_005-caret-down' : !open(), 'ofn-i_006-caret-up' : open()}"} diff --git a/app/views/producers/index.html.haml b/app/views/producers/index.html.haml index af6c2e4fd1..1558fc131c 100644 --- a/app/views/producers/index.html.haml +++ b/app/views/producers/index.html.haml @@ -1,7 +1,7 @@ - content_for(:title) do = t :producers_title -= inject_enterprises += inject_enterprises(@enterprises) .producers{"ng-controller" => "EnterprisesCtrl", "ng-cloak" => true} .row From 698d3672a636665b9741d52001e601b083824503 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 10 May 2019 01:10:19 +0100 Subject: [PATCH 04/16] List only current taxons for active enterprises --- app/models/enterprise.rb | 16 ++++++++++++++++ .../api/enterprise_shopfront_serializer.rb | 4 +++- spec/models/enterprise_spec.rb | 12 ++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index 162031889b..d5e45a3ae5 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -285,6 +285,14 @@ class Enterprise < ActiveRecord::Base select('DISTINCT spree_taxons.*') end + def current_distributed_taxons + Spree::Taxon + .select("DISTINCT spree_taxons.*") + .joins(products: :variants_including_master) + .joins("INNER JOIN (#{current_exchange_variants.to_sql}) \ + AS exchange_variants ON spree_variants.id = exchange_variants.variant_id") + end + # Return all taxons for all supplied products def supplied_taxons Spree::Taxon. @@ -325,6 +333,14 @@ class Enterprise < ActiveRecord::Base private + def current_exchange_variants + ExchangeVariant.joins(exchange: :order_cycle) + .merge(Exchange.outgoing) + .select("DISTINCT exchange_variants.variant_id, exchanges.receiver_id AS enterprise_id") + .where("exchanges.receiver_id = ?", id) + .merge(OrderCycle.active.with_distributor(id)) + end + def name_is_unique dups = Enterprise.where(name: name) dups = dups.where('id != ?', id) unless new_record? diff --git a/app/serializers/api/enterprise_shopfront_serializer.rb b/app/serializers/api/enterprise_shopfront_serializer.rb index 6b800ffcd7..e7b645def6 100644 --- a/app/serializers/api/enterprise_shopfront_serializer.rb +++ b/app/serializers/api/enterprise_shopfront_serializer.rb @@ -62,8 +62,10 @@ module Api end def taxons + taxons = active ? enterprise.current_distributed_taxons : enterprise.distributed_taxons + ActiveModel::ArraySerializer.new( - enterprise.distributed_taxons, each_serializer: Api::TaxonSerializer + taxons, each_serializer: Api::TaxonSerializer ) end diff --git a/spec/models/enterprise_spec.rb b/spec/models/enterprise_spec.rb index c648031e88..cdee0610ab 100644 --- a/spec/models/enterprise_spec.rb +++ b/spec/models/enterprise_spec.rb @@ -482,14 +482,26 @@ describe Enterprise do let(:supplier) { create(:supplier_enterprise) } let(:taxon1) { create(:taxon) } let(:taxon2) { create(:taxon) } + let(:taxon3) { create(:taxon) } let(:product1) { create(:simple_product, primary_taxon: taxon1, taxons: [taxon1]) } let(:product2) { create(:simple_product, primary_taxon: taxon1, taxons: [taxon1, taxon2]) } + let(:product3) { create(:simple_product, primary_taxon: taxon3) } + let(:oc) { create(:order_cycle, distributors: [distributor]) } + let(:ex) { create(:exchange, order_cycle: oc, incoming: false, sender: supplier, receiver: distributor) } it "gets all taxons of all distributed products" do allow(Spree::Product).to receive(:in_distributor).and_return [product1, product2] expect(distributor.distributed_taxons).to match_array [taxon1, taxon2] end + it "gets all taxons of all distributed products in open order cycles" do + Spree::Product.stub(:in_distributor).and_return [product1, product2, product3] + ex.variants << product1.variants.first + ex.variants << product3.variants.first + + distributor.current_distributed_taxons.should match_array [taxon1, taxon3] + end + it "gets all taxons of all supplied products" do allow(Spree::Product).to receive(:in_supplier).and_return [product1, product2] expect(supplier.supplied_taxons).to match_array [taxon1, taxon2] From ae8f1a92e8d6faa165bb66ff86681550927d0acc Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 9 May 2019 23:34:19 +0100 Subject: [PATCH 05/16] Remove "profiles" from /shops --- app/views/groups/show.html.haml | 4 ++-- .../shared/components/_show_profiles.html.haml | 7 ------- app/views/shops/_filters.html.haml | 4 +--- app/views/shops/_hubs_table.html.haml | 2 +- spec/features/consumer/shops_spec.rb | 15 --------------- 5 files changed, 4 insertions(+), 28 deletions(-) delete mode 100644 app/views/shared/components/_show_profiles.html.haml diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index bd15ce9a76..c0179e50f0 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -89,13 +89,13 @@ = t :groups_hubs = render "shared/components/enterprise_search" - = render "shops/filters", resource: "group_hubs", property_filters: "| searchEnterprises:query | taxons:activeTaxons | shipping:shippingTypes | showHubProfiles:show_profiles" + = render "shops/filters", resource: "group_hubs", property_filters: "| searchEnterprises:query | taxons:activeTaxons | shipping:shippingTypes" .row .small-12.columns .active_table %hub.active_table_node.row.animate-repeat{id: "{{hub.hash}}", - "ng-repeat" => "hub in filteredEnterprises = (group_hubs | searchEnterprises:query | taxons:activeTaxons | shipping:shippingTypes | showHubProfiles:show_profiles | properties:activeProperties:'distributed_properties' | orderBy:['-active', '+orders_close_at'])", + "ng-repeat" => "hub in filteredEnterprises = (group_hubs | searchEnterprises:query | taxons:activeTaxons | shipping:shippingTypes | properties:activeProperties:'distributed_properties' | orderBy:['-active', '+orders_close_at'])", "ng-class" => "{'is_profile' : hub.category == 'hub_profile', 'closed' : !open(), 'open' : open(), 'inactive' : !hub.active, 'current' : current()}", "ng-controller" => "GroupEnterpriseNodeCtrl"} .small-12.columns diff --git a/app/views/shared/components/_show_profiles.html.haml b/app/views/shared/components/_show_profiles.html.haml deleted file mode 100644 index 5a1b5ec1db..0000000000 --- a/app/views/shared/components/_show_profiles.html.haml +++ /dev/null @@ -1,7 +0,0 @@ -.small-12.medium-6.columns.text-right - .profile-checkbox - %button.button.secondary.tiny.right.help-btn.ng-scope{:popover => t(:components_profiles_popover, sitename: Spree::Config[:site_name]), "popover-placement" => "left"}>< - %i.ofn-i_013-help - %label - %input{"ng-model" => "show_profiles", type: "checkbox", name: "profile"} - = t :components_profiles_show diff --git a/app/views/shops/_filters.html.haml b/app/views/shops/_filters.html.haml index dedc473808..33d5ac3a5e 100644 --- a/app/views/shops/_filters.html.haml +++ b/app/views/shops/_filters.html.haml @@ -1,10 +1,8 @@ - resource ||= "visibleMatches" -- property_filters ||= "| closedShops:show_closed | taxons:activeTaxons | shipping:shippingTypes | showHubProfiles:show_profiles" +- property_filters ||= "| closedShops:show_closed | taxons:activeTaxons | shipping:shippingTypes" .row = render 'shared/components/filter_controls' - -# .small-12.medium-6.columns   - = render 'shared/components/show_profiles' .row.animate-show.filter-row{"ng-show" => "filtersActive"} .small-12.columns diff --git a/app/views/shops/_hubs_table.html.haml b/app/views/shops/_hubs_table.html.haml index 0c0b1a4442..76bd2a6695 100644 --- a/app/views/shops/_hubs_table.html.haml +++ b/app/views/shops/_hubs_table.html.haml @@ -1,5 +1,5 @@ .active_table - %hub.active_table_node.row{"ng-repeat" => "hub in #{enterprises}Filtered = (#{enterprises} | closedShops:show_closed | taxons:activeTaxons | properties:activeProperties:'distributed_properties' | shipping:shippingTypes | showHubProfiles:show_profiles | orderBy:['-active', '+distance', '+orders_close_at'])", + %hub.active_table_node.row{"ng-repeat" => "hub in #{enterprises}Filtered = (#{enterprises} | closedShops:show_closed | taxons:activeTaxons | properties:activeProperties:'distributed_properties' | shipping:shippingTypes | orderBy:['-active', '+distance', '+orders_close_at'])", "ng-class" => "{'is_profile' : hub.category == 'hub_profile', 'closed' : !open(), 'open' : open(), 'inactive' : !hub.active, 'current' : current()}", "ng-controller" => "HubNodeCtrl", id: "{{hub.hash}}"} diff --git a/spec/features/consumer/shops_spec.rb b/spec/features/consumer/shops_spec.rb index ea919341eb..f422a65a79 100644 --- a/spec/features/consumer/shops_spec.rb +++ b/spec/features/consumer/shops_spec.rb @@ -55,21 +55,6 @@ feature 'Shops', js: true do follow_active_table_node distributor.name expect(page).to have_current_path enterprise_shop_path(distributor) end - - describe "showing profiles" do - before do - check "Show profiles" - end - - it "still shows hubs" do - expect(page).to have_content distributor.name - end - - # https://github.com/openfoodfoundation/openfoodnetwork/issues/1718 - it "shows profiles" do - expect(page).to have_content profile.name - end - end end describe "showing available hubs" do From 1b8dfaf0490a77c0d3e0f369090a85a06bc28ff2 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 10 May 2019 01:52:27 +0100 Subject: [PATCH 06/16] Fix HashNavigation reload bug --- .../controllers/hub_node_controller.js.coffee | 11 +++++++++-- .../controllers/producer_node_controller.js.coffee | 11 +++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee index e587dcfb62..9b08a09139 100644 --- a/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee @@ -1,7 +1,11 @@ -Darkswarm.controller "HubNodeCtrl", ($scope, HashNavigation, Navigation, $location, $templateCache, CurrentHub, $http) -> +Darkswarm.controller "HubNodeCtrl", ($scope, HashNavigation, Navigation, $location, CurrentHub, $http, $timeout) -> $scope.shopfront_loading = false $scope.enterprise_details = [] + $timeout -> + if $scope.open() + $scope.load_shopfront() + # Toggles shopfront tabs open/closed. Fetches enterprise details from the api, diplays them and adds them # to $scope.enterprise_details, or simply displays the details again if previously fetched $scope.toggle = (event) -> @@ -14,6 +18,9 @@ Darkswarm.controller "HubNodeCtrl", ($scope, HashNavigation, Navigation, $locati $scope.toggle_tab(event) return + $scope.load_shopfront(event) + + $scope.load_shopfront = (event=null) -> $scope.shopfront_loading = true $scope.toggle_tab(event) @@ -26,7 +33,7 @@ Darkswarm.controller "HubNodeCtrl", ($scope, HashNavigation, Navigation, $locati console.error(data) $scope.toggle_tab = (event) -> - HashNavigation.toggle $scope.hub.hash if !angular.element(event.target).inheritedData('is-link') + HashNavigation.toggle $scope.hub.hash if event && !angular.element(event.target).inheritedData('is-link') # Returns boolean: pulldown tab is currently open/closed $scope.open = -> diff --git a/app/assets/javascripts/darkswarm/controllers/producer_node_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/producer_node_controller.js.coffee index e8aeb58af2..0cacb566d5 100644 --- a/app/assets/javascripts/darkswarm/controllers/producer_node_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/producer_node_controller.js.coffee @@ -1,7 +1,11 @@ -Darkswarm.controller "ProducerNodeCtrl", ($scope, HashNavigation, $anchorScroll, $http) -> +Darkswarm.controller "ProducerNodeCtrl", ($scope, HashNavigation, $anchorScroll, $http, $timeout) -> $scope.shopfront_loading = false $scope.enterprise_details = [] + $timeout -> + if $scope.open() + $scope.load_shopfront() + # Toggles shopfront tabs open/closed. Fetches enterprise details from the api, diplays them and adds them # to $scope.enterprise_details, or simply displays the details again if previously fetched $scope.toggle = (event) -> @@ -14,6 +18,9 @@ Darkswarm.controller "ProducerNodeCtrl", ($scope, HashNavigation, $anchorScroll, $scope.toggle_tab(event) return + $scope.load_shopfront(event) + + $scope.load_shopfront = (event=null) -> $scope.shopfront_loading = true $scope.toggle_tab(event) @@ -26,7 +33,7 @@ Darkswarm.controller "ProducerNodeCtrl", ($scope, HashNavigation, $anchorScroll, console.error(data) $scope.toggle_tab = (event) -> - HashNavigation.toggle $scope.producer.hash if !angular.element(event.target).inheritedData('is-link') + HashNavigation.toggle $scope.producer.hash if event && !angular.element(event.target).inheritedData('is-link') $scope.open = -> HashNavigation.active($scope.producer.hash) From aca1f92060d07c9244c63828480789bafc6dfdd7 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 10 May 2019 02:18:24 +0100 Subject: [PATCH 07/16] Adapt specs to new loading methods --- spec/controllers/shops_controller_spec.rb | 8 +------ spec/features/consumer/producers_spec.rb | 11 +++++++--- spec/features/consumer/shops_spec.rb | 5 +++++ .../services/enterprise_spec.js.coffee | 4 ---- .../enterprise_injection_data_spec.rb | 22 ------------------- .../serializers/enterprise_serializer_spec.rb | 8 +------ .../request/authentication_workflow.rb | 6 +++++ 7 files changed, 21 insertions(+), 43 deletions(-) diff --git a/spec/controllers/shops_controller_spec.rb b/spec/controllers/shops_controller_spec.rb index 5dc929b5b7..2cecfc2c96 100644 --- a/spec/controllers/shops_controller_spec.rb +++ b/spec/controllers/shops_controller_spec.rb @@ -1,21 +1,15 @@ require 'spec_helper' describe ShopsController, type: :controller do + include WebHelper render_views let!(:distributor) { create(:distributor_enterprise) } - let!(:invisible_distributor) { create(:distributor_enterprise, visible: false) } before do 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 - expect(response.body).to have_content(invisible_distributor.name) - end - it 'renders distributed product properties' do product_property = create(:property, presentation: 'eggs') product = create(:product, properties: [product_property]) diff --git a/spec/features/consumer/producers_spec.rb b/spec/features/consumer/producers_spec.rb index 8ba7585804..4d9298df62 100644 --- a/spec/features/consumer/producers_spec.rb +++ b/spec/features/consumer/producers_spec.rb @@ -5,6 +5,7 @@ feature ' I want to see a list of producers So that I can shop at hubs distributing their products ', js: true do + include AuthenticationWorkflow include WebHelper include UIComponentHelper @@ -21,6 +22,10 @@ feature ' let(:shop) { create(:distributor_enterprise) } let!(:er) { create(:enterprise_relationship, parent: shop, child: producer1) } + before :each do + use_api_as_unauthenticated_user + end + before do product1.set_property 'Organic', 'NASAA 12345' product2.set_property 'Biodynamic', 'ABC123' @@ -79,8 +84,8 @@ feature ' page.should have_content 'Fruit' # -- Properties - page.should have_content 'Organic' # Product property - page.should have_content 'Local' # Producer property + expect(page).to have_content 'Organic' # Product property + expect(page).to have_content 'Local' # Producer property end it "doesn't show invisible producers" do @@ -89,7 +94,7 @@ feature ' it "links to places to buy produce" do expand_active_table_node producer1.name - page.should have_link shop.name + expect(page).to have_link shop.name end end end diff --git a/spec/features/consumer/shops_spec.rb b/spec/features/consumer/shops_spec.rb index f422a65a79..55a822ba38 100644 --- a/spec/features/consumer/shops_spec.rb +++ b/spec/features/consumer/shops_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' feature 'Shops', js: true do include AuthenticationWorkflow include UIComponentHelper + include WebHelper let!(:distributor) { create(:distributor_enterprise, with_payment_and_shipping: true) } let!(:invisible_distributor) { create(:distributor_enterprise, visible: false) } @@ -13,6 +14,10 @@ feature 'Shops', js: true do let!(:producer) { create(:supplier_enterprise) } let!(:er) { create(:enterprise_relationship, parent: distributor, child: producer) } + before :each do + use_api_as_unauthenticated_user + end + before do producer.set_producer_property 'Organic', 'NASAA 12345' end diff --git a/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee index 67774aecbd..120e5c8032 100644 --- a/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee @@ -49,10 +49,6 @@ describe "Enterprises service", -> it "puts the same objects in enterprises and enterprises_by_id", -> expect(Enterprises.enterprises[0]).toBe Enterprises.enterprises_by_id["1"] - it "dereferences references to other enterprises", -> - expect(Enterprises.enterprises_by_id["1"].producers[0]).toBe enterprises[4] - expect(Enterprises.enterprises_by_id["5"].hubs[0]).toBe enterprises[0] - it "dereferences taxons", -> expect(Enterprises.enterprises[0].taxons[0]).toBe taxons[0] diff --git a/spec/lib/open_food_network/enterprise_injection_data_spec.rb b/spec/lib/open_food_network/enterprise_injection_data_spec.rb index 6cf917a002..e69de29bb2 100644 --- a/spec/lib/open_food_network/enterprise_injection_data_spec.rb +++ b/spec/lib/open_food_network/enterprise_injection_data_spec.rb @@ -1,22 +0,0 @@ -require 'spec_helper' - -module OpenFoodNetwork - describe EnterpriseInjectionData do - describe "relatives" do - let!(:enterprise) { create(:distributor_enterprise) } - let!(:producer) { create(:supplier_enterprise) } - let!(:producer_inactive) { create(:supplier_enterprise, sells: 'unspecified') } - let!(:er_p) { create(:enterprise_relationship, parent: producer, child: enterprise) } - let!(:er_pi) { create(:enterprise_relationship, parent: producer_inactive, child: enterprise) } - - it "only loads activated relatives" do - expect(subject.relatives[enterprise.id][:producers]).not_to include producer_inactive.id - end - - it "loads self where appropiate" do - expect(subject.relatives[producer.id][:producers]).to include producer.id - expect(subject.relatives[enterprise.id][:distributors]).to include enterprise.id - end - end - end -end diff --git a/spec/serializers/enterprise_serializer_spec.rb b/spec/serializers/enterprise_serializer_spec.rb index 7eb6ee3adc..5eaa3d3e48 100644 --- a/spec/serializers/enterprise_serializer_spec.rb +++ b/spec/serializers/enterprise_serializer_spec.rb @@ -10,8 +10,7 @@ describe Api::EnterpriseSerializer do all_distributed_taxons: { enterprise.id => [123] }, current_distributed_taxons: { enterprise.id => [123] }, supplied_taxons: { enterprise.id => [456] }, - shipping_method_services: {}, - relatives: { enterprise.id => { producers: [123], distributors: [456] } }) + shipping_method_services: {}) } it "serializes an enterprise" do @@ -23,11 +22,6 @@ describe Api::EnterpriseSerializer do expect(serializer.serializable_hash[:supplied_taxons]).to eq([{ id: 456 }]) end - it "serializes producers and hubs as ids only" do - expect(serializer.serializable_hash[:producers]).to eq([{ id: 123 }]) - expect(serializer.serializable_hash[:hubs]).to eq([{ id: 456 }]) - end - it "serializes icons" do expect(serializer.to_json).to match "map_005-hub.svg" end diff --git a/spec/support/request/authentication_workflow.rb b/spec/support/request/authentication_workflow.rb index 214c901d77..ac2cf66f6e 100644 --- a/spec/support/request/authentication_workflow.rb +++ b/spec/support/request/authentication_workflow.rb @@ -63,6 +63,12 @@ module AuthenticationWorkflow fill_in "password", with: user.password click_button "Login" end + + def use_api_as_unauthenticated_user + allow_any_instance_of(Api::BaseController).to receive(:spree_current_user) { + Spree::User.anonymous! + } + end end RSpec.configure do |config| From 4ec46c2db639eac4d7b41b9c899c3affe547da5b Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 9 May 2019 10:25:26 +0100 Subject: [PATCH 08/16] Fix groups index page --- .../darkswarm/services/groups.js.coffee | 13 +--------- app/controllers/groups_controller.rb | 4 --- app/helpers/injection_helper.rb | 10 ++++++++ app/serializers/api/group_list_serializer.rb | 16 ++++++++++++ app/views/groups/index.html.haml | 5 +--- .../api/group_list_serializer_spec.rb | 25 +++++++++++++++++++ 6 files changed, 53 insertions(+), 20 deletions(-) create mode 100644 app/serializers/api/group_list_serializer.rb create mode 100644 spec/serializers/api/group_list_serializer_spec.rb diff --git a/app/assets/javascripts/darkswarm/services/groups.js.coffee b/app/assets/javascripts/darkswarm/services/groups.js.coffee index e07d6c2055..69a7d2f564 100644 --- a/app/assets/javascripts/darkswarm/services/groups.js.coffee +++ b/app/assets/javascripts/darkswarm/services/groups.js.coffee @@ -1,14 +1,3 @@ -Darkswarm.factory 'Groups', (groups, Enterprises, Dereferencer) -> +Darkswarm.factory 'Groups', (groups) -> new class Groups groups: groups - groups_by_id: {} - constructor: -> - for group in @groups - @groups_by_id[group.id] = group - @dereference() - dereference: -> - for group in @groups - Dereferencer.dereference group.enterprises, Enterprises.enterprises_by_id - for enterprise in Enterprises.enterprises - Dereferencer.dereference enterprise.groups, @groups_by_id - diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index ce72038e70..6dcc582f86 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -1,10 +1,6 @@ class GroupsController < BaseController layout 'darkswarm' - def index - @groups = EnterpriseGroup.on_front_page.by_position - end - def show enable_embedded_shopfront @hide_menu = true if @shopfront_layout == 'embedded' diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index 87f3efe1d7..921648544c 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -12,6 +12,16 @@ module InjectionHelper ) end + def inject_groups + select_only = required_attributes EnterpriseGroup, Api::GroupListSerializer + + inject_json_ams( + 'groups', + EnterpriseGroup.on_front_page.by_position.select(select_only).includes(address: :state).all, + Api::GroupListSerializer + ) + end + def inject_enterprise_shopfront_list select_only = required_attributes Enterprise, Api::EnterpriseShopfrontListSerializer diff --git a/app/serializers/api/group_list_serializer.rb b/app/serializers/api/group_list_serializer.rb new file mode 100644 index 0000000000..46e851ab4f --- /dev/null +++ b/app/serializers/api/group_list_serializer.rb @@ -0,0 +1,16 @@ +module Api + class GroupListSerializer < ActiveModel::Serializer + attributes :id, :name, :permalink, :email, :website, :facebook, :instagram, + :linkedin, :twitter, :enterprises, :state, :address_id + + def state + object.address.state.abbr + end + + def enterprises + ActiveModel::ArraySerializer.new( + object.enterprises, each_serializer: Api::EnterpriseThinSerializer + ) + end + end +end diff --git a/app/views/groups/index.html.haml b/app/views/groups/index.html.haml index ad93d4f366..dabfd70b6c 100644 --- a/app/views/groups/index.html.haml +++ b/app/views/groups/index.html.haml @@ -1,10 +1,7 @@ - content_for(:title) do = t :groups_title -= inject_enterprises - -:javascript - angular.module('Darkswarm').value('groups', #{render partial: "json/groups", object: @groups}) += inject_groups #groups.pad-top.footer-pad{"ng-controller" => "GroupsCtrl"} .row diff --git a/spec/serializers/api/group_list_serializer_spec.rb b/spec/serializers/api/group_list_serializer_spec.rb new file mode 100644 index 0000000000..df69cf75c1 --- /dev/null +++ b/spec/serializers/api/group_list_serializer_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe Api::GroupListSerializer do + let!(:group) { create(:enterprise_group) } + let!(:producer) { create(:supplier_enterprise) } + + let(:serializer) { Api::GroupListSerializer.new group } + + before do + group.enterprises << producer + end + + it "serializes group attributes" do + expect(serializer.serializable_hash[:name]).to match group.name + end + + it "serializes abbreviated state" do + expect(serializer.serializable_hash[:state]).to eq group.address.state.abbr + end + + it "serialises an array of enterprises" do + expect(serializer.serializable_hash[:enterprises]).to be_a ActiveModel::ArraySerializer + expect(serializer.serializable_hash[:enterprises].to_json).to match producer.name + end +end From d5bd05875446b8697936f1bfbfb5d1d18fea77c2 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 9 May 2019 10:57:39 +0100 Subject: [PATCH 09/16] Fix groups show page It can definitely be cleaned up more in the future, but this should get us around a 90% reduction in page load times. --- .../controllers/group_page_controller.js.coffee | 10 ++++------ app/helpers/injection_helper.rb | 7 ++++++- app/views/groups/show.html.haml | 6 ------ 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee index da67840752..04c8e8c0b9 100644 --- a/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee @@ -1,12 +1,10 @@ -Darkswarm.controller "GroupPageCtrl", ($scope, group_enterprises, Enterprises, MapConfiguration, OfnMap, visibleFilter, Navigation) -> +Darkswarm.controller "GroupPageCtrl", ($scope, enterprises, Enterprises, MapConfiguration, OfnMap, visibleFilter, Navigation) -> $scope.Enterprises = Enterprises - all_enterprises_by_id = Enterprises.enterprises_by_id + enterprises_by_id = enterprises.map (enterprise) => + Enterprises.enterprises_by_id[enterprise.id] - dereferenced_enterprises = group_enterprises.map (enterprise) => - all_enterprises_by_id[enterprise.id] - - visible_enterprises = visibleFilter dereferenced_enterprises + visible_enterprises = visibleFilter enterprises_by_id # TODO: this is duplicate code with app/assets/javascripts/darkswarm/services/enterprises.js.coffee # It would be better to load only the needed enterprises (group + related shops). diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index 921648544c..e8f1c18687 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -37,7 +37,12 @@ module InjectionHelper end def inject_group_enterprises - inject_json_ams "group_enterprises", @group.enterprises.activated.all, Api::EnterpriseSerializer, enterprise_injection_data + inject_json_ams( + "enterprises", + @group.enterprises.activated.all, + Api::EnterpriseSerializer, + enterprise_injection_data + ) end def inject_current_hub diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index c0179e50f0..2808d7544e 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -5,12 +5,6 @@ - content_for(:image) do = @group.logo.url --# inject all enterprises as "enterprises" --# it could be more efficient to inject only the enterprises that are related to the group -= inject_enterprises - --# inject enterprises in this group --# further hubs and producers of these enterprises can't be resolved within this small subset = inject_group_enterprises #group-page.row.pad-top.footer-pad{"ng-controller" => "GroupPageCtrl"} From 4155b1763316010dfe3bb90dc986c0c6dfd7e48a Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 9 May 2019 11:34:00 +0100 Subject: [PATCH 10/16] Remove tests that reference removed code --- spec/controllers/groups_controller_spec.rb | 8 ++--- .../darkswarm/services/groups_spec.js.coffee | 35 ------------------- 2 files changed, 2 insertions(+), 41 deletions(-) delete mode 100644 spec/javascripts/unit/darkswarm/services/groups_spec.js.coffee diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index f134e6f4ca..ddd5bd6723 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -2,13 +2,9 @@ require 'spec_helper' describe GroupsController, type: :controller do render_views - let(:enterprise) { create(:distributor_enterprise) } + + let!(:enterprise) { create(:distributor_enterprise) } let!(:group) { create(:enterprise_group, enterprises: [enterprise], on_front_page: true) } - it "gets all visible groups" do - allow(EnterpriseGroup).to receive_message_chain :on_front_page, :by_position - expect(EnterpriseGroup).to receive :on_front_page - get :index - end it "loads all enterprises for group" do get :index diff --git a/spec/javascripts/unit/darkswarm/services/groups_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/groups_spec.js.coffee deleted file mode 100644 index 50e2159338..0000000000 --- a/spec/javascripts/unit/darkswarm/services/groups_spec.js.coffee +++ /dev/null @@ -1,35 +0,0 @@ -describe "Groups service", -> - Groups = null - Enterprises = null - CurrentHubMock = {} - Geo = {} - groups = [{ - id: 1 - name: "Test Group" - enterprises: [ - {id: 1}, - {id: 2} - ] - }] - enterprises = [ - {id: 1, name: "Test 1", groups: [{id: 1}]}, - {id: 2, name: "Test 2", groups: [{id: 1}]} - ] - - beforeEach -> - module 'Darkswarm' - angular.module('Darkswarm').value('groups', groups) - angular.module('Darkswarm').value('enterprises', enterprises) - module ($provide)-> - $provide.value "CurrentHub", CurrentHubMock - $provide.value "Geo", Geo - null - inject (_Groups_, _Enterprises_)-> - Groups = _Groups_ - Enterprises = _Enterprises_ - - it "dereferences group enterprises", -> - expect(Groups.groups[0].enterprises[0]).toBe enterprises[0] - - it "dereferences enterprise groups", -> - expect(Enterprises.enterprises[0].groups[0]).toBe groups[0] From 9703d848eff8a275c39878fe3d37bf560ce881eb Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Thu, 9 May 2019 12:55:50 +0100 Subject: [PATCH 11/16] Improve syntax in injection_helper --- app/helpers/injection_helper.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index e8f1c18687..748aed6510 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -5,7 +5,7 @@ module InjectionHelper def inject_enterprises(enterprises = Enterprise.activated.includes(address: :state).all) inject_json_ams( - 'enterprises', + "enterprises", enterprises, Api::EnterpriseSerializer, enterprise_injection_data @@ -16,7 +16,7 @@ module InjectionHelper select_only = required_attributes EnterpriseGroup, Api::GroupListSerializer inject_json_ams( - 'groups', + "groups", EnterpriseGroup.on_front_page.by_position.select(select_only).includes(address: :state).all, Api::GroupListSerializer ) @@ -26,7 +26,7 @@ module InjectionHelper select_only = required_attributes Enterprise, Api::EnterpriseShopfrontListSerializer inject_json_ams( - 'enterprises', + "enterprises", Enterprise.activated.select(select_only).includes(address: :state).all, Api::EnterpriseShopfrontListSerializer ) @@ -98,11 +98,7 @@ module InjectionHelper end def inject_saved_credit_cards - data = if spree_current_user - spree_current_user.credit_cards.with_payment_profile.all - else - [] - end + data = spree_current_user ? spree_current_user.credit_cards.with_payment_profile.all : [] inject_json_ams "savedCreditCards", data, Api::CreditCardSerializer end From 11e83af0b68d340d29ad9635c2d71b986c01c58f Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 10 May 2019 14:08:48 +0100 Subject: [PATCH 12/16] Remove duplication of enterprise angular controllers We can re-use the enterprise and producers controllers here in their respective groups tabs, no need for a third controller --- .../group_enterprise_node_controller.js.coffee | 12 ------------ app/views/groups/show.html.haml | 4 ++-- 2 files changed, 2 insertions(+), 14 deletions(-) delete mode 100644 app/assets/javascripts/darkswarm/controllers/group_enterprise_node_controller.js.coffee diff --git a/app/assets/javascripts/darkswarm/controllers/group_enterprise_node_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/group_enterprise_node_controller.js.coffee deleted file mode 100644 index 376320e458..0000000000 --- a/app/assets/javascripts/darkswarm/controllers/group_enterprise_node_controller.js.coffee +++ /dev/null @@ -1,12 +0,0 @@ -Darkswarm.controller "GroupEnterpriseNodeCtrl", ($scope, CurrentHub) -> - - $scope.active = false - - $scope.toggle = -> - $scope.active = !$scope.active - - $scope.open = -> - $scope.active - - $scope.current = -> - $scope.hub.id is CurrentHub.hub.id diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 2808d7544e..726aaff811 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -63,7 +63,7 @@ .active_table %producer.active_table_node.row.animate-repeat{id: "{{producer.path}}", "ng-repeat" => "producer in filteredEnterprises = (group_producers | searchEnterprises:query | taxons:activeTaxons | properties:activeProperties:'supplied_properties')", - "ng-controller" => "GroupEnterpriseNodeCtrl", + "ng-controller" => "ProducerNodeCtrl", "ng-class" => "{'closed' : !open(), 'open' : open(), 'inactive' : !producer.active}", id: "{{producer.hash}}"} @@ -91,7 +91,7 @@ %hub.active_table_node.row.animate-repeat{id: "{{hub.hash}}", "ng-repeat" => "hub in filteredEnterprises = (group_hubs | searchEnterprises:query | taxons:activeTaxons | shipping:shippingTypes | properties:activeProperties:'distributed_properties' | orderBy:['-active', '+orders_close_at'])", "ng-class" => "{'is_profile' : hub.category == 'hub_profile', 'closed' : !open(), 'open' : open(), 'inactive' : !hub.active, 'current' : current()}", - "ng-controller" => "GroupEnterpriseNodeCtrl"} + "ng-controller" => "HubNodeCtrl"} .small-12.columns = render 'shops/skinny' = render 'shops/fat' From a91ae8947b52d827fa8b82b31c5fa443931427e9 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 10 May 2019 15:04:29 +0100 Subject: [PATCH 13/16] Only query visible enterprises --- .../controllers/group_page_controller.js.coffee | 10 ++++------ .../darkswarm/services/enterprises.js.coffee | 7 +++---- .../javascripts/darkswarm/services/map.js.coffee | 4 ++-- app/controllers/producers_controller.rb | 1 + app/controllers/shops_controller.rb | 1 + app/helpers/injection_helper.rb | 2 +- .../api/enterprise_shopfront_list_serializer.rb | 2 +- app/serializers/api/enterprise_thin_serializer.rb | 2 +- app/views/producers/_fat.html.haml | 2 +- app/views/producers/index.html.haml | 2 +- app/views/shops/_filters.html.haml | 2 +- .../darkswarm/services/enterprise_spec.js.coffee | 12 ++---------- 12 files changed, 19 insertions(+), 28 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee index 04c8e8c0b9..0d2f244b32 100644 --- a/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee @@ -1,20 +1,18 @@ -Darkswarm.controller "GroupPageCtrl", ($scope, enterprises, Enterprises, MapConfiguration, OfnMap, visibleFilter, Navigation) -> +Darkswarm.controller "GroupPageCtrl", ($scope, enterprises, Enterprises, MapConfiguration, OfnMap, Navigation) -> $scope.Enterprises = Enterprises enterprises_by_id = enterprises.map (enterprise) => Enterprises.enterprises_by_id[enterprise.id] - visible_enterprises = visibleFilter enterprises_by_id - # TODO: this is duplicate code with app/assets/javascripts/darkswarm/services/enterprises.js.coffee # It would be better to load only the needed enterprises (group + related shops). - $scope.group_producers = visible_enterprises.filter (enterprise) -> + $scope.group_producers = enterprises_by_id.filter (enterprise) -> enterprise.category in ["producer_hub", "producer_shop", "producer"] - $scope.group_hubs = visible_enterprises.filter (enterprise) -> + $scope.group_hubs = enterprises_by_id.filter (enterprise) -> enterprise.category in ["hub", "hub_profile", "producer_hub", "producer_shop"] $scope.producers_to_filter = $scope.group_producers $scope.map = angular.copy MapConfiguration.options - $scope.mapMarkers = OfnMap.enterprise_markers visible_enterprises + $scope.mapMarkers = OfnMap.enterprise_markers enterprises_by_id $scope.embedded_layout = window.location.search.indexOf("embedded_shopfront=true") != -1 \ No newline at end of file diff --git a/app/assets/javascripts/darkswarm/services/enterprises.js.coffee b/app/assets/javascripts/darkswarm/services/enterprises.js.coffee index 6d667e0001..d3b7deec3e 100644 --- a/app/assets/javascripts/darkswarm/services/enterprises.js.coffee +++ b/app/assets/javascripts/darkswarm/services/enterprises.js.coffee @@ -1,4 +1,4 @@ -Darkswarm.factory 'Enterprises', (enterprises, CurrentHub, Taxons, Dereferencer, visibleFilter, Matcher, Geo, $rootScope) -> +Darkswarm.factory 'Enterprises', (enterprises, CurrentHub, Taxons, Dereferencer, Matcher, Geo, $rootScope) -> new class Enterprises enterprises_by_id: {} @@ -13,10 +13,9 @@ Darkswarm.factory 'Enterprises', (enterprises, CurrentHub, Taxons, Dereferencer, # Replace enterprise and taxons ids with actual objects. @dereferenceEnterprises() - @visible_enterprises = visibleFilter @enterprises - @producers = @visible_enterprises.filter (enterprise)-> + @producers = @enterprises.filter (enterprise)-> enterprise.category in ["producer_hub", "producer_shop", "producer"] - @hubs = @visible_enterprises.filter (enterprise)-> + @hubs = @enterprises.filter (enterprise)-> enterprise.category in ["hub", "hub_profile", "producer_hub", "producer_shop"] dereferenceEnterprises: -> diff --git a/app/assets/javascripts/darkswarm/services/map.js.coffee b/app/assets/javascripts/darkswarm/services/map.js.coffee index f4bad04acc..45370061fd 100644 --- a/app/assets/javascripts/darkswarm/services/map.js.coffee +++ b/app/assets/javascripts/darkswarm/services/map.js.coffee @@ -1,4 +1,4 @@ -Darkswarm.factory "OfnMap", (Enterprises, EnterpriseModal, visibleFilter) -> +Darkswarm.factory "OfnMap", (Enterprises, EnterpriseModal) -> new class OfnMap constructor: -> @enterprises = @enterprise_markers(Enterprises.enterprises) @@ -6,7 +6,7 @@ Darkswarm.factory "OfnMap", (Enterprises, EnterpriseModal, visibleFilter) -> enterprise.latitude != null || enterprise.longitude != null # Remove enterprises w/o lat or long enterprise_markers: (enterprises) -> - @extend(enterprise) for enterprise in visibleFilter(enterprises) + @extend(enterprise) for enterprise in enterprises # Adding methods to each enterprise extend: (enterprise) -> diff --git a/app/controllers/producers_controller.rb b/app/controllers/producers_controller.rb index 4dd7800003..0f45546fde 100644 --- a/app/controllers/producers_controller.rb +++ b/app/controllers/producers_controller.rb @@ -6,6 +6,7 @@ class ProducersController < BaseController def index @enterprises = Enterprise .activated + .visible .is_primary_producer .includes(address: :state) .includes(:properties) diff --git a/app/controllers/shops_controller.rb b/app/controllers/shops_controller.rb index 25c5995170..611baa11b8 100644 --- a/app/controllers/shops_controller.rb +++ b/app/controllers/shops_controller.rb @@ -6,6 +6,7 @@ class ShopsController < BaseController def index @enterprises = Enterprise .activated + .visible .is_distributor .includes(address: :state) .includes(:properties) diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index 748aed6510..e83a1dbe36 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -27,7 +27,7 @@ module InjectionHelper inject_json_ams( "enterprises", - Enterprise.activated.select(select_only).includes(address: :state).all, + Enterprise.activated.visible.select(select_only).includes(address: :state).all, Api::EnterpriseShopfrontListSerializer ) end diff --git a/app/serializers/api/enterprise_shopfront_list_serializer.rb b/app/serializers/api/enterprise_shopfront_list_serializer.rb index a8ded21bf1..dd83ce4323 100644 --- a/app/serializers/api/enterprise_shopfront_list_serializer.rb +++ b/app/serializers/api/enterprise_shopfront_list_serializer.rb @@ -2,7 +2,7 @@ module Api class EnterpriseShopfrontListSerializer < ActiveModel::Serializer attributes :name, :id, :latitude, :longitude, :is_primary_producer, :is_distributor, - :visible, :path, :icon, :icon_font, :producer_icon_font, :address_id, :sells, + :path, :icon, :icon_font, :producer_icon_font, :address_id, :sells, :permalink has_one :address, serializer: Api::AddressSerializer diff --git a/app/serializers/api/enterprise_thin_serializer.rb b/app/serializers/api/enterprise_thin_serializer.rb index 7127ab12bd..cdc4d6b9eb 100644 --- a/app/serializers/api/enterprise_thin_serializer.rb +++ b/app/serializers/api/enterprise_thin_serializer.rb @@ -1,6 +1,6 @@ module Api class EnterpriseThinSerializer < ActiveModel::Serializer - attributes :name, :id, :active, :path, :visible + attributes :name, :id, :active, :path has_one :address, serializer: Api::AddressSerializer diff --git a/app/views/producers/_fat.html.haml b/app/views/producers/_fat.html.haml index 2dc7454671..1526f4f198 100644 --- a/app/views/producers/_fat.html.haml +++ b/app/views/producers/_fat.html.haml @@ -78,7 +78,7 @@   .row.cta-container .columns.small-12 - %a.cta-hub{"ng-repeat" => "hub in producer.hubs | visible | orderBy:'-active'", + %a.cta-hub{"ng-repeat" => "hub in producer.hubs | orderBy:'-active'", "ng-href" => "{{::hub.path}}", "ng-attr-target" => "{{ embedded_layout ? '_blank' : undefined }}", "ng-class" => "::{primary: hub.active, secondary: !hub.active}"} %i.ofn-i_068-shop-reversed{"ng-if" => "::hub.active"} diff --git a/app/views/producers/index.html.haml b/app/views/producers/index.html.haml index 1558fc131c..c6b62b2725 100644 --- a/app/views/producers/index.html.haml +++ b/app/views/producers/index.html.haml @@ -16,7 +16,7 @@ .small-12.columns .active_table %producer.active_table_node.row.animate-repeat{id: "{{producer.path}}", - "ng-repeat" => "producer in filteredEnterprises = (Enterprises.producers | visible | searchEnterprises:query | taxons:activeTaxons | properties:activeProperties:'supplied_properties')", + "ng-repeat" => "producer in filteredEnterprises = (Enterprises.producers | searchEnterprises:query | taxons:activeTaxons | properties:activeProperties:'supplied_properties')", "ng-controller" => "ProducerNodeCtrl", "ng-class" => "{'closed' : !open(), 'open' : open(), 'inactive' : !producer.active}", id: "{{producer.hash}}"} diff --git a/app/views/shops/_filters.html.haml b/app/views/shops/_filters.html.haml index 33d5ac3a5e..1e62405be3 100644 --- a/app/views/shops/_filters.html.haml +++ b/app/views/shops/_filters.html.haml @@ -13,7 +13,7 @@ = t :hubs_filter_by = t :hubs_filter_type - %filter-selector.small-block-grid-2.medium-block-grid-4.large-block-grid-5{ "selector-set" => "filterSelectors", objects: "#{resource} | visible | taxonsOf", "active-selectors" => "activeTaxons" } + %filter-selector.small-block-grid-2.medium-block-grid-4.large-block-grid-5{ "selector-set" => "filterSelectors", objects: "#{resource} | taxonsOf", "active-selectors" => "activeTaxons" } .small-12.large-3.columns %h5.tdhead .light diff --git a/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee index 120e5c8032..8b393a6aab 100644 --- a/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/enterprise_spec.js.coffee @@ -20,11 +20,11 @@ describe "Enterprises service", -> {id: 1, visible: true, name: 'a', category: "hub", producers: [{id: 5}], taxons: [{id: 1}]}, {id: 2, visible: true, name: 'b', category: "hub", producers: [{id: 6}]} {id: 3, visible: true, name: 'c', category: "hub_profile"} - {id: 4, visible: false,name: 'd', category: "hub", producers: [{id: 7}]} + {id: 4, visible: true, name: 'd', category: "hub", producers: [{id: 7}]} {id: 5, visible: true, name: 'e', category: "producer_hub", hubs: [{id: 1}]}, {id: 6, visible: true, name: 'f', category: "producer_shop", hubs: [{id: 2}]}, {id: 7, visible: true, name: 'g', category: "producer", hubs: [{id: 2}]} - {id: 8, visible: false,name: 'h', category: "producer", hubs: [{id: 2}]} + {id: 8, visible: true, name: 'h', category: "producer", hubs: [{id: 2}]} ] H1: 0 beforeEach -> @@ -64,14 +64,6 @@ describe "Enterprises service", -> Enterprises.enterprises[4].active = false expect(Enterprises.producers[0].active).toBe false - it "only includes visible enterprises in hubs array", -> - expect(Enterprises.hubs).toContain Enterprises.enterprises[0] - expect(Enterprises.hubs).not.toContain Enterprises.enterprises[3] - - it "only includes visible enterprises in producers array", -> - expect(Enterprises.producers).toContain Enterprises.enterprises[4] - expect(Enterprises.producers).not.toContain Enterprises.enterprises[7] - it "includes hub, hub_profile, producer_hub and, producer_shop enterprises in hubs array", -> expect(Enterprises.hubs).toContain Enterprises.enterprises[0] expect(Enterprises.hubs).toContain Enterprises.enterprises[2] From 874fb884b8008e8a61588071d3c0e309da0842b5 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Fri, 10 May 2019 15:54:53 +0100 Subject: [PATCH 14/16] Refactor Angular controllers further --- .../group_page_controller.js.coffee | 18 +++--------------- .../controllers/hub_node_controller.js.coffee | 2 +- app/views/groups/show.html.haml | 6 +++--- app/views/producers/_filters.html.haml | 4 ++-- 4 files changed, 9 insertions(+), 21 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee index 0d2f244b32..7b44cb749b 100644 --- a/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/group_page_controller.js.coffee @@ -1,18 +1,6 @@ -Darkswarm.controller "GroupPageCtrl", ($scope, enterprises, Enterprises, MapConfiguration, OfnMap, Navigation) -> +Darkswarm.controller "GroupPageCtrl", ($scope, enterprises, Enterprises, MapConfiguration, OfnMap) -> $scope.Enterprises = Enterprises - enterprises_by_id = enterprises.map (enterprise) => - Enterprises.enterprises_by_id[enterprise.id] - - # TODO: this is duplicate code with app/assets/javascripts/darkswarm/services/enterprises.js.coffee - # It would be better to load only the needed enterprises (group + related shops). - $scope.group_producers = enterprises_by_id.filter (enterprise) -> - enterprise.category in ["producer_hub", "producer_shop", "producer"] - $scope.group_hubs = enterprises_by_id.filter (enterprise) -> - enterprise.category in ["hub", "hub_profile", "producer_hub", "producer_shop"] - - $scope.producers_to_filter = $scope.group_producers - $scope.map = angular.copy MapConfiguration.options - $scope.mapMarkers = OfnMap.enterprise_markers enterprises_by_id - $scope.embedded_layout = window.location.search.indexOf("embedded_shopfront=true") != -1 \ No newline at end of file + $scope.mapMarkers = OfnMap.enterprise_markers enterprises + $scope.embedded_layout = window.location.search.indexOf("embedded_shopfront=true") != -1 diff --git a/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee index 9b08a09139..8b0a1159e6 100644 --- a/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/hub_node_controller.js.coffee @@ -1,4 +1,4 @@ -Darkswarm.controller "HubNodeCtrl", ($scope, HashNavigation, Navigation, $location, CurrentHub, $http, $timeout) -> +Darkswarm.controller "HubNodeCtrl", ($scope, HashNavigation, CurrentHub, $http, $timeout) -> $scope.shopfront_loading = false $scope.enterprise_details = [] diff --git a/app/views/groups/show.html.haml b/app/views/groups/show.html.haml index 726aaff811..48ba7df8b2 100644 --- a/app/views/groups/show.html.haml +++ b/app/views/groups/show.html.haml @@ -62,7 +62,7 @@ .small-12.columns .active_table %producer.active_table_node.row.animate-repeat{id: "{{producer.path}}", - "ng-repeat" => "producer in filteredEnterprises = (group_producers | searchEnterprises:query | taxons:activeTaxons | properties:activeProperties:'supplied_properties')", + "ng-repeat" => "producer in filteredEnterprises = (Enterprises.producers | searchEnterprises:query | taxons:activeTaxons | properties:activeProperties:'supplied_properties')", "ng-controller" => "ProducerNodeCtrl", "ng-class" => "{'closed' : !open(), 'open' : open(), 'inactive' : !producer.active}", id: "{{producer.hash}}"} @@ -83,13 +83,13 @@ = t :groups_hubs = render "shared/components/enterprise_search" - = render "shops/filters", resource: "group_hubs", property_filters: "| searchEnterprises:query | taxons:activeTaxons | shipping:shippingTypes" + = render "shops/filters", resource: "Enterprises.hubs", property_filters: "| searchEnterprises:query | taxons:activeTaxons | shipping:shippingTypes" .row .small-12.columns .active_table %hub.active_table_node.row.animate-repeat{id: "{{hub.hash}}", - "ng-repeat" => "hub in filteredEnterprises = (group_hubs | searchEnterprises:query | taxons:activeTaxons | shipping:shippingTypes | properties:activeProperties:'distributed_properties' | orderBy:['-active', '+orders_close_at'])", + "ng-repeat" => "hub in filteredEnterprises = (Enterprises.hubs | searchEnterprises:query | taxons:activeTaxons | shipping:shippingTypes | properties:activeProperties:'distributed_properties' | orderBy:['-active', '+orders_close_at'])", "ng-class" => "{'is_profile' : hub.category == 'hub_profile', 'closed' : !open(), 'open' : open(), 'inactive' : !hub.active, 'current' : current()}", "ng-controller" => "HubNodeCtrl"} .small-12.columns diff --git a/app/views/producers/_filters.html.haml b/app/views/producers/_filters.html.haml index 17f260196a..283777cf09 100644 --- a/app/views/producers/_filters.html.haml +++ b/app/views/producers/_filters.html.haml @@ -11,13 +11,13 @@ .light = t :producers_filter = t :producers_filter_type - %filter-selector.small-block-grid-2.medium-block-grid-4.large-block-grid-6{"selector-set" => "filterSelectors", objects: "producers_to_filter | searchEnterprises:query | taxonsOf", "active-selectors" => "activeTaxons"} + %filter-selector.small-block-grid-2.medium-block-grid-4.large-block-grid-6{"selector-set" => "filterSelectors", objects: "Enterprises.producers | searchEnterprises:query | taxonsOf", "active-selectors" => "activeTaxons"} %h5.tdhead .light = t :producers_filter = t :producers_filter_property .filter-shopfront.property-selectors - %filter-selector{ "selector-set" => "filterSelectors", objects: "producers_to_filter | searchEnterprises:query | taxons:activeTaxons | propertiesOf:'supplied_properties'", "active-selectors" => "activeProperties"} + %filter-selector{ "selector-set" => "filterSelectors", objects: "Enterprises.producers | searchEnterprises:query | taxons:activeTaxons | propertiesOf:'supplied_properties'", "active-selectors" => "activeProperties"} = render partial: 'shared/components/filter_box' From 0aa8b1a30eaa1146750e72fd9141bca3a962a7a2 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sun, 12 May 2019 08:41:14 +0100 Subject: [PATCH 15/16] Make shop hidden ams injection consistent with other uses --- app/controllers/enterprises_controller.rb | 16 +--------------- app/views/enterprises/shop.html.haml | 2 +- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index e33a10a296..ebc139c461 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -17,15 +17,11 @@ class EnterprisesController < BaseController return redirect_to spree.cart_path unless enough_stock? set_noindex_meta_tag - enterprises = current_distributor + @enterprises = current_distributor .plus_relatives_and_oc_producers(shop_order_cycles) .activated .includes(address: :state) .all - - enterprises = inject_json_ams('enterprises', enterprises) - - render locals: { enterprises: enterprises } end def relatives @@ -111,14 +107,4 @@ class EnterprisesController < BaseController def set_noindex_meta_tag @noindex_meta_tag = true unless current_distributor.visible? end - - def inject_json_ams(name, object) - options = { - each_serializer: Api::EnterpriseSerializer, - data: OpenFoodNetwork::EnterpriseInjectionData.new - } - serializer_instance = ActiveModel::ArraySerializer.new(object, options) - - { name: name, json: serializer_instance.to_json } - end end diff --git a/app/views/enterprises/shop.html.haml b/app/views/enterprises/shop.html.haml index 041c8be07b..ed27346d2d 100644 --- a/app/views/enterprises/shop.html.haml +++ b/app/views/enterprises/shop.html.haml @@ -5,7 +5,7 @@ - content_for(:image) do = current_distributor.logo.url -= render partial: 'json/injection_ams', locals: enterprises += inject_enterprises(@enterprises) %shop.darkswarm - if @shopfront_layout == 'embedded' From 785595a9519d2fd5122f32991d053dbcc0c1fec7 Mon Sep 17 00:00:00 2001 From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com> Date: Sat, 11 May 2019 23:55:09 +0100 Subject: [PATCH 16/16] Reduce initial queries on shop page by ~95% --- .../tabs/about_us_controller.js.coffee | 4 ++-- .../tabs/producers_controller.js.coffee | 6 ++---- .../darkswarm/services/products.js.coffee | 4 ++-- .../darkswarm/services/shopfront.js.coffee | 8 ++++++++ app/controllers/enterprises_controller.rb | 7 ++----- app/helpers/injection_helper.rb | 8 ++++++++ .../api/enterprise_shopfront_serializer.rb | 5 ++++- app/views/enterprises/shop.html.haml | 2 +- app/views/shopping_shared/_about.html.haml | 4 ++-- app/views/shopping_shared/_producers.html.haml | 4 ++-- .../darkswarm/services/products_spec.js.coffee | 16 ++++++++++------ 11 files changed, 43 insertions(+), 25 deletions(-) create mode 100644 app/assets/javascripts/darkswarm/services/shopfront.js.coffee diff --git a/app/assets/javascripts/darkswarm/controllers/tabs/about_us_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/tabs/about_us_controller.js.coffee index c86cf12286..3fe16194f0 100644 --- a/app/assets/javascripts/darkswarm/controllers/tabs/about_us_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/tabs/about_us_controller.js.coffee @@ -1,2 +1,2 @@ -Darkswarm.controller "AboutUsCtrl", ($scope, CurrentHub) -> - $scope.CurrentHub = CurrentHub +Darkswarm.controller "AboutUsCtrl", ($scope, Shopfront) -> + $scope.shopfront = Shopfront.shopfront diff --git a/app/assets/javascripts/darkswarm/controllers/tabs/producers_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/tabs/producers_controller.js.coffee index 7a5ebb78f3..7d28868856 100644 --- a/app/assets/javascripts/darkswarm/controllers/tabs/producers_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/tabs/producers_controller.js.coffee @@ -1,4 +1,2 @@ -Darkswarm.controller "ProducersTabCtrl", ($scope, CurrentHub, Enterprises, EnterpriseModal) -> - # Injecting Enterprises so CurrentHub.producers is dereferenced. - # We should probably dereference here instead and separate out CurrentHub dereferencing from the Enterprise factory. - $scope.CurrentHub = CurrentHub +Darkswarm.controller "ProducersTabCtrl", ($scope, Shopfront, EnterpriseModal) -> + $scope.shopfront = Shopfront.shopfront diff --git a/app/assets/javascripts/darkswarm/services/products.js.coffee b/app/assets/javascripts/darkswarm/services/products.js.coffee index 6c6e222330..0a24cbd374 100644 --- a/app/assets/javascripts/darkswarm/services/products.js.coffee +++ b/app/assets/javascripts/darkswarm/services/products.js.coffee @@ -1,4 +1,4 @@ -Darkswarm.factory 'Products', ($resource, Enterprises, Dereferencer, Taxons, Properties, Cart, Variants) -> +Darkswarm.factory 'Products', ($resource, Shopfront, Dereferencer, Taxons, Properties, Cart, Variants) -> new class Products constructor: -> @update() @@ -31,7 +31,7 @@ Darkswarm.factory 'Products', ($resource, Enterprises, Dereferencer, Taxons, Pro dereference: -> for product in @products - product.supplier = Enterprises.enterprises_by_id[product.supplier.id] + product.supplier = Shopfront.producers_by_id[product.supplier.id] Dereferencer.dereference product.taxons, Taxons.taxons_by_id product.properties = angular.copy(product.properties_with_values) diff --git a/app/assets/javascripts/darkswarm/services/shopfront.js.coffee b/app/assets/javascripts/darkswarm/services/shopfront.js.coffee new file mode 100644 index 0000000000..3c4812a676 --- /dev/null +++ b/app/assets/javascripts/darkswarm/services/shopfront.js.coffee @@ -0,0 +1,8 @@ +Darkswarm.factory 'Shopfront', (shopfront) -> + new class Shopfront + shopfront: shopfront + producers_by_id: {} + + constructor: -> + for producer in shopfront.producers + @producers_by_id[producer.id] = producer diff --git a/app/controllers/enterprises_controller.rb b/app/controllers/enterprises_controller.rb index ebc139c461..18ca712c9f 100644 --- a/app/controllers/enterprises_controller.rb +++ b/app/controllers/enterprises_controller.rb @@ -4,6 +4,7 @@ class EnterprisesController < BaseController layout "darkswarm" helper Spree::ProductsHelper include OrderCyclesHelper + include SerializerHelper # These prepended filters are in the reverse order of execution prepend_before_filter :set_order_cycles, :require_distributor_chosen, :reset_order, only: :shop @@ -17,11 +18,7 @@ class EnterprisesController < BaseController return redirect_to spree.cart_path unless enough_stock? set_noindex_meta_tag - @enterprises = current_distributor - .plus_relatives_and_oc_producers(shop_order_cycles) - .activated - .includes(address: :state) - .all + @enterprise = current_distributor end def relatives diff --git a/app/helpers/injection_helper.rb b/app/helpers/injection_helper.rb index e83a1dbe36..aa85939fbe 100644 --- a/app/helpers/injection_helper.rb +++ b/app/helpers/injection_helper.rb @@ -22,6 +22,14 @@ module InjectionHelper ) end + def inject_enterprise_shopfront(enterprise) + inject_json_ams( + "shopfront", + enterprise, + Api::EnterpriseShopfrontSerializer + ) + end + def inject_enterprise_shopfront_list select_only = required_attributes Enterprise, Api::EnterpriseShopfrontListSerializer diff --git a/app/serializers/api/enterprise_shopfront_serializer.rb b/app/serializers/api/enterprise_shopfront_serializer.rb index e7b645def6..89c5e68f5b 100644 --- a/app/serializers/api/enterprise_shopfront_serializer.rb +++ b/app/serializers/api/enterprise_shopfront_serializer.rb @@ -51,7 +51,10 @@ module Api def producers ActiveModel::ArraySerializer.new( - enterprise.suppliers, each_serializer: Api::EnterpriseThinSerializer + enterprise.plus_relatives_and_oc_producers( + OrderCycle.not_closed.with_distributor(enterprise) + ), + each_serializer: Api::EnterpriseThinSerializer ) end diff --git a/app/views/enterprises/shop.html.haml b/app/views/enterprises/shop.html.haml index ed27346d2d..108ad893e9 100644 --- a/app/views/enterprises/shop.html.haml +++ b/app/views/enterprises/shop.html.haml @@ -5,7 +5,7 @@ - content_for(:image) do = current_distributor.logo.url -= inject_enterprises(@enterprises) += inject_enterprise_shopfront(@enterprise) %shop.darkswarm - if @shopfront_layout == 'embedded' diff --git a/app/views/shopping_shared/_about.html.haml b/app/views/shopping_shared/_about.html.haml index e4d03a1d2c..138f5ad44b 100644 --- a/app/views/shopping_shared/_about.html.haml +++ b/app/views/shopping_shared/_about.html.haml @@ -3,7 +3,7 @@ .panel .row .small-12.large-8.columns - %img.hero-img-small{"ng-src" => "{{::CurrentHub.hub.promo_image}}", "ng-if" => "::CurrentHub.hub.promo_image"} - %p{"ng-bind-html" => "::CurrentHub.hub.long_description"} + %img.hero-img-small{"ng-src" => "{{::shopfront.promo_image}}", "ng-if" => "::shopfront.promo_image"} + %p{"ng-bind-html" => "::shopfront.long_description"} .small-12.large-4.columns   diff --git a/app/views/shopping_shared/_producers.html.haml b/app/views/shopping_shared/_producers.html.haml index 754a26b265..ed22bebd00 100644 --- a/app/views/shopping_shared/_producers.html.haml +++ b/app/views/shopping_shared/_producers.html.haml @@ -4,9 +4,9 @@ .row .small-12.columns %h5 - = t :shopping_producers_of_hub, hub: '{{CurrentHub.hub.name}}' + = t :shopping_producers_of_hub, hub: '{{ shopfront.name }}' %ul.small-block-grid-2.large-block-grid-4 - %li{"ng-repeat" => "enterprise in CurrentHub.hub.producers"} + %li{"ng-repeat" => "enterprise in shopfront.producers"} %enterprise-modal %i.ofn-i_036-producers {{ enterprise.name }} diff --git a/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee b/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee index 62eeaf823f..0075750fba 100644 --- a/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee +++ b/spec/javascripts/unit/darkswarm/services/products_spec.js.coffee @@ -1,10 +1,10 @@ describe 'Products service', -> $httpBackend = null Products = null - Enterprises = null + Shopfront = null Variants = null Cart = null - CurrentHubMock = {} + shopfront = null currentOrder = null product = null productWithImage = null @@ -34,10 +34,14 @@ describe 'Products service', -> { id: 1, name: "some property" } taxons = { id: 2, name: "some taxon" } + shopfront = + producers: + id: 9, + name: "Test" module 'Darkswarm' module ($provide)-> - $provide.value "CurrentHub", CurrentHubMock + $provide.value "shopfront", shopfront $provide.value "currentOrder", currentOrder $provide.value "taxons", taxons $provide.value "properties", properties @@ -46,7 +50,7 @@ describe 'Products service', -> inject ($injector, _$httpBackend_)-> Products = $injector.get("Products") - Enterprises = $injector.get("Enterprises") + Shopfront = $injector.get("Shopfront") Properties = $injector.get("Properties") Variants = $injector.get("Variants") Cart = $injector.get("Cart") @@ -58,11 +62,11 @@ describe 'Products service', -> expect(Products.products[0].test).toEqual "cats" it "dereferences suppliers", -> - Enterprises.enterprises_by_id = + Shopfront.producers_by_id = {id: 9, name: "test"} $httpBackend.expectGET("/shop/products").respond([{supplier : {id: 9}, master: {}}]) $httpBackend.flush() - expect(Products.products[0].supplier).toBe Enterprises.enterprises_by_id["9"] + expect(Products.products[0].supplier).toBe Shopfront.producers_by_id["9"] it "dereferences taxons", -> product.taxons = [2]