openlp-core team mailing list archive

[Samuel Mehrbrodt <s.mehrbrodt@xxxxxxxxx>]

Samuel Mehrbrodt has proposed merging lp:~sam92/openlp/bug-1326664 into lp:openlp.

Requested reviews:
  OpenLP Core (openlp-core)
Related bugs:
  Bug #1326664 in OpenLP: "Can't import EasyWorship DB"
  https://bugs.launchpad.net/openlp/+bug/1326664

For more details, see:
https://code.launchpad.net/~sam92/openlp/bug-1326664/+merge/222471

Add proper error messages to EasyWorship import.
Also make sure that Songs.db (lowercase ending) is also being recognized.
-- 
https://code.launchpad.net/~sam92/openlp/bug-1326664/+merge/222471
Your team OpenLP Core is requested to review the proposed merge of lp:~sam92/openlp/bug-1326664 into lp:openlp.

=== modified file 'openlp/plugins/songs/lib/ewimport.py'
--- openlp/plugins/songs/lib/ewimport.py	2014-05-21 09:33:23 +0000
+++ openlp/plugins/songs/lib/ewimport.py	2014-06-09 08:00:49 +0000
@@ -200,11 +200,20 @@
         Import the songs from the database
         """
         # Open the DB and MB files if they exist
-        import_source_mb = self.import_source.replace('.DB', '.MB')
-        if not os.path.isfile(self.import_source) or not os.path.isfile(import_source_mb):
+        import_source_mb = self.import_source.replace('.DB', '.MB').replace('.db', '.mb')
+        if not os.path.isfile(self.import_source):
+            self.log_error(self.import_source, translate('SongsPlugin.EasyWorshipSongImport',
+                                                         'This file does not exist.'))
+            return
+        if not os.path.isfile(import_source_mb):
+            self.log_error(self.import_source, translate('SongsPlugin.EasyWorshipSongImport',
+                                                         'Could not find the "Songs.MB" file. It must be in the same '
+                                                         'folder as the "Songs.DB" file.'))
             return
         db_size = os.path.getsize(self.import_source)
         if db_size < 0x800:
+            self.log_error(self.import_source, translate('SongsPlugin.EasyWorshipSongImport',
+                                                         'This file is no valid EasyWorship Database.'))
             return
         db_file = open(self.import_source, 'rb')
         self.memo_file = open(import_source_mb, 'rb')
@@ -213,6 +222,8 @@
         if header_size != 0x800 or block_size < 1 or block_size > 4:
             db_file.close()
             self.memo_file.close()
+            self.log_error(self.import_source, translate('SongsPlugin.EasyWorshipSongImport',
+                                                         'This file is no valid EasyWorship Database.'))
             return
         # Take a stab at how text is encoded
         self.encoding = 'cp1252'
@@ -240,6 +251,8 @@
             self.encoding = 'cp874'
         self.encoding = retrieve_windows_encoding(self.encoding)
         if not self.encoding:
+            self.log_error(self.import_source, translate('SongsPlugin.EasyWorshipSongImport',
+                                                         'Could not retrieve encoding.'))
             return
         # Read the field description information
         db_file.seek(120)

=== modified file 'tests/functional/openlp_plugins/songs/test_ewimport.py'
--- tests/functional/openlp_plugins/songs/test_ewimport.py	2014-05-11 04:45:30 +0000
+++ tests/functional/openlp_plugins/songs/test_ewimport.py	2014-06-09 08:00:49 +0000
@@ -314,6 +314,26 @@
             mocked_os_path.isfile.assert_any_call('Songs.DB')
             mocked_os_path.isfile.assert_any_call('Songs.MB')
 
+    def do_import_source_invalid_test(self):
+        """
+        Test the :mod:`do_import` module produces an error when Songs.MB not found.
+        """
+        # GIVEN: A mocked out SongImport class, a mocked out "manager"
+        with patch('openlp.plugins.songs.lib.ewimport.SongImport'), \
+                patch('openlp.plugins.songs.lib.ewimport.os.path') as mocked_os_path:
+            mocked_manager = MagicMock()
+            importer = EasyWorshipSongImport(mocked_manager, filenames=[])
+            importer.log_error = MagicMock()
+            mocked_os_path.isfile.side_effect = [True, False]
+
+            # WHEN: do_import is supplied with an import source (Songs.MB missing)
+            importer.import_source = 'Songs.DB'
+            importer.do_import()
+
+            # THEN: do_import should have logged an error that the Songs.MB file could not be found.
+            importer.log_error.assert_any_call(importer.import_source, 'Could not find the "Songs.MB" file. It must be '
+                                                                       'in the same folder as the "Songs.DB" file.')
+
     def do_import_database_validity_test(self):
         """
         Test the :mod:`do_import` module handles invalid database files correctly