Address feedback - use form pattern and update error messaging

docs/developer/registry-access.md

@@ -130,23 +130,4 @@ domain = Domain.objects.get_or_create(name="<that-domain-here>")
 10. Add yourself as domain manager
 11. Go to the Registrar page and you should now see the expiring domain
 
-If you want to be in the org model mode, turn the `organization_feature` waffle flag on, and add that domain via Django Admin to a portfolio to be able to view it. 
+If you want to be in the org model mode, turn the `organization_feature` waffle flag on, and add that domain via Django Admin to a portfolio to be able to view it.
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-

src/registrar/forms/__init__.py

@@ -10,6 +10,7 @@ from .domain import (
     DomainDsdataFormset,
     DomainDsdataForm,
     DomainSuborganizationForm,
+    DomainRenewalForm,
 )
 from .portfolio import (
     PortfolioOrgAddressForm,

src/registrar/forms/domain.py

@@ -661,3 +661,15 @@ DomainDsdataFormset = formset_factory(
     extra=0,
     can_delete=True,
 )
+
+
+class DomainRenewalForm(forms.Form):
+    """Form making sure domain renewal ack is checked"""
+
+    is_policy_acknowledged = forms.BooleanField(
+        required=True,
+        label="I have read and agree to the requirements for operating a .gov domain.",
+        error_messages={
+            "required": "Check the box if you read and agree to the requirements for operating a .gov domain."
+        },
+    )

src/registrar/models/domain.py

@@ -337,13 +337,14 @@ class Domain(TimeStampedModel, DomainHelper):
             self._cache["ex_date"] = registry.send(request, cleaned=True).res_data[0].ex_date
             self.expiration_date = self._cache["ex_date"]
             self.save()
+
         except RegistryError as err:
             # if registry error occurs, log the error, and raise it as well
-            logger.error(f"registry error renewing domain: {err}")
+            logger.error(f"Registry error renewing domain '{self.name}': {err}")
             raise (err)
         except Exception as e:
             # exception raised during the save to registrar
-            logger.error(f"error updating expiration date in registrar: {e}")
+            logger.error(f"Error updating expiration date for domain '{self.name}' in registrar: {e}")
             raise (e)
 
     @Cache

src/registrar/templates/domain_renewal.html

@@ -4,6 +4,18 @@
 
 {% block domain_content %}
   {% block breadcrumb %}
+
+  <!-- Banner for if_policy_acknowledged -->
+  {% if form.is_policy_acknowledged.errors %}
+    <div class="usa-alert usa-alert--error usa-alert--slim margin-bottom-2">
+      <div class="usa-alert__body">
+        {% for error in form.is_policy_acknowledged.errors %}
+          <p class="usa-alert__text">{{ error }}</p>
+        {% endfor %}
+      </div>
+    </div>
+  {% endif %}
+
   {% if portfolio %}
   <!-- Navigation breadcrumbs -->
   <nav class="usa-breadcrumb padding-top-0" aria-label="Domain breadcrumb">
@@ -28,13 +40,13 @@
     <h2 class="text-bold text-primary-dark domain-name-wrap">Confirm the following information for accuracy</h2>
     <p>Review these details below. We <a href="https://get.gov/domains/requirements/#what-.gov-domain-registrants-must-do" class="usa-link">
       require</a> that you maintain accurate information for the domain.
-      The details you provide will only be used to support eh administration of .gov and won't be made public.
+      The details you provide will only be used to support the administration of .gov and won't be made public.
     </p>
     <p>If you would like to retire your domain instead, please <a href="https://get.gov/contact/" class="usa-link">
       contact us</a>. </p>
     <p><em>Required fields are marked with an asterisk (<abbr class="usa-hint usa-hint--required" title="required">*</abbr>).</em>
     </p>
-    </div>
+  </div>
 
     {% url 'user-profile' as url %}
     {% include "includes/summary_item.html" with title='Your Contact Information' value=user edit_link=url editable=is_editable  contact='true' %}
@@ -66,62 +78,50 @@
       {% include "includes/summary_item.html" with title='Domain managers' list=True users=True value=domain.permissions.all edit_link=url editable=is_editable %}
     {% endif %}
 
-        
+
     <div class="border-top-1px border-primary-dark padding-top-1 margin-top-3 margin-bottom-2">
 
       <fieldset class="usa-fieldset">
         <legend>
-          <h3 class="summary-item__title 
+        <h3 class="summary-item__title 
           font-sans-md 
           text-primary-dark 
           text-semibold 
           margin-top-0 
           margin-bottom-05 
           padding-right-1">
-          Acknowledgement of .gov domain requirements
+        Acknowledgement of .gov domain requirements </h3>          
-          </h3>          
         </legend>
-      
+    
-        {% if messages %}
+        <form method="post" action="{% url 'domain-renewal' pk=domain.id %}"> 
-          <ul class="messages"></ul>
+          {% csrf_token %}
-          {% for message in messages %}
+          <div class="usa-checkbox">
-              <p class="usa-error-message">{{ message }}</p>
+          {% if form.is_policy_acknowledged.errors %}
-          {% endfor %}
+            <p class="usa-error-message">{{ form.is_policy_acknowledged.errors|join:" " }}</p>
-        </ul>
+          {% endif %}
-        {% endif %}
+          <input 
-      
+            class="usa-checkbox__input" 
-        
+            id="renewal-checkbox" 
-  <form method="post" action="{% url 'domain-renewal' pk=domain.id %}"> 
+            type="checkbox" 
-    {% csrf_token %}
+            name="{{ form.is_policy_acknowledged.name }}">
-    <label>
+            <label class="usa-checkbox__label" for="renewal-checkbox">
-      <div class="usa-checkbox">
+              I read and agree to the 
-        <input 
+              <a href="https://get.gov/domains/requirements/" class="usa-link">
-          class="usa-checkbox__input" 
-          id="renewal-checkbox" 
-          type="checkbox" 
-          name="is_policy_acknowledged"
-          value="on"
-          {% if request.POST.is_policy_acknowledged %} checked {% endif %}
-          >
-          <label class="usa-checkbox__label" for="renewal-checkbox">
-            I read and agree to the 
-            <a href="https://get.gov/domains/requirements/" class="usa-link">
               requirements for operating a .gov domain
-            </a>.
+              </a>.
-            <abbr class="usa-hint usa-hint--required" title="required">*</abbr>
+              <abbr class="usa-hint usa-hint--required" title="required">*</abbr>
-          </label>
+            </label>
-      </div>
+          </div>
-    </fieldset>
+    
-    </div>    
+          <button
-   <button
+            type="submit"
-    type="submit"
+            name="submit_button"
-    name="submit_button"
+            value="next"
-    value="next"
+            class="usa-button margin-top-3"
-    class="usa-button"
+          > Submit
-    method="post"
+          </button>
-    >Submit</button>
+        </form>
-
+      </fieldset>
-  </form>
+    </div>  <!-- End of the acknowledgement section div -->
-
   </div>
 {% endblock %}  {# domain_content #}

src/registrar/tests/test_views_domain.py

@@ -454,7 +454,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview):
 
         self.user.save()
 
-    def todays_expiration_date(self):
+    def expiration_date_one_year_out(self):
         todays_date = datetime.today()
         new_expiration_date = todays_date.replace(year=todays_date.year + 1)
         return new_expiration_date
@@ -466,7 +466,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview):
         return True
 
     def custom_renew_domain(self):
