From 5f83ffa326a56a3981680e477929d372be404415 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 8 Oct 2024 09:36:00 +0100
Subject: [PATCH 01/22] Extract Pro WithTax into Taxable concern

Replacing wrapped Stripe objects with new Pro Plan which includes
Taxable.

Pro Plan will be expanded in later commits while refactoring the
checkout process.
---
 .../alaveteli_pro/plans_controller.rb         |  4 +-
 app/helpers/alaveteli_pro/account_helper.rb   |  2 +-
 app/models/alaveteli_pro/plan.rb              |  8 ++++
 app/models/alaveteli_pro/with_tax.rb          | 29 --------------
 app/models/concerns/taxable.rb                | 40 +++++++++++++++++++
 .../{with_tax_spec.rb => plan_spec.rb}        |  2 +-
 .../plans/index.html.erb_spec.rb              |  4 +-
 7 files changed, 54 insertions(+), 35 deletions(-)
 create mode 100644 app/models/alaveteli_pro/plan.rb
 delete mode 100644 app/models/alaveteli_pro/with_tax.rb
 create mode 100644 app/models/concerns/taxable.rb
 rename spec/models/alaveteli_pro/{with_tax_spec.rb => plan_spec.rb} (94%)

diff --git a/app/controllers/alaveteli_pro/plans_controller.rb b/app/controllers/alaveteli_pro/plans_controller.rb
index fde8cdfd79..61d9033568 100644
--- a/app/controllers/alaveteli_pro/plans_controller.rb
+++ b/app/controllers/alaveteli_pro/plans_controller.rb
@@ -7,7 +7,7 @@ class AlaveteliPro::PlansController < AlaveteliPro::BaseController
   def index
     default_plan_name = add_stripe_namespace('pro')
     stripe_plan = Stripe::Plan.retrieve(default_plan_name)
-    @plan = AlaveteliPro::WithTax.new(stripe_plan)
+    @plan = AlaveteliPro::Plan.new(stripe_plan)
     @pro_site_name = pro_site_name
   end
 
@@ -15,7 +15,7 @@ def show
     stripe_plan = Stripe::Plan.retrieve(
       id: plan_name, expand: ['product']
     )
-    @plan = AlaveteliPro::WithTax.new(stripe_plan)
+    @plan = AlaveteliPro::Plan.new(stripe_plan)
   rescue Stripe::InvalidRequestError
     raise ActiveRecord::RecordNotFound
   end
diff --git a/app/helpers/alaveteli_pro/account_helper.rb b/app/helpers/alaveteli_pro/account_helper.rb
index 6828b3f8e2..ee2901f0f6 100644
--- a/app/helpers/alaveteli_pro/account_helper.rb
+++ b/app/helpers/alaveteli_pro/account_helper.rb
@@ -19,6 +19,6 @@ def card_expiry_message(month, year)
   end
 
   def subscription_amount(subscription)
-    AlaveteliPro::WithTax.new(subscription).amount_with_tax
+    AlaveteliPro::Plan.new(subscription.plan).amount_with_tax
   end
 end
diff --git a/app/models/alaveteli_pro/plan.rb b/app/models/alaveteli_pro/plan.rb
new file mode 100644
index 0000000000..6aa7b9c2d8
--- /dev/null
+++ b/app/models/alaveteli_pro/plan.rb
@@ -0,0 +1,8 @@
+##
+# A wrapper for a Stripe::Plan
+#
+class AlaveteliPro::Plan < SimpleDelegator
+  include Taxable
+
+  tax :amount
+end
diff --git a/app/models/alaveteli_pro/with_tax.rb b/app/models/alaveteli_pro/with_tax.rb
deleted file mode 100644
index 8d1f4b1edb..0000000000
--- a/app/models/alaveteli_pro/with_tax.rb
+++ /dev/null
@@ -1,29 +0,0 @@
-#
-# Calculate amount + tax for a Stripe::Plan or Stripe::Subscription.
-#
-# Set STRIPE_TAX_RATE in config/general.yml to change the tax rate.
-#
-# Example
-#
-#   plan = Stripe::Plan.retrieve('pro')
-#   @plan = WithTax.new(plan)
-#   @plan.amount
-#   # => 833
-#   @plan.amount_with_tax
-#   # => 1000
-class AlaveteliPro::WithTax < SimpleDelegator
-  def amount_with_tax
-    # Need to use BigDecimal() here because SimpleDelegator is forwarding
-    # `#BigDecimal` to `#amount` in Ruby 2.0.
-    net = BigDecimal(amount * 0.01, 0).round(2)
-    vat = (net * tax_rate).round(2)
-    gross = net + vat
-    (gross * 100).floor
-  end
-
-  private
-
-  def tax_rate
-    @tax_rate ||= BigDecimal(AlaveteliConfiguration.stripe_tax_rate)
-  end
-end
diff --git a/app/models/concerns/taxable.rb b/app/models/concerns/taxable.rb
new file mode 100644
index 0000000000..20b011b121
--- /dev/null
+++ b/app/models/concerns/taxable.rb
@@ -0,0 +1,40 @@
+##
+# Calculate amount + tax for an attribute
+#
+# Set STRIPE_TAX_RATE in config/general.yml to change the tax rate.
+#
+# Example
+#   klass = Struct.new(:amount).include(Taxable)
+#   klass.tax(:amount)
+#   instance = klass.new(833)
+#   instance.amount
+#   # => 833
+#   instance.amount_with_tax
+#   # => 1000
+#
+module Taxable
+  extend ActiveSupport::Concern
+
+  included do
+    class << self
+      def tax(*attributes)
+        attributes.each do |attribute|
+          define_method("#{attribute}_with_tax") do
+            # Need to use BigDecimal() here because SimpleDelegator is forwarding
+            # `#BigDecimal` to `#amount` in Ruby 2.0.
+            net = BigDecimal(send(attribute) * 0.01, 0).round(2)
+            vat = (net * tax_rate).round(2)
+            gross = net + vat
+            (gross * 100).floor
+          end
+        end
+      end
+    end
+  end
+
+  private
+
+  def tax_rate
+    @tax_rate ||= BigDecimal(AlaveteliConfiguration.stripe_tax_rate)
+  end
+end
diff --git a/spec/models/alaveteli_pro/with_tax_spec.rb b/spec/models/alaveteli_pro/plan_spec.rb
similarity index 94%
rename from spec/models/alaveteli_pro/with_tax_spec.rb
rename to spec/models/alaveteli_pro/plan_spec.rb
index 5c0aba59ff..bffd0abdde 100644
--- a/spec/models/alaveteli_pro/with_tax_spec.rb
+++ b/spec/models/alaveteli_pro/plan_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-RSpec.describe AlaveteliPro::WithTax do
+RSpec.describe AlaveteliPro::Plan do
   let(:plan) { double(:plan, amount: 833) }
   subject { described_class.new(plan) }
 
diff --git a/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb b/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb
index 68b5ec0da6..7261098758 100644
--- a/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb
+++ b/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb
@@ -6,7 +6,7 @@
   let(:stripe_helper) { StripeMock.create_test_helper }
   let(:product) { stripe_helper.create_product }
 
-  let(:plan_with_tax) { AlaveteliPro::WithTax.new(stripe_plan) }
+  let(:plan) { AlaveteliPro::Plan.new(stripe_plan) }
   let(:cents_price) { 880 }
 
   let(:stripe_plan) do
@@ -18,7 +18,7 @@
   before do
     allow(AlaveteliConfiguration).to receive(:iso_currency_code).
         and_return('GBP')
-    assign :plan, plan_with_tax
+    assign :plan, plan
     assign :pro_site_name, 'Alaveteli<sup>Pro</sup>'
   end
 

From 69dfb917e4b8995add1048cad58e94f3a76be273 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Mon, 11 Nov 2024 12:06:14 +0000
Subject: [PATCH 02/22] Update Taxable concern

We're always returning integers there is no need to convert to floats
and back again. This can introduce rounding errors.
---
 app/models/concerns/taxable.rb | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/app/models/concerns/taxable.rb b/app/models/concerns/taxable.rb
index 20b011b121..cc63db5ba1 100644
--- a/app/models/concerns/taxable.rb
+++ b/app/models/concerns/taxable.rb
@@ -20,12 +20,9 @@ class << self
       def tax(*attributes)
         attributes.each do |attribute|
           define_method("#{attribute}_with_tax") do
-            # Need to use BigDecimal() here because SimpleDelegator is forwarding
-            # `#BigDecimal` to `#amount` in Ruby 2.0.
-            net = BigDecimal(send(attribute) * 0.01, 0).round(2)
-            vat = (net * tax_rate).round(2)
-            gross = net + vat
-            (gross * 100).floor
+            net = send(attribute)
+            vat = (net * tax_rate).round(0)
+            net + vat
           end
         end
       end
@@ -35,6 +32,6 @@ def tax(*attributes)
   private
 
   def tax_rate
-    @tax_rate ||= BigDecimal(AlaveteliConfiguration.stripe_tax_rate)
+    @tax_rate ||= AlaveteliConfiguration.stripe_tax_rate.to_f
   end
 end

From 0ee00bcbb94156d29e2498597b54a9d5dfc9bd98 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 8 Oct 2024 10:06:34 +0100
Subject: [PATCH 03/22] Update Pro Subscription method_missing

