Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: Detect and prevent the use of AlterModelTable #259

Open
JanChec opened this issue Jul 21, 2023 · 2 comments
Open

Enhancement: Detect and prevent the use of AlterModelTable #259

JanChec opened this issue Jul 21, 2023 · 2 comments

Comments

@JanChec
Copy link

JanChec commented Jul 21, 2023

Hello django-migration-linter maintainers,

First of all, thank you for maintaining this extremely helpful tool. I've used it in several projects and it has saved me from potential issues and bugs a number of times.

I am opening this issue to suggest an enhancement that I believe would increase the usefulness of the django-migration-linter further, especially for developers dealing with projects where maintaining backward compatibility is crucial.

Currently, as per my understanding, django-migration-linter does not detect or prevent the use of AlterModelTable, a migration operation in Django that changes the name of a database table. This operation is capable of breaking backward compatibility.

The absence of detection of such operations might potentially lead to issues in production if not caught in time. Therefore, it would be immensely beneficial if django-migration-linter could also detect and prevent the use of AlterModelTable.

Here is an example of a situation that might cause problems:

# Assume the initial model
class MyModel(models.Model):
    pass

# Now, we change the db_table parameter
class MyModel(models.Model):
    class Meta:
        db_table = 'my_new_table'

This will generate a migration with AlterModelTable:

migrations.AlterModelTable(
    name='mymodel',
    table='my_new_table',
),

I would like to suggest that django-migration-linter should flag this migration as a potential issue because it modifies the table name and could break backward compatibility.

Thank you for considering this enhancement. If you need more information or further clarification, feel free to ask. I would be more than happy to help.

@David-Wobrock
Copy link
Collaborator

Hey @JanChec,

I'm happy that the linter is useful to you 🙏

The linter currently checks for RENAME TABLE and ALTER TABLE .* RENAME TO statements to prevent table renaming.
See here: https://github.com/3YOURMIND/django-migration-linter/blob/v5.0.0/django_migration_linter/sql_analyser/base.py#L107-L109
Would that correspond to the AlterModelTable that you are describing? 🙂

@JanChec
Copy link
Author

JanChec commented Jul 25, 2023

Unfortunately, this doesn't prevent the case of moving the model from app to app when you have to split the state:

# In the migration in the previous app:

    operations = [
        migrations.SeparateDatabaseAndState(
            state_operations=[
                migrations.DeleteModel(
                    name='MyModel',
                ),
            ],
            database_operations=[
                migrations.AlterModelTable(
                    name='MyModel',
                    table='newapp_mymodel',
                )
            ],
        )
    ]

# And in the migration in the new app:

    operations = [
        migrations.SeparateDatabaseAndState(
            state_operations=[
                migrations.CreateModel(
                    name='MyModel',
                    # ...
                ),
            ],
            database_operations=[],
        )
    ]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants