Merge pull request #36 from alphagov/141882981-Add-question-type-to-c…

…ontent-loader-and-summary-display

141882981-Add-question-type-to-content-loader-and-summary-display

CHANGELOG.md

@@ -2,6 +2,29 @@
 
 Records breaking changes from major version bumps
 
+## 4.0.0
+
+PR: [#36](https://github.com/alphagov/digitalmarketplace-content-loader/pull/36/files)
+
+### What changed
+
+New question type `Date` and non-backwards compatible change to `Question.unformat_data` which now returns only data
+relevant to the given question.
+
+### Example Change
+
+#### Old
+```
+        >>> question.unformat_data({"thisQuestion": 'some data', "notThisQuestion": 'other data'})
+        {"thisQuestion": 'some data changed by unformat method', "notThisQuestion": 'other data'}
+```
+#### New
+```
+        >>> question.unformat_data({"thisQuestion": 'some data', "notThisQuestion": 'other data'})
+        {"thisQuestion": 'some data changed by unformat method'}
+```
+
+
 ## 3.0.0
 
 PR: [#25](https://github.com/alphagov/digitalmarketplace-content-loader/pull/25)
@@ -13,7 +36,6 @@ and multiple followups for a single question. This requires changing the questio
 syntax for listing followups, so the content loader is modified to support the new format
 and will only work with an updated frameworks repo.
 
-
 ### Example change
 
 #### Old

dmcontent/__init__.py

@@ -2,4 +2,4 @@
 from .errors import ContentTemplateError, QuestionNotFoundError
 from .questions import ContentQuestion
 
-__version__ = '3.6.0'
+__version__ = '4.0.0'

dmcontent/content_loader.py

@@ -353,8 +353,15 @@ def get_error_messages(self, errors, question_descriptor_from="label"):
 
         return errors_map
 
+    @staticmethod
+    def unformat_assurance(key, data):
+        return {
+            key + '--assurance': data[key].get('assurance', None),
+            key: data[key].get('value', None)
+        }
+
     def unformat_data(self, data):
-        """Unpack assurance information to be used in a form
+        """Method to process data into format for form, special assurance case or individual question level unformat.
 
         :param data: the service data as returned from the data API
         :type data: dict
@@ -372,10 +379,16 @@ def unformat_data(self, data):
         result = {}
         for key in data:
             if self._has_assurance(key):
-                result[key + '--assurance'] = data[key].get('assurance', None)
-                result[key] = data[key].get('value', None)
+                # If it's an assurance question do the unformatting here.
+                result.update(self.unformat_assurance(key, data))
             else:
-                result[key] = data[key]
+                question = self.get_question(key)
+                if question:
+                    # Otherwise if it is a legitimate question use the unformat method on the question.
+                    result.update(question.unformat_data(data))
+                else:
+                    # Otherwise it's not a question, default to returning the k: v pair.
+                    result[key] = data[key]
         return result
 
     def get_question(self, field_name):

dmcontent/questions.py

@@ -1,4 +1,8 @@
 from collections import OrderedDict, defaultdict
+from datetime import datetime
+
+from dmutils.formats import DATE_FORMAT, DISPLAY_DATE_FORMAT
+
 from .converters import convert_to_boolean, convert_to_number
 from .errors import ContentNotFoundError
 from .formats import format_price
@@ -96,7 +100,6 @@ def get_error_messages(self, errors, question_descriptor_from="label"):
         for field_name in sorted(error_fields):
             question = self.get_question(field_name)
             message_key = errors[field_name]
-
             validation_message = question.get_error_message(message_key, field_name)
 
             error_key = question.id
@@ -112,7 +115,7 @@ def get_error_messages(self, errors, question_descriptor_from="label"):
         return question_errors
 
     def unformat_data(self, data):
-        return data
+        return {self.id: data.get(self.id, None)}
 
     def get_error_message(self, message_key, field_name=None):
         """Return a single error message.
@@ -392,19 +395,19 @@ def unformat_data(self, data):
             "yesno-0": True,
             "yesno-1": False,
             "evidence-0": "Yes, I did."
-            "nonDynamicListKey": 'other data'
         }
         """
         result = {}
-        for key in data:
-            if key == self.id:
-                for question in self.questions:
-                    # For each question e.g. evidence-0, find if data exists for it and insert it into our result
-                    root, index = question.id.split('-')
-                    if root in data[self.id][int(index)]:
-                        result[question.id] = data[self.id][int(index)].get(root)
-            else:
-                result[key] = data[key]
+        dynamic_list_data = data.get(self.id, None)
+        if not dynamic_list_data:
+            return result
+        for question in self.questions:
+            # For each question e.g. evidence-0, find if data exists for it and insert it into our result
+            root, index = question.id.split('-')
+            question_data = dynamic_list_data[int(index)]
+            if root in question_data:
+                result.update({question.id: question_data.get(root)})
+
         return result
 
     def get_error_messages(self, errors, question_descriptor_from="label"):
@@ -464,12 +467,17 @@ def get_question(self, field_name):
         if self.id == field_name or field_name in self.fields.values():
             return self
 
+    def unformat_data(self, data):
+        """Get values from api data whose keys are in self.fields; this indicates they are related to this question."""
+        return {key: data[key] for key in data if key in self.fields.values()}
+
     def get_data(self, form_data):
-        return {
-            key: form_data[key] if form_data[key] else None
-            for key in self.fields.values()
-            if key in form_data
-        }
+        """
+        Return a subset of form_data containing only those key: value pairs whose key appears in the self.fields of
+        this question (therefore only those pairs relevant to this question).
+        Filter 0/ False values here and replace with None as they are handled with the optional flag.
+        """
+        return {key: form_data[key] or None for key in self.fields.values() if key in form_data}
 
     @property
     def form_fields(self):
@@ -544,6 +552,46 @@ def update_expected_values(options, parents, expected_set):
         return expected_values - selected_values_set
 
 
+class Date(Question):
+    """Class used as an interface for date data between forms, backend and summary pages."""
+
+    FIELDS = ('year', 'month', 'day')
+
+    def summary(self, service_data):
+        return DateSummary(self, service_data)
+
+    @staticmethod
+    def process_value(value):
+        """If there are any hyphens in the value then it does not validate."""
+        value = value.strip() if value else ''
+        if not value or '-' in value:
+            return ''
+        return value
+
+    def get_data(self, form_data):
+        """Retreive the fields from the POST data (form_data).
+
+        The d, m, y should be in the post as 'questionName-day', questionName-month ...
+        Extract them and format as '\d\d\d\d-\d{1,2}-\d{1,2}'.
+        https://code.tutsplus.com/tutorials/validating-data-with-json-schema-part-1--cms-25343
+        """
+        parts = []
+        for key in self.FIELDS:
+            identifier = '-'.join([self.id, key])
+            value = form_data.get(identifier, '')
+            parts.append(self.process_value(value))
+
+        return {self.id: '-'.join(parts) if any(parts) else None}
+
+    def unformat_data(self, data):
+        result = {}
+        value = data[self.id]
+        if value:
+            for partial_value, field in zip(value.split('-'), self.FIELDS):
+                result['-'.join([self.id, field])] = partial_value
+        return result
+
+
 class QuestionSummary(Question):
     def __init__(self, question, service_data):
         self.number = question.number
@@ -626,6 +674,22 @@ def answer_required(self):
             return self.is_empty
 
 
+class DateSummary(QuestionSummary):
+
+    def __init__(self, question, service_data):
+        super(DateSummary, self).__init__(question, service_data)
+        self._value = self._service_data.get(self.id, '')
+
+    @property
+    def value(self):
+        try:
+            return datetime.strptime(self._value, DATE_FORMAT).strftime(DISPLAY_DATE_FORMAT)
+        except ValueError:
+            # We may need to fall back to displaying a plain string value in the case of briefs in draft before
+            # the date field was introduced.
+            return self._value
+
+
 class MultiquestionSummary(QuestionSummary, Multiquestion):
     def __init__(self, question, service_data):
         super(MultiquestionSummary, self).__init__(question, service_data)
@@ -758,6 +822,7 @@ def _get_options_recursive(options):
     'list': List,
     'checkboxes': List,
     'checkbox_tree': Hierarchy,
+    'date': Date
 }
 
 

requirements.txt

@@ -1 +1,2 @@
--e .
+-e .
+git+https://github.com/alphagov/digitalmarketplace-utils.git@25.0.1#egg=digitalmarketplace-utils==25.0.1

setup.cfg

@@ -1,6 +1,6 @@
 [pep8]
-exclude = ./venv
+exclude = ./venv*
 max-line-length = 120
 
 [pytest]
-norecursedirs = venv
+norecursedirs = venv*

tests/test_content_loader.py

@@ -1699,11 +1699,11 @@ def test_inject_messages_into_section_and_section_summary(self):
 
 
 class TestReadYaml(object):
-    @mock.patch.object(builtins, 'open', return_value=io.StringIO(u'foo: bar'))
+    @mock.patch('dmcontent.content_loader.open', return_value=io.StringIO(u'foo: bar'))
     def test_loading_existant_file(self, mocked_open):
         assert read_yaml('anything.yml') == {'foo': 'bar'}
 
-    @mock.patch.object(builtins, 'open', side_effect=IOError)
+    @mock.patch('dmcontent.content_loader.open', side_effect=IOError)
     def test_file_not_found(self, mocked_open):
         with pytest.raises(IOError):
             assert read_yaml('something.yml')