Ubuntu Music App

Merge lp:~ahayzen/music-app/remix-add-remove-playlists-recent-out-of-sync into lp:music-app/remix

  1. remix-add-remove-playlists-recent-out-of-sync
  2. Merge into remix
Proposed by Andrew Hayzen
Status: Merged
Approved by: Victor Thompson
Approved revision: 722
Merged at revision: 720
Proposed branch: lp:~ahayzen/music-app/remix-add-remove-playlists-recent-out-of-sync
Merge into: lp:music-app/remix
Diff against target: 222 lines (+79/-43)
3 files modified
MusicaddtoPlaylist.qml (+22/-2)
common/ListItemActions/AddToPlaylist.qml (+2/-1)
common/SongsPage.qml (+55/-40)
To merge this branch: bzr merge lp:~ahayzen/music-app/remix-add-remove-playlists-recent-out-of-sync
Reviewer Review Type Date Requested Status
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Victor Thompson Approve
Review via email: mp+239931@code.launchpad.net

Commit message

* Fix for cover art getting out of sync on the SongsPage.qml
* Fix for addToPlaylists causing freeze to songsPage
* Ensure recent/playlists pages remain in sync as items are added/removed from the playlist

Description of the change

* Fix for cover art getting out of sync on the SongsPage.qml
* Fix for addToPlaylists causing freeze to songsPage
* Ensure recent/playlists pages remain in sync as items are added/removed from the playlist

To post a comment you must log in.
Revision history for this message
Victor Thompson (vthompson) wrote :

Can we consolidate some of this reused code to a helper function? I count at least 4 or 5 instances of the same logic.

review: Needs Fixing
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

FAILED: Continuous integration, rev:720
http://91.189.93.70:8080/job/music-app-remix-ci/212/
Executed test runs:
    UNSTABLE: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1372
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1372/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/212

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

review: Needs Fixing (continuous-integration)
lp:~ahayzen/music-app/remix-add-remove-playlists-recent-out-of-sync updated
721. By Andrew Hayzen

* Make generic helper

722. By Andrew Hayzen

* Fix so that 1 level addToPlaylist works again

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:721
http://91.189.93.70:8080/job/music-app-remix-ci/213/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1373
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1373/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/213

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

review: Approve (continuous-integration)
Revision history for this message
Victor Thompson (vthompson) wrote :

LGTM and I can not find any regressions.

review: Approve
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :

