Merge lp:~ahayzen/music-app/fix-1407814-save-order-correctly into lp:music-app/remix

[Andrew Hayzen]

* Fix for incorrect saving of queue

* Fix for incorrect saving of queue

The code used to start at index number 1, this has now been fixed so it starts at 0, which caused reordering/removals later on to break.

There is the potential that the queue may already be corrupted (or start at 1 instead of 0), this will remain 'corrupt' until the user clears the queue or selects to play an album (which clears the queue first) etc.

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

PASSED: Continuous integration, rev:771
http://91.189.93.70:8080/job/music-app-ci/1179/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/558
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/558/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/31

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

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

The refactor to get the next index inside the request is a good idea, nice work. This does seem to fix the problem. I'll try to do more testing tonight/tomorrow night.

[lp:~ahayzen/music-app/fix-1407814-save-order-correctly]

772. By Andrew Hayzen on 2015-01-06

* Ensure that getNextIndex() is done in 1 transaction

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

PASSED: Continuous integration, rev:772
http://91.189.93.70:8080/job/music-app-ci/1180/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/559
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/559/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/32

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

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

This doesn't seem to fix lp:1407840. Also, currently adding an album (Step #2 in the bug) at first caused the items in the queue to not be what was expected [1]. The first 3 songs in that screenshot should not be there and the currently playing item should still be what is in the toolbar.

[1] http://i.imgur.com/VArr7Ou.png

review: Needs Fixing

[Andrew Hayzen (ahayzen)]

This mp [2] fixes various issues todo with remove i propose to hold this mp until that has landed as this may then resolve bug 1407840.

2 - https://code.launchpad.net/~andrew-hayzen/music-app/fix-1409186-queue-delete-refactor/+merge/246036

[lp:~ahayzen/music-app/fix-1407814-save-order-correctly]

773. By Andrew Hayzen on 2015-01-11

* Merge of trunk

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

PASSED: Continuous integration, rev:773
http://91.189.93.70:8080/job/music-app-ci/1201/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/633
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/633/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/53

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

review: Approve (continuous-integration)

[lp:~ahayzen/music-app/fix-1407814-save-order-correctly]

774. By Andrew Hayzen on 2015-01-11

* Merge of trunk

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

PASSED: Continuous integration, rev:774
http://91.189.93.70:8080/job/music-app-ci/1206/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/640
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/640/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/58

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

review: Approve (continuous-integration)

[lp:~ahayzen/music-app/fix-1407814-save-order-correctly]

775. By Andrew Hayzen on 2015-01-11

* Merge of trunk

776. By Andrew Hayzen on 2015-01-11

* Fix for wrong use of for in, use for i=0; i < length; i++ instead

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

PASSED: Continuous integration, rev:776
http://91.189.93.70:8080/job/music-app-ci/1208/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/643
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/643/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/60

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

review: Approve (continuous-integration)

[lp:~ahayzen/music-app/fix-1407814-save-order-correctly]

777. By Andrew Hayzen on 2015-01-11

* Add ORDER BY to SELECT when resorting the indexes in a removeQueueList
* Use -1 as the temporary index for reorder instead of the calc'd max

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

PASSED: Continuous integration, rev:777
http://91.189.93.70:8080/job/music-app-ci/1209/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-vivid/645
        deb: http://91.189.93.70:8080/job/generic-mediumtests-vivid/645/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-vivid-amd64-ci/61

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

review: Approve (continuous-integration)

[Victor Thompson (vthompson)]

LGTM! Queue saving is working fine now. Thanks!

review: Approve

=== modified file 'meta-database.js'
--- meta-database.js	2015-01-10 17:36:58 +0000
+++ meta-database.js	2015-01-11 18:44:28 +0000
@@ -22,12 +22,17 @@
      return LocalStorage.openDatabaseSync("music-app-metadata", "1.0", "StorageDatabase", 1000000);
 }
 
-function createQueue() {
-    var db = getDatabase();
-    db.transaction(
-        function(tx) {
-            tx.executeSql("CREATE TABLE IF NOT EXISTS queue(ind INTEGER NOT NULL, filename TEXT)");
-      });
+function createQueue(tx) {
+    if (tx === undefined) {
+        var db = getDatabase();
+        db.transaction(
+            function(tx) {
+                createQueue(tx)
+            }
+        )
+    } else {
+        tx.executeSql("CREATE TABLE IF NOT EXISTS queue(ind INTEGER NOT NULL, filename TEXT)");
+    }
 }
 
 function clearQueue() {
@@ -39,11 +44,13 @@
       });
 }
 
