Merge lp:~etherpulse/podbird/download_wifi into lp:podbird/devel

[Eran Uzan]

Fix for the 'Provide setting option to disable downloads over cellular data' issue (lp:1456300).

Implemented by simply adding a switch to the settings and used the switch state to set the SingleDownload.allowMobileDownload flag provided by ubuntu API (Ubuntu.DownloadManager 1.2).

It will now allow manual download to work on mobile even though auto downloads are held back.

Also changed to devel branch.

[Nekhelesh Ramananthan (nik90)]

Thanks a lot for the above patch. May I ask on which device you tested this? The last time I implemented this patch, we noticed that during testing it did not work on BQ devices. And since then we haven't tested it to check if this was fixed.

review: Needs Information

[Eran Uzan (etherpulse)]

Hey Nekhelesh, I have a Meizu 5 PRO
I guess it will have to be tested on a BQ device to know if the support was added... not sure if it was as I couldn't find any bug in the ubuntu-download-manager that refer to it directly (there's was lp:1451090 but i`m not sure it`s related )

[Nekhelesh Ramananthan (nik90)]

Sorry for the late reply. My development laptop just died of old age. I am almost done getting a new development environment set up on another laptop. Looking through the code, I see a few areas which could be improved.

I think that the !mobileDl assigned to allowMobileDownload is a bit confusing to be honest. I would rather it be just mobileDl. This would make the following lines much more intuitive since Podbird should allow the user to download over mobile connection if they so wish to do so. But since you have assigned it as !mobileDl, we are forced to pass it as false in other parts of the code.

- podbird.downloadEpisode(episodeModel.get(index).image, episodeModel.get(index).name, episodeModel.get(index).guid, episodeModel.get(index).audiourl)
+ podbird.downloadEpisode(episodeModel.get(index).image, episodeModel.get(index).name, episodeModel.get(index).guid, episodeModel.get(index).audiourl, false)

- podbird.downloadEpisode(model.image, model.name, model.guid, model.audiourl)
+ podbird.downloadEpisode(model.image, model.name, model.guid, model.audiourl, false)

review: Needs Fixing

[Eran Uzan (etherpulse)]

Hey , Sorry to hear about your loss I hope the laptop had a fulfilling life of development and and creation ;)
I hope it won't be too much of an hassle....

About the negative logic it bothered me too but I kept it this way to support legacy code and to prevent unintended behaviour by other developers, As calling the function without the mobileDl argument will do the same thing as before I added the argument (as "!undefined" will be evaluated to "true").

I would suggest changing the name of the argument to disableMobileDl which then will make sense with the logic it self.

But I can change it if you want...

Hope your new laptop will serve you well,

[Nekhelesh Ramananthan (nik90)]

> Hey , Sorry to hear about your loss I hope the laptop had a fulfilling life of
> development and and creation ;)
> I hope it won't be too much of an hassle....
>
> About the negative logic it bothered me too but I kept it this way to support
> legacy code and to prevent unintended behaviour by other developers, As
> calling the function without the mobileDl argument will do the same thing as
> before I added the argument (as "!undefined" will be evaluated to "true").
>
> I would suggest changing the name of the argument to disableMobileDl which
> then will make sense with the logic it self.

I think this is a good idea. Can you rename the variable to disableMobileDownload? This way as you say your logic remains the same while making it more clear.

>
> But I can change it if you want...
>
>
> Hope your new laptop will serve you well,

Thanks :). I've got 16.04 up and running with the new SDK. Will test the code on the phone when I get home tonight.

[Nekhelesh Ramananthan (nik90)]

Hey Eran, I tested your branch on my BQ E4.5 and it works nicely. Podcasts are automatically downloaded only when on wifi. Nice work!

So here are the few final things before I merge your code,
1. Set the default value to True. By default, we will automatically download podcasts over wifi only. I think this will avoid any unpleasant surprises to new user.

2. Rename the variable to disableMobileDownload as mentioned before to make the code more clear.

review: Needs Fixing

[lp:~etherpulse/podbird/download_wifi]

156. By Eran Uzan on 2016-11-16

Change the wifi download limit default to true and chnage th e code to be a little clearer

[Nekhelesh Ramananthan (nik90)]

LGTM! Nice work! Thanks a lot for the patch!

review: Approve

=== modified file 'app/podbird.qml'
--- app/podbird.qml	2016-10-07 00:25:11 +0000
+++ app/podbird.qml	2016-11-16 21:49:49 +0000
@@ -117,6 +117,7 @@
         property bool showListView: true
         property int skipForward: 30
         property int skipBack: 10
+        property bool downloadOverWifiOnly: true
     }
 
     FileManager {
@@ -171,8 +172,8 @@
         }
     }
 
