👌 [#2112] PR feedback

src/open_inwoner/apimock/apis/esuite-read/formulieren-api/openstaande-taken.json

@@ -15,4 +15,4 @@
         "startdatum": "2023-10-11",
         "formulierLink": "https://maykinmedia.nl"
     }
-]
+]

src/open_inwoner/openzaak/tests/mocks.py

@@ -1,7 +1,7 @@
 from open_inwoner.openzaak.tests.shared import FORMS_ROOT
 
 
-class ESuiteData:
+class ESuiteSubmissionData:
     def __init__(self):
         self.submission_1 = {
             "url": "https://dmidoffice2.esuite-development.net/formulieren-provider/api/v1/8e3ae29c-7bc5-4f7d-a27c-b0c83c13328e",
@@ -28,6 +28,16 @@ def __init__(self):
             ],
         }
 
+    def install_mocks(self, m):
+        m.get(
+            f"{FORMS_ROOT}openstaande-inzendingen",
+            json=self.response,
+        )
+        return self
+
+
+class ESuiteTaskData:
+    def __init__(self):
         self.task1 = {
             "url": "https://maykinmedia.nl",
             "uuid": "fb72d8db-c3ee-4aa0-96c1-260b202cb208",
@@ -46,10 +56,6 @@ def __init__(self):
         }
 
     def install_mocks(self, m):
-        m.get(
-            f"{FORMS_ROOT}openstaande-inzendingen",
-            json=self.response,
-        )
         m.get(
             f"{FORMS_ROOT}openstaande-taken",
             json=[self.task1, self.task2],

src/open_inwoner/openzaak/tests/test_cases.py

@@ -37,7 +37,7 @@
     ZaakTypeStatusTypeConfigFactory,
 )
 from .helpers import generate_oas_component_cached
-from .mocks import ESuiteData
+from .mocks import ESuiteSubmissionData
 from .shared import CATALOGI_ROOT, ZAKEN_ROOT
 
 # Avoid redirects through `KvKLoginMiddleware`
@@ -868,7 +868,7 @@ def test_case_submission(self, m):
             login_type=LoginTypeChoices.digid, bsn="900222086", email="[email protected]"
         )
 
