[BUILD-393] Store customer_ids on user objects (#2119)

[BUILD-393] Store customer_ids on user objects (#2119)

  • store customer_id on users

  • add signals to create stripe customers

  • finish tests

  • fix tests

diff --git a/ankihub/memberships/migrations/0039_alter_membership_status.py b/ankihub/memberships/migrations/0039_alter_membership_status.py
new file mode 100644
index 0000000..339fea8
--- /dev/null
+++ b/ankihub/memberships/migrations/0039_alter_membership_status.py
@@ -0,0 +1,27 @@
+# Generated by Django 4.2.8 on 2024-05-28 20:40
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("memberships", "0038_organization_metadata"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="membership",
+            name="status",
+            field=models.CharField(
+                choices=[
+                    ("active", "Active"),
+                    ("inactive", "Inactive"),
+                    ("failed", "Failed"),
+                    ("pending", "Pending"),
+                    ("archived", "Archived"),
+                ],
+                default="inactive",
+                max_length=10,
+            ),
+        ),
+    ]
diff --git a/ankihub/memberships/models.py b/ankihub/memberships/models.py
index d331bcc..bca255f 100644
--- a/ankihub/memberships/models.py
+++ b/ankihub/memberships/models.py
@@ -53,7 +53,14 @@ class MembershipQueryset(models.QuerySet):
         )
 
     def get_ankihub_membership(self):
-        return self.get(issuer=None, organization__isnull=True)
+        qs = self.filter(
+            Q(issuer=None, organization__isnull=True)
+            & ~Q(status=MembershipStatusChoices.ARCHIVED)
+        )
+        membership = qs.order_by("status", "-created").first()
+        if not membership:
+            raise Membership.DoesNotExist
+        return membership
 
 
 class MembershipStatusChoices(models.TextChoices):
@@ -61,6 +68,7 @@ class MembershipStatusChoices(models.TextChoices):
     INACTIVE = "inactive", "Inactive"
     FAILED = "failed", "Failed"
     PENDING = "pending", "Pending"
+    ARCHIVED = "archived", "Archived"
 
 
 class MembershipTypesChoices(models.TextChoices):
@@ -192,6 +200,8 @@ class Membership(LifecycleModelMixin, TimeStampedModel):
         self.save(update_fields=["metadata"])
 
     def get_customer_id(self):
+        if self.user.customer_id:
+            return self.user.customer_id
         customer = self.metadata.get("customer")
         return customer.get("id") if customer else None
 
diff --git a/ankihub/memberships/services.py b/ankihub/memberships/services.py
index 1f8ff60..c309971 100644
--- a/ankihub/memberships/services.py
+++ b/ankihub/memberships/services.py
@@ -123,6 +123,20 @@ class MembershipHandler:
         log.warn("no memberships found. Unable to generate membership portal URL")
         return ""
 
+    def get_or_create_stripe_customer(self, user):
+        if not user.customer_id:
+            customer = self.client.Customer.create(
+                email=user.email,
+                name=user.name,
+                metadata={"ankihub_user_id": user.id},
+            )
+            log.info("created Stripe customer", user=user.id)
+            user.customer_id = customer.get("id")
+            user.save(update_fields=["customer_id"])
+        else:
+            customer = self.client.Customer.retrieve(id=user.customer_id)
+        return customer
+
     def get_or_create_membership(
         self, user, payment_method="stripe", organization=None
     ):
@@ -144,12 +158,7 @@ class MembershipHandler:
         except Membership.DoesNotExist:
             customer = None
             if payment_method == "stripe":
-                customer = self.client.Customer.create(
-                    email=user.email,
-                    name=user.name,
-                    metadata={"ankihub_user_id": user.id},
-                )
-                log.info("created Stripe customer", user=user.id)
+                customer = self.get_or_create_stripe_customer(user)
             membership = self.create_membership(
                 user=user, customer=customer, organization=organization
             )
@@ -162,6 +171,10 @@ class MembershipHandler:
             issuer=issuer,
         )
 
+        if not user.customer_id:
+            user.customer_id = customer.get("id")
+            user.save(update_fields=["customer_id"])
+
         if organization:
             organization.membership = membership
             organization.save(update_fields=["membership"])
diff --git a/ankihub/memberships/tests/test_models.py b/ankihub/memberships/tests/test_models.py
index dd1d6c2..3f7a729 100644
--- a/ankihub/memberships/tests/test_models.py
+++ b/ankihub/memberships/tests/test_models.py
@@ -292,6 +292,61 @@ class TestMembershipQueryset:
             "p3",
         }
 
