Sfoglia il codice sorgente

Allow UniqueConstraint in place of unique_together for TranslatableMixin's system check

Temidayo32 1 anno fa
parent
commit
9349e2ad9f

+ 1 - 0
CHANGELOG.txt

@@ -8,6 +8,7 @@ Changelog
  * Remember previous location on returning from page add/edit actions (Robert Rollins)
  * Update settings file in project settings to address Django 4.2 deprecations (Sage Abdullah)
  * Improve layout and accessibility of the image URL generator page, reduce reliance on JavaScript (Temidayo Azeez)
+ * Allow `UniqueConstraint` in place of `unique_together` for `TranslatableMixin`'s system check (Temidayo Azeez, Sage Abdullah)
  * Fix: Update system check for overwriting storage backends to recognise the `STORAGES` setting introduced in Django 4.2 (phijma-leukeleu)
  * Fix: Prevent password change form from raising a validation error when browser autocomplete fills in the "Old password" field (Chiemezuo Akujobi)
  * Fix: Ensure that the legacy dropdown options, when closed, do not get accidentally clicked by other interactions wide viewports (CheesyPhoenix, Christer Jensen)

+ 12 - 2
docs/reference/pages/model_reference.md

@@ -453,8 +453,6 @@ Pages already include this mixin, so there is no need to add it.
 
 ### Database fields
 
-The `locale` and `translation_key` fields have a unique key constraint to prevent the object being translated into a language more than once.
-
 ```{eval-rst}
 .. class:: TranslatableMixin
 
@@ -472,6 +470,18 @@ The `locale` and `translation_key` fields have a unique key constraint to preven
         This is shared with all translations of that instance so can be used for querying translations.
 ```
 
