diff --git a/src/open_inwoner/accounts/tests/test_profile_views.py b/src/open_inwoner/accounts/tests/test_profile_views.py index 153a3260e1..369b423547 100644 --- a/src/open_inwoner/accounts/tests/test_profile_views.py +++ b/src/open_inwoner/accounts/tests/test_profile_views.py @@ -1081,18 +1081,22 @@ def test_render_form_if_newsletters_are_found(self, m): self.config.save() m.get( - f"{self.config.api_root}member/{self.user.email}?list_id=list1", + f"{self.config.api_root}member?list_id=list1", json={ - "member": MemberFactory.build( - list_id="list1", - member_id="1234567", - email=self.user.email, - custom_fields=None, - ).model_dump() + "data": [ + { + "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", + f"{self.config.api_root}member?list_id=list2", status_code=400, ) @@ -1116,18 +1120,22 @@ def test_render_form_limit_newsletters_to_admin_selection(self, m): self.config.save() m.get( - f"{self.config.api_root}member/{self.user.email}?list_id=list1", + f"{self.config.api_root}member?list_id=list1", json={ - "member": MemberFactory.build( - list_id="list1", - member_id="1234567", - email=self.user.email, - custom_fields=None, - ).model_dump() + "data": [ + { + "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", + f"{self.config.api_root}member?list_id=list2", status_code=400, ) diff --git a/src/open_inwoner/accounts/views/profile.py b/src/open_inwoner/accounts/views/profile.py index f626243529..7c7cc85481 100644 --- a/src/open_inwoner/accounts/views/profile.py +++ b/src/open_inwoner/accounts/views/profile.py @@ -329,7 +329,7 @@ def crumbs(self): def get_form_kwargs(self) -> dict[str, Any]: kwargs = super().get_form_kwargs() - kwargs["user"] = self.request.user + kwargs["request"] = self.request return kwargs def form_valid(self, form): diff --git a/src/open_inwoner/laposta/client.py b/src/open_inwoner/laposta/client.py index 050ea58f5d..43ef6a4ba6 100644 --- a/src/open_inwoner/laposta/client.py +++ b/src/open_inwoner/laposta/client.py @@ -34,6 +34,8 @@ def get_lists(self) -> list[LapostaList]: return lists def create_subscription(self, list_id: str, user_data: UserData) -> Member | None: + # Ensure the current subscriptions for this email address are fetched again after + # this API call cache.delete(f"laposta_list_subscriptions:{user_data.email}") response = self.post( @@ -59,10 +61,18 @@ def create_subscription(self, list_id: str, user_data: UserData) -> Member | Non return Member(**data["member"]) - def remove_subscription(self, list_id: str, email: str) -> Member | None: + def remove_subscription( + self, list_id: str, member_id: str, email: str + ) -> Member | None: + # Ensure the current subscriptions for this email address are fetched again after + # this API call cache.delete(f"laposta_list_subscriptions:{email}") - response = self.delete(f"member/{quote(email)}", params={"list_id": list_id}) + # The API cannot deal with urlencoded email addresses that contain a `+`, + # so we use the member_id instead + response = self.delete( + f"member/{quote(member_id)}", params={"list_id": list_id} + ) if response.status_code == 400: data = response.json() error = data.get("error", {}) @@ -81,12 +91,21 @@ def remove_subscription(self, list_id: str, email: str) -> Member | None: @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 = [] + def get_subscriptions_for_email( + self, list_ids: list[str], email: str + ) -> dict[str, str]: + # The API cannot deal with urlencoded email addresses that contain a `+`, + # so instead we fetch all members for a given list and check if there is a + # member with a matching email + subscribed_to = {} for list_id in list_ids: - response = self.get(f"member/{quote(email)}", params={"list_id": list_id}) + response = self.get("member", params={"list_id": list_id}) if response.status_code == 200: - subscribed_to.append(list_id) + data = get_json_response(response) + for member_data in data["data"]: + member = Member(**member_data["member"]) + if member.email == email: + subscribed_to[list_id] = member.member_id return subscribed_to diff --git a/src/open_inwoner/laposta/forms.py b/src/open_inwoner/laposta/forms.py index 7c4feddf8d..d6fe32ac71 100644 --- a/src/open_inwoner/laposta/forms.py +++ b/src/open_inwoner/laposta/forms.py @@ -24,7 +24,7 @@ class NewsletterSubscriptionForm(forms.Form): widget=forms.widgets.CheckboxSelectMultiple, ) - def __init__(self, user=None, *args, **kwargs): + def __init__(self, request=None, *args, **kwargs): super().__init__(**kwargs) if laposta_client := create_laposta_client(): @@ -37,13 +37,11 @@ def __init__(self, user=None, *args, **kwargs): if choice[0] in limited_to ] - # 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 + subscription_mapping = laposta_client.get_subscriptions_for_email( + limited_to, request.user.email ) + request.session["subscription_mapping"] = subscription_mapping + self.fields["newsletters"].initial = list(subscription_mapping.keys()) def save(self, request, *args, **kwargs): newsletters = self.cleaned_data["newsletters"] @@ -87,7 +85,8 @@ def save(self, request, *args, **kwargs): unsubscribe_from_ids = existing_subscriptions - set(newsletters) for list_id in unsubscribe_from_ids: try: - client.remove_subscription(list_id, request.user.email) + member_id = request.session.get("subscription_mapping", {}).get(list_id) + client.remove_subscription(list_id, member_id, request.user.email) except (RequestException, ClientError): logger.exception( "Something went wrong while trying to delete subscription" diff --git a/src/open_inwoner/laposta/tests/test_forms.py b/src/open_inwoner/laposta/tests/test_forms.py index 3d5dc695e6..31521d2383 100644 --- a/src/open_inwoner/laposta/tests/test_forms.py +++ b/src/open_inwoner/laposta/tests/test_forms.py @@ -6,7 +6,7 @@ import requests_mock from open_inwoner.accounts.tests.factories import DigidUserFactory -from open_inwoner.utils.test import ClearCachesMixin +from open_inwoner.utils.test import ClearCachesMixin, SessionMiddleware from ..forms import NewsletterSubscriptionForm from ..models import LapostaConfig @@ -24,6 +24,9 @@ def setUp(self): self.request = RequestFactory().get("/") self.request.user = self.user + middleware = SessionMiddleware() + middleware.process_request(self.request) + self.request.session.save() self.config = LapostaConfig.get_solo() self.config.api_root = LAPOSTA_API_ROOT @@ -61,38 +64,44 @@ def test_save_form(self, m): self.setUpMocks(m) m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=123", + f"{LAPOSTA_API_ROOT}member?list_id=123", json={ - "member": MemberFactory.build( - list_id="123", - member_id="1234567", - email=self.user.email, - custom_fields=None, - ).model_dump() + "data": [ + { + "member": MemberFactory.build( + list_id="123", + member_id="1234567", + email=self.user.email, + custom_fields=None, + ).model_dump() + } + ], }, ) m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=456", + f"{LAPOSTA_API_ROOT}member?list_id=456", json={ - "member": MemberFactory.build( - list_id="456", - member_id="8765433", - email=self.user.email, - custom_fields=None, - ).model_dump() + "data": [ + { + "member": MemberFactory.build( + list_id="456", + member_id="8765433", + email=self.user.email, + custom_fields=None, + ).model_dump() + } + ], }, ) - m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=789", status_code=400 - ) + m.get(f"{LAPOSTA_API_ROOT}member?list_id=789", status_code=400) - form = NewsletterSubscriptionForm(data={}, user=self.user) + form = NewsletterSubscriptionForm(data={}, request=self.request) # User already has a subscription for the first two newsletters self.assertEqual(form["newsletters"].initial, ["123", "456"]) form = NewsletterSubscriptionForm( - data={"newsletters": ["456", "789"]}, user=self.user + data={"newsletters": ["456", "789"]}, request=self.request ) self.assertTrue(form.is_valid()) @@ -108,9 +117,7 @@ def test_save_form(self, m): ).model_dump() }, ) - delete_matcher = m.delete( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=123" - ) + delete_matcher = m.delete(f"{LAPOSTA_API_ROOT}member/1234567?list_id=123") form.save(self.request) @@ -141,17 +148,13 @@ def test_save_form_create_duplicate_subscription(self, m): """ self.setUpMocks(m) - m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=123", status_code=400 - ) - m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=456", status_code=400 - ) - m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=789", status_code=400 - ) + m.get(f"{LAPOSTA_API_ROOT}member?list_id=123", status_code=400) + m.get(f"{LAPOSTA_API_ROOT}member?list_id=456", status_code=400) + m.get(f"{LAPOSTA_API_ROOT}member?list_id=789", status_code=400) - form = NewsletterSubscriptionForm(data={"newsletters": ["789"]}, user=self.user) + form = NewsletterSubscriptionForm( + data={"newsletters": ["789"]}, request=self.request + ) self.assertTrue(form.is_valid()) @@ -182,30 +185,32 @@ def test_save_form_delete_non_existent_subscription(self, m): """ self.setUpMocks(m) + m.get(f"{LAPOSTA_API_ROOT}member?list_id=123", status_code=400) + m.get(f"{LAPOSTA_API_ROOT}member?list_id=456", status_code=400) m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=123", status_code=400 - ) - m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=456", status_code=400 - ) - m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=789", + f"{LAPOSTA_API_ROOT}member?list_id=789", json={ - "member": MemberFactory.build( - list_id="789", - member_id="1234567", - email=self.user.email, - custom_fields=None, - ).model_dump() + "data": [ + { + "member": MemberFactory.build( + list_id="789", + member_id="1234567", + email=self.user.email, + custom_fields=None, + ).model_dump() + } + ], }, ) - form = NewsletterSubscriptionForm(data={"newsletters": []}, user=self.user) + form = NewsletterSubscriptionForm( + data={"newsletters": []}, request=self.request + ) self.assertTrue(form.is_valid()) delete_matcher = m.delete( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=789", + f"{LAPOSTA_API_ROOT}member/1234567?list_id=789", json={ "error": { "type": "invalid_input", @@ -227,25 +232,27 @@ def test_save_form_raises_errors(self, m): """ self.setUpMocks(m) + m.get(f"{LAPOSTA_API_ROOT}member?list_id=123", status_code=400) m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=123", status_code=400 - ) - m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=456", + f"{LAPOSTA_API_ROOT}member?list_id=456", json={ - "member": MemberFactory.build( - list_id="456", - member_id="1234567", - email=self.user.email, - custom_fields=None, - ).model_dump() + "data": [ + { + "member": MemberFactory.build( + list_id="456", + member_id="1234567", + email=self.user.email, + custom_fields=None, + ).model_dump() + } + ], }, ) - m.get( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=789", status_code=400 - ) + m.get(f"{LAPOSTA_API_ROOT}member?list_id=789", status_code=400) - form = NewsletterSubscriptionForm(data={"newsletters": ["789"]}, user=self.user) + form = NewsletterSubscriptionForm( + data={"newsletters": ["789"]}, request=self.request + ) self.assertTrue(form.is_valid()) @@ -260,7 +267,7 @@ def test_save_form_raises_errors(self, m): status_code=500, ) delete_matcher = m.delete( - f"{LAPOSTA_API_ROOT}member/{self.user.email}?list_id=456", + f"{LAPOSTA_API_ROOT}member/1234567?list_id=456", json={ "error": { "type": "internal",