From ae2527f0ec3528eea2ff09aa6106a8a19daded31 Mon Sep 17 00:00:00 2001
From: Neil Martinsen-Burrell <neil.martinsen-burrell@gsa.gov>
Date: Fri, 5 May 2023 11:16:31 -0500
Subject: [PATCH] Tweaks for better PR

---
 src/registrar/assets/js/get-gov.js            |  7 +++++-
 src/registrar/models/domain.py                |  2 +-
 .../templates/domain_nameservers.html         |  9 +++++--
 src/registrar/tests/test_views.py             | 24 +++++++++++++++++--
 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js
index 4c9f8f25f..48996badb 100644
--- a/src/registrar/assets/js/get-gov.js
+++ b/src/registrar/assets/js/get-gov.js
@@ -230,6 +230,12 @@ function handleValidationClick(e) {
 })();
 
 
+/**
+ * An IIFE that attaches a click handler for our dynamic nameservers form
+ *
+ * Only does something on a single page, but it should be fast enough to run
+ * it everywhere.
+ */
 (function prepareForms() {
   let serverForm = document.querySelectorAll(".server-form")
   let container = document.querySelector("#form-container")
@@ -257,4 +263,3 @@ function handleValidationClick(e) {
       totalForms.setAttribute('value', `${formNum+1}`)
   }
 })();
-
diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index 1302be808..8e1aa3ace 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -218,7 +218,7 @@ class Domain(TimeStampedModel):
     def __str__(self) -> str:
         return self.name
 
-    def nameservers(self):
+    def nameservers(self) -> List[str]:
         """A list of the nameservers for this domain.
 
         TODO: call EPP to get this info instead of returning fake data.
diff --git a/src/registrar/templates/domain_nameservers.html b/src/registrar/templates/domain_nameservers.html
index 141e9225f..5f5662236 100644
--- a/src/registrar/templates/domain_nameservers.html
+++ b/src/registrar/templates/domain_nameservers.html
@@ -4,6 +4,11 @@
 {% block title %}Domain name servers | {{ domain.name }} | {% endblock %}
 
 {% block domain_content %}
+  {# this is right after the messages block in the parent template #}
+  {% for form in formset %}
+    {% include "includes/form_errors.html" with form=form %}
+  {% endfor %}
+
   <h1>Domain name servers</h1>
 
   <p>Before your domain can be used we'll need information about your domain
@@ -19,7 +24,7 @@
 
     {% for form in formset %}
     <div class="server-form">
-    {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".nameserver.com" %}
+    {% with sublabel_text="Example: ns"|concat:forloop.counter|concat:".example.com" %}
       {% if forloop.counter <= 2 %}
         {% with attr_required=True %}
           {% input_with_errors form.server %}
@@ -31,7 +36,7 @@
     </div>
     {% endfor %}
 
-    <button type="submit" name="submit_button" value="add_another" class="usa-button usa-button--unstyled display-block" id="add-form">
+    <button type="submit" name="submit_button" class="usa-button usa-button--unstyled display-block" id="add-form">
       <svg class="usa-icon" aria-hidden="true" focusable="false" role="img" width="24" height="24">
         <use xlink:href="{%static 'img/sprite.svg'%}#add_circle"></use>
       </svg><span class="margin-left-05">Add another name server</span>
diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py
index 2244f33c7..66af9cb01 100644
--- a/src/registrar/tests/test_views.py
+++ b/src/registrar/tests/test_views.py
@@ -1234,14 +1234,14 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest):
         self.assertContains(home_page, self.domain.name)
 
     def test_domain_nameservers(self):
-        """Can load domain's namerservers page."""
+        """Can load domain's nameservers page."""
         page = self.client.get(
             reverse("domain-nameservers", kwargs={"pk": self.domain.id})
         )
         self.assertContains(page, "Domain name servers")
 
     def test_domain_nameservers_form(self):
-        """Can change domain's namerservers.
+        """Can change domain's nameservers.
 
         Uses self.app WebTest because we need to interact with forms.
         """
@@ -1262,6 +1262,26 @@ class TestDomainDetail(TestWithDomainPermissions, WebTest):
         page = result.follow()
         self.assertContains(page, "The name servers for this domain have been updated")
 
+    def test_domain_nameservers_form_invalid(self):
+        """Can change domain's nameservers.
+
+        Uses self.app WebTest because we need to interact with forms.
+        """
+        nameservers_page = self.app.get(
+            reverse("domain-nameservers", kwargs={"pk": self.domain.id})
+        )
+        session_id = self.app.cookies[settings.SESSION_COOKIE_NAME]
+        self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id)
+        # first two nameservers are required, so if we empty one out we should
+        # get a form error
+        nameservers_page.form["form-0-server"] = ""
+        with less_console_noise():  # swallow logged warning message
+            result = nameservers_page.form.submit()
+        # form submission was a post with an error, response should be a 200
+        # error text appears twice, once at the top of the page, once around
+        # the field.
+        self.assertContains(result, "This field is required", count=2, status_code=200)
+
 
 class TestApplicationStatus(TestWithUser, WebTest):
     def setUp(self):