From 5fb3925bcf3841cb016bd6da2da99ca0fd9904d5 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Fri, 11 Oct 2024 17:02:38 +0100 Subject: [PATCH 1/3] Replace Stripe Plan API with the Prices API Using Prices provides more flexibility and will allow us to change the which prices are used on both current and future subscriptions. --- .../alaveteli_pro/plans_controller.rb | 6 +- .../alaveteli_pro/subscriptions_controller.rb | 18 +-- app/helpers/alaveteli_pro/plan_helper.rb | 45 -------- app/helpers/alaveteli_pro/price_helper.rb | 45 ++++++++ app/models/alaveteli_pro/plan.rb | 32 ------ app/models/alaveteli_pro/price.rb | 33 ++++++ app/models/alaveteli_pro/subscription.rb | 6 +- .../plans/_pricing_tiers.html.erb | 8 +- app/views/alaveteli_pro/plans/show.html.erb | 8 +- .../subscriptions/_subscription.html.erb | 6 +- config/general.yml-example | 21 ++++ lib/configuration.rb | 9 +- .../alaveteli_pro/plans_controller_spec.rb | 35 +++--- .../subscriptions_controller_spec.rb | 106 ++++++++++-------- .../helpers/alaveteli_pro/plan_helper_spec.rb | 57 ---------- .../alaveteli_pro/price_helper_spec.rb | 57 ++++++++++ spec/models/alaveteli_pro/plan_spec.rb | 104 ----------------- spec/models/alaveteli_pro/price_spec.rb | 104 +++++++++++++++++ .../plans/index.html.erb_spec.rb | 10 +- 19 files changed, 380 insertions(+), 330 deletions(-) delete mode 100644 app/helpers/alaveteli_pro/plan_helper.rb create mode 100644 app/helpers/alaveteli_pro/price_helper.rb delete mode 100644 app/models/alaveteli_pro/plan.rb create mode 100644 app/models/alaveteli_pro/price.rb delete mode 100644 spec/helpers/alaveteli_pro/plan_helper_spec.rb create mode 100644 spec/helpers/alaveteli_pro/price_helper_spec.rb delete mode 100644 spec/models/alaveteli_pro/plan_spec.rb create mode 100644 spec/models/alaveteli_pro/price_spec.rb diff --git a/app/controllers/alaveteli_pro/plans_controller.rb b/app/controllers/alaveteli_pro/plans_controller.rb index 63b4b15076..123f2fcb46 100644 --- a/app/controllers/alaveteli_pro/plans_controller.rb +++ b/app/controllers/alaveteli_pro/plans_controller.rb @@ -3,13 +3,13 @@ class AlaveteliPro::PlansController < AlaveteliPro::BaseController before_action :authenticate, :check_has_current_subscription, only: [:show] def index - @plans = AlaveteliPro::Plan.list + @prices = AlaveteliPro::Price.list @pro_site_name = pro_site_name end def show - @plan = AlaveteliPro::Plan.retrieve(params[:id]) - @plan || raise(ActiveRecord::RecordNotFound) + @price = AlaveteliPro::Price.retrieve(params[:id]) + @price || raise(ActiveRecord::RecordNotFound) end private diff --git a/app/controllers/alaveteli_pro/subscriptions_controller.rb b/app/controllers/alaveteli_pro/subscriptions_controller.rb index 1e05da2c12..892046c9c9 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 :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 :load_price, :load_coupon, only: [:create] def index @customer = current_user.pro_account.try(:stripe_customer) @@ -28,8 +28,8 @@ def create @pro_account.update_stripe_customer attributes = { - plan: @plan.id, - tax_percent: @plan.tax_percent, + items: [{ price: @price.id }], + tax_percent: @price.tax_percent, payment_behavior: 'allow_incomplete' } attributes[:coupon] = @coupon.id if @coupon @@ -62,7 +62,7 @@ def create end if flash[:error] - json_redirect_to plan_path(@plan) + json_redirect_to plan_path(@price) else redirect_to authorise_subscription_path(@subscription.id) end @@ -89,7 +89,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(@subscription.plan) + json_redirect_to plan_path(@subscription.price) elsif @subscription.active? current_user.add_role(:pro) @@ -168,16 +168,16 @@ def check_allowed_to_subscribe_to_pro end def check_has_current_subscription - # TODO: This doesn't take the plan in to account + # TODO: This doesn't take the price in to account return if @user.pro_account.try(:subscription?) flash[:notice] = _("You don't currently have a Pro subscription") redirect_to pro_plans_path end - def load_plan - @plan = AlaveteliPro::Plan.retrieve(params[:plan_id]) - @plan || redirect_to(pro_plans_path) + def load_price + @price = AlaveteliPro::Price.retrieve(params[:price_id]) + @price || redirect_to(pro_plans_path) end def load_coupon diff --git a/app/helpers/alaveteli_pro/plan_helper.rb b/app/helpers/alaveteli_pro/plan_helper.rb deleted file mode 100644 index af002445e8..0000000000 --- a/app/helpers/alaveteli_pro/plan_helper.rb +++ /dev/null @@ -1,45 +0,0 @@ -## -# Helper methods for formatting and displaying billing and plan information -# in the Alaveteli Pro interface -# -module AlaveteliPro::PlanHelper - def billing_frequency(plan) - if interval(plan) == 'day' && interval_count(plan) == 1 - _('Billed: Daily') - elsif interval(plan) == 'week' && interval_count(plan) == 1 - _('Billed: Weekly') - elsif interval(plan) == 'month' && interval_count(plan) == 1 - _('Billed: Monthly') - elsif interval(plan) == 'year' && interval_count(plan) == 1 - _('Billed: Annually') - else - _('Billed: every {{interval}}', interval: pluralize_interval(plan)) - end - end - - def billing_interval(plan) - if interval_count(plan) == 1 - _('per user, per {{interval}}', interval: 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/helpers/alaveteli_pro/price_helper.rb b/app/helpers/alaveteli_pro/price_helper.rb new file mode 100644 index 0000000000..23f9f724d9 --- /dev/null +++ b/app/helpers/alaveteli_pro/price_helper.rb @@ -0,0 +1,45 @@ +## +# Helper methods for formatting and displaying billing and price information +# in the Alaveteli Pro interface +# +module AlaveteliPro::PriceHelper + def billing_frequency(price) + if interval(price) == 'day' && interval_count(price) == 1 + _('Billed: Daily') + elsif interval(price) == 'week' && interval_count(price) == 1 + _('Billed: Weekly') + elsif interval(price) == 'month' && interval_count(price) == 1 + _('Billed: Monthly') + elsif interval(price) == 'year' && interval_count(price) == 1 + _('Billed: Annually') + else + _('Billed: every {{interval}}', interval: pluralize_interval(price)) + end + end + + def billing_interval(price) + if interval_count(price) == 1 + _('per user, per {{interval}}', interval: interval(price)) + else + _('per user, every {{interval}}', interval: pluralize_interval(price)) + end + end + + private + + def pluralize_interval(price) + count = interval_count(price) + interval = interval(price) + return interval if count == 1 + + pluralize(count, interval) + end + + def interval(price) + price.recurring['interval'] + end + + def interval_count(price) + price.recurring['interval_count'] + end +end diff --git a/app/models/alaveteli_pro/plan.rb b/app/models/alaveteli_pro/plan.rb deleted file mode 100644 index 024c108abf..0000000000 --- a/app/models/alaveteli_pro/plan.rb +++ /dev/null @@ -1,32 +0,0 @@ -## -# 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/models/alaveteli_pro/price.rb b/app/models/alaveteli_pro/price.rb new file mode 100644 index 0000000000..6bec30061f --- /dev/null +++ b/app/models/alaveteli_pro/price.rb @@ -0,0 +1,33 @@ +## +# A wrapper for a Stripe::Price +# +class AlaveteliPro::Price < SimpleDelegator + include Taxable + + tax :unit_amount + + def self.list + AlaveteliConfiguration.stripe_prices.map do |(_key, id)| + retrieve(id) + end + end + + def self.retrieve(id) + key = AlaveteliConfiguration.stripe_prices.key(id) + new(Stripe::Price.retrieve(key)) + rescue Stripe::InvalidRequestError + nil + end + + def to_param + AlaveteliConfiguration.stripe_prices[id] || id + end + + # product + def product + @product ||= ( + product_id = __getobj__.product + Stripe::Product.retrieve(product_id) if product_id + ) + end +end diff --git a/app/models/alaveteli_pro/subscription.rb b/app/models/alaveteli_pro/subscription.rb index df6b69ac33..7bfc742cc4 100644 --- a/app/models/alaveteli_pro/subscription.rb +++ b/app/models/alaveteli_pro/subscription.rb @@ -47,9 +47,9 @@ def delete Stripe::Subscription.cancel(id) end - # plan - def plan - @plan ||= AlaveteliPro::Plan.new(__getobj__.plan) + # price + def price + @price ||= AlaveteliPro::Price.new(items.first.price) end private diff --git a/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb b/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb index 9475367cbf..66f471f548 100644 --- a/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb +++ b/app/views/alaveteli_pro/plans/_pricing_tiers.html.erb @@ -1,5 +1,5 @@
- <% @plans.each do |plan| %> + <% @prices.each do |price| %>

