From 255d2535b44f88ee23efae9708300e7c107838cf Mon Sep 17 00:00:00 2001 From: Florian Strehl Date: Mon, 10 Jul 2023 18:41:59 +0200 Subject: [PATCH 1/4] Fixed duplicate through email cleaning #1956 --- evap/staff/forms.py | 4 ++-- evap/staff/tests/test_forms.py | 13 ++++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/evap/staff/forms.py b/evap/staff/forms.py index 28f73910c7..e2de5b2268 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -31,7 +31,7 @@ TextAnswer, UserProfile, ) -from evap.evaluation.tools import date_to_datetime +from evap.evaluation.tools import date_to_datetime, clean_email from evap.results.tools import STATES_WITH_RESULT_TEMPLATE_CACHING, STATES_WITH_RESULTS_CACHING, cache_results from evap.results.views import update_template_cache, update_template_cache_of_published_evaluations_in_course from evap.staff.tools import remove_user_from_represented_and_ccing_users @@ -991,7 +991,7 @@ def clean_email(self): if email is None: return None - user_with_same_email = UserProfile.objects.filter(email__iexact=email) + user_with_same_email = UserProfile.objects.filter(email__iexact=clean_email(email)) # make sure we don't take the instance itself into account if self.instance and self.instance.pk: diff --git a/evap/staff/tests/test_forms.py b/evap/staff/tests/test_forms.py index 6d7754b8ef..a18a6f5554 100644 --- a/evap/staff/tests/test_forms.py +++ b/evap/staff/tests/test_forms.py @@ -1,7 +1,7 @@ from unittest.mock import patch from django.forms.models import inlineformset_factory -from django.test import TestCase +from django.test import TestCase, override_settings from model_bakery import baker from evap.contributor.forms import EvaluationForm as ContributorEvaluationForm @@ -121,6 +121,11 @@ def test_evaluation_email_form(self): class UserFormTests(TestCase): + + @classmethod + def setUpTestData(cls): + cls.existing_user = baker.make(UserProfile, email="existing@example.com") + def test_user_form(self): """ Tests the UserForm with one valid and one invalid input dataset. @@ -135,6 +140,7 @@ def test_user_form(self): form = UserForm(instance=user, data=data) self.assertFalse(form.is_valid()) + @override_settings(INSTITUTION_EMAIL_REPLACEMENTS=[("institution.example.com", "example.com")]) def test_user_with_same_email(self): """ Tests whether the user form correctly handles email adresses @@ -155,6 +161,11 @@ def test_user_with_same_email(self): form = UserForm(instance=user, data=data) self.assertTrue(form.is_valid()) + data = {"email": "existing@institution.example.com"} + form = UserForm(instance=user, data=data) + self.assertFalse(form.is_valid()) + self.assertIn("A user with the email 'existing@institution.example.com' already exists", form.errors["email"]) + def test_user_cannot_be_removed_from_evaluation_already_voted_for(self): student = baker.make(UserProfile) baker.make(Evaluation, participants=[student], voters=[student], course__semester__is_active=True) From 6bb00e2ca6f1e4166207dd18a74a9a3e5adc83ad Mon Sep 17 00:00:00 2001 From: Florian Strehl Date: Mon, 10 Jul 2023 18:47:46 +0200 Subject: [PATCH 2/4] Fixed style issues --- evap/staff/forms.py | 2 +- evap/staff/tests/test_forms.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/evap/staff/forms.py b/evap/staff/forms.py index e2de5b2268..c026f77652 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -31,7 +31,7 @@ TextAnswer, UserProfile, ) -from evap.evaluation.tools import date_to_datetime, clean_email +from evap.evaluation.tools import clean_email, date_to_datetime from evap.results.tools import STATES_WITH_RESULT_TEMPLATE_CACHING, STATES_WITH_RESULTS_CACHING, cache_results from evap.results.views import update_template_cache, update_template_cache_of_published_evaluations_in_course from evap.staff.tools import remove_user_from_represented_and_ccing_users diff --git a/evap/staff/tests/test_forms.py b/evap/staff/tests/test_forms.py index a18a6f5554..9ea8f69a30 100644 --- a/evap/staff/tests/test_forms.py +++ b/evap/staff/tests/test_forms.py @@ -121,7 +121,6 @@ def test_evaluation_email_form(self): class UserFormTests(TestCase): - @classmethod def setUpTestData(cls): cls.existing_user = baker.make(UserProfile, email="existing@example.com") From b61c6f46fe409bd711fb3f43997212148253d638 Mon Sep 17 00:00:00 2001 From: Florian Strehl Date: Wed, 12 Jul 2023 11:42:39 +0200 Subject: [PATCH 3/4] Refactored clean_email usage --- evap/staff/forms.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/evap/staff/forms.py b/evap/staff/forms.py index c026f77652..63492a6dc6 100644 --- a/evap/staff/forms.py +++ b/evap/staff/forms.py @@ -987,11 +987,11 @@ def clean_evaluations_participating_in(self): return evaluations_participating_in def clean_email(self): - email = self.cleaned_data.get("email") + email = clean_email(self.cleaned_data.get("email")) if email is None: return None - user_with_same_email = UserProfile.objects.filter(email__iexact=clean_email(email)) + user_with_same_email = UserProfile.objects.filter(email__iexact=email) # make sure we don't take the instance itself into account if self.instance and self.instance.pk: @@ -999,7 +999,7 @@ def clean_email(self): if user_with_same_email.exists(): raise forms.ValidationError(_("A user with the email '%s' already exists") % email) - return email.lower() + return email def save(self, *args, **kw): super().save(*args, **kw) From 5143f43ffa2d479cc9b523890639d106b6f3bd19 Mon Sep 17 00:00:00 2001 From: Florian Strehl Date: Wed, 12 Jul 2023 11:47:32 +0200 Subject: [PATCH 4/4] Fixed failing test due to changed error message --- evap/staff/tests/test_forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/evap/staff/tests/test_forms.py b/evap/staff/tests/test_forms.py index 9ea8f69a30..d9f10a8940 100644 --- a/evap/staff/tests/test_forms.py +++ b/evap/staff/tests/test_forms.py @@ -163,7 +163,7 @@ def test_user_with_same_email(self): data = {"email": "existing@institution.example.com"} form = UserForm(instance=user, data=data) self.assertFalse(form.is_valid()) - self.assertIn("A user with the email 'existing@institution.example.com' already exists", form.errors["email"]) + self.assertIn("A user with the email 'existing@example.com' already exists", form.errors["email"]) def test_user_cannot_be_removed_from_evaluation_already_voted_for(self): student = baker.make(UserProfile)