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