feat: Add ACH webhook flows #1106

suejung-sentry · 2025-01-21T23:00:19Z

Handles ACH microdeposits delayed payment verification flow.

This PR handles the below new logical flows:

User creates initial subscription from free to paid with ACH microdeposits as the selected payment method. In the 2 days it takes for that to be completed, what should they see in the UI?
- added unverified_payment_methods list to GET /account-details
User async verifies the ACH microdeposits for the initial checkout session
- added webhook listener for payment_intent.succeeded
- once verified, we set the account as the default payment method on customer, subscription, invoice_settings
- we also upgrade the plan at this point (TODO - confirm this)
- on Stripe's end the "charges_automatically" initital invoice that was pending gets successfully fulfilled
User async verifies the ACH microdeposits for subsequent paymentMethodUpdate
- added webhook listener for setup_intent.succeeded
- once verified, we set the account as the default payment method on customer, subscription, invoice_settings
The initial checkout session by Stripe has a "charges_automatically" that issues an invoice that fails when trying to use the ach microdeposits payment method.
- ignore this case in the webhook listener for stripe invoice.payment_failed that does other stuff like marking the account delinquent
If the user abandons their initial checkout session that has a pending ACH, we delete that abandoned one before offering a way to make a new one
- added to the customer.subscription.deleted listener to ignore this case (vs. the one that wants to downgrade to free)
- adjusted the upgrade_plan logic to handle when we have this incomplete subscription

Other things of note:

Stripe's list payment methods api does not included unverified paymentIntent / setupIntent

Tested end-to-end flows:

Setup initial checkout session
- Card
- ACH instant
- ACH microdeposits
Change payment method
- Card
- ACH instant
- ACH microdeposits
Abandon ACH microdeposits

codecov/engineering-team#2622

codecov · 2025-01-21T23:11:25Z

Codecov Report

Attention: Patch coverage is 34.40860% with 61 lines in your changes missing coverage. Please review.

Project coverage is 95.56%. Comparing base (9f89174) to head (d163c61).
Report is 3 commits behind head on main.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
services/billing.py	30.18%	37 Missing ⚠️
billing/views.py	35.13%	24 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1106      +/-   ##
==========================================
- Coverage   96.09%   95.56%   -0.54%     
==========================================
  Files         832      835       +3     
  Lines       19507    19687     +180     
==========================================
+ Hits        18746    18813      +67     
- Misses        761      874     +113

