Merge 0ac90ee into fd1feb9

changelog.md

@@ -1,5 +1,11 @@
 ### 0.12.0 (Major Release)
 
+#### Pathway Save
+* Pathways steps define what subrecords are saved. By default this is whatever is defined as the `model`
+* The method definition of the pre_save step has changed so that it takes now takes `data` and `raw_data` as the first two arguments. `raw_data` being what is brought in by the server and `data` being what is eventually saved.
+* Pathways now have a `pre_save` method
+
+
 #### Misc Changes
 
 * Adds a method `.demographics()` to `opal.models.Patient` which returns the relevant demographics instance.

doc/docs/guides/pathways.md

@@ -44,23 +44,25 @@ class SimplePathway(pathway.PagePathway):
 ```
 
 This will give you form at `http://localhost:8000/pathway/#/simples` that
-lets the user create a patient with their demographics and past medical history
+lets the user create a patient with their demographics and past medical history. It will save any demographics or past medical history subrecords that have changed.
 
 ## Customising Steps
 
-When passed a model, a step will infer the details of form templates, display names et
-cetera from the subrecord. However a model is not required - you can also pass arbitrary
-chunks of html with the two required fields:
+When passed a model, a step will infer the details of form templates, display names, icons and will know to save that subrecord. However a model is not required - you can also pass arbitrary chunks of html with the two required fields:
 
 ``` python
 Step(
   template="pathway/steps/my_step.html"
-  display_name="My Display Only Step"
+  display_name="My Display Only Step",
+  save_data=False
 )
 ```
 
+This will display the template and will not save any data related to the step.
+
 Alternatively, you can override any fields that it would usually take from the model.
-The below example, will override the template and not just use the Demographics form template.
+
+The below example, will override the template and not use the Demographics form template.
 
 ``` python
 Step(
@@ -82,10 +84,16 @@ Step(
 )
 ```
 
-If you want to add any custom save logic for your step, you can put in a `pre_save` method.
-This is passed the full data dictionary that has been received from the client and the patient
-and episode that the pathways been saved for, if they exist (If you're saving a pathway for a
-new patient/episode, they won't have been created at this time).
+By default the `pre_save` method copies the data that the step will save onto the `data` variable (passed in to the method). If you wish to change what data will be saved here is a good place to do it.
+
+`pre_save` is passed:
+
+* `data` - the data that will be saved,
+* `raw_data` - the data that has been sent back from the front end.
+* `user` - the currently logged in user.
+* `patient` and `episode` if this is not saving a new patient/episode. If you're saving a pathway for a
+new patient/episode, they won't have been created at this time.
+
 
 ## Loading Data From Existing Episodes
 

opal/core/pathway/pathways.py

@@ -100,16 +100,23 @@ def redirect_url(self, user=None, patient=None, episode=None):
         episode = patient.episode_set.last()
         return "/#/patient/{0}/{1}".format(patient.id, episode.id)
 
+    def pre_save(self, data, raw_data, user, patient=None, episode=None):
+        pass
+
     @transaction.atomic
-    def save(self, data, user=None, patient=None, episode=None):
+    def save(self, raw_data, user=None, patient=None, episode=None):
         if patient and not episode:
             episode = patient.create_episode()
 
+        data = {}
+
         for step in self.get_steps():
             step.pre_save(
-                data, user, patient=patient, episode=episode
+                data, raw_data, user, patient=patient, episode=episode
             )
 
+        self.pre_save(data, raw_data, user, patient=patient, episode=episode)
+
         # if there is an episode, remove unchanged subrecords
         if patient:
             data = self.remove_unchanged_subrecords(episode, data, user)

opal/core/pathway/steps.py

@@ -70,11 +70,13 @@ class Step(object):
     step_controller = "DefaultStep"
     base_template = "pathway/steps/step_base_template.html"
     multiple_template = "pathway/steps/multi_save.html"
+    model = None
 
-    def __init__(self, model=None, multiple=None, **kwargs):
-        self.model = model
+    def __init__(self, model=None, multiple=None, save_data=True, **kwargs):
+        self.model = model or self.model
         self.other_args = kwargs
         self.multiple = multiple
+        self.save_data = save_data
 
         # We only infer from the model if the user did not pass in a value
         if self.multiple is None and self.model:
@@ -165,9 +167,25 @@ def to_dict(self):
         result.update(self.other_args)
         return result
 
-    def pre_save(self, data, user, patient=None, episode=None):
-        if self.multiple and self.delete_others:
-            delete_others(data, self.model, patient=patient, episode=episode)
+    def get_model_data(self, raw_data):
+        if self.model.get_api_name() in raw_data:
+            return raw_data[self.model.get_api_name()]
+        return []
+
+    def pre_save(self, data, raw_data, user, patient=None, episode=None):
+        if self.save_data:
+            if self.model:
+                data[self.model.get_api_name()] = self.get_model_data(raw_data)
+            else:
+                raise NotImplementedError(
+                    "No pre_save step defined for {}".format(
+                        self.get_api_name()
+                    )
+                )
+            if self.multiple and self.delete_others:
+                delete_others(
+                    data, self.model, patient=patient, episode=episode
+                )
 
 
 class FindPatientStep(Step):

opal/core/pathway/tests/pathway_test/pathways.py

@@ -8,6 +8,9 @@ class SomeComplicatedStep(steps.Step):
     step_controller = "SomeController"
     template = "Sometemplate.html"
 
+    def pre_save(self, *args, **kwargs):
+        pass
+
 
 class PagePathwayExample(pathways.PagePathway):
     display_name = "Dog Owner"

opal/core/pathway/tests/test_pathways.py

@@ -13,7 +13,9 @@
 from opal.tests.models import (
     DogOwner, Colour, PatientColour, FamousLastWords
 )
-from opal.core.pathway.tests.pathway_test.pathways import PagePathwayExample
+from opal.core.pathway.tests.pathway_test.pathways import (
+    PagePathwayExample, WizardPathwayExample
+)
 
 from opal.core.pathway.steps import Step, delete_others
 
@@ -31,15 +33,17 @@ class PathwayExample(pathways.Pathway):
         Step(model=DogOwner),
     )
 