Check in object respond to the method before forwarding. This is needed
to fix exceptions such as:
  private method `BigDecimal' called for #<Stripe::Subscription...
---
 app/models/alaveteli_pro/subscription.rb       |  8 ++++++--
 spec/models/alaveteli_pro/subscription_spec.rb | 12 +++++++-----
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/app/models/alaveteli_pro/subscription.rb b/app/models/alaveteli_pro/subscription.rb
index 32e44dfa25..a6ea687ef5 100644
--- a/app/models/alaveteli_pro/subscription.rb
+++ b/app/models/alaveteli_pro/subscription.rb
@@ -47,11 +47,15 @@ def delete
 
     private
 
-    def method_missing(*args)
+    def method_missing(method, *args, &block)
       # Forward missing methods such as #coupon= as on a blank subscription
       # this wouldn't be delegated due to how Stripe::APIResource instances
       # use meta programming to dynamically define setting methods.
-      __getobj__.public_send(*args)
+      if __getobj__.respond_to?(method)
+        __getobj__.public_send(method, *args, &block)
+      else
+        super
+      end
     end
   end
 end
diff --git a/spec/models/alaveteli_pro/subscription_spec.rb b/spec/models/alaveteli_pro/subscription_spec.rb
index 9df99e6287..4a8ab82b69 100644
--- a/spec/models/alaveteli_pro/subscription_spec.rb
+++ b/spec/models/alaveteli_pro/subscription_spec.rb
@@ -136,12 +136,14 @@
   end
 
   describe 'missing methods' do
+    let(:object) { OpenStruct.new(bar: nil) }
+
     it 'should delegate methods to object' do
-      mock_coupon = double(:coupon)
-      expect { subscription.coupon }.to raise_error(NoMethodError)
-      expect { subscription.coupon = mock_coupon }.to_not raise_error
-      expect(subscription.coupon).to eq mock_coupon
-      expect(subscription.__getobj__.coupon).to eq mock_coupon
+      value = double
+      expect { subscription.foo }.to raise_error(NoMethodError)
+      expect { subscription.bar = value }.to_not raise_error
+      expect(subscription.bar).to eq(value)
+      expect(subscription.__getobj__.bar).to eq(value)
     end
   end
 end

From 0cbd2216e3564768e863dac3bb809393a6a99279 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 8 Oct 2024 10:11:06 +0100
Subject: [PATCH 04/22] Add Pro Subscription#plan method

Load and cache subscription.plan, automatically wrapping in our Pro
Plan class.
---
 app/helpers/alaveteli_pro/account_helper.rb                  | 4 ----
 app/models/alaveteli_pro/subscription.rb                     | 5 +++++
 app/models/alaveteli_pro/subscription_with_discount.rb       | 1 -
 app/views/alaveteli_pro/subscriptions/_subscription.html.erb | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/app/helpers/alaveteli_pro/account_helper.rb b/app/helpers/alaveteli_pro/account_helper.rb
index ee2901f0f6..ab9d526026 100644
--- a/app/helpers/alaveteli_pro/account_helper.rb
+++ b/app/helpers/alaveteli_pro/account_helper.rb
@@ -17,8 +17,4 @@ def card_expiry_message(month, year)
       content_tag(:p, _('Expires soon'), class: 'card__expiring')
     end
   end
-
-  def subscription_amount(subscription)
-    AlaveteliPro::Plan.new(subscription.plan).amount_with_tax
-  end
 end
diff --git a/app/models/alaveteli_pro/subscription.rb b/app/models/alaveteli_pro/subscription.rb
index a6ea687ef5..1a7816d4c6 100644
--- a/app/models/alaveteli_pro/subscription.rb
+++ b/app/models/alaveteli_pro/subscription.rb
@@ -45,6 +45,11 @@ def delete
       Stripe::Subscription.cancel(id)
     end
 
+    # plan
+    def plan
+      @plan ||= AlaveteliPro::Plan.new(__getobj__.plan)
+    end
+
     private
 
     def method_missing(method, *args, &block)
diff --git a/app/models/alaveteli_pro/subscription_with_discount.rb b/app/models/alaveteli_pro/subscription_with_discount.rb
index b257c2d33d..37e1d069f8 100644
--- a/app/models/alaveteli_pro/subscription_with_discount.rb
+++ b/app/models/alaveteli_pro/subscription_with_discount.rb
@@ -19,7 +19,6 @@ class AlaveteliPro::SubscriptionWithDiscount < SimpleDelegator
 
   def initialize(subscription)
     super
-    @plan = subscription.plan
     @original_amount = subscription.plan.amount
     @discount = subscription.discount
     @coupon = fetch_coupon
diff --git a/app/views/alaveteli_pro/subscriptions/_subscription.html.erb b/app/views/alaveteli_pro/subscriptions/_subscription.html.erb
index 24115cbcc2..ce6ee7e9ce 100644
--- a/app/views/alaveteli_pro/subscriptions/_subscription.html.erb
+++ b/app/views/alaveteli_pro/subscriptions/_subscription.html.erb
@@ -9,7 +9,7 @@
 
   <label class="plan-overview__amount amount">
     <span>
-      <%= format_currency(subscription_amount(subscription)) %>
+      <%= format_currency(subscription.plan.amount_with_tax) %>
       <span class="amount__freq">
         <%= billing_frequency(subscription.plan.interval) %>
 

From 7ea907fb5f2c7533f6b37e13b9a40de202a3e6fd Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 8 Oct 2024 10:10:13 +0100
Subject: [PATCH 05/22] Update Pro Subscription

Automatically include Pro SubscriptionWithDiscount so we don't have to
manually wrap the instances in the controller.
---
 app/controllers/alaveteli_pro/subscriptions_controller.rb    | 5 ++---
 app/models/alaveteli_pro/subscription.rb                     | 2 ++
 app/models/alaveteli_pro/subscription_with_discount.rb       | 4 ++--
 .../alaveteli_pro/subscriptions_controller_spec.rb           | 2 +-
 spec/models/alaveteli_pro/subscription_with_discount_spec.rb | 2 +-
 5 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/app/controllers/alaveteli_pro/subscriptions_controller.rb b/app/controllers/alaveteli_pro/subscriptions_controller.rb
index 679bf35212..1373f3a169 100644
--- a/app/controllers/alaveteli_pro/subscriptions_controller.rb
+++ b/app/controllers/alaveteli_pro/subscriptions_controller.rb
@@ -12,9 +12,8 @@ class AlaveteliPro::SubscriptionsController < AlaveteliPro::BaseController
 
   def index
     @customer = current_user.pro_account.try(:stripe_customer)
-    @subscriptions = @customer.subscriptions.map do |subscription|
-      AlaveteliPro::SubscriptionWithDiscount.new(subscription)
-    end
+    @subscriptions = current_user.pro_account.subscriptions
+
     if @customer.default_source
       @card =
         @customer.
diff --git a/app/models/alaveteli_pro/subscription.rb b/app/models/alaveteli_pro/subscription.rb
index 1a7816d4c6..f2cc3354ae 100644
--- a/app/models/alaveteli_pro/subscription.rb
+++ b/app/models/alaveteli_pro/subscription.rb
@@ -4,6 +4,8 @@ module AlaveteliPro
   # and to add useful helper methods.
   #
   class Subscription < SimpleDelegator
+    include SubscriptionWithDiscount
+
     # state
     def active?
       status == 'active'
diff --git a/app/models/alaveteli_pro/subscription_with_discount.rb b/app/models/alaveteli_pro/subscription_with_discount.rb
index 37e1d069f8..58546cfd8d 100644
--- a/app/models/alaveteli_pro/subscription_with_discount.rb
+++ b/app/models/alaveteli_pro/subscription_with_discount.rb
@@ -5,7 +5,7 @@
 #
 #   # subscription with 50% off 'forever' discount
 #   subscription = Stripe::Subscription.retrieve('sub_1234')
-#   @subscription = SubscriptionWithDiscount.new(subscription)
+#   @subscription = AlaveteliPro::Subscription.new(subscription)
 #   @subscription.original_amount
 #   # => 833
 #   @subscription.amount
@@ -14,7 +14,7 @@
 #   # => true
 #   @subscription.free?
 #   # => false
-class AlaveteliPro::SubscriptionWithDiscount < SimpleDelegator
+module AlaveteliPro::SubscriptionWithDiscount
   attr_reader :original_amount, :coupon
 
   def initialize(subscription)
diff --git a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
index db4aa3cf5e..fd6d96524e 100644
--- a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
+++ b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
@@ -759,7 +759,7 @@
 
       it 'assigns subscriptions' do
         get :index
-        expect(assigns[:subscriptions].length).to eq(1)
+        expect(assigns[:subscriptions].count).to eq(1)
         expect(assigns[:subscriptions].first.id).
           to eq(customer.subscriptions.first.id)
       end
diff --git a/spec/models/alaveteli_pro/subscription_with_discount_spec.rb b/spec/models/alaveteli_pro/subscription_with_discount_spec.rb
index cad1525a53..284eb7c322 100644
--- a/spec/models/alaveteli_pro/subscription_with_discount_spec.rb
+++ b/spec/models/alaveteli_pro/subscription_with_discount_spec.rb
@@ -12,7 +12,7 @@
                           trial_start: trial_start, trial_end: trial_end)
   end
 
-  subject { described_class.new(subscription) }
+  subject { AlaveteliPro::Subscription.new(subscription) }
 
   describe '#amount' do
     context 'no discount is set' do

From 19769b187e2ebe564d3f679548cb932fe36a77e0 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 8 Oct 2024 10:49:17 +0100
Subject: [PATCH 06/22] Remove Pro SubscriptionWithDiscount initializer

This really isn't needed now this module is included in Pro
Subscription.
---
 .../subscription_with_discount.rb             | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/app/models/alaveteli_pro/subscription_with_discount.rb b/app/models/alaveteli_pro/subscription_with_discount.rb
index 58546cfd8d..6c44dfec0d 100644
--- a/app/models/alaveteli_pro/subscription_with_discount.rb
+++ b/app/models/alaveteli_pro/subscription_with_discount.rb
@@ -6,7 +6,7 @@
 #   # subscription with 50% off 'forever' discount
 #   subscription = Stripe::Subscription.retrieve('sub_1234')
 #   @subscription = AlaveteliPro::Subscription.new(subscription)
-#   @subscription.original_amount
+#   @subscription.plan.amount
 #   # => 833
 #   @subscription.amount
 #   # => 416
@@ -15,23 +15,14 @@
 #   @subscription.free?
 #   # => false
 module AlaveteliPro::SubscriptionWithDiscount
-  attr_reader :original_amount, :coupon
-
-  def initialize(subscription)
-    super
-    @original_amount = subscription.plan.amount
-    @discount = subscription.discount
-    @coupon = fetch_coupon
-  end
-
   def amount
-    net = BigDecimal((original_amount * 0.01), 0).round(2)
+    net = BigDecimal((plan.amount * 0.01), 0).round(2)
     net -= reduction(net)
     (net * 100).floor
   end
 
   def discounted?
-    amount < original_amount
+    amount < plan.amount
   end
 
   def discount_name
@@ -56,8 +47,8 @@ def trial?
     trial_start && trial_end
   end
 
-  def fetch_coupon
-    discount.coupon if discount && discount.coupon.valid
+  def coupon
+    @coupon ||= discount.coupon if discount && discount.coupon.valid
   end
 
   def reduction(net)

From da133f88a0107a9f63d8f095d801c573d93d85d3 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 8 Oct 2024 10:55:43 +0100
Subject: [PATCH 07/22] Update Pro Subscription discount calculation

Use whole integers instead of floats which can cause rounding errors.
---
 .../alaveteli_pro/subscription_with_discount.rb  | 16 +++++++---------
 .../subscription_with_discount_spec.rb           |  2 +-
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/app/models/alaveteli_pro/subscription_with_discount.rb b/app/models/alaveteli_pro/subscription_with_discount.rb
index 6c44dfec0d..a67683222d 100644
--- a/app/models/alaveteli_pro/subscription_with_discount.rb
+++ b/app/models/alaveteli_pro/subscription_with_discount.rb
@@ -16,9 +16,7 @@
 #   # => false
 module AlaveteliPro::SubscriptionWithDiscount
   def amount
-    net = BigDecimal((plan.amount * 0.01), 0).round(2)
-    net -= reduction(net)
-    (net * 100).floor
+    plan.amount - reduction
   end
 
   def discounted?
@@ -51,21 +49,21 @@ def coupon
     @coupon ||= discount.coupon if discount && discount.coupon.valid
   end
 
-  def reduction(net)
+  def reduction
     if coupon?
-      coupon_reduction(net)
+      coupon_reduction
     elsif trial?
-      net
+      plan.amount
     else
       0
     end
   end
 
-  def coupon_reduction(net)
+  def coupon_reduction
     if coupon.amount_off
-      BigDecimal((coupon.amount_off * 0.01), 0).round(2)
+      coupon.amount_off
     else
-      (net * coupon.percent_off / 100)
+      (plan.amount * coupon.percent_off / 100)
     end
   end
 end
diff --git a/spec/models/alaveteli_pro/subscription_with_discount_spec.rb b/spec/models/alaveteli_pro/subscription_with_discount_spec.rb
index 284eb7c322..1d14161445 100644
--- a/spec/models/alaveteli_pro/subscription_with_discount_spec.rb
+++ b/spec/models/alaveteli_pro/subscription_with_discount_spec.rb
@@ -28,7 +28,7 @@
       end
 
       it 'applies a percentage discount correctly' do
-        expect(subject.amount).to eq(416)
+        expect(subject.amount).to eq(417)
       end
     end
 

From 978eb4110a2cc8348afcb4ecc716b7999493e9b3 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 8 Oct 2024 10:56:24 +0100
Subject: [PATCH 08/22] Rename Pro Subscription#amount

Change to `#discounted_amount` to better represent when this method is
returning.
---
 app/models/alaveteli_pro/subscription_with_discount.rb |  8 ++++----
 .../alaveteli_pro/subscription_with_discount_spec.rb   | 10 +++++-----
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/app/models/alaveteli_pro/subscription_with_discount.rb b/app/models/alaveteli_pro/subscription_with_discount.rb
index a67683222d..066889abe5 100644
--- a/app/models/alaveteli_pro/subscription_with_discount.rb
+++ b/app/models/alaveteli_pro/subscription_with_discount.rb
@@ -8,19 +8,19 @@
 #   @subscription = AlaveteliPro::Subscription.new(subscription)
 #   @subscription.plan.amount
 #   # => 833
-#   @subscription.amount
+#   @subscription.discounted_amount
 #   # => 416
 #   @subscription.discounted?
 #   # => true
 #   @subscription.free?
 #   # => false
 module AlaveteliPro::SubscriptionWithDiscount
