django-cms / djangocms-text-ckeditor

Text Plugin for django CMS using CKEditor 4
https://www.django-cms.org/en/repositories-plugins/
BSD 3-Clause "New" or "Revised" License
164 stars 186 forks source link

Pending new migration with Django>=4.0 #634

Closed sveetch closed 1 year ago

sveetch commented 1 year ago

Bug Report

Expected behavior/code Correctly installed djangocms stack with text-ckeditor plugin should not have any pending migration from packages.

Actual Behavior Currently with Django>=4.0, a new pending migration is found by Django when performing a migration check (makemigrations --check).

From what i found it seems a change from Django 4.0 can lead to Migrations autodetector to produce a new migration because of foreign keys with related_name value that don't use the relation placeholders.

This has been officially related in https://docs.djangoproject.com/en/4.0/releases/4.0/#migrations-autodetector-changes , although there is no explicit information about behaviors.

There is some reports we can found on the web like this one:

https://github.com/jazzband/django-taggit/issues/784

Steps to Reproduce

  1. Install Django 4.0, DjangoCMS 3.11 and last djangocms-text-ckeditor version, run the migrate command;
  2. Then run command python manage.py makemigrations --check --dry-run -v 3
  3. You may see (at least) a new pending migration for djangocms-text-ckeditor that want to set cmsplugin_ptr.related_name to '%(app_label)s_%(class)s'

Environment

Possible Solution From current version, the migration 0003_set_related_name_for_cmsplugin_ptr.py is the last one to set relate_name and changing it from:

# Generated by Django 1.9.2 on 2016-03-04 04:58
import django.db.models.deletion
from django.db import migrations, models

class Migration(migrations.Migration):

    dependencies = [
        ('djangocms_text_ckeditor', '0002_remove_related_name_for_cmsplugin_ptr'),
    ]

    operations = [
        migrations.AlterField(
            model_name='text',
            name='cmsplugin_ptr',
            field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, related_name='djangocms_text_ckeditor_text', serialize=False, to='cms.CMSPlugin'),
        ),
    ]

To use the relation placeholders :

# Generated by Django 1.9.2 on 2016-03-04 04:58
import django.db.models.deletion
from django.db import migrations, models

class Migration(migrations.Migration):

    dependencies = [
        ('djangocms_text_ckeditor', '0002_remove_related_name_for_cmsplugin_ptr'),
    ]

    operations = [
        migrations.AlterField(
            model_name='text',
            name='cmsplugin_ptr',
            field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, related_name='%(app_label)s_%(class)s', serialize=False, to='cms.CMSPlugin'),
        ),
    ]

Is enough to fix the problem, no pending migration is found for djangocms-text-ckeditor and it seems safe for retro compatibility.

Additional context/Screenshots I'm aware this package is not explicitly defined as compatible with Django 4.0 but since DjangoCMS 3.11 include its support and that your package is hardly required by DjangoCMS, i thought it should be expected to be fixed.

Note that currently this is an issue in many CMS plugins like djangocms-picture, djangocms-snippet, etc..

fsbraun commented 1 year ago

@sveetch Thank you for this report. Indeed, Django 4.0 introduces a new migration.

@marksweb What are your thoughts on patchin the last migration touching cmsplugin_ptr? A second solution would be to add a new migration to the repo which changes the related_name.

marksweb commented 1 year ago

@fsbraun if there's no issue adding a new migration then that's my preference.

fsbraun commented 1 year ago

That’s the way we’ve gone with other packages.

fsbraun commented 1 year ago

Solved with #635