Merge lp:~tomasgroth/openlp/bugfixes6 into lp:openlp

Proposed by Tomas Groth
Status: Merged
Approved by: Raoul Snyman
Approved revision: 2451
Merged at revision: 2447
Proposed branch: lp:~tomasgroth/openlp/bugfixes6
Merge into: lp:openlp
Diff against target: 163 lines (+102/-6)
5 files modified
openlp/core/ui/settingsdialog.py (+1/-1)
openlp/core/ui/slidecontroller.py (+4/-2)
openlp/plugins/songs/lib/mediaitem.py (+3/-0)
openlp/plugins/songs/lib/openlyricsexport.py (+8/-3)
tests/functional/openlp_plugins/songs/test_openlyricsexport.py (+86/-0)
To merge this branch: bzr merge lp:~tomasgroth/openlp/bugfixes6
Reviewer Review Type Date Requested Status
Raoul Snyman Approve
Tim Bentley Approve
Review via email: mp+241909@code.launchpad.net

Description of the change

Fix for overwriting files on song export + test. Fixes bug 1216232.
Only show slide-dropdown on live-slidecontroller when it is populated. Fixes bug 1390238.
Make sure that the slidecontroller toolbar layout is correctly adjusted to fit its size. Fixes bug 1387304.
Mark a song edited from preview as coming from plugin. Fixes bug 1382672
Make the settingswindow higher to make room for the new settings. Fixes bug 1377283.