-    function downloadEpisode(image, title, guid, url) {
-        var singleDownload = singleDownloadComponent.createObject(podbird, {"image": image, "title": title, "guid": guid})
+    function downloadEpisode(image, title, guid, url, disableMobileDownload) {
+        var singleDownload = singleDownloadComponent.createObject(podbird, {"image": image, "title": title, "guid": guid , allowMobileDownload : !disableMobileDownload })
         singleDownload.download(url)
     }
 

=== modified file 'app/podcasts.js'
--- app/podcasts.js	2016-03-29 11:02:18 +0000
+++ app/podcasts.js	2016-11-16 21:49:49 +0000
@@ -360,7 +360,7 @@
             var loopCount = maxEpisodeDownload > rs2.rows.length ? rs2.rows.length : maxEpisodeDownload
             for (var j=0; j < loopCount; j++) {
                 if (!rs2.rows.item(j).downloadedfile && !rs2.rows.item(j).listened && rs2.rows.item(j).audiourl) {
-                    podbird.downloadEpisode(rs.rows.item(i).image, rs2.rows.item(j).name, rs2.rows.item(j).guid, rs2.rows.item(j).audiourl)
+                    podbird.downloadEpisode(rs.rows.item(i).image, rs2.rows.item(j).name, rs2.rows.item(j).guid, rs2.rows.item(j).audiourl, podbird.settings.downloadOverWifiOnly)
                     tx.executeSql("UPDATE Episode SET queued=1 WHERE guid = ?", [rs2.rows.item(j).guid]);
                 }
             }

=== modified file 'app/ui/EpisodesPage.qml'
--- app/ui/EpisodesPage.qml	2016-09-22 21:47:24 +0000
+++ app/ui/EpisodesPage.qml	2016-11-16 21:49:49 +0000
@@ -166,7 +166,7 @@
                                     episodeModel.setProperty(index, "queued", 1)
                                     tx.executeSql("UPDATE Episode SET queued=1 WHERE guid = ?", [episodeModel.get(index).guid]);
                                     if (episodeModel.get(index).audiourl) {
-                                        podbird.downloadEpisode(episodeModel.get(index).image, episodeModel.get(index).name, episodeModel.get(index).guid, episodeModel.get(index).audiourl)
+                                        podbird.downloadEpisode(episodeModel.get(index).image, episodeModel.get(index).name, episodeModel.get(index).guid, episodeModel.get(index).audiourl, false)
                                     } else {
                                         console.log("[ERROR]: Invalid download url: " + episodeModel.get(index).audiourl)
                                     }
@@ -633,7 +633,7 @@
                                 });
                                 episodeModel.setProperty(model.index, "queued", 1)
                                 if (model.audiourl) {
-                                    podbird.downloadEpisode(model.image, model.name, model.guid, model.audiourl)
+                                    podbird.downloadEpisode(model.image, model.name, model.guid, model.audiourl, false)
                                 } else {
                                     console.log("[ERROR]: Invalid download url: " + model.audiourl)
                                 }

=== modified file 'app/ui/EpisodesTab.qml'
--- app/ui/EpisodesTab.qml	2016-10-07 00:25:11 +0000
+++ app/ui/EpisodesTab.qml	2016-11-16 21:49:49 +0000
@@ -183,7 +183,7 @@
                                         episodesModel.setProperty(index, "queued", 1)
                                         tx.executeSql("UPDATE Episode SET queued=1 WHERE guid = ?", [episodesModel.get(index).guid]);
                                         if (episodesModel.get(index).audiourl) {
-                                            podbird.downloadEpisode(episodesModel.get(index).image, episodesModel.get(index).name, episodesModel.get(index).guid, episodesModel.get(index).audiourl)
+                                            podbird.downloadEpisode(episodesModel.get(index).image, episodesModel.get(index).name, episodesModel.get(index).guid, episodesModel.get(index).audiourl, false)
                                         } else {
                                             console.log("[ERROR]: Invalid download url: " + episodesModel.get(index).audiourl)
                                         }
@@ -597,7 +597,7 @@
                                     });
                                     episodesModel.setProperty(model.index, "queued", 1)
                                     if (model.audiourl) {
-                                        podbird.downloadEpisode(model.image, model.name, model.guid, model.audiourl)
+                                        podbird.downloadEpisode(model.image, model.name, model.guid, model.audiourl, false)
                                     } else {
                                         console.log("[ERROR]: Invalid download url: " + model.audiourl)
                                     }

=== modified file 'app/ui/SettingsPage.qml'
--- app/ui/SettingsPage.qml	2016-05-30 17:51:38 +0000
+++ app/ui/SettingsPage.qml	2016-11-16 21:49:49 +0000
@@ -212,6 +212,23 @@
                 onClicked: mainStack.push(Qt.resolvedUrl("../settings/DownloadSetting.qml"))
             }
 
