Skip to content

Commit

Permalink
Rename "total" fields in several models
Browse files Browse the repository at this point in the history
Aiming to provide more consistent and obvious naming for these fields.

Shipment:

`final_price`            -> `total` (and the display_ method)
`pre_tax_amount`         -> `total_excluding_vat`
`final_price_with_items` -> `total_with_items`

LineItem:

`final_amount`   -> `total` (and the display_ method)
`pre_tax_amount` -> `total_excluding_vat` (and the display_ method)

Order:

`pre_tax_item_amount` -> `item_total_excluding_vat`

ReturnAuthorization:

`pre_tax_total` -> `total_excluding_vat` (and the display_ method)

ReturnItem:

`pre_tax_amount` -> `total_excluding_vat` (and the display_ method)

CustomerReturn:

`pre_tax_total` -> `total_excluding_vat` (and the display_ method)
  • Loading branch information
jordan-brough committed Sep 13, 2017
1 parent 5d61419 commit dd24466
Show file tree
Hide file tree
Showing 16 changed files with 90 additions and 58 deletions.
4 changes: 2 additions & 2 deletions backend/app/views/spree/admin/customer_returns/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
<thead data-hook="customer_return_header">
<tr>
<th><%= Spree::CustomerReturn.human_attribute_name(:number) %></th>
<th><%= Spree::CustomerReturn.human_attribute_name(:pre_tax_total) %></th>
<th><%= Spree::CustomerReturn.human_attribute_name(:total_excluding_vat) %></th>
<th><%= Spree::CustomerReturn.human_attribute_name(:total) %></th>
<th><%= Spree::CustomerReturn.human_attribute_name(:created_at) %></th>
<th><%= Spree::CustomerReturn.human_attribute_name(:reimbursement_status) %></th>
Expand All @@ -26,7 +26,7 @@
<% @customer_returns.each do |customer_return| %>
<tr id="<%= spree_dom_id(customer_return) %>" data-hook="customer_return_row">
<td><%= link_to customer_return.number, edit_admin_order_customer_return_path(@order, customer_return) %></td>
<td><%= customer_return.display_pre_tax_total.to_html %></td>
<td><%= customer_return.display_total_excluding_vat.to_html %></td>
<td><%= customer_return.display_total.to_html %></td>
<td><%= pretty_time(customer_return.created_at) %></td>
<td>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
<tr>
<th><%= Spree::ReturnAuthorization.human_attribute_name(:number) %></th>
<th><%= Spree::ReturnAuthorization.human_attribute_name(:state) %></th>
<th><%= Spree::ReturnAuthorization.human_attribute_name(:pre_tax_total) %></th>
<th><%= Spree::ReturnAuthorization.human_attribute_name(:total_excluding_vat) %></th>
<th><%= Spree::ReturnAuthorization.human_attribute_name(:created_at) %></th>
<th class="actions"></th>
</tr>
Expand All @@ -36,7 +36,7 @@
) %>
</span>
</td>
<td><%= return_authorization.display_pre_tax_total.to_html %></td>
<td><%= return_authorization.display_total_excluding_vat.to_html %></td>
<td><%= pretty_time(return_authorization.created_at) %></td>
<td class="actions">
<% if can? :update, return_authorization %>
Expand Down
9 changes: 6 additions & 3 deletions core/app/models/spree/customer_return.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ class CustomerReturn < Spree::Base
accepts_nested_attributes_for :return_items

extend DisplayMoney
money_methods :pre_tax_total, :total, :amount
money_methods :pre_tax_total, :total, :total_excluding_vat, :amount
deprecate display_pre_tax_total: :display_total_excluding_vat, deprecator: Spree::Deprecation

delegate :currency, to: :order
delegate :id, to: :order, prefix: true, allow_nil: true
Expand All @@ -25,9 +26,11 @@ def total
return_items.map(&:total).sum
end

def pre_tax_total
return_items.map(&:pre_tax_amount).sum
def total_excluding_vat
return_items.map(&:total_excluding_vat).sum
end
alias pre_tax_total total_excluding_vat
deprecate pre_tax_total: :total_excluding_vat, deprecator: Spree::Deprecation

