Browse Source

make streamfield migration names from operations

- `operation_name_fragment` property added to operations
- `migration_name_fragment` property added to MigrateStreamData
- added tests, docs updated
- BaseBlockOperation inherits from `abc.ABC`
Sandil Ranasinghe 2 years ago
parent
commit
3ed84b36da

+ 14 - 1
docs/advanced_topics/streamfield_migrations.md

@@ -483,7 +483,15 @@ Block ids are not preserved here since the new blocks are structurally different
 
 #### Basic usage
 
-While this package comes with a set of operations for common use cases, there may be many instances where you need to define your own operation for mapping data. Making a custom operation is fairly straightforward. All you need to do is extend the `BaseBlockOperation` class and define the `apply` method.
+While this package comes with a set of operations for common use cases, there may be many instances where you need to define your own operation for mapping data. Making a custom operation is fairly straightforward. All you need to do is extend the `BaseBlockOperation` class and define the required methods,
+
+-   `apply`  
+    This applies the actual changes on the existing block value and returns the new block value.
+-   `operation_name_fragment`  
+    (`@property`) Returns a name to be used for generating migration names.
+
+(**NOTE:** `BaseBlockOperation` inherits from `abc.ABC`, so all of the required methods
+mentioned above have to be defined on any class inheriting from it.)
 
 For example, if we want to truncate the string in a `CharBlock` to a given length,
 
@@ -501,6 +509,11 @@ class MyBlockOperation(BaseBlockOperation):
         new_block_value = block_value[:self.length]
         return new_block_value
 
