diff --git a/changelog.md b/changelog.md index 210a968b9..f6279998d 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,19 @@ ### 0.13.0 (Major Release) +#### Episode.active + +The field `Episode.active` was previously implicitly set when calling `.set_tag_names()` to +something equivalent to the value of `bool(len(tag_names) > 0)`. + +As of 0.13.0 the value of `Episode.active` is checked whenever `.save()` is called, prior +to the database call. The correct value is looked up via `Episode.category.is_active()`. + +The default calculation of `.active` has also changed to be roughly equivalent to +` bool(self.episode.end is None)`. + +Applications are now able to easily _change_ this behaviour by overriding the `.is_active` +method of the relevant `EpisodeCategory`. + #### Coding systems for lookuplists Lookuplist entries may now have an associated coding system and code value stored against them. diff --git a/doc/docs/guides/episodes.md b/doc/docs/guides/episodes.md index d7cae2ebd..b69ebb9cb 100644 --- a/doc/docs/guides/episodes.md +++ b/doc/docs/guides/episodes.md @@ -54,25 +54,6 @@ class Application(application.OpalApplication): default_episode_category = MyCategory.display_name ``` -## Episode stages - -An Episode will frequently consist of a number of possible stages. For instance, -for an inpatient episode, a patient will first be an inpatient, and then an -be discharged, with an optional interim follow up stage for inpatients who have been -discharged but requrie further follow up. - -Opal stores the stage of an episode as a string in the `stage` property of an -`Episode`. The valid possible stages for a category are accessed from the -`get_stages` method of the category. - -``` -episode.category.get_stages() -# ['Inpatient', 'Followup', 'Discharged'] - -episode.category.has_stage('Followup') -# True -``` - ## Defining your own EpisodeCategory As EpisodeCategory is a [discoverable](discoverable) we can define our own to @@ -92,3 +73,55 @@ class DropInClinicEpisode(episodes.EpisodeCategory): detail_template = "detail/drop_in.html" ``` + +## Episode.active + +The field `.active` is used to distinguish Episodes which are ongoing. This field +is set implicitly by the `Episode.save()` method, and there will generally be no +need to set this field directly as part of application code. + +Whether an Episode is considered active is determined by the `.is_active()` method +of the relevant EpisodeCategory. + +The default implementation considers any Episode without an `.end` date to be active +and any Episode with one to be inactive. + +Applications may customise this behaviour by overriding the `.is_active()` method. + +For instance, to create a category which considered any Episode older than 2 weeks to +be inactive, one might override the method as follows: + +```python +# yourapp/episode_categories.py +import datetime +from opal.core import episodes + + +class TwoWeeksAndStopCaringEpisode(episodes.EpisodeCategory): + def is_active(self): + delta = datetime.date.today() - self.episode.start + return bool(delta >= datetime.timedelta(days=14)) + +``` + +(Note that this would not alter the value in the database immediately after those 2 +weeks, but would alter the value the next time the `Episode.save()` method was called.) + +## Episode stages + +An Episode will frequently consist of a number of possible stages. For instance, +for an inpatient episode, a patient will first be an inpatient, and then an +be discharged, with an optional interim follow up stage for inpatients who have been +discharged but requrie further follow up. + +Opal stores the stage of an episode as a string in the `stage` property of an +`Episode`. The valid possible stages for a category are accessed from the +`get_stages` method of the category. + +``` +episode.category.get_stages() +# ['Inpatient', 'Followup', 'Discharged'] + +episode.category.has_stage('Followup') +# True +``` diff --git a/doc/docs/reference/episode_categories.md b/doc/docs/reference/episode_categories.md index a432fde3c..0010eaa47 100644 --- a/doc/docs/reference/episode_categories.md +++ b/doc/docs/reference/episode_categories.md @@ -65,6 +65,17 @@ InpatientEpisode(episode).has_stage('Inpatient') # -> True ``` +### EpisodeCategory.is_active() + +Predicate function to determine whether this episode is active. + +The default implementation looks to see whether an end date has +been set on the episode. + +```python +InpatientEpisode(Episode()).is_active() +# -> True +``` ### EpisodeCategory.set_stage(stage, user, data) diff --git a/opal/core/episodes.py b/opal/core/episodes.py index 08b900726..a4ed3e1ac 100644 --- a/opal/core/episodes.py +++ b/opal/core/episodes.py @@ -50,6 +50,15 @@ def episode_visible_to(kls, episode, user): def __init__(self, episode): self.episode = episode + def is_active(self): + """ + Predicate function to determine whether this episode is active. + + The default implementation looks to see whether an end date has + been set on the episode. + """ + return bool(self.episode.end is None) + def get_stages(self): """ Return the list of string stages for this category diff --git a/opal/core/exceptions.py b/opal/core/exceptions.py index 454a57c66..ad42b62cf 100644 --- a/opal/core/exceptions.py +++ b/opal/core/exceptions.py @@ -39,5 +39,9 @@ class MissingTemplateError(Error): pass +class UnexpectedEpisodeCategoryNameError(Error): + pass + + class InvalidDataError(Error): pass diff --git a/opal/models.py b/opal/models.py index 32d31b7bc..1c2ed837d 100644 --- a/opal/models.py +++ b/opal/models.py @@ -689,6 +689,10 @@ class Episode(UpdatesFromDictMixin, TrackedModel): objects = managers.EpisodeQueryset.as_manager() + def __init__(self, *args, **kwargs): + super(Episode, self).__init__(*args, **kwargs) + self.__original_active = self.active + def __unicode__(self): return 'Episode {0}: {1} - {2}'.format( self.pk, self.start, self.end @@ -696,7 +700,31 @@ def __unicode__(self): def save(self, *args, **kwargs): created = not bool(self.id) + + current_active_value = self.active + category_active_value = self.category.is_active() + + if current_active_value != category_active_value: # Disagreement + if current_active_value != self.__original_active: + # The value of self.active has been set by some code somewhere + # not by __init__() e.g. the original database value at the + # time of instance initalization. + # + # Rather than overriding this silently we should raise a + # ValueError. + msg = "Value of Episode.active has been set to {} but " \ + "category.is_active() returns {}" + raise ValueError( + msg.format(current_active_value, category_active_value) + ) + + self.active = category_active_value super(Episode, self).save(*args, **kwargs) + + # Re-set this in case we changed it once post initialization and then + # the user subsequently saves this instance again + self.__original_active = self.active + if created: for subclass in episode_subrecords(): if subclass._is_singleton: @@ -705,10 +733,16 @@ def save(self, *args, **kwargs): @property def category(self): from opal.core import episodes - category = episodes.EpisodeCategory.filter( + categories = episodes.EpisodeCategory.filter( display_name=self.category_name - )[0] - return category(self) + ) + if len(categories) == 0: + msg = "Unable to find EpisodeCategory for category name {0}" + msg = msg.format(self.category_name) + raise exceptions.UnexpectedEpisodeCategoryNameError(msg) + else: + category = categories[0] + return category(self) def visible_to(self, user): """ @@ -731,20 +765,12 @@ def set_stage(self, stage, user, data): def set_tag_names(self, tag_names, user): """ - 1. Set the episode.active status - 2. Special case mine - 3. Archive dangling tags not in our current list. - 4. Add new tags. - 5. Ensure that we're setting the parents of child tags - 6. There is no step 6. + 1. Special case mine + 2. Archive dangling tags not in our current list. + 3. Add new tags. + 4. Ensure that we're setting the parents of child tags + 5. There is no step 6. """ - if len(tag_names) and not self.active: - self.active = True - self.save() - elif not len(tag_names) and self.active: - self.active = False - self.save() - if "mine" not in tag_names: self.tagging_set.filter(user=user, value='mine').update(archived=True) diff --git a/opal/tests/test_core_episodes.py b/opal/tests/test_core_episodes.py index 8c701da17..d72f4e75a 100644 --- a/opal/tests/test_core_episodes.py +++ b/opal/tests/test_core_episodes.py @@ -1,6 +1,8 @@ """ Unittests for opal.core.episodes """ +import datetime + from django.contrib.auth.models import User from opal.core import test @@ -17,7 +19,7 @@ def setUp(self): ) self.patient = Patient.objects.create() self.inpatient_episode = self.patient.create_episode( - category_name=episodes.InpatientEpisode + category_name=episodes.InpatientEpisode.display_name ) def test_episode_categories(self): @@ -42,6 +44,15 @@ def test_for_category(self): self.assertEqual(episodes.InpatientEpisode, episodes.EpisodeCategory.get('inpatient')) + def test_is_active(self): + category = episodes.InpatientEpisode(self.inpatient_episode) + self.assertTrue(category.is_active()) + + def test_is_active_end_date_set(self): + self.inpatient_episode.end = datetime.date.today() + category = episodes.InpatientEpisode(self.inpatient_episode) + self.assertFalse(category.is_active()) + def test_get_stages(self): stages = [ 'Inpatient', diff --git a/opal/tests/test_episode.py b/opal/tests/test_episode.py index 61bd0a063..6d9c7ed93 100644 --- a/opal/tests/test_episode.py +++ b/opal/tests/test_episode.py @@ -7,10 +7,11 @@ from django.contrib.auth.models import User -from opal.core import application, subrecords +from opal.core import application, exceptions, subrecords from opal.core.episodes import InpatientEpisode from opal.core.test import OpalTestCase from opal.models import Patient, Episode, Tagging, UserProfile +from opal import models from opal.tests import test_patient_lists # ensure the lists are loaded from opal.tests.models import ( @@ -27,10 +28,32 @@ def setUp(self): def test_singleton_subrecord_created(self): self.assertEqual(1, self.episode.episodename_set.count()) + def test_init_sets_original_active_value(self): + episode = models.Episode() + self.assertEqual(episode.active, episode._Episode__original_active) + + def test_save_sets_active_when_category_is_active_is_true(self): + with patch.object(self.episode.category, 'is_active') as activep: + activep.return_value = True + self.episode.save() + self.assertTrue(self.episode.active) + + def test_save_sets_active_when_category_is_active_is_false(self): + self.episode.end = datetime.date.today() + self.episode.save() + self.assertFalse(self.episode.active) + + def test_user_value_of_active_disagrees_with_category(self): + self.episode.active = False + with self.assertRaises(ValueError): + self.episode.save() + @patch('opal.models.application.get_app') - def test_default_category_name(self, getter): + @patch('opal.core.episodes.EpisodeCategory') + def test_default_category_name(self, category, getter): mock_app = getter.return_value mock_app.default_episode_category = 'MyEpisodeCategory' + category.filter.return_value = [InpatientEpisode] episode = self.patient.create_episode() self.assertEqual('MyEpisodeCategory', episode.category_name) @@ -43,6 +66,11 @@ def test_category(self): self.assertEqual(self.episode.category.__class__, InpatientEpisode) self.assertEqual(self.episode.category.episode, self.episode) + def test_category_name_not_found(self): + self.episode.category_name = 'Not A Real Category' + with self.assertRaises(exceptions.UnexpectedEpisodeCategoryNameError): + cat = self.episode.category + def test_visible_to(self): self.assertTrue(self.episode.visible_to(self.user)) @@ -89,13 +117,17 @@ def test_user_cannot_see_other_users_mine_tag(self): self.episode.set_tag_names(['carnivore', 'mine'], self.user) self.assertEqual(['carnivore'], list(self.episode.get_tag_names(other_user))) - def test_active_if_tagged_by_non_mine_tag(self): + def test_active_unchanged_if_tagged_by_non_mine_tag(self): + # Regression test - see github.com/openhealthcare/opal#1578 for details + before = self.episode.active self.episode.set_tag_names(['carnivore'], self.user) - self.assertTrue(self.episode.active) + self.assertEqual(before, self.episode.active) - def test_active_if_only_tagged_by_mine_tag(self): + def test_active_unchanged_if_only_tagged_by_mine_tag(self): + # Regression test - see github.com/openhealthcare/opal#1578 for details + before = self.episode.active self.episode.set_tag_names(['mine'], self.user) - self.assertTrue(self.episode.active) + self.assertEqual(before, self.episode.active) def test_set_tag_names_from_tagging_dict(self): self.episode.set_tag_names_from_tagging_dict({'inpatient': True}, self.user) diff --git a/opal/tests/test_models.py b/opal/tests/test_models.py index 24a7b29b7..fefdd74de 100644 --- a/opal/tests/test_models.py +++ b/opal/tests/test_models.py @@ -9,7 +9,7 @@ from django.utils import timezone from opal import models -from opal.core import exceptions, subrecords +from opal.core import application, exceptions, subrecords from opal.models import ( Subrecord, Tagging, Patient, InpatientAdmission, Symptom, ) @@ -64,18 +64,17 @@ def test_to_dict_episode_identical_to_episode_to_dict(self): episode_dict = episode.to_dict(self.user) self.assertEqual(episode_dict, patient.to_dict(self.user)['episodes'][episode.id]) - @patch("opal.models.application.get_app") - def test_created_with_the_default_episode(self, get_app): - test_app = MagicMock() - test_app.default_episode_category ="testcategory" - get_app.return_value = test_app + def test_created_with_the_default_episode(self): _, episode = self.new_patient_and_episode_please() - self.assertEqual(episode.category_name, "testcategory") + self.assertEqual( + application.get_app().default_episode_category, + episode.category_name + ) def test_create_episode_category(self): patient = models.Patient.objects.create() - e = patient.create_episode(category_name='testcategory') - self.assertEqual('testcategory', e.category_name) + e = patient.create_episode(category_name='Outpatient') + self.assertEqual('Outpatient', e.category_name) def test_bulk_update_patient_subrecords(self): original_patient = models.Patient() diff --git a/opal/tests/test_patient.py b/opal/tests/test_patient.py index 819eb7b0c..308dd31b1 100644 --- a/opal/tests/test_patient.py +++ b/opal/tests/test_patient.py @@ -1,6 +1,8 @@ """ Unittests for Patients """ +import datetime + from mock import patch from opal.core.test import OpalTestCase @@ -27,13 +29,14 @@ def test_can_create_episode(self): def test_get_active_episode(self): self.patient.create_episode() episode2 = self.patient.create_episode() - episode2.set_tag_names(['microbiology'], None) + episode2.active = True + episode2.save() self.assertEqual(episode2.id, self.patient.get_active_episode().id) def test_get_active_episode_with_no_episodes(self): self.assertIsNone(self.patient.get_active_episode()) def test_get_active_episode_with_no_active_episodes(self): - self.patient.create_episode() - self.patient.create_episode() + self.patient.create_episode(end=datetime.date.today()) + self.patient.create_episode(end=datetime.date.today()) self.assertIsNone(self.patient.get_active_episode()) diff --git a/opal/tests/test_patient_lists.py b/opal/tests/test_patient_lists.py index e97c8536f..23fa89ab6 100644 --- a/opal/tests/test_patient_lists.py +++ b/opal/tests/test_patient_lists.py @@ -1,6 +1,7 @@ """ Unittests for opal.core.patient_lists """ +import datetime import os from django.core.urlresolvers import reverse @@ -268,7 +269,7 @@ def test_to_dict_passes_queryset(self, serialised): def test_to_dict_inactive_episodes(self): p, e = self.new_patient_and_episode_please() - e.active = False + e.end = datetime.date.today() e.save() class All(patient_lists.PatientList): @@ -454,7 +455,7 @@ def test_to_dict_inactive_episodes(self): p, e = self.new_patient_and_episode_please() e.set_tag_names(['carnivore'], self.user) self.assertEqual(e.pk, TaggingTestNotSubTag().to_dict(self.user)[0]['id']) - e.active = False + e.end = datetime.date.today() e.save() self.assertEqual(1, len(TaggingTestNotSubTag().to_dict(self.user)))