From 67c29dd38fa5716a85f21f2b1fea782c32c0639c Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Tue, 16 May 2023 15:08:00 +1000 Subject: [PATCH 01/11] Activate DFC provider engine, remove feature toggle --- config/routes/api.rb | 20 +++++++++----------- lib/open_food_network/feature_toggle.rb | 3 --- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/config/routes/api.rb b/config/routes/api.rb index dbca958689..2344a2212c 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -6,18 +6,16 @@ Openfoodnetwork::Application.routes.draw do namespace :api do - constraints FeatureToggleConstraint.new(:dfc_provider) do - # Mount DFC API endpoints - # - # We're using the DFC Connector which produces DFC v1.7 data but the - # DFC prototype is still pointing to the old URL. We keep it for - # testing. - mount DfcProvider::Engine, at: '/dfc-v1.6/', as: :legacy_dfc_provider_engine + # Mount DFC API endpoints + # + # We're using the DFC Connector which produces DFC v1.7 data but the + # DFC prototype is still pointing to the old URL. We keep it for + # testing. + mount DfcProvider::Engine, at: '/dfc-v1.6/', as: :legacy_dfc_provider_engine - # The DFC API version depends on the version of the - # datafoodconsortium-connector gem. - mount DfcProvider::Engine, at: '/dfc-v1.7/' - end + # The DFC API version depends on the version of the + # datafoodconsortium-connector gem. + mount DfcProvider::Engine, at: '/dfc-v1.7/' namespace :v0 do resources :products do diff --git a/lib/open_food_network/feature_toggle.rb b/lib/open_food_network/feature_toggle.rb index 9252fcbcc9..667eb44e65 100644 --- a/lib/open_food_network/feature_toggle.rb +++ b/lib/open_food_network/feature_toggle.rb @@ -21,9 +21,6 @@ module OpenFoodNetwork "background_reports" => <<~DESC, Generate reports in a background process to limit memory consumption. DESC - "dfc_provider" => <<~DESC, - Enable the DFC compatible endpoint at /api/dfc-*. - DESC "match_shipping_categories" => <<~DESC, During checkout, show only shipping methods that support all shipping categories. Activating this feature for an enterprise owner From d338c61d2c96c671d5652e667d61366d0066ca19 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 10 May 2023 11:57:27 +1000 Subject: [PATCH 02/11] Convert PersonsController spec request spec It's much more realistic and also tests the routing and authentication. Besides using real authentication I also improved the matchers. --- .../dfc_provider/persons_controller_spec.rb | 45 ------------------- .../spec/requests/persons_spec.rb | 23 ++++++++++ engines/dfc_provider/spec/spec_helper.rb | 3 ++ .../spec/support/authorization_helper.rb | 5 +++ 4 files changed, 31 insertions(+), 45 deletions(-) delete mode 100644 engines/dfc_provider/spec/controllers/dfc_provider/persons_controller_spec.rb create mode 100644 engines/dfc_provider/spec/requests/persons_spec.rb diff --git a/engines/dfc_provider/spec/controllers/dfc_provider/persons_controller_spec.rb b/engines/dfc_provider/spec/controllers/dfc_provider/persons_controller_spec.rb deleted file mode 100644 index fc388f4913..0000000000 --- a/engines/dfc_provider/spec/controllers/dfc_provider/persons_controller_spec.rb +++ /dev/null @@ -1,45 +0,0 @@ -# frozen_string_literal: true - -require DfcProvider::Engine.root.join("spec/spec_helper") - -describe DfcProvider::PersonsController, type: :controller do - render_views - - let!(:user) { create(:user) } - - describe '.show' do - context 'with authorization token' do - before do - request.headers['Authorization'] = 'Bearer 123456.abcdef.123456' - end - - context 'with an authenticated user' do - before do - allow_any_instance_of(AuthorizationControl) - .to receive(:user) - .and_return(user) - end - - context 'given with an accessible id' do - before { api_get :show, id: user.id } - - it 'is successful' do - expect(response).to be_successful - end - - it 'renders the required content' do - expect(response.body).to include('dfc-b:Person') - end - end - - context 'with an other user id' do - before { api_get :show, id: create(:user).id } - - it 'is not found' do - expect(response).to be_not_found - end - end - end - end - end -end diff --git a/engines/dfc_provider/spec/requests/persons_spec.rb b/engines/dfc_provider/spec/requests/persons_spec.rb new file mode 100644 index 0000000000..6acea2d5f0 --- /dev/null +++ b/engines/dfc_provider/spec/requests/persons_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require DfcProvider::Engine.root.join("spec/spec_helper") + +describe "Persons", type: :request do + let(:user) { create(:oidc_user) } + let(:other_user) { create(:oidc_user) } + + describe :show do + it "returns the authenticated user" do + get person_path(user), headers: auth_header(user.uid) + expect(response).to have_http_status :ok + expect(response.body).to include "dfc-b:Person" + expect(response.body).to include "persons/#{user.id}" + end + + it "doesn't find another user" do + get person_path(other_user), headers: auth_header(user.uid) + expect(response).to have_http_status :not_found + expect(response.body).to_not include "dfc-b:Person" + end + end +end diff --git a/engines/dfc_provider/spec/spec_helper.rb b/engines/dfc_provider/spec/spec_helper.rb index 966c2242e4..6c2b4ea8b0 100644 --- a/engines/dfc_provider/spec/spec_helper.rb +++ b/engines/dfc_provider/spec/spec_helper.rb @@ -5,6 +5,9 @@ require_relative '../../../spec/spec_helper' Dir["#{File.dirname(__FILE__)}/support/**/*.rb"].sort.each { |f| require f } RSpec.configure do |config| + config.include AuthorizationHelper, type: :request + config.include DfcProvider::Engine.routes.url_helpers, type: :request + config.around(:each) do |example| # The DFC Connector fetches the context when loaded. VCR.use_cassette("dfc-context") do diff --git a/engines/dfc_provider/spec/support/authorization_helper.rb b/engines/dfc_provider/spec/support/authorization_helper.rb index 4d8a42fdad..b35cc34860 100644 --- a/engines/dfc_provider/spec/support/authorization_helper.rb +++ b/engines/dfc_provider/spec/support/authorization_helper.rb @@ -1,6 +1,11 @@ # frozen_string_literal: true module AuthorizationHelper + def auth_header(email) + token = allow_token_for(email: email) + { "Authorization" => "JWT #{token}" } + end + def authorise(email) token = allow_token_for(email: email) request.headers["Authorization"] = "JWT #{token}" From 742468efd20b086f7dc0676098aab36978c155b1 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 10 May 2023 14:24:49 +1000 Subject: [PATCH 03/11] Convert EnterpriseController spec to request spec Also testing request of unrelated enterprise instead of non-existing enterprise because authorisation is important to test. --- .../dfc_provider/enterprises_spec.rb | 54 ------------------- .../spec/requests/enterprises_spec.rb | 35 ++++++++++++ 2 files changed, 35 insertions(+), 54 deletions(-) delete mode 100644 engines/dfc_provider/spec/controllers/dfc_provider/enterprises_spec.rb create mode 100644 engines/dfc_provider/spec/requests/enterprises_spec.rb diff --git a/engines/dfc_provider/spec/controllers/dfc_provider/enterprises_spec.rb b/engines/dfc_provider/spec/controllers/dfc_provider/enterprises_spec.rb deleted file mode 100644 index 1567616207..0000000000 --- a/engines/dfc_provider/spec/controllers/dfc_provider/enterprises_spec.rb +++ /dev/null @@ -1,54 +0,0 @@ -# frozen_string_literal: true - -require DfcProvider::Engine.root.join("spec/spec_helper") - -describe DfcProvider::EnterprisesController, type: :controller do - render_views - - let!(:user) { create(:user) } - let!(:enterprise) { create(:distributor_enterprise, owner: user) } - let!(:product) { create(:simple_product, supplier: enterprise ) } - - describe '.show' do - context 'with authorization token' do - before do - request.headers['Authorization'] = 'Bearer 123456.abcdef.123456' - end - - context 'with an authenticated user' do - before do - allow_any_instance_of(AuthorizationControl) - .to receive(:user) - .and_return(user) - end - - context 'with an enterprise' do - context 'given with an id' do - before { api_get :show, id: 'default' } - - it 'is successful' do - expect(response).to be_successful - end - - it 'renders the required content' do - expect(response.body) - .to include(product.name) - expect(response.body) - .to include(product.sku) - expect(response.body) - .to include("offers/#{product.variants.first.id}") - end - end - - context 'given with a wrong id' do - before { api_get :show, id: 999 } - - it 'is not found' do - expect(response).to be_not_found - end - end - end - end - end - end -end diff --git a/engines/dfc_provider/spec/requests/enterprises_spec.rb b/engines/dfc_provider/spec/requests/enterprises_spec.rb new file mode 100644 index 0000000000..0d9e88eff9 --- /dev/null +++ b/engines/dfc_provider/spec/requests/enterprises_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require DfcProvider::Engine.root.join("spec/spec_helper") + +describe "Enterprises", type: :request do + let!(:user) { create(:oidc_user) } + let!(:enterprise) { create(:distributor_enterprise, owner: user) } + let!(:product) { create(:simple_product, supplier: enterprise ) } + + describe :show do + it "returns the default enterprise" do + get enterprise_path("default"), headers: auth_header(user.uid) + + expect(response).to have_http_status :ok + expect(response.body).to include(product.name) + expect(response.body).to include(product.sku) + expect(response.body).to include("offers/#{product.variants.first.id}") + end + + it "returns the requested enterprise" do + get enterprise_path(enterprise.id), headers: auth_header(user.uid) + + expect(response).to have_http_status :ok + expect(response.body).to include(product.name) + end + + it "returns not found for unrelated enterprise" do + other_enterprise = create(:distributor_enterprise) + get enterprise_path(other_enterprise.id), headers: auth_header(user.uid) + + expect(response).to have_http_status :not_found + expect(response.body).to_not include(product.name) + end + end +end From 207a15e55c85f019e04718564eb01535fe6a5ef4 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 10 May 2023 15:19:36 +1000 Subject: [PATCH 04/11] Convert SuppliedProducts controller spec to request spec --- .../supplied_products_controller_spec.rb | 80 ------------------- .../spec/requests/supplied_products_spec.rb | 58 ++++++++++++++ 2 files changed, 58 insertions(+), 80 deletions(-) delete mode 100644 engines/dfc_provider/spec/controllers/dfc_provider/supplied_products_controller_spec.rb create mode 100644 engines/dfc_provider/spec/requests/supplied_products_spec.rb diff --git a/engines/dfc_provider/spec/controllers/dfc_provider/supplied_products_controller_spec.rb b/engines/dfc_provider/spec/controllers/dfc_provider/supplied_products_controller_spec.rb deleted file mode 100644 index ca04ec9b91..0000000000 --- a/engines/dfc_provider/spec/controllers/dfc_provider/supplied_products_controller_spec.rb +++ /dev/null @@ -1,80 +0,0 @@ -# frozen_string_literal: true - -require DfcProvider::Engine.root.join("spec/spec_helper") - -describe DfcProvider::SuppliedProductsController, type: :controller do - include AuthorizationHelper - - render_views - - let!(:user) { create(:oidc_user) } - let!(:enterprise) { create(:distributor_enterprise, owner: user) } - let!(:product) { create(:simple_product, supplier: enterprise ) } - let!(:variant) { product.variants.first } - - describe '.show' do - context 'with authorization token' do - before do - request.headers['Authorization'] = 'Bearer 123456.abcdef.123456' - end - - context 'with an authenticated user' do - before do - allow_any_instance_of(AuthorizationControl) - .to receive(:user) - .and_return(user) - end - - context 'with an enterprise' do - context 'given with an id' do - before do - api_get :show, enterprise_id: 'default', id: variant.id - end - - it 'is successful' do - expect(response).to be_successful - end - - it 'renders the required content' do - expect(response.body).to include(variant.name) - end - end - - context 'given with a wrong id' do - before { api_get :show, enterprise_id: 'default', id: 999 } - - it 'is not found' do - expect(response).to be_not_found - end - end - end - end - end - end - - describe "#update" do - routes { DfcProvider::Engine.routes } - - it "requires authorisation" do - api_put :update, enterprise_id: "default", id: "0" - expect(response).to have_http_status :unauthorized - end - - describe "with authorisation" do - before { authorise user.email } - - it "updates the variant's name" do - params = { enterprise_id: enterprise.id, id: variant.id } - request_body = File.read(File.join(__dir__, "../../support/patch_product.json")) - - expect { - put(:update, params: params, body: request_body) - expect(response).to have_http_status :success - variant.reload - }.to change { - variant.name - } - end - end - end -end diff --git a/engines/dfc_provider/spec/requests/supplied_products_spec.rb b/engines/dfc_provider/spec/requests/supplied_products_spec.rb new file mode 100644 index 0000000000..766e38e641 --- /dev/null +++ b/engines/dfc_provider/spec/requests/supplied_products_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require DfcProvider::Engine.root.join("spec/spec_helper") + +describe "SuppliedProducts", type: :request do + let!(:user) { create(:oidc_user) } + let!(:enterprise) { create(:distributor_enterprise, owner: user) } + let!(:product) { create(:simple_product, supplier: enterprise ) } + let!(:variant) { product.variants.first } + + describe :show do + it "returns variants" do + get enterprise_supplied_product_path( + variant.id, enterprise_id: enterprise.id + ), headers: auth_header(user.uid) + + expect(response).to have_http_status :ok + expect(response.body).to include variant.name + end + + it "doesn't find unrelated variants" do + other_variant = create(:variant) + + get enterprise_supplied_product_path( + other_variant.id, enterprise_id: enterprise.id + ), headers: auth_header(user.uid) + + expect(response).to have_http_status :not_found + end + end + + describe :update do + it "requires authorisation" do + put enterprise_supplied_product_path( + variant.id, enterprise_id: enterprise.id + ), headers: {} + + expect(response).to have_http_status :unauthorized + end + + it "updates a variant's name" do + params = { enterprise_id: enterprise.id, id: variant.id } + request_body = DfcProvider::Engine.root.join("spec/support/patch_product.json").read + + expect { + put( + enterprise_supplied_product_path(params), + params: request_body, + headers: auth_header(user.uid) + ) + expect(response).to have_http_status :success + variant.reload + }.to change { + variant.name + } + end + end +end From dde4ea93346a7a586351a040d8e538cc8e526c80 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 10 May 2023 16:01:59 +1000 Subject: [PATCH 05/11] Convert CatalogItems controller spec to request spec --- .../catalog_items_controller_spec.rb | 141 ------------------ .../spec/requests/catalog_items_spec.rb | 97 ++++++++++++ engines/dfc_provider/spec/spec_helper.rb | 1 + 3 files changed, 98 insertions(+), 141 deletions(-) delete mode 100644 engines/dfc_provider/spec/controllers/dfc_provider/catalog_items_controller_spec.rb create mode 100644 engines/dfc_provider/spec/requests/catalog_items_spec.rb diff --git a/engines/dfc_provider/spec/controllers/dfc_provider/catalog_items_controller_spec.rb b/engines/dfc_provider/spec/controllers/dfc_provider/catalog_items_controller_spec.rb deleted file mode 100644 index 40b2227907..0000000000 --- a/engines/dfc_provider/spec/controllers/dfc_provider/catalog_items_controller_spec.rb +++ /dev/null @@ -1,141 +0,0 @@ -# frozen_string_literal: true - -require DfcProvider::Engine.root.join("spec/spec_helper") - -describe DfcProvider::CatalogItemsController, type: :controller do - include AuthorizationHelper - - render_views - - let!(:user) { create(:oidc_user) } - let!(:enterprise) { create(:distributor_enterprise, owner: user) } - let!(:product) { create(:simple_product, supplier: enterprise ) } - let!(:variant) { product.variants.first } - - describe '.index' do - context 'with authorization token' do - before { authorise user.email } - - context 'with an authenticated user' do - context 'with an enterprise' do - context 'given with an id' do - context 'related to the user' do - before { api_get :index, enterprise_id: 'default' } - - it 'is successful' do - expect(response).to have_http_status :success - end - - it 'renders the required content' do - expect(response.body) - .to include(variant.name) - expect(response.body) - .to include(variant.sku) - expect(response.body) - .to include("offers/#{variant.id}") - end - end - - context 'not related to the user' do - let(:enterprise) { create(:enterprise) } - - it 'returns not_found head' do - api_get :index, enterprise_id: enterprise.id - expect(response).to have_http_status :not_found - end - end - end - - context 'as default' do - before { api_get :index, enterprise_id: 'default' } - - it 'is successful' do - expect(response.status).to eq 200 - end - - it 'renders the required content' do - expect(response.body) - .to include(variant.name) - expect(response.body) - .to include(variant.sku) - expect(response.body) - .to include("offers/#{variant.id}") - end - end - end - - context 'without a recorded enterprise' do - let(:enterprise) { create(:enterprise) } - - it 'is not found' do - api_get :index, enterprise_id: 'default' - expect(response).to have_http_status :not_found - end - end - end - - context 'without an authenticated user' do - before { authorise "other@user.net" } - - it 'returns unauthorized head' do - authorise "other@user.net" - - api_get :index, enterprise_id: 'default' - expect(response.response_code).to eq(401) - end - end - end - - context 'without an authorization token' do - it 'returns unauthorized head' do - api_get :index, enterprise_id: enterprise.id - expect(response).to have_http_status :unauthorized - end - end - - context "when logged in as app user" do - it "is successful" do - sign_in user - api_get :index, enterprise_id: enterprise.id - expect(response).to have_http_status :success - end - end - end - - describe '.show' do - context 'with authorization token' do - before { authorise user.email } - - context 'with an authenticated user' do - context 'with an enterprise' do - context 'given with an id' do - before do - api_get :show, enterprise_id: enterprise.id, id: variant.id - end - - it 'is successful' do - expect(response).to have_http_status :success - end - - it 'renders the required content' do - expect(response.body).to include('dfc-b:CatalogItem') - expect(response.body).to include("offers/#{variant.id}") - end - end - - context 'with a variant not linked to the enterprise' do - before do - api_get :show, - enterprise_id: enterprise.id, - id: create(:simple_product).variants.first.id - end - - it 'is not found' do - expect(response).to have_http_status :not_found - end - end - end - end - end - end -end diff --git a/engines/dfc_provider/spec/requests/catalog_items_spec.rb b/engines/dfc_provider/spec/requests/catalog_items_spec.rb new file mode 100644 index 0000000000..f803354463 --- /dev/null +++ b/engines/dfc_provider/spec/requests/catalog_items_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require DfcProvider::Engine.root.join("spec/spec_helper") + +describe "CatalogItems", type: :request do + let(:user) { create(:oidc_user) } + let(:enterprise) { create(:distributor_enterprise, owner: user) } + let(:product) { create(:simple_product, supplier: enterprise ) } + let(:variant) { product.variants.first } + + describe :index do + it "returns not_found without enterprise" do + items_path = enterprise_catalog_items_path(enterprise_id: "default") + + get items_path, headers: auth_header(user.uid) + + expect(response).to have_http_status :not_found + end + + context "with existing variant" do + before { variant } + it "lists catalog items with offers of default enterprise" do + items_path = enterprise_catalog_items_path(enterprise_id: "default") + + get items_path, headers: auth_header(user.uid) + + expect(response).to have_http_status :ok + expect(response.body).to include variant.name + expect(response.body).to include variant.sku + expect(response.body).to include "offers/#{variant.id}" + end + + it "lists catalog items with offers of requested enterprise" do + items_path = enterprise_catalog_items_path(enterprise_id: enterprise.id) + + get items_path, headers: auth_header(user.uid) + + expect(response).to have_http_status :ok + expect(response.body).to include variant.name + expect(response.body).to include variant.sku + expect(response.body).to include "offers/#{variant.id}" + end + + it "returns not_found for unrelated enterprises" do + other_enterprise = create(:enterprise) + items_path = enterprise_catalog_items_path(enterprise_id: other_enterprise.id) + + get items_path, headers: auth_header(user.uid) + + expect(response).to have_http_status :not_found + end + + it "returns unauthorized for unauthenticated users" do + items_path = enterprise_catalog_items_path(enterprise_id: "default") + + get items_path, headers: {} + + expect(response).to have_http_status :unauthorized + end + + it "recognises app user sessions as logins" do + items_path = enterprise_catalog_items_path(enterprise_id: "default") + login_as user + + get items_path, headers: {} + + expect(response).to have_http_status :ok + end + end + end + + describe :show do + it "returns a catalog item with offer" do + item_path = enterprise_catalog_item_path( + variant, + enterprise_id: enterprise.id + ) + + get item_path, headers: auth_header(user.uid) + + expect(response).to have_http_status :ok + expect(response.body).to include "dfc-b:CatalogItem" + expect(response.body).to include "offers/#{variant.id}" + end + + it "returns not_found for unrelated variant" do + item_path = enterprise_catalog_item_path( + create(:variant), + enterprise_id: enterprise.id + ) + + get item_path, headers: auth_header(user.uid) + + expect(response).to have_http_status :not_found + end + end +end diff --git a/engines/dfc_provider/spec/spec_helper.rb b/engines/dfc_provider/spec/spec_helper.rb index 6c2b4ea8b0..2f805bc2b2 100644 --- a/engines/dfc_provider/spec/spec_helper.rb +++ b/engines/dfc_provider/spec/spec_helper.rb @@ -7,6 +7,7 @@ Dir["#{File.dirname(__FILE__)}/support/**/*.rb"].sort.each { |f| require f } RSpec.configure do |config| config.include AuthorizationHelper, type: :request config.include DfcProvider::Engine.routes.url_helpers, type: :request + config.include Warden::Test::Helpers, type: :request config.around(:each) do |example| # The DFC Connector fetches the context when loaded. From 7344eb678b0b924187ced8ab45c9e8319ca74ac5 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 10 May 2023 16:08:15 +1000 Subject: [PATCH 06/11] Remove now unused helper It was only used in controller specs which were all converted to request specs. --- engines/dfc_provider/spec/support/authorization_helper.rb | 5 ----- 1 file changed, 5 deletions(-) diff --git a/engines/dfc_provider/spec/support/authorization_helper.rb b/engines/dfc_provider/spec/support/authorization_helper.rb index b35cc34860..c6e32f86b5 100644 --- a/engines/dfc_provider/spec/support/authorization_helper.rb +++ b/engines/dfc_provider/spec/support/authorization_helper.rb @@ -6,11 +6,6 @@ module AuthorizationHelper { "Authorization" => "JWT #{token}" } end - def authorise(email) - token = allow_token_for(email: email) - request.headers["Authorization"] = "JWT #{token}" - end - def allow_token_for(payload) private_key = OpenSSL::PKey::RSA.generate 2048 allow(AuthorizationControl).to receive(:public_key). From 50ef06c973464da05dc96be82baba1e798feafd9 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 12 May 2023 16:17:31 +1000 Subject: [PATCH 07/11] Name DFC test data to clarify content The DFC has several products like PhysicalProduct and SuppliedProduct. Here we have a supplied product. --- engines/dfc_provider/spec/requests/supplied_products_spec.rb | 2 +- .../support/{patch_product.json => patch_supplied_product.json} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename engines/dfc_provider/spec/support/{patch_product.json => patch_supplied_product.json} (100%) diff --git a/engines/dfc_provider/spec/requests/supplied_products_spec.rb b/engines/dfc_provider/spec/requests/supplied_products_spec.rb index 766e38e641..9b0f3bd108 100644 --- a/engines/dfc_provider/spec/requests/supplied_products_spec.rb +++ b/engines/dfc_provider/spec/requests/supplied_products_spec.rb @@ -40,7 +40,7 @@ describe "SuppliedProducts", type: :request do it "updates a variant's name" do params = { enterprise_id: enterprise.id, id: variant.id } - request_body = DfcProvider::Engine.root.join("spec/support/patch_product.json").read + request_body = DfcProvider::Engine.root.join("spec/support/patch_supplied_product.json").read expect { put( diff --git a/engines/dfc_provider/spec/support/patch_product.json b/engines/dfc_provider/spec/support/patch_supplied_product.json similarity index 100% rename from engines/dfc_provider/spec/support/patch_product.json rename to engines/dfc_provider/spec/support/patch_supplied_product.json From 63837381e08bd13e06934f76f41d55b0e1b62727 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 12 May 2023 16:19:15 +1000 Subject: [PATCH 08/11] Update DFC test data with observation This request came from the current DFC prototype. The changes are probably implementing DFC v1.7. I observed by including `$request_body` in the nginx log on the staging server. --- .../dfc_provider/spec/support/patch_supplied_product.json | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/engines/dfc_provider/spec/support/patch_supplied_product.json b/engines/dfc_provider/spec/support/patch_supplied_product.json index 9e415bec9a..a13afd9299 100644 --- a/engines/dfc_provider/spec/support/patch_supplied_product.json +++ b/engines/dfc_provider/spec/support/patch_supplied_product.json @@ -7,12 +7,9 @@ "dfc-b": "http://static.datafoodconsortium.org/ontologies/DFC_BusinessOntology.owl#", "dfc-p": "http://static.datafoodconsortium.org/ontologies/DFC_ProductOntology.owl#", "dfc-t": "http://static.datafoodconsortium.org/ontologies/DFC_TechnicalOntology.owl#", - "dfc-u": "http://static.datafoodconsortium.org/data/units.rdf#", + "dfc-m": "http://static.datafoodconsortium.org/data/measures.rdf#", "dfc-pt": "http://static.datafoodconsortium.org/data/productTypes.rdf#", - "dfc-a": "http://static.datafoodconsortium.org/data/claims.rdf#", - "dfc-d": "http://static.datafoodconsortium.org/data/dimensions.rdf#", - "dfc-c": "http://static.datafoodconsortium.org/data/certifications.rdf#", - "dfc-g": "http://static.datafoodconsortium.org/data/geoOrigin.rdf#", + "dfc-f": "http://static.datafoodconsortium.org/data/facets.rdf#", "dfc-p:hasUnit": { "@type": "@id" }, From 48a52582e600cf789a955143b3df62f3015c578b Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 12 May 2023 16:25:34 +1000 Subject: [PATCH 09/11] Update variant description with DFC description We used the name before because the DFC Prototype only displays the description. --- .../dfc_provider/supplied_products_controller.rb | 4 +++- .../dfc_provider/spec/requests/supplied_products_spec.rb | 6 ++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/engines/dfc_provider/app/controllers/dfc_provider/supplied_products_controller.rb b/engines/dfc_provider/app/controllers/dfc_provider/supplied_products_controller.rb index d2ce02c9e0..78f513a5e1 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 @@ -15,7 +15,9 @@ module DfcProvider dfc_request = JSON.parse(request.body.read) return unless dfc_request.key?("dfc-b:description") - variant.product.update!(name: dfc_request["dfc-b:description"]) + variant.product.update!( + description: dfc_request["dfc-b:description"], + ) end private diff --git a/engines/dfc_provider/spec/requests/supplied_products_spec.rb b/engines/dfc_provider/spec/requests/supplied_products_spec.rb index 9b0f3bd108..593ce7c34d 100644 --- a/engines/dfc_provider/spec/requests/supplied_products_spec.rb +++ b/engines/dfc_provider/spec/requests/supplied_products_spec.rb @@ -38,7 +38,7 @@ describe "SuppliedProducts", type: :request do expect(response).to have_http_status :unauthorized end - it "updates a variant's name" do + it "updates a variant's attributes" do params = { enterprise_id: enterprise.id, id: variant.id } request_body = DfcProvider::Engine.root.join("spec/support/patch_supplied_product.json").read @@ -50,9 +50,7 @@ describe "SuppliedProducts", type: :request do ) expect(response).to have_http_status :success variant.reload - }.to change { - variant.name - } + }.to change { variant.description }.to("DFC-Pesto updated") end end end From 375b3a3cb0b0fc918f21f0242ba1bf2c29eab232 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 12 May 2023 16:49:47 +1000 Subject: [PATCH 10/11] Update variant's quantity from DFC --- .../controllers/dfc_provider/supplied_products_controller.rb | 5 +++++ engines/dfc_provider/spec/requests/supplied_products_spec.rb | 1 + .../dfc_provider/spec/support/patch_supplied_product.json | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) 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 78f513a5e1..034770bab2 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 @@ -18,6 +18,11 @@ module DfcProvider variant.product.update!( description: dfc_request["dfc-b:description"], ) + + # This input is DFC v1.6 currently sent by the DFC Prototype. + variant.update!( + unit_value: dfc_request["dfc-b:quantity"], + ) end private diff --git a/engines/dfc_provider/spec/requests/supplied_products_spec.rb b/engines/dfc_provider/spec/requests/supplied_products_spec.rb index 593ce7c34d..f48d761561 100644 --- a/engines/dfc_provider/spec/requests/supplied_products_spec.rb +++ b/engines/dfc_provider/spec/requests/supplied_products_spec.rb @@ -51,6 +51,7 @@ describe "SuppliedProducts", type: :request do expect(response).to have_http_status :success variant.reload }.to change { variant.description }.to("DFC-Pesto updated") + .and change { variant.unit_value }.to(17) end end end diff --git a/engines/dfc_provider/spec/support/patch_supplied_product.json b/engines/dfc_provider/spec/support/patch_supplied_product.json index a13afd9299..86230c7e7a 100644 --- a/engines/dfc_provider/spec/support/patch_supplied_product.json +++ b/engines/dfc_provider/spec/support/patch_supplied_product.json @@ -81,5 +81,5 @@ } }, "dfc-b:description": "DFC-Pesto updated", - "dfc-b:quantity": 0 + "dfc-b:quantity": 17 } From 0dcd87dda948b2f6fe297d880626334313d9922c Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 12 May 2023 16:50:28 +1000 Subject: [PATCH 11/11] Update stock and SKU from DFC The input has been observed with the nginx access log including $request_body when the DFC Protoype pushed an update. --- .../dfc_provider/catalog_items_controller.rb | 8 ++ engines/dfc_provider/config/routes.rb | 2 +- .../spec/requests/catalog_items_spec.rb | 18 ++++ .../spec/support/patch_catalog_item.json | 85 +++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 engines/dfc_provider/spec/support/patch_catalog_item.json 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 bb1cafbdb3..03a85910b0 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 @@ -23,6 +23,14 @@ module DfcProvider render json: DfcLoader.connector.export(catalog_item, *offers) end + def update + dfc_request = JSON.parse(request.body.read) + + variant.on_hand = dfc_request["dfc-b:stockLimitation"] + variant.sku = dfc_request["dfc-b:sku"] + variant.save! + end + private def variant diff --git a/engines/dfc_provider/config/routes.rb b/engines/dfc_provider/config/routes.rb index a551e620a1..f8fde9365b 100644 --- a/engines/dfc_provider/config/routes.rb +++ b/engines/dfc_provider/config/routes.rb @@ -2,7 +2,7 @@ DfcProvider::Engine.routes.draw do resources :enterprises, only: [:show] do - resources :catalog_items, only: [:index, :show] + resources :catalog_items, only: [:index, :show, :update] resources :supplied_products, only: [:show, :update] end resources :persons, only: [:show] diff --git a/engines/dfc_provider/spec/requests/catalog_items_spec.rb b/engines/dfc_provider/spec/requests/catalog_items_spec.rb index f803354463..5119e946e3 100644 --- a/engines/dfc_provider/spec/requests/catalog_items_spec.rb +++ b/engines/dfc_provider/spec/requests/catalog_items_spec.rb @@ -94,4 +94,22 @@ describe "CatalogItems", type: :request do expect(response).to have_http_status :not_found end end + + describe :update do + it "updates a variant's attributes" do + params = { enterprise_id: enterprise.id, id: variant.id } + request_body = DfcProvider::Engine.root.join("spec/support/patch_catalog_item.json").read + + expect { + put( + enterprise_catalog_item_path(params), + params: request_body, + headers: auth_header(user.uid) + ) + expect(response).to have_http_status :success + variant.reload + }.to change { variant.on_hand }.to(3) + .and change { variant.sku }.to("new-sku") + end + end end diff --git a/engines/dfc_provider/spec/support/patch_catalog_item.json b/engines/dfc_provider/spec/support/patch_catalog_item.json new file mode 100644 index 0000000000..37fc582307 --- /dev/null +++ b/engines/dfc_provider/spec/support/patch_catalog_item.json @@ -0,0 +1,85 @@ +{ + "@context": { + "rdfs": "http://www.w3.org/2000/01/rdf-schema#", + "skos": "http://www.w3.org/2004/02/skos/core#", + "dfc": "http://static.datafoodconsortium.org/ontologies/DFC_FullModel.owl#", + "dc": "http://purl.org/dc/elements/1.1/#", + "dfc-b": "http://static.datafoodconsortium.org/ontologies/DFC_BusinessOntology.owl#", + "dfc-p": "http://static.datafoodconsortium.org/ontologies/DFC_ProductOntology.owl#", + "dfc-t": "http://static.datafoodconsortium.org/ontologies/DFC_TechnicalOntology.owl#", + "dfc-m": "http://static.datafoodconsortium.org/data/measures.rdf#", + "dfc-pt": "http://static.datafoodconsortium.org/data/productTypes.rdf#", + "dfc-f": "http://static.datafoodconsortium.org/data/facets.rdf#", + "dfc-p:hasUnit": { + "@type": "@id" + }, + "dfc-b:hasUnit": { + "@type": "@id" + }, + "dfc-b:hasQuantity": { + "@type": "@id" + }, + "dfc-p:hasType": { + "@type": "@id" + }, + "dfc-b:hasType": { + "@type": "@id" + }, + "dfc-b:references": { + "@type": "@id" + }, + "dfc-b:referencedBy": { + "@type": "@id" + }, + "dfc-b:offeres": { + "@type": "@id" + }, + "dfc-b:supplies": { + "@type": "@id" + }, + "dfc-b:defines": { + "@type": "@id" + }, + "dfc-b:affiliates": { + "@type": "@id" + }, + "dfc-b:manages": { + "@type": "@id" + }, + "dfc-b:offeredThrough": { + "@type": "@id" + }, + "dfc-b:hasBrand": { + "@type": "@id" + }, + "dfc-b:hasGeographicalOrigin": { + "@type": "@id" + }, + "dfc-b:hasClaim": { + "@type": "@id" + }, + "dfc-b:hasAllergenDimension": { + "@type": "@id" + }, + "dfc-b:hasNutrimentDimension": { + "@type": "@id" + }, + "dfc-b:hasPhysicalDimension": { + "@type": "@id" + }, + "dfc:owner": { + "@type": "@id" + }, + "dfc-t:hostedBy": { + "@type": "@id" + }, + "dfc-t:hasPivot": { + "@type": "@id" + }, + "dfc-t:represent": { + "@type": "@id" + } + }, + "dfc-b:stockLimitation": "3", + "dfc-b:sku": "new-sku" +}