From 057eb413650b9c28b4facc4ece806a22fce0f08c Mon Sep 17 00:00:00 2001
From: Bart van der Schoor <bart@maykinmedia.nl>
Date: Thu, 11 Jan 2024 10:51:19 +0100
Subject: [PATCH] [#1760] PR feedback: changed hooks import/export structure &
 patches, fixed type annotations

---
 src/open_inwoner/cms/cases/views/status.py    |  8 +++----
 src/open_inwoner/openzaak/notifications.py    |  7 +++---
 .../openzaak/tests/test_case_detail.py        |  4 ++--
 .../tests/test_notification_zaak_status.py    |  2 +-
 .../plans/management/commands/plans_expire.py |  5 ++--
 .../plans/tests/test_plans_expire.py          | 15 ++++++++----
 src/open_inwoner/plans/tests/test_views.py    |  2 +-
 src/open_inwoner/plans/views.py               |  5 ++--
 src/open_inwoner/userfeed/adapters.py         |  3 ++-
 src/open_inwoner/userfeed/apps.py             | 16 ++++---------
 src/open_inwoner/userfeed/feed.py             |  2 +-
 src/open_inwoner/userfeed/hooks/__init__.py   | 12 ++++++++++
 .../management/commands/add_feed_message.py   |  2 +-
 src/open_inwoner/userfeed/models.py           | 23 +++++++++++--------
 14 files changed, 57 insertions(+), 49 deletions(-)

diff --git a/src/open_inwoner/cms/cases/views/status.py b/src/open_inwoner/cms/cases/views/status.py
index 66e2acf748..f3965094fd 100644
--- a/src/open_inwoner/cms/cases/views/status.py
+++ b/src/open_inwoner/cms/cases/views/status.py
@@ -55,8 +55,7 @@
     ZaakTypeStatusTypeConfig,
 )
 from open_inwoner.openzaak.utils import get_role_name_display, is_info_object_visible
-from open_inwoner.userfeed.hooks.case_document import case_documents_seen
-from open_inwoner.userfeed.hooks.case_status import case_status_seen
+from open_inwoner.userfeed import hooks
 from open_inwoner.utils.time import has_new_elements
 from open_inwoner.utils.translate import TranslationLookup
 from open_inwoner.utils.views import CommonPageMixin, LogMixin
@@ -220,9 +219,8 @@ def get_context_data(self, **kwargs):
                 self.case, self.resulttype_config_mapping
             )
 
-            # flag case seen in user feed
-            case_status_seen(self.request.user, self.case)
-            case_documents_seen(self.request.user, self.case)
+            hooks.case_status_seen(self.request.user, self.case)
+            hooks.case_documents_seen(self.request.user, self.case)
 
             context["case"] = {
                 "id": str(self.case.uuid),
diff --git a/src/open_inwoner/openzaak/notifications.py b/src/open_inwoner/openzaak/notifications.py
index 39b67bb744..d3f6b3ebe5 100644
--- a/src/open_inwoner/openzaak/notifications.py
+++ b/src/open_inwoner/openzaak/notifications.py
@@ -41,11 +41,10 @@
     is_info_object_visible,
     is_zaak_visible,
 )
+from open_inwoner.userfeed import hooks
 from open_inwoner.utils.logentry import system_action as log_system_action
 from open_inwoner.utils.url import build_absolute_url
 
-from ..userfeed.hooks.case_document import case_document_added_notification_received
-from ..userfeed.hooks.case_status import case_status_notification_received
 from .models import ZaakTypeStatusTypeConfig
 
 logger = logging.getLogger(__name__)
@@ -218,7 +217,7 @@ def handle_zaakinformatieobject_update(
     user: User, case: Zaak, zaak_info_object: ZaakInformatieObject
 ):
     # hook into userfeed
