Add "abbreviation" field to the publisher form. I noticed problems wi…

…th extra data being losts in the form when you submit and validation errors occur; so I added in tests to cover this, and fixed the edit_form.html. This also requires the corresponding fix in CKAN core in ckan/logic/action/get.py.

ckanext/dgu/forms/publisher_form.py

@@ -117,6 +117,7 @@ def form_to_db_schema(self, group_type=None):
             'contact-email': [ignore_missing, unicode, convert_to_extras],
             'contact-phone': [ignore_missing, unicode, convert_to_extras],
             'category': [validate_publisher_category, convert_to_extras],
+            'abbreviation': [ignore_missing, unicode, convert_to_extras],
         }
         schema.update( group_form_schema() )
         return schema
@@ -131,6 +132,7 @@ def db_to_form_schema(data, package_type=None):
             'contact-email': [convert_from_extras, ignore_missing, unicode],
             'contact-phone': [convert_from_extras, ignore_missing, unicode],
             'category': [convert_from_extras],
+            'abbreviation': [convert_from_extras, ignore_missing, unicode],
         }
         schema.update( default_group_schema() )
         return schema

ckanext/dgu/plugin.py

@@ -171,7 +171,7 @@ class PublisherPlugin(SingletonPlugin):
     def before_commit(self, session):
         """
         Before we commit a session we will check to see if any of the new
-        items are users so we
+        items are users so we can notify them to apply for publisher access.
         """
         from pylons.i18n import _
         from ckan.model.group import User

ckanext/dgu/tests/functional/test_publisher_forms.py

@@ -71,6 +71,7 @@ def test_1_edit_publisher(self):
         assert_equal(form['foi-name'].value, '')
         assert_equal(form['foi-email'].value, '')
         assert_equal(form['category'].value, 'grouping')
+        assert_equal(form['abbreviation'].value, 'NHS')
 
         # Make edit
         publisher_name = 'new-name'
@@ -83,6 +84,7 @@ def test_1_edit_publisher(self):
         form['foi-email'] = 'foi-comms@nhs.gov.uk'
         form['foi-phone'] = '0845 4567890'
         form['category'] = 'alb'
+        form['abbreviation'] = 'nhs'
         res = form.submit('save', status=302, extra_environ={'REMOTE_USER': 'nhsadmin'})
         assert_equal(res.header_dict['Location'], 'http://localhost/publisher/new-name')
 
@@ -96,12 +98,85 @@ def test_1_edit_publisher(self):
         assert_equal(publisher.extras['foi-email'], 'foi-comms@nhs.gov.uk')
         assert_equal(publisher.extras['foi-phone'], '0845 4567890')
         assert_equal(publisher.extras['category'], 'alb')
+        assert_equal(publisher.extras['abbreviation'], 'nhs')
 
         # restore name for other tests
         model.repo.new_revision()
         publisher.name = 'national-health-service'
         model.repo.commit_and_remove()
 
