Merge pull request #752 from WesternFriend/fix-subscription-bug

Refactor payments

accounts/models.py

@@ -1,7 +1,6 @@
 import datetime
 
 from django.contrib.auth.models import AbstractBaseUser, PermissionsMixin
-from django.core.exceptions import ObjectDoesNotExist
 from django.db import models
 
 from .managers import UserManager
@@ -19,22 +18,24 @@ class User(AbstractBaseUser, PermissionsMixin):
 
     objects = UserManager()
 
-    def __str__(self):
+    def __str__(self) -> str:
         return self.email
 
+    # TODO: refactor this to `get_active_subscriptions`
+    # and return a list of active subscriptions
+    # so we can allow use-cases where a user can have multiple active subscriptions
+    # such as managing subscriptions for a Meeting or a Group
     def get_active_subscription(self):
         """Get subscription that isn't expired for this user."""
         today = datetime.datetime.today()
 
-        try:
-            active_subscription = self.subscriptions.get(end_date__gte=today)
-        except ObjectDoesNotExist:
-            return None
-
-        return active_subscription
+        # using filter.first() instead of get() because get() throws an exception
+        # if there are multiple active subscriptions
+        # TODO: determine how to handle multiple active subscriptions
+        return self.subscriptions.filter(end_date__gte=today).first()  # type: ignore
 
     @property
-    def is_subscriber(self):
+    def is_subscriber(self) -> bool:
         """Check whether user has active subscription."""
 
         if self.get_active_subscription() is not None:

donations/models.py

@@ -1,4 +1,6 @@
+from typing import TYPE_CHECKING
 from django.db import models
+from django.http import HttpRequest, HttpResponse
 from django.shortcuts import redirect
 from django.template.response import TemplateResponse
 from django.urls import reverse
@@ -9,8 +11,14 @@
 
 from addresses.models import Address
 
+if TYPE_CHECKING:
+    from .forms import DonationForm, DonorAddressForm  # pragma: no cover
 
-def process_donation_request(request, donation_form, donor_address_form):
+
+def process_donation_forms(
+    donation_form: "DonationForm",
+    donor_address_form: "DonorAddressForm",
+) -> HttpResponse:
     """Process a donation form and redirect to payment."""
     # Create a temporary donation object to modify it's fields
     donation = donation_form.save(commit=False)
@@ -25,11 +33,15 @@ def process_donation_request(request, donation_form, donor_address_form):
     # Save donation with associated address
     donation.save()
 
-    # set the donation ID in the session
-    request.session["donation_id"] = donation.id
-
     # redirect for payment
-    return redirect(reverse("payment:process", kwargs={"previous_page": "donate"}))
+    return redirect(
+        reverse(
+            "payment:process_donation_payment",
+            kwargs={
+                "donation_id": donation.id,
+            },
+        ),
+    )
 
 
 class SuggestedDonationAmountsBlock(StructBlock):
@@ -42,7 +54,12 @@ class DonatePage(Page):
     intro = RichTextField(blank=True)
     suggested_donation_amounts = StreamField(
         StreamBlock(
-            [("suggested_donation_amounts", SuggestedDonationAmountsBlock(max_num=1))],
+            [
+                (
+                    "suggested_donation_amounts",
+                    SuggestedDonationAmountsBlock(max_num=1),
+                ),
+            ],
             max_num=1,
         ),
         null=True,
@@ -59,23 +76,41 @@ class DonatePage(Page):
     parent_page_types = ["home.HomePage"]
     subpage_types: list[str] = []
 
-    def serve(self, request, *args, **kwargs):
+    def serve(
+        self,
+        request: HttpRequest,
+        *args: tuple,
+        **kwargs: dict,
+    ) -> HttpResponse:
         # Avoid circular dependency
         from .forms import DonationForm, DonorAddressForm
 
         donor_address_form = DonorAddressForm(request.POST)
         donation_form = DonationForm(request.POST)
 
         if request.method == "POST" and donation_form.is_valid():
-            return process_donation_request(request, donation_form, donor_address_form)
+            return process_donation_forms(
+                donation_form,
+                donor_address_form,
+            )
 
         # Send donor address form to client
         # Note, we manually create the donation form in the template
-        context = self.get_context(request, *args, **kwargs)
+        context = self.get_context(
+            request,
+            *args,
+            **kwargs,
+        )
         context["donor_address_form"] = donor_address_form
 
         return TemplateResponse(
-            request, self.get_template(request, *args, **kwargs), context
+            request,
+            self.get_template(
+                request,
+                *args,
+                **kwargs,
+            ),
+            context,
         )
 
 
@@ -97,23 +132,44 @@ class DonationRecurrenceChoices(models.TextChoices):
         choices=DonationRecurrenceChoices.choices,
         default=DonationRecurrenceChoices.ONCE,
     )
-    donor_given_name = models.CharField(max_length=255)
-    donor_family_name = models.CharField(max_length=255)
-    donor_organization = models.CharField(max_length=255, null=True, blank=True)
-    donor_email = models.EmailField(help_text="Please enter your email")
+    donor_given_name = models.CharField(
+        max_length=255,
+    )
+    donor_family_name = models.CharField(
+        max_length=255,
+    )
+    donor_organization = models.CharField(
+        max_length=255,
+        null=True,
+        blank=True,
+    )
+    donor_email = models.EmailField(
+        help_text="Please enter your email",
+    )
     donor_address = models.ForeignKey(
-        to=DonorAddress, null=True, blank=True, on_delete=models.SET_NULL
+        to=DonorAddress,
+        null=True,
+        blank=True,
+        on_delete=models.SET_NULL,
     )
     paid = models.BooleanField(default=False)