Flag	Coverage Δ
unit	`95.34% <34.40%> (-0.67%)`	⬇️
unit-latest-uploader	`95.34% <34.40%> (-0.67%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

codecov-notifications · 2025-01-21T23:11:28Z

❌ 47 Tests Failed:

Tests completed	Failed	Passed	Skipped
2710	47	2663	7

View the top 3 failed tests by shortest run time

services/tests/test_billing.py::BillingServiceTests::test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists

Stack Traces | 0.011s run time

self = <services.tests.test_billing.BillingServiceTests testMethod=test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists>
delete_subscription_mock = <MagicMock name='delete_subscription' id='140320004568368'>
modify_subscription_mock = <MagicMock name='modify_subscription' id='140319812682672'>
create_checkout_session_mock = <MagicMock name='create_checkout_session' id='140320415332352'>
set_default_plan_data = <MagicMock name='set_default_plan_data' id='140320007216912'>

    @patch("shared.plan.service.PlanService.set_default_plan_data")
    @patch("services.tests.test_billing.MockPaymentService.create_checkout_session")
    @patch("services.tests.test_billing.MockPaymentService.modify_subscription")
    @patch("services.tests.test_billing.MockPaymentService.delete_subscription")
    def test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists(
        self,
        delete_subscription_mock,
        modify_subscription_mock,
        create_checkout_session_mock,
        set_default_plan_data,
    ):
        owner = OwnerFactory(stripe_subscription_id=10)
        desired_plan = {"value": PlanName.CODECOV_PRO_YEARLY.value, "quantity": 10}
>       self.billing_service.update_plan(owner, desired_plan)

services/tests/test_billing.py:1939: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <services.billing.BillingService object at 0x7f9ecc06f3e0>
owner = <Owner: Owner<github/darlenewaters>>
desired_plan = {'quantity': 10, 'value': 'users-pr-inappy'}

    def update_plan(self, owner, desired_plan):
        """
        Takes an owner and desired plan, and updates the owner's plan. Depending
        on current state, might create a stripe checkout session and return
        the checkout session's ID, which is a string. Otherwise returns None.
        """
        if desired_plan["value"] in FREE_PLAN_REPRESENTATIONS:
            if owner.stripe_subscription_id is not None:
                self.payment_service.delete_subscription(owner)
            else:
                plan_service = PlanService(current_org=owner)
                plan_service.set_default_plan_data()
        elif desired_plan["value"] in PAID_PLANS:
            if owner.stripe_subscription_id is not None:
                # if the existing subscription is incomplete, clean it up and create a new checkout session
>               subscription = self.payment_service.get_subscription(owner)
E               TypeError: MockPaymentService.get_subscription() missing 1 required positional argument: 'plan'

services/billing.py:891: TypeError

api/internal/tests/views/test_account_viewset.py::AccountViewSetTests::test_update_payment_method

Stack Traces | 0.035s run time

self = <MagicMock name='attach' id='140321256718704'>, args = ('pm_123',)
kwargs = {'customer': 'flsoe'}
msg = "Expected 'attach' to be called once. Called 0 times."

    def assert_called_once_with(self, /, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
>           raise AssertionError(msg)
E           AssertionError: Expected 'attach' to be called once. Called 0 times.

.../local/lib/python3.12/unittest/mock.py:960: AssertionError

During handling of the above exception, another exception occurred:

self = <test_account_viewset.AccountViewSetTests testMethod=test_update_payment_method>
modify_subscription_mock = <MagicMock name='modify' id='140321254596464'>
modify_customer_mock = <MagicMock name='modify' id='140321269107888'>
attach_payment_mock = <MagicMock name='attach' id='140321256718704'>
retrieve_subscription_mock = <MagicMock name='retrieve' id='140321268782224'>

    @patch("services.billing.stripe.Subscription.retrieve")
    @patch("services.billing.stripe.PaymentMethod.attach")
    @patch("services.billing.stripe.Customer.modify")
    @patch("services.billing.stripe.Subscription.modify")
    def test_update_payment_method(
        self,
        modify_subscription_mock,
        modify_customer_mock,
        attach_payment_mock,
        retrieve_subscription_mock,
    ):
        self.current_owner.stripe_customer_id = "flsoe"
        self.current_owner.stripe_subscription_id = "djfos"
        self.current_owner.save()
        f = open("..../tests/samples/stripe_invoice.json")
    
        default_payment_method = {
            "card": {
                "brand": "visa",
                "exp_month": 12,
                "exp_year": 2024,
                "last4": "abcd",
                "should be": "removed",
            }
        }
    
        subscription_params = {
            "default_payment_method": default_payment_method,
            "cancel_at_period_end": False,
            "current_period_end": 1633512445,
            "latest_invoice": json.load(f)["data"][0],
            "schedule_id": None,
            "collection_method": "charge_automatically",
            "tax_ids": None,
        }
    
        retrieve_subscription_mock.return_value = MockSubscription(subscription_params)
    
        payment_method_id = "pm_123"
        kwargs = {
            "service": self.current_owner.service,
            "owner_username": self.current_owner.username,
        }
        data = {"payment_method": payment_method_id}
        url = reverse("account_details-update-payment", kwargs=kwargs)
        response = self.client.patch(url, data=data, format="json")
        assert response.status_code == status.HTTP_200_OK
>       attach_payment_mock.assert_called_once_with(
            payment_method_id, customer=self.current_owner.stripe_customer_id
        )
E       AssertionError: Expected 'attach' to be called once. Called 0 times.

.../tests/views/test_account_viewset.py:1148: AssertionError

api/internal/tests/views/test_account_viewset.py::AccountViewSetTests::test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent

Stack Traces | 0.041s run time

self = <test_account_viewset.AccountViewSetTests testMethod=test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent>
mock_retrieve_subscription = <MagicMock name='retrieve' id='140321257607520'>

    @patch("services.billing.stripe.Subscription.retrieve")
    def test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent(
        self, mock_retrieve_subscription
    ):
        owner = OwnerFactory(
            admins=[self.current_owner.ownerid], stripe_subscription_id="sub_123"
        )
        self.current_owner.organizations = [owner.ownerid]
        self.current_owner.save()
    
        subscription_params = {
            "default_payment_method": None,
            "cancel_at_period_end": False,
            "current_period_end": 1633512445,
            "latest_invoice": None,
            "schedule_id": None,
            "collection_method": "charge_automatically",
            "tax_ids": None,
        }
    
        mock_retrieve_subscription.return_value = MockSubscription(subscription_params)
    
        response = self._retrieve(
            kwargs={"service": owner.service, "owner_username": owner.username}
        )
        assert response.status_code == status.HTTP_200_OK
>       assert response.data == {
            "activated_user_count": 0,
            "root_organization": None,
            "integration_id": owner.integration_id,
            "plan_auto_activate": owner.plan_auto_activate,
            "inactive_user_count": 1,
            "plan": {
                "marketing_name": "Developer",
                "value": PlanName.BASIC_PLAN_NAME.value,
                "billing_rate": None,
                "base_unit_price": 0,
                "benefits": [
                    "Up to 1 user",
                    "Unlimited public repositories",
                    "Unlimited private repositories",
                ],
                "quantity": 1,
            },
            "subscription_detail": {
                "latest_invoice": None,
                "default_payment_method": None,
                "cancel_at_period_end": False,
                "current_period_end": 1633512445,
                "customer": {"id": "cus_LK&*Hli8YLIO", "discount": None, "email": None},
                "collection_method": "charge_automatically",
                "trial_end": None,
                "tax_ids": None,
            },
            "checkout_session_id": None,
            "name": owner.name,
            "email": owner.email,
            "nb_active_private_repos": 0,
            "repo_total_credits": 99999999,
            "plan_provider": owner.plan_provider,
            "activated_student_count": 0,
            "student_count": 0,
            "schedule_detail": None,
            "uses_invoice": False,
            "delinquent": None,
        }
E       AssertionError: assert {'integration..._methods': []} == {'activated_s...t': None, ...}
E         
E         Omitting 18 identical items, use -vv to show
E         Left contains 1 more item:
E         {'unverified_payment_methods': []}
E         Use -v to get more diff

.../tests/views/test_account_viewset.py:409: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

github-actions · 2025-01-21T23:11:43Z

❌ 47 Tests Failed:

Tests completed	Failed	Passed	Skipped
2717	47	2664	6

View the top 3 failed tests by shortest run time

test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists

Stack Traces | 0.011s run time

self = &lt;services.tests.test_billing.BillingServiceTests testMethod=test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists&gt;
delete_subscription_mock = &lt;MagicMock name='delete_subscription' id='140320004568368'&gt;
modify_subscription_mock = &lt;MagicMock name='modify_subscription' id='140319812682672'&gt;
create_checkout_session_mock = &lt;MagicMock name='create_checkout_session' id='140320415332352'&gt;
set_default_plan_data = &lt;MagicMock name='set_default_plan_data' id='140320007216912'&gt;

    @patch("shared.plan.service.PlanService.set_default_plan_data")
    @patch("services.tests.test_billing.MockPaymentService.create_checkout_session")
    @patch("services.tests.test_billing.MockPaymentService.modify_subscription")
    @patch("services.tests.test_billing.MockPaymentService.delete_subscription")
    def test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists(
        self,
        delete_subscription_mock,
        modify_subscription_mock,
        create_checkout_session_mock,
        set_default_plan_data,
    ):
        owner = OwnerFactory(stripe_subscription_id=10)
        desired_plan = {"value": PlanName.CODECOV_PRO_YEARLY.value, "quantity": 10}
&gt;       self.billing_service.update_plan(owner, desired_plan)

services/tests/test_billing.py:1939: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = &lt;services.billing.BillingService object at 0x7f9ecc06f3e0&gt;
owner = &lt;Owner: Owner&lt;github/darlenewaters&gt;&gt;
desired_plan = {'quantity': 10, 'value': 'users-pr-inappy'}

    def update_plan(self, owner, desired_plan):
        """
        Takes an owner and desired plan, and updates the owner's plan. Depending
        on current state, might create a stripe checkout session and return
        the checkout session's ID, which is a string. Otherwise returns None.
        """
        if desired_plan["value"] in FREE_PLAN_REPRESENTATIONS:
            if owner.stripe_subscription_id is not None:
                self.payment_service.delete_subscription(owner)
            else:
                plan_service = PlanService(current_org=owner)
                plan_service.set_default_plan_data()
        elif desired_plan["value"] in PAID_PLANS:
            if owner.stripe_subscription_id is not None:
                # if the existing subscription is incomplete, clean it up and create a new checkout session
&gt;               subscription = self.payment_service.get_subscription(owner)
E               TypeError: MockPaymentService.get_subscription() missing 1 required positional argument: 'plan'

services/billing.py:891: TypeError

test_update_payment_method

Stack Traces | 0.035s run time

self = &lt;MagicMock name='attach' id='140321256718704'&gt;, args = ('pm_123',)
kwargs = {'customer': 'flsoe'}
msg = "Expected 'attach' to be called once. Called 0 times."

    def assert_called_once_with(self, /, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
&gt;           raise AssertionError(msg)
E           AssertionError: Expected 'attach' to be called once. Called 0 times.

/usr/local/lib/python3.12/unittest/mock.py:960: AssertionError

During handling of the above exception, another exception occurred:

self = &lt;test_account_viewset.AccountViewSetTests testMethod=test_update_payment_method&gt;
modify_subscription_mock = &lt;MagicMock name='modify' id='140321254596464'&gt;
modify_customer_mock = &lt;MagicMock name='modify' id='140321269107888'&gt;
attach_payment_mock = &lt;MagicMock name='attach' id='140321256718704'&gt;
retrieve_subscription_mock = &lt;MagicMock name='retrieve' id='140321268782224'&gt;

    @patch("services.billing.stripe.Subscription.retrieve")
    @patch("services.billing.stripe.PaymentMethod.attach")
    @patch("services.billing.stripe.Customer.modify")
    @patch("services.billing.stripe.Subscription.modify")
    def test_update_payment_method(
        self,
        modify_subscription_mock,
        modify_customer_mock,
        attach_payment_mock,
        retrieve_subscription_mock,
    ):
        self.current_owner.stripe_customer_id = "flsoe"
        self.current_owner.stripe_subscription_id = "djfos"
        self.current_owner.save()
        f = open("./services/tests/samples/stripe_invoice.json")
    
        default_payment_method = {
            "card": {
                "brand": "visa",
                "exp_month": 12,
                "exp_year": 2024,
                "last4": "abcd",
                "should be": "removed",
            }
        }
    
        subscription_params = {
            "default_payment_method": default_payment_method,
            "cancel_at_period_end": False,
            "current_period_end": 1633512445,
            "latest_invoice": json.load(f)["data"][0],
            "schedule_id": None,
            "collection_method": "charge_automatically",
            "tax_ids": None,
        }
    
        retrieve_subscription_mock.return_value = MockSubscription(subscription_params)
    
        payment_method_id = "pm_123"
        kwargs = {
            "service": self.current_owner.service,
            "owner_username": self.current_owner.username,
        }
        data = {"payment_method": payment_method_id}
        url = reverse("account_details-update-payment", kwargs=kwargs)
        response = self.client.patch(url, data=data, format="json")
        assert response.status_code == status.HTTP_200_OK
&gt;       attach_payment_mock.assert_called_once_with(
            payment_method_id, customer=self.current_owner.stripe_customer_id
        )
E       AssertionError: Expected 'attach' to be called once. Called 0 times.

api/internal/tests/views/test_account_viewset.py:1148: AssertionError

test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent

Stack Traces | 0.041s run time

self = &lt;test_account_viewset.AccountViewSetTests testMethod=test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent&gt;
mock_retrieve_subscription = &lt;MagicMock name='retrieve' id='140321257607520'&gt;

    @patch("services.billing.stripe.Subscription.retrieve")
    def test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent(
        self, mock_retrieve_subscription
    ):
        owner = OwnerFactory(
            admins=[self.current_owner.ownerid], stripe_subscription_id="sub_123"
        )
        self.current_owner.organizations = [owner.ownerid]
        self.current_owner.save()
    
        subscription_params = {
            "default_payment_method": None,
            "cancel_at_period_end": False,
            "current_period_end": 1633512445,
            "latest_invoice": None,
            "schedule_id": None,
            "collection_method": "charge_automatically",
            "tax_ids": None,
        }
    
        mock_retrieve_subscription.return_value = MockSubscription(subscription_params)
    
        response = self._retrieve(
            kwargs={"service": owner.service, "owner_username": owner.username}
        )
        assert response.status_code == status.HTTP_200_OK
&gt;       assert response.data == {
            "activated_user_count": 0,
            "root_organization": None,
            "integration_id": owner.integration_id,
            "plan_auto_activate": owner.plan_auto_activate,
            "inactive_user_count": 1,
            "plan": {
                "marketing_name": "Developer",
                "value": PlanName.BASIC_PLAN_NAME.value,
                "billing_rate": None,
                "base_unit_price": 0,
                "benefits": [
                    "Up to 1 user",
                    "Unlimited public repositories",
                    "Unlimited private repositories",
                ],
                "quantity": 1,
            },
            "subscription_detail": {
                "latest_invoice": None,
                "default_payment_method": None,
                "cancel_at_period_end": False,
                "current_period_end": 1633512445,
                "customer": {"id": "cus_LK&amp;*Hli8YLIO", "discount": None, "email": None},
                "collection_method": "charge_automatically",
                "trial_end": None,
                "tax_ids": None,
            },
            "checkout_session_id": None,
            "name": owner.name,
            "email": owner.email,
            "nb_active_private_repos": 0,
            "repo_total_credits": 99999999,
            "plan_provider": owner.plan_provider,
            "activated_student_count": 0,
            "student_count": 0,
            "schedule_detail": None,
            "uses_invoice": False,
            "delinquent": None,
        }
E       AssertionError: assert {'integration..._methods': []} == {'activated_s...t': None, ...}
E         
E         Omitting 18 identical items, use -vv to show
E         Left contains 1 more item:
E         {'unverified_payment_methods': []}
E         Use -v to get more diff

api/internal/tests/views/test_account_viewset.py:409: AssertionError

📣 Thoughts on this report? Let Codecov know! | Powered by Codecov

codecov-qa · 2025-01-23T23:11:45Z

❌ 47 Tests Failed:

Tests completed	Failed	Passed	Skipped
2710	47	2663	7

View the top 3 failed tests by shortest run time

services/tests/test_billing.py::BillingServiceTests::test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists

Stack Traces | 0.011s run time

self = <services.tests.test_billing.BillingServiceTests testMethod=test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists>
delete_subscription_mock = <MagicMock name='delete_subscription' id='140320004568368'>
modify_subscription_mock = <MagicMock name='modify_subscription' id='140319812682672'>
create_checkout_session_mock = <MagicMock name='create_checkout_session' id='140320415332352'>
set_default_plan_data = <MagicMock name='set_default_plan_data' id='140320007216912'>

    @patch("shared.plan.service.PlanService.set_default_plan_data")
    @patch("services.tests.test_billing.MockPaymentService.create_checkout_session")
    @patch("services.tests.test_billing.MockPaymentService.modify_subscription")
    @patch("services.tests.test_billing.MockPaymentService.delete_subscription")
    def test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists(
        self,
        delete_subscription_mock,
        modify_subscription_mock,
        create_checkout_session_mock,
        set_default_plan_data,
    ):
        owner = OwnerFactory(stripe_subscription_id=10)
        desired_plan = {"value": PlanName.CODECOV_PRO_YEARLY.value, "quantity": 10}
>       self.billing_service.update_plan(owner, desired_plan)

services/tests/test_billing.py:1939: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <services.billing.BillingService object at 0x7f9ecc06f3e0>
owner = <Owner: Owner<github/darlenewaters>>
desired_plan = {'quantity': 10, 'value': 'users-pr-inappy'}

    def update_plan(self, owner, desired_plan):
        """
        Takes an owner and desired plan, and updates the owner's plan. Depending
        on current state, might create a stripe checkout session and return
        the checkout session's ID, which is a string. Otherwise returns None.
        """
        if desired_plan["value"] in FREE_PLAN_REPRESENTATIONS:
            if owner.stripe_subscription_id is not None:
                self.payment_service.delete_subscription(owner)
            else:
                plan_service = PlanService(current_org=owner)
                plan_service.set_default_plan_data()
        elif desired_plan["value"] in PAID_PLANS:
            if owner.stripe_subscription_id is not None:
                # if the existing subscription is incomplete, clean it up and create a new checkout session
>               subscription = self.payment_service.get_subscription(owner)
E               TypeError: MockPaymentService.get_subscription() missing 1 required positional argument: 'plan'

services/billing.py:891: TypeError

api/internal/tests/views/test_account_viewset.py::AccountViewSetTests::test_update_payment_method

Stack Traces | 0.035s run time

self = <MagicMock name='attach' id='140321256718704'>, args = ('pm_123',)
kwargs = {'customer': 'flsoe'}
msg = "Expected 'attach' to be called once. Called 0 times."

    def assert_called_once_with(self, /, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
>           raise AssertionError(msg)
E           AssertionError: Expected 'attach' to be called once. Called 0 times.

.../local/lib/python3.12/unittest/mock.py:960: AssertionError

During handling of the above exception, another exception occurred:

self = <test_account_viewset.AccountViewSetTests testMethod=test_update_payment_method>
modify_subscription_mock = <MagicMock name='modify' id='140321254596464'>
modify_customer_mock = <MagicMock name='modify' id='140321269107888'>
attach_payment_mock = <MagicMock name='attach' id='140321256718704'>
retrieve_subscription_mock = <MagicMock name='retrieve' id='140321268782224'>

    @patch("services.billing.stripe.Subscription.retrieve")
    @patch("services.billing.stripe.PaymentMethod.attach")
    @patch("services.billing.stripe.Customer.modify")
    @patch("services.billing.stripe.Subscription.modify")
    def test_update_payment_method(
        self,
        modify_subscription_mock,
        modify_customer_mock,
        attach_payment_mock,
        retrieve_subscription_mock,
    ):
        self.current_owner.stripe_customer_id = "flsoe"
        self.current_owner.stripe_subscription_id = "djfos"
        self.current_owner.save()
        f = open("..../tests/samples/stripe_invoice.json")
    
        default_payment_method = {
            "card": {
                "brand": "visa",
                "exp_month": 12,
                "exp_year": 2024,
                "last4": "abcd",
                "should be": "removed",
            }
        }
    
        subscription_params = {
            "default_payment_method": default_payment_method,
            "cancel_at_period_end": False,
            "current_period_end": 1633512445,
            "latest_invoice": json.load(f)["data"][0],
            "schedule_id": None,
            "collection_method": "charge_automatically",
            "tax_ids": None,
        }
    
        retrieve_subscription_mock.return_value = MockSubscription(subscription_params)
    
        payment_method_id = "pm_123"
        kwargs = {
            "service": self.current_owner.service,
            "owner_username": self.current_owner.username,
        }
        data = {"payment_method": payment_method_id}
        url = reverse("account_details-update-payment", kwargs=kwargs)
        response = self.client.patch(url, data=data, format="json")
        assert response.status_code == status.HTTP_200_OK
>       attach_payment_mock.assert_called_once_with(
            payment_method_id, customer=self.current_owner.stripe_customer_id
        )
E       AssertionError: Expected 'attach' to be called once. Called 0 times.

.../tests/views/test_account_viewset.py:1148: AssertionError

api/internal/tests/views/test_account_viewset.py::AccountViewSetTests::test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent

Stack Traces | 0.041s run time

self = <test_account_viewset.AccountViewSetTests testMethod=test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent>
mock_retrieve_subscription = <MagicMock name='retrieve' id='140321257607520'>

    @patch("services.billing.stripe.Subscription.retrieve")
    def test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent(
        self, mock_retrieve_subscription
    ):
        owner = OwnerFactory(
            admins=[self.current_owner.ownerid], stripe_subscription_id="sub_123"
        )
        self.current_owner.organizations = [owner.ownerid]
        self.current_owner.save()
    
        subscription_params = {
            "default_payment_method": None,
            "cancel_at_period_end": False,
            "current_period_end": 1633512445,
            "latest_invoice": None,
            "schedule_id": None,
            "collection_method": "charge_automatically",
            "tax_ids": None,
        }
    
        mock_retrieve_subscription.return_value = MockSubscription(subscription_params)
    
        response = self._retrieve(
            kwargs={"service": owner.service, "owner_username": owner.username}
        )
        assert response.status_code == status.HTTP_200_OK
>       assert response.data == {
            "activated_user_count": 0,
            "root_organization": None,
            "integration_id": owner.integration_id,
            "plan_auto_activate": owner.plan_auto_activate,
            "inactive_user_count": 1,
            "plan": {
                "marketing_name": "Developer",
                "value": PlanName.BASIC_PLAN_NAME.value,
                "billing_rate": None,
                "base_unit_price": 0,
                "benefits": [
                    "Up to 1 user",
                    "Unlimited public repositories",
                    "Unlimited private repositories",
                ],
                "quantity": 1,
            },
            "subscription_detail": {
                "latest_invoice": None,
                "default_payment_method": None,
                "cancel_at_period_end": False,
                "current_period_end": 1633512445,
                "customer": {"id": "cus_LK&*Hli8YLIO", "discount": None, "email": None},
                "collection_method": "charge_automatically",
                "trial_end": None,
                "tax_ids": None,
            },
            "checkout_session_id": None,
            "name": owner.name,
            "email": owner.email,
            "nb_active_private_repos": 0,
            "repo_total_credits": 99999999,
            "plan_provider": owner.plan_provider,
            "activated_student_count": 0,
            "student_count": 0,
            "schedule_detail": None,
            "uses_invoice": False,
            "delinquent": None,
        }
E       AssertionError: assert {'integration..._methods': []} == {'activated_s...t': None, ...}
E         
E         Omitting 18 identical items, use -vv to show
E         Left contains 1 more item:
E         {'unverified_payment_methods': []}
E         Use -v to get more diff

.../tests/views/test_account_viewset.py:409: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

codecov-public-qa · 2025-01-23T23:11:50Z

❌ 47 Tests Failed:

Tests completed	Failed	Passed	Skipped
2711	47	2664	6

View the top 3 failed tests by shortest run time

services/tests/test_billing.py::BillingServiceTests::test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists

Stack Traces | 0.011s run time

self = <services.tests.test_billing.BillingServiceTests testMethod=test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists>
delete_subscription_mock = <MagicMock name='delete_subscription' id='140320004568368'>
modify_subscription_mock = <MagicMock name='modify_subscription' id='140319812682672'>
create_checkout_session_mock = <MagicMock name='create_checkout_session' id='140320415332352'>
set_default_plan_data = <MagicMock name='set_default_plan_data' id='140320007216912'>

    @patch("shared.plan.service.PlanService.set_default_plan_data")
    @patch("services.tests.test_billing.MockPaymentService.create_checkout_session")
    @patch("services.tests.test_billing.MockPaymentService.modify_subscription")
    @patch("services.tests.test_billing.MockPaymentService.delete_subscription")
    def test_update_plan_modifies_subscription_if_user_plan_and_subscription_exists(
        self,
        delete_subscription_mock,
        modify_subscription_mock,
        create_checkout_session_mock,
        set_default_plan_data,
    ):
        owner = OwnerFactory(stripe_subscription_id=10)
        desired_plan = {"value": PlanName.CODECOV_PRO_YEARLY.value, "quantity": 10}
>       self.billing_service.update_plan(owner, desired_plan)

services/tests/test_billing.py:1939: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <services.billing.BillingService object at 0x7f9ecc06f3e0>
owner = <Owner: Owner<github/darlenewaters>>
desired_plan = {'quantity': 10, 'value': 'users-pr-inappy'}

    def update_plan(self, owner, desired_plan):
        """
        Takes an owner and desired plan, and updates the owner's plan. Depending
        on current state, might create a stripe checkout session and return
        the checkout session's ID, which is a string. Otherwise returns None.
        """
        if desired_plan["value"] in FREE_PLAN_REPRESENTATIONS:
            if owner.stripe_subscription_id is not None:
                self.payment_service.delete_subscription(owner)
            else:
                plan_service = PlanService(current_org=owner)
                plan_service.set_default_plan_data()
        elif desired_plan["value"] in PAID_PLANS:
            if owner.stripe_subscription_id is not None:
                # if the existing subscription is incomplete, clean it up and create a new checkout session
>               subscription = self.payment_service.get_subscription(owner)
E               TypeError: MockPaymentService.get_subscription() missing 1 required positional argument: 'plan'

services/billing.py:891: TypeError

api/internal/tests/views/test_account_viewset.py::AccountViewSetTests::test_update_payment_method

Stack Traces | 0.035s run time

self = <MagicMock name='attach' id='140321256718704'>, args = ('pm_123',)
kwargs = {'customer': 'flsoe'}
msg = "Expected 'attach' to be called once. Called 0 times."

    def assert_called_once_with(self, /, *args, **kwargs):
        """assert that the mock was called exactly once and that that call was
        with the specified arguments."""
        if not self.call_count == 1:
            msg = ("Expected '%s' to be called once. Called %s times.%s"
                   % (self._mock_name or 'mock',
                      self.call_count,
                      self._calls_repr()))
>           raise AssertionError(msg)
E           AssertionError: Expected 'attach' to be called once. Called 0 times.

.../local/lib/python3.12/unittest/mock.py:960: AssertionError

During handling of the above exception, another exception occurred:

self = <test_account_viewset.AccountViewSetTests testMethod=test_update_payment_method>
modify_subscription_mock = <MagicMock name='modify' id='140321254596464'>
modify_customer_mock = <MagicMock name='modify' id='140321269107888'>
attach_payment_mock = <MagicMock name='attach' id='140321256718704'>
retrieve_subscription_mock = <MagicMock name='retrieve' id='140321268782224'>

    @patch("services.billing.stripe.Subscription.retrieve")
    @patch("services.billing.stripe.PaymentMethod.attach")
    @patch("services.billing.stripe.Customer.modify")
    @patch("services.billing.stripe.Subscription.modify")
    def test_update_payment_method(
        self,
        modify_subscription_mock,
        modify_customer_mock,
        attach_payment_mock,
        retrieve_subscription_mock,
    ):
        self.current_owner.stripe_customer_id = "flsoe"
        self.current_owner.stripe_subscription_id = "djfos"
        self.current_owner.save()
        f = open("..../tests/samples/stripe_invoice.json")
    
        default_payment_method = {
            "card": {
                "brand": "visa",
                "exp_month": 12,
                "exp_year": 2024,
                "last4": "abcd",
                "should be": "removed",
            }
        }
    
        subscription_params = {
            "default_payment_method": default_payment_method,
            "cancel_at_period_end": False,
            "current_period_end": 1633512445,
            "latest_invoice": json.load(f)["data"][0],
            "schedule_id": None,
            "collection_method": "charge_automatically",
            "tax_ids": None,
        }
    
        retrieve_subscription_mock.return_value = MockSubscription(subscription_params)
    
        payment_method_id = "pm_123"
        kwargs = {
            "service": self.current_owner.service,
            "owner_username": self.current_owner.username,
        }
        data = {"payment_method": payment_method_id}
        url = reverse("account_details-update-payment", kwargs=kwargs)
        response = self.client.patch(url, data=data, format="json")
        assert response.status_code == status.HTTP_200_OK
>       attach_payment_mock.assert_called_once_with(
            payment_method_id, customer=self.current_owner.stripe_customer_id
        )
E       AssertionError: Expected 'attach' to be called once. Called 0 times.

.../tests/views/test_account_viewset.py:1148: AssertionError

api/internal/tests/views/test_account_viewset.py::AccountViewSetTests::test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent

Stack Traces | 0.041s run time

self = <test_account_viewset.AccountViewSetTests testMethod=test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent>
mock_retrieve_subscription = <MagicMock name='retrieve' id='140321257607520'>

    @patch("services.billing.stripe.Subscription.retrieve")
    def test_retrieve_account_gets_none_for_schedule_details_when_schedule_is_nonexistent(
        self, mock_retrieve_subscription
    ):
        owner = OwnerFactory(
            admins=[self.current_owner.ownerid], stripe_subscription_id="sub_123"
        )
        self.current_owner.organizations = [owner.ownerid]
        self.current_owner.save()
    
        subscription_params = {
            "default_payment_method": None,
            "cancel_at_period_end": False,
            "current_period_end": 1633512445,
            "latest_invoice": None,
            "schedule_id": None,
            "collection_method": "charge_automatically",
            "tax_ids": None,
        }
    
        mock_retrieve_subscription.return_value = MockSubscription(subscription_params)
    
        response = self._retrieve(
            kwargs={"service": owner.service, "owner_username": owner.username}
        )
        assert response.status_code == status.HTTP_200_OK
>       assert response.data == {
            "activated_user_count": 0,
            "root_organization": None,
            "integration_id": owner.integration_id,
            "plan_auto_activate": owner.plan_auto_activate,
            "inactive_user_count": 1,
            "plan": {
                "marketing_name": "Developer",
                "value": PlanName.BASIC_PLAN_NAME.value,
                "billing_rate": None,
                "base_unit_price": 0,
                "benefits": [
                    "Up to 1 user",
                    "Unlimited public repositories",
                    "Unlimited private repositories",
                ],
                "quantity": 1,
            },
            "subscription_detail": {
                "latest_invoice": None,
                "default_payment_method": None,
                "cancel_at_period_end": False,
                "current_period_end": 1633512445,
                "customer": {"id": "cus_LK&*Hli8YLIO", "discount": None, "email": None},
                "collection_method": "charge_automatically",
                "trial_end": None,
                "tax_ids": None,
            },
            "checkout_session_id": None,
            "name": owner.name,
            "email": owner.email,
            "nb_active_private_repos": 0,
            "repo_total_credits": 99999999,
            "plan_provider": owner.plan_provider,
            "activated_student_count": 0,
            "student_count": 0,
            "schedule_detail": None,
            "uses_invoice": False,
            "delinquent": None,
        }
E       AssertionError: assert {'integration..._methods': []} == {'activated_s...t': None, ...}
E         
E         Omitting 18 identical items, use -vv to show
E         Left contains 1 more item:
E         {'unverified_payment_methods': []}
E         Use -v to get more diff

.../tests/views/test_account_viewset.py:409: AssertionError

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

suejung-sentry · 2025-01-24T05:36:29Z

billing/constants.py

@@ -17,6 +17,8 @@ class StripeWebhookEvents:
        "customer.updated",
        "invoice.payment_failed",
        "invoice.payment_succeeded",
+        "payment_intent.succeeded",
+        "setup_intent.succeeded",


TODO - these need to be turned on in Stripe dashboard

ajay-sentry · 2025-01-25T00:34:07Z

services/billing.py

+
+    def _cleanup_incomplete_subscription(self, subscription, owner):
+        latest_invoice = subscription.latest_invoice
+        if not latest_invoice or not latest_invoice.payment_intent:


may need to do a safe access here incase the attribute doesn't exist

ajay-sentry · 2025-01-25T00:36:05Z

services/billing.py

@@ -802,8 +887,18 @@ def update_plan(self, owner, desired_plan):
                plan_service.set_default_plan_data()
        elif desired_plan["value"] in PAID_PLANS:
            if owner.stripe_subscription_id is not None:
+                # if the existing subscription is incomplete, clean it up and create a new checkout session
+                subscription = self.payment_service.get_subscription(owner)
+                if subscription and subscription.status == "incomplete":


same here for safe access

ajay-sentry · 2025-01-25T00:37:36Z

services/billing.py

@@ -758,6 +837,9 @@ def apply_cancellation_discount(self, owner: Owner):
    def create_setup_intent(self, owner):
        pass

+    def get_unverified_payment_methods(self, owner):
+        return []


should this be a pass?

ajay-sentry · 2025-01-25T00:38:48Z

services/billing.py

+        payment_intents = stripe.PaymentIntent.list(
+            customer=owner.stripe_customer_id, limit=100
+        )
+        for intent in payment_intents.data:


will this and the below data object always exist?

ajay-sentry · 2025-01-25T00:40:42Z

services/billing.py

+        )
+        for intent in payment_intents.data:
+            if (
+                hasattr(intent, "next_action")


could maybe do a .get() here instead

ajay-sentry · 2025-01-25T00:40:49Z

services/billing.py

+        )
+        for intent in setup_intents.data:
+            if (
+                hasattr(intent, "next_action")


similar here

ajay-sentry · 2025-01-25T00:41:44Z

services/billing.py

@@ -718,6 +752,51 @@ def create_setup_intent(self, owner: Owner) -> stripe.SetupIntent:
            customer=owner.stripe_customer_id,
        )

+    def _get_unverified_payment_methods(self, owner):
+        log.info(
+            "Getting unverified payment methods", extra=dict(owner_id=owner.ownerid)


should we add the customerid to this log too?

ajay-sentry · 2025-01-25T00:43:48Z

services/billing.py

+        unverified_payment_methods = []
+
+        # Check payment intents
+        payment_intents = stripe.PaymentIntent.list(


I'm a little surprised stripe doesn't have a shared API for these two considering the properties being pulled off of each are the same

feat: Add ACH webhook flows

b66b68e

wip

e1389b3

handle plan upgrade flow

802225d

suejung-sentry marked this pull request as ready for review January 24, 2025 05:35

suejung-sentry commented Jan 24, 2025

View reviewed changes

suejung-sentry added 3 commits January 23, 2025 21:39

cleanup

bd566d0

fix delete subscription

48b2e89

cleanup

d163c61

suejung-sentry force-pushed the sshin/3227 branch from 27940a4 to d163c61 Compare January 24, 2025 21:28

suejung-sentry mentioned this pull request Jan 24, 2025

[Testing] Confirm the "manual bank account" flow codecov/engineering-team#3227

Closed

ajay-sentry reviewed Jan 25, 2025

View reviewed changes

services/billing.py

)

for intent in setup_intents.data:

if (

hasattr(intent, "next_action")

Copy link

Contributor

ajay-sentry Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar here

ajay-sentry reviewed Jan 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add ACH webhook flows #1106

feat: Add ACH webhook flows #1106

suejung-sentry commented Jan 21, 2025 •

edited

Loading

codecov bot commented Jan 21, 2025 •

edited

Loading

codecov-notifications bot commented Jan 21, 2025 •

edited by codecov-staging bot

Loading

github-actions bot commented Jan 21, 2025 •

edited

Loading

codecov-qa bot commented Jan 23, 2025 •

edited

Loading

codecov-public-qa bot commented Jan 23, 2025 •

edited

Loading

suejung-sentry Jan 24, 2025

ajay-sentry Jan 25, 2025

ajay-sentry Jan 25, 2025

ajay-sentry Jan 25, 2025

ajay-sentry Jan 25, 2025

ajay-sentry Jan 25, 2025

ajay-sentry Jan 25, 2025

ajay-sentry Jan 25, 2025

ajay-sentry Jan 25, 2025

feat: Add ACH webhook flows #1106

Are you sure you want to change the base?

feat: Add ACH webhook flows #1106

Conversation

suejung-sentry commented Jan 21, 2025 • edited Loading

codecov bot commented Jan 21, 2025 • edited Loading

Codecov Report

codecov-notifications bot commented Jan 21, 2025 • edited by codecov-staging bot Loading

❌ 47 Tests Failed:

github-actions bot commented Jan 21, 2025 • edited Loading

❌ 47 Tests Failed:

codecov-qa bot commented Jan 23, 2025 • edited Loading

❌ 47 Tests Failed:

codecov-public-qa bot commented Jan 23, 2025 • edited Loading

❌ 47 Tests Failed:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suejung-sentry commented Jan 21, 2025 •

edited

Loading

codecov bot commented Jan 21, 2025 •

edited

Loading

codecov-notifications bot commented Jan 21, 2025 •

edited by codecov-staging bot

Loading

github-actions bot commented Jan 21, 2025 •

edited

Loading

codecov-qa bot commented Jan 23, 2025 •

edited

Loading

codecov-public-qa bot commented Jan 23, 2025 •

edited

Loading