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

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):
+class RemoveFieldOrSkip(migrations.RemoveField):
-    """Hack method to delete columns (if they already exist).
+    """Custom RemoveField operation which will fail gracefully if the field does not exist
 
-    Due to an improper set of migrations merges,
+    Ref: https://stackoverflow.com/questions/58518726/how-to-ignore-a-specific-migration
-    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
     """
 
-    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
+    def database_forwards(self, app_label, schema_editor, from_state, to_state) -> None:
-    try:
+        """Forwards migration *attempts* to remove existing fields, but will fail gracefully if they do not exist"""
-        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)")
 
-    try:
+        try:
-        print("Checking for column 'choices' in table 'part_partparametertemplate'")
+            super().database_forwards(app_label, schema_editor, from_state, to_state)
-        PartParameterTemplate._meta.get_field('choices')
+            print(f'Removed field {self.name} from model {self.model_name}')
-        schema_editor.execute("ALTER TABLE part_partparametertemplate DROP COLUMN choices;")
+        except Exception as exc:
-    except (AttributeError, FieldDoesNotExist):
+            pass
-        print("Column 'choices' does not exist (skipping)")
+
+    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(
+        RemoveFieldOrSkip(
-            delete_columns, reverse_code=migrations.RunPython.noop
+            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'),

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)