PASSED: Continuous integration, rev:722
http://91.189.93.70:8080/job/music-app-remix-ci/215/
Executed test runs:
    SUCCESS: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1376
        deb: http://91.189.93.70:8080/job/generic-mediumtests-utopic-python3/1376/artifact/work/output/*zip*/output.zip
    SUCCESS: http://91.189.93.70:8080/job/music-app-remix-utopic-amd64-ci/215

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

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'MusicaddtoPlaylist.qml'
--- MusicaddtoPlaylist.qml 2014-10-24 18:18:07 +0000
+++ MusicaddtoPlaylist.qml 2014-10-29 03:01:56 +0000
@@ -23,6 +23,7 @@
23import Ubuntu.Components.ListItems 1.0 as ListItem23import Ubuntu.Components.ListItems 1.0 as ListItem
24import Ubuntu.Components.Popups 1.024import Ubuntu.Components.Popups 1.0
25import QtQuick.LocalStorage 2.025import QtQuick.LocalStorage 2.0
26import "meta-database.js" as Library
26import "playlists.js" as Playlists27import "playlists.js" as Playlists
27import "common"28import "common"
2829
@@ -42,6 +43,7 @@
42 visible: false43 visible: false
4344
44 property var chosenElements: []45 property var chosenElements: []
46 property var page
4547
46 head {48 head {
47 actions: [49 actions: [
@@ -85,8 +87,26 @@
85 Playlists.addToPlaylist(name, chosenElements[i])87 Playlists.addToPlaylist(name, chosenElements[i])
86 }88 }
8789
88 playlistModel.filterPlaylists();90 // Check that the parent parent page is not being refiltered
89 albumTracksModel.filterPlaylistTracks(name)91 if (page !== undefined && page.page !== undefined && page.page.title === i18n.tr("Playlists")) {
92 page.page.changed = true
93 } else {
94 playlistModel.filterPlaylists();
95 }
96
97 if (Library.recentContainsPlaylist(name)) {
98 // Check that the parent parent page is not being refiltered
99 if (page !== undefined && page.page !== undefined && page.page.title === i18n.tr("Recent")) {
100 page.page.changed = true
101 } else {
102 recentModel.filterRecent()
103 }
104 }
105
106 if (page !== undefined && name === page.line2 && page.playlistChanged !== undefined) {
107 page.playlistChanged = true
108 page.covers = Playlists.getPlaylistCovers(name)
109 }
90110
91 musicToolbar.goBack(); // go back to the previous page111 musicToolbar.goBack(); // go back to the previous page
92 }112 }
93113
=== modified file 'common/ListItemActions/AddToPlaylist.qml'
--- common/ListItemActions/AddToPlaylist.qml 2014-10-26 16:18:12 +0000
+++ common/ListItemActions/AddToPlaylist.qml 2014-10-29 03:01:56 +0000
@@ -25,12 +25,13 @@
25 text: i18n.tr("Add to playlist")25 text: i18n.tr("Add to playlist")
2626
27 property bool primed: false27 property bool primed: false
28 property var page
2829
29 onTriggered: {30 onTriggered: {
30 console.debug("Debug: Add track to playlist");31 console.debug("Debug: Add track to playlist");
3132
32 var comp = Qt.createComponent("../../MusicaddtoPlaylist.qml")33 var comp = Qt.createComponent("../../MusicaddtoPlaylist.qml")
33 var addToPlaylist = comp.createObject(mainPageStack, {"chosenElements": [makeDict(model)]});34 var addToPlaylist = comp.createObject(mainPageStack, {"chosenElements": [makeDict(model)], "page": page});
3435
35 if (addToPlaylist == null) { // Error Handling36 if (addToPlaylist == null) { // Error Handling
36 console.log("Error creating object");37 console.log("Error creating object");
3738
=== modified file 'common/SongsPage.qml'
--- common/SongsPage.qml 2014-10-29 00:49:40 +0000
+++ common/SongsPage.qml 2014-10-29 03:01:56 +0000
@@ -45,6 +45,44 @@
45 property alias album: songsModel.album45 property alias album: songsModel.album
46 property alias genre: songsModel.genre46 property alias genre: songsModel.genre
4747
48 property bool playlistChanged: false
49
50 onVisibleChanged: {
51 if (playlistChanged) {
52 playlistChanged = false
53 refreshWaitTimer.start()
54 }
55 }
56
57 Timer { // FIXME: workaround for when the playlist is deleted and the delegate being deleting causes freezing
58 id: refreshWaitTimer
59 interval: 250
60 onTriggered: albumTracksModel.filterPlaylistTracks(line2)
61 }
62
63 function playlistChangedHelper()
64 {
65 // if parent Playlists then set changed otherwise refilter
66 if (songStackPage.page.title === i18n.tr("Playlists")) {
67 if (songStackPage.page !== undefined) {
68 songStackPage.page.changed = true
69 }
70 } else {
71 playlistModel.filterPlaylists()
72 }
73
74 if (Library.recentContainsPlaylist(songStackPage.line2)) {
75 // if parent Recent then set changed otherwise refilter
76 if (songStackPage.page.title === i18n.tr("Recent")) {
77 if (songStackPage.page !== undefined) {
78 songStackPage.page.changed = true
79 }
80 } else {
81 recentModel.filterRecent()
82 }
83 }
84 }
85
48 state: albumtrackslist.state === "multiselectable" ? "selection" : (songStackPage.line1 === i18n.tr("Playlist") ? "playlist" : "album")86 state: albumtrackslist.state === "multiselectable" ? "selection" : (songStackPage.line1 === i18n.tr("Playlist") ? "playlist" : "album")
49 states: [87 states: [
50 PageHeadState {88 PageHeadState {
@@ -115,7 +153,7 @@
115 }153 }
116154
117 var comp = Qt.createComponent("../MusicaddtoPlaylist.qml")155 var comp = Qt.createComponent("../MusicaddtoPlaylist.qml")
118 var addToPlaylist = comp.createObject(mainPageStack, {"chosenElements": items});156 var addToPlaylist = comp.createObject(mainPageStack, {"chosenElements": items, "page": songStackPage});
119157
120 if (addToPlaylist == null) { // Error Handling158 if (addToPlaylist == null) { // Error Handling
121 console.log("Error creating object");159 console.log("Error creating object");
@@ -157,8 +195,12 @@
157195
158 albumtrackslist.closeSelection()196 albumtrackslist.closeSelection()
159197
160 songStackPage.page.changed = true198 playlistChangedHelper() // update recent/playlist models
199
161 albumTracksModel.filterPlaylistTracks(songStackPage.line2)200 albumTracksModel.filterPlaylistTracks(songStackPage.line2)
201
202 // refresh cover art
203 songStackPage.covers = Playlists.getPlaylistCovers(songStackPage.line2)
162 }204 }
163 }205 }
164 ]206 ]
@@ -368,7 +410,7 @@
368410
369 },411 },
370 AddToPlaylist {412 AddToPlaylist {
371413 page: songStackPage
372 }414 }
373 ]415 ]
374416
@@ -399,8 +441,12 @@
399 onTriggered: {441 onTriggered: {
400 Playlists.removeFromPlaylist(songStackPage.line2, model.i)442 Playlists.removeFromPlaylist(songStackPage.line2, model.i)
401443
402 songStackPage.page.changed = true444 playlistChangedHelper() // update recent/playlist models
445
403 albumTracksModel.filterPlaylistTracks(songStackPage.line2)446 albumTracksModel.filterPlaylistTracks(songStackPage.line2)
447
448 // refresh cover art
449 songStackPage.covers = Playlists.getPlaylistCovers(songStackPage.line2)
404 }450 }
405 }451 }
406 }452 }
@@ -470,28 +516,13 @@
470 console.debug("Debug: User changed name from "+playlistName.placeholderText+" to "+playlistName.text)516 console.debug("Debug: User changed name from "+playlistName.placeholderText+" to "+playlistName.text)
471517
472 if (Playlists.renamePlaylist(playlistName.placeholderText, playlistName.text) === true) {518 if (Playlists.renamePlaylist(playlistName.placeholderText, playlistName.text) === true) {
473 // if parent Playlists then set changed otherwise refilter
474 if (songStackPage.page.title === i18n.tr("Playlists")) {
475 if (songStackPage.page !== undefined) {
476 songStackPage.page.changed = true
477 }
478 } else {
479 playlistModel.filterPlaylists()
480 }
481519
482 if (Library.recentContainsPlaylist(playlistName.placeholderText)) {520 if (Library.recentContainsPlaylist(playlistName.placeholderText)) {
483 Library.recentRenamePlaylist(playlistName.placeholderText, playlistName.text)521 Library.recentRenamePlaylist(playlistName.placeholderText, playlistName.text)
484
485 // if parent Recent then set changed otherwise refilter
486 if (songStackPage.page.title === i18n.tr("Recent")) {
487 if (songStackPage.page !== undefined) {
488 songStackPage.page.changed = true
489 }
490 } else {
491 recentModel.filterRecent()
492 }
493 }522 }
494523
524 playlistChangedHelper() // update recent/playlist models
525
495 PopupUtils.close(dialogEditPlaylist)526 PopupUtils.close(dialogEditPlaylist)
496527
497 line2 = playlistName.text528 line2 = playlistName.text
@@ -533,25 +564,9 @@
533564
534 if (Library.recentContainsPlaylist(dialogRemovePlaylist.oldPlaylistName)) {565 if (Library.recentContainsPlaylist(dialogRemovePlaylist.oldPlaylistName)) {
535 Library.recentRemovePlaylist(dialogRemovePlaylist.oldPlaylistName)566 Library.recentRemovePlaylist(dialogRemovePlaylist.oldPlaylistName)
536567 }
537 // if parent Recent then set changed otherwise refilter568
538 if (songStackPage.page.title === i18n.tr("Recent")) {569 playlistChangedHelper() // update recent/playlist models
539 if (songStackPage.page !== undefined) {
540 songStackPage.page.changed = true
541 }
542 } else {
543 recentModel.filterRecent()
544 }
545 }
546
547 // if parent Playlists then set changed otherwise refilter
548 if (songStackPage.page.title === i18n.tr("Playlists")) {
549 if (songStackPage.page !== undefined) {
550 songStackPage.page.changed = true
551 }
552 } else {
553 playlistModel.filterPlaylists()
554 }
555570
556 songStackPage.page = undefined571 songStackPage.page = undefined
557 PopupUtils.close(dialogRemovePlaylist)572 PopupUtils.close(dialogRemovePlaylist)

Subscribers

People subscribed via source and target branches