def amount
return_items.sum(:amount)
Expand Down
29 changes: 18 additions & 11 deletions core/app/models/spree/line_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,30 +59,37 @@ def discounted_amount
end
deprecate discounted_amount: :total_before_tax, deprecator: Spree::Deprecation

# @return [BigDecimal] the amount of this line item, taking into
# consideration all its adjustments.
def total
amount + adjustment_total
end
alias final_amount total
deprecate final_amount: :total, deprecator: Spree::Deprecation

# @return [BigDecimal] the amount of this item, taking into consideration
# all non-tax adjustments.
def total_before_tax
amount + adjustments.select { |a| !a.tax? && a.eligible? }.sum(&:amount)
end

# @return [BigDecimal] the amount of this line item, taking into
# consideration all its adjustments.
def final_amount
amount + adjustment_total
end
alias total final_amount

# @return [BigDecimal] the amount of this line item before included tax
# @return [BigDecimal] the amount of this line item before VAT tax
# @note just like `amount`, this does not include any additional tax
def pre_tax_amount
def total_excluding_vat
total_before_tax - included_tax_total
end
alias pre_tax_amount total_excluding_vat
deprecate pre_tax_amount: :total_excluding_vat, deprecator: Spree::Deprecation

extend Spree::DisplayMoney
money_methods :amount, :discounted_amount, :final_amount, :pre_tax_amount, :price,
money_methods :amount, :discounted_amount, :price,
:included_tax_total, :additional_tax_total,
:total_before_tax
:total, :total_before_tax, :total_excluding_vat
deprecate display_discounted_amount: :display_total_before_tax, deprecator: Spree::Deprecation
alias display_final_amount display_total
deprecate display_final_amount: :display_total, deprecator: Spree::Deprecation
alias display_pre_tax_amount display_total_excluding_vat
deprecate display_pre_tax_amount: :display_total_excluding_vat, deprecator: Spree::Deprecation
alias discounted_money display_discounted_amount
deprecate discounted_money: :display_total_before_tax, deprecator: Spree::Deprecation

Expand Down
12 changes: 7 additions & 5 deletions core/app/models/spree/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -185,11 +185,6 @@ def amount
line_items.map(&:amount).sum
end

# Sum of all line item amounts pre-tax
def pre_tax_item_amount
line_items.to_a.sum(&:pre_tax_amount)
end

# Sum of all line item amounts after promotions, before added tax
def discounted_item_amount
line_items.to_a.sum(&:discounted_amount)
Expand All @@ -200,6 +195,13 @@ def item_total_before_tax
line_items.to_a.sum(&:total_before_tax)
end

# Sum of all line item amounts pre-tax
def item_total_excluding_vat
line_items.to_a.sum(&:total_excluding_vat)
end
alias pre_tax_item_amount item_total_excluding_vat
deprecate pre_tax_item_amount: :item_total_excluding_vat, deprecator: Spree::Deprecation

def currency
self[:currency] || Spree::Config[:currency]
end
Expand Down
9 changes: 6 additions & 3 deletions core/app/models/spree/return_authorization.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,16 @@ class ReturnAuthorization < Spree::Base
end

extend DisplayMoney
money_methods :pre_tax_total, :amount
money_methods :pre_tax_total, :amount, :total_excluding_vat
deprecate display_pre_tax_total: :display_total_excluding_vat, deprecator: Spree::Deprecation

self.whitelisted_ransackable_attributes = ['memo']

def pre_tax_total
return_items.map(&:pre_tax_amount).sum
def total_excluding_vat
return_items.map(&:total_excluding_vat).sum
end
alias pre_tax_total total_excluding_vat
deprecate pre_tax_total: :total_excluding_vat, deprecator: Spree::Deprecation

def amount
return_items.sum(:amount)
Expand Down
9 changes: 6 additions & 3 deletions core/app/models/spree/return_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ class ReturnItem < Spree::Base
end

