Merge pull request #24 from sleitner/master

SAG Review

ckanext/cfpb_extrafields/options.py

@@ -8,23 +8,29 @@ def form_select_format(l):
 def resource_type():
     a = ["Dashboard", "Database", "Data Dictionary", "Data File", "Documentation", "Report"]
     return form_select_format(a)
+
 def format():
     return ["CSV", "ASCII", "STATA", "SQL", "PDF", "SAS", "XML", "word doc",
            "excel", "TXT", "TAB", "JSON"] #+other
+
 def storage_location():
     return ["Research Server", "Z Drive", "SES", "RightNow"] #+other
+
 def sensitivity_level():
     a = ["Public", "Low", "Medium", "High"]
     return form_select_format(a)
+
 def file_sizes():
     a = ["<500 MB","501-1,000 MB","1001-10,000 MB",">10,000 MB"]
     return form_select_format(a)
+
 def approximate_total_size():
     return file_sizes()
 
 # datasets
 def legal_authority_for_collection():
     return ["Market Monitoring", "Supervision", "Enforcement", "Consumer Response"]
+
 def privacy_pia_title():
     return ["CFPB Business Intelligence Tool",
             "Certain Supervision, Enforcement, and Fair Lending Data used for Market Research",
@@ -49,6 +55,7 @@ def privacy_pia_title():
             "Scheduling and Examination System (SES) PIA",
             "SES November 2012 update",
             "N/A",]
+
 def privacy_sorn_number():
     return ["CFPB.001 - Freedom of Information Act/Privacy Act System",
             "CFPB.002 - Depository Institution Supervision Database",
@@ -78,10 +85,13 @@ def privacy_sorn_number():
             "CFPB.026 - Biographies",
             "Government-wide",
             "N/A",]
+
 def relevant_governing_documents():
     return ["Contract", "MOU", "NDA", "Interagency Agreement", "Other"] 
+
 def content_spatial():
     return [""]
