From b5c5ab8e61f9433556d17cf93f9b1a28b1057d34 Mon Sep 17 00:00:00 2001
From: RobertJoonas <56999674+RobertJoonas@users.noreply.github.com>
Date: Tue, 18 Jun 2024 11:47:16 +0300
Subject: [PATCH] ignore irrelevant paddle webhook (#4240)

---
 lib/plausible/billing/billing.ex        | 17 ++++++++++++++++-
 test/plausible/billing/billing_test.exs | 15 +++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/lib/plausible/billing/billing.ex b/lib/plausible/billing/billing.ex
index 10488656a..0bb29e5bc 100644
--- a/lib/plausible/billing/billing.ex
+++ b/lib/plausible/billing/billing.ex
@@ -137,7 +137,22 @@ defmodule Plausible.Billing do
   defp handle_subscription_updated(params) do
     subscription = Repo.get_by(Subscription, paddle_subscription_id: params["subscription_id"])
 
-    if subscription do
+    # In a situation where the subscription is paused and a payment succeeds, we
+    # get notified of two "subscription_updated" webhook alerts from Paddle at the
+    # same time.
+    #
+    #   * one with an `old_status` of "paused", and a `status` of "past_due"
+    #   * the other with an `old_status` of "past_due", and a `status` of "active"
+    #
+    # https://developer.paddle.com/classic/guides/zg9joji1mzu0mduy-payment-failures
+    #
+    # Relying on the time when the webhooks are sent has caused issues where
+    # subscriptions have ended up `past_due` after a successful payment. Therefore,
+    # we're now explicitly ignoring the first webhook (with the update that's not
+    # relevant to us).
+    irrelevant? = params["old_status"] == "paused" && params["status"] == "past_due"
+
+    if subscription && not irrelevant? do
       subscription
       |> Subscription.changeset(format_subscription(params))
       |> Repo.update!()
diff --git a/test/plausible/billing/billing_test.exs b/test/plausible/billing/billing_test.exs
index e4b19cd85..082a1e448 100644
--- a/test/plausible/billing/billing_test.exs
+++ b/test/plausible/billing/billing_test.exs
@@ -212,6 +212,21 @@ defmodule Plausible.BillingTest do
       assert subscription.next_bill_amount == "12.00"
     end
 
+    test "status update from 'paused' to 'past_due' is ignored" do
+      user = insert(:user)
+      subscription = insert(:subscription, user: user, status: Subscription.Status.paused())
+
+      %{@subscription_updated_params | "old_status" => "paused", "status" => "past_due"}
+      |> Map.merge(%{
+        "subscription_id" => subscription.paddle_subscription_id,
+        "passthrough" => user.id
+      })
+      |> Billing.subscription_updated()
+
+      subscription = Repo.get_by(Subscription, user_id: user.id)
+      assert subscription.status == Subscription.Status.paused()
+    end
+
     test "unlocks sites if subscription is changed from past_due to active" do
       user = insert(:user)
       subscription = insert(:subscription, user: user, status: Subscription.Status.past_due())