Skip to content

Commit

Permalink
Update Stripe Webhook handling
Browse files Browse the repository at this point in the history
Before this change, webhooks would be ignored if the main object
plan/price ID didn't match the namespace.

This change means we now look up the price ID against those configured
in config/general.yml.

Note: Since only Stripe plans are returned in the webhooks generated
from the stripe-ruby-mock gem the specs still rely on setting up plans
are can't be migrated to prices.
  • Loading branch information
gbp committed Nov 27, 2024
1 parent 8fbf59f commit 0597768
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 27 deletions.
12 changes: 4 additions & 8 deletions app/controllers/alaveteli_pro/stripe_webhooks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ def notify_exception(error)
ExceptionNotifier.notify_exception(error, env: request.env)
end

# ignore any that don't match our plan namespace
def filter_hooks
plans = []
case @stripe_event.data.object.object
Expand All @@ -118,17 +117,14 @@ def filter_hooks
plans = plan_ids(@stripe_event.data.object.lines)
end

# ignore any plans that don't start with our namespace
plans.delete_if { |plan| !plan_matches_namespace?(plan) }
# ignore any prices that aren't configured
plans.delete_if do |price_id|
!AlaveteliConfiguration.stripe_prices.key?(price_id)
end

raise UnknownPlanStripeWebhookError if plans.empty?
end

def plan_matches_namespace?(plan_id)
AlaveteliConfiguration.stripe_namespace == '' ||
plan_id =~ /^#{AlaveteliConfiguration.stripe_namespace}/
end

def plan_ids(items)
items.map { |item| item.plan.id if item.plan }.compact.uniq
end
Expand Down
55 changes: 36 additions & 19 deletions spec/controllers/alaveteli_pro/stripe_webhooks_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

let(:stripe_plan) do
stripe_helper.create_plan(
id: 'test', product: product.id,
id: 'plan_123', product: product.id,
amount: 10, currency: 'gbp'
)
end
Expand All @@ -26,8 +26,8 @@
plan: stripe_plan.id)
end

let(:paid_invoice) do
invoice = Stripe::Invoice.create(
let(:invoice) do
Stripe::Invoice.create(
lines: [
{
data: {
Expand All @@ -42,13 +42,16 @@
],
subscription: stripe_subscription.id
)
invoice.pay
end

let(:paid_invoice) { invoice.pay }

let(:charge) { Stripe::Charge.retrieve(paid_invoice.charge) }

let(:stripe_event) do
StripeMock.mock_webhook_event('customer.subscription.deleted')
StripeMock.mock_webhook_event(
'customer.subscription.deleted', items: stripe_subscription.items
)
end

let(:payload) do
Expand All @@ -63,8 +66,8 @@ def send_request
end

before do
allow(AlaveteliConfiguration).to receive(:stripe_namespace).
and_return('')
allow(AlaveteliConfiguration).to receive(:stripe_prices).
and_return('plan_123' => 'pro')
allow(AlaveteliConfiguration).to receive(:stripe_webhook_secret).
and_return(config_secret)
StripeMock.start
Expand Down Expand Up @@ -183,8 +186,8 @@ def send_request

context 'when using namespaced plans' do
before do
allow(AlaveteliConfiguration).to receive(:stripe_namespace).
and_return('WDTK')
allow(AlaveteliConfiguration).to receive(:stripe_prices).
and_return('WDTK-test' => 'pro')
end

context 'the webhook does not reference our plan namespace' do
Expand Down Expand Up @@ -236,7 +239,9 @@ def send_request

context 'the webhook data does not have namespaced plans' do
let(:stripe_event) do
StripeMock.mock_webhook_event('invoice.payment_succeeded')
StripeMock.mock_webhook_event(
'invoice.payment_succeeded', lines: paid_invoice.lines
)
end

it 'does not raise an error when trying to filter on plan name' do
Expand All @@ -252,7 +257,9 @@ def send_request

describe 'a payment fails' do
let(:stripe_event) do
StripeMock.mock_webhook_event('invoice.payment_failed')
StripeMock.mock_webhook_event(
'invoice.payment_failed', lines: invoice.lines
)
end

let(:customer_id) { stripe_event.data.object.customer }
Expand Down Expand Up @@ -298,7 +305,9 @@ def send_request

describe 'a customer moves to a new billing period' do
let(:stripe_event) do
StripeMock.mock_webhook_event('subscription-renewed')
StripeMock.mock_webhook_event(
'subscription-renewed', items: stripe_subscription.items
)
end

it 'handles the event' do
Expand All @@ -313,7 +322,9 @@ def send_request

describe 'a trial ends' do
let(:stripe_event) do
StripeMock.mock_webhook_event('trial-ended-first-payment-failed')
StripeMock.mock_webhook_event(
'trial-ended-first-payment-failed', items: stripe_subscription.items
)
end

it 'handles the event' do
Expand All @@ -328,7 +339,9 @@ def send_request

describe 'a customer cancels' do
let(:stripe_event) do
StripeMock.mock_webhook_event('subscription-cancelled')
StripeMock.mock_webhook_event(
'subscription-cancelled', items: stripe_subscription.items
)
end

it 'handles the event' do
Expand Down Expand Up @@ -371,9 +384,12 @@ def send_request

context 'when there is a charge for an invoice' do
let(:stripe_event) do
StripeMock.mock_webhook_event('invoice.payment_succeeded',
charge: paid_invoice.charge,
subscription: stripe_subscription.id)
StripeMock.mock_webhook_event(
'invoice.payment_succeeded',
lines: paid_invoice.lines,
charge: paid_invoice.charge,
subscription: stripe_subscription.id
)
end

it 'updates the charge description with the site and plan name' do
Expand All @@ -384,8 +400,9 @@ def send_request

context 'when there is no charge for an invoice' do
let(:stripe_event) do
StripeMock.mock_webhook_event('invoice.payment_succeeded',
charge: nil)
StripeMock.mock_webhook_event(
'invoice.payment_succeeded', lines: paid_invoice.lines, charge: nil
)
end

it 'does not attempt to update the nil charge' do
Expand Down

0 comments on commit 0597768

Please sign in to comment.