-class ColourPathway(Pathway):
-    display_name = "colour"
+
+class FamousLastWordsPathway(Pathway):
+    display_name = "Famous Last Words"
     icon = "fa fa-something"
     template_url = "/somewhere"
 
     steps = (
         FamousLastWords,
     )
 
+
 class OveridePathway(Pathway):
 
     @classmethod
@@ -147,14 +151,22 @@ def test_init_raises(self):
     def test_pre_save_no_delete(self):
         multi_save = Step(model=Colour, multiple=True, delete_others=False)
         multi_save.pre_save(
-            {'colour': []}, Colour, patient=self.patient, episode=self.episode
+            {},
+            {'colour': []},
+            Colour,
+            patient=self.patient,
+            episode=self.episode
         )
         self.assertEqual(Colour.objects.get().id, self.existing_colour.id)
 
     def test_pre_save_with_delete(self):
         multi_save = Step(model=Colour, multiple=True)
         multi_save.pre_save(
-            {'colour': []}, Colour, patient=self.patient, episode=self.episode
+            {},
+            {'colour': []},
+            Colour,
+            patient=self.patient,
+            episode=self.episode
         )
         self.assertEqual(Colour.objects.count(), 0)
 
@@ -256,11 +268,19 @@ def test_existing_patient_new_episode_save(self):
             DogOwner.objects.filter(episode_id=episode.id).exists()
         )
 
+    def test_pre_save_called(self):
+        pathway = PathwayExample()
+        with mock.patch.object(pathway, 'pre_save') as ps:
+            patient, episode = self.new_patient_and_episode_please()
+            post_data = {"demographics": [{"hospital_number": "101"}]}
+            pathway.save(raw_data=post_data, user=self.user, patient=patient)
+        self.assertTrue(ps.called)
+
     def test_users_patient_passed_in(self):
-        pathway = PagePathwayExample()
+        pathway = PathwayExample()
         patient, episode = self.new_patient_and_episode_please()
         post_data = {"demographics": [{"hospital_number": "101"}]}
