[#939] Fix tests

    * refactor logging, move logic from DocumentDeleteView.delete() to
      form_valid()
    * normalize data structure and correct type annotation in `InboxView`
      (confusion between queryset and list)
    * refactor logging in the context of DigiD user change/creation
      since db logging caused `ValueError: save() prohibited to prevent
      data loss due to unsaved related object 'content_object'`

requirements/base.in

@@ -37,7 +37,8 @@ django-view-breadcrumbs
 markdown
 django_better_admin_arrayfield
 humanfriendly
-git+https://github.com/maykinmedia/mail-editor.git@36ea073847bbea4aef3c97723f988927876b7ec9#egg=mail-editor
+# mail-editor
+git+https://github.com/maykinmedia/mail-editor.git@016ccba3f0b5c2021d0886f35fcb5ef3788d889d#egg=mail-editor
 fontawesomefree
 django-timeline-logger
 django-csp

requirements/base.txt

@@ -356,7 +356,7 @@ lxml==4.9.1
     #   maykin-python3-saml
     #   svglib
     #   xmlsec
-mail-editor @ git+https://github.com/maykinmedia/mail-editor.git@36ea073847bbea4aef3c97723f988927876b7ec9
+mail-editor @ git+https://github.com/maykinmedia/mail-editor.git@016ccba3f0b5c2021d0886f35fcb5ef3788d889d
     # via -r requirements/base.in
 mail-parser==3.15.0
     # via django-yubin

requirements/ci.txt

@@ -620,7 +620,7 @@ lxml==4.9.1
     #   pyquery
     #   svglib
     #   xmlsec
-mail-editor @ git+https://github.com/maykinmedia/mail-editor.git@36ea073847bbea4aef3c97723f988927876b7ec9
+mail-editor @ git+https://github.com/maykinmedia/mail-editor.git@016ccba3f0b5c2021d0886f35fcb5ef3788d889d
     # via
     #   -c requirements/base.txt
     #   -r requirements/base.txt

requirements/dev.txt

@@ -715,7 +715,7 @@ lxml==4.9.1
     #   pyquery
     #   svglib
     #   xmlsec
-mail-editor @ git+https://github.com/maykinmedia/mail-editor.git@36ea073847bbea4aef3c97723f988927876b7ec9
+mail-editor @ git+https://github.com/maykinmedia/mail-editor.git@016ccba3f0b5c2021d0886f35fcb5ef3788d889d
     # via
     #   -c requirements/ci.txt
     #   -r requirements/ci.txt

src/open_inwoner/accounts/migrations/0073_alter_user_user_contacts.py

@@ -0,0 +1,24 @@
+# Generated by Django 4.2.10 on 2024-02-13 13:58
+
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("accounts", "0072_merge_20240129_1610"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="user",
+            name="user_contacts",
+            field=models.ManyToManyField(
+                blank=True,
+                help_text="The contacts of the specific user",
+                to=settings.AUTH_USER_MODEL,
+                verbose_name="Contacts",
+            ),
+        ),
+    ]

src/open_inwoner/accounts/tests/test_admin.py

@@ -1,5 +1,5 @@
 from django.urls import reverse
-from django.utils.translation import ugettext as _
+from django.utils.translation import gettext as _
 
 from django_webtest import WebTest
 from maykin_2fa.test import disable_admin_mfa

src/open_inwoner/accounts/views/documents.py

@@ -1,5 +1,4 @@
 from django.contrib.auth.mixins import LoginRequiredMixin
-from django.http.response import HttpResponseRedirect
 from django.urls.base import reverse_lazy
 from django.utils.translation import gettext as _
 from django.views.generic import DeleteView
@@ -20,9 +19,6 @@ def get_queryset(self):
         base_qs = super().get_queryset()
         return base_qs.filter(owner=self.request.user)
 
-    def delete(self, request, *args, **kwargs):
-        object = self.get_object()
-        super().delete(request, *args, **kwargs)
-
-        self.log_deletion(object, _("file was deleted"))
-        return HttpResponseRedirect(self.success_url)
+    def form_valid(self, form):
+        self.log_deletion(self.object, _("file was deleted"))
+        return super().form_valid(form)

src/open_inwoner/accounts/views/inbox.py

@@ -1,5 +1,5 @@
 import logging
-from typing import Optional
+from typing import List, Optional
 from urllib.parse import unquote
 
 from django.contrib.auth.mixins import LoginRequiredMixin
@@ -105,7 +105,7 @@ def get_other_user(self, conversations: dict) -> Optional[User]:
 
         return get_object_or_404(User, uuid=other_user_uuid)
 
-    def get_messages(self, other_user: User) -> MessageQuerySet:
+    def get_messages(self, other_user: User) -> List[MessageQuerySet]:
         """
         Returns the messages (MessageType) of the current conversation.
         """
