From 8b249ee050d992a55a4c76365b9b5df7e664c7e4 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Thu, 14 Sep 2023 18:02:48 +0100 Subject: [PATCH 1/5] Adds unit tests for different attributes --- .../services/order_invoice_comparator_spec.rb | 260 ++++++++++++++++-- 1 file changed, 243 insertions(+), 17 deletions(-) diff --git a/spec/services/order_invoice_comparator_spec.rb b/spec/services/order_invoice_comparator_spec.rb index 5470fa3929..c3cd679179 100644 --- a/spec/services/order_invoice_comparator_spec.rb +++ b/spec/services/order_invoice_comparator_spec.rb @@ -115,19 +115,217 @@ describe OrderInvoiceComparator do end end - context "a non-relevant associated model is updated" do - let(:distributor){ order.distributor } - it "returns false" do - distributor.update!(name: 'THIS IS A NEW NAME', abn: 'This is a new ABN') - expect(subject).to be false - end - end + context "changes on an associated order object" do + describe "detecting relevant associated object changes" do + context "adjustment changes" do + it "creating a new adjustment returns true" do + create(:adjustment, order_id: order.id) + order.reload + expect(subject).to be true + end - context "a relevant associated object is updated" do - let(:line_item){ order.line_items.first } - it "return true" do - line_item.update!(quantity: line_item.quantity + 1) - expect(subject).to be true + context "with an existing adjustment" do + let!(:adjustment) { create(:adjustment, order_id: order.id) } + + it "editing the amount returns true" do + adjustment.update!(amount: 123) + order.reload + expect(subject).to be true + end + + it "changing the adjustment type" do + adjustment.update!(adjustable_type: "Spree::Payment") + order.reload + expect(subject).to be true + end + + it "deleting the label" do + order.all_adjustments.destroy_all + order.reload + expect(subject).to be true + end + end + end + + context "line item changes" do + let(:line_item){ order.line_items.first } + + context "on quantitity" do + it "return true" do + line_item.update!(quantity: line_item.quantity + 1) + expect(subject).to be true + end + end + + context "on variant id" do + it "return true" do + line_item.update!(variant_id: Spree::Variant.first.id) + order.reload + expect(subject).to be true + end + end + end + + context "bill address changes on" do + let(:bill_address) { Spree::Address.where(id: order.bill_address_id) } + it "first name" do + bill_address.update!(firstname: "Jane") + order.reload + expect(subject).to be true + end + + it "last name" do + bill_address.update!(lastname: "Jones") + order.reload + expect(subject).to be true + end + + it "address (1)" do + bill_address.update!(address1: "Rue du Fromage 66") + order.reload + expect(subject).to be true + end + + it "address (2)" do + bill_address.update!(address2: "South by Southwest") + order.reload + expect(subject).to be true + end + + it "city" do + bill_address.update!(city: "Antibes") + order.reload + expect(subject).to be true + end + + it "zipcode" do + bill_address.update!(zipcode: "04229") + order.reload + expect(subject).to be true + end + + it "phone" do + bill_address.update!(phone: "111-222-333") + order.reload + expect(subject).to be true + end + + it "company" do + bill_address.update!(company: "A Company Name") + order.reload + expect(subject).to be true + end + end + + context "ship address changes on" do + let(:ship_address) { Spree::Address.where(id: order.ship_address_id) } + it "first name" do + ship_address.update!(firstname: "Jane") + order.reload + expect(subject).to be true + end + + it "last name" do + ship_address.update!(lastname: "Jones") + order.reload + expect(subject).to be true + end + + it "address (1)" do + ship_address.update!(address1: "Rue du Fromage 66") + order.reload + expect(subject).to be true + end + + it "address (2)" do + ship_address.update!(address2: "South by Southwest") + order.reload + expect(subject).to be true + end + + it "city" do + ship_address.update!(city: "Antibes") + order.reload + expect(subject).to be true + end + + it "zipcode" do + ship_address.update!(zipcode: "04229") + order.reload + expect(subject).to be true + end + + it "phone" do + ship_address.update!(phone: "111-222-333") + order.reload + expect(subject).to be true + end + + it "company" do + ship_address.update!(company: "A Company Name") + order.reload + expect(subject).to be true + end + end + + context "customer changes on" do + let(:customer){ order.customer } + + context "code" do + it "return false" do + customer.update!(code: 98_754) + order.reload + expect(subject).to be false + end + end + + context "email" do + it "return false" do + customer.update!(email: "customer@email.org") + order.reload + expect(subject).to be false + end + end + end + + context "payment changes on" do + let(:payment) { create(:payment, order_id: order.id) } + context "amount" do + it "returns true" do + payment.update!(amount: 222) + order.reload + expect(subject).to be true + end + end + + context "payment method" do + let(:payment_method) { create(:payment_method) } + + it "returns true" do + payment.update!(payment_method_id: payment_method.id) + order.reload + expect(subject).to be true + end + end + end + end + + describe "ignoring non-relevant associated object changes" do + context "customer changes" do + let(:customer){ create(:customer) } + it "returns false" do + order.update!(customer_id: customer.id) + expect(subject).to be false + end + end + + context "distributor changes" do + let(:distributor){ order.distributor } + it "returns false" do + distributor.update!(name: 'THIS IS A NEW NAME', abn: 'This is a new ABN') + expect(subject).to be false + end + end end end end @@ -159,6 +357,20 @@ describe OrderInvoiceComparator do order.reload expect(subject).to be false end + + context "payment changes on" do + context "state" do + let(:payment) { order.payments.first } + it "returns true" do + expect { + payment.started_processing + }.to change { payment.state }.from("checkout").to("processing") + + order.reload + expect(subject).to be true + end + end + end end context "a non-relevant associated model is updated" do @@ -170,11 +382,25 @@ describe OrderInvoiceComparator do end context "a relevant associated object is updated" do - let(:payment){ order.payments.first } - it "return true" do - expect(payment.state).to_not eq 'completed' - payment.update!(state: 'completed') - expect(subject).to be true + describe "detecting relevant associated object changes" do + context "adjustment changes" do + context "with an existing adjustment" do + let!(:adjustment) { create(:adjustment, order_id: order.id) } + it "changing the label returns true" do + adjustment.update!(label: "It's a new label") + order.reload + expect(subject).to be true + end + end + end + context "payment changes" do + let(:payment){ order.payments.first } + it "return true" do + expect(payment.state).to_not eq 'completed' + payment.update!(state: 'completed') + expect(subject).to be true + end + end end end end From 8a1a14112b904016535eda468b37ae1098b6e447 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Mon, 9 Oct 2023 18:10:35 +0100 Subject: [PATCH 2/5] Moves tests to shared examples file --- ...invoice_comparator_shared_examples_spec.rb | 89 +++++++++++++++++++ .../services/order_invoice_comparator_spec.rb | 17 ---- 2 files changed, 89 insertions(+), 17 deletions(-) create mode 100644 spec/services/order_invoice_comparator_shared_examples_spec.rb diff --git a/spec/services/order_invoice_comparator_shared_examples_spec.rb b/spec/services/order_invoice_comparator_shared_examples_spec.rb new file mode 100644 index 0000000000..4e52c1cd19 --- /dev/null +++ b/spec/services/order_invoice_comparator_shared_examples_spec.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require 'spec_helper' + +shared_examples "attribute changes - payment total" do |boolean, type| + before do + Spree::Order.where(id: order.id).update_all(payment_total: order.payment_total + 10) + end + + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end +end + +shared_examples "attribute changes - order total" do |boolean, type| + before do + Spree::Order.where(id: order.id).update_all(total: order.total + 10) + end + + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end +end + +shared_examples "attribute changes - order state: cancelled" do |boolean, type| + before do + order.cancel! + end + + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end +end + +describe OrderInvoiceComparator do + describe '#can_generate_new_invoice?' do + # this passes 'order' as argument to the invoice comparator + let(:order) { create(:completed_order_with_fees) } + let!(:invoice_data_generator){ InvoiceDataGenerator.new(order) } + let!(:invoice){ + create(:invoice, + order:, + data: invoice_data_generator.serialize_for_invoice) + } + let(:subject) { + OrderInvoiceComparator.new(order).can_generate_new_invoice? + } + + context "changes on the order object" do + describe "detecting relevant" do + it_behaves_like "attribute changes - payment total", true, "relevant" + it_behaves_like "attribute changes - order total", true, "relevant" + end + + describe "detecting non-relevant" do + pending('A new invoice should not be generated upon order state change') do + it_behaves_like "attribute changes - order state: cancelled", false, "non-relevant" + end + end + end + end + + describe '#can_update_latest_invoice?' do + let!(:order) { create(:completed_order_with_fees) } + let!(:invoice_data_generator){ InvoiceDataGenerator.new(order) } + let!(:invoice){ + create(:invoice, + order:, + data: invoice_data_generator.serialize_for_invoice) + } + let(:subject) { + OrderInvoiceComparator.new(order).can_update_latest_invoice? + } + + context "changes on the order object" do + describe "detecting relevant" do + it_behaves_like "attribute changes - order state: cancelled", true, "relevant" + end + + describe "detecting non-relevant" do + it_behaves_like "attribute changes - payment total", false, "non-relevant" + it_behaves_like "attribute changes - order total", false, "non-relevant" + end + end + end +end diff --git a/spec/services/order_invoice_comparator_spec.rb b/spec/services/order_invoice_comparator_spec.rb index c3cd679179..05e2f46809 100644 --- a/spec/services/order_invoice_comparator_spec.rb +++ b/spec/services/order_invoice_comparator_spec.rb @@ -18,29 +18,12 @@ describe OrderInvoiceComparator do context "changes on the order object" do describe "detecting relevant attribute changes" do - it "returns true if a relevant attribute changes" do - Spree::Order.where(id: order.id).update_all(payment_total: order.payment_total + 10) - order.reload - expect(subject).to be true - end - it "returns true if a relevant attribute changes" do Spree::Order.where(id: order.id).update_all(total: order.total + 10) order.reload expect(subject).to be true end - it "returns true if a relevant attribute changes - order state: cancelled" do - order.cancel! - expect(subject).to be true - end - - it "returns true if a relevant attribute changes - order state: resumed" do - order.cancel! - order.resume! - expect(subject).to be true - end - context "additional tax total changes" do let(:order) do create(:order_with_taxes, product_price: 110, tax_rate_amount: 0.1, From 530cdacc77d76fd861d8140ca2d60231eba38e93 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Mon, 9 Oct 2023 19:29:06 +0100 Subject: [PATCH 3/5] Restructures tests as shared examples Merges test files --- ...invoice_comparator_shared_examples_spec.rb | 89 --- .../services/order_invoice_comparator_spec.rb | 696 +++++++++--------- 2 files changed, 354 insertions(+), 431 deletions(-) delete mode 100644 spec/services/order_invoice_comparator_shared_examples_spec.rb diff --git a/spec/services/order_invoice_comparator_shared_examples_spec.rb b/spec/services/order_invoice_comparator_shared_examples_spec.rb deleted file mode 100644 index 4e52c1cd19..0000000000 --- a/spec/services/order_invoice_comparator_shared_examples_spec.rb +++ /dev/null @@ -1,89 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -shared_examples "attribute changes - payment total" do |boolean, type| - before do - Spree::Order.where(id: order.id).update_all(payment_total: order.payment_total + 10) - end - - it "returns #{boolean} if a #{type} attribute changes" do - order.reload - expect(subject).to be boolean - end -end - -shared_examples "attribute changes - order total" do |boolean, type| - before do - Spree::Order.where(id: order.id).update_all(total: order.total + 10) - end - - it "returns #{boolean} if a #{type} attribute changes" do - order.reload - expect(subject).to be boolean - end -end - -shared_examples "attribute changes - order state: cancelled" do |boolean, type| - before do - order.cancel! - end - - it "returns #{boolean} if a #{type} attribute changes" do - order.reload - expect(subject).to be boolean - end -end - -describe OrderInvoiceComparator do - describe '#can_generate_new_invoice?' do - # this passes 'order' as argument to the invoice comparator - let(:order) { create(:completed_order_with_fees) } - let!(:invoice_data_generator){ InvoiceDataGenerator.new(order) } - let!(:invoice){ - create(:invoice, - order:, - data: invoice_data_generator.serialize_for_invoice) - } - let(:subject) { - OrderInvoiceComparator.new(order).can_generate_new_invoice? - } - - context "changes on the order object" do - describe "detecting relevant" do - it_behaves_like "attribute changes - payment total", true, "relevant" - it_behaves_like "attribute changes - order total", true, "relevant" - end - - describe "detecting non-relevant" do - pending('A new invoice should not be generated upon order state change') do - it_behaves_like "attribute changes - order state: cancelled", false, "non-relevant" - end - end - end - end - - describe '#can_update_latest_invoice?' do - let!(:order) { create(:completed_order_with_fees) } - let!(:invoice_data_generator){ InvoiceDataGenerator.new(order) } - let!(:invoice){ - create(:invoice, - order:, - data: invoice_data_generator.serialize_for_invoice) - } - let(:subject) { - OrderInvoiceComparator.new(order).can_update_latest_invoice? - } - - context "changes on the order object" do - describe "detecting relevant" do - it_behaves_like "attribute changes - order state: cancelled", true, "relevant" - end - - describe "detecting non-relevant" do - it_behaves_like "attribute changes - payment total", false, "non-relevant" - it_behaves_like "attribute changes - order total", false, "non-relevant" - end - end - end -end diff --git a/spec/services/order_invoice_comparator_spec.rb b/spec/services/order_invoice_comparator_spec.rb index 05e2f46809..5d90014e2c 100644 --- a/spec/services/order_invoice_comparator_spec.rb +++ b/spec/services/order_invoice_comparator_spec.rb @@ -2,6 +2,317 @@ require 'spec_helper' +shared_examples "attribute changes - payment total" do |boolean, type| + before do + Spree::Order.where(id: order.id).update_all(payment_total: order.payment_total + 10) + end + + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end +end + +shared_examples "attribute changes - order total" do |boolean, type| + before do + Spree::Order.where(id: order.id).update_all(total: order.total + 10) + end + + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end +end + +shared_examples "attribute changes - order state: cancelled" do |boolean, type| + before do + order.cancel! + end + + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end +end + +shared_examples "attribute changes - tax total changes" do |boolean, type, included_boolean| + let(:order) do + create(:order_with_taxes, product_price: 110, tax_rate_amount: 0.1, + included_in_price: included_boolean) + .tap do |order| + order.create_tax_charge! + order.update_shipping_fees! + end + end + + context "if included is #{included_boolean}" do + before do + Spree::TaxRate.first.update!(amount: 0.15) + order.create_tax_charge! + end + + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be true + end + end +end + +shared_examples "attribute changes - shipping method" do |boolean, type| + let(:shipping_method) { create(:shipping_method) } + + before do + Spree::ShippingRate.first.update(shipping_method_id: shipping_method.id) + end + + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end +end + +shared_examples "no attribute changes" do + it "returns false if no attribute has changed" do + expect(subject).to be false + end +end + +shared_examples "attribute changes - special insctructions" do |boolean, type| + before do + order.update!(special_instructions: "A very special insctruction.") + end + it "returns #{boolean} if a #{type} attribute changes" do + expect(subject).to be boolean + end +end + +shared_examples "attribute changes - note" do |boolean, type| + before do + order.update!(note: "THIS IS A NEW NOTE") + end + it "returns #{boolean} if a #{type} attribute changes" do + expect(subject).to be boolean + end +end + +shared_examples "associated attribute changes - adjustments (create)" do |boolean, type| + before { create(:adjustment, order_id: order.id) } + context "creating an adjustment" do + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end + end + + context "with an existing adjustment" do + let!(:adjustment) { create(:adjustment, order_id: order.id) } + + context "editing the amount" do + before { adjustment.update!(amount: 123) } + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end + end + + context "changing the adjustment type" do + before { adjustment.update!(adjustable_type: "Spree::Payment") } + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end + end + + context "deleting an adjustment" do + before { order.all_adjustments.destroy_all } + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end + end + end +end + +shared_examples "associated attribute changes - adjustments (update)" do |boolean, type| + context "adjustment changes" do + let!(:adjustment) { create(:adjustment, order_id: order.id) } + + context "with an existing adjustment" do + before { adjustment.update!(label: "It's a new label") } + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end + end + end +end + +shared_examples "associated attribute changes - line items" do |boolean, type| + context "line item changes" do + let(:line_item){ order.line_items.first } + context "on quantitity" do + before { line_item.update!(quantity: line_item.quantity + 1) } + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end + end + + context "on variant id" do + before { line_item.update!(variant_id: Spree::Variant.first.id) } + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end + end + end +end + +shared_examples "associated attribute changes - bill address" do |boolean, type| + context "bill address - a #{type}" do + let(:bill_address) { Spree::Address.where(id: order.bill_address_id) } + it "first name" do + bill_address.update!(firstname: "Jane") + order.reload + expect(subject).to be boolean + end + + it "last name" do + bill_address.update!(lastname: "Jones") + order.reload + expect(subject).to be boolean + end + + it "address (1)" do + bill_address.update!(address1: "Rue du Fromage 66") + order.reload + expect(subject).to be boolean + end + + it "address (2)" do + bill_address.update!(address2: "South by Southwest") + order.reload + expect(subject).to be boolean + end + + it "city" do + bill_address.update!(city: "Antibes") + order.reload + expect(subject).to be boolean + end + + it "zipcode" do + bill_address.update!(zipcode: "04229") + order.reload + expect(subject).to be boolean + end + + it "phone" do + bill_address.update!(phone: "111-222-333") + order.reload + expect(subject).to be boolean + end + + it "company" do + bill_address.update!(company: "A Company Name") + order.reload + expect(subject).to be boolean + end + end +end + +shared_examples "associated attribute changes - ship address" do |boolean, type| + context "ship address - a #{type}" do + let(:ship_address) { Spree::Address.where(id: order.ship_address_id) } + it "first name" do + ship_address.update!(firstname: "Jane") + order.reload + expect(subject).to be boolean + end + + it "last name" do + ship_address.update!(lastname: "Jones") + order.reload + expect(subject).to be boolean + end + + it "address (1)" do + ship_address.update!(address1: "Rue du Fromage 66") + order.reload + expect(subject).to be boolean + end + + it "address (2)" do + ship_address.update!(address2: "South by Southwest") + order.reload + expect(subject).to be boolean + end + + it "city" do + ship_address.update!(city: "Antibes") + order.reload + expect(subject).to be boolean + end + + it "zipcode" do + ship_address.update!(zipcode: "04229") + order.reload + expect(subject).to be boolean + end + + it "phone" do + ship_address.update!(phone: "111-222-333") + order.reload + expect(subject).to be boolean + end + + it "company" do + ship_address.update!(company: "A Company Name") + order.reload + expect(subject).to be boolean + end + end +end + +shared_examples "associated attribute changes - payments" do |boolean, type| + context "payment changes on" do + let(:payment) { create(:payment, order_id: order.id) } + context "amount" do + before { payment.update!(amount: 222) } + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end + end + + context "payment changes on" do + let(:payment_method) { create(:payment_method) } + context "payment method" do + before { payment.update!(payment_method_id: payment_method.id) } + it "returns #{boolean} if a #{type} attribute changes" do + order.reload + expect(subject).to be boolean + end + end + end + end +end + +shared_examples "attribute changes - payment state" do |boolean, type| + let(:payment) { order.payments.first } + context "payment changes on" do + context "state" do + it "returns #{boolean} if a #{type} attribute changes" do + expect { + payment.started_processing + }.to change { payment.state }.from("checkout").to("processing") + order.reload + expect(subject).to be boolean + end + end + end +end + describe OrderInvoiceComparator do describe '#can_generate_new_invoice?' do # this passes 'order' as argument to the invoice comparator @@ -17,298 +328,31 @@ describe OrderInvoiceComparator do } context "changes on the order object" do - describe "detecting relevant attribute changes" do - it "returns true if a relevant attribute changes" do - Spree::Order.where(id: order.id).update_all(total: order.total + 10) - order.reload - expect(subject).to be true - end - - context "additional tax total changes" do - let(:order) do - create(:order_with_taxes, product_price: 110, tax_rate_amount: 0.1, - included_in_price: false) - .tap do |order| - order.create_tax_charge! - order.update_shipping_fees! - end - end - - it "returns true" do - Spree::TaxRate.first.update!(amount: 0.15) - order.create_tax_charge! - order.reload - expect(subject).to be true - end - end - - context "included tax total changes" do - let(:order) do - create(:order_with_taxes, product_price: 110, tax_rate_amount: 0.1, - included_in_price: true) - .tap do |order| - order.create_tax_charge! - order.update_shipping_fees! - end - end - - it "returns true" do - Spree::TaxRate.first.update!(amount: 0.15) - order.create_tax_charge! - order.reload - expect(subject).to be true - end - end - - context "shipping method changes" do - let(:shipping_method) { create(:shipping_method) } - it "returns true" do - Spree::ShippingRate.first.update(shipping_method_id: shipping_method.id) - expect(subject).to be true - end - end + describe "detecting relevant" do + it_behaves_like "attribute changes - payment total", true, "relevant" + it_behaves_like "attribute changes - order total", true, "relevant" + it_behaves_like "attribute changes - tax total changes", true, "relevant", false + it_behaves_like "attribute changes - tax total changes", true, "relevant", true + it_behaves_like "attribute changes - shipping method", true, "relevant" + it_behaves_like "associated attribute changes - adjustments (create)", true, "relevant" + it_behaves_like "associated attribute changes - bill address", true, "relevant" + it_behaves_like "associated attribute changes - ship address", true, "relevant" + it_behaves_like "associated attribute changes - line items", true, "relevant" + it_behaves_like "associated attribute changes - payments", true, "relevant" end - describe "ignoring non-relevant attribute changes" do - it "returns false if the order didn't change" do - expect(subject).to be false + describe "detecting non-relevant" do + it_behaves_like "attribute changes - order state: cancelled", false, "non-relevant" do + before { pending } end - - it "returns false if an attribute which should not change, changes" do - Spree::Order.where(id: order.id).update_all(number: 'R631504404') - order.reload - expect(subject).to be false - end - - it "returns false if an attribute which should not change, changes" do - Spree::Order.where(id: order.id).update_all(currency: 'EUR') - order.reload - expect(subject).to be false - end - - it "returns false if a non-relevant attribute changes" do - order.update!(special_instructions: "A very special insctruction.") - expect(subject).to be false - end - - it "returns false if a non-relevant attribute changes" do - order.update!(note: "THIS IS A NEW NOTE") - expect(subject).to be false - end - end - end - - context "changes on an associated order object" do - describe "detecting relevant associated object changes" do - context "adjustment changes" do - it "creating a new adjustment returns true" do - create(:adjustment, order_id: order.id) - order.reload - expect(subject).to be true - end - - context "with an existing adjustment" do - let!(:adjustment) { create(:adjustment, order_id: order.id) } - - it "editing the amount returns true" do - adjustment.update!(amount: 123) - order.reload - expect(subject).to be true - end - - it "changing the adjustment type" do - adjustment.update!(adjustable_type: "Spree::Payment") - order.reload - expect(subject).to be true - end - - it "deleting the label" do - order.all_adjustments.destroy_all - order.reload - expect(subject).to be true - end - end - end - - context "line item changes" do - let(:line_item){ order.line_items.first } - - context "on quantitity" do - it "return true" do - line_item.update!(quantity: line_item.quantity + 1) - expect(subject).to be true - end - end - - context "on variant id" do - it "return true" do - line_item.update!(variant_id: Spree::Variant.first.id) - order.reload - expect(subject).to be true - end - end - end - - context "bill address changes on" do - let(:bill_address) { Spree::Address.where(id: order.bill_address_id) } - it "first name" do - bill_address.update!(firstname: "Jane") - order.reload - expect(subject).to be true - end - - it "last name" do - bill_address.update!(lastname: "Jones") - order.reload - expect(subject).to be true - end - - it "address (1)" do - bill_address.update!(address1: "Rue du Fromage 66") - order.reload - expect(subject).to be true - end - - it "address (2)" do - bill_address.update!(address2: "South by Southwest") - order.reload - expect(subject).to be true - end - - it "city" do - bill_address.update!(city: "Antibes") - order.reload - expect(subject).to be true - end - - it "zipcode" do - bill_address.update!(zipcode: "04229") - order.reload - expect(subject).to be true - end - - it "phone" do - bill_address.update!(phone: "111-222-333") - order.reload - expect(subject).to be true - end - - it "company" do - bill_address.update!(company: "A Company Name") - order.reload - expect(subject).to be true - end - end - - context "ship address changes on" do - let(:ship_address) { Spree::Address.where(id: order.ship_address_id) } - it "first name" do - ship_address.update!(firstname: "Jane") - order.reload - expect(subject).to be true - end - - it "last name" do - ship_address.update!(lastname: "Jones") - order.reload - expect(subject).to be true - end - - it "address (1)" do - ship_address.update!(address1: "Rue du Fromage 66") - order.reload - expect(subject).to be true - end - - it "address (2)" do - ship_address.update!(address2: "South by Southwest") - order.reload - expect(subject).to be true - end - - it "city" do - ship_address.update!(city: "Antibes") - order.reload - expect(subject).to be true - end - - it "zipcode" do - ship_address.update!(zipcode: "04229") - order.reload - expect(subject).to be true - end - - it "phone" do - ship_address.update!(phone: "111-222-333") - order.reload - expect(subject).to be true - end - - it "company" do - ship_address.update!(company: "A Company Name") - order.reload - expect(subject).to be true - end - end - - context "customer changes on" do - let(:customer){ order.customer } - - context "code" do - it "return false" do - customer.update!(code: 98_754) - order.reload - expect(subject).to be false - end - end - - context "email" do - it "return false" do - customer.update!(email: "customer@email.org") - order.reload - expect(subject).to be false - end - end - end - - context "payment changes on" do - let(:payment) { create(:payment, order_id: order.id) } - context "amount" do - it "returns true" do - payment.update!(amount: 222) - order.reload - expect(subject).to be true - end - end - - context "payment method" do - let(:payment_method) { create(:payment_method) } - - it "returns true" do - payment.update!(payment_method_id: payment_method.id) - order.reload - expect(subject).to be true - end - end - end - end - - describe "ignoring non-relevant associated object changes" do - context "customer changes" do - let(:customer){ create(:customer) } - it "returns false" do - order.update!(customer_id: customer.id) - expect(subject).to be false - end - end - - context "distributor changes" do - let(:distributor){ order.distributor } - it "returns false" do - distributor.update!(name: 'THIS IS A NEW NAME', abn: 'This is a new ABN') - expect(subject).to be false - end + it_behaves_like "no attribute changes" + it_behaves_like "attribute changes - special insctructions", false, "non-relevant" + it_behaves_like "attribute changes - note", false, "non-relevant" + it_behaves_like "associated attribute changes - adjustments (update)", false, + "non-relevant" do + before { pending } end + it_behaves_like "attribute changes - payment state", false, "non-relevant" end end end @@ -326,63 +370,31 @@ describe OrderInvoiceComparator do } context "changes on the order object" do - it "returns false if the order didn't change" do - expect(subject).to be false + describe "detecting relevant" do + it_behaves_like "attribute changes - order state: cancelled", true, "relevant" + it_behaves_like "attribute changes - special insctructions", true, "relevant" + it_behaves_like "attribute changes - note", true, "relevant" + it_behaves_like "associated attribute changes - adjustments (update)", true, "relevant" + it_behaves_like "attribute changes - payment state", true, "relevant" end - it "returns true if a relevant attribute changes" do - order.update!(note: "THIS IS A NEW NOTE") - expect(subject).to be true - end - - it "returns false if a non-relevant attribute changes" do - Spree::Order.where(id: order.id).update_all(payment_total: order.payment_total + 10) - order.reload - expect(subject).to be false - end - - context "payment changes on" do - context "state" do - let(:payment) { order.payments.first } - it "returns true" do - expect { - payment.started_processing - }.to change { payment.state }.from("checkout").to("processing") - - order.reload - expect(subject).to be true - end + describe "detecting non-relevant" do + it_behaves_like "attribute changes - payment total", false, "non-relevant" + it_behaves_like "attribute changes - order total", false, "non-relevant" + it_behaves_like "attribute changes - tax total changes", false, "non-relevant", false + it_behaves_like "attribute changes - tax total changes", false, "non-relevant", true + it_behaves_like "attribute changes - shipping method", false, "non-relevant" + it_behaves_like "no attribute changes" + it_behaves_like "associated attribute changes - adjustments (create)", false, + "non-relevant" do + before { pending } end - end - end - - context "a non-relevant associated model is updated" do - let(:distributor){ order.distributor } - it "returns false" do - distributor.update!(name: 'THIS IS A NEW NAME', abn: 'This is a new ABN') - expect(subject).to be false - end - end - - context "a relevant associated object is updated" do - describe "detecting relevant associated object changes" do - context "adjustment changes" do - context "with an existing adjustment" do - let!(:adjustment) { create(:adjustment, order_id: order.id) } - it "changing the label returns true" do - adjustment.update!(label: "It's a new label") - order.reload - expect(subject).to be true - end - end - end - context "payment changes" do - let(:payment){ order.payments.first } - it "return true" do - expect(payment.state).to_not eq 'completed' - payment.update!(state: 'completed') - expect(subject).to be true - end + it_behaves_like "associated attribute changes - line items", false, "non-relevant" + it_behaves_like "associated attribute changes - bill address", false, "non-relevant" + it_behaves_like "associated attribute changes - ship address", false, "non-relevant" + it_behaves_like "associated attribute changes - payments", false, + "non-relevant" do |_variable| + before { pending } end end end From 2004abc8bec22d6b4f2aa63506bc6874f744db3d Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Wed, 18 Oct 2023 11:09:25 +0100 Subject: [PATCH 4/5] Sets pending test - issue #11350 changing the payment_total should not generate a new invoice; rather, it should update the current invoice --- spec/services/order_invoice_comparator_spec.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/services/order_invoice_comparator_spec.rb b/spec/services/order_invoice_comparator_spec.rb index 5d90014e2c..b4c8a7ff67 100644 --- a/spec/services/order_invoice_comparator_spec.rb +++ b/spec/services/order_invoice_comparator_spec.rb @@ -329,7 +329,6 @@ describe OrderInvoiceComparator do context "changes on the order object" do describe "detecting relevant" do - it_behaves_like "attribute changes - payment total", true, "relevant" it_behaves_like "attribute changes - order total", true, "relevant" it_behaves_like "attribute changes - tax total changes", true, "relevant", false it_behaves_like "attribute changes - tax total changes", true, "relevant", true @@ -342,6 +341,9 @@ describe OrderInvoiceComparator do end describe "detecting non-relevant" do + it_behaves_like "attribute changes - payment total", false, "relevant" do + before { pending("a payment capture shouldn't trigger a new invoice - issue #11350") } + end it_behaves_like "attribute changes - order state: cancelled", false, "non-relevant" do before { pending } end @@ -371,6 +373,9 @@ describe OrderInvoiceComparator do context "changes on the order object" do describe "detecting relevant" do + it_behaves_like "attribute changes - payment total", true, "relevant" do + before { pending("a payment capture shouldn't trigger a new invoice - issue #11350") } + end it_behaves_like "attribute changes - order state: cancelled", true, "relevant" it_behaves_like "attribute changes - special insctructions", true, "relevant" it_behaves_like "attribute changes - note", true, "relevant" @@ -379,7 +384,6 @@ describe OrderInvoiceComparator do end describe "detecting non-relevant" do - it_behaves_like "attribute changes - payment total", false, "non-relevant" it_behaves_like "attribute changes - order total", false, "non-relevant" it_behaves_like "attribute changes - tax total changes", false, "non-relevant", false it_behaves_like "attribute changes - tax total changes", false, "non-relevant", true From c05716cea98b801618c0e595b3c78f79681eee36 Mon Sep 17 00:00:00 2001 From: filipefurtad0 Date: Thu, 19 Oct 2023 15:44:04 +0100 Subject: [PATCH 5/5] Addresses review suggestions from @dacook I've noticed that it was necessary to include a reference to the order like 'order.adjustments << create(:adjustment, order:)' for the tests to pass Updates test case description --- .../services/order_invoice_comparator_spec.rb | 29 +++++++------------ 1 file changed, 11 insertions(+), 18 deletions(-) diff --git a/spec/services/order_invoice_comparator_spec.rb b/spec/services/order_invoice_comparator_spec.rb index b4c8a7ff67..1b9ed9de32 100644 --- a/spec/services/order_invoice_comparator_spec.rb +++ b/spec/services/order_invoice_comparator_spec.rb @@ -45,7 +45,7 @@ shared_examples "attribute changes - tax total changes" do |boolean, type, inclu end end - context "if included is #{included_boolean}" do + context "if included_in_price is #{included_boolean}" do before do Spree::TaxRate.first.update!(amount: 0.15) order.create_tax_charge! @@ -96,29 +96,26 @@ shared_examples "attribute changes - note" do |boolean, type| end shared_examples "associated attribute changes - adjustments (create)" do |boolean, type| - before { create(:adjustment, order_id: order.id) } + before { order.adjustments << create(:adjustment, order:) } context "creating an adjustment" do it "returns #{boolean} if a #{type} attribute changes" do - order.reload expect(subject).to be boolean end end context "with an existing adjustment" do - let!(:adjustment) { create(:adjustment, order_id: order.id) } + before { order.adjustments << create(:adjustment, order:) } context "editing the amount" do - before { adjustment.update!(amount: 123) } + before { order.adjustments.first.update!(amount: 123) } it "returns #{boolean} if a #{type} attribute changes" do - order.reload expect(subject).to be boolean end end context "changing the adjustment type" do - before { adjustment.update!(adjustable_type: "Spree::Payment") } + before { order.adjustments.first.update!(adjustable_type: "Spree::Payment") } it "returns #{boolean} if a #{type} attribute changes" do - order.reload expect(subject).to be boolean end end @@ -314,15 +311,16 @@ shared_examples "attribute changes - payment state" do |boolean, type| end describe OrderInvoiceComparator do + let!(:invoice){ + create(:invoice, + order:, + data: invoice_data_generator.serialize_for_invoice) + } + describe '#can_generate_new_invoice?' do # this passes 'order' as argument to the invoice comparator let(:order) { create(:completed_order_with_fees) } let!(:invoice_data_generator){ InvoiceDataGenerator.new(order) } - let!(:invoice){ - create(:invoice, - order:, - data: invoice_data_generator.serialize_for_invoice) - } let(:subject) { OrderInvoiceComparator.new(order).can_generate_new_invoice? } @@ -362,11 +360,6 @@ describe OrderInvoiceComparator do describe '#can_update_latest_invoice?' do let!(:order) { create(:completed_order_with_fees) } let!(:invoice_data_generator){ InvoiceDataGenerator.new(order) } - let!(:invoice){ - create(:invoice, - order:, - data: invoice_data_generator.serialize_for_invoice) - } let(:subject) { OrderInvoiceComparator.new(order).can_update_latest_invoice? }