Merge lp:~tomasgroth/openlp/bugfixes7 into lp:openlp

[Tomas Groth]

Fix upgrade on song db with lost version. Fixes bug 1391638.
Treat slide notes and servicemanager notes differently in the web remote and stage view. Fixes bug 1390015.
When escaping live display stop looping to prevent display to reappear. Fixes bug 1266271.

[Tomas Groth (tomasgroth)]

CI is still having issues :(

lp:~tomasgroth/openlp/bugfixes7 (revision 2453)
[SUCCESS] http://ci.openlp.org/job/Branch-01-Pull/793/
[SUCCESS] http://ci.openlp.org/job/Branch-02-Functional-Tests/726/
[FAILURE] http://ci.openlp.org/job/Branch-03-Interface-Tests/672/

[Tim Bentley (trb143)]

Looks good.

review: Approve

[Raoul Snyman (raoul-snyman)]

review: Approve

=== modified file 'openlp/core/ui/slidecontroller.py'
--- openlp/core/ui/slidecontroller.py	2014-11-13 11:21:54 +0000
+++ openlp/core/ui/slidecontroller.py	2014-11-25 22:34:10 +0000
@@ -493,6 +493,11 @@
         """
         self.display.setVisible(False)
         self.media_controller.media_stop(self)
+        # Stop looping if active
+        if self.play_slides_loop.isChecked():
+            self.on_play_slides_loop(False)
+        elif self.play_slides_once.isChecked():
+            self.on_play_slides_once(False)
 
     def toggle_display(self, action):
         """

=== modified file 'openlp/plugins/remotes/html/openlp.js'
--- openlp/plugins/remotes/html/openlp.js	2014-09-11 09:52:32 +0000
+++ openlp/plugins/remotes/html/openlp.js	2014-11-25 22:34:10 +0000
@@ -67,8 +67,12 @@
         var ul = $("#service-manager > div[data-role=content] > ul[data-role=listview]");
         ul.html("");
         $.each(data.results.items, function (idx, value) {
+          var text = value["title"];
+          if (value["notes"]) {
+            text += ' - ' + value["notes"];
+          }
           var li = $("<li data-icon=\"false\">").append(
-            $("<a href=\"#\">").attr("value", parseInt(idx, 10)).text(value["title"]));
+            $("<a href=\"#\">").attr("value", parseInt(idx, 10)).text(text));
           li.attr("uuid", value["id"])
           li.children("a").click(OpenLP.setItem);
           ul.append(li);
@@ -98,8 +102,8 @@
           } else {
             text += slide["text"];
           }
-          if (slide["notes"]) {
-            text += ("<div style='font-size:smaller;font-weight:normal'>" + slide["notes"] + "</div>");
+          if (slide["slide_notes"]) {
+            text += ("<div style='font-size:smaller;font-weight:normal'>" + slide["slide_notes"] + "</div>");
           }
           text = text.replace(/\n/g, '<br />');
           if (slide["img"]) {

=== modified file 'openlp/plugins/remotes/html/stage.js'
--- openlp/plugins/remotes/html/stage.js	2014-07-14 12:41:27 +0000
+++ openlp/plugins/remotes/html/stage.js	2014-11-25 22:34:10 +0000
@@ -114,8 +114,8 @@
         text += "<br /><img src='" + slide["img"].replace("/thumbnails/", "/thumbnails320x240/") + "'><br />";
     }
     // use notes if available
-    if (slide["notes"]) {
-        text += '<br />' + slide["notes"];
+    if (slide["slide_notes"]) {
+        text += '<br />' + slide["slide_notes"];
     }
     text = text.replace(/\n/g, "<br />");
     $("#currentslide").html(text);

=== modified file 'openlp/plugins/remotes/lib/httprouter.py'
--- openlp/plugins/remotes/lib/httprouter.py	2014-10-27 12:52:56 +0000
+++ openlp/plugins/remotes/lib/httprouter.py	2014-11-25 22:34:10 +0000
@@ -521,7 +521,7 @@
                     if current_item.is_capable(ItemCapabilities.HasDisplayTitle):
                         item['title'] = str(frame['display_title'])
                     if current_item.is_capable(ItemCapabilities.HasNotes):
-                        item['notes'] = str(frame['notes'])
+                        item['slide_notes'] = str(frame['notes'])
                     if current_item.is_capable(ItemCapabilities.HasThumbnails) and \
                             Settings().value('remotes/thumbnails'):
                         # If the file is under our app directory tree send the portion after the match
@@ -531,8 +531,6 @@
                     item['text'] = str(frame['title'])
                     item['html'] = str(frame['title'])
                 item['selected'] = (self.live_controller.selected_row == index)
-                if current_item.notes:
-                    item['notes'] = item.get('notes', '') + '\n' + current_item.notes
                 data.append(item)
         json_data = {'results': {'slides': data}}
         if current_item:

=== modified file 'openlp/plugins/songs/lib/upgrade.py'
--- openlp/plugins/songs/lib/upgrade.py	2014-07-17 21:04:58 +0000
+++ openlp/plugins/songs/lib/upgrade.py	2014-11-25 22:34:10 +0000
@@ -32,10 +32,11 @@
 """
 import logging
 
-from sqlalchemy import Column, ForeignKey, types
+from sqlalchemy import Table, Column, ForeignKey, types
 from sqlalchemy.sql.expression import func, false, null, text
 
 from openlp.core.lib.db import get_upgrade_op
+from openlp.core.common import trace_error_handler
 
 log = logging.getLogger(__name__)
 __version__ = 4
@@ -57,12 +58,16 @@
     :param metadata:
     """
     op = get_upgrade_op(session)
-    op.drop_table('media_files_songs')
-    op.add_column('media_files', Column('song_id', types.Integer(), server_default=null()))
-    op.add_column('media_files', Column('weight', types.Integer(), server_default=text('0')))
-    if metadata.bind.url.get_dialect().name != 'sqlite':
-        # SQLite doesn't support ALTER TABLE ADD CONSTRAINT
-        op.create_foreign_key('fk_media_files_song_id', 'media_files', 'songs', ['song_id', 'id'])
+    songs_table = Table('songs', metadata, autoload=True)
+    if 'media_files_songs' in [t.name for t in metadata.tables.values()]:
+        op.drop_table('media_files_songs')
+        op.add_column('media_files', Column('song_id', types.Integer(), server_default=null()))
+        op.add_column('media_files', Column('weight', types.Integer(), server_default=text('0')))
+        if metadata.bind.url.get_dialect().name != 'sqlite':
+            # SQLite doesn't support ALTER TABLE ADD CONSTRAINT
+            op.create_foreign_key('fk_media_files_song_id', 'media_files', 'songs', ['song_id', 'id'])
+    else:
+        log.warning('Skipping upgrade_1 step of upgrading the song db')
 
 
 def upgrade_2(session, metadata):
@@ -72,8 +77,12 @@
     This upgrade adds a create_date and last_modified date to the songs table
     """
     op = get_upgrade_op(session)
-    op.add_column('songs', Column('create_date', types.DateTime(), default=func.now()))
-    op.add_column('songs', Column('last_modified', types.DateTime(), default=func.now()))
+    songs_table = Table('songs', metadata, autoload=True)
+    if 'create_date' not in [col.name for col in songs_table.c.values()]:
+        op.add_column('songs', Column('create_date', types.DateTime(), default=func.now()))
+        op.add_column('songs', Column('last_modified', types.DateTime(), default=func.now()))
+    else:
+        log.warning('Skipping upgrade_2 step of upgrading the song db')
 
 
 def upgrade_3(session, metadata):
@@ -83,10 +92,14 @@
     This upgrade adds a temporary song flag to the songs table
     """
     op = get_upgrade_op(session)
-    if metadata.bind.url.get_dialect().name == 'sqlite':
-        op.add_column('songs', Column('temporary', types.Boolean(create_constraint=False), server_default=false()))
+    songs_table = Table('songs', metadata, autoload=True)
+    if 'temporary' not in [col.name for col in songs_table.c.values()]:
+        if metadata.bind.url.get_dialect().name == 'sqlite':
+            op.add_column('songs', Column('temporary', types.Boolean(create_constraint=False), server_default=false()))
+        else:
+            op.add_column('songs', Column('temporary', types.Boolean(), server_default=false()))
     else:
-        op.add_column('songs', Column('temporary', types.Boolean(), server_default=false()))
+        log.warning('Skipping upgrade_3 step of upgrading the song db')
 
 
 def upgrade_4(session, metadata):
@@ -98,11 +111,15 @@
     # Since SQLite doesn't support changing the primary key of a table, we need to recreate the table
     # and copy the old values
     op = get_upgrade_op(session)
-    op.create_table('authors_songs_tmp',
-                    Column('author_id', types.Integer(), ForeignKey('authors.id'), primary_key=True),
-                    Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
-                    Column('author_type', types.String(), primary_key=True,
-                           nullable=False, server_default=text('""')))
-    op.execute('INSERT INTO authors_songs_tmp SELECT author_id, song_id, "" FROM authors_songs')
-    op.drop_table('authors_songs')
-    op.rename_table('authors_songs_tmp', 'authors_songs')
+    songs_table = Table('songs', metadata)
+    if 'author_type' not in [col.name for col in songs_table.c.values()]:
+        op.create_table('authors_songs_tmp',
+                        Column('author_id', types.Integer(), ForeignKey('authors.id'), primary_key=True),
+                        Column('song_id', types.Integer(), ForeignKey('songs.id'), primary_key=True),
+                        Column('author_type', types.String(), primary_key=True,
+                               nullable=False, server_default=text('""')))
+        op.execute('INSERT INTO authors_songs_tmp SELECT author_id, song_id, "" FROM authors_songs')
+        op.drop_table('authors_songs')
+        op.rename_table('authors_songs_tmp', 'authors_songs')
+    else:
+        log.warning('Skipping upgrade_4 step of upgrading the song db')

=== modified file 'tests/functional/openlp_core_ui/test_slidecontroller.py'
--- tests/functional/openlp_core_ui/test_slidecontroller.py	2014-07-22 20:06:48 +0000
+++ tests/functional/openlp_core_ui/test_slidecontroller.py	2014-11-25 22:34:10 +0000
@@ -225,6 +225,10 @@
         Registry().register('media_controller', mocked_media_controller)
         slide_controller = SlideController(None)
         slide_controller.display = mocked_display
+        play_slides = MagicMock()
+        play_slides.isChecked.return_value = False
+        slide_controller.play_slides_loop = play_slides
+        slide_controller.play_slides_once = play_slides
 
         # WHEN: live_escape() is called
         slide_controller.live_escape()

=== modified file 'tests/functional/openlp_plugins/songs/test_db.py'
--- tests/functional/openlp_plugins/songs/test_db.py	2014-07-17 21:16:00 +0000
+++ tests/functional/openlp_plugins/songs/test_db.py	2014-11-25 22:34:10 +0000
@@ -29,9 +29,15 @@
 """
 This module contains tests for the db submodule of the Songs plugin.
 """
+import os
+import shutil
 from unittest import TestCase
+from tempfile import mkdtemp
 
 from openlp.plugins.songs.lib.db import Song, Author, AuthorType
+from openlp.plugins.songs.lib import upgrade
+from openlp.core.lib.db import upgrade_db
+from tests.utils.constants import TEST_RESOURCES_PATH
 
 
 class TestDB(TestCase):
@@ -39,6 +45,18 @@
     Test the functions in the :mod:`db` module.
     """
 
+    def setUp(self):
+        """
+        Setup for tests
+        """
+        self.tmp_folder = mkdtemp()
+
+    def tearDown(self):
+        """
+        Clean up after tests
+        """
+        shutil.rmtree(self.tmp_folder)
+
     def test_add_author(self):
         """
         Test adding an author to a song
@@ -153,3 +171,37 @@
 
         # THEN: It should return the name with the type in brackets
         self.assertEqual("John Doe (Words)", display_name)
+
+    def test_upgrade_old_song_db(self):
+        """
+        Test that we can upgrade an old song db to the current schema
+        """
+        # GIVEN: An old song db
+        old_db_path = os.path.join(TEST_RESOURCES_PATH, "songs", 'songs-1.9.7.sqlite')
+        old_db_tmp_path = os.path.join(self.tmp_folder, 'songs-1.9.7.sqlite')
+        shutil.copyfile(old_db_path, old_db_tmp_path)
+        db_url = 'sqlite:///' + old_db_tmp_path
+
+        # WHEN: upgrading the db
+        updated_to_version, latest_version = upgrade_db(db_url, upgrade)
+
+        # Then the song db should have been upgraded to the latest version
+        self.assertEqual(updated_to_version, latest_version,
+                         'The song DB should have been upgrade to the latest version')
+
+    def test_upgrade_invalid_song_db(self):
+        """
+        Test that we can upgrade an invalid song db to the current schema
+        """
+        # GIVEN: A song db with invalid version
+        invalid_db_path = os.path.join(TEST_RESOURCES_PATH, "songs", 'songs-2.2-invalid.sqlite')
+        invalid_db_tmp_path = os.path.join(self.tmp_folder, 'songs-2.2-invalid.sqlite')
+        shutil.copyfile(invalid_db_path, invalid_db_tmp_path)
+        db_url = 'sqlite:///' + invalid_db_tmp_path
+
+        # WHEN: upgrading the db
+        updated_to_version, latest_version = upgrade_db(db_url, upgrade)
+
+        # Then the song db should have been upgraded to the latest version without errors
+        self.assertEqual(updated_to_version, latest_version,
+                         'The song DB should have been upgrade to the latest version')

=== added directory 'tests/resources/songs'
=== added file 'tests/resources/songs/songs-1.9.7.sqlite'
Binary files tests/resources/songs/songs-1.9.7.sqlite	1970-01-01 00:00:00 +0000 and tests/resources/songs/songs-1.9.7.sqlite	2014-11-25 22:34:10 +0000 differ
=== added file 'tests/resources/songs/songs-2.2-invalid.sqlite'
Binary files tests/resources/songs/songs-2.2-invalid.sqlite	1970-01-01 00:00:00 +0000 and tests/resources/songs/songs-2.2-invalid.sqlite	2014-11-25 22:34:10 +0000 differ