openlp-core team mailing list archive

[Tomas Groth <tomasgroth@xxxxxxxx>]

Tomas Groth has proposed merging lp:~tomasgroth/openlp/bugfixes3 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1247025 in OpenLP: "Wrong position when drag and drop an item in the service list"
  https://bugs.launchpad.net/openlp/+bug/1247025
  Bug #1387286 in OpenLP: "CD clip gives unsupported filetype error"
  https://bugs.launchpad.net/openlp/+bug/1387286

For more details, see:
https://code.launchpad.net/~tomasgroth/openlp/bugfixes3/+merge/240341

Fix for 3 bugs:
#1387286, playing audio CD clips on windows.
#1247025, Position fix for d'n'd to servicelist, copied from lp:~oliwee/openlp/bug-1247025.
Fix traceback on theme import in windows.
-- 
https://code.launchpad.net/~tomasgroth/openlp/bugfixes3/+merge/240341
Your team OpenLP Core is requested to review the proposed merge of lp:~tomasgroth/openlp/bugfixes3 into lp:openlp.

=== modified file 'openlp/core/lib/mediamanageritem.py'
--- openlp/core/lib/mediamanageritem.py	2014-05-19 19:19:05 +0000
+++ openlp/core/lib/mediamanageritem.py	2014-10-31 22:50:44 +0000
@@ -563,8 +563,11 @@
                 self.add_to_service(replace=self.remote_triggered)
             else:
                 items = self.list_view.selectedIndexes()
+                drop_position = self.service_manager.get_drop_position()
                 for item in items:
-                    self.add_to_service(item)
+                    self.add_to_service(item, position=drop_position)
+                    if drop_position != -1:
+                        drop_position += 1
 
     def add_to_service_remote(self, message):
         """
@@ -574,18 +577,19 @@
         """
         self.add_to_service(message[0], remote=message[1])
 
-    def add_to_service(self, item=None, replace=None, remote=False):
+    def add_to_service(self, item=None, replace=None, remote=False, position=-1):
         """
         Add this item to the current service.
 
         :param item: Item to be processed
         :param replace: Replace the existing item
         :param remote: Triggered from remote
+        :param position: Position to place item
         """
         service_item = self.build_service_item(item, True, remote=remote, context=ServiceItemContext.Service)
         if service_item:
             service_item.from_plugin = False
-            self.service_manager.add_service_item(service_item, replace=replace)
+            self.service_manager.add_service_item(service_item, replace=replace, position=position)
 
     def on_add_edit_click(self):
         """

=== modified file 'openlp/core/ui/media/vlcplayer.py'
--- openlp/core/ui/media/vlcplayer.py	2014-10-30 22:53:06 +0000
+++ openlp/core/ui/media/vlcplayer.py	2014-10-31 22:50:44 +0000
@@ -168,6 +168,8 @@
         path = os.path.normcase(file_path)
         # create the media
         if controller.media_info.media_type == MediaType.CD:
+            if is_win():
+                path = '/' + path
             display.vlc_media = display.vlc_instance.media_new_location('cdda://' + path)
             display.vlc_media_player.set_media(display.vlc_media)
             display.vlc_media_player.play()

=== modified file 'openlp/core/ui/servicemanager.py'
--- openlp/core/ui/servicemanager.py	2014-08-25 20:34:54 +0000
+++ openlp/core/ui/servicemanager.py	2014-10-31 22:50:44 +0000
@@ -324,7 +324,7 @@
         self.inactive = build_icon(':/media/auto-start_inactive.png')
         self.service_items = []
         self.suffixes = []
-        self.drop_position = 0
+        self.drop_position = -1
         self.service_id = 0
         # is a new service and has not been saved
         self._modified = False
@@ -1377,7 +1377,8 @@
                 self.live_controller.replace_service_manager_item(new_item)
                 self.set_modified()
 
-    def add_service_item(self, item, rebuild=False, expand=None, replace=False, repaint=True, selected=False):
+    def add_service_item(self, item, rebuild=False, expand=None, replace=False, repaint=True, selected=False,
+                         position=-1):
         """
         Add a Service item to the list
 
@@ -1387,11 +1388,13 @@
         :param replace: Is the service item a replacement (Default False)
         :param repaint: Do we need to repaint the service item list (Default True)
         :param selected: Has the item been selected (Default False)
