From df30c1af98d088b1d8eb7e04d9f9bbd9c63b2d50 Mon Sep 17 00:00:00 2001 From: Filipe <49817236+filipefurtad0@users.noreply.github.com> Date: Thu, 13 Jun 2024 08:47:05 -0600 Subject: [PATCH 1/2] Revert "[Invoices] Notify if any order cannot be invoiced on bulk invoice sending" --- app/reflexes/admin/orders_reflex.rb | 49 +++++++------------ spec/system/admin/orders/bulk_actions_spec.rb | 17 +------ 2 files changed, 19 insertions(+), 47 deletions(-) diff --git a/app/reflexes/admin/orders_reflex.rb b/app/reflexes/admin/orders_reflex.rb index 160877bb51..dedf46cdf2 100644 --- a/app/reflexes/admin/orders_reflex.rb +++ b/app/reflexes/admin/orders_reflex.rb @@ -33,9 +33,19 @@ module Admin end def bulk_invoice(params) - visible_orders = bulk_load_orders(params) + visible_orders = editable_orders.invoiceable.where(id: params[:bulk_ids]) - return if notify_if_abn_related_issue(visible_orders) + if Spree::Config.enterprise_number_required_on_invoices? + distributors_without_abn = Enterprise.where( + id: visible_orders.select(:distributor_id), + abn: nil, + ) + + if distributors_without_abn.exists? + render_business_number_required_error(distributors_without_abn) + return + end + end cable_ready.append( selector: "#orders-index", @@ -84,15 +94,15 @@ module Admin end def send_invoices(params) - orders = bulk_load_orders(params) + count = 0 + editable_orders.invoiceable.where(id: params[:bulk_ids]).find_each do |o| + next unless o.distributor.can_invoice? - return if notify_if_abn_related_issue(orders) - - orders.each do |o| Spree::OrderMailer.invoice_email(o.id, current_user_id: current_user.id).deliver_later + count += 1 end - success("admin.send_invoice_feedback", orders.size) + success("admin.send_invoice_feedback", count) end private @@ -124,30 +134,5 @@ module Admin enterprise_name: distributor_names.join(", ")) morph_admin_flashes end - - def bulk_load_orders(params) - editable_orders.invoiceable.where(id: params[:bulk_ids]) - end - - def notify_if_abn_related_issue(orders) - return false unless abn_required? - - distributors = distributors_without_abn(orders) - return false if distributors.empty? - - render_business_number_required_error(distributors) - true - end - - def abn_required? - Spree::Config.enterprise_number_required_on_invoices? - end - - def distributors_without_abn(orders) - Enterprise.where( - id: orders.select(:distributor_id), - abn: [nil, ""], - ) - end end end diff --git a/spec/system/admin/orders/bulk_actions_spec.rb b/spec/system/admin/orders/bulk_actions_spec.rb index 72ccc44d95..e93cfe4b5f 100644 --- a/spec/system/admin/orders/bulk_actions_spec.rb +++ b/spec/system/admin/orders/bulk_actions_spec.rb @@ -363,22 +363,9 @@ RSpec.describe ' } must have a valid ABN before invoices can be used." end end - context "ABN is null" do + it_behaves_like "should not print the invoice" + context "with legal invoices feature", feature: :invoices do it_behaves_like "should not print the invoice" - context "with legal invoices feature", feature: :invoices do - it_behaves_like "should not print the invoice" - end - end - context "ABN is empty string" do - before do - order4.distributor.update(abn: "123456789") - order5.distributor.update(abn: "") - end - - it_behaves_like "should not print the invoice" - context "with legal invoices feature", feature: :invoices do - it_behaves_like "should not print the invoice" - end end end end From 6a4a2383784218b69f4b6a1671c087a5e3b8adaf Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 14 Jun 2024 10:47:35 +1000 Subject: [PATCH 2/2] Avoid flakiness with Capybara features Capybara should be clever enought to scroll to an element. The old method failed nine times in CI. I couldn't reproduce it locally but let's see if this is better. --- .../complex_editing_multiple_product_pages_spec.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/system/admin/order_cycles/complex_editing_multiple_product_pages_spec.rb b/spec/system/admin/order_cycles/complex_editing_multiple_product_pages_spec.rb index 23aa655a77..ddc888700e 100644 --- a/spec/system/admin/order_cycles/complex_editing_multiple_product_pages_spec.rb +++ b/spec/system/admin/order_cycles/complex_editing_multiple_product_pages_spec.rb @@ -36,10 +36,7 @@ RSpec.describe ' end it "select all products" do - checkbox_id = "order_cycle_incoming_exchange_0_select_all_variants" - elmnt = find_field(id: checkbox_id) - scroll_to(elmnt, align: :top) - check checkbox_id + check "Select All 2 Variants" expect_all_products_loaded