Merge pull request #820 from openhealthcare/seperated-out-to-dict

Seperated out to dict

doc/docs/reference/mixins.md

@@ -0,0 +1,27 @@
+## OPAL mixins
+
+### SerialisableFields
+provides the fields that are on the model for example
+if we have an allergy model with a field drug
+it might serialise like below
+
+    Allergy._get_fieldnames_to_serialize() -> ["id", "drug"]
+
+
+### ToDictMixin
+provides a method that serialises a model
+to a dictionary for example
+if we have an allergy model with a field drug
+it might serialise like below
+
+    allergy.to_dict() -> {"id": 1, "drug": "penicillin"}
+
+### UpdateFromDict
+provides a method that updates a model
+based on a dictionary of fields, for example
+
+For example on a new allergy
+
+    allergy.update_from_dict({"drug": "penicillin"})
+
+will update the allergy to have the drug penicillin

doc/docs/reference/reference_guides.md

@@ -5,9 +5,10 @@ The following reference guides are available:
 ### Models
 |
 -|-
-[opal.models.Episode model](episode.md)| The central Episode model
+[opal.models.Episode](episode.md)| The central Episode model
 [opal.models.Patient](patient.md) | The Patient model
-[opal.models.Subrecord base class](subrecords.md)|Base class for subrecords of Episodes or Patients
+[opal.models.Subrecord](subrecords.md) | for subrecords of Episodes or Patients
+[opal.models.*](mixins.md) | Mixin helpers for useful functionality
 
 ### OPAL Core
 |

doc/docs/reference/subrecords.md

@@ -3,6 +3,8 @@
 OPAL Subrecords are models that relate to either Patients or Episodes, and inherit from
 base classes `opal.models.PatientSubrecord` or `opal.models.EpisodeSubrecord`
 
+They themselves inherit from the mixins `opal.models.ToDictMixin`, `opal.models.UpdateFromDict`
+
 ### Properties
 
 #### Subrecord._angular_service
@@ -127,15 +129,6 @@ Keywords:
 Classmethod to add a custom footer to a modal, used for example to denote if
 the data from a model has been sourced from an external source
 
-#### Subrecord.update_from_dict()
-An instance method that will update a model with a dictionary. This method is used
-to provides a hook for changing the way a subrecord handles being updated from serialised
-data.
-
-For example on a new allergy
-    allergy.update_from_dict({"drug": "penicillin"})
-
-will update the allergy to have the correct drug
 
 #### Subrecord.bulk_update_from_dicts()
 

doc/mkdocs.yml

@@ -36,6 +36,7 @@ pages:
       - Episode Categories: reference/episode_categories.md
       - reference/patient.md
       - Subrecords: reference/subrecords.md
+      - Mixins: reference/mixins.md
       - OpalApplication: reference/opal_application.md
       - reference/episode_service.md
       - reference/item_service.md

opal/models.py

@@ -54,12 +54,11 @@ def deserialize_date(value):
     return dt.date()
 
 
-class UpdatesFromDictMixin(object):
+class SerialisableFields(object):
     """
-    Mixin class to provide the serialization/deserialization
-    fields, as well as update logic for our JSON APIs.
+    Mixin class that handles the getting of fields
+    and field types for serialisation/deserialization
     """