-        data = ESuiteData().install_mocks(m)
+        data = ESuiteSubmissionData().install_mocks(m)
 
         response = self.app.get(
             self.inner_url, user=user, headers={"HX-Request": "true"}

src/open_inwoner/userfeed/apps.py

@@ -9,8 +9,6 @@ class UserFeedConfig(AppConfig):
     def ready(self):
         auto_import_adapters()
 
-        from .hooks.external_task import fetch_open_tasks  # noqa
-
 
 def auto_import_adapters():
     """

src/open_inwoner/userfeed/feed.py

@@ -13,6 +13,7 @@
     get_item_adapter_class,
     get_types_for_unpublished_cms_apps,
 )
+from open_inwoner.userfeed.hooks.external_task import update_user_tasks
 from open_inwoner.userfeed.models import FeedItemData
 from open_inwoner.userfeed.summarize import SUMMARIES
 
@@ -48,6 +49,8 @@ def get_feed(user: User, with_history: bool = False) -> Feed:
         # empty feed
         return Feed()
 
+    update_user_tasks(user)
+
     # core filters
     display_filter = Q(completed_at__isnull=True)
     if with_history:

src/open_inwoner/userfeed/hooks/external_task.py

@@ -1,8 +1,5 @@
-import dataclasses
 from typing import List
 
-from django.contrib.auth.signals import user_logged_in
-from django.dispatch import receiver
 from django.utils.translation import gettext_lazy as _
 
 from open_inwoner.accounts.choices import LoginTypeChoices
@@ -21,34 +18,55 @@ class OpenTaskFeedItem(FeedItem):
     base_message = _("Open task that is yet to be completed")
 
     @property
-    def message(self) -> str:
-        return self.get_data("naam", super().title)
+    def title(self) -> str:
+        return f"{self.base_title} ({self.get_data('task_identificatie')})"
 
     @property
-    def action_url(self) -> str:
-        return self.get_data("formulier_link")
-
-
-def create_external_task_items(user: User, openstaande_taken: List[OpenTask]):
-    existing_uuids = FeedItemData.objects.filter(
-        type=FeedItemType.external_task,
-        user=user,
-    ).values_list("ref_uuid", flat=True)
-    existing_uuids = set(str(uuid) for uuid in existing_uuids)
-
+    def message(self) -> str:
+        return self.get_data("task_name", super().message)
+
+
+def update_external_task_items(user: User, openstaande_taken: List[OpenTask]):
+    """
+    Creates items for OpenTasks if they do not exist yet, updates existing items if the
+    data changed and marks existing items as complete if no OpenTask exists for that
+    uuid anymore
+    """
+    existing_uuid_mapping = {
+        str(item.ref_uuid): item
+        for item in FeedItemData.objects.filter(
+            type=FeedItemType.external_task,
+            user=user,
+        )
+    }
+    existing_uuids = set(existing_uuid_mapping.keys())
+
+    update_data = []
     create_data = []
     for task in openstaande_taken:
-        if task.uuid in existing_uuids:
+        type_data = {
+            "action_url": task.formulier_link,
+            "task_name": task.naam,
+            "task_identificatie": task.identificatie,
+        }
+        if existing_item := existing_uuid_mapping.get(task.uuid):
+            if existing_item.type_data != type_data:
+                existing_item.type_data = type_data
+                update_data.append(existing_item)
             continue
 
-        data = {
-            "user": user,
-            "type": FeedItemType.external_task,
-            "ref_uuid": task.uuid,
-            "action_required": True,
-            "type_data": dataclasses.asdict(task),
-        }
-        create_data.append(FeedItemData(**data))
+        create_data.append(
+            FeedItemData(
+                user=user,
+                type=FeedItemType.external_task,
+                ref_uuid=task.uuid,
+                action_required=True,
+                type_data=type_data,
+            )
+        )
+
+    # TODO we could maybe use `bulk_create` once we upgraded to Django 4.x
+    FeedItemData.objects.bulk_update(update_data, ["type_data"], batch_size=100)
     FeedItemData.objects.bulk_create(create_data)
 
     # Mark all tasks with UUIDs not occurring in the fetched results as completed
@@ -58,12 +76,11 @@ def create_external_task_items(user: User, openstaande_taken: List[OpenTask]):
     ).mark_completed()
 
 
-@receiver(user_logged_in)
-def fetch_open_tasks(sender, user, request, *args, **kwargs):
+def update_user_tasks(user: User):
     if user.login_type == LoginTypeChoices.digid:
         if client := build_client("form"):
             tasks = client.fetch_open_tasks(user.bsn)
-            create_external_task_items(user, tasks)
+            update_external_task_items(user, tasks)
 
 
 register_item_adapter(OpenTaskFeedItem, FeedItemType.external_task)

src/open_inwoner/userfeed/tests/test_external_tasks.py

@@ -1,15 +1,19 @@
-from django.contrib.auth import user_logged_in
-from django.test import RequestFactory, TestCase
+from unittest.mock import call, patch
+
+from django.test import TestCase
+from django.utils.translation import gettext as _
 
 import requests_mock
 from zgw_consumers.test.factories import ServiceFactory
 
-from open_inwoner.accounts.models import User
 from open_inwoner.accounts.tests.factories import DigidUserFactory
+from open_inwoner.cms.plugins.cms_plugins import UserFeedPlugin
+from open_inwoner.cms.tests import cms_tools
 from open_inwoner.openzaak.models import OpenZaakConfig
-from open_inwoner.openzaak.tests.mocks import ESuiteData
+from open_inwoner.openzaak.tests.mocks import ESuiteTaskData
 from open_inwoner.openzaak.tests.shared import FORMS_ROOT
 from open_inwoner.userfeed.choices import FeedItemType
+from open_inwoner.userfeed.hooks.external_task import update_user_tasks
 from open_inwoner.userfeed.models import FeedItemData
 from open_inwoner.userfeed.tests.factories import FeedItemDataFactory
 
@@ -24,34 +28,51 @@ def setUp(self):
         self.config.form_service = ServiceFactory(api_root=FORMS_ROOT)
         self.config.save()
 
-    @requests_mock.Mocker()
-    def test_fetch_tasks_after_login(self, m):
-        request = RequestFactory().get("/dummy")
-        request.user = self.user
+    def test_userfeed_plugin_render_triggers_update_open_tasks(self):
+        FeedItemDataFactory.create(
+            type=FeedItemType.external_task,
+            user=self.user,
+            ref_uuid="f3100eea-bef4-44bb-b55b-8715d23fa77f",
+            type_data={
+                "task_name": "Aanvullende informatie gewenst",
+                "task_identificatie": "4321-2023",
+                "action_url": "https://maykinmedia.nl",
+            },
+        )
 
-        ESuiteData().install_mocks(m)
+        with patch("open_inwoner.userfeed.feed.update_user_tasks") as mock:
+            html, context = cms_tools.render_plugin(
+                UserFeedPlugin, plugin_data={}, user=self.user
+            )
 
-        user_logged_in.send(User, user=self.user, request=request)
+            # `cms_tools.render_plugin` renders twice
+            mock.assert_has_calls([call(self.user), call(self.user)])
 
-        qs_after_first_login = FeedItemData.objects.all()
+            self.assertIn(f"{_('Open task')} (4321-2023)", html)
+            self.assertIn("Aanvullende informatie gewenst", html)
+
+    @requests_mock.Mocker()
+    def test_update_user_tasks_create(self, m):
+        ESuiteTaskData().install_mocks(m)
 
         with self.subTest("feed item data created on initial login"):
-            self.assertEqual(qs_after_first_login.count(), 2)
+            update_user_tasks(self.user)
 
-            item1, item2 = qs_after_first_login
+            items_after_first_login = list(FeedItemData.objects.all())
+
+            self.assertEqual(len(items_after_first_login), 2)
+
+            item1, item2 = items_after_first_login
 
             self.assertEqual(item1.user, self.user)
             self.assertEqual(item1.action_required, True)
             self.assertEqual(item1.type, FeedItemType.external_task)
             self.assertEqual(
                 item1.type_data,
                 {
-                    "url": "https://maykinmedia.nl",
-                    "naam": "Aanvullende informatie gewenst",
-                    "uuid": "fb72d8db-c3ee-4aa0-96c1-260b202cb208",
-                    "startdatum": "2023-11-14",
-                    "identificatie": "1234-2023",
-                    "formulier_link": "https://maykinmedia.nl",
+                    "task_name": "Aanvullende informatie gewenst",
+                    "task_identificatie": "1234-2023",
+                    "action_url": "https://maykinmedia.nl",
                 },
             )
             self.assertEqual(
@@ -64,47 +85,38 @@ def test_fetch_tasks_after_login(self, m):
             self.assertEqual(
                 item2.type_data,
                 {
-                    "url": "https://maykinmedia.nl",
-                    "naam": "Aanvullende informatie gewenst",
-                    "uuid": "d74f6a5c-297d-43a3-a923-1774164d852d",
-                    "startdatum": "2023-10-11",
-                    "identificatie": "4321-2023",
-                    "formulier_link": "https://maykinmedia.nl",
+                    "task_name": "Aanvullende informatie gewenst",
+                    "task_identificatie": "4321-2023",
+                    "action_url": "https://maykinmedia.nl",
                 },
             )
             self.assertEqual(
                 str(item2.ref_uuid), "d74f6a5c-297d-43a3-a923-1774164d852d"
             )
 
         with self.subTest("import is idempotent"):
-            user_logged_in.send(User, user=self.user, request=request)
+            update_user_tasks(self.user)
 
             qs_after_second_login = FeedItemData.objects.all()
 
-            self.assertEqual(set(qs_after_first_login), set(qs_after_second_login))
+            self.assertEqual(set(items_after_first_login), set(qs_after_second_login))
 
     @requests_mock.Mocker()
-    def test_complete_tasks_after_login(self, m):
-        request = RequestFactory().get("/dummy")
-        request.user = self.user
-
-        ESuiteData().install_mocks(m)
+    def test_update_user_tasks_complete_items(self, m):
+        ESuiteTaskData().install_mocks(m)
 
         old_feed_item = FeedItemDataFactory.create(
             type=FeedItemType.external_task,
             user=self.user,
             ref_uuid="f3100eea-bef4-44bb-b55b-8715d23fa77f",
             type_data={
-                "url": "https://maykinmedia.nl",
-                "naam": "Aanvullende informatie gewenst",
-                "uuid": "d74f6a5c-297d-43a3-a923-1774164d852d",
-                "startdatum": "2023-10-11",
-                "identificatie": "4321-2023",
-                "formulier_link": "https://maykinmedia.nl",
+                "task_name": "Aanvullende informatie gewenst",
+                "task_identificatie": "4321-2023",
+                "action_url": "https://maykinmedia.nl",
             },
         )
 
-        user_logged_in.send(User, user=self.user, request=request)
+        update_user_tasks(self.user)
 
         qs_after_first_login = FeedItemData.objects.all()
 
@@ -113,3 +125,35 @@ def test_complete_tasks_after_login(self, m):
         old_feed_item.refresh_from_db()
 
         self.assertTrue(old_feed_item.is_completed)
+
+    @requests_mock.Mocker()
+    def test_update_user_tasks_update_type_data(self, m):
+        ESuiteTaskData().install_mocks(m)
+
+        outdated_feed_item = FeedItemDataFactory.create(
+            type=FeedItemType.external_task,
+            user=self.user,
+            ref_uuid="fb72d8db-c3ee-4aa0-96c1-260b202cb208",
+            type_data={
+                "task_name": "outdated_title",
+                "task_identificatie": "outdated_id",
+                "action_url": "https://outdated.url",
+            },
+        )
+
+        update_user_tasks(self.user)
+
+        qs_after_first_login = FeedItemData.objects.all()
+
+        self.assertEqual(qs_after_first_login.count(), 2)
+
+        outdated_feed_item.refresh_from_db()
+
+        self.assertEqual(
+            outdated_feed_item.type_data,
+            {
+                "task_name": "Aanvullende informatie gewenst",
+                "task_identificatie": "1234-2023",
+                "action_url": "https://maykinmedia.nl",
+            },
+        )