To post a comment you must log in.
Revision history for this message
Tomas Groth (tomasgroth) wrote :
Revision history for this message
Tim Bentley (trb143) :
review: Approve
Revision history for this message
Raoul Snyman (raoul-snyman) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'openlp/core/ui/settingsdialog.py'
--- openlp/core/ui/settingsdialog.py 2014-10-25 20:26:19 +0000
+++ openlp/core/ui/settingsdialog.py 2014-11-16 22:48:24 +0000
@@ -46,7 +46,7 @@
46 """46 """
47 settings_dialog.setObjectName('settings_dialog')47 settings_dialog.setObjectName('settings_dialog')
48 settings_dialog.setWindowIcon(build_icon(u':/icon/openlp-logo.svg'))48 settings_dialog.setWindowIcon(build_icon(u':/icon/openlp-logo.svg'))
49 settings_dialog.resize(800, 500)49 settings_dialog.resize(800, 700)
50 self.dialog_layout = QtGui.QGridLayout(settings_dialog)50 self.dialog_layout = QtGui.QGridLayout(settings_dialog)
51 self.dialog_layout.setObjectName('dialog_layout')51 self.dialog_layout.setObjectName('dialog_layout')
52 self.dialog_layout.setMargin(8)52 self.dialog_layout.setMargin(8)
5353
=== modified file 'openlp/core/ui/slidecontroller.py'
--- openlp/core/ui/slidecontroller.py 2014-11-01 07:47:26 +0000
+++ openlp/core/ui/slidecontroller.py 2014-11-16 22:48:24 +0000
@@ -683,7 +683,8 @@
683 self.play_slides_loop.setChecked(False)683 self.play_slides_loop.setChecked(False)
684 self.play_slides_loop.setIcon(build_icon(':/media/media_time.png'))684 self.play_slides_loop.setIcon(build_icon(':/media/media_time.png'))
685 if item.is_text():685 if item.is_text():
686 if Settings().value(self.main_window.songs_settings_section + '/display songbar') and self.slide_list:686 if (Settings().value(self.main_window.songs_settings_section + '/display songbar')
687 and not self.song_menu.menu().isEmpty()):
687 self.toolbar.set_widget_visible(['song_menu'], True)688 self.toolbar.set_widget_visible(['song_menu'], True)
688 if item.is_capable(ItemCapabilities.CanLoop) and len(item.get_frames()) > 1:689 if item.is_capable(ItemCapabilities.CanLoop) and len(item.get_frames()) > 1:
689 self.toolbar.set_widget_visible(LOOP_LIST)690 self.toolbar.set_widget_visible(LOOP_LIST)
@@ -691,7 +692,8 @@
691 self.mediabar.show()692 self.mediabar.show()
692 self.previous_item.setVisible(not item.is_media())693 self.previous_item.setVisible(not item.is_media())
693 self.next_item.setVisible(not item.is_media())694 self.next_item.setVisible(not item.is_media())
694 self.set_blank_menu()695 # The layout of the toolbar is size dependent, so make sure it fits
696 self.on_controller_size_changed(self.controller.width())
695 # Work-around for OS X, hide and then show the toolbar697 # Work-around for OS X, hide and then show the toolbar
696 # See bug #791050698 # See bug #791050
697 self.toolbar.show()699 self.toolbar.show()
698700
=== modified file 'openlp/plugins/songs/lib/mediaitem.py'
--- openlp/plugins/songs/lib/mediaitem.py 2014-07-09 12:45:54 +0000
+++ openlp/plugins/songs/lib/mediaitem.py 2014-11-16 22:48:24 +0000
@@ -339,6 +339,9 @@
339 self.remote_song = -1339 self.remote_song = -1
340 self.remote_triggered = None340 self.remote_triggered = None
341 if item:341 if item:
342 if preview:
343 # A song can only be edited if it comes from plugin, so we set it again for the new item.
344 item.from_plugin = True
342 return item345 return item
343 return None346 return None
344347
345348
=== modified file 'openlp/plugins/songs/lib/openlyricsexport.py'
--- openlp/plugins/songs/lib/openlyricsexport.py 2014-07-03 16:54:51 +0000
+++ openlp/plugins/songs/lib/openlyricsexport.py 2014-11-16 22:48:24 +0000
@@ -75,9 +75,14 @@
75 filename = '%s (%s)' % (song.title, ', '.join([author.display_name for author in song.authors]))75 filename = '%s (%s)' % (song.title, ', '.join([author.display_name for author in song.authors]))
76 filename = clean_filename(filename)76 filename = clean_filename(filename)
77 # Ensure the filename isn't too long for some filesystems77 # Ensure the filename isn't too long for some filesystems
78 filename = '%s.xml' % filename[0:250 - len(self.save_path)]78 filename_with_ext = '%s.xml' % filename[0:250 - len(self.save_path)]
79 # Make sure we're not overwriting an existing file
80 conflicts = 0
81 while os.path.exists(os.path.join(self.save_path, filename_with_ext)):
82 conflicts += 1
83 filename_with_ext = '%s-%d.xml' % (filename[0:247 - len(self.save_path)], conflicts)
79 # Pass a file object, because lxml does not cope with some special84 # Pass a file object, because lxml does not cope with some special
80 # characters in the path (see lp:757673 and lp:744337).85 # characters in the path (see lp:757673 and lp:744337).
81 tree.write(open(os.path.join(self.save_path, filename), 'wb'), encoding='utf-8', xml_declaration=True,86 tree.write(open(os.path.join(self.save_path, filename_with_ext), 'wb'), encoding='utf-8',
82 pretty_print=True)87 xml_declaration=True, pretty_print=True)
83 return True88 return True
8489
=== added file 'tests/functional/openlp_plugins/songs/test_openlyricsexport.py'
--- tests/functional/openlp_plugins/songs/test_openlyricsexport.py 1970-01-01 00:00:00 +0000
+++ tests/functional/openlp_plugins/songs/test_openlyricsexport.py 2014-11-16 22:48:24 +0000
@@ -0,0 +1,86 @@
1# -*- coding: utf-8 -*-
2# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
3
4###############################################################################
5# OpenLP - Open Source Lyrics Projection #
6# --------------------------------------------------------------------------- #
7# Copyright (c) 2008-2014 Raoul Snyman #
8# Portions copyright (c) 2008-2014 Tim Bentley, Gerald Britton, Jonathan #
9# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, #
10# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. #
11# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, #
12# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, #
13# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, #
14# Frode Woldsund, Martin Zibricky, Patrick Zimmermann #
15# --------------------------------------------------------------------------- #
16# This program is free software; you can redistribute it and/or modify it #
17# under the terms of the GNU General Public License as published by the Free #
18# Software Foundation; version 2 of the License. #
19# #
20# This program is distributed in the hope that it will be useful, but WITHOUT #
21# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or #
22# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for #
23# more details. #
24# #
25# You should have received a copy of the GNU General Public License along #
26# with this program; if not, write to the Free Software Foundation, Inc., 59 #
27# Temple Place, Suite 330, Boston, MA 02111-1307 USA #
28###############################################################################
29"""
30This module contains tests for the OpenLyrics song importer.
31"""
32
33import os
34import shutil
35from unittest import TestCase
36from tempfile import mkdtemp
37
38from tests.functional import MagicMock, patch
39from tests.helpers.testmixin import TestMixin
40from openlp.plugins.songs.lib.openlyricsexport import OpenLyricsExport
41from openlp.core.common import Registry
42
43
44class TestOpenLyricsExport(TestCase, TestMixin):
45 """
46 Test the functions in the :mod:`openlyricsexport` module.
47 """
48 def setUp(self):
49 """
50 Create the registry
51 """
52 Registry.create()
53 self.temp_folder = mkdtemp()
54
55 def tearDown(self):
56 """
57 Cleanup
58 """
59 shutil.rmtree(self.temp_folder)
60
61 def export_same_filename_test(self):
62 """
63 Test that files is not overwritten if songs has same title and author
64 """
65 # GIVEN: A mocked song_to_xml, 2 mocked songs, a mocked application and an OpenLyricsExport instance
66 with patch('openlp.plugins.songs.lib.openlyricsexport.OpenLyrics.song_to_xml') as mocked_song_to_xml:
67 mocked_song_to_xml.return_value = '<?xml version="1.0" encoding="UTF-8"?>\n<empty/>'
68 author = MagicMock()
69 author.display_name = 'Test Author'
70 song = MagicMock()
71 song.authors = [author]
72 song.title = 'Test Title'
73 parent = MagicMock()
74 parent.stop_export_flag = False
75 mocked_application_object = MagicMock()
76 Registry().register('application', mocked_application_object)
77 ol_export = OpenLyricsExport(parent, [song, song], self.temp_folder)
78
79 # WHEN: Doing the export
80 ol_export.do_export()
81
82 # THEN: The exporter should have created 2 files
83 self.assertTrue(os.path.exists(os.path.join(self.temp_folder,
84 '%s (%s).xml' % (song.title, author.display_name))))
85 self.assertTrue(os.path.exists(os.path.join(self.temp_folder,
86 '%s (%s)-1.xml' % (song.title, author.display_name))))