Status: | Merged |
---|---|
Approved by: | Andreas Preikschat |
Approved revision: | 2264 |
Merged at revision: | 2283 |
Proposed branch: | lp:~phill-ridout/openlp/171891 |
Merge into: | lp:openlp |
Diff against target: |
281 lines (+218/-14) 2 files modified
openlp/plugins/songs/lib/foilpresenterimport.py (+23/-14) tests/functional/openlp_plugins/songs/test_foilpresenterimport.py (+195/-0) |
To merge this branch: | bzr merge lp:~phill-ridout/openlp/171891 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Andreas Preikschat (community) | Approve | ||
Tim Bentley | Approve | ||
Review via email: mp+176773@code.launchpad.net |
This proposal supersedes a proposal from 2013-07-24.
Commit message
Description of the change
changed skip_song to save_song & changed logic arround
To post a comment you must log in.
Revision history for this message
Phill (phill-ridout) wrote : Posted in a previous version of this proposal | # |
Revision history for this message
Tim Bentley (trb143) wrote : | # |
Looks ok and tests pass
review:
Approve
Revision history for this message
Andreas Preikschat (googol-deactivatedaccount) : | # |
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'openlp/plugins/songs/lib/foilpresenterimport.py' |
2 | --- openlp/plugins/songs/lib/foilpresenterimport.py 2013-03-07 08:05:43 +0000 |
3 | +++ openlp/plugins/songs/lib/foilpresenterimport.py 2013-07-24 20:07:33 +0000 |
4 | @@ -96,6 +96,7 @@ |
5 | |
6 | from lxml import etree, objectify |
7 | |
8 | +from openlp.core.lib import translate |
9 | from openlp.core.ui.wizard import WizardStrings |
10 | from openlp.plugins.songs.lib import clean_song, VerseType |
11 | from openlp.plugins.songs.lib.songimport import SongImport |
12 | @@ -115,7 +116,7 @@ |
13 | """ |
14 | log.debug(u'initialise FoilPresenterImport') |
15 | SongImport.__init__(self, manager, **kwargs) |
16 | - self.FoilPresenter = FoilPresenter(self.manager) |
17 | + self.FoilPresenter = FoilPresenter(self.manager, self) |
18 | |
19 | def doImport(self): |
20 | """ |
21 | @@ -202,8 +203,9 @@ |
22 | <copyright> tag. |
23 | |
24 | """ |
25 | - def __init__(self, manager): |
26 | + def __init__(self, manager, importer): |
27 | self.manager = manager |
28 | + self.importer = importer |
29 | |
30 | def xml_to_song(self, xml): |
31 | """ |
32 | @@ -222,22 +224,23 @@ |
33 | song.search_lyrics = u'' |
34 | song.verse_order = u'' |
35 | song.search_title = u'' |
36 | - # Because "text" seems to be an reserverd word, we have to recompile it. |
37 | + self.save_song = True |
38 | + # Because "text" seems to be an reserved word, we have to recompile it. |
39 | xml = re.compile(u'<text>').sub(u'<text_>', xml) |
40 | xml = re.compile(u'</text>').sub(u'</text_>', xml) |
41 | song_xml = objectify.fromstring(xml) |
42 | - foilpresenterfolie = song_xml |
43 | - self._process_copyright(foilpresenterfolie, song) |
44 | - self._process_cclinumber(foilpresenterfolie, song) |
45 | - self._process_titles(foilpresenterfolie, song) |
46 | + self._process_copyright(song_xml, song) |
47 | + self._process_cclinumber(song_xml, song) |
48 | + self._process_titles(song_xml, song) |
49 | # The verse order is processed with the lyrics! |
50 | - self._process_lyrics(foilpresenterfolie, song) |
51 | - self._process_comments(foilpresenterfolie, song) |
52 | - self._process_authors(foilpresenterfolie, song) |
53 | - self._process_songbooks(foilpresenterfolie, song) |
54 | - self._process_topics(foilpresenterfolie, song) |
55 | - clean_song(self.manager, song) |
56 | - self.manager.save_object(song) |
57 | + self._process_lyrics(song_xml, song) |
58 | + self._process_comments(song_xml, song) |
59 | + self._process_authors(song_xml, song) |
60 | + self._process_songbooks(song_xml, song) |
61 | + self._process_topics(song_xml, song) |
62 | + if self.save_song: |
63 | + clean_song(self.manager, song) |
64 | + self.manager.save_object(song) |
65 | |
66 | def _child(self, element): |
67 | """ |
68 | @@ -420,6 +423,12 @@ |
69 | VerseType.tags[VerseType.Intro]: 1, |
70 | VerseType.tags[VerseType.PreChorus]: 1 |
71 | } |
72 | + if not hasattr(foilpresenterfolie.strophen, u'strophe'): |
73 | + self.importer.logError(self._child(foilpresenterfolie.titel), |
74 | + unicode(translate('SongsPlugin.FoilPresenterSongImport', |
75 | + 'Invalid Foilpresenter song file. No verses found.'))) |
76 | + self.save_song = False |
77 | + return |
78 | for strophe in foilpresenterfolie.strophen.strophe: |
79 | text = self._child(strophe.text_) if hasattr(strophe, u'text_') else u'' |
80 | verse_name = self._child(strophe.key) |
81 | |
82 | === added file 'tests/functional/openlp_plugins/songs/test_foilpresenterimport.py' |
83 | --- tests/functional/openlp_plugins/songs/test_foilpresenterimport.py 1970-01-01 00:00:00 +0000 |
84 | +++ tests/functional/openlp_plugins/songs/test_foilpresenterimport.py 2013-07-24 20:07:33 +0000 |
85 | @@ -0,0 +1,195 @@ |
86 | +# -*- coding: utf-8 -*- |
87 | +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 |
88 | + |
89 | +############################################################################### |
90 | +# OpenLP - Open Source Lyrics Projection # |
91 | +# --------------------------------------------------------------------------- # |
92 | +# Copyright (c) 2008-2013 Raoul Snyman # |
93 | +# Portions copyright (c) 2008-2013 Tim Bentley, Gerald Britton, Jonathan # |
94 | +# Corwin, Samuel Findlay, Michael Gorven, Scott Guerrieri, Matthias Hub, # |
95 | +# Meinert Jordan, Armin Köhler, Erik Lundin, Edwin Lunando, Brian T. Meyer. # |
96 | +# Joshua Miller, Stevan Pettit, Andreas Preikschat, Mattias Põldaru, # |
97 | +# Christian Richter, Philip Ridout, Simon Scudder, Jeffrey Smith, # |
98 | +# Maikel Stuivenberg, Martin Thompson, Jon Tibble, Dave Warnock, # |
99 | +# Frode Woldsund, Martin Zibricky, Patrick Zimmermann # |
100 | +# --------------------------------------------------------------------------- # |
101 | +# This program is free software; you can redistribute it and/or modify it # |
102 | +# under the terms of the GNU General Public License as published by the Free # |
103 | +# Software Foundation; version 2 of the License. # |
104 | +# # |
105 | +# This program is distributed in the hope that it will be useful, but WITHOUT # |
106 | +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # |
107 | +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # |
108 | +# more details. # |
109 | +# # |
110 | +# You should have received a copy of the GNU General Public License along # |
111 | +# with this program; if not, write to the Free Software Foundation, Inc., 59 # |
112 | +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # |
113 | +############################################################################### |
114 | +""" |
115 | +This module contains tests for the SongShow Plus song importer. |
116 | +""" |
117 | + |
118 | +import os |
119 | +from unittest import TestCase |
120 | +from mock import patch, MagicMock |
121 | + |
122 | +from openlp.plugins.songs.lib import VerseType |
123 | +from openlp.plugins.songs.lib.foilpresenterimport import FoilPresenter |
124 | + |
125 | +TEST_PATH = os.path.abspath( |
126 | + os.path.join(os.path.dirname(__file__), u'..', u'..', u'..', u'/resources/foilpresentersongs')) |
127 | + |
128 | + |
129 | +class TestFoilPresenter(TestCase): |
130 | + """ |
131 | + Test the functions in the :mod:`foilpresenterimport` module. |
132 | + """ |
133 | + #TODO: The following modules still need tests written for |
134 | + # xml_to_song |
135 | + # _child |
136 | + # _process_authors |
137 | + # _process_cclinumber |
138 | + # _process_comments |
139 | + # _process_copyright |
140 | + # _process_lyrics |
141 | + # _process_songbooks |
142 | + # _process_titles |
143 | + # _process_topics |
144 | + |
145 | + def setUp(self): |
146 | + self.child_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._child') |
147 | + self.clean_song_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.clean_song') |
148 | + self.objectify_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.objectify') |
149 | + self.process_authors_patcher = \ |
150 | + patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_authors') |
151 | + self.process_cclinumber_patcher = \ |
152 | + patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_cclinumber') |
153 | + self.process_comments_patcher = \ |
154 | + patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_comments') |
155 | + self.process_lyrics_patcher = \ |
156 | + patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_lyrics') |
157 | + self.process_songbooks_patcher = \ |
158 | + patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_songbooks') |
159 | + self.process_titles_patcher = \ |
160 | + patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_titles') |
161 | + self.process_topics_patcher = \ |
162 | + patch(u'openlp.plugins.songs.lib.foilpresenterimport.FoilPresenter._process_topics') |
163 | + self.re_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.re') |
164 | + self.song_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.Song') |
165 | + self.song_xml_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.SongXML') |
166 | + self.translate_patcher = patch(u'openlp.plugins.songs.lib.foilpresenterimport.translate') |
167 | + |
168 | + self.mocked_child = self.child_patcher.start() |
169 | + self.mocked_clean_song = self.clean_song_patcher.start() |
170 | + self.mocked_objectify = self.objectify_patcher.start() |
171 | + self.mocked_process_authors = self.process_authors_patcher.start() |
172 | + self.mocked_process_cclinumber = self.process_cclinumber_patcher.start() |
173 | + self.mocked_process_comments = self.process_comments_patcher.start() |
174 | + self.mocked_process_lyrics = self.process_lyrics_patcher.start() |
175 | + self.mocked_process_songbooks = self.process_songbooks_patcher.start() |
176 | + self.mocked_process_titles = self.process_titles_patcher.start() |
177 | + self.mocked_process_topics = self.process_topics_patcher.start() |
178 | + self.mocked_re = self.re_patcher.start() |
179 | + self.mocked_song = self.song_patcher.start() |
180 | + self.mocked_song_xml = self.song_xml_patcher.start() |
181 | + self.mocked_translate = self.translate_patcher.start() |
182 | + self.mocked_child.return_value = u'Element Text' |
183 | + self.mocked_translate.return_value = u'Translated String' |
184 | + self.mocked_manager = MagicMock() |
185 | + self.mocked_song_import = MagicMock() |
186 | + |
187 | + def tearDown(self): |
188 | + self.child_patcher.stop() |
189 | + self.clean_song_patcher.stop() |
190 | + self.objectify_patcher.stop() |
191 | + self.process_authors_patcher.stop() |
192 | + self.process_cclinumber_patcher.stop() |
193 | + self.process_comments_patcher.stop() |
194 | + self.process_lyrics_patcher.stop() |
195 | + self.process_songbooks_patcher.stop() |
196 | + self.process_titles_patcher.stop() |
197 | + self.process_topics_patcher.stop() |
198 | + self.re_patcher.stop() |
199 | + self.song_patcher.stop() |
200 | + self.song_xml_patcher.stop() |
201 | + self.translate_patcher.stop() |
202 | + |
203 | + def create_foil_presenter_test(self): |
204 | + """ |
205 | + Test creating an instance of the FoilPresenter class |
206 | + """ |
207 | + # GIVEN: A mocked out "manager" and "SongImport" instance |
208 | + mocked_manager = MagicMock() |
209 | + mocked_song_import = MagicMock() |
210 | + |
211 | + # WHEN: An FoilPresenter instance is created |
212 | + foil_presenter_instance = FoilPresenter(mocked_manager, mocked_song_import) |
213 | + |
214 | + # THEN: The instance should not be None |
215 | + self.assertIsNotNone(foil_presenter_instance, u'FoilPresenter instance should not be none') |
216 | + |
217 | + def no_xml_test(self): |
218 | + """ |
219 | + Test calling xml_to_song with out the xml argument |
220 | + """ |
221 | + # GIVEN: A mocked out "manager" and "SongImport" as well as an foil_presenter instance |
222 | + mocked_manager = MagicMock() |
223 | + mocked_song_import = MagicMock() |
224 | + foil_presenter_instance = FoilPresenter(mocked_manager, mocked_song_import) |
225 | + |
226 | + # WHEN: xml_to_song is called without valid an argument |
227 | + for arg in [None, False, 0, u'']: |
228 | + result = foil_presenter_instance.xml_to_song(arg) |
229 | + |
230 | + # Then: xml_to_song should return False |
231 | + self.assertEqual(result, None, u'xml_to_song should return None when called with %s' % arg) |
232 | + |
233 | + def encoding_declaration_removal_test(self): |
234 | + """ |
235 | + Test that the encoding declaration is removed |
236 | + """ |
237 | + # GIVEN: A reset mocked out re and an instance of foil_presenter |
238 | + self.mocked_re.reset() |
239 | + foil_presenter_instance = FoilPresenter(self.mocked_manager, self.mocked_song_import) |
240 | + |
241 | + # WHEN: xml_to_song is called with a string with an xml encoding declaration |
242 | + foil_presenter_instance.xml_to_song(u'<?xml version="1.0" encoding="UTF-8"?>\n<foilpresenterfolie>') |
243 | + |
244 | + # THEN: the xml encoding declaration should have been stripped |
245 | + self.mocked_re.compile.sub.called_with(u'\n<foilpresenterfolie>') |
246 | + |
247 | + def no_encoding_declaration_test(self): |
248 | + """ |
249 | + Check that the xml sting is left intact when no encoding declaration is made |
250 | + """ |
251 | + # GIVEN: A reset mocked out re and an instance of foil_presenter |
252 | + self.mocked_re.reset() |
253 | + foil_presenter_instance = FoilPresenter(self.mocked_manager, self.mocked_song_import) |
254 | + |
255 | + # WHEN: xml_to_song is called with a string without an xml encoding declaration |
256 | + foil_presenter_instance.xml_to_song(u'<foilpresenterfolie>') |
257 | + |
258 | + # THEN: the string shiuld have been left intact |
259 | + self.mocked_re.compile.sub.called_with(u'<foilpresenterfolie>') |
260 | + |
261 | + def process_lyrics_no_verses_test(self): |
262 | + """ |
263 | + Test that _process_lyrics handles song files that have no verses. |
264 | + """ |
265 | + # GIVEN: A mocked foilpresenterfolie with no attribute strophe, a mocked song and a |
266 | + # foil presenter instance |
267 | + self.process_lyrics_patcher.stop() |
268 | + self.mocked_song_xml.reset() |
269 | + mock_foilpresenterfolie = MagicMock() |
270 | + del mock_foilpresenterfolie.strophen.strophe |
271 | + mocked_song = MagicMock() |
272 | + foil_presenter_instance = FoilPresenter(self.mocked_manager, self.mocked_song_import) |
273 | + |
274 | + # WHEN: _process_lyrics is called |
275 | + result = foil_presenter_instance._process_lyrics(mock_foilpresenterfolie, mocked_song) |
276 | + |
277 | + # THEN: _process_lyrics should return None and the song_import logError method should have been called once |
278 | + self.assertIsNone(result) |
279 | + self.mocked_song_import.logError.assert_called_once_with(u'Element Text', u'Translated String') |
280 | + self.process_lyrics_patcher.start() |
281 | \ No newline at end of file |
Quite where I got 171891 from I do not know. The correct bug number is: 1174039