+        :param position: The position where the item is dropped (Default -1)
         """
         # if not passed set to config value
         if expand is None:
             expand = Settings().value('advanced/expand service item')
         item.from_service = True
+        self.drop_position = position
         if replace:
             s_item, child = self.find_service_item()
             item.merge(self.service_items[s_item]['service_item'])
@@ -1401,7 +1404,7 @@
         else:
             item.render()
             # nothing selected for dnd
-            if self.drop_position == 0:
+            if self.drop_position == -1:
                 if isinstance(item, list):
                     for ind_item in item:
                         self.service_items.append({'service_item': ind_item,
@@ -1421,7 +1424,7 @@
             # if rebuilding list make sure live is fixed.
             if rebuild:
                 self.live_controller.replace_service_manager_item(item)
-        self.drop_position = 0
+        self.drop_position = -1
         self.set_modified()
 
     def make_preview(self):
@@ -1604,7 +1607,7 @@
                             item.setSelected(True)
                             replace = True
                     else:
-                        self.drop_position = self._get_parent_item_data(item)
+                        self.drop_position = self._get_parent_item_data(item) - 1
                 Registry().execute('%s_add_service_item' % plugin, replace)
 
     def update_theme_list(self, theme_list):
@@ -1666,3 +1669,9 @@
         """
         setting_dialog = PrintServiceForm()
         setting_dialog.exec_()
+
+    def get_drop_position(self):
+        """
+        Getter for drop_position. Used in: MediaManagerItem
+        """
+        return self.drop_position

=== modified file 'openlp/core/ui/thememanager.py'
--- openlp/core/ui/thememanager.py	2014-08-04 17:12:03 +0000
+++ openlp/core/ui/thememanager.py	2014-10-31 22:50:44 +0000
@@ -563,12 +563,12 @@
             else:
                 abort_import = False
             for name in theme_zip.namelist():
-                name = name.replace('/', os.path.sep)
-                split_name = name.split(os.path.sep)
+                out_name = name.replace('/', os.path.sep)
+                split_name = out_name.split(os.path.sep)
                 if split_name[-1] == '' or len(split_name) == 1:
                     # is directory or preview file
                     continue
-                full_name = os.path.join(directory, name)
+                full_name = os.path.join(directory, out_name)
                 check_directory_exists(os.path.dirname(full_name))
                 if os.path.splitext(name)[1].lower() == '.xml':
                     file_xml = str(theme_zip.read(name), 'utf-8')

=== modified file 'openlp/plugins/media/forms/mediaclipselectorform.py'
--- openlp/plugins/media/forms/mediaclipselectorform.py	2014-09-08 20:49:33 +0000
+++ openlp/plugins/media/forms/mediaclipselectorform.py	2014-10-31 22:50:44 +0000
@@ -219,7 +219,7 @@
             return
         # VLC behaves a bit differently on windows and linux when loading, which creates problems when trying to
         # detect if we're dealing with a DVD or CD, so we use different loading approaches depending on the OS.
-        if os.name == 'nt':
+        if is_win():
             # If the given path is in the format "D:\" or "D:", prefix it with "/" to make VLC happy
             pattern = re.compile('^\w:\\\\*$')
             if pattern.match(path):

=== modified file 'tests/functional/openlp_core_ui/test_thememanager.py'
--- tests/functional/openlp_core_ui/test_thememanager.py	2014-09-10 08:07:10 +0000
+++ tests/functional/openlp_core_ui/test_thememanager.py	2014-10-31 22:50:44 +0000
@@ -31,9 +31,11 @@
 """
 import zipfile
 import os
+import shutil
 
 from unittest import TestCase
 from tests.interfaces import MagicMock
+from tempfile import mkdtemp
 
 from openlp.core.ui import ThemeManager
 from openlp.core.common import Registry
@@ -135,3 +137,25 @@
 
             # THEN: The mocked_copyfile should not have been called
             self.assertTrue(mocked_copyfile.called, 'shutil.copyfile should be called')
+
+    def unzip_theme_test(self):
+        """
+        Test that unzipping of themes works
+        """
+        # GIVEN: A theme file, a output folder and some mocked out internal functions
+        with patch('openlp.core.ui.thememanager.critical_error_message_box') \
+                as mocked_critical_error_message_box:
+            theme_manager = ThemeManager(None)
+            theme_manager._create_theme_from_xml = MagicMock()
+            theme_manager.generate_and_save_image = MagicMock()
+            theme_manager.path = ''
+            folder = mkdtemp()
+            theme_file = os.path.join(TEST_RESOURCES_PATH, 'themes', 'Moss_on_tree.otz')
+
+            # WHEN: We try to unzip it
+            theme_manager.unzip_theme(theme_file, folder)
+
+            # THEN: Files should be unpacked
+            self.assertTrue(os.path.exists(os.path.join(folder, 'Moss on tree', 'Moss on tree.xml')))
+            self.assertEqual(mocked_critical_error_message_box.call_count, 0, 'No errors should have happened')
+            shutil.rmtree(folder)

=== added file 'tests/resources/themes/Moss_on_tree.otz'
Binary files tests/resources/themes/Moss_on_tree.otz	1970-01-01 00:00:00 +0000 and tests/resources/themes/Moss_on_tree.otz	2014-10-31 22:50:44 +0000 differ