From a6c08fe2ad2ec82a87583da3aeff3178be213f6e Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 5 Mar 2025 16:18:38 +1100 Subject: [PATCH 01/12] Reset stock for absent products in DFC catalog --- .../admin/dfc_product_imports_controller.rb | 19 +++++++++++++++++++ .../dfc_product_imports/import.html.haml | 3 +++ config/locales/en.yml | 1 + spec/system/admin/dfc_product_import_spec.rb | 18 ++++++++++++++++-- 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/dfc_product_imports_controller.rb b/app/controllers/admin/dfc_product_imports_controller.rb index 4e87b6b2ac..4b45f4543b 100644 --- a/app/controllers/admin/dfc_product_imports_controller.rb +++ b/app/controllers/admin/dfc_product_imports_controller.rb @@ -58,6 +58,7 @@ module Admin end @count = imported.compact.count + @reset_count = reset_absent_variants(catalog).count rescue ActionController::ParameterMissing => e flash[:error] = e.message redirect_to admin_product_import_path @@ -80,5 +81,23 @@ module Admin ] end end + + def reset_absent_variants(catalog) + present_ids = catalog.products.map(&:semanticId) + absent_variants = @enterprise.supplied_variants + .includes(:semantic_links).references(:semantic_links) + .where.not(semantic_links: { semantic_id: present_ids }) + + catalog_url = FdcUrlBuilder.new(present_ids.first).catalog_url + absent_variants.select do |variant| + # Variants that were in the same catalog before: + variant.semantic_links.map(&:semantic_id).any? do |semantic_id| + FdcUrlBuilder.new(semantic_id).catalog_url == catalog_url + end + end.map do |variant| + variant.on_demand = false + variant.on_hand = 0 + end + end end end diff --git a/app/views/admin/dfc_product_imports/import.html.haml b/app/views/admin/dfc_product_imports/import.html.haml index 60a3f7a05a..4d54558c7c 100644 --- a/app/views/admin/dfc_product_imports/import.html.haml +++ b/app/views/admin/dfc_product_imports/import.html.haml @@ -5,3 +5,6 @@ %p= t(".imported_products") = @count + +%p= t(".reset_products") += @reset_count diff --git a/config/locales/en.yml b/config/locales/en.yml index bc312c91ba..e805e26559 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -866,6 +866,7 @@ en: import: title: "DFC product catalog import" imported_products: "Imported products:" + reset_products: "Stock reset for absent products:" enterprise_fees: index: title: "Enterprise Fees" diff --git a/spec/system/admin/dfc_product_import_spec.rb b/spec/system/admin/dfc_product_import_spec.rb index 03c86eb379..f5f0ae9c97 100644 --- a/spec/system/admin/dfc_product_import_spec.rb +++ b/spec/system/admin/dfc_product_import_spec.rb @@ -9,6 +9,7 @@ RSpec.describe "DFC Product Import" do let(:user) { create(:oidc_user, owned_enterprises: [enterprise]) } let(:enterprise) { create(:supplier_enterprise, name: "Saucy preserves") } let(:source_product) { create(:product, name: "Sauce", supplier_id: enterprise.id) } + let(:old_product) { create(:product, name: "Best Sauce of 1995", supplier_id: enterprise.id) } before do login_as user @@ -52,12 +53,21 @@ RSpec.describe "DFC Product Import" do it "imports from a FDC catalog", vcr: true do user.update!(oidc_account: build(:testdfc_account)) - # One product is existing in OFN + + # One current 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 linked_variant.semantic_links << SemanticLink.new(semantic_id: product_id) + # One outdated product still exists in OFN + old_product_id = + "https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts/445194664-1995" + unlinked_variant = old_product.variants.first + unlinked_variant.semantic_links << SemanticLink.new(semantic_id: old_product_id) + unlinked_variant.on_demand = true + unlinked_variant.on_hand = 3 + visit admin_product_import_path url = "https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts" @@ -83,14 +93,18 @@ RSpec.describe "DFC Product Import" do expect { click_button "Import" expect(page).to have_content "Imported products: 3" + expect(page).to have_content "Stock reset for absent products: 1" linked_variant.reload - }.to change { enterprise.supplied_products.count }.by(2) # 1 updated, 2 new + unlinked_variant.reload + }.to change { enterprise.supplied_products.count }.by(2) # 1 updated, 2 new, 1 reset .and change { linked_variant.display_name } .and change { linked_variant.unit_value } # 18.85 wholesale variant price divided by 12 cans in the slab. .and change { linked_variant.price }.to(1.57) .and change { linked_variant.on_demand }.to(true) .and change { linked_variant.on_hand }.by(0) + .and change { unlinked_variant.on_demand }.to(false) + .and change { unlinked_variant.on_hand }.to(0) product = Spree::Product.last expect(product.variants[0].semantic_links).to be_present From 11be8360e3f16d6bc7a61035274a568a07a4b7cb Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Wed, 5 Mar 2025 16:23:02 +1100 Subject: [PATCH 02/12] Prettier and localised display of import stats --- app/views/admin/dfc_product_imports/import.html.haml | 6 ++---- config/locales/en.yml | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/app/views/admin/dfc_product_imports/import.html.haml b/app/views/admin/dfc_product_imports/import.html.haml index 4d54558c7c..e84a56f3cc 100644 --- a/app/views/admin/dfc_product_imports/import.html.haml +++ b/app/views/admin/dfc_product_imports/import.html.haml @@ -3,8 +3,6 @@ = render partial: 'spree/admin/shared/product_sub_menu' -%p= t(".imported_products") -= @count +%p= t(".imported_products", count: @count) -%p= t(".reset_products") -= @reset_count +%p= t(".reset_products", count: @reset_count) diff --git a/config/locales/en.yml b/config/locales/en.yml index e805e26559..2f1a41b4e7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -865,8 +865,8 @@ en: invalid_url: This catalog URL is not valid. import: title: "DFC product catalog import" - imported_products: "Imported products:" - reset_products: "Stock reset for absent products:" + imported_products: "Imported products: %{count}" + reset_products: "Stock reset for absent products: %{count}" enterprise_fees: index: title: "Enterprise Fees" From a2e68e1f3ca4152afc39e19b69ee2535fc09a59f Mon Sep 17 00:00:00 2001 From: Maikel Date: Thu, 6 Mar 2025 08:49:59 +1100 Subject: [PATCH 03/12] Explain stock reset further Co-authored-by: David Cook --- app/controllers/admin/dfc_product_imports_controller.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/controllers/admin/dfc_product_imports_controller.rb b/app/controllers/admin/dfc_product_imports_controller.rb index 4b45f4543b..cfd7ba339a 100644 --- a/app/controllers/admin/dfc_product_imports_controller.rb +++ b/app/controllers/admin/dfc_product_imports_controller.rb @@ -82,6 +82,12 @@ module Admin end end + # Reset stock for any variants that were removed from the catalog. + # + # When variants are removed from the remote catalog, there not for sale + # anymore. We prevent them from being sold by reseting stock to zero. + # We don't delete the variant because it may come back at a later time and + # we don't want to lose the connection to previous orders. def reset_absent_variants(catalog) present_ids = catalog.products.map(&:semanticId) absent_variants = @enterprise.supplied_variants From c60718feea239fa441bf52855b41570d4f8f1fce Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 6 Mar 2025 09:29:35 +1100 Subject: [PATCH 04/12] List absent products in import preview --- .../admin/dfc_product_imports_controller.rb | 29 +++++++++++-------- .../_absent_variant.html.haml | 8 +++++ .../dfc_product_imports/import.html.haml | 2 +- .../admin/dfc_product_imports/index.html.haml | 2 ++ config/locales/en.yml | 10 +++++++ spec/system/admin/dfc_product_import_spec.rb | 2 ++ 6 files changed, 40 insertions(+), 13 deletions(-) create mode 100644 app/views/admin/dfc_product_imports/_absent_variant.html.haml diff --git a/app/controllers/admin/dfc_product_imports_controller.rb b/app/controllers/admin/dfc_product_imports_controller.rb index cfd7ba339a..d5d2699808 100644 --- a/app/controllers/admin/dfc_product_imports_controller.rb +++ b/app/controllers/admin/dfc_product_imports_controller.rb @@ -21,6 +21,7 @@ module Admin # Render table and let user decide which ones to import. @items = list_products(catalog) + @absent_items = absent_variants(catalog) rescue URI::InvalidURIError flash[:error] = t ".invalid_url" redirect_to admin_product_import_path @@ -89,21 +90,25 @@ module Admin # We don't delete the variant because it may come back at a later time and # we don't want to lose the connection to previous orders. def reset_absent_variants(catalog) - present_ids = catalog.products.map(&:semanticId) - absent_variants = @enterprise.supplied_variants - .includes(:semantic_links).references(:semantic_links) - .where.not(semantic_links: { semantic_id: present_ids }) - - catalog_url = FdcUrlBuilder.new(present_ids.first).catalog_url - absent_variants.select do |variant| - # Variants that were in the same catalog before: - variant.semantic_links.map(&:semantic_id).any? do |semantic_id| - FdcUrlBuilder.new(semantic_id).catalog_url == catalog_url - end - end.map do |variant| + absent_variants(catalog).map do |variant| variant.on_demand = false variant.on_hand = 0 end end + + def absent_variants(catalog) + present_ids = catalog.products.map(&:semanticId) + catalog_url = FdcUrlBuilder.new(present_ids.first).catalog_url + + @enterprise.supplied_variants + .includes(:semantic_links).references(:semantic_links) + .where.not(semantic_links: { semantic_id: present_ids }) + .select do |variant| + # Variants that were in the same catalog before: + variant.semantic_links.map(&:semantic_id).any? do |semantic_id| + FdcUrlBuilder.new(semantic_id).catalog_url == catalog_url + end + end + end end end diff --git a/app/views/admin/dfc_product_imports/_absent_variant.html.haml b/app/views/admin/dfc_product_imports/_absent_variant.html.haml new file mode 100644 index 0000000000..e0bd9bad91 --- /dev/null +++ b/app/views/admin/dfc_product_imports/_absent_variant.html.haml @@ -0,0 +1,8 @@ +%tr + %td + %label + ❌ + = absent_variant.product_and_full_name + %td + = t(".reset") + = link_to(absent_variant.product_id, edit_admin_product_path(absent_variant.product_id)) diff --git a/app/views/admin/dfc_product_imports/import.html.haml b/app/views/admin/dfc_product_imports/import.html.haml index e84a56f3cc..63ca01a33b 100644 --- a/app/views/admin/dfc_product_imports/import.html.haml +++ b/app/views/admin/dfc_product_imports/import.html.haml @@ -5,4 +5,4 @@ %p= t(".imported_products", count: @count) -%p= t(".reset_products", count: @reset_count) +%p= t(".reset_products", count: @reset_count) if @reset_count.positive? diff --git a/app/views/admin/dfc_product_imports/index.html.haml b/app/views/admin/dfc_product_imports/index.html.haml index 720705d373..1b170e01da 100644 --- a/app/views/admin/dfc_product_imports/index.html.haml +++ b/app/views/admin/dfc_product_imports/index.html.haml @@ -6,6 +6,7 @@ %p= t('.catalog_url', count: @items.count, catalog_url: @catalog_url) %p= t('.enterprise', enterprise_name: @enterprise.name) + %p= t('.absent_products', count: @absent_items.count) %br = form_with url: main_app.import_admin_dfc_product_imports_path, html: { "data-controller": "checked" } do |form| @@ -32,6 +33,7 @@ = link_to(existing_product.id, edit_admin_product_path(existing_product)) - else = t(".new") + = render partial: "absent_variant", collection: @absent_items %span{ "data-controller": "checked-feedback", "data-checked-feedback-translation-value": "admin.dfc_product_imports.index.selected" } = t(".selected", count: @items.count) diff --git a/config/locales/en.yml b/config/locales/en.yml index 2f1a41b4e7..1added29b5 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -850,9 +850,19 @@ en: connection_invalid_html: | Connecting with your OIDC account failed. Please refresh your OIDC connection at: %{oidc_settings_link} + absent_variant: + reset: "Reset stock" index: title: "DFC product catalog" catalog_url: "%{count} products to be imported from: %{catalog_url}" + absent_products: + zero: "" + one: | + One product is no longer in the catalog. + It will be marked as unavailable by resetting stock to zero. + other: | + %{count} products are no longer in the catalog. + They will be marked as unavailable by resetting stock to zero. enterprise: "Import to enterprise: %{enterprise_name}" select_all: "Select/deselect all" update: Update diff --git a/spec/system/admin/dfc_product_import_spec.rb b/spec/system/admin/dfc_product_import_spec.rb index f5f0ae9c97..83c784b570 100644 --- a/spec/system/admin/dfc_product_import_spec.rb +++ b/spec/system/admin/dfc_product_import_spec.rb @@ -76,11 +76,13 @@ RSpec.describe "DFC Product Import" do click_button "Preview" expect(page).to have_content "4 products to be imported" + expect(page).to have_content "One product is no longer" 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(page).to have_content "Chia Seed, Organic - Retail pack, 300g" + expect(page).to have_content "Best Sauce of 1995 - 1g Reset stock" # I can select all uncheck "Chia Seed, Organic - Case, 8 x 300g" From 2a81d26ef1e3b8ee59bb6d7e87f7adf132af4c38 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 6 Mar 2025 09:31:15 +1100 Subject: [PATCH 05/12] Simplify complex method --- app/controllers/admin/dfc_product_imports_controller.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/app/controllers/admin/dfc_product_imports_controller.rb b/app/controllers/admin/dfc_product_imports_controller.rb index d5d2699808..30964fa313 100644 --- a/app/controllers/admin/dfc_product_imports_controller.rb +++ b/app/controllers/admin/dfc_product_imports_controller.rb @@ -14,7 +14,6 @@ module Admin def index # Fetch DFC catalog JSON for preview - api = DfcRequest.new(spree_current_user) @catalog_url = params.require(:catalog_url).strip @catalog_json = api.call(@catalog_url) catalog = DfcCatalog.from_json(@catalog_json) @@ -67,6 +66,10 @@ module Admin private + def api + @api ||= DfcRequest.new(spree_current_user) + end + def load_enterprise @enterprise = OpenFoodNetwork::Permissions.new(spree_current_user) .managed_product_enterprises.is_primary_producer From e35a29cc29700b6db681a8f7b95d34c9349b71a1 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 20 Mar 2025 16:25:15 +1100 Subject: [PATCH 06/12] Remember local stock of backordered products When retail variants are mapped to wholesale variants, we usually have a some leftover stock at the end of an order cycle. For example, we backordered a slab of 12 cans of tomatoes but our customers bought only 9 of those. Then we have 3 left for the next order cycle. Even when the product is not available for backorder with the supplier, we still want to sell off our leftover stock, the three cans of tomatoes in our example. And it might be that the product will come back in the future. --- .../admin/dfc_product_imports_controller.rb | 17 +++++++++++++---- spec/system/admin/dfc_product_import_spec.rb | 2 +- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/app/controllers/admin/dfc_product_imports_controller.rb b/app/controllers/admin/dfc_product_imports_controller.rb index 30964fa313..69e08280c0 100644 --- a/app/controllers/admin/dfc_product_imports_controller.rb +++ b/app/controllers/admin/dfc_product_imports_controller.rb @@ -88,14 +88,23 @@ module Admin # Reset stock for any variants that were removed from the catalog. # - # When variants are removed from the remote catalog, there not for sale - # anymore. We prevent them from being sold by reseting stock to zero. + # When variants are removed from the remote catalog, we can't place + # backorders for them anymore. If our copy of the product has limited + # stock then we need to set the stock to zero to prevent any more sales. + # + # But if our product is on-demand/backorderable then our stock level is + # a representation of remaining local stock. We then need to limit sales + # to this local stock and set on-demand to false. + # # We don't delete the variant because it may come back at a later time and # we don't want to lose the connection to previous orders. def reset_absent_variants(catalog) absent_variants(catalog).map do |variant| - variant.on_demand = false - variant.on_hand = 0 + if variant.on_demand + variant.on_demand = false + else + variant.on_hand = 0 + end end end diff --git a/spec/system/admin/dfc_product_import_spec.rb b/spec/system/admin/dfc_product_import_spec.rb index 83c784b570..a7d8b53c3f 100644 --- a/spec/system/admin/dfc_product_import_spec.rb +++ b/spec/system/admin/dfc_product_import_spec.rb @@ -106,7 +106,7 @@ RSpec.describe "DFC Product Import" do .and change { linked_variant.on_demand }.to(true) .and change { linked_variant.on_hand }.by(0) .and change { unlinked_variant.on_demand }.to(false) - .and change { unlinked_variant.on_hand }.to(0) + .and change { unlinked_variant.on_hand }.by(0) product = Spree::Product.last expect(product.variants[0].semantic_links).to be_present From c43bd0c24b8cfc0ddc95b05cf95d0271258b8c20 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 27 Mar 2025 12:51:57 +1100 Subject: [PATCH 07/12] Move variant reset code to shared service --- .../admin/dfc_product_imports_controller.rb | 13 +--------- app/services/dfc_catalog_importer.rb | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 12 deletions(-) create mode 100644 app/services/dfc_catalog_importer.rb diff --git a/app/controllers/admin/dfc_product_imports_controller.rb b/app/controllers/admin/dfc_product_imports_controller.rb index 69e08280c0..9b98598275 100644 --- a/app/controllers/admin/dfc_product_imports_controller.rb +++ b/app/controllers/admin/dfc_product_imports_controller.rb @@ -109,18 +109,7 @@ module Admin end def absent_variants(catalog) - present_ids = catalog.products.map(&:semanticId) - catalog_url = FdcUrlBuilder.new(present_ids.first).catalog_url - - @enterprise.supplied_variants - .includes(:semantic_links).references(:semantic_links) - .where.not(semantic_links: { semantic_id: present_ids }) - .select do |variant| - # Variants that were in the same catalog before: - variant.semantic_links.map(&:semantic_id).any? do |semantic_id| - FdcUrlBuilder.new(semantic_id).catalog_url == catalog_url - end - end + DfcCatalogImporter.new(@enterprise.supplied_variants, catalog).absent_variants end end end diff --git a/app/services/dfc_catalog_importer.rb b/app/services/dfc_catalog_importer.rb new file mode 100644 index 0000000000..c411353674 --- /dev/null +++ b/app/services/dfc_catalog_importer.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class DfcCatalogImporter + attr_reader :catalog, :existing_variants + + def initialize(existing_variants, catalog) + @existing_variants = existing_variants + @catalog = catalog + end + + def absent_variants + present_ids = catalog.products.map(&:semanticId) + catalog_url = FdcUrlBuilder.new(present_ids.first).catalog_url + + existing_variants + .includes(:semantic_links).references(:semantic_links) + .where.not(semantic_links: { semantic_id: present_ids }) + .select do |variant| + # Variants that were in the same catalog before: + variant.semantic_links.map(&:semantic_id).any? do |semantic_id| + FdcUrlBuilder.new(semantic_id).catalog_url == catalog_url + end + end + end +end From 48e3b5b05e3728005cd7f58b4154cc1f3888eb90 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 27 Mar 2025 13:33:52 +1100 Subject: [PATCH 08/12] Move more reset variant code for re-use --- .../admin/dfc_product_imports_controller.rb | 20 +---------------- app/services/dfc_catalog_importer.rb | 22 +++++++++++++++++++ 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/app/controllers/admin/dfc_product_imports_controller.rb b/app/controllers/admin/dfc_product_imports_controller.rb index 9b98598275..9b7c15a4e6 100644 --- a/app/controllers/admin/dfc_product_imports_controller.rb +++ b/app/controllers/admin/dfc_product_imports_controller.rb @@ -86,26 +86,8 @@ module Admin end end - # Reset stock for any variants that were removed from the catalog. - # - # When variants are removed from the remote catalog, we can't place - # backorders for them anymore. If our copy of the product has limited - # stock then we need to set the stock to zero to prevent any more sales. - # - # But if our product is on-demand/backorderable then our stock level is - # a representation of remaining local stock. We then need to limit sales - # to this local stock and set on-demand to false. - # - # We don't delete the variant because it may come back at a later time and - # we don't want to lose the connection to previous orders. def reset_absent_variants(catalog) - absent_variants(catalog).map do |variant| - if variant.on_demand - variant.on_demand = false - else - variant.on_hand = 0 - end - end + DfcCatalogImporter.new(@enterprise.supplied_variants, catalog).reset_absent_variants end def absent_variants(catalog) diff --git a/app/services/dfc_catalog_importer.rb b/app/services/dfc_catalog_importer.rb index c411353674..ab5837caee 100644 --- a/app/services/dfc_catalog_importer.rb +++ b/app/services/dfc_catalog_importer.rb @@ -8,6 +8,28 @@ class DfcCatalogImporter @catalog = catalog end + # Reset stock for any variants that were removed from the catalog. + # + # When variants are removed from the remote catalog, we can't place + # backorders for them anymore. If our copy of the product has limited + # stock then we need to set the stock to zero to prevent any more sales. + # + # But if our product is on-demand/backorderable then our stock level is + # a representation of remaining local stock. We then need to limit sales + # to this local stock and set on-demand to false. + # + # We don't delete the variant because it may come back at a later time and + # we don't want to lose the connection to previous orders. + def reset_absent_variants + absent_variants.map do |variant| + if variant.on_demand + variant.on_demand = false + else + variant.on_hand = 0 + end + end + end + def absent_variants present_ids = catalog.products.map(&:semanticId) catalog_url = FdcUrlBuilder.new(present_ids.first).catalog_url From 9e3ff412f94275c9f0c2ef0d18adf546e100e816 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 27 Mar 2025 13:35:59 +1100 Subject: [PATCH 09/12] Simplify controller --- .../admin/dfc_product_imports_controller.rb | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/app/controllers/admin/dfc_product_imports_controller.rb b/app/controllers/admin/dfc_product_imports_controller.rb index 9b7c15a4e6..eae8c092b5 100644 --- a/app/controllers/admin/dfc_product_imports_controller.rb +++ b/app/controllers/admin/dfc_product_imports_controller.rb @@ -20,7 +20,7 @@ module Admin # Render table and let user decide which ones to import. @items = list_products(catalog) - @absent_items = absent_variants(catalog) + @absent_items = importer(catalog).absent_variants rescue URI::InvalidURIError flash[:error] = t ".invalid_url" redirect_to admin_product_import_path @@ -58,7 +58,7 @@ module Admin end @count = imported.compact.count - @reset_count = reset_absent_variants(catalog).count + @reset_count = importer(catalog).reset_absent_variants.count rescue ActionController::ParameterMissing => e flash[:error] = e.message redirect_to admin_product_import_path @@ -86,12 +86,8 @@ module Admin end end - def reset_absent_variants(catalog) - DfcCatalogImporter.new(@enterprise.supplied_variants, catalog).reset_absent_variants - end - - def absent_variants(catalog) - DfcCatalogImporter.new(@enterprise.supplied_variants, catalog).absent_variants + def importer(catalog) + DfcCatalogImporter.new(@enterprise.supplied_variants, catalog) end end end From 1a5b6bbc8f2ec5ef07d58a7a816ceebec101b216 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 27 Mar 2025 14:40:50 +1100 Subject: [PATCH 10/12] Reset stock for unavailable products on OC open --- app/jobs/open_order_cycle_job.rb | 4 ++++ spec/jobs/open_order_cycle_job_spec.rb | 22 ++++++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/jobs/open_order_cycle_job.rb b/app/jobs/open_order_cycle_job.rb index 80c4e1d7b8..b11660ad94 100644 --- a/app/jobs/open_order_cycle_job.rb +++ b/app/jobs/open_order_cycle_job.rb @@ -57,6 +57,10 @@ class OpenOrderCycleJob < ApplicationJob catalog = DfcCatalog.load(dfc_user, catalog_url) catalog.apply_wholesale_values! + variants = Spree::Variant.where(id: catalog_links.pluck(:subject_id)) + importer = DfcCatalogImporter.new(variants, catalog) + importer.reset_absent_variants + catalog_links.each do |link| catalog_item = catalog.item(link.semantic_id) diff --git a/spec/jobs/open_order_cycle_job_spec.rb b/spec/jobs/open_order_cycle_job_spec.rb index 5c8574535e..c6d1d1d27e 100644 --- a/spec/jobs/open_order_cycle_job_spec.rb +++ b/spec/jobs/open_order_cycle_job_spec.rb @@ -33,21 +33,34 @@ RSpec.describe OpenOrderCycleJob do let(:enterprise) { create(:supplier_enterprise) } let!(:variant) { create(:variant, name: "Sauce", supplier_id: enterprise.id) } + let!(:variant_discontinued) { + create(:variant, name: "Shiraz 1971", supplier_id: enterprise.id) + } let!(:order_cycle) { - create(:simple_order_cycle, orders_open_at: now, - suppliers: [enterprise], variants: [variant]) + create( + :simple_order_cycle, + orders_open_at: now, + suppliers: [enterprise], + variants: [variant, variant_discontinued] + ) } it "synchronises products from a FDC catalog", vcr: true do user.update!(oidc_account: build(:testdfc_account)) - # One product is existing in OFN + # One current product is existing in OFN product_id = "https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts/44519466467635" variant.semantic_links << SemanticLink.new(semantic_id: product_id) + # One discontinued product is existing in OFN + old_product_id = + "https://env-0105831.jcloud-ver-jpe.ik-server.com/api/dfc/Enterprises/test-hodmedod/SuppliedProducts/44519466467635-disc" + variant_discontinued.semantic_links << SemanticLink.new(semantic_id: old_product_id) + expect { subject variant.reload + variant_discontinued.reload order_cycle.reload }.to change { order_cycle.opened_at } .and change { enterprise.supplied_products.count }.by(0) # It shouldn't add, only update @@ -57,7 +70,8 @@ RSpec.describe OpenOrderCycleJob do .and change { variant.price }.to(1.57) .and change { variant.on_demand }.to(true) .and change { variant.on_hand }.by(0) - .and query_database 46 + .and change { variant_discontinued.on_hand }.to(0) + .and query_database 58 end end From 406018e7ebbeb538b47ef4c0ed614f855f82925f Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Thu, 27 Mar 2025 14:54:14 +1100 Subject: [PATCH 11/12] Simplify resetting variants I was hoping to reduce the query count but it stayed the same. In fact, I'm expecting the query count to be higher with this version. The DfcCatalogImporter queries all variants and links at once while the OC job is not pre-loading the variants at the moment. We can optimise the job though. If we kept the old version and there were multiple catalogs per variant then we would call the importer with the reset multiple times. The job is iterating through each link only once though. So depending on the ratio of catalogs to variants, I'm not sure which version would be more efficient. No version is properly dealing with the edge-case of multiple catalogs per variant anyway. Imagine there are two catalogs with different stock levels. Which one do we choose? Or do we add up? And if the variant disappears from one catalog we still want to sell it through the other. But depending on the order of processing, we may reset the variant if it's missing in the last catalog. But let's worry about that when it actually happens. Maybe it will be better to restrict variants to one catalog. --- app/jobs/open_order_cycle_job.rb | 10 +++++----- app/services/dfc_catalog_importer.rb | 14 +++++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/app/jobs/open_order_cycle_job.rb b/app/jobs/open_order_cycle_job.rb index b11660ad94..c2f7dc9469 100644 --- a/app/jobs/open_order_cycle_job.rb +++ b/app/jobs/open_order_cycle_job.rb @@ -57,14 +57,14 @@ class OpenOrderCycleJob < ApplicationJob catalog = DfcCatalog.load(dfc_user, catalog_url) catalog.apply_wholesale_values! - variants = Spree::Variant.where(id: catalog_links.pluck(:subject_id)) - importer = DfcCatalogImporter.new(variants, catalog) - importer.reset_absent_variants - catalog_links.each do |link| catalog_item = catalog.item(link.semantic_id) - SuppliedProductImporter.update_product(catalog_item, link.subject) if catalog_item + if catalog_item + SuppliedProductImporter.update_product(catalog_item, link.subject) + else + DfcCatalogImporter.reset_variant(link.subject) + end end end end diff --git a/app/services/dfc_catalog_importer.rb b/app/services/dfc_catalog_importer.rb index ab5837caee..16d81ac6d3 100644 --- a/app/services/dfc_catalog_importer.rb +++ b/app/services/dfc_catalog_importer.rb @@ -1,6 +1,14 @@ # frozen_string_literal: true class DfcCatalogImporter + def self.reset_variant(variant) + if variant.on_demand + variant.on_demand = false + else + variant.on_hand = 0 + end + end + attr_reader :catalog, :existing_variants def initialize(existing_variants, catalog) @@ -22,11 +30,7 @@ class DfcCatalogImporter # we don't want to lose the connection to previous orders. def reset_absent_variants absent_variants.map do |variant| - if variant.on_demand - variant.on_demand = false - else - variant.on_hand = 0 - end + self.class.reset_variant(variant) end end From cf55a920da1fa728018d4383e0c2241882b94587 Mon Sep 17 00:00:00 2001 From: David Cook Date: Mon, 14 Apr 2025 15:39:44 +1000 Subject: [PATCH 12/12] Manually add variable for translation Normally I'd avoid editing these files, but this results in translations appearing exactly as they did prior to the PR. Ideally I would notify translators to re-translate but I don't know that there's a good process for that. Hopefully this is fine and doesn't cause any conflicts... --- config/locales/el.yml | 2 +- config/locales/en_CA.yml | 2 +- config/locales/en_FR.yml | 2 +- config/locales/en_GB.yml | 2 +- config/locales/en_IE.yml | 2 +- config/locales/es.yml | 2 +- config/locales/fr.yml | 2 +- config/locales/fr_CA.yml | 2 +- config/locales/hu.yml | 2 +- config/locales/nb.yml | 2 +- config/locales/ru.yml | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) diff --git a/config/locales/el.yml b/config/locales/el.yml index c600850757..45a3bda075 100644 --- a/config/locales/el.yml +++ b/config/locales/el.yml @@ -725,7 +725,7 @@ el: other: "%{count}επιλέχθηκε" import: Εισαγωγή import: - imported_products: "Εισαγόμενα προϊόντα:" + imported_products: "Εισαγόμενα προϊόντα: %{count}" enterprise_fees: index: title: "Τέλη επιχείρησης" diff --git a/config/locales/en_CA.yml b/config/locales/en_CA.yml index a9ed396751..840a879813 100644 --- a/config/locales/en_CA.yml +++ b/config/locales/en_CA.yml @@ -798,7 +798,7 @@ en_CA: invalid_url: This catalog URL is not valid. import: title: "DFC product catalog import" - imported_products: "Imported products:" + imported_products: "Imported products: %{count}" enterprise_fees: index: title: "Enterprise Fees" diff --git a/config/locales/en_FR.yml b/config/locales/en_FR.yml index ca7953798d..0bae68c049 100644 --- a/config/locales/en_FR.yml +++ b/config/locales/en_FR.yml @@ -798,7 +798,7 @@ en_FR: invalid_url: This catalog URL is not valid. import: title: "DFC product catalog import" - imported_products: "Imported products:" + imported_products: "Imported products: %{count}" enterprise_fees: index: title: "Enterprise Fees" diff --git a/config/locales/en_GB.yml b/config/locales/en_GB.yml index 90644562eb..bcf71e2bb8 100644 --- a/config/locales/en_GB.yml +++ b/config/locales/en_GB.yml @@ -771,7 +771,7 @@ en_GB: other: "%{count} selected" import: Import import: - imported_products: "Imported products:" + imported_products: "Imported products: %{count}" enterprise_fees: index: title: "Enterprise Fees" diff --git a/config/locales/en_IE.yml b/config/locales/en_IE.yml index 70a08c065e..fc141f7fba 100644 --- a/config/locales/en_IE.yml +++ b/config/locales/en_IE.yml @@ -771,7 +771,7 @@ en_IE: other: "%{count} selected" import: Import import: - imported_products: "Imported products:" + imported_products: "Imported products: %{count}" enterprise_fees: index: title: "Enterprise Fees" diff --git a/config/locales/es.yml b/config/locales/es.yml index 11fddddb0d..3f8abf4a96 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -765,7 +765,7 @@ es: new: Nuevo import: Importar import: - imported_products: "Productos importados:" + imported_products: "Productos importados: %{count}" enterprise_fees: index: title: "Comisiones de la Organización" diff --git a/config/locales/fr.yml b/config/locales/fr.yml index 4487bf161e..b948f139e9 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -797,7 +797,7 @@ fr: invalid_url: L'URL de ce catalogue n'est pas valide. import: title: "Import du catalogue produit DFC" - imported_products: "Produits importés :" + imported_products: "Produits importés : %{count}" enterprise_fees: index: title: "Marges et Commissions" diff --git a/config/locales/fr_CA.yml b/config/locales/fr_CA.yml index 5b506a3fb3..ad611a2aab 100644 --- a/config/locales/fr_CA.yml +++ b/config/locales/fr_CA.yml @@ -797,7 +797,7 @@ fr_CA: import: Importer import: title: "Import du catalogue produit DFC" - imported_products: "Produits importés :" + imported_products: "Produits importés : %{count}" enterprise_fees: index: title: "Marges et Commissions" diff --git a/config/locales/hu.yml b/config/locales/hu.yml index 57ad612eab..e6311d8930 100644 --- a/config/locales/hu.yml +++ b/config/locales/hu.yml @@ -760,7 +760,7 @@ hu: other: "%{count}kiválasztva" import: Importálás import: - imported_products: "Importált termékek:" + imported_products: "Importált termékek: %{count}" enterprise_fees: index: title: "Vállalkozási díjak" diff --git a/config/locales/nb.yml b/config/locales/nb.yml index 4382966b58..addf797520 100644 --- a/config/locales/nb.yml +++ b/config/locales/nb.yml @@ -771,7 +771,7 @@ nb: other: "%{count} valgt" import: Import import: - imported_products: "Importerte produkter:" + imported_products: "Importerte produkter: %{count}" enterprise_fees: index: title: "Bedriftsavgifter" diff --git a/config/locales/ru.yml b/config/locales/ru.yml index 3c93a67783..c8ed9f781c 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -711,7 +711,7 @@ ru: other: "выбрано %{count}" import: Импорт import: - imported_products: "Внесенные товары:" + imported_products: "Внесенные товары: %{count}" enterprise_fees: index: title: "Сборы Предприятия"