[pitivi] undo: Fix video transition props propagating

[Alexandru Băluț <alexbalut src gnome org>]

commit da56e23d3de040b7c8a4534d9a618b10dd01e913
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Thu Jun 23 21:49:38 2022 +0200

    undo: Fix video transition props propagating

 pitivi/undo/timeline.py     | 53 +++++++++++++++++++++++++--------------------
 tests/test_undo_timeline.py | 19 ++++++++++++++--
 2 files changed, 47 insertions(+), 25 deletions(-)
---
diff --git a/pitivi/undo/timeline.py b/pitivi/undo/timeline.py
index f12f419b1..d0293278f 100644
--- a/pitivi/undo/timeline.py
+++ b/pitivi/undo/timeline.py
@@ -392,6 +392,12 @@ class TransitionClipAction(UndoableAction):
         self.duration = ges_clip.props.duration
         self.track_element = track_element
 
+        self.properties = []
+        for property_name in TRANSITION_PROPS:
+            field_name = property_name.replace("-", "_")
+            value = self.track_element.get_property(field_name)
+            self.properties.append((property_name, value))
+
     def do(self):
         raise NotImplementedError()
 
@@ -419,6 +425,28 @@ class TransitionClipAction(UndoableAction):
                 return track_element
         return None
 
+    def find_and_update_track_element(self):
+        """Searches the transition clip created automatically to update it.
+
+        When the user moves clips forming a transition to a different layer,
+        the transition clip is removed and recreated automatically and the
+        props are also copied automatically by GES.
+
+        When we undo/redo such an operation, we have to:
+         - identify the corresponding transition object created automatically,
+         - remember the link between the initial object and the new object,
+         - copy the transition element props ourselves from the obsolete to
+           the new element, as GES has no idea what's going on.
+        """
+        track_element = self.find_video_transition()
+        if not track_element:
+            raise ConditionsNotReadyYetError("transition has not been created yet")
+
+        UndoableAutomaticObjectAction.update_object(self.track_element, track_element)
+
+        for prop_name, value in self.properties:
+            track_element.set_property(prop_name, value)
+
 
 class TransitionClipAddedAction(TransitionClipAction):
 
@@ -430,12 +458,7 @@ class TransitionClipAddedAction(TransitionClipAction):
         return cls(ges_layer, ges_clip, track_element)
 
     def do(self):
-        """Searches the transition clip created automatically to update it."""
-        track_element = self.find_video_transition()
-        if not track_element:
-            raise ConditionsNotReadyYetError("transition missing when re-doing ADD")
-
-        UndoableAutomaticObjectAction.update_object(self.track_element, track_element)
+        self.find_and_update_track_element()
 
     def undo(self):
         # The transition will be removed automatically, no need to do it here.
@@ -444,15 +467,6 @@ class TransitionClipAddedAction(TransitionClipAction):
 
 class TransitionClipRemovedAction(TransitionClipAction):
 
-    def __init__(self, ges_layer, ges_clip, track_element):
-        TransitionClipAction.__init__(self, ges_layer, ges_clip, track_element)
-
-        self.properties = []
-        for property_name in TRANSITION_PROPS:
-            field_name = property_name.replace("-", "_")
-            value = self.track_element.get_property(field_name)
-            self.properties.append((property_name, value))
-
     @classmethod
     def new(cls, ges_layer, ges_clip):
         track_element = cls.get_video_element(ges_clip)
@@ -465,14 +479,7 @@ class TransitionClipRemovedAction(TransitionClipAction):
         pass
 
     def undo(self):
-        # Search the transition clip created automatically to update it.
-        track_element = self.find_video_transition()
-        if not track_element:
-            raise ConditionsNotReadyYetError("transition missing when un-doing REMOVE")
-
-        UndoableAutomaticObjectAction.update_object(self.track_element, track_element)
-        for prop_name, value in self.properties:
-            track_element.set_property(prop_name, value)
+        self.find_and_update_track_element()
 
 
 class LayerAdded(UndoableAction):
diff --git a/tests/test_undo_timeline.py b/tests/test_undo_timeline.py
index aaa390255..6dee7ae15 100644
--- a/tests/test_undo_timeline.py
+++ b/tests/test_undo_timeline.py
@@ -577,9 +577,12 @@ class TestLayerObserver(common.TestCase):
     def test_move_transition_to_different_layer_video(self):
         uri = common.get_sample_uri("30fps_numeroted_frames_red.mkv")
         asset = GES.UriClipAsset.request_sync(uri)
-        self.__check_move_transition_to_different_layer(asset)
+        self.__check_move_transition_to_different_layer(asset,
+                                                        border=123,
+                                                        invert=True,
+                                                        
transition_type=GES.VideoStandardTransitionType.BAR_WIPE_LR)
 
-    def __check_move_transition_to_different_layer(self, asset):
+    def __check_move_transition_to_different_layer(self, asset, **props):
         clip1 = asset.extract()
         clip1.set_start(0 * Gst.SECOND)
         self.layer.add_clip(clip1)
@@ -597,6 +600,10 @@ class TestLayerObserver(common.TestCase):
             self.click_clip(clip, expect_selected=True, ctrl_key=True)
         self.timeline_container.group_action.activate()
 
+        track_element = self.get_transition_element(self.layer)
+        for name, value in props.items():
+            track_element.set_property(name, value)
+
         layer2 = self.timeline.append_layer()
         with self.action_log.started("move clips to different layer"):
             editing_context = EditingContext(clip1, self.timeline, GES.EditMode.EDIT_NORMAL, 
GES.Edge.EDGE_NONE, self.app)
@@ -604,16 +611,24 @@ class TestLayerObserver(common.TestCase):
             editing_context.finish()
         self.assertEqual(len(self.layer.get_clips()), 0)
         self.assertEqual(len(layer2.get_clips()), 3)
+        self.__check_transition_element(layer2, props)
 
         with self.project.pipeline.commit_timeline_after():
             self.action_log.undo()
         self.assertEqual(len(self.layer.get_clips()), 3)
         self.assertEqual(len(layer2.get_clips()), 0)
+        self.__check_transition_element(self.layer, props)
 
         with self.project.pipeline.commit_timeline_after():
             self.action_log.redo()
         self.assertEqual(len(self.layer.get_clips()), 0)
         self.assertEqual(len(layer2.get_clips()), 3)
+        self.__check_transition_element(layer2, props)
+
+    def __check_transition_element(self, layer: GES.Layer, props):
+        track_element = self.get_transition_element(layer)
+        for name, value in props.items():
+            self.assertEqual(track_element.get_property(name), value)
 
     @common.setup_timeline
     def test_transition_type(self):