From 3e324f2491765e74cbcb0433207ff0837fa73069 Mon Sep 17 00:00:00 2001
From: Dylan McCall <dylan@dylanmccall.ca>
Date: Thu, 4 Jan 2024 10:24:07 -0800
Subject: [PATCH 1/3] components: Move duplicate isOffline code to a mixin

Originally, the same code was used in two different components.

https://phabricator.endlessm.com/T35137
---
 .../assets/src/components/DiscoveryNavBar.vue | 23 ++----------------
 .../assets/src/mixins/isOfflineMixin.js       | 24 +++++++++++++++++++
 .../src/views/welcome/PackReadyPage.vue       | 18 ++------------
 3 files changed, 28 insertions(+), 37 deletions(-)
 create mode 100644 kolibri_explore_plugin/assets/src/mixins/isOfflineMixin.js

diff --git a/kolibri_explore_plugin/assets/src/components/DiscoveryNavBar.vue b/kolibri_explore_plugin/assets/src/components/DiscoveryNavBar.vue
index 39fab5345..91d4a5fe3 100644
--- a/kolibri_explore_plugin/assets/src/components/DiscoveryNavBar.vue
+++ b/kolibri_explore_plugin/assets/src/components/DiscoveryNavBar.vue
@@ -78,6 +78,7 @@
   import { mapMutations } from 'vuex';
   import { assets } from 'ek-components';
   import commonExploreStrings from '../views/commonExploreStrings';
+  import isOfflineMixin from '../mixins/isOfflineMixin';
   import { PageNames } from '../constants';
 
   export default {
@@ -88,12 +89,7 @@
       MessageReplyTextOutlineIcon,
       ArrowDownIcon,
     },
-    mixins: [commonExploreStrings],
-    data() {
-      return {
-        isOffline: false,
-      };
-    },
+    mixins: [commonExploreStrings, isOfflineMixin],
     computed: {
       logo() {
         return assets.EndlessLogo;
@@ -116,15 +112,6 @@
         return !!plugin_data.androidApplicationId && !!plugin_data.windowsApplicationId;
       },
     },