+ 	ListItem {
+                ListItemLayout {
+                    id: downloadWifiOnlyLayout
+                    title.text: i18n.tr("Only download over wifi")
+                    title.color: podbird.appTheme.baseText
+                    summary.text: i18n.tr("Download episodes only when the device is using WiFi")
+                    summary.color: podbird.appTheme.baseSubText
+                    Switch {
+                        SlotsLayout.position: SlotsLayout.Last
+                        checked: podbird.settings.downloadOverWifiOnly
+                        onClicked: podbird.settings.downloadOverWifiOnly = checked
+                    }
+                }
+                divider.visible: false
+                height: downloadWifiOnlyLayout.height
+            }
+
             ListItem {
                 id: refreshArtListItem
 

=== modified file 'po/podbird.nik90.pot'
--- po/podbird.nik90.pot	2016-10-07 00:25:11 +0000
+++ po/podbird.nik90.pot	2016-11-16 21:49:49 +0000
@@ -8,7 +8,7 @@
 msgstr ""
 "Project-Id-Version: \n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2016-10-06 23:37+0000\n"
+"POT-Creation-Date: 2016-10-24 19:03+0000\n"
 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <[e-mail]>\n"
@@ -55,7 +55,7 @@
 
 #. TRANSLATORS: About as in information about the app
 #: ../app/settings/About.qml:28 ../app/settings/About.qml:46
-#: ../app/ui/SettingsPage.qml:321
+#: ../app/ui/SettingsPage.qml:338
 msgid "About"
 msgstr ""
 
@@ -223,7 +223,7 @@
 msgstr ""
 
 #: ../app/ui/EpisodesPage.qml:356 ../app/ui/EpisodesTab.qml:418
-#: ../app/ui/SearchPage.qml:179 ../app/ui/SettingsPage.qml:303
+#: ../app/ui/SearchPage.qml:179 ../app/ui/SettingsPage.qml:320
 msgid "Close"
 msgstr ""
 
@@ -518,43 +518,51 @@
 msgid "Default number of new episodes to download for each podcast"
 msgstr ""
 
-#: ../app/ui/SettingsPage.qml:223
+#: ../app/ui/SettingsPage.qml:218
+msgid "Only download over wifi"
+msgstr ""
+
+#: ../app/ui/SettingsPage.qml:220
+msgid "Download episodes only when the device is using WiFi"
+msgstr ""
+
+#: ../app/ui/SettingsPage.qml:240
 msgid "Refresh podcast artwork"
 msgstr ""
 
-#: ../app/ui/SettingsPage.qml:225
+#: ../app/ui/SettingsPage.qml:242
 msgid ""
 "Update all podcasts artwork and fix missing ones (this only works with "
 "podcasts added via iTunes® search)"
 msgstr ""
 
-#: ../app/ui/SettingsPage.qml:254
+#: ../app/ui/SettingsPage.qml:271
 msgid "Storage Settings"
 msgstr ""
 
-#: ../app/ui/SettingsPage.qml:265
+#: ../app/ui/SettingsPage.qml:282
 msgid "Delete orphaned files and links"
 msgstr ""
 
-#: ../app/ui/SettingsPage.qml:267
+#: ../app/ui/SettingsPage.qml:284
 msgid "Free space by removing orphaned downloaded files and links"
 msgstr ""
 
-#: ../app/ui/SettingsPage.qml:297
+#: ../app/ui/SettingsPage.qml:314
 msgid "Removed Orphaned files and links"
 msgstr ""
 
-#: ../app/ui/SettingsPage.qml:297
+#: ../app/ui/SettingsPage.qml:314
 msgid "No Orphans found!"
 msgstr ""
 
-#: ../app/ui/SettingsPage.qml:298
+#: ../app/ui/SettingsPage.qml:315
 msgid ""
 "All orphaned files have been deleted to recover disk space. Orphaned links "
 "pointing at invalid files have also been cleaned up."
 msgstr ""
 
-#: ../app/ui/SettingsPage.qml:300
+#: ../app/ui/SettingsPage.qml:317
 msgid ""
 "No orphaned files have been found to recover disk space. Podbird database is "
 "clean."
@@ -562,11 +570,11 @@
 
 #. TRANSLATORS: Shortened form of "Miscellaneous" which is shown to denote other setting options
 #. that doesn't fit into any other category.
-#: ../app/ui/SettingsPage.qml:315
+#: ../app/ui/SettingsPage.qml:332
 msgid "Misc."
 msgstr ""
 
-#: ../app/ui/SettingsPage.qml:331
+#: ../app/ui/SettingsPage.qml:348
 msgid "Report Bug"
 msgstr ""