a new invoice should refer the latest invoice. #11704
Conversation
|
this PR is based on #11696 |
abdellani
force-pushed
the
add-reference-to-previous-invoices
branch
from
28a9bb6
to
5723644
Compare
abdellani
force-pushed
the
add-reference-to-previous-invoices
branch
2 times, most recently
from
ca424c4
to
ff2fd99
Compare
abdellani
force-pushed
the
add-reference-to-previous-invoices
branch
from
ff2fd99
to
915afd8
Compare
| @@ -30,4 +30,8 @@ def cancel_previous_invoices | |||
| def display_number | |||
| "#{order.distributor.id}-#{number}" | |||
| end | |||
|
|
|||
| def previous_invoice | |||
| order.invoices.where("id < ?", id).first | |||
There was a problem hiding this comment.
If an order can have more that one previous invoice, we should use ordering to make sure it's the most recent one.
There was a problem hiding this comment.
I'm intrigued to see that the spec does cover this. But it still seems worth making it explicit here.
There was a problem hiding this comment.
Hi @dacook
Invoices are sorted by default as follows:
default_scope { order(created_at: :desc) }| @@ -46,6 +46,8 @@ | |||
| %br | |||
| = "#{t :invoice_number}:" | |||
| = @order.display_number | |||
| - if @order.previous_invoice.present? | |||
| = "#{t :invoice_cancel_and_replace_invoice} #{ @order.previous_invoice.display_number}" | |||
There was a problem hiding this comment.
I'm not sure if it will matter in other languages, but just in case I'd suggest using interpolation here, just in case some translations want to write something like
"cancels and replaces number %{canceled_invoice_number} invoice."
| context "Order doesn't have previous invoices" do | ||
| it "should display the invoice number" do | ||
| login_as_admin | ||
| visit spree.print_admin_order_path(order, params: {}) |
There was a problem hiding this comment.
probably can leave out params: {} (also below)
| @@ -1991,6 +1991,7 @@ en: | |||
| invoice_column_price_per_unit_without_taxes: "Price Per unit (Excl. tax)" | |||
| invoice_column_tax_rate: "Tax rate" | |||
| invoice_tax_total: "GST Total:" | |||
| invoice_cancel_and_replace_invoice: "cancels and replaces invoice" | |||
There was a problem hiding this comment.
I just realised that these invoice keys could be scoped under a namespace.
|
This is looking good! I noticed some other bugs while testing, but not linked to this PR so will open new issues - merging :) |
RachL
added
the
technical changes only
What? Why?
What should we test?
One possible scenario:
Release notes
Changelog Category (reviewers may add a label for the release notes):
The title of the pull request will be included in the release notes.
Dependencies
Documentation updates