From 4958bfd04b884d3c17b84e3f90eaf89d84dfd042 Mon Sep 17 00:00:00 2001 From: Iwo Herka Date: Mon, 4 Jun 2018 17:19:05 +0200 Subject: [PATCH] Remove Sites framework dependency (#9) (#15) This merge does two things: 1. Removes completely dependency on the Sites framework (which was implemented erroneously). Specifically, site field on Attribute and related constraints. With those changes, attributes are now global. That is, all attributes are visible to all entities. As a result, slug in now unique. 2. Content-type field is used to limit entities that can be assigned an attribute (see get_all_attributes method on Entity). This way attributes with content_type assigned are, for all intended purposes, private to Entity of that content type. The problem is that those attributes are still visible to other entities. Moreover, originally, attributes had non-unique slugs that are unique together with content_type and site. With removal of site (and because content_type is nullable) a situation might exist in which two attributes exist with the same slug/name but one with content_type and one with NULL. Simple solution to all this hassle is to drop private attributes altogether, which is what this pull-request does. Resolves: #9 Commits: * Remove sites dependency; fix Attribute uniqueness (#9) * Add uniqueness test to attributes * Remove content_type from Attribute class * Update runtests * Remove redudant call * Fix wrong assertion in attribute test * Fix string formatting --- eav/admin.py | 3 +- eav/migrations/0001_initial.py | 35 +++++-------------- eav/migrations/0002_auto_20161014_0157.py | 35 ------------------- eav/models.py | 23 +++--------- eav/registry.py | 2 +- runtests | 2 +- .../{limiting_attributes.py => attributes.py} | 24 +++++++++---- 7 files changed, 33 insertions(+), 91 deletions(-) delete mode 100644 eav/migrations/0002_auto_20161014_0157.py rename tests/{limiting_attributes.py => attributes.py} (70%) diff --git a/eav/admin.py b/eav/admin.py index 120fad3..884d297 100644 --- a/eav/admin.py +++ b/eav/admin.py @@ -76,8 +76,7 @@ class BaseEntityInline(InlineModelAdmin): return [(None, {'fields': form.fields.keys()})] class AttributeAdmin(ModelAdmin): - list_display = ('name', 'content_type', 'slug', 'datatype', 'description', 'site') - list_filter = ['site'] + list_display = ('name', 'slug', 'datatype', 'description') prepopulated_fields = {'slug': ('name',)} admin.site.register(Attribute, AttributeAdmin) diff --git a/eav/migrations/0001_initial.py b/eav/migrations/0001_initial.py index 89d6ba3..c57ab5a 100644 --- a/eav/migrations/0001_initial.py +++ b/eav/migrations/0001_initial.py @@ -1,11 +1,7 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.9.10 on 2016-10-13 05:56 -from __future__ import unicode_literals +# Generated by Django 2.0.4 on 2018-06-01 09:36 -import django.contrib.sites.managers from django.db import migrations, models import django.db.models.deletion -import django.db.models.manager import django.utils.timezone import eav.fields @@ -16,7 +12,6 @@ class Migration(migrations.Migration): dependencies = [ ('contenttypes', '0002_remove_content_type_name'), - ('sites', '0001_initial'), ] operations = [ @@ -25,21 +20,18 @@ class Migration(migrations.Migration): fields=[ ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), ('name', models.CharField(help_text='User-friendly attribute name', max_length=100, verbose_name='name')), - ('slug', eav.fields.EavSlugField(help_text='Short unique attribute label', verbose_name='slug')), + ('slug', eav.fields.EavSlugField(help_text='Short unique attribute label', unique=True, verbose_name='slug')), ('description', models.CharField(blank=True, help_text='Short description', max_length=256, null=True, verbose_name='description')), ('type', models.CharField(blank=True, max_length=20, null=True, verbose_name='type')), - ('datatype', eav.fields.EavDatatypeField(choices=[(b'text', 'Text'), (b'float', 'Float'), (b'int', 'Integer'), (b'date', 'Date'), (b'bool', 'True / False'), (b'object', 'Django Object'), (b'enum', 'Multiple Choice')], max_length=6, verbose_name='data type')), + ('datatype', eav.fields.EavDatatypeField(choices=[('text', 'Text'), ('float', 'Float'), ('int', 'Integer'), ('date', 'Date'), ('bool', 'True / False'), ('object', 'Django Object'), ('enum', 'Multiple Choice')], max_length=6, verbose_name='data type')), ('created', models.DateTimeField(default=django.utils.timezone.now, editable=False, verbose_name='created')), ('modified', models.DateTimeField(auto_now=True, verbose_name='modified')), ('required', models.BooleanField(default=False, verbose_name='required')), + ('display_order', models.PositiveIntegerField(default=1, verbose_name='display order')), ], options={ 'ordering': ['name'], }, - managers=[ - ('objects', django.db.models.manager.Manager()), - ('on_site', django.contrib.sites.managers.CurrentSiteManager()), - ], ), migrations.CreateModel( name='EnumGroup', @@ -68,10 +60,10 @@ class Migration(migrations.Migration): ('generic_value_id', models.IntegerField(blank=True, null=True)), ('created', models.DateTimeField(default=django.utils.timezone.now, verbose_name='created')), ('modified', models.DateTimeField(auto_now=True, verbose_name='modified')), - ('attribute', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='eav.Attribute', verbose_name='attribute')), - ('entity_ct', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='value_entities', to='contenttypes.ContentType')), - ('generic_value_ct', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='value_values', to='contenttypes.ContentType')), - ('value_enum', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='eav_values', to='eav.EnumValue')), + ('attribute', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, to='eav.Attribute', verbose_name='attribute')), + ('entity_ct', models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='value_entities', to='contenttypes.ContentType')), + ('generic_value_ct', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='value_values', to='contenttypes.ContentType')), + ('value_enum', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, related_name='eav_values', to='eav.EnumValue')), ], ), migrations.AddField( @@ -82,15 +74,6 @@ class Migration(migrations.Migration): migrations.AddField( model_name='attribute', name='enum_group', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='eav.EnumGroup', verbose_name='choice group'), - ), - migrations.AddField( - model_name='attribute', - name='site', - field=models.ForeignKey(default=1, on_delete=django.db.models.deletion.CASCADE, to='sites.Site', verbose_name='site'), - ), - migrations.AlterUniqueTogether( - name='attribute', - unique_together=set([('site', 'slug')]), + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.PROTECT, to='eav.EnumGroup', verbose_name='choice group'), ), ] diff --git a/eav/migrations/0002_auto_20161014_0157.py b/eav/migrations/0002_auto_20161014_0157.py deleted file mode 100644 index 70c3bd9..0000000 --- a/eav/migrations/0002_auto_20161014_0157.py +++ /dev/null @@ -1,35 +0,0 @@ -# -*- coding: utf-8 -*- -# Generated by Django 1.9.10 on 2016-10-13 16:57 -from __future__ import unicode_literals - -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ('contenttypes', '0002_remove_content_type_name'), - ('eav', '0001_initial'), - ] - - operations = [ - migrations.AlterModelOptions( - name='attribute', - options={'ordering': ['content_type', 'name']}, - ), - migrations.AddField( - model_name='attribute', - name='content_type', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='contenttypes.ContentType', verbose_name='content type'), - ), - migrations.AddField( - model_name='attribute', - name='display_order', - field=models.PositiveIntegerField(default=1, verbose_name='display order'), - ), - migrations.AlterUniqueTogether( - name='attribute', - unique_together=set([('site', 'content_type', 'slug')]), - ), - ] diff --git a/eav/models.py b/eav/models.py index 30982f5..e86c49e 100644 --- a/eav/models.py +++ b/eav/models.py @@ -13,8 +13,6 @@ Along with the :class:`Entity` helper class. from django.conf import settings from django.contrib.contenttypes import fields as generic from django.contrib.contenttypes.models import ContentType -from django.contrib.sites.managers import CurrentSiteManager -from django.contrib.sites.models import Site from django.core.exceptions import ValidationError from django.db import models from django.utils import timezone @@ -128,8 +126,7 @@ class Attribute(models.Model): ''' class Meta: - ordering = ['content_type', 'name'] - unique_together = ('site', 'content_type', 'slug') + ordering = ['name'] TYPE_TEXT = 'text' TYPE_FLOAT = 'float' @@ -152,17 +149,7 @@ class Attribute(models.Model): name = models.CharField(_(u"name"), max_length=100, help_text=_(u"User-friendly attribute name")) - content_type = models.ForeignKey(ContentType, - on_delete=models.PROTECT, - blank=True, null=True, - verbose_name=_(u"content type")) - - site = models.ForeignKey(Site, verbose_name=_(u"site"), - on_delete=models.PROTECT, - blank=True, null=True, - default=settings.SITE_ID) - - slug = EavSlugField(_(u"slug"), max_length=50, db_index=True, + slug = EavSlugField(_(u"slug"), max_length=50, db_index=True, unique=True, help_text=_(u"Short unique attribute label")) description = models.CharField(_(u"description"), max_length=256, @@ -192,7 +179,6 @@ class Attribute(models.Model): display_order = models.PositiveIntegerField(_(u"display order"), default=1) objects = models.Manager() - on_site = CurrentSiteManager() def get_validators(self): ''' @@ -298,7 +284,7 @@ class Attribute(models.Model): value_obj.save() def __unicode__(self): - return u"%s.%s (%s)" % (self.content_type, self.name, self.get_datatype_display()) + return u"%s (%s)" % (self.name, self.get_datatype_display()) class Value(models.Model): @@ -433,8 +419,7 @@ class Entity(object): Return a query set of all :class:`Attribute` objects that can be set for this entity. ''' - return self.model._eav_config_cls.get_attributes().filter( - models.Q(content_type__isnull=True) | models.Q(content_type=self.ct)).order_by('display_order') + return self.model._eav_config_cls.get_attributes().order_by('display_order') def _hasattr(self, attribute_slug): ''' diff --git a/eav/registry.py b/eav/registry.py index b4b0896..8b463ac 100644 --- a/eav/registry.py +++ b/eav/registry.py @@ -39,7 +39,7 @@ class EavConfig(object): By default, all :class:`~eav.models.Attribute` object apply to an entity, unless you provide a custom EavConfig class overriding this. ''' - return Attribute.on_site.all() + return Attribute.objects.all() class Registry(object): diff --git a/runtests b/runtests index 6713756..4a79792 100755 --- a/runtests +++ b/runtests @@ -17,7 +17,7 @@ if __name__ == "__main__": 'tests.queries', 'tests.registry', 'tests.data_validation', - 'tests.limiting_attributes', + 'tests.attributes', 'tests.misc_models', 'tests.set_and_get' ] diff --git a/tests/limiting_attributes.py b/tests/attributes.py similarity index 70% rename from tests/limiting_attributes.py rename to tests/attributes.py index 9a41ad6..8ac5f51 100644 --- a/tests/limiting_attributes.py +++ b/tests/attributes.py @@ -1,13 +1,17 @@ +from django.core.exceptions import ValidationError from django.test import TestCase import eav -from eav.registry import EavConfig from eav.models import Attribute, Value +from eav.registry import EavConfig -from .models import Patient, Encounter +from .models import Encounter, Patient -class LimittingAttributes(TestCase): +class Attributes(TestCase): + ''' + TODO: Explain this test. + ''' def setUp(self): class EncounterEavConfig(EavConfig): @@ -26,16 +30,22 @@ class LimittingAttributes(TestCase): Attribute.objects.create(name='age', datatype=Attribute.TYPE_INT) Attribute.objects.create(name='height', datatype=Attribute.TYPE_FLOAT) Attribute.objects.create(name='weight', datatype=Attribute.TYPE_FLOAT) + Attribute.objects.create(name='color', datatype=Attribute.TYPE_TEXT) def tearDown(self): eav.unregister(Encounter) eav.unregister(Patient) def test_get_attribute_querysets(self): - self.assertEqual(Patient._eav_config_cls \ - .get_attributes().count(), 3) - self.assertEqual(Encounter._eav_config_cls \ - .get_attributes().count(), 1) + self.assertEqual(Patient._eav_config_cls.get_attributes().count(), 4) + self.assertEqual(Encounter._eav_config_cls.get_attributes().count(), 1) + + def test_duplicate_attributs(self): + ''' + Ensure that no two Attributes with the same slug can exist. + ''' + with self.assertRaises(ValidationError): + Attribute.objects.create(name='height', datatype=Attribute.TYPE_FLOAT) def test_setting_attributes(self): p = Patient.objects.create(name='Jon')