+
 def frequency_standards():
 #http://dublincore.org/groups/collections/frequency/
     a=["Triennial", "Biennial","Annual",
@@ -92,10 +102,13 @@ def frequency_standards():
     # want shorter options first
     a.reverse() 
     return form_select_format(a)
+
 def content_periodicity():
     return frequency_standards()
+
 def update_frequency():
     return frequency_standards()
+
 def pra_exclusion():
     return ["5 C.F.R. 1320.3(h)(1)",
             "5 C.F.R. 1320.3(h)(2)",
@@ -108,9 +121,11 @@ def pra_exclusion():
             "5 C.F.R. 1320.3(h)(9)",
             "5 C.F.R. 1320.3(h)(10)",
             "N/A",]
+
 def privacy_pia_notes():
     return ["PIA Published", "No PIA - No PII", "No PIA - Only Employee PII", 
             "No PIA - All PII Aggregated"]
+
 def transfer_method():
     a=["Website", "SFTP", "FTP", "Physical Media", "Email", "Connect Direct", "Other"]
     return form_select_format(a)

ckanext/cfpb_extrafields/plugin.py

@@ -38,6 +38,7 @@ def popup_data_source_names():
 def popup_usage_restrictions():
     return "Enter instructions for what users can and cannot do with the data."
 
+
 import pylons.config as config
 def github_api_url():
     return config['ckan.ckanext_cfpb_extrafields.github_api_url']
@@ -59,6 +60,8 @@ def parse_resource_related_gist(data_related_items, resource_id):
 
 class ExampleIDatasetFormPlugin(p.SingletonPlugin, tk.DefaultDatasetForm):
 
+
+    # modify ckan behavior on changes/(saves/updates/deletes) to resources
     p.implements(p.IResourceController)
     def _which_check_keys_changed(self, old, new):
         check_keys = ['resource_type', 'privacy_contains_pii']
@@ -80,12 +83,14 @@ def _delete_and_rebuild_datadict(self, resource):
                 ds.delete_datastore_json(resource['id'], 'datadict')
             # don't fail if the filter is bad! (e.g., title_colname doesn't exist)
             except (tk.ObjectNotFound, tk.ValidationError), err:
+                # code review: write tests for this.
                 pass
             ds.create_datastore(resource['id'], json_title='datadict', json_record=json_record)
         return
-
+
+    # This function is a template/starter for a more fine-grained email-on-change functionality
+    # than the default notification that CKAN provides. It is not currently used.
     def _email_on_change(self, context, resource, field):
-        # unfinished! 
         # if specified fields have changed notify the relevant people
         if self.changed[field]:
             # print 'trigger email on change to '+field 
@@ -126,14 +131,13 @@ def after_update(self, context, resource):
 
     def before_delete(self, context, resource, resources):
         return
-
     def after_delete(self, context, resources):
         return
-
     def before_show(self, resource_dict):
         return
 
 
+    # functions that are accessible via h.* in the Jinja templates
     p.implements(p.ITemplateHelpers)
     def get_helpers(self):
         return {'clean_select_multi': v.clean_select_multi,
@@ -167,6 +171,7 @@ def get_helpers(self):
             }
 
 
+    # the main extra fields update/show functionality
     p.implements(p.IDatasetForm)
     def _modify_package_schema(self, schema):
         schema.update({
@@ -438,6 +443,7 @@ def update_config(self, config):
         tk.add_resource('fanstatic','cfpb_extrafields')
 
 
+    # filters that let users progressively narrow the datasets they're searching
     p.implements(p.IFacets)
     def _change_facets(self, facets_dict):
         dummy_facets = facets_dict
@@ -455,6 +461,7 @@ def _change_facets(self, facets_dict):
         facets_dict['res_format'] = p.toolkit._('Formats')
         return facets_dict
 
+    # now return the same altered search facets for the dataset, group and organization page
     def dataset_facets(self, facets_dict, package_type):
         return self._change_facets(facets_dict)
     def group_facets(self, facets_dict, group_type, package_type):

ckanext/cfpb_extrafields/templates/package/snippets/resource_type-specific/cfpb_extrafields_dashboard.html

@@ -15,7 +15,7 @@
 
   <!-- format persists unless reset -->
   <div hidden class='core-field'>
-  <input id="format" value="not applicable" name="format"/>
+  <input id="format" value="dashboard" name="format"/>
   </div>
 
   <div class='core-field'>

ckanext/cfpb_extrafields/templates/package/snippets/resource_type-specific/cfpb_extrafields_database.html

@@ -22,7 +22,7 @@
 
   <!-- format persists unless reset -->
   <div hidden class='core-field'>
-  <input id="format" value="not applicable" name="format"/>
+  <input id="format" value="database" name="format"/>
   </div>
 
   {{ form.input('database_server', type='text', label=_("database server"),

ckanext/cfpb_extrafields/templates/package/snippets/resource_type-specific/cfpb_extrafields_datadictionary.html

@@ -23,7 +23,7 @@
 
   <!-- format persists unless reset -->
   <div hidden class='core-field'>
-  <input id="format" value="not applicable" name="format"/>
+  <input id="format" value="data dictionary" name="format"/>
   </div>
   <!-- end cfpb_extrafields_datadictionary.html snippet -->
 {% endif %}

ckanext/cfpb_extrafields/templates/package/snippets/resource_type-specific/cfpb_extrafields_template.html

@@ -18,7 +18,7 @@
 
   <!-- format persists unless reset -->
   <div hidden class='core-field'>
-  <input id="format" value="not applicable" name="format"/>
+  <input id="format" value="TEMPLATE" name="format"/>
   </div>
 
   <div class='core-field'>

ckanext/cfpb_extrafields/tests.py

@@ -1,13 +1,12 @@
 import unittest
-
 from nose_parameterized import parameterized
 from nose.tools import assert_equal
 import mock
-
 import validators as v
 
 
 class TestValidators(unittest.TestCase):
+
 
     @parameterized.expand([
         (u'"asdf","asdf,asdf"',[u'asdf', u'asdf,asdf']),
@@ -24,55 +23,64 @@ def test_clean_select_multi(self, ms, expected):
         print ms
         print expected
         assert_equal(v.clean_select_multi(ms), expected)
-
+
+
     @parameterized.expand([("note0 note1",), ("http://www.~/hi",), ])
     def test_input_value_validator(self, input):
         assert_equal(input, v.input_value_validator(input))
+
     @parameterized.expand([("__Other",), ])
     @mock.patch("ckanext.cfpb_extrafields.validators.Invalid")
     def test_input_value_validator_invalid(self, input, mi):
         mi.side_effect = Exception("")
         with self.assertRaises(Exception):
             v.input_value_validator(input)
 
-    @parameterized.expand(
-        [(.1,), (.0001,), (1,), (1000000,), (9999.9999999,), ]
-    )
+
+    @parameterized.expand([(.1,), (.0001,), (1,), (1000000,), (9999.9999999,), ])
     def test_positive_number_validator_valid(self, input):
         result = v.positive_number_validator(input)
         assert_equal(input, result)
-    @parameterized.expand(
-        [("a",), ("<script>alert('hi');</script>",), (-.01,), (-1,), (-1000000,), (-9999.9999999,), ]
-    )
+        
+    @parameterized.expand([("a",), ("<script>alert('hi');</script>",),
+                           (-.01,), (-1,), (-1000000,), (-9999.9999999,), ])
     @mock.patch("ckanext.cfpb_extrafields.validators.Invalid")
     def test_positive_number_validator_invalid(self, input, mi):
         mi.side_effect = Exception("")
         with self.assertRaises(Exception):
             v.positive_number_validator(input)
 
+
     @parameterized.expand([("DI66666",), ("DI00001",), ])
     def test_dig_id_validator(self, input):
         assert_equal(input, v.dig_id_validator(input))
+
     @parameterized.expand([("a",), ("64444",), ("DI7777",), ("DI-12345",), ("DI1234",), ])
     @mock.patch("ckanext.cfpb_extrafields.validators.Invalid")
     def test_dig_id_validator_invalid(self, input, mi):
         mi.side_effect = Exception("")
         with self.assertRaises(Exception):
             v.dig_id_validator(input)
 
+
     @parameterized.expand([("2010-10-01",), ("1995-01-01",), ("2100-10-20",), (None,), ])
     def test_reasonable_date_validator(self, input):
         assert_equal(input, v.reasonable_date_validator(input))
-    @parameterized.expand([("a",), ("1500-01-01",), ("27901-01-01",), ("2012/12/21",), ("2012/1/1",), ("2012-21-01",), ("2012-10-a1",), ]) #, ("2012-10-1",)
+
+    @parameterized.expand([("a",), ("1500-01-01",), ("27901-01-01",),
+                           ("2012/12/21",), ("2012/1/1",), ("2012-21-01",),
+                           ("2012-10-a1",), ]) 
     @mock.patch("ckanext.cfpb_extrafields.validators.Invalid")
     def test_reasonable_date_validator(self, input, mi):
         mi.side_effect = Exception("")
         with self.assertRaises(Exception):
             v.reasonable_date_validator(input)
 
+
     @parameterized.expand([("5555-6666",), ("6666-4444",), ])
     def test_pra_control_num_validator_valid(self, input):
         assert_equal(input, v.pra_control_num_validator(input))
+
     @parameterized.expand([("a",), ("a666-4444",), ("7777-a888",), ("aaaa-bbbb",), ("77778888",), ])
     @mock.patch("ckanext.cfpb_extrafields.validators.Invalid")
     def test_pra_control_num_validator_invalid(self, input, mi):