From a33dad66db626bc33a7ff432cd8859f1e4752f9a 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 +++++++ .../alaveteli_pro/{plan.rb => price.rb} | 14 ++- 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 | 15 +++ lib/configuration.rb | 1 + .../alaveteli_pro/plans_controller_spec.rb | 38 +++--- .../subscriptions_controller_spec.rb | 105 +++++++++------- .../helpers/alaveteli_pro/plan_helper_spec.rb | 51 -------- .../alaveteli_pro/price_helper_spec.rb | 51 ++++++++ spec/models/alaveteli_pro/plan_spec.rb | 104 ---------------- spec/models/alaveteli_pro/price_spec.rb | 115 ++++++++++++++++++ .../plans/index.html.erb_spec.rb | 10 +- 18 files changed, 351 insertions(+), 295 deletions(-) delete mode 100644 app/helpers/alaveteli_pro/plan_helper.rb create mode 100644 app/helpers/alaveteli_pro/price_helper.rb rename app/models/alaveteli_pro/{plan.rb => price.rb} (58%) 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 7bc3fb5065..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: 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/helpers/alaveteli_pro/price_helper.rb b/app/helpers/alaveteli_pro/price_helper.rb new file mode 100644 index 0000000000..eb278cbce6 --- /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: interval(price)) + end + end + + def billing_interval(price) + if interval_count(price) == 1 + _('per user, per {{interval}}', interval: pluralize_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/price.rb similarity index 58% rename from app/models/alaveteli_pro/plan.rb rename to app/models/alaveteli_pro/price.rb index 024c108abf..3aaacf9cce 100644 --- a/app/models/alaveteli_pro/plan.rb +++ b/app/models/alaveteli_pro/price.rb @@ -1,19 +1,23 @@ ## -# A wrapper for a Stripe::Plan +# A wrapper for a Stripe::Price # -class AlaveteliPro::Plan < SimpleDelegator +class AlaveteliPro::Price < SimpleDelegator extend AlaveteliPro::StripeNamespace include AlaveteliPro::StripeNamespace include Taxable - tax :amount + tax :unit_amount def self.list - [retrieve('pro')] + AlaveteliConfiguration.stripe_price_ids.inject([]) do |arr, id| + price = retrieve(id) + arr << price if price + arr + end end def self.retrieve(id) - new(Stripe::Plan.retrieve(add_stripe_namespace(id))) + new(Stripe::Price.retrieve(add_stripe_namespace(id))) rescue Stripe::InvalidRequestError nil 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..63e49ecf1a 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -1202,6 +1202,21 @@ STRIPE_SECRET_KEY: '' # --- STRIPE_NAMESPACE: '' +# List of Stripe Subscription Prices IDs which if a user signs up to will grant +# them access to Alaveteli Pro. Rendered on the Pro pricing pages in the ordered +# defined here. +# +# STRIPE_PRICE_IDS - Array Stripe Price IDs +# +# Examples: +# +# STRIPE_PRICE_IDS: +# - pro +# - pro-annual-billing +# +# --- +STRIPE_PRICE_IDS: [] + # 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..2629833fc5 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_PRICE_IDS: [], STRIPE_PUBLISHABLE_KEY: '', STRIPE_SECRET_KEY: '', STRIPE_TAX_RATE: '0.20', diff --git a/spec/controllers/alaveteli_pro/plans_controller_spec.rb b/spec/controllers/alaveteli_pro/plans_controller_spec.rb index 0a12f51164..e25b38e1bf 100644 --- a/spec/controllers/alaveteli_pro/plans_controller_spec.rb +++ b/spec/controllers/alaveteli_pro/plans_controller_spec.rb @@ -7,15 +7,20 @@ 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 + before do + allow(AlaveteliConfiguration).to receive(:stripe_price_ids). + and_return(['pro']) + end + + 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 +46,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 +73,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 @@ -94,7 +99,7 @@ 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 +118,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 +142,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 +160,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..7a4a7de766 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,9 +161,9 @@ 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 @@ -179,7 +179,7 @@ post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => 'coupon_code' } end @@ -201,7 +201,7 @@ post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -235,8 +235,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 +248,7 @@ post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro' + 'price_id' => 'pro' } end end @@ -259,7 +258,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 +282,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -308,7 +307,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -333,7 +332,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -358,7 +357,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -383,7 +382,7 @@ StripeMock.prepare_error(error, :create_subscription) post :create, params: { 'stripe_token' => token, - 'plan_id' => 'pro', + 'price_id' => 'pro', 'coupon_code' => '' } end @@ -408,7 +407,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 +432,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 +452,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 +480,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 +578,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 +750,21 @@ let(:user) { FactoryBot.create(:pro_user) } let!(:customer) do - stripe_helper.create_plan(id: 'test', product: product.id) - customer = Stripe::Customer.create({ + price = stripe_helper.create_price(id: 'test', product: product.id) + 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 +783,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,7 +820,7 @@ 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(:price) { stripe_helper.create_price(id: 'test', product: product.id) } let(:customer) do customer = Stripe::Customer.create({ @@ -819,7 +832,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 +866,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 +996,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 bdcab955b1..0000000000 --- a/spec/helpers/alaveteli_pro/plan_helper_spec.rb +++ /dev/null @@ -1,51 +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 - 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..e459231cc2 --- /dev/null +++ b/spec/helpers/alaveteli_pro/price_helper_spec.rb @@ -0,0 +1,51 @@ +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 + 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..79c404a8e9 --- /dev/null +++ b/spec/models/alaveteli_pro/price_spec.rb @@ -0,0 +1,115 @@ +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(:price_ids) { %w[price_1 price_2 price_3] } + let(:price_1) { double('AlaveteliPro::Price') } + let(:price_2) { double('AlaveteliPro::Price') } + + before do + allow(AlaveteliConfiguration).to receive(:stripe_price_ids). + and_return(price_ids) + allow(described_class).to receive(:retrieve).with('price_1'). + and_return(price_1) + allow(described_class).to receive(:retrieve).with('price_2'). + and_return(price_2) + allow(described_class).to receive(:retrieve).with('price_3'). + and_return(nil) + 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('test_pro'). + and_return(stripe_price) + allow(described_class).to receive(:add_stripe_namespace).with('pro'). + and_return('test_pro') + + price = described_class.retrieve('pro') + expect(price).to be_an_instance_of(described_class) + end + + it 'returns nil if Stripe::InvalidRequestError is raised' do + allow(Stripe::Price).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 + price = described_class.new(double('stripe_price', id: 'test_pro')) + allow(price).to receive(:remove_stripe_namespace).with('test_pro'). + and_return('pro') + + 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..ccff9152fc 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: 'pro', 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 c314f96f55ee01ef07bbbe878e6d21cd3d7c7ca1 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Mon, 21 Oct 2024 18:10:25 +0100 Subject: [PATCH 2/3] fixup! Replace Stripe Plan API with the Prices API --- .../alaveteli_pro/stripe_namespace.rb | 6 ++++-- app/models/alaveteli_pro/price.rb | 15 ++++++------- config/general.yml-example | 16 +++++++------- lib/configuration.rb | 10 +++++++-- .../alaveteli_pro/plans_controller_spec.rb | 4 ++-- spec/models/alaveteli_pro/price_spec.rb | 21 ++++++++++++++----- 6 files changed, 46 insertions(+), 26 deletions(-) diff --git a/app/controllers/concerns/alaveteli_pro/stripe_namespace.rb b/app/controllers/concerns/alaveteli_pro/stripe_namespace.rb index ea9829ac34..740739ef2b 100644 --- a/app/controllers/concerns/alaveteli_pro/stripe_namespace.rb +++ b/app/controllers/concerns/alaveteli_pro/stripe_namespace.rb @@ -1,15 +1,17 @@ module AlaveteliPro::StripeNamespace extend ActiveSupport::Concern - def add_stripe_namespace(string) + def add_stripe_namespace(string, prefix: nil) return string if namespace.blank? + return string if prefix && string.start_with?(/#{prefix}_/) return string if string.start_with?(/#{namespace}-/) [namespace, string].join('-') end - def remove_stripe_namespace(string) + def remove_stripe_namespace(string, prefix: nil) return string if namespace.blank? + return string if prefix && string.start_with?(/#{prefix}_/) return string unless string.start_with?(/#{namespace}-/) string.sub(/^#{namespace}-/, '') diff --git a/app/models/alaveteli_pro/price.rb b/app/models/alaveteli_pro/price.rb index 3aaacf9cce..d22316d89d 100644 --- a/app/models/alaveteli_pro/price.rb +++ b/app/models/alaveteli_pro/price.rb @@ -9,21 +9,22 @@ class AlaveteliPro::Price < SimpleDelegator tax :unit_amount def self.list - AlaveteliConfiguration.stripe_price_ids.inject([]) do |arr, id| - price = retrieve(id) - arr << price if price + AlaveteliConfiguration.stripe_prices.inject([]) do |arr, (_, id)| + arr << retrieve(id) arr end end def self.retrieve(id) - new(Stripe::Price.retrieve(add_stripe_namespace(id))) - rescue Stripe::InvalidRequestError - nil + new(Stripe::Price.retrieve(add_stripe_namespace(id, prefix: 'price'))) end def to_param - remove_stripe_namespace(id) + AlaveteliConfiguration.stripe_prices[id] + end + + def ===(other) + self.id == other.id end # product diff --git a/config/general.yml-example b/config/general.yml-example index 63e49ecf1a..87253054b3 100644 --- a/config/general.yml-example +++ b/config/general.yml-example @@ -1202,20 +1202,20 @@ STRIPE_SECRET_KEY: '' # --- STRIPE_NAMESPACE: '' -# List of Stripe Subscription Prices IDs which if a user signs up to will grant -# them access to Alaveteli Pro. Rendered on the Pro pricing pages in the ordered -# defined here. +# 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_PRICE_IDS - Array Stripe Price IDs +# STRIPE_PRICES - Hash of objects with key equla to the Stripe Price IDs as the +# value equal to parameterise short human readable string # # Examples: # -# STRIPE_PRICE_IDS: -# - pro -# - pro-annual-billing +# STRIPE_PRICES: +# price_123: pro +# price:456: pro-annual-billing # # --- -STRIPE_PRICE_IDS: [] +STRIPE_PRICES: [] # Stripe.com webhook secret. Only required for Alaveteli Pro. # diff --git a/lib/configuration.rb b/lib/configuration.rb index 2629833fc5..f986ecaeca 100644 --- a/lib/configuration.rb +++ b/lib/configuration.rb @@ -109,7 +109,7 @@ module AlaveteliConfiguration SMTP_MAILER_PORT: 25, SMTP_MAILER_USER_NAME: '', STRIPE_NAMESPACE: '', - STRIPE_PRICE_IDS: [], + STRIPE_PRICES: {}, STRIPE_PUBLISHABLE_KEY: '', STRIPE_SECRET_KEY: '', STRIPE_TAX_RATE: '0.20', @@ -146,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 e25b38e1bf..36795cf8c6 100644 --- a/spec/controllers/alaveteli_pro/plans_controller_spec.rb +++ b/spec/controllers/alaveteli_pro/plans_controller_spec.rb @@ -8,8 +8,8 @@ let(:product) { stripe_helper.create_product } before do - allow(AlaveteliConfiguration).to receive(:stripe_price_ids). - and_return(['pro']) + allow(AlaveteliConfiguration).to receive(:stripe_prices). + and_return(pro: { enabled: true }) end let!(:pro_price) do diff --git a/spec/models/alaveteli_pro/price_spec.rb b/spec/models/alaveteli_pro/price_spec.rb index 79c404a8e9..05f3f28855 100644 --- a/spec/models/alaveteli_pro/price_spec.rb +++ b/spec/models/alaveteli_pro/price_spec.rb @@ -7,18 +7,29 @@ describe '.list' do before { described_class.instance_variable_set(:@list, nil) } - let(:price_ids) { %w[price_1 price_2 price_3] } - let(:price_1) { double('AlaveteliPro::Price') } - let(:price_2) { double('AlaveteliPro::Price') } + let(:prices) do + { + price_1: { id: 'price_1', enabled: true }, + price_2: { enabled: true }, + price_3: { id: 'price_3', enabled: false }, # not enabled + price_4: { enabled: true } # does not exist + } + end + + let(:price_1) { AlaveteliPro::Price.new(double('Stripe::Price')) } + let(:price_2) { AlaveteliPro::Price.new(double('Stripe::Price')) } + let(:price_3) { AlaveteliPro::Price.new(double('Stripe::Price')) } before do - allow(AlaveteliConfiguration).to receive(:stripe_price_ids). - and_return(price_ids) + allow(AlaveteliConfiguration).to receive(:stripe_prices). + and_return(prices) allow(described_class).to receive(:retrieve).with('price_1'). and_return(price_1) allow(described_class).to receive(:retrieve).with('price_2'). and_return(price_2) allow(described_class).to receive(:retrieve).with('price_3'). + and_return(price_3) + allow(described_class).to receive(:retrieve).with('price_4'). and_return(nil) end From 64f8790779dde3bd0e2740706f9a1b99e1721b22 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Tue, 22 Oct 2024 17:40:35 +0100 Subject: [PATCH 3/3] WIP Add task to migration subscription to new prices --- lib/tasks/subscriptions.rake | 47 ++++++++++++++++++++++++++++++++++++ 1 file changed, 47 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..41bfe7201c --- /dev/null +++ b/lib/tasks/subscriptions.rake @@ -0,0 +1,47 @@ +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 = AlaveteliPro::Price.retrieve(ENV['OLD_PRICE']) if ENV['OLD_PRICE'] + new_price = AlaveteliPro::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 + 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