From 9bb29352085099803f4bdb3434cec34d61702bc7 Mon Sep 17 00:00:00 2001
From: Philip Ridout <phill.ridout@gmail.com>
Date: Sat, 30 Sep 2017 22:45:54 +0100
Subject: [PATCH] Tidy ups

---
 openlp/plugins/songs/lib/importers/songbeamer.py | 12 +++++++-----
 openlp/plugins/songs/lib/importers/songimport.py |  4 +---
 openlp/plugins/songs/lib/importers/videopsalm.py |  7 ++-----
 openlp/plugins/songs/lib/upgrade.py              |  1 -
 4 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/openlp/plugins/songs/lib/importers/songbeamer.py b/openlp/plugins/songs/lib/importers/songbeamer.py
index 609fec1e2..346b1588d 100644
--- a/openlp/plugins/songs/lib/importers/songbeamer.py
+++ b/openlp/plugins/songs/lib/importers/songbeamer.py
@@ -29,6 +29,7 @@ import base64
 import math
 
 from openlp.core.common import Settings, is_win, is_macosx, get_file_encoding
+from openlp.core.common.path import Path
 from openlp.plugins.songs.lib import VerseType
 from openlp.plugins.songs.lib.importers.songimport import SongImport
 
@@ -412,14 +413,15 @@ class SongBeamerImport(SongImport):
         """
         # The path is relative to SongBeamers Song folder
         if is_win():
-            user_doc_folder = os.path.expandvars('$DOCUMENTS')
+            user_doc_path = Path(os.path.expandvars('$DOCUMENTS'))
         elif is_macosx():
-            user_doc_folder = os.path.join(os.path.expanduser('~'), 'Documents')
+            user_doc_path = Path.home() / 'Documents'
         else:
             # SongBeamer only runs on mac and win...
             return
-        audio_file_path = os.path.normpath(os.path.join(user_doc_folder, 'SongBeamer', 'Songs', audio_file_path))
-        if os.path.isfile(audio_file_path):
+        audio_file_path = user_doc_path / 'SongBeamer' / 'Songs' / audio_file_path
+        if audio_file_path.is_file():
             self.add_media_file(audio_file_path)
         else:
-            log.debug('Could not import mediafile "%s" since it does not exists!' % audio_file_path)
+            log.debug('Could not import mediafile "{audio_file_path}" since it does not exists!'
+                      .format(audio_file_path=audio_file_path))
diff --git a/openlp/plugins/songs/lib/importers/songimport.py b/openlp/plugins/songs/lib/importers/songimport.py
index 510ca4e3a..33689525f 100644
--- a/openlp/plugins/songs/lib/importers/songimport.py
+++ b/openlp/plugins/songs/lib/importers/songimport.py
@@ -270,12 +270,10 @@ class SongImport(QtCore.QObject):
             return
         self.authors.append((author, type))
 
-    def add_media_file(self, filename, weight=0):
+    def add_media_file(self, file_path, weight=0):
         """
         Add a media file to the list
         """
-        # TODO: File path
-        file_path = Path(filename)
         if file_path in [x[0] for x in self.media_files]:
             return
         self.media_files.append((file_path, weight))
diff --git a/openlp/plugins/songs/lib/importers/videopsalm.py b/openlp/plugins/songs/lib/importers/videopsalm.py
index cbfa7508f..0e14481a5 100644
--- a/openlp/plugins/songs/lib/importers/videopsalm.py
+++ b/openlp/plugins/songs/lib/importers/videopsalm.py
@@ -87,10 +87,7 @@ class VideoPsalmImport(SongImport):
             songs = songbook['Songs']
             self.import_wizard.progress_bar.setMaximum(len(songs))
             songbook_name = songbook['Text']
-            # TODO: This probably never worked!
-            # Originally: media_folder = os.path.normpath(os.path.join(os.path.dirname(song_file.name), '..', 'Audio'))
-            # song_file.name returned just the file name, so it had no directory to operate on.
-            media_folder = Path('..', 'Audio')
+            media_path = Path('..', 'Audio')
             for song in songs:
                 self.song_book_name = songbook_name
                 if 'Text' in song:
@@ -115,7 +112,7 @@ class VideoPsalmImport(SongImport):
                 if 'Theme' in song:
                     self.topics = song['Theme'].splitlines()
                 if 'AudioFile' in song:
-                    self.add_media_file(str(media_folder / song['AudioFile']))
+                    self.add_media_file(media_path / song['AudioFile'])
                 if 'Memo1' in song:
                     self.add_comment(song['Memo1'])
                 if 'Memo2' in song:
diff --git a/openlp/plugins/songs/lib/upgrade.py b/openlp/plugins/songs/lib/upgrade.py
index 5cf24b0b6..bc7c95624 100644
--- a/openlp/plugins/songs/lib/upgrade.py
+++ b/openlp/plugins/songs/lib/upgrade.py
@@ -172,7 +172,6 @@ def upgrade_7(session, metadata):
     """
     Version 7 upgrade - Move file path from old db to JSON encoded path to new db. Upgrade added in 2.5 dev
     """
-    # TODO: Test
     log.debug('Starting upgrade_7 for file_path to JSON')
     old_table = Table('media_files', metadata, autoload=True)
     if 'file_path' not in [col.name for col in old_table.c.values()]: