From 35d7bf7a3bdf91ab61f32aeee99bf7d183a504bd Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 23 Jan 2025 11:07:24 +1100 Subject: [PATCH 01/13] Add dfc-b:isVariantOf to supplied products --- .../dfc_provider/app/services/supplied_product_builder.rb | 5 +++++ engines/dfc_provider/config/routes.rb | 1 + engines/dfc_provider/spec/requests/supplied_products_spec.rb | 1 + swagger/dfc.yaml | 4 ++++ 4 files changed, 11 insertions(+) diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index ff567ff52b..5ab8f2a686 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -10,6 +10,10 @@ class SuppliedProductBuilder < DfcBuilder variant.supplier_id, spree_product_id: variant.product_id ) + product_group_id = urls.enterprise_product_group_url( + enterprise_id: variant.supplier_id, + id: variant.product_id, + ) DfcProvider::SuppliedProduct.new( id, @@ -17,6 +21,7 @@ class SuppliedProductBuilder < DfcBuilder description: variant.description, productType: product_type(variant), quantity: QuantitativeValueBuilder.quantity(variant), + isVariantOf: [product_group_id], spree_product_uri: product_uri, spree_product_id: variant.product.id, image_url: variant.product&.image&.url(:product) diff --git a/engines/dfc_provider/config/routes.rb b/engines/dfc_provider/config/routes.rb index 8fc0850729..a5b5b0ceff 100644 --- a/engines/dfc_provider/config/routes.rb +++ b/engines/dfc_provider/config/routes.rb @@ -6,6 +6,7 @@ DfcProvider::Engine.routes.draw do resources :catalog_items, only: [:index, :show, :update] resources :offers, only: [:show, :update] resources :supplied_products, only: [:create, :show, :update] + resources :product_groups, only: [:show] resources :social_medias, only: [:show] end resources :enterprise_groups, only: [:index, :show] do diff --git a/engines/dfc_provider/spec/requests/supplied_products_spec.rb b/engines/dfc_provider/spec/requests/supplied_products_spec.rb index cb707b3fdb..b16c6bb6e5 100644 --- a/engines/dfc_provider/spec/requests/supplied_products_spec.rb +++ b/engines/dfc_provider/spec/requests/supplied_products_spec.rb @@ -191,6 +191,7 @@ RSpec.describe "SuppliedProducts", type: :request, swagger_doc: "dfc.yaml", rswa run_test! do expect(response.body).to include variant.name + expect(json_response["dfc-b:isVariantOf"]).to eq "http://test.host/api/dfc/enterprises/10000/product_groups/90000" expect(json_response["ofn:spree_product_id"]).to eq 90_000 expect(json_response["dfc-b:hasType"]).to eq("dfc-pt:processed-vegetable") expect(json_response["ofn:image"]).to include("logo-white.png") diff --git a/swagger/dfc.yaml b/swagger/dfc.yaml index 0724baad61..f49e7180b2 100644 --- a/swagger/dfc.yaml +++ b/swagger/dfc.yaml @@ -177,6 +177,7 @@ paths: "@type": dfc-b:QuantitativeValue dfc-b:hasUnit: dfc-m:Gram dfc-b:value: 1.0 + dfc-b:isVariantOf: http://test.host/api/dfc/enterprises/10000/product_groups/90000 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 @@ -463,6 +464,7 @@ paths: dfc-b:hasUnit: dfc-m:Gram dfc-b:value: 1.0 dfc-b:image: http://test.host/rails/active_storage/url/logo-white.png + dfc-b:isVariantOf: http://test.host/api/dfc/enterprises/10000/product_groups/90000 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 @@ -615,6 +617,7 @@ paths: "@type": dfc-b:QuantitativeValue dfc-b:hasUnit: dfc-m:Gram dfc-b:value: 6.0 + dfc-b:isVariantOf: http://test.host/api/dfc/enterprises/10000/product_groups/90000 ofn:spree_product_id: 90000 ofn:spree_product_uri: http://test.host/api/dfc/enterprises/10000?spree_product_id=90000 dfc-b:image: http://test.host/rails/active_storage/url/logo-white.png @@ -675,6 +678,7 @@ paths: dfc-b:hasUnit: dfc-m:Gram dfc-b:value: 1.0 dfc-b:image: http://test.host/rails/active_storage/url/logo-white.png + dfc-b:isVariantOf: http://test.host/api/dfc/enterprises/10000/product_groups/90000 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 From d39da6d0da8f67bc6ab7d5cab7b19793a2d1989c Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 23 Jan 2025 12:14:57 +1100 Subject: [PATCH 02/13] Provide endpoint to show a product group Our Spree::Product corresponds to a DFC SuppliedProduct with variants. --- .../dfc_provider/product_groups_controller.rb | 14 ++++++ .../app/services/product_group_builder.rb | 17 +++++++ .../app/services/supplied_product_builder.rb | 9 ++-- .../spec/requests/product_groups_spec.rb | 49 +++++++++++++++++++ swagger/dfc.yaml | 28 +++++++++++ 5 files changed, 114 insertions(+), 3 deletions(-) create mode 100644 engines/dfc_provider/app/controllers/dfc_provider/product_groups_controller.rb create mode 100644 engines/dfc_provider/app/services/product_group_builder.rb create mode 100644 engines/dfc_provider/spec/requests/product_groups_spec.rb diff --git a/engines/dfc_provider/app/controllers/dfc_provider/product_groups_controller.rb b/engines/dfc_provider/app/controllers/dfc_provider/product_groups_controller.rb new file mode 100644 index 0000000000..ab527381ba --- /dev/null +++ b/engines/dfc_provider/app/controllers/dfc_provider/product_groups_controller.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# Show Spree::Product as SuppliedProduct with variants. +module DfcProvider + class ProductGroupsController < DfcProvider::ApplicationController + before_action :check_enterprise + + def show + spree_product = current_enterprise.supplied_products.find(params[:id]) + product = ProductGroupBuilder.product_group(spree_product) + render json: DfcIo.export(product) + end + end +end diff --git a/engines/dfc_provider/app/services/product_group_builder.rb b/engines/dfc_provider/app/services/product_group_builder.rb new file mode 100644 index 0000000000..fd25b21f84 --- /dev/null +++ b/engines/dfc_provider/app/services/product_group_builder.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class ProductGroupBuilder < DfcBuilder + def self.product_group(product) + id = urls.enterprise_product_group_url( + enterprise_id: product.variants.first.supplier_id, + id: product.id, + ) + variants = product.variants.map do |spree_variant| + SuppliedProductBuilder.semantic_id(spree_variant) + end + + DataFoodConsortium::Connector::SuppliedProduct.new( + id, variants:, + ) + end +end diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index 5ab8f2a686..c442302bc8 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -1,11 +1,14 @@ # frozen_string_literal: true class SuppliedProductBuilder < DfcBuilder - def self.supplied_product(variant) - id = urls.enterprise_supplied_product_url( + def self.semantic_id(variant) + urls.enterprise_supplied_product_url( enterprise_id: variant.supplier_id, id: variant.id, ) + end + + def self.supplied_product(variant) product_uri = urls.enterprise_url( variant.supplier_id, spree_product_id: variant.product_id @@ -16,7 +19,7 @@ class SuppliedProductBuilder < DfcBuilder ) DfcProvider::SuppliedProduct.new( - id, + semantic_id(variant), name: variant.product_and_full_name, description: variant.description, productType: product_type(variant), diff --git a/engines/dfc_provider/spec/requests/product_groups_spec.rb b/engines/dfc_provider/spec/requests/product_groups_spec.rb new file mode 100644 index 0000000000..b3d1f3dbff --- /dev/null +++ b/engines/dfc_provider/spec/requests/product_groups_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require_relative "../swagger_helper" + +RSpec.describe "ProductGroups", swagger_doc: "dfc.yaml", rswag_autodoc: true do + let!(:user) { create(:oidc_user) } + let!(:enterprise) { create(:distributor_enterprise, id: 10_000, owner: user) } + let!(:product) { + create( + :product_with_image, + id: 90_000, + name: "Pesto", description: "Basil Pesto", + variants: [variant] + ) + } + let(:variant) { + build(:base_variant, id: 10_001, unit_value: 1, primary_taxon: taxon, supplier: enterprise) + } + let(:taxon) { + build( + :taxon, + name: "Processed Vegetable", + dfc_id: "https://github.com/datafoodconsortium/taxonomies/releases/latest/download/productTypes.rdf#processed-vegetable" + ) + } + + before { login_as user } + + path "/api/dfc/enterprises/{enterprise_id}/product_groups/{id}" do + parameter name: :enterprise_id, in: :path, type: :string + parameter name: :id, in: :path, type: :string + + let(:enterprise_id) { enterprise.id } + + get "Show ProductGroup" do + produces "application/json" + + response "200", "success" do + let(:id) { product.id } + + run_test! do + expect(json_response["@id"]).to eq "http://test.host/api/dfc/enterprises/10000/product_groups/90000" + + expect(json_response["dfc-b:hasVariant"]).to eq "http://test.host/api/dfc/enterprises/10000/supplied_products/10001" + end + end + end + end +end diff --git a/swagger/dfc.yaml b/swagger/dfc.yaml index f49e7180b2..b40e8ab013 100644 --- a/swagger/dfc.yaml +++ b/swagger/dfc.yaml @@ -554,6 +554,34 @@ paths: "@type": dfc-b:Person '404': description: not found + "/api/dfc/enterprises/{enterprise_id}/product_groups/{id}": + parameters: + - name: enterprise_id + in: path + required: true + schema: + type: string + - name: id + in: path + required: true + schema: + type: string + get: + summary: Show ProductGroup + tags: + - ProductGroups + responses: + '200': + description: success + content: + application/json: + examples: + test_example: + value: + "@context": https://www.datafoodconsortium.org + "@id": http://test.host/api/dfc/enterprises/10000/product_groups/90000 + "@type": dfc-b:SuppliedProduct + dfc-b:hasVariant: http://test.host/api/dfc/enterprises/10000/supplied_products/10001 "/api/dfc/enterprises/{enterprise_id}/social_medias/{name}": get: summary: Show social media From 42b6ecbf319e23b70ee9c973e93553e8208445f3 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 23 Jan 2025 12:41:12 +1100 Subject: [PATCH 03/13] Move rswag specifc config to rswag helper --- spec/base_spec_helper.rb | 23 ----------------------- spec/swagger_helper.rb | 23 +++++++++++++++++++++++ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/spec/base_spec_helper.rb b/spec/base_spec_helper.rb index f8b326003a..f8c30b3f07 100644 --- a/spec/base_spec_helper.rb +++ b/spec/base_spec_helper.rb @@ -177,29 +177,6 @@ RSpec.configure do |config| ActionController::Base.perform_caching = caching end - # Take example responses from Rswag specs for API documentation. - # https://github.com/rswag/rswag#enable-auto-generation-examples-from-responses - config.after(:each, :rswag_autodoc) do |example| - # Categories and group operations of the same API endpoint. - example.metadata[:operation][:tags] ||= [self.class.top_level_description] - - next if response&.body.blank? - - # Include response as example in the documentation. - example.metadata[:response][:content] ||= {} - example.metadata[:response][:content].deep_merge!( - { - "application/json" => { - examples: { - test_example: { - value: JSON.parse(response.body, symbolize_names: true) - } - } - } - } - ) - end - # Show javascript errors in test output with `js_debug: true` config.after(:each, :js_debug) do errors = page.driver.browser.manage.logs.get(:browser) diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index b3a3676cbc..1e44212f11 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -71,6 +71,29 @@ RSpec.configure do |config| # the key, this may want to be changed to avoid putting yaml in json files. # Defaults to json. Accepts ':json' and ':yaml'. config.openapi_format = :yaml + + # Take example responses from Rswag specs for API documentation. + # https://github.com/rswag/rswag#enable-auto-generation-examples-from-responses + config.after(:each, :rswag_autodoc) do |example| + # Categories and group operations of the same API endpoint. + example.metadata[:operation][:tags] ||= [self.class.top_level_description] + + next if response&.body.blank? + + # Include response as example in the documentation. + example.metadata[:response][:content] ||= {} + example.metadata[:response][:content].deep_merge!( + { + "application/json" => { + examples: { + test_example: { + value: JSON.parse(response.body, symbolize_names: true) + } + } + } + } + ) + end end module RswagExtension From 67075162035fa6089c44d00df09acb749a649da1 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 23 Jan 2025 12:44:37 +1100 Subject: [PATCH 04/13] Automatically document responses for DFC API w/o tag --- engines/dfc_provider/spec/requests/addresses_spec.rb | 2 +- .../dfc_provider/spec/requests/affiliate_sales_data_spec.rb | 2 +- engines/dfc_provider/spec/requests/catalog_items_spec.rb | 3 +-- .../spec/requests/enterprise_groups/affiliated_by_spec.rb | 3 +-- engines/dfc_provider/spec/requests/enterprise_groups_spec.rb | 2 +- engines/dfc_provider/spec/requests/enterprises_spec.rb | 2 +- engines/dfc_provider/spec/requests/offers_spec.rb | 2 +- engines/dfc_provider/spec/requests/persons_spec.rb | 2 +- engines/dfc_provider/spec/requests/product_groups_spec.rb | 2 +- engines/dfc_provider/spec/requests/social_medias_spec.rb | 2 +- engines/dfc_provider/spec/requests/supplied_products_spec.rb | 2 +- spec/swagger_helper.rb | 2 +- 12 files changed, 12 insertions(+), 14 deletions(-) diff --git a/engines/dfc_provider/spec/requests/addresses_spec.rb b/engines/dfc_provider/spec/requests/addresses_spec.rb index 9e8693b03c..7c06914078 100644 --- a/engines/dfc_provider/spec/requests/addresses_spec.rb +++ b/engines/dfc_provider/spec/requests/addresses_spec.rb @@ -2,7 +2,7 @@ require_relative "../swagger_helper" -RSpec.describe "Addresses", type: :request, swagger_doc: "dfc.yaml", rswag_autodoc: true do +RSpec.describe "Addresses", type: :request, swagger_doc: "dfc.yaml" do let(:user) { create(:oidc_user) } let(:address) { create(:address, id: 40_000) } let(:result) { json_response } diff --git a/engines/dfc_provider/spec/requests/affiliate_sales_data_spec.rb b/engines/dfc_provider/spec/requests/affiliate_sales_data_spec.rb index e3ca35e8ad..6e736fbaae 100644 --- a/engines/dfc_provider/spec/requests/affiliate_sales_data_spec.rb +++ b/engines/dfc_provider/spec/requests/affiliate_sales_data_spec.rb @@ -2,7 +2,7 @@ require_relative "../swagger_helper" -RSpec.describe "AffiliateSalesData", swagger_doc: "dfc.yaml", rswag_autodoc: true do +RSpec.describe "AffiliateSalesData", swagger_doc: "dfc.yaml" do let(:user) { create(:oidc_user) } before { login_as user } diff --git a/engines/dfc_provider/spec/requests/catalog_items_spec.rb b/engines/dfc_provider/spec/requests/catalog_items_spec.rb index 03e1ba468a..a48f6c0eba 100644 --- a/engines/dfc_provider/spec/requests/catalog_items_spec.rb +++ b/engines/dfc_provider/spec/requests/catalog_items_spec.rb @@ -2,8 +2,7 @@ require_relative "../swagger_helper" -RSpec.describe "CatalogItems", type: :request, swagger_doc: "dfc.yaml", - rswag_autodoc: true do +RSpec.describe "CatalogItems", type: :request, swagger_doc: "dfc.yaml" do let(:user) { create(:oidc_user, id: 12_345) } let(:enterprise) { create( diff --git a/engines/dfc_provider/spec/requests/enterprise_groups/affiliated_by_spec.rb b/engines/dfc_provider/spec/requests/enterprise_groups/affiliated_by_spec.rb index fdee727fa8..e6ef549213 100644 --- a/engines/dfc_provider/spec/requests/enterprise_groups/affiliated_by_spec.rb +++ b/engines/dfc_provider/spec/requests/enterprise_groups/affiliated_by_spec.rb @@ -2,8 +2,7 @@ require_relative "../../swagger_helper" -RSpec.describe "EnterpriseGroups::AffiliatedBy", type: :request, swagger_doc: "dfc.yaml", - rswag_autodoc: true do +RSpec.describe "EnterpriseGroups::AffiliatedBy", type: :request, swagger_doc: "dfc.yaml" do let(:user) { create(:oidc_user, id: 12_345) } let(:group) { create( diff --git a/engines/dfc_provider/spec/requests/enterprise_groups_spec.rb b/engines/dfc_provider/spec/requests/enterprise_groups_spec.rb index 8b3f161893..4836a689f5 100644 --- a/engines/dfc_provider/spec/requests/enterprise_groups_spec.rb +++ b/engines/dfc_provider/spec/requests/enterprise_groups_spec.rb @@ -2,7 +2,7 @@ require_relative "../swagger_helper" -RSpec.describe "EnterpriseGroups", type: :request, swagger_doc: "dfc.yaml", rswag_autodoc: true do +RSpec.describe "EnterpriseGroups", type: :request, swagger_doc: "dfc.yaml" do let(:user) { create(:oidc_user, id: 12_345) } let(:group) { create( diff --git a/engines/dfc_provider/spec/requests/enterprises_spec.rb b/engines/dfc_provider/spec/requests/enterprises_spec.rb index 13c9f249be..254d04fc11 100644 --- a/engines/dfc_provider/spec/requests/enterprises_spec.rb +++ b/engines/dfc_provider/spec/requests/enterprises_spec.rb @@ -2,7 +2,7 @@ require_relative "../swagger_helper" -RSpec.describe "Enterprises", type: :request, swagger_doc: "dfc.yaml", rswag_autodoc: true do +RSpec.describe "Enterprises", type: :request, swagger_doc: "dfc.yaml" do let!(:user) { create(:oidc_user) } let!(:enterprise) do create( diff --git a/engines/dfc_provider/spec/requests/offers_spec.rb b/engines/dfc_provider/spec/requests/offers_spec.rb index 20c0780489..8e3ffe8dab 100644 --- a/engines/dfc_provider/spec/requests/offers_spec.rb +++ b/engines/dfc_provider/spec/requests/offers_spec.rb @@ -2,7 +2,7 @@ require_relative "../swagger_helper" -RSpec.describe "Offers", type: :request, swagger_doc: "dfc.yaml", rswag_autodoc: true do +RSpec.describe "Offers", type: :request, swagger_doc: "dfc.yaml" do let!(:user) { create(:oidc_user) } let!(:enterprise) { create(:distributor_enterprise, id: 10_000, owner: user) } let!(:product) { diff --git a/engines/dfc_provider/spec/requests/persons_spec.rb b/engines/dfc_provider/spec/requests/persons_spec.rb index a7ef52c0c4..c6379affd7 100644 --- a/engines/dfc_provider/spec/requests/persons_spec.rb +++ b/engines/dfc_provider/spec/requests/persons_spec.rb @@ -2,7 +2,7 @@ require_relative "../swagger_helper" -RSpec.describe "Persons", type: :request, swagger_doc: "dfc.yaml", rswag_autodoc: true do +RSpec.describe "Persons", type: :request, swagger_doc: "dfc.yaml" do let(:user) { create(:oidc_user, id: 10_000) } let(:other_user) { create(:oidc_user) } diff --git a/engines/dfc_provider/spec/requests/product_groups_spec.rb b/engines/dfc_provider/spec/requests/product_groups_spec.rb index b3d1f3dbff..56aab98377 100644 --- a/engines/dfc_provider/spec/requests/product_groups_spec.rb +++ b/engines/dfc_provider/spec/requests/product_groups_spec.rb @@ -2,7 +2,7 @@ require_relative "../swagger_helper" -RSpec.describe "ProductGroups", swagger_doc: "dfc.yaml", rswag_autodoc: true do +RSpec.describe "ProductGroups", swagger_doc: "dfc.yaml" do let!(:user) { create(:oidc_user) } let!(:enterprise) { create(:distributor_enterprise, id: 10_000, owner: user) } let!(:product) { diff --git a/engines/dfc_provider/spec/requests/social_medias_spec.rb b/engines/dfc_provider/spec/requests/social_medias_spec.rb index 71e7bfdbee..32c479f593 100644 --- a/engines/dfc_provider/spec/requests/social_medias_spec.rb +++ b/engines/dfc_provider/spec/requests/social_medias_spec.rb @@ -2,7 +2,7 @@ require_relative "../swagger_helper" -RSpec.describe "SocialMedias", type: :request, swagger_doc: "dfc.yaml", rswag_autodoc: true do +RSpec.describe "SocialMedias", type: :request, swagger_doc: "dfc.yaml" do let(:user) { create(:oidc_user) } let(:enterprise) do create( diff --git a/engines/dfc_provider/spec/requests/supplied_products_spec.rb b/engines/dfc_provider/spec/requests/supplied_products_spec.rb index b16c6bb6e5..2f7855d20d 100644 --- a/engines/dfc_provider/spec/requests/supplied_products_spec.rb +++ b/engines/dfc_provider/spec/requests/supplied_products_spec.rb @@ -2,7 +2,7 @@ require_relative "../swagger_helper" -RSpec.describe "SuppliedProducts", type: :request, swagger_doc: "dfc.yaml", rswag_autodoc: true do +RSpec.describe "SuppliedProducts", type: :request, swagger_doc: "dfc.yaml" do let!(:user) { create(:oidc_user) } let!(:enterprise) { create(:distributor_enterprise, id: 10_000, owner: user) } let!(:product) { diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index 1e44212f11..28dbd93d0d 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -74,7 +74,7 @@ RSpec.configure do |config| # Take example responses from Rswag specs for API documentation. # https://github.com/rswag/rswag#enable-auto-generation-examples-from-responses - config.after(:each, :rswag_autodoc) do |example| + config.after(:each, swagger_doc: "dfc.yaml") do |example| # Categories and group operations of the same API endpoint. example.metadata[:operation][:tags] ||= [self.class.top_level_description] From 2043d1f8df60c704af281616dd87a3f9e6116411 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 23 Jan 2025 12:53:13 +1100 Subject: [PATCH 05/13] Remove custom syntactical sugar It just makes Rswag specs look more different to other request specs and I found that discouraging. It's good to know that the parameter is just specified with `let` and that it works exactly in the same way as `let` in other specs. The downside is maybe that it's not obvious that those `let` statements have to correspond with the parameters for the request but error messages will tell you if you got it wrong. And there's also the `parameter` declaration to make that clear. --- spec/requests/api/v1/customers_spec.rb | 30 +++++++++++++------------- spec/swagger_helper.rb | 7 ------ 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/spec/requests/api/v1/customers_spec.rb b/spec/requests/api/v1/customers_spec.rb index 1259e99e03..9af5ad71d1 100644 --- a/spec/requests/api/v1/customers_spec.rb +++ b/spec/requests/api/v1/customers_spec.rb @@ -33,8 +33,8 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap produces "application/json" response "200", "Customers list" do - param(:enterprise_id) { enterprise1.id } - param("extra_fields[customer]") { :balance } + let(:enterprise_id) { enterprise1.id } + let("extra_fields[customer]") { :balance } schema '$ref': "#/components/schemas/customers_collection" run_test! @@ -176,7 +176,7 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap } response "201", "Minimal customer created" do - param(:customer) do + let(:customer) do { email: "test@example.com", enterprise_id: enterprise1.id.to_s @@ -196,7 +196,7 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap end response "201", "Example customer created" do - param(:customer) do + let(:customer) do CustomerSchema.writable_attributes.transform_values do |attribute| attribute[:example] end.merge( @@ -219,7 +219,7 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap end response "422", "Unpermitted parameter" do - param(:customer) do + let(:customer) do { email: "test@example.com", enterprise_id: enterprise1.id.to_s, @@ -234,7 +234,7 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap end response "422", "Unprocessable entity" do - param(:customer) { {} } + let(:customer) { {} } schema '$ref': "#/components/schemas/error_response" run_test! do @@ -252,7 +252,7 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap produces "application/json" response "200", "Customer" do - param(:id) { customer1.id } + let(:id) { customer1.id } schema CustomerSchema.schema( require_all: true, extra_fields: { name: :balance, required: true } @@ -269,7 +269,7 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap end response "404", "Not found" do - param(:id) { 0 } + let(:id) { 0 } schema '$ref': "#/components/schemas/error_response" run_test! do @@ -281,7 +281,7 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap before { logout } response "401", "Unauthorized" do - param(:id) { customer1.id } + let(:id) { customer1.id } schema '$ref': "#/components/schemas/error_response" run_test! do @@ -350,8 +350,8 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap } response "200", "Customer updated" do - param(:id) { customer1.id } - param(:customer) do + let(:id) { customer1.id } + let(:customer) do { email: "test@example.com", enterprise_id: enterprise1.id.to_s @@ -421,8 +421,8 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap end response "422", "Unprocessable entity" do - param(:id) { customer1.id } - param(:customer) { {} } + let(:id) { customer1.id } + let(:customer) { {} } schema '$ref': "#/components/schemas/error_response" run_test! @@ -435,7 +435,7 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap produces "application/json" response "200", "Customer deleted" do - param(:id) { customer1.id } + let(:id) { customer1.id } schema '$ref': "#/components/schemas/customer" run_test! @@ -450,7 +450,7 @@ RSpec.describe "Customers", type: :request, swagger_doc: "v1.yaml", feature: :ap produces "application/json" response "200", "Customers list" do - param(:enterprise_id) { enterprise1.id } + let(:enterprise_id) { enterprise1.id } schema '$ref': "#/components/schemas/customers_collection" run_test! diff --git a/spec/swagger_helper.rb b/spec/swagger_helper.rb index 28dbd93d0d..266cf47311 100644 --- a/spec/swagger_helper.rb +++ b/spec/swagger_helper.rb @@ -95,10 +95,3 @@ RSpec.configure do |config| ) end end - -module RswagExtension - def param(args, &) - let(args) { instance_eval(&) } - end -end -Rswag::Specs::ExampleGroupHelpers.prepend RswagExtension From fbdc6c9bd0a4e002c8ccd4e879bb6b151ba3998a Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 23 Jan 2025 16:22:24 +1100 Subject: [PATCH 06/13] Split growing supplied product builder --- .../admin/dfc_product_imports_controller.rb | 4 +- .../supplied_products_controller.rb | 4 +- .../app/services/supplied_product_builder.rb | 87 +----- .../app/services/supplied_product_importer.rb | 90 ++++++ .../services/supplied_product_builder_spec.rb | 268 ---------------- .../supplied_product_importer_spec.rb | 293 ++++++++++++++++++ 6 files changed, 388 insertions(+), 358 deletions(-) create mode 100644 engines/dfc_provider/app/services/supplied_product_importer.rb create mode 100644 engines/dfc_provider/spec/services/supplied_product_importer_spec.rb diff --git a/app/controllers/admin/dfc_product_imports_controller.rb b/app/controllers/admin/dfc_product_imports_controller.rb index 0c545f37bf..8b7bb07e10 100644 --- a/app/controllers/admin/dfc_product_imports_controller.rb +++ b/app/controllers/admin/dfc_product_imports_controller.rb @@ -48,9 +48,9 @@ module Admin existing_variant = @enterprise.supplied_variants.linked_to(semantic_id) if existing_variant - SuppliedProductBuilder.update_product(subject, existing_variant) + SuppliedProductImporter.update_product(subject, existing_variant) else - SuppliedProductBuilder.store_product(subject, @enterprise) + SuppliedProductImporter.store_product(subject, @enterprise) end end 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 438c9c86a3..51857b80b4 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,7 @@ module DfcProvider return head :bad_request unless supplied_product - variant = SuppliedProductBuilder.store_product( + variant = SuppliedProductImporter.store_product( supplied_product, current_enterprise, ) @@ -33,7 +33,7 @@ module DfcProvider return head :bad_request unless supplied_product - SuppliedProductBuilder.update_product(supplied_product, variant) + SuppliedProductImporter.update_product(supplied_product, variant) end private diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index c442302bc8..63398b3f89 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -31,96 +31,11 @@ class SuppliedProductBuilder < DfcBuilder ) end - def self.store_product(subject, enterprise) - return unless subject.is_a? DataFoodConsortium::Connector::SuppliedProduct - - variant = SuppliedProductBuilder.import_variant(subject, enterprise) - product = variant.product - - product.save! if product.new_record? - variant.save! if variant.new_record? - - variant - end - - def self.update_product(supplied_product, variant) - apply(supplied_product, variant) - - variant.product.save! - variant.save! - - variant - end - - def self.import_variant(supplied_product, supplier) - product = referenced_spree_product(supplied_product, supplier) - - if product - Spree::Variant.new( product:, supplier:, price: 0,).tap do |variant| - apply(supplied_product, variant) - end - else - product = import_product(supplied_product, supplier) - product.variants.first.tap { |variant| apply(supplied_product, variant) } - end.tap do |variant| - link = supplied_product.semanticId - variant.semantic_links.new(semantic_id: link) if link.present? - end - end - - def self.referenced_spree_product(supplied_product, supplier) - 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)) - - supplier.supplied_products.find_by(id: params["spree_product_id"]) - elsif id.present? - supplier.supplied_products.find_by(id:) - end - end - - def self.import_product(supplied_product, supplier) - Spree::Product.new( - name: supplied_product.name, - description: supplied_product.description, - price: 0, # will be in DFC Offer - supplier_id: supplier.id, - primary_taxon_id: taxon(supplied_product).id, - image: ImageBuilder.import(supplied_product.image), - ).tap do |product| - QuantitativeValueBuilder.apply(supplied_product.quantity, product) - product.ensure_standard_variant - end - end - - def self.apply(supplied_product, variant) - variant.product.assign_attributes(description: supplied_product.description) - - variant.display_name = supplied_product.name - variant.primary_taxon = taxon(supplied_product) - QuantitativeValueBuilder.apply(supplied_product.quantity, variant) - - catalog_item = supplied_product&.catalogItems&.first - offer = catalog_item&.offers&.first - CatalogItemBuilder.apply_stock(catalog_item, variant) - OfferBuilder.apply(offer, variant) - end - def self.product_type(variant) taxon_dfc_id = variant.primary_taxon&.dfc_id DataFoodConsortium::Connector::SKOSParser.concepts[taxon_dfc_id] end - def self.taxon(supplied_product) - ProductTypeImporter.taxon(supplied_product.productType) - end - - private_class_method :product_type, :taxon + private_class_method :product_type end diff --git a/engines/dfc_provider/app/services/supplied_product_importer.rb b/engines/dfc_provider/app/services/supplied_product_importer.rb new file mode 100644 index 0000000000..4426064913 --- /dev/null +++ b/engines/dfc_provider/app/services/supplied_product_importer.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +class SuppliedProductImporter < DfcBuilder + def self.store_product(subject, enterprise) + return unless subject.is_a? DataFoodConsortium::Connector::SuppliedProduct + + variant = import_variant(subject, enterprise) + product = variant.product + + product.save! if product.new_record? + variant.save! if variant.new_record? + + variant + end + + def self.update_product(supplied_product, variant) + apply(supplied_product, variant) + + variant.product.save! + variant.save! + + variant + end + + def self.import_variant(supplied_product, supplier) + product = referenced_spree_product(supplied_product, supplier) + + if product + Spree::Variant.new( product:, supplier:, price: 0,).tap do |variant| + apply(supplied_product, variant) + end + else + product = import_product(supplied_product, supplier) + product.variants.first.tap { |variant| apply(supplied_product, variant) } + end.tap do |variant| + link = supplied_product.semanticId + variant.semantic_links.new(semantic_id: link) if link.present? + end + end + + def self.referenced_spree_product(supplied_product, supplier) + 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)) + + supplier.supplied_products.find_by(id: params["spree_product_id"]) + elsif id.present? + supplier.supplied_products.find_by(id:) + end + end + + def self.import_product(supplied_product, supplier) + Spree::Product.new( + name: supplied_product.name, + description: supplied_product.description, + price: 0, # will be in DFC Offer + supplier_id: supplier.id, + primary_taxon_id: taxon(supplied_product).id, + image: ImageBuilder.import(supplied_product.image), + ).tap do |product| + QuantitativeValueBuilder.apply(supplied_product.quantity, product) + product.ensure_standard_variant + end + end + + def self.apply(supplied_product, variant) + variant.product.assign_attributes(description: supplied_product.description) + + variant.display_name = supplied_product.name + variant.primary_taxon = taxon(supplied_product) + QuantitativeValueBuilder.apply(supplied_product.quantity, variant) + + catalog_item = supplied_product&.catalogItems&.first + offer = catalog_item&.offers&.first + CatalogItemBuilder.apply_stock(catalog_item, variant) + OfferBuilder.apply(offer, variant) + end + + def self.taxon(supplied_product) + ProductTypeImporter.taxon(supplied_product.productType) + end + + private_class_method :taxon +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 bd222af048..e76523ecb4 100644 --- a/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_builder_spec.rb @@ -83,272 +83,4 @@ RSpec.describe SuppliedProductBuilder do ) end end - - describe ".store_product" do - let(:subject) { builder.store_product(product, supplier) } - let(:product) { - DfcIo.import(product_json).find do |subject| - subject.is_a? DataFoodConsortium::Connector::SuppliedProduct - end - } - let(:product_json) { ExampleJson.read("product.GET") } - - before do - taxon.save! - end - - it "stores a new Spree Product and Variant" do - expect { subject }.to change { - Spree::Product.count - }.by(1) - - expect(subject).to be_a(Spree::Variant) - expect(subject).to be_valid - expect(subject).to be_persisted - expect(subject.name).to eq("Fillet Steak - 201g x 1 Steak") - expect(subject.variant_unit).to eq("items") - expect(subject.variant_unit_scale).to eq(nil) - expect(subject.variant_unit_with_scale).to eq("items") - expect(subject.unit_value).to eq(1) - end - end - - describe ".update_product" do - let(:subject) { builder.update_product(product, variant) } - let(:product) { - DfcIo.import(product_json).find do |subject| - subject.is_a? DataFoodConsortium::Connector::SuppliedProduct - end - } - let(:product_json) { ExampleJson.read("product.GET") } - - it "updates a variant" do - variant # Create test data first - - expect { subject }.not_to change { - Spree::Variant.count - } - - expect(subject).to eq variant - expect(subject.display_name).to eq "Fillet Steak - 201g x 1 Steak" - expect(subject.variant_unit).to eq "items" - expect(subject.unit_value).to eq 1 - expect(subject.on_demand).to eq false - expect(subject.on_hand).to eq 11 - end - end - - describe ".import_product" 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, - image: "https://cd.net/tomato.png?v=5", - ) - end - let(:product_type) { DfcLoader.connector.PRODUCT_TYPES.VEGETABLE.NON_LOCAL_VEGETABLE } - let!(:taxon) { - create( - :taxon, - name: "Non local vegetable", - dfc_id: "https://github.com/datafoodconsortium/taxonomies/releases/latest/download/productTypes.rdf#non-local-vegetable" - ) - } - - before do - stub_request(:get, "https://cd.net/tomato.png?v=5").to_return( - status: 200, - body: black_logo_path.read, - ) - end - - it "creates a new Spree::Product" do - product = builder.import_product(supplied_product, supplier) - - expect(product).to be_a(Spree::Product) - expect(product.name).to eq("Tomato") - expect(product.description).to eq("Awesome tomato") - expect(product.variant_unit).to eq("weight") - expect(product.image).to be_present - expect(product.image.attachment).to be_attached - expect(product.image.url(:product)).to match /^http.*tomato\.png/ - end - - describe "taxon" do - it "assigns the taxon matching the DFC product type" do - product = builder.import_product(supplied_product, supplier) - - expect(product.variants.first.primary_taxon).to eq(taxon) - end - end - end - - describe ".import_variant" do - let(:imported_variant) { builder.import_variant(supplied_product, supplier) } - 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, - catalogItems: [catalog_item], - ) - end - let(:product_type) { DfcLoader.connector.PRODUCT_TYPES.VEGETABLE.NON_LOCAL_VEGETABLE } - let(:catalog_item) { - DataFoodConsortium::Connector::CatalogItem.new( - nil, - # On-demand is expressed as negative stock. - # And some APIs send strings instead of numbers... - stockLimitation: "-1", - offers: [offer], - ) - } - let(:offer) { - DataFoodConsortium::Connector::Offer.new( - nil, - price: DataFoodConsortium::Connector::Price.new(value: "15.50"), - ) - } - - it "creates a new Spree::Product and variant" do - create(:taxon) - - expect(imported_variant).to be_a(Spree::Variant) - expect(imported_variant).to be_valid - expect(imported_variant.id).to be_nil - expect(imported_variant.semantic_links.size).to eq 1 - - link = imported_variant.semantic_links[0] - expect(link.semantic_id).to eq "https://example.net/tomato" - - imported_product = imported_variant.product - expect(imported_product).to be_a(Spree::Product) - expect(imported_product).to be_valid - 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") - - # Stock can only be checked when persisted: - imported_product.save! - expect(imported_variant.price).to eq 15.50 - expect(imported_variant.on_demand).to eq true - expect(imported_variant.on_hand).to eq 0 - end - - context "with spree_product_id supplied" do - let(:imported_variant) { builder.import_variant(supplied_product, supplier) } - - 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.FRUIT_JUICE } - let!(:new_taxon) { - create( - :taxon, - name: "Fruit Juice", - dfc_id: "https://github.com/datafoodconsortium/taxonomies/releases/latest/download/productTypes.rdf#fruit-juice" - ) - } - - 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_variant.primary_taxon).to eq(new_taxon) - end - - context "when spree_product_uri doesn't 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, supplier) } - 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 product of another enterprise" do - variant.save! - create(:product, id: 8, supplier_id: create(:enterprise).id) - - 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 = - "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/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb new file mode 100644 index 0000000000..845d2cb3d0 --- /dev/null +++ b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb @@ -0,0 +1,293 @@ +# frozen_string_literal: true + +require_relative "../spec_helper" + +RSpec.describe SuppliedProductImporter do + include FileHelper + + subject(:importer) { described_class } + let(:variant) { + create(:variant, id: 5, product: spree_product, primary_taxon: taxon, supplier:) + } + let(:spree_product) { + create(:product, id: 6) + } + let(:supplier) { + create(:supplier_enterprise, id: 7) + } + let(:taxon) { + build( + :taxon, + name: "Soft Drink", + dfc_id: "https://github.com/datafoodconsortium/taxonomies/releases/latest/download/productTypes.rdf#soft-drink" + ) + } + + describe ".store_product" do + let(:subject) { importer.store_product(product, supplier) } + let(:product) { + DfcIo.import(product_json).find do |subject| + subject.is_a? DataFoodConsortium::Connector::SuppliedProduct + end + } + let(:product_json) { ExampleJson.read("product.GET") } + + before do + taxon.save! + end + + it "stores a new Spree Product and Variant" do + expect { subject }.to change { + Spree::Product.count + }.by(1) + + expect(subject).to be_a(Spree::Variant) + expect(subject).to be_valid + expect(subject).to be_persisted + expect(subject.name).to eq("Fillet Steak - 201g x 1 Steak") + expect(subject.variant_unit).to eq("items") + expect(subject.variant_unit_scale).to eq(nil) + expect(subject.variant_unit_with_scale).to eq("items") + expect(subject.unit_value).to eq(1) + end + end + + describe ".update_product" do + let(:subject) { importer.update_product(product, variant) } + let(:product) { + DfcIo.import(product_json).find do |subject| + subject.is_a? DataFoodConsortium::Connector::SuppliedProduct + end + } + let(:product_json) { ExampleJson.read("product.GET") } + + it "updates a variant" do + variant # Create test data first + + expect { subject }.not_to change { + Spree::Variant.count + } + + expect(subject).to eq variant + expect(subject.display_name).to eq "Fillet Steak - 201g x 1 Steak" + expect(subject.variant_unit).to eq "items" + expect(subject.unit_value).to eq 1 + expect(subject.on_demand).to eq false + expect(subject.on_hand).to eq 11 + end + end + + describe ".import_product" 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, + image: "https://cd.net/tomato.png?v=5", + ) + end + let(:product_type) { DfcLoader.connector.PRODUCT_TYPES.VEGETABLE.NON_LOCAL_VEGETABLE } + let!(:taxon) { + create( + :taxon, + name: "Non local vegetable", + dfc_id: "https://github.com/datafoodconsortium/taxonomies/releases/latest/download/productTypes.rdf#non-local-vegetable" + ) + } + + before do + stub_request(:get, "https://cd.net/tomato.png?v=5").to_return( + status: 200, + body: black_logo_path.read, + ) + end + + it "creates a new Spree::Product" do + product = importer.import_product(supplied_product, supplier) + + expect(product).to be_a(Spree::Product) + expect(product.name).to eq("Tomato") + expect(product.description).to eq("Awesome tomato") + expect(product.variant_unit).to eq("weight") + expect(product.image).to be_present + expect(product.image.attachment).to be_attached + expect(product.image.url(:product)).to match /^http.*tomato\.png/ + end + + describe "taxon" do + it "assigns the taxon matching the DFC product type" do + product = importer.import_product(supplied_product, supplier) + + expect(product.variants.first.primary_taxon).to eq(taxon) + end + end + end + + describe ".import_variant" do + let(:imported_variant) { importer.import_variant(supplied_product, supplier) } + 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, + catalogItems: [catalog_item], + ) + end + let(:product_type) { DfcLoader.connector.PRODUCT_TYPES.VEGETABLE.NON_LOCAL_VEGETABLE } + let(:catalog_item) { + DataFoodConsortium::Connector::CatalogItem.new( + nil, + # On-demand is expressed as negative stock. + # And some APIs send strings instead of numbers... + stockLimitation: "-1", + offers: [offer], + ) + } + let(:offer) { + DataFoodConsortium::Connector::Offer.new( + nil, + price: DataFoodConsortium::Connector::Price.new(value: "15.50"), + ) + } + + it "creates a new Spree::Product and variant" do + create(:taxon) + + expect(imported_variant).to be_a(Spree::Variant) + expect(imported_variant).to be_valid + expect(imported_variant.id).to be_nil + expect(imported_variant.semantic_links.size).to eq 1 + + link = imported_variant.semantic_links[0] + expect(link.semantic_id).to eq "https://example.net/tomato" + + imported_product = imported_variant.product + expect(imported_product).to be_a(Spree::Product) + expect(imported_product).to be_valid + 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") + + # Stock can only be checked when persisted: + imported_product.save! + expect(imported_variant.price).to eq 15.50 + expect(imported_variant.on_demand).to eq true + expect(imported_variant.on_hand).to eq 0 + end + + context "with spree_product_id supplied" do + let(:imported_variant) { importer.import_variant(supplied_product, supplier) } + + 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.FRUIT_JUICE } + let!(:new_taxon) { + create( + :taxon, + name: "Fruit Juice", + dfc_id: "https://github.com/datafoodconsortium/taxonomies/releases/latest/download/productTypes.rdf#fruit-juice" + ) + } + + 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_variant.primary_taxon).to eq(new_taxon) + end + + context "when spree_product_uri doesn't 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) { importer.referenced_spree_product(supplied_product, supplier) } + 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 product of another enterprise" do + variant.save! + create(:product, id: 8, supplier_id: create(:enterprise).id) + + 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 = + "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 From c1e0c6ed34845fb69766064fbeb7c206d00afdf6 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 23 Jan 2025 16:32:02 +1100 Subject: [PATCH 07/13] Import variants for existing products via new DFC attribute --- .../app/services/supplied_product_importer.rb | 43 ++++++++++++++----- .../supplied_product_importer_spec.rb | 9 ++++ 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/engines/dfc_provider/app/services/supplied_product_importer.rb b/engines/dfc_provider/app/services/supplied_product_importer.rb index 4426064913..6b4a62a27f 100644 --- a/engines/dfc_provider/app/services/supplied_product_importer.rb +++ b/engines/dfc_provider/app/services/supplied_product_importer.rb @@ -38,21 +38,44 @@ class SuppliedProductImporter < DfcBuilder end end + # DEPRECATION WARNING + # Reference by custom `ofn:spree_product_id` and `ofn:spree_product_uri` + # properties is now replaced by the official `dfc-b:isVariantOf`. + # We will remove the old methods at some point. def self.referenced_spree_product(supplied_product, supplier) - uri = supplied_product.spree_product_uri - id = supplied_product.spree_product_id + spree_product(supplied_product, supplier) || + spree_product_from_uri(supplied_product, supplier) || + spree_product_from_id(supplied_product, supplier) + end - if uri.present? - route = Rails.application.routes.recognize_path(uri) - params = Rack::Utils.parse_nested_query(URI.parse(uri).query) + def self.spree_product(supplied_product, supplier) + supplied_product.isVariantOf.lazy.map do |group| + group_id = group.semanticId + route = Rails.application.routes.recognize_path(group_id) # Check that the given URI points to us: - return unless uri == urls.enterprise_url(route.merge(params)) + next if group_id != urls.enterprise_technical_product_url(route) - supplier.supplied_products.find_by(id: params["spree_product_id"]) - elsif id.present? - supplier.supplied_products.find_by(id:) - end + supplier.supplied_products.find_by(id: route[:id]) + end.compact.first + end + + def self.spree_product_from_uri(supplied_product, supplier) + uri = supplied_product.spree_product_uri + return if uri.blank? + + 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)) + + supplier.supplied_products.find_by(id: params["spree_product_id"]) + end + + def self.spree_product_from_id(supplied_product, supplier) + id = supplied_product.spree_product_id + supplier.supplied_products.find_by(id:) if id.present? end def self.import_product(supplied_product, supplier) diff --git a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb index 845d2cb3d0..abbd435284 100644 --- a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb @@ -261,6 +261,15 @@ RSpec.describe SuppliedProductImporter do expect(result).to eq nil end + it "returns a product referenced by semantic id" do + variant.save! + supplied_product.isVariantOf << + DataFoodConsortium::Connector::TechnicalProduct.new( + "http://test.host/api/dfc/enterprises/7/technical_products/6" + ) + expect(result).to eq spree_product + end + it "returns a product referenced by URI" do variant.save! supplied_product.spree_product_uri = From 516759062fc753154cd9208cd9e3d236dddea91e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 23 Jan 2025 16:44:40 +1100 Subject: [PATCH 08/13] Import variants for the same product group When importing another catalog, it's probably referring to external product groups. Storing the external link allows us to group several variants and replicate the same structure within OFN. --- app/models/spree/product.rb | 1 + .../app/services/supplied_product_importer.rb | 30 +++++++++++++++---- .../supplied_product_importer_spec.rb | 25 ++++++++++++++-- 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index 81b1225c25..171f6de744 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -36,6 +36,7 @@ module Spree searchable_scopes :active, :with_properties has_one :image, class_name: "Spree::Image", as: :viewable, dependent: :destroy + has_one :semantic_link, as: :subject, dependent: :delete has_many :product_properties, dependent: :destroy has_many :properties, through: :product_properties diff --git a/engines/dfc_provider/app/services/supplied_product_importer.rb b/engines/dfc_provider/app/services/supplied_product_importer.rb index 6b4a62a27f..e887869de9 100644 --- a/engines/dfc_provider/app/services/supplied_product_importer.rb +++ b/engines/dfc_provider/app/services/supplied_product_importer.rb @@ -44,6 +44,7 @@ class SuppliedProductImporter < DfcBuilder # We will remove the old methods at some point. def self.referenced_spree_product(supplied_product, supplier) spree_product(supplied_product, supplier) || + spree_product_linked(supplied_product, supplier) || spree_product_from_uri(supplied_product, supplier) || spree_product_from_id(supplied_product, supplier) end @@ -51,15 +52,28 @@ class SuppliedProductImporter < DfcBuilder def self.spree_product(supplied_product, supplier) supplied_product.isVariantOf.lazy.map do |group| group_id = group.semanticId - route = Rails.application.routes.recognize_path(group_id) + id = begin + route = Rails.application.routes.recognize_path(group_id) - # Check that the given URI points to us: - next if group_id != urls.enterprise_technical_product_url(route) + # Check that the given URI points to us: + next if group_id != urls.enterprise_technical_product_url(route) - supplier.supplied_products.find_by(id: route[:id]) + route[:id] + rescue ActionController::RoutingError + next + end + + supplier.supplied_products.find_by(id:) end.compact.first end + def self.spree_product_linked(supplied_product, supplier) + semantic_ids = supplied_product.isVariantOf.map(&:semanticId) + supplier.supplied_products.includes(:semantic_link) + .where(semantic_link: { semantic_id: semantic_ids }) + .first + end + def self.spree_product_from_uri(supplied_product, supplier) uri = supplied_product.spree_product_uri return if uri.blank? @@ -86,6 +100,7 @@ class SuppliedProductImporter < DfcBuilder supplier_id: supplier.id, primary_taxon_id: taxon(supplied_product).id, image: ImageBuilder.import(supplied_product.image), + semantic_link: semantic_link(supplied_product), ).tap do |product| QuantitativeValueBuilder.apply(supplied_product.quantity, product) product.ensure_standard_variant @@ -105,9 +120,14 @@ class SuppliedProductImporter < DfcBuilder OfferBuilder.apply(offer, variant) end + def self.semantic_link(supplied_product) + semantic_id = supplied_product.isVariantOf.first&.semanticId + + SemanticLink.new(semantic_id:) if semantic_id.present? + end + def self.taxon(supplied_product) ProductTypeImporter.taxon(supplied_product.productType) end - private_class_method :taxon end diff --git a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb index abbd435284..9f6bd6a98c 100644 --- a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb @@ -186,7 +186,7 @@ RSpec.describe SuppliedProductImporter do expect(imported_variant.on_hand).to eq 0 end - context "with spree_product_id supplied" do + context "linked to product group" do let(:imported_variant) { importer.import_variant(supplied_product, supplier) } let(:supplied_product) do @@ -229,7 +229,13 @@ RSpec.describe SuppliedProductImporter do value: 2, ), productType: product_type, - spree_product_uri: "http://another.host/api/dfc/enterprises/10/supplied_products/50" + spree_product_uri: "http://another.host/api/dfc/enterprises/10/supplied_products/50", + isVariantOf: [technical_product], + ) + end + let(:technical_product) do + DataFoodConsortium::Connector::TechnicalProduct.new( + "http://test.host/api/dfc/enterprises/7/technical_products/6" ) end @@ -243,6 +249,8 @@ RSpec.describe SuppliedProductImporter do expect(imported_product.name).to eq("Tomato") expect(imported_product.description).to eq("Awesome tomato") expect(imported_product.variant_unit).to eq("weight") + expect(imported_product.semantic_link.semantic_id) + .to eq "http://test.host/api/dfc/enterprises/7/technical_products/6" end end end @@ -270,6 +278,19 @@ RSpec.describe SuppliedProductImporter do expect(result).to eq spree_product end + it "returns a product referenced by external URI" do + variant.save! + supplied_product.isVariantOf << + DataFoodConsortium::Connector::TechnicalProduct.new( + "http://example.net/product_group" + ) + SemanticLink.create!( + subject: spree_product, + semantic_id: "http://example.net/product_group", + ) + expect(result).to eq spree_product + end + it "returns a product referenced by URI" do variant.save! supplied_product.spree_product_uri = From fba7c24ebdd7572ff95737115afbd66acf89d9cb Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 24 Jan 2025 17:14:51 +1100 Subject: [PATCH 09/13] Product group can be present or just linked --- .../app/services/supplied_product_builder.rb | 7 ++----- .../app/services/supplied_product_importer.rb | 13 +++++++++---- .../services/supplied_product_importer_spec.rb | 16 ++++++++-------- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/engines/dfc_provider/app/services/supplied_product_builder.rb b/engines/dfc_provider/app/services/supplied_product_builder.rb index 63398b3f89..d2d1751be2 100644 --- a/engines/dfc_provider/app/services/supplied_product_builder.rb +++ b/engines/dfc_provider/app/services/supplied_product_builder.rb @@ -13,10 +13,7 @@ class SuppliedProductBuilder < DfcBuilder variant.supplier_id, spree_product_id: variant.product_id ) - product_group_id = urls.enterprise_product_group_url( - enterprise_id: variant.supplier_id, - id: variant.product_id, - ) + product_group = ProductGroupBuilder.product_group(variant.product) DfcProvider::SuppliedProduct.new( semantic_id(variant), @@ -24,7 +21,7 @@ class SuppliedProductBuilder < DfcBuilder description: variant.description, productType: product_type(variant), quantity: QuantitativeValueBuilder.quantity(variant), - isVariantOf: [product_group_id], + isVariantOf: [product_group], spree_product_uri: product_uri, spree_product_id: variant.product.id, image_url: variant.product&.image&.url(:product) diff --git a/engines/dfc_provider/app/services/supplied_product_importer.rb b/engines/dfc_provider/app/services/supplied_product_importer.rb index e887869de9..9d7920e820 100644 --- a/engines/dfc_provider/app/services/supplied_product_importer.rb +++ b/engines/dfc_provider/app/services/supplied_product_importer.rb @@ -51,12 +51,14 @@ class SuppliedProductImporter < DfcBuilder def self.spree_product(supplied_product, supplier) supplied_product.isVariantOf.lazy.map do |group| - group_id = group.semanticId + # We may have an object or just the id here: + group_id = group.try(:semanticId) || group + id = begin route = Rails.application.routes.recognize_path(group_id) # Check that the given URI points to us: - next if group_id != urls.enterprise_technical_product_url(route) + next if group_id != urls.enterprise_product_group_url(route) route[:id] rescue ActionController::RoutingError @@ -68,7 +70,9 @@ class SuppliedProductImporter < DfcBuilder end def self.spree_product_linked(supplied_product, supplier) - semantic_ids = supplied_product.isVariantOf.map(&:semanticId) + semantic_ids = supplied_product.isVariantOf.map do |id_or_object| + id_or_object.try(:semanticId) || id_or_object + end supplier.supplied_products.includes(:semantic_link) .where(semantic_link: { semantic_id: semantic_ids }) .first @@ -121,7 +125,8 @@ class SuppliedProductImporter < DfcBuilder end def self.semantic_link(supplied_product) - semantic_id = supplied_product.isVariantOf.first&.semanticId + group = supplied_product.isVariantOf.first + semantic_id = group.try(:semanticId) || semantic_id SemanticLink.new(semantic_id:) if semantic_id.present? end diff --git a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb index 9f6bd6a98c..ae2f0083b5 100644 --- a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb @@ -230,12 +230,12 @@ RSpec.describe SuppliedProductImporter do ), productType: product_type, spree_product_uri: "http://another.host/api/dfc/enterprises/10/supplied_products/50", - isVariantOf: [technical_product], + isVariantOf: [product_group], ) end - let(:technical_product) do - DataFoodConsortium::Connector::TechnicalProduct.new( - "http://test.host/api/dfc/enterprises/7/technical_products/6" + let(:product_group) do + DataFoodConsortium::Connector::SuppliedProduct.new( + "http://test.host/api/dfc/enterprises/7/product_groups/6" ) end @@ -250,7 +250,7 @@ RSpec.describe SuppliedProductImporter do expect(imported_product.description).to eq("Awesome tomato") expect(imported_product.variant_unit).to eq("weight") expect(imported_product.semantic_link.semantic_id) - .to eq "http://test.host/api/dfc/enterprises/7/technical_products/6" + .to eq "http://test.host/api/dfc/enterprises/7/product_groups/6" end end end @@ -272,8 +272,8 @@ RSpec.describe SuppliedProductImporter do it "returns a product referenced by semantic id" do variant.save! supplied_product.isVariantOf << - DataFoodConsortium::Connector::TechnicalProduct.new( - "http://test.host/api/dfc/enterprises/7/technical_products/6" + DataFoodConsortium::Connector::SuppliedProduct.new( + "http://test.host/api/dfc/enterprises/7/product_groups/6" ) expect(result).to eq spree_product end @@ -281,7 +281,7 @@ RSpec.describe SuppliedProductImporter do it "returns a product referenced by external URI" do variant.save! supplied_product.isVariantOf << - DataFoodConsortium::Connector::TechnicalProduct.new( + DataFoodConsortium::Connector::SuppliedProduct.new( "http://example.net/product_group" ) SemanticLink.create!( From 3d435ae7819e1b13d50e40447f95222fbe5abf72 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 13 Feb 2025 13:38:14 +1100 Subject: [PATCH 10/13] Include product group objects in our catalog response And when we import a catalog, we don't try to import those product groups as Spree::Variant. We just see them as reference to Spree::Product. --- .../controllers/dfc_provider/catalog_items_controller.rb | 1 + engines/dfc_provider/app/services/dfc_catalog.rb | 6 +++++- swagger/dfc.yaml | 3 +++ 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/engines/dfc_provider/app/controllers/dfc_provider/catalog_items_controller.rb b/engines/dfc_provider/app/controllers/dfc_provider/catalog_items_controller.rb index c8b2ba59ff..e9e6fc2232 100644 --- a/engines/dfc_provider/app/controllers/dfc_provider/catalog_items_controller.rb +++ b/engines/dfc_provider/app/controllers/dfc_provider/catalog_items_controller.rb @@ -19,6 +19,7 @@ module DfcProvider *person.affiliatedOrganizations, *person.affiliatedOrganizations.flat_map(&:catalogItems), *person.affiliatedOrganizations.flat_map(&:catalogItems).map(&:product), + *person.affiliatedOrganizations.flat_map(&:catalogItems).map(&:product).flat_map(&:isVariantOf), *person.affiliatedOrganizations.flat_map(&:catalogItems).flat_map(&:offers), ) end diff --git a/engines/dfc_provider/app/services/dfc_catalog.rb b/engines/dfc_provider/app/services/dfc_catalog.rb index 08649494c7..431454bd54 100644 --- a/engines/dfc_provider/app/services/dfc_catalog.rb +++ b/engines/dfc_provider/app/services/dfc_catalog.rb @@ -13,9 +13,13 @@ class DfcCatalog @graph = graph end + # List all products in this catalog. + # These are SuppliedProduct objects which may be grouped as variants. + # But we don't return the parent products having variants. def products @products ||= @graph.select do |subject| - subject.is_a? DataFoodConsortium::Connector::SuppliedProduct + subject.is_a?(DataFoodConsortium::Connector::SuppliedProduct) && + subject.variants.blank? end end diff --git a/swagger/dfc.yaml b/swagger/dfc.yaml index b40e8ab013..66a924ceed 100644 --- a/swagger/dfc.yaml +++ b/swagger/dfc.yaml @@ -180,6 +180,9 @@ paths: dfc-b:isVariantOf: http://test.host/api/dfc/enterprises/10000/product_groups/90000 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/product_groups/90000 + "@type": dfc-b:SuppliedProduct + dfc-b:hasVariant: http://test.host/api/dfc/enterprises/10000/supplied_products/10001 - "@id": http://test.host/api/dfc/enterprises/10000/offers/10001 "@type": dfc-b:Offer dfc-b:hasPrice: From 11a1d4e09ec30c795a2a1318a0d646cf7db0c3da Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 13 Feb 2025 13:42:58 +1100 Subject: [PATCH 11/13] Reduce complexity of controller --- .../dfc_provider/catalog_items_controller.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/engines/dfc_provider/app/controllers/dfc_provider/catalog_items_controller.rb b/engines/dfc_provider/app/controllers/dfc_provider/catalog_items_controller.rb index e9e6fc2232..00d0b63da7 100644 --- a/engines/dfc_provider/app/controllers/dfc_provider/catalog_items_controller.rb +++ b/engines/dfc_provider/app/controllers/dfc_provider/catalog_items_controller.rb @@ -13,14 +13,15 @@ module DfcProvider EnterpriseBuilder.enterprise(enterprise) end person.affiliatedOrganizations = enterprises + catalog_items = enterprises.flat_map(&:catalogItems) render json: DfcIo.export( person, - *person.affiliatedOrganizations, - *person.affiliatedOrganizations.flat_map(&:catalogItems), - *person.affiliatedOrganizations.flat_map(&:catalogItems).map(&:product), - *person.affiliatedOrganizations.flat_map(&:catalogItems).map(&:product).flat_map(&:isVariantOf), - *person.affiliatedOrganizations.flat_map(&:catalogItems).flat_map(&:offers), + *enterprises, + *catalog_items, + *catalog_items.map(&:product), + *catalog_items.map(&:product).flat_map(&:isVariantOf), + *catalog_items.flat_map(&:offers), ) end From 5d495b94b36b4e25c3658e3e4ac8636b7203a7be Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 13 Feb 2025 15:02:37 +1100 Subject: [PATCH 12/13] Use product group attributes for Spree::Product updates --- .../dfc_provider/app/services/product_group_builder.rb | 9 +++++++++ .../app/services/supplied_product_importer.rb | 2 +- .../spec/services/supplied_product_importer_spec.rb | 9 +++++++++ swagger/dfc.yaml | 2 ++ 4 files changed, 21 insertions(+), 1 deletion(-) diff --git a/engines/dfc_provider/app/services/product_group_builder.rb b/engines/dfc_provider/app/services/product_group_builder.rb index fd25b21f84..50c1088914 100644 --- a/engines/dfc_provider/app/services/product_group_builder.rb +++ b/engines/dfc_provider/app/services/product_group_builder.rb @@ -12,6 +12,15 @@ class ProductGroupBuilder < DfcBuilder DataFoodConsortium::Connector::SuppliedProduct.new( id, variants:, + name: product.name, ) end + + def self.apply(supplied_product, spree_product) + description = supplied_product.isVariantOf.first.try(:description) || + supplied_product.description + name = supplied_product.isVariantOf.first.try(:name) + spree_product.description = description if description.present? + spree_product.name = name if name.present? + end end diff --git a/engines/dfc_provider/app/services/supplied_product_importer.rb b/engines/dfc_provider/app/services/supplied_product_importer.rb index 9d7920e820..cbeb8d3cdf 100644 --- a/engines/dfc_provider/app/services/supplied_product_importer.rb +++ b/engines/dfc_provider/app/services/supplied_product_importer.rb @@ -112,7 +112,7 @@ class SuppliedProductImporter < DfcBuilder end def self.apply(supplied_product, variant) - variant.product.assign_attributes(description: supplied_product.description) + ProductGroupBuilder.apply(supplied_product, variant.product) variant.display_name = supplied_product.name variant.primary_taxon = taxon(supplied_product) diff --git a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb index ae2f0083b5..2a4fc0e856 100644 --- a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb @@ -218,6 +218,15 @@ RSpec.describe SuppliedProductImporter do expect(imported_variant.primary_taxon).to eq(new_taxon) end + it "copies name and description from parent product" do + supplied_product.isVariantOf << DfcProvider::SuppliedProduct.new( + "some-id", name: "Our tomatoes", description: "Choose a variety." + ) + imported_product = imported_variant.product + expect(imported_product.name).to eq "Our tomatoes" + expect(imported_product.description).to eq "Choose a variety." + end + context "when spree_product_uri doesn't match the server host" do let(:supplied_product) do DfcProvider::SuppliedProduct.new( diff --git a/swagger/dfc.yaml b/swagger/dfc.yaml index 66a924ceed..eb38ea6532 100644 --- a/swagger/dfc.yaml +++ b/swagger/dfc.yaml @@ -182,6 +182,7 @@ paths: ofn:spree_product_uri: http://test.host/api/dfc/enterprises/10000?spree_product_id=90000 - "@id": http://test.host/api/dfc/enterprises/10000/product_groups/90000 "@type": dfc-b:SuppliedProduct + dfc-b:name: Apple dfc-b:hasVariant: http://test.host/api/dfc/enterprises/10000/supplied_products/10001 - "@id": http://test.host/api/dfc/enterprises/10000/offers/10001 "@type": dfc-b:Offer @@ -584,6 +585,7 @@ paths: "@context": https://www.datafoodconsortium.org "@id": http://test.host/api/dfc/enterprises/10000/product_groups/90000 "@type": dfc-b:SuppliedProduct + dfc-b:name: Pesto dfc-b:hasVariant: http://test.host/api/dfc/enterprises/10000/supplied_products/10001 "/api/dfc/enterprises/{enterprise_id}/social_medias/{name}": get: From 699db020986ea759c288f7534c439438768d424d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 18 Feb 2025 16:02:30 +1100 Subject: [PATCH 13/13] Remove enterprise id from DFC product group URL A Spree::Product represented as product group is not directly associated to an enterprise. In theory, it could have multiple enterprises through its variants. So we better don't include the id in the URL. ``` -http://test.host/api/dfc/enterprises/10000/product_groups/90000 +http://test.host/api/dfc/product_groups/90000 ``` This makes it simpler as well. --- .../dfc_provider/product_groups_controller.rb | 10 +++++++--- .../app/services/product_group_builder.rb | 5 +---- .../app/services/supplied_product_importer.rb | 2 +- engines/dfc_provider/config/routes.rb | 2 +- .../spec/requests/product_groups_spec.rb | 4 ++-- .../spec/requests/supplied_products_spec.rb | 2 +- .../services/supplied_product_importer_spec.rb | 6 +++--- swagger/dfc.yaml | 14 +++++++------- 8 files changed, 23 insertions(+), 22 deletions(-) diff --git a/engines/dfc_provider/app/controllers/dfc_provider/product_groups_controller.rb b/engines/dfc_provider/app/controllers/dfc_provider/product_groups_controller.rb index ab527381ba..35c8702f7f 100644 --- a/engines/dfc_provider/app/controllers/dfc_provider/product_groups_controller.rb +++ b/engines/dfc_provider/app/controllers/dfc_provider/product_groups_controller.rb @@ -3,12 +3,16 @@ # Show Spree::Product as SuppliedProduct with variants. module DfcProvider class ProductGroupsController < DfcProvider::ApplicationController - before_action :check_enterprise - def show - spree_product = current_enterprise.supplied_products.find(params[:id]) + spree_product = permissions.visible_products.find(params[:id]) product = ProductGroupBuilder.product_group(spree_product) render json: DfcIo.export(product) end + + private + + def permissions + OpenFoodNetwork::Permissions.new(current_user) + end end end diff --git a/engines/dfc_provider/app/services/product_group_builder.rb b/engines/dfc_provider/app/services/product_group_builder.rb index 50c1088914..c8c8e6f62d 100644 --- a/engines/dfc_provider/app/services/product_group_builder.rb +++ b/engines/dfc_provider/app/services/product_group_builder.rb @@ -2,10 +2,7 @@ class ProductGroupBuilder < DfcBuilder def self.product_group(product) - id = urls.enterprise_product_group_url( - enterprise_id: product.variants.first.supplier_id, - id: product.id, - ) + id = urls.product_group_url(id: product.id) variants = product.variants.map do |spree_variant| SuppliedProductBuilder.semantic_id(spree_variant) end diff --git a/engines/dfc_provider/app/services/supplied_product_importer.rb b/engines/dfc_provider/app/services/supplied_product_importer.rb index cbeb8d3cdf..c524741596 100644 --- a/engines/dfc_provider/app/services/supplied_product_importer.rb +++ b/engines/dfc_provider/app/services/supplied_product_importer.rb @@ -58,7 +58,7 @@ class SuppliedProductImporter < DfcBuilder route = Rails.application.routes.recognize_path(group_id) # Check that the given URI points to us: - next if group_id != urls.enterprise_product_group_url(route) + next if group_id != urls.product_group_url(route) route[:id] rescue ActionController::RoutingError diff --git a/engines/dfc_provider/config/routes.rb b/engines/dfc_provider/config/routes.rb index a5b5b0ceff..647094b96b 100644 --- a/engines/dfc_provider/config/routes.rb +++ b/engines/dfc_provider/config/routes.rb @@ -6,13 +6,13 @@ DfcProvider::Engine.routes.draw do resources :catalog_items, only: [:index, :show, :update] resources :offers, only: [:show, :update] resources :supplied_products, only: [:create, :show, :update] - resources :product_groups, only: [:show] resources :social_medias, only: [:show] end resources :enterprise_groups, only: [:index, :show] do resources :affiliated_by, only: [:create, :destroy], module: 'enterprise_groups' end resources :persons, only: [:show] + resources :product_groups, only: [:show] resource :affiliate_sales_data, only: [:show] end diff --git a/engines/dfc_provider/spec/requests/product_groups_spec.rb b/engines/dfc_provider/spec/requests/product_groups_spec.rb index 56aab98377..215bfb3733 100644 --- a/engines/dfc_provider/spec/requests/product_groups_spec.rb +++ b/engines/dfc_provider/spec/requests/product_groups_spec.rb @@ -26,7 +26,7 @@ RSpec.describe "ProductGroups", swagger_doc: "dfc.yaml" do before { login_as user } - path "/api/dfc/enterprises/{enterprise_id}/product_groups/{id}" do + path "/api/dfc/product_groups/{id}" do parameter name: :enterprise_id, in: :path, type: :string parameter name: :id, in: :path, type: :string @@ -39,7 +39,7 @@ RSpec.describe "ProductGroups", swagger_doc: "dfc.yaml" do let(:id) { product.id } run_test! do - expect(json_response["@id"]).to eq "http://test.host/api/dfc/enterprises/10000/product_groups/90000" + expect(json_response["@id"]).to eq "http://test.host/api/dfc/product_groups/90000" expect(json_response["dfc-b:hasVariant"]).to eq "http://test.host/api/dfc/enterprises/10000/supplied_products/10001" end diff --git a/engines/dfc_provider/spec/requests/supplied_products_spec.rb b/engines/dfc_provider/spec/requests/supplied_products_spec.rb index 2f7855d20d..a5f7d857ab 100644 --- a/engines/dfc_provider/spec/requests/supplied_products_spec.rb +++ b/engines/dfc_provider/spec/requests/supplied_products_spec.rb @@ -191,7 +191,7 @@ RSpec.describe "SuppliedProducts", type: :request, swagger_doc: "dfc.yaml" do run_test! do expect(response.body).to include variant.name - expect(json_response["dfc-b:isVariantOf"]).to eq "http://test.host/api/dfc/enterprises/10000/product_groups/90000" + expect(json_response["dfc-b:isVariantOf"]).to eq "http://test.host/api/dfc/product_groups/90000" expect(json_response["ofn:spree_product_id"]).to eq 90_000 expect(json_response["dfc-b:hasType"]).to eq("dfc-pt:processed-vegetable") expect(json_response["ofn:image"]).to include("logo-white.png") diff --git a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb index 2a4fc0e856..d344f1653b 100644 --- a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb +++ b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb @@ -244,7 +244,7 @@ RSpec.describe SuppliedProductImporter do end let(:product_group) do DataFoodConsortium::Connector::SuppliedProduct.new( - "http://test.host/api/dfc/enterprises/7/product_groups/6" + "http://test.host/api/dfc/product_groups/6" ) end @@ -259,7 +259,7 @@ RSpec.describe SuppliedProductImporter do expect(imported_product.description).to eq("Awesome tomato") expect(imported_product.variant_unit).to eq("weight") expect(imported_product.semantic_link.semantic_id) - .to eq "http://test.host/api/dfc/enterprises/7/product_groups/6" + .to eq "http://test.host/api/dfc/product_groups/6" end end end @@ -282,7 +282,7 @@ RSpec.describe SuppliedProductImporter do variant.save! supplied_product.isVariantOf << DataFoodConsortium::Connector::SuppliedProduct.new( - "http://test.host/api/dfc/enterprises/7/product_groups/6" + "http://test.host/api/dfc/product_groups/6" ) expect(result).to eq spree_product end diff --git a/swagger/dfc.yaml b/swagger/dfc.yaml index eb38ea6532..81b21a3bd3 100644 --- a/swagger/dfc.yaml +++ b/swagger/dfc.yaml @@ -177,10 +177,10 @@ paths: "@type": dfc-b:QuantitativeValue dfc-b:hasUnit: dfc-m:Gram dfc-b:value: 1.0 - dfc-b:isVariantOf: http://test.host/api/dfc/enterprises/10000/product_groups/90000 + dfc-b:isVariantOf: http://test.host/api/dfc/product_groups/90000 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/product_groups/90000 + - "@id": http://test.host/api/dfc/product_groups/90000 "@type": dfc-b:SuppliedProduct dfc-b:name: Apple dfc-b:hasVariant: http://test.host/api/dfc/enterprises/10000/supplied_products/10001 @@ -468,7 +468,7 @@ paths: dfc-b:hasUnit: dfc-m:Gram dfc-b:value: 1.0 dfc-b:image: http://test.host/rails/active_storage/url/logo-white.png - dfc-b:isVariantOf: http://test.host/api/dfc/enterprises/10000/product_groups/90000 + dfc-b:isVariantOf: http://test.host/api/dfc/product_groups/90000 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 @@ -558,7 +558,7 @@ paths: "@type": dfc-b:Person '404': description: not found - "/api/dfc/enterprises/{enterprise_id}/product_groups/{id}": + "/api/dfc/product_groups/{id}": parameters: - name: enterprise_id in: path @@ -583,7 +583,7 @@ paths: test_example: value: "@context": https://www.datafoodconsortium.org - "@id": http://test.host/api/dfc/enterprises/10000/product_groups/90000 + "@id": http://test.host/api/dfc/product_groups/90000 "@type": dfc-b:SuppliedProduct dfc-b:name: Pesto dfc-b:hasVariant: http://test.host/api/dfc/enterprises/10000/supplied_products/10001 @@ -650,7 +650,7 @@ paths: "@type": dfc-b:QuantitativeValue dfc-b:hasUnit: dfc-m:Gram dfc-b:value: 6.0 - dfc-b:isVariantOf: http://test.host/api/dfc/enterprises/10000/product_groups/90000 + dfc-b:isVariantOf: http://test.host/api/dfc/product_groups/90000 ofn:spree_product_id: 90000 ofn:spree_product_uri: http://test.host/api/dfc/enterprises/10000?spree_product_id=90000 dfc-b:image: http://test.host/rails/active_storage/url/logo-white.png @@ -711,7 +711,7 @@ paths: dfc-b:hasUnit: dfc-m:Gram dfc-b:value: 1.0 dfc-b:image: http://test.host/rails/active_storage/url/logo-white.png - dfc-b:isVariantOf: http://test.host/api/dfc/enterprises/10000/product_groups/90000 + dfc-b:isVariantOf: http://test.host/api/dfc/product_groups/90000 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