[pitivi] undo: Fix clip ungroup undo losing video element

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

commit eb168b1ee4361a3ef38f70242d6e8746491a0fcc
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Tue Jan 3 12:02:52 2017 +0100

    undo: Fix clip ungroup undo losing video element
    
    Fixes https://phabricator.freedesktop.org/T7620
    
    Reviewed-by: Thibault Saunier <tsaunier gnome org>
    Differential Revision: https://phabricator.freedesktop.org/D1582

 pitivi/undo/timeline.py     |   31 ++++++++++++++++++++---------
 pitivi/undo/undo.py         |    3 ++
 tests/test_undo_timeline.py |   45 ++++++++++++++++++++++++++++++++++++++----
 3 files changed, 64 insertions(+), 15 deletions(-)
---
diff --git a/pitivi/undo/timeline.py b/pitivi/undo/timeline.py
index 16a00b8..ee1aa47 100644
--- a/pitivi/undo/timeline.py
+++ b/pitivi/undo/timeline.py
@@ -176,7 +176,10 @@ class TrackElementAction(UndoableAction):
         self.clip.remove(self.track_element)
 
 
-class EffectAddedAction(TrackElementAction):
+class TrackElementAdded(TrackElementAction):
+
+    def __repr__(self):
+        return "<TrackElementAdded %s, %s>" % (self.clip, self.track_element)
 
     def do(self):
         self.add()
@@ -188,12 +191,16 @@ class EffectAddedAction(TrackElementAction):
         st = Gst.Structure.new_empty("container-add-child")
         st["container-name"] = self.clip.get_name()
         st["asset-id"] = self.track_element.get_id()
-        st["child-type"] = GObject.type_name(
-            self.track_element.get_asset().get_extractable_type())
+        asset = self.track_element.get_asset()
+        if asset:
+            st["child-type"] = GObject.type_name(asset.get_extractable_type())
         return st
 
 
-class EffectRemovedAction(TrackElementAction):
+class TrackElementRemoved(TrackElementAction):
+
+    def __repr__(self):
+        return "<TrackElementRemoved %s, %s>" % (self.clip, self.track_element)
 
     def do(self):
         self.remove()
@@ -267,6 +274,9 @@ class ClipAdded(UndoableAction):
         self.layer = layer
         self.clip = clip
 
+    def __repr__(self):
+        return "<ClipAdded %s>" % self.clip
+
     def do(self):
         self.clip.set_name(None)
         self.layer.add_clip(self.clip)
@@ -303,6 +313,9 @@ class ClipRemoved(ExpandableUndoableAction):
         self.clip = clip
         self.transition_removed_actions = []
 
+    def __repr__(self):
+        return "<ClipRemoved %s>" % self.clip
+
     def expand(self, action):
         if not isinstance(action, TransitionClipRemovedAction):
             return False
@@ -691,16 +704,14 @@ class LayerObserver(MetaContainerObserver, Loggable):
 
     def _clipTrackElementAddedCb(self, clip, ges_track_element):
         self._connectToTrackElement(ges_track_element)
-        if isinstance(ges_track_element, GES.BaseEffect):
-            action = EffectAddedAction(clip, ges_track_element)
-            self.action_log.push(action)
+        action = TrackElementAdded(clip, ges_track_element)
+        self.action_log.push(action)
 
     def _clipTrackElementRemovedCb(self, clip, ges_track_element):
         self.debug("%s REMOVED from %s", ges_track_element, clip)
         self._disconnectFromTrackElement(ges_track_element)
-        if isinstance(ges_track_element, GES.BaseEffect):
-            action = EffectRemovedAction(clip, ges_track_element)
-            self.action_log.push(action)
+        action = TrackElementRemoved(clip, ges_track_element)
+        self.action_log.push(action)
 
     def __layer_moved_cb(self, ges_layer, unused_param):
         current = ges_layer.props.priority
diff --git a/pitivi/undo/undo.py b/pitivi/undo/undo.py
index 35de307..eaaa692 100644
--- a/pitivi/undo/undo.py
+++ b/pitivi/undo/undo.py
@@ -399,6 +399,9 @@ class PropertyChangedAction(UndoableAutomaticObjectAction):
         self.old_value = old_value
         self.new_value = new_value
 
+    def __repr__(self):
+        return "<PropertyChanged %s.%s: %s -> %s>" % (self.auto_object, self.field_name, self.old_value, 
self.new_value)
+
     def do(self):
         self.auto_object.set_property(self.field_name, self.new_value)
 
