Merge lp:~vthompson/music-app/lookup-file into lp:music-app/trusty

[Victor Thompson]

* Use mediascanner SongsModel.lookup() function to play song from Music Scope
* Catch the filled signal for the SongsModel used for playing an album from the Music Scope

* Use mediascanner SongsModel.lookup() function to play song from Music Scope
* Catch the filled signal for the SongsModel used for playing an album from the Music Scope

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

FAILED: Continuous integration, rev:549
http://91.189.93.70:8080/job/music-app-ci/973/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1292
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1292/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-utopic-amd64-ci/197

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-ci/973/rebuild

review: Needs Fixing (continuous-integration)

[Andrew Hayzen (ahayzen)]

Looks good so far, see inline diff comments. Also I wonder if selectedAlbum should be set back to false at the end of onFilled?

review: Needs Fixing

[Victor Thompson (vthompson)]

I'll reset selectedAlbum back to false... I don't think we'd catch the filled signal again. BUT if it's called when a new song is added by ms2 we wouldn't want the album to reload.

I think we should still have the track var be set to false by default. The code that handles lookup [1] throws an exception when found which should exit the function... leaving the track variable false as we are assuming.

[1] http://bazaar.launchpad.net/~mediascanner-team/mediascanner2/trunk/view/head:/src/mediascanner/MediaStore.cc#L394

[lp:~vthompson/music-app/lookup-file]

550. By Victor Thompson on 2014-08-06

Reset selectedAlbum... just in case.

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:550
http://91.189.93.70:8080/job/music-app-ci/982/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1334
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1334/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-utopic-amd64-ci/206

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-ci/982/rebuild

review: Approve (continuous-integration)

[Andrew Hayzen (ahayzen)]

What would happen if album A was selected in the scope, the user changed the queue, and then album A was selected again. I think nothing would happen as the model would not Fill again?

Suggest blanking the filter at the end of onFilled and then inside onFilled wrap all the code in a check to ensure the filter is not blank.

review: Needs Fixing

[lp:~vthompson/music-app/lookup-file]

551. By Victor Thompson on 2014-08-07

Clear filter

[Andrew Hayzen (ahayzen)]

Looks good :)

review: Approve

[Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot)]

PASSED: Continuous integration, rev:551
http://91.189.93.70:8080/job/music-app-ci/983/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1340
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic/1340/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-utopic-amd64-ci/207

Click here to trigger a rebuild:
http://91.189.93.70:8080/job/music-app-ci/983/rebuild

review: Approve (continuous-integration)

=== modified file 'music-app.qml'
--- music-app.qml	2014-08-06 23:41:30 +0000
+++ music-app.qml	2014-08-07 00:46:20 +0000
@@ -205,6 +205,7 @@
         target: UriHandler
 
         function processAlbum(uri) {
+            selectedAlbum = true;
             var split = uri.split("/");
 
             if (split.length < 2) {
@@ -215,16 +216,6 @@
             // Filter by artist and album
             songsAlbumArtistModel.artist = decodeURIComponent(split[0]);
             songsAlbumArtistModel.album = decodeURIComponent(split[1]);
-
-            // Play album it tracks exist
-            if (songsAlbumArtistModel.rowCount > 0) {
-                // trackClicked(model, index, play, clear=true) will clear the model
-                trackClicked(songsAlbumArtistModel, 0, true, true);
-            }
-            else {
-                console.debug("Unknown artist-album " + uri + ", skipping")
-                return;
-            }
         }
 
         function processFile(uri, play) {
@@ -232,13 +223,8 @@
 
             var track = false;
 
-            // Search for track in songs model
-            for (var i=0; i < allSongsModel.rowCount; i++) {
-                if (decodeURIComponent(allSongsModel.get(i, allSongsModel.RoleModelData).filename) === uri) {
-                    track = allSongsModel.get(i, allSongsModel.RoleModelData);
-                    break;
-                }
-            }
+            // Lookup track in songs model
+            track = musicStore.lookup(decodeURIComponent(uri));
 
             if (!track) {
                 console.debug("Unknown file " + uri + ", skipping")
@@ -383,6 +369,7 @@
     property string timestamp // used to scrobble
     property var chosenElement: null
     property bool toolbarShown: musicToolbar.shown
+    property bool selectedAlbum: false
     signal collapseExpand();
     signal collapseSwipeDelete(int index);
     signal onToolbarShownChanged(bool shown, var currentPage, var currentTab)
@@ -518,6 +505,20 @@
     SongsModel {
         id: songsAlbumArtistModel
         store: musicStore
+        onFilled: {
+            // Play album it tracks exist
+            if (rowCount > 0 && selectedAlbum) {
+                trackClicked(songsAlbumArtistModel, 0, true, true);
+            } else if (selectedAlbum) {
+                console.debug("Unknown artist-album " + artist + "/" + album + ", skipping")
+            }
+
+            selectedAlbum = false;
+
+            // Clear filter for artist and album
+            songsAlbumArtistModel.artist = ""
+            songsAlbumArtistModel.album = ""
+        }
     }
 
     // WHERE THE MAGIC HAPPENS