From 447ff3cffded277f18b147b1b0b0a596e8b33874 Mon Sep 17 00:00:00 2001 From: David Cook Date: Thu, 6 Feb 2025 16:23:26 +1100 Subject: [PATCH] Show list of products to import If there's a matching product in OFN already, a link will appear. --- .../admin/dfc_product_imports_controller.rb | 51 ++++++++++++++----- app/models/spree/ability.rb | 2 +- .../admin/dfc_product_imports/index.html.haml | 24 ++++++++- .../product_import/_dfc_import_form.html.haml | 2 +- config/locales/en.yml | 9 +++- spec/system/admin/dfc_product_import_spec.rb | 34 ++++++++----- 6 files changed, 90 insertions(+), 32 deletions(-) diff --git a/app/controllers/admin/dfc_product_imports_controller.rb b/app/controllers/admin/dfc_product_imports_controller.rb index 8e43a65281..860e3fc15c 100644 --- a/app/controllers/admin/dfc_product_imports_controller.rb +++ b/app/controllers/admin/dfc_product_imports_controller.rb @@ -5,32 +5,49 @@ require "private_address_check/tcpsocket_ext" module Admin class DfcProductImportsController < Spree::Admin::BaseController + before_action :load_enterprise + # Define model class for `can?` permissions: def model_class self.class end - def import - # The plan: - # - # * Fetch DFC catalog as JSON from URL. - enterprise = OpenFoodNetwork::Permissions.new(spree_current_user) - .managed_product_enterprises.is_primary_producer - .find(params.require(:enterprise_id)) + def index + # Fetch DFC catalog JSON for preview + api = DfcRequest.new(spree_current_user) + @catalog_url = params.require(:catalog_url) + @catalog_json = api.call(@catalog_url) + graph = DfcIo.import(@catalog_json) + catalog = DfcCatalog.new(graph) - catalog_url = params.require(:catalog_url) - catalog = DfcCatalog.load(spree_current_user, catalog_url) + # Render table and let user decide which ones to import. + @items = catalog.products.map do |subject| + [ + subject, + @enterprise.supplied_variants.linked_to(subject.semanticId)&.product + ] + end + rescue Faraday::Error, + Addressable::URI::InvalidURIError, + ActionController::ParameterMissing => e + flash[:error] = e.message + redirect_to admin_product_import_path + end + + def import + # Load DFC catalog JSON + graph = DfcIo.import(params.require(:catalog_json)) + catalog = DfcCatalog.new(graph) catalog.apply_wholesale_values! - # * First step: import all products for given enterprise. - # * Second step: render table and let user decide which ones to import. + # Import all selected products for given enterprise. imported = catalog.products.map do |subject| - existing_variant = enterprise.supplied_variants.linked_to(subject.semanticId) + existing_variant = @enterprise.supplied_variants.linked_to(subject.semanticId) if existing_variant SuppliedProductBuilder.update_product(subject, existing_variant) else - SuppliedProductBuilder.store_product(subject, enterprise) + SuppliedProductBuilder.store_product(subject, @enterprise) end end @@ -41,5 +58,13 @@ module Admin flash[:error] = e.message redirect_to admin_product_import_path end + + private + + def load_enterprise + @enterprise = OpenFoodNetwork::Permissions.new(spree_current_user) + .managed_product_enterprises.is_primary_producer + .find(params.require(:enterprise_id)) + end end end diff --git a/app/models/spree/ability.rb b/app/models/spree/ability.rb index 7ca34c2c12..3bce37ff4d 100644 --- a/app/models/spree/ability.rb +++ b/app/models/spree/ability.rb @@ -238,7 +238,7 @@ module Spree can [:admin, :index, :guide, :import, :save, :save_data, :validate_data, :reset_absent_products], ProductImport::ProductImporter - can [:admin, :index], ::Admin::DfcProductImportsController + can [:admin, :index, :import], ::Admin::DfcProductImportsController # Reports page can [:admin, :index, :show, :create], ::Admin::ReportsController diff --git a/app/views/admin/dfc_product_imports/index.html.haml b/app/views/admin/dfc_product_imports/index.html.haml index 4b832fd79d..ad594df8af 100644 --- a/app/views/admin/dfc_product_imports/index.html.haml +++ b/app/views/admin/dfc_product_imports/index.html.haml @@ -3,4 +3,26 @@ = render partial: 'spree/admin/shared/product_sub_menu' -watch this space +%p= t('.catalog_url', catalog_url: @catalog_url) +%p= t('.enterprise', enterprise_name: @enterprise.name) +%br + += form_with url: main_app.import_admin_dfc_product_imports_path do |form| + -# This is a very inefficient way of holding a json blob. Maybe base64 encode or store as a temporary file + = form.hidden_field :enterprise_id, value: @enterprise.id + = form.hidden_field :catalog_json, value: @catalog_json + + %table + %tbody + - @items.each do |supplied_product, existing_product| + %tr{id: supplied_product.semanticId } + %td= supplied_product.name + %td + - if existing_product.present? + Update + = link_to(existing_product.id, edit_admin_product_path(existing_product)) + - else + New + + %br + = form.submit t(".import") diff --git a/app/views/admin/product_import/_dfc_import_form.html.haml b/app/views/admin/product_import/_dfc_import_form.html.haml index e70cd219f8..deb7c01a09 100644 --- a/app/views/admin/product_import/_dfc_import_form.html.haml +++ b/app/views/admin/product_import/_dfc_import_form.html.haml @@ -13,4 +13,4 @@ = form.select :enterprise_id, options_from_collection_for_select(@producers, :id, :name, @producers.first&.id), { "data-controller": "tom-select", class: "primary" } %br %br - = form.submit t(".import") + = form.submit t(".preview") diff --git a/config/locales/en.yml b/config/locales/en.yml index 5ee7ba0659..c115fd5642 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -846,8 +846,13 @@ en: map: Map dfc_product_imports: + index: + title: "DFC product catalog" + catalog_url: "Products to be imported from: %{catalog_url}" + enterprise: "Import to enterprise: %{enterprise_name}" + import: Import import: - title: "Importing a DFC product catalog" + title: "DFC product catalog import" imported_products: "Imported products:" enterprise_fees: index: @@ -1036,7 +1041,7 @@ en: title: "Import from DFC catalog" enterprise: "Enterprise" catalog_url: "DFC catalog URL" - import: "Import" + preview: Preview import: review: Review import: Import diff --git a/spec/system/admin/dfc_product_import_spec.rb b/spec/system/admin/dfc_product_import_spec.rb index bae9881278..ffa742caa8 100644 --- a/spec/system/admin/dfc_product_import_spec.rb +++ b/spec/system/admin/dfc_product_import_spec.rb @@ -7,8 +7,8 @@ RSpec.describe "DFC Product Import" do include AuthorizationHelper let(:user) { create(:oidc_user, owned_enterprises: [enterprise]) } - let(:enterprise) { create(:supplier_enterprise) } - let(:source_product) { create(:product, supplier_id: enterprise.id) } + let(:enterprise) { create(:supplier_enterprise, name: "Saucy preserves") } + let(:source_product) { create(:product, name: "Sauce", supplier_id: enterprise.id) } before do login_as user @@ -20,13 +20,16 @@ RSpec.describe "DFC Product Import" do it "imports from given catalog" do visit admin_product_import_path - select enterprise.name, from: "Enterprise" - # We are testing against our own catalog for now but we want to replace # this with the URL of another app when available. host = Rails.application.default_url_options[:host] url = "http://#{host}/api/dfc/enterprises/#{enterprise.id}/catalog_items" fill_in "catalog_url", with: url + select enterprise.name, from: "Enterprise" + click_button "Preview" + + expect(page).to have_content "Saucy preserves" + expect(page).to have_content "Sauce - 1g New" # By feeding our own catalog to the import, we are effectively cloning the # products. But the DFC product references the spree_product_id which @@ -34,16 +37,15 @@ RSpec.describe "DFC Product Import" do # a new independent product. expect { click_button "Import" + expect(page).to have_content "Imported products: 1" }.to change { source_product.variants.count }.by(1) - - expect(page).to have_content "Importing a DFC product catalog" - expect(page).to have_content "Imported products: 1" end it "imports from a FDC catalog", vcr: true do user.update!(oidc_account: build(:testdfc_account)) + # One product is existing in OFN product_id = "https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts/44519466467635" linked_variant = source_product.variants.first @@ -51,13 +53,19 @@ RSpec.describe "DFC Product Import" do visit admin_product_import_path - select enterprise.name, from: "Enterprise" - url = "https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts" fill_in "catalog_url", with: url + select enterprise.name, from: "Enterprise" + click_button "Preview" + + expect(page).to have_content "Saucy preserves" + expect(page).not_to have_content "Sauce - 1g" # Does not show other product + expect(page).to have_content "Beans - Retail can, 400g (can) Update" # existing product + expect(page).to have_content "Beans - Case, 12 x 400g (can) New" expect { click_button "Import" + expect(page).to have_content "Imported products: 4" linked_variant.reload }.to change { enterprise.supplied_products.count } .and change { linked_variant.display_name } @@ -67,8 +75,6 @@ RSpec.describe "DFC Product Import" do .and change { linked_variant.on_demand }.to(true) .and change { linked_variant.on_hand }.by(0) - expect(page).to have_content "Importing a DFC product catalog" - product = Spree::Product.last expect(product.variants[0].semantic_links).to be_present expect(product.image).to be_present @@ -86,18 +92,18 @@ RSpec.describe "DFC Product Import" do select enterprise.name, from: "Enterprise" fill_in "catalog_url", with: url - expect { click_button "Import" }.not_to change { Spree::Variant.count } + expect { click_button "Preview" }.not_to change { Spree::Variant.count } expect(page).to have_content "the server responded with status 401" select enterprise.name, from: "Enterprise" fill_in "catalog_url", with: "badurl" - click_button "Import" + click_button "Preview" expect(page).to have_content "Absolute URI missing hierarchical segment: 'http://:80'" select enterprise.name, from: "Enterprise" fill_in "catalog_url", with: "" - click_button "Import" + click_button "Preview" expect(page).to have_content "param is missing or the value is empty: catalog_url" end end