+    def test_2_new_validation_error(self):
+        # Load form
+        offset = url_for('/publisher/new')
+        res = self.app.get(offset, status=200, extra_environ={'REMOTE_USER': 'sysadmin'})
+        assert 'Add A Publisher' in res, res
+        form = res.forms['publisher-edit']
+
+        # Fill in form
+        form['title'] = 'New publisher'
+        form['name'] = '' # cause validation error
+        form['description'] = 'New description'
+        form['contact-name'] = 'Head of Comms'
+        form['contact-email'] = 'comms@nhs.gov.uk'
+        form['contact-phone'] = '01234 4567890'
+        form['foi-name'] = 'Head of FOI Comms'
+        form['foi-email'] = 'foi-comms@nhs.gov.uk'
+        form['foi-phone'] = '0845 4567890'
+        form['category'] = 'grouping'
+        form['abbreviation'] = 'tn2'
+        res = form.submit('save', status=200, extra_environ={'REMOTE_USER': 'sysadmin'})
+        assert 'Errors in form' in res.body
+
+        # Check redisplayed form
+        form = res.forms['publisher-edit']
+        assert_equal(form['title'].value, 'New publisher')
+        assert_equal(form['description'].value, 'New description')
+        assert_equal(form['contact-name'].value, 'Head of Comms')
+        assert_equal(form['contact-email'].value, 'comms@nhs.gov.uk')
+        assert_equal(form['contact-phone'].value, '01234 4567890')
+        assert_equal(form['foi-name'].value, 'Head of FOI Comms')
+        assert_equal(form['foi-email'].value, 'foi-comms@nhs.gov.uk')
+        assert_equal(form['foi-phone'].value, '0845 4567890')
+        assert_equal(form['category'].value, 'grouping')
+        assert_equal(form['abbreviation'].value, 'tn2')
+
+    def test_2_edit_validation_error(self):
+        # Load form
+        publisher_name = 'national-health-service'
+        group = model.Group.by_name(publisher_name)
+        offset = url_for('/publisher/edit/%s' % publisher_name)
+        res = self.app.get(offset, status=200, extra_environ={'REMOTE_USER': 'nhsadmin'})
+        assert 'Edit: %s' % group.title in res, res
+        form = res.forms['publisher-edit']
+
+        # Fill in form
+        # TODO form['title'] = 'Edit publisher'
+        form['name'] = '' # cause validation error
+        form['description'] = 'New description'
+        form['contact-name'] = 'Head of Comms'
+        form['contact-email'] = 'comms@nhs.gov.uk'
+        form['contact-phone'] = '01234 4567890'
+        form['foi-name'] = 'Head of FOI Comms'
+        form['foi-email'] = 'foi-comms@nhs.gov.uk'
+        form['foi-phone'] = '0845 4567890'
+        form['category'] = 'grouping'
+        form['abbreviation'] = 'tn2'
+        res = form.submit('save', status=200, extra_environ={'REMOTE_USER': 'sysadmin'})
+        assert 'Errors in form' in res.body
+
+        # Check redisplayed form
+        form = res.forms['publisher-edit']
+        # TODO assert_equal(form['title'].value, 'New publisher')
+        assert_equal(form['description'].value, 'New description')
+        assert_equal(form['contact-name'].value, 'Head of Comms')
+        assert_equal(form['contact-email'].value, 'comms@nhs.gov.uk')
+        assert_equal(form['contact-phone'].value, '01234 4567890')
+        assert_equal(form['foi-name'].value, 'Head of FOI Comms')
+        assert_equal(form['foi-email'].value, 'foi-comms@nhs.gov.uk')
+        assert_equal(form['foi-phone'].value, '0845 4567890')
+        assert_equal(form['category'].value, 'grouping')
+        assert_equal(form['abbreviation'].value, 'tn2')
+
     def test_2_edit_publisher_does_not_affect_others(self):
         publisher_name = 'national-health-service'
         def check_related_publisher_properties():

ckanext/dgu/testtools/create_test_data.py

@@ -35,7 +35,8 @@ class DguCreateTestData(CreateTestData):
          'title': 'National Health Service',
          'contact-email': 'contact@nhs.gov.uk',
          'parent': 'dept-health',
-         'category': 'grouping'},
+         'category': 'grouping',
+         'abbreviation': 'NHS'},
         {'name': 'barnsley-primary-care-trust',
          'title': 'Barnsley Primary Care Trust',
          'contact-email': 'contact@barnsley.nhs.gov.uk',

ckanext/dgu/theme/public/css/dgu.css

@@ -1146,7 +1146,7 @@ div#form-tabs a.disabled:hover {
   background: transparent url(../images/icons/error.png) right center no-repeat;
 }
 