@@ -122,12 +122,10 @@ def get_status(self, messages: MessageQuerySet) -> str:
         """
         Returns the status string of the conversation.
         """
-        try:
-            return f"{_('Laatste bericht ontvangen op')} {formats.date_format(messages[-1].created_on)}"
-        except IndexError:
-            return ""
-        except AssertionError:
+        messages = list(messages)
+        if not messages:
             return ""
+        return f"{_('Laatste bericht ontvangen op')} {formats.date_format(messages[-1].created_on)}"
 
     def mark_messages_seen(self, other_user: Optional[User]):
         if not other_user:

src/open_inwoner/cms/banner/migrations/0004_alter_bannerimage_cmsplugin_ptr_and_more.py

@@ -0,0 +1,41 @@
+# Generated by Django 4.2.10 on 2024-02-13 13:58
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("cms", "0022_auto_20180620_1551"),
+        ("banner", "0003_auto_20230511_1125"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="bannerimage",
+            name="cmsplugin_ptr",
+            field=models.OneToOneField(
+                auto_created=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                parent_link=True,
+                primary_key=True,
+                related_name="%(app_label)s_%(class)s",
+                serialize=False,
+                to="cms.cmsplugin",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="bannertext",
+            name="cmsplugin_ptr",
+            field=models.OneToOneField(
+                auto_created=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                parent_link=True,
+                primary_key=True,
+                related_name="%(app_label)s_%(class)s",
+                serialize=False,
+                to="cms.cmsplugin",
+            ),
+        ),
+    ]

src/open_inwoner/cms/plugins/migrations/0004_alter_userfeed_cmsplugin_ptr_and_more.py

@@ -0,0 +1,41 @@
+# Generated by Django 4.2.10 on 2024-02-13 13:58
+
+import django.db.models.deletion
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("cms", "0022_auto_20180620_1551"),
+        ("plugins", "0003_userfeed_title"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="userfeed",
+            name="cmsplugin_ptr",
+            field=models.OneToOneField(
+                auto_created=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                parent_link=True,
+                primary_key=True,
+                related_name="%(app_label)s_%(class)s",
+                serialize=False,
+                to="cms.cmsplugin",
+            ),
+        ),
+        migrations.AlterField(
+            model_name="videoplayer",
+            name="cmsplugin_ptr",
+            field=models.OneToOneField(
+                auto_created=True,
+                on_delete=django.db.models.deletion.CASCADE,
+                parent_link=True,
+                primary_key=True,
+                related_name="%(app_label)s_%(class)s",
+                serialize=False,
+                to="cms.cmsplugin",
+            ),
+        ),
+    ]

src/open_inwoner/cms/plugins/tests/test_userfeed.py

@@ -1,6 +1,6 @@
 from django.test import TestCase
 from django.utils.html import strip_tags
-from django.utils.translation import ngettext, ugettext as _
+from django.utils.translation import gettext as _, ngettext
 
 from pyquery import PyQuery as PQ
 

src/open_inwoner/cms/tests/cms_tools.py

@@ -2,7 +2,9 @@
 
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
-from django.contrib.sessions.middleware import SessionMiddleware
+from django.contrib.sessions.middleware import (
+    SessionMiddleware as DefaultSessionMiddleWare,
+)
 from django.test import RequestFactory
 from django.utils.module_loading import import_string
 
@@ -15,6 +17,15 @@
 from open_inwoner.cms.extensions.models import CommonExtension
 
 
