Merge pull request #148 from dmarcelino/child_url_paths

Fixes descendant_url_path on page.save()
This commit is contained in:
Diogo Marques 2017-12-19 14:56:48 +00:00 committed by GitHub
commit eecab72275
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 121 additions and 26 deletions

View file

@ -1,10 +1,11 @@
# coding: utf-8
import copy
import logging
import types
from django.core.exceptions import ValidationError, FieldDoesNotExist
from django.core.urlresolvers import reverse
from django.db import transaction
from django.db import transaction, connection
from django.http import Http404
from django.utils.translation import trans_real
from django.utils.translation import ugettext_lazy as _
@ -35,6 +36,7 @@ COMPOSED_PANEL_CLASSES = [MultiFieldPanel, FieldRowPanel] + CUSTOM_COMPOSED_PANE
class WagtailTranslator(object):
_patched_models = []
_page_fields_tables = []
def __init__(self, model):
# Check if this class was already patched
@ -50,6 +52,11 @@ class WagtailTranslator(object):
WagtailTranslator._patched_models.append(model)
# compile all tables that are holding page translated fields (title_xx, slug_xx, url_path_xx)
options = translator.get_options_for_model(model)
if 'url_path' in options.local_fields.keys() and model._meta.db_table is not 'wagtailcore_page':
WagtailTranslator._page_fields_tables.append(model._meta.db_table)
def _patch_page_models(self, model):
# PANEL PATCHING
@ -111,7 +118,10 @@ class WagtailTranslator(object):
model.relative_url = _new_relative_url
model.url = _new_url
_patch_clean(model)
_patch_save(model)
if not model.save.__name__.startswith('localized'):
descriptor = LocalizedSaveDescriptor(model.save)
setattr(model, 'save', descriptor)
def _patch_other_models(self, model):
if hasattr(model, 'edit_handler'):
@ -266,11 +276,6 @@ def _new_set_url_path(self, parent):
# which always has a url_path of '/'
setattr(self, localized_url_path_field, '/')
# update url_path for children pages
for child in self.get_children().specific():
child.set_url_path(self.specific)
child.save()
return self.url_path
@ -416,35 +421,87 @@ def _patch_clean(model):
model.clean = clean
def _patch_save(model):
old_save = model.save
def save(self, *args, **kwargs):
def _localized_update_descendant_url_paths(self, old_url_path, new_url_path, language):
localized_url_path = build_localized_fieldname('url_path', language)
for db_table in WagtailTranslator._page_fields_tables:
cursor = connection.cursor()
if connection.vendor == 'sqlite':
update_statement = """
UPDATE {db_table}
SET {localized_url_path} = %s || substr({localized_url_path}, %s)
WHERE EXISTS (SELECT * FROM wagtailcore_page AS p
WHERE p.id = {db_table}.page_ptr_id AND p.path LIKE %s)
AND page_ptr_id <> %s
""".format(db_table=db_table, localized_url_path=localized_url_path)
elif connection.vendor == 'mysql':
update_statement = """
UPDATE {db_table} t
JOIN wagtailcore_page p ON p.id = t.page_ptr_id
SET {localized_url_path}= CONCAT(%s, substring({localized_url_path}, %s))
WHERE p.path LIKE %s AND t.page_ptr_id <> %s
""".format(db_table=db_table, localized_url_path=localized_url_path)
elif connection.vendor in ('mssql', 'microsoft'):
update_statement = """
UPDATE t
SET {localized_url_path}= CONCAT(%s, (SUBSTRING({localized_url_path}, 0, %s)))
FROM {db_table} t
JOIN wagtailcore_page p
ON p.id = t.page_ptr_id
WHERE p.path LIKE %s AND t.page_ptr_id <> %s
""".format(db_table=db_table, localized_url_path=localized_url_path)
else:
update_statement = """
UPDATE {db_table} as t
SET {localized_url_path} = %s || substring({localized_url_path} from %s)
FROM wagtailcore_page AS p
WHERE p.id = t.page_ptr_id AND p.path LIKE %s AND t.page_ptr_id <> %s
""".format(db_table=db_table, localized_url_path=localized_url_path)
cursor.execute(update_statement, [new_url_path, len(old_url_path) + 1, self.path + '%', self.page_ptr_id])
class LocalizedSaveDescriptor(object):
def __init__(self, f):
self.func = f
self.__name__ = 'localized_{}'.format(f.__name__)
def __call__(self, *args, **kwargs):
# when updating, save doesn't check if slug_xx has changed so it can only detect changes in slug
# from current language. We need to ensure that if a given localized slug changes we call set_url_path
if not self.id: # creating a record, wagtail will call set_url_path, nothing to do.
return old_save(self, *args, **kwargs)
instance = args[0]
if not instance.id: # creating a record, wagtail will call set_url_path, nothing to do.
return self.func(*args, **kwargs)
current_language = get_language()
old_record = None
call_set_url_path = False
changed_localized_slugs = []
for language in mt_settings.AVAILABLE_LANGUAGES:
localized_slug = build_localized_fieldname('slug', language)
# similar logic used in save
if not ('update_fields' in kwargs and localized_slug not in kwargs['update_fields']):
old_record = old_record or Page.objects.get(id=self.id).specific
if getattr(old_record, localized_slug) != getattr(self, localized_slug):
if language == current_language:
# Wagtail's save will detect slug change so it will call set_url_path
call_set_url_path = False
break
call_set_url_path = True
old_record = old_record or instance.__class__.objects.get(id=instance.id)
if getattr(old_record, localized_slug) != getattr(instance, localized_slug):
changed_localized_slugs.append(language)
if call_set_url_path:
self.set_url_path(self.get_parent())
return old_save(self, *args, **kwargs)
# if any language other than current language had it slug changed
# we'll execute set_url_path
if len(changed_localized_slugs) > 1 or \
(len(changed_localized_slugs) == 1 and changed_localized_slugs[0] != get_language()):
instance.set_url_path(instance.get_parent())
result = self.func(*args, **kwargs)
# update children paths
for language in changed_localized_slugs:
localized_url_path = build_localized_fieldname('url_path', language)
old_url_path = getattr(old_record, localized_url_path)
new_url_path = getattr(instance, localized_url_path)
_localized_update_descendant_url_paths(instance, old_url_path, new_url_path, language)
return result
def __get__(self, instance, owner=None):
return types.MethodType(self, instance) if instance else self
model.save = save
def _patch_stream_field_meaningful_value(field):
old_meaningful_value = field.meaningful_value
@ -459,6 +516,7 @@ def _patch_stream_field_meaningful_value(field):
field.meaningful_value = meaningful_value.__get__(field)
def _patch_pre_save(field):
if not ORIGINAL_SLUG_LANGUAGE:
return
@ -482,6 +540,7 @@ def _patch_pre_save(field):
field.pre_save = pre_save.__get__(field)
def patch_wagtail_models():
# After all models being registered the Page or BaseSetting subclasses and snippets are patched
registered_models = translator.get_registered_models()
@ -494,3 +553,4 @@ def patch_wagtail_models():
for model_class in registered_models:
if issubclass(model_class, Page) or model_class in get_snippet_models() or issubclass(model_class, BaseSetting):
WagtailTranslator(model_class)

