openlp-core team mailing list archive

[Tim Bentley <tim.bentley@xxxxxxxxx>]

Late night reviews not good idea,

Just one question!

Diff comments:

> === modified file 'openlp/plugins/bibles/bibleplugin.py'
> --- openlp/plugins/bibles/bibleplugin.py	2015-02-11 20:56:13 +0000
> +++ openlp/plugins/bibles/bibleplugin.py	2015-03-10 21:40:23 +0000
> @@ -113,12 +113,13 @@
>                                          'existing Bibles.\nShould OpenLP upgrade now?'),
>                      QtGui.QMessageBox.StandardButtons(QtGui.QMessageBox.Yes | QtGui.QMessageBox.No)) == \
>                      QtGui.QMessageBox.Yes:
> -                self.on_tools_upgrade_Item_triggered()
> +                self.on_tools_upgrade_item_triggered()
>  
>      def add_import_menu_item(self, import_menu):
>          """
> +        Add an import menu item
>  
> -        :param import_menu:
> +        :param import_menu: The menu to insert the menu item into.
>          """
>          self.import_bible_item = create_action(import_menu, 'importBibleItem',
>                                                 text=translate('BiblesPlugin', '&Bible'), visible=False,
> @@ -127,8 +128,9 @@
>  
>      def add_export_menu_item(self, export_menu):
>          """
> +        Add an export menu item
>  
> -        :param export_menu:
> +        :param export_menu: The menu to insert the menu item into.
>          """
>          self.export_bible_item = create_action(export_menu, 'exportBibleItem',
>                                                 text=translate('BiblesPlugin', '&Bible'), visible=False)
> @@ -145,10 +147,10 @@
>              tools_menu, 'toolsUpgradeItem',
>              text=translate('BiblesPlugin', '&Upgrade older Bibles'),
>              statustip=translate('BiblesPlugin', 'Upgrade the Bible databases to the latest format.'),
> -            visible=False, triggers=self.on_tools_upgrade_Item_triggered)
> +            visible=False, triggers=self.on_tools_upgrade_item_triggered)
>          tools_menu.addAction(self.tools_upgrade_item)
>  
> -    def on_tools_upgrade_Item_triggered(self):
> +    def on_tools_upgrade_item_triggered(self):
>          """
>          Upgrade older bible databases.
>          """
> @@ -159,10 +161,16 @@
>              self.media_item.reload_bibles()
>  
>      def on_bible_import_click(self):
> +        """
> +        Show the Bible Import wizard
> +        """
>          if self.media_item:
>              self.media_item.on_import_click()
>  
>      def about(self):
> +        """
> +        Return the about text for the plugin manager
> +        """
>          about_text = translate('BiblesPlugin', '<strong>Bible Plugin</strong>'
>                                 '<br />The Bible plugin provides the ability to display Bible '
>                                 'verses from different sources during the service.')
> 
> === modified file 'openlp/plugins/media/lib/mediaitem.py'
> --- openlp/plugins/media/lib/mediaitem.py	2015-01-18 13:39:21 +0000
> +++ openlp/plugins/media/lib/mediaitem.py	2015-03-10 21:40:23 +0000
> @@ -340,6 +340,7 @@
>          media.sort(key=lambda file_name: get_locale_key(os.path.split(str(file_name))[1]))
>          for track in media:
>              track_info = QtCore.QFileInfo(track)
> +            item_name = None
>              if track.startswith('optical:'):
>                  # Handle optical based item
>                  (file_name, title, audio_track, subtitle_track, start, end, clip_name) = parse_optical_path(track)
> @@ -364,7 +365,8 @@
>                      item_name.setIcon(VIDEO_ICON)
>                  item_name.setData(QtCore.Qt.UserRole, track)
>                  item_name.setToolTip(track)
> -            self.list_view.addItem(item_name)
> +            if item_name:
> +                self.list_view.addItem(item_name)
>  
>      def get_list(self, type=MediaType.Audio):
>          """
> 
> === modified file 'openlp/plugins/media/mediaplugin.py'
> --- openlp/plugins/media/mediaplugin.py	2015-01-18 13:39:21 +0000
> +++ openlp/plugins/media/mediaplugin.py	2015-03-10 21:40:23 +0000
> @@ -19,12 +19,15 @@
>  # with this program; if not, write to the Free Software Foundation, Inc., 59  #
>  # Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
>  ###############################################################################
> +"""
> +The Media plugin
> +"""
>  
>  import logging
>  
>  from PyQt4 import QtCore
>  
> -from openlp.core.common import translate
> +from openlp.core.common import Settings, translate
>  from openlp.core.lib import Plugin, StringContent, build_icon
>  from openlp.plugins.media.lib import MediaMediaItem, MediaTab
>  
> @@ -40,6 +43,9 @@
>  
>  
>  class MediaPlugin(Plugin):
> +    """
> +    The media plugin adds the ability to playback audio and video content.
> +    """
>      log.info('%s MediaPlugin loaded', __name__)
>  
>      def __init__(self):
> @@ -50,14 +56,38 @@
>          # passed with drag and drop messages
>          self.dnd_id = 'Media'
>  
> +    def initialise(self):
> +        """
> +        Override the inherited initialise() method in order to upgrade the media before trying to load it
> +        """
> +        # FIXME: Remove after 2.2 release.
> +        # This is needed to load the list of media from the config saved before the settings rewrite.
> +        if self.media_item_class is not None:
> +            loaded_list = Settings().get_files_from_config(self)
> +            # Now save the list to the config using our Settings class.
> +            if loaded_list:
> +                Settings().setValue('%s/%s files' % (self.settings_section, self.name), loaded_list)
> +        super().initialise()
> +
> +    def app_startup(self):
> +        """
> +        Override app_startup() in order to do nothing
> +        """
> +        pass
> +
>      def create_settings_tab(self, parent):
>          """
>          Create the settings Tab
> +
> +        :param parent:
>          """
>          visible_name = self.get_string(StringContent.VisibleName)
>          self.settings_tab = MediaTab(parent, self.name, visible_name['title'], self.icon_path)
>  
>      def about(self):
> +        """
> +        Return the about text for the plugin manager
> +        """
>          about_text = translate('MediaPlugin', '<strong>Media Plugin</strong>'
>                                 '<br />The media plugin provides playback of audio and video.')
>          return about_text
> 
> === added directory 'tests/functional/openlp_plugins/media'
> === added file 'tests/functional/openlp_plugins/media/__init__.py'
> --- tests/functional/openlp_plugins/media/__init__.py	1970-01-01 00:00:00 +0000
> +++ tests/functional/openlp_plugins/media/__init__.py	2015-03-10 21:40:23 +0000
> @@ -0,0 +1,1 @@
> +__author__ = 'raoul'
> 
> === added file 'tests/functional/openlp_plugins/media/test_mediaplugin.py'
> --- tests/functional/openlp_plugins/media/test_mediaplugin.py	1970-01-01 00:00:00 +0000
> +++ tests/functional/openlp_plugins/media/test_mediaplugin.py	2015-03-10 21:40:23 +0000
> @@ -0,0 +1,59 @@
> +# -*- coding: utf-8 -*-
> +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
> +
> +###############################################################################
> +# OpenLP - Open Source Lyrics Projection                                      #
> +# --------------------------------------------------------------------------- #
> +# Copyright (c) 2008-2015 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                          #
> +###############################################################################
> +"""
> +Test the media plugin
> +"""
> +from unittest import TestCase
> +
> +from openlp.core import Registry
> +from openlp.plugins.media.mediaplugin import MediaPlugin
> +
> +from tests.functional import MagicMock, patch
> +from tests.helpers.testmixin import TestMixin
> +
> +
> +class MediaPluginTest(TestCase, TestMixin):
> +    """
> +    Test the media plugin
> +    """
> +    def setUp(self):
> +        Registry.create()
> +
> +    @patch(u'openlp.plugins.media.mediaplugin.Plugin.initialise')
> +    @patch(u'openlp.plugins.media.mediaplugin.Settings')
> +    def initialise_test(self, MockedSettings, mocked_initialise):

Arn't these the wrong way round.

> +        """
> +        Test that the initialise() method overwrites the built-in one, but still calls it
> +        """
> +        # GIVEN: A media plugin instance and a mocked settings object
> +        media_plugin = MediaPlugin()
> +        mocked_settings = MagicMock()
> +        mocked_settings.get_files_from_config.return_value = True  # Not the real value, just need something "true-ish"
> +        MockedSettings.return_value = mocked_settings
> +
> +        # WHEN: initialise() is called
> +        media_plugin.initialise()
> +
> +        # THEN: The settings should be upgraded and the base initialise() method should be called
> +        mocked_settings.get_files_from_config.assert_called_with(media_plugin)
> +        mocked_settings.setValue.assert_called_with('media/media files', True)
> +        mocked_initialise.assert_called_with()
> 


-- 
https://code.launchpad.net/~raoul-snyman/openlp/bug-1411433/+merge/252517
Your team OpenLP Core is requested to review the proposed merge of lp:~raoul-snyman/openlp/bug-1411433 into lp:openlp.