-
     @classmethod
     def _get_fieldnames_to_serialize(cls):
         """
@@ -88,22 +87,6 @@ def _get_fieldnames_to_serialize(cls):
 
         return fieldnames
 
-    @classmethod
-    def _get_fieldnames_to_extract(cls):
-        """
-        Return a list of fieldname to extract - which means dumping
-        PID fields.
-        """
-        fieldnames = cls._get_fieldnames_to_serialize()
-        if hasattr(cls, 'pid_fields'):
-            for fname in cls.pid_fields:
-                if fname in fieldnames:
-                    fieldnames.remove(fname)
-                    if cls._get_field_type(fname) == ForeignKeyOrFreeText:
-                        fieldnames.remove(fname + '_fk_id')
-                        fieldnames.remove(fname + '_ft')
-        return fieldnames
-
     @classmethod
     def _get_field_type(cls, name):
         try:
@@ -125,7 +108,6 @@ def _get_field_type(cls, name):
 
         raise exceptions.UnexpectedFieldNameError('Unexpected fieldname: %s' % name)
 
-
     @classmethod
     def _get_field_title(cls, name):
         try:
@@ -139,6 +121,64 @@ def _get_field_title(cls, name):
             # else its foreign key or free text
             return getattr(cls, name).verbose_name.title()
 
+
+    @classmethod
+    def build_field_schema(cls):
+        field_schema = []
+        for fieldname in cls._get_fieldnames_to_serialize():
+            if fieldname in ['id', 'patient_id', 'episode_id']:
+                continue
+            elif fieldname.endswith('_fk_id'):
+                continue
+            elif fieldname.endswith('_ft'):
+                continue
+
+            getter = getattr(cls, 'get_field_type_for_' + fieldname, None)
+            if getter is None:
+                field = cls._get_field_type(fieldname)
+                if field in [models.CharField, ForeignKeyOrFreeText]:
+                    field_type = 'string'
+                else:
+                    field_type = camelcase_to_underscore(field.__name__[:-5])
+            else:
+                field_type = getter()
+            lookup_list = None
+            if cls._get_field_type(fieldname) == ForeignKeyOrFreeText:
+                fld = getattr(cls, fieldname)
+                lookup_list = camelcase_to_underscore(fld.foreign_model.__name__)
+            title = cls._get_field_title(fieldname)
+
+            field_schema.append({'name': fieldname,
+                                 'title': title,
+                                 'type': field_type,
+                                 'lookup_list': lookup_list,
+                                 'model': cls.__name__
+                                 })
+        return field_schema
+
+
+class UpdatesFromDictMixin(SerialisableFields):
+    """
+    Mixin class to provide the deserialization
+    fields, as well as update logic for our JSON APIs.
+    """
+
+    @classmethod
+    def _get_fieldnames_to_extract(cls):
+        """
+        Return a list of fieldname to extract - which means dumping
+        PID fields.
+        """
+        fieldnames = cls._get_fieldnames_to_serialize()
+        if hasattr(cls, 'pid_fields'):
+            for fname in cls.pid_fields:
+                if fname in fieldnames:
+                    fieldnames.remove(fname)
+                    if cls._get_field_type(fname) == ForeignKeyOrFreeText:
+                        fieldnames.remove(fname + '_fk_id')
+                        fieldnames.remove(fname + '_ft')
+        return fieldnames
+
     @classmethod
     def get_field_type_for_consistency_token(cls):
         return 'token'
@@ -192,10 +232,15 @@ def save_many_to_many(self, name, values, field_type):
         field.add(*to_add)
         field.remove(*to_remove)
 
-    def update_from_dict(self, data, user, force=False):
+    def update_from_dict(self, data, user, force=False, fields=None):
         logging.info("updating {0} with {1} for {2}".format(
-            self.__class__.__name__, data, user)
-        )
+            self.__class__.__name__, data, user
+        ))
+
+        if fields is None:
+            fields = set(self._get_fieldnames_to_serialize())
+
+
         if self.consistency_token and not force:
             try:
                 consistency_token = data.pop('consistency_token')
@@ -208,8 +253,6 @@ def update_from_dict(self, data, user, force=False):
             if consistency_token != self.consistency_token:
                 raise exceptions.ConsistencyError
 
-        fields = set(self._get_fieldnames_to_serialize())
-
         post_save = []
 
         unknown_fields = set(data.keys()) - fields
@@ -254,6 +297,35 @@ def update_from_dict(self, data, user, force=False):
             some_func()
 
 
+class ToDictMixin(SerialisableFields):
+    """ serialises a model to a dictionary
+    """
+
+    def to_dict(self, user, fields=None):
+        """
+        Allow a subset of FIELDNAMES
+        """
+
+        if fields is None:
+            fields = self._get_fieldnames_to_serialize()
+
+        d = {}
+        for name in fields:
+            getter = getattr(self, 'get_' + name, None)
+            if getter is not None:
+                value = getter(user)
+            else:
+                field_type = self._get_field_type(name)
+                if field_type == models.fields.related.ManyToManyField:
+                    qs = getattr(self, name).all()
+                    value = [i.to_dict(user) for i in qs]
+                else:
+                    value = getattr(self, name)
+            d[name] = value
+
+        return d
+
+
 class Filter(models.Model):
     """
     Saved filters for users extracting data.
@@ -625,17 +697,6 @@ def start(self):
     def end(self):
         return self.category.end
 