extend DisplayMoney
money_methods :pre_tax_amount, :amount, :total
money_methods :pre_tax_amount, :amount, :total, :total_excluding_vat
deprecate display_pre_tax_amount: :display_total_excluding_vat, deprecator: Spree::Deprecation

# @return [Boolean] true when this retur item is in a complete reception
# state
Expand Down Expand Up @@ -159,10 +160,12 @@ def total
amount + additional_tax_total
end

# @return [BigDecimal] the cost of the item before tax
def pre_tax_amount
# @return [BigDecimal] the cost of the item before VAT tax
def total_excluding_vat
amount - included_tax_total
end
alias pre_tax_amount total_excluding_vat
deprecate pre_tax_amount: :total_excluding_vat, deprecator: Spree::Deprecation

# @note This uses the exchange_variant_engine configured on the class.
# @param stock_locations [Array<Spree::StockLocation>] the stock locations to check
Expand Down
31 changes: 21 additions & 10 deletions core/app/models/spree/shipment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,10 @@ def can_transition_from_canceled_to_ready?
extend DisplayMoney
money_methods(
:cost, :amount, :discounted_cost, :final_price, :item_cost,
:total_before_tax,
:total, :total_before_tax,
)
deprecate display_discounted_cost: :display_total_before_tax, deprecator: Spree::Deprecation
deprecate display_final_price: :display_total, deprecator: Spree::Deprecation
alias_attribute :amount, :cost

def add_shipping_method(shipping_method, selected = false)
Expand Down Expand Up @@ -124,19 +125,33 @@ def discounted_cost
alias discounted_amount discounted_cost
deprecate discounted_amount: :total_before_tax, deprecator: Spree::Deprecation

# @return [BigDecimal] the amount of this shipment, taking into
# consideration all its adjustments.
def total
cost + adjustment_total
end
alias final_price total
deprecate final_price: :total, deprecator: Spree::Deprecation

# @return [BigDecimal] the amount of this item, taking into consideration
# all non-tax adjustments.
def total_before_tax
amount + adjustments.select { |a| !a.tax? && a.eligible? }.sum(&:amount)
end

def final_price
cost + adjustment_total
# @return [BigDecimal] the amount of this shipment before VAT tax
# @note just like `cost`, this does not include any additional tax
def total_excluding_vat
total_before_tax - included_tax_total
end
alias pre_tax_amount total_excluding_vat
deprecate pre_tax_amount: :total_excluding_vat, deprecator: Spree::Deprecation

def final_price_with_items
item_cost + final_price
def total_with_items
total + item_cost
end
alias final_price_with_items total_with_items
deprecate final_price_with_items: :total_with_items, deprecator: Spree::Deprecation

def editable_by?(_user)
!shipped?
Expand Down Expand Up @@ -168,17 +183,13 @@ def inventory_units_for_item(line_item, variant = nil)
end

def item_cost
line_items.map(&:final_amount).sum
line_items.map(&:total).sum
end

def line_items
inventory_units.includes(:line_item).map(&:line_item).uniq
end

def pre_tax_amount
total_before_tax - included_tax_total
end

def ready_or_pending?
ready? || pending?
end
Expand Down
3 changes: 3 additions & 0 deletions core/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ en:
number: Return Number
pre_tax_total: Pre-Tax Total
total: Total
total_excluding_vat: Pre-Tax Total
created_at: "Date/Time"
reimbursement_status: Reimbursement status
name: Name
Expand Down Expand Up @@ -250,6 +251,7 @@ en:
spree/return_authorization:
amount: Amount
pre_tax_total: Pre-Tax Total
total_excluding_vat: Pre-Tax Total
spree/return_item:
acceptance_status: Acceptance status
acceptance_status_errors: Acceptance errors
Expand Down Expand Up @@ -2017,6 +2019,7 @@ en:
to: to
to_add_variants_you_must_first_define: To add variants, you must first define
total: Total
total_excluding_vat: Pre-Tax Total
total_per_item: Total per item
total_pre_tax_refund: Total Pre-Tax Refund
total_price: Total price
Expand Down
2 changes: 1 addition & 1 deletion core/spec/models/spree/inventory_unit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@
end