-    created() {
-      this.isOffline = !navigator.onLine;
-      window.addEventListener('offline', this.onOffline);
-      window.addEventListener('online', this.onOnline);
-    },
-    destroyed() {
-      window.removeEventListener('offline', this.onOffline);
-      window.removeEventListener('online', this.onOnline);
-    },
     methods: {
       ...mapMutations({
         setSearchResult: 'topicsRoot/SET_SEARCH_RESULT',
@@ -158,12 +145,6 @@
       currentIsSearch() {
         return this.$route.name === PageNames.SEARCH;
       },
-      onOffline() {
-        this.isOffline = true;
-      },
-      onOnline() {
-        this.isOffline = false;
-      },
       onLogoClick(event) {
         if (event.ctrlKey) {
           this.$store.commit('topicsRoot/SET_SHOW_SIDE_NAV', true);
diff --git a/kolibri_explore_plugin/assets/src/mixins/isOfflineMixin.js b/kolibri_explore_plugin/assets/src/mixins/isOfflineMixin.js
new file mode 100644
index 000000000..f2b31e851
--- /dev/null
+++ b/kolibri_explore_plugin/assets/src/mixins/isOfflineMixin.js
@@ -0,0 +1,24 @@
+export default {
+  data() {
+    return {
+      isOffline: false,
+    };
+  },
+  created() {
+    this.isOffline = !navigator.onLine;
+    window.addEventListener('offline', this.onOffline);
+    window.addEventListener('online', this.onOnline);
+  },
+  destroyed() {
+    window.removeEventListener('offline', this.onOffline);
+    window.removeEventListener('online', this.onOnline);
+  },
+  methods: {
+    onOffline() {
+      this.isOffline = true;
+    },
+    onOnline() {
+      this.isOffline = false;
+    },
+  },
+};
diff --git a/kolibri_explore_plugin/assets/src/views/welcome/PackReadyPage.vue b/kolibri_explore_plugin/assets/src/views/welcome/PackReadyPage.vue
index f402c6eb2..869aa4399 100644
--- a/kolibri_explore_plugin/assets/src/views/welcome/PackReadyPage.vue
+++ b/kolibri_explore_plugin/assets/src/views/welcome/PackReadyPage.vue
@@ -28,6 +28,7 @@
 <script>
 
   import { PageNames } from '../../constants';
+  import isOfflineMixin from '../../mixins/isOfflineMixin';
   import WelcomeBase from './WelcomeBase';
 
   export default {
@@ -35,33 +36,18 @@
     components: {
       WelcomeBase,
     },
+    mixins: [isOfflineMixin],
     data() {
       return {
-        isOffline: false,
         PageNames,
       };
     },
-    created() {
-      this.isOffline = !navigator.onLine;
-      window.addEventListener('offline', this.onOffline);
-      window.addEventListener('online', this.onOnline);
-    },
-    destroyed() {
-      window.removeEventListener('offline', this.onOffline);
-      window.removeEventListener('online', this.onOnline);
-    },
     methods: {
       downloadContent() {
         const grade = this.$route.params.grade;
         const name = this.$route.params.name;
         this.$router.push({ name: PageNames.DOWNLOAD, params: { grade, name } });
       },
-      onOffline() {
-        this.isOffline = true;
-      },
-      onOnline() {
-        this.isOffline = false;
-      },
     },
     $trs: {
       packReadyTitle: {

From 721c7d5a28f0ecdae44f3095fd360c3905043827 Mon Sep 17 00:00:00 2001
From: Dylan McCall <dylan@dylanmccall.ca>
Date: Thu, 4 Jan 2024 13:11:16 -0800
Subject: [PATCH 2/3] Pack selection: Require internet only if download is
 required

By using a new `isDownloadRequired` property in the get_collection_info
API, we can determine ahead of time if the selected content pack will
require an internet connection. Using that information, we can disable
the Download button only if a connection is required.

https://phabricator.endlessm.com/T35137
---
 .../assets/src/views/welcome/PackReadyPage.vue        | 11 ++++++++++-
 kolibri_explore_plugin/collectionviews.py             | 11 +++++++++++
 kolibri_explore_plugin/test/test_collectionviews.py   |  2 ++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/kolibri_explore_plugin/assets/src/views/welcome/PackReadyPage.vue b/kolibri_explore_plugin/assets/src/views/welcome/PackReadyPage.vue
index 869aa4399..15c8c2400 100644
--- a/kolibri_explore_plugin/assets/src/views/welcome/PackReadyPage.vue
+++ b/kolibri_explore_plugin/assets/src/views/welcome/PackReadyPage.vue
@@ -6,7 +6,7 @@
       <b-button
         pill
         class="mb-4 mt-4"
-        :disabled="isOffline"
+        :disabled="isDownloadRequired && isOffline"
         variant="primary"
         @click="downloadContent"
       >
@@ -29,6 +29,7 @@
 
   import { PageNames } from '../../constants';
   import isOfflineMixin from '../../mixins/isOfflineMixin';
+  import { getCollectionInfo } from '../../modules/coreExplore/utils';
   import WelcomeBase from './WelcomeBase';
 
   export default {
@@ -40,8 +41,16 @@
     data() {
       return {
         PageNames,
+        isDownloadRequired: true,
       };
     },
+    mounted() {
+      return getCollectionInfo(this.$route.params.grade, this.$route.params.name).then(
+        collectionsInfo => {
+          this.isDownloadRequired = collectionsInfo.isDownloadRequired;
+        }
+      );
+    },
     methods: {
       downloadContent() {
         const grade = this.$route.params.grade;
diff --git a/kolibri_explore_plugin/collectionviews.py b/kolibri_explore_plugin/collectionviews.py
index 7863df051..c2f00f56b 100644
--- a/kolibri_explore_plugin/collectionviews.py
+++ b/kolibri_explore_plugin/collectionviews.py
@@ -1,5 +1,6 @@
 # Copyright 2022-2023 Endless OS Foundation LLC
 # SPDX-License-Identifier: GPL-2.0-or-later
+import itertools
 import logging
 import os
 import time
@@ -147,6 +148,15 @@ def set_availability(self, free_space_gb):
     def get_channels_count(self):
         return len(self.get_channel_ids())
 
+    def is_download_required(self):
+        return any(
+            itertools.chain(
+                self.get_channelimport_tasks(),
+                self.get_contentimport_tasks(),
+                self.get_contentthumbnail_tasks(),
+            )
+        )
+
     def get_channelimport_tasks(self):
         """Return a serializable object to create channelimport tasks
 
@@ -685,6 +695,7 @@ def _get_collections_info_by_grade_name(grade, name):
         "metadata": manifest.metadata,
         "available": manifest.available,
         "channelsCount": manifest.get_channels_count(),
+        "isDownloadRequired": manifest.is_download_required(),
     }
 
 
diff --git a/kolibri_explore_plugin/test/test_collectionviews.py b/kolibri_explore_plugin/test/test_collectionviews.py
index 538e84cbb..b74c7b5ad 100644
--- a/kolibri_explore_plugin/test/test_collectionviews.py
+++ b/kolibri_explore_plugin/test/test_collectionviews.py
@@ -53,6 +53,7 @@ def test_get_collection_info():
                     "metadata": collection_data["metadata"],
                     "available": True,
                     "channelsCount": len(collection_data["channels"]),
+                    "isDownloadRequired": True,
                 }
             }
 
@@ -81,6 +82,7 @@ def test_get_all_collections_info():
                     "metadata": collection_data["metadata"],
                     "available": True,
                     "channelsCount": len(collection_data["channels"]),
+                    "isDownloadRequired": True,
                 }
             )
     expected_data = {"allCollectionsInfo": all_collections_info}

From 0b773ee1a1e90f3f5463392db894ebeea0bb39e4 Mon Sep 17 00:00:00 2001
From: Dylan McCall <dylan@dylanmccall.ca>
Date: Thu, 4 Jan 2024 13:18:17 -0800
Subject: [PATCH 3/3] collectionviews: Use generators for collection tasks

By using generators instead of functions returning lists, we enable a
small performance optimization when determining if a download is
required.

https://phabricator.endlessm.com/T35137
---
 kolibri_explore_plugin/collectionviews.py | 69 ++++++++---------------
 1 file changed, 23 insertions(+), 46 deletions(-)

diff --git a/kolibri_explore_plugin/collectionviews.py b/kolibri_explore_plugin/collectionviews.py
index c2f00f56b..b94c41aa4 100644
--- a/kolibri_explore_plugin/collectionviews.py
+++ b/kolibri_explore_plugin/collectionviews.py
@@ -151,18 +151,17 @@ def get_channels_count(self):
     def is_download_required(self):
         return any(
             itertools.chain(
-                self.get_channelimport_tasks(),
-                self.get_contentimport_tasks(),
-                self.get_contentthumbnail_tasks(),
+                self.iter_channelimport_tasks(),
+                self.iter_contentimport_tasks(),
+                self.iter_contentthumbnail_tasks(),
             )
         )
 
-    def get_channelimport_tasks(self):
+    def iter_channelimport_tasks(self):
         """Return a serializable object to create channelimport tasks
 
         For all the channels in this content manifest.
         """
-        tasks = []
         for channel_id, channel_version in self.get_latest_channels():
             metadata = get_channel_metadata(channel_id)
             if metadata and metadata.version >= channel_version:
@@ -171,16 +170,14 @@ def get_channelimport_tasks(self):
                     "already present"
                 )
                 continue
-            tasks.append(get_remotechannelimport_task(channel_id))
-        return tasks
+            yield get_remotechannelimport_task(channel_id)
 
-    def get_extra_channelimport_tasks(self):
+    def iter_extra_channelimport_tasks(self):
         """Return a serializable object to create extra channelimport tasks
 
         For all channels featured in Endless Key content manifests. In addition
         to the channel metadata, all thumbnails are downloaded.
         """
-        tasks = []
         for channel_id, channel_version in self.get_latest_extra_channels():
             # Check if the channel metadata and thumbnails are already
             # available.
@@ -202,21 +199,15 @@ def get_extra_channelimport_tasks(self):
                     )
                     continue
 
-            tasks.append(
-                get_remoteimport_task(
-                    channel_id, node_ids=[], all_thumbnails=True
-                )
+            yield get_remoteimport_task(
+                channel_id, node_ids=[], all_thumbnails=True
             )
 
-        return tasks
-
-    def get_contentimport_tasks(self):
+    def iter_contentimport_tasks(self):
         """Return a serializable object to create contentimport tasks
 
         For all the channels in this content manifest.
         """
-        tasks = []
-
         for channel_id in self.get_channel_ids():
             channel_metadata = get_channel_metadata(channel_id)
             node_ids = list(
@@ -236,15 +227,11 @@ def get_contentimport_tasks(self):
                 )
                 continue
 
-            tasks.append(
-                get_remotecontentimport_task(
-                    channel_id, channel_metadata.name, node_ids
-                )
+            yield get_remotecontentimport_task(
+                channel_id, channel_metadata.name, node_ids
             )
 
-        return tasks
-
-    def get_applyexternaltags_tasks(self):
+    def iter_applyexternaltags_tasks(self):
         """Return a serializable object to create applyexternaltags tasks
 
         As defined in this content manifest metadata.
@@ -252,22 +239,16 @@ def get_applyexternaltags_tasks(self):
         if "tagged_node_ids" not in self.metadata:
             return []
 
-        tasks = []
-
         for tagged in self.metadata["tagged_node_ids"]:
             node_id = tagged["node_id"]
             tags = tagged["tags"]
-            tasks.append(get_applyexternaltags_task(node_id, tags))
+            yield get_applyexternaltags_task(node_id, tags)
 
-        return tasks
-
-    def get_contentthumbnail_tasks(self):
+    def iter_contentthumbnail_tasks(self):
         """Return a serializable object to create thumbnail contentimport tasks
 
         For all the channels in this content manifest.
         """
-        tasks = []
-
         for channel_id in self.get_channel_ids():
             # Check if the desired thumbnail nodes are already available.
             num_resources, _, _ = get_import_export_data(
@@ -283,14 +264,10 @@ def get_contentthumbnail_tasks(self):
                 )
                 continue
 
-            tasks.append(
-                get_remotecontentimport_task(
-                    channel_id, node_ids=[], all_thumbnails=True
-                )
+            yield get_remotecontentimport_task(
+                channel_id, node_ids=[], all_thumbnails=True
             )
 
-        return tasks
-
     def _get_node_ids_for_channel(self, channel_metadata, channel_id):
         """Get node IDs regardless of the version
 
@@ -583,11 +560,11 @@ def _set_next_stage(self, user):
         while not tasks and self._stage != DownloadStage.COMPLETED:
             self._stage = DownloadStage(self._stage + 1)
             if self._stage == DownloadStage.IMPORTING_CHANNELS:
-                tasks = self._content_manifest.get_channelimport_tasks()
+                tasks = self._content_manifest.iter_channelimport_tasks()
             elif self._stage == DownloadStage.IMPORTING_CONTENT:
-                tasks = self._content_manifest.get_contentimport_tasks()
+                tasks = self._content_manifest.iter_contentimport_tasks()
             elif self._stage == DownloadStage.APPLYING_EXTERNAL_TAGS:
-                tasks = self._content_manifest.get_applyexternaltags_tasks()
+                tasks = self._content_manifest.iter_applyexternaltags_tasks()
 
         if self._stage == DownloadStage.COMPLETED:
             logger.info("Download completed!")
@@ -595,17 +572,17 @@ def _set_next_stage(self, user):
             # Download the manifest content thumbnails and the extra channels
             # in the background.
             thumbnail_tasks = (
-                self._content_manifest.get_contentthumbnail_tasks()
+                self._content_manifest.iter_contentthumbnail_tasks()
             )
             extra_channel_tasks = (
-                self._content_manifest.get_extra_channelimport_tasks()
+                self._content_manifest.iter_extra_channelimport_tasks()
             )
-            for task in thumbnail_tasks + extra_channel_tasks:
+            for task in itertools.chain(thumbnail_tasks, extra_channel_tasks):
                 BackgroundTask.create_from_task_data(task)
             logger.info("Starting background download tasks")
             enqueue_next_background_task()
 
-        self._tasks_pending = tasks
+        self._tasks_pending = list(tasks)
         self._tasks_previously_completed.extend(self._tasks_completed)
         self._tasks_completed = []
         logger.info(f"Started download stage: {self._stage.name}")