From e277c3a64c346f4a7239639488b1c7decccb6927 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Fri, 19 May 2023 10:52:16 -0500 Subject: [PATCH 1/7] Form to edit a domain's authorizing official --- src/registrar/config/urls.py | 5 +++ src/registrar/forms/__init__.py | 2 +- src/registrar/forms/domain.py | 40 +++++++++++++++++ src/registrar/models/contact.py | 3 ++ .../domain_authorizing_official.html | 43 ++++++++++++++++++ src/registrar/templates/domain_sidebar.html | 2 +- src/registrar/views/__init__.py | 1 + src/registrar/views/domain.py | 45 ++++++++++++++++++- 8 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 src/registrar/templates/domain_authorizing_official.html diff --git a/src/registrar/config/urls.py b/src/registrar/config/urls.py index 204e5ca92..1087a2f19 100644 --- a/src/registrar/config/urls.py +++ b/src/registrar/config/urls.py @@ -83,6 +83,11 @@ urlpatterns = [ views.DomainNameserversView.as_view(), name="domain-nameservers", ), + path( + "domain//authorizing-official", + views.DomainAuthorizingOfficialView.as_view(), + name="domain-authorizing-official", + ), path( "domain//users/add", views.DomainAddUserView.as_view(), diff --git a/src/registrar/forms/__init__.py b/src/registrar/forms/__init__.py index 6c1b5d8cf..8e35825aa 100644 --- a/src/registrar/forms/__init__.py +++ b/src/registrar/forms/__init__.py @@ -1,2 +1,2 @@ from .application_wizard import * -from .domain import DomainAddUserForm, NameserverFormset +from .domain import DomainAddUserForm, NameserverFormset, ContactForm diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index cca0bf5c9..1c8033a2b 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -3,6 +3,9 @@ from django import forms from django.forms import formset_factory +from phonenumber_field.widgets import RegionalPhoneNumberWidget + +from ..models import Contact class DomainAddUserForm(forms.Form): @@ -22,3 +25,40 @@ NameserverFormset = formset_factory( DomainNameserverForm, extra=1, ) + + +class ContactForm(forms.ModelForm): + + """Form for updating contacts.""" + + class Meta: + model = Contact + fields = ["first_name", "middle_name", "last_name", "title", "email", "phone"] + widgets = { + "first_name": forms.TextInput, + "middle_name": forms.TextInput, + "last_name": forms.TextInput, + "title": forms.TextInput, + "email": forms.EmailInput, + "phone": RegionalPhoneNumberWidget, + } + + # the database fields have blank=True so ModelForm doesn't create + # required fields by default. Use this list in __init__ to mark each + # of these fields as required + required = [ + "first_name", + "last_name", + "title", + "email", + "phone" + ] + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + # take off maxlength attribute for the phone number field + # which interferes with out input_with_errors template tag + self.fields['phone'].widget.attrs.pop('maxlength', None) + + for field_name in self.required: + self.fields[field_name].required = True diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index d5d32a7ae..cbfde7a23 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -20,6 +20,7 @@ class Contact(TimeStampedModel): null=True, blank=True, help_text="First name", + verbose_name="first name / given name", db_index=True, ) middle_name = models.TextField( @@ -31,12 +32,14 @@ class Contact(TimeStampedModel): null=True, blank=True, help_text="Last name", + verbose_name="last name / family name", db_index=True, ) title = models.TextField( null=True, blank=True, help_text="Title", + verbose_name="title or role in your organization", ) email = models.TextField( null=True, diff --git a/src/registrar/templates/domain_authorizing_official.html b/src/registrar/templates/domain_authorizing_official.html new file mode 100644 index 000000000..445db5707 --- /dev/null +++ b/src/registrar/templates/domain_authorizing_official.html @@ -0,0 +1,43 @@ +{% extends "domain_base.html" %} +{% load static field_helpers%} + +{% block title %}Domain authorizing official | {{ domain.name }} | {% endblock %} + +{% block domain_content %} + {# this is right after the messages block in the parent template #} + {% include "includes/form_errors.html" with form=form %} + +

Authorizing official

+ +