diff --git a/tests/test_undo_timeline.py b/tests/test_undo_timeline.py
index 072995e..96a7c96 100644
--- a/tests/test_undo_timeline.py
+++ b/tests/test_undo_timeline.py
@@ -32,7 +32,7 @@ from pitivi.timeline.timeline import TimelineContainer
 from pitivi.undo.project import AssetAddedAction
 from pitivi.undo.timeline import ClipAdded
 from pitivi.undo.timeline import ClipRemoved
-from pitivi.undo.timeline import EffectAddedAction
+from pitivi.undo.timeline import TrackElementAdded
 from pitivi.undo.undo import PropertyChangedAction
 from pitivi.utils.ui import LAYER_HEIGHT
 from pitivi.utils.ui import URI_TARGET_ENTRY
@@ -166,6 +166,41 @@ class TestTimelineObserver(BaseTestUndoTimeline):
             self.assertIsNone(clip1.get_parent())
             self.assertIsNone(clip2.get_parent())
 
+    def test_ungroup_group_clip(self):
+        self.setup_timeline_container()
+        timeline = self.timeline_container.timeline
+
+        uri = common.get_sample_uri("tears_of_steel.webm")
+        asset = GES.UriClipAsset.request_sync(uri)
+        clip = asset.extract()
+        self.layer.add_clip(clip)
+        clips = list(self.getTimelineClips())
+        self.assertEqual(len(clips), 1, clips)
+        self.assertEqual(len(clips[0].get_children(False)), 2)
+
+        timeline.selection.select([clip])
+        timeline.resetSelectionGroup()
+        timeline.current_group.add(clip)
+        self.timeline_container.ungroup_action.activate(None)
+        clips = list(self.getTimelineClips())
+        self.assertEqual(len(clips), 2, clips)
+        self.assertEqual(len(clips[0].get_children(False)), 1)
+        self.assertEqual(len(clips[1].get_children(False)), 1)
+
+        for i in range(2):
+            # Undo ungrouping.
+            self.action_log.undo()
+            clips = list(self.getTimelineClips())
+            self.assertEqual(len(clips), 1, clips)
+            self.assertEqual(len(clips[0].get_children(False)), 2)
+
+            # Redo ungrouping.
+            self.action_log.redo()
+            clips = list(self.getTimelineClips())
+            self.assertEqual(len(clips), 2, clips)
+            self.assertEqual(len(clips[0].get_children(False)), 1)
+            self.assertEqual(len(clips[1].get_children(False)), 1)
+
 
 class TestLayerObserver(BaseTestUndoTimeline):
 
@@ -221,7 +256,7 @@ class TestLayerObserver(BaseTestUndoTimeline):
             self.layer.add_clip(clip1)
 
         stack, = self.action_log.undo_stacks
-        self.assertEqual(len(stack.done_actions), 1, stack.done_actions)
+        self.assertEqual(len(stack.done_actions), 2, stack.done_actions)
         self.assertTrue(isinstance(stack.done_actions[0], ClipAdded))
         self.assertTrue(clip1 in self.getTimelineClips())
 
@@ -332,7 +367,7 @@ class TestLayerObserver(BaseTestUndoTimeline):
         stack = stacks[0]
         self.assertEqual(1, len(stack.done_actions), stack.done_actions)
         action = stack.done_actions[0]
-        self.assertTrue(isinstance(action, EffectAddedAction))
+        self.assertTrue(isinstance(action, TrackElementAdded))
 
         self.assertTrue(effect1 in clip1.get_children(True))
         self.assertEqual(1, len([effect for effect in
@@ -362,7 +397,7 @@ class TestLayerObserver(BaseTestUndoTimeline):
         stack = stacks[0]
         self.assertEqual(1, len(stack.done_actions), stack.done_actions)
         action = stack.done_actions[0]
-        self.assertTrue(isinstance(action, EffectAddedAction))
+        self.assertTrue(isinstance(action, TrackElementAdded))
 
         self.assertTrue(effect1 in clip1.get_children(True))
         self.assertEqual(1, len([effect for effect in
@@ -622,7 +657,7 @@ class TestTimelineElementObserver(BaseTestUndoTimeline):
         stack = stacks[0]
         self.assertEqual(1, len(stack.done_actions), stack.done_actions)
         action = stack.done_actions[0]
-        self.assertTrue(isinstance(action, EffectAddedAction))
+        self.assertTrue(isinstance(action, TrackElementAdded))
 
         self.assertTrue(effect1 in clip1.get_children(True))
         self.assertEqual(1, len([effect for effect in