From bf3a2d4d1ef305ab74b07986a170ef6089a0f2b6 Mon Sep 17 00:00:00 2001 From: vasileios Date: Thu, 14 Apr 2022 11:28:19 +0200 Subject: [PATCH 1/2] [#468] Added validation when a contact is created in admin --- src/open_inwoner/accounts/admin.py | 24 ++++++ src/open_inwoner/accounts/tests/test_admin.py | 79 +++++++++++++++++++ 2 files changed, 103 insertions(+) create mode 100644 src/open_inwoner/accounts/tests/test_admin.py diff --git a/src/open_inwoner/accounts/admin.py b/src/open_inwoner/accounts/admin.py index fd646078ee..d46868f919 100644 --- a/src/open_inwoner/accounts/admin.py +++ b/src/open_inwoner/accounts/admin.py @@ -1,3 +1,4 @@ +from django import forms from django.contrib import admin from django.contrib.auth.admin import UserAdmin from django.urls import reverse, reverse_lazy @@ -79,6 +80,28 @@ class ActionAdmin(UUIDAdminFirstInOrder, PrivateMediaMixin, admin.ModelAdmin): private_media_fields = ("file",) +class CustomContactForm(forms.ModelForm): + class Meta: + model = Contact + fields = "__all__" + + def clean(self, *args, **kwargs): + cleaned_data = super().clean(*args, **kwargs) + + contact_email = cleaned_data["email"] + contact_user = cleaned_data["contact_user"] + if contact_email and not contact_user: + raise forms.ValidationError( + _('"Contact gebruiker" is mandatory when "E-mailadres" is filled in.') + ) + if contact_user and contact_email != contact_user.email: + raise forms.ValidationError( + _( + 'The email addresses of "E-mailadres" and "Contact gebruiker" do not match.' + ) + ) + + @admin.register(Contact) class ContactAdmin(UUIDAdminFirstInOrder, admin.ModelAdmin): readonly_fields = ("uuid",) @@ -94,6 +117,7 @@ class ContactAdmin(UUIDAdminFirstInOrder, admin.ModelAdmin): "contact_user", "created_by", ) + form = CustomContactForm @admin.register(Document) diff --git a/src/open_inwoner/accounts/tests/test_admin.py b/src/open_inwoner/accounts/tests/test_admin.py new file mode 100644 index 0000000000..0697f545cb --- /dev/null +++ b/src/open_inwoner/accounts/tests/test_admin.py @@ -0,0 +1,79 @@ +from django.urls import reverse +from django.utils.translation import ugettext_lazy as _ + +from django_webtest import WebTest + +from ..models import Contact +from .factories import ContactFactory, UserFactory + + +class TestContactValidation(WebTest): + def setUp(self): + self.user = UserFactory(is_superuser=True, is_staff=True) + self.contact = ContactFactory.build() + + def test_contact_is_saved_without_email_provided(self): + url = reverse("admin:accounts_contact_add") + form = self.app.get(url, user=self.user).forms.get("contact_form") + form["first_name"] = self.contact.first_name + form["last_name"] = self.contact.last_name + form["created_by"] = self.user.id + response = form.submit() + self.assertRedirects(response, reverse("admin:accounts_contact_changelist")) + self.assertEquals(Contact.objects.count(), 1) + + def test_contact_is_not_saved_with_email_and_missing_contact_user(self): + url = reverse("admin:accounts_contact_add") + form = self.app.get(url, user=self.user).forms.get("contact_form") + form["first_name"] = self.contact.first_name + form["last_name"] = self.contact.last_name + form["email"] = self.contact.email + form["created_by"] = self.user.id + response = form.submit() + self.assertEquals( + response.context["errors"], + [[_('"Contact gebruiker" is mandatory when "E-mailadres" is filled in.')]], + ) + self.assertEquals(Contact.objects.count(), 0) + + def test_contact_is_not_saved_with_email_and_different_contact_user(self): + other_user = UserFactory() + url = reverse("admin:accounts_contact_add") + form = self.app.get(url, user=self.user).forms.get("contact_form") + form["first_name"] = self.contact.first_name + form["last_name"] = self.contact.last_name + form["email"] = self.contact.email + form["created_by"] = self.user.id + form["contact_user"] = other_user.id + response = form.submit() + self.assertEquals( + response.context["errors"], + [ + [ + _( + 'The email addresses of "E-mailadres" and "Contact gebruiker" do not match.' + ) + ] + ], + ) + self.assertEquals(Contact.objects.count(), 0) + + def test_contact_is_not_saved_with_contact_user_and_missing_email(self): + url = reverse("admin:accounts_contact_add") + form = self.app.get(url, user=self.user).forms.get("contact_form") + form["first_name"] = self.contact.first_name + form["last_name"] = self.contact.last_name + form["created_by"] = self.user.id + form["contact_user"] = self.user.id + response = form.submit() + self.assertEquals( + response.context["errors"], + [ + [ + _( + 'The email addresses of "E-mailadres" and "Contact gebruiker" do not match.' + ) + ] + ], + ) + self.assertEquals(Contact.objects.count(), 0) From 64e424a0e1ed748d31d073d07930945e4c6f29a6 Mon Sep 17 00:00:00 2001 From: vasileios Date: Mon, 2 May 2022 16:17:07 +0200 Subject: [PATCH 2/2] [#468] Refactored according to PR review, steps 1,2 --- src/open_inwoner/accounts/admin.py | 24 ------ src/open_inwoner/accounts/models.py | 2 +- src/open_inwoner/accounts/query.py | 2 +- src/open_inwoner/accounts/tests/test_admin.py | 79 ------------------- .../accounts/tests/test_contact_views.py | 8 +- .../templates/pages/plans/detail.html | 2 +- 6 files changed, 10 insertions(+), 107 deletions(-) delete mode 100644 src/open_inwoner/accounts/tests/test_admin.py diff --git a/src/open_inwoner/accounts/admin.py b/src/open_inwoner/accounts/admin.py index d46868f919..fd646078ee 100644 --- a/src/open_inwoner/accounts/admin.py +++ b/src/open_inwoner/accounts/admin.py @@ -1,4 +1,3 @@ -from django import forms from django.contrib import admin from django.contrib.auth.admin import UserAdmin from django.urls import reverse, reverse_lazy @@ -80,28 +79,6 @@ class ActionAdmin(UUIDAdminFirstInOrder, PrivateMediaMixin, admin.ModelAdmin): private_media_fields = ("file",) -class CustomContactForm(forms.ModelForm): - class Meta: - model = Contact - fields = "__all__" - - def clean(self, *args, **kwargs): - cleaned_data = super().clean(*args, **kwargs) - - contact_email = cleaned_data["email"] - contact_user = cleaned_data["contact_user"] - if contact_email and not contact_user: - raise forms.ValidationError( - _('"Contact gebruiker" is mandatory when "E-mailadres" is filled in.') - ) - if contact_user and contact_email != contact_user.email: - raise forms.ValidationError( - _( - 'The email addresses of "E-mailadres" and "Contact gebruiker" do not match.' - ) - ) - - @admin.register(Contact) class ContactAdmin(UUIDAdminFirstInOrder, admin.ModelAdmin): readonly_fields = ("uuid",) @@ -117,7 +94,6 @@ class ContactAdmin(UUIDAdminFirstInOrder, admin.ModelAdmin): "contact_user", "created_by", ) - form = CustomContactForm @admin.register(Document) diff --git a/src/open_inwoner/accounts/models.py b/src/open_inwoner/accounts/models.py index 7a82bb14e4..173e2a5978 100644 --- a/src/open_inwoner/accounts/models.py +++ b/src/open_inwoner/accounts/models.py @@ -268,7 +268,7 @@ def is_not_active(self) -> bool: return not self.is_active() def get_message_url(self) -> str: - url = furl(reverse("accounts:inbox")).add({"with": self.email}).url + url = furl(reverse("accounts:inbox")).add({"with": self.other_user_email}).url return f"{url}#messages-last" def get_created_by_name(self): diff --git a/src/open_inwoner/accounts/query.py b/src/open_inwoner/accounts/query.py index 6d19020f2d..a60ba2af7e 100644 --- a/src/open_inwoner/accounts/query.py +++ b/src/open_inwoner/accounts/query.py @@ -134,7 +134,7 @@ def get_extended_contacts_for_user(self, me): ) .annotate( other_user_email=Case( - When(created_by=me, then=F("email")), + When(created_by=me, then=F("contact_user__email")), default=F("created_by__email"), ) ) diff --git a/src/open_inwoner/accounts/tests/test_admin.py b/src/open_inwoner/accounts/tests/test_admin.py deleted file mode 100644 index 0697f545cb..0000000000 --- a/src/open_inwoner/accounts/tests/test_admin.py +++ /dev/null @@ -1,79 +0,0 @@ -from django.urls import reverse -from django.utils.translation import ugettext_lazy as _ - -from django_webtest import WebTest - -from ..models import Contact -from .factories import ContactFactory, UserFactory - - -class TestContactValidation(WebTest): - def setUp(self): - self.user = UserFactory(is_superuser=True, is_staff=True) - self.contact = ContactFactory.build() - - def test_contact_is_saved_without_email_provided(self): - url = reverse("admin:accounts_contact_add") - form = self.app.get(url, user=self.user).forms.get("contact_form") - form["first_name"] = self.contact.first_name - form["last_name"] = self.contact.last_name - form["created_by"] = self.user.id - response = form.submit() - self.assertRedirects(response, reverse("admin:accounts_contact_changelist")) - self.assertEquals(Contact.objects.count(), 1) - - def test_contact_is_not_saved_with_email_and_missing_contact_user(self): - url = reverse("admin:accounts_contact_add") - form = self.app.get(url, user=self.user).forms.get("contact_form") - form["first_name"] = self.contact.first_name - form["last_name"] = self.contact.last_name - form["email"] = self.contact.email - form["created_by"] = self.user.id - response = form.submit() - self.assertEquals( - response.context["errors"], - [[_('"Contact gebruiker" is mandatory when "E-mailadres" is filled in.')]], - ) - self.assertEquals(Contact.objects.count(), 0) - - def test_contact_is_not_saved_with_email_and_different_contact_user(self): - other_user = UserFactory() - url = reverse("admin:accounts_contact_add") - form = self.app.get(url, user=self.user).forms.get("contact_form") - form["first_name"] = self.contact.first_name - form["last_name"] = self.contact.last_name - form["email"] = self.contact.email - form["created_by"] = self.user.id - form["contact_user"] = other_user.id - response = form.submit() - self.assertEquals( - response.context["errors"], - [ - [ - _( - 'The email addresses of "E-mailadres" and "Contact gebruiker" do not match.' - ) - ] - ], - ) - self.assertEquals(Contact.objects.count(), 0) - - def test_contact_is_not_saved_with_contact_user_and_missing_email(self): - url = reverse("admin:accounts_contact_add") - form = self.app.get(url, user=self.user).forms.get("contact_form") - form["first_name"] = self.contact.first_name - form["last_name"] = self.contact.last_name - form["created_by"] = self.user.id - form["contact_user"] = self.user.id - response = form.submit() - self.assertEquals( - response.context["errors"], - [ - [ - _( - 'The email addresses of "E-mailadres" and "Contact gebruiker" do not match.' - ) - ] - ], - ) - self.assertEquals(Contact.objects.count(), 0) diff --git a/src/open_inwoner/accounts/tests/test_contact_views.py b/src/open_inwoner/accounts/tests/test_contact_views.py index 62a6d0c114..18313b9314 100644 --- a/src/open_inwoner/accounts/tests/test_contact_views.py +++ b/src/open_inwoner/accounts/tests/test_contact_views.py @@ -61,8 +61,14 @@ def test_contact_filter(self): self.assertContains(response, begeleider.first_name) def test_contact_list_show_link_to_messages(self): + other_user = UserFactory() + contact = ContactFactory.create(created_by=self.user, contact_user=other_user) + contacts = Contact.objects.get_extended_contacts_for_user(self.user) + extended_contact = contacts.get(id=contact.id) message_link = ( - furl(reverse("accounts:inbox")).add({"with": self.contact.email}).url + furl(reverse("accounts:inbox")) + .add({"with": extended_contact.other_user_email}) + .url ) response = self.app.get(self.list_url, user=self.user) self.assertContains(response, message_link) diff --git a/src/open_inwoner/templates/pages/plans/detail.html b/src/open_inwoner/templates/pages/plans/detail.html index 182d563b89..d8504d196a 100644 --- a/src/open_inwoner/templates/pages/plans/detail.html +++ b/src/open_inwoner/templates/pages/plans/detail.html @@ -49,7 +49,7 @@

{{ contact.get_name }}

{{ contact.function }}

{{ contact.phonenumber }}

-

{{ contact.email }}

+

{{ contact.contact_user.email }}

{% else %}

{{ contact.get_created_by_name }}

{{ contact.function }}