diff --git a/changelog.md b/changelog.md index 7ac8d336d..20ee352fd 100644 --- a/changelog.md +++ b/changelog.md @@ -38,6 +38,26 @@ hand craft data files for new applications. Use of `Model._title` to set a display name of a subrecord has issued a warning for several releases - this has now been removed and will no longer work. + +#### Free text or foreign key fields are now, by default case insensitive + +This can be adjusted with a flag on the field. + +Existing fk_or_ft fields could therefore still have the field set as free text. + +This change is not accompanied by a retrospective migration so your existing fk_or_ft may be +stored in a case sensitive manner. It is recommended you migrate all of your fk_or_ft fields +as this will give you consistent behaviour. + +##### For example. + +Prior to this change if I had an allergy for "paracetomol" but an entry in the models.Drug +table of "Paracetomol", it would be stored as free text in the `Allergies.drug` field, because +it was case sensitive. Going forward after this change it will be saved as a foreign key. This +change will not be made retrospecively however so you would need to add a migration that resaved +the Allergies.drug. + + #### Misc Changes * The undocumented Reopen Episode flow included in Opal < 0.8.0 has now been completely removed, diff --git a/doc/docs/reference/core_fields.md b/doc/docs/reference/core_fields.md index 107f0dba0..018ec1591 100644 --- a/doc/docs/reference/core_fields.md +++ b/doc/docs/reference/core_fields.md @@ -1,4 +1,4 @@ -# opal.core.fields + # opal.core.fields The `opal.core.fields` module contains helper functions for working with fields, as well as custom Opal field definitions. @@ -32,3 +32,34 @@ enum('one', '2', 'III') A field that stores it's value as a generic foreign key to an Opal LookupList or as the value in a CharField. + +By default this is case insensitive, pass in `case_sensitive=True` to make +it case sensitive when matching against lookup lists or synonyms. + +e.g. +```python +class Duck(object): + name = ForeignKeyOrFreeText(Name) + show = ForiegnKeyOrFreeText(Show, case_sensitive=True) + +Name.objects.create(name="Scrooge") +Show.objects.create(name="Duck Tales") + +scrooge = Duck() + +# by default we are case insensitive, so this will be saved +# as a foreign key to a Name object +scrooge.name = "scrooge" +# ie now +scrooge.name == "Scrooge" +scrooge.name_fk == Name.objects.get(name="Scrooge") + + +# this is not a case sensitive field so will be stored on the model as free +# text +scrooge.show = "duck tales" + +# ie +scrooge.show_ft == "duck tales" + +``` diff --git a/doc/docs/reference/upgrading.md b/doc/docs/reference/upgrading.md index 99b885d63..7497cc2cc 100644 --- a/doc/docs/reference/upgrading.md +++ b/doc/docs/reference/upgrading.md @@ -3,6 +3,15 @@ This document provides instructions for specific steps required to upgrading your Opal application to a later version where there are extra steps required. +### 0.13.0 -> 0.12.0 - 0.11.2 + +#### Free text or foreign key fields are now, by default case insensitive + +It is recommended you resave all model values for fk_or_ft fields as this will give you +consistent behaviour. Otherwise fk_ft values wihch differ from fkt values only by +case prior to this upgrade will be stored as ft and those afterwards will be stored as +the relevant fk. + ### 0.11.1 -> 0.11.2 This bugfix release should be entirely backwards compatible. @@ -15,7 +24,8 @@ This bugfix release should be entirely backwards compatible. Please upgrade django-compressor version to 2.2, ie update your requirements to -# requirements.txt +##### requirements.txt + django-compressor==2.2 ### 0.10.0 -> 0.10.1 diff --git a/opal/core/fields.py b/opal/core/fields.py index 02cd75f19..8973c1f36 100644 --- a/opal/core/fields.py +++ b/opal/core/fields.py @@ -42,6 +42,7 @@ def __init__( related_name=None, verbose_name=None, help_text=None, + case_sensitive=False, default=None ): self.foreign_model = foreign_model @@ -50,6 +51,11 @@ def __init__( self.default = default self.help_text = help_text + if case_sensitive: + self.fk_synonym_lookup_arg = "name" + else: + self.fk_synonym_lookup_arg = "name__iexact" + # for use in the fields, lookup lists essentially have # a max length based on the char field that's used internally self.max_length = 255 @@ -123,8 +129,11 @@ def __set__(self, inst, val): val = val.strip() try: from opal.models import Synonym - synonym = Synonym.objects.get( - content_type=content_type, name=val) + kwargs = { + "content_type": content_type, + self.fk_synonym_lookup_arg: val + } + synonym = Synonym.objects.get(**kwargs) vals.append(synonym.content_object.name) except Synonym.DoesNotExist: vals.append(val) @@ -135,7 +144,9 @@ def __set__(self, inst, val): else: val = vals[0] try: - foreign_obj = self.foreign_model.objects.get(name=val) + foreign_obj = self.foreign_model.objects.get( + **{self.fk_synonym_lookup_arg: val} + ) setattr(inst, self.fk_field_name, foreign_obj) setattr(inst, self.ft_field_name, '') except self.foreign_model.DoesNotExist: diff --git a/opal/tests/models.py b/opal/tests/models.py index f6fc3df35..e0f56cc4b 100644 --- a/opal/tests/models.py +++ b/opal/tests/models.py @@ -134,6 +134,15 @@ class Meta: proxy = True +class SensitiveDogOwner(models.EpisodeSubrecord): + name = dmodels.CharField( + max_length=200, default="Catherine" + ) + dog = fields.ForeignKeyOrFreeText( + Dog, case_sensitive=True + ) + + class Colour(models.EpisodeSubrecord): _advanced_searchable = False _exclude_from_extract = True diff --git a/opal/tests/test_core_fields.py b/opal/tests/test_core_fields.py index d8449654f..630c01270 100644 --- a/opal/tests/test_core_fields.py +++ b/opal/tests/test_core_fields.py @@ -1,6 +1,7 @@ """ Unittests for the opal.core.fields module """ +import mock from django.contrib.contenttypes.models import ContentType from django.db import models @@ -74,6 +75,77 @@ def test_synonyms_addition(self): alsation_owner.dog = "German Shepherd" self.assertEqual(alsation_owner.dog, "Alsation") + def test_case_insensitive_fk(self): + alsation = test_models.Dog.objects.create(name="Alsation") + _, episode = self.new_patient_and_episode_please() + alsation_owner = test_models.SpanielOwner.objects.create( + episode=episode + ) + alsation_owner.dog = alsation.name.lower() + alsation_owner.save() + self.assertEqual(alsation_owner.dog, alsation.name) + self.assertEqual(alsation_owner.dog_fk_id, alsation.id) + + def test_case_insensitive_synonym(self): + ct = ContentType.objects.get_for_model( + test_models.Dog + ) + alsation = test_models.Dog.objects.create(name="Alsation") + Synonym.objects.create( + content_type=ct, + name="German Shepherd", + object_id=alsation.id + ) + _, episode = self.new_patient_and_episode_please() + alsation_owner = test_models.SpanielOwner.objects.create( + episode=episode + ) + alsation_owner.dog = alsation.name.lower() + alsation_owner.save() + alsation_owner.dog = "german shepherd" + self.assertEqual(alsation_owner.dog, "Alsation") + self.assertEqual(alsation_owner.dog_fk_id, alsation.id) + + def test_case_sensitive_fk(self): + alsation = test_models.Dog.objects.create(name="Alsation") + _, episode = self.new_patient_and_episode_please() + alsation_owner = test_models.SensitiveDogOwner.objects.create( + episode=episode + ) + alsation_owner.dog = alsation.name.lower() + alsation_owner.save() + self.assertEqual(alsation_owner.dog, alsation.name.lower()) + self.assertEqual(alsation_owner.dog_ft, alsation.name.lower()) + + alsation_owner.dog = alsation.name + alsation_owner.save() + self.assertEqual(alsation_owner.dog, alsation.name) + self.assertEqual(alsation_owner.dog_fk_id, alsation.id) + + def test_case_sensitive_synonym(self): + ct = ContentType.objects.get_for_model( + test_models.Dog + ) + alsation = test_models.Dog.objects.create(name="Alsation") + Synonym.objects.create( + content_type=ct, + name="German Shepherd", + object_id=alsation.id + ) + _, episode = self.new_patient_and_episode_please() + alsation_owner = test_models.SensitiveDogOwner.objects.create( + episode=episode + ) + alsation_owner.dog = alsation.name.lower() + alsation_owner.save() + alsation_owner.dog = "german shepherd" + self.assertEqual(alsation_owner.dog, "german shepherd") + self.assertEqual(alsation_owner.dog_ft, "german shepherd") + + alsation_owner.dog = "German Shepherd" + self.assertEqual(alsation_owner.dog, alsation.name) + self.assertEqual(alsation_owner.dog_fk_id, alsation.id) + def test_delete(self): alsation = test_models.Dog.objects.create(name="Alsation") _, episode = self.new_patient_and_episode_please()