From bae76a2af077dbaab34446c24f7434f3a85af968 Mon Sep 17 00:00:00 2001 From: Sage Abdullah Date: Tue, 22 Feb 2022 19:38:51 +0700 Subject: [PATCH] Replace `content_json` `TextField` with `content` `JSONField` in `PageRevision` --- docs/reference/pages/model_reference.rst | 11 +++-- docs/releases/2.17.md | 7 +++ docs/topics/streamfield.rst | 4 +- wagtail/admin/tests/pages/test_edit_page.py | 6 +-- wagtail/core/actions/copy_page.py | 5 +- wagtail/core/actions/publish_page_revision.py | 4 +- .../commands/publish_scheduled_pages.py | 8 ++-- .../core/management/commands/replace_text.py | 12 +++-- .../0067_alter_pagerevision_content_json.py | 27 +++++++++++ wagtail/core/models/__init__.py | 46 +++++++++---------- wagtail/core/tests/test_page_model.py | 13 ++---- 11 files changed, 90 insertions(+), 53 deletions(-) create mode 100644 wagtail/core/migrations/0067_alter_pagerevision_content_json.py diff --git a/docs/reference/pages/model_reference.rst b/docs/reference/pages/model_reference.rst index 373f83eebc..f831d7f385 100644 --- a/docs/reference/pages/model_reference.rst +++ b/docs/reference/pages/model_reference.rst @@ -490,7 +490,7 @@ The ``locale`` and ``translation_key`` fields have a unique key constraint to pr Every time a page is edited a new ``PageRevision`` is created and saved to the database. It can be used to find the full history of all changes that have been made to a page and it also provides a place for new changes to be kept before going live. - Revisions can be created from any :class:`~wagtail.core.models.Page` object by calling its :meth:`~Page.save_revision` method -- The content of the page is JSON-serialised and stored in the :attr:`~PageRevision.content_json` field +- The content of the page is JSON-serialisable and stored in the :attr:`~PageRevision.content` field - You can retrieve a ``PageRevision`` as a :class:`~wagtail.core.models.Page` object by calling the :meth:`~PageRevision.as_page_object` method Database fields @@ -520,12 +520,17 @@ Database fields This links to the user that created the revision - .. attribute:: content_json + .. attribute:: content - (text) + (dict) This field contains the JSON content for the page at the time the revision was created + .. versionchanged:: 2.17 + + The field has been renamed from ``content_json`` to ``content`` and it now uses :class:`~django.db.models.JSONField` instead of + :class:`~django.db.models.TextField`. + Managers ~~~~~~~~ diff --git a/docs/releases/2.17.md b/docs/releases/2.17.md index e839c91411..da95aef4e1 100644 --- a/docs/releases/2.17.md +++ b/docs/releases/2.17.md @@ -24,6 +24,7 @@ Here are other changes related to the redesign: * Remove UI code for legacy browser support: polyfills, IE11 workarounds, Modernizr (Thibaud Colas) * Remove redirect auto-creation recipe from documentation as this feature is now supported in Wagtail core (Andy Babic) * Remove IE11 warnings (Gianluca De Cola) + * Replace `content_json` `TextField` with `content` `JSONField` in `PageRevision` (Sage Abdullah) ### Bug fixes @@ -40,3 +41,9 @@ Here are other changes related to the redesign: * IE11 support was officially dropped in Wagtail 2.15, as of this release there will no longer be a warning shown to users of this browser. * Wagtail is fully compatible with Microsoft Edge, Microsoft’s replacement for Internet Explorer. You may consider using its `IE mode `_ to keep access to IE11-only sites, while other sites and apps like Wagtail can leverage modern browser capabilities. + +## Replaced `content_json` `TextField` with `content` `JSONField` in `PageRevision` + +* The `content_json` field in the `PageRevision` model has been renamed to `content`. +* The field now internally uses `JSONField` instead of `TextField`. +* If you have a large number of `PageRevision` objects, running the migrations might take a while. diff --git a/docs/topics/streamfield.rst b/docs/topics/streamfield.rst index b1a35b7216..af985a883b 100644 --- a/docs/topics/streamfield.rst +++ b/docs/topics/streamfield.rst @@ -677,10 +677,10 @@ Note that the above migration will work on published Page objects only. If you a page.save() for revision in page.revisions.all(): - revision_data = json.loads(revision.content_json) + revision_data = revision.content revision_data, changed = pagerevision_converter(revision_data) if changed: - revision.content_json = json.dumps(revision_data, cls=DjangoJSONEncoder) + revision.content = revision_data revision.save() diff --git a/wagtail/admin/tests/pages/test_edit_page.py b/wagtail/admin/tests/pages/test_edit_page.py index 95f7d64ba2..f071091e1d 100644 --- a/wagtail/admin/tests/pages/test_edit_page.py +++ b/wagtail/admin/tests/pages/test_edit_page.py @@ -326,7 +326,7 @@ class TestPageEdit(TestCase, WagtailTestUtils): def test_edit_post_scheduled(self): # put go_live_at and expire_at several days away from the current date, to avoid - # false matches in content_json__contains tests + # false matches in content__ tests go_live_at = timezone.now() + datetime.timedelta(days=10) expire_at = timezone.now() + datetime.timedelta(days=20) post_data = { @@ -358,12 +358,12 @@ class TestPageEdit(TestCase, WagtailTestUtils): # But a revision with go_live_at and expire_at in their content json *should* exist self.assertTrue( PageRevision.objects.filter( - page=child_page_new, content_json__contains=str(go_live_at.date()) + page=child_page_new, content__go_live_at__startswith=str(go_live_at.date()) ).exists() ) self.assertTrue( PageRevision.objects.filter( - page=child_page_new, content_json__contains=str(expire_at.date()) + page=child_page_new, content__expire_at__startswith=str(expire_at.date()) ).exists() ) diff --git a/wagtail/core/actions/copy_page.py b/wagtail/core/actions/copy_page.py index 0d6f8567d8..518902d570 100644 --- a/wagtail/core/actions/copy_page.py +++ b/wagtail/core/actions/copy_page.py @@ -1,4 +1,3 @@ -import json import logging import uuid @@ -182,7 +181,7 @@ class CopyPageAction: revision.page = page_copy # Update ID fields in content - revision_content = json.loads(revision.content_json) + revision_content = revision.content revision_content["pk"] = page_copy.pk for child_relation in get_all_child_relations(specific_page): @@ -207,7 +206,7 @@ class CopyPageAction: copied_child_object.pk if copied_child_object else None ) - revision.content_json = json.dumps(revision_content) + revision.content = revision_content # Save revision.save() diff --git a/wagtail/core/actions/publish_page_revision.py b/wagtail/core/actions/publish_page_revision.py index 6628bbcab1..494fbb2db2 100644 --- a/wagtail/core/actions/publish_page_revision.py +++ b/wagtail/core/actions/publish_page_revision.py @@ -150,9 +150,7 @@ class PublishPageRevisionAction: ) # Update alias pages - page.update_aliases( - revision=revision, user=user, _content_json=revision.content_json - ) + page.update_aliases(revision=revision, user=user, _content=revision.content) if log_action: data = None diff --git a/wagtail/core/management/commands/publish_scheduled_pages.py b/wagtail/core/management/commands/publish_scheduled_pages.py index 15ff7f4c2b..5cefdf1da4 100644 --- a/wagtail/core/management/commands/publish_scheduled_pages.py +++ b/wagtail/core/management/commands/publish_scheduled_pages.py @@ -1,5 +1,3 @@ -import json - from django.core.management.base import BaseCommand from django.utils import dateparse, timezone @@ -7,7 +5,7 @@ from wagtail.core.models import Page, PageRevision def revision_date_expired(r): - expiry_str = json.loads(r.content_json).get("expire_at") + expiry_str = r.content.get("expire_at") if not expiry_str: return False expire_at = dateparse.parse_datetime(expiry_str) @@ -72,7 +70,7 @@ class Command(BaseCommand): self.stdout.write("Expiry datetime\t\tSlug\t\tName") self.stdout.write("---------------\t\t----\t\t----") for er in expired_revs: - rev_data = json.loads(er.content_json) + rev_data = er.content self.stdout.write( "{0}\t{1}\t{2}".format( dateparse.parse_datetime( @@ -100,7 +98,7 @@ class Command(BaseCommand): self.stdout.write("Go live datetime\t\tSlug\t\tName") self.stdout.write("---------------\t\t\t----\t\t----") for rp in revs_for_publishing: - rev_data = json.loads(rp.content_json) + rev_data = rp.content self.stdout.write( "{0}\t\t{1}\t{2}".format( rp.approved_go_live_at.strftime("%Y-%m-%d %H:%M"), diff --git a/wagtail/core/management/commands/replace_text.py b/wagtail/core/management/commands/replace_text.py index 649b5fbfc2..0e95a4f072 100644 --- a/wagtail/core/management/commands/replace_text.py +++ b/wagtail/core/management/commands/replace_text.py @@ -1,5 +1,8 @@ +import json + from django.core.management.base import BaseCommand from django.db import models +from django.db.models.functions import Cast from modelcluster.models import get_all_child_relations from wagtail.core.models import PageRevision, get_page_models @@ -32,9 +35,12 @@ class Command(BaseCommand): from_text = options["from_text"] to_text = options["to_text"] - for revision in PageRevision.objects.filter(content_json__contains=from_text): - revision.content_json = revision.content_json.replace(from_text, to_text) - revision.save(update_fields=["content_json"]) + for revision in PageRevision.objects.annotate( + content_text=Cast("content", output_field=models.TextField()) + ).filter(content_text__contains=from_text): + replacement = revision.content_text.replace(from_text, to_text) + revision.content = json.loads(replacement) + revision.save(update_fields=["content"]) for page_class in get_page_models(): self.stdout.write("scanning %s" % page_class._meta.verbose_name) diff --git a/wagtail/core/migrations/0067_alter_pagerevision_content_json.py b/wagtail/core/migrations/0067_alter_pagerevision_content_json.py new file mode 100644 index 0000000000..6713e068cd --- /dev/null +++ b/wagtail/core/migrations/0067_alter_pagerevision_content_json.py @@ -0,0 +1,27 @@ +# Generated by Django 4.0.2 on 2022-02-22 13:06 + +import django.core.serializers.json +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("wagtailcore", "0066_collection_management_permissions"), + ] + + operations = [ + migrations.AlterField( + model_name="pagerevision", + name="content_json", + field=models.JSONField( + encoder=django.core.serializers.json.DjangoJSONEncoder, + verbose_name="content JSON", + ), + ), + migrations.RenameField( + model_name="pagerevision", + old_name="content_json", + new_name="content", + ), + ] diff --git a/wagtail/core/models/__init__.py b/wagtail/core/models/__init__.py index f11eca671e..19a2d71e9c 100644 --- a/wagtail/core/models/__init__.py +++ b/wagtail/core/models/__init__.py @@ -10,7 +10,6 @@ as Page. """ import functools -import json import logging import uuid from io import StringIO @@ -25,6 +24,7 @@ from django.core.cache import cache from django.core.exceptions import PermissionDenied, ValidationError from django.core.handlers.base import BaseHandler from django.core.handlers.wsgi import WSGIRequest +from django.core.serializers.json import DjangoJSONEncoder from django.db import models, transaction from django.db.models import DEFERRED, Q, Value from django.db.models.expressions import OuterRef, Subquery @@ -900,7 +900,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): # Create revision revision = self.revisions.create( - content_json=self.to_json(), + content=self.serializable_data(), user=user, submitted_for_moderation=submitted_for_moderation, approved_go_live_at=approved_go_live_at, @@ -990,7 +990,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): return self.specific def update_aliases( - self, *, revision=None, user=None, _content_json=None, _updated_ids=None + self, *, revision=None, user=None, _content=None, _updated_ids=None ): """ Publishes all aliases that follow this page with the latest content from this page. @@ -1005,8 +1005,8 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): specific_self = self.specific # Only compute this if necessary since it's quite a heavy operation - if _content_json is None: - _content_json = self.to_json() + if _content is None: + _content = self.serializable_data() # A list of IDs that have already been updated. This is just in case someone has # created an alias loop (which is impossible to do with the UI Wagtail provides) @@ -1029,7 +1029,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): ] # Copy field content - alias_updated = alias.with_content_json(_content_json) + alias_updated = alias.with_content_json(_content) # Publish the alias if it's currently in draft alias_updated.live = True @@ -1115,7 +1115,7 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): alias.update_aliases( revision=revision, - _content_json=_content_json, + _content=_content, _updated_ids=_updated_ids, ) @@ -1936,10 +1936,10 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): context["action_url"] = action_url return TemplateResponse(request, self.password_required_template, context) - def with_content_json(self, content_json): + def with_content_json(self, content): """ Returns a new version of the page with field values updated to reflect changes - in the provided ``content_json`` (which usually comes from a previously-saved + in the provided ``content`` (which usually comes from a previously-saved page revision). Certain field values are preserved in order to prevent errors if the returned @@ -1960,24 +1960,22 @@ class Page(AbstractPage, index.Indexed, ClusterableModel, metaclass=PageBase): * ``wagtail_admin_comments`` (COMMENTS_RELATION_NAME) """ - data = json.loads(content_json) - # Old revisions (pre Wagtail 2.15) may have saved comment data under the name 'comments' # rather than the current relation name as set by COMMENTS_RELATION_NAME; # if a 'comments' field exists and looks like our comments model, alter the data to use # COMMENTS_RELATION_NAME before restoring if ( - COMMENTS_RELATION_NAME not in data - and "comments" in data - and isinstance(data["comments"], list) - and len(data["comments"]) - and isinstance(data["comments"][0], dict) - and "contentpath" in data["comments"][0] + COMMENTS_RELATION_NAME not in content + and "comments" in content + and isinstance(content["comments"], list) + and len(content["comments"]) + and isinstance(content["comments"][0], dict) + and "contentpath" in content["comments"][0] ): - data[COMMENTS_RELATION_NAME] = data["comments"] - del data["comments"] + content[COMMENTS_RELATION_NAME] = content["comments"] + del content["comments"] - obj = self.specific_class.from_serializable_data(data) + obj = self.specific_class.from_serializable_data(content) # These should definitely never change between revisions obj.id = self.id @@ -2153,7 +2151,9 @@ class PageRevision(models.Model): blank=True, on_delete=models.SET_NULL, ) - content_json = models.TextField(verbose_name=_("content JSON")) + content = models.JSONField( + verbose_name=_("content JSON"), encoder=DjangoJSONEncoder + ) approved_go_live_at = models.DateTimeField( verbose_name=_("approved go live at"), null=True, blank=True, db_index=True ) @@ -2200,7 +2200,7 @@ class PageRevision(models.Model): ) def as_page_object(self): - return self.page.specific.with_content_json(self.content_json) + return self.page.specific.with_content_json(self.content) def approve_moderation(self, user=None): if self.submitted_for_moderation: @@ -3467,7 +3467,7 @@ class WorkflowState(models.Model): return PageRevision.objects.filter( page_id=self.page_id, id__in=self.task_states.values_list("page_revision_id", flat=True), - ).defer("content_json") + ).defer("content") def _get_applicable_task_states(self): """Returns the set of task states whose status applies to the current revision""" diff --git a/wagtail/core/tests/test_page_model.py b/wagtail/core/tests/test_page_model.py index 1cc73e7aac..0a9d2fc7ae 100644 --- a/wagtail/core/tests/test_page_model.py +++ b/wagtail/core/tests/test_page_model.py @@ -1,5 +1,4 @@ import datetime -import json import unittest from unittest.mock import Mock @@ -1493,7 +1492,7 @@ class TestCopyPage(TestCase): # Check that the ids within the revision were updated correctly new_revision = new_christmas_event.revisions.first() - new_revision_content = json.loads(new_revision.content_json) + new_revision_content = new_revision.content self.assertEqual(new_revision_content["pk"], new_christmas_event.id) self.assertEqual( new_revision_content["speakers"][0]["page"], new_christmas_event.id @@ -3310,7 +3309,7 @@ class TestPageWithContentJSON(TestCase): # Take a json representation of the page and update it # with some alternative values - content = json.loads(original_page.to_json()) + content = original_page.serializable_data() content.update( title="About them", draft_title="About them", @@ -3333,10 +3332,8 @@ class TestPageWithContentJSON(TestCase): owner=1, ) - # Convert values back to json and pass them to with_content_json() - # to get an updated version of the page - content_json = json.dumps(content) - updated_page = original_page.with_content_json(content_json) + # Pass the values to with_content_json() to get an updated version of the page + updated_page = original_page.with_content_json(content) # The following attributes values should have changed for attr_name in ("title", "slug", "content", "url_path", "show_in_menus"): @@ -3345,7 +3342,7 @@ class TestPageWithContentJSON(TestCase): ) # The following attribute values should have been preserved, - # despite new values being provided in content_json + # despite new values being provided in content for attr_name in ( "pk", "path",