-    case_document_added_notification_received(user, case, zaak_info_object)
+    hooks.case_document_added_notification_received(user, case, zaak_info_object)
 
     note = UserCaseInfoObjectNotification.objects.record_if_unique_notification(
         user,
@@ -350,7 +349,7 @@ def _handle_status_notification(notification: Notification, case: Zaak, inform_u
 
 def handle_status_update(user: User, case: Zaak, status: Status):
     # hook into userfeed
-    case_status_notification_received(user, case, status)
+    hooks.case_status_notification_received(user, case, status)
 
     # email notification
     note = UserCaseStatusNotification.objects.record_if_unique_notification(
diff --git a/src/open_inwoner/openzaak/tests/test_case_detail.py b/src/open_inwoner/openzaak/tests/test_case_detail.py
index 336052c381..ae056c73df 100644
--- a/src/open_inwoner/openzaak/tests/test_case_detail.py
+++ b/src/open_inwoner/openzaak/tests/test_case_detail.py
@@ -585,8 +585,8 @@ def _setUpMocks(self, m, use_eindstatus=True):
             ),
         )
 
-    @patch("open_inwoner.cms.cases.views.status.case_status_seen")
-    @patch("open_inwoner.cms.cases.views.status.case_documents_seen")
+    @patch("open_inwoner.userfeed.hooks.case_status_seen")
+    @patch("open_inwoner.userfeed.hooks.case_documents_seen")
     def test_status_is_retrieved_when_user_logged_in_via_digid(
         self, m, mock_hook_status: Mock, mock_hook_documents: Mock
     ):
diff --git a/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py b/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py
index 12e45a031a..89d2d93cd6 100644
--- a/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py
+++ b/src/open_inwoner/openzaak/tests/test_notification_zaak_status.py
@@ -473,7 +473,7 @@ def test_status_bails_when_skip_informeren_is_set_and_zaaktypeconfig_is_not_foun
 @override_settings(ZGW_LIMIT_NOTIFICATIONS_FREQUENCY=3600)
 @freeze_time("2023-01-01 01:00:00")
 class NotificationHandlerUserMessageTestCase(AssertTimelineLogMixin, TestCase):
-    @patch("open_inwoner.openzaak.notifications.case_status_notification_received")
+    @patch("open_inwoner.userfeed.hooks.case_status_notification_received")
     @patch("open_inwoner.openzaak.notifications.send_case_update_email")
     def test_handle_status_update(self, mock_send: Mock, mock_feed_hook: Mock):
         """
diff --git a/src/open_inwoner/plans/management/commands/plans_expire.py b/src/open_inwoner/plans/management/commands/plans_expire.py
index 1b2eb62eea..b268a897cb 100644
--- a/src/open_inwoner/plans/management/commands/plans_expire.py
+++ b/src/open_inwoner/plans/management/commands/plans_expire.py
@@ -10,7 +10,7 @@
 
 from open_inwoner.accounts.models import User
 from open_inwoner.plans.models import Plan
-from open_inwoner.userfeed.hooks.plan import plan_expiring
+from open_inwoner.userfeed import hooks
 from open_inwoner.utils.url import build_absolute_url
 
 logger = logging.getLogger(__name__)
@@ -49,9 +49,8 @@ def handle(self, *args, **options):
                 f"The email was sent to the user {receiver} about {plans.count()} expiring plans"
             )
 
-            # hook into userfeed
             for p in plans:
-                plan_expiring(receiver, p)
+                hooks.plan_expiring(receiver, p)
 
     def send_email(self, receiver: User, plans: List[Plan]):
         plan_list_link = build_absolute_url(reverse("collaborate:plan_list"))
diff --git a/src/open_inwoner/plans/tests/test_plans_expire.py b/src/open_inwoner/plans/tests/test_plans_expire.py
index ea5ef9cd00..4ee02cd9e2 100644
--- a/src/open_inwoner/plans/tests/test_plans_expire.py
+++ b/src/open_inwoner/plans/tests/test_plans_expire.py
@@ -32,6 +32,15 @@ def test_notify_about_expiring_plan(self):
         self.assertIn(plan.goal, html_body)
         self.assertIn(reverse("collaborate:plan_list"), html_body)
 
+    @patch("open_inwoner.userfeed.hooks.plan_expiring")
+    def test_notify_about_expiring_plan_userfeed_hook(self, mock_plan_expiring: Mock):
+        user = UserFactory()
+        plan = PlanFactory(end_date=date.today(), created_by=user)
+
+        call_command("plans_expire")
+
+        mock_plan_expiring.assert_called_once()
+
     def test_no_notification_about_expiring_plan_when_disabled(self):
         user = UserFactory(plans_notifications=False)
         plan = PlanFactory(end_date=date.today(), created_by=user)
@@ -56,8 +65,7 @@ def test_notify_about_expiring_plan_inactive_user(self):
         call_command("plans_expire")
         self.assertEqual(len(mail.outbox), 0)
 
-    @patch("open_inwoner.plans.management.commands.plans_expire.plan_expiring")
-    def test_notify_about_expiring_plan(self, mock_plan_expiring: Mock):
+    def test_notify_about_expiring_plan_email_contact(self):
         user = UserFactory()
         contact = UserFactory()
         plan = PlanFactory(end_date=date.today(), created_by=user)
@@ -89,9 +97,6 @@ def test_notify_about_expiring_plan(self, mock_plan_expiring: Mock):
         self.assertIn(plan.goal, html_body2)
         self.assertIn(reverse("collaborate:plan_list"), html_body2)
 
-        # check userfeed hook was called
-        self.assertEqual(mock_plan_expiring.call_count, 2)
-
     def test_notify_only_user_with_active_notifications(self):
         user = UserFactory()
         contact = UserFactory(plans_notifications=False)
diff --git a/src/open_inwoner/plans/tests/test_views.py b/src/open_inwoner/plans/tests/test_views.py
index edf316232f..bb05b08f95 100644
--- a/src/open_inwoner/plans/tests/test_views.py
+++ b/src/open_inwoner/plans/tests/test_views.py
@@ -148,7 +148,7 @@ def test_plan_detail_contacts(self):
         self.assertContains(response, self.contact.get_full_name())
         self.assertNotContains(response, new_contact.get_full_name())
 
-    @patch("open_inwoner.plans.views.plan_completed")
+    @patch("open_inwoner.userfeed.hooks.plan_completed")
     def test_plan_detail_userfeed_hook(self, mock_plan_completed: Mock):
         self.plan.end_date = date.today()
         self.plan.save()
diff --git a/src/open_inwoner/plans/views.py b/src/open_inwoner/plans/views.py
index 45da0f8f38..7751efa45e 100644
--- a/src/open_inwoner/plans/views.py
+++ b/src/open_inwoner/plans/views.py
@@ -24,11 +24,11 @@
     ActionUpdateView,
     BaseActionFilter,
 )
+from open_inwoner.userfeed import hooks
 from open_inwoner.utils.logentry import get_change_message
 from open_inwoner.utils.mixins import ExportMixin
 from open_inwoner.utils.views import CommonPageMixin, LogMixin
 
-from ..userfeed.hooks.plan import plan_completed
 from .forms import PlanForm, PlanGoalForm, PlanListFilterForm
 from .models import Plan
 
@@ -225,9 +225,8 @@ def get_context_data(self, **kwargs):
         )
         context["actions"] = self.get_actions(actions)
 
-        # hook into userfeed
         if obj.end_date < date.today():
-            plan_completed(self.request.user, obj)
+            hooks.plan_completed(self.request.user, obj)
 
         return context
 
diff --git a/src/open_inwoner/userfeed/adapters.py b/src/open_inwoner/userfeed/adapters.py
index 22acb20608..8b7166b754 100644
--- a/src/open_inwoner/userfeed/adapters.py
+++ b/src/open_inwoner/userfeed/adapters.py
@@ -1,4 +1,5 @@
 from open_inwoner.userfeed.adapter import FeedItem
+from open_inwoner.userfeed.choices import FeedItemType
 
 
 def get_item_adapter_class(type: str) -> type[FeedItem]:
@@ -9,7 +10,7 @@ def get_item_adapter_class(type: str) -> type[FeedItem]:
 feed_adapter_map = dict()
 
 
-def register_item_adapter(adapter_class: type[FeedItem], feed_type: str):
+def register_item_adapter(adapter_class: type[FeedItem], feed_type: FeedItemType):
     # NOTE this function could be upgraded to work as class decorator
 
     if feed_type in feed_adapter_map and adapter_class != feed_adapter_map[feed_type]:
diff --git a/src/open_inwoner/userfeed/apps.py b/src/open_inwoner/userfeed/apps.py
index 384cf25942..e68053282f 100644
--- a/src/open_inwoner/userfeed/apps.py
+++ b/src/open_inwoner/userfeed/apps.py
@@ -11,19 +11,11 @@ class UserFeedConfig(AppConfig):
     verbose_name = "User Feed"
 
     def ready(self):
-        auto_import_hooks()
+        auto_import_adapters()
 
 
-def auto_import_hooks():
+def auto_import_adapters():
     """
-    import files from hooks directory
-
-    this expects things calling their register_xyz() function at module level
+    import files from hooks directory to register adapters
     """
-
-    hooks_dir = Path(__file__).parent / "hooks"
-    for f in os.listdir(hooks_dir):
-        name, ext = os.path.splitext(f)
-        if ext == ".py" and name != "__init__":
-            path = f"open_inwoner.userfeed.hooks.{name}"
-            import_module(path)
+    import open_inwoner.userfeed.hooks  # noqa
diff --git a/src/open_inwoner/userfeed/feed.py b/src/open_inwoner/userfeed/feed.py
index 4570adb154..b84d2ace87 100644
--- a/src/open_inwoner/userfeed/feed.py
+++ b/src/open_inwoner/userfeed/feed.py
@@ -24,7 +24,7 @@ class Feed:
     items: list[FeedItem] = dataclasses.field(default_factory=list)
     summary: list[str] = dataclasses.field(default_factory=list)
 
-    def has_display(self) -> int:
+    def has_display(self) -> bool:
         return self.total_items > 0
 
     def action_required(self) -> int:
diff --git a/src/open_inwoner/userfeed/hooks/__init__.py b/src/open_inwoner/userfeed/hooks/__init__.py
index e69de29bb2..5ffb0d9fb4 100644
--- a/src/open_inwoner/userfeed/hooks/__init__.py
+++ b/src/open_inwoner/userfeed/hooks/__init__.py
@@ -0,0 +1,12 @@
+from .case_document import (
+    CaseDocumentAddedFeedItem,
+    case_document_added_notification_received,
+    case_documents_seen,
+)
+from .case_status import (
+    CaseStatusUpdateFeedItem,
+    case_status_notification_received,
+    case_status_seen,
+)
+from .common import simple_message
+from .plan import PlanExpiresFeedItem, plan_completed, plan_expiring
diff --git a/src/open_inwoner/userfeed/management/commands/add_feed_message.py b/src/open_inwoner/userfeed/management/commands/add_feed_message.py
index 41b3f10218..b0ca04edb1 100644
--- a/src/open_inwoner/userfeed/management/commands/add_feed_message.py
+++ b/src/open_inwoner/userfeed/management/commands/add_feed_message.py
@@ -33,7 +33,7 @@ def handle(self, *args, **options):
         try:
             user = User.objects.get(pk=options["user"])
         except User.DoesNotExist:
-            self.stdout.write("user_id not found, use one off:")
+            self.stdout.write("user_id not found, use one of:")
             for user in User.objects.filter(is_active=True, is_staff=True).order_by(
                 "id"
             ):
diff --git a/src/open_inwoner/userfeed/models.py b/src/open_inwoner/userfeed/models.py
index f4d7bf1df5..74af825711 100644
--- a/src/open_inwoner/userfeed/models.py
+++ b/src/open_inwoner/userfeed/models.py
@@ -1,4 +1,5 @@
-from typing import Optional
+from typing import Optional, Union
+from uuid import UUID
 
 from django.contrib.contenttypes.fields import GenericForeignKey
 from django.contrib.contenttypes.models import ContentType
@@ -17,10 +18,10 @@ class FeedItemManager(models.Manager):
     def mark_uuid_completed(
         self,
         user: User,
-        type: str,
-        ref_uuid,
+        type: FeedItemType,
+        ref_uuid: Union[str, UUID],
         ref_key: Optional[str] = None,
-        force=False,
+        force: bool = False,
     ):
         qs = self.filter(
             user=user,
@@ -34,10 +35,10 @@ def mark_uuid_completed(
     def mark_object_completed(
         self,
         user: User,
-        type: str,
-        ref_object,
+        type: FeedItemType,
+        ref_object: models.Model,
         ref_key: Optional[str] = None,
-        force=False,
+        force: bool = False,
     ):
         qs = self.filter(
             user=user,
@@ -61,7 +62,9 @@ def mark_completed(self, force: bool = False):
             completed_at=timezone.now(),
         )
 
-    def filter_ref_object(self, ref_object):
+    def filter_ref_object(
+        self, ref_object: models.Model
+    ) -> models.QuerySet["FeedItemData"]:
         return self.filter(
             ref_object_id=ref_object.id,
             ref_object_type=ContentType.objects.get_for_model(ref_object),
@@ -84,7 +87,7 @@ class FeedItemData(models.Model):
     )
 
     @property
-    def is_completed(self):
+    def is_completed(self) -> bool:
         return self.completed_at is not None
 
     auto_expire_at = models.DateTimeField(
@@ -124,7 +127,7 @@ def is_completed(self):
     ref_object_id = models.PositiveIntegerField(blank=True, null=True)
 
     @cached_property
-    def ref_object(self):
+    def ref_object(self) -> Optional[models.Model]:
         # don't raise but return None
         if self.ref_object_type_id and self.ref_object_id:
             return self.ref_object_field