From fa16bd4e427901c7c353450b85b9c14d11836e8d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Sun, 18 Aug 2019 18:16:37 +0300 Subject: [PATCH] Prepare for 3DS 2 --- hc/payments/models.py | 42 +-- hc/payments/tests/test_get_client_token.py | 6 +- hc/payments/tests/test_payment_method.py | 67 +---- ...et_plan.py => test_update_subscription.py} | 50 ++-- hc/payments/urls.py | 6 +- hc/payments/views.py | 30 +-- static/css/billing.css | 7 +- static/js/billing.js | 111 +++++--- templates/accounts/billing.html | 253 +++++++++--------- 9 files changed, 279 insertions(+), 293 deletions(-) rename hc/payments/tests/{test_set_plan.py => test_update_subscription.py} (80%) diff --git a/hc/payments/models.py b/hc/payments/models.py index 5903791c..d4bfa12b 100644 --- a/hc/payments/models.py +++ b/hc/payments/models.py @@ -66,11 +66,9 @@ class Subscription(models.Model): @property def payment_method(self): - if not self.payment_method_token: - return None - if not hasattr(self, "_pm"): - self._pm = braintree.PaymentMethod.find(self.payment_method_token) + o = self._get_braintree_subscription() + self._pm = braintree.PaymentMethod.find(o.payment_method_token) return self._pm def _get_braintree_subscription(self): @@ -79,43 +77,19 @@ class Subscription(models.Model): return self._sub def get_client_token(self): + assert self.customer_id return braintree.ClientToken.generate({"customer_id": self.customer_id}) def update_payment_method(self, nonce): - # Create customer record if it does not exist: - if not self.customer_id: - result = braintree.Customer.create({"email": self.user.email}) - if not result.is_success: - return result + assert self.subscription_id - self.customer_id = result.customer.id - self.save() - - # Create payment method - result = braintree.PaymentMethod.create( - { - "customer_id": self.customer_id, - "payment_method_nonce": nonce, - "options": {"make_default": True}, - } + result = braintree.Subscription.update( + self.subscription_id, {"payment_method_nonce": nonce} ) if not result.is_success: return result - self.payment_method_token = result.payment_method.token - self.save() - - # Update an existing subscription to use this payment method - if self.subscription_id: - result = braintree.Subscription.update( - self.subscription_id, - {"payment_method_token": self.payment_method_token}, - ) - - if not result.is_success: - return result - def update_address(self, post_data): # Create customer record if it does not exist: if not self.customer_id: @@ -141,9 +115,9 @@ class Subscription(models.Model): if not result.is_success: return result - def setup(self, plan_id): + def setup(self, plan_id, nonce): result = braintree.Subscription.create( - {"payment_method_token": self.payment_method_token, "plan_id": plan_id} + {"payment_method_nonce": nonce, "plan_id": plan_id} ) if result.is_success: diff --git a/hc/payments/tests/test_get_client_token.py b/hc/payments/tests/test_get_client_token.py index eb83a3c7..4256902d 100644 --- a/hc/payments/tests/test_get_client_token.py +++ b/hc/payments/tests/test_get_client_token.py @@ -7,10 +7,14 @@ from hc.test import BaseTestCase class GetClientTokenTestCase(BaseTestCase): @patch("hc.payments.models.braintree") def test_it_works(self, mock_braintree): + sub = Subscription(user=self.alice) + sub.customer_id = "fake-customer-id" + sub.save() + mock_braintree.ClientToken.generate.return_value = "test-token" self.client.login(username="alice@example.org", password="password") - r = self.client.get("/pricing/get_client_token/") + r = self.client.get("/pricing/token/") self.assertContains(r, "test-token", status_code=200) # A subscription object should have been created diff --git a/hc/payments/tests/test_payment_method.py b/hc/payments/tests/test_payment_method.py index 1a2b7ec6..f6a42064 100644 --- a/hc/payments/tests/test_payment_method.py +++ b/hc/payments/tests/test_payment_method.py @@ -5,23 +5,13 @@ from hc.test import BaseTestCase class UpdatePaymentMethodTestCase(BaseTestCase): - def _setup_mock(self, mock): - """ Set up Braintree calls that the controller will use. """ - - mock.PaymentMethod.create.return_value.is_success = True - mock.PaymentMethod.create.return_value.payment_method.token = "fake" - @patch("hc.payments.models.braintree") def test_it_retrieves_paypal(self, mock): - self._setup_mock(mock) - mock.paypal_account.PayPalAccount = dict mock.credit_card.CreditCard = list mock.PaymentMethod.find.return_value = {"email": "foo@example.org"} - self.sub = Subscription(user=self.alice) - self.sub.payment_method_token = "fake-token" - self.sub.save() + Subscription.objects.create(user=self.alice) self.client.login(username="alice@example.org", password="password") r = self.client.get("/accounts/profile/billing/payment_method/") @@ -29,65 +19,12 @@ class UpdatePaymentMethodTestCase(BaseTestCase): @patch("hc.payments.models.braintree") def test_it_retrieves_cc(self, mock): - self._setup_mock(mock) - mock.paypal_account.PayPalAccount = list mock.credit_card.CreditCard = dict mock.PaymentMethod.find.return_value = {"masked_number": "1***2"} - self.sub = Subscription(user=self.alice) - self.sub.payment_method_token = "fake-token" - self.sub.save() + Subscription.objects.create(user=self.alice) self.client.login(username="alice@example.org", password="password") r = self.client.get("/accounts/profile/billing/payment_method/") self.assertContains(r, "1***2") - - @patch("hc.payments.models.braintree") - def test_it_creates_payment_method(self, mock): - self._setup_mock(mock) - - self.sub = Subscription(user=self.alice) - self.sub.customer_id = "test-customer" - self.sub.save() - - self.client.login(username="alice@example.org", password="password") - form = {"payment_method_nonce": "test-nonce"} - r = self.client.post("/accounts/profile/billing/payment_method/", form) - - self.assertRedirects(r, "/accounts/profile/billing/") - - @patch("hc.payments.models.braintree") - def test_it_creates_customer(self, mock): - self._setup_mock(mock) - - mock.Customer.create.return_value.is_success = True - mock.Customer.create.return_value.customer.id = "test-customer-id" - - self.sub = Subscription(user=self.alice) - self.sub.save() - - self.client.login(username="alice@example.org", password="password") - form = {"payment_method_nonce": "test-nonce"} - self.client.post("/accounts/profile/billing/payment_method/", form) - - self.sub.refresh_from_db() - self.assertEqual(self.sub.customer_id, "test-customer-id") - - @patch("hc.payments.models.braintree") - def test_it_updates_subscription(self, mock): - self._setup_mock(mock) - - self.sub = Subscription(user=self.alice) - self.sub.customer_id = "test-customer" - self.sub.subscription_id = "fake-id" - self.sub.save() - - mock.Customer.create.return_value.is_success = True - mock.Customer.create.return_value.customer.id = "test-customer-id" - - self.client.login(username="alice@example.org", password="password") - form = {"payment_method_nonce": "test-nonce"} - self.client.post("/accounts/profile/billing/payment_method/", form) - - self.assertTrue(mock.Subscription.update.called) diff --git a/hc/payments/tests/test_set_plan.py b/hc/payments/tests/test_update_subscription.py similarity index 80% rename from hc/payments/tests/test_set_plan.py rename to hc/payments/tests/test_update_subscription.py index f5fbd696..da073405 100644 --- a/hc/payments/tests/test_set_plan.py +++ b/hc/payments/tests/test_update_subscription.py @@ -4,17 +4,17 @@ from hc.payments.models import Subscription from hc.test import BaseTestCase -class SetPlanTestCase(BaseTestCase): +class UpdateSubscriptionTestCase(BaseTestCase): def _setup_mock(self, mock): """ Set up Braintree calls that the controller will use. """ mock.Subscription.create.return_value.is_success = True mock.Subscription.create.return_value.subscription.id = "t-sub-id" - def run_set_plan(self, plan_id="P20"): - form = {"plan_id": plan_id} + def run_update(self, plan_id="P20", nonce="fake-nonce"): + form = {"plan_id": plan_id, "nonce": nonce} self.client.login(username="alice@example.org", password="password") - return self.client.post("/pricing/set_plan/", form, follow=True) + return self.client.post("/pricing/update/", form, follow=True) @patch("hc.payments.models.braintree") def test_it_works(self, mock): @@ -24,8 +24,9 @@ class SetPlanTestCase(BaseTestCase): self.profile.sms_sent = 1 self.profile.save() - r = self.run_set_plan() + r = self.run_update() self.assertRedirects(r, "/accounts/profile/billing/") + self.assertContains(r, "Your billing plan has been updated!") # Subscription should be filled out: sub = Subscription.objects.get(user=self.alice) @@ -42,7 +43,10 @@ class SetPlanTestCase(BaseTestCase): self.assertEqual(self.profile.sms_sent, 0) # braintree.Subscription.cancel should have not been called - assert not mock.Subscription.cancel.called + # because there was no previous subscription + self.assertFalse(mock.Subscription.cancel.called) + + self.assertTrue(mock.Subscription.create.called) @patch("hc.payments.models.braintree") def test_yearly_works(self, mock): @@ -52,7 +56,7 @@ class SetPlanTestCase(BaseTestCase): self.profile.sms_sent = 1 self.profile.save() - r = self.run_set_plan("Y192") + r = self.run_update("Y192") self.assertRedirects(r, "/accounts/profile/billing/") # Subscription should be filled out: @@ -80,7 +84,7 @@ class SetPlanTestCase(BaseTestCase): self.profile.sms_sent = 1 self.profile.save() - r = self.run_set_plan("P80") + r = self.run_update("P80") self.assertRedirects(r, "/accounts/profile/billing/") # Subscription should be filled out: @@ -114,8 +118,9 @@ class SetPlanTestCase(BaseTestCase): self.profile.sms_sent = 1 self.profile.save() - r = self.run_set_plan("") + r = self.run_update("") self.assertRedirects(r, "/accounts/profile/billing/") + self.assertContains(r, "Your billing plan has been updated!") # Subscription should be cleared sub = Subscription.objects.get(user=self.alice) @@ -130,10 +135,10 @@ class SetPlanTestCase(BaseTestCase): self.assertEqual(self.profile.team_limit, 2) self.assertEqual(self.profile.sms_limit, 0) - assert mock.Subscription.cancel.called + self.assertTrue(mock.Subscription.cancel.called) def test_bad_plan_id(self): - r = self.run_set_plan(plan_id="this-is-wrong") + r = self.run_update(plan_id="this-is-wrong") self.assertEqual(r.status_code, 400) @patch("hc.payments.models.braintree") @@ -144,16 +149,16 @@ class SetPlanTestCase(BaseTestCase): sub.subscription_id = "prev-sub" sub.save() - r = self.run_set_plan() + r = self.run_update() self.assertRedirects(r, "/accounts/profile/billing/") - assert mock.Subscription.cancel.called + self.assertTrue(mock.Subscription.cancel.called) @patch("hc.payments.models.braintree") def test_subscription_creation_failure(self, mock): mock.Subscription.create.return_value.is_success = False mock.Subscription.create.return_value.message = "sub failure" - r = self.run_set_plan() + r = self.run_update() self.assertRedirects(r, "/accounts/profile/billing/") self.assertContains(r, "sub failure") @@ -171,7 +176,7 @@ class SetPlanTestCase(BaseTestCase): mock.Subscription.create.return_value.is_success = False mock.Subscription.create.return_value.message = "sub failure" - r = self.run_set_plan() + r = self.run_update() # It should cancel the current plan self.assertTrue(mock.Subscription.cancel.called) @@ -183,3 +188,18 @@ class SetPlanTestCase(BaseTestCase): # And it should show the error message from API: self.assertRedirects(r, "/accounts/profile/billing/") self.assertContains(r, "sub failure") + + @patch("hc.payments.models.braintree") + def test_it_updates_payment_method(self, mock): + # Initial state: the user has a subscription and a high check limit: + sub = Subscription.objects.for_user(self.alice) + sub.plan_id = "P20" + sub.subscription_id = "old-sub-id" + sub.save() + + r = self.run_update() + + # It should update the existing subscription + self.assertTrue(mock.Subscription.update.called) + self.assertRedirects(r, "/accounts/profile/billing/") + self.assertContains(r, "Your payment method has been updated!") diff --git a/hc/payments/urls.py b/hc/payments/urls.py index e0fabb41..b72d9ed7 100644 --- a/hc/payments/urls.py +++ b/hc/payments/urls.py @@ -19,9 +19,7 @@ urlpatterns = [ path( "invoice/pdf//", views.pdf_invoice, name="hc-invoice-pdf" ), - path("pricing/set_plan/", views.set_plan, name="hc-set-plan"), - path( - "pricing/get_client_token/", views.get_client_token, name="hc-get-client-token" - ), + path("pricing/update/", views.update, name="hc-update-subscription"), + path("pricing/token/", views.token, name="hc-get-client-token"), path("pricing/charge/", views.charge_webhook), ] diff --git a/hc/payments/views.py b/hc/payments/views.py index 85e5e993..d864754f 100644 --- a/hc/payments/views.py +++ b/hc/payments/views.py @@ -20,7 +20,7 @@ from hc.payments.models import Subscription @login_required -def get_client_token(request): +def token(request): sub = Subscription.objects.for_user(request.user) return JsonResponse({"client_token": sub.get_client_token()}) @@ -96,13 +96,21 @@ def log_and_bail(request, result): @login_required @require_POST -def set_plan(request): +def update(request): plan_id = request.POST["plan_id"] + nonce = request.POST["nonce"] + if plan_id not in ("", "P20", "P80", "Y192", "Y768"): return HttpResponseBadRequest() sub = Subscription.objects.for_user(request.user) - if sub.plan_id == plan_id: + # If plan_id has not changed then just update the payment method: + if plan_id == sub.plan_id: + error = sub.update_payment_method(nonce) + if error: + return log_and_bail(request, error) + + request.session["payment_method_status"] = "success" return redirect("hc-billing") # Cancel the previous plan and reset limits: @@ -116,9 +124,10 @@ def set_plan(request): profile.save() if plan_id == "": + request.session["set_plan_status"] = "success" return redirect("hc-billing") - result = sub.setup(plan_id) + result = sub.setup(plan_id, nonce) if not result.is_success: return log_and_bail(request, result) @@ -161,19 +170,6 @@ def address(request): @login_required def payment_method(request): sub = get_object_or_404(Subscription, user=request.user) - - if request.method == "POST": - if "payment_method_nonce" not in request.POST: - return HttpResponseBadRequest() - - nonce = request.POST["payment_method_nonce"] - error = sub.update_payment_method(nonce) - if error: - return log_and_bail(request, error) - - request.session["payment_method_status"] = "success" - return redirect("hc-billing") - ctx = {"sub": sub, "pm": sub.payment_method} return render(request, "payments/payment_method.html", ctx) diff --git a/static/css/billing.css b/static/css/billing.css index ec834e4e..6163a54c 100644 --- a/static/css/billing.css +++ b/static/css/billing.css @@ -12,7 +12,7 @@ } @media (min-width: 992px) { - #change-billing-plan-modal .modal-dialog { + #change-billing-plan-modal .modal-dialog, #payment-method-modal .modal-dialog, #please-wait-modal .modal-dialog { width: 850px; } } @@ -86,3 +86,8 @@ color: #777; } +#please-wait-modal .modal-body { + text-align: center; + padding: 100px; + font-size: 18px; +} \ No newline at end of file diff --git a/static/js/billing.js b/static/js/billing.js index c336186e..b805ebbe 100644 --- a/static/js/billing.js +++ b/static/js/billing.js @@ -1,45 +1,92 @@ $(function () { - var clientTokenRequested = false; - function requestClientToken() { - if (!clientTokenRequested) { - clientTokenRequested = true; - $.getJSON("/pricing/get_client_token/", setupDropin); + var preloadedToken = null; + function getToken(callback) { + if (preloadedToken) { + callback(preloadedToken); + } else { + $.getJSON("/pricing/token/", function(response) { + preloadedToken = response.client_token; + callback(response.client_token); + }); } } - function setupDropin(data) { - braintree.dropin.create({ - authorization: data.client_token, - container: "#dropin", - paypal: { flow: 'vault' } - }, function(createErr, instance) { - $("#payment-form-submit").click(function() { - instance.requestPaymentMethod(function (requestPaymentMethodErr, payload) { - $("#pmm-nonce").val(payload.nonce); - $("#payment-form").submit(); + // Preload client token: + if ($("#billing-address").length) { + getToken(function(token){}); + } + + function getAmount(planId) { + return planId.substr(1); + } + + function showPaymentMethodForm(planId) { + $("#plan-id").val(planId); + $("#nonce").val(""); + + if (planId == "") { + // Don't need a payment method when switching to the free plan + // -- can submit the form right away: + $("#update-subscription-form").submit(); + return; + } + + $("#payment-form-submit").prop("disabled", true); + $("#payment-method-modal").modal("show"); + + getToken(function(token) { + braintree.dropin.create({ + authorization: token, + container: "#dropin", + threeDSecure: { + amount: getAmount(planId), + }, + paypal: { flow: 'vault' }, + preselectVaultedPaymentMethod: false + }, function(createErr, instance) { + $("#payment-form-submit").off().click(function() { + instance.requestPaymentMethod(function (err, payload) { + $("#payment-method-modal").modal("hide"); + $("#please-wait-modal").modal("show"); + + $("#nonce").val(payload.nonce); + $("#update-subscription-form").submit(); + }); }); - }).prop("disabled", false); + + $("#payment-method-modal").off("hidden.bs.modal").on("hidden.bs.modal", function() { + instance.teardown(); + }); + + instance.on("paymentMethodRequestable", function() { + $("#payment-form-submit").prop("disabled", false); + }); + + instance.on("noPaymentMethodRequestable", function() { + $("#payment-form-submit").prop("disabled", true); + }); + + }); }); } - $("#update-payment-method").hover(requestClientToken); - - $("#update-payment-method").click(function() { - requestClientToken(); - $("#payment-form").attr("action", this.dataset.action); - $("#payment-form-submit").text("Update Payment Method"); - $("#payment-method-modal").modal("show"); + $("#change-plan-btn").click(function() { + $("#change-billing-plan-modal").modal("hide"); + showPaymentMethodForm(this.dataset.planId); }); + $("#update-payment-method").click(function() { + showPaymentMethodForm($("#old-plan-id").val()); + }); - $("#billing-history").load( "/accounts/profile/billing/history/" ); - $("#billing-address").load( "/accounts/profile/billing/address/", function() { + $("#billing-history").load("/accounts/profile/billing/history/"); + $("#billing-address").load("/accounts/profile/billing/address/", function() { $("#billing-address input").each(function(idx, obj) { $("#" + obj.name).val(obj.value); }); }); - $("#payment-method").load( "/accounts/profile/billing/payment_method/", function() { + $("#payment-method").load("/accounts/profile/billing/payment_method/", function() { $("#next-billing-date").text($("#nbd").val()); }); @@ -94,9 +141,7 @@ $(function () { if ($("#plan-business-plus").hasClass("selected")) { planId = period == "monthly" ? "P80" : "Y768"; } - - $("#plan-id").val(planId); - + if (planId == $("#old-plan-id").val()) { $("#change-plan-btn") .attr("disabled", "disabled") @@ -105,10 +150,14 @@ $(function () { } else { var caption = "Change Billing Plan"; if (planId) { - caption += " And Pay $" + planId.substr(1) + " Now"; + var amount = planId.substr(1); + caption += " And Pay $" + amount + " Now"; } - $("#change-plan-btn").removeAttr("disabled").text(caption); + $("#change-plan-btn") + .removeAttr("disabled") + .text(caption) + .attr("data-plan-id", planId); } } updateChangePlanForm(); diff --git a/templates/accounts/billing.html b/templates/accounts/billing.html index ee79c4b9..5131baf7 100644 --- a/templates/accounts/billing.html +++ b/templates/accounts/billing.html @@ -76,16 +76,13 @@ {% endif %} + {% if sub.subscription_id %}

Payment Method

- {% if sub.payment_method_token %}

loading…

- {% else %} -

Not set

- {% endif %}
{% endif %}
+ {% endif %}
@@ -170,110 +168,104 @@