From 242459fae43d3e48b068ca27b9f2b69d3ee01d24 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 18 Jan 2022 12:12:37 +0100 Subject: [PATCH 1/5] Create the test that actually failed and should not --- .../system/consumer/shopping/shopping_spec.rb | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/spec/system/consumer/shopping/shopping_spec.rb b/spec/system/consumer/shopping/shopping_spec.rb index e22b841bcf..2539e4476c 100644 --- a/spec/system/consumer/shopping/shopping_spec.rb +++ b/spec/system/consumer/shopping/shopping_spec.rb @@ -242,6 +242,30 @@ describe "As a consumer I want to shop with a distributor", js: true do expect(page).not_to have_content product.name end + context "when supplier uses property" do + let(:product3) { create(:simple_product, supplier: supplier, inherits_properties: false) } + + before do + add_variant_to_order_cycle(exchange, product3.variants.first) + property = create(:property, presentation: 'certified') + supplier.update!(properties: [property]) + end + + it "filters product by properties" do + visit shop_path + + expect(page).to have_content product2.name + expect(page).to have_content product3.name + + expect(page).to have_selector ".sticky-shop-filters-container .property-selectors span", text: "certified" + find(".sticky-shop-filters-container .property-selectors span", text: 'certified').click + expect(page).to have_content "Results for certified" + + expect(page).to have_content product2.name + expect(page).not_to have_content product3.name + end + end + it "returns search results for products where the search term matches one of the product's variant names" do visit shop_path fill_in "search", with: "Badg" # For variant with display_name "Badgers" From f8002ae49a27fb2600c6e2410d8613b479b9c9f5 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Tue, 18 Jan 2022 16:29:32 +0100 Subject: [PATCH 2/5] Add some controller specs that handle the supplier_property filtering with product that don't inherits properties --- .../api/v0/order_cycles_controller_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spec/controllers/api/v0/order_cycles_controller_spec.rb b/spec/controllers/api/v0/order_cycles_controller_spec.rb index 27e04ee456..e646bf3f3e 100644 --- a/spec/controllers/api/v0/order_cycles_controller_spec.rb +++ b/spec/controllers/api/v0/order_cycles_controller_spec.rb @@ -70,6 +70,25 @@ module Api expect(product_ids).to include product1.id, product2.id expect(product_ids).to_not include product3.id end + + context "with supplier properties" do + let!(:supplier_property) { create(:property, presentation: 'Certified Organic') } + let!(:supplier) { create(:supplier_enterprise, properties: [supplier_property]) } + + before do + product1.update!(supplier: supplier) + product2.update!(supplier: supplier) + product3.update!(supplier: supplier, inherits_properties: false) + end + + it "filter out the product that don't inherits from supplier properties" do + api_get :products, id: order_cycle.id, distributor: distributor.id, + q: { properties_id_or_supplier_properties_id_in_any: [supplier_property.id] } + + expect(product_ids).to include product1.id, product2.id + expect(product_ids).to_not include product3.id + end + end end context "with taxon filters" do From 572a65b324d95fab018b8a8a511fcb9c1295b3a6 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 1 Feb 2022 12:53:48 +1100 Subject: [PATCH 3/5] Create a scope to retrieve product based on its properties both inherited and own product properties Update the product spec to test the scope that concern properties --- app/models/spree/product.rb | 15 ++++++++++- spec/models/spree/product_spec.rb | 45 +++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index dc8ba0c7a9..6742ab6e34 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -32,7 +32,7 @@ module Spree searchable_attributes :supplier_id, :primary_taxon_id, :meta_keywords searchable_associations :supplier, :properties, :primary_taxon, :variants, :master - searchable_scopes :active + searchable_scopes :active, :with_properties has_many :product_option_types, dependent: :destroy # We have an after_destroy callback on Spree::ProductOptionType. However, if we @@ -70,6 +70,19 @@ module Spree has_many :stock_items, through: :variants + has_many :supplier_properties, through: :supplier, source: :properties + + scope :with_properties, ->(*property_ids) { + left_outer_joins(:product_properties). + left_outer_joins(:supplier_properties). + where(inherits_properties: true). + where(producer_properties: { property_id: property_ids }). + or( + where(spree_product_properties: { property_id: property_ids }) + ). + distinct + } + delegate_belongs_to :master, :sku, :price, :currency, :display_amount, :display_price, :weight, :height, :width, :depth, :is_master, :cost_currency, :price_in, :amount_in, :unit_value, :unit_description diff --git a/spec/models/spree/product_spec.rb b/spec/models/spree/product_spec.rb index 4ca904fa14..65781c515b 100644 --- a/spec/models/spree/product_spec.rb +++ b/spec/models/spree/product_spec.rb @@ -199,6 +199,51 @@ module Spree end end + describe "supplier properties" do + subject { create(:product) } + + it "has no supplier properties to start with" do + expect(subject.supplier_properties).to eq [] + end + + it "doesn't include product properties" do + subject.set_property("certified", "organic") + expect(subject.supplier_properties).to eq [] + end + + it "includes the supplier's properties" do + subject.supplier.set_producer_property("certified", "yes") + expect(subject.supplier_properties.map(&:presentation)).to eq ["certified"] + end + end + + describe ".with_properties scope" do + let!(:product_without_wanted_property_on_supplier) { create(:product, supplier: supplier_without_wanted_property) } + let!(:product_with_wanted_property_on_supplier) { create(:product, supplier: supplier_with_wanted_property) } + let!(:product_with_wanted_property) { create(:product, properties: [wanted_property]) } + let!(:product_without_wanted_property_property) { create(:product, properties: [unwanted_property]) } + let!(:product_with_wanted_property_and_on_supplier) { create(:product, properties: [wanted_property], supplier: supplier_with_wanted_property) } + let!(:product_ignoring_property) { create(:product, supplier: supplier_with_wanted_property, inherits_properties: false) } + let(:supplier_with_wanted_property) { create(:supplier_enterprise, properties: [wanted_property]) } + let(:supplier_without_wanted_property) { create(:supplier_enterprise, properties: [unwanted_property]) } + let(:wanted_property) { create(:property, presentation: 'Certified Organic') } + let(:unwanted_property) { create(:property, presentation: 'Latest Hype') } + + it "returns no products without a property id" do + expect(Spree::Product.with_properties([])).to eq [] + end + + it "returns only products with the wanted property set both on supplier and on the product itself" do + expect( + Spree::Product.with_properties([wanted_property.id]) + ).to match_array [ + product_with_wanted_property_on_supplier, + product_with_wanted_property, + product_with_wanted_property_and_on_supplier + ] + end + end + # Regression tests for Spree #2352 context "classifications and taxons" do it "is joined through classifications" do From dfe8ed26d92bc9463a8fff95bd57224dbdec5189 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Wed, 2 Feb 2022 09:57:23 +0100 Subject: [PATCH 4/5] Update tests and use ransack to search with the new scope: with_properties --- app/controllers/api/v0/order_cycles_controller.rb | 2 +- spec/controllers/api/v0/order_cycles_controller_spec.rb | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/controllers/api/v0/order_cycles_controller.rb b/app/controllers/api/v0/order_cycles_controller.rb index 110edc7920..50ec014f97 100644 --- a/app/controllers/api/v0/order_cycles_controller.rb +++ b/app/controllers/api/v0/order_cycles_controller.rb @@ -81,7 +81,7 @@ module Api def permitted_ransack_params [:name_or_meta_keywords_or_variants_display_as_or_variants_display_name_or_supplier_name_cont, - :properties_id_or_supplier_properties_id_in_any, + :properties_id_or_supplier_properties_id_in_any, :with_properties, :primary_taxon_id_in_any] end diff --git a/spec/controllers/api/v0/order_cycles_controller_spec.rb b/spec/controllers/api/v0/order_cycles_controller_spec.rb index e646bf3f3e..8faac2bd71 100644 --- a/spec/controllers/api/v0/order_cycles_controller_spec.rb +++ b/spec/controllers/api/v0/order_cycles_controller_spec.rb @@ -65,8 +65,9 @@ module Api context "with property filters" do it "filters by product property" do api_get :products, id: order_cycle.id, distributor: distributor.id, - q: { properties_id_or_supplier_properties_id_in_any: [property1.id, property2.id] } + q: { with_properties: [property1.id, property2.id] } + expect(response.status).to eq 200 expect(product_ids).to include product1.id, product2.id expect(product_ids).to_not include product3.id end @@ -83,8 +84,9 @@ module Api it "filter out the product that don't inherits from supplier properties" do api_get :products, id: order_cycle.id, distributor: distributor.id, - q: { properties_id_or_supplier_properties_id_in_any: [supplier_property.id] } + q: { with_properties: [supplier_property.id] } + expect(response.status).to eq 200 expect(product_ids).to include product1.id, product2.id expect(product_ids).to_not include product3.id end From 4a3e0ea9b9d2390dffa1a629469e352f5b6110c7 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Bellet Date: Thu, 3 Feb 2022 09:36:22 +0100 Subject: [PATCH 5/5] Use the right scope to retrieve products based on properties --- .../darkswarm/controllers/products_controller.js.coffee | 2 +- app/controllers/api/v0/order_cycles_controller.rb | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee b/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee index d2f18b6362..521859f7ab 100644 --- a/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee +++ b/app/assets/javascripts/darkswarm/controllers/products_controller.js.coffee @@ -67,7 +67,7 @@ angular.module('Darkswarm').controller "ProductsCtrl", ($scope, $sce, $filter, $ page: page || $scope.page, per_page: $scope.per_page, 'q[name_or_meta_keywords_or_variants_display_as_or_variants_display_name_or_supplier_name_cont]': $scope.query, - 'q[properties_id_or_supplier_properties_id_in_any][]': $scope.activeProperties, + 'q[with_properties][]': $scope.activeProperties, 'q[primary_taxon_id_in_any][]': $scope.activeTaxons } diff --git a/app/controllers/api/v0/order_cycles_controller.rb b/app/controllers/api/v0/order_cycles_controller.rb index 50ec014f97..e1c39b2a47 100644 --- a/app/controllers/api/v0/order_cycles_controller.rb +++ b/app/controllers/api/v0/order_cycles_controller.rb @@ -81,8 +81,7 @@ module Api def permitted_ransack_params [:name_or_meta_keywords_or_variants_display_as_or_variants_display_name_or_supplier_name_cont, - :properties_id_or_supplier_properties_id_in_any, :with_properties, - :primary_taxon_id_in_any] + :with_properties, :primary_taxon_id_in_any] end def distributor