Review Comments

This commit is contained in:
Tim Bentley 2016-09-19 19:51:48 +01:00
parent 8f94d10c8d
commit c8dad716cc
4 changed files with 54 additions and 58 deletions

View File

@ -379,7 +379,7 @@ class Settings(QtCore.QSettings):
'shortcuts/themeScreen': [QtGui.QKeySequence(QtCore.Qt.Key_T)], 'shortcuts/themeScreen': [QtGui.QKeySequence(QtCore.Qt.Key_T)],
'shortcuts/toolsReindexItem': [], 'shortcuts/toolsReindexItem': [],
'shortcuts/toolsFindDuplicates': [], 'shortcuts/toolsFindDuplicates': [],
'shortcuts/ReportSongList': [], 'shortcuts/toolsSongListReport': [],
'shortcuts/toolsAlertItem': [QtGui.QKeySequence(QtCore.Qt.Key_F7)], 'shortcuts/toolsAlertItem': [QtGui.QKeySequence(QtCore.Qt.Key_F7)],
'shortcuts/toolsFirstTimeWizard': [], 'shortcuts/toolsFirstTimeWizard': [],
'shortcuts/toolsOpenDataFolder': [], 'shortcuts/toolsOpenDataFolder': [],

View File

@ -22,12 +22,12 @@
""" """
The :mod:`db` module provides the ability to provide a csv file of all songs The :mod:`db` module provides the ability to provide a csv file of all songs
""" """
import csv
import logging import logging
import os
from PyQt5 import QtWidgets from PyQt5 import QtWidgets
from openlp.core.common import Registry, check_directory_exists, translate from openlp.core.common import Registry, translate
from openlp.core.lib.ui import critical_error_message_box from openlp.core.lib.ui import critical_error_message_box
from openlp.plugins.songs.lib.db import Song from openlp.plugins.songs.lib.db import Song
@ -42,45 +42,47 @@ def report_song_list():
""" """
main_window = Registry().get('main_window') main_window = Registry().get('main_window')
plugin = Registry().get('songs').plugin plugin = Registry().get('songs').plugin
path = QtWidgets.QFileDialog.getExistingDirectory( report_file_name, filter_used = QtWidgets.QFileDialog.getSaveFileName(
main_window, translate('SongPlugin.ReportSongList', 'Output File Location')) main_window, translate('SongPlugin.ReportSongList', 'Output File Location'))
if not path: if not report_file_name:
main_window.error_message( main_window.error_message(
translate('SongPlugin.ReportSongList', 'Output Path Not Selected'), translate('SongPlugin.ReportSongList', 'Output Path Not Selected'),
translate('SongPlugin.ReportSongList', 'You have not set a valid output location for your' translate('SongPlugin.ReportSongList', 'You have not set a valid output location for your '
'report. \nPlease select an existing path ' 'report. \nPlease select an existing path '
'on your computer.') 'on your computer.')
) )
return return
check_directory_exists(path) if not report_file_name.endswith('csv'):
report_file_name = os.path.join(path, 'song_index.csv') report_file_name += '.csv'
file_handle = None file_handle = None
try: try:
file_handle = open(report_file_name, 'wb') 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) song_list = plugin.manager.get_all_objects(Song)
for song in song_list: for song in song_list:
record = '\"{title}\",'.format(title=song.title)
record += '\"{title}\",'.format(title=song.alternate_title)
record += '\"{title}\",'.format(title=song.copyright)
author_list = [] author_list = []
for author_song in song.authors_songs: for author_song in song.authors_songs:
author_list.append(author_song.author.display_name) author_list.append(author_song.author.display_name)
author_string = '\"{name}\"'.format(name=' | '.join(author_list)) author_string = '{name}'.format(name=' | '.join(author_list))
book_list = [] book_list = []
for book_song in song.songbook_entries: for book_song in song.songbook_entries:
if hasattr(book_song, 'entry') and book_song.entry: 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_list.append('{name} #{entry}'.format(name=book_song.songbook.name, entry=book_song.entry))
book_string = '\"{name}\"'.format(name=' | '.join(book_list)) book_string = '{name}'.format(name=' | '.join(book_list))
topic_list = [] topic_list = []
for topic_song in song.topics: for topic_song in song.topics:
if hasattr(topic_song, 'name'): if hasattr(topic_song, 'name'):
topic_list.append(topic_song.name) topic_list.append(topic_song.name)
topic_string = '\"{name}\"'.format(name=' | '.join(topic_list)) topic_string = '{name}'.format(name=' | '.join(topic_list))
record += '{title},'.format(title=author_string) writer.writerow({'Title': song.title,
record += '{title},'.format(title=book_string) 'Alternative Title': song.alternate_title,
record += '{title},'.format(title=topic_string) 'Copyright': song.copyright,
record += '\n' 'Author(s)': author_string,
file_handle.write(record.encode('utf-8')) 'Song Book': book_string,
'Topic': topic_string})
main_window.information_message( main_window.information_message(
translate('SongPlugin.ReportSongList', 'Report Creation'), translate('SongPlugin.ReportSongList', 'Report Creation'),
translate('SongPlugin.ReportSongList', translate('SongPlugin.ReportSongList',

View File

@ -155,7 +155,7 @@ class SongsPlugin(Plugin):
self.tools_menu = tools_menu self.tools_menu = tools_menu
self.song_tools_menu = QtWidgets.QMenu(tools_menu) self.song_tools_menu = QtWidgets.QMenu(tools_menu)
self.song_tools_menu.setObjectName('song_tools_menu') self.song_tools_menu.setObjectName('song_tools_menu')
self.song_tools_menu.setTitle(translate('SongsPlugin', 'Song Tools')) self.song_tools_menu.setTitle(translate('SongsPlugin', 'Songs'))
self.tools_reindex_item = create_action( self.tools_reindex_item = create_action(
tools_menu, 'toolsReindexItem', tools_menu, 'toolsReindexItem',
text=translate('SongsPlugin', '&Re-index Songs'), text=translate('SongsPlugin', '&Re-index Songs'),
@ -168,8 +168,8 @@ class SongsPlugin(Plugin):
statustip=translate('SongsPlugin', 'Find and remove duplicate songs in the song database.'), statustip=translate('SongsPlugin', 'Find and remove duplicate songs in the song database.'),
triggers=self.on_tools_find_duplicates_triggered, can_shortcuts=True) triggers=self.on_tools_find_duplicates_triggered, can_shortcuts=True)
self.tools_report_song_list = create_action( self.tools_report_song_list = create_action(
tools_menu, 'ReportSongList', tools_menu, 'toolsSongListReport',
text=translate('SongsPlugin', 'Export List of Songs'), text=translate('SongsPlugin', 'Song List Report'),
statustip=translate('SongsPlugin', 'Produce a CSV file of all the songs in the database.'), statustip=translate('SongsPlugin', 'Produce a CSV file of all the songs in the database.'),
triggers=self.on_tools_report_song_list_triggered) triggers=self.on_tools_report_song_list_triggered)

View File

@ -545,8 +545,8 @@ class TestServiceManager(TestCase):
self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1, self.assertEqual(service_manager.theme_menu.menuAction().setVisible.call_count, 1,
'Should have be called once') 'Should have be called once')
@patch(u'openlp.core.ui.servicemanager.Settings') @patch('openlp.core.ui.servicemanager.Settings')
@patch(u'PyQt5.QtCore.QTimer.singleShot') @patch('PyQt5.QtCore.QTimer.singleShot')
def test_single_click_preview_true(self, mocked_singleShot, MockedSettings): 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 Test that when "Preview items when clicked in Service Manager" enabled the preview timer starts
@ -562,8 +562,8 @@ class TestServiceManager(TestCase):
mocked_singleShot.assert_called_with(PyQt5.QtWidgets.QApplication.instance().doubleClickInterval(), mocked_singleShot.assert_called_with(PyQt5.QtWidgets.QApplication.instance().doubleClickInterval(),
service_manager.on_single_click_preview_timeout) service_manager.on_single_click_preview_timeout)
@patch(u'openlp.core.ui.servicemanager.Settings') @patch('openlp.core.ui.servicemanager.Settings')
@patch(u'PyQt5.QtCore.QTimer.singleShot') @patch('PyQt5.QtCore.QTimer.singleShot')
def test_single_click_preview_false(self, mocked_singleShot, MockedSettings): 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 Test that when "Preview items when clicked in Service Manager" disabled the preview timer doesn't start
@ -578,9 +578,9 @@ class TestServiceManager(TestCase):
# THEN: timer should not be started # THEN: timer should not be started
self.assertEqual(mocked_singleShot.call_count, 0, 'Should not be called') self.assertEqual(mocked_singleShot.call_count, 0, 'Should not be called')
@patch(u'openlp.core.ui.servicemanager.Settings') @patch('openlp.core.ui.servicemanager.Settings')
@patch(u'PyQt5.QtCore.QTimer.singleShot') @patch('PyQt5.QtCore.QTimer.singleShot')
@patch(u'openlp.core.ui.servicemanager.ServiceManager.make_live') @patch('openlp.core.ui.servicemanager.ServiceManager.make_live')
def test_single_click_preview_double(self, mocked_make_live, mocked_singleShot, MockedSettings): 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 Test that when a double click has registered the preview timer doesn't start
@ -597,7 +597,7 @@ class TestServiceManager(TestCase):
mocked_make_live.assert_called_with() mocked_make_live.assert_called_with()
self.assertEqual(mocked_singleShot.call_count, 0, 'Should not be called') 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): 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 Test that when a single click has been registered, the item is sent to preview
@ -610,8 +610,8 @@ class TestServiceManager(TestCase):
self.assertEqual(mocked_make_preview.call_count, 1, self.assertEqual(mocked_make_preview.call_count, 1,
'ServiceManager.make_preview() should have been called once') 'ServiceManager.make_preview() should have been called once')
@patch(u'openlp.core.ui.servicemanager.ServiceManager.make_preview') @patch('openlp.core.ui.servicemanager.ServiceManager.make_preview')
@patch(u'openlp.core.ui.servicemanager.ServiceManager.make_live') @patch('openlp.core.ui.servicemanager.ServiceManager.make_live')
def test_single_click_timeout_double(self, mocked_make_live, mocked_make_preview): 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 Test that when a double click has been registered, the item does not goes to preview
@ -624,9 +624,9 @@ class TestServiceManager(TestCase):
# THEN: make_preview() should not have been called # THEN: make_preview() should not have been called
self.assertEqual(mocked_make_preview.call_count, 0, 'ServiceManager.make_preview() should not be 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('openlp.core.ui.servicemanager.shutil.copy')
@patch(u'openlp.core.ui.servicemanager.zipfile') @patch('openlp.core.ui.servicemanager.zipfile')
@patch(u'openlp.core.ui.servicemanager.ServiceManager.save_file_as') @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): 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 Test that when a PermissionError is raised when trying to save a file, it is handled correctly
@ -653,9 +653,9 @@ class TestServiceManager(TestCase):
self.assertTrue(result) self.assertTrue(result)
mocked_save_file_as.assert_called_with() mocked_save_file_as.assert_called_with()
@patch(u'openlp.core.ui.servicemanager.shutil.copy') @patch('openlp.core.ui.servicemanager.shutil.copy')
@patch(u'openlp.core.ui.servicemanager.zipfile') @patch('openlp.core.ui.servicemanager.zipfile')
@patch(u'openlp.core.ui.servicemanager.ServiceManager.save_file_as') @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): 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 Test that when a PermissionError is raised when trying to save a local file, it is handled correctly
@ -681,13 +681,12 @@ class TestServiceManager(TestCase):
self.assertTrue(result) self.assertTrue(result)
mocked_save_file_as.assert_called_with() mocked_save_file_as.assert_called_with()
@patch(u'openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items') @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
def test_theme_change_global(self, mocked_regenerate_service_items): def test_theme_change_global(self, mocked_regenerate_service_items):
""" """
Test that when a Toolbar theme combobox displays correctly 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
# GIVEN: A service manager, a service to save with a theme level of the renderer
mocked_renderer = MagicMock() mocked_renderer = MagicMock()
service_manager = ServiceManager(None) service_manager = ServiceManager(None)
Registry().register('renderer', mocked_renderer) Registry().register('renderer', mocked_renderer)
@ -700,17 +699,15 @@ class TestServiceManager(TestCase):
result = service_manager.theme_change() result = service_manager.theme_change()
# THEN: The the theme toolbar should not be visible # THEN: The the theme toolbar should not be visible
self.assertFalse(service_manager.toolbar.actions['theme_combo_box'].isVisible(), self.assertFalse(service_manager.toolbar.actions['theme_combo_box'].isVisible(),
'The visibility should be False') 'The visibility should be False')
@patch(u'openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items') @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
def test_theme_change_service(self, mocked_regenerate_service_items): def test_theme_change_service(self, mocked_regenerate_service_items):
""" """
Test that when a Toolbar theme combobox displays correctly 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
# GIVEN: A service manager, a service to save with a theme level of the renderer
mocked_renderer = MagicMock() mocked_renderer = MagicMock()
service_manager = ServiceManager(None) service_manager = ServiceManager(None)
Registry().register('renderer', mocked_renderer) Registry().register('renderer', mocked_renderer)
@ -718,22 +715,20 @@ class TestServiceManager(TestCase):
service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock()) service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock()) service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
# WHEN: The service manager has a Global theme # WHEN: The service manager has a Service theme
mocked_renderer.theme_level = ThemeLevel.Service mocked_renderer.theme_level = ThemeLevel.Service
result = service_manager.theme_change() result = service_manager.theme_change()
# THEN: The the theme toolbar should not be visible # THEN: The the theme toolbar should be visible
self.assertTrue(service_manager.toolbar.actions['theme_combo_box'].isVisible(), self.assertTrue(service_manager.toolbar.actions['theme_combo_box'].isVisible(),
'The visibility should be True') 'The visibility should be True')
@patch(u'openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items') @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items')
def test_theme_change_song(self, mocked_regenerate_service_items): def test_theme_change_song(self, mocked_regenerate_service_items):
""" """
Test that when a Toolbar theme combobox displays correctly 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
# GIVEN: A service manager, a service to save with a theme level of the renderer
mocked_renderer = MagicMock() mocked_renderer = MagicMock()
service_manager = ServiceManager(None) service_manager = ServiceManager(None)
Registry().register('renderer', mocked_renderer) Registry().register('renderer', mocked_renderer)
@ -741,11 +736,10 @@ class TestServiceManager(TestCase):
service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock()) service_manager.toolbar.add_toolbar_action('theme_combo_box', triggers=MagicMock())
service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock()) service_manager.toolbar.add_toolbar_action('theme_label', triggers=MagicMock())
# WHEN: The service manager has a Global theme # WHEN: The service manager has a Song theme
mocked_renderer.theme_level = ThemeLevel.Song mocked_renderer.theme_level = ThemeLevel.Song
result = service_manager.theme_change() result = service_manager.theme_change()
# THEN: The the theme toolbar should not be visible # THEN: The the theme toolbar should be visible
self.assertTrue(service_manager.toolbar.actions['theme_combo_box'].isVisible(), self.assertTrue(service_manager.toolbar.actions['theme_combo_box'].isVisible(),
'The visibility should be True') 'The visibility should be True')