View file

@ -24,6 +24,8 @@ class TestSlugPage1(WagtailPage):
class TestSlugPage2(WagtailPage):
pass
class TestSlugPage1Subclass(TestSlugPage1):
pass
class PatchTestPage(WagtailPage):
description = models.CharField(max_length=50)

View file

@ -488,3 +488,31 @@ class WagtailModeltranslationTest(WagtailModeltranslationTestBase):
self.assertEqual(grandchild.url_path_de, '/child/grandchild1/')
self.assertEqual(grandchild.url_path_en, '/child/grandchild1_en/')
# Children url paths should update when parent changes
child.slug_en = 'child_en'
child.save()
self.assertEqual(child.url_path_de, '/child/')
self.assertEqual(child.url_path_en, '/child_en/')
# We should retrieve grandchild with the below command:
# grandchild_new = models.TestSlugPage1.objects.get(id=grandchild.id)
# but it's exhibiting strange behaviour during tests. See:
# https://github.com/infoportugal/wagtail-modeltranslation/issues/103#issuecomment-352006610
grandchild_new = models.TestSlugPage1._default_manager.raw("""
SELECT page_ptr_id, url_path_en, url_path_de FROM {}
WHERE page_ptr_id=%s LIMIT 1
""".format(models.TestSlugPage1._meta.db_table), [grandchild.page_ptr_id])[0]
self.assertEqual(grandchild_new.url_path_en, '/child_en/grandchild1_en/')
self.assertEqual(grandchild_new.url_path_de, '/child/grandchild1/')
def test_page_fields_tables(self):
from wagtail_modeltranslation.patch_wagtailadmin import WagtailTranslator
self.assertIn(models.TestSlugPage1, WagtailTranslator._patched_models)
self.assertIn('tests_testslugpage1', WagtailTranslator._page_fields_tables)
self.assertIn(models.TestSlugPage1Subclass, WagtailTranslator._patched_models)
self.assertNotIn('tests_testslugpage1subclass', WagtailTranslator._page_fields_tables)
self.assertNotIn('wagtailcore_page', WagtailTranslator._page_fields_tables)

View file

@ -4,7 +4,7 @@ from modeltranslation.translator import translator, register, TranslationOptions
from wagtail_modeltranslation.tests.models import TestRootPage, TestSlugPage1, TestSlugPage2, PatchTestPage, \
PatchTestSnippet, FieldPanelPage, ImageChooserPanelPage, FieldRowPanelPage, MultiFieldPanelPage, InlinePanelPage, \
FieldPanelSnippet, ImageChooserPanelSnippet, FieldRowPanelSnippet, MultiFieldPanelSnippet, PageInlineModel, \
BaseInlineModel, StreamFieldPanelPage, StreamFieldPanelSnippet, SnippetInlineModel, InlinePanelSnippet
BaseInlineModel, StreamFieldPanelPage, StreamFieldPanelSnippet, SnippetInlineModel, InlinePanelSnippet, TestSlugPage1Subclass
from wagtail_modeltranslation.translator import WagtailTranslationOptions
@ -25,6 +25,11 @@ class TestSlugPage2TranslationOptions(WagtailTranslationOptions):
fields = ()
@register(TestSlugPage1Subclass)
class TestSlugPage1SubclassTranslationOptions(WagtailTranslationOptions):
pass
@register(PatchTestPage)
class PatchTestPageTranslationOptions(WagtailTranslationOptions):
fields = ('description',)