Your authorizing official is the person within your organization who can + authorize domain requests. This is generally the highest-ranking or + highest-elected official in your organization. Read more about who can serve + as an authorizing official.

+ + {% include "includes/required_fields.html" %} + +
+ {% csrf_token %} + + {% input_with_errors form.first_name %} + + {% input_with_errors form.middle_name %} + + {% input_with_errors form.last_name %} + + {% input_with_errors form.title %} + + {% input_with_errors form.email %} + + {% input_with_errors form.phone %} + + + + +
+ +{% endblock %} {# domain_content #} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index c50cc59bd..ab259a4a7 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -31,7 +31,7 @@
  • - {% url 'todo' as url %} + {% url 'domain-authorizing-official' pk=domain.id as url %} diff --git a/src/registrar/views/__init__.py b/src/registrar/views/__init__.py index 9f7fe139e..2eae93370 100644 --- a/src/registrar/views/__init__.py +++ b/src/registrar/views/__init__.py @@ -1,6 +1,7 @@ from .application import * from .domain import ( DomainView, + DomainAuthorizingOfficialView, DomainNameserversView, DomainUsersView, DomainAddUserView, diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 8a9095de3..8b9186958 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -12,7 +12,7 @@ from django.views.generic.edit import DeleteView, FormMixin from registrar.models import Domain, DomainInvitation, User, UserDomainRole -from ..forms import DomainAddUserForm, NameserverFormset +from ..forms import DomainAddUserForm, NameserverFormset, ContactForm from ..utility.email import send_templated_email, EmailSendingError from .utility import DomainPermission @@ -29,6 +29,49 @@ class DomainView(DomainPermission, DetailView): context_object_name = "domain" +class DomainAuthorizingOfficialView(DomainPermission, FormMixin, DetailView): + + """Domain authorizing official editing view.""" + + model = Domain + template_name = "domain_authorizing_official.html" + context_object_name = "domain" + form_class = ContactForm + + def get_form_kwargs(self, *args, **kwargs): + """Add domain_info.authorizing_official instance to make a bound form.""" + form_kwargs = super().get_form_kwargs(*args, **kwargs) + form_kwargs["instance"] = self.get_object().domain_info.authorizing_official + return form_kwargs + + def get_success_url(self): + """Redirect to the overview page for the domain.""" + return reverse("domain-authorizing-official", kwargs={"pk": self.object.pk}) + + def post(self, request, *args, **kwargs): + """Form submission posts to this view. + + This post method harmonizes using DetailView and FormMixin together. + """ + self.object = self.get_object() + form = self.get_form() + if form.is_valid(): + return self.form_valid(form) + else: + return self.form_invalid(form) + + def form_valid(self, form): + """The form is valid, save the authorizing official.""" + domain = self.get_object() + form.save() + + messages.success( + self.request, "The authorizing official for this domain has been updated." + ) + # superclass has the redirect + return super().form_valid(form) + + class DomainNameserversView(DomainPermission, FormMixin, DetailView): """Domain nameserver editing view.""" From 18fa93a330bef41643fd792a1b261cd2c36cadc7 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Mon, 22 May 2023 12:14:03 -0500 Subject: [PATCH 2/7] Unit tests for page load, correct form info --- src/registrar/tests/test_views.py | 86 +++++++++++++++++-------------- 1 file changed, 48 insertions(+), 38 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index f1dfec73a..976984f35 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -13,6 +13,7 @@ import boto3_mocking # type: ignore from registrar.models import ( DomainApplication, Domain, + DomainInformation, DomainInvitation, Contact, Website, @@ -1029,12 +1030,16 @@ class TestWithDomainPermissions(TestWithUser): def setUp(self): super().setUp() self.domain, _ = Domain.objects.get_or_create(name="igorville.gov") + self.domain_information, _ = DomainInformation.objects.get_or_create( + creator=self.user, domain=self.domain + ) self.role, _ = UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain, role=UserDomainRole.Roles.ADMIN ) def tearDown(self): try: + self.domain_information.delete() self.domain.delete() self.role.delete() except ValueError: # pass if already deleted @@ -1045,50 +1050,37 @@ class TestWithDomainPermissions(TestWithUser): class TestDomainPermissions(TestWithDomainPermissions): def test_not_logged_in(self): """Not logged in gets a redirect to Login.""" - response = self.client.get(reverse("domain", kwargs={"pk": self.domain.id})) - self.assertEqual(response.status_code, 302) - - response = self.client.get( - reverse("domain-users", kwargs={"pk": self.domain.id}) - ) - self.assertEqual(response.status_code, 302) - - response = self.client.get( - reverse("domain-users-add", kwargs={"pk": self.domain.id}) - ) - self.assertEqual(response.status_code, 302) - - response = self.client.get( - reverse("domain-nameservers", kwargs={"pk": self.domain.id}) - ) - self.assertEqual(response.status_code, 302) + for view_name in [ + "domain", + "domain-users", + "domain-users-add", + "domain-nameservers", + "domain-authorizing-official", + ]: + with self.subTest(view_name=view_name): + response = self.client.get( + reverse(view_name, kwargs={"pk": self.domain.id}) + ) + self.assertEqual(response.status_code, 302) def test_no_domain_role(self): """Logged in but no role gets 403 Forbidden.""" self.client.force_login(self.user) self.role.delete() # user no longer has a role on this domain - with less_console_noise(): - response = self.client.get(reverse("domain", kwargs={"pk": self.domain.id})) - self.assertEqual(response.status_code, 403) - - with less_console_noise(): - response = self.client.get( - reverse("domain-users", kwargs={"pk": self.domain.id}) - ) - self.assertEqual(response.status_code, 403) - - with less_console_noise(): - response = self.client.get( - reverse("domain-users-add", kwargs={"pk": self.domain.id}) - ) - self.assertEqual(response.status_code, 403) - - with less_console_noise(): - response = self.client.get( - reverse("domain-nameservers", kwargs={"pk": self.domain.id}) - ) - self.assertEqual(response.status_code, 403) + for view_name in [ + "domain", + "domain-users", + "domain-users-add", + "domain-nameservers", + "domain-authorizing-official", + ]: + with self.subTest(view_name=view_name): + with less_console_noise(): + response = self.client.get( + reverse(view_name, kwargs={"pk": self.domain.id}) + ) + self.assertEqual(response.status_code, 403) class TestDomainDetail(TestWithDomainPermissions, WebTest): @@ -1282,6 +1274,24 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest): # the field. self.assertContains(result, "This field is required", count=2, status_code=200) + def test_domain_authorizing_official(self): + """Can load domain's authorizing official page.""" + page = self.client.get( + reverse("domain-authorizing-official", kwargs={"pk": self.domain.id}) + ) + # once on the sidebar, once in the title + self.assertContains(page, "Authorizing official", count=2) + + def test_domain_authorizing_official_content(self): + """Authorizing official information appears on the page.""" + self.domain_information.authorizing_official = Contact(first_name="Testy") + self.domain_information.authorizing_official.save() + self.domain_information.save() + page = self.app.get( + reverse("domain-authorizing-official", kwargs={"pk": self.domain.id}) + ) + self.assertContains(page, "Testy") + class TestApplicationStatus(TestWithUser, WebTest): def setUp(self): From 8adda8d32753dc430519195de9cd33d7c53b6c26 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Mon, 22 May 2023 12:35:47 -0500 Subject: [PATCH 3/7] Migrations to change verbose names --- ...t_name_alter_contact_last_name_and_more.py | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 src/registrar/migrations/0021_alter_contact_first_name_alter_contact_last_name_and_more.py diff --git a/src/registrar/migrations/0021_alter_contact_first_name_alter_contact_last_name_and_more.py b/src/registrar/migrations/0021_alter_contact_first_name_alter_contact_last_name_and_more.py new file mode 100644 index 000000000..353809639 --- /dev/null +++ b/src/registrar/migrations/0021_alter_contact_first_name_alter_contact_last_name_and_more.py @@ -0,0 +1,44 @@ +# Generated by Django 4.2 on 2023-05-22 17:35 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0020_remove_domaininformation_security_email"), + ] + + operations = [ + migrations.AlterField( + model_name="contact", + name="first_name", + field=models.TextField( + blank=True, + db_index=True, + help_text="First name", + null=True, + verbose_name="first name / given name", + ), + ), + migrations.AlterField( + model_name="contact", + name="last_name", + field=models.TextField( + blank=True, + db_index=True, + help_text="Last name", + null=True, + verbose_name="last name / family name", + ), + ), + migrations.AlterField( + model_name="contact", + name="title", + field=models.TextField( + blank=True, + help_text="Title", + null=True, + verbose_name="title or role in your organization", + ), + ), + ] From 7d42c5e63b99354c69dc4936d5fd2d8e2ec0b0ad Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Mon, 22 May 2023 12:38:03 -0500 Subject: [PATCH 4/7] fix linting errors --- src/registrar/forms/domain.py | 11 +++-------- src/registrar/views/domain.py | 1 - 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index cff8f8edd..e6fbc8fee 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -7,6 +7,7 @@ from phonenumber_field.widgets import RegionalPhoneNumberWidget from ..models import Contact + class DomainAddUserForm(forms.Form): """Form for adding a user to a domain.""" @@ -46,19 +47,13 @@ class ContactForm(forms.ModelForm): # the database fields have blank=True so ModelForm doesn't create # required fields by default. Use this list in __init__ to mark each # of these fields as required - required = [ - "first_name", - "last_name", - "title", - "email", - "phone" - ] + required = ["first_name", "last_name", "title", "email", "phone"] def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # take off maxlength attribute for the phone number field # which interferes with out input_with_errors template tag - self.fields['phone'].widget.attrs.pop('maxlength', None) + self.fields["phone"].widget.attrs.pop("maxlength", None) for field_name in self.required: self.fields[field_name].required = True diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 5cf050319..989361d1e 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -67,7 +67,6 @@ class DomainAuthorizingOfficialView(DomainPermission, FormMixin, DetailView): def form_valid(self, form): """The form is valid, save the authorizing official.""" - domain = self.get_object() form.save() messages.success( From b5706c3829534a352d7aa6e1da1dc4ee20cb8691 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 1 Jun 2023 13:50:58 -0500 Subject: [PATCH 5/7] Fix merge issues --- ...t_name_alter_contact_last_name_and_more.py | 44 ------------------- src/registrar/tests/test_views.py | 2 + 2 files changed, 2 insertions(+), 44 deletions(-) delete mode 100644 src/registrar/migrations/0021_alter_contact_first_name_alter_contact_last_name_and_more.py diff --git a/src/registrar/migrations/0021_alter_contact_first_name_alter_contact_last_name_and_more.py b/src/registrar/migrations/0021_alter_contact_first_name_alter_contact_last_name_and_more.py deleted file mode 100644 index 353809639..000000000 --- a/src/registrar/migrations/0021_alter_contact_first_name_alter_contact_last_name_and_more.py +++ /dev/null @@ -1,44 +0,0 @@ -# Generated by Django 4.2 on 2023-05-22 17:35 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - dependencies = [ - ("registrar", "0020_remove_domaininformation_security_email"), - ] - - operations = [ - migrations.AlterField( - model_name="contact", - name="first_name", - field=models.TextField( - blank=True, - db_index=True, - help_text="First name", - null=True, - verbose_name="first name / given name", - ), - ), - migrations.AlterField( - model_name="contact", - name="last_name", - field=models.TextField( - blank=True, - db_index=True, - help_text="Last name", - null=True, - verbose_name="last name / family name", - ), - ), - migrations.AlterField( - model_name="contact", - name="title", - field=models.TextField( - blank=True, - help_text="Title", - null=True, - verbose_name="title or role in your organization", - ), - ), - ] diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 0ca0aa036..bf1cef2f2 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1312,6 +1312,8 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest): self.domain_information.save() page = self.app.get( reverse("domain-authorizing-official", kwargs={"pk": self.domain.id}) + ) + self.assertContains(page, "Testy") def test_domain_your_contact_information(self): """Can load domain's your contact information page.""" From 4623a5dfb26b3e9256424adf380d91f43b7dd0b0 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 1 Jun 2023 14:25:40 -0500 Subject: [PATCH 6/7] Add email validation by using EmailField in Contact --- .../migrations/0024_alter_contact_email.py | 19 +++++++++++++++++++ src/registrar/models/contact.py | 2 +- src/registrar/tests/test_forms.py | 18 ++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 src/registrar/migrations/0024_alter_contact_email.py diff --git a/src/registrar/migrations/0024_alter_contact_email.py b/src/registrar/migrations/0024_alter_contact_email.py new file mode 100644 index 000000000..f512d5d82 --- /dev/null +++ b/src/registrar/migrations/0024_alter_contact_email.py @@ -0,0 +1,19 @@ +# Generated by Django 4.2.1 on 2023-06-01 19:23 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0023_alter_contact_first_name_alter_contact_last_name_and_more"), + ] + + operations = [ + migrations.AlterField( + model_name="contact", + name="email", + field=models.EmailField( + blank=True, db_index=True, help_text="Email", max_length=254, null=True + ), + ), + ] diff --git a/src/registrar/models/contact.py b/src/registrar/models/contact.py index cbfde7a23..41ed9f2c5 100644 --- a/src/registrar/models/contact.py +++ b/src/registrar/models/contact.py @@ -41,7 +41,7 @@ class Contact(TimeStampedModel): help_text="Title", verbose_name="title or role in your organization", ) - email = models.TextField( + email = models.EmailField( null=True, blank=True, help_text="Email", diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 94f985fab..d682d2cd2 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -15,6 +15,7 @@ from registrar.forms.application_wizard import ( AnythingElseForm, TypeOfWorkForm, ) +from registrar.forms.domain import ContactForm class TestFormValidation(TestCase): @@ -277,3 +278,20 @@ class TestFormValidation(TestCase): for error in form.non_field_errors() ) ) + + +class TestContactForm(TestCase): + + def test_contact_form_email_invalid(self): + form = ContactForm(data={"email": "example.net"}) + self.assertEqual( + form.errors["email"], + ["Enter a valid email address."] + ) + + def test_contact_form_email_invalid2(self): + form = ContactForm(data={"email": "@"}) + self.assertEqual( + form.errors["email"], + ["Enter a valid email address."] + ) From d31a5aa5126cab9ff73acc85c99b72efe26765da Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell Date: Thu, 1 Jun 2023 14:47:19 -0500 Subject: [PATCH 7/7] fix linting errors --- src/registrar/forms/domain.py | 31 ------------------------------- src/registrar/tests/test_forms.py | 11 ++--------- 2 files changed, 2 insertions(+), 40 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 24012461c..e6fbc8fee 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -64,34 +64,3 @@ class DomainSecurityEmailForm(forms.Form): """Form for adding or editing a security email to a domain.""" security_email = forms.EmailField(label="Security email") - - -class ContactForm(forms.ModelForm): - - """Form for updating contacts.""" - - class Meta: - model = Contact - fields = ["first_name", "middle_name", "last_name", "title", "email", "phone"] - widgets = { - "first_name": forms.TextInput, - "middle_name": forms.TextInput, - "last_name": forms.TextInput, - "title": forms.TextInput, - "email": forms.EmailInput, - "phone": RegionalPhoneNumberWidget, - } - - # the database fields have blank=True so ModelForm doesn't create - # required fields by default. Use this list in __init__ to mark each - # of these fields as required - required = ["first_name", "last_name", "title", "email", "phone"] - - def __init__(self, *args, **kwargs): - super().__init__(*args, **kwargs) - # take off maxlength attribute for the phone number field - # which interferes with our input_with_errors template tag - self.fields["phone"].widget.attrs.pop("maxlength", None) - - for field_name in self.required: - self.fields[field_name].required = True diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index d682d2cd2..173362943 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -281,17 +281,10 @@ class TestFormValidation(TestCase): class TestContactForm(TestCase): - def test_contact_form_email_invalid(self): form = ContactForm(data={"email": "example.net"}) - self.assertEqual( - form.errors["email"], - ["Enter a valid email address."] - ) + self.assertEqual(form.errors["email"], ["Enter a valid email address."]) def test_contact_form_email_invalid2(self): form = ContactForm(data={"email": "@"}) - self.assertEqual( - form.errors["email"], - ["Enter a valid email address."] - ) + self.assertEqual(form.errors["email"], ["Enter a valid email address."])