-p.field_error {
+p.field_error, span.field_error {
   background: transparent url(../images/icons/error.png) left center no-repeat;
   padding-left: 20px;
   color: #FF6666;

ckanext/dgu/theme/templates/publisher/edit_form.html

@@ -22,8 +22,15 @@ <h2>Errors in form</h2>
 <input type="hidden" id="approval_status" name="approval_status" value="pending" />
 
 <?python
-publisher_extras = dict([(extra_dict['key'], extra_dict['value']) \
-                         for extra_dict in data.get('extras', {})])
+# Note when you get a fresh form the extras are in data['extras']. But
+# on validation error, the submitted values appear in data[key] with the original
+# values in data['extras']. Therefore populate the form with the data[key] values
+# in preference, and fall back on the data['extra'] values.
+original_extra_fields = dict([(extra_dict['key'], extra_dict['value']) \
+                              for extra_dict in data.get('extras', {})])
+for key, value in original_extra_fields.items():
+    if key not in data:
+        data[key] = value
 ?>
 
 <p>Details about the organisation that publishes data on data.gov.uk</p>
@@ -41,7 +48,7 @@ <h2>Errors in form</h2>
   </div>
 
   <div class="control-group">
-    <label class="control-label" for="title">Url</label>
+    <label class="control-label" for="url">Url</label>
     <div class="controls name-field">
       <div class="input-prepend">
         <span class="add-on">${h.url_for('publisher_index')+'/'}</span><input maxlength="100" name="name" type="text" class="js-url-input" value="${data.get('name', '')}" />
@@ -53,7 +60,15 @@ <h2>Errors in form</h2>
   </div>
 
   <div class="control-group">
-    <label class="control-label" for="title">Publisher Description</label>
+    <label class="control-label" for="name">Name Abbreviation (optional)</label>
+    <div class="controls">
+      <input id="abbreviation" name="abbreviation" type="text" value="${data.get('abbreviation', '')}"/>
+      <span class="field_error" py:if="errors.get('abbreviation', '')">${errors.get('name', '')}</span>
+    </div>
+  </div>
+
+  <div class="control-group">
+    <label class="control-label" for="description">Publisher Description</label>
     <div class="controls description-field">
       <textarea style="width: 350px;" rows="3" name="description" id="notes" placeholder="${_('Start with a summary sentence ...')}">${data.get('description','')}</textarea>
     </div>
@@ -96,14 +111,15 @@ <h2>Errors in form</h2>
   <div class="control-group">
     <label class="control-label" for="state">Category</label>
     <div class="controls">
-      <select id="category" name="category" value="${publisher_extras.get('contact-name', '')}">
+      <select id="category" name="category" value="${data.get('contact-name', '')}">
         <option value=""></option>
         <py:for each="cat_name, cat_desc in c.categories">
-          <option value="${cat_name}" py:attrs="{'selected': 'selected' if publisher_extras.get('category', '') == cat_name else None}" >
+          <option value="${cat_name}" py:attrs="{'selected': 'selected' if data.get('category', '') == cat_name else None}" >
             ${cat_desc}
           </option>
         </py:for>
       </select>
+      <span class="field_error" py:if="errors.get('category', '')">${errors.get('category', '')}</span>
     </div>
   </div>
 </fieldset>
@@ -115,20 +131,20 @@ <h3>Contact Details</h3>
   <div class="control-group">
     <label class="control-label" for="contact-name">Name</label>
     <div class="controls">
-      <input id="contact-name" maxlength="64" name="contact-name" type="text" class="" value="${publisher_extras.get('contact-name', '')}" />
+      <input id="contact-name" maxlength="64" name="contact-name" type="text" class="" value="${data.get('contact-name', '')}" />
       <p class="help-block">e.g. Barnsley Council Data Enquiries Helpline</p>
     </div>
   </div>
   <div class="control-group">
     <label class="control-label" for="contact-email">Email</label>
     <div class="controls">
-      <input id="contact-email" maxlength="100" name="contact-email" type="text" class="" value="${publisher_extras.get('contact-email', '')}" />
+      <input id="contact-email" maxlength="100" name="contact-email" type="text" class="" value="${data.get('contact-email', '')}" />
     </div>
   </div>
   <div class="control-group">
     <label class="control-label" for="contact-name">Telephone</label>
     <div class="controls">
-      <input id="contact-phone" maxlength="30" name="contact-phone" type="text" class="" value="${publisher_extras.get('contact-phone', '')}" />
+      <input id="contact-phone" maxlength="30" name="contact-phone" type="text" class="" value="${data.get('contact-phone', '')}" />
     </div>
   </div>
 </fieldset>
@@ -138,20 +154,20 @@ <h3>FOI Request Details</h3>
   <div class="control-group">
     <label class="control-label" for="foi-name">Name</label>
     <div class="controls">
-      <input id="foi-name" maxlength="64" name="foi-name" type="text" class="" value="${publisher_extras.get('foi-name', '')}" />
+      <input id="foi-name" maxlength="64" name="foi-name" type="text" class="" value="${data.get('foi-name', '')}" />
       <p class="help-block">e.g. DfT FOI Enquiry Service</p>
     </div>
   </div>
   <div class="control-group">
     <label class="control-label" for="foi-email">Email</label>
     <div class="controls">
-      <input id="foi-email" maxlength="100" name="foi-email" type="text" class="" value="${publisher_extras.get('foi-email', '')}" />
+      <input id="foi-email" maxlength="100" name="foi-email" type="text" class="" value="${data.get('foi-email', '')}" />
     </div>
   </div>
   <div class="control-group">
     <label class="control-label" for="foi-name">Telephone</label>
     <div class="controls">
-      <input id="foi-phone" maxlength="30" name="foi-phone" type="text" class="" value="${publisher_extras.get('foi-phone', '')}" />
+      <input id="foi-phone" maxlength="30" name="foi-phone" type="text" class="" value="${data.get('foi-phone', '')}" />
     </div>
   </div>
 </fieldset>