+class SessionMiddleware(DefaultSessionMiddleWare):
+    """
+    The `SessionMiddleware` __init__ expects a `get_response` argument in Django 4.2
+    """
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(get_response=lambda x: "dummy")
+
+
 def create_homepage():
     """
     helper to create an empty, published homepage

src/open_inwoner/configurations/emails.py

@@ -3,7 +3,6 @@
 from django.db.models import F
 from django.utils import timezone
 
-from django_yubin.constants import RESULT_FAILED
 from django_yubin.models import Log
 from mail_editor.helpers import find_template
 
@@ -21,7 +20,7 @@ def inform_admins_about_failing_emails():
     period_start = now - timedelta(days=1)
 
     failed_email_logs = (
-        Log.objects.filter(date__gt=period_start, result=RESULT_FAILED)
+        Log.objects.filter(date__gt=period_start)
         .annotate(subject=F("message__subject"), recipient=F("message__to_address"))
         .values("subject", "recipient", "date")
     )

src/open_inwoner/configurations/tests/test_emails.py

@@ -2,7 +2,6 @@
 from django.core.management import call_command
 from django.test import TestCase
 
-from django_yubin.constants import RESULT_FAILED
 from django_yubin.models import Log, Message
 from freezegun import freeze_time
 
@@ -20,11 +19,8 @@ def test_no_recipients_configured(self):
                 subject="Some subject",
                 from_address="[email protected]",
                 to_address="[email protected]",
-                encoded_message="foo",
-            )
-            Log.objects.create(
-                message=message, result=RESULT_FAILED, log_message="Some "
             )
+            Log.objects.create(message=message, log_message="Some ")
 
         with freeze_time("2024-01-02T00:00:00"):
             call_command("send_failed_mail_digest")
@@ -41,11 +37,8 @@ def test_no_failing_emails_in_past_24_hours(self):
                 subject="Some subject",
                 from_address="[email protected]",
                 to_address="[email protected]",
-                encoded_message="foo",
-            )
-            Log.objects.create(
-                message=message, result=RESULT_FAILED, log_message="Some msg"
             )
+            Log.objects.create(message=message, log_message="Some msg")
 
         with freeze_time("2024-01-02T00:00:00"):
             call_command("send_failed_mail_digest")
@@ -62,22 +55,16 @@ def test_send_daily_failing_email_digest(self):
                 subject="Should not show up in email",
                 from_address="[email protected]",
                 to_address="[email protected]",
-                encoded_message="foo",
-            )
-            Log.objects.create(
-                message=message, result=RESULT_FAILED, log_message="Some msg"
             )
+            Log.objects.create(message=message, log_message="Some msg")
 
         with freeze_time("2024-01-01T12:00:00"):
             message = Message.objects.create(
                 subject="Should show up in email",
                 from_address="[email protected]",
                 to_address="[email protected]",
-                encoded_message="bar",
-            )
-            Log.objects.create(
-                message=message, result=RESULT_FAILED, log_message="Some msg"
             )
+            Log.objects.create(message=message, log_message="Some msg")
 
         with freeze_time("2024-01-02T00:00:00"):
             call_command("send_failed_mail_digest")

src/open_inwoner/haalcentraal/signals.py

@@ -5,7 +5,6 @@
 
 from open_inwoner.accounts.choices import LoginTypeChoices
 from open_inwoner.accounts.models import User
-from open_inwoner.utils.logentry import system_action
 
 from .utils import update_brp_data_in_db
 
@@ -19,8 +18,6 @@ def on_bsn_change(instance, **kwargs):
         and instance.is_prepopulated is False
         and instance.login_type == LoginTypeChoices.digid
     ):
-        system_action(
-            "Retrieving data from haal centraal based on BSN", content_object=instance
-        )
+        logger.info("Retrieving data for %s from haal centraal based on BSN", instance)
 
         update_brp_data_in_db(instance)

src/open_inwoner/haalcentraal/tests/test_signal.py

@@ -1,4 +1,5 @@
 import logging
+from unittest.mock import patch
 
 from django.test import TestCase, override_settings
 from django.utils.translation import gettext as _
@@ -275,23 +276,9 @@ def test_signal_updates_logging(self, m):
             first_name="", last_name="", login_type=LoginTypeChoices.digid
         )
         user.bsn = "999993847"
-        user.save()
 
-        log_entry = TimelineLog.objects.filter(object_id=user.id)[1]
-
-        self.assertEquals(
-            log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00"
-        )
-        self.assertEquals(log_entry.object_id, str(user.id))
-        self.assertEquals(
-            log_entry.extra_data,
-            {
-                "message": _("data was retrieved from haal centraal"),
-                "log_level": logging.INFO,
-                "action_flag": list(LOG_ACTIONS[5]),
-                "content_object_repr": str(user),
-            },
-        )
+        with self.assertLogs("open_inwoner.haalcentraal.signals"):
+            user.save()
 
     @requests_mock.Mocker()
     def test_single_entry_is_logged_when_there_is_an_error(self, m):
@@ -317,8 +304,6 @@ def test_single_entry_is_logged_when_there_is_an_error(self, m):
             first_name="", last_name="", login_type=LoginTypeChoices.digid
         )
         user.bsn = "999993847"
-        user.save()
-
-        log_entries = TimelineLog.objects.count()
 
-        self.assertEqual(log_entries, 1)
+        with self.assertLogs("open_inwoner.haalcentraal.signals"):
+            user.save()

src/open_inwoner/haalcentraal/utils.py

@@ -2,11 +2,9 @@
 from typing import Optional
 
 from django.conf import settings
-from django.utils.translation import gettext as _
 
 from open_inwoner.haalcentraal.api import BRP_1_3, BRP_2_1, BRPAPI
 from open_inwoner.haalcentraal.api_models import BRPData
-from open_inwoner.utils.logentry import system_action
 
 logger = logging.getLogger(__name__)
 
@@ -41,4 +39,4 @@ def update_brp_data_in_db(user, initial=True):
     if initial is False:
         user.save()
 
-    system_action(_("data was retrieved from haal centraal"), content_object=user)
+    logger.info("Retrieving data for %s from haal centraal based on BSN", user)