Merge lp:~trb143/openlp/reporting into lp:openlp

[Tim Bentley]

My dad needed a report of all the songs on their database, they had 1800.
Made this into a reporting option and cleaned up the menu.
Fixed some errors spotted as well

Fixed issues and comments
1800 songs takes about 3 secs to run on my i7

lp:~trb143/openlp/reporting (revision 2701)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1801/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1712/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1650/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1406/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/996/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/1064/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/932/
[SUCCESS] https://ci.openlp.io/job/Branch-05c-Code_Analysis2/93/

[Raoul Snyman (raoul-snyman)]

1. There's a "csv" module, use it.
2. I should be able to specify the file name, not just where to save it to.
3. Have you tested this with > 1000 songs? How long does it take? Some sort of progress window necessary?
4. You call it a report internally, but you're not very specific for the user. Rather call it a "Song List Report".

More comments inline.

review: Needs Fixing

[Phill (phill-ridout)]

See my inline comments about your test doc stings and comments.

And also see my inline comments with regard to the Unicode literals. They're not required, and seem only to be implemented to ease porting from py2! https://www.python.org/dev/peps/pep-0414/#proposal

Are there any tests for the new module/method you've addded:
reporting.py
on_tools_report_song_list_triggered

[Phill (phill-ridout)]

Still missing tests for your new module. Guess its up to superfly if he's happy with that.

Also, just a few inline comments

[Raoul Snyman (raoul-snyman)]

Couple extra things:

1. The save dialog says "output folder" or something, just make it say "Save file"
2. There's no list of file extensions in the dialog, and no default file name and/or extension.

review: Needs Fixing

[Raoul Snyman (raoul-snyman)]

review: Approve

=== modified file 'openlp/core/common/settings.py'
--- openlp/core/common/settings.py	2016-09-02 16:19:28 +0000
+++ openlp/core/common/settings.py	2016-10-26 18:03:31 +0000
@@ -381,6 +381,7 @@
             'shortcuts/themeScreen': [QtGui.QKeySequence(QtCore.Qt.Key_T)],
             'shortcuts/toolsReindexItem': [],
             'shortcuts/toolsFindDuplicates': [],
+            'shortcuts/toolsSongListReport': [],
             'shortcuts/toolsAlertItem': [QtGui.QKeySequence(QtCore.Qt.Key_F7)],
             'shortcuts/toolsFirstTimeWizard': [],
             'shortcuts/toolsOpenDataFolder': [],

=== modified file 'openlp/plugins/songs/lib/openlyricsxml.py'
--- openlp/plugins/songs/lib/openlyricsxml.py	2016-08-05 19:57:25 +0000
+++ openlp/plugins/songs/lib/openlyricsxml.py	2016-10-26 18:03:31 +0000
@@ -458,7 +458,7 @@
                 self._add_tag_to_formatting(tag, tags_element)
         # Replace end tags.
         for tag in end_tags:
-            text = text.replace('{{{tag}}}'.format(tag=tag), '</tag>')
+            text = text.replace('{{/{tag}}}'.format(tag=tag), '</tag>')
         # Replace \n with <br/>.
         text = text.replace('\n', '<br/>')
         element = etree.XML('<lines>{text}</lines>'.format(text=text))

=== modified file 'openlp/plugins/songs/lib/songcompare.py'
--- openlp/plugins/songs/lib/songcompare.py	2015-12-31 22:46:06 +0000
+++ openlp/plugins/songs/lib/songcompare.py	2016-10-26 18:03:31 +0000
@@ -46,13 +46,13 @@
 MAX_TYPO_SIZE = 3
 
 
-def songs_probably_equal(song_tupel):
+def songs_probably_equal(song_tuple):
     """
     Calculate and return whether two songs are probably equal.
 
-    :param song_tupel: A tuple of two songs to compare.
+    :param song_tuple: A tuple of two songs to compare.
     """
-    song1, song2 = song_tupel
+    song1, song2 = song_tuple
     pos1, lyrics1 = song1
     pos2, lyrics2 = song2
     if len(lyrics1) < len(lyrics2):

