From e34b559ec97cad53d00d6f75fed4c05e68771377 Mon Sep 17 00:00:00 2001 From: Paul Schilling Date: Tue, 29 Aug 2023 09:44:41 +0200 Subject: [PATCH] [#1672] update logging tests - the format of log messages had changed due to the change of the user account's __str__ method --- src/open_inwoner/accounts/backends.py | 12 +---- src/open_inwoner/accounts/forms.py | 13 +++--- src/open_inwoner/accounts/models.py | 18 +++----- src/open_inwoner/accounts/tests/test_auth.py | 2 +- .../accounts/tests/test_auth_2fa_sms.py | 2 +- .../accounts/tests/test_contact_views.py | 15 ------- .../accounts/tests/test_logging.py | 44 ++++++++++--------- .../accounts/tests/test_message.py | 2 +- .../accounts/tests/test_profile_views.py | 10 ++--- .../haalcentraal/tests/test_signal.py | 2 +- src/open_inwoner/plans/tests/test_logging.py | 2 +- src/open_inwoner/search/tests/test_logging.py | 2 +- 12 files changed, 49 insertions(+), 75 deletions(-) diff --git a/src/open_inwoner/accounts/backends.py b/src/open_inwoner/accounts/backends.py index 8e7e3990e0..012e0c7f79 100644 --- a/src/open_inwoner/accounts/backends.py +++ b/src/open_inwoner/accounts/backends.py @@ -23,20 +23,10 @@ class UserModelEmailBackend(ModelBackend): """ def authenticate( - self, - request, - login_type=None, - username=None, - password=None, - user=None, - token=None, - **kwargs + self, request, username=None, password=None, user=None, token=None, **kwargs ): config = SiteConfiguration.get_solo() - if login_type == "digid": - return None - if username and password and not config.login_2fa_sms: try: user = get_user_model().objects.get(email__iexact=username) diff --git a/src/open_inwoner/accounts/forms.py b/src/open_inwoner/accounts/forms.py index 588d66747a..6251124363 100644 --- a/src/open_inwoner/accounts/forms.py +++ b/src/open_inwoner/accounts/forms.py @@ -333,14 +333,18 @@ def find_user( Q(first_name__iexact=first_name) & Q(last_name__iexact=last_name) ) + existing_users = existing_users.filter(is_active=True) + # no active users with the given specs - if not (existing_users := existing_users.filter(is_active=True)): + if not existing_users: raise ValidationError( _("The user cannot be added, their account has been deleted.") ) - # multiple users with the given specs - if existing_users.count() > 1: + # check if there are multiple users with the given specs + try: + existing_user = existing_users.get() + except User.MultipleObjectsReturned: raise ValidationError( _( "We're having trouble finding an account with this information." @@ -348,9 +352,6 @@ def find_user( ) ) - # exactly 1 user - existing_user = existing_users.get() - # contact already exists if self.user.has_contact(existing_user): raise ValidationError( diff --git a/src/open_inwoner/accounts/models.py b/src/open_inwoner/accounts/models.py index 9599a6eddb..920ff1c5e8 100644 --- a/src/open_inwoner/accounts/models.py +++ b/src/open_inwoner/accounts/models.py @@ -224,7 +224,9 @@ def __init__(self, *args, **kwargs): self._old_bsn = self.bsn def __str__(self): - return f"{self.first_name} {self.last_name} ({self.email})" + return ( + f"{self.first_name} {self.last_name} ({self.get_contact_email()})".strip() + ) def clean(self, *args, **kwargs): """Reject non-unique emails, except for users with login_type DigiD""" @@ -240,7 +242,7 @@ def clean(self, *args, **kwargs): return # account has been deactivated - if any(user.bsn == self.bsn and user.is_not_active for user in existing_users): + if any(user.bsn == self.bsn and not user.is_active for user in existing_users): raise ValidationError( {"email": ValidationError(_("This account has been deactivated"))} ) @@ -253,11 +255,7 @@ def clean(self, *args, **kwargs): # some account does not have login_type digid raise ValidationError( - { - "email": ValidationError( - _("The user cannot be added. Please contact us for help.") - ) - } + {"email": ValidationError(_("This email is already taken."))} ) @property @@ -356,10 +354,6 @@ def get_contact_type_display(self) -> str: choice = ContactTypeChoices.get_choice(self.contact_type) return choice.label - @property - def is_not_active(self): - return not self.is_active - def get_contact_email(self): return self.email if "@example.org" not in self.email else "" @@ -376,7 +370,7 @@ def has_contact(self, user): """ :returns: `True` if the subject has `user` as contact, `False` otherwise """ - return user in self.user_contacts.all() + return self.user_contacts.filter(id=user.id).exists() def get_plan_contact_new_count(self): return ( diff --git a/src/open_inwoner/accounts/tests/test_auth.py b/src/open_inwoner/accounts/tests/test_auth.py index 8cea95591f..57a15e6ca3 100644 --- a/src/open_inwoner/accounts/tests/test_auth.py +++ b/src/open_inwoner/accounts/tests/test_auth.py @@ -676,7 +676,7 @@ class DuplicateEmailRegistrationTest(WebTest): @classmethod def setUpTestData(cls): - cls.msg_dupes = _("The user cannot be added. Please contact us for help.") + cls.msg_dupes = _("This email is already taken.") cls.msg_inactive = _("This account has been deactivated") # diff --git a/src/open_inwoner/accounts/tests/test_auth_2fa_sms.py b/src/open_inwoner/accounts/tests/test_auth_2fa_sms.py index 553f609117..949c74ebc3 100644 --- a/src/open_inwoner/accounts/tests/test_auth_2fa_sms.py +++ b/src/open_inwoner/accounts/tests/test_auth_2fa_sms.py @@ -131,7 +131,7 @@ def test_login_fails_with_sms_failure(self, mock_gateway_send): "Het versturen van een SMS-bericht aan {phonenumber} is mislukt. Inloggen afgebroken." ).format(phonenumber=self.user.phonenumber), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": f"{self.user.first_name} {self.user.last_name} ({self.user.email})", }, ) diff --git a/src/open_inwoner/accounts/tests/test_contact_views.py b/src/open_inwoner/accounts/tests/test_contact_views.py index 49d02dde5f..248dc0a5c1 100644 --- a/src/open_inwoner/accounts/tests/test_contact_views.py +++ b/src/open_inwoner/accounts/tests/test_contact_views.py @@ -40,13 +40,11 @@ def test_contact_list_login_required(self): def test_contact_list_filled(self): response = self.app.get(self.list_url, user=self.user) - self.assertEqual(response.status_code, 200) self.assertContains(response, self.contact.first_name) def test_contact_list_only_show_personal_contacts(self): other_user = UserFactory() response = self.app.get(self.list_url, user=other_user) - self.assertEqual(response.status_code, 200) self.assertNotContains(response, self.contact.first_name) def test_list_shows_pending_invitations(self): @@ -62,14 +60,12 @@ def test_contact_filter(self): self.user.user_contacts.add(begeleider) response = self.app.get(self.list_url, user=self.user) - self.assertEqual(response.status_code, 200) self.assertContains(response, self.contact.first_name) self.assertContains(response, begeleider.first_name) form = response.forms["contact-filter"] form["type"] = ContactTypeChoices.begeleider response = form.submit() - self.assertEqual(response.status_code, 200) self.assertNotContains(response, self.contact.first_name) self.assertContains(response, begeleider.first_name) @@ -81,7 +77,6 @@ def test_contact_filter_without_any_contacts(self): form = response.forms["contact-filter"] form["type"] = ContactTypeChoices.contact response = form.submit() - self.assertEqual(response.status_code, 200) self.assertContains( response, _( @@ -129,7 +124,6 @@ def test_contact_list_show_reversed(self): other_contact.user_contacts.add(self.user) response = self.app.get(self.list_url, user=self.user) - self.assertEqual(response.status_code, 200) self.assertContains(response, "reverse_contact_user_should_be_found") def test_contact_create_login_required(self): @@ -139,7 +133,6 @@ def test_contact_create_login_required(self): def test_new_user_contact_not_created_and_invite_sent(self): contacts_before = list(self.user.user_contacts.all()) response = self.app.get(self.create_url, user=self.user) - self.assertEqual(response.status_code, 200) form = response.forms["contact-form"] form["first_name"] = "John" @@ -176,8 +169,6 @@ def test_existing_user_contact(self): response = form.submit(user=self.user) pending_invitation = self.user.contacts_for_approval.first() - self.assertEqual(response.status_code, 302) - response = response.follow() self.assertContains(response, existing_user.email) self.assertNotContains(response, existing_user.first_name) @@ -214,7 +205,6 @@ def test_adding_same_contact_fails(self): ) ] } - self.assertEqual(response.status_code, 200) self.assertEqual(response.context["form"].errors, expected_errors) def test_adding_inactive_contact_fails(self): @@ -232,7 +222,6 @@ def test_adding_inactive_contact_fails(self): response = form.submit() - self.assertEqual(response.status_code, 200) self.assertEqual(response.context["form"].errors, expected_errors) def test_user_cannot_add_themselves(self): @@ -247,7 +236,6 @@ def test_user_cannot_add_themselves(self): response = form.submit() - self.assertEqual(response.status_code, 200) self.assertEqual(response.context["form"].errors, expected_errors) def test_adding_contact_with_invalid_first_name_chars_fails(self): @@ -321,7 +309,6 @@ def test_digid_contact_with_duplicate_email_success(self, m): response = form.submit(user=self.digid_user) pending_invitation = self.digid_user.contacts_for_approval.first() - self.assertEqual(response.status_code, 302) self.assertContains(response.follow(), existing_user.email) self.assertEqual(existing_user, pending_invitation) @@ -349,7 +336,6 @@ def test_digid_contact_duplicate_email_case_insensitive_success(self, m): response = form.submit(user=self.digid_user) pending_invitation = self.digid_user.contacts_for_approval.first() - self.assertEqual(response.status_code, 302) self.assertContains(response.follow(), existing_user.email) self.assertEqual(existing_user, pending_invitation) @@ -361,7 +347,6 @@ def test_email_required(self): form["last_name"] = self.contact.last_name response = form.submit(user=self.user) expected_errors = {"email": ["Dit veld is vereist."]} - self.assertEqual(response.status_code, 200) self.assertEqual(response.context["form"].errors, expected_errors) def test_users_contact_is_removed(self): diff --git a/src/open_inwoner/accounts/tests/test_logging.py b/src/open_inwoner/accounts/tests/test_logging.py index c71ab7007b..4d2f0afc4b 100644 --- a/src/open_inwoner/accounts/tests/test_logging.py +++ b/src/open_inwoner/accounts/tests/test_logging.py @@ -67,7 +67,7 @@ def test_registration_is_logged(self): { "message": _("user was created"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": user.email, + "content_object_repr": str(user), }, ) @@ -79,6 +79,8 @@ def test_users_modification_is_logged(self): form.submit() log_entry = TimelineLog.objects.last() + self.user.refresh_from_db() + self.assertEqual( log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) @@ -88,7 +90,7 @@ def test_users_modification_is_logged(self): { "message": _("profile was modified"), "action_flag": list(LOG_ACTIONS[CHANGE]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) @@ -112,7 +114,7 @@ def test_categories_modification_is_logged(self): { "message": _("categories were modified"), "action_flag": list(LOG_ACTIONS[CHANGE]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) @@ -134,7 +136,7 @@ def test_user_notifications_update_is_logged(self, mock_cms_page_display): { "message": _("users notifications were modified"), "action_flag": list(LOG_ACTIONS[CHANGE]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) @@ -151,7 +153,7 @@ def test_login_via_admin_is_logged(self): { "message": _("user was logged in via admin page"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) @@ -168,7 +170,7 @@ def test_login_via_frontend_using_email_is_logged(self): { "message": _("user was logged in via frontend using email"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) @@ -186,7 +188,7 @@ def test_login_via_frontend_using_digid_is_logged(self): { "message": _("user was logged in via frontend using digid"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": user.email, + "content_object_repr": str(user), }, ) @@ -203,7 +205,7 @@ def test_logout_is_logged(self): { "message": _("user was logged out"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) @@ -222,7 +224,7 @@ def test_users_deletion_is_logged(self): { "message": _("user was deleted via frontend"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) @@ -249,7 +251,7 @@ def test_password_change_is_logged(self): { "message": _("password was changed"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": user.email, + "content_object_repr": str(user), }, ) @@ -298,7 +300,7 @@ def test_password_reset_confirm_is_logged(self): "message": _("password reset was completed"), "log_level": logging.INFO, "action_flag": list(LOG_ACTIONS[5]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) @@ -328,7 +330,7 @@ def test_accepted_invite_is_logged(self): "log_level": logging.INFO, "action_flag": list(LOG_ACTIONS[5]), "content_object_repr": _("For: {invitee} (2021-10-18)").format( - invitee=self.invitee.email + invitee=f"{self.invitee.first_name} {self.invitee.last_name} ({self.invitee.email})" ), }, ) @@ -354,7 +356,7 @@ def test_expired_invite_is_logged(self): "log_level": logging.INFO, "action_flag": list(LOG_ACTIONS[5]), "content_object_repr": _("For: {invitee} (2021-09-18)").format( - invitee=self.invitee.email + invitee=f"{self.invitee.first_name} {self.invitee.last_name} ({self.invitee.email})" ), }, ) @@ -640,13 +642,14 @@ def test_created_message_action_from_contacts_is_logged(self): log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) self.assertEqual(log_entry.content_object.id, Message.objects.get().id) + self.assertEqual( log_entry.extra_data, { "message": _("message was created"), "action_flag": list(LOG_ACTIONS[ADDITION]), - "content_object_repr": _("From: {me}, To: {other} (2021-10-18)").format( - me=self.user.email, other=self.other_user.email + "content_object_repr": _("From: {}, To: {} (2021-10-18)").format( + str(self.user), str(self.other_user) ), }, ) @@ -665,13 +668,14 @@ def test_created_message_action_from_start_is_logged(self): log_entry.timestamp.strftime("%m/%d/%Y, %H:%M:%S"), "10/18/2021, 13:00:00" ) self.assertEqual(log_entry.content_object.id, Message.objects.get().id) + self.assertEqual( log_entry.extra_data, { "message": _("message was created"), "action_flag": list(LOG_ACTIONS[ADDITION]), - "content_object_repr": _("From: {me}, To: {other} (2021-10-18)").format( - me=self.user.email, other=self.other_user.email + "content_object_repr": _("From: {}, To: {} (2021-10-18)").format( + str(self.user), str(self.other_user) ), }, ) @@ -722,7 +726,7 @@ def test_profile_export_is_logged(self): { "message": _("file profile.pdf was exported"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) @@ -744,7 +748,7 @@ def test_action_export_is_logged(self): action_uuid=self.action1.uuid ), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) @@ -764,6 +768,6 @@ def test_action_list_export_is_logged(self): { "message": _("file actions.pdf was exported"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": str(self.user), }, ) diff --git a/src/open_inwoner/accounts/tests/test_message.py b/src/open_inwoner/accounts/tests/test_message.py index 0f6d3d626a..f2bf45b50e 100644 --- a/src/open_inwoner/accounts/tests/test_message.py +++ b/src/open_inwoner/accounts/tests/test_message.py @@ -22,6 +22,6 @@ def test_str(self): message = MessageFactory(sender=self.sender, receiver=self.receiver) self.assertEqual( str(message), - "From: person-a@example.com, To: person-b@example.com (2021-12-21)", + "From: Person A (person-a@example.com), To: Person B (person-b@example.com) (2021-12-21)", ) message.delete() diff --git a/src/open_inwoner/accounts/tests/test_profile_views.py b/src/open_inwoner/accounts/tests/test_profile_views.py index 627995815e..ec513f986c 100644 --- a/src/open_inwoner/accounts/tests/test_profile_views.py +++ b/src/open_inwoner/accounts/tests/test_profile_views.py @@ -634,8 +634,8 @@ class MyDataTests(HaalCentraalMixin, WebTest): def setUp(self): self.user = UserFactory( bsn="999993847", - first_name="", - last_name="", + first_name="Merel", + last_name="Kooyman", login_type=LoginTypeChoices.digid, ) self.url = reverse("profile:data") @@ -656,7 +656,7 @@ def test_expected_response_is_returned_brp_v_2(self, m): { "message": _("user requests for brp data"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": f"{self.user.first_name} {self.user.last_name} ({self.user.email})", }, ) @@ -677,7 +677,7 @@ def test_expected_response_is_returned_brp_v_1_3(self, m): { "message": _("user requests for brp data"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": f"{self.user.first_name} {self.user.last_name} ({self.user.email})", }, ) @@ -710,7 +710,7 @@ def test_wrong_date_format_shows_birthday_none_brp_v_1_3(self, m): { "message": _("user requests for brp data"), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": f"({self.user.email})", }, ) diff --git a/src/open_inwoner/haalcentraal/tests/test_signal.py b/src/open_inwoner/haalcentraal/tests/test_signal.py index 65c4a76d04..61ac82984e 100644 --- a/src/open_inwoner/haalcentraal/tests/test_signal.py +++ b/src/open_inwoner/haalcentraal/tests/test_signal.py @@ -298,7 +298,7 @@ def test_signal_updates_logging(self, m): "message": _("data was retrieved from haal centraal"), "log_level": logging.INFO, "action_flag": list(LOG_ACTIONS[5]), - "content_object_repr": user.email, + "content_object_repr": f"{user.first_name} {user.last_name} ({user.email})", }, ) diff --git a/src/open_inwoner/plans/tests/test_logging.py b/src/open_inwoner/plans/tests/test_logging.py index d6d6bc6afb..1249891ae5 100644 --- a/src/open_inwoner/plans/tests/test_logging.py +++ b/src/open_inwoner/plans/tests/test_logging.py @@ -244,6 +244,6 @@ def test_plan_export_is_logged(self): plan_uuid=self.plan.uuid ), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": self.user.email, + "content_object_repr": f"{self.user.first_name} {self.user.last_name} ({self.user.email})", }, ) diff --git a/src/open_inwoner/search/tests/test_logging.py b/src/open_inwoner/search/tests/test_logging.py index f8efbf71c0..4f1621cca2 100644 --- a/src/open_inwoner/search/tests/test_logging.py +++ b/src/open_inwoner/search/tests/test_logging.py @@ -32,7 +32,7 @@ def test_search_query_of_logged_in_user_is_logged(self): query="search for something" ), "action_flag": list(LOG_ACTIONS[4]), - "content_object_repr": user.email, + "content_object_repr": f"{user.first_name} {user.last_name} ({user.email})", }, )