-  def amount
+  def discounted_amount
     plan.amount - reduction
   end
 
   def discounted?
-    amount < plan.amount
+    discounted_amount < plan.amount
   end
 
   def discount_name
@@ -32,7 +32,7 @@ def discount_name
   end
 
   def free?
-    amount == 0
+    discounted_amount == 0
   end
 
   private
diff --git a/spec/models/alaveteli_pro/subscription_with_discount_spec.rb b/spec/models/alaveteli_pro/subscription_with_discount_spec.rb
index 1d14161445..e109a58afa 100644
--- a/spec/models/alaveteli_pro/subscription_with_discount_spec.rb
+++ b/spec/models/alaveteli_pro/subscription_with_discount_spec.rb
@@ -14,10 +14,10 @@
 
   subject { AlaveteliPro::Subscription.new(subscription) }
 
-  describe '#amount' do
+  describe '#discounted_amount' do
     context 'no discount is set' do
       it 'returns the original stripe plan amount' do
-        expect(subject.amount).to eq(833)
+        expect(subject.discounted_amount).to eq(833)
       end
     end
 
@@ -28,7 +28,7 @@
       end
 
       it 'applies a percentage discount correctly' do
-        expect(subject.amount).to eq(417)
+        expect(subject.discounted_amount).to eq(417)
       end
     end
 
@@ -38,7 +38,7 @@
       end
 
       it 'applies an amount_off discount correctly' do
-        expect(subject.amount).to eq(633)
+        expect(subject.discounted_amount).to eq(633)
       end
     end
 
@@ -46,7 +46,7 @@
       let(:trial) { true }
 
       it 'returns 0' do
-        expect(subject.amount).to eq(0)
+        expect(subject.discounted_amount).to eq(0)
       end
     end
   end

From f89b0bf15fdda26c3febe5cf07438b9219cbe3a3 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 8 Oct 2024 10:59:48 +0100
Subject: [PATCH 09/22] Rename SubscriptionWithDiscount

Move Discount module under Pro Subscription namespace.
---
 app/models/alaveteli_pro/subscription.rb                        | 2 +-
 .../{subscription_with_discount.rb => subscription/discount.rb} | 2 +-
 .../discount_spec.rb}                                           | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
 rename app/models/alaveteli_pro/{subscription_with_discount.rb => subscription/discount.rb} (96%)
 rename spec/models/alaveteli_pro/{subscription_with_discount_spec.rb => subscrioption/discount_spec.rb} (98%)

diff --git a/app/models/alaveteli_pro/subscription.rb b/app/models/alaveteli_pro/subscription.rb
index f2cc3354ae..df6b69ac33 100644
--- a/app/models/alaveteli_pro/subscription.rb
+++ b/app/models/alaveteli_pro/subscription.rb
@@ -4,7 +4,7 @@ module AlaveteliPro
   # and to add useful helper methods.
   #
   class Subscription < SimpleDelegator
-    include SubscriptionWithDiscount
+    include Subscription::Discount
 
     # state
     def active?
diff --git a/app/models/alaveteli_pro/subscription_with_discount.rb b/app/models/alaveteli_pro/subscription/discount.rb
similarity index 96%
rename from app/models/alaveteli_pro/subscription_with_discount.rb
rename to app/models/alaveteli_pro/subscription/discount.rb
index 066889abe5..f237be4d7b 100644
--- a/app/models/alaveteli_pro/subscription_with_discount.rb
+++ b/app/models/alaveteli_pro/subscription/discount.rb
@@ -14,7 +14,7 @@
 #   # => true
 #   @subscription.free?
 #   # => false
-module AlaveteliPro::SubscriptionWithDiscount
+module AlaveteliPro::Subscription::Discount
   def discounted_amount
     plan.amount - reduction
   end
diff --git a/spec/models/alaveteli_pro/subscription_with_discount_spec.rb b/spec/models/alaveteli_pro/subscrioption/discount_spec.rb
similarity index 98%
rename from spec/models/alaveteli_pro/subscription_with_discount_spec.rb
rename to spec/models/alaveteli_pro/subscrioption/discount_spec.rb
index e109a58afa..9e0d218ced 100644
--- a/spec/models/alaveteli_pro/subscription_with_discount_spec.rb
+++ b/spec/models/alaveteli_pro/subscrioption/discount_spec.rb
@@ -1,6 +1,6 @@
 require 'spec_helper'
 
-RSpec.describe AlaveteliPro::SubscriptionWithDiscount do
+RSpec.describe AlaveteliPro::Subscription::Discount do
   let(:plan) { double(:plan, amount: 833) }
   let(:coupon) { nil }
   let(:trial) { nil }

From 3cf4414993141089222eba6edc74ae7c00576b74 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Tue, 8 Oct 2024 12:21:51 +0100
Subject: [PATCH 10/22] Improve rendering of subscription views

- Show discount price and name instead of coupon code
- Remove leading space around billing date
- Minor CSS layout changes to ensure subscription sign up and
  subscription show views are consistent.
---
 .../responsive/_settings_layout.scss          | 13 +++------
 .../responsive/_settings_style.scss           |  5 ----
 .../alaveteli_pro/subscription/discount.rb    |  6 ++++-
 app/views/alaveteli_pro/plans/show.html.erb   |  8 ++----
 .../_cancel_subscription.html.erb             |  2 +-
 .../subscriptions/_subscription.html.erb      | 27 +++++++++----------
 .../subscrioption/discount_spec.rb            | 25 ++++++++++++++---
 7 files changed, 45 insertions(+), 41 deletions(-)

diff --git a/app/assets/stylesheets/responsive/_settings_layout.scss b/app/assets/stylesheets/responsive/_settings_layout.scss
index acece5b1e6..f87f8faecc 100644
--- a/app/assets/stylesheets/responsive/_settings_layout.scss
+++ b/app/assets/stylesheets/responsive/_settings_layout.scss
@@ -43,21 +43,14 @@ $settings-collapse: 50em; //where the dashboard layout needs to change
 .plan-overview__desc {
   flex: 0 0 100%;
   @include respond-min( $settings-collapse ){
-    flex: 0 0 30%;
-  }
-}
-
-.plan-overview__details{
-  flex: 0 0 70%;
-  @include respond-min( $settings-collapse ){
-    flex: 0 0 30%;
+    flex: 0 0 40%;
   }
 }
 
 .plan-overview__amount {
-  flex: 0 0 30%;
+  flex: 0 0 100%;
   @include respond-min( $settings-collapse ){
-    flex: 0 0 30%;
+    flex: 0 0 60%;
   }
 }
 
diff --git a/app/assets/stylesheets/responsive/_settings_style.scss b/app/assets/stylesheets/responsive/_settings_style.scss
index 7f8c13daea..bcf8586712 100644
--- a/app/assets/stylesheets/responsive/_settings_style.scss
+++ b/app/assets/stylesheets/responsive/_settings_style.scss
@@ -20,11 +20,6 @@
   font-weight: bold;
 }
 
-.plan-overview__details{
-  font-size: 0.875em;
-  color: #787774;
-}
-
 .plan-overview__amount {
   font-size: 1em;
   text-align: right;
diff --git a/app/models/alaveteli_pro/subscription/discount.rb b/app/models/alaveteli_pro/subscription/discount.rb
index f237be4d7b..d7234518db 100644
--- a/app/models/alaveteli_pro/subscription/discount.rb
+++ b/app/models/alaveteli_pro/subscription/discount.rb
@@ -15,6 +15,10 @@
 #   @subscription.free?
 #   # => false
 module AlaveteliPro::Subscription::Discount
+  include Taxable
+
+  tax :discounted_amount
+
   def discounted_amount
     plan.amount - reduction
   end
@@ -25,7 +29,7 @@ def discounted?
 
   def discount_name
     if coupon?
-      coupon.id
+      coupon.name
     elsif trial?
       'PROBETA'
     end
diff --git a/app/views/alaveteli_pro/plans/show.html.erb b/app/views/alaveteli_pro/plans/show.html.erb
index f878c5afe2..be2512ba31 100644
--- a/app/views/alaveteli_pro/plans/show.html.erb
+++ b/app/views/alaveteli_pro/plans/show.html.erb
@@ -31,14 +31,10 @@
             <div class="plan-overview__desc">
               <%= @plan.product.name %>
             </div>
-            <div class="plan-overview__details">
+            <div class="plan-overview__amount">
+              <%= format_currency(@plan.amount_with_tax) %>
               <%= billing_frequency(@plan.interval) %>
             </div>
-            <label class="plan-overview__amount amount">
-              <span>
-              <%= format_currency(@plan.amount_with_tax) %>
-              </span>
-            </label>
           </div>
         </div>
 
diff --git a/app/views/alaveteli_pro/subscriptions/_cancel_subscription.html.erb b/app/views/alaveteli_pro/subscriptions/_cancel_subscription.html.erb
index 9dcf283dcf..165cdb731c 100644
--- a/app/views/alaveteli_pro/subscriptions/_cancel_subscription.html.erb
+++ b/app/views/alaveteli_pro/subscriptions/_cancel_subscription.html.erb
@@ -26,7 +26,7 @@
     <%= _('Subscription cancellations are applied at the end of the ' \
           'current billing period (<strong>{{date}}</strong>).',
           date: simple_date(Time.zone.at(subscription.current_period_end),
-                            format: :text)) %>
+                            format: :text).strip) %>
     <%= _('Until that date you will continue to have full access to ' \
           '{{pro_site_name}}.',
           pro_site_name: pro_site_name) %>
diff --git a/app/views/alaveteli_pro/subscriptions/_subscription.html.erb b/app/views/alaveteli_pro/subscriptions/_subscription.html.erb
index ce6ee7e9ce..0bb9a233a7 100644
--- a/app/views/alaveteli_pro/subscriptions/_subscription.html.erb
+++ b/app/views/alaveteli_pro/subscriptions/_subscription.html.erb
@@ -4,22 +4,19 @@
     <%= subscription.plan.product.name %>
   </div>
 
-  <div class="plan-overview__details">
+  <div class="plan-overview__amount">
+    <%= format_currency(subscription.plan.amount_with_tax) %>
+    <%= billing_frequency(subscription.plan.interval) %>
+    <% if subscription.discounted? %>
+      <br>
+      <%= _('<strong>{{discounted_amount}}</strong> with discount ' \
+            '<strong>{{discount_name}}</strong> applied',
+            discount_name: subscription.discount_name,
+            discounted_amount: format_currency(
+              subscription.discounted_amount_with_tax
+            )) %>
+    <% end %>
   </div>
-
-  <label class="plan-overview__amount amount">
-    <span>
-      <%= format_currency(subscription.plan.amount_with_tax) %>
-      <span class="amount__freq">
-        <%= billing_frequency(subscription.plan.interval) %>
-
-        <% if subscription.discounted? %>
-          <%= _('(with <b>{{discount_name}}</b> discount applied)',
-                :discount_name => subscription.discount_name) %>
-        <% end %>
-      </span>
-    </span>
-  </label>
 </div>
 
 <div>
diff --git a/spec/models/alaveteli_pro/subscrioption/discount_spec.rb b/spec/models/alaveteli_pro/subscrioption/discount_spec.rb
index 9e0d218ced..e7cf8a7c94 100644
--- a/spec/models/alaveteli_pro/subscrioption/discount_spec.rb
+++ b/spec/models/alaveteli_pro/subscrioption/discount_spec.rb
@@ -14,6 +14,25 @@
 
   subject { AlaveteliPro::Subscription.new(subscription) }
 
+  describe '#discounted_amount_with_tax' do
+    context 'with the default tax rate' do
+      it 'adds 20% tax to the plan amount' do
+        expect(subject.discounted_amount_with_tax).to eq(1000)
+      end
+    end
+
+    context 'with a custom tax rate' do
+      before do
+        allow(AlaveteliConfiguration).
+          to receive(:stripe_tax_rate).and_return('0.25')
+      end
+
+      it 'adds 25% tax to the plan amount' do
+        expect(subject.discounted_amount_with_tax).to eq(1041)
+      end
+    end
+  end
+
   describe '#discounted_amount' do
     context 'no discount is set' do
       it 'returns the original stripe plan amount' do
