Improves the rendering of forms and refactors some of their logic.

Changes are: * Display invalid input in editable input fields instead of adding it to the end of the error message. This should improve user experience in case of typos etc. where they now can simply edit there previously entered value. * Use a single variable for form data and form errors, instead of using two. Makes the code a bit cleaner.
Dunedan · Aug 4, 2015 · ecea796 · ecea796
1 parent 17ecba6
commit ecea796
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 59 deletions.
diff --git a/binder/templates/bcommon/add_cname_record_form.html b/binder/templates/bcommon/add_cname_record_form.html
@@ -6,56 +6,55 @@
 <form class="form-horizontal" action="{% url "add_cname_result" %}" method="post">{% csrf_token %}
   <legend>Create CNAME record</legend>
 
-  <div class="form-group{% if form_errors.dns_server %} has-error{% endif %}">
+  <div class="form-group{% if form.dns_server.errors %} has-error{% endif %}">
     <label for="dns_server" class="col-sm-3 control-label">DNS Server:</label>
     <div class="col-sm-5 col-md-4">
       <input id="dns_server" name="dns_server" type="text" class="form-control" value="{{dns_server.hostname}}" readonly="readonly" />
     </div>
-    {% if form_errors.dns_server %}
+    {% if form.dns_server.errors %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger">
-        {{ form_errors.dns_server|stringformat:"s"|striptags }}
-        {% if form_data.dns_server %} Previous Value: {{ form_data.dns_server }}{% endif %}
+        {{ form.dns_server.errors|stringformat:"s"|striptags }}
+        {% if form.dns_server.value %} Previous Value: {{ form.dns_server.value }}{% endif %}
       </div>
     </div>
     {% endif %}
   </div>
 
-  <div class="form-group{% if form_errors.originating_record %} has-error{% endif %}">
+  <div class="form-group{% if form.originating_record.errors %} has-error{% endif %}">
     <label for="originating_record" class="col-sm-3 control-label">Originating Record: </label>
     <div class="col-sm-5 col-md-4">
       <input type="text" id="originating_record" name="originating_record" class="form-control" value="{{originating_record}}" readonly="readonly"/>
     </div>
-    {% if form_errors.originating_record %}
+    {% if form.originating_record.errors %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger">
-        {{ form_errors.originating_record|stringformat:"s"|striptags }}
-        {% if form_data.originating_record %} Previous Value: {{ form_data.originating_record }}{% endif %}
+        {{ form.originating_record.errors|stringformat:"s"|striptags }}
+        {% if form.originating_record.value %} Previous Value: {{ form.originating_record.value }}{% endif %}
       </div>
     </div>
     {% endif %}
   </div>
 
-  <div class="form-group{% if form_errors.cname %} has-error{% endif %}">
+  <div class="form-group{% if form.cname.errors %} has-error{% endif %}">
     <label for="cname" class="col-sm-3 control-label">CNAME: </label>
     <div class="col-sm-5 col-md-4">
       <div class="input-group">
-        <input id="cname" name="cname" type="text" class="form-control" />
+        <input id="cname" name="cname" type="text" class="form-control"{% if form.cname.value %} value="{{ form.cname.value }}"{% endif %}/>
         <span class="input-group-addon">.{{zone_name}}</span>
         <input type="hidden" name="zone_name" value="{{zone_name}}"/>
       </div>
     </div>
-    {% if form_errors.cname  %}
+    {% if form.cname.errors  %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger">
-        CNAME: {{ form_errors.cname|stringformat:"s"|striptags }}
-        {% if form_data.cnamr %} Previous Value: {{ form_data.cname }}{% endif %}
+        {{ form.cname.errors|stringformat:"s"|striptags }}
       </div>
     </div>
     {% endif %}
   </div>
 
-  <div class="form-group{% if form_errors.ttl %} has-error{% endif %}">
+  <div class="form-group{% if form.ttl.errors %} has-error{% endif %}">
     <label for="ttl" class="col-sm-3 control-label">TTL: </label>
     <div class="col-sm-5 col-md-4">
       <select id="ttl" name="ttl" class="form-control">
@@ -66,17 +65,17 @@
         {% endfor %}
       </select>
     </div>
-    {% if form_errors.ttl %}
+    {% if form.ttl.errors %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger">
-        {{ form_errors.ttl|stringformat:"s"|striptags }}
-        {% if form_data.ttl %} Previous Value: {{ form_data.ttl }}{% endif %}
+        {{ form.ttl.errors|stringformat:"s"|striptags }}
+        {% if form.ttl.value %} Previous choice: {{ form.ttl.value }}{% endif %}
       </div>
     </div>
     {% endif %}
   </div>
 
-  <div class="form-group{% if form_errors.key_name %} has-error{% endif %}">
+  <div class="form-group{% if form.key_name.errors %} has-error{% endif %}">
     <label for="key_name" class="col-sm-3 control-label">TSIG Key:</label>
     <div class="col-sm-5 col-md-4">
       <select id="key_name" name="key_name" class="form-control">
@@ -87,11 +86,11 @@
         {% endfor %}
       </select>
     </div>
-    {% if form_errors.key_name %}
+    {% if form.key_name.errors %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger">
-        {{ form_errors.key_name|stringformat:"s"|striptags }}
-        {% if form_data.key_name %} Previous Value: {{ form_data.key_name }}{% endif %}
+        {{ form.key_name.errors|stringformat:"s"|striptags }}
+        {% if form.key_name.value %} Previous choice: {{ form.key_name.value }}{% endif %}
       </div>
     </div>
     {% endif %}

diff --git a/binder/templates/bcommon/add_record_form.html b/binder/templates/bcommon/add_record_form.html
@@ -6,41 +6,40 @@
 <form class="form-horizontal" action="{% url "add_record_result" %}" method="POST">{% csrf_token %}
   <legend>Create Record</legend>
 
-  <div class="form-group{% if form_errors.dns_server %} has-error{% endif %}">
+  <div class="form-group{% if form.dns_server.errors %} has-error{% endif %}">
     <label for="dns_server" class="col-sm-3 control-label">DNS Server:</label>
     <div class="col-sm-5 col-md-4">
       <input id="dns_server" name="dns_server" type="text" class="form-control" value="{{dns_server.hostname}}" readonly="readonly" />
     </div>
-    {% if form_errors.dns_server %}
+    {% if form.dns_server.errors %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger">
-        {{ form_errors.dns_server|stringformat:"s"|striptags }}
-        {% if form_data.dns_server %}Previous Value: {{ form_data.dns_server }}{% endif %}
+        {{ form.dns_server.errors|stringformat:"s"|striptags }}
+        {% if form.dns_server.value %}Previous Value: {{ form.dns_server.value }}{% endif %}
       </div>
     </div>
     {% endif %}
   </div>
 
-  <div class="form-group{% if form_errors.record_name %} has-error{% endif %}">
+  <div class="form-group{% if form.record_name.errors %} has-error{% endif %}">
     <label for="record_name" class="col-sm-3 control-label">Record Name:</label>
     <div class="col-sm-5 col-md-4">
       <div class="input-group">
-        <input id="record_name" name="record_name" type="text" class="form-control" />
+        <input id="record_name" name="record_name" type="text" class="form-control"{% if form.record_name.value %} value="{{ form.record_name.value }}"{% endif %} />
         <span class="input-group-addon">.{{zone_name}}</span>
         <input type="hidden" name="zone_name" value="{{zone_name}}" />
       </div>
     </div>
-    {% if form_errors.record_name %}
+    {% if form.record_name.errors %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger form-control-static">
-        {{ form_errors.record_name|stringformat:"s"|striptags }}
-        {% if form_data.record_name %} Previous Value: {{ form_data.record_name }}{% endif %}
+        {{ form.record_name.errors|stringformat:"s"|striptags }}
       </div>
     </div>
     {% endif %}
   </div>
 
-  <div class="form-group{% if form_errors.record_type %} has-error{% endif %}">
+  <div class="form-group{% if form.record_type.errors %} has-error{% endif %}">
     <label for="record_type" class="col-sm-3 control-label">Record Type:</label>
     <div class="col-sm-5 col-md-4">
       <select id="record_type" name="record_type" class="form-control">
@@ -53,32 +52,31 @@
       {% endif %}
       </select>
     </div>
-    {% if form_errors.record_type %}
+    {% if form.record_type.errors %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger">
-        {{ form_errors.record_type|stringformat:"s"|striptags }}
-        {% if form_data.record_type %} Previous Value: {{ form_data.record_type }}{% endif %}
+        {{ form.record_type.errors|stringformat:"s"|striptags }}
+        {% if form.record_type.value %} Previous Value: {{ form.record_type.value }}{% endif %}
       </div>
     </div>
     {% endif %}
   </div>
 
-  <div class="form-group{% if form_errors.record_data %} has-error{% endif %}">
+  <div class="form-group{% if form.record_data.errors %} has-error{% endif %}">
     <label for="record_data" class="col-sm-3 control-label">Record Data:</label>
     <div class="col-sm-5 col-md-4">
-      <input id="record_data" name="record_data" type="text" class="form-control" />
+      <input id="record_data" name="record_data" type="text" class="form-control"{% if form.record_data.value %} value="{{ form.record_data.value }}"{% endif %} />
     </div>
-    {% if form_errors.record_data %}
+    {% if form.record_data.errors %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger">
-        {{ form_errors.record_data|stringformat:"s"|striptags }}
-        {% if form_data.record_data %} Previous Value: {{ form_data.record_data }}{% endif %}
+        {{ form.record_data.errors|stringformat:"s"|striptags }}
       </div>
     </div>
     {% endif %}
   </div>
 
-  <div class="form-group{% if form_errors.ttl %} has-error{% endif %}">
+  <div class="form-group{% if form.ttl.errors %} has-error{% endif %}">
     <label for="ttl" class="col-sm-3 control-label">TTL: </label>
     <div class="col-sm-5 col-md-4">
       <select id="ttl" name="ttl" class="form-control">
@@ -89,11 +87,11 @@
         {% endfor %}
       </select>
     </div>
-    {% if form_errors.ttl %}
+    {% if form.ttl.errors %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger">
-        {{ form_errors.ttl|stringformat:"s"|striptags }}
-        {% if form_data.ttl %} Previous Value: {{ form_data.ttl }}{% endif %}
+        {{ form.ttl.errors|stringformat:"s"|striptags }}
+        {% if form.ttl.value %} Previous choice: {{ form.ttl.value }}{% endif %}
       </div>
     </div>
     {% endif %}
@@ -108,7 +106,7 @@
   </div>
   {% endif %}
 
-  <div class="form-group{% if form_errors.key_name %} has-error{% endif %}">
+  <div class="form-group{% if form.key_name.errors %} has-error{% endif %}">
     <label for="key_name" class="col-sm-3 control-label">TSIG Key:</label>
     <div class="col-sm-5 col-md-4">
       <select id="key_name" name="key_name" class="form-control">
@@ -119,11 +117,11 @@
         {% endfor %}
       </select>
     </div>
-    {% if form_errors.key_name %}
+    {% if form.key_name.errors %}
     <div class="col-sm-4 col-md-5">
       <div class="alert alert-danger">
-        {{ form_errors.key_name|stringformat:"s"|striptags }}
-        {% if form_data.key_name %} Previous Value: {{ form_data.key_name }}{% endif %}
+        {{ form.key_name.errors|stringformat:"s"|striptags }}
+        {% if form.key_name.value %} Previous choice: {{ form.key_name.value }}{% endif %}
       </div>
     </div>
     {% endif %}

diff --git a/binder/views.py b/binder/views.py
@@ -120,8 +120,7 @@ def view_add_record_result(request):
                    "tsig_keys": models.Key.objects.all(),
                    "ttl_choices": settings.TTL_CHOICES,
                    "record_type_choices": settings.RECORD_TYPE_CHOICES,
-                   "form_errors": form.errors,
-                   "form_data": request.POST})
+                   "form": form})
 
 
 def view_add_cname_record(request, dns_server, zone_name, record_name):
@@ -145,15 +144,15 @@ def view_add_cname_result(request):
     add_cname_response = ""
     form = forms.FormAddCnameRecord(request.POST)
     if form.is_valid():
-        cd = form.cleaned_data
+        form_cleaned = form.cleaned_data
         try:
             add_cname_response = helpers.add_cname_record(
-                cd["dns_server"],
-                cd["zone_name"],
-                cd["cname"],
-                str(cd["originating_record"]),
-                cd["ttl"],
-                cd["key_name"])
+                form_cleaned["dns_server"],
+                form_cleaned["zone_name"],
+                form_cleaned["cname"],
+                str(form_cleaned["originating_record"]),
+                form_cleaned["ttl"],
+                form_cleaned["key_name"])
         except exceptions.RecordException, err:
             errors = err
 
@@ -168,10 +167,9 @@ def view_add_cname_result(request):
                    "zone_name": request.POST["zone_name"],
                    "record_name": request.POST["cname"],
                    "originating_record": request.POST["originating_record"],
-                   "form_data": request.POST,
-                   "form_errors": form.errors,
                    "ttl_choices": settings.TTL_CHOICES,
-                   "tsig_keys": models.Key.objects.all()})
+                   "tsig_keys": models.Key.objects.all(),
+                   "form": form})
 
 
 def view_delete_record(request):