+    
+    @property
+    def operation_name_fragment(self):
+        return "truncate_{}".format(self.length)
+
 ```
 
 #### block_value

+ 11 - 0
wagtail/blocks/migrations/migrate_operation.py

@@ -1,5 +1,6 @@
 import json
 import logging
+from collections import OrderedDict
 from django.db.models import JSONField, F, Q, Subquery, OuterRef
 from django.db.models.functions import Cast
 from django.db.migrations import RunPython
@@ -78,6 +79,16 @@ class MigrateStreamData(RunPython):
 
         return (self.__class__.__qualname__, args, kwargs)
 
+    @property
+    def migration_name_fragment(self):
+        # We are using an OrderedDict here to essentially get the functionality of an ordered set
+        # so that names generated will be consistent.
+        fragments = OrderedDict(
+            (op.operation_name_fragment, None)
+            for op, _ in self.operations_and_block_paths
+        )
+        return "_".join(fragments.keys())
+
     def migrate_stream_data_forward(self, apps, schema_editor):
         model = apps.get_model(self.app_name, self.model_name)
 

+ 45 - 2
wagtail/blocks/migrations/operations.py

@@ -1,13 +1,20 @@
+from abc import ABC, abstractmethod
 from wagtail.blocks.migrations.utils import formatted_list_child_generator
 from django.utils.deconstruct import deconstructible
 
 
-class BaseBlockOperation:
+class BaseBlockOperation(ABC):
     def __init__(self):
         pass
 
+    @abstractmethod
     def apply(self, block_value):
-        raise NotImplementedError
+        pass
+
+    @property
+    @abstractmethod
+    def operation_name_fragment(self):
+        pass
 
 
 @deconstructible
@@ -37,6 +44,10 @@ class RenameStreamChildrenOperation(BaseBlockOperation):
                 mapped_block_value.append(child_block)
         return mapped_block_value
 
+    @property
+    def operation_name_fragment(self):
+        return "rename_{}_to_{}".format(self.old_name, self.new_name)
+
 
 @deconstructible
 class RenameStructChildrenOperation(BaseBlockOperation):
@@ -65,6 +76,10 @@ class RenameStructChildrenOperation(BaseBlockOperation):
                 mapped_block_value[child_key] = child_value
         return mapped_block_value
 
+    @property
+    def operation_name_fragment(self):
+        return "rename_{}_to_{}".format(self.old_name, self.new_name)
+
 
 @deconstructible
 class RemoveStreamChildrenOperation(BaseBlockOperation):
@@ -89,6 +104,10 @@ class RemoveStreamChildrenOperation(BaseBlockOperation):
             if child_block["type"] != self.name
         ]
 
+    @property
+    def operation_name_fragment(self):
+        return "remove_{}".format(self.name)
+
 
 @deconstructible
 class RemoveStructChildrenOperation(BaseBlockOperation):
@@ -113,6 +132,10 @@ class RemoveStructChildrenOperation(BaseBlockOperation):
             if child_key != self.name
         }
 
+    @property
+    def operation_name_fragment(self):
+        return "remove_{}".format(self.name)
+
 
 class StreamChildrenToListBlockOperation(BaseBlockOperation):
     """Combines StreamBlock children of the given type into a new ListBlock
@@ -151,6 +174,10 @@ class StreamChildrenToListBlockOperation(BaseBlockOperation):
             new_temp_blocks.append({**block, "type": "item"})
         self.temp_blocks = new_temp_blocks
 
+    @property
+    def operation_name_fragment(self):
+        return "{}_to_list_block_{}".format(self.block_name, self.list_block_name)
+
 
 class StreamChildrenToStreamBlockOperation(BaseBlockOperation):
     """Combines StreamBlock children of the given types into a new StreamBlock
@@ -183,6 +210,10 @@ class StreamChildrenToStreamBlockOperation(BaseBlockOperation):
         mapped_block_value.append(new_stream_block)
         return mapped_block_value
 
+    @property
+    def operation_name_fragment(self):
+        return "{}_to_stream_block".format("_".join(self.block_names))
+
 
 class AlterBlockValueOperation(BaseBlockOperation):
     """Alters the value of each block to the given value
@@ -198,6 +229,10 @@ class AlterBlockValueOperation(BaseBlockOperation):
     def apply(self, block_value):
         return self.new_value
 
+    @property
+    def operation_name_fragment(self):
+        return "alter_block_value"
+
 
 class StreamChildrenToStructBlockOperation(BaseBlockOperation):
     """Move each StreamBlock child of the given type inside a new StructBlock
@@ -266,6 +301,10 @@ class StreamChildrenToStructBlockOperation(BaseBlockOperation):
                 mapped_block_value.append(child_block)
         return mapped_block_value
 
+    @property
+    def operation_name_fragment(self):
+        return "{}_to_struct_block_{}".format(self.block_name, self.struct_block_name)
+
 
 class ListChildrenToStructBlockOperation(BaseBlockOperation):
     def __init__(self, block_name):
@@ -282,3 +321,7 @@ class ListChildrenToStructBlockOperation(BaseBlockOperation):
                 {**child_block, "value": {self.block_name: child_block["value"]}}
             )
         return mapped_block_value
+
+    @property
+    def operation_name_fragment(self):
+        return "list_block_items_to_{}".format(self.block_name)

+ 13 - 5
wagtail/test/streamfield_migrations/testutils.py

@@ -10,11 +10,7 @@ class MigrationTestMixin:
     default_operation_and_block_path = []
     app_name = None
 
-    def apply_migration(
-        self,
-        revisions_from=None,
-        operations_and_block_path=None,
-    ):
+    def init_migration(self, revisions_from=None, operations_and_block_path=None):
         migration = Migration(
             "test_migration", "wagtail_streamfield_migration_toolkit_test"
         )
@@ -28,6 +24,18 @@ class MigrationTestMixin:
         )
         migration.operations = [migration_operation]
 
+        return migration
+
+    def apply_migration(
+        self,
+        revisions_from=None,
+        operations_and_block_path=None,
+    ):
+        migration = self.init_migration(
+            revisions_from=revisions_from,
+            operations_and_block_path=operations_and_block_path,
+        )
+
         loader = MigrationLoader(connection=connection)
         loader.build_graph()
         project_state = loader.project_state()

+ 59 - 0
wagtail/tests/streamfield_migrations/test_migration_names.py

@@ -0,0 +1,59 @@
+from django.test import TestCase
+
+from wagtail.blocks.migrations.operations import (
+    RemoveStreamChildrenOperation,
+    RenameStreamChildrenOperation,
+)
+from wagtail.test.streamfield_migrations import models
+from wagtail.test.streamfield_migrations.testutils import MigrationTestMixin
+
+
+class MigrationNameTest(TestCase, MigrationTestMixin):
+    model = models.SamplePage
+    app_name = "wagtail_streamfield_migration_toolkit_test"
+
+    def test_rename(self):
+        operations_and_block_path = [
+            (
+                RenameStreamChildrenOperation(old_name="char1", new_name="renamed1"),
+                "",
+            )
+        ]
+        migration = self.init_migration(
+            operations_and_block_path=operations_and_block_path
+        )
+
+        suggested_name = migration.suggest_name()
+        self.assertEqual(suggested_name, "rename_char1_to_renamed1")
+
+    def test_remove(self):
+        operations_and_block_path = [
+            (
+                RemoveStreamChildrenOperation(name="char1"),
+                "",
+            )
+        ]
+        migration = self.init_migration(
+            operations_and_block_path=operations_and_block_path
+        )
+
+        suggested_name = migration.suggest_name()
+        self.assertEqual(suggested_name, "remove_char1")
+
+    def test_multiple(self):
+        operations_and_block_path = [
+            (
+                RenameStreamChildrenOperation(old_name="char1", new_name="renamed1"),
+                "",
+            ),
+            (
+                RemoveStreamChildrenOperation(name="char1"),
+                "simplestruct",
+            ),
+        ]
+        migration = self.init_migration(
+            operations_and_block_path=operations_and_block_path
+        )
+
+        suggested_name = migration.suggest_name()
+        self.assertEqual(suggested_name, "rename_char1_to_renamed1_remove_char1")