+The `translation_key` and `locale` fields have a unique key constraint to prevent the object being translated into a language more than once.
+
+```{note}
+This is currently enforced via {attr}`~django.db.models.Options.unique_together` in `TranslatableMixin.Meta`, but may be replaced with a {class}`~django.db.models.UniqueConstraint` in `TranslatableMixin.Meta.constraints` in the future.
+
+If your model defines a [`Meta` class](django:ref/models/options) (either with a new definition or inheriting `TranslatableMixin.Meta` explicitly), be mindful when setting `unique_together` or {attr}`~django.db.models.Options.constraints`. Ensure that there is either a `unique_together` or a `UniqueConstraint` (not both) on `translation_key` and `locale`. There is a system check for this.
+```
+
+```{versionchanged} 6.0
+The system check for `translation_key` and `locale` unique key constraint now allows a `UniqueConstraint` in `Meta.constraints` instead of `unique_together` in `Meta`.
+```
+
 ### Methods and properties
 
 ```{eval-rst}

+ 1 - 0
docs/releases/6.0.md

@@ -18,6 +18,7 @@ depth: 1
  * Remember previous location on returning from page add/edit actions (Robert Rollins)
  * Update settings file in project settings to address Django 4.2 deprecations (Sage Abdullah)
  * Improve layout and accessibility of the image URL generator page, reduce reliance on JavaScript (Temidayo Azeez)
+ * Allow `UniqueConstraint` in place of `unique_together` for {class}`~wagtail.models.TranslatableMixin`'s system check (Temidayo Azeez, Sage Abdullah)
 
 ### Bug fixes
 

+ 39 - 12
wagtail/models/i18n.py

@@ -188,28 +188,55 @@ class TranslatableMixin(models.Model):
     @classmethod
     def check(cls, **kwargs):
         errors = super().check(**kwargs)
+        # No need to check on multi-table-inheritance children as it only needs to be applied to
+        # the table that has the translation_key/locale fields
         is_translation_model = cls.get_translation_model() is cls
+        if not is_translation_model:
+            return errors
 
-        # Raise error if subclass has removed the unique_together constraint
-        # No need to check this on multi-table-inheritance children though as it only needs to be applied to
-        # the table that has the translation_key/locale fields
-        if (
-            is_translation_model
-            and ("translation_key", "locale") not in cls._meta.unique_together
-        ):
+        unique_constraint_fields = ("translation_key", "locale")
+
+        has_unique_constraint = any(
+            isinstance(constraint, models.UniqueConstraint)
+            and set(constraint.fields) == set(unique_constraint_fields)
+            for constraint in cls._meta.constraints
+        )
+
+        has_unique_together = unique_constraint_fields in cls._meta.unique_together
+
+        # Raise error if subclass has removed constraints
+        if not (has_unique_constraint or has_unique_together):
             errors.append(
                 checks.Error(
-                    "{}.{} is missing a unique_together constraint for the translation key and locale fields".format(
-                        cls._meta.app_label, cls.__name__
-                    ),
-                    hint="Add ('translation_key', 'locale') to {}.Meta.unique_together".format(
-                        cls.__name__
+                    "%s is missing a UniqueConstraint for the fields: %s."
+                    % (cls._meta.label, unique_constraint_fields),
+                    hint=(
+                        "Add models.UniqueConstraint(fields=%s, "
+                        "name='unique_translation_key_locale_%s_%s') to %s.Meta.constraints."
+                        % (
+                            unique_constraint_fields,
+                            cls._meta.app_label,
+                            cls._meta.model_name,
+                            cls.__name__,
+                        )
                     ),
                     obj=cls,
                     id="wagtailcore.E003",
                 )
             )
 
+        # Raise error if subclass has both UniqueConstraint and unique_together
+        if has_unique_constraint and has_unique_together:
+            errors.append(
+                checks.Error(
+                    "%s should not have both UniqueConstraint and unique_together for: %s."
+                    % (cls._meta.label, unique_constraint_fields),
+                    hint="Remove unique_together in favor of UniqueConstraint.",
+                    obj=cls,
+                    id="wagtailcore.E003",
+                )
+            )
+
         return errors
 
     @property

+ 70 - 1
wagtail/tests/test_translatablemixin.py

@@ -2,6 +2,7 @@ from unittest.mock import patch
 
 from django.conf import settings
 from django.core import checks
+from django.db import models
 from django.test import TestCase, override_settings
 
 from wagtail.models import Locale
@@ -188,14 +189,82 @@ class TestLocalized(TestCase):
 
 
 class TestSystemChecks(TestCase):
-    def test_raises_error_if_unique_together_constraint_missing(self):
+    def test_unique_together_raises_no_error(self):
+        # The default unique_together should not raise an error
+        errors = TestModel.check()
+        self.assertEqual(len(errors), 0)
+
+    def test_unique_constraint_raises_no_error(self):
+        # Allow replacing unique_together with UniqueConstraint
+        # https://github.com/wagtail/wagtail/issues/11098
         previous_unique_together = TestModel._meta.unique_together
         try:
             TestModel._meta.unique_together = []
+            TestModel._meta.constraints = [
+                models.UniqueConstraint(
+                    fields=["translation_key", "locale"],
+                    name="unique_translation_key_locale_%(app_label)s_%(class)s",
+                )
+            ]
             errors = TestModel.check()
+
         finally:
             TestModel._meta.unique_together = previous_unique_together
+            TestModel._meta.constraints = []
+
+        self.assertEqual(len(errors), 0)
+
+    def test_raises_error_if_both_unique_constraint_and_unique_together_are_missing(
+        self,
+    ):
+        # The model has unique_together and not UniqueConstraint, remove
+        # unique_together to trigger the error
+        previous_unique_together = TestModel._meta.unique_together
+        try:
+            TestModel._meta.unique_together = []
+            errors = TestModel.check()
+        finally:
+            TestModel._meta.unique_together = previous_unique_together
+
+        self.assertEqual(len(errors), 1)
+        self.assertIsInstance(errors[0], checks.Error)
+        self.assertEqual(errors[0].id, "wagtailcore.E003")
+        self.assertEqual(
+            errors[0].msg,
+            "i18n.TestModel is missing a UniqueConstraint for the fields: "
+            "('translation_key', 'locale').",
+        )
+        self.assertEqual(
+            errors[0].hint,
+            "Add models.UniqueConstraint(fields=('translation_key', 'locale'), "
+            "name='unique_translation_key_locale_i18n_testmodel') to "
+            "TestModel.Meta.constraints.",
+        )
+
+    def test_error_with_both_unique_constraint_and_unique_together(self):
+        # The model already has unique_together, add a UniqueConstraint
+        # to trigger the error
+        try:
+            TestModel._meta.constraints = [
+                models.UniqueConstraint(
+                    fields=["translation_key", "locale"],
+                    name="unique_translation_key_locale_%(app_label)s_%(class)s",
+                )
+            ]
+            errors = TestModel.check()
+
+        finally:
+            TestModel._meta.constraints = []
 
         self.assertEqual(len(errors), 1)
         self.assertIsInstance(errors[0], checks.Error)
         self.assertEqual(errors[0].id, "wagtailcore.E003")
+        self.assertEqual(
+            errors[0].msg,
+            "i18n.TestModel should not have both UniqueConstraint and unique_together for: "
+            "('translation_key', 'locale').",
+        )
+        self.assertEqual(
+            errors[0].hint,
+            "Remove unique_together in favor of UniqueConstraint.",
+        )