From 42d0f167737b930334b3a260eba31937ec85dcea Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 10 Mar 2021 15:34:54 +0100 Subject: [PATCH] Abstract order balance in a simple PORO This new class lets us [Branch by abstraction](https://www.martinfowler.com/bliki/BranchByAbstraction.html) by encapsulating an order's balance. As a result, that's the only place where we need to check the feature toggle, instead of every place where `#outstanding_balance` is called (quite some). That would be very hard to review and it'd be more likely to introduce bugs. What I like about this is that we also managed to group together the data and logic that we had spread in a few places and have it nicely encapsulated. So, where we had a number, we'll now have an object. Once we fully change all `#outstanding_balance` consumers to use this new abstraction we'll be able to remove the methods this class replaces. These are: `Spree::Order#outstanding_balance?`, `Spree::Order#display_oustanding_balance` and `OrderHelper.outstanding_balance_label`. This is just the first step. I'll follow this up with a PR per page/mailer/whatever where we use the balance to replace it with an instance of `OrderBalance`. That is, splitting up what I explored in https://github.com/openfoodfoundation/openfoodnetwork/pull/6959 but in very small and manageable pieces. --- app/models/order_balance.rb | 33 +++++ spec/models/order_balance_spec.rb | 219 ++++++++++++++++++++++++++++++ 2 files changed, 252 insertions(+) create mode 100644 app/models/order_balance.rb create mode 100644 spec/models/order_balance_spec.rb diff --git a/app/models/order_balance.rb b/app/models/order_balance.rb new file mode 100644 index 0000000000..58fb03a353 --- /dev/null +++ b/app/models/order_balance.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class OrderBalance + def initialize(order) + @order = order + end + + def label + to_f.negative? ? I18n.t(:credit_owed) : I18n.t(:balance_due) + end + + def amount + Spree::Money.new(to_f, currency: order.currency) + end + + def to_f + if customer_balance_enabled? + order.new_outstanding_balance + else + order.outstanding_balance + end + end + + delegate :zero?, to: :to_f + + private + + attr_reader :order + + def customer_balance_enabled? + OpenFoodNetwork::FeatureToggle.enabled?(:customer_balance, order.user) + end +end diff --git a/spec/models/order_balance_spec.rb b/spec/models/order_balance_spec.rb new file mode 100644 index 0000000000..93d5ca5d2c --- /dev/null +++ b/spec/models/order_balance_spec.rb @@ -0,0 +1,219 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe OrderBalance do + subject(:order_balance) { described_class.new(order) } + let(:order) { build(:order) } + let(:user) { order.user } + + describe '#label' do + context 'when the customer_balance feature is disabled' do + before do + allow(OpenFoodNetwork::FeatureToggle) + .to receive(:enabled?).with(:customer_balance, user) { false } + end + + context 'when the balance is postive' do + before do + allow(order).to receive(:outstanding_balance) { 10 } + end + + it "returns 'balance due'" do + expect(order_balance.label).to eq(I18n.t(:balance_due)) + end + end + + context 'when the balance is negative' do + before do + allow(order).to receive(:outstanding_balance) { -10 } + end + + it "returns 'credit owed'" do + expect(order_balance.label).to eq(I18n.t(:credit_owed)) + end + end + + context 'when the balance is zero' do + before do + allow(order).to receive(:outstanding_balance) { 0 } + end + + it "returns 'balance due'" do + expect(order_balance.label).to eq(I18n.t(:balance_due)) + end + end + end + + context 'when the customer_balance feature is enabled' do + before do + allow(OpenFoodNetwork::FeatureToggle) + .to receive(:enabled?).with(:customer_balance, user) { true } + end + + context 'when the balance is postive' do + before do + allow(order).to receive(:new_outstanding_balance) { 10 } + end + + it "returns 'balance due'" do + expect(order_balance.label).to eq(I18n.t(:balance_due)) + end + end + + context 'when the balance is negative' do + before do + allow(order).to receive(:new_outstanding_balance) { -10 } + end + + it "returns 'credit owed'" do + expect(order_balance.label).to eq(I18n.t(:credit_owed)) + end + end + + context 'when the balance is zero' do + before do + allow(order).to receive(:new_outstanding_balance) { 0 } + end + + it "returns 'balance due'" do + expect(order_balance.label).to eq(I18n.t(:balance_due)) + end + end + end + end + + describe '#amount' do + context 'when the customer_balance feature is disabled' do + before do + allow(order).to receive(:outstanding_balance) { 10 } + end + + before do + allow(OpenFoodNetwork::FeatureToggle) + .to receive(:enabled?).with(:customer_balance, user) { false } + end + + it 'returns the balance wraped in a Money object' do + expect(order_balance.amount).to eq(Spree::Money.new(10, currency: ENV['currency'])) + end + end + + context 'when the customer_balance feature is enabled' do + before do + allow(order).to receive(:new_outstanding_balance) { 20 } + end + + before do + allow(OpenFoodNetwork::FeatureToggle) + .to receive(:enabled?).with(:customer_balance, user) { true } + end + + it 'returns the balance wraped in a Money object' do + expect(order_balance.amount).to eq(Spree::Money.new(20, currency: ENV['currency'])) + end + end + end + + describe '#zero?' do + context 'when the customer_balance feature is disabled' do + before do + allow(OpenFoodNetwork::FeatureToggle) + .to receive(:enabled?).with(:customer_balance, user) { false } + end + + context 'when the balance is zero' do + before do + allow(order).to receive(:outstanding_balance) { 0 } + end + + it 'returns true' do + expect(order_balance.zero?).to eq(true) + end + end + + context 'when the balance is positive' do + before do + allow(order).to receive(:outstanding_balance) { 10 } + end + + it 'returns false' do + expect(order_balance.zero?).to eq(false) + end + end + + context 'when the balance is negative' do + before do + allow(order).to receive(:outstanding_balance) { -10 } + end + + it 'returns false' do + expect(order_balance.zero?).to eq(false) + end + end + end + + context 'when the customer_balance feature is enabled' do + before do + allow(OpenFoodNetwork::FeatureToggle) + .to receive(:enabled?).with(:customer_balance, user) { true } + end + + context 'when the balance is zero' do + before do + allow(order).to receive(:new_outstanding_balance) { 0 } + end + + it 'returns true' do + expect(order_balance.zero?).to eq(true) + end + end + + context 'when the balance is positive' do + before do + allow(order).to receive(:new_outstanding_balance) { 10 } + end + + it 'returns false' do + expect(order_balance.zero?).to eq(false) + end + end + + context 'when the balance is negative' do + before do + allow(order).to receive(:new_outstanding_balance) { -10 } + end + + it 'returns false' do + expect(order_balance.zero?).to eq(false) + end + end + end + end + + describe '#to_f' do + context 'when the customer_balance feature is disabled' do + before do + allow(OpenFoodNetwork::FeatureToggle) + .to receive(:enabled?).with(:customer_balance, user) { false } + end + + it 'calls #outstanding_balance' do + expect(order).to receive(:outstanding_balance) + order_balance.to_f + end + end + + context 'when the customer_balance feature is enabled' do + before do + allow(OpenFoodNetwork::FeatureToggle) + .to receive(:enabled?).with(:customer_balance, user) { true } + end + + it 'calls #new_outstanding_balance' do + expect(order).to receive(:new_outstanding_balance) + order_balance.to_f + end + end + end +end