-    braintree_transaction_id = models.CharField(max_length=255, null=True, blank=True)
-    braintree_subscription_id = models.CharField(max_length=255, null=True, blank=True)
+    braintree_transaction_id = models.CharField(
+        max_length=255,
+        null=True,
+        blank=True,
+    )
+    braintree_subscription_id = models.CharField(
+        max_length=255,
+        null=True,
+        blank=True,
+    )
     # TODO: add date fields for created, payment_completed, updated
 
-    def get_total_cost(self):
+    def get_total_cost(self) -> int:
         # Add get_total_cost method to conform to payment page
         return self.amount
 
-    def recurring(self):
+    def recurring(self) -> bool:
         """Determine whether Donation is recurring.
 
         Return True if Donation recurrence is "monthly" or "yearly",

donations/tests.py

@@ -1,13 +1,13 @@
 from django.test import TestCase, RequestFactory
 from .forms import DonationForm, DonorAddressForm
 from django.urls import reverse
-from .models import process_donation_request, DonatePage, Donation
+from .models import process_donation_forms, DonatePage, Donation
 from django.http import HttpResponseRedirect
 from wagtail.models import Page
 
 
 class DonationModelTest(TestCase):
-    def test_donation_created(self):
+    def test_donation_created(self) -> None:
         # Create and save Donation object
         donation = Donation(amount=100, recurrence="monthly")
         donation.save()
@@ -16,7 +16,7 @@ def test_donation_created(self):
         self.assertEqual(donation.amount, 100)
         self.assertEqual(donation.recurrence, "monthly")
 
-    def test_process_donation_request_works_correctly(self):
+    def test_process_donation_request_works_correctly(self) -> None:
         # Create completed form objects
         donation_form = DonationForm(
             {
@@ -40,25 +40,29 @@ def test_process_donation_request_works_correctly(self):
         )
 
         # Create response using completed form objects
-        response = process_donation_request(
-            self.client, donation_form, donor_address_form
+        response = process_donation_forms(
+            donation_form,
+            donor_address_form,
         )
 
         # Test that response is an HttpResponseRedirect
         self.assertIsInstance(response, HttpResponseRedirect)
 
         # Test that the response redirects to correct URL
-        expected_url = reverse("payment:process", kwargs={"previous_page": "donate"})
+        expected_url = reverse(
+            "payment:process_donation_payment",
+            kwargs={"donation_id": "3"},
+        )
         self.assertEqual(response.url, expected_url)
 
-    def test_total_cost_method(self):
+    def test_total_cost_method(self) -> None:
         # Create donation object with specified amount
         donation = Donation(amount=100)
 
         # Test that get_total_cost returns specified amount
         self.assertEqual(donation.get_total_cost(), 100)
 
-    def test_recurring(self):
+    def test_recurring(self) -> None:
         # Create donations with each recurrence possibility
         donation_monthly = Donation(
             amount=100, recurrence=Donation.DonationRecurrenceChoices.MONTHLY
@@ -79,22 +83,22 @@ def test_recurring(self):
 
 
 class DonatePageTest(TestCase):
-    def setUp(self):
+    def setUp(self) -> None:
         self.factory = RequestFactory()
         self.donate_page = DonatePage(
             title="Donate", slug="donate", path="00010001", depth=2, numchild=0
         )
         self.home_page = Page.objects.get(slug="home")
         self.home_page.add_child(instance=self.donate_page)
 
-    def test_serve_with_get(self):
+    def test_serve_with_get(self) -> None:
         request = self.factory.get("/donate/")
         response = self.donate_page.serve(request)
 
         # Test that the response contains the donor_address_form in the context
         self.assertIn("donor_address_form", response.context_data)
 
-    def test_serve_with_post_with_valid_data(self):
+    def test_serve_with_post_with_valid_data(self) -> None:
         # Create post request with DonationForm and DonorAddressForm data
         request = self.factory.post(
             "/donate/",
@@ -124,15 +128,15 @@ def test_serve_with_post_with_valid_data(self):
         self.assertEqual(response.status_code, 302)
 
         # Test that the redirect URL is the donate page
-        self.assertEqual(response.url, "/payment/process/donate")
+        self.assertEqual(response.url, "/payment/process/donation/1")
 
         # Get donation object associated with donor address
         donation = Donation.objects.get(donor_address__street_address="123 Main Street")
         # Test that the retrieved donation object's donor
         # address is the same as the one provided in the form
         self.assertEqual(donation.donor_address.street_address, "123 Main Street")
 
-    def test_serve_with_post_with_invalid_data(self):
+    def test_serve_with_post_with_invalid_data(self) -> None:
         # Make a POST request to the donation page with the form data
         response = self.client.post(
             "/donate/",

orders/forms.py

@@ -1,10 +1,15 @@
+from typing import Any
 from django import forms
 
 from .models import Order
 
 
 class OrderCreateForm(forms.ModelForm):
-    def __init__(self, *args, **kwargs):
+    def __init__(
+        self,
+        *args: Any,
+        **kwargs: Any,
+    ) -> None:
         super().__init__(*args, **kwargs)
         self.fields["shipping_cost"].widget = forms.HiddenInput()