django-cms / djangocms-style

django CMS Style is a plugin for django CMS that allows you to create a HTML container containing classes, styles, ids and other attributes.
https://marketplace.django-cms.org/en/addons/browse/djangocms-style/
Other
38 stars 22 forks source link

Changing settings results in missing migrations #32

Closed bhrutledge closed 7 years ago

bhrutledge commented 7 years ago

I just upgraded to 2.0.0, and discovered that setting DJANGOCMS_STYLE_CHOICES and DJANGOCMS_STYLE_TEMPLATES results in changing the choices for class_name and template, which necessitates new migrations.

I've pasted the relevant bits below. It looks any djangocms plugin that uses this pattern (-link, -picture, etc.) will display this behavior. I am not an expert on migrations, so it's not clear to me what problems it might cause, but it make me nervous about future conflicts, and having migrations outside of source control.

In my settings.py:

DJANGOCMS_STYLE_CHOICES = [
    'm-section',
    'm-highlight',
    'm-recommended',
    'm-content-icon',
    'm-content-title',
    'm-content-body',
    'm-column',
]

DJANGOCMS_STYLE_TEMPLATES = [
    ('recommended', 'Recommended For You'),
]

Output of python manage.py makemigrations:

Migrations for 'djangocms_style':
  0008_auto_20161116_1358.py:
    - Alter field class_name on style
    - Alter field template on style

Contents of venv/lib/python2.7/site-packages/djangocms_style/migrations/0008_auto_20161116_1358.py:

# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import migrations, models

class Migration(migrations.Migration):

    dependencies = [
        ('djangocms_style', '0007_style_template'),
    ]

    operations = [
        migrations.AlterField(
            model_name='style',
            name='class_name',
            field=models.CharField(default=b'm-section', max_length=255, verbose_name='Class name', blank=True, choices=[(b'm-section', b'm-section'), (b'm-highlight', b'm-highlight'), (b'm-recommended', b'm-recommended'), (b'm-content-icon', b'm-content-icon'), (b'm-content-title', b'm-content-title'), (b'm-content-body', b'm-content-body'), (b'm-column', b'm-column')]),
        ),
        migrations.AlterField(
            model_name='style',
            name='template',
            field=models.CharField(default='default', max_length=255, verbose_name='Template', choices=[('default', 'Default'), (b'recommended', b'Recommended For You')]),
        ),
    ]
jsma commented 7 years ago

If you feel up to fixing this yourself, look through the official migrations and find the instances where it points to these raw choices values. You can fork this app and edit the migrations to instead point to the variable name instead of the raw values and then submit a pull request.

FinalAngel commented 7 years ago

this has apparently also been raise on Django itself https://code.djangoproject.com/ticket/22837 @jsma do you know of a way to avoid it?

jsma commented 7 years ago

Can't find examples at the moment of where we've fixed this before in the django CMS ecosystem, but any time a model field points to a variable or callable for choices like https://github.com/divio/djangocms-style/blob/master/djangocms_style/models.py#L69, you need to edit the migrations to import this variable/callable and use that in the migration as well. Otherwise anytime that variable/callable returns a different result than it did last month (e.g. a setting changed), it's going to trigger new migrations. Apologies, but don't have time at the moment to put together a PR.