From 462c763cd13f40e46daf43f187ee7169ba5512dc Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 6 Mar 2024 14:27:33 +1100 Subject: [PATCH 1/5] Add spree_product_uri to SuppliedProduct Also update SuppliedProductBuilder and specs --- .../app/services/supplied_product_builder.rb | 27 ++- .../lib/dfc_provider/supplied_product.rb | 9 +- .../services/supplied_product_builder_spec.rb | 194 +++++++++++++++++- swagger/dfc.yaml | 4 + 4 files changed, 226 insertions(+), 8 deletions(-) diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index ea475da1eca..84b14f53af2 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -6,6 +6,10 @@ def self.supplied_product(variant) enterprise_id: variant.product.supplier_id, id: variant.id, ) + product_uri = urls.enterprise_url( + variant.product.supplier_id, + spree_product_id: variant.product_id + ) DfcProvider::SuppliedProduct.new( id, @@ -13,16 +17,16 @@ def self.supplied_product(variant) description: variant.description, productType: product_type(variant), quantity: QuantitativeValueBuilder.quantity(variant), + spree_product_uri: product_uri, spree_product_id: variant.product.id, image_url: variant.product&.image&.url(:product) ) end def self.import_variant(supplied_product) - product_id = supplied_product.spree_product_id + product = referenced_spree_product(supplied_product) - if product_id.present? - product = Spree::Product.find(product_id) + if product Spree::Variant.new( product:, price: 0, @@ -36,6 +40,23 @@ def self.import_variant(supplied_product) end end + def self.referenced_spree_product(supplied_product) + uri = supplied_product.spree_product_uri + id = supplied_product.spree_product_id + + if uri.present? + route = Rails.application.routes.recognize_path(uri) + params = Rack::Utils.parse_nested_query(URI.parse(uri).query) + + # Check that the given URI points to us: + return unless uri == urls.enterprise_url(route.merge(params)) + + Spree::Product.find(params["spree_product_id"]) + elsif id.present? + Spree::Product.find(id) + end + end + def self.import_product(supplied_product) Spree::Product.new( name: supplied_product.name, diff --git a/engines/dfc_provider/lib/dfc_provider/supplied_product.rb b/engines/dfc_provider/lib/dfc_provider/supplied_product.rb index 8e8b2582065..f2ebc9f61fc 100644 --- a/engines/dfc_provider/lib/dfc_provider/supplied_product.rb +++ b/engines/dfc_provider/lib/dfc_provider/supplied_product.rb @@ -2,15 +2,20 @@ module DfcProvider class SuppliedProduct < DataFoodConsortium::Connector::SuppliedProduct - attr_accessor :spree_product_id, :image + attr_accessor :spree_product_id, :spree_product_uri, :image - def initialize(semantic_id, spree_product_id: nil, image_url: nil, **properties) + def initialize( + semantic_id, spree_product_id: nil, spree_product_uri: nil, image_url: nil, **properties + ) super(semantic_id, **properties) self.spree_product_id = spree_product_id + self.spree_product_uri = spree_product_uri self.image = image_url + # This is now replaced by spree_product_uri, keeping it for backward compatibility register_ofn_property("spree_product_id") + register_ofn_property("spree_product_uri") # Temporary solution, will be replaced by "dfc_b:image" in future version of the DFC connector register_ofn_property("image") end diff --git a/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb b/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb index b8e41df7428..f4e1518735c 100644 --- a/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb @@ -7,11 +7,16 @@ subject(:builder) { described_class } let(:variant) { - build(:variant, id: 5).tap do |v| - v.product.supplier_id = 7 - v.product.primary_taxon = taxon + build(:variant, id: 5, product: spree_product) + } + let(:spree_product) { + create(:product, id: 6, supplier:).tap do |p| + p.primary_taxon = taxon end } + let(:supplier) { + build(:supplier_enterprise, id: 7) + } let(:taxon) { build( :taxon, @@ -79,6 +84,14 @@ expect(product.image).to eq variant.product.image.url(:product) end + + it "assigns the product uri" do + product = builder.supplied_product(variant) + + expect(product.spree_product_uri).to eq( + "http://test.host/api/dfc/enterprises/7?spree_product_id=6" + ) + end end describe ".import_product" do @@ -130,4 +143,179 @@ end end end + + describe ".import_variant" do + let(:imported_variant) { builder.import_variant(supplied_product) } + let(:supplied_product) do + DfcProvider::SuppliedProduct.new( + "https://example.net/tomato", + name: "Tomato", + description: "Awesome tomato", + quantity: DataFoodConsortium::Connector::QuantitativeValue.new( + unit: DfcLoader.connector.MEASURES.KILOGRAM, + value: 2, + ), + productType: product_type, + ) + end + let(:product_type) { DfcLoader.connector.PRODUCT_TYPES.VEGETABLE.NON_LOCAL_VEGETABLE } + + it "creates a new Spree::Product and variant" do + expect(imported_variant).to be_a(Spree::Variant) + expect(imported_variant.id).to be_nil + + imported_product = imported_variant.product + expect(imported_product).to be_a(Spree::Product) + expect(imported_product.id).to be_nil + expect(imported_product.name).to eq("Tomato") + expect(imported_product.description).to eq("Awesome tomato") + expect(imported_product.variant_unit).to eq("weight") + end + + context "with spree_product_id supplied" do + let(:imported_variant) { builder.import_variant(supplied_product) } + + let(:supplied_product) do + DfcProvider::SuppliedProduct.new( + "https://example.net/tomato", + name: "Tomato", + description: "Better Awesome tomato", + quantity: DataFoodConsortium::Connector::QuantitativeValue.new( + unit: DfcLoader.connector.MEASURES.KILOGRAM, + value: 2, + ), + productType: product_type, + spree_product_id: variant.product.id + ) + end + let(:product_type) { DfcLoader.connector.PRODUCT_TYPES.DRINK.SOFT_DRINK } + let!(:new_taxon) { + create( + :taxon, + name: "Soft Drink", + dfc_id: "https://github.com/datafoodconsortium/taxonomies/releases/latest/download/productTypes.rdf#soft-drink" + ) + } + + it "update an existing Spree::Product" do + imported_product = imported_variant.product + expect(imported_product.id).to eq(spree_product.id) + expect(imported_product.description).to eq("Better Awesome tomato") + expect(imported_product.primary_taxon).to eq(new_taxon) + end + + it "adds a new variant" do + expect(imported_variant.id).to be_nil + expect(imported_variant.product).to eq(spree_product) + expect(imported_variant.display_name).to eq("Tomato") + expect(imported_variant.unit_value).to eq(2000) + end + end + + context "with spree_product_uri supplied" do + let(:imported_variant) { builder.import_variant(supplied_product) } + let(:product_type) { DfcLoader.connector.PRODUCT_TYPES.DRINK.SOFT_DRINK } + let!(:new_taxon) { + create( + :taxon, + name: "Soft Drink", + dfc_id: "https://github.com/datafoodconsortium/taxonomies/releases/latest/download/productTypes.rdf#soft-drink" + ) + } + + context "when spree_product_uri match the server host" do + let(:supplied_product) do + variant.save! # referenced in spree_product_id + + DfcProvider::SuppliedProduct.new( + "https://example.net/tomato", + name: "Tomato", + description: "Better Awesome tomato", + quantity: DataFoodConsortium::Connector::QuantitativeValue.new( + unit: DfcLoader.connector.MEASURES.KILOGRAM, + value: 2, + ), + productType: product_type, + spree_product_uri: "http://test.host/api/dfc/enterprises/7?spree_product_id=6" + ) + end + + it "update an existing Spree::Product" do + imported_product = imported_variant.product + expect(imported_product.id).to eq(spree_product.id) + expect(imported_product.description).to eq("Better Awesome tomato") + expect(imported_product.primary_taxon).to eq(new_taxon) + end + + it "adds a new variant" do + expect(imported_variant.id).to be_nil + expect(imported_variant.product).to eq(spree_product) + expect(imported_variant.display_name).to eq("Tomato") + expect(imported_variant.unit_value).to eq(2000) + end + end + + context "when doesn't spree_product_uri match the server host" do + let(:supplied_product) do + DfcProvider::SuppliedProduct.new( + "https://example.net/tomato", + name: "Tomato", + description: "Awesome tomato", + quantity: DataFoodConsortium::Connector::QuantitativeValue.new( + unit: DfcLoader.connector.MEASURES.KILOGRAM, + value: 2, + ), + productType: product_type, + spree_product_uri: "http://another.host/api/dfc/enterprises/10/supplied_products/50" + ) + end + + it "creates a new Spree::Product and variant" do + expect(imported_variant).to be_a(Spree::Variant) + expect(imported_variant.id).to be_nil + + imported_product = imported_variant.product + expect(imported_product).to be_a(Spree::Product) + expect(imported_product.id).to be_nil + expect(imported_product.name).to eq("Tomato") + expect(imported_product.description).to eq("Awesome tomato") + expect(imported_product.variant_unit).to eq("weight") + end + end + end + end + + describe ".referenced_spree_product" do + let(:result) { builder.referenced_spree_product(supplied_product) } + let(:supplied_product) do + DfcProvider::SuppliedProduct.new( + "https://example.net/tomato", + name: "Tomato", + ) + end + + it "returns nil when no reference is given" do + expect(result).to eq nil + end + + it "returns a product referenced by URI" do + variant.save! + supplied_product.spree_product_uri = + "http://test.host/api/dfc/enterprises/7?spree_product_id=6" + expect(result).to eq spree_product + end + + it "doesn't return a foreign product referenced by URI" do + variant.save! + supplied_product.spree_product_uri = + "http://another.host/api/dfc/enterprises/7?spree_product_id=6" + expect(result).to eq nil + end + + it "returns a product referenced by id" do + variant.save! + supplied_product.spree_product_id = "6" + expect(result).to eq spree_product + end + end end diff --git a/swagger/dfc.yaml b/swagger/dfc.yaml index 9ba35ae2726..acddec32376 100644 --- a/swagger/dfc.yaml +++ b/swagger/dfc.yaml @@ -121,6 +121,7 @@ paths: dfc-b:usageOrStorageCondition: '' dfc-b:totalTheoreticalStock: 0.0 ofn:spree_product_id: 90000 + ofn:spree_product_uri: http://test.host/api/dfc/enterprises/10000?spree_product_id=90000 - "@id": http://test.host/api/dfc/enterprises/10000/offers/10001 "@type": dfc-b:Offer dfc-b:hasPrice: 19.99 @@ -400,6 +401,7 @@ paths: dfc-b:usageOrStorageCondition: '' dfc-b:totalTheoreticalStock: 0.0 ofn:spree_product_id: 90000 + ofn:spree_product_uri: http://test.host/api/dfc/enterprises/10000?spree_product_id=90000 ofn:image: http://test.host/rails/active_storage/url/logo-white.png - "@id": http://test.host/api/dfc/enterprises/10000/catalog_items/10001 "@type": dfc-b:CatalogItem @@ -552,6 +554,7 @@ paths: dfc-b:usageOrStorageCondition: '' dfc-b:totalTheoreticalStock: 0.0 ofn:spree_product_id: 90000 + ofn:spree_product_uri: http://test.host/api/dfc/enterprises/10000?spree_product_id=142 requestBody: content: application/json: @@ -611,6 +614,7 @@ paths: dfc-b:usageOrStorageCondition: '' dfc-b:totalTheoreticalStock: 0.0 ofn:spree_product_id: 90000 + ofn:spree_product_uri: http://test.host/api/dfc/enterprises/10000?spree_product_id=90000 ofn:image: http://test.host/rails/active_storage/url/logo-white.png '404': description: not found From a4b7a8f95d25ce5da33ac6cf6dbccf0ad47a838a Mon Sep 17 00:00:00 2001 From: Gaetan Craig-Riou Date: Wed, 6 Mar 2024 16:20:32 +1100 Subject: [PATCH 2/5] Spec creating variant via DFC API --- .../spec/requests/supplied_products_spec.rb | 30 +++++++++++++++++++ swagger/dfc.yaml | 6 ++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/engines/dfc_provider/spec/requests/supplied_products_spec.rb b/engines/dfc_provider/spec/requests/supplied_products_spec.rb index 8ca59d0d847..6f22460e032 100644 --- a/engines/dfc_provider/spec/requests/supplied_products_spec.rb +++ b/engines/dfc_provider/spec/requests/supplied_products_spec.rb @@ -142,6 +142,36 @@ '"ofn:spree_product_id":90000' ) end + + context "when supplying spree_product_uri matching the host" do + it "creates a variant for the existing product" do |example| + supplied_product[:'ofn:spree_product_uri'] = + "http://test.host/api/dfc/enterprises/10000?spree_product_id=90000" + supplied_product[:'dfc-b:hasQuantity'][:'dfc-b:value'] = 6 + + expect { + submit_request(example.metadata) + product.variants.reload + } + .to change { product.variants.count }.by(1) + + # Creates a variant for existing product + variant_id = json_response["@id"].split("/").last.to_i + new_variant = Spree::Variant.find(variant_id) + expect(product.variants).to include(new_variant) + expect(new_variant.unit_value).to eq 6 + + # Insert static value to keep documentation deterministic: + response.body.gsub!( + "supplied_products/#{variant_id}", + "supplied_products/10001" + ) + .gsub!( + %r{active_storage/[0-9A-Za-z/=-]*/logo-white.png}, + "active_storage/url/logo-white.png", + ) + end + end end end end diff --git a/swagger/dfc.yaml b/swagger/dfc.yaml index acddec32376..ba441c7dd76 100644 --- a/swagger/dfc.yaml +++ b/swagger/dfc.yaml @@ -542,7 +542,7 @@ paths: "@context": https://www.datafoodconsortium.org "@id": http://test.host/api/dfc/enterprises/10000/supplied_products/10001 "@type": dfc-b:SuppliedProduct - dfc-b:name: Apple (6g) + dfc-b:name: Pesto - Apple (6g) dfc-b:description: A delicious heritage apple dfc-b:hasType: dfc-pt:non-local-vegetable dfc-b:hasQuantity: @@ -554,7 +554,8 @@ paths: dfc-b:usageOrStorageCondition: '' dfc-b:totalTheoreticalStock: 0.0 ofn:spree_product_id: 90000 - ofn:spree_product_uri: http://test.host/api/dfc/enterprises/10000?spree_product_id=142 + ofn:spree_product_uri: http://test.host/api/dfc/enterprises/10000?spree_product_id=90000 + ofn:image: http://test.host/rails/active_storage/url/logo-white.png requestBody: content: application/json: @@ -575,6 +576,7 @@ paths: dfc-b:usageOrStorageCondition: '' dfc-b:totalTheoreticalStock: 0.0 ofn:spree_product_id: 90000 + ofn:spree_product_uri: http://test.host/api/dfc/enterprises/10000?spree_product_id=90000 "/api/dfc/enterprises/{enterprise_id}/supplied_products/{id}": parameters: - name: enterprise_id From 85a47e61fdbe3c11f00e41b75ac1b9f41da93002 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 12 Mar 2024 13:02:26 +1100 Subject: [PATCH 3/5] Create variants only for own products --- .../supplied_products_controller.rb | 5 ++++- .../app/services/supplied_product_builder.rb | 10 +++++----- .../services/supplied_product_builder_spec.rb | 17 +++++++++++++---- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/engines/dfc_provider/app/controllers/dfc_provider/supplied_products_controller.rb b/engines/dfc_provider/app/controllers/dfc_provider/supplied_products_controller.rb index d4142f25d6d..6d3dce2e58c 100644 --- a/engines/dfc_provider/app/controllers/dfc_provider/supplied_products_controller.rb +++ b/engines/dfc_provider/app/controllers/dfc_provider/supplied_products_controller.rb @@ -14,7 +14,10 @@ def create return head :bad_request unless supplied_product - variant = SuppliedProductBuilder.import_variant(supplied_product) + variant = SuppliedProductBuilder.import_variant( + supplied_product, + current_enterprise, + ) product = variant.product if product.new_record? diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index 84b14f53af2..835d08261de 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -23,8 +23,8 @@ def self.supplied_product(variant) ) end - def self.import_variant(supplied_product) - product = referenced_spree_product(supplied_product) + def self.import_variant(supplied_product, supplier) + product = referenced_spree_product(supplied_product, supplier) if product Spree::Variant.new( @@ -40,7 +40,7 @@ def self.import_variant(supplied_product) end end - def self.referenced_spree_product(supplied_product) + def self.referenced_spree_product(supplied_product, supplier) uri = supplied_product.spree_product_uri id = supplied_product.spree_product_id @@ -51,9 +51,9 @@ def self.referenced_spree_product(supplied_product) # Check that the given URI points to us: return unless uri == urls.enterprise_url(route.merge(params)) - Spree::Product.find(params["spree_product_id"]) + supplier.supplied_products.find_by(id: params["spree_product_id"]) elsif id.present? - Spree::Product.find(id) + supplier.supplied_products.find_by(id:) end end diff --git a/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb b/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb index f4e1518735c..49e33e68854 100644 --- a/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb @@ -145,7 +145,7 @@ end describe ".import_variant" do - let(:imported_variant) { builder.import_variant(supplied_product) } + let(:imported_variant) { builder.import_variant(supplied_product, supplier) } let(:supplied_product) do DfcProvider::SuppliedProduct.new( "https://example.net/tomato", @@ -173,7 +173,7 @@ end context "with spree_product_id supplied" do - let(:imported_variant) { builder.import_variant(supplied_product) } + let(:imported_variant) { builder.import_variant(supplied_product, supplier) } let(:supplied_product) do DfcProvider::SuppliedProduct.new( @@ -213,7 +213,7 @@ end context "with spree_product_uri supplied" do - let(:imported_variant) { builder.import_variant(supplied_product) } + let(:imported_variant) { builder.import_variant(supplied_product, supplier) } let(:product_type) { DfcLoader.connector.PRODUCT_TYPES.DRINK.SOFT_DRINK } let!(:new_taxon) { create( @@ -286,7 +286,7 @@ end describe ".referenced_spree_product" do - let(:result) { builder.referenced_spree_product(supplied_product) } + let(:result) { builder.referenced_spree_product(supplied_product, supplier) } let(:supplied_product) do DfcProvider::SuppliedProduct.new( "https://example.net/tomato", @@ -305,6 +305,15 @@ expect(result).to eq spree_product end + it "doesn't return a product of another enterprise" do + variant.save! + create(:product, id: 8, supplier: create(:enterprise)) + + supplied_product.spree_product_uri = + "http://test.host/api/dfc/enterprises/7?spree_product_id=8" + expect(result).to eq nil + end + it "doesn't return a foreign product referenced by URI" do variant.save! supplied_product.spree_product_uri = From 1674d8ab5cd098f05cf4de462160f0e2a003243d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 12 Mar 2024 13:05:48 +1100 Subject: [PATCH 4/5] Simplify DFC product controller --- .../dfc_provider/supplied_products_controller.rb | 10 ++-------- .../app/services/supplied_product_builder.rb | 1 + 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/engines/dfc_provider/app/controllers/dfc_provider/supplied_products_controller.rb b/engines/dfc_provider/app/controllers/dfc_provider/supplied_products_controller.rb index 6d3dce2e58c..57d25c19298 100644 --- a/engines/dfc_provider/app/controllers/dfc_provider/supplied_products_controller.rb +++ b/engines/dfc_provider/app/controllers/dfc_provider/supplied_products_controller.rb @@ -20,14 +20,8 @@ def create ) product = variant.product - if product.new_record? - product.supplier = current_enterprise - product.save! - end - - if variant.new_record? - variant.save! - end + product.save! if product.new_record? + variant.save! if variant.new_record? supplied_product = SuppliedProductBuilder.supplied_product(variant) render json: DfcIo.export(supplied_product) diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index 835d08261de..4e2b3d947bf 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -35,6 +35,7 @@ def self.import_variant(supplied_product, supplier) end else product = import_product(supplied_product) + product.supplier = supplier product.ensure_standard_variant product.variants.first end From d253effc291a37ee4ae566c3ad0832085aad4131 Mon Sep 17 00:00:00 2001 From: Maikel Date: Tue, 12 Mar 2024 16:32:01 +1100 Subject: [PATCH 5/5] Fix typo in spec description Co-authored-by: Gaetan Craig-Riou <40413322+rioug@users.noreply.github.com> --- .../dfc_provider/spec/services/supplied_product_builder_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb b/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb index 49e33e68854..97c22362095 100644 --- a/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb @@ -255,7 +255,7 @@ end end - context "when doesn't spree_product_uri match the server host" do + context "when spree_product_uri doesn't match the server host" do let(:supplied_product) do DfcProvider::SuppliedProduct.new( "https://example.net/tomato",