From 151c2acaa5f125fa74a178de7713d66eca33c1c4 Mon Sep 17 00:00:00 2001 From: Peter Siemens Date: Tue, 30 Oct 2018 22:00:03 -0700 Subject: [PATCH 1/7] Add unique_together constraints to Article, Page models --- dispatch/modules/content/models.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/dispatch/modules/content/models.py b/dispatch/modules/content/models.py index fe262512c..dccf62fb4 100644 --- a/dispatch/modules/content/models.py +++ b/dispatch/modules/content/models.py @@ -12,8 +12,8 @@ from django.db.models import ( Model, DateTimeField, CharField, TextField, PositiveIntegerField, - ImageField, FileField, BooleanField, UUIDField, ForeignKey, - ManyToManyField, SlugField, SET_NULL, CASCADE, F) + ImageField, FileField, BooleanField, NullBooleanField, UUIDField, + ForeignKey, ManyToManyField, SlugField, SET_NULL, CASCADE, F) from django.conf import settings from django.core.validators import MaxValueValidator from django.utils import timezone @@ -70,10 +70,10 @@ class Publishable(Model): preview_id = UUIDField(default=uuid.uuid4) revision_id = PositiveIntegerField(default=0, db_index=True) - head = BooleanField(default=False, db_index=True) + head = NullBooleanField(default=None, db_index=True, null=True) - is_published = BooleanField(default=False, db_index=True) - is_active = BooleanField(default=True) + is_published = NullBooleanField(default=None, db_index=True) + is_active = NullBooleanField(default=True) published_version = PositiveIntegerField(null=True) latest_version = PositiveIntegerField(null=True) @@ -356,6 +356,13 @@ class Article(Publishable, AuthorMixin): reading_time = CharField(max_length=100, choices=READING_CHOICES, default='anytime') + class Meta: + unique_together = ( + ('slug', 'head'), + ('parent', 'slug', 'head'), + ('parent', 'slug', 'is_published'), + ) + AuthorModel = Author @property @@ -441,6 +448,13 @@ class Page(Publishable): parent_page = ForeignKey('Page', related_name='parent_page_fk', null=True) title = CharField(max_length=255) + class Meta: + unique_together = ( + ('slug', 'head'), + ('parent', 'slug', 'head'), + ('parent', 'slug', 'is_published'), + ) + def get_author_string(self): return None From c3de22eed22aab473a366039eaba82976741eba9 Mon Sep 17 00:00:00 2001 From: Peter Siemens Date: Tue, 30 Oct 2018 22:00:51 -0700 Subject: [PATCH 2/7] Update Publishable save method to use None for head, is_published fields --- dispatch/modules/content/models.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/dispatch/modules/content/models.py b/dispatch/modules/content/models.py index dccf62fb4..2611a3ba6 100644 --- a/dispatch/modules/content/models.py +++ b/dispatch/modules/content/models.py @@ -143,7 +143,7 @@ def is_parent(self): def publish(self): # Unpublish last published version - type(self).objects.filter(parent=self.parent, is_published=True).update(is_published=False, published_at=None) + type(self).objects.filter(parent=self.parent, is_published=True).update(is_published=None, published_at=None) self.is_published = True if self.published_at is None: self.published_at = timezone.now() @@ -156,7 +156,7 @@ def publish(self): return self def unpublish(self): - type(self).objects.filter(parent=self.parent, is_published=True).update(is_published=False, published_at=None) + type(self).objects.filter(parent=self.parent, is_published=True).update(is_published=None, published_at=None) self.is_published = False # Unset published version for all articles @@ -184,7 +184,9 @@ def save(self, revision=True, *args, **kwargs): if not self.is_parent(): # If this is a revision, delete the old head of the list. - type(self).objects.filter(parent=self.parent, head=True).update(head=False) + type(self).objects \ + .filter(parent=self.parent, head=True) \ + .update(head=None) # Clear the instance id to force Django to save a new instance. # Both fields (pk, id) required for this to work -- something to do with model inheritance @@ -197,6 +199,7 @@ def save(self, revision=True, *args, **kwargs): # Raise integrity error if instance with given slug already exists. if type(self).objects.filter(slug=self.slug).exclude(parent=self.parent).exists(): raise IntegrityError("%s with slug '%s' already exists." % (type(self).__name__, self.slug)) + self.is_published = None # Set created_at to current time, but only for first version if not self.created_at: @@ -223,7 +226,10 @@ def save(self, revision=True, *args, **kwargs): if revision: # Set latest version for all articles - type(self).objects.filter(parent=self.parent).update(latest_version=self.revision_id) + type(self).objects \ + .filter(parent=self.parent) \ + .update(latest_version=self.revision_id) + self.latest_version = self.revision_id return self From e7e175093a71a1adf52a8592242037bac171b501 Mon Sep 17 00:00:00 2001 From: Peter Siemens Date: Tue, 30 Oct 2018 22:01:20 -0700 Subject: [PATCH 3/7] Remove Publishable integrity checks, rely on DB constraints instead --- dispatch/modules/content/models.py | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/dispatch/modules/content/models.py b/dispatch/modules/content/models.py index 2611a3ba6..096f9af87 100644 --- a/dispatch/modules/content/models.py +++ b/dispatch/modules/content/models.py @@ -194,11 +194,6 @@ def save(self, revision=True, *args, **kwargs): self.id = None # New version is unpublished by default - self.is_published = False - - # Raise integrity error if instance with given slug already exists. - if type(self).objects.filter(slug=self.slug).exclude(parent=self.parent).exists(): - raise IntegrityError("%s with slug '%s' already exists." % (type(self).__name__, self.slug)) self.is_published = None # Set created_at to current time, but only for first version @@ -209,14 +204,6 @@ def save(self, revision=True, *args, **kwargs): if revision: self.updated_at = timezone.now() - # Check that there is only one 'head' - if self.is_conflicting_head(): - raise IntegrityError("%s with head=True already exists." % (type(self).__name__,)) - - # Check that there is only one version with this revision_id - if self.is_conflicting_revision_id(): - raise IntegrityError("%s with revision_id=%s already exists." % (self.revision_id, type(self).__name__)) - super(Publishable, self).save(*args, **kwargs) # Update the parent foreign key @@ -240,12 +227,6 @@ def delete(self, *args, **kwargs): return super(Publishable, self).delete(*args, **kwargs) return self.parent.delete() - def is_conflicting_head(self): - return self.head is True and type(self).objects.filter(parent=self.parent, head=True).exclude(id=self.id).exists() - - def is_conflicting_revision_id(self): - return type(self).objects.filter(parent=self.parent, id=self.id).count() > 1 - def save_featured_image(self, data): """ Handles saving the featured image. From fbb47a9bd6d52709383a071abe9592f359422254 Mon Sep 17 00:00:00 2001 From: Peter Siemens Date: Tue, 30 Oct 2018 22:01:44 -0700 Subject: [PATCH 4/7] Clean up code formatting in modules/content/models.py --- dispatch/modules/content/models.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/dispatch/modules/content/models.py b/dispatch/modules/content/models.py index 096f9af87..fe7ee7431 100644 --- a/dispatch/modules/content/models.py +++ b/dispatch/modules/content/models.py @@ -225,6 +225,7 @@ def save(self, revision=True, *args, **kwargs): def delete(self, *args, **kwargs): if self.parent == self: return super(Publishable, self).delete(*args, **kwargs) + return self.parent.delete() def save_featured_image(self, data): @@ -308,7 +309,9 @@ def get_previous_revision(self): if self.parent == self: return self try: - revision = type(self).objects.filter(parent=self.parent).order_by('-pk')[1] + revision = type(self).objects \ + .filter(parent=self.parent) \ + .order_by('-pk')[1] return revision except: return self @@ -317,7 +320,6 @@ class Meta: abstract = True class Article(Publishable, AuthorMixin): - parent = ForeignKey('Article', related_name='article_parent', blank=True, null=True) headline = CharField(max_length=255) @@ -427,7 +429,7 @@ def get_published_articles(self): return Article.objects.filter(subsection=self, is_published=True).order_by('-published_at') def get_absolute_url(self): - """ Returns the subsection URL. """ + """Returns the subsection URL.""" return "%s%s/" % (settings.BASE_URL, self.slug) class Page(Publishable): From 614184a7254008e66a5a43dc3a2ef476c1b001a7 Mon Sep 17 00:00:00 2001 From: Peter Siemens Date: Tue, 30 Oct 2018 22:01:53 -0700 Subject: [PATCH 5/7] Update dispatch/migrations/0022_publishable_constraints.py --- .../0022_publishable_constraints.py | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 dispatch/migrations/0022_publishable_constraints.py diff --git a/dispatch/migrations/0022_publishable_constraints.py b/dispatch/migrations/0022_publishable_constraints.py new file mode 100644 index 000000000..f1219d7da --- /dev/null +++ b/dispatch/migrations/0022_publishable_constraints.py @@ -0,0 +1,93 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11 on 2018-10-30 18:59 +from __future__ import unicode_literals + +from django.db import migrations, models + +from dispatch.models import Article, Page + +def fix_latest_head(model, item): + latest = model.objects. \ + filter(parent=item.parent). \ + order_by('-id')[0] + + model.objects. \ + filter(parent=item.parent). \ + update(head=None) + + latest.head = True + latest.save(revision=False) + + duplicates = model.objects. \ + filter(slug=item.slug). \ + exclude(parent=item.parent). \ + delete() + +def fix_latest_published(model, item): + latest_items = model.objects. \ + filter(parent_id=item.parent_id, is_published=True). \ + order_by('-id') + + if not latest_items: + return + + model.objects. \ + filter(parent=item.parent). \ + update(is_published=None) + + latest = latest_items[0] + latest.is_published = True + latest.save(revision=False) + +def fix_items(model): + seen = set() + for item in model.objects.order_by('-id'): + if item.slug in seen: + continue + + fix_latest_head(model, item) + fix_latest_published(model, item) + + seen.add(item.slug) + +def remove_duplicates(apps, schema_editor): + fix_items(Article) + fix_items(Page) + +class Migration(migrations.Migration): + + dependencies = [ + ('dispatch', '0021_podcast_fields'), + ] + + operations = [ + migrations.AlterField( + model_name='article', + name='head', + field=models.NullBooleanField(db_index=True, default=False), + ), + migrations.AlterField( + model_name='article', + name='is_published', + field=models.NullBooleanField(db_index=True, default=False), + ), + migrations.AlterField( + model_name='page', + name='head', + field=models.NullBooleanField(db_index=True, default=False), + ), + migrations.AlterField( + model_name='page', + name='is_published', + field=models.NullBooleanField(db_index=True, default=False), + ), + migrations.RunPython(remove_duplicates), + migrations.AlterUniqueTogether( + name='article', + unique_together=set([('parent', 'slug', 'is_published'), ('slug', 'head'), ('parent', 'slug', 'head')]), + ), + migrations.AlterUniqueTogether( + name='page', + unique_together=set([('parent', 'slug', 'is_published'), ('slug', 'head'), ('parent', 'slug', 'head')]), + ), + ] From d9a084217044616ac651d75e6ad885652b507977 Mon Sep 17 00:00:00 2001 From: Peter Siemens Date: Tue, 30 Oct 2018 22:51:58 -0700 Subject: [PATCH 6/7] Set is_published=None when unpublished Publishable --- dispatch/modules/content/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dispatch/modules/content/models.py b/dispatch/modules/content/models.py index fe7ee7431..88c473383 100644 --- a/dispatch/modules/content/models.py +++ b/dispatch/modules/content/models.py @@ -157,7 +157,7 @@ def publish(self): def unpublish(self): type(self).objects.filter(parent=self.parent, is_published=True).update(is_published=None, published_at=None) - self.is_published = False + self.is_published = None # Unset published version for all articles type(self).objects.filter(parent=self.parent).update(published_version=None) From 50a854812683b9d33574a1840c8ee26086ffc555 Mon Sep 17 00:00:00 2001 From: Peter Siemens Date: Tue, 30 Oct 2018 22:55:26 -0700 Subject: [PATCH 7/7] Add custom NullBooleanField to convert null values to False --- dispatch/api/fields.py | 17 +++++++++++++++-- dispatch/api/serializers.py | 6 +++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/dispatch/api/fields.py b/dispatch/api/fields.py index e77cb1753..561332680 100644 --- a/dispatch/api/fields.py +++ b/dispatch/api/fields.py @@ -4,15 +4,28 @@ from rest_framework.exceptions import ValidationError class JSONField(Field): - def to_internal_value(self, data): return data def to_representation(self, value): return value -class PrimaryKeyField(Field): +class NullBooleanField(Field): + """Forces a Django NullBooleanField to always return False for null values.""" + + def get_attribute(self, instance): + """Overrides the default get_attribute method to convert None values to False.""" + attr = super(NullBooleanField, self).get_attribute(instance) + return True if attr else False + + def to_internal_value(self, data): + return True if data else None + + def to_representation(self, value): + return True if value else False + +class PrimaryKeyField(Field): def __init__(self, serializer, *args, **kwargs): super(PrimaryKeyField, self).__init__(*args, **kwargs) diff --git a/dispatch/api/serializers.py b/dispatch/api/serializers.py index 5c1f63680..371a61b21 100644 --- a/dispatch/api/serializers.py +++ b/dispatch/api/serializers.py @@ -18,7 +18,7 @@ from dispatch.api.validators import ( FilenameValidator, ImageGalleryValidator, PasswordValidator, SlugValidator, AuthorValidator, TemplateValidator) -from dispatch.api.fields import JSONField, PrimaryKeyField, ForeignKeyField +from dispatch.api.fields import JSONField, NullBooleanField, PrimaryKeyField, ForeignKeyField class PersonSerializer(DispatchModelSerializer): """Serializes the Person model.""" @@ -608,6 +608,8 @@ class ArticleSerializer(DispatchModelSerializer, DispatchPublishableSerializer): id = serializers.ReadOnlyField(source='parent_id') slug = serializers.SlugField(validators=[SlugValidator()]) + is_published = NullBooleanField(read_only=True) + section = SectionSerializer(read_only=True) section_id = serializers.IntegerField(write_only=True) @@ -770,6 +772,8 @@ class PageSerializer(DispatchModelSerializer, DispatchPublishableSerializer): id = serializers.ReadOnlyField(source='parent_id') slug = serializers.SlugField(validators=[SlugValidator()]) + is_published = NullBooleanField(read_only=True) + featured_image = ImageAttachmentSerializer(required=False, allow_null=True) featured_video = VideoAttachmentSerializer(required=False, allow_null=True)