♻️ [#2257] Fetch subscriptions for user directly from Laposta

task: https://taiga.maykinmedia.nl/project/open-inwoner/task/2257

src/open_inwoner/accounts/tests/test_profile_views.py

@@ -17,7 +17,7 @@
 from open_inwoner.cms.profile.cms_appconfig import ProfileConfig
 from open_inwoner.haalcentraal.tests.mixins import HaalCentraalMixin
 from open_inwoner.laposta.models import LapostaConfig
-from open_inwoner.laposta.tests.factories import LapostaListFactory, SubscriptionFactory
+from open_inwoner.laposta.tests.factories import LapostaListFactory, MemberFactory
 from open_inwoner.openklant.models import OpenKlantConfig
 from open_inwoner.pdc.tests.factories import CategoryFactory
 from open_inwoner.plans.tests.factories import PlanFactory
@@ -1077,7 +1077,24 @@ def test_do_not_render_form_if_no_newsletters_are_found(self, m):
     def test_render_form_if_newsletters_are_found(self, m):
         self.setUpMocks(m)
 
-        SubscriptionFactory.create(list_id=self.list1.list_id, user=self.user)
+        self.config.limit_list_selection_to = ["list1", "list2"]
+        self.config.save()
+
+        m.get(
+            f"{self.config.api_root}member/{self.user.email}?list_id=list1",
+            json={
+                "member": MemberFactory.build(
+                    list_id="list1",
+                    member_id="1234567",
+                    email=self.user.email,
+                    custom_fields=None,
+                ).model_dump()
+            },
+        )
+        m.get(
+            f"{self.config.api_root}member/{self.user.email}?list_id=list2",
+            status_code=400,
+        )
 
         response = self.app.get(self.profile_url, user=self.user)
 
@@ -1098,7 +1115,21 @@ def test_render_form_limit_newsletters_to_admin_selection(self, m):
         self.config.limit_list_selection_to = ["list1"]
         self.config.save()
 
-        SubscriptionFactory.create(list_id=self.list1.list_id, user=self.user)
+        m.get(
+            f"{self.config.api_root}member/{self.user.email}?list_id=list1",
+            json={
+                "member": MemberFactory.build(
+                    list_id="list1",
+                    member_id="1234567",
+                    email=self.user.email,
+                    custom_fields=None,
+                ).model_dump()
+            },
+        )
+        m.get(
+            f"{self.config.api_root}member/{self.user.email}?list_id=list2",
+            status_code=400,
+        )
 
         response = self.app.get(self.profile_url, user=self.user)
 

src/open_inwoner/laposta/admin.py

@@ -6,7 +6,7 @@
 
 from .choices import get_list_choices
 from .client import create_laposta_client
-from .models import LapostaConfig, Subscription
+from .models import LapostaConfig
 
 
 class LapostaConfigForm(forms.ModelForm):
@@ -49,10 +49,3 @@ class LapostaConfigAdmin(SingletonModelAdmin):
             {"fields": ("limit_list_selection_to",)},
         ),
     )
-
-
-@admin.register(Subscription)
-class SubscriptionAdmin(admin.ModelAdmin):
-    list_display = ["list_id", "user", "member_id"]
-    list_filter = ["list_id", "user"]
-    search_fields = ("user__first_name", "user__last_name", "user__email")

src/open_inwoner/laposta/client.py

@@ -1,6 +1,8 @@
 import logging
+from urllib.parse import quote
 
 from django.conf import settings
+from django.core.cache import cache
 
 from ape_pie.client import APIClient
 from requests.exceptions import RequestException
@@ -32,7 +34,11 @@ def get_lists(self) -> list[LapostaList]:
         return lists
 
     def create_subscription(self, list_id: str, user_data: UserData) -> Member | None:
-        response = self.post("member", data={"list_id": list_id, **user_data.dict()})
+        cache.delete(f"laposta_list_subscriptions:{user_data.email}")
+
+        response = self.post(
+            "member", data={"list_id": list_id, **user_data.model_dump()}
+        )
 
         if response.status_code == 400:
             data = response.json()
@@ -53,9 +59,10 @@ def create_subscription(self, list_id: str, user_data: UserData) -> Member | Non
 
         return Member(**data["member"])
 
-    def remove_subscription(self, list_id: str, member_id: str) -> Member | None:
-        response = self.delete(f"member/{member_id}", params={"list_id": list_id})
+    def remove_subscription(self, list_id: str, email: str) -> Member | None:
+        cache.delete(f"laposta_list_subscriptions:{email}")
 
+        response = self.delete(f"member/{quote(email)}", params={"list_id": list_id})
         if response.status_code == 400:
             data = response.json()
             error = data.get("error", {})
@@ -71,6 +78,17 @@ def remove_subscription(self, list_id: str, member_id: str) -> Member | None:
 
         return Member(**data["member"])
 
+    @cache_result(
+        "laposta_list_subscriptions:{email}", timeout=settings.CACHE_LAPOSTA_API_TIMEOUT
+    )
+    def get_subscriptions_for_email(self, list_ids: list[str], email: str) -> list[str]:
+        subscribed_to = []
+        for list_id in list_ids:
+            response = self.get(f"member/{quote(email)}", params={"list_id": list_id})
+            if response.status_code == 200:
+                subscribed_to.append(list_id)
+        return subscribed_to
+
 
 def create_laposta_client() -> LapostaClient | None:
     config = LapostaConfig.get_solo()