=== added file 'openlp/plugins/songs/reporting.py'
--- openlp/plugins/songs/reporting.py	1970-01-01 00:00:00 +0000
+++ openlp/plugins/songs/reporting.py	2016-10-26 18:03:31 +0000
@@ -0,0 +1,106 @@
+# -*- coding: utf-8 -*-
+# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
+
+###############################################################################
+# OpenLP - Open Source Lyrics Projection                                      #
+# --------------------------------------------------------------------------- #
+# Copyright (c) 2008-2016 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                          #
+###############################################################################
+"""
+The :mod:`db` module provides the ability to provide a csv file of all songs
+"""
+import csv
+import logging
+
+from PyQt5 import QtWidgets
+
+from openlp.core.common import Registry, translate
+from openlp.core.lib.ui import critical_error_message_box
+from openlp.plugins.songs.lib.db import Song
+
+
+log = logging.getLogger(__name__)
+
+
+def report_song_list():
+    """
+    Export the song list as a CSV file.
+    :return: Nothing
+    """
+    main_window = Registry().get('main_window')
+    plugin = Registry().get('songs').plugin
+    report_file_name, filter_used = QtWidgets.QFileDialog.getSaveFileName(
+        main_window,
+        translate('SongPlugin.ReportSongList', 'Save File'),
+        translate('SongPlugin.ReportSongList', 'song_extract.csv'),
+        translate('SongPlugin.ReportSongList', 'CSV format (*.csv)'))
+
+    if not report_file_name:
+        main_window.error_message(
+            translate('SongPlugin.ReportSongList', 'Output Path Not Selected'),
+            translate('SongPlugin.ReportSongList', 'You have not set a valid output location for your '
+                                                   'report. \nPlease select an existing path '
+                                                   'on your computer.')
+        )
+        return
+    if not report_file_name.endswith('csv'):
+        report_file_name += '.csv'
+    file_handle = None
+    Registry().get('application').set_busy_cursor()
+    try:
+        file_handle = open(report_file_name, 'wt')
+        fieldnames = ('Title', 'Alternative Title', 'Copyright', 'Author(s)', 'Song Book', 'Topic')
+        writer = csv.DictWriter(file_handle, fieldnames=fieldnames, quoting=csv.QUOTE_ALL)
+        headers = dict((n, n) for n in fieldnames)
+        writer.writerow(headers)
+        song_list = plugin.manager.get_all_objects(Song)
+        for song in song_list:
+            author_list = []
+            for author_song in song.authors_songs:
+                author_list.append(author_song.author.display_name)
+            author_string = ' | '.join(author_list)
+            book_list = []
+            for book_song in song.songbook_entries:
+                if hasattr(book_song, 'entry') and book_song.entry:
+                    book_list.append('{name} #{entry}'.format(name=book_song.songbook.name, entry=book_song.entry))
+            book_string = ' | '.join(book_list)
+            topic_list = []
+            for topic_song in song.topics:
+                if hasattr(topic_song, 'name'):
+                    topic_list.append(topic_song.name)
+            topic_string = ' | '.join(topic_list)
+            writer.writerow({'Title': song.title,
+                             'Alternative Title': song.alternate_title,
+                             'Copyright': song.copyright,
+                             'Author(s)': author_string,
+                             'Song Book': book_string,
+                             'Topic': topic_string})
+        Registry().get('application').set_normal_cursor()
+        main_window.information_message(
+            translate('SongPlugin.ReportSongList', 'Report Creation'),
+            translate('SongPlugin.ReportSongList',
+                      'Report \n{name} \nhas been successfully created. ').format(name=report_file_name)
+        )
+    except OSError as ose:
+        Registry().get('application').set_normal_cursor()
+        log.exception('Failed to write out song usage records')
+        critical_error_message_box(translate('SongPlugin.ReportSongList', 'Song Extraction Failed'),
+                                   translate('SongPlugin.ReportSongList',
+                                             'An error occurred while extracting: {error}'
+                                             ).format(error=ose.strerror))
+    finally:
+        if file_handle:
+            file_handle.close()