-    @property
-    def is_discharged(self):
-        """
-        Predicate property to determine if we're discharged.
-        """
-        if not self.active:
-            return True
-        if self.discharge_date:
-            return True
-        return False
-
     @property
     def category(self):
         from opal.core import episodes
@@ -774,7 +835,7 @@ def to_dict(self, user, shallow=False):
         return d
 
 
-class Subrecord(UpdatesFromDictMixin, TrackedModel, models.Model):
+class Subrecord(UpdatesFromDictMixin, ToDictMixin, TrackedModel, models.Model):
     consistency_token = models.CharField(max_length=8)
     _is_singleton = False
     _advanced_searchable = True
@@ -805,40 +866,6 @@ def get_display_name(cls):
         else:
             return cls._meta.object_name
 
-    @classmethod
-    def build_field_schema(cls):
-        field_schema = []
-        for fieldname in cls._get_fieldnames_to_serialize():
-            if fieldname in ['id', 'patient_id', 'episode_id']:
-                continue
-            elif fieldname.endswith('_fk_id'):
-                continue
-            elif fieldname.endswith('_ft'):
-                continue
-
-            getter = getattr(cls, 'get_field_type_for_' + fieldname, None)
-            if getter is None:
-                field = cls._get_field_type(fieldname)
-                if field in [models.CharField, ForeignKeyOrFreeText]:
-                    field_type = 'string'
-                else:
-                    field_type = camelcase_to_underscore(field.__name__[:-5])
-            else:
-                field_type = getter()
-            lookup_list = None
-            if cls._get_field_type(fieldname) == ForeignKeyOrFreeText:
-                fld = getattr(cls, fieldname)
-                lookup_list = camelcase_to_underscore(fld.foreign_model.__name__)
-            title = cls._get_field_title(fieldname)
-
-            field_schema.append({'name': fieldname,
-                                 'title': title,
-                                 'type': field_type,
-                                 'lookup_list': lookup_list,
-                                 'model': cls.__name__
-                                 })
-        return field_schema
-
     @classmethod
     def _build_template_selection(cls, episode_type=None, patient_list=None, suffix=None, prefix=None):
         name = cls.get_api_name()
@@ -942,30 +969,6 @@ def bulk_update_from_dicts(
 
             subrecord.update_from_dict(a_dict, user, force=force)
 
-    def _to_dict(self, user, fieldnames):
-        """
-        Allow a subset of FIELDNAMES
-        """
-
-        d = {}
-        for name in fieldnames:
-            getter = getattr(self, 'get_' + name, None)
-            if getter is not None:
-                value = getter(user)
-            else:
-                field_type = self._get_field_type(name)
-                if field_type == models.fields.related.ManyToManyField:
-                    qs = getattr(self, name).all()
-                    value = [i.to_dict(user) for i in qs]
-                else:
-                    value = getattr(self, name)
-            d[name] = value
-
-        return d
-
-    def to_dict(self, user):
-        return self._to_dict(user, self._get_fieldnames_to_serialize())
-
 
 class PatientSubrecord(Subrecord):
     patient = models.ForeignKey(Patient)

opal/tests/test_core_schemas.py

@@ -90,6 +90,7 @@ def test_list_records(self, tagging, subrecords):
             'tagging': tagging_serialized,
             'colour': colour_serialized
         }
+
         self.assertEqual(expected, schemas.list_records())
 
 

opal/tests/test_episode.py

@@ -25,25 +25,6 @@ def setUp(self):
     def test_singleton_subrecord_created(self):
         self.assertEqual(1, self.episode.episodename_set.count())
 
-    def test_is_discharged_inactive(self):
-        self.episode.active = False
-        self.assertEqual(True, self.episode.is_discharged)
-
-    def test_is_discharged_discharge_date(self):
-        self.episode.active = True
-        self.episode.discharge_date = datetime.date(2010, 2, 24)
-        self.assertEqual(True, self.episode.is_discharged)
-
-    def test_is_discharged_not_discharged(self):
-        self.episode.active = True
-        self.assertEqual(False, self.episode.is_discharged)
-
-    def test_is_discharged_from_date(self):
-        today = datetime.date.today()
-        yesterday = today - datetime.timedelta(days=1)
-        self.episode.discharge_date = yesterday
-        self.assertEqual(True, self.episode.is_discharged)
-
     def test_category(self):
         self.episode.category_name = 'Inpatient'
         self.assertEqual(self.episode.category.__class__, InpatientEpisode)