[pitivi] undo: Fix effect toggling not undoable

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

commit fe42556f537f5caa407c44eefac9ace181538e9d
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Thu Dec 8 15:42:59 2016 +0100

    undo: Fix effect toggling not undoable
    
    The "active" property of effects was not monitored.
    
    Removed the custom ActivePropertyChanged class since GObjectObserver
    takes care of everything.
    
    Fixes https://phabricator.freedesktop.org/T7641
    
    Reviewed-by: Thibault Saunier <tsaunier gnome org>
    Differential Revision: https://phabricator.freedesktop.org/D1539

 pitivi/undo/timeline.py     |   41 ++++++++++++--------------
 tests/test_undo_timeline.py |   65 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 74 insertions(+), 32 deletions(-)
---
diff --git a/pitivi/undo/timeline.py b/pitivi/undo/timeline.py
index ba6e743..fc317cc 100644
--- a/pitivi/undo/timeline.py
+++ b/pitivi/undo/timeline.py
@@ -124,6 +124,24 @@ class TimelineElementObserver(Loggable):
         self.action_log.push(action)
 
 
+class TrackElementObserver(TimelineElementObserver):
+    """Monitors the props of a track element.
+
+    Reports UndoableActions.
+
+    Args:
+        ges_track_element (GES.TrackElement): The object to be monitored.
+    """
+
+    def __init__(self, ges_track_element, action_log):
+        TimelineElementObserver.__init__(self, ges_track_element, action_log)
+        self.gobject_observer = GObjectObserver(ges_track_element, ("active",), action_log)
+
+    def release(self):
+        TimelineElementObserver.release(self)
+        self.gobject_observer.release()
+
+
 class TrackElementAction(UndoableAction):
 
     def __init__(self, clip, track_element):
@@ -525,22 +543,6 @@ class KeyframeChangedAction(UndoableAction):
         self.control_source.set(time, value)
 
 
-class ActivePropertyChanged(UndoableAction):
-
-    def __init__(self, effect_action, active):
-        UndoableAction.__init__(self)
-        self.effect_action = effect_action
-        self.active = not active
-
-    def do(self):
-        self.effect_action.track_element.active = self.active
-        self.active = not self.active
-
-    def undo(self):
-        self.effect_action.track_element.active = self.active
-        self.active = not self.active
-
-
 class ControlSourceSetAction(SimpleUndoableAction):
 
     def __init__(self, action_info):
@@ -637,7 +639,7 @@ class LayerObserver(MetaContainerObserver, Loggable):
                               self._controlBindingAddedCb)
         if isinstance(track_element, GES.BaseEffect) or \
                 isinstance(track_element, GES.VideoSource):
-            observer = TimelineElementObserver(track_element, self.action_log)
+            observer = TrackElementObserver(track_element, self.action_log)
             self.track_element_observers[track_element] = observer
 
     def _disconnectFromTrackElement(self, track_element):
@@ -691,11 +693,6 @@ class LayerObserver(MetaContainerObserver, Loggable):
             action = EffectRemovedAction(clip, ges_track_element)
             self.action_log.push(action)
 
-    def _trackElementActiveChangedCb(self, track_element, active, add_effect_action):
-        """Handles an effect is (de)activated on a clip in the timeline."""
-        action = ActivePropertyChanged(add_effect_action, active)
-        self.action_log.push(action)
-
     def __layer_moved_cb(self, ges_layer, unused_param):
         current = ges_layer.props.priority
         action = LayerMoved(ges_layer, self.priority, current)
diff --git a/tests/test_undo_timeline.py b/tests/test_undo_timeline.py
index f4096c6..abbe7d3 100644
--- a/tests/test_undo_timeline.py
+++ b/tests/test_undo_timeline.py
@@ -78,6 +78,11 @@ class BaseTestUndoTimeline(TestCase):
         mainloop.run()
         self.assertTrue(self.timeline.props.auto_transition)
 
+    def assert_effect_count(self, clip, count):
+        effects = [effect for effect in clip.get_children(True)
+                   if isinstance(effect, GES.Effect)]
+        self.assertEqual(len(effects), count)
+
 
 class TestTimelineObserver(BaseTestUndoTimeline):
 
@@ -358,20 +363,13 @@ class TestLayerObserver(BaseTestUndoTimeline):
 
         with self.action_log.started("remove effect"):
             clip1.remove(effect1)
-
-        self.assertEqual(0, len([effect for effect in
-                                 clip1.get_children(True)
-                                 if isinstance(effect, GES.Effect)]))
+        self.assert_effect_count(clip1, 0)
 
         self.action_log.undo()
-        self.assertEqual(1, len([effect for effect in
-                                 clip1.get_children(True)
-                                 if isinstance(effect, GES.Effect)]))
+        self.assert_effect_count(clip1, 1)
 
         self.action_log.redo()
-        self.assertEqual(0, len([effect for effect in
-                                 clip1.get_children(True)
-                                 if isinstance(effect, GES.Effect)]))
+        self.assert_effect_count(clip1, 0)
 
     def test_move_clip(self):
         self._wait_until_project_loaded()
@@ -673,6 +671,53 @@ class TestGObjectObserver(BaseTestUndoTimeline):
         self.action_log.redo()
         self.assertEqual(20, clip1.get_priority())
 
+    def test_effect_toggling(self):
+        clip1 = GES.TitleClip()
+        self.layer.add_clip(clip1)
+
+        effect1 = GES.Effect.new("agingtv")
+        with self.action_log.started("add effect"):
+            clip1.add(effect1)
+        self.assertTrue(effect1.props.active)
+        self.assert_effect_count(clip1, 1)
+
+        with self.action_log.started("toggle effect"):
+            effect1.props.active = False
+        self.assertFalse(effect1.props.active)
+
+        with self.action_log.started("remove effect"):
+            clip1.remove(effect1)
+        self.assert_effect_count(clip1, 0)
+
+        # Undo effect removing.
+        self.action_log.undo()
+        self.assert_effect_count(clip1, 1)
+
+        # Undo effect toggling.
+        self.action_log.undo()
+        self.assertTrue(effect1.props.active)
+
+        # Redo effect toggling.
+        self.action_log.redo()
+        self.assertFalse(effect1.props.active)
+
+        # Undo effect toggling.
+        self.action_log.undo()
+        self.assertTrue(effect1.props.active)
+
+        # Undo effect add.
+        self.action_log.undo()
+        self.assertFalse(effect1 in clip1.get_children(True))
+
+        # Redo effect add.
+        self.action_log.redo()
+        self.assertTrue(effect1 in clip1.get_children(True))
+        self.assertTrue(effect1.props.active)
+
+        # Redo effect toggling.
+        self.action_log.redo()
+        self.assertFalse(effect1.props.active)
+
 
 class TestDragDropUndo(BaseTestUndoTimeline):