From 699db020986ea759c288f7534c439438768d424d Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 18 Feb 2025 16:02:30 +1100 Subject: [PATCH] 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