=== modified file 'openlp/plugins/songs/songsplugin.py'
--- openlp/plugins/songs/songsplugin.py	2016-03-31 16:34:22 +0000
+++ openlp/plugins/songs/songsplugin.py	2016-10-26 18:03:31 +0000
@@ -36,6 +36,7 @@
 from openlp.core.lib import Plugin, StringContent, build_icon
 from openlp.core.lib.db import Manager
 from openlp.core.lib.ui import create_action
+from openlp.plugins.songs import reporting
 from openlp.plugins.songs.forms.duplicatesongremovalform import DuplicateSongRemovalForm
 from openlp.plugins.songs.forms.songselectform import SongSelectForm
 from openlp.plugins.songs.lib import clean_song, upgrade
@@ -102,13 +103,13 @@
         self.songselect_form.initialise()
         self.song_import_item.setVisible(True)
         self.song_export_item.setVisible(True)
-        self.tools_reindex_item.setVisible(True)
-        self.tools_find_duplicates.setVisible(True)
+        self.song_tools_menu.menuAction().setVisible(True)
         action_list = ActionList.get_instance()
         action_list.add_action(self.song_import_item, UiStrings().Import)
         action_list.add_action(self.song_export_item, UiStrings().Export)
         action_list.add_action(self.tools_reindex_item, UiStrings().Tools)
         action_list.add_action(self.tools_find_duplicates, UiStrings().Tools)
+        action_list.add_action(self.tools_report_song_list, UiStrings().Tools)
 
     def add_import_menu_item(self, import_menu):
         """
@@ -151,19 +152,37 @@
         :param tools_menu: The actual **Tools** menu item, so that your actions can use it as their parent.
         """
         log.info('add tools menu')
+        self.tools_menu = tools_menu
+        self.song_tools_menu = QtWidgets.QMenu(tools_menu)
+        self.song_tools_menu.setObjectName('song_tools_menu')
+        self.song_tools_menu.setTitle(translate('SongsPlugin', 'Songs'))
         self.tools_reindex_item = create_action(
             tools_menu, 'toolsReindexItem',
             text=translate('SongsPlugin', '&Re-index Songs'),
             icon=':/plugins/plugin_songs.png',
             statustip=translate('SongsPlugin', 'Re-index the songs database to improve searching and ordering.'),
-            visible=False, triggers=self.on_tools_reindex_item_triggered)
-        tools_menu.addAction(self.tools_reindex_item)
+            triggers=self.on_tools_reindex_item_triggered)
         self.tools_find_duplicates = create_action(
             tools_menu, 'toolsFindDuplicates',
             text=translate('SongsPlugin', 'Find &Duplicate Songs'),
             statustip=translate('SongsPlugin', 'Find and remove duplicate songs in the song database.'),
-            visible=False, triggers=self.on_tools_find_duplicates_triggered, can_shortcuts=True)
-        tools_menu.addAction(self.tools_find_duplicates)
+            triggers=self.on_tools_find_duplicates_triggered, can_shortcuts=True)
+        self.tools_report_song_list = create_action(
+            tools_menu, 'toolsSongListReport',
+            text=translate('SongsPlugin', 'Song List Report'),
+            statustip=translate('SongsPlugin', 'Produce a CSV file of all the songs in the database.'),
+            triggers=self.on_tools_report_song_list_triggered)
+
+        self.tools_menu.addAction(self.song_tools_menu.menuAction())
+        self.song_tools_menu.addAction(self.tools_reindex_item)
+        self.song_tools_menu.addAction(self.tools_find_duplicates)
+        self.song_tools_menu.addAction(self.tools_report_song_list)
+
+        self.song_tools_menu.menuAction().setVisible(False)
+
+    @staticmethod
+    def on_tools_report_song_list_triggered():
+        reporting.report_song_list()
 
     def on_tools_reindex_item_triggered(self):
         """
@@ -326,13 +345,13 @@
         self.manager.finalise()
         self.song_import_item.setVisible(False)
         self.song_export_item.setVisible(False)
-        self.tools_reindex_item.setVisible(False)
-        self.tools_find_duplicates.setVisible(False)
         action_list = ActionList.get_instance()
         action_list.remove_action(self.song_import_item, UiStrings().Import)
         action_list.remove_action(self.song_export_item, UiStrings().Export)
         action_list.remove_action(self.tools_reindex_item, UiStrings().Tools)
         action_list.remove_action(self.tools_find_duplicates, UiStrings().Tools)
+        action_list.add_action(self.tools_report_song_list, UiStrings().Tools)
+        self.song_tools_menu.menuAction().setVisible(False)
         super(SongsPlugin, self).finalise()
 
     def new_service_created(self):

=== modified file 'tests/functional/openlp_core_ui/test_servicemanager.py'
--- tests/functional/openlp_core_ui/test_servicemanager.py	2016-07-17 19:46:06 +0000
+++ tests/functional/openlp_core_ui/test_servicemanager.py	2016-10-26 18:03:31 +0000
@@ -28,6 +28,7 @@
 import PyQt5
 
 from openlp.core.common import Registry, ThemeLevel
+from openlp.core.ui.lib.toolbar import OpenLPToolbar
 from openlp.core.lib import ServiceItem, ServiceItemType, ItemCapabilities
 from openlp.core.ui import ServiceManager
 
@@ -544,8 +545,8 @@
         self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
                          'Should have be called once')
 
-    @patch(u'openlp.core.ui.servicemanager.Settings')
-    @patch(u'PyQt5.QtCore.QTimer.singleShot')
+    @patch('openlp.core.ui.servicemanager.Settings')
+    @patch('PyQt5.QtCore.QTimer.singleShot')
     def test_single_click_preview_true(self, mocked_singleShot, MockedSettings):
         """
         Test that when "Preview items when clicked in Service Manager" enabled the preview timer starts