+    def test_get_ankihub_membership(self, user):
+        active_membership = MembershipFactory(
+            user=user, status=MembershipStatusChoices.ACTIVE, created="2024-01-01"
+        )
+        MembershipFactory(
+            user=user, status=MembershipStatusChoices.FAILED, created="2024-02-02"
+        )
+        MembershipFactory(
+            user=user, status=MembershipStatusChoices.ARCHIVED, created="2024-03-03"
+        )
+        membership = Membership.objects.filter_active_by_user(
+            user=user
+        ).get_ankihub_membership()
+        assert membership == active_membership
+
+    def test_get_ankihub_membership_returns_failed(self, user):
+        failed_membership = MembershipFactory(
+            user=user, status=MembershipStatusChoices.FAILED, created="2024-02-02"
+        )
+        MembershipFactory(
+            user=user, status=MembershipStatusChoices.ARCHIVED, created="2024-03-03"
+        )
+        membership = Membership.objects.filter_active_by_user(
+            user=user
+        ).get_ankihub_membership()
+        assert membership == failed_membership
+
+    def test_get_ankihub_membership_return_last_created(self, user):
+        expected_membership = MembershipFactory(
+            user=user, status=MembershipStatusChoices.FAILED, created="2024-03-03"
+        )
+        MembershipFactory(
+            user=user, status=MembershipStatusChoices.FAILED, created="2024-02-02"
+        )
+        membership = Membership.objects.filter_active_by_user(
+            user=user
+        ).get_ankihub_membership()
+        assert membership == expected_membership
+
+    def test_get_ankihub_membership_error_if_archived(self, user):
+        MembershipFactory(
+            user=user, status=MembershipStatusChoices.ARCHIVED, created="2024-03-03"
+        )
+        with pytest.raises(Membership.DoesNotExist):
+            Membership.objects.filter_active_by_user(user=user).get_ankihub_membership()
+
+    def test_get_ankihub_membership_error_if_organization(self, user):
+        membership = MembershipFactory(
+            user=user,
+            status=MembershipStatusChoices.ACTIVE,
+        )
+        OrganizationFactory(owner=user, membership=membership)
+        with pytest.raises(Membership.DoesNotExist):
+            Membership.objects.filter_active_by_user(user=user).get_ankihub_membership()
+
 
 @pytest.mark.django_db
 class TestMembershipMetrics:
diff --git a/ankihub/memberships/tests/test_services.py b/ankihub/memberships/tests/test_services.py
index 4a76354..eb6867f 100644
--- a/ankihub/memberships/tests/test_services.py
+++ b/ankihub/memberships/tests/test_services.py
@@ -213,7 +213,9 @@ def test_get_membership_portal_url_without_metadata(user: User) -> None:
 
 
 def test_get_or_create_membership(mocker: MockerFixture, user: User) -> None:
-    mocker.patch.object(stripe.Customer, "create", return_value={"id": "test"})
+    mocker.patch.object(
+        MembershipHandler, "get_or_create_stripe_customer", return_value={"id": "test"}
+    )
 
     sh = MembershipHandler()
     membership = sh.get_or_create_membership(user)
@@ -227,7 +229,9 @@ def test_get_or_create_membership(mocker: MockerFixture, user: User) -> None:
 def test_get_or_create_membership_for_organization(

[... diff too long, it was truncated ...]

GitHub
sha: e4051ee025c5c3844c084a31c090fa99112f4b4b

Should we also create a default, inactive Membership at this point? of course, that should only be the case if they created their account through the normal flow and aren’t part of an organization. This way, couldn’t we guarantee that new users will only have a single membership?

@augdiebold, shouldn’t this just return something like Membership.objects.get(metadata__customer__id=self.user.customer_id)?

It seems weird to have the customer ID in the Memberbership.metadata__customer__id and in User.customer_id.

@augusto.diebold, I think we should update the create_membership method to only store the information we actually need in the metadata field. e.g.,

  • customer id
  • email

:question: Questions

  • what’s the relationship between User.customer_id and Membership.metadata__customer__id? Which is the source of truth? Do we want/need to have both? Why not store it in a single place?

I did not know the fields inside a JSON field were queryable. I think we can keep storing the metadata, as it is the response from the membership creation from stripe and just create a customer_id field in the Membership model.

1 Like

@andrew I was discussing with @claudiom about the customer_id field and the whole workflow. What about keep having multiple memberships and just archive the last one every time the users upgrade their plans? This way we would have data to track the behavior of our users.

The customer portal would also be available, as the customer_id is being stored in the User object.

If we do it that way, our Membership would be more similar to Stripe’s subscription object. i.e., in Stripe, a user can have multiple subscriptions. If they cancel and later re-subscribe, they have an entirely new Subscription object. Maybe it’s better this way if our data model mirrors what is happening in Stripe.

Canceling subscriptions disables creating new invoices for the subscription and stops automatic collection of all invoices from the subscription by setting auto_advance to false . It also deletes the subscription. If your customer wants to resubscribe, you need to collect new payment information from them and create a new subscription.