diff --git a/Gemfile b/Gemfile index bdbef5a1bf..0023f29245 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' @@ -180,8 +180,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 0f923ab625..0bf1671919 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -23,16 +23,6 @@ GIT strip_attributes (1.12.0) activemodel (>= 3.0, < 8.0) -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: @@ -529,7 +519,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) @@ -664,8 +658,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.11) 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/stripe_webhooks_controller.rb b/app/controllers/alaveteli_pro/stripe_webhooks_controller.rb index cbae7eef91..924e1b1347 100644 --- a/app/controllers/alaveteli_pro/stripe_webhooks_controller.rb +++ b/app/controllers/alaveteli_pro/stripe_webhooks_controller.rb @@ -57,13 +57,14 @@ def invoice_payment_succeeded charge = Stripe::Charge.retrieve(charge_id) subscription_id = @stripe_event.data.object.subscription - subscription = Stripe::Subscription.retrieve(subscription_id) - plan_name = subscription.plan.name - - charge.description = - "#{ pro_site_name }: #{ plan_name }" - - charge.save + subscription = Stripe::Subscription.retrieve( + id: subscription_id, expand: ['plan.product'] + ) + plan_name = subscription.plan.product.name + + Stripe::Charge.update( + charge.id, description: "#{pro_site_name}: #{plan_name}" + ) end end diff --git a/app/controllers/alaveteli_pro/subscriptions_controller.rb b/app/controllers/alaveteli_pro/subscriptions_controller.rb index e7bbb87514..679bf35212 100644 --- a/app/controllers/alaveteli_pro/subscriptions_controller.rb +++ b/app/controllers/alaveteli_pro/subscriptions_controller.rb @@ -49,16 +49,14 @@ def create @pro_account.token = @token @pro_account.update_stripe_customer - @subscription = @pro_account.subscriptions.build - @subscription.update_attributes( + attributes = { plan: params.require(:plan_id), tax_percent: tax_percent, payment_behavior: 'allow_incomplete' - ) - - @subscription.coupon = coupon_code if coupon_code? + } + attributes[:coupon] = coupon_code if coupon_code? - @subscription.save + @subscription = @pro_account.subscriptions.create(attributes) rescue ProAccount::CardError, Stripe::CardError => e @@ -151,13 +149,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/alaveteli_pro/subscription_collection.rb b/app/models/alaveteli_pro/subscription_collection.rb index 7e2bb67167..8a65f9b817 100644 --- a/app/models/alaveteli_pro/subscription_collection.rb +++ b/app/models/alaveteli_pro/subscription_collection.rb @@ -15,12 +15,9 @@ def initialize(customer) @customer = customer end - def build + def create(attributes = {}) AlaveteliPro::Subscription.new( - Stripe::Subscription.new.tap do |subscription| - params = { customer: @customer } - subscription.update_attributes(params) - end + Stripe::Subscription.create(attributes.merge(customer: @customer.id)) ) end diff --git a/app/models/pro_account.rb b/app/models/pro_account.rb index d668a17847..b12fae425c 100644 --- a/app/models/pro_account.rb +++ b/app/models/pro_account.rb @@ -49,30 +49,32 @@ def update_stripe_customer return unless feature_enabled?(:pro_pricing) @subscriptions = nil unless stripe_customer - @stripe_customer = stripe_customer || Stripe::Customer.new - update_email - update_source + attributes = {} + attributes[:email] = user.email if stripe_customer.try(:email) != user.email + attributes[:source] = @token.id if @token + + @stripe_customer = ( + if attributes.empty? + stripe_customer + elsif stripe_customer + Stripe::Customer.update(stripe_customer.id, attributes) + else + Stripe::Customer.create(attributes) + end + ) - stripe_customer.save - update(stripe_customer_id: stripe_customer.id) + update(stripe_customer_id: @stripe_customer.id) end private - def update_email - return unless stripe_customer.try(:email) != user.email - - stripe_customer.email = user.email - end - - def update_source - return unless @token - - stripe_customer.source = @token.id - 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/doc/CHANGES.md b/doc/CHANGES.md index de6464bc10..ffbca9d1f9 100644 --- a/doc/CHANGES.md +++ b/doc/CHANGES.md @@ -2,6 +2,7 @@ ## Highlighted Features +* Upgrade Stripe API version (Graeme Porteous) * Drop support for Azure storage (Graeme Porteous) * Add basic Citation searching in admin UI (Gareth Rees) * Improve citations admin to allow title and description updates (Graeme @@ -140,6 +141,10 @@ `config/nginx-ssl.conf.example` and update your production configuration if needed. +* _Note:_ If you have Pro pricing enabled, this release changes the Stripe API + version from `2017-01-27` to `2020-03-02`. No changes should be necessary to + your Stripe account. + # 0.44.0.1 ## Highlighted Features 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/stripe_webhooks_controller_spec.rb b/spec/controllers/alaveteli_pro/stripe_webhooks_controller_spec.rb index 3c3078b801..41a38e282d 100644 --- a/spec/controllers/alaveteli_pro/stripe_webhooks_controller_spec.rb +++ b/spec/controllers/alaveteli_pro/stripe_webhooks_controller_spec.rb @@ -7,7 +7,7 @@ let(:signing_secret) { config_secret } let(:stripe_helper) { StripeMock.create_test_helper } - let(:product) { stripe_helper.create_product } + let(:product) { stripe_helper.create_product(name: 'Test') } let(:stripe_customer) do Stripe::Customer.create(source: stripe_helper.generate_card_token, @@ -16,7 +16,7 @@ let(:stripe_plan) do stripe_helper.create_plan( - id: 'test', name: 'Test', product: product.id, + id: 'test', product: product.id, amount: 10, currency: 'gbp' ) end @@ -37,7 +37,7 @@ currency: stripe_plan.currency, type: 'subscription' }, - plan: { id: stripe_plan.id, name: stripe_plan.name } + plan: { id: stripe_plan.id } } ], subscription: stripe_subscription.id @@ -203,7 +203,7 @@ def send_request context 'the webhook is for a matching namespaced plan' do let(:stripe_plan) do stripe_helper.create_plan( - id: 'WDTK-test', name: 'Test', product: product.id, + id: 'WDTK-test', product: product.id, amount: 10, currency: 'gbp' ) end @@ -219,7 +219,16 @@ def send_request end it 'returns a 200 OK response' do + allow(Stripe::Subscription).to receive(:retrieve).with( + id: stripe_subscription.id, expand: ['plan.product'] + ).and_return(stripe_subscription) + + allow(stripe_subscription.plan).to receive(:product).and_return( + product + ) + send_request + expect(response.status).to eq(200) expect(response.body).to match('OK') end @@ -330,6 +339,14 @@ def send_request describe 'updating the Stripe charge description when a payment succeeds' do before do + allow(Stripe::Subscription).to receive(:retrieve).with( + id: stripe_subscription.id, expand: ['plan.product'] + ).and_return(stripe_subscription) + + allow(stripe_subscription.plan).to receive(:product).and_return( + product + ) + send_request 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 ce0ef784ee..3b2f1524a5 100644 --- a/spec/models/alaveteli_pro/invoice_spec.rb +++ b/spec/models/alaveteli_pro/invoice_spec.rb @@ -1,16 +1,23 @@ require 'spec_helper' +require 'stripe_mock' RSpec.describe AlaveteliPro::Invoice, type: :model do + before { StripeMock.start } + after { StripeMock.stop } + let(:invoice) { AlaveteliPro::Invoice.new(stripe_invoice) } let(:stripe_invoice) do - double('Stripe::Invoice', - status: 'open', charge: 'ch_123', date: 1722211200, amount_paid: 0) + Stripe::Invoice.create( + id: 'in_123', + status: 'open', + charge: 'ch_123', + created: 1722211200, + amount_paid: 0 + ) end - let(:stripe_charge) do - double('Stripe::Charge', receipt_url: 'http://example.com/receipt') - end + let(:stripe_charge) { Stripe::Charge.new(id: 'ch_123') } before do allow(Stripe::Charge). @@ -46,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 @@ -66,6 +73,12 @@ end describe '#receipt_url' do + before do + allow(stripe_charge).to receive(:receipt_url).and_return( + 'http://example.com/receipt' + ) + end + it 'delegates receipt_url to the charge' do expect(invoice.receipt_url).to eq('http://example.com/receipt') end diff --git a/spec/models/alaveteli_pro/subscription_collection_spec.rb b/spec/models/alaveteli_pro/subscription_collection_spec.rb index f79916dfc8..27a19bde79 100644 --- a/spec/models/alaveteli_pro/subscription_collection_spec.rb +++ b/spec/models/alaveteli_pro/subscription_collection_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +require 'stripe_mock' RSpec.describe AlaveteliPro::SubscriptionCollection do let(:collection) { described_class.new(customer) } @@ -26,11 +27,30 @@ end end - describe '#build' do + describe '#create' do + before { StripeMock.start } + after { StripeMock.stop } + + let(:stripe_helper) { StripeMock.create_test_helper } + + let(:product) { stripe_helper.create_product } + + let(:plan) do + stripe_helper.create_plan( + id: 'pro', product: product.id, amount: 1000 + ) + end + + let(:customer) do + Stripe::Customer.create( + email: 'bob@example.com', source: stripe_helper.generate_card_token + ) + end + let(:collection) { described_class.new(customer) } - let(:subscription) { collection.build } + let(:subscription) { collection.create(plan: plan.id) } - it 'should build new subscription' do + it 'should create new subscription' do expect(subscription).to be_a AlaveteliPro::Subscription end @@ -38,8 +58,8 @@ expect(subscription.__getobj__).to be_a Stripe::Subscription end - it 'should set customer object' do - expect(subscription.customer).to eq customer + it 'should set customer ID' do + expect(subscription.customer).to eq customer.id end end diff --git a/spec/models/pro_account_spec.rb b/spec/models/pro_account_spec.rb index 2c0df6cac5..ae9a61cf24 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 @@ -69,9 +69,9 @@ end it 'creates Stripe customer' do - allow(customer).to receive(:save) + allow(Stripe::Customer).to receive(:create).and_call_original pro_account.update_stripe_customer - expect(customer).to have_received(:save) + expect(Stripe::Customer).to have_received(:create) end it 'sets Stripe customer email' do @@ -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