From 842d7a93d53844af21d959ab093d76e032df3c28 Mon Sep 17 00:00:00 2001 From: Oliver Date: Thu, 8 Jun 2023 21:12:57 +1000 Subject: [PATCH] Fix for faulty data migrations (#4987) * Update part.migrations.0112 - Add custom migration class which handles errors * Add unit test for migration - Ensure that the new fields are added to the model * Update reference to PR --- .../migrations/0112_auto_20230525_1606.py | 98 +++++++++++++------ InvenTree/part/test_migrations.py | 35 +++++++ 2 files changed, 104 insertions(+), 29 deletions(-) diff --git a/InvenTree/part/migrations/0112_auto_20230525_1606.py b/InvenTree/part/migrations/0112_auto_20230525_1606.py index ff7d296336..43e7d2abf5 100644 --- a/InvenTree/part/migrations/0112_auto_20230525_1606.py +++ b/InvenTree/part/migrations/0112_auto_20230525_1606.py @@ -1,56 +1,96 @@ # Generated by Django 3.2.19 on 2023-05-31 12:05 -from django.core.exceptions import FieldDoesNotExist +""" +Note: This is a total hack method to delete columns (if they already exist). + +Due to an improper set of migrations merges, +there may exist a situation where the columns (defined in the migrations below) already exist. + +In this case, we want to delete the columns, and then re-add them. + +Original error: https://github.com/inventree/InvenTree/pull/4898 +1st fix: https://github.com/inventree/InvenTree/pull/4961 +2nd fix: https://github.com/inventree/InvenTree/pull/4977 +3rd fix: https://github.com/inventree/InvenTree/pull/4987 +""" + from django.db import migrations, models -def delete_columns(apps, schema_editor): - """Hack method to delete columns (if they already exist). +class RemoveFieldOrSkip(migrations.RemoveField): + """Custom RemoveField operation which will fail gracefully if the field does not exist - Due to an improper set of migrations merges, - there may exist a situation where the columns (defined in the migrations below) already exist. - - In this case, we want to delete the columns, and then re-add them. - - Original error: https://github.com/inventree/InvenTree/pull/4898 - Attempted fix: https://github.com/inventree/InvenTree/pull/4961 - This fix: https://github.com/inventree/InvenTree/pull/4977 + Ref: https://stackoverflow.com/questions/58518726/how-to-ignore-a-specific-migration """ - PartParameterTemplate = apps.get_model('part', 'PartParameterTemplate') + def database_backwards(self, app_label, schema_editor, from_state, to_state) -> None: + # Backwards migration should not do anything + pass - # Check if the 'checkbox' column exists - try: - print("Checking for column 'checkbox' in table 'part_partparametertemplate'") - PartParameterTemplate._meta.get_field('checkbox') - schema_editor.execute("ALTER TABLE part_partparametertemplate DROP COLUMN checkbox;") - except (AttributeError, FieldDoesNotExist): - print("Column 'checkbox' does not exist (skipping)") + def database_forwards(self, app_label, schema_editor, from_state, to_state) -> None: + """Forwards migration *attempts* to remove existing fields, but will fail gracefully if they do not exist""" - try: - print("Checking for column 'choices' in table 'part_partparametertemplate'") - PartParameterTemplate._meta.get_field('choices') - schema_editor.execute("ALTER TABLE part_partparametertemplate DROP COLUMN choices;") - except (AttributeError, FieldDoesNotExist): - print("Column 'choices' does not exist (skipping)") + try: + super().database_forwards(app_label, schema_editor, from_state, to_state) + print(f'Removed field {self.name} from model {self.model_name}') + except Exception as exc: + pass + + def state_forwards(self, app_label, state) -> None: + try: + super().state_forwards(app_label, state) + except Exception: + pass + +class AddFieldOrSkip(migrations.AddField): + """Custom AddField operation which will fail gracefully if the field already exists + + Ref: https://stackoverflow.com/questions/58518726/how-to-ignore-a-specific-migration + """ + + def database_backwards(self, app_label, schema_editor, from_state, to_state) -> None: + # Backwards migration should not do anything + pass + + def database_forwards(self, app_label, schema_editor, from_state, to_state) -> None: + """Forwards migration *attempts* to remove existing fields, but will fail gracefully if they do not exist""" + + try: + super().database_forwards(app_label, schema_editor, from_state, to_state) + print(f'Added field {self.name} to model {self.model_name}') + except Exception as exc: + pass + + def state_forwards(self, app_label, state) -> None: + try: + super().state_forwards(app_label, state) + except Exception: + pass class Migration(migrations.Migration): + atomic = False + dependencies = [ ('part', '0111_auto_20230521_1350'), ] operations = [ - migrations.RunPython( - delete_columns, reverse_code=migrations.RunPython.noop + RemoveFieldOrSkip( + model_name='partparametertemplate', + name='checkbox', ), - migrations.AddField( + RemoveFieldOrSkip( + model_name='partparametertemplate', + name='choices', + ), + AddFieldOrSkip( model_name='partparametertemplate', name='checkbox', field=models.BooleanField(default=False, help_text='Is this parameter a checkbox?', verbose_name='Checkbox'), ), - migrations.AddField( + AddFieldOrSkip( model_name='partparametertemplate', name='choices', field=models.CharField(blank=True, help_text='Valid choices for this parameter (comma-separated)', max_length=5000, verbose_name='Choices'), diff --git a/InvenTree/part/test_migrations.py b/InvenTree/part/test_migrations.py index f2eed6f315..bd0e3aa173 100644 --- a/InvenTree/part/test_migrations.py +++ b/InvenTree/part/test_migrations.py @@ -189,3 +189,38 @@ class PartUnitsMigrationTest(MigratorTestCase): self.assertEqual(part_2.units, 'inch') self.assertEqual(part_3.units, '') self.assertEqual(part_4.units, 'percent') + + +class TestPartParameterTemplateMigration(MigratorTestCase): + """Test for data migration of PartParameterTemplate + + Ref: https://github.com/inventree/InvenTree/pull/4987 + """ + + migrate_from = ('part', '0110_alter_part_units') + migrate_to = ('part', '0113_auto_20230531_1205') + + def prepare(self): + """Prepare some parts with units""" + + PartParameterTemplate = self.old_state.apps.get_model('part', 'partparametertemplate') + + # Create a test template + template = PartParameterTemplate.objects.create(name='Template 1', description='a part parameter template') + + # Ensure that the 'choices' and 'checkbox' fields do not exist + with self.assertRaises(AttributeError): + template.choices + + with self.assertRaises(AttributeError): + template.checkbox + + def test_units_migration(self): + """Test that the new fields have been added correctly""" + + PartParameterTemplate = self.new_state.apps.get_model('part', 'partparametertemplate') + + template = PartParameterTemplate.objects.get(name='Template 1') + + self.assertEqual(template.choices, '') + self.assertEqual(template.checkbox, False)