@@ -95,11 +114,11 @@
 
     context 'with a coupon' do
       let(:coupon) do
-        double(:coupon, id: 'COUPON_ID', valid: true)
+        double(:coupon, name: 'COUPON_NAME', valid: true)
       end
 
-      it 'returns ID of coupon' do
-        expect(subject.discount_name).to eq('COUPON_ID')
+      it 'returns name of coupon' do
+        expect(subject.discount_name).to eq('COUPON_NAME')
       end
     end
 

From c61de8d9f183e4d078cf67ba15e10765f4764422 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Thu, 17 Oct 2024 13:04:41 +0100
Subject: [PATCH 11/22] Add AlaveteliPro::AccountHelper tests

---
 .../alaveteli_pro/account_helper_spec.rb      | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 spec/helpers/alaveteli_pro/account_helper_spec.rb

diff --git a/spec/helpers/alaveteli_pro/account_helper_spec.rb b/spec/helpers/alaveteli_pro/account_helper_spec.rb
new file mode 100644
index 0000000000..f615de2022
--- /dev/null
+++ b/spec/helpers/alaveteli_pro/account_helper_spec.rb
@@ -0,0 +1,33 @@
+require 'spec_helper'
+
+describe AlaveteliPro::AccountHelper do
+  describe '#billing_frequency' do
+    it 'returns correct billing frequency for day' do
+      expect(helper.billing_frequency('day')).to eq('Billed: Daily')
+    end
+
+    it 'returns correct billing frequency for week' do
+      expect(helper.billing_frequency('week')).to eq('Billed: Weekly')
+    end
+
+    it 'returns correct billing frequency for month' do
+      expect(helper.billing_frequency('month')).to eq('Billed: Monthly')
+    end
+
+    it 'returns correct billing frequency for year' do
+      expect(helper.billing_frequency('year')).to eq('Billed: Annually')
+    end
+  end
+
+  describe '#card_expiry_message' do
+    it 'returns expiry message when card expires this month' do
+      allow(Date).to receive(:today).and_return(Date.new(2023, 5, 1))
+      expect(helper.card_expiry_message(5, 2023)).to eq('<p class="card__expiring">Expires soon</p>')
+    end
+
+    it 'returns nil when card does not expire this month' do
+      allow(Date).to receive(:today).and_return(Date.new(2023, 5, 1))
+      expect(helper.card_expiry_message(6, 2023)).to be_nil
+    end
+  end
+end

From ad10207fbd5f370266bf9aaac374eb8d41563ed6 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Thu, 17 Oct 2024 13:08:39 +0100
Subject: [PATCH 12/22] Extract billing helper method into its own file

---
 app/helpers/alaveteli_pro/account_helper.rb   | 13 ------------
 app/helpers/alaveteli_pro/plan_helper.rb      | 18 ++++++++++++++++
 .../alaveteli_pro/account_helper_spec.rb      | 18 ----------------
 .../helpers/alaveteli_pro/plan_helper_spec.rb | 21 +++++++++++++++++++
 4 files changed, 39 insertions(+), 31 deletions(-)
 create mode 100644 app/helpers/alaveteli_pro/plan_helper.rb
 create mode 100644 spec/helpers/alaveteli_pro/plan_helper_spec.rb

diff --git a/app/helpers/alaveteli_pro/account_helper.rb b/app/helpers/alaveteli_pro/account_helper.rb
index ab9d526026..71a4287b29 100644
--- a/app/helpers/alaveteli_pro/account_helper.rb
+++ b/app/helpers/alaveteli_pro/account_helper.rb
@@ -1,17 +1,4 @@
 module AlaveteliPro::AccountHelper
-  def billing_frequency(billing_unit)
-    case billing_unit
-    when 'day'
-      _('Billed: Daily')
-    when 'week'
-      _('Billed: Weekly')
-    when 'month'
-      _('Billed: Monthly')
-    when 'year'
-      _('Billed: Annually')
-    end
-  end
-
   def card_expiry_message(month, year)
     if month == Date.today.month && year == Date.today.year
       content_tag(:p, _('Expires soon'), class: 'card__expiring')
diff --git a/app/helpers/alaveteli_pro/plan_helper.rb b/app/helpers/alaveteli_pro/plan_helper.rb
new file mode 100644
index 0000000000..e357dd1c9b
--- /dev/null
+++ b/app/helpers/alaveteli_pro/plan_helper.rb
@@ -0,0 +1,18 @@
+##
+# Helper methods for formatting and displaying billing and plan information
+# in the Alaveteli Pro interface
+#
+module AlaveteliPro::PlanHelper
+  def billing_frequency(billing_unit)
+    case billing_unit
+    when 'day'
+      _('Billed: Daily')
+    when 'week'
+      _('Billed: Weekly')
+    when 'month'
+      _('Billed: Monthly')
+    when 'year'
+      _('Billed: Annually')
+    end
+  end
+end
diff --git a/spec/helpers/alaveteli_pro/account_helper_spec.rb b/spec/helpers/alaveteli_pro/account_helper_spec.rb
index f615de2022..722808dc24 100644
--- a/spec/helpers/alaveteli_pro/account_helper_spec.rb
+++ b/spec/helpers/alaveteli_pro/account_helper_spec.rb
@@ -1,24 +1,6 @@
 require 'spec_helper'
 
 describe AlaveteliPro::AccountHelper do
-  describe '#billing_frequency' do
-    it 'returns correct billing frequency for day' do
-      expect(helper.billing_frequency('day')).to eq('Billed: Daily')
-    end
-
-    it 'returns correct billing frequency for week' do
-      expect(helper.billing_frequency('week')).to eq('Billed: Weekly')
-    end
-
-    it 'returns correct billing frequency for month' do
-      expect(helper.billing_frequency('month')).to eq('Billed: Monthly')
-    end
-
-    it 'returns correct billing frequency for year' do
-      expect(helper.billing_frequency('year')).to eq('Billed: Annually')
-    end
-  end
-
   describe '#card_expiry_message' do
     it 'returns expiry message when card expires this month' do
       allow(Date).to receive(:today).and_return(Date.new(2023, 5, 1))
diff --git a/spec/helpers/alaveteli_pro/plan_helper_spec.rb b/spec/helpers/alaveteli_pro/plan_helper_spec.rb
new file mode 100644
index 0000000000..ef17ac3c81
--- /dev/null
+++ b/spec/helpers/alaveteli_pro/plan_helper_spec.rb
@@ -0,0 +1,21 @@
+require 'spec_helper'
+
+describe AlaveteliPro::PlanHelper do
+  describe '#billing_frequency' do
+    it 'returns correct billing frequency for day' do
+      expect(helper.billing_frequency('day')).to eq('Billed: Daily')
+    end
+
+    it 'returns correct billing frequency for week' do
+      expect(helper.billing_frequency('week')).to eq('Billed: Weekly')
+    end
+
+    it 'returns correct billing frequency for month' do
+      expect(helper.billing_frequency('month')).to eq('Billed: Monthly')
+    end
+
+    it 'returns correct billing frequency for year' do
+      expect(helper.billing_frequency('year')).to eq('Billed: Annually')
+    end
+  end
+end

From 49f3ec7a7c85a47324cc9d95bff41a7d8168f100 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Fri, 11 Oct 2024 17:02:38 +0100
Subject: [PATCH 13/22] Update billing helper methods

Improve billing frequency helper method:
1. accept Stripe Plan as and argument
2. take into account plans interval count
3. work with all interval types

Add billing interval helper method to output `per user, per month` or
`per user, every 2 months` depending on how the Stripe Plan has been
configured.
---
 app/helpers/alaveteli_pro/plan_helper.rb      | 39 +++++++++++++---
 .../plans/_pricing_tiers.html.erb             | 18 ++------
 app/views/alaveteli_pro/plans/show.html.erb   |  2 +-
 .../subscriptions/_subscription.html.erb      |  2 +-
 .../helpers/alaveteli_pro/plan_helper_spec.rb | 46 +++++++++++++++----
 .../plans/index.html.erb_spec.rb              |  1 +
 6 files changed, 79 insertions(+), 29 deletions(-)

diff --git a/app/helpers/alaveteli_pro/plan_helper.rb b/app/helpers/alaveteli_pro/plan_helper.rb
index e357dd1c9b..7bc3fb5065 100644
--- a/app/helpers/alaveteli_pro/plan_helper.rb
+++ b/app/helpers/alaveteli_pro/plan_helper.rb
@@ -3,16 +3,43 @@
 # in the Alaveteli Pro interface
 #
 module AlaveteliPro::PlanHelper
-  def billing_frequency(billing_unit)
-    case billing_unit
-    when 'day'
+  def billing_frequency(plan)
+    if interval(plan) == 'day' && interval_count(plan) == 1
       _('Billed: Daily')
-    when 'week'
+    elsif interval(plan) == 'week' && interval_count(plan) == 1
       _('Billed: Weekly')
-    when 'month'
+    elsif interval(plan) == 'month' && interval_count(plan) == 1
       _('Billed: Monthly')
-    when 'year'
+    elsif interval(plan) == 'year' && interval_count(plan) == 1
       _('Billed: Annually')
+    else
+      _('Billed: every {{interval}}', interval: interval(plan))
     end
   end
+
+  def billing_interval(plan)
+    if interval_count(plan) == 1
+      _('per user, per {{interval}}', interval: pluralize_interval(plan))
+    else
+      _('per user, every {{interval}}', interval: pluralize_interval(plan))
+    end
+  end
+
+  private
+
+  def pluralize_interval(plan)
+    count = interval_count(plan)
+    interval = interval(plan)
+    return interval if count == 1
+
+    pluralize(count, interval)
+  end
+
+  def interval(plan)
+    plan.interval
+  end
+
+  def interval_count(plan)
+    plan.interval_count
+  end
 end
diff --git a/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb b/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb
index 5f4c5b7452..7cc771ce25 100644
--- a/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb
+++ b/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb
@@ -8,19 +8,11 @@
       </p>
 
       <p class="price-label">
-        <% if @plan.interval == 'month' %>
-          <%= _('<span class="price-label__amount">{{monthly_price}}</span> ' \
-                'per user, per month',
-                monthly_price: format_currency(
-                                 @plan.amount_with_tax,
-                                 no_cents_if_whole: true)) %>
-        <% elsif @plan.interval == 'year' %>
-          <%= _('<span class="price-label__amount">{{yearly_price}}</span> ' \
-                'per user, per year',
-                yearly_price: format_currency(
-                                @plan.amount_with_tax,
-                                no_cents_if_whole: true)) %>
-        <% end %>
+        <span class="price-label__amount">
+          <%= format_currency(@plan.amount_with_tax, no_cents_if_whole: true) %>
+        </span>
+
+        <%= billing_interval(@plan) %>
       </p>
     </div>
 
diff --git a/app/views/alaveteli_pro/plans/show.html.erb b/app/views/alaveteli_pro/plans/show.html.erb
index be2512ba31..16fb3eedce 100644
--- a/app/views/alaveteli_pro/plans/show.html.erb
+++ b/app/views/alaveteli_pro/plans/show.html.erb
@@ -33,7 +33,7 @@
             </div>
             <div class="plan-overview__amount">
               <%= format_currency(@plan.amount_with_tax) %>
-              <%= billing_frequency(@plan.interval) %>
+              <%= billing_frequency(@plan) %>
             </div>
           </div>
         </div>
diff --git a/app/views/alaveteli_pro/subscriptions/_subscription.html.erb b/app/views/alaveteli_pro/subscriptions/_subscription.html.erb
index 0bb9a233a7..d46ad91a98 100644
--- a/app/views/alaveteli_pro/subscriptions/_subscription.html.erb
+++ b/app/views/alaveteli_pro/subscriptions/_subscription.html.erb
@@ -6,7 +6,7 @@
 
   <div class="plan-overview__amount">
     <%= format_currency(subscription.plan.amount_with_tax) %>