src/open_inwoner/laposta/forms.py

@@ -8,7 +8,7 @@
 from requests.exceptions import RequestException
 
 from open_inwoner.laposta.api_models import UserData
-from open_inwoner.laposta.models import LapostaConfig, Subscription
+from open_inwoner.laposta.models import LapostaConfig
 from open_inwoner.utils.api import ClientError
 
 from .choices import get_list_choices
@@ -37,10 +37,13 @@ def __init__(self, user=None, *args, **kwargs):
                     if choice[0] in limited_to
                 ]
 
-            self.fields["newsletters"].initial = [
-                subscription.list_id
-                for subscription in Subscription.objects.filter(user=user)
-            ]
+            # TODO get list_ids of lists the user is subscribed to
+            # self.existing_subscriptions = laposta_client.get_subscriptions_for_email(limited_to, user.email)
+            self.fields[
+                "newsletters"
+            ].initial = laposta_client.get_subscriptions_for_email(
+                limited_to, user.email
+            )
 
     def save(self, request, *args, **kwargs):
         newsletters = self.cleaned_data["newsletters"]
@@ -57,19 +60,16 @@ def save(self, request, *args, **kwargs):
             custom_fields=None,
             options=None,
         )
+        limited_to = LapostaConfig.get_solo().limit_list_selection_to
         existing_subscriptions = set(
-            Subscription.objects.filter(user=request.user).values_list(
-                "list_id", flat=True
-            )
+            client.get_subscriptions_for_email(limited_to, request.user.email)
         )
-
-        to_create = []
         for list_id in newsletters:
             if list_id in existing_subscriptions:
                 continue
 
             try:
-                member = client.create_subscription(list_id, user_data)
+                client.create_subscription(list_id, user_data)
             except (RequestException, ClientError):
                 logger.exception(
                     "Something went wrong while trying to create subscription"
@@ -83,28 +83,11 @@ def save(self, request, *args, **kwargs):
                         ).format(list_name=list_name_mapping[list_id])
                     ),
                 )
-            else:
-                if member:
-                    to_create.append(
-                        Subscription(
-                            list_id=member.list_id,
-                            member_id=member.member_id,
-                            user=request.user,
-                        )
-                    )
-
-        if to_create:
-            Subscription.objects.bulk_create(to_create)
 
         unsubscribe_from_ids = existing_subscriptions - set(newsletters)
-        unsubscribe_from = Subscription.objects.filter(
-            user=request.user, list_id__in=unsubscribe_from_ids
-        )
-
-        to_delete_ids = []
-        for subscription in unsubscribe_from:
+        for list_id in unsubscribe_from_ids:
             try:
-                client.remove_subscription(subscription.list_id, subscription.member_id)
+                client.remove_subscription(list_id, request.user.email)
             except (RequestException, ClientError):
                 logger.exception(
                     "Something went wrong while trying to delete subscription"
@@ -115,11 +98,6 @@ def save(self, request, *args, **kwargs):
                         _(
                             "Something went wrong while trying to unsubscribe "
                             "from '{list_name}', please try again later"
-                        ).format(list_name=list_name_mapping[subscription.list_id])
+                        ).format(list_name=list_name_mapping[list_id])
                     ),
                 )
-            else:
-                to_delete_ids.append(subscription.list_id)
-        Subscription.objects.filter(
-            user=request.user, list_id__in=to_delete_ids
-        ).delete()

src/open_inwoner/laposta/migrations/0004_delete_subscription.py

@@ -0,0 +1,16 @@
+# Generated by Django 4.2.11 on 2024-04-05 09:30
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("laposta", "0003_lapostaconfig_limit_list_selection_to"),
+    ]
+
+    operations = [
+        migrations.DeleteModel(
+            name="Subscription",
+        ),
+    ]

src/open_inwoner/laposta/models.py

@@ -51,22 +51,3 @@ def get_client_session_kwargs(self) -> dict[str, HTTPBasicAuth]:
         return {
             "auth": HTTPBasicAuth(self.basic_auth_username, self.basic_auth_password)
         }
-
-
-class Subscription(models.Model):
-    list_id = models.CharField(
-        verbose_name=_("List ID"),
-        help_text=_("Unique ID of the list in the API"),
-        max_length=50,
-    )
-    member_id = models.CharField(
-        verbose_name=_("Member ID"),
-        help_text=_("Unique ID of the member for this list in the API"),
-        max_length=50,
-    )
-    user = models.ForeignKey(
-        "accounts.User", verbose_name=_("User"), on_delete=models.CASCADE
-    )
-
-    class Meta:
-        unique_together = [["list_id", "member_id"]]

src/open_inwoner/laposta/tests/factories.py

@@ -1,10 +1,6 @@
-import factory
 from polyfactory.factories.pydantic_factory import ModelFactory
 
-from open_inwoner.accounts.tests.factories import UserFactory
-
 from ..api_models import LapostaList, Member
-from ..models import Subscription
 
 
 class LapostaListFactory(ModelFactory[LapostaList]):
@@ -13,10 +9,3 @@ class LapostaListFactory(ModelFactory[LapostaList]):
 
 class MemberFactory(ModelFactory[Member]):
     __model__ = Member
-
-
-class SubscriptionFactory(factory.django.DjangoModelFactory):
-    class Meta:
-        model = Subscription
-
-    user = factory.SubFactory(UserFactory)