-function addQueueItem(ind,filename) {
+function addQueueItem(filename) {
     var db = getDatabase();
     var res="";
 
     db.transaction(function(tx) {
+        var ind = getNextIndex(tx);
+
         var rs = tx.executeSql('INSERT OR REPLACE INTO queue (ind, filename) VALUES (?,?);', [ind, filename]);
               if (rs.rowsAffected > 0) {
                 console.log("QUEUE add OK")
@@ -56,10 +63,12 @@
     );
 }
 
-function addQueueList(ind, items) {
+function addQueueList(items) {
     var db = getDatabase();
 
     db.transaction(function(tx) {
+        var ind = getNextIndex(tx);
+
         for (var i = 0; i < items.length; i++) {
             tx.executeSql('INSERT OR REPLACE INTO queue (ind, filename) VALUES (?,?);', [i + ind, items[i].filename]);
         }
@@ -67,21 +76,34 @@
     );
 }
 
+// Get the next index for the queue
+function getNextIndex(tx) {
+    var ind;
+
+    if (tx === undefined) {
+        var db = getDatabase();
+
+        db.transaction(function(tx) {
+            ind = getNextIndex(tx);
+        });
+    } else {
+        var rs = tx.executeSql('SELECT MAX(ind) FROM queue')
+        ind = isQueueEmpty(tx) ? 0 : rs.rows.item(0)["MAX(ind)"] + 1
+    }
+
+    return ind;
+}
+
 function moveQueueItem(from, to) {
     var db = getDatabase();
     var res="";
 
     db.transaction(function(tx) {
-
-        // Generate new index number if records exist, otherwise use 0
-        var rs = tx.executeSql('SELECT MAX(ind) FROM queue')
-
-        var nextIndex = isNaN(rs.rows.item(0)["MAX(ind)"]) ? 0 : rs.rows.item(
-                                                                   0)["MAX(ind)"] + 1
-
+        // Track to move put as -1 for now
         tx.executeSql('UPDATE queue SET ind=? WHERE ind=?;',
-                      [nextIndex, from])
+                      [-1, from])
 
+        // Shuffle tracks inbetween from->to
         if (from > to) {
             for (var i = from-1; i >= to; i--) {
                 tx.executeSql('UPDATE queue SET ind=? WHERE ind=?;',
@@ -94,8 +116,9 @@
             }
         }
 
+        // Switch moving track to its new position
         tx.executeSql('UPDATE queue SET ind=? WHERE ind=?;',
-                      [to, nextIndex])
+                      [to, -1])
 
     })
 }
@@ -109,8 +132,8 @@
 
         var rs = tx.executeSql('SELECT MAX(ind) FROM queue')
 
-        var lastIndex = isNaN(rs.rows.item(0)["MAX(ind)"]) ? 0 : rs.rows.item(
-                                                           0)["MAX(ind)"]
+        var lastIndex = isQueueEmpty() ? 0 : rs.rows.item(0)["MAX(ind)"]
+
         for(var i = ind+1; i <= lastIndex; i++) {
             tx.executeSql('UPDATE queue SET ind=? WHERE ind=?;',
                           [i-1, i])
@@ -125,18 +148,19 @@
 function removeQueueList(list)
 {
     var db = getDatabase()
+    var i;
     var res = false
 
     db.transaction(function (tx) {
         // Remove all the deleted indexes
-        for (var ind in list) {
-            tx.executeSql('DELETE FROM queue WHERE ind=?;', [ind])
+        for (i=0; i < list.length; i++) {
+            tx.executeSql('DELETE FROM queue WHERE ind=?;', [list[i]])
         }
 
         // Rebuild queue in order
-        var rs = tx.executeSql('SELECT ind FROM queue')
+        var rs = tx.executeSql('SELECT ind FROM queue ORDER BY ind ASC')
 
-        for (var i=0; i < rs.rows.length; i++) {
+        for (i=0; i < rs.rows.length; i++) {
             tx.executeSql('UPDATE queue SET ind=? WHERE ind=?;',
                           [i, rs.rows.item(i).ind])
         }
@@ -160,16 +184,23 @@
     return res;
 }
 
-function isQueueEmpty() {
-    var db = getDatabase();
-    var res = 0;
-
-    db.transaction( function(tx) {
-        createQueue();
+function isQueueEmpty(tx) {
+    var empty = false;
+
+    if (tx === undefined) {
+        var db = getDatabase();
+        var res = 0;
+
+        db.transaction( function(tx) {
+            empty = isQueueEmpty(tx)
+        });
+    } else {
+        createQueue(tx);
         var rs = tx.executeSql("SELECT count(*) as value FROM queue")
-        res = rs.rows.item(0).value === 0
-    });
-    return res
+        empty = rs.rows.item(0).value === 0
+    }
+
+    return empty
 }
 
 function createRecent() {

=== modified file 'music-app.qml'
--- music-app.qml	2015-01-11 17:04:55 +0000
+++ music-app.qml	2015-01-11 18:44:28 +0000
@@ -606,7 +606,6 @@
         }
 
         var items = []
-        var previousQueueSize = trackQueue.model.count
 
         for (var i=0; i < model.rowCount; i++) {
             items.push(model.get(i, model.RoleModelData))
@@ -615,7 +614,7 @@
         }
 
         // Add model to queue storage
-        Library.addQueueList(previousQueueSize, items);
+        Library.addQueueList(items);
     }
 
     // Converts an duration in ms to a formated string ("minutes:seconds")
@@ -803,7 +802,7 @@
         function append(listElement)
         {
             model.append(makeDict(listElement))
-            Library.addQueueItem(trackQueue.model.count, listElement.filename)
+            Library.addQueueItem(listElement.filename)
         }
 
         function clear()