describe "#current_or_new_return_item" do
before { allow(inventory_unit).to receive_messages(pre_tax_amount: 100.0) }
before { allow(inventory_unit).to receive_messages(total_excluding_vat: 100.0) }

subject { inventory_unit.current_or_new_return_item }

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@

context 'with a removed item' do
before do
item_amount = item_1.final_amount
item_amount = item_1.total
order.contents.remove(item_1.variant)
create(:refund, payment: payment, amount: item_amount)
end
Expand Down
4 changes: 2 additions & 2 deletions core/spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -995,14 +995,14 @@ def generate
end
end

describe "#pre_tax_item_amount" do
describe "#item_total_excluding_vat" do
it "sums all of the line items' pre tax amounts" do
subject.line_items = [
Spree::LineItem.new(price: 10, quantity: 2, included_tax_total: 15.0),
Spree::LineItem.new(price: 30, quantity: 1, included_tax_total: 16.0)
]
# (2*10)-15 + 30-16 = 5 + 14 = 19
expect(subject.pre_tax_item_amount).to eq 19.0
expect(subject.item_total_excluding_vat).to eq 19.0
end
end

Expand Down
4 changes: 2 additions & 2 deletions core/spec/models/spree/reimbursement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@
return_item.reload
expect(return_item.included_tax_total).to be > 0
expect(return_item.included_tax_total).to eq line_item.included_tax_total
expect(reimbursement.total).to eq((line_item.pre_tax_amount + line_item.included_tax_total).round(2, :down))
expect(Spree::Refund.last.amount).to eq((line_item.pre_tax_amount + line_item.included_tax_total).round(2, :down))
expect(reimbursement.total).to eq((line_item.total_excluding_vat + line_item.included_tax_total).round(2, :down))
expect(Spree::Refund.last.amount).to eq((line_item.total_excluding_vat + line_item.included_tax_total).round(2, :down))
end
end

Expand Down
10 changes: 5 additions & 5 deletions core/spec/models/spree/return_authorization_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
end
end

describe "#pre_tax_total" do
describe "#total_excluding_vat" do
let(:amount_1) { 15.0 }
let!(:return_item_1) { create(:return_item, return_authorization: return_authorization, amount: amount_1) }

Expand All @@ -87,17 +87,17 @@
let(:amount_3) { 5.0 }
let!(:return_item_3) { create(:return_item, return_authorization: return_authorization, amount: amount_3) }

subject { return_authorization.reload.pre_tax_total }
subject { return_authorization.reload.total_excluding_vat }

it "sums it's associated return_item's amounts" do
expect(subject).to eq(amount_1 + amount_2 + amount_3)
end
end

describe "#display_pre_tax_total" do
describe "#display_total_excluding_vat" do
it "returns a Spree::Money" do
allow(return_authorization).to receive_messages(pre_tax_total: 21.22)
expect(return_authorization.display_pre_tax_total).to eq(Spree::Money.new(21.22))
allow(return_authorization).to receive_messages(total_excluding_vat: 21.22)
expect(return_authorization.display_total_excluding_vat).to eq(Spree::Money.new(21.22))
end
end

Expand Down
6 changes: 3 additions & 3 deletions core/spec/models/spree/return_item_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,13 +140,13 @@
end
end

describe "#display_pre_tax_amount" do
describe "#display_total_excluding_vat" do
let(:amount) { 21.22 }
let(:included_tax_total) { 1.22 }
let(:return_item) { build(:return_item, amount: amount, included_tax_total: included_tax_total) }

it "returns a Spree::Money" do
expect(return_item.display_pre_tax_amount).to eq Spree::Money.new(amount - included_tax_total)
expect(return_item.display_total_excluding_vat).to eq Spree::Money.new(amount - included_tax_total)
end
end

Expand Down Expand Up @@ -558,7 +558,7 @@
end
end

describe "exchange pre_tax_amount" do
describe "exchange amount" do
let(:return_item) { build(:return_item) }

context "the return item is intended to be exchanged" do
Expand Down
Loading

0 comments on commit dd24466

Please sign in to comment.