From 6eda6784439e2056074ecc4bed840744cb263a19 Mon Sep 17 00:00:00 2001 From: Graeme Porteous Date: Wed, 9 Oct 2024 10:28:26 +0100 Subject: [PATCH] Upgrade Stripe API version Bumps the API version to the latest version support by stripe-ruby-mock. Also remove gem lock allowing us to upgrade both stripe-ruby-mock and the main stripe gem. Fixes broken tests and other changes which aren't testable, such as: 1. Invoice#date being rename Invoice#created 2. Plan#name being moved to Plan#product & Product#name and not loaded/ expanded automatically. --- Gemfile | 5 ++-- Gemfile.lock | 20 +++++-------- .../alaveteli_pro/plans_controller.rb | 4 ++- .../alaveteli_pro/subscriptions_controller.rb | 10 ++----- app/models/alaveteli_pro/invoice.rb | 2 +- app/models/alaveteli_pro/subscription.rb | 8 +++++ app/models/pro_account.rb | 7 ++++- .../alaveteli_pro/invoices/_invoice.html.erb | 2 +- app/views/alaveteli_pro/plans/show.html.erb | 2 +- .../subscriptions/_subscription.html.erb | 2 +- config/initializers/stripe.rb | 2 +- .../alaveteli_pro/plans_controller_spec.rb | 2 +- .../subscriptions_controller_spec.rb | 29 +++++++++++++------ spec/lib/alaveteli_pro/metrics_report_spec.rb | 8 +---- spec/models/alaveteli_pro/invoice_spec.rb | 6 ++-- spec/models/pro_account_spec.rb | 14 ++++++--- 16 files changed, 69 insertions(+), 54 deletions(-) diff --git a/Gemfile b/Gemfile index ea229ede5d..cab0da032d 100644 --- a/Gemfile +++ b/Gemfile @@ -126,7 +126,7 @@ gem 'sidekiq', '~> 6.5.12' gem 'sidekiq-limit_fetch', '~> 4.4.1' gem 'statistics2', '~> 0.54' gem 'strip_attributes', git: 'https://github.com/mysociety/strip_attributes.git', branch: 'globalize3-rails7' -gem 'stripe', '~> 5.55.0' +gem 'stripe', '~> 11.7.0' gem 'syck', '~> 1.4.1', require: false gem 'syslog_protocol', '~> 0.9.0' gem 'vpim', '~> 24.2.20' @@ -181,8 +181,7 @@ group :test do gem 'simplecov', '~> 0.22.0' gem 'simplecov-lcov', '~> 0.7.0' gem 'capybara', '~> 3.40.0' - gem 'stripe-ruby-mock', git: 'https://github.com/stripe-ruby-mock/stripe-ruby-mock', - ref: '6ceea96' + gem 'stripe-ruby-mock', '~> 4.0.0' gem 'rails-controller-testing' end diff --git a/Gemfile.lock b/Gemfile.lock index 13083948f8..8649215ee1 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -32,16 +32,6 @@ GIT concurrent-ruby (~> 1.0) rack (> 1, < 3) -GIT - remote: https://github.com/stripe-ruby-mock/stripe-ruby-mock - revision: 6ceea9679bb573cb8bc6830f1bdf670b220a9859 - ref: 6ceea96 - specs: - stripe-ruby-mock (3.1.0.rc3) - dante (>= 0.2.0) - multi_json (~> 1.0) - stripe (> 5, < 6) - PATH remote: gems/alaveteli_features specs: @@ -532,7 +522,11 @@ GEM statistics2 (0.54) stimulus-rails (1.3.4) railties (>= 6.0.0) - stripe (5.55.0) + stripe (11.7.0) + stripe-ruby-mock (4.0.0) + dante (>= 0.2.0) + multi_json (~> 1.0) + stripe (> 5, < 12) syck (1.4.1) syslog_protocol (0.9.2) text (1.3.1) @@ -668,8 +662,8 @@ DEPENDENCIES statistics2 (~> 0.54) stimulus-rails (~> 1.3.4) strip_attributes! - stripe (~> 5.55.0) - stripe-ruby-mock! + stripe (~> 11.7.0) + stripe-ruby-mock (~> 4.0.0) syck (~> 1.4.1) syslog_protocol (~> 0.9.0) turbo-rails (~> 2.0.10) diff --git a/app/controllers/alaveteli_pro/plans_controller.rb b/app/controllers/alaveteli_pro/plans_controller.rb index 23f4f5fa51..fde8cdfd79 100644 --- a/app/controllers/alaveteli_pro/plans_controller.rb +++ b/app/controllers/alaveteli_pro/plans_controller.rb @@ -12,7 +12,9 @@ def index end def show - stripe_plan = Stripe::Plan.retrieve(plan_name) + stripe_plan = Stripe::Plan.retrieve( + id: plan_name, expand: ['product'] + ) @plan = AlaveteliPro::WithTax.new(stripe_plan) rescue Stripe::InvalidRequestError raise ActiveRecord::RecordNotFound diff --git a/app/controllers/alaveteli_pro/subscriptions_controller.rb b/app/controllers/alaveteli_pro/subscriptions_controller.rb index e7bbb87514..24fd5a2f74 100644 --- a/app/controllers/alaveteli_pro/subscriptions_controller.rb +++ b/app/controllers/alaveteli_pro/subscriptions_controller.rb @@ -151,13 +151,9 @@ def destroy @customer = current_user.pro_account.try(:stripe_customer) raise ActiveRecord::RecordNotFound unless @customer - @subscription = Stripe::Subscription.retrieve(params[:id]) - - unless @subscription.customer == @customer.id - raise ActiveRecord::RecordNotFound - end - - @subscription.delete(at_period_end: true) + @subscription = current_user.pro_account.subscriptions. + retrieve(params[:id]) + @subscription.update(cancel_at_period_end: true) flash[:notice] = _('You have successfully cancelled your subscription ' \ 'to {{pro_site_name}}', diff --git a/app/models/alaveteli_pro/invoice.rb b/app/models/alaveteli_pro/invoice.rb index 750fb0827c..b43b113b61 100644 --- a/app/models/alaveteli_pro/invoice.rb +++ b/app/models/alaveteli_pro/invoice.rb @@ -14,7 +14,7 @@ def paid? end # attributes - def date + def created Time.at(super).to_date end diff --git a/app/models/alaveteli_pro/subscription.rb b/app/models/alaveteli_pro/subscription.rb index 78f3e6c011..32e44dfa25 100644 --- a/app/models/alaveteli_pro/subscription.rb +++ b/app/models/alaveteli_pro/subscription.rb @@ -37,6 +37,14 @@ def require_authorisation? ].include?(payment_intent.status) end + def update(attributes) + __setobj__(Stripe::Subscription.update(id, attributes)) + end + + def delete + Stripe::Subscription.cancel(id) + end + private def method_missing(*args) diff --git a/app/models/pro_account.rb b/app/models/pro_account.rb index d668a17847..d227314c8b 100644 --- a/app/models/pro_account.rb +++ b/app/models/pro_account.rb @@ -73,6 +73,11 @@ def update_source end def stripe_customer! - Stripe::Customer.retrieve(stripe_customer_id) if stripe_customer_id + return unless stripe_customer_id + + Stripe::Customer.retrieve( + id: stripe_customer_id, + expand: ['subscriptions.data.plan.product'] + ) end end diff --git a/app/views/alaveteli_pro/invoices/_invoice.html.erb b/app/views/alaveteli_pro/invoices/_invoice.html.erb index 6ca5480be9..318cb631fa 100644 --- a/app/views/alaveteli_pro/invoices/_invoice.html.erb +++ b/app/views/alaveteli_pro/invoices/_invoice.html.erb @@ -1,5 +1,5 @@
  • - <%= invoice.date.to_fs(:long) %> + <%= invoice.created.to_fs(:long) %> <%= _('Invoice {{invoice_number}} for {{invoice_amount}}', invoice_number: invoice.number, diff --git a/app/views/alaveteli_pro/plans/show.html.erb b/app/views/alaveteli_pro/plans/show.html.erb index b450621dca..f878c5afe2 100644 --- a/app/views/alaveteli_pro/plans/show.html.erb +++ b/app/views/alaveteli_pro/plans/show.html.erb @@ -29,7 +29,7 @@

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

    - <%= @plan.name %> + <%= @plan.product.name %>
    <%= billing_frequency(@plan.interval) %> diff --git a/app/views/alaveteli_pro/subscriptions/_subscription.html.erb b/app/views/alaveteli_pro/subscriptions/_subscription.html.erb index 63445b8217..24115cbcc2 100644 --- a/app/views/alaveteli_pro/subscriptions/_subscription.html.erb +++ b/app/views/alaveteli_pro/subscriptions/_subscription.html.erb @@ -1,7 +1,7 @@
    - <%= subscription.plan.name %> + <%= subscription.plan.product.name %>
    diff --git a/config/initializers/stripe.rb b/config/initializers/stripe.rb index 1168991b82..5f3e204f25 100644 --- a/config/initializers/stripe.rb +++ b/config/initializers/stripe.rb @@ -1,5 +1,5 @@ Stripe.api_key = AlaveteliConfiguration.stripe_secret_key -Stripe.api_version = '2017-01-27' +Stripe.api_version = '2020-03-02' Stripe.enable_telemetry = false module Stripe diff --git a/spec/controllers/alaveteli_pro/plans_controller_spec.rb b/spec/controllers/alaveteli_pro/plans_controller_spec.rb index 16bddf17e3..1c689bd623 100644 --- a/spec/controllers/alaveteli_pro/plans_controller_spec.rb +++ b/spec/controllers/alaveteli_pro/plans_controller_spec.rb @@ -138,7 +138,7 @@ subscription = Stripe::Subscription.create(customer: customer, plan: 'pro') - subscription.delete + Stripe::Subscription.cancel(subscription.id) user.create_pro_account(stripe_customer_id: customer.id) get :show, params: { id: 'pro' } end diff --git a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb index 4a548902a5..db4aa3cf5e 100644 --- a/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb +++ b/spec/controllers/alaveteli_pro/subscriptions_controller_spec.rb @@ -898,18 +898,29 @@ Stripe::Subscription.create(customer: customer, plan: plan.id) end - it 'raises an error' do + before do sign_in user - expect { - delete :destroy, params: { id: other_subscription.id } - }.to raise_error ActiveRecord::RecordNotFound + delete :destroy, params: { id: other_subscription.id } + end + + it 'sends an exception email' do + mail = ActionMailer::Base.deliveries.first + expect(mail.subject).to match(/Stripe::InvalidRequestError/) + end + + it 'renders an error message' do + expect(flash[:error]).to match(/There was a problem/) + end + + it 'redirects to the subscriptions page' do + expect(response).to redirect_to(subscriptions_path) end end context 'when we are rate limited' do before do error = Stripe::RateLimitError.new - StripeMock.prepare_error(error, :cancel_subscription) + StripeMock.prepare_error(error, :update_subscription) delete :destroy, params: { id: subscription.id } end @@ -930,7 +941,7 @@ context 'when Stripe receives an invalid request' do before do error = Stripe::InvalidRequestError.new('message', 'param') - StripeMock.prepare_error(error, :cancel_subscription) + StripeMock.prepare_error(error, :update_subscription) delete :destroy, params: { id: subscription.id } end @@ -951,7 +962,7 @@ context 'when we cannot authenticate with Stripe' do before do error = Stripe::AuthenticationError.new - StripeMock.prepare_error(error, :cancel_subscription) + StripeMock.prepare_error(error, :update_subscription) delete :destroy, params: { id: subscription.id } end @@ -972,7 +983,7 @@ context 'when we cannot connect to Stripe' do before do error = Stripe::APIConnectionError.new - StripeMock.prepare_error(error, :cancel_subscription) + StripeMock.prepare_error(error, :update_subscription) delete :destroy, params: { id: subscription.id } end @@ -993,7 +1004,7 @@ context 'when Stripe returns a generic error' do before do error = Stripe::StripeError.new - StripeMock.prepare_error(error, :cancel_subscription) + StripeMock.prepare_error(error, :update_subscription) delete :destroy, params: { id: subscription.id } end diff --git a/spec/lib/alaveteli_pro/metrics_report_spec.rb b/spec/lib/alaveteli_pro/metrics_report_spec.rb index 7d07514455..bc08f9d95a 100644 --- a/spec/lib/alaveteli_pro/metrics_report_spec.rb +++ b/spec/lib/alaveteli_pro/metrics_report_spec.rb @@ -148,13 +148,7 @@ let!(:pending_cancel_sub) do subscription = Stripe::Subscription.create(customer: customer, plan: pro_plan.id) - - # NOTE: - in later API versions, at_period_end is no longer - # available for delete so we'd have to call something like - # this instead: - # Stripe::Subscription.update(subscription.id, - # cancel_at_period_end: true) - subscription.delete(at_period_end: true) + Stripe::Subscription.update(subscription.id, cancel_at_period_end: true) subscription end diff --git a/spec/models/alaveteli_pro/invoice_spec.rb b/spec/models/alaveteli_pro/invoice_spec.rb index c794d0d86f..3b2f1524a5 100644 --- a/spec/models/alaveteli_pro/invoice_spec.rb +++ b/spec/models/alaveteli_pro/invoice_spec.rb @@ -12,7 +12,7 @@ id: 'in_123', status: 'open', charge: 'ch_123', - date: 1722211200, + created: 1722211200, amount_paid: 0 ) end @@ -53,10 +53,10 @@ end end - describe '#date' do + describe '#created' do it 'returns a date object for the invoice' do with_env_tz 'UTC' do - expect(invoice.date).to eq(Date.new(2024, 7, 29)) + expect(invoice.created).to eq(Date.new(2024, 7, 29)) end end end diff --git a/spec/models/pro_account_spec.rb b/spec/models/pro_account_spec.rb index 2c0df6cac5..6df4665624 100644 --- a/spec/models/pro_account_spec.rb +++ b/spec/models/pro_account_spec.rb @@ -37,7 +37,7 @@ end let(:past_due_sub) do - subscription.save + subscription StripeMock.mark_subscription_as_past_due(subscription) Stripe::Subscription.retrieve(subscription.id) end @@ -158,7 +158,7 @@ subject { pro_account.subscription? } context 'when there is an active subscription' do - before { subscription.save } + before { subscription } it { is_expected.to eq true } end @@ -168,12 +168,18 @@ end context 'when there is an expiring subscription' do - before { subscription.delete(at_period_end: true) } + before do + Stripe::Subscription.update(subscription.id, cancel_at_period_end: true) + end + it { is_expected.to eq true } end context 'when an existing subscription is cancelled' do - before { subscription.delete } + before do + Stripe::Subscription.cancel(subscription.id) + end + it { is_expected.to eq false } end