From 3a5b7255adfb7eefbd9a48b56fdb7654b636d7cb Mon Sep 17 00:00:00 2001 From: David Beitey <david@davidjb.com> Date: Thu, 3 Dec 2020 17:36:47 +1000 Subject: [PATCH] Change UniqueConstraint for wider database support (#6607) This fixes #6393 by modifying the constraint to use an IN condition which supports both Postgres and SQL Server. Previously, the `|` (OR) condition was only supported by Postgres because SQL Server only supports AND conditions. The implementation follows suggestions from @gasman in https://github.com/wagtail/wagtail/issues/6393#issuecomment-732161057: * Migration 0050 is modified to not break on SQL Server * Added migration 0060 to add or replace the constraint Additionally, this allows for and documents a `DATABASE_DRIVER` env variable to be set for testing, to allow a different SQL Server driver (e.g. FreeTDS on Mac/Linux); and adds the specific `host_is_server` option for FreeTDS (won't affect SQL Server Native Client on CI). --- CHANGELOG.txt | 1 + docs/contributing/developing.rst | 2 ++ docs/releases/2.12.rst | 1 + ...0050_workflow_rejected_to_needs_changes.py | 2 +- .../0060_fix_workflow_unique_constraint.py | 21 +++++++++++++++++++ wagtail/core/models.py | 4 ++-- wagtail/tests/settings.py | 3 ++- 7 files changed, 30 insertions(+), 4 deletions(-) create mode 100644 wagtail/core/migrations/0060_fix_workflow_unique_constraint.py diff --git a/CHANGELOG.txt b/CHANGELOG.txt index e2fe6c7e01..1cca4eaa42 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -15,6 +15,7 @@ Changelog * Fix: Stop menu icon overlapping the breadcrumb on small viewport widths in page editor (Karran Besen) * Fix: Make sure document chooser pagination preserves the selected collection when moving between pages (Alex Sa) * Fix: Gracefully handle oEmbed endpoints returning non-JSON responses (Matt Westcott) + * Fix: Fix unique constraint on WorkflowState for SQL Server compatibility (David Beitey) 2.11.3 (10.12.2020) diff --git a/docs/contributing/developing.rst b/docs/contributing/developing.rst index cc269ed5ce..488b636891 100644 --- a/docs/contributing/developing.rst +++ b/docs/contributing/developing.rst @@ -147,6 +147,8 @@ If you need to use different connection settings, use the following environment * ``DATABASE_PORT`` +It is also possible to set ``DATABASE_DRIVER``, which corresponds to the `driver` value within `OPTIONS` if an SQL Server engine is used. + Testing Elasticsearch --------------------- diff --git a/docs/releases/2.12.rst b/docs/releases/2.12.rst index 5ddda869d7..dbb06e916f 100644 --- a/docs/releases/2.12.rst +++ b/docs/releases/2.12.rst @@ -43,6 +43,7 @@ Bug fixes * Stop menu icon overlapping the breadcrumb on small viewport widths in page editor (Karran Besen) * Make sure document chooser pagination preserves the selected collection when moving between pages (Alex Sa) * Gracefully handle oEmbed endpoints returning non-JSON responses (Matt Westcott) + * Fix unique constraint on WorkflowState for SQL Server compatibility (David Beitey) Upgrade considerations diff --git a/wagtail/core/migrations/0050_workflow_rejected_to_needs_changes.py b/wagtail/core/migrations/0050_workflow_rejected_to_needs_changes.py index 747eecbeb2..0178b74b25 100644 --- a/wagtail/core/migrations/0050_workflow_rejected_to_needs_changes.py +++ b/wagtail/core/migrations/0050_workflow_rejected_to_needs_changes.py @@ -21,6 +21,6 @@ class Migration(migrations.Migration): ), migrations.AddConstraint( model_name='workflowstate', - constraint=models.UniqueConstraint(condition=models.Q(('status', 'in_progress'), ('status', 'needs_changes'), _connector='OR'), fields=('page',), name='unique_in_progress_workflow'), + constraint=models.UniqueConstraint(condition=models.Q(status__in=('in_progress', 'needs_changes')), fields=('page',), name='unique_in_progress_workflow'), ), ] diff --git a/wagtail/core/migrations/0060_fix_workflow_unique_constraint.py b/wagtail/core/migrations/0060_fix_workflow_unique_constraint.py new file mode 100644 index 0000000000..cea03cc449 --- /dev/null +++ b/wagtail/core/migrations/0060_fix_workflow_unique_constraint.py @@ -0,0 +1,21 @@ +# Generated by Django 3.1.3 on 2020-11-27 01:19 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('wagtailcore', '0059_apply_collection_ordering'), + ] + + operations = [ + migrations.RemoveConstraint( + model_name='workflowstate', + name='unique_in_progress_workflow', + ), + migrations.AddConstraint( + model_name='workflowstate', + constraint=models.UniqueConstraint(condition=models.Q(status__in=('in_progress', 'needs_changes')), fields=('page',), name='unique_in_progress_workflow'), + ), + ] diff --git a/wagtail/core/models.py b/wagtail/core/models.py index a1bec2fd6e..01ffcccdf7 100644 --- a/wagtail/core/models.py +++ b/wagtail/core/models.py @@ -4316,9 +4316,9 @@ class WorkflowState(models.Model): class Meta: verbose_name = _('Workflow state') verbose_name_plural = _('Workflow states') - # prevent multiple STATUS_IN_PROGRESS/STATUS_NEEDS_CHANGES workflows for the same page. This is not supported by MySQL, so is checked additionally on save. + # prevent multiple STATUS_IN_PROGRESS/STATUS_NEEDS_CHANGES workflows for the same page. This is only supported by specific databases (e.g. Postgres, SQL Server), so is checked additionally on save. constraints = [ - models.UniqueConstraint(fields=['page'], condition=(Q(status='in_progress') | Q(status='needs_changes')), name='unique_in_progress_workflow') + models.UniqueConstraint(fields=['page'], condition=Q(status__in=('in_progress', 'needs_changes')), name='unique_in_progress_workflow') ] diff --git a/wagtail/tests/settings.py b/wagtail/tests/settings.py index 09196de07f..d0df9d3de8 100644 --- a/wagtail/tests/settings.py +++ b/wagtail/tests/settings.py @@ -33,8 +33,9 @@ if DATABASES['default']['ENGINE'] != 'django.db.backends.sqlite3': # Add extra options when mssql is used (on for example appveyor) if DATABASES['default']['ENGINE'] == 'sql_server.pyodbc': DATABASES['default']['OPTIONS'] = { - 'driver': 'SQL Server Native Client 11.0', + 'driver': os.environ.get('DATABASE_DRIVER', 'SQL Server Native Client 11.0'), 'MARS_Connection': 'True', + 'host_is_server': True, # Applies to FreeTDS driver only }