From 74a72877d8408113f6ef6d1bf36893abc2424986 Mon Sep 17 00:00:00 2001
From: Raoul Snyman <raoul@snyman.info>
Date: Wed, 23 Sep 2020 15:25:37 -0700
Subject: [PATCH] Fix two bugs in the MediaShout importer

- Fix a bug where the "Themes" table was never read due to an incorrect "if" statement
- Fix a bug where the "Groups" table was presumed to exist, but doesn't always (bug #35)
---
 .../plugins/songs/lib/importers/mediashout.py | 12 ++++---
 .../openlp_plugins/songs/test_mediashout.py   | 35 +++++++++++++++++++
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/openlp/plugins/songs/lib/importers/mediashout.py b/openlp/plugins/songs/lib/importers/mediashout.py
index 08da694da..b12b7145c 100644
--- a/openlp/plugins/songs/lib/importers/mediashout.py
+++ b/openlp/plugins/songs/lib/importers/mediashout.py
@@ -76,13 +76,15 @@ class MediaShoutImport(SongImport):
             cursor.execute('SELECT Type, Number, POrder FROM PlayOrder WHERE Record = ? ORDER BY POrder',
                            float(song.Record))
             verse_order = cursor.fetchall()
-            if cursor.tables(table='TableName', tableType='TABLE').fetchone():
+            topics = []
+            if cursor.tables(table='Themes', tableType='TABLE').fetchone():
                 cursor.execute('SELECT Name FROM Themes INNER JOIN SongThemes ON SongThemes.ThemeId = Themes.ThemeId '
                                'WHERE SongThemes.Record = ?', float(song.Record))
-                topics = cursor.fetchall()
-            cursor.execute('SELECT Name FROM Groups INNER JOIN SongGroups ON SongGroups.GroupId = Groups.GroupId '
-                           'WHERE SongGroups.Record = ?', float(song.Record))
-            topics += cursor.fetchall()
+                topics += cursor.fetchall()
+            if cursor.tables(table='Groups', tableType='TABLE').fetchone():
+                cursor.execute('SELECT Name FROM Groups INNER JOIN SongGroups ON SongGroups.GroupId = Groups.GroupId '
+                               'WHERE SongGroups.Record = ?', float(song.Record))
+                topics += cursor.fetchall()
             self.process_song(song, verses, verse_order, topics)
 
     def process_song(self, song, verses, verse_order, topics):
diff --git a/tests/functional/openlp_plugins/songs/test_mediashout.py b/tests/functional/openlp_plugins/songs/test_mediashout.py
index 6d326505a..33094fc32 100644
--- a/tests/functional/openlp_plugins/songs/test_mediashout.py
+++ b/tests/functional/openlp_plugins/songs/test_mediashout.py
@@ -116,6 +116,41 @@ class TestMediaShoutImport(TestCase):
         assert expected_execute_calls == mocked_cursor.execute.call_args_list
         mocked_process_song.assert_called_once_with(song, [verse], [play_order], [theme, group])
 
+    @patch('openlp.plugins.songs.lib.importers.mediashout.pyodbc')
+    def test_do_import_without_topics(self, mocked_pyodbc):
+        """
+        Test the MediaShoutImport do_import method with the Themes and Groups tables don't exist
+        """
+        SongRecord = namedtuple('SongRecord', 'Record, Title, Author, Copyright, SongID, CCLI, Notes')
+        VerseRecord = namedtuple('VerseRecord', 'Type, Number, Text')
+        PlayOrderRecord = namedtuple('PlayOrderRecord', 'Type, Number, POrder')
+        song = SongRecord(1, 'Amazing Grace', 'William Wilberforce', 'Public Domain', 1, '654321', '')
+        verse = VerseRecord('Verse', 1, 'Amazing grace, how sweet the sound\nThat saved a wretch like me')
+        play_order = PlayOrderRecord('Verse', 1, 1)
+
+        # GIVEN: A MediaShoutImport instance and a bunch of stuff mocked out
+        importer = MediaShoutImport(MagicMock(), file_path='mediashout.db')
+        mocked_cursor = MagicMock()
+        mocked_cursor.fetchall.side_effect = [[song], [verse], [play_order]]
+        mocked_cursor.tables.fetchone.return_value = False
+        mocked_connection = MagicMock()
+        mocked_connection.cursor.return_value = mocked_cursor
+        mocked_pyodbc.connect.return_value = mocked_connection
+
+        # WHEN: do_import is called
+        with patch.object(importer, 'import_wizard'), \
+                patch.object(importer, 'process_song') as mocked_process_song:
+            importer.do_import()
+
+        # THEN: The songs should have been imported
+        expected_execute_calls = [
+            call('SELECT Record, Title, Author, Copyright, SongID, CCLI, Notes FROM Songs ORDER BY Title'),
+            call('SELECT Type, Number, Text FROM Verses WHERE Record = ? ORDER BY Type, Number', 1.0),
+            call('SELECT Type, Number, POrder FROM PlayOrder WHERE Record = ? ORDER BY POrder', 1.0)
+        ]
+        assert expected_execute_calls == mocked_cursor.execute.call_args_list
+        mocked_process_song.assert_called_once_with(song, [verse], [play_order])
+
     @patch('openlp.plugins.songs.lib.importers.mediashout.pyodbc')
     def test_do_import_breaks_on_stop(self, mocked_pyodbc):
         """