@@ -561,8 +562,8 @@
         mocked_singleShot.assert_called_with(PyQt5.QtWidgets.QApplication.instance().doubleClickInterval(),
                                              service_manager.on_single_click_preview_timeout)
 
-    @patch(u'openlp.core.ui.servicemanager.Settings')
-    @patch(u'PyQt5.QtCore.QTimer.singleShot')
+    @patch('openlp.core.ui.servicemanager.Settings')
+    @patch('PyQt5.QtCore.QTimer.singleShot')
     def test_single_click_preview_false(self, mocked_singleShot, MockedSettings):
         """
         Test that when "Preview items when clicked in Service Manager" disabled the preview timer doesn't start
@@ -577,9 +578,9 @@
         # THEN: timer should not be started
         self.assertEqual(mocked_singleShot.call_count, 0, 'Should not be called')
 
-    @patch(u'openlp.core.ui.servicemanager.Settings')
-    @patch(u'PyQt5.QtCore.QTimer.singleShot')
-    @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_live')
+    @patch('openlp.core.ui.servicemanager.Settings')
+    @patch('PyQt5.QtCore.QTimer.singleShot')
+    @patch('openlp.core.ui.servicemanager.ServiceManager.make_live')
     def test_single_click_preview_double(self, mocked_make_live, mocked_singleShot, MockedSettings):
         """
         Test that when a double click has registered the preview timer doesn't start
@@ -596,7 +597,7 @@
         mocked_make_live.assert_called_with()
         self.assertEqual(mocked_singleShot.call_count, 0, 'Should not be called')
 
-    @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_preview')
+    @patch('openlp.core.ui.servicemanager.ServiceManager.make_preview')
     def test_single_click_timeout_single(self, mocked_make_preview):
         """
         Test that when a single click has been registered, the item is sent to preview
@@ -609,8 +610,8 @@
         self.assertEqual(mocked_make_preview.call_count, 1,
                          'ServiceManager.make_preview() should have been called once')
 
-    @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_preview')
-    @patch(u'openlp.core.ui.servicemanager.ServiceManager.make_live')
+    @patch('openlp.core.ui.servicemanager.ServiceManager.make_preview')
+    @patch('openlp.core.ui.servicemanager.ServiceManager.make_live')
     def test_single_click_timeout_double(self, mocked_make_live, mocked_make_preview):
         """
         Test that when a double click has been registered, the item does not goes to preview
