[pitivi] timeline: Fix undo layer creation when dragging asset on separator

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

commit 60544607269e4d99a540673f88a22e07626f0501
Author: Alexandru Băluț <alexandru balut gmail com>
Date:   Sun Jun 5 00:01:05 2016 +0200

    timeline: Fix undo layer creation when dragging asset on separator
    
    When dragging an asset from the media library on a separator in the
    timeline a new layer is created and the undo operation should revert its
    addition.
    
    Differential Revision: https://phabricator.freedesktop.org/D1059

 pitivi/timeline/timeline.py |   36 +++++++++----------
 pitivi/utils/ui.py          |    6 ---
 tests/test_undo_timeline.py |   80 ++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 96 insertions(+), 26 deletions(-)
---
diff --git a/pitivi/timeline/timeline.py b/pitivi/timeline/timeline.py
index 3bb7a32..3b2feb1 100644
--- a/pitivi/timeline/timeline.py
+++ b/pitivi/timeline/timeline.py
@@ -305,10 +305,10 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
         # To be able to receive assets dragged from the media library.
         self.drag_dest_add_uri_targets()
 
-        self.connect("drag-motion", self.__dragMotionCb)
-        self.connect("drag-leave", self.__dragLeaveCb)
-        self.connect("drag-drop", self.__dragDropCb)
-        self.connect("drag-data-received", self.__dragDataReceivedCb)
+        self.connect("drag-motion", self._drag_motion_cb)
+        self.connect("drag-leave", self._drag_leave_cb)
+        self.connect("drag-drop", self._drag_drop_cb)
+        self.connect("drag-data-received", self._drag_data_received_cb)
 
     def sendFakeEvent(self, event, event_widget=None):
         # Member usefull for testsing
@@ -739,9 +739,6 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
         self.queue_draw()
 
     def __createClips(self, x, y):
-        if self.dropping_clips:
-            return False
-
         x = self.adjustCoords(x=x)
 
         placement = 0
@@ -785,7 +782,7 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
 
         return True
 
-    def __dragMotionCb(self, unused_widget, context, x, y, timestamp):
+    def _drag_motion_cb(self, unused_widget, context, x, y, timestamp):
         target = self.drag_dest_find_target(context, None)
         if not target:
             Gdk.drag_status(context, 0, timestamp)
@@ -796,14 +793,15 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
             # Ask for the details.
             self.drag_get_data(context, target, timestamp)
         elif target.name() == URI_TARGET_ENTRY.target:
-            if not self.__createClips(x, y):
-                # The clips are already created.
-                self.__dragUpdate(self, x, y)
+            if not self.dropping_clips:
+                # The preview clips have not been created yet.
+                self.__createClips(x, y)
+            self.__dragUpdate(self, x, y)
 
         Gdk.drag_status(context, Gdk.DragAction.COPY, timestamp)
         return True
 
-    def __dragLeaveCb(self, unused_widget, context, unused_timestamp):
+    def _drag_leave_cb(self, unused_widget, context, unused_timestamp):
         # De-highlight the separators. We still need to remember them.
         # See how __on_separators is used in __dragDropCb for details
         self._setSeparatorsPrelight(False)
@@ -832,7 +830,7 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
         self.dropData = None
         self.dropping_clips = False
 
-    def __dragDropCb(self, unused_widget, context, x, y, timestamp):
+    def _drag_drop_cb(self, unused_widget, context, x, y, timestamp):
         # Same as in insertEnd: this value changes during insertion, snapshot
         # it
         zoom_was_fitted = self.parent.zoomed_fitted
@@ -842,13 +840,13 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
         self.cleanDropData()
         if target == URI_TARGET_ENTRY.target:
             if self.__last_clips_on_leave:
-                if self.__on_separators:
-                    created_layer = self.__getDroppedLayer()
-                else:
-                    created_layer = None
                 pipeline = self._project.pipeline
                 with self.app.action_log.started("add clip",
                                                  CommitTimelineFinalizingAction(pipeline)):
+                    if self.__on_separators:
+                        created_layer = self.__getDroppedLayer()
+                    else:
+                        created_layer = None
                     for layer, clip in self.__last_clips_on_leave:
                         if created_layer:
                             layer = created_layer
@@ -864,8 +862,8 @@ class Timeline(Gtk.EventBox, Zoomable, Loggable):
         Gtk.drag_finish(context, success, False, timestamp)
         return success
 
-    def __dragDataReceivedCb(self, unused_widget, unused_context, unused_x,
-                             unused_y, selection_data, unused_info, timestamp):
+    def _drag_data_received_cb(self, unused_widget, unused_context, unused_x,
+                               unused_y, selection_data, unused_info, timestamp):
         data_type = selection_data.get_data_type().name()
         if not self.dropDataReady:
             self.__last_clips_on_leave = None
diff --git a/pitivi/utils/ui.py b/pitivi/utils/ui.py
index 65b7ab5..41cd6e9 100644
--- a/pitivi/utils/ui.py
+++ b/pitivi/utils/ui.py
@@ -50,7 +50,6 @@ from pitivi.utils.misc import path_from_uri
 
 
 # Dimensions in pixels
-TRACK_SPACING = 8
 EXPANDED_SIZE = 65
 CONTROL_WIDTH = 240
 
@@ -61,13 +60,8 @@ PLAYHEAD_WIDTH = 1
 PLAYHEAD_COLOR = (255, 0, 0)
 SNAPBAR_WIDTH = 5
 SNAPBAR_COLOR = (127, 153, 204)
-CANVAS_SPACING = 21
-KEYFRAME_SIZE = 8
 LAYER_HEIGHT = 130
 