-        self.domain_with_ip.expiration_date = self.todays_expiration_date()
+        self.domain_with_ip.expiration_date = self.expiration_date_one_year_out()
         self.domain_with_ip.save()
 
     @override_flag("domain_renewal", active=True)
@@ -659,7 +659,7 @@ class TestDomainDetailDomainRenewal(TestDomainOverview):
             self.assertRedirects(response, reverse("domain", kwargs={"pk": self.domain_with_ip.id}))
 
             # Check for the updated expiration
-            formatted_new_expiration_date = self.todays_expiration_date().strftime("%b. %-d, %Y")
+            formatted_new_expiration_date = self.expiration_date_one_year_out().strftime("%b. %-d, %Y")
             redirect_response = self.client.get(reverse("domain", kwargs={"pk": self.domain_with_ip.id}), follow=True)
             self.assertContains(redirect_response, formatted_new_expiration_date)
 

src/registrar/views/domain.py

@@ -12,11 +12,11 @@ from django.contrib import messages
 from django.contrib.messages.views import SuccessMessageMixin
 from django.db import IntegrityError
 from django.http import HttpResponseRedirect
-from django.shortcuts import redirect, render
+from django.shortcuts import redirect, render, get_object_or_404
 from django.urls import reverse
 from django.views.generic.edit import FormMixin
 from django.conf import settings
-from registrar.forms.domain import DomainSuborganizationForm
+from registrar.forms.domain import DomainSuborganizationForm, DomainRenewalForm
 from registrar.models import (
     Domain,
     DomainRequest,
@@ -364,30 +364,32 @@ class DomainRenewalView(DomainBaseView):
         self._update_session_with_domain()
 
     def post(self, request, pk):
-        domain = Domain.objects.filter(id=pk).first()
+        domain = get_object_or_404(Domain, id=pk)
 
-        # Check if the checkbox is checked
+        form = DomainRenewalForm(request.POST)
-        is_policy_acknowledged = request.POST.get("is_policy_acknowledged", None)
-        if is_policy_acknowledged != "on":
-            messages.error(
-                request, "Check the box if you read and agree to the requirements for operating a .gov domain."
-            )
-            return render(
-                request,
-                "domain_renewal.html",
-                {
-                    "domain": domain,
-                    "form": request.POST,
-                },
-            )
 
-        if "submit_button" in request.POST:
+        if form.is_valid():
-            try:
+            # check for key in the post request data
-                domain.renew_domain()
+            if "submit_button" in request.POST:
-                messages.success(request, "This domain has been renewed for one year.")
+                try:
-            except Exception as e:
+                    domain.renew_domain()
-                messages.error(request, "This domain has not been renewed for one year, error was %s" % e)
+                    messages.success(request, "This domain has been renewed for one year.")
-        return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk}))
+                except Exception as e:
+                    messages.error(
+                        request,
+                        "This domain has not been renewed for one year, please email help@get.gov if this problem persists.",
+                    )
+            return HttpResponseRedirect(reverse("domain", kwargs={"pk": pk}))
+
+        # if not valid, render the template with error messages
+        return render(
+            request,
+            "domain_renewal.html",
+            {
+                "domain": domain,
+                "form": form,
+            },
+        )
 
 
 class DomainOrgNameAddressView(DomainFormBaseView):