-        pathway.save(data=post_data, user=self.user, patient=patient)
+        pathway.save(raw_data=post_data, user=self.user, patient=patient)
         demographics = patient.demographics()
         self.assertEqual(
             demographics.hospital_number,
@@ -272,7 +292,10 @@ def test_users_episode_passed_in(self):
         patient, episode = self.new_patient_and_episode_please()
         post_data = {"dog_owner": [{"name": "fido"}]}
         pathway.save(
-            data=post_data, user=self.user, patient=patient, episode=episode
+            raw_data=post_data,
+            user=self.user,
+            patient=patient,
+            episode=episode
         )
         self.assertEqual(
             episode.dogowner_set.get().name,
@@ -313,7 +336,7 @@ def test_existing_patient_existing_episode_save(self):
 class TestRemoveUnChangedSubrecords(OpalTestCase):
     def setUp(self):
         self.patient, self.episode = self.new_patient_and_episode_please()
-        self.pathway_example = ColourPathway()
+        self.pathway_example = FamousLastWordsPathway()
 
     def test_dont_update_subrecords_that_havent_changed(self, subrecords):
         subrecords.return_value = [Colour]
@@ -430,7 +453,7 @@ def test_integration(self, subrecords):
         )
         dumped = json.loads(json.dumps(provided_dict, cls=OpalSerializer))
 
-        self.pathway_example.save(
+        WizardPathwayExample().save(
             dumped, self.user, self.patient, self.episode
         )
 
@@ -450,13 +473,18 @@ def setUp(self):
         self.patient, self.episode = self.new_patient_and_episode_please()
 
     def test_get_slug(self):
-        self.assertEqual('colourpathway', ColourPathway.get_slug())
+        self.assertEqual(
+            'famouslastwordspathway', FamousLastWordsPathway.get_slug()
+        )
 
     def test_get_slug_from_attribute(self):
         self.assertEqual('dog-owner', PathwayExample.get_slug())
 
     def test_get_absolute_url(self):
-        self.assertEqual('/pathway/#/colourpathway/', ColourPathway.get_absolute_url())
+        self.assertEqual(
+            '/pathway/#/famouslastwordspathway/',
+            FamousLastWordsPathway.get_absolute_url()
+        )
 
     def test_get_icon(self):
         self.assertEqual('fa fa-tintin', PathwayExample.get_icon())
@@ -465,14 +493,14 @@ def test_get_display_name(self):
         self.assertEqual('Dog Owner', PathwayExample.get_display_name())
 
     def test_as_menuitem(self):
-        menu = ColourPathway.as_menuitem()
-        self.assertEqual('/pathway/#/colourpathway/', menu.href)
-        self.assertEqual('/pathway/#/colourpathway/', menu.activepattern)
+        menu = FamousLastWordsPathway.as_menuitem()
+        self.assertEqual('/pathway/#/famouslastwordspathway/', menu.href)
+        self.assertEqual('/pathway/#/famouslastwordspathway/', menu.activepattern)
         self.assertEqual('fa fa-something', menu.icon)
-        self.assertEqual('colour', menu.display)
+        self.assertEqual('Famous Last Words', menu.display)
 
     def test_as_menuitem_from_kwargs(self):
-        menu = ColourPathway.as_menuitem(
+        menu = FamousLastWordsPathway.as_menuitem(
             href="/Blue", activepattern="/B",
             icon="fa-sea", display="Bleu"
         )
@@ -490,7 +518,9 @@ def test_as_menuitem_uses_getter_for_display(self):
         self.assertEqual('Overridden', menu.display)
 
     def test_slug(self):
-        self.assertEqual('colourpathway', ColourPathway().slug)
+            self.assertEqual(
+                'famouslastwordspathway', FamousLastWordsPathway().slug
+            )
 
     def test_get_by_hyphenated_slug(self):
         self.assertEqual(PathwayExample, Pathway.get('dog-owner'))

opal/core/pathway/tests/test_steps.py

@@ -1,6 +1,7 @@
 """
 unittests for opal.core.pathway.steps
 """
+import copy
 from django.core.urlresolvers import reverse
 from mock import MagicMock
 
@@ -80,6 +81,15 @@ def test_arguments_passed_in_overide_model(self):
             step_dict["model_api_name"], "some_model_api_step"
         )
 
+    def test_model_default(self):
+        class SomeStep(Step):
+            model = test_models.Colour
+
+        a = SomeStep()
+        self.assertEqual(
+            a.model, test_models.Colour
+        )
+
     def test_to_dict_use_class_attributes(self):
         expected = dict(
             api_name="somecomplicatedstep",
@@ -118,6 +128,41 @@ def test_model_has_no_form_template(self):
         with self.assertRaises(exceptions.MissingTemplateError):
             step.get_template()
 
+    def test_pre_save_defined(self):
+        step = Step(
+            template="some_template.html",
+            display_name="A Step",
+            api_name="a_step"
+        )
+
+        with self.assertRaises(NotImplementedError) as er:
+            step.pre_save({}, {}, self.user)
+        self.assertEqual(
+            str(er.exception),
+            "No pre_save step defined for a_step"
+        )
+
+    def test_pre_save(self):
+        class SomeStep(Step):
+            model = test_models.Colour
+
+        step = SomeStep()
+        data = {}
+        colour_data = {
+            test_models.Colour.get_api_name(): [
+                dict(name="blue"),
+                dict(name="green")
+            ],
+        }
+        raw_data = copy.copy(colour_data)
+        raw_data["other"] = [
+            {"other": "data"}
+        ]
+        step.pre_save(data, raw_data, self.user)
+        self.assertEqual(
+            data, colour_data
+        )
+
 
 class HelpTextStepTestCase(OpalTestCase):
     def test_get_help_text(self):