-    <%= billing_frequency(subscription.plan.interval) %>
+    <%= billing_frequency(subscription.plan) %>
     <% if subscription.discounted? %>
       <br>
       <%= _('<strong>{{discounted_amount}}</strong> with discount ' \
diff --git a/spec/helpers/alaveteli_pro/plan_helper_spec.rb b/spec/helpers/alaveteli_pro/plan_helper_spec.rb
index ef17ac3c81..bdcab955b1 100644
--- a/spec/helpers/alaveteli_pro/plan_helper_spec.rb
+++ b/spec/helpers/alaveteli_pro/plan_helper_spec.rb
@@ -1,21 +1,51 @@
 require 'spec_helper'
 
 describe AlaveteliPro::PlanHelper do
+  let(:plan) { double('Plan') }
+
   describe '#billing_frequency' do
-    it 'returns correct billing frequency for day' do
-      expect(helper.billing_frequency('day')).to eq('Billed: Daily')
+    it 'returns "Billed: Daily" for daily plan' do
+      allow(plan).to receive(:interval).and_return('day')
+      allow(plan).to receive(:interval_count).and_return(1)
+      expect(helper.billing_frequency(plan)).to eq('Billed: Daily')
+    end
+
+    it 'returns "Billed: Weekly" for weekly plan' do
+      allow(plan).to receive(:interval).and_return('week')
+      allow(plan).to receive(:interval_count).and_return(1)
+      expect(helper.billing_frequency(plan)).to eq('Billed: Weekly')
+    end
+
+    it 'returns "Billed: Monthly" for monthly plan' do
+      allow(plan).to receive(:interval).and_return('month')
+      allow(plan).to receive(:interval_count).and_return(1)
+      expect(helper.billing_frequency(plan)).to eq('Billed: Monthly')
+    end
+
+    it 'returns "Billed: Annually" for yearly plan' do
+      allow(plan).to receive(:interval).and_return('year')
+      allow(plan).to receive(:interval_count).and_return(1)
+      expect(helper.billing_frequency(plan)).to eq('Billed: Annually')
     end
 
-    it 'returns correct billing frequency for week' do
-      expect(helper.billing_frequency('week')).to eq('Billed: Weekly')
+    it 'returns custom message for other intervals' do
+      allow(plan).to receive(:interval).and_return('quarter')
+      allow(plan).to receive(:interval_count).and_return(1)
+      expect(helper.billing_frequency(plan)).to eq('Billed: every quarter')
     end
+  end
 
-    it 'returns correct billing frequency for month' do
-      expect(helper.billing_frequency('month')).to eq('Billed: Monthly')
+  describe '#billing_interval' do
+    it 'returns singular interval for interval_count of 1' do
+      allow(plan).to receive(:interval).and_return('month')
+      allow(plan).to receive(:interval_count).and_return(1)
+      expect(helper.billing_interval(plan)).to eq('per user, per month')
     end
 
-    it 'returns correct billing frequency for year' do
-      expect(helper.billing_frequency('year')).to eq('Billed: Annually')
+    it 'returns plural interval for interval_count greater than 1' do
+      allow(plan).to receive(:interval).and_return('month')
+      allow(plan).to receive(:interval_count).and_return(3)
+      expect(helper.billing_interval(plan)).to eq('per user, every 3 months')
     end
   end
 end
diff --git a/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb b/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb
index 7261098758..2aa55c7a32 100644
--- a/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb
+++ b/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb
@@ -1,4 +1,5 @@
 require 'spec_helper'
+require 'stripe_mock'
 
 RSpec.describe 'alaveteli_pro/plans/index' do
   before { StripeMock.start }

From 89debb6e6cc4ce9913f822caf3fd46782258b48c Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Fri, 11 Oct 2024 17:02:38 +0100
Subject: [PATCH 14/22] Update Stripe namespace concern

Handle strings correctly irrespective of whether they start with the
namespace or not.
---
 .../concerns/alaveteli_pro/stripe_namespace.rb         | 10 ++++++++--
 .../concerns/alaveteli_pro/stripe_namespace_spec.rb    |  9 +++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/app/controllers/concerns/alaveteli_pro/stripe_namespace.rb b/app/controllers/concerns/alaveteli_pro/stripe_namespace.rb
index 8e437b3797..ea9829ac34 100644
--- a/app/controllers/concerns/alaveteli_pro/stripe_namespace.rb
+++ b/app/controllers/concerns/alaveteli_pro/stripe_namespace.rb
@@ -2,11 +2,17 @@ module AlaveteliPro::StripeNamespace
   extend ActiveSupport::Concern
 
   def add_stripe_namespace(string)
-    namespace.blank? ? string : [namespace, string].join('-')
+    return string if namespace.blank?
+    return string if string.start_with?(/#{namespace}-/)
+
+    [namespace, string].join('-')
   end
 
   def remove_stripe_namespace(string)
-    namespace.blank? ? string : string.sub(/^#{namespace}-/, '')
+    return string if namespace.blank?
+    return string unless string.start_with?(/#{namespace}-/)
+
+    string.sub(/^#{namespace}-/, '')
   end
 
   private
diff --git a/spec/controllers/concerns/alaveteli_pro/stripe_namespace_spec.rb b/spec/controllers/concerns/alaveteli_pro/stripe_namespace_spec.rb
index 4f65320313..75015a2fe3 100644
--- a/spec/controllers/concerns/alaveteli_pro/stripe_namespace_spec.rb
+++ b/spec/controllers/concerns/alaveteli_pro/stripe_namespace_spec.rb
@@ -10,12 +10,21 @@
     end
 
     describe '#add_stripe_namespace' do
+      it 'returns string if it already starts with the namespce' do
+        expect(add_stripe_namespace('namespace-string')).
+          to eq('namespace-string')
+      end
+
       it 'prepend namespace to string' do
         expect(add_stripe_namespace('string')).to eq('namespace-string')
       end
     end
 
     describe '#remove_stripe_namespace' do
+      it 'returns string if does not start with the namespace' do
+        expect(remove_stripe_namespace('string')).to eq('string')
+      end
+
       it 'removes namespace from string' do
         expect(remove_stripe_namespace('namespace-string')).to eq('string')
       end

From 75de33c59e805b421e7e9130a86e8f7204be2731 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Mon, 4 Nov 2024 12:05:07 +0000
Subject: [PATCH 15/22] Move Pro StripeNamespace concern

---
 .../concerns/alaveteli_pro/stripe_namespace.rb               | 5 +++++
 .../concerns/alaveteli_pro/stripe_namespace_spec.rb          | 0
 2 files changed, 5 insertions(+)
 rename app/{controllers => models}/concerns/alaveteli_pro/stripe_namespace.rb (71%)
 rename spec/{controllers => models}/concerns/alaveteli_pro/stripe_namespace_spec.rb (100%)

diff --git a/app/controllers/concerns/alaveteli_pro/stripe_namespace.rb b/app/models/concerns/alaveteli_pro/stripe_namespace.rb
similarity index 71%
rename from app/controllers/concerns/alaveteli_pro/stripe_namespace.rb
rename to app/models/concerns/alaveteli_pro/stripe_namespace.rb
index ea9829ac34..feb7ce217a 100644
--- a/app/controllers/concerns/alaveteli_pro/stripe_namespace.rb
+++ b/app/models/concerns/alaveteli_pro/stripe_namespace.rb
@@ -1,3 +1,8 @@
+##
+# This module is a concern that provides functionality for handling namespaced
+# Stripe identifiers. The namespace helps prevent collisions between Stripe
+# objects in different applications.
+#
 module AlaveteliPro::StripeNamespace
   extend ActiveSupport::Concern
 
diff --git a/spec/controllers/concerns/alaveteli_pro/stripe_namespace_spec.rb b/spec/models/concerns/alaveteli_pro/stripe_namespace_spec.rb
similarity index 100%
rename from spec/controllers/concerns/alaveteli_pro/stripe_namespace_spec.rb
rename to spec/models/concerns/alaveteli_pro/stripe_namespace_spec.rb

From 6f985755a1e52dca4f8c4a86661cc264ef341503 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Fri, 11 Oct 2024 17:02:38 +0100
Subject: [PATCH 16/22] Add Pro Plan list and retrieve methods

Allowing us to simplify the controller actions so we're always dealing
with Plan objects.
---
 .../alaveteli_pro/plans_controller.rb         | 18 +----
 .../alaveteli_pro/subscriptions_controller.rb | 19 ++----
 app/models/alaveteli_pro/plan.rb              | 24 +++++++
 .../plans/_pricing_tiers.html.erb             | 48 ++++++-------
 .../alaveteli_pro/plans_controller_spec.rb    |  2 +-
 .../subscriptions_controller_spec.rb          | 23 +++++--
 spec/models/alaveteli_pro/plan_spec.rb        | 67 +++++++++++++++++++
 .../plans/index.html.erb_spec.rb              |  2 +-
 8 files changed, 144 insertions(+), 59 deletions(-)

diff --git a/app/controllers/alaveteli_pro/plans_controller.rb b/app/controllers/alaveteli_pro/plans_controller.rb
index 61d9033568..63b4b15076 100644
--- a/app/controllers/alaveteli_pro/plans_controller.rb
+++ b/app/controllers/alaveteli_pro/plans_controller.rb
@@ -1,31 +1,19 @@
 class AlaveteliPro::PlansController < AlaveteliPro::BaseController
-  include AlaveteliPro::StripeNamespace
-
   skip_before_action :pro_user_authenticated?
   before_action :authenticate, :check_has_current_subscription, only: [:show]
 
   def index
-    default_plan_name = add_stripe_namespace('pro')
-    stripe_plan = Stripe::Plan.retrieve(default_plan_name)
-    @plan = AlaveteliPro::Plan.new(stripe_plan)
+    @plans = AlaveteliPro::Plan.list
     @pro_site_name = pro_site_name
   end
 
   def show
-    stripe_plan = Stripe::Plan.retrieve(
-      id: plan_name, expand: ['product']
-    )
-    @plan = AlaveteliPro::Plan.new(stripe_plan)
-  rescue Stripe::InvalidRequestError
-    raise ActiveRecord::RecordNotFound
+    @plan = AlaveteliPro::Plan.retrieve(params[:id])
+    @plan || raise(ActiveRecord::RecordNotFound)
   end
 
   private
 
-  def plan_name
-    add_stripe_namespace(params.require(:id))
-  end
-
   def authenticate
     authenticated? || ask_to_login(
       pro: true,
diff --git a/app/controllers/alaveteli_pro/subscriptions_controller.rb b/app/controllers/alaveteli_pro/subscriptions_controller.rb
index 1373f3a169..fc5c8becb7 100644
--- a/app/controllers/alaveteli_pro/subscriptions_controller.rb
+++ b/app/controllers/alaveteli_pro/subscriptions_controller.rb
@@ -7,7 +7,7 @@ class AlaveteliPro::SubscriptionsController < AlaveteliPro::BaseController
   before_action :authenticate, only: [:create, :authorise]
   before_action :check_allowed_to_subscribe_to_pro, only: [:create]
   before_action :prevent_duplicate_submission, only: [:create]
-  before_action :check_plan_exists, only: [:create]
+  before_action :load_plan, only: [:create]
   before_action :check_has_current_subscription, only: [:index]
 
   def index
@@ -49,7 +49,7 @@ def create
       @pro_account.update_stripe_customer
 
       attributes = {
-        plan: params.require(:plan_id),
+        plan: @plan.id,
         tax_percent: tax_percent,
         payment_behavior: 'allow_incomplete'
       }
@@ -83,7 +83,7 @@ def create
     end
 
     if flash[:error]
-      json_redirect_to plan_path(non_namespaced_plan_id)
+      json_redirect_to plan_path(@plan)
     else
       redirect_to authorise_subscription_path(@subscription.id)
     end
@@ -110,9 +110,7 @@ def authorise
       flash[:error] = _('There was a problem authorising your payment. You ' \
                         'have not been charged. Please try again.')
 
-      json_redirect_to plan_path(
-        remove_stripe_namespace(@subscription.plan.id)
-      )
+      json_redirect_to plan_path(@subscription.plan)
 
     elsif @subscription.active?
       current_user.add_role(:pro)
@@ -198,10 +196,6 @@ def check_has_current_subscription
     redirect_to pro_plans_path
   end
 
-  def non_namespaced_plan_id
-    remove_stripe_namespace(params[:plan_id]) if params[:plan_id]
-  end
-
   def coupon_code?
     params[:coupon_code].present?
   end
@@ -227,8 +221,9 @@ def tax_percent
     (BigDecimal(AlaveteliConfiguration.stripe_tax_rate).to_f * 100).to_f
   end
 
-  def check_plan_exists
-    redirect_to(pro_plans_path) unless non_namespaced_plan_id
+  def load_plan
+    @plan = AlaveteliPro::Plan.retrieve(params[:plan_id])
+    @plan || redirect_to(pro_plans_path)
   end
 
   def prevent_duplicate_submission
diff --git a/app/models/alaveteli_pro/plan.rb b/app/models/alaveteli_pro/plan.rb
index 6aa7b9c2d8..024c108abf 100644
--- a/app/models/alaveteli_pro/plan.rb
+++ b/app/models/alaveteli_pro/plan.rb
@@ -2,7 +2,31 @@
 # A wrapper for a Stripe::Plan
 #
 class AlaveteliPro::Plan < SimpleDelegator
+  extend AlaveteliPro::StripeNamespace
+  include AlaveteliPro::StripeNamespace
   include Taxable
 
   tax :amount
+
+  def self.list
+    [retrieve('pro')]
+  end
+
+  def self.retrieve(id)
+    new(Stripe::Plan.retrieve(add_stripe_namespace(id)))
+  rescue Stripe::InvalidRequestError
+    nil
+  end
+
+  def to_param
+    remove_stripe_namespace(id)
+  end
+
+  # product
+  def product
+    @product ||= (
+      product_id = __getobj__.product
+      Stripe::Product.retrieve(product_id) if product_id
+    )
+  end
 end
diff --git a/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb b/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb
index 7cc771ce25..9475367cbf 100644
--- a/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb
+++ b/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb
@@ -1,32 +1,34 @@
 <div class="pricing__tiers">
-  <div class="pricing__tier pricing__tier--primary">
-    <div class="pricing__tier__heading">
-      <h2><%= _('Professional') %></h2>
+  <% @plans.each do |plan| %>
+    <div class="pricing__tier pricing__tier--primary">
+      <div class="pricing__tier__heading">
+        <h2><%= _('Professional') %></h2>
 
-      <p class="pricing__tier__subhead">
-        <%= _('For journalists, academics and power users') %>
-      </p>
+        <p class="pricing__tier__subhead">
+          <%= _('For journalists, academics and power users') %>
+        </p>
 
-      <p class="price-label">
-        <span class="price-label__amount">
-          <%= format_currency(@plan.amount_with_tax, no_cents_if_whole: true) %>
-        </span>
+        <p class="price-label">
+          <span class="price-label__amount">
+            <%= format_currency(plan.amount_with_tax, no_cents_if_whole: true) %>
+          </span>
 
-        <%= billing_interval(@plan) %>
-      </p>
-    </div>
+          <%= billing_interval(plan) %>
+        </p>
+      </div>
 
-    <div class="pricing__tier__content">
-      <h3><%= _('Featuring') %></h3>
+      <div class="pricing__tier__content">
+        <h3><%= _('Featuring') %></h3>
 
-      <ul class="pricing__tier__features">
-        <li><%= _('Public requests for information') %></li>
-        <li><%= _('Private requests for information') %></li>
-        <li><%= _('Dashboard and workflow features') %></li>
-        <li><%= _('Friendly support') %></li>
-      </ul>
+        <ul class="pricing__tier__features">
+          <li><%= _('Public requests for information') %></li>
+          <li><%= _('Private requests for information') %></li>
+          <li><%= _('Dashboard and workflow features') %></li>
+          <li><%= _('Friendly support') %></li>
+        </ul>
 
-      <%= link_to _('Sign up'), plan_path('pro'), class: 'button button-pop' %>
+        <%= link_to _('Sign up'), plan_path(plan), class: 'button button-pop' %>
+      </div>
     </div>
-  </div>
+  <% end %>
 </div>
diff --git a/spec/controllers/alaveteli_pro/plans_controller_spec.rb b/spec/controllers/alaveteli_pro/plans_controller_spec.rb
index 1c689bd623..0a12f51164 100644
--- a/spec/controllers/alaveteli_pro/plans_controller_spec.rb
+++ b/spec/controllers/alaveteli_pro/plans_controller_spec.rb
@@ -41,7 +41,7 @@
     end
 
     it 'uses the default plan for pricing info' do
-      expect(assigns(:plan)).to eq(pro_plan)
+      expect(assigns(:plans)).to eq([pro_plan])
     end
   end
 
diff --git a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
index fd6d96524e..9c1b51a6ed 100644
--- a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
+++ b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
@@ -20,11 +20,6 @@
       amount_off: 1000,
       currency: 'gbp'
     )
-    stripe_helper.create_coupon(
-      id: 'ALAVETELI-COUPON_CODE',
-      amount_off: 1000,
-      currency: 'gbp'
-    )
     allow(AlaveteliConfiguration).
       to receive(:stripe_tax_rate).and_return('0.25')
   end
@@ -78,7 +73,7 @@
         end
 
         it 'subscribes the user to the plan' do
-          expect(assigns(:subscription).plan.id).to eq('pro')
+          expect(assigns(:subscription).plan.id).to eq(plan.id)
           expect(assigns(:pro_account).stripe_customer_id).
             to eq(assigns(:subscription).customer)
         end
@@ -166,10 +161,22 @@
       end
 
       context 'with Stripe namespace and coupon code' do
+        let!(:plan) do
+          stripe_helper.create_plan(
+            id: 'alaveteli-pro', product: product.id, amount: 1000
+          )
+        end
+
         before do
           allow(AlaveteliConfiguration).to receive(:stripe_namespace).
             and_return('alaveteli')
 
+          stripe_helper.create_coupon(
+            id: 'ALAVETELI-COUPON_CODE',
+            amount_off: 1000,
+            currency: 'gbp'
+          )
+
           post :create, params: {
             'stripe_token' => token,
             'plan_id' => 'pro',
@@ -562,11 +569,13 @@
 
       context 'subscription invoice open' do
         before do
+          plan = double(id: 'pro', to_param: 'pro')
+
           subscription = double(
             :subscription,
             require_authorisation?: false,
             invoice_open?: true,
-            plan: double(id: 'pro')
+            plan: plan
           )
 
           allow(pro_account.subscriptions).to receive(:retrieve).with('1').
diff --git a/spec/models/alaveteli_pro/plan_spec.rb b/spec/models/alaveteli_pro/plan_spec.rb
index bffd0abdde..0ac162bcc4 100644
--- a/spec/models/alaveteli_pro/plan_spec.rb
+++ b/spec/models/alaveteli_pro/plan_spec.rb
@@ -4,6 +4,73 @@
   let(:plan) { double(:plan, amount: 833) }
   subject { described_class.new(plan) }
 
+  describe '.list' do
+    before { described_class.instance_variable_set(:@list, nil) }
+
+    it 'returns an array with one pro plan' do
+      pro_plan = double('pro_plan')
+      allow(described_class).to receive(:retrieve).with('pro').
+        and_return(pro_plan)
+
+      expect(described_class.list).to eq([pro_plan])
+    end
+  end
+
+  describe '.retrieve' do
+    it 'retrieves a plan from Stripe' do
+      stripe_plan = double('stripe_plan')
+      allow(Stripe::Plan).to receive(:retrieve).with('test_pro').
+        and_return(stripe_plan)
+      allow(described_class).to receive(:add_stripe_namespace).with('pro').
+        and_return('test_pro')
+
+      plan = described_class.retrieve('pro')
+      expect(plan).to be_an_instance_of(described_class)
+    end
+
+    it 'returns nil if Stripe::InvalidRequestError is raised' do
+      allow(Stripe::Plan).to receive(:retrieve).
+        and_raise(Stripe::InvalidRequestError.new('', ''))
+
+      expect(described_class.retrieve('invalid')).to be_nil
+    end
+  end
+
+  describe '#to_param' do
+    it 'removes the stripe namespace from the id' do
+      plan = described_class.new(double('stripe_plan', id: 'test_pro'))
+      allow(plan).to receive(:remove_stripe_namespace).with('test_pro').
+        and_return('pro')
+
+      expect(plan.to_param).to eq('pro')
+    end
+  end
+
+  describe '#product' do
+    let(:stripe_plan) { double('stripe_plan', product: 'prod_123') }
+    let(:plan) { described_class.new(stripe_plan) }
+
+    it 'retrieves the product from Stripe' do
+      product = double('product')
+      allow(Stripe::Product).to receive(:retrieve).with('prod_123').
+        and_return(product)
+
+      expect(plan.product).to eq(product)
+    end
+
+    it 'memoizes the result' do
+      expect(Stripe::Product).to receive(:retrieve).once.
+        and_return(double('product'))
+      2.times { plan.product }
+    end
+
+    it 'returns nil if there is no product_id' do
+      allow(stripe_plan).to receive(:product).and_return(nil)
+
+      expect(plan.product).to be_nil
+    end
+  end
+
   describe '#amount_with_tax' do
     context 'with the default tax rate' do
       it 'adds 20% tax to the plan amount' do
diff --git a/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb b/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb
index 2aa55c7a32..3892685f00 100644
--- a/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb
+++ b/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb
@@ -19,7 +19,7 @@
   before do
     allow(AlaveteliConfiguration).to receive(:iso_currency_code).
         and_return('GBP')
-    assign :plan, plan
+    assign :plans, [plan]
     assign :pro_site_name, 'Alaveteli<sup>Pro</sup>'
   end
 

From 8c77c4c3700da3e50d7bec98c7db7b525e3f63b4 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Fri, 11 Oct 2024 17:02:38 +0100
Subject: [PATCH 17/22] Move tax percent to Pro Plan instance

---
 app/controllers/alaveteli_pro/subscriptions_controller.rb | 6 +-----
 app/models/concerns/taxable.rb                            | 8 ++++++++
 spec/models/alaveteli_pro/plan_spec.rb                    | 8 ++++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/app/controllers/alaveteli_pro/subscriptions_controller.rb b/app/controllers/alaveteli_pro/subscriptions_controller.rb
index fc5c8becb7..794bbe7ead 100644
--- a/app/controllers/alaveteli_pro/subscriptions_controller.rb
+++ b/app/controllers/alaveteli_pro/subscriptions_controller.rb
@@ -50,7 +50,7 @@ def create
 
       attributes = {
         plan: @plan.id,
-        tax_percent: tax_percent,
+        tax_percent: @plan.tax_percent,
         payment_behavior: 'allow_incomplete'
       }
       attributes[:coupon] = coupon_code if coupon_code?
@@ -217,10 +217,6 @@ def referral_coupon
       end
   end
 
-  def tax_percent
-    (BigDecimal(AlaveteliConfiguration.stripe_tax_rate).to_f * 100).to_f
-  end
-
   def load_plan
     @plan = AlaveteliPro::Plan.retrieve(params[:plan_id])
     @plan || redirect_to(pro_plans_path)
diff --git a/app/models/concerns/taxable.rb b/app/models/concerns/taxable.rb
index cc63db5ba1..cf38e3305e 100644
--- a/app/models/concerns/taxable.rb
+++ b/app/models/concerns/taxable.rb
@@ -11,6 +11,10 @@
 #   # => 833
 #   instance.amount_with_tax
 #   # => 1000
+#   AlaveteliConfiguration.stripe_tax_rate
+#   # => '0.20'
+#   instance.tax_percent
+#   # => 20.0
 #
 module Taxable
   extend ActiveSupport::Concern
@@ -29,6 +33,10 @@ def tax(*attributes)
     end
   end
 
+  def tax_percent
+    (tax_rate.to_f * 100).to_f
+  end
+
   private
 
   def tax_rate
diff --git a/spec/models/alaveteli_pro/plan_spec.rb b/spec/models/alaveteli_pro/plan_spec.rb
index 0ac162bcc4..cf57274b78 100644
--- a/spec/models/alaveteli_pro/plan_spec.rb
+++ b/spec/models/alaveteli_pro/plan_spec.rb
@@ -93,4 +93,12 @@
   it 'delegates to the stripe plan' do
     expect(subject.amount).to eq(833)
   end
+
+  describe '#tax_percent' do
+    it 'returns the tax rate as a percentage' do
+      allow(AlaveteliConfiguration).to receive(:stripe_tax_rate).
+        and_return('0.20')
+      expect(subject.tax_percent).to eq(20.0)
+    end
+  end
 end

From 3ee848c19fd6249da278f354bd009f006d397ae3 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Fri, 11 Oct 2024 17:02:38 +0100
Subject: [PATCH 18/22] Add Pro Coupon referral and retrieve methods

Again simplify controllers by moving coupons loading into delegated
classes.
---
 .../alaveteli_pro/subscriptions_controller.rb | 36 ++-------
 app/models/alaveteli_pro/coupon.rb            | 26 ++++++
 .../subscriptions/index.html.erb              |  6 +-
 .../subscriptions_controller_spec.rb          | 58 ++-----------
 spec/models/alaveteli_pro/coupon_spec.rb      | 81 +++++++++++++++++++
 5 files changed, 121 insertions(+), 86 deletions(-)
 create mode 100644 app/models/alaveteli_pro/coupon.rb
 create mode 100644 spec/models/alaveteli_pro/coupon_spec.rb

diff --git a/app/controllers/alaveteli_pro/subscriptions_controller.rb b/app/controllers/alaveteli_pro/subscriptions_controller.rb
index 794bbe7ead..4bff8070b1 100644
--- a/app/controllers/alaveteli_pro/subscriptions_controller.rb
+++ b/app/controllers/alaveteli_pro/subscriptions_controller.rb
@@ -1,13 +1,11 @@
 class AlaveteliPro::SubscriptionsController < AlaveteliPro::BaseController
-  include AlaveteliPro::StripeNamespace
-
   skip_before_action :html_response, only: [:create, :authorise]
   skip_before_action :pro_user_authenticated?, only: [:create, :authorise]
 
   before_action :authenticate, only: [:create, :authorise]
   before_action :check_allowed_to_subscribe_to_pro, only: [:create]
   before_action :prevent_duplicate_submission, only: [:create]
-  before_action :load_plan, only: [:create]
+  before_action :load_plan, :load_coupon, only: [:create]
   before_action :check_has_current_subscription, only: [:index]
 
   def index
@@ -19,11 +17,6 @@ def index
         @customer.
           sources.select { |card| card.id == @customer.default_source }.first
     end
-
-    if referral_coupon
-      @discount_code = remove_stripe_namespace(referral_coupon.id)
-      @discount_terms = referral_coupon.metadata.humanized_terms
-    end
   end
 
   # TODO: remove reminder of Stripe params once shipped
@@ -53,7 +46,7 @@ def create
         tax_percent: @plan.tax_percent,
         payment_behavior: 'allow_incomplete'
       }
-      attributes[:coupon] = coupon_code if coupon_code?
+      attributes[:coupon] = @coupon.id if @coupon
 
       @subscription = @pro_account.subscriptions.create(attributes)
 
@@ -196,32 +189,15 @@ def check_has_current_subscription
     redirect_to pro_plans_path
   end
 
-  def coupon_code?
-    params[:coupon_code].present?
-  end
-
-  def coupon_code
-    add_stripe_namespace(params.require(:coupon_code)).upcase
-  end
-
-  def referral_coupon
-    coupon_code =
-      add_stripe_namespace(AlaveteliConfiguration.pro_referral_coupon)
-
-    @referral_coupon ||=
-      unless coupon_code.blank?
-        begin
-          Stripe::Coupon.retrieve(coupon_code)
-        rescue Stripe::StripeError
-        end
-      end
-  end
-
   def load_plan
     @plan = AlaveteliPro::Plan.retrieve(params[:plan_id])
     @plan || redirect_to(pro_plans_path)
   end
 
+  def load_coupon
+    @coupon = AlaveteliPro::Coupon.retrieve(params[:coupon_code])
+  end
+
   def prevent_duplicate_submission
     # TODO: This doesn't take the plan in to account
     return unless @user.pro_account.try(:subscription?)
diff --git a/app/models/alaveteli_pro/coupon.rb b/app/models/alaveteli_pro/coupon.rb
new file mode 100644
index 0000000000..e6a0dacfee
--- /dev/null
+++ b/app/models/alaveteli_pro/coupon.rb
@@ -0,0 +1,26 @@
+##
+# A wrapper for a Stripe::Coupon
+#
+class AlaveteliPro::Coupon < SimpleDelegator
+  extend AlaveteliPro::StripeNamespace
+  include AlaveteliPro::StripeNamespace
+
+  def self.referral
+    id = add_stripe_namespace(AlaveteliConfiguration.pro_referral_coupon)
+    retrieve(id) if id
+  end
+
+  def self.retrieve(id)
+    new(Stripe::Coupon.retrieve(add_stripe_namespace(id)))
+  rescue Stripe::InvalidRequestError
+    nil
+  end
+
+  def to_param
+    remove_stripe_namespace(id)
+  end
+
+  def terms
+    metadata.humanized_terms || name
+  end
+end
diff --git a/app/views/alaveteli_pro/subscriptions/index.html.erb b/app/views/alaveteli_pro/subscriptions/index.html.erb
index e6c434d774..21a382f72c 100644
--- a/app/views/alaveteli_pro/subscriptions/index.html.erb
+++ b/app/views/alaveteli_pro/subscriptions/index.html.erb
@@ -42,14 +42,14 @@
           <% end %>
         </div>
 
-        <% if @discount_code && @discount_terms %>
+        <% if @referral = AlaveteliPro::Coupon.referral %>
           <div class="settings-section">
             <h3><%= _('Refer a friend') %></h3>
             <p class="settings__item">
               <%= _('Share the code {{discount_code}} with a friend to give ' \
                     'them {{discount_terms}} on signup.',
-                    discount_code: content_tag(:strong, @discount_code),
-                    discount_terms: @discount_terms) %>
+                    discount_code: content_tag(:strong, @referral.to_param),
+                    discount_terms: @referral.terms) %>
             </p>
           </div>
         <% end %>
diff --git a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
index 9c1b51a6ed..dad74b45bb 100644
--- a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
+++ b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
@@ -16,7 +16,7 @@
 
   before do
     stripe_helper.create_coupon(
-      id: 'COUPON_CODE',
+      id: 'coupon_code',
       amount_off: 1000,
       currency: 'gbp'
     )
@@ -156,7 +156,7 @@
         include_examples 'successful example'
 
         it 'uses coupon code' do
-          expect(assigns(:subscription).discount.coupon.id).to eq('COUPON_CODE')
+          expect(assigns(:subscription).discount.coupon.id).to eq('coupon_code')
         end
       end
 
@@ -172,7 +172,7 @@
             and_return('alaveteli')
 
           stripe_helper.create_coupon(
-            id: 'ALAVETELI-COUPON_CODE',
+            id: 'alaveteli-coupon_code',
             amount_off: 1000,
             currency: 'gbp'
           )
@@ -187,8 +187,8 @@
         include_examples 'successful example'
 
         it 'uses namespaced coupon code' do
-          expect(assigns(:subscription).discount.coupon.id).to eq(
-            'ALAVETELI-COUPON_CODE')
+          expect(assigns(:subscription).discount.coupon.id).
+            to eq('alaveteli-coupon_code')
         end
       end
 
@@ -777,54 +777,6 @@
         get :index
         expect(assigns[:card].id).to eq(customer.default_source)
       end
-
-      context 'if a PRO_REFERRAL_COUPON is blank' do
-        it 'does not assign the discount code' do
-          get :index
-          expect(assigns[:discount_code]).to be_nil
-        end
-
-        it 'does not assign the discount terms' do
-          get :index
-          expect(assigns[:discount_terms]).to be_nil
-        end
-      end
-
-      context 'if a PRO_REFERRAL_COUPON is set' do
-        before do
-          allow(AlaveteliConfiguration).
-            to receive(:pro_referral_coupon).and_return('PROREFERRAL')
-          allow(AlaveteliConfiguration).
-            to receive(:stripe_namespace).and_return('ALAVETELI')
-        end
-
-        let!(:coupon) do
-          stripe_helper.create_coupon(
-            percent_off: 50,
-            duration: 'repeating',
-            duration_in_months: 1,
-            id: 'ALAVETELI-PROREFERRAL',
-            metadata: { humanized_terms: '50% off for 1 month' }
-          )
-        end
-
-        it 'assigns the discount code, stripping the stripe namespace' do
-          get :index
-          expect(assigns[:discount_code]).to eq('PROREFERRAL')
-        end
-
-        it 'assigns the discount terms' do
-          get :index
-          expect(assigns[:discount_terms]).to eq('50% off for 1 month')
-        end
-
-        it 'rescues from any stripe error' do
-          error = Stripe::InvalidRequestError.new('Coupon expired', 'param')
-          StripeMock.prepare_error(error, :get_coupon)
-          get :index
-          expect(assigns[:discount_code]).to be_nil
-        end
-      end
     end
   end
 
diff --git a/spec/models/alaveteli_pro/coupon_spec.rb b/spec/models/alaveteli_pro/coupon_spec.rb
new file mode 100644
index 0000000000..658b6ae4df
--- /dev/null
+++ b/spec/models/alaveteli_pro/coupon_spec.rb
@@ -0,0 +1,81 @@
+require 'spec_helper'
+
+RSpec.describe AlaveteliPro::Coupon do
+  describe '.referral' do
+    before { described_class.instance_variable_set(:@referral, nil) }
+
+    it 'returns a coupon for the referral configuration' do
+      stripe_coupon = double('Stripe::Coupon')
+      allow(AlaveteliConfiguration).to receive(:pro_referral_coupon).
+        and_return('REFERRAL')
+      allow(described_class).to receive(:add_stripe_namespace).
+        and_return('namespace_REFERRAL')
+      allow(Stripe::Coupon).to receive(:retrieve).with('namespace_REFERRAL').
+        and_return(stripe_coupon)
+
+      expect(described_class.referral).to be_an_instance_of(described_class)
+      expect(described_class.referral).to eq(stripe_coupon)
+    end
+
+    it 'returns nil if no referral coupon is configured' do
+      allow(AlaveteliConfiguration).to receive(:pro_referral_coupon).
+        and_return(nil)
+
+      expect(described_class.referral).to be_nil
+    end
+  end
+
+  describe '.retrieve' do
+    it 'retrieves a Stripe coupon and wraps it' do
+      stripe_coupon = double('Stripe::Coupon')
+      allow(Stripe::Coupon).to receive(:retrieve).and_return(stripe_coupon)
+      allow(described_class).to receive(:add_stripe_namespace).
+        and_return('namespace_ID')
+
+      coupon = described_class.retrieve('ID')
+      expect(coupon).to be_a(described_class)
+      expect(coupon.__getobj__).to eq(stripe_coupon)
+    end
+
+    it 'returns nil if the coupon is not found' do
+      allow(Stripe::Coupon).to receive(:retrieve).
+        and_raise(Stripe::InvalidRequestError.new('', ''))
+
+      expect(described_class.retrieve('NONEXISTENT')).to be_nil
+    end
+  end
+
+  describe '#to_param' do
+    it 'removes the stripe namespace from the id' do
+      coupon = described_class.new(double('Stripe::Coupon', id: 'namespace_ID'))
+      allow(coupon).to receive(:remove_stripe_namespace).and_return('ID')
+
+      expect(coupon.to_param).to eq('ID')
+    end
+  end
+
+  describe '#terms' do
+    it 'returns humanized terms from metadata if present' do
+      coupon = described_class.new(
+        double(
+          'Stripe::Coupon',
+          metadata: double(humanized_terms: 'Human readable terms')
+        )
+      )
+
+      expect(coupon.terms).to eq('Human readable terms')
+    end
+
+    it 'returns name if humanized terms are not present' do
+      coupon = described_class.new(
+        double(
+          'Stripe::Coupon',
+          metadata: double(humanized_terms: nil),
+          name: 'Coupon Name'
+        )
+      )
+
+      expect(coupon.terms).to eq('Coupon Name')
+    end
+  end
+end

From 3f5ba36bf526b73112a4de55fa6e07c34185f34e Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Fri, 11 Oct 2024 17:02:38 +0100
Subject: [PATCH 19/22] Add Pro Customer

Further simplification of controllers by moving default source handling
into a delegated class.
---
 .../alaveteli_pro/subscriptions_controller.rb |  6 ---
 app/models/alaveteli_pro/customer.rb          | 12 +++++
 app/models/pro_account.rb                     |  5 +-
 .../subscriptions/index.html.erb              |  2 +-
 .../subscriptions_controller_spec.rb          |  5 --
 spec/models/alaveteli_pro/customer_spec.rb    | 51 +++++++++++++++++++
 6 files changed, 65 insertions(+), 16 deletions(-)
 create mode 100644 app/models/alaveteli_pro/customer.rb
 create mode 100644 spec/models/alaveteli_pro/customer_spec.rb

diff --git a/app/controllers/alaveteli_pro/subscriptions_controller.rb b/app/controllers/alaveteli_pro/subscriptions_controller.rb
index 4bff8070b1..6c6f333eef 100644
--- a/app/controllers/alaveteli_pro/subscriptions_controller.rb
+++ b/app/controllers/alaveteli_pro/subscriptions_controller.rb
@@ -11,12 +11,6 @@ class AlaveteliPro::SubscriptionsController < AlaveteliPro::BaseController
   def index
     @customer = current_user.pro_account.try(:stripe_customer)
     @subscriptions = current_user.pro_account.subscriptions
-
-    if @customer.default_source
-      @card =
-        @customer.
-          sources.select { |card| card.id == @customer.default_source }.first
-    end
   end
 
   # TODO: remove reminder of Stripe params once shipped
diff --git a/app/models/alaveteli_pro/customer.rb b/app/models/alaveteli_pro/customer.rb
new file mode 100644
index 0000000000..cd9ef2decc
--- /dev/null
+++ b/app/models/alaveteli_pro/customer.rb
@@ -0,0 +1,12 @@
+##
+# A wrapper for a Stripe::Customer
+#
+class AlaveteliPro::Customer < SimpleDelegator
+  def self.retrieve(id)
+    new(Stripe::Customer.retrieve(id))
+  end
+
+  def default_source
+    @default_source ||= sources.find { |c| c.id == __getobj__.default_source }
+  end
+end
diff --git a/app/models/pro_account.rb b/app/models/pro_account.rb
index b12fae425c..81494ccd5b 100644
--- a/app/models/pro_account.rb
+++ b/app/models/pro_account.rb
@@ -72,9 +72,6 @@ def update_stripe_customer
   def stripe_customer!
     return unless stripe_customer_id
 
-    Stripe::Customer.retrieve(
-      id: stripe_customer_id,
-      expand: ['subscriptions.data.plan.product']
-    )
+    AlaveteliPro::Customer.retrieve(stripe_customer_id)
   end
 end
diff --git a/app/views/alaveteli_pro/subscriptions/index.html.erb b/app/views/alaveteli_pro/subscriptions/index.html.erb
index 21a382f72c..ae0614b813 100644
--- a/app/views/alaveteli_pro/subscriptions/index.html.erb
+++ b/app/views/alaveteli_pro/subscriptions/index.html.erb
@@ -36,7 +36,7 @@
         <div class="settings-section">
           <h3><%= _('Your payment details') %></h3>
           <% if @customer.default_source %>
-            <%= render partial: 'card', locals: { card: @card } %>
+            <%= render partial: 'card', locals: { card: @customer.default_source } %>
           <% else %>
             <%= render partial: 'add_card' %>
           <% end %>
diff --git a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
index dad74b45bb..47cd0c38a7 100644
--- a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
+++ b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb
@@ -772,11 +772,6 @@
         expect(assigns[:subscriptions].first.id).
           to eq(customer.subscriptions.first.id)
       end
-
-      it 'assigns the default source as card' do
-        get :index
-        expect(assigns[:card].id).to eq(customer.default_source)
-      end
     end
   end
 
diff --git a/spec/models/alaveteli_pro/customer_spec.rb b/spec/models/alaveteli_pro/customer_spec.rb
new file mode 100644
index 0000000000..669787c2a7
--- /dev/null
+++ b/spec/models/alaveteli_pro/customer_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+require 'stripe_mock'
+
+RSpec.describe AlaveteliPro::Customer do
+  describe '.retrieve' do
+    it 'returns a new customer instance when successful' do
+      stripe_customer = double('Stripe::Customer')
+      allow(Stripe::Customer).to receive(:retrieve).with('cus_123').
+        and_return(stripe_customer)
+
+      customer = described_class.retrieve('cus_123')
+      expect(customer).to be_an_instance_of(described_class)
+      expect(customer).to eq(stripe_customer)
+    end
+
+    it 'raises Stripe::InvalidRequestError when customer does not exist' do
+      StripeMock.start
+
+      error = Stripe::InvalidRequestError.new('', '')
+      StripeMock.prepare_error(error)
+
+      expect { described_class.retrieve('cus_123') }.to raise_error(
+        Stripe::InvalidRequestError
+      )
+
+      StripeMock.stop
+    end
+  end
+
+  describe '#default_source' do
+    let(:stripe_customer) do
+      double('Stripe::Customer', default_source: 'card_123')
+    end
+
+    let(:customer) { described_class.new(stripe_customer) }
+
+    it 'returns the default source from the customer sources' do
+      source1 = double('Stripe::Card', id: 'card_123')
+      source2 = double('Stripe::Card', id: 'card_456')
+      allow(customer).to receive(:sources).and_return([source1, source2])
+
+      expect(customer.default_source).to eq(source1)
+    end
+
+    it 'memoizes the default source' do
+      source = double('Stripe::Card', id: 'card_123')
+      expect(customer).to receive(:sources).once.and_return([source])
+      2.times { expect(customer.default_source) }
+    end
+  end
+end

From 0ea24b31c92dbd54062806d9b48a190026241e6d Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Fri, 11 Oct 2024 17:02:38 +0100
Subject: [PATCH 20/22] Minor cleanup of controller

---
 .../alaveteli_pro/subscriptions_controller.rb | 24 +++++++------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/app/controllers/alaveteli_pro/subscriptions_controller.rb b/app/controllers/alaveteli_pro/subscriptions_controller.rb
index 6c6f333eef..1e05da2c12 100644
--- a/app/controllers/alaveteli_pro/subscriptions_controller.rb
+++ b/app/controllers/alaveteli_pro/subscriptions_controller.rb
@@ -1,33 +1,25 @@
 class AlaveteliPro::SubscriptionsController < AlaveteliPro::BaseController
+  before_action :check_has_current_subscription, only: [:index]
+
   skip_before_action :html_response, only: [:create, :authorise]
   skip_before_action :pro_user_authenticated?, only: [:create, :authorise]
-
   before_action :authenticate, only: [:create, :authorise]
+
   before_action :check_allowed_to_subscribe_to_pro, only: [:create]
   before_action :prevent_duplicate_submission, only: [:create]
   before_action :load_plan, :load_coupon, only: [:create]
-  before_action :check_has_current_subscription, only: [:index]
 
   def index
     @customer = current_user.pro_account.try(:stripe_customer)
     @subscriptions = current_user.pro_account.subscriptions
   end
 
-  # TODO: remove reminder of Stripe params once shipped
-  #
-  # params =>
-  # {"utf8"=>"✓",
-  #  "authenticity_token"=>"Ono2YgLcl1eC1gGzyd7Vf5HJJhOek31yFpT+8z+tKoo=",
-  #  "stripe_token"=>"tok_s3kr3t…",
-  #  "controller"=>"alaveteli_pro/subscriptions",
-  #  "action"=>"create",
-  #  "plan_id"=>"WDTK-pro"}
   def create
     begin
       @pro_account = current_user.pro_account ||= current_user.build_pro_account
 
       # Ensure previous incomplete subscriptions are cancelled to prevent them
-      # from using the new card
+      # from using the new token/card
       @pro_account.subscriptions.incomplete.map(&:delete)
 
       @token = Stripe::Token.retrieve(params[:stripe_token])
@@ -114,10 +106,10 @@ def authorise
     end
 
   rescue Stripe::RateLimitError,
-          Stripe::InvalidRequestError,
-          Stripe::AuthenticationError,
-          Stripe::APIConnectionError,
-          Stripe::StripeError => e
+         Stripe::InvalidRequestError,
+         Stripe::AuthenticationError,
+         Stripe::APIConnectionError,
+         Stripe::StripeError => e
     if send_exception_notifications?
       ExceptionNotifier.notify_exception(e, env: request.env)
     end

From f1e196330be0aa94371d15756e1c4f9631a05326 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Mon, 11 Nov 2024 12:09:42 +0000
Subject: [PATCH 21/22] fixup! Update Pro Subscription discount calculation

---
 app/models/alaveteli_pro/subscription/discount.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/models/alaveteli_pro/subscription/discount.rb b/app/models/alaveteli_pro/subscription/discount.rb
index d7234518db..ab96e5000a 100644
--- a/app/models/alaveteli_pro/subscription/discount.rb
+++ b/app/models/alaveteli_pro/subscription/discount.rb
@@ -24,7 +24,7 @@ def discounted_amount
   end
 
   def discounted?
-    discounted_amount < plan.amount
+    reduction > 0
   end
 
   def discount_name

From 0994a53c15a1391e785dc4bf7da4d7e3cfe84619 Mon Sep 17 00:00:00 2001
From: Graeme Porteous <graeme@rgbp.co.uk>
Date: Mon, 11 Nov 2024 12:01:22 +0000
Subject: [PATCH 22/22] fixup! Update billing helper methods

---
 app/helpers/alaveteli_pro/plan_helper.rb       | 4 ++--
 spec/helpers/alaveteli_pro/plan_helper_spec.rb | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/app/helpers/alaveteli_pro/plan_helper.rb b/app/helpers/alaveteli_pro/plan_helper.rb
index 7bc3fb5065..af002445e8 100644
--- a/app/helpers/alaveteli_pro/plan_helper.rb
+++ b/app/helpers/alaveteli_pro/plan_helper.rb
@@ -13,13 +13,13 @@ def billing_frequency(plan)
     elsif interval(plan) == 'year' && interval_count(plan) == 1
       _('Billed: Annually')
     else
-      _('Billed: every {{interval}}', interval: interval(plan))
+      _('Billed: every {{interval}}', interval: pluralize_interval(plan))
     end
   end
 
   def billing_interval(plan)
     if interval_count(plan) == 1
-      _('per user, per {{interval}}', interval: pluralize_interval(plan))
+      _('per user, per {{interval}}', interval: interval(plan))
     else
       _('per user, every {{interval}}', interval: pluralize_interval(plan))
     end
diff --git a/spec/helpers/alaveteli_pro/plan_helper_spec.rb b/spec/helpers/alaveteli_pro/plan_helper_spec.rb
index bdcab955b1..ae85075529 100644
--- a/spec/helpers/alaveteli_pro/plan_helper_spec.rb
+++ b/spec/helpers/alaveteli_pro/plan_helper_spec.rb
@@ -33,6 +33,12 @@
       allow(plan).to receive(:interval_count).and_return(1)
       expect(helper.billing_frequency(plan)).to eq('Billed: every quarter')
     end
+
+    it 'returns custom message for intervals with count greater then 1' do
+      allow(plan).to receive(:interval).and_return('week')
+      allow(plan).to receive(:interval_count).and_return(2)
+      expect(helper.billing_frequency(plan)).to eq('Billed: every 2 weeks')
+    end
   end
 
   describe '#billing_interval' do