@@ -623,9 +624,9 @@
         # THEN: make_preview() should not have been called
         self.assertEqual(mocked_make_preview.call_count, 0, 'ServiceManager.make_preview() should not be called')
 
-    @patch(u'openlp.core.ui.servicemanager.shutil.copy')
-    @patch(u'openlp.core.ui.servicemanager.zipfile')
-    @patch(u'openlp.core.ui.servicemanager.ServiceManager.save_file_as')
+    @patch('openlp.core.ui.servicemanager.shutil.copy')
+    @patch('openlp.core.ui.servicemanager.zipfile')
+    @patch('openlp.core.ui.servicemanager.ServiceManager.save_file_as')
     def test_save_file_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy):
         """
         Test that when a PermissionError is raised when trying to save a file, it is handled correctly
@@ -652,9 +653,9 @@
         self.assertTrue(result)
         mocked_save_file_as.assert_called_with()
 
-    @patch(u'openlp.core.ui.servicemanager.shutil.copy')
-    @patch(u'openlp.core.ui.servicemanager.zipfile')
-    @patch(u'openlp.core.ui.servicemanager.ServiceManager.save_file_as')
+    @patch('openlp.core.ui.servicemanager.shutil.copy')
+    @patch('openlp.core.ui.servicemanager.zipfile')
+    @patch('openlp.core.ui.servicemanager.ServiceManager.save_file_as')
     def test_save_local_file_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy):
         """
         Test that when a PermissionError is raised when trying to save a local file, it is handled correctly
@@ -679,3 +680,66 @@
         # THEN: The "save_as" method is called to save the service
         self.assertTrue(result)
         mocked_save_file_as.assert_called_with()
+
+    @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
+    def test_theme_change_global(self, mocked_regenerate_service_items):
+        """
+        Test that when a Toolbar theme combobox displays correctly when the theme is set to Global
+        """
+        # GIVEN: A service manager, a service to display with a theme level in the renderer
+        mocked_renderer = MagicMock()
+        service_manager = ServiceManager(None)
+        Registry().register('renderer', mocked_renderer)
+        service_manager.toolbar = OpenLPToolbar(None)
+        service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
+        service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
+
+        # WHEN: The service manager has a Global theme
+        mocked_renderer.theme_level = ThemeLevel.Global
+        result = service_manager.theme_change()
+
+        # THEN: The the theme toolbar should not be visible
+        self.assertFalse(service_manager.toolbar.actions['theme_combo_box'].isVisible(),
+                         'The visibility should be False')
+
+    @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
+    def test_theme_change_service(self, mocked_regenerate_service_items):
+        """
+        Test that when a Toolbar theme combobox displays correctly when the theme is set to Theme
+        """
+        # GIVEN: A service manager, a service to display with a theme level in the renderer
+        mocked_renderer = MagicMock()
+        service_manager = ServiceManager(None)
+        Registry().register('renderer', mocked_renderer)
+        service_manager.toolbar = OpenLPToolbar(None)
+        service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
+        service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
+
+        # WHEN: The service manager has a Service theme
+        mocked_renderer.theme_level = ThemeLevel.Service
+        result = service_manager.theme_change()
+
+        # THEN: The the theme toolbar should be visible
+        self.assertTrue(service_manager.toolbar.actions['theme_combo_box'].isVisible(),
+                        'The visibility should be True')
+
+    @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
+    def test_theme_change_song(self, mocked_regenerate_service_items):
+        """
+        Test that when a Toolbar theme combobox displays correctly when the theme is set to Song
+        """
+        # GIVEN: A service manager, a service to display with a theme level in the renderer
+        mocked_renderer = MagicMock()
+        service_manager = ServiceManager(None)
+        Registry().register('renderer', mocked_renderer)
+        service_manager.toolbar = OpenLPToolbar(None)
+        service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
+        service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
+
+        # WHEN: The service manager has a Song theme
+        mocked_renderer.theme_level = ThemeLevel.Song
+        result = service_manager.theme_change()
+
+        # THEN: The the theme toolbar should  be visible
+        self.assertTrue(service_manager.toolbar.actions['theme_combo_box'].isVisible(),
+                        'The visibility should be True')