From 53b210d9cdd287500a16f09b6f55a641290643e7 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 31 Mar 2017 19:41:38 +0100 Subject: [PATCH 01/32] Changeged 'combined' bible verses and made a seperate 'saved' list --- openlp/core/ui/lib/listwidgetwithdnd.py | 5 +- openlp/plugins/bibles/lib/mediaitem.py | 158 +++++++++++++----- .../test_listwidgetwithdnd.py | 31 ---- .../openlp_plugins/bibles/test_mediaitem.py | 100 ++++++----- 4 files changed, 176 insertions(+), 118 deletions(-) mode change 100644 => 100755 openlp/core/ui/lib/listwidgetwithdnd.py mode change 100644 => 100755 openlp/plugins/bibles/lib/mediaitem.py mode change 100644 => 100755 tests/functional/openlp_core_ui_lib/test_listwidgetwithdnd.py mode change 100644 => 100755 tests/functional/openlp_plugins/bibles/test_mediaitem.py diff --git a/openlp/core/ui/lib/listwidgetwithdnd.py b/openlp/core/ui/lib/listwidgetwithdnd.py old mode 100644 new mode 100755 index 662e99ded..b1e564eee --- a/openlp/core/ui/lib/listwidgetwithdnd.py +++ b/openlp/core/ui/lib/listwidgetwithdnd.py @@ -44,7 +44,6 @@ class ListWidgetWithDnD(QtWidgets.QListWidget): self.setSelectionMode(QtWidgets.QAbstractItemView.ExtendedSelection) self.setAlternatingRowColors(True) self.setContextMenuPolicy(QtCore.Qt.CustomContextMenu) - self.locked = False def activateDnD(self): """ @@ -54,15 +53,13 @@ class ListWidgetWithDnD(QtWidgets.QListWidget): self.setDragDropMode(QtWidgets.QAbstractItemView.DragDrop) Registry().register_function(('%s_dnd' % self.mime_data_text), self.parent().load_file) - def clear(self, search_while_typing=False, override_lock=False): + def clear(self, search_while_typing=False): """ Re-implement clear, so that we can customise feedback when using 'Search as you type' :param search_while_typing: True if we want to display the customised message :return: None """ - if self.locked and not override_lock: - return if search_while_typing: self.no_results_text = UiStrings().ShortResults else: diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py old mode 100644 new mode 100755 index 9344fab63..bdb08c289 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -57,6 +57,17 @@ class BibleSearch(object): Combined = 3 +class ResultsTab(object): + Saved = 0 + Search = 1 + + +class SearchStatus(object): + SearchButton = 0 + SearchAsYouType = 1 + NotEnoughText = 2 + + class BibleMediaItem(MediaManagerItem): """ This is the custom media manager item for Bibles. @@ -78,6 +89,9 @@ class BibleMediaItem(MediaManagerItem): self.sort_icon = build_icon(':/bibles/bibles_book_sort.png') self.bible = None self.second_bible = None + self.saved_results = [] + self.current_results = [] + self.search_status = SearchStatus().SearchButton # TODO: Make more central and clean up after! self.search_timer = QtCore.QTimer() self.search_timer.setInterval(200) @@ -176,15 +190,18 @@ class BibleMediaItem(MediaManagerItem): # Note: If we use QPushButton instead of the QToolButton, the icon will be larger than the Lock icon. self.clear_button = QtWidgets.QToolButton(self) self.clear_button.setIcon(self.clear_icon) - self.lock_button = QtWidgets.QToolButton(self) - self.lock_button.setIcon(self.unlock_icon) - self.lock_button.setCheckable(True) + self.save_results_button = QtWidgets.QToolButton(self) + self.save_results_button.setIcon(self.unlock_icon) self.search_button_layout.addWidget(self.clear_button) - self.search_button_layout.addWidget(self.lock_button) + self.search_button_layout.addWidget(self.save_results_button) self.search_button = QtWidgets.QPushButton(self) self.search_button_layout.addWidget(self.search_button) self.general_bible_layout.addRow(self.search_button_layout) self.page_layout.addWidget(self.options_widget) + self.results_view_tab = QtWidgets.QTabBar(self) + self.results_view_tab.addTab('') + self.results_view_tab.addTab('') + self.page_layout.addWidget(self.results_view_tab) def setupUi(self): super().setupUi() @@ -211,12 +228,15 @@ class BibleMediaItem(MediaManagerItem): # Buttons self.book_order_button.toggled.connect(self.on_book_order_button_toggled) self.clear_button.clicked.connect(self.on_clear_button_clicked) - self.lock_button.toggled.connect(self.on_lock_button_toggled) + self.save_results_button.clicked.connect(self.on_save_results_button_clicked) self.search_button.clicked.connect(self.on_search_button_clicked) # Other stuff self.search_edit.returnPressed.connect(self.on_search_button_clicked) self.search_tab_bar.currentChanged.connect(self.on_search_tab_bar_current_changed) + self.results_view_tab.currentChanged.connect(self.on_results_view_tab_current_changed) self.search_edit.textChanged.connect(self.on_search_edit_text_changed) + self.on_results_view_tab_total_update(ResultsTab.Saved) + self.on_results_view_tab_total_update(ResultsTab.Search) def retranslateUi(self): log.debug('retranslateUi') @@ -225,9 +245,9 @@ class BibleMediaItem(MediaManagerItem): self.style_combo_box.setItemText(LayoutStyle.VersePerSlide, UiStrings().VersePerSlide) self.style_combo_box.setItemText(LayoutStyle.VersePerLine, UiStrings().VersePerLine) self.style_combo_box.setItemText(LayoutStyle.Continuous, UiStrings().Continuous) - self.clear_button.setToolTip(translate('BiblesPlugin.MediaItem', 'Clear the search results.')) - self.lock_button.setToolTip( - translate('BiblesPlugin.MediaItem', 'Toggle to keep or clear the previous results.')) + self.clear_button.setToolTip(translate('BiblesPlugin.MediaItem', 'Clear the results on the current tab.')) + self.save_results_button.setToolTip( + translate('BiblesPlugin.MediaItem', 'Add the search results to the saved list.')) self.search_button.setText(UiStrings().Search) def on_focus(self): @@ -423,6 +443,35 @@ class BibleMediaItem(MediaManagerItem): self.select_tab.setVisible(not search_tab) self.on_focus() + def on_results_view_tab_current_changed(self, index): + """ + Update list_widget with the contents of the selected list + + :param index: The index of the tab that has been changed to. (int) + :return: None + """ + if index == ResultsTab.Saved: + self.add_built_results_to_list_widget(self.saved_results) + elif index == ResultsTab.Search: + self.add_built_results_to_list_widget(self.current_results) + + def on_results_view_tab_total_update(self, index): + """ + Update the result total count on the tab with the given index. + + :param index: Index of the tab to update (int) + :return: None + """ + string = '' + count = 0 + if index == ResultsTab.Saved: + string = translate('BiblesPlugin.MediaItem', 'Saved ({result_count})') + count = len(self.saved_results) + elif index == ResultsTab.Search: + string = translate('BiblesPlugin.MediaItem', 'Results ({result_count})') + count = len(self.current_results) + self.results_view_tab.setTabText(index, string.format(result_count = count)) + def on_book_order_button_toggled(self, checked): """ Change the sort order of the book names @@ -442,22 +491,25 @@ class BibleMediaItem(MediaManagerItem): :return: None """ + current_index = self.results_view_tab.currentIndex() + if current_index == ResultsTab.Saved: + self.saved_results = [] + elif current_index == ResultsTab.Search: + self.current_results = [] + self.search_edit.clear() + self.on_focus() + self.on_results_view_tab_total_update(current_index) self.list_view.clear() - self.search_edit.clear() - self.on_focus() - def on_lock_button_toggled(self, checked): + def on_save_results_button_clicked(self): """ - Toggle the lock button, if Search tab is used, set focus to search field. + Toggle the lock button. - :param checked: The state of the toggle button. (bool) :return: None """ - self.list_view.locked = checked - if checked: - self.sender().setIcon(self.lock_icon) - else: - self.sender().setIcon(self.unlock_icon) + for verse in self.list_view.selectedItems(): + self.saved_results.append(verse.data(QtCore.Qt.UserRole)) + self.on_results_view_tab_total_update(ResultsTab.Saved) def on_style_combo_box_index_changed(self, index): """ @@ -497,9 +549,11 @@ class BibleMediaItem(MediaManagerItem): if critical_error_message_box( message=translate('BiblesPlugin.MediaItem', 'OpenLP cannot combine single and dual Bible verse search results. ' - 'Do you want to clear your search results and start a new search?'), + 'Do you want to clear your results and start a new search?'), parent=self, question=True) == QtWidgets.QMessageBox.Yes: - self.list_view.clear(override_lock=True) + self.display_results() + self.saved_results = [] + self.on_results_view_tab_total_update(ResultsTab.Saved) else: self.second_combo_box.setCurrentIndex(self.second_combo_box.findData(self.second_bible)) return @@ -602,6 +656,8 @@ class BibleMediaItem(MediaManagerItem): :return: None """ + self.search_timer.stop() + self.search_status = SearchStatus().SearchButton if not self.bible: self.main_window.information_message(UiStrings().BibleNoBiblesTitle, UiStrings().BibleNoBibles) return @@ -613,6 +669,7 @@ class BibleMediaItem(MediaManagerItem): elif self.select_tab.isVisible(): self.select_search() self.search_button.setEnabled(True) + self.results_view_tab.setCurrentIndex(ResultsTab.Search) self.application.set_normal_cursor() def select_search(self): @@ -636,13 +693,14 @@ class BibleMediaItem(MediaManagerItem): :return: None """ + self.search_results = [] verse_refs = self.plugin.manager.parse_ref(self.bible.name, search_text) self.search_results = self.plugin.manager.get_verses(self.bible.name, verse_refs, True) if self.second_bible and self.search_results: self.search_results = self.plugin.manager.get_verses(self.second_bible.name, verse_refs, True) self.display_results() - def on_text_search(self, text, search_while_type=False): + def on_text_search(self, text): """ We are doing a 'Text Search'. This search is called on def text_search by 'Search' Text and Combined Searches. @@ -663,7 +721,7 @@ class BibleMediaItem(MediaManagerItem): verse=verse.verse, bible_name=self.second_bible.name)) not_found_count += 1 self.search_results = filtered_search_results - if not_found_count != 0 and not search_while_type: + if not_found_count != 0 and self.search_status == SearchStatus.SearchButton: self.main_window.information_message( translate('BiblesPlugin.MediaItem', 'Verses not found'), translate('BiblesPlugin.MediaItem', @@ -673,22 +731,23 @@ class BibleMediaItem(MediaManagerItem): ).format(second_name=self.second_bible.name, name=self.bible.name, count=not_found_count)) self.display_results() - def text_search(self, search_while_type=False): + def text_search(self): """ This triggers the proper 'Search' search based on which search type is used. "Eg. "Reference Search", "Text Search" or "Combined search". """ + self.search_results = [] log.debug('text_search called') text = self.search_edit.text() if text == '': - self.list_view.clear() + self.display_results() return - self.list_view.clear(search_while_typing=search_while_type) + self.on_results_view_tab_total_update(ResultsTab.Search) if self.search_edit.current_search_type() == BibleSearch.Reference: if get_reference_match('full').match(text): # Valid reference found. Do reference search. self.text_reference_search(text) - elif not search_while_type: + elif self.search_status == SearchStatus.SearchButton: self.main_window.information_message( translate('BiblesPlugin.BibleManager', 'Scripture Reference Error'), translate('BiblesPlugin.BibleManager', @@ -700,10 +759,12 @@ class BibleMediaItem(MediaManagerItem): self.text_reference_search(text) else: # It can only be a 'Combined' search without a valid reference, or a 'Text' search - if search_while_type: - if len(text) > 8 and VALID_TEXT_SEARCH.search(text): - self.on_text_search(text, True) - elif VALID_TEXT_SEARCH.search(text): + if self.search_status == SearchStatus().SearchAsYouType: + if len(text) <= 8: + self.search_status = SearchStatus.NotEnoughText + self.display_results() + return + if VALID_TEXT_SEARCH.search(text): self.on_text_search(text) def on_search_edit_text_changed(self): @@ -724,7 +785,9 @@ class BibleMediaItem(MediaManagerItem): :return: None """ - self.text_search(True) + self.search_status = SearchStatus().SearchAsYouType + self.text_search() + self.results_view_tab.setCurrentIndex(ResultsTab.Search) def display_results(self): """ @@ -732,14 +795,16 @@ class BibleMediaItem(MediaManagerItem): :return: None """ - self.list_view.clear() - if self.search_results: - items = self.build_display_results(self.bible, self.second_bible, self.search_results) - for item in items: - self.list_view.addItem(item) - self.list_view.selectAll() + self.current_results = self.build_display_results(self.bible, self.second_bible, self.search_results) self.search_results = [] - self.second_search_results = [] + self.add_built_results_to_list_widget(self.current_results) + + def add_built_results_to_list_widget(self, results): + self.list_view.clear(self.search_status == SearchStatus.NotEnoughText) + for item in self.build_list_widget_items(results): + self.list_view.addItem(item) + self.list_view.selectAll() + self.on_results_view_tab_total_update(ResultsTab.Search) def build_display_results(self, bible, second_bible, search_results): """ @@ -789,11 +854,19 @@ class BibleMediaItem(MediaManagerItem): bible_text = '{book} {chapter:d}{sep}{verse:d} ({version}, {second_version})' else: bible_text = '{book} {chapter:d}{sep}{verse:d} ({version})' - bible_verse = QtWidgets.QListWidgetItem(bible_text.format(sep=verse_separator, **data)) - bible_verse.setData(QtCore.Qt.UserRole, data) - items.append(bible_verse) + data['item_title'] = bible_text.format(sep=verse_separator, **data) + items.append(data) return items + def build_list_widget_items(self, items): + list_widget_items = [] + for data in items: + bible_verse = QtWidgets.QListWidgetItem(data['item_title']) + bible_verse.setData(QtCore.Qt.UserRole, data) + list_widget_items.append(bible_verse) + return list_widget_items + + def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False, context=ServiceItemContext.Service): """ @@ -910,4 +983,5 @@ class BibleMediaItem(MediaManagerItem): """ reference = self.plugin.manager.parse_ref(self.bible.name, item_id) search_results = self.plugin.manager.get_verses(self.bible.name, reference, False) - return self.build_display_results(self.bible, None, search_results) + items = self.build_display_results(self.bible, None, search_results) + return self.build_list_widget_items(items) diff --git a/tests/functional/openlp_core_ui_lib/test_listwidgetwithdnd.py b/tests/functional/openlp_core_ui_lib/test_listwidgetwithdnd.py old mode 100644 new mode 100755 index 8bb30e50f..4c1585f76 --- a/tests/functional/openlp_core_ui_lib/test_listwidgetwithdnd.py +++ b/tests/functional/openlp_core_ui_lib/test_listwidgetwithdnd.py @@ -33,37 +33,6 @@ class TestListWidgetWithDnD(TestCase): """ Test the :class:`~openlp.core.lib.listwidgetwithdnd.ListWidgetWithDnD` class """ - def test_clear_locked(self): - """ - Test the clear method the list is 'locked' - """ - with patch('openlp.core.ui.lib.listwidgetwithdnd.QtWidgets.QListWidget.clear') as mocked_clear_super_method: - # GIVEN: An instance of ListWidgetWithDnD - widget = ListWidgetWithDnD() - - # WHEN: The list is 'locked' and clear has been called - widget.locked = True - widget.clear() - - # THEN: The super method should not have been called (i.e. The list not cleared) - self.assertFalse(mocked_clear_super_method.called) - - def test_clear_overide_locked(self): - """ - Test the clear method the list is 'locked', but clear is called with 'override_lock' set to True - """ - with patch('openlp.core.ui.lib.listwidgetwithdnd.QtWidgets.QListWidget.clear') as mocked_clear_super_method: - # GIVEN: An instance of ListWidgetWithDnD - widget = ListWidgetWithDnD() - - # WHEN: The list is 'locked' and clear has been called with override_lock se to True - widget.locked = True - widget.clear(override_lock=True) - - # THEN: The super method should have been called (i.e. The list is cleared regardless whether it is locked - # or not) - mocked_clear_super_method.assert_called_once_with() - def test_clear(self): """ Test the clear method when called without any arguments. diff --git a/tests/functional/openlp_plugins/bibles/test_mediaitem.py b/tests/functional/openlp_plugins/bibles/test_mediaitem.py old mode 100644 new mode 100755 index fd2d159f9..55aa867aa --- a/tests/functional/openlp_plugins/bibles/test_mediaitem.py +++ b/tests/functional/openlp_plugins/bibles/test_mediaitem.py @@ -31,7 +31,8 @@ from tests.helpers.testmixin import TestMixin from openlp.core.common import Registry from openlp.core.lib import MediaManagerItem -from openlp.plugins.bibles.lib.mediaitem import BibleMediaItem, BibleSearch, get_reference_separators, VALID_TEXT_SEARCH +from openlp.plugins.bibles.lib.mediaitem import BibleMediaItem, BibleSearch, ResultsTab, SearchStatus, \ + get_reference_separators, VALID_TEXT_SEARCH class TestBibleMediaItemModulefunctions(TestCase): @@ -143,6 +144,7 @@ class TestMediaItem(TestCase, TestMixin): self.media_item = BibleMediaItem(None, self.mocked_plugin) self.media_item.settings_section = 'bibles' + self.media_item.results_view_tab = MagicMock() self.mocked_book_1 = MagicMock(**{'get_name.return_value': 'Book 1', 'book_reference_id': 1}) self.mocked_book_2 = MagicMock(**{'get_name.return_value': 'Book 2', 'book_reference_id': 2}) @@ -156,6 +158,7 @@ class TestMediaItem(TestCase, TestMixin): self.mocked_bible_2 = MagicMock(**{'get_books.return_value': self.book_list_2}) self.mocked_bible_2.name = 'Bible 2' + def test_media_item_instance(self): """ When creating an instance of C test that it is also an instance of @@ -658,56 +661,66 @@ class TestMediaItem(TestCase, TestMixin): # THEN: The select_book_combo_box model sort should have been reset self.media_item.select_book_combo_box.model().sort.assert_called_once_with(-1) - def test_on_clear_button_clicked(self): + def test_on_clear_button_clicked_saved_tab(self): """ - Test on_clear_button_clicked + Test on_clear_button_clicked when the saved tab is selected + """ + # GIVEN: An instance of :class:`MediaManagerItem` and mocked out saved_tab and select_tab and a mocked out + # list_view and search_edit + self.media_item.list_view = MagicMock() + self.media_item.search_edit = MagicMock() + self.media_item.results_view_tab = MagicMock(**{'currentIndex.return_value': ResultsTab.Saved}) + self.media_item.saved_results = ['Some', 'Results'] + with patch.object(self.media_item, 'on_focus'): + + # WHEN: Calling on_clear_button_clicked + self.media_item.on_clear_button_clicked() + + # THEN: The list_view should be cleared + self.assertEqual(self.media_item.saved_results, []) + self.media_item.list_view.clear.assert_called_once_with() + + def test_on_clear_button_clicked_search_tab(self): + """ + Test on_clear_button_clicked when the search tab is selected """ # GIVEN: An instance of :class:`MediaManagerItem` and mocked out search_tab and select_tab and a mocked out # list_view and search_edit self.media_item.list_view = MagicMock() self.media_item.search_edit = MagicMock() + self.media_item.results_view_tab = MagicMock(**{'currentIndex.return_value': ResultsTab.Search}) + self.media_item.current_results = ['Some', 'Results'] with patch.object(self.media_item, 'on_focus'): # WHEN: Calling on_clear_button_clicked self.media_item.on_clear_button_clicked() # THEN: The list_view and the search_edit should be cleared + self.assertEqual(self.media_item.current_results, []) self.media_item.list_view.clear.assert_called_once_with() self.media_item.search_edit.clear.assert_called_once_with() - def test_on_lock_button_toggled_search_tab_lock_icon(self): + + def test_on_save_results_button_clicked(self): """ - Test that "on_lock_button_toggled" toggles the lock properly. + Test that "on_save_results_button_clicked" saves the results. """ - # GIVEN: An instance of :class:`MediaManagerItem` a mocked sender and list_view - self.media_item.list_view = MagicMock() - self.media_item.lock_icon = 'lock icon' - mocked_sender_instance = MagicMock() - with patch.object(self.media_item, 'sender', return_value=mocked_sender_instance): + # GIVEN: An instance of :class:`MediaManagerItem` and a mocked list_view + result_1 = MagicMock(**{'data.return_value': 'R1'}) + result_2 = MagicMock(**{'data.return_value': 'R2'}) + result_3 = MagicMock(**{'data.return_value': 'R3'}) + self.media_item.list_view = MagicMock(**{'selectedItems.return_value': [result_1, result_2, result_3]}) - # WHEN: When the lock_button is checked - self.media_item.on_lock_button_toggled(True) + with patch.object(self.media_item, 'on_results_view_tab_total_update') as \ + mocked_on_results_view_tab_total_update : - # THEN: list_view should be 'locked' and the lock icon set - self.assertTrue(self.media_item.list_view.locked) - mocked_sender_instance.setIcon.assert_called_once_with('lock icon') + # WHEN: When the save_results_button is clicked + self.media_item.on_save_results_button_clicked() - def test_on_lock_button_toggled_unlock_icon(self): - """ - Test that "on_lock_button_toggled" toggles the lock properly. - """ - # GIVEN: An instance of :class:`MediaManagerItem` a mocked sender and list_view - self.media_item.list_view = MagicMock() - self.media_item.unlock_icon = 'unlock icon' - mocked_sender_instance = MagicMock() - with patch.object(self.media_item, 'sender', return_value=mocked_sender_instance): - - # WHEN: When the lock_button is unchecked - self.media_item.on_lock_button_toggled(False) - - # THEN: list_view should be 'unlocked' and the unlock icon set - self.assertFalse(self.media_item.list_view.locked) - mocked_sender_instance.setIcon.assert_called_once_with('unlock icon') + # THEN: The selected results in the list_view should be added to the 'saved_results' list. And the saved_tab + # total should be updated. + self.assertEqual(self.media_item.saved_results, ['R1', 'R2', 'R3']) + mocked_on_results_view_tab_total_update.assert_called_once_with(ResultsTab.Saved) def test_on_style_combo_box_changed(self): """ @@ -815,7 +828,9 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.list_view = MagicMock(**{'count.return_value': 5}) self.media_item.style_combo_box = MagicMock() self.media_item.select_book_combo_box = MagicMock() + self.media_item.search_results = ['list', 'of', 'results'] with patch.object(self.media_item, 'initialise_advanced_bible') as mocked_initialise_advanced_bible, \ + patch.object(self.media_item, 'display_results'), \ patch('openlp.plugins.bibles.lib.mediaitem.critical_error_message_box', return_value=QtWidgets.QMessageBox.Yes) as mocked_critical_error_message_box: @@ -825,9 +840,8 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.second_combo_box = MagicMock(**{'currentData.return_value': self.mocked_bible_1}) self.media_item.on_second_combo_box_index_changed(5) - # THEN: The list_view should be cleared and the selected bible should be set as the current bible + # THEN: The selected bible should be set as the current bible self.assertTrue(mocked_critical_error_message_box.called) - self.media_item.list_view.clear.assert_called_once_with(override_lock=True) self.media_item.style_combo_box.setEnabled.assert_called_once_with(False) self.assertTrue(mocked_initialise_advanced_bible.called) self.assertEqual(self.media_item.second_bible, self.mocked_bible_1) @@ -841,7 +855,9 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.list_view = MagicMock(**{'count.return_value': 5}) self.media_item.style_combo_box = MagicMock() self.media_item.select_book_combo_box = MagicMock() + self.media_item.search_results = ['list', 'of', 'results'] with patch.object(self.media_item, 'initialise_advanced_bible') as mocked_initialise_advanced_bible, \ + patch.object(self.media_item, 'display_results'), \ patch('openlp.plugins.bibles.lib.mediaitem.critical_error_message_box', return_value=QtWidgets.QMessageBox.Yes) as mocked_critical_error_message_box: # WHEN: The previously is a bible new selection is None and the user selects yes @@ -850,9 +866,8 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.second_combo_box = MagicMock(**{'currentData.return_value': None}) self.media_item.on_second_combo_box_index_changed(0) - # THEN: The list_view should be cleared and the selected bible should be set as the current bible + # THEN: The selected bible should be set as the current bible self.assertTrue(mocked_critical_error_message_box.called) - self.media_item.list_view.clear.assert_called_once_with(override_lock=True) self.media_item.style_combo_box.setEnabled.assert_called_once_with(True) self.assertFalse(mocked_initialise_advanced_bible.called) self.assertEqual(self.media_item.second_bible, None) @@ -1388,8 +1403,9 @@ class TestMediaItem(TestCase, TestMixin): # WHEN: Calling on_search_timer_timeout self.media_item.on_search_timer_timeout() - # THEN: The text_search method should have been called with True - mocked_text_search.assert_called_once_with(True) + # THEN: The search_status should be set to SearchAsYouType and text_search should have been called + self.assertEqual(self.media_item.search_status, SearchStatus().SearchAsYouType) + mocked_text_search.assert_called_once_with() def test_display_results_no_results(self): """ @@ -1407,7 +1423,6 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.display_results() # THEN: No items should be added to the list - self.media_item.list_view.clear.assert_called_once_with() self.assertFalse(self.media_item.list_view.addItem.called) def test_display_results_results(self): @@ -1415,7 +1430,10 @@ class TestMediaItem(TestCase, TestMixin): Test the display_results method when there are items to display """ # GIVEN: An instance of BibleMediaItem and a mocked build_display_results which returns a list of results - with patch.object(self.media_item, 'build_display_results', return_value=['list', 'items']): + with patch.object(self.media_item, 'build_display_results', return_value=[ + {'item_title': 'Title 1'}, {'item_title': 'Title 2'}]), \ + patch.object(self.media_item, 'add_built_results_to_list_widget') as \ + mocked_add_built_results_to_list_widget: self.media_item.search_results = ['results'] self.media_item.list_view = MagicMock() @@ -1423,5 +1441,5 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.display_results() # THEN: addItem should have been with the display items - self.media_item.list_view.clear.assert_called_once_with() - self.assertEqual(self.media_item.list_view.addItem.call_args_list, [call('list'), call('items')]) + mocked_add_built_results_to_list_widget.assert_called_once_with( + [{'item_title': 'Title 1'}, {'item_title': 'Title 2'}]) From eee14a654a378ca43fbd70b4fabc194cf205ad0e Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 2 Apr 2017 21:28:55 +0100 Subject: [PATCH 02/32] Tests and added icon --- openlp/core/ui/lib/listwidgetwithdnd.py | 9 +++++ openlp/plugins/bibles/lib/mediaitem.py | 18 +++++----- resources/images/bibles_save_results.png | Bin 0 -> 735 bytes resources/images/bibles_search_lock.png | Bin 452 -> 0 bytes resources/images/bibles_search_unlock.png | Bin 440 -> 0 bytes resources/images/network_ssl.png | Bin 577 -> 0 bytes resources/images/openlp-2.qrc | 6 ++-- .../test_listwidgetwithdnd.py | 33 ++++++++++++++++++ 8 files changed, 53 insertions(+), 13 deletions(-) create mode 100644 resources/images/bibles_save_results.png delete mode 100644 resources/images/bibles_search_lock.png delete mode 100644 resources/images/bibles_search_unlock.png delete mode 100644 resources/images/network_ssl.png diff --git a/openlp/core/ui/lib/listwidgetwithdnd.py b/openlp/core/ui/lib/listwidgetwithdnd.py index b1e564eee..2d7874b7a 100755 --- a/openlp/core/ui/lib/listwidgetwithdnd.py +++ b/openlp/core/ui/lib/listwidgetwithdnd.py @@ -125,6 +125,15 @@ class ListWidgetWithDnD(QtWidgets.QListWidget): else: event.ignore() + def allItems(self): + """ + An generator to list all the items in the widget + + :return: a generator + """ + for row in range(self.count()): + yield self.item(row) + def paintEvent(self, event): """ Re-implement paintEvent so that we can add 'No Results' text when the listWidget is empty. diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index bdb08c289..ec06f6529 100755 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -84,8 +84,7 @@ class BibleMediaItem(MediaManagerItem): :param kwargs: Keyword arguments to pass to the super method. (dict) """ self.clear_icon = build_icon(':/bibles/bibles_search_clear.png') - self.lock_icon = build_icon(':/bibles/bibles_search_lock.png') - self.unlock_icon = build_icon(':/bibles/bibles_search_unlock.png') + self.save_results_icon = build_icon(':/bibles/bibles_save_results.png') self.sort_icon = build_icon(':/bibles/bibles_book_sort.png') self.bible = None self.second_bible = None @@ -191,7 +190,7 @@ class BibleMediaItem(MediaManagerItem): self.clear_button = QtWidgets.QToolButton(self) self.clear_button.setIcon(self.clear_icon) self.save_results_button = QtWidgets.QToolButton(self) - self.save_results_button.setIcon(self.unlock_icon) + self.save_results_button.setIcon(self.save_results_icon) self.search_button_layout.addWidget(self.clear_button) self.search_button_layout.addWidget(self.save_results_button) self.search_button = QtWidgets.QPushButton(self) @@ -201,6 +200,7 @@ class BibleMediaItem(MediaManagerItem): self.results_view_tab = QtWidgets.QTabBar(self) self.results_view_tab.addTab('') self.results_view_tab.addTab('') + self.results_view_tab.setCurrentIndex(ResultsTab.Search) self.page_layout.addWidget(self.results_view_tab) def setupUi(self): @@ -492,18 +492,18 @@ class BibleMediaItem(MediaManagerItem): :return: None """ current_index = self.results_view_tab.currentIndex() + for item in self.list_view.selectedItems(): + self.list_view.takeItem(self.list_view.row(item)) + results = [item.data(QtCore.Qt.UserRole) for item in self.list_view.allItems()] if current_index == ResultsTab.Saved: - self.saved_results = [] + self.saved_results = results elif current_index == ResultsTab.Search: - self.current_results = [] - self.search_edit.clear() - self.on_focus() + self.current_results = results self.on_results_view_tab_total_update(current_index) - self.list_view.clear() def on_save_results_button_clicked(self): """ - Toggle the lock button. + Add the selected verses to the saved_results list. :return: None """ diff --git a/resources/images/bibles_save_results.png b/resources/images/bibles_save_results.png new file mode 100644 index 0000000000000000000000000000000000000000..ee119c39534672b3a1375114f663084a2df19412 GIT binary patch literal 735 zcmV<50wDc~P)gN){_oC^+))NOgXG{^FC*Aj?x!st*p|7`*Fw zuBn!NvuAMsOr_j+&VD)d^3uueCyx8lK>!8~!qC5}he*W;pu4=99fQL)-wSGvbg6n( z>dp|a79TsZwX1dBm9k%U<%o*LT_FgnHrh)uXbhTE#>l`zNSn!p?>u<-gt>5rj@!&_ zm8$&v?-@j{aRZmGpiIeiF~$bz02-w;TAfAS{`yEWY_k+4v{gU8Qp(2S(fi4dyL&1< z2q~DgEIeD%+qVj_c8-$k;I(>LYDd&%o!Ux+=msZ9NOSYZ#%3f+qS@)z^iBj80xZP7 zTnscCwGvg|=i5S)>0duSxFvhsFWEW>EM0QF*L$B&-Lx4&16Te&jXBC#sA$(B|LcL#hA>H(T@1;zrWHrzCO6A2l9cFn*5T975T|M3bpntkFJcRS zK~$|5hZ}Xg-;(a7J~oRXG-hJi2~?N9RH=!r$zncGU%S4nTk6%^n&@f~`v=AP4n6n- RkB|TW002ovPDHLkV1lh{OacG^ literal 0 HcmV?d00001 diff --git a/resources/images/bibles_search_lock.png b/resources/images/bibles_search_lock.png deleted file mode 100644 index ac2fd6f90f473f0e497f9ec65e01b26e8fcbc1e4..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 452 zcmV;#0XzPQP){*xqu6zo97u8y1ZJ6plT?@BQ4ok{ z+TdZhC=vE=s3yPtE(pTv`m^2X6qh6^;cz(S^xkLL z&o;NmD#TR4R~VS);WMLje^R3O!hz^@Kh)i9`%foV1%w+4~N55nzQfvtjFUy&15ol zJ!2#oB>{Mz*BamI^ZDd-It@MZtw0pTVxvG11UZ#T84D7^G_I5}oJWRnxZ00xZx^Co z8#WQ`#K^LN3SNTc+x1dZRgv)F~$k_|^fFLKc#)GiFZk*b-n u5N$*!(L?kT14JLuO<0LmqJ?PE_q_mVmxN%}APaE-0000#GNQ#XgniT2Vo?-i9TY0a1g_U iov;yRqKoKkeyR`IorwpbwM(D?00001r;P)(<+F=08q_00t&F1I%F#z!2z#$c(OoM@BmdoR*iyz z*z*Dgm?;aVIHxQOoC9YDX3HNE9{l7AL>!(vR2-fx9e4_&D+m7@K~?ZQ@I52tgDr%# zI?^7bJVX2?C;6!YJG+6uH;wH;lOVo2JV9(6~ytwJuv$Vf4-@_QLj?2 P00000NkvXXu0mjf+JX4) diff --git a/resources/images/openlp-2.qrc b/resources/images/openlp-2.qrc index e221474ff..1a50dbe78 100644 --- a/resources/images/openlp-2.qrc +++ b/resources/images/openlp-2.qrc @@ -34,8 +34,7 @@ bibles_search_text.png bibles_search_reference.png bibles_search_clear.png - bibles_search_unlock.png - bibles_search_lock.png + bibles_save_results.png plugin_alerts.png @@ -144,7 +143,6 @@ network_server.png - network_ssl.png network_auth.png @@ -188,4 +186,4 @@ android_app_qr.png ios_app_qr.png - \ No newline at end of file + diff --git a/tests/functional/openlp_core_ui_lib/test_listwidgetwithdnd.py b/tests/functional/openlp_core_ui_lib/test_listwidgetwithdnd.py index 4c1585f76..0b79f7ab1 100755 --- a/tests/functional/openlp_core_ui_lib/test_listwidgetwithdnd.py +++ b/tests/functional/openlp_core_ui_lib/test_listwidgetwithdnd.py @@ -23,6 +23,7 @@ This module contains tests for the openlp.core.lib.listwidgetwithdnd module """ from unittest import TestCase +from types import GeneratorType from openlp.core.common.uistrings import UiStrings from openlp.core.ui.lib.listwidgetwithdnd import ListWidgetWithDnD @@ -59,6 +60,38 @@ class TestListWidgetWithDnD(TestCase): # THEN: The results text should be the 'short results' text. self.assertEqual(widget.no_results_text, UiStrings().ShortResults) + def test_all_items_no_list_items(self): + """ + Test allItems when there are no items in the list widget + """ + # GIVEN: An instance of ListWidgetWithDnD + widget = ListWidgetWithDnD() + with patch.object(widget, 'count', return_value=0), \ + patch.object(widget, 'item', side_effect=lambda x: [][x]): + + # WHEN: Calling allItems + result = widget.allItems() + + # THEN: An instance of a Generator object should be returned. The generator should not yeild any results + self.assertIsInstance(result, GeneratorType) + self.assertEqual(list(result), []) + + def test_all_items_list_items(self): + """ + Test allItems when the list widget contains some items. + """ + # GIVEN: An instance of ListWidgetWithDnD + widget = ListWidgetWithDnD() + with patch.object(widget, 'count', return_value=2), \ + patch.object(widget, 'item', side_effect=lambda x: [5, 3][x]): + + # WHEN: Calling allItems + result = widget.allItems() + + # THEN: An instance of a Generator object should be returned. The generator should not yeild any results + self.assertIsInstance(result, GeneratorType) + self.assertEqual(list(result), [5, 3]) + def test_paint_event(self): """ Test the paintEvent method when the list is not empty From a3146d7a2c2832977c211b4a1014950d996e9846 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Mon, 3 Apr 2017 21:28:16 +0100 Subject: [PATCH 03/32] PEP Fixes --- openlp/plugins/bibles/lib/mediaitem.py | 3 +-- tests/functional/openlp_plugins/bibles/test_mediaitem.py | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index ec06f6529..0fbabab31 100755 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -470,7 +470,7 @@ class BibleMediaItem(MediaManagerItem): elif index == ResultsTab.Search: string = translate('BiblesPlugin.MediaItem', 'Results ({result_count})') count = len(self.current_results) - self.results_view_tab.setTabText(index, string.format(result_count = count)) + self.results_view_tab.setTabText(index, string.format(result_count=count)) def on_book_order_button_toggled(self, checked): """ @@ -866,7 +866,6 @@ class BibleMediaItem(MediaManagerItem): list_widget_items.append(bible_verse) return list_widget_items - def generate_slide_data(self, service_item, item=None, xml_version=False, remote=False, context=ServiceItemContext.Service): """ diff --git a/tests/functional/openlp_plugins/bibles/test_mediaitem.py b/tests/functional/openlp_plugins/bibles/test_mediaitem.py index 55aa867aa..7ed904145 100755 --- a/tests/functional/openlp_plugins/bibles/test_mediaitem.py +++ b/tests/functional/openlp_plugins/bibles/test_mediaitem.py @@ -158,7 +158,6 @@ class TestMediaItem(TestCase, TestMixin): self.mocked_bible_2 = MagicMock(**{'get_books.return_value': self.book_list_2}) self.mocked_bible_2.name = 'Bible 2' - def test_media_item_instance(self): """ When creating an instance of C test that it is also an instance of @@ -700,7 +699,6 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.list_view.clear.assert_called_once_with() self.media_item.search_edit.clear.assert_called_once_with() - def test_on_save_results_button_clicked(self): """ Test that "on_save_results_button_clicked" saves the results. @@ -712,7 +710,7 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.list_view = MagicMock(**{'selectedItems.return_value': [result_1, result_2, result_3]}) with patch.object(self.media_item, 'on_results_view_tab_total_update') as \ - mocked_on_results_view_tab_total_update : + mocked_on_results_view_tab_total_update: # WHEN: When the save_results_button is clicked self.media_item.on_save_results_button_clicked() From fc9ed3bc2c5cc89876aba55f9edb0ab11b10f933 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Mon, 3 Apr 2017 21:32:13 +0100 Subject: [PATCH 04/32] PEP Fixes --- openlp/core/ui/lib/listwidgetwithdnd.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openlp/core/ui/lib/listwidgetwithdnd.py b/openlp/core/ui/lib/listwidgetwithdnd.py index 2d7874b7a..1758d46fb 100755 --- a/openlp/core/ui/lib/listwidgetwithdnd.py +++ b/openlp/core/ui/lib/listwidgetwithdnd.py @@ -128,7 +128,7 @@ class ListWidgetWithDnD(QtWidgets.QListWidget): def allItems(self): """ An generator to list all the items in the widget - + :return: a generator """ for row in range(self.count()): From 902a2161fbe078aa8b512e85580b27a97b45fcab Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 5 May 2017 22:00:59 +0100 Subject: [PATCH 06/32] fixes --- openlp/plugins/bibles/lib/__init__.py | 6 ++-- openlp/plugins/bibles/lib/db.py | 13 +++++++ openlp/plugins/bibles/lib/manager.py | 49 ++++++------------------- openlp/plugins/bibles/lib/mediaitem.py | 50 +++++++++++++++++--------- 4 files changed, 60 insertions(+), 58 deletions(-) diff --git a/openlp/plugins/bibles/lib/__init__.py b/openlp/plugins/bibles/lib/__init__.py index bb190ab65..e4025411c 100644 --- a/openlp/plugins/bibles/lib/__init__.py +++ b/openlp/plugins/bibles/lib/__init__.py @@ -341,10 +341,10 @@ def parse_reference(reference, bible, language_selection, book_ref_id=False): if not book_ref_id: book_ref_id = bible.get_book_ref_id_by_localised_name(book, language_selection) elif not bible.get_book_by_book_ref_id(book_ref_id): - return False + return [] # We have not found the book so do not continue if not book_ref_id: - return False + return [] ranges = match.group('ranges') range_list = get_reference_match('range_separator').split(ranges) ref_list = [] @@ -403,7 +403,7 @@ def parse_reference(reference, bible, language_selection, book_ref_id=False): return ref_list else: log.debug('Invalid reference: {text}'.format(text=reference)) - return None + return [] class SearchResults(object): diff --git a/openlp/plugins/bibles/lib/db.py b/openlp/plugins/bibles/lib/db.py index aaa90efe4..acfb85a26 100644 --- a/openlp/plugins/bibles/lib/db.py +++ b/openlp/plugins/bibles/lib/db.py @@ -158,6 +158,7 @@ class BibleDB(Manager): self.get_name() if 'path' in kwargs: self.path = kwargs['path'] + self._is_web_bible = None def get_name(self): """ @@ -426,6 +427,18 @@ class BibleDB(Manager): return 0 return count + @property + def is_web_bible(self): + """ + A read only property indicating if the bible is a 'web bible' + + :return: If the bible is a web bible. + :rtype: bool + """ + if self._is_web_bible == None: + self._is_web_bible = bool(self.get_object(BibleMeta, 'download_source')) + return self._is_web_bible + def dump_bible(self): """ Utility debugging method to dump the contents of a bible. diff --git a/openlp/plugins/bibles/lib/manager.py b/openlp/plugins/bibles/lib/manager.py index 6154c83a9..fb3a00e64 100644 --- a/openlp/plugins/bibles/lib/manager.py +++ b/openlp/plugins/bibles/lib/manager.py @@ -142,8 +142,8 @@ class BibleManager(OpenLPMixin, RegistryProperties): log.debug('Bible Name: "{name}"'.format(name=name)) self.db_cache[name] = bible # Look to see if lazy load bible exists and get create getter. - source = self.db_cache[name].get_object(BibleMeta, 'download_source') - if source: + if self.db_cache[name].is_web_bible: + source = self.db_cache[name].get_object(BibleMeta, 'download_source') download_name = self.db_cache[name].get_object(BibleMeta, 'download_name').value meta_proxy = self.db_cache[name].get_object(BibleMeta, 'proxy_server') web_bible = HTTPBible(self.parent, path=self.path, file=filename, download_source=source.value, @@ -278,7 +278,7 @@ class BibleManager(OpenLPMixin, RegistryProperties): :param show_error: """ if not bible or not ref_list: - return None + return [] return self.db_cache[bible].get_verses(ref_list, show_error) def get_language_selection(self, bible): @@ -308,8 +308,13 @@ class BibleManager(OpenLPMixin, RegistryProperties): :param bible: The bible to search in (unicode). :param second_bible: The second bible (unicode). We do not search in this bible. :param text: The text to search for (unicode). + + :return: The search results if valid, or None if the search is invalid. + :rtype: None, list """ log.debug('BibleManager.verse_search("{bible}", "{text}")'.format(bible=bible, text=text)) + if not text: + return None # If no bibles are installed, message is given. if not bible: self.main_window.information_message( @@ -317,8 +322,7 @@ class BibleManager(OpenLPMixin, RegistryProperties): UiStrings().BibleNoBibles) return None # Check if the bible or second_bible is a web bible. - web_bible = self.db_cache[bible].get_object(BibleMeta, 'download_source') - if web_bible: + if self.db_cache[bible].is_web_bible: # If either Bible is Web, cursor is reset to normal and message is given. self.application.set_normal_cursor() self.main_window.information_message( @@ -328,41 +332,8 @@ class BibleManager(OpenLPMixin, RegistryProperties): 'This means that the currently selected Bible is a Web Bible.') ) return None - # Shorter than 3 char searches break OpenLP with very long search times, thus they are blocked. - if len(text) - text.count(' ') < 3: - return None # Fetch the results from db. If no results are found, return None, no message is given for this. - elif text: - return self.db_cache[bible].verse_search(text) - else: - return None - - def verse_search_while_typing(self, bible, second_bible, text): - """ - Does a verse search for the given bible and text. - This is used during "Search while typing" - It's the same thing as the normal text search, but it does not show the web Bible error. - (It would result in the error popping every time a char is entered or removed) - It also does not have a minimum text len, this is set in mediaitem.py - - :param bible: The bible to search in (unicode). - :param second_bible: The second bible (unicode). We do not search in this bible. - :param text: The text to search for (unicode). - """ - # If no bibles are installed, message is given. - if not bible: - return None - # Check if the bible or second_bible is a web bible. - web_bible = self.db_cache[bible].get_object(BibleMeta, 'download_source') - second_web_bible = '' - if second_bible: - second_web_bible = self.db_cache[second_bible].get_object(BibleMeta, 'download_source') - if web_bible or second_web_bible: - # If either Bible is Web, cursor is reset to normal and search ends w/o any message. - self.application.set_normal_cursor() - return None - # Fetch the results from db. If no results are found, return None, no message is given for this. - elif text: + if text: return self.db_cache[bible].verse_search(text) else: return None diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index 0fbabab31..7f6d077dc 100755 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -175,8 +175,10 @@ class BibleMediaItem(MediaManagerItem): self.select_tab.setVisible(False) self.page_layout.addWidget(self.select_tab) # General Search Opions - self.options_widget = QtWidgets.QGroupBox(translate('BiblesPlugin.MediaItem', 'Options'), self) - self.general_bible_layout = QtWidgets.QFormLayout(self.options_widget) + self.options_tab = QtWidgets.QWidget() + self.options_tab.setSizePolicy(QtWidgets.QSizePolicy.Minimum, QtWidgets.QSizePolicy.Minimum) + self.search_tab_bar.addTab(translate('BiblesPlugin.MediaItem', 'Options')) + self.general_bible_layout = QtWidgets.QFormLayout(self.options_tab) self.version_combo_box = create_horizontal_adjusting_combo_box(self, 'version_combo_box') self.general_bible_layout.addRow('{version}:'.format(version=UiStrings().Version), self.version_combo_box) self.second_combo_box = create_horizontal_adjusting_combo_box(self, 'second_combo_box') @@ -184,19 +186,23 @@ class BibleMediaItem(MediaManagerItem): self.style_combo_box = create_horizontal_adjusting_combo_box(self, 'style_combo_box') self.style_combo_box.addItems(['', '', '']) self.general_bible_layout.addRow(UiStrings().LayoutStyle, self.style_combo_box) - self.search_button_layout = QtWidgets.QHBoxLayout() + self.options_tab.setVisible(False) + self.page_layout.addWidget(self.options_tab) + # This widget is the easier way to reset the spacing of search_button_layout. (Because page_layout has had its + # spacing set to 0) + self.search_button_widget = QtWidgets.QWidget() + self.search_button_layout = QtWidgets.QHBoxLayout(self.search_button_widget) self.search_button_layout.addStretch() # Note: If we use QPushButton instead of the QToolButton, the icon will be larger than the Lock icon. - self.clear_button = QtWidgets.QToolButton(self) + self.clear_button = QtWidgets.QPushButton() self.clear_button.setIcon(self.clear_icon) - self.save_results_button = QtWidgets.QToolButton(self) + self.save_results_button = QtWidgets.QPushButton() self.save_results_button.setIcon(self.save_results_icon) self.search_button_layout.addWidget(self.clear_button) self.search_button_layout.addWidget(self.save_results_button) self.search_button = QtWidgets.QPushButton(self) self.search_button_layout.addWidget(self.search_button) - self.general_bible_layout.addRow(self.search_button_layout) - self.page_layout.addWidget(self.options_widget) + self.page_layout.addWidget(self.search_button_widget) self.results_view_tab = QtWidgets.QTabBar(self) self.results_view_tab.addTab('') self.results_view_tab.addTab('') @@ -438,9 +444,13 @@ class BibleMediaItem(MediaManagerItem): :param index: The tab selected (int) :return: None """ - search_tab = index == 0 - self.search_tab.setVisible(search_tab) - self.select_tab.setVisible(not search_tab) + if index == 0 or index == 1: + self.search_button.setEnabled(True) + else: + self.search_button.setEnabled(False) + self.search_tab.setVisible(index == 0) + self.select_tab.setVisible(index == 1) + self.options_tab.setVisible(index == 2) self.on_focus() def on_results_view_tab_current_changed(self, index): @@ -542,16 +552,15 @@ class BibleMediaItem(MediaManagerItem): :return: None """ new_selection = self.second_combo_box.currentData() - if self.list_view.count(): + if self.saved_results: # Exclusive or (^) the new and previous selections to detect if the user has switched between single and # dual bible mode if (new_selection is None) ^ (self.second_bible is None): if critical_error_message_box( message=translate('BiblesPlugin.MediaItem', 'OpenLP cannot combine single and dual Bible verse search results. ' - 'Do you want to clear your results and start a new search?'), + 'Do you want to clear your saved results?'), parent=self, question=True) == QtWidgets.QMessageBox.Yes: - self.display_results() self.saved_results = [] self.on_results_view_tab_total_update(ResultsTab.Saved) else: @@ -706,6 +715,8 @@ class BibleMediaItem(MediaManagerItem): This search is called on def text_search by 'Search' Text and Combined Searches. """ self.search_results = self.plugin.manager.verse_search(self.bible.name, text) + if self.search_results is None: + return if self.second_bible and self.search_results: filtered_search_results = [] not_found_count = 0 @@ -774,9 +785,12 @@ class BibleMediaItem(MediaManagerItem): :return: None """ - if Settings().value('bibles/is search while typing enabled'): - if not self.search_timer.isActive(): - self.search_timer.start() + if not Settings().value('bibles/is search while typing enabled') or \ + not self.bible or self.bible.is_web_bible or \ + (self.second_bible and self.bible.is_web_bible): + return + if not self.search_timer.isActive(): + self.search_timer.start() def on_search_timer_timeout(self): """ @@ -969,6 +983,8 @@ class BibleMediaItem(MediaManagerItem): """ Search for some Bible verses (by reference). """ + if self.bible is None: + return [] reference = self.plugin.manager.parse_ref(self.bible.name, string) search_results = self.plugin.manager.get_verses(self.bible.name, reference, showError) if search_results: @@ -980,6 +996,8 @@ class BibleMediaItem(MediaManagerItem): """ Create a media item from an item id. """ + if self.bible is None: + return [] reference = self.plugin.manager.parse_ref(self.bible.name, item_id) search_results = self.plugin.manager.get_verses(self.bible.name, reference, False) items = self.build_display_results(self.bible, None, search_results) From 2747f370a4efebe00dc924cb98e5e1b2f99b0885 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 6 May 2017 11:34:56 +0100 Subject: [PATCH 07/32] Fixed up test --- .../openlp_plugins/bibles/test_lib_parse_reference.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py b/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py index b6fed9f3a..fc6e9e5de 100644 --- a/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py +++ b/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py @@ -108,7 +108,7 @@ class TestBibleManager(TestCase, TestMixin): # WHEN asking to parse the bible reference results = parse_reference('Raoul 1', self.manager.db_cache['tests'], MagicMock()) # THEN a verse array should be returned - self.assertEqual(False, results, "The bible Search should return False") + self.assertEqual([], results, "The bible Search should return an empty list") def test_parse_reference_five(self): """ From 4e88913aa043dce9f83d122f25755cc9a8baa91f Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 6 May 2017 11:45:09 +0100 Subject: [PATCH 08/32] PEP fixes --- openlp/plugins/bibles/lib/manager.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/openlp/plugins/bibles/lib/manager.py b/openlp/plugins/bibles/lib/manager.py index fb3a00e64..4588b53e0 100644 --- a/openlp/plugins/bibles/lib/manager.py +++ b/openlp/plugins/bibles/lib/manager.py @@ -305,10 +305,11 @@ class BibleManager(OpenLPMixin, RegistryProperties): """ Does a verse search for the given bible and text. - :param bible: The bible to search in (unicode). - :param second_bible: The second bible (unicode). We do not search in this bible. - :param text: The text to search for (unicode). - + :param bible: The bible to search + :type bible: str + :param text: The text to search for + :type text: str + :return: The search results if valid, or None if the search is invalid. :rtype: None, list """ From b0c5b4177b65d17e07a757c926bee5ca6c1ab7ac Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 6 May 2017 11:51:54 +0100 Subject: [PATCH 09/32] mor PEP fixes --- openlp/plugins/bibles/lib/db.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openlp/plugins/bibles/lib/db.py b/openlp/plugins/bibles/lib/db.py index acfb85a26..9afdd2efc 100644 --- a/openlp/plugins/bibles/lib/db.py +++ b/openlp/plugins/bibles/lib/db.py @@ -431,11 +431,11 @@ class BibleDB(Manager): def is_web_bible(self): """ A read only property indicating if the bible is a 'web bible' - + :return: If the bible is a web bible. :rtype: bool """ - if self._is_web_bible == None: + if self._is_web_bible is None: self._is_web_bible = bool(self.get_object(BibleMeta, 'download_source')) return self._is_web_bible From 7485e53a72d17e1f1fdca4c1b3db78230b67257d Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 7 May 2017 11:12:08 +0100 Subject: [PATCH 10/32] A few minor changes + annother test --- openlp/plugins/bibles/lib/mediaitem.py | 46 ++++++++++++++++++++------ 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index 7f6d077dc..d7f260fbb 100755 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -22,6 +22,7 @@ import logging import re +from enum import Enum, unique from PyQt5 import QtCore, QtWidgets @@ -48,26 +49,45 @@ def get_reference_separators(): 'list': get_reference_separator('sep_l_display')} -class BibleSearch(object): +@unique +class BibleSearch(Enum): """ - Enumeration class for the different search methods for the "Search" tab. + Enumeration class for the different search types for the "Search" tab. """ Reference = 1 Text = 2 Combined = 3 -class ResultsTab(object): +@unique +class ResultsTab(Enum): + """ + Enumeration class for the different tabs for the results list. + """ Saved = 0 Search = 1 -class SearchStatus(object): +@unique +class SearchStatus(Enum): + """ + Enumeration class for the different search methods. + """ SearchButton = 0 SearchAsYouType = 1 NotEnoughText = 2 +@unique +class SearchTabs(Enum): + """ + Enumeration class for the tabs on the media item. + """ + Search = 0 + Select = 1 + Options = 2 + + class BibleMediaItem(MediaManagerItem): """ This is the custom media manager item for Bibles. @@ -267,8 +287,10 @@ class BibleMediaItem(MediaManagerItem): if self.search_tab.isVisible(): self.search_edit.setFocus() self.search_edit.selectAll() - else: + if self.select_tab.isVisible(): self.select_book_combo_box.setFocus() + if self.options_tab.isVisible(): + self.version_combo_box.setFocus() def config_update(self): """ @@ -441,16 +463,17 @@ class BibleMediaItem(MediaManagerItem): """ Show the selected tab and set focus to it - :param index: The tab selected (int) + :param index: The tab selected + :type index: int :return: None """ - if index == 0 or index == 1: + if index == SearchTabs.Search or index == SearchTabs.Select: self.search_button.setEnabled(True) else: self.search_button.setEnabled(False) - self.search_tab.setVisible(index == 0) - self.select_tab.setVisible(index == 1) - self.options_tab.setVisible(index == 2) + self.search_tab.setVisible(index == SearchTabs.Search) + self.select_tab.setVisible(index == SearchTabs.Select) + self.options_tab.setVisible(index == SearchTabs.Options) self.on_focus() def on_results_view_tab_current_changed(self, index): @@ -588,7 +611,8 @@ class BibleMediaItem(MediaManagerItem): log.warning('Not enough chapters in %s', book_ref_id) critical_error_message_box(message=translate('BiblesPlugin.MediaItem', 'Bible not fully loaded.')) else: - self.search_button.setEnabled(True) + if self.select_tab.isVisible(): + self.search_button.setEnabled(True) self.adjust_combo_box(1, self.chapter_count, self.from_chapter) self.adjust_combo_box(1, self.chapter_count, self.to_chapter) self.adjust_combo_box(1, verse_count, self.from_verse) From 04063a4371ec99a8c8fbc5e4ec7405eeaf7c8d87 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 7 May 2017 19:39:17 +0100 Subject: [PATCH 11/32] Annother fix --- openlp/plugins/bibles/lib/mediaitem.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/openlp/plugins/bibles/lib/mediaitem.py b/openlp/plugins/bibles/lib/mediaitem.py index d7f260fbb..f0ef34dd6 100755 --- a/openlp/plugins/bibles/lib/mediaitem.py +++ b/openlp/plugins/bibles/lib/mediaitem.py @@ -22,7 +22,7 @@ import logging import re -from enum import Enum, unique +from enum import IntEnum, unique from PyQt5 import QtCore, QtWidgets @@ -50,7 +50,7 @@ def get_reference_separators(): @unique -class BibleSearch(Enum): +class BibleSearch(IntEnum): """ Enumeration class for the different search types for the "Search" tab. """ @@ -60,7 +60,7 @@ class BibleSearch(Enum): @unique -class ResultsTab(Enum): +class ResultsTab(IntEnum): """ Enumeration class for the different tabs for the results list. """ @@ -69,7 +69,7 @@ class ResultsTab(Enum): @unique -class SearchStatus(Enum): +class SearchStatus(IntEnum): """ Enumeration class for the different search methods. """ @@ -79,7 +79,7 @@ class SearchStatus(Enum): @unique -class SearchTabs(Enum): +class SearchTabs(IntEnum): """ Enumeration class for the tabs on the media item. """ @@ -110,7 +110,7 @@ class BibleMediaItem(MediaManagerItem): self.second_bible = None self.saved_results = [] self.current_results = [] - self.search_status = SearchStatus().SearchButton + self.search_status = SearchStatus.SearchButton # TODO: Make more central and clean up after! self.search_timer = QtCore.QTimer() self.search_timer.setInterval(200) @@ -690,7 +690,7 @@ class BibleMediaItem(MediaManagerItem): :return: None """ self.search_timer.stop() - self.search_status = SearchStatus().SearchButton + self.search_status = SearchStatus.SearchButton if not self.bible: self.main_window.information_message(UiStrings().BibleNoBiblesTitle, UiStrings().BibleNoBibles) return @@ -794,7 +794,7 @@ class BibleMediaItem(MediaManagerItem): self.text_reference_search(text) else: # It can only be a 'Combined' search without a valid reference, or a 'Text' search - if self.search_status == SearchStatus().SearchAsYouType: + if self.search_status == SearchStatus.SearchAsYouType: if len(text) <= 8: self.search_status = SearchStatus.NotEnoughText self.display_results() @@ -823,7 +823,7 @@ class BibleMediaItem(MediaManagerItem): :return: None """ - self.search_status = SearchStatus().SearchAsYouType + self.search_status = SearchStatus.SearchAsYouType self.text_search() self.results_view_tab.setCurrentIndex(ResultsTab.Search) From 48a87780d4e11e049bd1ae80bfdc0e1092494193 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sat, 13 May 2017 08:47:22 +0100 Subject: [PATCH 12/32] theme clean up --- openlp/core/common/__init__.py | 10 + openlp/core/lib/theme.py | 515 +++++++++--------- openlp/core/ui/thememanager.py | 36 +- .../functional/openlp_core_lib/test_theme.py | 51 +- 4 files changed, 330 insertions(+), 282 deletions(-) diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index b7d38ad4f..ac61d9519 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -438,3 +438,13 @@ def get_file_encoding(filename): return detector.result except OSError: log.exception('Error detecting file encoding') + + +def json_default(o): + """ + Function to help save objects as JSON + + :param o: object + :return: the object dictionary + """ + return o.__dict__ diff --git a/openlp/core/lib/theme.py b/openlp/core/lib/theme.py index 421086e2b..a4397ea0f 100644 --- a/openlp/core/lib/theme.py +++ b/openlp/core/lib/theme.py @@ -150,7 +150,7 @@ INTEGER_LIST = ['size', 'line_adjustment', 'x', 'height', 'y', 'width', 'shadow_ 'horizontal_align', 'vertical_align', 'wrap_style'] -class ThemeXML(object): +class Theme(object): """ A class to encapsulate the Theme XML. """ @@ -195,183 +195,183 @@ class ThemeXML(object): self.background_filename = self.background_filename.strip() self.background_filename = os.path.join(path, self.theme_name, self.background_filename) - def _new_document(self, name): - """ - Create a new theme XML document. - """ - self.theme_xml = Document() - self.theme = self.theme_xml.createElement('theme') - self.theme_xml.appendChild(self.theme) - self.theme.setAttribute('version', '2.0') - self.name = self.theme_xml.createElement('name') - text_node = self.theme_xml.createTextNode(name) - self.name.appendChild(text_node) - self.theme.appendChild(self.name) + # def _new_document(self, name): + # """ + # Create a new theme XML document. + # """ + # self.theme_xml = Document() + # self.theme = self.theme_xml.createElement('theme') + # self.theme_xml.appendChild(self.theme) + # self.theme.setAttribute('version', '2.0') + # self.name = self.theme_xml.createElement('name') + # text_node = self.theme_xml.createTextNode(name) + # self.name.appendChild(text_node) + # self.theme.appendChild(self.name) - def add_background_transparent(self): - """ - Add a transparent background. - """ - background = self.theme_xml.createElement('background') - background.setAttribute('type', 'transparent') - self.theme.appendChild(background) + # def add_background_transparent(self): + # """ + # Add a transparent background. + # """ + # background = self.theme_xml.createElement('background') + # background.setAttribute('type', 'transparent') + # self.theme.appendChild(background) - def add_background_solid(self, bkcolor): - """ - Add a Solid background. + # def add_background_solid(self, bkcolor): + # """ + # Add a Solid background. + # + # :param bkcolor: The color of the background. + # """ + # background = self.theme_xml.createElement('background') + # background.setAttribute('type', 'solid') + # self.theme.appendChild(background) + # self.child_element(background, 'color', str(bkcolor)) - :param bkcolor: The color of the background. - """ - background = self.theme_xml.createElement('background') - background.setAttribute('type', 'solid') - self.theme.appendChild(background) - self.child_element(background, 'color', str(bkcolor)) + # def add_background_gradient(self, startcolor, endcolor, direction): + # """ + # Add a gradient background. + # + # :param startcolor: The gradient's starting colour. + # :param endcolor: The gradient's ending colour. + # :param direction: The direction of the gradient. + # """ + # background = self.theme_xml.createElement('background') + # background.setAttribute('type', 'gradient') + # self.theme.appendChild(background) + # # Create startColor element + # self.child_element(background, 'startColor', str(startcolor)) + # # Create endColor element + # self.child_element(background, 'endColor', str(endcolor)) + # # Create direction element + # self.child_element(background, 'direction', str(direction)) - def add_background_gradient(self, startcolor, endcolor, direction): - """ - Add a gradient background. + # def add_background_image(self, filename, border_color): + # """ + # Add a image background. + # + # :param filename: The file name of the image. + # :param border_color: + # """ + # background = self.theme_xml.createElement('background') + # background.setAttribute('type', 'image') + # self.theme.appendChild(background) + # # Create Filename element + # self.child_element(background, 'filename', filename) + # # Create endColor element + # self.child_element(background, 'borderColor', str(border_color)) + # + # def add_background_video(self, filename, border_color): + # """ + # Add a video background. + # + # :param filename: The file name of the video. + # :param border_color: + # """ + # background = self.theme_xml.createElement('background') + # background.setAttribute('type', 'video') + # self.theme.appendChild(background) + # # Create Filename element + # self.child_element(background, 'filename', filename) + # # Create endColor element + # self.child_element(background, 'borderColor', str(border_color)) - :param startcolor: The gradient's starting colour. - :param endcolor: The gradient's ending colour. - :param direction: The direction of the gradient. - """ - background = self.theme_xml.createElement('background') - background.setAttribute('type', 'gradient') - self.theme.appendChild(background) - # Create startColor element - self.child_element(background, 'startColor', str(startcolor)) - # Create endColor element - self.child_element(background, 'endColor', str(endcolor)) - # Create direction element - self.child_element(background, 'direction', str(direction)) - - def add_background_image(self, filename, border_color): - """ - Add a image background. - - :param filename: The file name of the image. - :param border_color: - """ - background = self.theme_xml.createElement('background') - background.setAttribute('type', 'image') - self.theme.appendChild(background) - # Create Filename element - self.child_element(background, 'filename', filename) - # Create endColor element - self.child_element(background, 'borderColor', str(border_color)) - - def add_background_video(self, filename, border_color): - """ - Add a video background. - - :param filename: The file name of the video. - :param border_color: - """ - background = self.theme_xml.createElement('background') - background.setAttribute('type', 'video') - self.theme.appendChild(background) - # Create Filename element - self.child_element(background, 'filename', filename) - # Create endColor element - self.child_element(background, 'borderColor', str(border_color)) - - def add_font(self, name, color, size, override, fonttype='main', bold='False', italics='False', - line_adjustment=0, xpos=0, ypos=0, width=0, height=0, outline='False', outline_color='#ffffff', - outline_pixel=2, shadow='False', shadow_color='#ffffff', shadow_pixel=5): - """ - Add a Font. - - :param name: The name of the font. - :param color: The colour of the font. - :param size: The size of the font. - :param override: Whether or not to override the default positioning of the theme. - :param fonttype: The type of font, ``main`` or ``footer``. Defaults to ``main``. - :param bold: - :param italics: The weight of then font Defaults to 50 Normal - :param line_adjustment: Does the font render to italics Defaults to 0 Normal - :param xpos: The X position of the text block. - :param ypos: The Y position of the text block. - :param width: The width of the text block. - :param height: The height of the text block. - :param outline: Whether or not to show an outline. - :param outline_color: The colour of the outline. - :param outline_pixel: How big the Shadow is - :param shadow: Whether or not to show a shadow. - :param shadow_color: The colour of the shadow. - :param shadow_pixel: How big the Shadow is - """ - background = self.theme_xml.createElement('font') - background.setAttribute('type', fonttype) - self.theme.appendChild(background) - # Create Font name element - self.child_element(background, 'name', name) - # Create Font color element - self.child_element(background, 'color', str(color)) - # Create Proportion name element - self.child_element(background, 'size', str(size)) - # Create weight name element - self.child_element(background, 'bold', str(bold)) - # Create italics name element - self.child_element(background, 'italics', str(italics)) - # Create indentation name element - self.child_element(background, 'line_adjustment', str(line_adjustment)) - # Create Location element - element = self.theme_xml.createElement('location') - element.setAttribute('override', str(override)) - element.setAttribute('x', str(xpos)) - element.setAttribute('y', str(ypos)) - element.setAttribute('width', str(width)) - element.setAttribute('height', str(height)) - background.appendChild(element) - # Shadow - element = self.theme_xml.createElement('shadow') - element.setAttribute('shadowColor', str(shadow_color)) - element.setAttribute('shadowSize', str(shadow_pixel)) - value = self.theme_xml.createTextNode(str(shadow)) - element.appendChild(value) - background.appendChild(element) - # Outline - element = self.theme_xml.createElement('outline') - element.setAttribute('outlineColor', str(outline_color)) - element.setAttribute('outlineSize', str(outline_pixel)) - value = self.theme_xml.createTextNode(str(outline)) - element.appendChild(value) - background.appendChild(element) - - def add_display(self, horizontal, vertical, transition): - """ - Add a Display options. - - :param horizontal: The horizontal alignment of the text. - :param vertical: The vertical alignment of the text. - :param transition: Whether the slide transition is active. - """ - background = self.theme_xml.createElement('display') - self.theme.appendChild(background) - # Horizontal alignment - element = self.theme_xml.createElement('horizontalAlign') - value = self.theme_xml.createTextNode(str(horizontal)) - element.appendChild(value) - background.appendChild(element) - # Vertical alignment - element = self.theme_xml.createElement('verticalAlign') - value = self.theme_xml.createTextNode(str(vertical)) - element.appendChild(value) - background.appendChild(element) - # Slide Transition - element = self.theme_xml.createElement('slideTransition') - value = self.theme_xml.createTextNode(str(transition)) - element.appendChild(value) - background.appendChild(element) - - def child_element(self, element, tag, value): - """ - Generic child element creator. - """ - child = self.theme_xml.createElement(tag) - child.appendChild(self.theme_xml.createTextNode(value)) - element.appendChild(child) - return child + # def add_font(self, name, color, size, override, fonttype='main', bold='False', italics='False', + # line_adjustment=0, xpos=0, ypos=0, width=0, height=0, outline='False', outline_color='#ffffff', + # outline_pixel=2, shadow='False', shadow_color='#ffffff', shadow_pixel=5): + # """ + # Add a Font. + # + # :param name: The name of the font. + # :param color: The colour of the font. + # :param size: The size of the font. + # :param override: Whether or not to override the default positioning of the theme. + # :param fonttype: The type of font, ``main`` or ``footer``. Defaults to ``main``. + # :param bold: + # :param italics: The weight of then font Defaults to 50 Normal + # :param line_adjustment: Does the font render to italics Defaults to 0 Normal + # :param xpos: The X position of the text block. + # :param ypos: The Y position of the text block. + # :param width: The width of the text block. + # :param height: The height of the text block. + # :param outline: Whether or not to show an outline. + # :param outline_color: The colour of the outline. + # :param outline_pixel: How big the Shadow is + # :param shadow: Whether or not to show a shadow. + # :param shadow_color: The colour of the shadow. + # :param shadow_pixel: How big the Shadow is + # """ + # background = self.theme_xml.createElement('font') + # background.setAttribute('type', fonttype) + # self.theme.appendChild(background) + # # Create Font name element + # self.child_element(background, 'name', name) + # # Create Font color element + # self.child_element(background, 'color', str(color)) + # # Create Proportion name element + # self.child_element(background, 'size', str(size)) + # # Create weight name element + # self.child_element(background, 'bold', str(bold)) + # # Create italics name element + # self.child_element(background, 'italics', str(italics)) + # # Create indentation name element + # self.child_element(background, 'line_adjustment', str(line_adjustment)) + # # Create Location element + # element = self.theme_xml.createElement('location') + # element.setAttribute('override', str(override)) + # element.setAttribute('x', str(xpos)) + # element.setAttribute('y', str(ypos)) + # element.setAttribute('width', str(width)) + # element.setAttribute('height', str(height)) + # background.appendChild(element) + # # Shadow + # element = self.theme_xml.createElement('shadow') + # element.setAttribute('shadowColor', str(shadow_color)) + # element.setAttribute('shadowSize', str(shadow_pixel)) + # value = self.theme_xml.createTextNode(str(shadow)) + # element.appendChild(value) + # background.appendChild(element) + # # Outline + # element = self.theme_xml.createElement('outline') + # element.setAttribute('outlineColor', str(outline_color)) + # element.setAttribute('outlineSize', str(outline_pixel)) + # value = self.theme_xml.createTextNode(str(outline)) + # element.appendChild(value) + # background.appendChild(element) + # + # def add_display(self, horizontal, vertical, transition): + # """ + # Add a Display options. + # + # :param horizontal: The horizontal alignment of the text. + # :param vertical: The vertical alignment of the text. + # :param transition: Whether the slide transition is active. + # """ + # background = self.theme_xml.createElement('display') + # self.theme.appendChild(background) + # # Horizontal alignment + # element = self.theme_xml.createElement('horizontalAlign') + # value = self.theme_xml.createTextNode(str(horizontal)) + # element.appendChild(value) + # background.appendChild(element) + # # Vertical alignment + # element = self.theme_xml.createElement('verticalAlign') + # value = self.theme_xml.createTextNode(str(vertical)) + # element.appendChild(value) + # background.appendChild(element) + # # Slide Transition + # element = self.theme_xml.createElement('slideTransition') + # value = self.theme_xml.createTextNode(str(transition)) + # element.appendChild(value) + # background.appendChild(element) + # + # def child_element(self, element, tag, value): + # """ + # Generic child element creator. + # """ + # child = self.theme_xml.createElement(tag) + # child.appendChild(self.theme_xml.createTextNode(value)) + # element.appendChild(child) + # return child def set_default_header_footer(self): """ @@ -386,25 +386,34 @@ class ThemeXML(object): self.font_footer_y = current_screen['size'].height() * 9 / 10 self.font_footer_height = current_screen['size'].height() / 10 - def dump_xml(self): - """ - Dump the XML to file used for debugging - """ - return self.theme_xml.toprettyxml(indent=' ') + # def dump_xml(self): + # """ + # Dump the XML to file used for debugging + # """ + # return self.theme_xml.toprettyxml(indent=' ') - def extract_xml(self): - """ - Print out the XML string. - """ - self._build_xml_from_attrs() - return self.theme_xml.toxml('utf-8').decode('utf-8') + # def extract_xml(self): + # """ + # Print out the XML string. + # """ + # self._build_xml_from_attrs() + # return self.theme_xml.toxml('utf-8').decode('utf-8') + # + # def extract_formatted_xml(self): + # """ + # Pull out the XML string formatted for human consumption + # """ + # self._build_xml_from_attrs() + # return self.theme_xml.toprettyxml(indent=' ', newl='\n', encoding='utf-8') - def extract_formatted_xml(self): + def load_theme(self, theme): """ Pull out the XML string formatted for human consumption + + :param theme: the theme string """ - self._build_xml_from_attrs() - return self.theme_xml.toprettyxml(indent=' ', newl='\n', encoding='utf-8') + jsn = json.loads(theme) + self.expand_json(jsn) def parse(self, xml): """ @@ -461,7 +470,8 @@ class ThemeXML(object): if element.tag == 'name': self._create_attr('theme', element.tag, element.text) - def _translate_tags(self, master, element, value): + @staticmethod + def _translate_tags(master, element, value): """ Clean up XML removing and redefining tags """ @@ -514,71 +524,70 @@ class ThemeXML(object): theme_strings = [] for key in dir(self): if key[0:1] != '_': - # TODO: Due to bound methods returned, I don't know how to write a proper test theme_strings.append('{key:>30}: {value}'.format(key=key, value=getattr(self, key))) return '\n'.join(theme_strings) - def _build_xml_from_attrs(self): - """ - Build the XML from the varables in the object - """ - self._new_document(self.theme_name) - if self.background_type == BackgroundType.to_string(BackgroundType.Solid): - self.add_background_solid(self.background_color) - elif self.background_type == BackgroundType.to_string(BackgroundType.Gradient): - self.add_background_gradient( - self.background_start_color, - self.background_end_color, - self.background_direction - ) - elif self.background_type == BackgroundType.to_string(BackgroundType.Image): - filename = os.path.split(self.background_filename)[1] - self.add_background_image(filename, self.background_border_color) - elif self.background_type == BackgroundType.to_string(BackgroundType.Video): - filename = os.path.split(self.background_filename)[1] - self.add_background_video(filename, self.background_border_color) - elif self.background_type == BackgroundType.to_string(BackgroundType.Transparent): - self.add_background_transparent() - self.add_font( - self.font_main_name, - self.font_main_color, - self.font_main_size, - self.font_main_override, 'main', - self.font_main_bold, - self.font_main_italics, - self.font_main_line_adjustment, - self.font_main_x, - self.font_main_y, - self.font_main_width, - self.font_main_height, - self.font_main_outline, - self.font_main_outline_color, - self.font_main_outline_size, - self.font_main_shadow, - self.font_main_shadow_color, - self.font_main_shadow_size - ) - self.add_font( - self.font_footer_name, - self.font_footer_color, - self.font_footer_size, - self.font_footer_override, 'footer', - self.font_footer_bold, - self.font_footer_italics, - 0, # line adjustment - self.font_footer_x, - self.font_footer_y, - self.font_footer_width, - self.font_footer_height, - self.font_footer_outline, - self.font_footer_outline_color, - self.font_footer_outline_size, - self.font_footer_shadow, - self.font_footer_shadow_color, - self.font_footer_shadow_size - ) - self.add_display( - self.display_horizontal_align, - self.display_vertical_align, - self.display_slide_transition - ) + # def _build_xml_from_attrs(self): + # """ + # Build the XML from the varables in the object + # """ + # self._new_document(self.theme_name) + # if self.background_type == BackgroundType.to_string(BackgroundType.Solid): + # self.add_background_solid(self.background_color) + # elif self.background_type == BackgroundType.to_string(BackgroundType.Gradient): + # self.add_background_gradient( + # self.background_start_color, + # self.background_end_color, + # self.background_direction + # ) + # elif self.background_type == BackgroundType.to_string(BackgroundType.Image): + # filename = os.path.split(self.background_filename)[1] + # self.add_background_image(filename, self.background_border_color) + # elif self.background_type == BackgroundType.to_string(BackgroundType.Video): + # filename = os.path.split(self.background_filename)[1] + # self.add_background_video(filename, self.background_border_color) + # elif self.background_type == BackgroundType.to_string(BackgroundType.Transparent): + # self.add_background_transparent() + # self.add_font( + # self.font_main_name, + # self.font_main_color, + # self.font_main_size, + # self.font_main_override, 'main', + # self.font_main_bold, + # self.font_main_italics, + # self.font_main_line_adjustment, + # self.font_main_x, + # self.font_main_y, + # self.font_main_width, + # self.font_main_height, + # self.font_main_outline, + # self.font_main_outline_color, + # self.font_main_outline_size, + # self.font_main_shadow, + # self.font_main_shadow_color, + # self.font_main_shadow_size + # ) + # self.add_font( + # self.font_footer_name, + # self.font_footer_color, + # self.font_footer_size, + # self.font_footer_override, 'footer', + # self.font_footer_bold, + # self.font_footer_italics, + # 0, # line adjustment + # self.font_footer_x, + # self.font_footer_y, + # self.font_footer_width, + # self.font_footer_height, + # self.font_footer_outline, + # self.font_footer_outline_color, + # self.font_footer_outline_size, + # self.font_footer_shadow, + # self.font_footer_shadow_color, + # self.font_footer_shadow_size + # ) + # self.add_display( + # self.display_horizontal_align, + # self.display_vertical_align, + # self.display_slide_transition + # ) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index f5eca3656..ba31d1954 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -22,6 +22,7 @@ """ The Theme Manager manages adding, deleteing and modifying of themes. """ +import json import os import zipfile import shutil @@ -30,10 +31,10 @@ from xml.etree.ElementTree import ElementTree, XML from PyQt5 import QtCore, QtGui, QtWidgets from openlp.core.common import Registry, RegistryProperties, AppLocation, Settings, OpenLPMixin, RegistryMixin, \ - check_directory_exists, UiStrings, translate, is_win, get_filesystem_encoding, delete_file + check_directory_exists, UiStrings, translate, is_win, get_filesystem_encoding, delete_file, json_default from openlp.core.lib import FileDialog, ImageSource, ValidationError, get_text_file_string, build_icon, \ check_item_selected, create_thumb, validate_thumb -from openlp.core.lib.theme import ThemeXML, BackgroundType +from openlp.core.lib.theme import Theme, BackgroundType from openlp.core.lib.ui import critical_error_message_box, create_widget_action from openlp.core.ui import FileRenameForm, ThemeForm from openlp.core.ui.lib import OpenLPToolbar @@ -245,7 +246,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage their customisations. :param field: """ - theme = ThemeXML() + theme = Theme() theme.set_default_header_footer() self.theme_form.theme = theme self.theme_form.exec() @@ -452,7 +453,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage files = AppLocation.get_files(self.settings_section, '.png') # No themes have been found so create one if not files: - theme = ThemeXML() + theme = Theme() theme.theme_name = UiStrings().Default self._write_theme(theme, None, None) Settings().setValue(self.settings_section + '/global theme', theme.theme_name) @@ -515,7 +516,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage xml = get_text_file_string(xml_file) if not xml: self.log_debug('No theme data - using default theme') - return ThemeXML() + return Theme() else: return self._create_theme_from_xml(xml, self.path) @@ -646,16 +647,16 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage :param image_to: Where the Theme Image is to be saved to """ name = theme.theme_name - theme_pretty_xml = theme.extract_formatted_xml() + theme_pretty = json.dumps(theme, default=json_default) theme_dir = os.path.join(self.path, name) check_directory_exists(theme_dir) - theme_file = os.path.join(theme_dir, name + '.xml') + theme_file = os.path.join(theme_dir, name + '.json') if self.old_background_image and image_to != self.old_background_image: delete_file(self.old_background_image) out_file = None try: out_file = open(theme_file, 'w', encoding='utf-8') - out_file.write(theme_pretty_xml.decode('utf-8')) + out_file.write(theme_pretty) except IOError: self.log_exception('Saving theme to file failed') finally: @@ -717,7 +718,8 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage """ return os.path.join(self.path, theme + '.png') - def _create_theme_from_xml(self, theme_xml, image_path): + @staticmethod + def _create_theme_from_xml(theme_xml, image_path): """ Return a theme object using information parsed from XML @@ -725,11 +727,25 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage :param image_path: Where the theme image is stored :return: Theme data. """ - theme = ThemeXML() + theme = Theme() theme.parse(theme_xml) theme.extend_image_filename(image_path) return theme + @staticmethod + def _create_theme_from_json(theme_json, image_path): + """ + Return a theme object using information parsed from JSON + + :param theme_json: The Theme data object. + :param image_path: Where the theme image is stored + :return: Theme data. + """ + theme = Theme() + theme.load_theme(theme_json) + theme.extend_image_filename(image_path) + return theme + def _validate_theme_action(self, select_text, confirm_title, confirm_text, test_plugin=True, confirm=True): """ Check to see if theme has been selected and the destructive action is allowed. diff --git a/tests/functional/openlp_core_lib/test_theme.py b/tests/functional/openlp_core_lib/test_theme.py index 7401698d9..04a31c5d5 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -22,39 +22,35 @@ """ Package to test the openlp.core.lib.theme package. """ +import json from unittest import TestCase import os -from openlp.core.lib.theme import ThemeXML +from openlp.core.common import json_default +from openlp.core.lib.theme import Theme -class TestThemeXML(TestCase): +class TestTheme(TestCase): """ - Test the ThemeXML class + Test the ThemeL class """ def test_new_theme(self): """ - Test the ThemeXML constructor + Test the Theme constructor """ # GIVEN: The ThemeXML class # WHEN: A theme object is created - default_theme = ThemeXML() + default_theme = Theme() # THEN: The default values should be correct - self.assertEqual('#000000', default_theme.background_border_color, - 'background_border_color should be "#000000"') - self.assertEqual('solid', default_theme.background_type, 'background_type should be "solid"') - self.assertEqual(0, default_theme.display_vertical_align, 'display_vertical_align should be 0') - self.assertEqual('Arial', default_theme.font_footer_name, 'font_footer_name should be "Arial"') - self.assertFalse(default_theme.font_main_bold, 'font_main_bold should be False') - self.assertEqual(47, len(default_theme.__dict__), 'The theme should have 47 attributes') + self.check_theme(default_theme) def test_expand_json(self): """ Test the expand_json method """ # GIVEN: A ThemeXML object and some JSON to "expand" - theme = ThemeXML() + theme = Theme() theme_json = { 'background': { 'border_color': '#000000', @@ -77,18 +73,14 @@ class TestThemeXML(TestCase): theme.expand_json(theme_json) # THEN: The attributes should be set on the object - self.assertEqual('#000000', theme.background_border_color, 'background_border_color should be "#000000"') - self.assertEqual('solid', theme.background_type, 'background_type should be "solid"') - self.assertEqual(0, theme.display_vertical_align, 'display_vertical_align should be 0') - self.assertFalse(theme.font_footer_bold, 'font_footer_bold should be False') - self.assertEqual('Arial', theme.font_main_name, 'font_main_name should be "Arial"') + self.check_theme(theme) def test_extend_image_filename(self): """ Test the extend_image_filename method """ # GIVEN: A theme object - theme = ThemeXML() + theme = Theme() theme.theme_name = 'MyBeautifulTheme ' theme.background_filename = ' video.mp4' theme.background_type = 'video' @@ -101,3 +93,24 @@ class TestThemeXML(TestCase): expected_filename = os.path.join(path, 'MyBeautifulTheme', 'video.mp4') self.assertEqual(expected_filename, theme.background_filename) self.assertEqual('MyBeautifulTheme', theme.theme_name) + + def test_save_retrieve(self): + """ + Load a dummy theme, save it and reload it + """ + # GIVEN: The default Theme class + # WHEN: A theme object is created + default_theme = Theme() + # THEN: The default values should be correct + save_theme_json = json.dumps(default_theme, default=json_default) + lt = Theme() + lt.load_theme(save_theme_json) + self.check_theme(lt) + + def check_theme(self, theme): + self.assertEqual('#000000', theme.background_border_color, 'background_border_color should be "#000000"') + self.assertEqual('solid', theme.background_type, 'background_type should be "solid"') + self.assertEqual(0, theme.display_vertical_align, 'display_vertical_align should be 0') + self.assertFalse(theme.font_footer_bold, 'font_footer_bold should be False') + self.assertEqual('Arial', theme.font_main_name, 'font_main_name should be "Arial"') + self.assertEqual(47, len(theme.__dict__), 'The theme should have 47 attributes') From 3d1be586fefc45c88a66b7757e1e6f7ac661c943 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sat, 13 May 2017 09:22:48 +0100 Subject: [PATCH 13/32] fix editing themes --- openlp/core/ui/thememanager.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index ba31d1954..c02be62e2 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -512,13 +512,20 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage :return: The theme object. """ self.log_debug('get theme data for theme {name}'.format(name=theme_name)) - xml_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.xml') - xml = get_text_file_string(xml_file) - if not xml: + theme_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.json') + jsn = True + if not theme_file: + theme_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.xml') + jsn = False + theme_data = get_text_file_string(theme_file) + if not theme_data: self.log_debug('No theme data - using default theme') return Theme() else: - return self._create_theme_from_xml(xml, self.path) + if jsn: + return self._create_theme_from_json(theme_data, self.path) + else: + return self._create_theme_from_xml(theme_data, self.path) def over_write_message_box(self, theme_name): """ From 08a58844c6aead18167af2aa4d579dada9e7629b Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sat, 13 May 2017 09:37:38 +0100 Subject: [PATCH 14/32] fix editing theme load and save --- openlp/core/ui/thememanager.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index c02be62e2..d6d05d1bb 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -513,11 +513,12 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage """ self.log_debug('get theme data for theme {name}'.format(name=theme_name)) theme_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.json') - jsn = True - if not theme_file: - theme_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.xml') - jsn = False theme_data = get_text_file_string(theme_file) + jsn = True + if not theme_data: + theme_file = os.path.join(self.path, str(theme_name), str(theme_name) + '.xml') + theme_data = get_text_file_string(theme_file) + jsn = False if not theme_data: self.log_debug('No theme data - using default theme') return Theme() From fa89be414cfc0e769bd1f18d830cea61b61fecfe Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sun, 21 May 2017 08:11:36 +0100 Subject: [PATCH 15/32] Clean up theme export --- openlp/core/ui/servicemanager.py | 2 +- openlp/core/ui/thememanager.py | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/openlp/core/ui/servicemanager.py b/openlp/core/ui/servicemanager.py index a5da351c7..cf30245bf 100644 --- a/openlp/core/ui/servicemanager.py +++ b/openlp/core/ui/servicemanager.py @@ -698,7 +698,7 @@ class ServiceManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ServiceMa translate('OpenLP.ServiceManager', 'OpenLP Service Files (*.osz);; OpenLP Service Files - lite (*.oszl)')) else: - file_name, filter_uesd = QtWidgets.QFileDialog.getSaveFileName( + file_name, filter_used = QtWidgets.QFileDialog.getSaveFileName( self.main_window, UiStrings().SaveService, path, translate('OpenLP.ServiceManager', 'OpenLP Service Files (*.osz);;')) if not file_name: diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index d6d05d1bb..415b4b726 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -379,11 +379,10 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage critical_error_message_box(message=translate('OpenLP.ThemeManager', 'You have not selected a theme.')) return theme = item.data(QtCore.Qt.UserRole) - path = QtWidgets.QFileDialog.getExistingDirectory(self, - translate('OpenLP.ThemeManager', - 'Save Theme - ({name})').format(name=theme), - Settings().value(self.settings_section + - '/last directory export')) + path, filter_used = QtWidgets.QFileDialog.getSaveFileName(self.main_window, + translate('OpenLP.ThemeManager', 'Save Theme - ({name})').format(name=theme), + Settings().value(self.settings_section + '/last directory export'), + translate('OpenLP.ThemeManager', 'OpenLP Themes (*.otz)')) self.application.set_busy_cursor() if path: Settings().setValue(self.settings_section + '/last directory export', path) @@ -394,13 +393,12 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage 'Your theme has been successfully exported.')) self.application.set_normal_cursor() - def _export_theme(self, path, theme): + def _export_theme(self, theme_path, theme): """ Create the zipfile with the theme contents. - :param path: Location where the zip file will be placed + :param theme_path: Location where the zip file will be placed :param theme: The name of the theme to be exported """ - theme_path = os.path.join(path, theme + '.otz') theme_zip = None try: theme_zip = zipfile.ZipFile(theme_path, 'w') From 0387286a93db53d8c3583d9bfd1140b2ec80a687 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sun, 21 May 2017 16:30:02 +0100 Subject: [PATCH 16/32] fix up import functionality --- openlp/core/ui/thememanager.py | 35 ++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 415b4b726..c8b2eaa38 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -556,16 +556,24 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage abort_import = True try: theme_zip = zipfile.ZipFile(file_name) - xml_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.xml'] - if len(xml_file) != 1: - self.log_error('Theme contains "{val:d}" XML files'.format(val=len(xml_file))) - raise ValidationError - xml_tree = ElementTree(element=XML(theme_zip.read(xml_file[0]))).getroot() - theme_version = xml_tree.get('version', default=None) - if not theme_version or float(theme_version) < 2.0: - self.log_error('Theme version is less than 2.0') - raise ValidationError - theme_name = xml_tree.find('name').text.strip() + json_theme = False + json_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.json'] + if len(json_file) != 1: + xml_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.xml'] + if len(xml_file) != 1: + self.log_error('Theme contains "{val:d}" theme files'.format(val=len(xml_file))) + raise ValidationError + xml_tree = ElementTree(element=XML(theme_zip.read(xml_file[0]))).getroot() + theme_version = xml_tree.get('version', default=None) + if not theme_version or float(theme_version) < 2.0: + self.log_error('Theme version is less than 2.0') + raise ValidationError + theme_name = xml_tree.find('name').text.strip() + else: + theme = Theme() + theme.load_theme(theme_zip.read(json_file[0]).decode("utf-8")) + theme_name = theme.theme_name + json_theme = True theme_folder = os.path.join(directory, theme_name) theme_exists = os.path.exists(theme_folder) if theme_exists and not self.over_write_message_box(theme_name): @@ -581,7 +589,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage continue full_name = os.path.join(directory, out_name) check_directory_exists(os.path.dirname(full_name)) - if os.path.splitext(name)[1].lower() == '.xml': + if os.path.splitext(name)[1].lower() == '.xml' or os.path.splitext(name)[1].lower() == '.json': file_xml = str(theme_zip.read(name), 'utf-8') out_file = open(full_name, 'w', encoding='utf-8') out_file.write(file_xml) @@ -604,7 +612,10 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage if not abort_import: # As all files are closed, we can create the Theme. if file_xml: - theme = self._create_theme_from_xml(file_xml, self.path) + if json_theme: + theme = self._create_theme_from_json(file_xml, self.path) + else: + theme = self._create_theme_from_xml(file_xml, self.path) self.generate_and_save_image(theme_name, theme) # Only show the error message, when IOError was not raised (in # this case the error message has already been shown). From 7fbeb10207bfcc217c43e2213091a4fb922a03f1 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sun, 21 May 2017 17:02:02 +0100 Subject: [PATCH 17/32] text fixes --- tests/functional/openlp_core_lib/test_renderer.py | 4 ++-- tests/functional/openlp_core_lib/test_theme.py | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/functional/openlp_core_lib/test_renderer.py b/tests/functional/openlp_core_lib/test_renderer.py index a2a156094..69d93ab5e 100644 --- a/tests/functional/openlp_core_lib/test_renderer.py +++ b/tests/functional/openlp_core_lib/test_renderer.py @@ -29,7 +29,7 @@ from PyQt5 import QtCore from openlp.core.common import Registry from openlp.core.lib import Renderer, ScreenList, ServiceItem, FormattingTags from openlp.core.lib.renderer import words_split, get_start_tags -from openlp.core.lib.theme import ThemeXML +from openlp.core.lib.theme import Theme from tests.functional import MagicMock, patch @@ -189,7 +189,7 @@ class TestRenderer(TestCase): # GIVEN: test object and data mock_lyrics_css.return_value = ' FORMAT CSS; ' mock_outline_css.return_value = ' OUTLINE CSS; ' - theme_data = ThemeXML() + theme_data = Theme() theme_data.font_main_name = 'Arial' theme_data.font_main_size = 20 theme_data.font_main_color = '#FFFFFF' diff --git a/tests/functional/openlp_core_lib/test_theme.py b/tests/functional/openlp_core_lib/test_theme.py index 04a31c5d5..28da8e505 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -38,7 +38,7 @@ class TestTheme(TestCase): """ Test the Theme constructor """ - # GIVEN: The ThemeXML class + # GIVEN: The Theme class # WHEN: A theme object is created default_theme = Theme() @@ -49,7 +49,7 @@ class TestTheme(TestCase): """ Test the expand_json method """ - # GIVEN: A ThemeXML object and some JSON to "expand" + # GIVEN: A Theme object and some JSON to "expand" theme = Theme() theme_json = { 'background': { @@ -69,7 +69,7 @@ class TestTheme(TestCase): } } - # WHEN: ThemeXML.expand_json() is run + # WHEN: Theme.expand_json() is run theme.expand_json(theme_json) # THEN: The attributes should be set on the object @@ -86,7 +86,7 @@ class TestTheme(TestCase): theme.background_type = 'video' path = os.path.expanduser('~') - # WHEN: ThemeXML.extend_image_filename is run + # WHEN: Theme.extend_image_filename is run theme.extend_image_filename(path) # THEN: The filename of the background should be correct From cad03b9abd04bcaa94e744d3dfa94432bbfe7595 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Sun, 21 May 2017 17:06:40 +0100 Subject: [PATCH 18/32] fix tests --- openlp/core/lib/theme.py | 264 ------------------ .../functional/openlp_core_lib/test_theme.py | 2 +- .../openlp_core_ui_lib/test_thememanager.py | 37 +++ 3 files changed, 38 insertions(+), 265 deletions(-) create mode 100644 tests/functional/openlp_core_ui_lib/test_thememanager.py diff --git a/openlp/core/lib/theme.py b/openlp/core/lib/theme.py index a4397ea0f..ff803acd8 100644 --- a/openlp/core/lib/theme.py +++ b/openlp/core/lib/theme.py @@ -26,7 +26,6 @@ import os import logging import json -from xml.dom.minidom import Document from lxml import etree, objectify from openlp.core.common import AppLocation, de_hump @@ -195,184 +194,6 @@ class Theme(object): self.background_filename = self.background_filename.strip() self.background_filename = os.path.join(path, self.theme_name, self.background_filename) - # def _new_document(self, name): - # """ - # Create a new theme XML document. - # """ - # self.theme_xml = Document() - # self.theme = self.theme_xml.createElement('theme') - # self.theme_xml.appendChild(self.theme) - # self.theme.setAttribute('version', '2.0') - # self.name = self.theme_xml.createElement('name') - # text_node = self.theme_xml.createTextNode(name) - # self.name.appendChild(text_node) - # self.theme.appendChild(self.name) - - # def add_background_transparent(self): - # """ - # Add a transparent background. - # """ - # background = self.theme_xml.createElement('background') - # background.setAttribute('type', 'transparent') - # self.theme.appendChild(background) - - # def add_background_solid(self, bkcolor): - # """ - # Add a Solid background. - # - # :param bkcolor: The color of the background. - # """ - # background = self.theme_xml.createElement('background') - # background.setAttribute('type', 'solid') - # self.theme.appendChild(background) - # self.child_element(background, 'color', str(bkcolor)) - - # def add_background_gradient(self, startcolor, endcolor, direction): - # """ - # Add a gradient background. - # - # :param startcolor: The gradient's starting colour. - # :param endcolor: The gradient's ending colour. - # :param direction: The direction of the gradient. - # """ - # background = self.theme_xml.createElement('background') - # background.setAttribute('type', 'gradient') - # self.theme.appendChild(background) - # # Create startColor element - # self.child_element(background, 'startColor', str(startcolor)) - # # Create endColor element - # self.child_element(background, 'endColor', str(endcolor)) - # # Create direction element - # self.child_element(background, 'direction', str(direction)) - - # def add_background_image(self, filename, border_color): - # """ - # Add a image background. - # - # :param filename: The file name of the image. - # :param border_color: - # """ - # background = self.theme_xml.createElement('background') - # background.setAttribute('type', 'image') - # self.theme.appendChild(background) - # # Create Filename element - # self.child_element(background, 'filename', filename) - # # Create endColor element - # self.child_element(background, 'borderColor', str(border_color)) - # - # def add_background_video(self, filename, border_color): - # """ - # Add a video background. - # - # :param filename: The file name of the video. - # :param border_color: - # """ - # background = self.theme_xml.createElement('background') - # background.setAttribute('type', 'video') - # self.theme.appendChild(background) - # # Create Filename element - # self.child_element(background, 'filename', filename) - # # Create endColor element - # self.child_element(background, 'borderColor', str(border_color)) - - # def add_font(self, name, color, size, override, fonttype='main', bold='False', italics='False', - # line_adjustment=0, xpos=0, ypos=0, width=0, height=0, outline='False', outline_color='#ffffff', - # outline_pixel=2, shadow='False', shadow_color='#ffffff', shadow_pixel=5): - # """ - # Add a Font. - # - # :param name: The name of the font. - # :param color: The colour of the font. - # :param size: The size of the font. - # :param override: Whether or not to override the default positioning of the theme. - # :param fonttype: The type of font, ``main`` or ``footer``. Defaults to ``main``. - # :param bold: - # :param italics: The weight of then font Defaults to 50 Normal - # :param line_adjustment: Does the font render to italics Defaults to 0 Normal - # :param xpos: The X position of the text block. - # :param ypos: The Y position of the text block. - # :param width: The width of the text block. - # :param height: The height of the text block. - # :param outline: Whether or not to show an outline. - # :param outline_color: The colour of the outline. - # :param outline_pixel: How big the Shadow is - # :param shadow: Whether or not to show a shadow. - # :param shadow_color: The colour of the shadow. - # :param shadow_pixel: How big the Shadow is - # """ - # background = self.theme_xml.createElement('font') - # background.setAttribute('type', fonttype) - # self.theme.appendChild(background) - # # Create Font name element - # self.child_element(background, 'name', name) - # # Create Font color element - # self.child_element(background, 'color', str(color)) - # # Create Proportion name element - # self.child_element(background, 'size', str(size)) - # # Create weight name element - # self.child_element(background, 'bold', str(bold)) - # # Create italics name element - # self.child_element(background, 'italics', str(italics)) - # # Create indentation name element - # self.child_element(background, 'line_adjustment', str(line_adjustment)) - # # Create Location element - # element = self.theme_xml.createElement('location') - # element.setAttribute('override', str(override)) - # element.setAttribute('x', str(xpos)) - # element.setAttribute('y', str(ypos)) - # element.setAttribute('width', str(width)) - # element.setAttribute('height', str(height)) - # background.appendChild(element) - # # Shadow - # element = self.theme_xml.createElement('shadow') - # element.setAttribute('shadowColor', str(shadow_color)) - # element.setAttribute('shadowSize', str(shadow_pixel)) - # value = self.theme_xml.createTextNode(str(shadow)) - # element.appendChild(value) - # background.appendChild(element) - # # Outline - # element = self.theme_xml.createElement('outline') - # element.setAttribute('outlineColor', str(outline_color)) - # element.setAttribute('outlineSize', str(outline_pixel)) - # value = self.theme_xml.createTextNode(str(outline)) - # element.appendChild(value) - # background.appendChild(element) - # - # def add_display(self, horizontal, vertical, transition): - # """ - # Add a Display options. - # - # :param horizontal: The horizontal alignment of the text. - # :param vertical: The vertical alignment of the text. - # :param transition: Whether the slide transition is active. - # """ - # background = self.theme_xml.createElement('display') - # self.theme.appendChild(background) - # # Horizontal alignment - # element = self.theme_xml.createElement('horizontalAlign') - # value = self.theme_xml.createTextNode(str(horizontal)) - # element.appendChild(value) - # background.appendChild(element) - # # Vertical alignment - # element = self.theme_xml.createElement('verticalAlign') - # value = self.theme_xml.createTextNode(str(vertical)) - # element.appendChild(value) - # background.appendChild(element) - # # Slide Transition - # element = self.theme_xml.createElement('slideTransition') - # value = self.theme_xml.createTextNode(str(transition)) - # element.appendChild(value) - # background.appendChild(element) - # - # def child_element(self, element, tag, value): - # """ - # Generic child element creator. - # """ - # child = self.theme_xml.createElement(tag) - # child.appendChild(self.theme_xml.createTextNode(value)) - # element.appendChild(child) - # return child - def set_default_header_footer(self): """ Set the header and footer size into the current primary screen. @@ -386,26 +207,6 @@ class Theme(object): self.font_footer_y = current_screen['size'].height() * 9 / 10 self.font_footer_height = current_screen['size'].height() / 10 - # def dump_xml(self): - # """ - # Dump the XML to file used for debugging - # """ - # return self.theme_xml.toprettyxml(indent=' ') - - # def extract_xml(self): - # """ - # Print out the XML string. - # """ - # self._build_xml_from_attrs() - # return self.theme_xml.toxml('utf-8').decode('utf-8') - # - # def extract_formatted_xml(self): - # """ - # Pull out the XML string formatted for human consumption - # """ - # self._build_xml_from_attrs() - # return self.theme_xml.toprettyxml(indent=' ', newl='\n', encoding='utf-8') - def load_theme(self, theme): """ Pull out the XML string formatted for human consumption @@ -526,68 +327,3 @@ class Theme(object): if key[0:1] != '_': theme_strings.append('{key:>30}: {value}'.format(key=key, value=getattr(self, key))) return '\n'.join(theme_strings) - - # def _build_xml_from_attrs(self): - # """ - # Build the XML from the varables in the object - # """ - # self._new_document(self.theme_name) - # if self.background_type == BackgroundType.to_string(BackgroundType.Solid): - # self.add_background_solid(self.background_color) - # elif self.background_type == BackgroundType.to_string(BackgroundType.Gradient): - # self.add_background_gradient( - # self.background_start_color, - # self.background_end_color, - # self.background_direction - # ) - # elif self.background_type == BackgroundType.to_string(BackgroundType.Image): - # filename = os.path.split(self.background_filename)[1] - # self.add_background_image(filename, self.background_border_color) - # elif self.background_type == BackgroundType.to_string(BackgroundType.Video): - # filename = os.path.split(self.background_filename)[1] - # self.add_background_video(filename, self.background_border_color) - # elif self.background_type == BackgroundType.to_string(BackgroundType.Transparent): - # self.add_background_transparent() - # self.add_font( - # self.font_main_name, - # self.font_main_color, - # self.font_main_size, - # self.font_main_override, 'main', - # self.font_main_bold, - # self.font_main_italics, - # self.font_main_line_adjustment, - # self.font_main_x, - # self.font_main_y, - # self.font_main_width, - # self.font_main_height, - # self.font_main_outline, - # self.font_main_outline_color, - # self.font_main_outline_size, - # self.font_main_shadow, - # self.font_main_shadow_color, - # self.font_main_shadow_size - # ) - # self.add_font( - # self.font_footer_name, - # self.font_footer_color, - # self.font_footer_size, - # self.font_footer_override, 'footer', - # self.font_footer_bold, - # self.font_footer_italics, - # 0, # line adjustment - # self.font_footer_x, - # self.font_footer_y, - # self.font_footer_width, - # self.font_footer_height, - # self.font_footer_outline, - # self.font_footer_outline_color, - # self.font_footer_outline_size, - # self.font_footer_shadow, - # self.font_footer_shadow_color, - # self.font_footer_shadow_size - # ) - # self.add_display( - # self.display_horizontal_align, - # self.display_vertical_align, - # self.display_slide_transition - # ) diff --git a/tests/functional/openlp_core_lib/test_theme.py b/tests/functional/openlp_core_lib/test_theme.py index 28da8e505..85692f0a2 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -32,7 +32,7 @@ from openlp.core.lib.theme import Theme class TestTheme(TestCase): """ - Test the ThemeL class + Test the Theme class """ def test_new_theme(self): """ diff --git a/tests/functional/openlp_core_ui_lib/test_thememanager.py b/tests/functional/openlp_core_ui_lib/test_thememanager.py new file mode 100644 index 000000000..51face165 --- /dev/null +++ b/tests/functional/openlp_core_ui_lib/test_thememanager.py @@ -0,0 +1,37 @@ +# -*- coding: utf-8 -*- +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 + +############################################################################### +# OpenLP - Open Source Lyrics Projection # +# --------------------------------------------------------------------------- # +# Copyright (c) 2008-2017 OpenLP Developers # +# --------------------------------------------------------------------------- # +# This program is free software; you can redistribute it and/or modify it # +# under the terms of the GNU General Public License as published by the Free # +# Software Foundation; version 2 of the License. # +# # +# This program is distributed in the hope that it will be useful, but WITHOUT # +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # +# more details. # +# # +# You should have received a copy of the GNU General Public License along # +# with this program; if not, write to the Free Software Foundation, Inc., 59 # +# Temple Place, Suite 330, Boston, MA 02111-1307 USA # +############################################################################### +""" +Package to test the openlp.core.ui.lib.theme package. +""" +import json +from unittest import TestCase +import os + +from openlp.core.common import json_default +from openlp.core.lib.theme import Theme + + +class TestThemeManager(TestCase): + """ + Test the ThemeManager class + """ + From b7001116da284557c2a451a29efb9620b90fea30 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Tue, 23 May 2017 05:59:35 +0100 Subject: [PATCH 19/32] start to fix tests --- openlp/core/common/__init__.py | 9 ++++++--- openlp/core/ui/thememanager.py | 10 ++++++---- tests/functional/openlp_core_ui/test_thememanager.py | 8 ++++---- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index ac61d9519..8b4a223eb 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -440,11 +440,14 @@ def get_file_encoding(filename): log.exception('Error detecting file encoding') -def json_default(o): +def json_default(obj): """ Function to help save objects as JSON - :param o: object + :param obj: object :return: the object dictionary """ - return o.__dict__ + try: + return obj.__dict__ + except: + raise TypeError("Unserializable object {} of type {}".format(obj, type(obj))) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index c8b2eaa38..3b2c19222 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -379,10 +379,12 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage critical_error_message_box(message=translate('OpenLP.ThemeManager', 'You have not selected a theme.')) return theme = item.data(QtCore.Qt.UserRole) - path, filter_used = QtWidgets.QFileDialog.getSaveFileName(self.main_window, - translate('OpenLP.ThemeManager', 'Save Theme - ({name})').format(name=theme), - Settings().value(self.settings_section + '/last directory export'), - translate('OpenLP.ThemeManager', 'OpenLP Themes (*.otz)')) + path, filter_used = \ + QtWidgets.QFileDialog.getSaveFileName(self.main_window, + translate('OpenLP.ThemeManager', 'Save Theme - ({name})'). + format(name=theme), + Settings().value(self.settings_section + '/last directory export'), + translate('OpenLP.ThemeManager', 'OpenLP Themes (*.otz)')) self.application.set_busy_cursor() if path: Settings().setValue(self.settings_section + '/last directory export', path) diff --git a/tests/functional/openlp_core_ui/test_thememanager.py b/tests/functional/openlp_core_ui/test_thememanager.py index 264a9f3ce..c9254e4d0 100644 --- a/tests/functional/openlp_core_ui/test_thememanager.py +++ b/tests/functional/openlp_core_ui/test_thememanager.py @@ -26,7 +26,6 @@ import os import shutil from unittest import TestCase -from tempfile import mkdtemp from PyQt5 import QtWidgets from tempfile import mkdtemp @@ -65,7 +64,7 @@ class TestThemeManager(TestCase): mocked_zipfile_init.return_value = None # WHEN: The theme is exported - theme_manager._export_theme(os.path.join('some', 'path'), 'Default') + theme_manager._export_theme(os.path.join('some', 'path', 'Default.otz'), 'Default') # THEN: The zipfile should be created at the given path mocked_zipfile_init.assert_called_with(os.path.join('some', 'path', 'Default.otz'), 'w') @@ -128,8 +127,9 @@ class TestThemeManager(TestCase): theme_manager.path = '' mocked_theme = MagicMock() mocked_theme.theme_name = 'themename' - mocked_theme.extract_formatted_xml = MagicMock() - mocked_theme.extract_formatted_xml.return_value = 'fake_theme_xml'.encode() + mocked_theme.filename = "filename" + # mocked_theme.extract_formatted_xml = MagicMock() + # mocked_theme.extract_formatted_xml.return_value = 'fake_theme_xml'.encode() # WHEN: Calling _write_theme with path to different images file_name1 = os.path.join(TEST_RESOURCES_PATH, 'church.jpg') From 32668193c642872152535e3d568097f6f1fea63d Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Wed, 24 May 2017 20:31:48 +0100 Subject: [PATCH 20/32] fix up tests --- openlp/core/lib/theme.py | 12 +++++++++++- openlp/core/ui/thememanager.py | 2 +- tests/functional/openlp_core_lib/test_theme.py | 2 +- tests/functional/openlp_core_ui/test_thememanager.py | 5 ++--- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/openlp/core/lib/theme.py b/openlp/core/lib/theme.py index 808086fd8..541cd8fab 100644 --- a/openlp/core/lib/theme.py +++ b/openlp/core/lib/theme.py @@ -209,13 +209,23 @@ class Theme(object): def load_theme(self, theme): """ - Pull out the XML string formatted for human consumption + Convert the JSON file and expand it. :param theme: the theme string """ jsn = json.loads(theme) self.expand_json(jsn) + def export_theme(self): + """ + Loop through the fields and build a dictionary of them + + """ + theme_data = {} + for attr, value in self.__dict__.items(): + theme_data["{attr}".format(attr=attr)] = value + return json.dumps(theme_data) + def parse(self, xml): """ Read in an XML string and parse it. diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 3b2c19222..3fb2da1c9 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -666,7 +666,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage :param image_to: Where the Theme Image is to be saved to """ name = theme.theme_name - theme_pretty = json.dumps(theme, default=json_default) + theme_pretty = theme.export_theme() theme_dir = os.path.join(self.path, name) check_directory_exists(theme_dir) theme_file = os.path.join(theme_dir, name + '.json') diff --git a/tests/functional/openlp_core_lib/test_theme.py b/tests/functional/openlp_core_lib/test_theme.py index 85692f0a2..db5291d23 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -102,7 +102,7 @@ class TestTheme(TestCase): # WHEN: A theme object is created default_theme = Theme() # THEN: The default values should be correct - save_theme_json = json.dumps(default_theme, default=json_default) + save_theme_json = default_theme.export_theme() lt = Theme() lt.load_theme(save_theme_json) self.check_theme(lt) diff --git a/tests/functional/openlp_core_ui/test_thememanager.py b/tests/functional/openlp_core_ui/test_thememanager.py index 393d60ecd..a30eda7fa 100644 --- a/tests/functional/openlp_core_ui/test_thememanager.py +++ b/tests/functional/openlp_core_ui/test_thememanager.py @@ -149,14 +149,13 @@ class TestThemeManager(TestCase): theme_manager.path = self.temp_folder mocked_theme = MagicMock() mocked_theme.theme_name = 'theme 愛 name' - mocked_theme.extract_formatted_xml = MagicMock() - mocked_theme.extract_formatted_xml.return_value = 'fake theme 愛 XML'.encode() + mocked_theme.export_theme.return_value = "{}" # WHEN: Calling _write_theme with a theme with a name with special characters in it theme_manager._write_theme(mocked_theme, None, None) # THEN: It should have been created - self.assertTrue(os.path.exists(os.path.join(self.temp_folder, 'theme 愛 name', 'theme 愛 name.xml')), + self.assertTrue(os.path.exists(os.path.join(self.temp_folder, 'theme 愛 name', 'theme 愛 name.json')), 'Theme with special characters should have been created!') def test_over_write_message_box_yes(self): From 3b7852b569a05f3cae44249aaeb5ce00bc47d114 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Wed, 24 May 2017 20:55:30 +0100 Subject: [PATCH 21/32] clean up pep8 --- openlp/core/lib/theme.py | 2 +- openlp/core/ui/lib/pathedit.py | 6 +++--- openlp/plugins/presentations/presentationplugin.py | 2 +- openlp/plugins/songusage/forms/songusagedetaildialog.py | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/openlp/core/lib/theme.py b/openlp/core/lib/theme.py index 541cd8fab..fb78f7443 100644 --- a/openlp/core/lib/theme.py +++ b/openlp/core/lib/theme.py @@ -210,7 +210,7 @@ class Theme(object): def load_theme(self, theme): """ Convert the JSON file and expand it. - + :param theme: the theme string """ jsn = json.loads(theme) diff --git a/openlp/core/ui/lib/pathedit.py b/openlp/core/ui/lib/pathedit.py index 238bcb00d..74f37c581 100755 --- a/openlp/core/ui/lib/pathedit.py +++ b/openlp/core/ui/lib/pathedit.py @@ -46,16 +46,16 @@ class PathEdit(QtWidgets.QWidget): :param parent: The parent of the widget. This is just passed to the super method. :type parent: QWidget or None - + :param dialog_caption: Used to customise the caption in the QFileDialog. :param dialog_caption: str - + :param default_path: The default path. This is set as the path when the revert button is clicked :type default_path: str :param show_revert: Used to determin if the 'revert button' should be visible. :type show_revert: bool - + :return: None :rtype: None """ diff --git a/openlp/plugins/presentations/presentationplugin.py b/openlp/plugins/presentations/presentationplugin.py index 884f155a2..210f8a531 100644 --- a/openlp/plugins/presentations/presentationplugin.py +++ b/openlp/plugins/presentations/presentationplugin.py @@ -1,4 +1,4 @@ - # -*- coding: utf-8 -*- +# -*- coding: utf-8 -*- # vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 ############################################################################### diff --git a/openlp/plugins/songusage/forms/songusagedetaildialog.py b/openlp/plugins/songusage/forms/songusagedetaildialog.py index 74c8c89c8..082173bf5 100644 --- a/openlp/plugins/songusage/forms/songusagedetaildialog.py +++ b/openlp/plugins/songusage/forms/songusagedetaildialog.py @@ -69,7 +69,7 @@ class Ui_SongUsageDetailDialog(object): self.file_horizontal_layout.setSpacing(8) self.file_horizontal_layout.setContentsMargins(8, 8, 8, 8) self.file_horizontal_layout.setObjectName('file_horizontal_layout') - self.report_path_edit = PathEdit(self.file_group_box, path_type = PathType.Directories, show_revert=False) + self.report_path_edit = PathEdit(self.file_group_box, path_type=PathType.Directories, show_revert=False) self.file_horizontal_layout.addWidget(self.report_path_edit) self.vertical_layout.addWidget(self.file_group_box) self.button_box = create_button_box(song_usage_detail_dialog, 'button_box', ['cancel', 'ok']) From 7c1fffdf41d0c037ad96e1e3574869258994014b Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Wed, 24 May 2017 21:01:46 +0100 Subject: [PATCH 22/32] clean up pep8 --- openlp/core/common/__init__.py | 13 ------------- openlp/core/ui/lib/pathedit.py | 2 +- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/openlp/core/common/__init__.py b/openlp/core/common/__init__.py index aab74d004..fde02506d 100644 --- a/openlp/core/common/__init__.py +++ b/openlp/core/common/__init__.py @@ -482,16 +482,3 @@ def get_file_encoding(filename): return detector.result except OSError: log.exception('Error detecting file encoding') - - -def json_default(obj): - """ - Function to help save objects as JSON - - :param obj: object - :return: the object dictionary - """ - try: - return obj.__dict__ - except: - raise TypeError("Unserializable object {} of type {}".format(obj, type(obj))) diff --git a/openlp/core/ui/lib/pathedit.py b/openlp/core/ui/lib/pathedit.py index 74f37c581..c489daa33 100755 --- a/openlp/core/ui/lib/pathedit.py +++ b/openlp/core/ui/lib/pathedit.py @@ -72,7 +72,7 @@ class PathEdit(QtWidgets.QWidget): Set up the widget :param show_revert: Show or hide the revert button :type show_revert: bool - + :return: None :rtype: None """ From 2faf588c99e2d0ccf72d1fda1e30cc8d63652ba0 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Wed, 24 May 2017 21:04:48 +0100 Subject: [PATCH 23/32] clean up files --- openlp/core/ui/thememanager.py | 2 +- tests/functional/openlp_core_lib/test_theme.py | 2 -- tests/functional/openlp_core_ui_lib/test_thememanager.py | 6 +----- 3 files changed, 2 insertions(+), 8 deletions(-) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 3fb2da1c9..73056568f 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -31,7 +31,7 @@ from xml.etree.ElementTree import ElementTree, XML from PyQt5 import QtCore, QtGui, QtWidgets from openlp.core.common import Registry, RegistryProperties, AppLocation, Settings, OpenLPMixin, RegistryMixin, \ - check_directory_exists, UiStrings, translate, is_win, get_filesystem_encoding, delete_file, json_default + check_directory_exists, UiStrings, translate, is_win, get_filesystem_encoding, delete_file from openlp.core.lib import FileDialog, ImageSource, ValidationError, get_text_file_string, build_icon, \ check_item_selected, create_thumb, validate_thumb from openlp.core.lib.theme import Theme, BackgroundType diff --git a/tests/functional/openlp_core_lib/test_theme.py b/tests/functional/openlp_core_lib/test_theme.py index db5291d23..bb90e574a 100644 --- a/tests/functional/openlp_core_lib/test_theme.py +++ b/tests/functional/openlp_core_lib/test_theme.py @@ -22,11 +22,9 @@ """ Package to test the openlp.core.lib.theme package. """ -import json from unittest import TestCase import os -from openlp.core.common import json_default from openlp.core.lib.theme import Theme diff --git a/tests/functional/openlp_core_ui_lib/test_thememanager.py b/tests/functional/openlp_core_ui_lib/test_thememanager.py index 51face165..d8924fe58 100644 --- a/tests/functional/openlp_core_ui_lib/test_thememanager.py +++ b/tests/functional/openlp_core_ui_lib/test_thememanager.py @@ -22,12 +22,8 @@ """ Package to test the openlp.core.ui.lib.theme package. """ -import json -from unittest import TestCase -import os -from openlp.core.common import json_default -from openlp.core.lib.theme import Theme +from unittest import TestCase class TestThemeManager(TestCase): From a775f0ddb8881a46d7c17a67b7898a3efec50753 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Wed, 24 May 2017 21:09:54 +0100 Subject: [PATCH 24/32] remove extra file --- .../openlp_core_ui_lib/test_thememanager.py | 33 ------------------- 1 file changed, 33 deletions(-) delete mode 100644 tests/functional/openlp_core_ui_lib/test_thememanager.py diff --git a/tests/functional/openlp_core_ui_lib/test_thememanager.py b/tests/functional/openlp_core_ui_lib/test_thememanager.py deleted file mode 100644 index d8924fe58..000000000 --- a/tests/functional/openlp_core_ui_lib/test_thememanager.py +++ /dev/null @@ -1,33 +0,0 @@ -# -*- coding: utf-8 -*- -# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 - -############################################################################### -# OpenLP - Open Source Lyrics Projection # -# --------------------------------------------------------------------------- # -# Copyright (c) 2008-2017 OpenLP Developers # -# --------------------------------------------------------------------------- # -# This program is free software; you can redistribute it and/or modify it # -# under the terms of the GNU General Public License as published by the Free # -# Software Foundation; version 2 of the License. # -# # -# This program is distributed in the hope that it will be useful, but WITHOUT # -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or # -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for # -# more details. # -# # -# You should have received a copy of the GNU General Public License along # -# with this program; if not, write to the Free Software Foundation, Inc., 59 # -# Temple Place, Suite 330, Boston, MA 02111-1307 USA # -############################################################################### -""" -Package to test the openlp.core.ui.lib.theme package. -""" - -from unittest import TestCase - - -class TestThemeManager(TestCase): - """ - Test the ThemeManager class - """ - From 48ca33092ab6bd9ed62f7aadd54e439d771e0576 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Fri, 26 May 2017 14:44:28 +0100 Subject: [PATCH 25/32] PEP --- openlp/core/ui/lib/pathedit.py | 8 ++++---- openlp/plugins/presentations/presentationplugin.py | 2 +- openlp/plugins/songusage/forms/songusagedetaildialog.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/openlp/core/ui/lib/pathedit.py b/openlp/core/ui/lib/pathedit.py index 238bcb00d..c489daa33 100755 --- a/openlp/core/ui/lib/pathedit.py +++ b/openlp/core/ui/lib/pathedit.py @@ -46,16 +46,16 @@ class PathEdit(QtWidgets.QWidget): :param parent: The parent of the widget. This is just passed to the super method. :type parent: QWidget or None - + :param dialog_caption: Used to customise the caption in the QFileDialog. :param dialog_caption: str - + :param default_path: The default path. This is set as the path when the revert button is clicked :type default_path: str :param show_revert: Used to determin if the 'revert button' should be visible. :type show_revert: bool - + :return: None :rtype: None """ @@ -72,7 +72,7 @@ class PathEdit(QtWidgets.QWidget): Set up the widget :param show_revert: Show or hide the revert button :type show_revert: bool - + :return: None :rtype: None """ diff --git a/openlp/plugins/presentations/presentationplugin.py b/openlp/plugins/presentations/presentationplugin.py index 884f155a2..210f8a531 100644 --- a/openlp/plugins/presentations/presentationplugin.py +++ b/openlp/plugins/presentations/presentationplugin.py @@ -1,4 +1,4 @@ - # -*- coding: utf-8 -*- +# -*- coding: utf-8 -*- # vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 ############################################################################### diff --git a/openlp/plugins/songusage/forms/songusagedetaildialog.py b/openlp/plugins/songusage/forms/songusagedetaildialog.py index 74c8c89c8..082173bf5 100644 --- a/openlp/plugins/songusage/forms/songusagedetaildialog.py +++ b/openlp/plugins/songusage/forms/songusagedetaildialog.py @@ -69,7 +69,7 @@ class Ui_SongUsageDetailDialog(object): self.file_horizontal_layout.setSpacing(8) self.file_horizontal_layout.setContentsMargins(8, 8, 8, 8) self.file_horizontal_layout.setObjectName('file_horizontal_layout') - self.report_path_edit = PathEdit(self.file_group_box, path_type = PathType.Directories, show_revert=False) + self.report_path_edit = PathEdit(self.file_group_box, path_type=PathType.Directories, show_revert=False) self.file_horizontal_layout.addWidget(self.report_path_edit) self.vertical_layout.addWidget(self.file_group_box) self.button_box = create_button_box(song_usage_detail_dialog, 'button_box', ['cancel', 'ok']) From fc5c6561e766debdad2bc8ea57d883200e817de0 Mon Sep 17 00:00:00 2001 From: Tim Bentley Date: Tue, 30 May 2017 14:55:39 +0100 Subject: [PATCH 26/32] Minor tweeks --- openlp/core/ui/thememanager.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/openlp/core/ui/thememanager.py b/openlp/core/ui/thememanager.py index 73056568f..59576808b 100644 --- a/openlp/core/ui/thememanager.py +++ b/openlp/core/ui/thememanager.py @@ -506,7 +506,7 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage def get_theme_data(self, theme_name): """ - Returns a theme object from an XML file + Returns a theme object from an XML or JSON file :param theme_name: Name of the theme to load from file :return: The theme object. @@ -556,11 +556,13 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage out_file = None file_xml = None abort_import = True + json_theme = False + theme_name = "" try: theme_zip = zipfile.ZipFile(file_name) - json_theme = False json_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.json'] if len(json_file) != 1: + # TODO: remove XML handling at some point but would need a auto conversion to run first. xml_file = [name for name in theme_zip.namelist() if os.path.splitext(name)[1].lower() == '.xml'] if len(xml_file) != 1: self.log_error('Theme contains "{val:d}" theme files'.format(val=len(xml_file))) @@ -572,9 +574,9 @@ class ThemeManager(OpenLPMixin, RegistryMixin, QtWidgets.QWidget, Ui_ThemeManage raise ValidationError theme_name = xml_tree.find('name').text.strip() else: - theme = Theme() - theme.load_theme(theme_zip.read(json_file[0]).decode("utf-8")) - theme_name = theme.theme_name + new_theme = Theme() + new_theme.load_theme(theme_zip.read(json_file[0]).decode("utf-8")) + theme_name = new_theme.theme_name json_theme = True theme_folder = os.path.join(directory, theme_name) theme_exists = os.path.exists(theme_folder) From 4a3e4e1ad459fa48812a1971821d250f0eb3bf3e Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 30 May 2017 22:05:18 +0200 Subject: [PATCH 27/32] Fix songshowplus encoding issue. --- openlp/plugins/songs/lib/importers/songshowplus.py | 6 +++--- .../songshowplussongs/a mighty fortress is our god.json | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/openlp/plugins/songs/lib/importers/songshowplus.py b/openlp/plugins/songs/lib/importers/songshowplus.py index edb16c74e..4848349f0 100644 --- a/openlp/plugins/songs/lib/importers/songshowplus.py +++ b/openlp/plugins/songs/lib/importers/songshowplus.py @@ -23,7 +23,6 @@ The :mod:`songshowplus` module provides the functionality for importing SongShow Plus songs into the OpenLP database. """ -import chardet import os import logging import re @@ -226,6 +225,7 @@ class SongShowPlusImport(SongImport): def decode(self, data): try: - return str(data, chardet.detect(data)['encoding']) + # Don't question this, it works... + return data.decode('utf-8').encode('cp1251').decode('cp1251') except: - return str(data, retrieve_windows_encoding()) + return data.decode(retrieve_windows_encoding()) diff --git a/tests/resources/songshowplussongs/a mighty fortress is our god.json b/tests/resources/songshowplussongs/a mighty fortress is our god.json index 2788ad05c..96453c0df 100644 --- a/tests/resources/songshowplussongs/a mighty fortress is our god.json +++ b/tests/resources/songshowplussongs/a mighty fortress is our god.json @@ -15,7 +15,7 @@ "v1" ], [ - "Did we in our own strength confide, our striving would be losing;\r\nWere not the right Man on our side, the Man of God’s own choosing:\r\nDost ask who that may be? Christ Jesus, it is He;\r\nLord Sabaoth, His Name, from age to age the same,\r\nAnd He must win the battle.\r\n", + "Did we in our own strength confide, our striving would be losing;\r\nWere not the right Man on our side, the Man of God’s own choosing:\r\nDost ask who that may be? Christ Jesus, it is He;\r\nLord Sabaoth, His Name, from age to age the same,\r\nAnd He must win the battle.\r\n", "v2" ], [ @@ -23,7 +23,7 @@ "v3" ], [ - "That word above all earthly powers, no thanks to them, abideth;\r\nThe Spirit and the gifts are ours through Him Who with us sideth:\r\nLet goods and kindred go, this mortal life also;\r\nThe body they may kill: God’s truth abideth still,\r\nHis kingdom is forever.\r\n", + "That word above all earthly powers, no thanks to them, abideth;\r\nThe Spirit and the gifts are ours through Him Who with us sideth:\r\nLet goods and kindred go, this mortal life also;\r\nThe body they may kill: God’s truth abideth still,\r\nHis kingdom is forever.\r\n", "v4" ] ] From ad55a8d2541b483b910d86513fffd294d5371907 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 30 May 2017 22:06:27 +0200 Subject: [PATCH 28/32] Fix issue where enable-chord settings was not setup correctly for tests. --- .../openlp_plugins/songs/test_editverseform.py | 8 +++++++- .../openlp_plugins/songs/forms/test_editsongform.py | 9 ++++++++- .../openlp_plugins/songs/forms/test_editverseform.py | 9 ++++++++- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/tests/functional/openlp_plugins/songs/test_editverseform.py b/tests/functional/openlp_plugins/songs/test_editverseform.py index 0cb08e827..3e97abbb7 100644 --- a/tests/functional/openlp_plugins/songs/test_editverseform.py +++ b/tests/functional/openlp_plugins/songs/test_editverseform.py @@ -27,10 +27,15 @@ from unittest.mock import MagicMock from PyQt5 import QtCore +from openlp.core.common import Settings from openlp.plugins.songs.forms.editverseform import EditVerseForm from tests.helpers.testmixin import TestMixin +__default_settings__ = { + 'songs/enable chords': True, +} + class TestEditVerseForm(TestCase, TestMixin): """ @@ -40,9 +45,10 @@ class TestEditVerseForm(TestCase, TestMixin): """ Set up the components need for all tests. """ - self.edit_verse_form = EditVerseForm(None) self.setup_application() self.build_settings() + Settings().extend_default_settings(__default_settings__) + self.edit_verse_form = EditVerseForm(None) QtCore.QLocale.setDefault(QtCore.QLocale('en_GB')) def tearDown(self): diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py index b82bb29ae..e7d8308fa 100644 --- a/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py +++ b/tests/interfaces/openlp_plugins/songs/forms/test_editsongform.py @@ -27,12 +27,16 @@ from unittest.mock import MagicMock from PyQt5 import QtWidgets -from openlp.core.common import Registry +from openlp.core.common import Registry, Settings from openlp.core.common.uistrings import UiStrings from openlp.plugins.songs.forms.editsongform import EditSongForm from tests.helpers.testmixin import TestMixin +__default_settings__ = { + 'songs/enable chords': True, +} + class TestEditSongForm(TestCase, TestMixin): """ @@ -48,12 +52,15 @@ class TestEditSongForm(TestCase, TestMixin): self.main_window = QtWidgets.QMainWindow() Registry().register('main_window', self.main_window) Registry().register('theme_manager', MagicMock()) + self.build_settings() + Settings().extend_default_settings(__default_settings__) self.form = EditSongForm(MagicMock(), self.main_window, MagicMock()) def tearDown(self): """ Delete all the C++ objects at the end so that we don't have a segfault """ + self.destroy_settings() del self.form del self.main_window diff --git a/tests/interfaces/openlp_plugins/songs/forms/test_editverseform.py b/tests/interfaces/openlp_plugins/songs/forms/test_editverseform.py index a28be8df2..78c5c3016 100644 --- a/tests/interfaces/openlp_plugins/songs/forms/test_editverseform.py +++ b/tests/interfaces/openlp_plugins/songs/forms/test_editverseform.py @@ -26,10 +26,14 @@ from unittest import TestCase from PyQt5 import QtCore, QtTest, QtWidgets -from openlp.core.common import Registry +from openlp.core.common import Registry, Settings from openlp.plugins.songs.forms.editverseform import EditVerseForm from tests.helpers.testmixin import TestMixin +__default_settings__ = { + 'songs/enable chords': True, +} + class TestEditVerseForm(TestCase, TestMixin): """ @@ -44,12 +48,15 @@ class TestEditVerseForm(TestCase, TestMixin): self.setup_application() self.main_window = QtWidgets.QMainWindow() Registry().register('main_window', self.main_window) + self.build_settings() + Settings().extend_default_settings(__default_settings__) self.form = EditVerseForm() def tearDown(self): """ Delete all the C++ objects at the end so that we don't have a segfault """ + self.destroy_settings() del self.form del self.main_window From 1dc0b0a4b56913bb2ff97f1f432a0ae6249e5954 Mon Sep 17 00:00:00 2001 From: Tomas Groth Date: Tue, 30 May 2017 22:15:20 +0200 Subject: [PATCH 29/32] pep8 fixes --- openlp/core/ui/lib/pathedit.py | 8 ++++---- openlp/plugins/presentations/presentationplugin.py | 2 +- openlp/plugins/songusage/forms/songusagedetaildialog.py | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/openlp/core/ui/lib/pathedit.py b/openlp/core/ui/lib/pathedit.py index 238bcb00d..c489daa33 100755 --- a/openlp/core/ui/lib/pathedit.py +++ b/openlp/core/ui/lib/pathedit.py @@ -46,16 +46,16 @@ class PathEdit(QtWidgets.QWidget): :param parent: The parent of the widget. This is just passed to the super method. :type parent: QWidget or None - + :param dialog_caption: Used to customise the caption in the QFileDialog. :param dialog_caption: str - + :param default_path: The default path. This is set as the path when the revert button is clicked :type default_path: str :param show_revert: Used to determin if the 'revert button' should be visible. :type show_revert: bool - + :return: None :rtype: None """ @@ -72,7 +72,7 @@ class PathEdit(QtWidgets.QWidget): Set up the widget :param show_revert: Show or hide the revert button :type show_revert: bool - + :return: None :rtype: None """ diff --git a/openlp/plugins/presentations/presentationplugin.py b/openlp/plugins/presentations/presentationplugin.py index 884f155a2..210f8a531 100644 --- a/openlp/plugins/presentations/presentationplugin.py +++ b/openlp/plugins/presentations/presentationplugin.py @@ -1,4 +1,4 @@ - # -*- coding: utf-8 -*- +# -*- coding: utf-8 -*- # vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4 ############################################################################### diff --git a/openlp/plugins/songusage/forms/songusagedetaildialog.py b/openlp/plugins/songusage/forms/songusagedetaildialog.py index 74c8c89c8..082173bf5 100644 --- a/openlp/plugins/songusage/forms/songusagedetaildialog.py +++ b/openlp/plugins/songusage/forms/songusagedetaildialog.py @@ -69,7 +69,7 @@ class Ui_SongUsageDetailDialog(object): self.file_horizontal_layout.setSpacing(8) self.file_horizontal_layout.setContentsMargins(8, 8, 8, 8) self.file_horizontal_layout.setObjectName('file_horizontal_layout') - self.report_path_edit = PathEdit(self.file_group_box, path_type = PathType.Directories, show_revert=False) + self.report_path_edit = PathEdit(self.file_group_box, path_type=PathType.Directories, show_revert=False) self.file_horizontal_layout.addWidget(self.report_path_edit) self.vertical_layout.addWidget(self.file_group_box) self.button_box = create_button_box(song_usage_detail_dialog, 'button_box', ['cancel', 'ok']) From 34f8f11efddd0a44d05ebf8a6372a08fefe43f64 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 3 Jun 2017 23:34:19 +0100 Subject: [PATCH 30/32] Bible tests fixed --- .../openlp_plugins/bibles/test_mediaitem.py | 103 +++++++++++------- 1 file changed, 64 insertions(+), 39 deletions(-) diff --git a/tests/functional/openlp_plugins/bibles/test_mediaitem.py b/tests/functional/openlp_plugins/bibles/test_mediaitem.py index 7ed904145..f9c5f64d0 100755 --- a/tests/functional/openlp_plugins/bibles/test_mediaitem.py +++ b/tests/functional/openlp_plugins/bibles/test_mediaitem.py @@ -31,7 +31,7 @@ from tests.helpers.testmixin import TestMixin from openlp.core.common import Registry from openlp.core.lib import MediaManagerItem -from openlp.plugins.bibles.lib.mediaitem import BibleMediaItem, BibleSearch, ResultsTab, SearchStatus, \ +from openlp.plugins.bibles.lib.mediaitem import BibleMediaItem, BibleSearch, ResultsTab, SearchStatus, SearchTabs, \ get_reference_separators, VALID_TEXT_SEARCH @@ -206,29 +206,63 @@ class TestMediaItem(TestCase, TestMixin): """ Test the correct widget gets focus when the BibleMediaItem receives focus """ - # GIVEN: An instance of :class:`MediaManagerItem` and a mocked out search_tab and search_edit + # GIVEN: An instance of :class:`MediaManagerItem` and mocked out tabs and primary widgets self.media_item.search_tab = MagicMock(**{'isVisible.return_value': True}) self.media_item.search_edit = MagicMock() + self.media_item.select_tab = MagicMock(**{'isVisible.return_value': False}) + self.media_item.select_book_combo_box = MagicMock() + self.media_item.options_tab = MagicMock(**{'isVisible.return_value': False}) + self.media_item.version_combo_box = MagicMock() # WHEN: Calling on_focus self.media_item.on_focus() - # THEN: setFocus and selectAll should have been called on search_edit - self.assertEqual(self.media_item.search_edit.mock_calls, [call.setFocus(), call.selectAll()]) + # THEN: search_edit should now have focus and its text selected + self.media_item.search_edit.assert_has_calls([call.setFocus(), call.selectAll()]) + self.media_item.select_book_combo_box.assert_not_called() + self.media_item.version_combo_box.setFocus.assert_not_called() - def test_on_focus_search_tab_not_visible(self): + def test_on_focus_select_tab_visible(self): """ Test the correct widget gets focus when the BibleMediaItem receives focus """ - # GIVEN: An instance of :class:`MediaManagerItem` and a mocked out search_tab and select_book_combo_box + # GIVEN: An instance of :class:`MediaManagerItem` and mocked out tabs and primary widgets self.media_item.search_tab = MagicMock(**{'isVisible.return_value': False}) + self.media_item.search_edit = MagicMock() + self.media_item.select_tab = MagicMock(**{'isVisible.return_value': True}) self.media_item.select_book_combo_box = MagicMock() + self.media_item.options_tab = MagicMock(**{'isVisible.return_value': False}) + self.media_item.version_combo_box = MagicMock() # WHEN: Calling on_focus self.media_item.on_focus() - # THEN: setFocus should have been called on select_book_combo_box - self.assertTrue(self.media_item.select_book_combo_box.setFocus.called) + # THEN: select_book_combo_box should have focus + self.media_item.search_edit.setFocus.assert_not_called() + self.media_item.search_edit.selectAll.assert_not_called() + self.media_item.select_book_combo_box.setFocus.assert_called_once_with() + self.media_item.version_combo_box.setFocus.assert_not_called() + + def test_on_focus_options_tab_visible(self): + """ + Test the correct widget gets focus when the BibleMediaItem receives focus + """ + # GIVEN: An instance of :class:`MediaManagerItem` and mocked out tabs and primary widgets + self.media_item.search_tab = MagicMock(**{'isVisible.return_value': False}) + self.media_item.search_edit = MagicMock() + self.media_item.select_tab = MagicMock(**{'isVisible.return_value': False}) + self.media_item.select_book_combo_box = MagicMock() + self.media_item.options_tab = MagicMock(**{'isVisible.return_value': True}) + self.media_item.version_combo_box = MagicMock() + + # WHEN: Calling on_focus + self.media_item.on_focus() + + # THEN: version_combo_box have received focus + self.media_item.search_edit.setFocus.assert_not_called() + self.media_item.search_edit.selectAll.assert_not_called() + self.media_item.select_book_combo_box.setFocus.assert_not_called() + self.media_item.version_combo_box.setFocus.assert_called_once_with() def test_config_update_show_second_bible(self): """ @@ -611,12 +645,15 @@ class TestMediaItem(TestCase, TestMixin): # GIVEN: An instance of :class:`MediaManagerItem` and mocked out search_tab and select_tab self.media_item.search_tab = MagicMock() self.media_item.select_tab = MagicMock() + self.media_item.options_tab = MagicMock() + self.media_item.search_button = MagicMock() with patch.object(self.media_item, 'on_focus'): # WHEN: The search_tab has been selected - self.media_item.on_search_tab_bar_current_changed(0) + self.media_item.on_search_tab_bar_current_changed(SearchTabs.Search) - # THEN: search_tab should be setVisible and select_tab should be hidder + # THEN: The search_button should be enabled, search_tab should be setVisible and select_tab should be hidden + self.media_item.search_button.setEnabled.assert_called_once_with(True) self.media_item.search_tab.setVisible.assert_called_once_with(True) self.media_item.select_tab.setVisible.assert_called_once_with(False) @@ -627,12 +664,15 @@ class TestMediaItem(TestCase, TestMixin): # GIVEN: An instance of :class:`MediaManagerItem` and mocked out search_tab and select_tab self.media_item.search_tab = MagicMock() self.media_item.select_tab = MagicMock() + self.media_item.options_tab = MagicMock() + self.media_item.search_button = MagicMock() with patch.object(self.media_item, 'on_focus'): # WHEN: The select_tab has been selected - self.media_item.on_search_tab_bar_current_changed(1) + self.media_item.on_search_tab_bar_current_changed(SearchTabs.Select) - # THEN: search_tab should be setVisible and select_tab should be hidder + # THEN: The search_button should be enabled, select_tab should be setVisible and search_tab should be hidden + self.media_item.search_button.setEnabled.assert_called_once_with(True) self.media_item.search_tab.setVisible.assert_called_once_with(False) self.media_item.select_tab.setVisible.assert_called_once_with(True) @@ -660,44 +700,23 @@ class TestMediaItem(TestCase, TestMixin): # THEN: The select_book_combo_box model sort should have been reset self.media_item.select_book_combo_box.model().sort.assert_called_once_with(-1) - def test_on_clear_button_clicked_saved_tab(self): - """ - Test on_clear_button_clicked when the saved tab is selected - """ - # GIVEN: An instance of :class:`MediaManagerItem` and mocked out saved_tab and select_tab and a mocked out - # list_view and search_edit - self.media_item.list_view = MagicMock() - self.media_item.search_edit = MagicMock() - self.media_item.results_view_tab = MagicMock(**{'currentIndex.return_value': ResultsTab.Saved}) - self.media_item.saved_results = ['Some', 'Results'] - with patch.object(self.media_item, 'on_focus'): - - # WHEN: Calling on_clear_button_clicked - self.media_item.on_clear_button_clicked() - - # THEN: The list_view should be cleared - self.assertEqual(self.media_item.saved_results, []) - self.media_item.list_view.clear.assert_called_once_with() - - def test_on_clear_button_clicked_search_tab(self): + def test_on_clear_button_clicked(self): """ Test on_clear_button_clicked when the search tab is selected """ # GIVEN: An instance of :class:`MediaManagerItem` and mocked out search_tab and select_tab and a mocked out # list_view and search_edit - self.media_item.list_view = MagicMock() - self.media_item.search_edit = MagicMock() + self.media_item.list_view = MagicMock(**{'selectedItems.return_value': ['Some', 'Results']}) self.media_item.results_view_tab = MagicMock(**{'currentIndex.return_value': ResultsTab.Search}) - self.media_item.current_results = ['Some', 'Results'] - with patch.object(self.media_item, 'on_focus'): + with patch.object(self.media_item, 'on_results_view_tab_total_update'): # WHEN: Calling on_clear_button_clicked self.media_item.on_clear_button_clicked() # THEN: The list_view and the search_edit should be cleared self.assertEqual(self.media_item.current_results, []) - self.media_item.list_view.clear.assert_called_once_with() - self.media_item.search_edit.clear.assert_called_once_with() + self.assertEqual(self.media_item.list_view.takeItem.call_count, 2) + self.media_item.list_view.row.assert_has_calls([call('Some'), call('Results')]) def test_on_save_results_button_clicked(self): """ @@ -809,6 +828,7 @@ class TestMediaItem(TestCase, TestMixin): # to the dialog box self.media_item.second_bible = None self.media_item.second_combo_box = MagicMock(**{'currentData.return_value': self.mocked_bible_1}) + self.media_item.saved_results = ['saved_results'] self.media_item.on_second_combo_box_index_changed(5) # THEN: The list_view should be cleared and the currently selected bible should not be channged @@ -836,6 +856,7 @@ class TestMediaItem(TestCase, TestMixin): # to the dialog box self.media_item.second_bible = None self.media_item.second_combo_box = MagicMock(**{'currentData.return_value': self.mocked_bible_1}) + self.media_item.saved_results = ['saved_results'] self.media_item.on_second_combo_box_index_changed(5) # THEN: The selected bible should be set as the current bible @@ -862,6 +883,7 @@ class TestMediaItem(TestCase, TestMixin): # to the dialog box self.media_item.second_bible = self.mocked_bible_1 self.media_item.second_combo_box = MagicMock(**{'currentData.return_value': None}) + self.media_item.saved_results = ['saved_results'] self.media_item.on_second_combo_box_index_changed(0) # THEN: The selected bible should be set as the current bible @@ -902,6 +924,7 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.select_book_combo_box = MagicMock(**{'currentData.return_value': 2}) self.media_item.bible = self.mocked_bible_1 self.mocked_plugin.manager.get_verse_count_by_book_ref_id.return_value = 6 + self.media_item.select_tab = MagicMock(**{'isVisible.return_value': True}) self.media_item.search_button = MagicMock() with patch.object(self.media_item, 'adjust_combo_box') as mocked_adjust_combo_box: # WHEN: Calling on_advanced_book_combo_box @@ -1383,6 +1406,8 @@ class TestMediaItem(TestCase, TestMixin): # 'bibles/is search while typing enabled' is requested self.setting_values = {'bibles/is search while typing enabled': True} self.media_item.search_timer.isActive.return_value = False + self.media_item.bible = self.mocked_bible_1 + self.media_item.bible.is_web_bible = False # WHEN: Calling on_search_edit_text_changed self.media_item.on_search_edit_text_changed() @@ -1402,7 +1427,7 @@ class TestMediaItem(TestCase, TestMixin): self.media_item.on_search_timer_timeout() # THEN: The search_status should be set to SearchAsYouType and text_search should have been called - self.assertEqual(self.media_item.search_status, SearchStatus().SearchAsYouType) + self.assertEqual(self.media_item.search_status, SearchStatus.SearchAsYouType) mocked_text_search.assert_called_once_with() def test_display_results_no_results(self): From a3d95acabd5717439b6e929570b884d521cf9081 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sat, 3 Jun 2017 23:52:11 +0100 Subject: [PATCH 31/32] and one renderer test --- tests/functional/openlp_core_lib/test_renderer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/functional/openlp_core_lib/test_renderer.py b/tests/functional/openlp_core_lib/test_renderer.py index 85fb774e1..feaefcd4d 100644 --- a/tests/functional/openlp_core_lib/test_renderer.py +++ b/tests/functional/openlp_core_lib/test_renderer.py @@ -182,13 +182,15 @@ class TestRenderer(TestCase): @patch('openlp.core.lib.renderer.QtWebKitWidgets.QWebView') @patch('openlp.core.lib.renderer.build_lyrics_format_css') @patch('openlp.core.lib.renderer.build_lyrics_outline_css') - def test_set_text_rectangle(self, mock_outline_css, mock_lyrics_css, mock_webview): + @patch('openlp.core.lib.renderer.build_chords_css') + def test_set_text_rectangle(self, mock_build_chords_css, mock_outline_css, mock_lyrics_css, mock_webview): """ Test set_text_rectangle returns a proper html string """ # GIVEN: test object and data mock_lyrics_css.return_value = ' FORMAT CSS; ' mock_outline_css.return_value = ' OUTLINE CSS; ' + mock_build_chords_css.return_value = ' CHORDS CSS; ' theme_data = Theme() theme_data.font_main_name = 'Arial' theme_data.font_main_size = 20 From 114a2a12a8e43232d5db746e5fa5c53332d933f9 Mon Sep 17 00:00:00 2001 From: Philip Ridout Date: Sun, 4 Jun 2017 00:27:19 +0100 Subject: [PATCH 32/32] fixed some seg faults --- tests/interfaces/openlp_core_ui/test_projectoreditform.py | 2 +- tests/interfaces/openlp_plugins/bibles/test_lib_manager.py | 1 + .../openlp_plugins/bibles/test_lib_parse_reference.py | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/interfaces/openlp_core_ui/test_projectoreditform.py b/tests/interfaces/openlp_core_ui/test_projectoreditform.py index 8e1758b31..b4a476875 100644 --- a/tests/interfaces/openlp_core_ui/test_projectoreditform.py +++ b/tests/interfaces/openlp_core_ui/test_projectoreditform.py @@ -45,8 +45,8 @@ class TestProjectorEditForm(TestCase, TestMixin): :return: None """ - self.build_settings() self.setup_application() + self.build_settings() Registry.create() with patch('openlp.core.lib.projector.db.init_url') as mocked_init_url: if os.path.exists(TEST_DB): diff --git a/tests/interfaces/openlp_plugins/bibles/test_lib_manager.py b/tests/interfaces/openlp_plugins/bibles/test_lib_manager.py index 6582b123f..663c16b0c 100644 --- a/tests/interfaces/openlp_plugins/bibles/test_lib_manager.py +++ b/tests/interfaces/openlp_plugins/bibles/test_lib_manager.py @@ -38,6 +38,7 @@ class TestBibleManager(TestCase, TestMixin): """ Set up the environment for testing bible queries with 1 Timothy 3 """ + self.setup_application() self.build_settings() Registry.create() Registry().register('service_list', MagicMock()) diff --git a/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py b/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py index 1916f267b..eec46c762 100644 --- a/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py +++ b/tests/interfaces/openlp_plugins/bibles/test_lib_parse_reference.py @@ -38,6 +38,7 @@ class TestBibleManager(TestCase, TestMixin): """ Set up the environment for testing bible parse reference """ + self.setup_application() self.build_settings() Registry.create() Registry().register('service_list', MagicMock())