-# Layer creation blocking time in s
-LAYER_CREATION_BLOCK_TIME = 0.2
-
 # Drag and drop
 FILE_TARGET_ENTRY = Gtk.TargetEntry.new("text/plain", 0, 0)
 URI_TARGET_ENTRY = Gtk.TargetEntry.new("text/uri-list", 0, 0)
diff --git a/tests/test_undo_timeline.py b/tests/test_undo_timeline.py
index 08864ae..76bac13 100644
--- a/tests/test_undo_timeline.py
+++ b/tests/test_undo_timeline.py
@@ -24,6 +24,7 @@ from gi.repository import Gdk
 from gi.repository import GES
 from gi.repository import Gst
 from gi.repository import GstController
+from gi.repository import Gtk
 
 from pitivi.timeline.layer import Layer
 from pitivi.timeline.timeline import Timeline
@@ -34,6 +35,9 @@ from pitivi.undo.timeline import EffectAddedAction
 from pitivi.undo.timeline import TimelineObserver
 from pitivi.undo.undo import PropertyChangedAction
 from pitivi.undo.undo import UndoableActionLog
+from pitivi.utils.ui import CONTROL_WIDTH
+from pitivi.utils.ui import LAYER_HEIGHT
+from pitivi.utils.ui import URI_TARGET_ENTRY
 from tests import common
 
 
@@ -308,7 +312,6 @@ class TestTimelineUndo(TestCase):
         self.action_log.undo()
         layers = self.timeline.get_layers()
         self.assertEqual(len(layers), 1)
-        layers = self.timeline.get_layers()
         self.assertEqual(layers[0], self.layer)
         self.assertEqual(layers[0].get_clips(), [clip])
 
@@ -319,6 +322,81 @@ class TestTimelineUndo(TestCase):
         self.assertEqual(layers[0].get_clips(), [])
         self.assertEqual(layers[1].get_clips(), [clip])
 
+    def test_media_library_asset_dragged_on_separator(self):
+        """Simulate dragging an asset from the media library to the timeline."""
+        project = self.app.project_manager.current_project
+
+        uri = common.get_sample_uri("tears_of_steel.webm")
+        asset = GES.UriClipAsset.request_sync(uri)
+        self.assertTrue(project.add_asset(asset))
+
+        timeline_ui = Timeline(container=None, app=self.app)
+        timeline_ui.setProject(project)
+        timeline_ui.get_parent = mock.MagicMock()
+        timeline_ui.parent = mock.MagicMock()
+
+        layers = self.timeline.get_layers()
+        self.assertEqual(len(layers), 1)
+
+        # Events emitted while dragging an asset on a separator in the timeline:
+        # motion, receive, motion, leave, drop.
+        with mock.patch.object(Gdk, "drag_status") as drag_status_mock:
+            with mock.patch.object(Gtk, "drag_finish") as drag_finish_mock:
+                target = mock.Mock()
+                target.name.return_value = URI_TARGET_ENTRY.target
+                timeline_ui.drag_dest_find_target = mock.Mock(return_value=target)
+                timeline_ui.drag_get_data = mock.Mock()
+                timeline_ui._drag_motion_cb(None, None, 0, 0, 0)
+                self.assertTrue(timeline_ui.drag_get_data.called)
+
+                self.assertFalse(timeline_ui.dropDataReady)
+                selection_data = mock.Mock()
+                selection_data.get_data_type = mock.Mock(return_value=target)
+                selection_data.get_uris.return_value = [asset.props.id]
+                self.assertIsNone(timeline_ui.dropData)
+                self.assertFalse(timeline_ui.dropDataReady)
+                timeline_ui._drag_data_received_cb(None, None, 0, 0, selection_data, None, 0)
+                self.assertEqual(timeline_ui.dropData, [asset.props.id])
+                self.assertTrue(timeline_ui.dropDataReady)
+
+                timeline_ui.drag_get_data.reset_mock()
+                self.assertIsNone(timeline_ui.draggingElement)
+                self.assertFalse(timeline_ui.dropping_clips)
+
+                def translate_coordinates(widget, x, y):
+                    return x, y
+                timeline_ui.translate_coordinates = translate_coordinates
+                timeline_ui._drag_motion_cb(None, None, CONTROL_WIDTH, LAYER_HEIGHT * 2, 0)
+                self.assertFalse(timeline_ui.drag_get_data.called)
+                self.assertIsNotNone(timeline_ui.draggingElement)
+                self.assertTrue(timeline_ui.dropping_clips)
+
+                timeline_ui._drag_leave_cb(None, None, None)
+                self.assertIsNone(timeline_ui.draggingElement)
+                self.assertFalse(timeline_ui.dropping_clips)
+
+                timeline_ui._drag_drop_cb(None, None, 0, 0, 0)
+
+        # A clip has been created on a new layer below the existing layer.
+        layers = self.timeline.get_layers()
+        self.assertEqual(layers[0], self.layer)
+        self.assertEqual(layers[0].get_clips(), [])
+        self.assertEqual(len(layers), 2)
+        self.assertEqual(len(layers[1].get_clips()), 1)
+
+        self.action_log.undo()
+        layers = self.timeline.get_layers()
+        self.assertEqual(len(layers), 1)
+        self.assertEqual(layers[0], self.layer)
+        self.assertEqual(layers[0].get_clips(), [])
+
+        self.action_log.redo()
+        layers = self.timeline.get_layers()
+        self.assertEqual(len(layers), 2)
+        self.assertEqual(layers[0], self.layer)
+        self.assertEqual(layers[0].get_clips(), [])
+        self.assertEqual(len(layers[1].get_clips()), 1)
+
     def testTrackElementPropertyChanged(self):
         clip1 = GES.TitleClip()
         self.layer.add_clip(clip1)