From 6e9089ad4792c16e0a33f0ec0ab9179c8f659e9e Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Mon, 11 Dec 2023 09:42:55 +0100 Subject: [PATCH 1/5] check ABN before bulk printing --- app/reflexes/admin/orders_reflex.rb | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/app/reflexes/admin/orders_reflex.rb b/app/reflexes/admin/orders_reflex.rb index b3145214a1..a50979d2d9 100644 --- a/app/reflexes/admin/orders_reflex.rb +++ b/app/reflexes/admin/orders_reflex.rb @@ -32,13 +32,16 @@ module Admin end def bulk_invoice(params) + visible_orders = editable_orders.where(id: params[:bulk_ids]).filter(&:invoiceable?) + return unless all_distributors_can_invoice?(visible_orders) + cable_ready.append( selector: "#orders-index", html: render(partial: "spree/admin/orders/bulk/invoice_modal") ).broadcast BulkInvoiceJob.perform_later( - params[:bulk_ids], + visible_orders.pluck(:id), "tmp/invoices/#{Time.zone.now.to_i}-#{SecureRandom.hex(2)}.pdf", channel: SessionChannel.for_request(request), current_user_id: current_user.id @@ -106,5 +109,16 @@ module Admin def set_param_for_controller params[:id] = @order.number end + + def all_distributors_can_invoice?(orders) + distributors = orders.map(&:distributor).uniq.reject(&:can_invoice?) + + return true if distributors.empty? + + flash[:error] = I18n.t(:must_have_valid_business_number, + enterprise_name: distributors.map(&:name).join(", ")) + morph_admin_flashes + false + end end end From b669b804c4c60a9dfdb74ce73bfd81bc8f65d7fd Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Thu, 14 Dec 2023 14:46:24 +0100 Subject: [PATCH 2/5] update tests --- spec/system/admin/orders_spec.rb | 135 ++++++++++++++++++++++++++++++- 1 file changed, 131 insertions(+), 4 deletions(-) diff --git a/spec/system/admin/orders_spec.rb b/spec/system/admin/orders_spec.rb index b9f29d296b..b465d3947b 100644 --- a/spec/system/admin/orders_spec.rb +++ b/spec/system/admin/orders_spec.rb @@ -583,10 +583,13 @@ describe ' reader.pages.map(&:text) end + let(:order4_selector){ "#order_#{order4.id} input[name='bulk_ids[]']" } + let(:order5_selector){ "#order_#{order5.id} input[name='bulk_ids[]']" } + shared_examples "can bulk print invoices from 2 orders" do it "bulk prints invoices in pdf format" do - page.find("#listing_orders tbody tr:nth-child(1) input[name='bulk_ids[]']").click - page.find("#listing_orders tbody tr:nth-child(2) input[name='bulk_ids[]']").click + page.find(order4_selector).click + page.find(order5_selector).click page.find("span.icon-reorder", text: "ACTIONS").click within ".ofn-drop-down .menu" do @@ -621,10 +624,134 @@ describe ' end end - it_behaves_like "can bulk print invoices from 2 orders" + shared_examples "should ignore the non invoiceable order" do + it "bulk prints invoices in pdf format" do + page.find(order4_selector).click + page.find(order5_selector).click - context "with legal invoices feature", feature: :invoices do + page.find("span.icon-reorder", text: "ACTIONS").click + within ".ofn-drop-down .menu" do + expect { + page.find("span", text: "Print Invoices").click # Prints invoices in bulk + }.to enqueue_job(BulkInvoiceJob).exactly(:once) + end + + expect(page).to have_content "Compiling Invoices" + expect(page).to have_content "Please wait until the PDF is ready " \ + "before closing this modal." + + perform_enqueued_jobs(only: BulkInvoiceJob) + + expect(page).to have_content "Bulk Invoice created" + + within ".modal-content" do + expect(page).to have_link(class: "button", text: "VIEW FILE", + href: /invoices/) + + invoice_content = extract_pdf_content + + expect(invoice_content).to have_content("TAX INVOICE", count: 1) + expect(invoice_content).to_not have_content(order4.number.to_s) + expect(invoice_content).to have_content(order5.number.to_s) + expect(invoice_content).to_not have_content(distributor4.name.to_s) + expect(invoice_content).to have_content(distributor5.name.to_s) + expect(invoice_content).to_not have_content(order_cycle4.name.to_s) + expect(invoice_content).to have_content(order_cycle5.name.to_s) + end + end + end + + context "ABN is not required" do + before do + allow(Spree::Config).to receive(:enterprise_number_required_on_invoices?) + .and_return false + end it_behaves_like "can bulk print invoices from 2 orders" + + context "with legal invoices feature", feature: :invoices do + it_behaves_like "can bulk print invoices from 2 orders" + end + + context "one of the two orders is not invoiceable" do + before do + order4.cancel! + assert(!order4.invoiceable?) + assert(order5.invoiceable?) + end + + it_behaves_like "should ignore the non invoiceable order" + context "with legal invoices feature", feature: :invoices do + it_behaves_like "should ignore the non invoiceable order" + end + end + end + + context "ABN is required" do + before do + allow(Spree::Config).to receive(:enterprise_number_required_on_invoices?) + .and_return true + end + context "All the distributors setup the ABN" do + before do + order4.distributor.update(abn: "123456789") + order5.distributor.update(abn: "987654321") + end + context "all the orders are invoiceable (completed/resumed)" do + before do + assert(order4.invoiceable?) + assert(order5.invoiceable?) + end + it_behaves_like "can bulk print invoices from 2 orders" + context "with legal invoices feature", feature: :invoices do + it_behaves_like "can bulk print invoices from 2 orders" + end + end + + context "one of the two orders is not invoiceable" do + before do + order4.cancel! + assert(!order4.invoiceable?) + assert(order5.invoiceable?) + end + + it_behaves_like "should ignore the non invoiceable order" + context "with legal invoices feature", feature: :invoices do + it_behaves_like "should ignore the non invoiceable order" + end + end + end + context "the distributor of one of the order didn't set the ABN" do + before do + order4.distributor.update(abn: "123456789") + order5.distributor.update(abn: nil) + end + + shared_examples "should not print the invoice" do + it "should render a warning message" do + page.find(order4_selector).click + page.find(order5_selector).click + + page.find("span.icon-reorder", text: "ACTIONS").click + within ".ofn-drop-down .menu" do + expect { + page.find("span", text: "Print Invoices").click # Prints invoices in bulk + }.to_not enqueue_job(BulkInvoiceJob) + end + + expect(page).to_not have_content "Compiling Invoices" + expect(page).to_not have_content "Please wait until the PDF is ready " \ + "before closing this modal." + + expect(page).to have_content "#{ + order5.distributor.name + } must have a valid ABN before invoices can be sent." + end + 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 f582bffbc506d85b8bca9a27dd69b5ec613af509 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 29 Dec 2023 15:27:34 +0100 Subject: [PATCH 3/5] remove assertions before tests --- spec/system/admin/orders_spec.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/spec/system/admin/orders_spec.rb b/spec/system/admin/orders_spec.rb index b465d3947b..4f5ef06f1f 100644 --- a/spec/system/admin/orders_spec.rb +++ b/spec/system/admin/orders_spec.rb @@ -675,8 +675,6 @@ describe ' context "one of the two orders is not invoiceable" do before do order4.cancel! - assert(!order4.invoiceable?) - assert(order5.invoiceable?) end it_behaves_like "should ignore the non invoiceable order" @@ -697,10 +695,6 @@ describe ' order5.distributor.update(abn: "987654321") end context "all the orders are invoiceable (completed/resumed)" do - before do - assert(order4.invoiceable?) - assert(order5.invoiceable?) - end it_behaves_like "can bulk print invoices from 2 orders" context "with legal invoices feature", feature: :invoices do it_behaves_like "can bulk print invoices from 2 orders" @@ -710,8 +704,6 @@ describe ' context "one of the two orders is not invoiceable" do before do order4.cancel! - assert(!order4.invoiceable?) - assert(order5.invoiceable?) end it_behaves_like "should ignore the non invoiceable order" From 64b42b128424ca39ffcc1a23f477a65f2b3ca7bc Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Mon, 19 Feb 2024 09:26:27 +0100 Subject: [PATCH 4/5] improve all_distributors_can_invoice? --- app/reflexes/admin/orders_reflex.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/app/reflexes/admin/orders_reflex.rb b/app/reflexes/admin/orders_reflex.rb index a50979d2d9..fe84ebf7ea 100644 --- a/app/reflexes/admin/orders_reflex.rb +++ b/app/reflexes/admin/orders_reflex.rb @@ -33,7 +33,11 @@ module Admin def bulk_invoice(params) visible_orders = editable_orders.where(id: params[:bulk_ids]).filter(&:invoiceable?) - return unless all_distributors_can_invoice?(visible_orders) + if Spree::Config.enterprise_number_required_on_invoices? && + !all_distributors_can_invoice?(visible_orders) + render_business_number_required_error(visible_orders) + return + end cable_ready.append( selector: "#orders-index", @@ -111,14 +115,17 @@ module Admin end def all_distributors_can_invoice?(orders) - distributors = orders.map(&:distributor).uniq.reject(&:can_invoice?) + distributor_ids = orders.map(&:distributor_id) + Enterprise.where(id: distributor_ids, abn: nil).empty? + end - return true if distributors.empty? + def render_business_number_required_error(orders) + distributor_ids = orders.map(&:distributor_id) + distributor_names = Enterprise.where(id: distributor_ids, abn: nil).pluck(:name) flash[:error] = I18n.t(:must_have_valid_business_number, - enterprise_name: distributors.map(&:name).join(", ")) + enterprise_name: distributor_names.join(", ")) morph_admin_flashes - false end end end From 8370a5fed06ad8a984f10e6aa4e8dc30c508957c Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Mon, 19 Feb 2024 11:00:55 +0100 Subject: [PATCH 5/5] fix existing tests --- spec/system/admin/orders_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/system/admin/orders_spec.rb b/spec/system/admin/orders_spec.rb index 4f5ef06f1f..4f2d58b189 100644 --- a/spec/system/admin/orders_spec.rb +++ b/spec/system/admin/orders_spec.rb @@ -736,7 +736,7 @@ describe ' expect(page).to have_content "#{ order5.distributor.name - } must have a valid ABN before invoices can be sent." + } must have a valid ABN before invoices can be used." end end it_behaves_like "should not print the invoice"