From e20b06bb9776d406a5ca14f443b04931e9074af7 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 14 Oct 2015 10:30:22 +1100 Subject: [PATCH 01/25] Adding basic route and controller for business model configuration --- ...business_model_configuration_controller.rb | 3 +++ config/routes.rb | 2 ++ ...ess_model_configuration_controller_spec.rb | 26 +++++++++++++++++++ 3 files changed, 31 insertions(+) create mode 100644 app/controllers/admin/business_model_configuration_controller.rb create mode 100644 spec/controllers/admin/business_model_configuration_controller_spec.rb diff --git a/app/controllers/admin/business_model_configuration_controller.rb b/app/controllers/admin/business_model_configuration_controller.rb new file mode 100644 index 0000000000..0ac2a00eb7 --- /dev/null +++ b/app/controllers/admin/business_model_configuration_controller.rb @@ -0,0 +1,3 @@ +class Admin::BusinessModelConfigurationController < Spree::Admin::BaseController + +end diff --git a/config/routes.rb b/config/routes.rb index eb8c228b91..902855db2a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -116,6 +116,8 @@ Openfoodnetwork::Application.routes.draw do end end + resource :business_model_configuration, only: [:edit] + resource :account, only: [:show], controller: 'account' end diff --git a/spec/controllers/admin/business_model_configuration_controller_spec.rb b/spec/controllers/admin/business_model_configuration_controller_spec.rb new file mode 100644 index 0000000000..df80bce9e6 --- /dev/null +++ b/spec/controllers/admin/business_model_configuration_controller_spec.rb @@ -0,0 +1,26 @@ +require 'spec_helper' + +describe Admin::AccountsAndBillingSettingsController, type: :controller do + let(:user) { create(:user) } + let(:admin) { create(:admin_user) } + + describe "edit" do + context "as an enterprise user" do + before { allow(controller).to receive(:spree_current_user) { user } } + + it "does not allow access" do + spree_get :edit + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "as super admin" do + before { allow(controller).to receive(:spree_current_user) { admin } } + + it "allows access" do + spree_get :edit + expect(response).to_not redirect_to spree.unauthorized_path + end + end + end +end From 58031408f199374139a48d369918c8e63d725722 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 15 Oct 2015 10:57:52 +1100 Subject: [PATCH 02/25] Adding basic business model configuration variables to app config --- app/models/spree/app_configuration_decorator.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb index 2e1c838e95..0ecdc0e4ca 100644 --- a/app/models/spree/app_configuration_decorator.rb +++ b/app/models/spree/app_configuration_decorator.rb @@ -16,4 +16,9 @@ Spree::AppConfiguration.class_eval do preference :default_accounts_shipping_method_id, :integer, default: nil preference :auto_update_invoices, :boolean, default: false preference :auto_finalize_invoices, :boolean, default: false + + # Business Model Configuration + preference :account_invoices_monthly_fixed, :decimal, default: 0 + preference :account_invoices_monthly_rate, :decimal, default: 0 + preference :account_invoices_monthly_cap, :decimal, default: 0 end From cb9b61f393214269936f442a425819f6c3d46821 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 15 Oct 2015 10:58:44 +1100 Subject: [PATCH 03/25] WIP: Adding an edit view for BMC variables --- .../admin/openfoodnetwork.css.scss | 4 ++ .../edit.html.haml | 60 +++++++++++++++++++ config/routes.rb | 2 +- 3 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 app/views/admin/business_model_configuration/edit.html.haml diff --git a/app/assets/stylesheets/admin/openfoodnetwork.css.scss b/app/assets/stylesheets/admin/openfoodnetwork.css.scss index c43b4098e7..d5a53e11c4 100644 --- a/app/assets/stylesheets/admin/openfoodnetwork.css.scss +++ b/app/assets/stylesheets/admin/openfoodnetwork.css.scss @@ -6,6 +6,10 @@ text-align: right; } +.underline { + text-decoration: underline; +} + table .blank-action { display: inline-block; width: 29px; diff --git a/app/views/admin/business_model_configuration/edit.html.haml b/app/views/admin/business_model_configuration/edit.html.haml new file mode 100644 index 0000000000..373c0584a6 --- /dev/null +++ b/app/views/admin/business_model_configuration/edit.html.haml @@ -0,0 +1,60 @@ += render :partial => 'spree/admin/shared/configuration_menu' + +- content_for :page_title do + = t(:business_model_configuration) + + +%p + This page allows configuration of the rates at which shops will be charged for use of the Open Food Network. +%br +%p + Total monthly bill will be: Fixed Charge + (Turnover x Percentage), unless this value exceeds the Monthly Cap, in which case the Monthly Cap will be charged instead. + += render 'spree/shared/error_messages', target: @settings + +%fieldset.no-border-bottom + %legend Settings + = form_for @settings, as: :enterprise_types, url: main_app.admin_business_model_configuration_path, :method => :put do |f| + .row + .one.column.alpha + .field + = label :nothing, "" + %br + Bill   = + .two.columns + .field + = f.label :account_invoices_monthly_fixed, t(:fixed_amount) + %span.with-tip.icon-question-sign{'data-powertip' => "A fixed monthly charge for ALL enterprises who are set up as a shop."} + %br + = f.number_field :account_invoices_monthly_fixed, min: 0.0, class: "fullwidth" + .one.column.text-center + .field + = label :nothing, "" + %br + = "+" + .two.columns.text-center + .field + = label :nothing, "" + %br + (  Turnover   x + .two.columns + .field + = f.label :account_invoices_monthly_rate, t(:percentage) + %span.with-tip.icon-question-sign{'data-powertip' => "A rate (0.0 - 1.0) that will be applied to the total turnover of each shop."} + %br + = f.number_field :account_invoices_monthly_rate, min: 0.0, max: 1.0, step: 0.01, class: "fullwidth" + .one.column + .field + = label :nothing, "" + %br + ) + .two.columns.omega + .field + = f.label :account_invoices_monthly_cap, t(:capped_at) + %span.with-tip.icon-question-sign{'data-powertip' => "A cap on the amount that shops will be charged each month."} + %br + = f.number_field :account_invoices_monthly_cap, min: 0.0, class: "fullwidth" + + .row + .twelve.columns.alpha.omega.form-buttons{"data-hook" => "buttons"} + = button t(:update), 'icon-refresh', value: "update" diff --git a/config/routes.rb b/config/routes.rb index 902855db2a..c8771560e9 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -116,7 +116,7 @@ Openfoodnetwork::Application.routes.draw do end end - resource :business_model_configuration, only: [:edit] + resource :business_model_configuration, only: [:edit, :update], controller: 'business_model_configuration' resource :account, only: [:show], controller: 'account' end From 044e42354354b271cab73161486a668b25b4b2dd Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 15 Oct 2015 13:50:14 +1100 Subject: [PATCH 04/25] Adding update logic to business model config controller --- ...business_model_configuration_controller.rb | 29 ++++++++- .../business_model_configuration_validator.rb | 19 ++++++ ...ess_model_configuration_controller_spec.rb | 59 ++++++++++++++++++- 3 files changed, 105 insertions(+), 2 deletions(-) create mode 100644 lib/open_food_network/business_model_configuration_validator.rb diff --git a/app/controllers/admin/business_model_configuration_controller.rb b/app/controllers/admin/business_model_configuration_controller.rb index 0ac2a00eb7..e421b2e597 100644 --- a/app/controllers/admin/business_model_configuration_controller.rb +++ b/app/controllers/admin/business_model_configuration_controller.rb @@ -1,3 +1,30 @@ -class Admin::BusinessModelConfigurationController < Spree::Admin::BaseController +require 'open_food_network/business_model_configuration_validator' +class Admin::BusinessModelConfigurationController < Spree::Admin::BaseController + before_filter :load_settings, only: [:edit, :update] + before_filter :require_valid_settings, only: [:update] + + def update + Spree::Config.set(params[:settings]) + flash[:success] = t(:successfully_updated, :resource => t(:business_model_configuration)) + redirect_to_edit + end + + private + + def redirect_to_edit + redirect_to main_app.edit_admin_business_model_configuration_path + end + + def load_settings + @settings = OpenFoodNetwork::BusinessModelConfigurationValidator.new(params[:settings] || { + account_invoices_monthly_fixed: Spree::Config[:account_invoices_monthly_fixed], + account_invoices_monthly_rate: Spree::Config[:account_invoices_monthly_rate], + account_invoices_monthly_cap: Spree::Config[:account_invoices_monthly_cap] + }) + end + + def require_valid_settings + render :edit unless @settings.valid? + end end diff --git a/lib/open_food_network/business_model_configuration_validator.rb b/lib/open_food_network/business_model_configuration_validator.rb new file mode 100644 index 0000000000..292894daea --- /dev/null +++ b/lib/open_food_network/business_model_configuration_validator.rb @@ -0,0 +1,19 @@ +# This class is a lightweight model used to validate preferences for business model configuration +# when they are submitted to the BusinessModelConfigurationController + +module OpenFoodNetwork + class BusinessModelConfigurationValidator + include ActiveModel::Validations + + attr_accessor :account_invoices_monthly_fixed, :account_invoices_monthly_rate, :account_invoices_monthly_cap + + validates :account_invoices_monthly_fixed, presence: true, numericality: { greater_than_or_equal_to: 0 } + validates :account_invoices_monthly_rate, presence: true, numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 1 } + validates :account_invoices_monthly_cap, presence: true, numericality: { greater_than_or_equal_to: 0 } + + def initialize(attr, button=nil) + attr.each { |k,v| instance_variable_set("@#{k}", v) } + @button = button + end + end +end diff --git a/spec/controllers/admin/business_model_configuration_controller_spec.rb b/spec/controllers/admin/business_model_configuration_controller_spec.rb index df80bce9e6..7aed5cfbf7 100644 --- a/spec/controllers/admin/business_model_configuration_controller_spec.rb +++ b/spec/controllers/admin/business_model_configuration_controller_spec.rb @@ -1,9 +1,17 @@ require 'spec_helper' -describe Admin::AccountsAndBillingSettingsController, type: :controller do +describe Admin::BusinessModelConfigurationController, type: :controller do let(:user) { create(:user) } let(:admin) { create(:admin_user) } + before do + Spree::Config.set({ + account_invoices_monthly_fixed: 5, + account_invoices_monthly_rate: 0.02, + account_invoices_monthly_cap: 50 + }) + end + describe "edit" do context "as an enterprise user" do before { allow(controller).to receive(:spree_current_user) { user } } @@ -23,4 +31,53 @@ describe Admin::AccountsAndBillingSettingsController, type: :controller do end end end + + describe "update" do + context "as an enterprise user" do + before { allow(controller).to receive(:spree_current_user) { user } } + + it "does not allow access" do + spree_get :update + expect(response).to redirect_to spree.unauthorized_path + end + end + + context "as super admin" do + before {allow(controller).to receive(:spree_current_user) { admin } } + let(:params) { { settings: { } } } + + context "when settings are invalid" do + before do + params[:settings][:account_invoices_monthly_fixed] = '' + params[:settings][:account_invoices_monthly_rate] = '2' + params[:settings][:account_invoices_monthly_cap] = '-1' + spree_get :update, params + end + + it "does not allow them to be set" do + expect(response).to render_template :edit + expect(assigns(:settings).errors.count).to be 4 + expect(Spree::Config.account_invoices_monthly_fixed).to eq 5 + expect(Spree::Config.account_invoices_monthly_rate).to eq 0.02 + expect(Spree::Config.account_invoices_monthly_cap).to eq 50 + end + end + + context "when required settings are valid" do + before do + params[:settings][:account_invoices_monthly_fixed] = '10' + params[:settings][:account_invoices_monthly_rate] = '0.05' + params[:settings][:account_invoices_monthly_cap] = '30' + end + + it "sets global config to the specified values" do + spree_get :update, params + expect(assigns(:settings).errors.count).to be 0 + expect(Spree::Config.account_invoices_monthly_fixed).to eq 10 + expect(Spree::Config.account_invoices_monthly_rate).to eq 0.05 + expect(Spree::Config.account_invoices_monthly_cap).to eq 30 + end + end + end + end end From a40a03905fbfec5cc23f5dfb56930c0c69ea7baf Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 15 Oct 2015 14:14:21 +1100 Subject: [PATCH 05/25] Reorganising BMC edit view --- .../edit.html.haml | 32 +++++------- .../business_model_configuration_spec.rb | 51 +++++++++++++++++++ 2 files changed, 64 insertions(+), 19 deletions(-) create mode 100644 spec/features/admin/business_model_configuration_spec.rb diff --git a/app/views/admin/business_model_configuration/edit.html.haml b/app/views/admin/business_model_configuration/edit.html.haml index 373c0584a6..a00e76fa17 100644 --- a/app/views/admin/business_model_configuration/edit.html.haml +++ b/app/views/admin/business_model_configuration/edit.html.haml @@ -1,57 +1,51 @@ = render :partial => 'spree/admin/shared/configuration_menu' - content_for :page_title do - = t(:business_model_configuration) - - -%p - This page allows configuration of the rates at which shops will be charged for use of the Open Food Network. -%br -%p - Total monthly bill will be: Fixed Charge + (Turnover x Percentage), unless this value exceeds the Monthly Cap, in which case the Monthly Cap will be charged instead. + %h1.page-title= t(:business_model_configuration) + %a.with-tip{ 'data-powertip' => "Configure the rate at which shops will be charged each month for use of the Open Food Network." } What's this? = render 'spree/shared/error_messages', target: @settings %fieldset.no-border-bottom - %legend Settings - = form_for @settings, as: :enterprise_types, url: main_app.admin_business_model_configuration_path, :method => :put do |f| + %legend=t(:monthly_bill_calculation_settings) + = form_for @settings, as: :settings, url: main_app.admin_business_model_configuration_path, :method => :put do |f| .row .one.column.alpha .field = label :nothing, "" %br - Bill   = + %h6 Bill   = .two.columns .field - = f.label :account_invoices_monthly_fixed, t(:fixed_amount) - %span.with-tip.icon-question-sign{'data-powertip' => "A fixed monthly charge for ALL enterprises who are set up as a shop."} + = f.label :account_invoices_monthly_fixed, t(:fixed_charge) + %span.with-tip.icon-question-sign{'data-powertip' => "A fixed monthly charge for ALL enterprises who are set up as a shop, regardless of how much produce they sell."} %br = f.number_field :account_invoices_monthly_fixed, min: 0.0, class: "fullwidth" .one.column.text-center .field = label :nothing, "" %br - = "+" - .two.columns.text-center + %h6 + + .two.columns .field = label :nothing, "" %br - (  Turnover   x + %h6 (  Turnover   × .two.columns .field = f.label :account_invoices_monthly_rate, t(:percentage) - %span.with-tip.icon-question-sign{'data-powertip' => "A rate (0.0 - 1.0) that will be applied to the total turnover of each shop."} + %span.with-tip.icon-question-sign{'data-powertip' => "When greater than zero, this rate (0.0 - 1.0) will be applied to the total turnover of each shop and added to any fixed charges (to the left) to calculate the monthly bill."} %br = f.number_field :account_invoices_monthly_rate, min: 0.0, max: 1.0, step: 0.01, class: "fullwidth" .one.column .field = label :nothing, "" %br - ) + %h6 ) .two.columns.omega .field = f.label :account_invoices_monthly_cap, t(:capped_at) - %span.with-tip.icon-question-sign{'data-powertip' => "A cap on the amount that shops will be charged each month."} + %span.with-tip.icon-question-sign{'data-powertip' => "When greater than zero, this value will be used as a cap on the amount that shops will be charged each month."} %br = f.number_field :account_invoices_monthly_cap, min: 0.0, class: "fullwidth" diff --git a/spec/features/admin/business_model_configuration_spec.rb b/spec/features/admin/business_model_configuration_spec.rb new file mode 100644 index 0000000000..2a97f983bc --- /dev/null +++ b/spec/features/admin/business_model_configuration_spec.rb @@ -0,0 +1,51 @@ +require 'spec_helper' + +feature 'Business Model Configuration' do + include AuthenticationWorkflow + include WebHelper + + describe "updating" do + let!(:admin) { create(:admin_user) } + + before do + Spree::Config.set({ + account_invoices_monthly_fixed: 5, + account_invoices_monthly_rate: 0.02, + account_invoices_monthly_cap: 50 + }) + end + + before do + quick_login_as_admin + end + + context "as an admin user", js: true do + # it "loads the page" do + # visit spree.admin_path + # click_link "Configuration" + # click_link "Accounts & Billing" + # + # expect(page).to have_select2 "settings_accounts_distributor_id" + # select2_select accounts_distributor.name, from: "settings_accounts_distributor_id" + # expect(page).to have_select "settings_default_accounts_payment_method_id" + # expect(page).to have_select "settings_default_accounts_shipping_method_id" + # expect(page).to have_link "Update User Invoices", href: start_job_admin_accounts_and_billing_settings_path(job: { name: 'update_account_invoices'}) + # expect(page).to have_link "Finalise User Invoices", href: start_job_admin_accounts_and_billing_settings_path(job: { name: 'finalize_account_invoices'}) + # end + + it "attributes can be changed", js: true do + visit edit_admin_business_model_configuration_path + + fill_in "settings_account_invoices_monthly_fixed", with: 10 + fill_in "settings_account_invoices_monthly_rate", with: 0.05 + fill_in "settings_account_invoices_monthly_cap", with: 30 + + click_button "Update" + + expect(Spree::Config.account_invoices_monthly_fixed).to eq 10 + expect(Spree::Config.account_invoices_monthly_rate).to eq 0.05 + expect(Spree::Config.account_invoices_monthly_cap).to eq 30 + end + end + end +end From da325780b150c41cab10f7b2d3dd6334203c6c45 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 15 Oct 2015 15:13:17 +1100 Subject: [PATCH 06/25] Adding BMC link to configurations menu --- ...iness_model_configuration.html.haml.deface | 4 ++++ .../business_model_configuration_spec.rb | 21 ++++++++----------- 2 files changed, 13 insertions(+), 12 deletions(-) create mode 100644 app/overrides/spree/admin/shared/_configuration_menu/add_business_model_configuration.html.haml.deface diff --git a/app/overrides/spree/admin/shared/_configuration_menu/add_business_model_configuration.html.haml.deface b/app/overrides/spree/admin/shared/_configuration_menu/add_business_model_configuration.html.haml.deface new file mode 100644 index 0000000000..7d5c311a7b --- /dev/null +++ b/app/overrides/spree/admin/shared/_configuration_menu/add_business_model_configuration.html.haml.deface @@ -0,0 +1,4 @@ +// insert_bottom "[data-hook='admin_configurations_sidebar_menu']" + +%li + = link_to 'Business Model', main_app.edit_admin_business_model_configuration_path diff --git a/spec/features/admin/business_model_configuration_spec.rb b/spec/features/admin/business_model_configuration_spec.rb index 2a97f983bc..dbcaf7e8bc 100644 --- a/spec/features/admin/business_model_configuration_spec.rb +++ b/spec/features/admin/business_model_configuration_spec.rb @@ -20,18 +20,15 @@ feature 'Business Model Configuration' do end context "as an admin user", js: true do - # it "loads the page" do - # visit spree.admin_path - # click_link "Configuration" - # click_link "Accounts & Billing" - # - # expect(page).to have_select2 "settings_accounts_distributor_id" - # select2_select accounts_distributor.name, from: "settings_accounts_distributor_id" - # expect(page).to have_select "settings_default_accounts_payment_method_id" - # expect(page).to have_select "settings_default_accounts_shipping_method_id" - # expect(page).to have_link "Update User Invoices", href: start_job_admin_accounts_and_billing_settings_path(job: { name: 'update_account_invoices'}) - # expect(page).to have_link "Finalise User Invoices", href: start_job_admin_accounts_and_billing_settings_path(job: { name: 'finalize_account_invoices'}) - # end + it "loads the page" do + visit spree.admin_path + click_link "Configuration" + click_link "Business Model" + + expect(page).to have_field "settings_account_invoices_monthly_fixed", with: 5.0 + expect(page).to have_field "settings_account_invoices_monthly_rate", with: 0.02 + expect(page).to have_field "settings_account_invoices_monthly_cap", with: 50.0 + end it "attributes can be changed", js: true do visit edit_admin_business_model_configuration_path From 5b72f53738709a5a513fb304fd691504ef03ff11 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 28 Oct 2015 11:39:01 +1100 Subject: [PATCH 07/25] Adding helper for describing monthly billing charges --- .../business_model_configuration_helper.rb | 44 ++++++++++++ config/locales/en.yml | 7 +- lib/spree/money_decorator.rb | 5 ++ ...usiness_model_configuration_helper_spec.rb | 69 +++++++++++++++++++ 4 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 app/helpers/admin/business_model_configuration_helper.rb create mode 100644 spec/helpers/admin/business_model_configuration_helper_spec.rb diff --git a/app/helpers/admin/business_model_configuration_helper.rb b/app/helpers/admin/business_model_configuration_helper.rb new file mode 100644 index 0000000000..115fcf33ae --- /dev/null +++ b/app/helpers/admin/business_model_configuration_helper.rb @@ -0,0 +1,44 @@ +module Admin + module BusinessModelConfigurationHelper + def monthly_bill_description + plus = monthly_bill_includes_fixed? && monthly_bill_includes_rate? ? " + " : "" + + if fixed_description.empty? && rate_description.empty? + t(:free).upcase + elsif monthly_bill_includes_cap? && monthly_bill_includes_rate? # only care about cap if there is a rate too + "#{fixed_description}#{plus}#{rate_description}{joiner}#{cap_description} #{t(:per_month).upcase}" + else + "#{fixed_description}#{plus}#{rate_description} #{t(:per_month).upcase}" + end + end + + private + + def fixed_description + fixed_amount = Spree::Money.new(Spree::Config[:account_invoices_monthly_fixed], {currency: Spree::Config[:currency]} ).rounded + monthly_bill_includes_fixed? ? "#{fixed_amount}" : "" + end + + def rate_description + percentage = (Spree::Config[:account_invoices_monthly_rate]*100).round(2) + monthly_bill_includes_rate? ? t(:percentage_of_sales, percentage: "#{percentage}%").upcase : "" + end + + def cap_description + cap_amount = Spree::Money.new(Spree::Config[:account_invoices_monthly_cap], { currency: Spree::Config[:currency] }).rounded + monthly_bill_includes_cap? ? "#{t(:capped_at_cap, cap: cap_amount).upcase}" : "" + end + + def monthly_bill_includes_fixed? + Spree::Config[:account_invoices_monthly_fixed] > 0 + end + + def monthly_bill_includes_rate? + Spree::Config[:account_invoices_monthly_rate] > 0 + end + + def monthly_bill_includes_cap? + Spree::Config[:account_invoices_monthly_cap] > 0 + end + end +end diff --git a/config/locales/en.yml b/config/locales/en.yml index 373440ebfe..5a95fdb567 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -30,6 +30,12 @@ en: confirm_send_invoice: "An invoice for this order will be sent to the customer. Are you sure you want to continue?" confirm_resend_order_confirmation: "Are you sure you want to resend the order confirmation email?" must_have_valid_business_number: "%{enterprise_name} must have a valid ABN before invoices can be sent." + invoice: "Invoice" + percentage_of_sales: "%{percentage} of sales" + capped_at: "capped at" + capped_at_cap: "capped at %{cap}" + per_month: "per month" + free: "free" sort_order_cycles_on_shopfront_by: "Sort Order Cycles On Shopfront By" @@ -55,7 +61,6 @@ en: footer_links_md: "Links" footer_about_url: "About URL" footer_tos_url: "Terms of Service URL" - invoice: "Invoice" name: Name first_name: First Name diff --git a/lib/spree/money_decorator.rb b/lib/spree/money_decorator.rb index fed92b8210..3479bbd9a2 100644 --- a/lib/spree/money_decorator.rb +++ b/lib/spree/money_decorator.rb @@ -4,4 +4,9 @@ Spree::Money.class_eval do def self.currency_symbol Money.new(0, Spree::Config[:currency]).symbol end + + def rounded + @options[:no_cents] = true if @money.amount % 1 == 0 + to_s + end end diff --git a/spec/helpers/admin/business_model_configuration_helper_spec.rb b/spec/helpers/admin/business_model_configuration_helper_spec.rb new file mode 100644 index 0000000000..0c44d5c742 --- /dev/null +++ b/spec/helpers/admin/business_model_configuration_helper_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe Admin::BusinessModelConfigurationHelper do + describe "describing monthly bills for enterprises" do + context "when a fixed cost is included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 10) } + + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.05) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "$10 + 5.0% OF SALES{joiner}CAPPED AT $20 PER MONTH" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "$10 + 5.0% OF SALES PER MONTH" } + end + end + + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "$10 PER MONTH" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "$10 PER MONTH" } + end + end + end + + context "when a fixed cost is not included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 0) } + + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.05) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "5.0% OF SALES{joiner}CAPPED AT $20 PER MONTH" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "5.0% OF SALES PER MONTH" } + end + end + + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "FREE" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "FREE" } + end + end + end + end +end From c1d04af5ccfc3003f7878a064de348caf8ea4bf2 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 28 Oct 2015 11:40:01 +1100 Subject: [PATCH 08/25] Implementing monthly billing description logic in required pages --- .../monthly_pricing_description.js.coffee | 8 +++++ .../admin/panels/enterprise_package.html.haml | 22 ++++++------- app/helpers/admin/injection_helper.rb | 6 ++++ .../enterprises/_change_type_form.html.haml | 32 ++++--------------- app/views/admin/enterprises/index.html.haml | 1 + 5 files changed, 32 insertions(+), 37 deletions(-) create mode 100644 app/assets/javascripts/admin/enterprises/directives/monthly_pricing_description.js.coffee diff --git a/app/assets/javascripts/admin/enterprises/directives/monthly_pricing_description.js.coffee b/app/assets/javascripts/admin/enterprises/directives/monthly_pricing_description.js.coffee new file mode 100644 index 0000000000..6331fa2ca5 --- /dev/null +++ b/app/assets/javascripts/admin/enterprises/directives/monthly_pricing_description.js.coffee @@ -0,0 +1,8 @@ +angular.module("admin.enterprises").directive "monthlyPricingDescription", (monthlyBillDescription) -> + restrict: 'E' + scope: + joiner: "@" + template: "" + link: (scope, element, attrs) -> + joiners = { comma: ", ", newline: "
" } + scope.billDescription = monthlyBillDescription.replace("{joiner}", joiners[scope.joiner]) diff --git a/app/assets/javascripts/templates/admin/panels/enterprise_package.html.haml b/app/assets/javascripts/templates/admin/panels/enterprise_package.html.haml index c42d8910e1..7f143d1071 100644 --- a/app/assets/javascripts/templates/admin/panels/enterprise_package.html.haml +++ b/app/assets/javascripts/templates/admin/panels/enterprise_package.html.haml @@ -15,7 +15,8 @@ %h3 Hub Shop %p - %strong COST: 2% OF SALES, CAPPED AT $50 PER MONTH + %strong + %monthly-pricing-description{ joiner: "comma" } %p Your enterprise is the backbone of your local food system. You aggregate produce from other enterprises and can sell it through your shop on the Open Food Network. @@ -53,7 +54,8 @@ %h3 Producer Shop %p - %strong COST: 2% OF SALES, CAPPED AT $50 PER MONTH + %strong + %monthly-pricing-description{ joiner: "comma" } %p Sell your products directly to customers through your very own Open Food Network shopfront. @@ -63,7 +65,8 @@ %h3 Producer Hub %p - %strong COST: 2% OF SALES, CAPPED AT $50 PER MONTH + %strong + %monthly-pricing-description{ joiner: "comma" } %p Your enterprise is the backbone of your local food system. You can sell your own produce as well as produce aggregated from other enterprises through your shopfront on the Open Food Network. @@ -94,9 +97,7 @@ %h3 Hub Shop %p Sell produce from others .bottom - \2% OF SALES - %br - CAPPED AT $50 PER MONTH + %monthly-pricing-description{ joiner: "newline" } %div{ ng: { switch: { when: "true" } } } %a.button.selector.producer-profile{ ng: { click: "enterprise.owned && (enterprise.sells='none')", class: "{selected: enterprise.sells=='none', disabled: !enterprise.owned}" } } @@ -109,17 +110,14 @@ %h3 Producer Shop %p Sell your own produce .bottom - \2% OF SALES - %br - CAPPED AT $50 PER MONTH + %monthly-pricing-description{ joiner: "newline" } + %a.button.selector.producer-hub{ ng: { click: "enterprise.owned && (enterprise.sells='any')", class: "{selected: enterprise.sells=='any', disabled: !enterprise.owned}" } } .top %h3 Producer Hub %p Sell produce from self and others .bottom - \2% OF SALES - %br - CAPPED AT $50 PER MONTH + %monthly-pricing-description{ joiner: "newline" } %a.button.update.fullwidth{ ng: { show: "enterprise.owned", class: "{disabled: saved() && !saving, saving: saving}", click: "save()" } } %span{ ng: {hide: "saved() || saving" } } diff --git a/app/helpers/admin/injection_helper.rb b/app/helpers/admin/injection_helper.rb index 490e3dc00b..6036447d9b 100644 --- a/app/helpers/admin/injection_helper.rb +++ b/app/helpers/admin/injection_helper.rb @@ -1,5 +1,7 @@ module Admin module InjectionHelper + include BusinessModelConfigurationHelper + def admin_inject_enterprise admin_inject_json_ams "admin.enterprises", "enterprise", @enterprise, Api::Admin::EnterpriseSerializer end @@ -78,6 +80,10 @@ module Admin admin_inject_json_ams_array "admin.orders", "orderCycles", @order_cycles, Api::Admin::BasicOrderCycleSerializer, current_user: spree_current_user end + def admin_inject_monthly_bill_description + render partial: "admin/json/injection_ams", locals: {ngModule: "admin.enterprises", name: "monthlyBillDescription", json: monthly_bill_description.to_json} + end + def admin_inject_spree_api_key render partial: "admin/json/injection_ams", locals: {ngModule: 'ofn.admin', name: 'SpreeApiKey', json: "'#{@spree_api_key.to_s}'"} end diff --git a/app/views/admin/enterprises/_change_type_form.html.haml b/app/views/admin/enterprises/_change_type_form.html.haml index 740857e381..6474af1344 100644 --- a/app/views/admin/enterprises/_change_type_form.html.haml +++ b/app/views/admin/enterprises/_change_type_form.html.haml @@ -1,4 +1,5 @@ = admin_inject_enterprise += admin_inject_monthly_bill_description = form_for @enterprise, url: main_app.register_admin_enterprise_path(@enterprise), html: { name: "change_type", id: "change_type", novalidate: true, "ng-app" => "admin.enterprises", "ng-controller"=> 'changeTypeFormCtrl' } do |change_type_form| @@ -16,9 +17,6 @@ .bottom ALWAYS FREE %p.description Add your products to Open Food Network, allowing hubs to stock your products in their stores. - %br - %br - Having a profile, and making connections within your local food system through the Open Food Network will always be free. .producer_shop.option.one-third.column %a.full-width.button.selector{ ng: { click: "sells='own'", class: "{selected: sells=='own'}" } } @@ -26,17 +24,13 @@ %h3 Producer Shop %p Sell your own produce .bottom - \%2 OF SALES - %br - CAPPED AT $50 PER MONTH + %monthly-pricing-description{ joiner: "newline" } + %p.description Sell your products directly to customers through your very own Open Food Network shopfront. %br %br A Producer Shop is for your produce only, if you want to sell produce grown/produced off site, select 'Producer Hub'. - %br - %br - You will be billed for 2% of your actual transactions, capped at $50 a month (so if you don’t sell anything you don’t pay anything, but you never pay more than $50 a month). .full_hub.option.one-third.column.omega %a.full-width.button.selector{ ng: { click: "sells='any'", class: "{selected: sells=='any'}" } } @@ -44,15 +38,10 @@ %h3 Producer Hub %p Sell produce from self and others .bottom - \%2 OF SALES - %br - CAPPED AT $50 PER MONTH + %monthly-pricing-description{ joiner: "newline" } + %p.description Your enterprise is the backbone of your local food system. You can sell your own produce as well as produce aggregated from other enterprises through your shopfront on the Open Food Network. - %br - %br - You will be billed for 2% of your actual transactions, capped at $50 a month (so if you don’t sell anything you don’t pay anything, but you never pay more than $50 a month). - -# %p.description -# Test out having your own shopfront with full access to all Shopfront features for 30 days. @@ -69,9 +58,6 @@ .bottom ALWAYS FREE %p.description People can find and contact you on the Open Food Network. Your enterprise will be visible on the map, and will be searchable in listings. - %br - %br - Having a profile, and making connections within your local food system through the Open Food Network will always be free. .full_hub.option.one-third.column %a.full-width.button.selector{ ng: { click: "sells='any'", class: "{selected: sells=='any'}" } } @@ -79,14 +65,10 @@ %h3 Hub Shop %p Sell produce from others .bottom - \%2 OF SALES - %br - CAPPED AT $50 PER MONTH + %monthly-pricing-description{ joiner: "newline" } + %p.description Your enterprise is the backbone of your local food system. You aggregate produce from other enterprises and can sell it through your shop on the Open Food Network. - %br - %br - You will be billed for 2% of your actual transactions, capped at $50 a month (so if you don’t sell anything you don’t pay anything, but you never pay more than $50 a month). .row diff --git a/app/views/admin/enterprises/index.html.haml b/app/views/admin/enterprises/index.html.haml index 826d09336c..62bc881728 100644 --- a/app/views/admin/enterprises/index.html.haml +++ b/app/views/admin/enterprises/index.html.haml @@ -8,6 +8,7 @@ %li#new_product_link = button_link_to "New Enterprise", main_app.new_admin_enterprise_path, :icon => 'icon-plus', :id => 'admin_new_enterprise_link' += admin_inject_monthly_bill_description = render 'admin/shared/enterprises_sub_menu' = render :partial => 'spree/shared/error_messages', :locals => { :target => @enterprise_set } From 85f61364f8386151a0f8a94e08e8236a282e1ae5 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 28 Oct 2015 12:15:53 +1100 Subject: [PATCH 09/25] BillablePeriods use global config to calculate bills --- app/models/billable_period.rb | 16 +++-- spec/models/billable_period_spec.rb | 107 ++++++++++++++++++++++++++-- 2 files changed, 109 insertions(+), 14 deletions(-) diff --git a/app/models/billable_period.rb b/app/models/billable_period.rb index 79f4b3eaba..635a0b9a28 100644 --- a/app/models/billable_period.rb +++ b/app/models/billable_period.rb @@ -15,14 +15,16 @@ class BillablePeriod < ActiveRecord::Base end def bill - # Will make this more sophisicated in the future in that it will use global config variables to calculate + fixed = Spree::Config[:account_invoices_monthly_fixed] + rate = Spree::Config[:account_invoices_monthly_rate] + cap = Spree::Config[:account_invoices_monthly_cap] + return 0 if trial? - if ['own', 'any'].include? sells - bill = (turnover * 0.02).round(2) - bill > 50 ? 50 : bill - else - 0 - end + return 0 unless ['own', 'any'].include?(sells) + + bill = fixed + (turnover * rate).round(2) + return bill unless cap > 0 + [bill, cap].min end def label diff --git a/spec/models/billable_period_spec.rb b/spec/models/billable_period_spec.rb index 1ad974d804..b53a0bc36b 100644 --- a/spec/models/billable_period_spec.rb +++ b/spec/models/billable_period_spec.rb @@ -1,15 +1,18 @@ require 'spec_helper' -describe Customer, type: :model do +describe BillablePeriod, type: :model do + + require 'spec_helper' + describe 'ensure_correct_adjustment' do let!(:start_of_july) { Time.zone.now.beginning_of_year + 6.months } let!(:user) { create(:user) } let!(:invoice) { create(:order, user: user) } - let!(:billable_period) { create(:billable_period, owner: user, begins_at: start_of_july, ends_at: start_of_july + 12.days) } + let!(:subject) { create(:billable_period, owner: user, begins_at: start_of_july, ends_at: start_of_july + 12.days) } before do - allow(billable_period).to receive(:bill) { 99 } - allow(billable_period).to receive(:adjustment_label) { "Label for adjustment" } + allow(subject).to receive(:bill) { 99 } + allow(subject).to receive(:adjustment_label) { "Label for adjustment" } Spree::Config.set({ account_bill_inc_tax: true }) Spree::Config.set({ account_bill_tax_rate: 0.1 }) end @@ -17,11 +20,101 @@ describe Customer, type: :model do context "when no adjustment currently exists" do it "creates an adjustment on the given order" do expect(invoice.total_tax).to eq 0.0 - expect(billable_period.adjustment).to be nil - billable_period.ensure_correct_adjustment_for(invoice) - expect(billable_period.adjustment).to be_a Spree::Adjustment + expect(subject.adjustment).to be nil + subject.ensure_correct_adjustment_for(invoice) + expect(subject.adjustment).to be_a Spree::Adjustment expect(invoice.total_tax).to eq 9.0 end end end + + + describe "calculating monthly bills for enterprises" do + let!(:subject) { create(:billable_period, turnover: 100) } + + context "when a fixed cost is included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 10) } + + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.5) } + + context "when the bill is capped" do + context "at a level higher than the fixed charge plus the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 65) } + it { expect(subject.bill).to eq 60 } + end + + context "at a level lower than the fixed charge plus the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 55) } + it { expect(subject.bill).to eq 55 } + end + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 60 } + end + end + + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + context "at a level higher than the fixed charge" do + before { Spree::Config.set(:account_invoices_monthly_cap, 15) } + it { expect(subject.bill).to eq 10 } + end + + context "at a level lower than the fixed charge" do + before { Spree::Config.set(:account_invoices_monthly_cap, 5) } + it { expect(subject.bill).to eq 5 } + end + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 10 } + end + end + end + + context "when a fixed cost is not included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 0) } + + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.5) } + + context "when the bill is capped" do + context "at a level higher than the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 55) } + it { expect(subject.bill).to eq 50 } + end + + context "at a level lower than the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 45) } + it { expect(subject.bill).to eq 45 } + end + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 50 } + end + end + + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(subject.bill).to eq 0 } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 0 } + end + end + end + end end From ca3c464fda8ff8f275a9a0b3a51b1b3988345641 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 18 Nov 2015 18:07:24 +1100 Subject: [PATCH 10/25] Reorganising busines model config settings, moving account invoice tax rate in busines model config settings --- ...business_model_configuration_controller.rb | 3 +- .../business_model_configuration_helper.rb | 8 +- app/models/billable_period.rb | 13 +- .../spree/app_configuration_decorator.rb | 3 +- .../edit.html.haml | 57 +++---- .../spree/admin/tax_settings/edit.html.haml | 9 -- config/locales/en.yml | 4 +- lib/open_food_network/bill_calculator.rb | 20 +++ .../business_model_configuration_validator.rb | 3 +- ...ess_model_configuration_controller_spec.rb | 7 +- .../business_model_configuration_spec.rb | 4 + spec/features/admin/tax_settings_spec.rb | 10 +- ...usiness_model_configuration_helper_spec.rb | 142 +++++++++++++----- spec/models/billable_period_spec.rb | 3 +- 14 files changed, 177 insertions(+), 109 deletions(-) create mode 100644 lib/open_food_network/bill_calculator.rb diff --git a/app/controllers/admin/business_model_configuration_controller.rb b/app/controllers/admin/business_model_configuration_controller.rb index e421b2e597..312a2e3208 100644 --- a/app/controllers/admin/business_model_configuration_controller.rb +++ b/app/controllers/admin/business_model_configuration_controller.rb @@ -20,7 +20,8 @@ class Admin::BusinessModelConfigurationController < Spree::Admin::BaseController @settings = OpenFoodNetwork::BusinessModelConfigurationValidator.new(params[:settings] || { account_invoices_monthly_fixed: Spree::Config[:account_invoices_monthly_fixed], account_invoices_monthly_rate: Spree::Config[:account_invoices_monthly_rate], - account_invoices_monthly_cap: Spree::Config[:account_invoices_monthly_cap] + account_invoices_monthly_cap: Spree::Config[:account_invoices_monthly_cap], + account_invoices_tax_rate: Spree::Config[:account_invoices_tax_rate] }) end diff --git a/app/helpers/admin/business_model_configuration_helper.rb b/app/helpers/admin/business_model_configuration_helper.rb index 115fcf33ae..dcfebb1969 100644 --- a/app/helpers/admin/business_model_configuration_helper.rb +++ b/app/helpers/admin/business_model_configuration_helper.rb @@ -6,9 +6,9 @@ module Admin if fixed_description.empty? && rate_description.empty? t(:free).upcase elsif monthly_bill_includes_cap? && monthly_bill_includes_rate? # only care about cap if there is a rate too - "#{fixed_description}#{plus}#{rate_description}{joiner}#{cap_description} #{t(:per_month).upcase}" + "#{fixed_description}#{plus}#{rate_description}{joiner}#{cap_description} #{t(:per_month).upcase}#{tax_description.upcase}" else - "#{fixed_description}#{plus}#{rate_description} #{t(:per_month).upcase}" + "#{fixed_description}#{plus}#{rate_description} #{t(:per_month).upcase}#{tax_description.upcase}" end end @@ -29,6 +29,10 @@ module Admin monthly_bill_includes_cap? ? "#{t(:capped_at_cap, cap: cap_amount).upcase}" : "" end + def tax_description + Spree::Config[:account_invoices_tax_rate] > 0 ? ", #{t(:plus_tax).upcase}" : "" + end + def monthly_bill_includes_fixed? Spree::Config[:account_invoices_monthly_fixed] > 0 end diff --git a/app/models/billable_period.rb b/app/models/billable_period.rb index 635a0b9a28..3f7591b4de 100644 --- a/app/models/billable_period.rb +++ b/app/models/billable_period.rb @@ -15,16 +15,9 @@ class BillablePeriod < ActiveRecord::Base end def bill - fixed = Spree::Config[:account_invoices_monthly_fixed] - rate = Spree::Config[:account_invoices_monthly_rate] - cap = Spree::Config[:account_invoices_monthly_cap] - return 0 if trial? return 0 unless ['own', 'any'].include?(sells) - - bill = fixed + (turnover * rate).round(2) - return bill unless cap > 0 - [bill, cap].min + OpenFoodNetwork::BillCalculator.new(turnover: turnover).bill end def label @@ -54,8 +47,8 @@ class BillablePeriod < ActiveRecord::Base self.adjustment = invoice.adjustments.new( adjustment_attrs, :without_protection => true ) end - if Spree::Config.account_bill_inc_tax - adjustment.set_included_tax! Spree::Config.account_bill_tax_rate + if Spree::Config.account_invoices_tax_rate > 0 + adjustment.set_included_tax! Spree::Config.account_invoices_tax_rate else adjustment.set_included_tax! 0 end diff --git a/app/models/spree/app_configuration_decorator.rb b/app/models/spree/app_configuration_decorator.rb index 0ecdc0e4ca..fc7a8171cc 100644 --- a/app/models/spree/app_configuration_decorator.rb +++ b/app/models/spree/app_configuration_decorator.rb @@ -7,8 +7,6 @@ Spree::AppConfiguration.class_eval do # Tax Preferences preference :products_require_tax_category, :boolean, default: false preference :shipping_tax_rate, :decimal, default: 0 - preference :account_bill_inc_tax, :boolean, default: false - preference :account_bill_tax_rate, :decimal, default: 0 # Accounts & Billing Preferences preference :accounts_distributor_id, :integer, default: nil @@ -21,4 +19,5 @@ Spree::AppConfiguration.class_eval do preference :account_invoices_monthly_fixed, :decimal, default: 0 preference :account_invoices_monthly_rate, :decimal, default: 0 preference :account_invoices_monthly_cap, :decimal, default: 0 + preference :account_invoices_tax_rate, :decimal, default: 0 end diff --git a/app/views/admin/business_model_configuration/edit.html.haml b/app/views/admin/business_model_configuration/edit.html.haml index a00e76fa17..1eb42623ed 100644 --- a/app/views/admin/business_model_configuration/edit.html.haml +++ b/app/views/admin/business_model_configuration/edit.html.haml @@ -10,44 +10,31 @@ %legend=t(:monthly_bill_calculation_settings) = form_for @settings, as: :settings, url: main_app.admin_business_model_configuration_path, :method => :put do |f| .row - .one.column.alpha - .field - = label :nothing, "" - %br - %h6 Bill   = + .three.columns.alpha + = f.label :account_invoices_monthly_fixed, t(:fixed_charge) + %span.with-tip.icon-question-sign{'data-powertip' => "A fixed monthly charge for ALL enterprises who are set up as a shop, regardless of how much produce they sell."} .two.columns - .field - = f.label :account_invoices_monthly_fixed, t(:fixed_charge) - %span.with-tip.icon-question-sign{'data-powertip' => "A fixed monthly charge for ALL enterprises who are set up as a shop, regardless of how much produce they sell."} - %br - = f.number_field :account_invoices_monthly_fixed, min: 0.0, class: "fullwidth" - .one.column.text-center - .field - = label :nothing, "" - %br - %h6 + + = f.number_field :account_invoices_monthly_fixed, min: 0.0, class: "fullwidth" .two.columns - .field - = label :nothing, "" - %br - %h6 (  Turnover   × - .two.columns - .field - = f.label :account_invoices_monthly_rate, t(:percentage) - %span.with-tip.icon-question-sign{'data-powertip' => "When greater than zero, this rate (0.0 - 1.0) will be applied to the total turnover of each shop and added to any fixed charges (to the left) to calculate the monthly bill."} - %br - = f.number_field :account_invoices_monthly_rate, min: 0.0, max: 1.0, step: 0.01, class: "fullwidth" - .one.column - .field - = label :nothing, "" - %br - %h6 ) +   + .three.columns + = f.label :account_invoices_monthly_rate, t(:percentage_of_turnover) + %span.with-tip.icon-question-sign{'data-powertip' => "When greater than zero, this rate (0.0 - 1.0) will be applied to the total turnover of each shop and added to any fixed charges (to the left) to calculate the monthly bill."} .two.columns.omega - .field - = f.label :account_invoices_monthly_cap, t(:capped_at) - %span.with-tip.icon-question-sign{'data-powertip' => "When greater than zero, this value will be used as a cap on the amount that shops will be charged each month."} - %br - = f.number_field :account_invoices_monthly_cap, min: 0.0, class: "fullwidth" + = f.number_field :account_invoices_monthly_rate, min: 0.0, max: 1.0, step: 0.01, class: "fullwidth" + .row + .three.columns.alpha + = f.label :account_invoices_monthly_cap, t(:monthly_cap_excl_tax) + %span.with-tip.icon-question-sign{'data-powertip' => "When greater than zero, this value will be used as a cap on the amount that shops will be charged each month."} + .two.columns + = f.number_field :account_invoices_monthly_cap, min: 0.0, class: "fullwidth" + .two.columns +   + .three.columns + = f.label :account_invoices_tax_rate, t(:tax_rate) + %span.with-tip.icon-question-sign{'data-powertip' => "Tax rate that applies to the the monthly bill that enterprises are charged for using the system."} + .two.columns.omega + = f.number_field :account_invoices_tax_rate, min: 0.0, max: 1.0, step: 0.01, class: "fullwidth" .row .twelve.columns.alpha.omega.form-buttons{"data-hook" => "buttons"} diff --git a/app/views/spree/admin/tax_settings/edit.html.haml b/app/views/spree/admin/tax_settings/edit.html.haml index 102ec0609e..edd5a3b401 100644 --- a/app/views/spree/admin/tax_settings/edit.html.haml +++ b/app/views/spree/admin/tax_settings/edit.html.haml @@ -19,14 +19,5 @@ = number_field_tag "preferences[shipping_tax_rate]", Spree::Config[:shipping_tax_rate].to_f, in: 0.0..1.0, step: 0.01 = label_tag nil, t(:shipping_tax_rate) - .field.align-center{"data-hook" => "billing_tax"} - = hidden_field_tag 'preferences[account_bill_inc_tax]', '0' - = check_box_tag 'preferences[account_bill_inc_tax]', '1', Spree::Config[:account_bill_inc_tax] - = label_tag nil, t(:account_bill_inc_tax) - - .field.align-center{ "data-hook" => "account_bill_tax_rate" } - = number_field_tag "preferences[account_bill_tax_rate]", Spree::Config[:account_bill_tax_rate].to_f, in: 0.0..1.0, step: 0.01 - = label_tag nil, t(:account_bill_tax_rate) - .form-buttons{"data-hook" => "buttons"} = button t(:update), 'icon-refresh' diff --git a/config/locales/en.yml b/config/locales/en.yml index 5a95fdb567..913a9c7657 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -32,10 +32,12 @@ en: must_have_valid_business_number: "%{enterprise_name} must have a valid ABN before invoices can be sent." invoice: "Invoice" percentage_of_sales: "%{percentage} of sales" - capped_at: "capped at" + percentage_of_turnover: "Percentage of turnover" + monthly_cap_excl_tax: "monthly cap (excl. GST)" capped_at_cap: "capped at %{cap}" per_month: "per month" free: "free" + plus_tax: "plus GST" sort_order_cycles_on_shopfront_by: "Sort Order Cycles On Shopfront By" diff --git a/lib/open_food_network/bill_calculator.rb b/lib/open_food_network/bill_calculator.rb new file mode 100644 index 0000000000..4fd308c53c --- /dev/null +++ b/lib/open_food_network/bill_calculator.rb @@ -0,0 +1,20 @@ +module OpenFoodNetwork + class BulkCoopReport + attr_accessor :turnover, :fixed, :rate, :cap, :trial + + def initialize(opts={}) + @turnover = opts[:turnover] || 0 + @fixed = opts[:fixed] || Spree::Config[:account_invoices_monthly_fixed] + @rate = opts[:rate] || Spree::Config[:account_invoices_monthly_rate] + @cap = opts[:cap] || Spree::Config[:account_invoices_monthly_cap] + @tax_rate = Spree::Config[:account_bill_tax_rate] + end + + def bill + bill = fixed + (turnover * rate) + bill = bill * (1 + tax_rate) + return bill unless cap > 0 + [bill, cap].min + end + end +end diff --git a/lib/open_food_network/business_model_configuration_validator.rb b/lib/open_food_network/business_model_configuration_validator.rb index 292894daea..d83d94ffc1 100644 --- a/lib/open_food_network/business_model_configuration_validator.rb +++ b/lib/open_food_network/business_model_configuration_validator.rb @@ -5,11 +5,12 @@ module OpenFoodNetwork class BusinessModelConfigurationValidator include ActiveModel::Validations - attr_accessor :account_invoices_monthly_fixed, :account_invoices_monthly_rate, :account_invoices_monthly_cap + attr_accessor :account_invoices_monthly_fixed, :account_invoices_monthly_rate, :account_invoices_monthly_cap, :account_invoices_tax_rate validates :account_invoices_monthly_fixed, presence: true, numericality: { greater_than_or_equal_to: 0 } validates :account_invoices_monthly_rate, presence: true, numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 1 } validates :account_invoices_monthly_cap, presence: true, numericality: { greater_than_or_equal_to: 0 } + validates :account_invoices_tax_rate, presence: true, numericality: { greater_than_or_equal_to: 0, less_than_or_equal_to: 1 } def initialize(attr, button=nil) attr.each { |k,v| instance_variable_set("@#{k}", v) } diff --git a/spec/controllers/admin/business_model_configuration_controller_spec.rb b/spec/controllers/admin/business_model_configuration_controller_spec.rb index 7aed5cfbf7..3418408417 100644 --- a/spec/controllers/admin/business_model_configuration_controller_spec.rb +++ b/spec/controllers/admin/business_model_configuration_controller_spec.rb @@ -8,7 +8,8 @@ describe Admin::BusinessModelConfigurationController, type: :controller do Spree::Config.set({ account_invoices_monthly_fixed: 5, account_invoices_monthly_rate: 0.02, - account_invoices_monthly_cap: 50 + account_invoices_monthly_cap: 50, + account_invoices_tax_rate: 0.1 }) end @@ -51,6 +52,7 @@ describe Admin::BusinessModelConfigurationController, type: :controller do params[:settings][:account_invoices_monthly_fixed] = '' params[:settings][:account_invoices_monthly_rate] = '2' params[:settings][:account_invoices_monthly_cap] = '-1' + params[:settings][:account_invoices_tax_rate] = '4' spree_get :update, params end @@ -60,6 +62,7 @@ describe Admin::BusinessModelConfigurationController, type: :controller do expect(Spree::Config.account_invoices_monthly_fixed).to eq 5 expect(Spree::Config.account_invoices_monthly_rate).to eq 0.02 expect(Spree::Config.account_invoices_monthly_cap).to eq 50 + expect(Spree::Config.account_invoices_tax_rate).to eq 0.1 end end @@ -68,6 +71,7 @@ describe Admin::BusinessModelConfigurationController, type: :controller do params[:settings][:account_invoices_monthly_fixed] = '10' params[:settings][:account_invoices_monthly_rate] = '0.05' params[:settings][:account_invoices_monthly_cap] = '30' + params[:settings][:account_invoices_tax_rate] = '0.15' end it "sets global config to the specified values" do @@ -76,6 +80,7 @@ describe Admin::BusinessModelConfigurationController, type: :controller do expect(Spree::Config.account_invoices_monthly_fixed).to eq 10 expect(Spree::Config.account_invoices_monthly_rate).to eq 0.05 expect(Spree::Config.account_invoices_monthly_cap).to eq 30 + expect(Spree::Config.account_invoices_tax_rate).to eq 0.15 end end end diff --git a/spec/features/admin/business_model_configuration_spec.rb b/spec/features/admin/business_model_configuration_spec.rb index dbcaf7e8bc..61da09293f 100644 --- a/spec/features/admin/business_model_configuration_spec.rb +++ b/spec/features/admin/business_model_configuration_spec.rb @@ -12,6 +12,7 @@ feature 'Business Model Configuration' do account_invoices_monthly_fixed: 5, account_invoices_monthly_rate: 0.02, account_invoices_monthly_cap: 50 + account_invoices_tax_rate: 0.1 }) end @@ -28,6 +29,7 @@ feature 'Business Model Configuration' do expect(page).to have_field "settings_account_invoices_monthly_fixed", with: 5.0 expect(page).to have_field "settings_account_invoices_monthly_rate", with: 0.02 expect(page).to have_field "settings_account_invoices_monthly_cap", with: 50.0 + expect(page).to have_field "settings_account_invoices_tax_rate", with: 0.1 end it "attributes can be changed", js: true do @@ -36,12 +38,14 @@ feature 'Business Model Configuration' do fill_in "settings_account_invoices_monthly_fixed", with: 10 fill_in "settings_account_invoices_monthly_rate", with: 0.05 fill_in "settings_account_invoices_monthly_cap", with: 30 + fill_in "settings_account_invoices_tax_rate", with: 0.15 click_button "Update" expect(Spree::Config.account_invoices_monthly_fixed).to eq 10 expect(Spree::Config.account_invoices_monthly_rate).to eq 0.05 expect(Spree::Config.account_invoices_monthly_cap).to eq 30 + expect(Spree::Config.settings_account_invoices_tax_rate).to eq 0.15 end end end diff --git a/spec/features/admin/tax_settings_spec.rb b/spec/features/admin/tax_settings_spec.rb index 9894ee8a72..4b2dd4486c 100644 --- a/spec/features/admin/tax_settings_spec.rb +++ b/spec/features/admin/tax_settings_spec.rb @@ -11,9 +11,7 @@ feature 'Account and Billing Settings' do Spree::Config.set({ products_require_tax_category: false, shipment_inc_vat: false, - shipping_tax_rate: 0, - account_bill_inc_tax: false, - account_bill_tax_rate: 0 + shipping_tax_rate: 0 }) end @@ -30,8 +28,6 @@ feature 'Account and Billing Settings' do expect(page).to have_unchecked_field 'preferences_products_require_tax_category' expect(page).to have_unchecked_field 'preferences_shipment_inc_vat' expect(page).to have_field 'preferences_shipping_tax_rate' - expect(page).to have_unchecked_field 'preferences_account_bill_inc_tax' - expect(page).to have_field 'preferences_account_bill_tax_rate' end it "attributes can be changed" do @@ -40,16 +36,12 @@ feature 'Account and Billing Settings' do check 'preferences_products_require_tax_category' check 'preferences_shipment_inc_vat' fill_in 'preferences_shipping_tax_rate', with: '0.12' - check 'preferences_account_bill_inc_tax' - fill_in 'preferences_account_bill_tax_rate', with: '0.05' click_button "Update" expect(Spree::Config.products_require_tax_category).to be true expect(Spree::Config.shipment_inc_vat).to be true expect(Spree::Config.shipping_tax_rate).to eq 0.12 - expect(Spree::Config.account_bill_inc_tax).to be true - expect(Spree::Config.account_bill_tax_rate).to eq 0.05 end end end diff --git a/spec/helpers/admin/business_model_configuration_helper_spec.rb b/spec/helpers/admin/business_model_configuration_helper_spec.rb index 0c44d5c742..a10771cba7 100644 --- a/spec/helpers/admin/business_model_configuration_helper_spec.rb +++ b/spec/helpers/admin/business_model_configuration_helper_spec.rb @@ -2,66 +2,136 @@ require 'spec_helper' describe Admin::BusinessModelConfigurationHelper do describe "describing monthly bills for enterprises" do - context "when a fixed cost is included" do - before { Spree::Config.set(:account_invoices_monthly_fixed, 10) } + context "when tax is applied to the service change" do + before { Spree::Config.set(:account_invoices_tax_rate, 0.1) } + context "when a fixed cost is included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 10) } - context "when a percentage of turnover is included" do - before { Spree::Config.set(:account_invoices_monthly_rate, 0.05) } + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.05) } - context "when the bill is capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 20) } - it { expect(helper.monthly_bill_description).to eq "$10 + 5.0% OF SALES{joiner}CAPPED AT $20 PER MONTH" } + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "$10 + 5.0% OF SALES{joiner}CAPPED AT $20 PER MONTH, PLUS GST" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "$10 + 5.0% OF SALES PER MONTH, PLUS GST" } + end end - context "when the bill is not capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 0) } - it { expect(helper.monthly_bill_description).to eq "$10 + 5.0% OF SALES PER MONTH" } + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "$10 PER MONTH, PLUS GST" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "$10 PER MONTH, PLUS GST" } + end end end - context "when a percentage of turnover is not included" do - before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + context "when a fixed cost is not included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 0) } - context "when the bill is capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 20) } - it { expect(helper.monthly_bill_description).to eq "$10 PER MONTH" } + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.05) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "5.0% OF SALES{joiner}CAPPED AT $20 PER MONTH, PLUS GST" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "5.0% OF SALES PER MONTH, PLUS GST" } + end end - context "when the bill is not capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 0) } - it { expect(helper.monthly_bill_description).to eq "$10 PER MONTH" } + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "FREE" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "FREE" } + end end end end - context "when a fixed cost is not included" do - before { Spree::Config.set(:account_invoices_monthly_fixed, 0) } + context "when tax is applied to the service change" do + before { Spree::Config.set(:account_invoices_tax_rate, 0.0) } + context "when a fixed cost is included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 10) } - context "when a percentage of turnover is included" do - before { Spree::Config.set(:account_invoices_monthly_rate, 0.05) } + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.05) } - context "when the bill is capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 20) } - it { expect(helper.monthly_bill_description).to eq "5.0% OF SALES{joiner}CAPPED AT $20 PER MONTH" } + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "$10 + 5.0% OF SALES{joiner}CAPPED AT $20 PER MONTH" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "$10 + 5.0% OF SALES PER MONTH" } + end end - context "when the bill is not capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 0) } - it { expect(helper.monthly_bill_description).to eq "5.0% OF SALES PER MONTH" } + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "$10 PER MONTH" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "$10 PER MONTH" } + end end end - context "when a percentage of turnover is not included" do - before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + context "when a fixed cost is not included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 0) } - context "when the bill is capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 20) } - it { expect(helper.monthly_bill_description).to eq "FREE" } + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.05) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "5.0% OF SALES{joiner}CAPPED AT $20 PER MONTH" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "5.0% OF SALES PER MONTH" } + end end - context "when the bill is not capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 0) } - it { expect(helper.monthly_bill_description).to eq "FREE" } + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(helper.monthly_bill_description).to eq "FREE" } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(helper.monthly_bill_description).to eq "FREE" } + end end end end diff --git a/spec/models/billable_period_spec.rb b/spec/models/billable_period_spec.rb index b53a0bc36b..5a12ce98e3 100644 --- a/spec/models/billable_period_spec.rb +++ b/spec/models/billable_period_spec.rb @@ -13,8 +13,7 @@ describe BillablePeriod, type: :model do before do allow(subject).to receive(:bill) { 99 } allow(subject).to receive(:adjustment_label) { "Label for adjustment" } - Spree::Config.set({ account_bill_inc_tax: true }) - Spree::Config.set({ account_bill_tax_rate: 0.1 }) + Spree::Config.set({ account_invoices_tax_rate: 0.1 }) end context "when no adjustment currently exists" do From 76d4f74f6b46aed2735a34927f685ab9b9464bac Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 19 Nov 2015 08:25:22 +1100 Subject: [PATCH 11/25] Ammending BillablePeriod spec to be ensure tax is charged correctly --- app/models/billable_period.rb | 2 + lib/open_food_network/bill_calculator.rb | 6 +- spec/models/billable_period_spec.rb | 189 +++++++++++++++++------ 3 files changed, 146 insertions(+), 51 deletions(-) diff --git a/app/models/billable_period.rb b/app/models/billable_period.rb index 3f7591b4de..062f70dde8 100644 --- a/app/models/billable_period.rb +++ b/app/models/billable_period.rb @@ -1,3 +1,5 @@ +require 'open_food_network/bill_calculator' + class BillablePeriod < ActiveRecord::Base belongs_to :enterprise belongs_to :owner, class_name: 'Spree::User' diff --git a/lib/open_food_network/bill_calculator.rb b/lib/open_food_network/bill_calculator.rb index 4fd308c53c..bf0c9456c3 100644 --- a/lib/open_food_network/bill_calculator.rb +++ b/lib/open_food_network/bill_calculator.rb @@ -1,13 +1,13 @@ module OpenFoodNetwork - class BulkCoopReport - attr_accessor :turnover, :fixed, :rate, :cap, :trial + class BillCalculator + attr_accessor :turnover, :fixed, :rate, :cap, :tax_rate def initialize(opts={}) @turnover = opts[:turnover] || 0 @fixed = opts[:fixed] || Spree::Config[:account_invoices_monthly_fixed] @rate = opts[:rate] || Spree::Config[:account_invoices_monthly_rate] @cap = opts[:cap] || Spree::Config[:account_invoices_monthly_cap] - @tax_rate = Spree::Config[:account_bill_tax_rate] + @tax_rate = opts[:tax_rate] || Spree::Config[:account_invoices_tax_rate] end def bill diff --git a/spec/models/billable_period_spec.rb b/spec/models/billable_period_spec.rb index 5a12ce98e3..3e0fb5dbdf 100644 --- a/spec/models/billable_period_spec.rb +++ b/spec/models/billable_period_spec.rb @@ -31,87 +31,180 @@ describe BillablePeriod, type: :model do describe "calculating monthly bills for enterprises" do let!(:subject) { create(:billable_period, turnover: 100) } - context "when a fixed cost is included" do - before { Spree::Config.set(:account_invoices_monthly_fixed, 10) } + context "when no tax is charged" do + before { Spree::Config.set(:account_invoices_tax_rate, 0) } - context "when a percentage of turnover is included" do - before { Spree::Config.set(:account_invoices_monthly_rate, 0.5) } + context "when a fixed cost is included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 10) } - context "when the bill is capped" do - context "at a level higher than the fixed charge plus the product of the rate and turnover" do - before { Spree::Config.set(:account_invoices_monthly_cap, 65) } - it { expect(subject.bill).to eq 60 } + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.5) } + + context "when the bill is capped" do + context "at a level higher than the fixed charge plus the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 65) } + it { expect(subject.bill).to eq 60 } + end + + context "at a level lower than the fixed charge plus the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 55) } + it { expect(subject.bill).to eq 55 } + end end - context "at a level lower than the fixed charge plus the product of the rate and turnover" do - before { Spree::Config.set(:account_invoices_monthly_cap, 55) } - it { expect(subject.bill).to eq 55 } + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 60 } end end - context "when the bill is not capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 0) } - it { expect(subject.bill).to eq 60 } + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + context "at a level higher than the fixed charge" do + before { Spree::Config.set(:account_invoices_monthly_cap, 15) } + it { expect(subject.bill).to eq 10 } + end + + context "at a level lower than the fixed charge" do + before { Spree::Config.set(:account_invoices_monthly_cap, 5) } + it { expect(subject.bill).to eq 5 } + end + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 10 } + end end end - context "when a percentage of turnover is not included" do - before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + context "when a fixed cost is not included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 0) } - context "when the bill is capped" do - context "at a level higher than the fixed charge" do - before { Spree::Config.set(:account_invoices_monthly_cap, 15) } - it { expect(subject.bill).to eq 10 } + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.5) } + + context "when the bill is capped" do + context "at a level higher than the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 55) } + it { expect(subject.bill).to eq 50 } + end + + context "at a level lower than the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 45) } + it { expect(subject.bill).to eq 45 } + end end - context "at a level lower than the fixed charge" do - before { Spree::Config.set(:account_invoices_monthly_cap, 5) } - it { expect(subject.bill).to eq 5 } + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 50 } end end - context "when the bill is not capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 0) } - it { expect(subject.bill).to eq 10 } + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(subject.bill).to eq 0 } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 0 } + end end end end - context "when a fixed cost is not included" do - before { Spree::Config.set(:account_invoices_monthly_fixed, 0) } + context "when tax is charged" do + before { Spree::Config.set(:account_invoices_tax_rate, 0.1) } - context "when a percentage of turnover is included" do - before { Spree::Config.set(:account_invoices_monthly_rate, 0.5) } + context "when a fixed cost is included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 10) } - context "when the bill is capped" do - context "at a level higher than the product of the rate and turnover" do - before { Spree::Config.set(:account_invoices_monthly_cap, 55) } - it { expect(subject.bill).to eq 50 } + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.5) } + + context "when the bill is capped" do + context "at a level higher than the fixed charge plus the product of the rate and turnover plus tax" do + before { Spree::Config.set(:account_invoices_monthly_cap, 67) } + it { expect(subject.bill).to eq 66 } + end + + context "at a level lower than the fixed charge plus the product of the rate and turnover plus tax" do + before { Spree::Config.set(:account_invoices_monthly_cap, 65) } + it { expect(subject.bill).to eq 65 } + end end - context "at a level lower than the product of the rate and turnover" do - before { Spree::Config.set(:account_invoices_monthly_cap, 45) } - it { expect(subject.bill).to eq 45 } + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 66 } end end - context "when the bill is not capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 0) } - it { expect(subject.bill).to eq 50 } + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + context "at a level higher than the fixed charge plus tax" do + before { Spree::Config.set(:account_invoices_monthly_cap, 12) } + it { expect(subject.bill).to eq 11 } + end + + context "at a level lower than the fixed charge plus tax" do + before { Spree::Config.set(:account_invoices_monthly_cap, 10) } + it { expect(subject.bill).to eq 10 } + end + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 11 } + end end end - context "when a percentage of turnover is not included" do - before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + context "when a fixed cost is not included" do + before { Spree::Config.set(:account_invoices_monthly_fixed, 0) } - context "when the bill is capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 20) } - it { expect(subject.bill).to eq 0 } + context "when a percentage of turnover is included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0.5) } + + context "when the bill is capped" do + context "at a level higher than the product of the rate and turnover plus tax" do + before { Spree::Config.set(:account_invoices_monthly_cap, 56) } + it { expect(subject.bill).to eq 55 } + end + + context "at a level lower than the product of the rate and turnover plus_tax" do + before { Spree::Config.set(:account_invoices_monthly_cap, 54) } + it { expect(subject.bill).to eq 54 } + end + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 55 } + end end - context "when the bill is not capped" do - before { Spree::Config.set(:account_invoices_monthly_cap, 0) } - it { expect(subject.bill).to eq 0 } + context "when a percentage of turnover is not included" do + before { Spree::Config.set(:account_invoices_monthly_rate, 0) } + + context "when the bill is capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 20) } + it { expect(subject.bill).to eq 0 } + end + + context "when the bill is not capped" do + before { Spree::Config.set(:account_invoices_monthly_cap, 0) } + it { expect(subject.bill).to eq 0 } + end end end end From 0ed8cf973d372c3b765e59ff7603485bbbdb7f41 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 19 Nov 2015 08:37:51 +1100 Subject: [PATCH 12/25] Adding an example bill calculator to business model config edit page To allow super admin to see the effects of any changes they make to BMC settings --- .../accounts_and_billing_settings.js.coffee | 2 +- app/assets/javascripts/admin/all.js | 1 + .../business_model_configuration.js.coffee | 1 + ...s_model_configuration_controller.js.coffee | 21 ++++ .../directives/watchValueAs.js.coffee | 2 +- app/assets/stylesheets/admin/orders.css.scss | 18 +++ .../edit.html.haml | 107 ++++++++++++------ config/locales/en.yml | 1 + 8 files changed, 119 insertions(+), 34 deletions(-) create mode 100644 app/assets/javascripts/admin/business_model_configuration/business_model_configuration.js.coffee create mode 100644 app/assets/javascripts/admin/business_model_configuration/controllers/business_model_configuration_controller.js.coffee rename app/assets/javascripts/admin/{accounts_and_billing_settings => utils}/directives/watchValueAs.js.coffee (72%) diff --git a/app/assets/javascripts/admin/accounts_and_billing_settings/accounts_and_billing_settings.js.coffee b/app/assets/javascripts/admin/accounts_and_billing_settings/accounts_and_billing_settings.js.coffee index d4f544e300..06ee4fa4ef 100644 --- a/app/assets/javascripts/admin/accounts_and_billing_settings/accounts_and_billing_settings.js.coffee +++ b/app/assets/javascripts/admin/accounts_and_billing_settings/accounts_and_billing_settings.js.coffee @@ -1 +1 @@ -angular.module("admin.accounts_and_billing_settings", []) +angular.module("admin.accounts_and_billing_settings", ["admin.utils"]) diff --git a/app/assets/javascripts/admin/all.js b/app/assets/javascripts/admin/all.js index 30c8e1a7b9..c0fa530626 100644 --- a/app/assets/javascripts/admin/all.js +++ b/app/assets/javascripts/admin/all.js @@ -23,6 +23,7 @@ //= require_tree ../templates/admin //= require ./admin //= require ./accounts_and_billing_settings/accounts_and_billing_settings +//= require ./business_model_configuration/business_model_configuration //= require ./customers/customers //= require ./dropdown/dropdown //= require ./enterprises/enterprises diff --git a/app/assets/javascripts/admin/business_model_configuration/business_model_configuration.js.coffee b/app/assets/javascripts/admin/business_model_configuration/business_model_configuration.js.coffee new file mode 100644 index 0000000000..cecb7c397e --- /dev/null +++ b/app/assets/javascripts/admin/business_model_configuration/business_model_configuration.js.coffee @@ -0,0 +1 @@ +angular.module("admin.businessModelConfiguration", ["admin.utils"]) diff --git a/app/assets/javascripts/admin/business_model_configuration/controllers/business_model_configuration_controller.js.coffee b/app/assets/javascripts/admin/business_model_configuration/controllers/business_model_configuration_controller.js.coffee new file mode 100644 index 0000000000..ca757c673d --- /dev/null +++ b/app/assets/javascripts/admin/business_model_configuration/controllers/business_model_configuration_controller.js.coffee @@ -0,0 +1,21 @@ +angular.module("admin.businessModelConfiguration").controller "BusinessModelConfigCtrl", ($scope, $filter) -> + $scope.turnover = 1000 + + $scope.bill = -> + return $filter('currency')(0) unless $scope.fixed || $scope.rate + Number($scope.fixed) + Number($scope.turnover) * Number($scope.rate) + + $scope.cappedBill = -> + return $scope.bill() if !$scope.cap? || Number($scope.cap) == 0 + Math.min($scope.bill(), Number($scope.cap)) + + $scope.capReached = -> + return "No" if !$scope.cap? || Number($scope.cap) == 0 + if $scope.bill() >= Number($scope.cap) then "Yes" else "No" + + $scope.includedTax = -> + return 0 if !$scope.taxRate? || Number($scope.taxRate) == 0 + ($scope.cappedBill() * Number($scope.taxRate)) + + $scope.total = -> + $scope.cappedBill() + $scope.includedTax() diff --git a/app/assets/javascripts/admin/accounts_and_billing_settings/directives/watchValueAs.js.coffee b/app/assets/javascripts/admin/utils/directives/watchValueAs.js.coffee similarity index 72% rename from app/assets/javascripts/admin/accounts_and_billing_settings/directives/watchValueAs.js.coffee rename to app/assets/javascripts/admin/utils/directives/watchValueAs.js.coffee index a14288db55..701858cd56 100644 --- a/app/assets/javascripts/admin/accounts_and_billing_settings/directives/watchValueAs.js.coffee +++ b/app/assets/javascripts/admin/utils/directives/watchValueAs.js.coffee @@ -1,4 +1,4 @@ -angular.module("admin.accounts_and_billing_settings").directive "watchValueAs", -> +angular.module("admin.utils").directive "watchValueAs", -> restrict: 'A' scope: { value: "=watchValueAs" diff --git a/app/assets/stylesheets/admin/orders.css.scss b/app/assets/stylesheets/admin/orders.css.scss index d91bb28934..4676d93b1a 100644 --- a/app/assets/stylesheets/admin/orders.css.scss +++ b/app/assets/stylesheets/admin/orders.css.scss @@ -56,3 +56,21 @@ div#group_buy_calculation { text-align: center; } } + +.input-symbol { + position: relative; + &.before { + + span { + position: absolute; + transform: translate(0,-50%); + top:50%; + pointer-events:none; + margin-left: 1em; + } + + input { + text-indent:1em; + } + } +} diff --git a/app/views/admin/business_model_configuration/edit.html.haml b/app/views/admin/business_model_configuration/edit.html.haml index 1eb42623ed..09a5949456 100644 --- a/app/views/admin/business_model_configuration/edit.html.haml +++ b/app/views/admin/business_model_configuration/edit.html.haml @@ -6,36 +6,79 @@ = render 'spree/shared/error_messages', target: @settings -%fieldset.no-border-bottom - %legend=t(:monthly_bill_calculation_settings) - = form_for @settings, as: :settings, url: main_app.admin_business_model_configuration_path, :method => :put do |f| - .row - .three.columns.alpha - = f.label :account_invoices_monthly_fixed, t(:fixed_charge) - %span.with-tip.icon-question-sign{'data-powertip' => "A fixed monthly charge for ALL enterprises who are set up as a shop, regardless of how much produce they sell."} - .two.columns - = f.number_field :account_invoices_monthly_fixed, min: 0.0, class: "fullwidth" - .two.columns -   - .three.columns - = f.label :account_invoices_monthly_rate, t(:percentage_of_turnover) - %span.with-tip.icon-question-sign{'data-powertip' => "When greater than zero, this rate (0.0 - 1.0) will be applied to the total turnover of each shop and added to any fixed charges (to the left) to calculate the monthly bill."} - .two.columns.omega - = f.number_field :account_invoices_monthly_rate, min: 0.0, max: 1.0, step: 0.01, class: "fullwidth" - .row - .three.columns.alpha - = f.label :account_invoices_monthly_cap, t(:monthly_cap_excl_tax) - %span.with-tip.icon-question-sign{'data-powertip' => "When greater than zero, this value will be used as a cap on the amount that shops will be charged each month."} - .two.columns - = f.number_field :account_invoices_monthly_cap, min: 0.0, class: "fullwidth" - .two.columns -   - .three.columns - = f.label :account_invoices_tax_rate, t(:tax_rate) - %span.with-tip.icon-question-sign{'data-powertip' => "Tax rate that applies to the the monthly bill that enterprises are charged for using the system."} - .two.columns.omega - = f.number_field :account_invoices_tax_rate, min: 0.0, max: 1.0, step: 0.01, class: "fullwidth" +.row{ ng: { app: 'admin.businessModelConfiguration', controller: "BusinessModelConfigCtrl" } } + .five.columns.omega + %fieldset.no-border-bottom + %legend=t(:bill_calculation_settings) + %p + Adjust the amount that enterprises will be billed each month for use of the OFN. + %br + = form_for @settings, as: :settings, url: main_app.admin_business_model_configuration_path, :method => :put do |f| + .row + .three.columns.alpha + = f.label :account_invoices_monthly_fixed, t(:fixed_monthly_charge) + %span.with-tip.icon-question-sign{'data-powertip' => "A fixed monthly charge for ALL enterprises who are set up as a shop, regardless of how much produce they sell."} + .two.columns.omega + .input-symbol.before + %span= Spree::Money.currency_symbol + = f.number_field :account_invoices_monthly_fixed, min: 0.0, class: "fullwidth", 'watch-value-as' => 'fixed' + .row + .three.columns.alpha + = f.label :account_invoices_monthly_rate, t(:percentage_of_turnover) + %span.with-tip.icon-question-sign{'data-powertip' => "When greater than zero, this rate (0.0 - 1.0) will be applied to the total turnover of each shop and added to any fixed charges (to the left) to calculate the monthly bill."} + .two.columns.omega + = f.number_field :account_invoices_monthly_rate, min: 0.0, max: 1.0, step: 0.01, class: "fullwidth", 'watch-value-as' => 'rate' + .row + .three.columns.alpha + = f.label :account_invoices_monthly_cap, t(:monthly_cap_excl_tax) + %span.with-tip.icon-question-sign{'data-powertip' => "When greater than zero, this value will be used as a cap on the amount that shops will be charged each month."} + .two.columns.omega + .input-symbol.before + %span= Spree::Money.currency_symbol + = f.number_field :account_invoices_monthly_cap, min: 0.0, class: "fullwidth", 'watch-value-as' => 'cap' + .row + .three.columns.alpha + = f.label :account_invoices_tax_rate, t(:tax_rate) + %span.with-tip.icon-question-sign{'data-powertip' => "Tax rate that applies to the the monthly bill that enterprises are charged for using the system."} + .two.columns.omega + = f.number_field :account_invoices_tax_rate, min: 0.0, max: 1.0, step: 0.01, class: "fullwidth", 'watch-value-as' => 'taxRate' - .row - .twelve.columns.alpha.omega.form-buttons{"data-hook" => "buttons"} - = button t(:update), 'icon-refresh', value: "update" + .row + .five.columns.alpha.omega.form-buttons{"data-hook" => "buttons"} + = button t(:update), 'icon-refresh', value: "update" + + .two.columns +   + + .five.columns.alpha + %fieldset.no-border-bottom + %legend=t(:example_bill_calculator) + %p + Alter the example turnover to visualise the effect of the settings to the left. + %br + .row + .three.columns.alpha + = label_tag :turnover, t(:example_monthly_turnover) + %span.with-tip.icon-question-sign{'data-powertip' => "An example monthly turnover for an enterprise which will be used to generate calculate an example monthly bill below."} + .two.columns.omega + .input-symbol.before + %span= Spree::Money.currency_symbol + %input.fullwidth{ id: 'turnover', type: "number", ng: { model: 'turnover' } } + .row + .three.columns.alpha + = label_tag :cap_reached, t(:cap_reached?) + %span.with-tip.icon-question-sign{'data-powertip' => "Whether the cap (specified to the left) has been reached, given the settings and the turnover provided."} + .two.columns.omega + %input.fullwidth{ id: 'cap_reached', type: "text", readonly: true, ng: { value: 'capReached()' } } + .row + .three.columns.alpha + = label_tag :included_tax, t(:included_tax) + %span.with-tip.icon-question-sign{'data-powertip' => "The total tax included in the example monthly bill, given the settings and the turnover provided."} + .two.columns.omega + %input.fullwidth{ id: 'included_tax', type: "text", readonly: true, ng: { value: 'includedTax() | currency' } } + .row + .three.columns.alpha + = label_tag :total_incl_tax, t(:total_monthly_bill_incl_tax) + %span.with-tip.icon-question-sign{'data-powertip' => "The example total monthly bill with tax included, given the settings and the turnover provided."} + .two.columns.omega + %input.fullwidth{ id: 'total_incl_tax', type: "text", readonly: true, ng: { value: 'total() | currency' } } diff --git a/config/locales/en.yml b/config/locales/en.yml index 913a9c7657..c9df13af92 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -38,6 +38,7 @@ en: per_month: "per month" free: "free" plus_tax: "plus GST" + total_monthly_bill_incl_tax: "Total Monthly Bill (Incl. Tax)" sort_order_cycles_on_shopfront_by: "Sort Order Cycles On Shopfront By" From 199a3c38f9f62604b1119c1727ede3e4981815b3 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 19 Nov 2015 10:45:36 +1100 Subject: [PATCH 13/25] Bill is capped before tax is applied, like we say it is --- lib/open_food_network/bill_calculator.rb | 5 ++-- spec/models/billable_period_spec.rb | 32 +++++++++++++----------- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/lib/open_food_network/bill_calculator.rb b/lib/open_food_network/bill_calculator.rb index bf0c9456c3..84457d6530 100644 --- a/lib/open_food_network/bill_calculator.rb +++ b/lib/open_food_network/bill_calculator.rb @@ -12,9 +12,8 @@ module OpenFoodNetwork def bill bill = fixed + (turnover * rate) - bill = bill * (1 + tax_rate) - return bill unless cap > 0 - [bill, cap].min + bill = cap > 0 ? [bill, cap].min : bill + bill * (1 + tax_rate) end end end diff --git a/spec/models/billable_period_spec.rb b/spec/models/billable_period_spec.rb index 3e0fb5dbdf..3037ebc6bc 100644 --- a/spec/models/billable_period_spec.rb +++ b/spec/models/billable_period_spec.rb @@ -130,14 +130,16 @@ describe BillablePeriod, type: :model do before { Spree::Config.set(:account_invoices_monthly_rate, 0.5) } context "when the bill is capped" do - context "at a level higher than the fixed charge plus the product of the rate and turnover plus tax" do - before { Spree::Config.set(:account_invoices_monthly_cap, 67) } + context "at a level higher than the fixed charge plus the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 61) } it { expect(subject.bill).to eq 66 } end - context "at a level lower than the fixed charge plus the product of the rate and turnover plus tax" do - before { Spree::Config.set(:account_invoices_monthly_cap, 65) } - it { expect(subject.bill).to eq 65 } + context "at a level lower than the fixed charge plus the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 59) } + it { + expect(subject.bill.to_f).to eq 64.9 + } end end @@ -151,14 +153,14 @@ describe BillablePeriod, type: :model do before { Spree::Config.set(:account_invoices_monthly_rate, 0) } context "when the bill is capped" do - context "at a level higher than the fixed charge plus tax" do - before { Spree::Config.set(:account_invoices_monthly_cap, 12) } + context "at a level higher than the fixed charge" do + before { Spree::Config.set(:account_invoices_monthly_cap, 11) } it { expect(subject.bill).to eq 11 } end - context "at a level lower than the fixed charge plus tax" do - before { Spree::Config.set(:account_invoices_monthly_cap, 10) } - it { expect(subject.bill).to eq 10 } + context "at a level lower than the fixed charge" do + before { Spree::Config.set(:account_invoices_monthly_cap, 9) } + it { expect(subject.bill.to_f).to eq 9.9 } end end @@ -176,14 +178,14 @@ describe BillablePeriod, type: :model do before { Spree::Config.set(:account_invoices_monthly_rate, 0.5) } context "when the bill is capped" do - context "at a level higher than the product of the rate and turnover plus tax" do - before { Spree::Config.set(:account_invoices_monthly_cap, 56) } + context "at a level higher than the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 51) } it { expect(subject.bill).to eq 55 } end - context "at a level lower than the product of the rate and turnover plus_tax" do - before { Spree::Config.set(:account_invoices_monthly_cap, 54) } - it { expect(subject.bill).to eq 54 } + context "at a level lower than the product of the rate and turnover" do + before { Spree::Config.set(:account_invoices_monthly_cap, 49) } + it { expect(subject.bill.to_f).to eq 53.9 } end end From d46712de84b659eff3d75e2dcc154d52dcdc52ab Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 19 Nov 2015 11:27:20 +1100 Subject: [PATCH 14/25] Obsolete BillablePeriods only deleted if their associated order is not already complete --- app/jobs/update_billable_periods.rb | 8 +++++--- spec/jobs/update_billable_periods_spec.rb | 3 +++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/jobs/update_billable_periods.rb b/app/jobs/update_billable_periods.rb index 598d5f8e7b..d775b093f3 100644 --- a/app/jobs/update_billable_periods.rb +++ b/app/jobs/update_billable_periods.rb @@ -91,10 +91,10 @@ class UpdateBillablePeriods def clean_up_untouched_billable_periods_for(enterprise, job_start_time) # Snag and then delete any BillablePeriods which overlap - obsolete_billable_periods = enterprise.billable_periods.where('ends_at > (?) AND begins_at < (?) AND updated_at < (?)', start_date, end_date, job_start_time) + obsolete_billable_periods = enterprise.billable_periods.where('ends_at > (?) AND begins_at < (?) AND billable_periods.updated_at < (?)', start_date, end_date, job_start_time) if obsolete_billable_periods.any? - current_billable_periods = enterprise.billable_periods.where('ends_at >= (?) AND begins_at <= (?) AND updated_at > (?)', start_date, end_date, job_start_time) + current_billable_periods = enterprise.billable_periods.where('ends_at >= (?) AND begins_at <= (?) AND billable_periods.updated_at > (?)', start_date, end_date, job_start_time) Delayed::Worker.logger.info "#{enterprise.name} #{start_date.strftime("%F %T")} #{job_start_time.strftime("%F %T")}" Delayed::Worker.logger.info "#{obsolete_billable_periods.first.updated_at.strftime("%F %T")}" @@ -105,7 +105,9 @@ class UpdateBillablePeriods }) end - obsolete_billable_periods.each(&:delete) + obsolete_billable_periods.includes({ account_invoice: :order}). + where('spree_orders.state <> \'complete\' OR account_invoices.order_id IS NULL'). + each(&:delete) end private diff --git a/spec/jobs/update_billable_periods_spec.rb b/spec/jobs/update_billable_periods_spec.rb index bfdbfa8434..d868550e7e 100644 --- a/spec/jobs/update_billable_periods_spec.rb +++ b/spec/jobs/update_billable_periods_spec.rb @@ -524,6 +524,8 @@ describe UpdateBillablePeriods do let!(:bp6) { create(:billable_period, enterprise: enterprise, updated_at: job_start_time - 5.seconds, begins_at: start_of_july - 10.days, ends_at: start_of_july - 5.days ) } # Updated before start but ends at start_date (ie. not after start_date, so should be ignored) EDGE CASE let!(:bp7) { create(:billable_period, enterprise: enterprise, updated_at: job_start_time - 5.seconds, begins_at: start_of_july - 5.days, ends_at: start_of_july ) } + # Updated before start, but order is already complete, so should not be deleted + let!(:bp8) { create(:billable_period, enterprise: enterprise, updated_at: job_start_time - 5.seconds, begins_at: start_of_july, ends_at: start_of_july + 10.days, account_invoice: create(:account_invoice, order: create(:order, state: 'complete', completed_at: 5.minutes.ago))) } before do allow(Bugsnag).to receive(:notify) @@ -540,6 +542,7 @@ describe UpdateBillablePeriods do expect(bp5.reload.deleted_at).to be_nil expect(bp6.reload.deleted_at).to be_nil expect(bp7.reload.deleted_at).to be_nil + expect(bp8.reload.deleted_at).to be_nil end it "notifies bugsnag" do From 0370723edd5200882491360ad4ee8c17d0b96be2 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 19 Nov 2015 12:21:39 +1100 Subject: [PATCH 15/25] Attempt to add a sensible name and contact number to Account Invoice addresses --- app/jobs/update_account_invoices.rb | 6 ++++- spec/jobs/update_account_invoices_spec.rb | 29 ++++++++++++++++++----- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/app/jobs/update_account_invoices.rb b/app/jobs/update_account_invoices.rb index c0597a9ba3..70ba5e53c1 100644 --- a/app/jobs/update_account_invoices.rb +++ b/app/jobs/update_account_invoices.rb @@ -35,7 +35,11 @@ class UpdateAccountInvoices billable_periods = account_invoice.billable_periods.order(:enterprise_id, :begins_at).reject{ |bp| bp.turnover == 0 } if billable_periods.any? - address = billable_periods.first.enterprise.address + oldest_enterprise = billable_periods.first.enterprise + address = oldest_enterprise.address.dup + first, space, last = (oldest_enterprise.contact || "").partition(' ') + address.update_attributes(phone: oldest_enterprise.phone || "none") + address.update_attributes(firstname: first, lastname: last) if first.present? && last.present? account_invoice.order.update_attributes(bill_address: address, ship_address: address) end diff --git a/spec/jobs/update_account_invoices_spec.rb b/spec/jobs/update_account_invoices_spec.rb index a97811fc86..42198f4ff9 100644 --- a/spec/jobs/update_account_invoices_spec.rb +++ b/spec/jobs/update_account_invoices_spec.rb @@ -145,6 +145,7 @@ describe UpdateAccountInvoices do context "where the order is not complete" do before do allow(invoice_order).to receive(:complete?) { false } + old_billable_period.enterprise.update_attributes(contact: "Firstname Lastname Something Else", phone: '12345') updater.update(june_account_invoice) end @@ -158,8 +159,13 @@ describe UpdateAccountInvoices do it "assigns a addresses to the order" do expect(invoice_order.billing_address).to be_a Spree::Address expect(invoice_order.shipping_address).to be_a Spree::Address - expect(invoice_order.billing_address).to eq old_billable_period.enterprise.address - expect(invoice_order.shipping_address).to eq old_billable_period.enterprise.address + expect(invoice_order.shipping_address).to eq invoice_order.billing_address + [:address1, :address2, :city, :zipcode, :state_id, :country_id].each do |attr| + expect(invoice_order.billing_address[attr]).to eq old_billable_period.enterprise.address[attr] + end + expect(invoice_order.billing_address.firstname).to eq "Firstname" + expect(invoice_order.billing_address.lastname).to eq "Lastname Something Else" + expect(invoice_order.billing_address.phone).to eq "12345" end it "saves the order" do @@ -336,6 +342,7 @@ describe UpdateAccountInvoices do before do Spree::Config.set({ accounts_distributor_id: accounts_distributor.id }) + billable_period2.enterprise.update_attributes(contact: 'Anna Karenina', phone: '3433523') end context "when no invoice_order currently exists" do @@ -354,8 +361,13 @@ describe UpdateAccountInvoices do expect(invoice_order.state).to eq 'cart' expect(invoice_order.bill_address).to be_a Spree::Address expect(invoice_order.ship_address).to be_a Spree::Address - expect(invoice_order.bill_address).to eq billable_period2.enterprise.address - expect(invoice_order.ship_address).to eq billable_period2.enterprise.address + expect(invoice_order.shipping_address).to eq invoice_order.billing_address + [:address1, :address2, :city, :zipcode, :state_id, :country_id].each do |attr| + expect(invoice_order.billing_address[attr]).to eq billable_period2.enterprise.address[attr] + end + expect(invoice_order.billing_address.firstname).to eq "Anna" + expect(invoice_order.billing_address.lastname).to eq "Karenina" + expect(invoice_order.billing_address.phone).to eq "3433523" end end @@ -393,8 +405,13 @@ describe UpdateAccountInvoices do expect(invoice_order.state).to eq 'cart' expect(invoice_order.bill_address).to be_a Spree::Address expect(invoice_order.ship_address).to be_a Spree::Address - expect(invoice_order.bill_address).to eq billable_period2.enterprise.address - expect(invoice_order.ship_address).to eq billable_period2.enterprise.address + expect(invoice_order.shipping_address).to eq invoice_order.billing_address + [:address1, :address2, :city, :zipcode, :state_id, :country_id].each do |attr| + expect(invoice_order.billing_address[attr]).to eq billable_period2.enterprise.address[attr] + end + expect(invoice_order.billing_address.firstname).to eq "Anna" + expect(invoice_order.billing_address.lastname).to eq "Karenina" + expect(invoice_order.billing_address.phone).to eq "3433523" end end From f49722ba90d68e884bd5228bbe6919f7b59d58fb Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 19 Nov 2015 15:01:49 +1100 Subject: [PATCH 16/25] Fixing broken BMC specs --- .../business_model_configuration_controller_spec.rb | 2 +- .../admin/business_model_configuration_spec.rb | 4 ++-- spec/jobs/finalize_account_invoices_spec.rb | 4 ++-- spec/jobs/update_account_invoices_spec.rb | 12 ++++++------ 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/spec/controllers/admin/business_model_configuration_controller_spec.rb b/spec/controllers/admin/business_model_configuration_controller_spec.rb index 3418408417..b0ae86d4ac 100644 --- a/spec/controllers/admin/business_model_configuration_controller_spec.rb +++ b/spec/controllers/admin/business_model_configuration_controller_spec.rb @@ -58,7 +58,7 @@ describe Admin::BusinessModelConfigurationController, type: :controller do it "does not allow them to be set" do expect(response).to render_template :edit - expect(assigns(:settings).errors.count).to be 4 + expect(assigns(:settings).errors.count).to be 5 expect(Spree::Config.account_invoices_monthly_fixed).to eq 5 expect(Spree::Config.account_invoices_monthly_rate).to eq 0.02 expect(Spree::Config.account_invoices_monthly_cap).to eq 50 diff --git a/spec/features/admin/business_model_configuration_spec.rb b/spec/features/admin/business_model_configuration_spec.rb index 61da09293f..05d5367437 100644 --- a/spec/features/admin/business_model_configuration_spec.rb +++ b/spec/features/admin/business_model_configuration_spec.rb @@ -11,7 +11,7 @@ feature 'Business Model Configuration' do Spree::Config.set({ account_invoices_monthly_fixed: 5, account_invoices_monthly_rate: 0.02, - account_invoices_monthly_cap: 50 + account_invoices_monthly_cap: 50, account_invoices_tax_rate: 0.1 }) end @@ -45,7 +45,7 @@ feature 'Business Model Configuration' do expect(Spree::Config.account_invoices_monthly_fixed).to eq 10 expect(Spree::Config.account_invoices_monthly_rate).to eq 0.05 expect(Spree::Config.account_invoices_monthly_cap).to eq 30 - expect(Spree::Config.settings_account_invoices_tax_rate).to eq 0.15 + expect(Spree::Config.account_invoices_tax_rate).to eq 0.15 end end end diff --git a/spec/jobs/finalize_account_invoices_spec.rb b/spec/jobs/finalize_account_invoices_spec.rb index e169df93e5..552a909f3f 100644 --- a/spec/jobs/finalize_account_invoices_spec.rb +++ b/spec/jobs/finalize_account_invoices_spec.rb @@ -200,9 +200,9 @@ describe FinalizeAccountInvoices do invoice.reload expect(invoice.completed_at).to_not be_nil - expect(invoice.total).to eq billable_period1.bill + expect(invoice.total).to eq billable_period1.bill.round(2) expect(invoice.payments.count).to eq 1 - expect(invoice.payments.first.amount).to eq billable_period1.bill + expect(invoice.payments.first.amount).to eq billable_period1.bill.round(2) expect(invoice.state).to eq 'complete' end end diff --git a/spec/jobs/update_account_invoices_spec.rb b/spec/jobs/update_account_invoices_spec.rb index 42198f4ff9..fd2a490d8c 100644 --- a/spec/jobs/update_account_invoices_spec.rb +++ b/spec/jobs/update_account_invoices_spec.rb @@ -152,7 +152,7 @@ describe UpdateAccountInvoices do it "creates adjustments for each billing item" do adjustments = invoice_order.adjustments expect(adjustments.map(&:source_id)).to eq [old_billable_period.id] - expect(adjustments.map(&:amount)).to eq [old_billable_period.bill] + expect(adjustments.map(&:amount)).to eq [old_billable_period.bill.round(2)] expect(adjustments.map(&:label)).to eq [old_billable_period.adjustment_label] end @@ -189,7 +189,7 @@ describe UpdateAccountInvoices do it "creates adjustments for each billing item" do adjustments = july_account_invoice.order.adjustments expect(adjustments.map(&:source_id)).to eq [billable_period1.id, billable_period2.id] - expect(adjustments.map(&:amount)).to eq [billable_period1.bill, billable_period2.bill] + expect(adjustments.map(&:amount)).to eq [billable_period1.bill.round(2), billable_period2.bill.round(2)] expect(adjustments.map(&:label)).to eq [billable_period1.adjustment_label, billable_period2.adjustment_label] end @@ -355,8 +355,8 @@ describe UpdateAccountInvoices do expect(user.orders.first).to eq invoice_order expect(invoice_order.completed_at).to be_nil billable_adjustments = invoice_order.adjustments.where('source_type = (?)', 'BillablePeriod') - expect(billable_adjustments.map(&:amount)).to eq [billable_period2.bill, billable_period3.bill] - expect(invoice_order.total).to eq billable_period2.bill + billable_period3.bill + expect(billable_adjustments.map(&:amount)).to eq [billable_period2.bill.round(2), billable_period3.bill.round(2)] + expect(invoice_order.total).to eq (billable_period2.bill + billable_period3.bill).round(2) expect(invoice_order.payments.count).to eq 0 expect(invoice_order.state).to eq 'cart' expect(invoice_order.bill_address).to be_a Spree::Address @@ -399,8 +399,8 @@ describe UpdateAccountInvoices do expect(invoice_order.completed_at).to be_nil billable_adjustments = invoice_order.adjustments.where('source_type = (?)', 'BillablePeriod') expect(billable_adjustments).to_not include billable_adjustment - expect(billable_adjustments.map(&:amount)).to eq [billable_period2.bill, billable_period3.bill] - expect(invoice_order.total).to eq billable_period2.bill + billable_period3.bill + expect(billable_adjustments.map(&:amount)).to eq [billable_period2.bill.round(2), billable_period3.bill.round(2)] + expect(invoice_order.total).to eq (billable_period2.bill + billable_period3.bill).round(2) expect(invoice_order.payments.count).to eq 0 expect(invoice_order.state).to eq 'cart' expect(invoice_order.bill_address).to be_a Spree::Address From fe0652e243f1fb00ec6eac0e1ee828ac22f98438 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 9 Dec 2015 14:19:57 +1100 Subject: [PATCH 17/25] When updating a line_item quantity from 0, final_weight_volume is recalculated from the variants unit value --- app/models/spree/line_item_decorator.rb | 12 ++--- spec/models/spree/line_item_spec.rb | 64 ++++++++++++++++++------- 2 files changed, 52 insertions(+), 24 deletions(-) diff --git a/app/models/spree/line_item_decorator.rb b/app/models/spree/line_item_decorator.rb index ee4384b460..9db9ae1269 100644 --- a/app/models/spree/line_item_decorator.rb +++ b/app/models/spree/line_item_decorator.rb @@ -7,7 +7,7 @@ Spree::LineItem.class_eval do attr_accessible :max_quantity, :final_weight_volume, :price attr_accessible :final_weight_volume, :price, :as => :api - before_save :calculate_final_weight_volume, unless: :final_weight_volume_changed? + before_save :calculate_final_weight_volume, if: :quantity_changed?, unless: :final_weight_volume_changed? after_save :update_units delegate :unit_description, to: :variant @@ -88,12 +88,10 @@ Spree::LineItem.class_eval do private def calculate_final_weight_volume - if quantity_changed? - if final_weight_volume.present? - self.final_weight_volume = final_weight_volume * quantity / quantity_was - elsif variant.andand.unit_value - self.final_weight_volume = ((variant.andand.unit_value) * quantity) - end + if final_weight_volume.present? && quantity_was > 0 + self.final_weight_volume = final_weight_volume * quantity / quantity_was + elsif variant.andand.unit_value.present? + self.final_weight_volume = variant.andand.unit_value * quantity end end end diff --git a/spec/models/spree/line_item_spec.rb b/spec/models/spree/line_item_spec.rb index ecbd3ae610..6bb18fafa3 100644 --- a/spec/models/spree/line_item_spec.rb +++ b/spec/models/spree/line_item_spec.rb @@ -165,27 +165,57 @@ module Spree end context "and quantity is changed" do - context "and a final_weight_volume has been set" do - before do - expect(expect(li.final_weight_volume).to eq 3000) - attrs.merge!( quantity: 4 ) - li.update_attributes(attrs) + context "from > 0" do + context "and a final_weight_volume has been set" do + before do + expect(li.final_weight_volume).to eq 3000 + attrs.merge!( quantity: 4 ) + li.update_attributes(attrs) + end + + it "scales the final_weight_volume based on the change in quantity" do + expect(li.final_weight_volume).to eq 4000 + end end - it "calculates a final_weight_volume from the variants unit_value" do - expect(li.final_weight_volume).to eq 4000 + context "and a final_weight_volume has not been set" do + before do + li.update_attributes(final_weight_volume: nil) + attrs.merge!( quantity: 1 ) + li.update_attributes(attrs) + end + + it "calculates a final_weight_volume from the variants unit_value" do + expect(li.final_weight_volume).to eq 1000 + end end end - context "and a final_weight_volume has not been set" do - before do - li.update_attributes(final_weight_volume: nil) - attrs.merge!( quantity: 1 ) - li.update_attributes(attrs) + context "from 0" do + before { li.update_attributes(quantity: 0) } + + context "and a final_weight_volume has been set" do + before do + expect(li.final_weight_volume).to eq 0 + attrs.merge!( quantity: 4 ) + li.update_attributes(attrs) + end + + it "recalculates a final_weight_volume from the variants unit_value" do + expect(li.final_weight_volume).to eq 4000 + end end - it "calculates a final_weight_volume from the variants unit_value" do - expect(li.final_weight_volume).to eq 1000 + context "and a final_weight_volume has not been set" do + before do + li.update_attributes(final_weight_volume: nil) + attrs.merge!( quantity: 1 ) + li.update_attributes(attrs) + end + + it "calculates a final_weight_volume from the variants unit_value" do + expect(li.final_weight_volume).to eq 1000 + end end end end @@ -196,7 +226,7 @@ module Spree describe "generating the full name" do let(:li) { LineItem.new } - context "when display_name is blank" do + context "when display_name is blank" do before do li.stub(:unit_to_display) { 'unit_to_display' } li.stub(:display_name) { '' } @@ -223,7 +253,7 @@ module Spree li.stub(:unit_to_display) { '10kg' } li.stub(:display_name) { '10kg Box' } end - + it "returns display_name" do li.full_name.should == '10kg Box' end @@ -234,7 +264,7 @@ module Spree li.stub(:unit_to_display) { '1 Loaf' } li.stub(:display_name) { 'Spelt Sourdough' } end - + it "returns unit_to_display" do li.full_name.should == 'Spelt Sourdough (1 Loaf)' end From ecd11702c391c5df7b7b7cb9c3a5ad6f0d1295f2 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 10 Dec 2015 12:31:05 +1100 Subject: [PATCH 18/25] Fixing up enterprise user account page --- app/controllers/admin/account_controller.rb | 3 -- app/helpers/admin/account_helper.rb | 14 ++++++ app/models/spree/ability_decorator.rb | 5 ++ app/views/admin/account/show.html.haml | 51 +++++++++++++-------- 4 files changed, 51 insertions(+), 22 deletions(-) create mode 100644 app/helpers/admin/account_helper.rb diff --git a/app/controllers/admin/account_controller.rb b/app/controllers/admin/account_controller.rb index a752b2cfdd..00531cd426 100644 --- a/app/controllers/admin/account_controller.rb +++ b/app/controllers/admin/account_controller.rb @@ -2,8 +2,5 @@ class Admin::AccountController < Spree::Admin::BaseController def show @invoices = spree_current_user.account_invoices - # @enterprises = Enterprise.where(id: BillablePeriod.where(owner_id: spree_current_user).map(&:enterprise_id)) - # .group_by('enterprise.id').joins(:billable_periods) - # .select('SUM(billable_periods.turnover) AS turnover').order('turnover DESC') end end diff --git a/app/helpers/admin/account_helper.rb b/app/helpers/admin/account_helper.rb new file mode 100644 index 0000000000..0292522b74 --- /dev/null +++ b/app/helpers/admin/account_helper.rb @@ -0,0 +1,14 @@ +module Admin + module AccountHelper + def invoice_description_for(invoice) + month = t(:abbr_month_names, :scope => :date)[invoice.month] + year = invoice.year + star = invoice.order.nil? || invoice.order.completed? ? "" : "*" + "#{month} #{year}#{star}" + end + + def invoice_total_for(invoice) + invoice.order.andand.display_total || Spree::Money.new(0, { :currency => Spree::Config[:currency] }) + end + end +end diff --git a/app/models/spree/ability_decorator.rb b/app/models/spree/ability_decorator.rb index 82054ad792..25e0ced2cc 100644 --- a/app/models/spree/ability_decorator.rb +++ b/app/models/spree/ability_decorator.rb @@ -92,6 +92,11 @@ class AbilityDecorator can [:admin, :known_users], :search can [:admin, :show], :account + + # For printing own account invoice orders + can [:print], Spree::Order do |order| + order.user == user + end end def add_product_management_abilities(user) diff --git a/app/views/admin/account/show.html.haml b/app/views/admin/account/show.html.haml index 809d52eb2f..09a6862d2e 100644 --- a/app/views/admin/account/show.html.haml +++ b/app/views/admin/account/show.html.haml @@ -7,34 +7,47 @@ %h4= t(:no_invoices_to_display) - @invoices.order('year DESC, month DESC').each do |invoice| - - order = invoice.order .row.invoice_title - .eight.columns.alpha - %h4= "#{t(:abbr_month_names, :scope => :date)[invoice.month]} #{invoice.year}#{invoice.order.completed? ? "" : "*"}" - .eight.columns.omega.text-right - %h4.balance= invoice.order.display_total + .two.columns.alpha + %h4= invoice_description_for(invoice) + .two.columns.text-right + %h5 + - if invoice.order.andand.complete? + %a{ href: print_admin_order_url(invoice.order), :target => "_blank"} + %i.icon-print + = t(:print) + - else +   + .ten.columns +   + .two.columns.omega.text-right + %h4.balance= invoice_total_for(invoice) %table.invoice_summary - %col{ width: '20%' } - %col{ width: '60%' } - %col{ width: '20%' } + %col{ width: '25%' } + %col{ width: '62.5%' } + %col{ width: '12.5%' } %thead %th Date %th= t(:description) %th= t(:charge) - - invoice.billable_periods.select{ |bp| bp.bill > 0}.each do |billable_period| - %tr - %td.text-center= "#{billable_period.begins_at.strftime("%d/%m/%Y")}" - %td= billable_period.label - %td.text-right= billable_period.display_bill - - order.adjustments.where('source_type <> (?)', "BillablePeriod").each do |adjustment| - %tr - %td.text-center   - %td= adjustment.label - %td.text-right= adjustment.display_amount + - if order = invoice.order + - invoice.billable_periods.select{ |bp| bp.bill > 0}.each do |billable_period| + %tr + %td.text-center= "#{billable_period.begins_at.strftime("%d/%m/%Y")}" + %td= billable_period.label + -# Using amount from the actual adjustment on the order here so that we avoid recalculating the bill + -# at a future date with different settings to those used at the time the invoice was finalized + %td.text-right= billable_period.adjustment.display_amount + - order.adjustments.where('source_type <> (?)', "BillablePeriod").reject{ |a| a.amount == 0 }.each do |adjustment| + %tr + %td.text-center   + %td= adjustment.label + %td.text-right= adjustment.display_amount %tr.total %td.text-center   %td= t(:total).upcase - %td.text-right= order.display_total + %td.text-right= invoice_total_for(invoice) + -# - if @enterprises.empty? -# %h4 No enterprises to display From bac4fcbd8f82484853d532d99c47fb4eef177d39 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 10 Dec 2015 16:39:48 +1100 Subject: [PATCH 19/25] Make sure that adjustments for billable periods have a valid order to attach to when creating/updating --- app/jobs/update_account_invoices.rb | 2 +- app/models/billable_period.rb | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/jobs/update_account_invoices.rb b/app/jobs/update_account_invoices.rb index 70ba5e53c1..4f53bec6ee 100644 --- a/app/jobs/update_account_invoices.rb +++ b/app/jobs/update_account_invoices.rb @@ -38,7 +38,7 @@ class UpdateAccountInvoices oldest_enterprise = billable_periods.first.enterprise address = oldest_enterprise.address.dup first, space, last = (oldest_enterprise.contact || "").partition(' ') - address.update_attributes(phone: oldest_enterprise.phone || "none") + address.update_attributes(phone: oldest_enterprise.phone) if oldest_enterprise.phone.present? address.update_attributes(firstname: first, lastname: last) if first.present? && last.present? account_invoice.order.update_attributes(bill_address: address, ship_address: address) end diff --git a/app/models/billable_period.rb b/app/models/billable_period.rb index 062f70dde8..d66c51aa48 100644 --- a/app/models/billable_period.rb +++ b/app/models/billable_period.rb @@ -44,6 +44,7 @@ class BillablePeriod < ActiveRecord::Base def ensure_correct_adjustment_for(invoice) if adjustment # adjustment.originator = enterprise.package + adjustment.adjustable = invoice adjustment.update_attributes( label: adjustment_label, amount: bill ) else self.adjustment = invoice.adjustments.new( adjustment_attrs, :without_protection => true ) From 83e3fb98f741f77df2598daa5e18f17cb869193d Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Fri, 11 Dec 2015 13:15:24 +1100 Subject: [PATCH 20/25] Only display billable period adjustments where the amount in > 0 Rather than where the bill (which is calculated according to current settings) is > 0 --- app/views/admin/account/show.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/admin/account/show.html.haml b/app/views/admin/account/show.html.haml index 09a6862d2e..00ba61cd1d 100644 --- a/app/views/admin/account/show.html.haml +++ b/app/views/admin/account/show.html.haml @@ -31,7 +31,7 @@ %th= t(:description) %th= t(:charge) - if order = invoice.order - - invoice.billable_periods.select{ |bp| bp.bill > 0}.each do |billable_period| + - invoice.billable_periods.select{ |bp| bp.adjustment.andand.amount.andand > 0}.each do |billable_period| %tr %td.text-center= "#{billable_period.begins_at.strftime("%d/%m/%Y")}" %td= billable_period.label From 26a2f1a28050b9638a3c68aa29bab183f2832f2d Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Wed, 16 Dec 2015 23:10:47 +1100 Subject: [PATCH 21/25] Adjustments on account invoices are created based on presence of a bill Rather than of turnover --- app/jobs/update_account_invoices.rb | 2 +- spec/jobs/update_account_invoices_spec.rb | 63 +++++++++++++---------- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/app/jobs/update_account_invoices.rb b/app/jobs/update_account_invoices.rb index 4f53bec6ee..9a08c77383 100644 --- a/app/jobs/update_account_invoices.rb +++ b/app/jobs/update_account_invoices.rb @@ -32,7 +32,7 @@ class UpdateAccountInvoices invoice_order: account_invoice.order.as_json }) else - billable_periods = account_invoice.billable_periods.order(:enterprise_id, :begins_at).reject{ |bp| bp.turnover == 0 } + billable_periods = account_invoice.billable_periods.order(:enterprise_id, :begins_at).reject{ |bp| bp.bill == 0 } if billable_periods.any? oldest_enterprise = billable_periods.first.enterprise diff --git a/spec/jobs/update_account_invoices_spec.rb b/spec/jobs/update_account_invoices_spec.rb index fd2a490d8c..fb2817ca35 100644 --- a/spec/jobs/update_account_invoices_spec.rb +++ b/spec/jobs/update_account_invoices_spec.rb @@ -7,17 +7,28 @@ end describe UpdateAccountInvoices do let(:year) { Time.zone.now.year } + before do + # Make sure that bills are > 0 + Spree::Config.set(:account_invoices_monthly_fixed, 5) + Spree::Config.set(:account_invoices_monthly_rate, 0.02) + Spree::Config.set(:account_invoices_monthly_cap, 50) + end + describe "units specs" do let!(:start_of_july) { Time.zone.local(year, 7) } let!(:updater) { UpdateAccountInvoices.new } let!(:user) { create(:user) } - let!(:old_billable_period) { create(:billable_period, owner: user, begins_at: start_of_july - 1.month, ends_at: start_of_july) } - let!(:billable_period1) { create(:billable_period, owner: user, begins_at: start_of_july, ends_at: start_of_july + 12.days) } - let!(:billable_period2) { create(:billable_period, owner: user, begins_at: start_of_july + 12.days, ends_at: start_of_july + 20.days) } - let(:june_account_invoice) { old_billable_period.account_invoice } - let(:july_account_invoice) { billable_period1.account_invoice } + let!(:june_billable_period1) { create(:billable_period, owner: user, begins_at: start_of_july - 1.month, ends_at: start_of_july - 20.days) } + let!(:june_billable_period2) { create(:billable_period, owner: user, begins_at: start_of_july - 20.days, ends_at: start_of_july - 10.days, turnover: 45, sells: "none" ) } + let!(:june_billable_period3) { create(:billable_period, owner: user, begins_at: start_of_july - 10.days, ends_at: start_of_july - 1.days, turnover: 0, sells: "any" ) } + let!(:july_billable_period1) { create(:billable_period, owner: user, begins_at: start_of_july, ends_at: start_of_july + 12.days) } + let!(:july_billable_period2) { create(:billable_period, owner: user, begins_at: start_of_july + 12.days, ends_at: start_of_july + 20.days) } + let!(:july_billable_period3) { create(:billable_period, owner: user, begins_at: start_of_july + 20.days, ends_at: start_of_july + 25.days, turnover: 45, sells: 'none') } + let!(:july_billable_period4) { create(:billable_period, owner: user, begins_at: start_of_july + 25.days, ends_at: start_of_july + 28.days, turnover: 0, sells: 'any') } + let(:june_account_invoice) { june_billable_period1.account_invoice } + let(:july_account_invoice) { july_billable_period1.account_invoice } describe "perform" do let(:accounts_distributor) { double(:accounts_distributor) } @@ -145,15 +156,15 @@ describe UpdateAccountInvoices do context "where the order is not complete" do before do allow(invoice_order).to receive(:complete?) { false } - old_billable_period.enterprise.update_attributes(contact: "Firstname Lastname Something Else", phone: '12345') + june_billable_period1.enterprise.update_attributes(contact: "Firstname Lastname Something Else", phone: '12345') updater.update(june_account_invoice) end - it "creates adjustments for each billing item" do + it "creates adjustments for each billing item where bill is not 0" do adjustments = invoice_order.adjustments - expect(adjustments.map(&:source_id)).to eq [old_billable_period.id] - expect(adjustments.map(&:amount)).to eq [old_billable_period.bill.round(2)] - expect(adjustments.map(&:label)).to eq [old_billable_period.adjustment_label] + expect(adjustments.map(&:source_id)).to eq [june_billable_period1.id, june_billable_period3.id] + expect(adjustments.map(&:amount)).to eq [june_billable_period1.bill.round(2), june_billable_period3.bill.round(2)] + expect(adjustments.map(&:label)).to eq [june_billable_period1.adjustment_label, june_billable_period3.adjustment_label] end it "assigns a addresses to the order" do @@ -161,7 +172,7 @@ describe UpdateAccountInvoices do expect(invoice_order.shipping_address).to be_a Spree::Address expect(invoice_order.shipping_address).to eq invoice_order.billing_address [:address1, :address2, :city, :zipcode, :state_id, :country_id].each do |attr| - expect(invoice_order.billing_address[attr]).to eq old_billable_period.enterprise.address[attr] + expect(invoice_order.billing_address[attr]).to eq june_billable_period1.enterprise.address[attr] end expect(invoice_order.billing_address.firstname).to eq "Firstname" expect(invoice_order.billing_address.lastname).to eq "Lastname Something Else" @@ -186,11 +197,11 @@ describe UpdateAccountInvoices do updater.update(july_account_invoice) end - it "creates adjustments for each billing item" do + it "creates adjustments for each billing item where bill is not 0" do adjustments = july_account_invoice.order.adjustments - expect(adjustments.map(&:source_id)).to eq [billable_period1.id, billable_period2.id] - expect(adjustments.map(&:amount)).to eq [billable_period1.bill.round(2), billable_period2.bill.round(2)] - expect(adjustments.map(&:label)).to eq [billable_period1.adjustment_label, billable_period2.adjustment_label] + expect(adjustments.map(&:source_id)).to eq [july_billable_period1.id, july_billable_period2.id,july_billable_period4.id] + expect(adjustments.map(&:amount)).to eq [july_billable_period1.bill.round(2), july_billable_period2.bill.round(2), july_billable_period4.bill.round(2)] + expect(adjustments.map(&:label)).to eq [july_billable_period1.adjustment_label, july_billable_period2.adjustment_label, july_billable_period4.adjustment_label] end it "saves the order" do @@ -334,15 +345,15 @@ describe UpdateAccountInvoices do let!(:accounts_distributor) { create(:distributor_enterprise) } let!(:user) { create(:user) } - let!(:billable_period1) { create(:billable_period, sells: 'any', owner: user, begins_at: start_of_july - 1.month, ends_at: start_of_july) } - let!(:billable_period2) { create(:billable_period, owner: user, begins_at: start_of_july, ends_at: start_of_july + 10.days) } - let!(:billable_period3) { create(:billable_period, owner: user, begins_at: start_of_july + 12.days, ends_at: start_of_july + 20.days) } - let!(:july_account_invoice) { billable_period2.account_invoice } + let!(:july_billable_period1) { create(:billable_period, sells: 'any', owner: user, begins_at: start_of_july - 1.month, ends_at: start_of_july) } + let!(:july_billable_period2) { create(:billable_period, owner: user, begins_at: start_of_july, ends_at: start_of_july + 10.days) } + let!(:july_billable_period3) { create(:billable_period, owner: user, begins_at: start_of_july + 12.days, ends_at: start_of_july + 20.days) } + let!(:july_account_invoice) { july_billable_period2.account_invoice } let!(:august_account_invoice) { create(:account_invoice, user: user, year: july_account_invoice.year, month: 8)} before do Spree::Config.set({ accounts_distributor_id: accounts_distributor.id }) - billable_period2.enterprise.update_attributes(contact: 'Anna Karenina', phone: '3433523') + july_billable_period2.enterprise.update_attributes(contact: 'Anna Karenina', phone: '3433523') end context "when no invoice_order currently exists" do @@ -355,15 +366,15 @@ describe UpdateAccountInvoices do expect(user.orders.first).to eq invoice_order expect(invoice_order.completed_at).to be_nil billable_adjustments = invoice_order.adjustments.where('source_type = (?)', 'BillablePeriod') - expect(billable_adjustments.map(&:amount)).to eq [billable_period2.bill.round(2), billable_period3.bill.round(2)] - expect(invoice_order.total).to eq (billable_period2.bill + billable_period3.bill).round(2) + expect(billable_adjustments.map(&:amount)).to eq [july_billable_period2.bill.round(2), july_billable_period3.bill.round(2)] + expect(invoice_order.total).to eq (july_billable_period2.bill + july_billable_period3.bill).round(2) expect(invoice_order.payments.count).to eq 0 expect(invoice_order.state).to eq 'cart' expect(invoice_order.bill_address).to be_a Spree::Address expect(invoice_order.ship_address).to be_a Spree::Address expect(invoice_order.shipping_address).to eq invoice_order.billing_address [:address1, :address2, :city, :zipcode, :state_id, :country_id].each do |attr| - expect(invoice_order.billing_address[attr]).to eq billable_period2.enterprise.address[attr] + expect(invoice_order.billing_address[attr]).to eq july_billable_period2.enterprise.address[attr] end expect(invoice_order.billing_address.firstname).to eq "Anna" expect(invoice_order.billing_address.lastname).to eq "Karenina" @@ -399,15 +410,15 @@ describe UpdateAccountInvoices do expect(invoice_order.completed_at).to be_nil billable_adjustments = invoice_order.adjustments.where('source_type = (?)', 'BillablePeriod') expect(billable_adjustments).to_not include billable_adjustment - expect(billable_adjustments.map(&:amount)).to eq [billable_period2.bill.round(2), billable_period3.bill.round(2)] - expect(invoice_order.total).to eq (billable_period2.bill + billable_period3.bill).round(2) + expect(billable_adjustments.map(&:amount)).to eq [july_billable_period2.bill.round(2), july_billable_period3.bill.round(2)] + expect(invoice_order.total).to eq (july_billable_period2.bill + july_billable_period3.bill).round(2) expect(invoice_order.payments.count).to eq 0 expect(invoice_order.state).to eq 'cart' expect(invoice_order.bill_address).to be_a Spree::Address expect(invoice_order.ship_address).to be_a Spree::Address expect(invoice_order.shipping_address).to eq invoice_order.billing_address [:address1, :address2, :city, :zipcode, :state_id, :country_id].each do |attr| - expect(invoice_order.billing_address[attr]).to eq billable_period2.enterprise.address[attr] + expect(invoice_order.billing_address[attr]).to eq july_billable_period2.enterprise.address[attr] end expect(invoice_order.billing_address.firstname).to eq "Anna" expect(invoice_order.billing_address.lastname).to eq "Karenina" From 555f63902639713368d8f94df9b032ee854f0cb3 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 17 Dec 2015 09:16:19 +1100 Subject: [PATCH 22/25] Fixing unstable update_account_invoice specs Rounding first then summing != summing first then rounding --- spec/jobs/update_account_invoices_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/jobs/update_account_invoices_spec.rb b/spec/jobs/update_account_invoices_spec.rb index fb2817ca35..87d601b60a 100644 --- a/spec/jobs/update_account_invoices_spec.rb +++ b/spec/jobs/update_account_invoices_spec.rb @@ -367,7 +367,7 @@ describe UpdateAccountInvoices do expect(invoice_order.completed_at).to be_nil billable_adjustments = invoice_order.adjustments.where('source_type = (?)', 'BillablePeriod') expect(billable_adjustments.map(&:amount)).to eq [july_billable_period2.bill.round(2), july_billable_period3.bill.round(2)] - expect(invoice_order.total).to eq (july_billable_period2.bill + july_billable_period3.bill).round(2) + expect(invoice_order.total).to eq july_billable_period2.bill.round(2) + july_billable_period3.bill.round(2) expect(invoice_order.payments.count).to eq 0 expect(invoice_order.state).to eq 'cart' expect(invoice_order.bill_address).to be_a Spree::Address @@ -411,7 +411,7 @@ describe UpdateAccountInvoices do billable_adjustments = invoice_order.adjustments.where('source_type = (?)', 'BillablePeriod') expect(billable_adjustments).to_not include billable_adjustment expect(billable_adjustments.map(&:amount)).to eq [july_billable_period2.bill.round(2), july_billable_period3.bill.round(2)] - expect(invoice_order.total).to eq (july_billable_period2.bill + july_billable_period3.bill).round(2) + expect(invoice_order.total).to eq july_billable_period2.bill.round(2) + july_billable_period3.bill.round(2) expect(invoice_order.payments.count).to eq 0 expect(invoice_order.state).to eq 'cart' expect(invoice_order.bill_address).to be_a Spree::Address From b00406067967b4884100f88b3de37ae34949789d Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 17 Dec 2015 11:33:18 +1100 Subject: [PATCH 23/25] Printing invoices doesn't fail when order has no order cycle --- app/views/spree/admin/orders/invoice.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/orders/invoice.html.haml b/app/views/spree/admin/orders/invoice.html.haml index 75377b800e..0913ee27db 100644 --- a/app/views/spree/admin/orders/invoice.html.haml +++ b/app/views/spree/admin/orders/invoice.html.haml @@ -14,7 +14,7 @@ %td{width: "10%" }   %td{ :align => "right" } - %h4= @order.order_cycle.name + %h4= @order.order_cycle.andand.name %tr{ valign: "top" } %td{ :align => "left" } %strong= "From: #{@order.distributor.name}" From e0da49e4df789c9049842797dcfd7690be0b65b2 Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 17 Dec 2015 11:34:06 +1100 Subject: [PATCH 24/25] Formatting dates for account invoices in rails time zone --- app/jobs/finalize_account_invoices.rb | 2 +- app/jobs/update_account_invoices.rb | 2 +- app/jobs/update_billable_periods.rb | 2 +- app/models/billable_period.rb | 4 ++-- app/views/admin/account/show.html.haml | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/jobs/finalize_account_invoices.rb b/app/jobs/finalize_account_invoices.rb index 4baa841db6..25b614bebc 100644 --- a/app/jobs/finalize_account_invoices.rb +++ b/app/jobs/finalize_account_invoices.rb @@ -51,7 +51,7 @@ class FinalizeAccountInvoices job: "FinalizeAccountInvoices", error: "end_date is in the future", data: { - end_date: end_date.localtime.strftime("%F %T"), + end_date: end_date.in_time_zone.strftime("%F %T"), now: Time.zone.now.strftime("%F %T") } }) diff --git a/app/jobs/update_account_invoices.rb b/app/jobs/update_account_invoices.rb index 9a08c77383..16112ef870 100644 --- a/app/jobs/update_account_invoices.rb +++ b/app/jobs/update_account_invoices.rb @@ -85,7 +85,7 @@ class UpdateAccountInvoices job: "UpdateAccountInvoices", error: "end_date is in the future", data: { - end_date: end_date.localtime.strftime("%F %T"), + end_date: end_date.in_time_zone.strftime("%F %T"), now: Time.zone.now.strftime("%F %T") } }) diff --git a/app/jobs/update_billable_periods.rb b/app/jobs/update_billable_periods.rb index d775b093f3..80f19d961a 100644 --- a/app/jobs/update_billable_periods.rb +++ b/app/jobs/update_billable_periods.rb @@ -118,7 +118,7 @@ class UpdateBillablePeriods job: "UpdateBillablePeriods", error: "end_date is in the future", data: { - end_date: end_date.localtime.strftime("%F %T"), + end_date: end_date.in_time_zone.strftime("%F %T"), now: Time.zone.now.strftime("%F %T") } }) diff --git a/app/models/billable_period.rb b/app/models/billable_period.rb index d66c51aa48..d604b5a59c 100644 --- a/app/models/billable_period.rb +++ b/app/models/billable_period.rb @@ -31,8 +31,8 @@ class BillablePeriod < ActiveRecord::Base end def adjustment_label - begins = begins_at.localtime.strftime("%d/%m/%y") - ends = ends_at.localtime.strftime("%d/%m/%y") + begins = begins_at.in_time_zone.strftime("%d/%m/%y") + ends = ends_at.in_time_zone.strftime("%d/%m/%y") "#{label} [#{begins} - #{ends}]" end diff --git a/app/views/admin/account/show.html.haml b/app/views/admin/account/show.html.haml index 00ba61cd1d..808402ed16 100644 --- a/app/views/admin/account/show.html.haml +++ b/app/views/admin/account/show.html.haml @@ -64,8 +64,8 @@ -# %th Bill -# - enterprise.billable_periods.each do |billable_period| -# %tr --# %td= billable_period.begins_at.localtime.strftime("%F %T") --# %td= billable_period.ends_at.localtime.strftime("%F %T") +-# %td= billable_period.begins_at.in_time_zone.strftime("%F %T") +-# %td= billable_period.ends_at.in_time_zone.strftime("%F %T") -# %td= billable_period.sells -# %td= billable_period.trial? -# %td= billable_period.display_turnover From 72f7e545dcccd96e7123a625c15a9059ed30ec4d Mon Sep 17 00:00:00 2001 From: Rob Harrington Date: Thu, 17 Dec 2015 12:04:24 +1100 Subject: [PATCH 25/25] Making sure bill > 0 for finalize account invoice spec --- spec/jobs/finalize_account_invoices_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/jobs/finalize_account_invoices_spec.rb b/spec/jobs/finalize_account_invoices_spec.rb index 552a909f3f..d4b250d42f 100644 --- a/spec/jobs/finalize_account_invoices_spec.rb +++ b/spec/jobs/finalize_account_invoices_spec.rb @@ -184,6 +184,11 @@ describe FinalizeAccountInvoices do Spree::Config.set({ accounts_distributor_id: accounts_distributor.id }) Spree::Config.set({ default_accounts_payment_method_id: pm.id }) Spree::Config.set({ default_accounts_shipping_method_id: sm.id }) + + # Make sure that bills are > 0 + Spree::Config.set(:account_invoices_monthly_fixed, 5) + Spree::Config.set(:account_invoices_monthly_rate, 0.02) + Spree::Config.set(:account_invoices_monthly_cap, 50) end context "finalizing an invoice" do