<%= _('Professional') %>

@@ -10,10 +10,10 @@

- <%= format_currency(plan.amount_with_tax, no_cents_if_whole: true) %> + <%= format_currency(price.unit_amount_with_tax, no_cents_if_whole: true) %> - <%= billing_interval(plan) %> + <%= billing_interval(price) %>

@@ -27,7 +27,7 @@
  • <%= _('Friendly support') %>
    • - <%= link_to _('Sign up'), plan_path(plan), class: 'button button-pop' %> + <%= link_to _('Sign up'), plan_path(price), class: 'button button-pop' %>
    <% end %> diff --git a/app/views/alaveteli_pro/plans/show.html.erb b/app/views/alaveteli_pro/plans/show.html.erb index 16fb3eedce..5b4be17931 100644 --- a/app/views/alaveteli_pro/plans/show.html.erb +++ b/app/views/alaveteli_pro/plans/show.html.erb @@ -29,11 +29,11 @@

    <%= _('Selected plan') %>

    - <%= @plan.product.name %> + <%= @price.product.name %>
    - <%= format_currency(@plan.amount_with_tax) %> - <%= billing_frequency(@plan) %> + <%= format_currency(@price.unit_amount_with_tax) %> + <%= billing_frequency(@price) %>
    @@ -65,7 +65,7 @@
    - <%= hidden_field_tag 'plan_id', @plan.id %> + <%= hidden_field_tag 'price_id', @price.id %> <%= submit_tag _('Subscribe'), id: 'js-stripe-submit', disabled: true, data: { disable_with: 'Processing...' } %> <%= link_to _('Cancel'), pro_plans_path, class: 'settings__cancel-button' %>

    diff --git a/app/views/alaveteli_pro/subscriptions/_subscription.html.erb b/app/views/alaveteli_pro/subscriptions/_subscription.html.erb index d46ad91a98..c021268225 100644 --- a/app/views/alaveteli_pro/subscriptions/_subscription.html.erb +++ b/app/views/alaveteli_pro/subscriptions/_subscription.html.erb @@ -1,12 +1,12 @@
    - <%= subscription.plan.product.name %> + <%= subscription.price.product.name %>
    - <%= format_currency(subscription.plan.amount_with_tax) %> - <%= billing_frequency(subscription.plan) %> + <%= format_currency(subscription.price.unit_amount_with_tax) %> + <%= billing_frequency(subscription.price) %> <% if subscription.discounted? %>
    <%= _('{{discounted_amount}} with discount ' \ diff --git a/config/general.yml-example b/config/general.yml-example index 630a11fbe2..0100848c45 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -1202,6 +1202,27 @@ STRIPE_SECRET_KEY: '' # --- STRIPE_NAMESPACE: '' +# List of Stripe Prices which if a user signs up to will grant them access to +# Alaveteli Pro. Rendered on the Pro pricing pages in the order defined here. +# +# STRIPE_PRICES - Hash of objects with key equal to the Stripe Price IDs as the +# value equal to parameterise short human readable string. +# +# Note: Historical Stripe Price IDs listed here should include STRIPE_NAMESPACE. +# +# STRIPE_PRICES = Hash (default: { pro: 'pro' }) +# +# Examples: +# +# STRIPE_PRICES: +# ALAVETELI-pro: pro +# price_123: pro-new-price +# price_456: pro-annual-billing +# +# --- +STRIPE_PRICES: + pro: pro + # Stripe.com webhook secret. Only required for Alaveteli Pro. # # STRIPE_WEBHOOK_SECRET: - String (default: '') diff --git a/lib/configuration.rb b/lib/configuration.rb index 00f3aef75f..a83c18f364 100644 --- a/lib/configuration.rb +++ b/lib/configuration.rb @@ -109,6 +109,7 @@ module AlaveteliConfiguration SMTP_MAILER_PORT: 25, SMTP_MAILER_USER_NAME: '', STRIPE_NAMESPACE: '', + STRIPE_PRICES: { pro: 'pro' }, STRIPE_PUBLISHABLE_KEY: '', STRIPE_SECRET_KEY: '', STRIPE_TAX_RATE: '0.20', @@ -145,7 +146,13 @@ def self.get(key, default) def self.method_missing(name) key = name.to_s.upcase if DEFAULTS.key?(key.to_sym) - get(key, DEFAULTS[key.to_sym]) + value = get(key, DEFAULTS[key.to_sym]) + case value + when Hash + value.with_indifferent_access + else + value + end else super end diff --git a/spec/controllers/alaveteli_pro/plans_controller_spec.rb b/spec/controllers/alaveteli_pro/plans_controller_spec.rb index 0a12f51164..f3ab399675 100644 --- a/spec/controllers/alaveteli_pro/plans_controller_spec.rb +++ b/spec/controllers/alaveteli_pro/plans_controller_spec.rb @@ -7,15 +7,15 @@ let(:stripe_helper) { StripeMock.create_test_helper } let(:product) { stripe_helper.create_product } - let!(:pro_plan) do - stripe_helper.create_plan( - id: 'pro', product: product.id, amount: 1000 + let!(:pro_price) do + stripe_helper.create_price( + id: 'pro', product: product.id, unit_amount: 1000 ) end - let!(:alaveteli_pro_plan) do - stripe_helper.create_plan( - id: 'alaveteli-pro', product: product.id, amount: 1000 + let!(:alaveteli_pro_price) do + stripe_helper.create_price( + id: 'alaveteli-pro', product: product.id, unit_amount: 1000 ) end @@ -41,7 +41,7 @@ end it 'uses the default plan for pricing info' do - expect(assigns(:plans)).to eq([pro_plan]) + expect(assigns(:prices)).to eq([pro_price]) end end @@ -68,13 +68,13 @@ sign_in user end - context 'with a valid plan' do + context 'with a valid price' do before do get :show, params: { id: 'pro' } end - it 'finds the specified plan' do - expect(assigns(:plan)).to eq(pro_plan) + it 'finds the specified price' do + expect(assigns(:price)).to eq(pro_price) end it 'renders the plan page' do @@ -88,13 +88,15 @@ context 'with a Stripe namespace' do before do + allow(AlaveteliConfiguration).to receive(:stripe_prices). + and_return('alaveteli-pro' => 'pro') allow(AlaveteliConfiguration).to receive(:stripe_namespace). and_return('alaveteli') get :show, params: { id: 'pro' } end it 'finds the specified plan' do - expect(assigns(:plan)).to eq(alaveteli_pro_plan) + expect(assigns(:price)).to eq(alaveteli_pro_price) end it 'renders the plan page' do @@ -113,7 +115,9 @@ Stripe::Customer.create(email: user.email, source: stripe_helper.generate_card_token) - Stripe::Subscription.create(customer: customer, plan: 'pro') + Stripe::Subscription.create( + customer: customer, items: [{ price: 'pro' }] + ) user.create_pro_account(stripe_customer_id: customer.id) user.add_role(:pro) get :show, params: { id: 'pro' } @@ -135,8 +139,9 @@ Stripe::Customer.create(email: user.email, source: stripe_helper.generate_card_token) - subscription = - Stripe::Subscription.create(customer: customer, plan: 'pro') + subscription = Stripe::Subscription.create( + customer: customer, items: [{ price: 'pro' }] + ) Stripe::Subscription.cancel(subscription.id) user.create_pro_account(stripe_customer_id: customer.id) @@ -152,7 +157,7 @@ end end - context 'with an invalid plan' do + context 'with an invalid price' do it 'returns ActiveRecord::RecordNotFound' do expect { get :show, params: { id: 'invalid-123' } diff --git a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb index 47cd0c38a7..98bfbdf76d 100644 --- a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb +++ b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb @@ -8,9 +8,9 @@ let(:product) { stripe_helper.create_product } - let!(:plan) do - stripe_helper.create_plan( - id: 'pro', product: product.id, amount: 1000 + let!(:price) do + stripe_helper.create_price( + id: 'pro', product: product.id, unit_amount: 1000 ) end @@ -72,14 +72,14 @@ to eq(user.email) end - it 'subscribes the user to the plan' do - expect(assigns(:subscription).plan.id).to eq(plan.id) + it 'subscribes the user to the price' do + expect(assigns(:subscription).price.id).to eq(price.id) expect(assigns(:pro_account).stripe_customer_id). to eq(assigns(:subscription).customer) end - it 'sets subscription plan amount and tax percentage' do - expect(assigns(:subscription).plan.amount).to eq 1000 + it 'sets subscription price unit amount and tax percentage' do + expect(assigns(:subscription).price.unit_amount).to eq 1000 expect(assigns(:subscription).tax_percent).to eql 25.0 end @@ -109,14 +109,14 @@ sign_in user post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } # reset user so authenticated_user reloads controller.instance_variable_set(:@user, nil) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -136,7 +136,7 @@ before do post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -148,7 +148,7 @@ before do post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => 'coupon_code' } end @@ -161,13 +161,15 @@ end context 'with Stripe namespace and coupon code' do - let!(:plan) do - stripe_helper.create_plan( - id: 'alaveteli-pro', product: product.id, amount: 1000 + let!(:price) do + stripe_helper.create_price( + id: 'alaveteli-pro', product: product.id, unit_amount: 1000 ) end before do + allow(AlaveteliConfiguration).to receive(:stripe_prices). + and_return('alaveteli-pro' => 'pro') allow(AlaveteliConfiguration).to receive(:stripe_namespace). and_return('alaveteli') @@ -179,7 +181,7 @@ post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => 'coupon_code' } end @@ -201,7 +203,7 @@ post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -235,8 +237,7 @@ # we can't create a subscription in the incomplete status so we have # to need a lot of stubs. subscription = Stripe::Subscription.create( - customer: customer, - plan: 'pro' + customer: customer, items: [{ price: 'pro' }] ) subs = double(:subscription_collection).as_null_object @@ -249,7 +250,7 @@ post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro' + 'price_id' => 'pro' } end end @@ -259,7 +260,7 @@ StripeMock.prepare_card_error(:card_declined, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -283,7 +284,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -308,7 +309,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -333,7 +334,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -358,7 +359,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -383,7 +384,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -408,7 +409,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => 'INVALID' } end @@ -433,7 +434,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => 'EXPIRED' } end @@ -453,12 +454,12 @@ end context 'when invalid params are submitted' do - it 'redirects to the plan page if there is a plan' do - post :create, params: { plan_id: 'pro' } + it 'redirects to the plan page if there is a price' do + post :create, params: { price_id: 'pro' } expect(response).to redirect_to(plan_path('pro')) end - it 'redirects to the pricing page if there is no plan' do + it 'redirects to the pricing page if there is no price' do post :create expect(response).to redirect_to(pro_plans_path) end @@ -481,10 +482,20 @@ context 'with a signed-in user' do let(:token) { stripe_helper.generate_card_token } - let(:customer) { Stripe::Customer.create(source: token, plan: 'pro') } + let(:customer) do + Stripe::Customer.create(source: token) + end + + let(:subscription) do + Stripe::Subscription.create( + customer: customer, items: [{ price: price.id }] + ) + end + let(:pro_account) do FactoryBot.create(:pro_account, stripe_customer_id: customer.id) end + let(:user) { pro_account.user } before do @@ -569,13 +580,13 @@ context 'subscription invoice open' do before do - plan = double(id: 'pro', to_param: 'pro') + price = double(id: 'pro', to_param: 'pro') subscription = double( :subscription, require_authorisation?: false, invoice_open?: true, - plan: plan + price: price ) allow(pro_account.subscriptions).to receive(:retrieve).with('1'). @@ -741,16 +752,20 @@ let(:user) { FactoryBot.create(:pro_user) } let!(:customer) do - stripe_helper.create_plan(id: 'test', product: product.id) - customer = Stripe::Customer.create({ + customer = Stripe::Customer.create( email: user.email, - source: stripe_helper.generate_card_token, - plan: 'test' - }) + source: stripe_helper.generate_card_token + ) user.pro_account.update!(stripe_customer_id: customer.id) customer end + let!(:subscription) do + Stripe::Subscription.create( + customer: customer, items: [{ price: price.id }] + ) + end + before do sign_in user end @@ -769,8 +784,7 @@ it 'assigns subscriptions' do get :index expect(assigns[:subscriptions].count).to eq(1) - expect(assigns[:subscriptions].first.id). - to eq(customer.subscriptions.first.id) + expect(assigns[:subscriptions].first.id).to eq(subscription.id) end end end @@ -807,8 +821,6 @@ context 'with a signed-in user' do let(:user) { FactoryBot.create(:pro_user) } - let(:plan) { stripe_helper.create_plan(id: 'test', product: product.id) } - let(:customer) do customer = Stripe::Customer.create({ email: user.email, @@ -819,7 +831,9 @@ end let(:subscription) do - Stripe::Subscription.create(customer: customer, plan: plan.id) + Stripe::Subscription.create( + customer: customer, items: [{ price: price.id }] + ) end before do @@ -851,7 +865,9 @@ email: 'test@example.org', source: stripe_helper.generate_card_token }) - Stripe::Subscription.create(customer: customer, plan: plan.id) + Stripe::Subscription.create( + customer: customer, items: [{ price: price.id }] + ) end before do @@ -979,7 +995,7 @@ end context 'when invalid params are submitted' do - it 'redirects to the plan page if there is a plan' do + it 'redirects to the plan page if there is a price' do delete :destroy, params: { id: 'unknown' } expect(response).to redirect_to(subscriptions_path) end diff --git a/spec/helpers/alaveteli_pro/plan_helper_spec.rb b/spec/helpers/alaveteli_pro/plan_helper_spec.rb deleted file mode 100644 index ae85075529..0000000000 --- a/spec/helpers/alaveteli_pro/plan_helper_spec.rb +++ /dev/null @@ -1,57 +0,0 @@ -require 'spec_helper' - -describe AlaveteliPro::PlanHelper do - let(:plan) { double('Plan') } - - describe '#billing_frequency' do - 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 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 - - 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 - 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 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/helpers/alaveteli_pro/price_helper_spec.rb b/spec/helpers/alaveteli_pro/price_helper_spec.rb new file mode 100644 index 0000000000..616e3dff4d --- /dev/null +++ b/spec/helpers/alaveteli_pro/price_helper_spec.rb @@ -0,0 +1,57 @@ +require 'spec_helper' + +describe AlaveteliPro::PriceHelper do + let(:price) { double('Stripe::Price') } + + describe '#billing_frequency' do + it 'returns "Billed: Daily" for daily price' do + allow(price).to receive(:recurring). + and_return('interval' => 'day', 'interval_count' => 1) + expect(helper.billing_frequency(price)).to eq('Billed: Daily') + end + + it 'returns "Billed: Weekly" for weekly price' do + allow(price).to receive(:recurring). + and_return('interval' => 'week', 'interval_count' => 1) + expect(helper.billing_frequency(price)).to eq('Billed: Weekly') + end + + it 'returns "Billed: Monthly" for monthly price' do + allow(price).to receive(:recurring). + and_return('interval' => 'month', 'interval_count' => 1) + expect(helper.billing_frequency(price)).to eq('Billed: Monthly') + end + + it 'returns "Billed: Annually" for yearly price' do + allow(price).to receive(:recurring). + and_return('interval' => 'year', 'interval_count' => 1) + expect(helper.billing_frequency(price)).to eq('Billed: Annually') + end + + it 'returns custom message for other intervals' do + allow(price).to receive(:recurring). + and_return('interval' => 'quarter', 'interval_count' => 1) + expect(helper.billing_frequency(price)).to eq('Billed: every quarter') + end + + it 'returns custom message for intervals with count greater then 1' do + allow(price).to receive(:recurring). + and_return('interval' => 'week', 'interval_count' => 2) + expect(helper.billing_frequency(price)).to eq('Billed: every 2 weeks') + end + end + + describe '#billing_interval' do + it 'returns singular interval for interval_count of 1' do + allow(price).to receive(:recurring). + and_return('interval' => 'month', 'interval_count' => 1) + expect(helper.billing_interval(price)).to eq('per user, per month') + end + + it 'returns plural interval for interval_count greater than 1' do + allow(price).to receive(:recurring). + and_return('interval' => 'month', 'interval_count' => 3) + expect(helper.billing_interval(price)).to eq('per user, every 3 months') + end + end +end diff --git a/spec/models/alaveteli_pro/plan_spec.rb b/spec/models/alaveteli_pro/plan_spec.rb deleted file mode 100644 index cf57274b78..0000000000 --- a/spec/models/alaveteli_pro/plan_spec.rb +++ /dev/null @@ -1,104 +0,0 @@ -require 'spec_helper' - -RSpec.describe AlaveteliPro::Plan do - 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 - expect(subject.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.amount_with_tax).to eq(1041) - end - end - end - - 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 diff --git a/spec/models/alaveteli_pro/price_spec.rb b/spec/models/alaveteli_pro/price_spec.rb new file mode 100644 index 0000000000..d018dc711b --- /dev/null +++ b/spec/models/alaveteli_pro/price_spec.rb @@ -0,0 +1,104 @@ +require 'spec_helper' + +RSpec.describe AlaveteliPro::Price do + let(:price) { double(:price, unit_amount: 833) } + subject { described_class.new(price) } + + describe '.list' do + before { described_class.instance_variable_set(:@list, nil) } + + let(:prices) { { 'price_1' => 'pro', 'price_2' => 'price_2' } } + let(:price_1) { AlaveteliPro::Price.new(double('Stripe::Price')) } + let(:price_2) { AlaveteliPro::Price.new(double('Stripe::Price')) } + + before do + allow(AlaveteliConfiguration).to receive(:stripe_prices). + and_return(prices) + allow(described_class).to receive(:retrieve).with('pro'). + and_return(price_1) + allow(described_class).to receive(:retrieve).with('price_2'). + and_return(price_2) + end + + it 'returns a list of retrieved prices' do + expect(described_class.list).to eq([price_1, price_2]) + end + end + + describe '.retrieve' do + it 'retrieves a price from Stripe' do + stripe_price = double('stripe_price') + allow(Stripe::Price).to receive(:retrieve).with('pro'). + and_return(stripe_price) + price = described_class.retrieve('pro') + expect(price).to be_an_instance_of(described_class) + end + end + + describe '#to_param' do + it 'returns the configured key for the id' do + allow(AlaveteliConfiguration).to receive(:stripe_prices). + and_return('price_123' => 'pro') + + price = described_class.new(double('stripe_price', id: 'price_123')) + + expect(price.to_param).to eq('pro') + end + end + + describe '#product' do + let(:stripe_price) { double('stripe_price', product: 'prod_123') } + let(:price) { described_class.new(stripe_price) } + + it 'retrieves the product from Stripe' do + product = double('product') + allow(Stripe::Product).to receive(:retrieve).with('prod_123'). + and_return(product) + + expect(price.product).to eq(product) + end + + it 'memoizes the result' do + expect(Stripe::Product).to receive(:retrieve).once. + and_return(double('product')) + 2.times { price.product } + end + + it 'returns nil if there is no product_id' do + allow(stripe_price).to receive(:product).and_return(nil) + + expect(price.product).to be_nil + end + end + + describe '#unit_amount_with_tax' do + context 'with the default tax rate' do + it 'adds 20% tax to the price unit_amount' do + expect(subject.unit_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 price unit_amount' do + expect(subject.unit_amount_with_tax).to eq(1041) + end + end + end + + it 'delegates to the stripe price' do + expect(subject.unit_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 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 3892685f00..eaba2f75c1 100644 --- a/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb +++ b/spec/views/alaveteli_pro/plans/index.html.erb_spec.rb @@ -7,19 +7,19 @@ let(:stripe_helper) { StripeMock.create_test_helper } let(:product) { stripe_helper.create_product } - let(:plan) { AlaveteliPro::Plan.new(stripe_plan) } + let(:price) { AlaveteliPro::Price.new(stripe_price) } let(:cents_price) { 880 } - let(:stripe_plan) do - stripe_helper.create_plan( - id: 'pro', product: product.id, amount: cents_price + let(:stripe_price) do + stripe_helper.create_price( + id: 'price_123', product: product.id, unit_amount: cents_price ) end before do allow(AlaveteliConfiguration).to receive(:iso_currency_code). and_return('GBP') - assign :plans, [plan] + assign :prices, [price] assign :pro_site_name, 'AlaveteliPro' end From 60e8068f2b99ca977d401b6f073abd690f2eeaf7 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Tue, 22 Oct 2024 17:40:35 +0100 Subject: [PATCH 2/3] Add task to migration subscription to new prices Run via: `rails subscriptions:migrate_price OLD_PRICE=pro NEW_PRICE=price_123` --- lib/tasks/subscriptions.rake | 50 ++++++++++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) create mode 100644 lib/tasks/subscriptions.rake diff --git a/lib/tasks/subscriptions.rake b/lib/tasks/subscriptions.rake new file mode 100644 index 0000000000..6d083df99a --- /dev/null +++ b/lib/tasks/subscriptions.rake @@ -0,0 +1,50 @@ +namespace :subscriptions do + desc 'Migrate Stripe subscription to new price' + task migrate_price: :environment do + old_price, new_price = *load_prices + + scope = Stripe::Subscription.list(price: old_price.id) + count = scope.data.size + + scope.auto_paging_each.with_index do |subscription, index| + item = subscription.items.first + Stripe::Subscription.update( + subscription.id, + items: [{ id: item.id, price: new_price.id }], + proration_behavior: 'none' + ) + + erase_line + print "Migrated subscriptions #{index + 1}/#{count}" + end + + erase_line + puts "Migrating all subscriptions completed." + end + + def load_prices + old_price = Stripe::Price.retrieve(ENV['OLD_PRICE']) if ENV['OLD_PRICE'] + new_price = Stripe::Price.retrieve(ENV['NEW_PRICE']) if ENV['NEW_PRICE'] + + if !old_price + puts "ERROR: Can't find OLD_PRICE" + exit 1 + elsif !new_price + puts "ERROR: Can't find NEW_PRICE" + exit 1 + elsif old_price.recurring != new_price.recurring + puts "ERROR: Price interval and interval_count need to match" + exit 1 + elsif !AlaveteliPro::Price.list.map(&:id).include?(new_price.id) + puts "ERROR: New price is not defined in general.yml" + exit 1 + end + + [old_price, new_price] + end + + def erase_line + # https://en.wikipedia.org/wiki/ANSI_escape_code#Escape_sequences + print "\e[1G\e[K" + end +end From 54c67ccb0222cb9c406c92e8a1cf2d47cefde19d Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Mon, 4 Nov 2024 12:14:56 +0000 Subject: [PATCH 3/3] Update changelog --- doc/CHANGES.md | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/doc/CHANGES.md b/doc/CHANGES.md index ffbca9d1f9..1062b45b27 100644 --- a/doc/CHANGES.md +++ b/doc/CHANGES.md @@ -2,6 +2,7 @@ ## Highlighted Features +* Migrated from Stripe Plans to Stripe Prices (Graeme Porteous) * Upgrade Stripe API version (Graeme Porteous) * Drop support for Azure storage (Graeme Porteous) * Add basic Citation searching in admin UI (Gareth Rees) @@ -94,7 +95,6 @@ * Don't show users that have closed their account or been banned on leaderboards (Chris Mytton) - ## Upgrade Notes * _Required:_ This upgrade requires upgrading Ruby from 3.0 to 3.1 or later. @@ -145,6 +145,13 @@ version from `2017-01-27` to `2020-03-02`. No changes should be necessary to your Stripe account. +* _Optional:_ We have moved from Stripe Plans to Stripe Prices. Previously we + hardcoded the Stripe Plan ID of `pro`, but with changes to the Stripe + dashboard this ID can no longer be created. Migration to the Prices API will + allow for more flexibly, pricing changes, and multiple price points - for + example annual pricing. For new prices you need to configure `STRIPE_PRICES` + in `config/general.yml`. + # 0.44.0.1 ## Highlighted Features