From 8fecc9db4bd89868e0c556e14f53d9e6eec91ec8 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Sat, 24 Jun 2023 20:33:41 +0100 Subject: [PATCH 1/6] render date without time on invoices list + sort invoices in the reverse order of the creation time --- .../admin/invoices/_invoices_table.html.haml | 4 +- spec/system/admin/orders/invoices_spec.rb | 156 +++++++++++------- 2 files changed, 100 insertions(+), 60 deletions(-) diff --git a/app/views/spree/admin/invoices/_invoices_table.html.haml b/app/views/spree/admin/invoices/_invoices_table.html.haml index 7d312a7139..52cf9cb606 100644 --- a/app/views/spree/admin/invoices/_invoices_table.html.haml +++ b/app/views/spree/admin/invoices/_invoices_table.html.haml @@ -7,12 +7,12 @@ %th= t(:status) %th= t(:invoice_file) %tbody - - @order.invoices.each do |invoice| + - @order.invoices.order('number desc').each do |invoice| - tr_class = cycle('odd', 'even') - tr_id = spree_dom_id(invoice) %tr{:class => tr_class, "data-hook" => "invoice_row", :id => tr_id} %td.align-center.created_at - = pretty_time(invoice.date) + = invoice.presenter.invoice_date %td.align-center.label = invoice.number %td.align-center.label diff --git a/spec/system/admin/orders/invoices_spec.rb b/spec/system/admin/orders/invoices_spec.rb index cdd3a8de6e..e4a2c89f75 100644 --- a/spec/system/admin/orders/invoices_spec.rb +++ b/spec/system/admin/orders/invoices_spec.rb @@ -4,7 +4,7 @@ require 'system_helper' describe ' As an administrator - I want to create an invoice for an order + I want to list/create an invoice for an order ' do include WebHelper include AuthenticationHelper @@ -31,74 +31,114 @@ describe ' login_as_admin visit spree.edit_admin_order_path(order) end + describe 'creating invoices' do + context 'when the order has no invoices' do + it 'creates an invoice for the order' do + click_link 'Invoices' - context 'when the order has no invoices' do - it 'creates an invoice for the order' do - click_link 'Invoices' + expect { + click_link 'New Invoice' + expect(page).to have_no_link "New Invoice" + }.to change { order.invoices.count }.by(1) - expect { - click_link 'New Invoice' - expect(page).to have_no_link "New Invoice" - }.to change { order.invoices.count }.by(1) + invoice = order.invoices.first + expect(invoice.cancelled).to eq false + expect(invoice.number).to eq 1 + end + end - invoice = order.invoices.first - expect(invoice.cancelled).to eq false - expect(invoice.number).to eq 1 + context 'when the order has an invoice' do + let!(:latest_invoice){ create(:invoice, order:, number: 1, cancelled: false) } + + context 'order not updated since latest invoice' do + it 'should not render new invoice button' do + click_link 'Invoices' + expect(page).to_not have_link 'New Invoice' + end + end + + # For reference check: + # https://docs.google.com/spreadsheets/d/1hOM6UL4mWeRCFLcDQ3fTkbhbUQ2WvIUCCA1IerDBtUA/edit#gid=0 + context 'order updated since latest invoice' do + context 'changes require regenerating' do + let(:new_note){ 'new note' } + before do + order.update!(note: new_note) + end + + it 'updates the lastest invoice for the order' do + click_link 'Invoices' + expect { + click_link 'New Invoice' + expect(page).to have_no_link "New Invoice" + }.to change { order.reload.invoices.count }.by(0) + .and change { latest_invoice.reload.presenter.note }.from("").to(new_note) + + expect(latest_invoice.reload.cancelled).to eq false + end + end + + context 'changes require generating a new invoice' do + before do + order.line_items.first.update!(quantity: 2) + end + + it 'creates a new invoice for the order' do + click_link 'Invoices' + expect { + click_link 'New Invoice' + expect(page).to have_no_link "New Invoice" + }.to change { order.reload.invoices.count }.by(1) + + expect(latest_invoice.reload.cancelled).to eq true + expect(latest_invoice.presenter.sorted_line_items.first.quantity).to eq 1 + + new_invoice = order.invoices.last + expect(new_invoice.cancelled).to eq false + expect(new_invoice.number).to eq 2 + expect(new_invoice.presenter.sorted_line_items.first.quantity).to eq 2 + end + end + end end end - context 'when the order has an invoice' do - let!(:latest_invoice){ create(:invoice, order:, number: 1, cancelled: false) } - - context 'order not updated since latest invoice' do - it 'should not render new invoice button' do - click_link 'Invoices' - expect(page).to_not have_link 'New Invoice' - end + describe 'listing invoices' do + before do + create(:invoice, order:, number: 1, cancelled: true) + create(:invoice, order:, number: 2, cancelled: false) end - # For reference check: - # https://docs.google.com/spreadsheets/d/1hOM6UL4mWeRCFLcDQ3fTkbhbUQ2WvIUCCA1IerDBtUA/edit#gid=0 - context 'order updated since latest invoice' do - context 'changes require regenerating' do - let(:new_note){ 'new note' } - before do - order.update!(note: new_note) - end + let(:row1){ + [ + Time.current.to_date.to_s, + "2", + order.total, + "Active", + "Download" + ].join(" ") + } - it 'updates the lastest invoice for the order' do - click_link 'Invoices' - expect { - click_link 'New Invoice' - expect(page).to have_no_link "New Invoice" - }.to change { order.reload.invoices.count }.by(0) - .and change { latest_invoice.reload.presenter.note }.from("").to(new_note) + let(:row2){ + [ + Time.current.to_date.to_s, + "1", + order.total, + "Cancelled", + "Download" + ].join(" ") + } - expect(latest_invoice.reload.cancelled).to eq false - end - end + let(:table_content){ + [ + row1, + row2 + ].join(" ") + } - context 'changes require generating a new invoice' do - before do - order.line_items.first.update!(quantity: 2) - end - - it 'creates a new invoice for the order' do - click_link 'Invoices' - expect { - click_link 'New Invoice' - expect(page).to have_no_link "New Invoice" - }.to change { order.reload.invoices.count }.by(1) - - expect(latest_invoice.reload.cancelled).to eq true - expect(latest_invoice.presenter.sorted_line_items.first.quantity).to eq 1 - - new_invoice = order.invoices.last - expect(new_invoice.cancelled).to eq false - expect(new_invoice.number).to eq 2 - expect(new_invoice.presenter.sorted_line_items.first.quantity).to eq 2 - end - end + it 'should list the invoices on the reverse order of creation' do + click_link 'Invoices' + expect(page).to have_content table_content end end end From 1a8c0f18b708af00fef9c48543f1ac0b6a32b377 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Tue, 11 Jul 2023 10:54:29 +0100 Subject: [PATCH 2/6] order invoices by "created_at desc" on the default scope --- app/models/invoice.rb | 1 + app/services/order_invoice_comparator.rb | 2 +- app/views/spree/admin/invoices/_invoices_table.html.haml | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/invoice.rb b/app/models/invoice.rb index b86d362b5b..b66b0c5ba5 100644 --- a/app/models/invoice.rb +++ b/app/models/invoice.rb @@ -5,6 +5,7 @@ class Invoice < ApplicationRecord serialize :data, Hash before_validation :serialize_order after_create :cancel_previous_invoices + default_scope { order(created_at: :desc) } def presenter @presenter ||= Invoice::DataPresenter.new(self) diff --git a/app/services/order_invoice_comparator.rb b/app/services/order_invoice_comparator.rb index 24c02fced8..bd19f9dabe 100644 --- a/app/services/order_invoice_comparator.rb +++ b/app/services/order_invoice_comparator.rb @@ -77,7 +77,7 @@ class OrderInvoiceComparator end def latest_invoice - @latest_invoice ||= invoices.last.presenter + @latest_invoice ||= invoices.first.presenter end def serialize_for_invoice diff --git a/app/views/spree/admin/invoices/_invoices_table.html.haml b/app/views/spree/admin/invoices/_invoices_table.html.haml index 52cf9cb606..6ac93077d3 100644 --- a/app/views/spree/admin/invoices/_invoices_table.html.haml +++ b/app/views/spree/admin/invoices/_invoices_table.html.haml @@ -7,7 +7,7 @@ %th= t(:status) %th= t(:invoice_file) %tbody - - @order.invoices.order('number desc').each do |invoice| + - @order.invoices.each do |invoice| - tr_class = cycle('odd', 'even') - tr_id = spree_dom_id(invoice) %tr{:class => tr_class, "data-hook" => "invoice_row", :id => tr_id} From d044959cec8246bce04283f3e43099373093f4ac Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 10:35:04 +0100 Subject: [PATCH 3/6] fix date formatting implement a new helper `pretty_date` --- app/helpers/spree/base_helper.rb | 4 ++++ .../admin/invoices/_invoices_table.html.haml | 2 +- spec/helpers/spree/base_helper_spec.rb | 6 ++++++ spec/system/admin/orders/invoices_spec.rb | 18 ++++++++++-------- 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/app/helpers/spree/base_helper.rb b/app/helpers/spree/base_helper.rb index 92e38aa8a1..f11c5532d9 100644 --- a/app/helpers/spree/base_helper.rb +++ b/app/helpers/spree/base_helper.rb @@ -39,5 +39,9 @@ module Spree [I18n.l(time.to_date, format: :long), time.strftime("%l:%M %p")].join(" ") end + + def pretty_date(date) + I18n.l(date.to_date, format: :long) + end end end diff --git a/app/views/spree/admin/invoices/_invoices_table.html.haml b/app/views/spree/admin/invoices/_invoices_table.html.haml index 6ac93077d3..aa0394eed5 100644 --- a/app/views/spree/admin/invoices/_invoices_table.html.haml +++ b/app/views/spree/admin/invoices/_invoices_table.html.haml @@ -12,7 +12,7 @@ - tr_id = spree_dom_id(invoice) %tr{:class => tr_class, "data-hook" => "invoice_row", :id => tr_id} %td.align-center.created_at - = invoice.presenter.invoice_date + = pretty_date(invoice.date) %td.align-center.label = invoice.number %td.align-center.label diff --git a/spec/helpers/spree/base_helper_spec.rb b/spec/helpers/spree/base_helper_spec.rb index 22f716a3e4..b0a3ac3428 100644 --- a/spec/helpers/spree/base_helper_spec.rb +++ b/spec/helpers/spree/base_helper_spec.rb @@ -55,4 +55,10 @@ describe Spree::BaseHelper do expect(pretty_time(DateTime.new(2012, 5, 6, 13, 33))).to eq "May 06, 2012 1:33 PM" end end + + context "pretty_date" do + it "prints in a format" do + expect(pretty_date(DateTime.new(2012, 5, 6, 13, 33))).to eq "May 06, 2012" + end + end end diff --git a/spec/system/admin/orders/invoices_spec.rb b/spec/system/admin/orders/invoices_spec.rb index e4a2c89f75..0ec2776045 100644 --- a/spec/system/admin/orders/invoices_spec.rb +++ b/spec/system/admin/orders/invoices_spec.rb @@ -4,7 +4,7 @@ require 'system_helper' describe ' As an administrator - I want to list/create an invoice for an order + I want to manage invoices for an order ' do include WebHelper include AuthenticationHelper @@ -93,7 +93,7 @@ describe ' expect(latest_invoice.reload.cancelled).to eq true expect(latest_invoice.presenter.sorted_line_items.first.quantity).to eq 1 - new_invoice = order.invoices.last + new_invoice = order.invoices.first # first invoice is the latest expect(new_invoice.cancelled).to eq false expect(new_invoice.number).to eq 2 expect(new_invoice.presenter.sorted_line_items.first.quantity).to eq 2 @@ -104,14 +104,11 @@ describe ' end describe 'listing invoices' do - before do - create(:invoice, order:, number: 1, cancelled: true) - create(:invoice, order:, number: 2, cancelled: false) - end + let(:date){ Time.current.to_date } let(:row1){ [ - Time.current.to_date.to_s, + I18n.l(date, format: :long), "2", order.total, "Active", @@ -121,7 +118,7 @@ describe ' let(:row2){ [ - Time.current.to_date.to_s, + I18n.l(date, format: :long), "1", order.total, "Cancelled", @@ -136,6 +133,11 @@ describe ' ].join(" ") } + before do + create(:invoice, order:, number: 1, cancelled: true, date:) + create(:invoice, order:, number: 2, cancelled: false, date:) + end + it 'should list the invoices on the reverse order of creation' do click_link 'Invoices' expect(page).to have_content table_content From 5358802ab5cd94115b20a02de9e255cbf8f24a25 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Fri, 14 Jul 2023 10:41:50 +0100 Subject: [PATCH 4/6] replace Presenter#invoice_date with a delagator --- app/models/invoice/data_presenter.rb | 8 ++------ spec/system/admin/orders/invoices_spec.rb | 1 + 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/app/models/invoice/data_presenter.rb b/app/models/invoice/data_presenter.rb index 65cffe82f6..ccb0eef66c 100644 --- a/app/models/invoice/data_presenter.rb +++ b/app/models/invoice/data_presenter.rb @@ -4,8 +4,8 @@ class Invoice class DataPresenter attr_reader :invoice - delegate :data, :date, to: :invoice - delegate :number, to: :invoice, prefix: true + delegate :data, to: :invoice + delegate :number, :date, to: :invoice, prefix: true FINALIZED_NON_SUCCESSFUL_STATES = %w(canceled returned).freeze @@ -83,10 +83,6 @@ class Invoice all_eligible_adjustments.select { |a| a.originator_type == 'Spree::TaxRate' } end - def invoice_date - date - end - def paid? data[:payment_state] == 'paid' || data[:payment_state] == 'credit_owed' end diff --git a/spec/system/admin/orders/invoices_spec.rb b/spec/system/admin/orders/invoices_spec.rb index 0ec2776045..26b8644a6b 100644 --- a/spec/system/admin/orders/invoices_spec.rb +++ b/spec/system/admin/orders/invoices_spec.rb @@ -31,6 +31,7 @@ describe ' login_as_admin visit spree.edit_admin_order_path(order) end + describe 'creating invoices' do context 'when the order has no invoices' do it 'creates an invoice for the order' do From c21ffd338bf80a0ce63e998e036cf7bd8b75debe Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Tue, 1 Aug 2023 09:27:16 +0100 Subject: [PATCH 5/6] fix only date is expected on the invoices list table. time is not supposed to be rendered on the invoices list table (only date) --- spec/system/admin/order_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/system/admin/order_spec.rb b/spec/system/admin/order_spec.rb index 37b0043c00..93acf8be30 100644 --- a/spec/system/admin/order_spec.rb +++ b/spec/system/admin/order_spec.rb @@ -1044,7 +1044,7 @@ describe ' let(:table_contents) { [ - Invoice.first.created_at.strftime('%B %d, %Y 12:00 AM').to_s, + Invoice.first.created_at.strftime('%B %d, %Y').to_s, "1", "0.0", "Active", From 6c53f3b8b0dd82f6a8c9d7516269581b3d56a8a8 Mon Sep 17 00:00:00 2001 From: Mohamed ABDELLANI Date: Tue, 1 Aug 2023 09:52:38 +0100 Subject: [PATCH 6/6] move pretty_date helper into invoice presenter --- app/helpers/spree/base_helper.rb | 4 ---- app/models/invoice/data_presenter.rb | 4 ++++ .../spree/admin/invoices/_invoices_table.html.haml | 2 +- spec/helpers/spree/base_helper_spec.rb | 6 ------ spec/models/invoice/data_presenter_spec.rb | 14 ++++++++++++++ 5 files changed, 19 insertions(+), 11 deletions(-) create mode 100644 spec/models/invoice/data_presenter_spec.rb diff --git a/app/helpers/spree/base_helper.rb b/app/helpers/spree/base_helper.rb index f11c5532d9..92e38aa8a1 100644 --- a/app/helpers/spree/base_helper.rb +++ b/app/helpers/spree/base_helper.rb @@ -39,9 +39,5 @@ module Spree [I18n.l(time.to_date, format: :long), time.strftime("%l:%M %p")].join(" ") end - - def pretty_date(date) - I18n.l(date.to_date, format: :long) - end end end diff --git a/app/models/invoice/data_presenter.rb b/app/models/invoice/data_presenter.rb index ccb0eef66c..0245d2dd5b 100644 --- a/app/models/invoice/data_presenter.rb +++ b/app/models/invoice/data_presenter.rb @@ -79,6 +79,10 @@ class Invoice end.sort_by { |tax| tax[:rate_amount] } end + def display_date + I18n.l(invoice_date.to_date, format: :long) + end + def all_tax_adjustments all_eligible_adjustments.select { |a| a.originator_type == 'Spree::TaxRate' } end diff --git a/app/views/spree/admin/invoices/_invoices_table.html.haml b/app/views/spree/admin/invoices/_invoices_table.html.haml index aa0394eed5..4c876319cd 100644 --- a/app/views/spree/admin/invoices/_invoices_table.html.haml +++ b/app/views/spree/admin/invoices/_invoices_table.html.haml @@ -12,7 +12,7 @@ - tr_id = spree_dom_id(invoice) %tr{:class => tr_class, "data-hook" => "invoice_row", :id => tr_id} %td.align-center.created_at - = pretty_date(invoice.date) + = invoice.presenter.display_date %td.align-center.label = invoice.number %td.align-center.label diff --git a/spec/helpers/spree/base_helper_spec.rb b/spec/helpers/spree/base_helper_spec.rb index b0a3ac3428..22f716a3e4 100644 --- a/spec/helpers/spree/base_helper_spec.rb +++ b/spec/helpers/spree/base_helper_spec.rb @@ -55,10 +55,4 @@ describe Spree::BaseHelper do expect(pretty_time(DateTime.new(2012, 5, 6, 13, 33))).to eq "May 06, 2012 1:33 PM" end end - - context "pretty_date" do - it "prints in a format" do - expect(pretty_date(DateTime.new(2012, 5, 6, 13, 33))).to eq "May 06, 2012" - end - end end diff --git a/spec/models/invoice/data_presenter_spec.rb b/spec/models/invoice/data_presenter_spec.rb new file mode 100644 index 0000000000..864044d502 --- /dev/null +++ b/spec/models/invoice/data_presenter_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Invoice::DataPresenter do + context "#display_date" do + let(:invoice) { double(:invoice, date: '2023-08-01') } + + let(:presenter) { Invoice::DataPresenter.new(invoice) } + it "prints in a format" do + expect(presenter.display_date).to eq "August 01, 2023" + end + end +end