Skip to content

Commit

Permalink
Merge a5ec36d into 3282503
Browse files Browse the repository at this point in the history
  • Loading branch information
davidmiller committed Oct 26, 2018
2 parents 3282503 + a5ec36d commit d630279
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 5 deletions.
20 changes: 20 additions & 0 deletions changelog.md
Expand Up @@ -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,
Expand Down
33 changes: 32 additions & 1 deletion 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.
Expand Down Expand Up @@ -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"

```
12 changes: 11 additions & 1 deletion doc/docs/reference/upgrading.md
Expand Up @@ -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.
Expand All @@ -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
Expand Down
17 changes: 14 additions & 3 deletions opal/core/fields.py
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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:
Expand Down
9 changes: 9 additions & 0 deletions opal/tests/models.py
Expand Up @@ -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
Expand Down
72 changes: 72 additions & 0 deletions 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

Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit d630279

Please sign in to comment.