Update download request fetching on topic and topic content pages #11391

rtibbles · 2023-10-11T20:58:14Z

Summary

Makes the ContentRequest endpoint accept contentnode_id and contentnode_ids filtering parameters
Fetches download requests on the topics page by passing the loaded contentnode_ids and further when doing requests for 'more'
Fetches just the download request for the specific resource on the content page
Cleans up reactivity on topic content page when a resource is downloaded
Fixes an unnoticed bug where the networkDevices map was being filtered on, rather than just being looked up in
Refactors the MyDownload page polling behaviour into the downloadRequest composable
Reuses this polling behaviour on the TopicsContentPage to ensure we get timely status updates of individual resource downloads

References

Reviewer guidance

Ensure there are no regressions on the MyDownloads page.
Ensure that Download Requests are properly displayed on the topics page and the topics content page.
Ensure that adding a new download in both places properly updates the frontend in a timely fashion.

Testing checklist

Contributor has fully tested the PR manually
If there are any front-end changes, before/after screenshots are included
Critical user journeys are covered by Gherkin stories
Critical and brittle code paths are covered by unit tests

PR process

PR has the correct target branch and milestone
PR has 'needs review' or 'work-in-progress' label
If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
If this is an important user-facing change, PR or related issue has a 'changelog' label
If this includes an internal dependency change, a link to the diff is provided

Reviewer checklist

Automated test coverage is satisfactory
PR is fully functional
PR has been tested for accessibility regressions
External dependency files were updated if necessary (yarn and pip)
Documentation is updated
Contributor is in AUTHORS.md

…de_id__in.

Update content node progress fetching logic for remote browsing to filter similarly.

github-actions · 2023-10-11T21:52:30Z

Build Artifacts

Asset type	Download link
PEX file	kolibri-0.16.0b6.dev0_git.114.g8787f9d3.pex
Windows Installer (EXE)	kolibri-0.16.0b6.dev0+git.114.g8787f9d3-unsigned.exe
Debian Package	kolibri_0.16.0b6.dev0+git.114.g8787f9d3-0ubuntu1_all.deb
Mac Installer (DMG)	kolibri-0.16.0b6.dev0+git.114.g8787f9d3-0.3.0.dmg
Android Package (APK)	kolibri-0.16.0b6.dev0+git.114.g8787f9d3-0.1.0-debug.apk
TAR file	kolibri-0.16.0b6.dev0+git.114.g8787f9d3.tar.gz
WHL file	kolibri-0.16.0b6.dev0+git.114.g8787f9d3-py2.py3-none-any.whl

marcellamaki

I had some questions, but no blockers, just learning questions. Thanks @rtibbles !

marcellamaki · 2023-10-12T13:49:52Z

kolibri/plugins/learn/assets/src/views/TopicsPage/index.vue

+        const children = topic.children ? topic.children.results : [];
+        for (const child of children) {
+          if (child.kind === ContentNodeKinds.TOPIC) {
+            contentnode_id__in.push(..._getAllDescendantChildren(child));


I know recursion isn't actually all that uncommon, but I do enjoy seeing in the wild where it makes complete sense and not just in a documentation page from my bootcamp classes where I was only understanding it theoretically and felt so confused as a junior dev

marcellamaki · 2023-10-12T13:50:50Z

kolibri/plugins/learn/assets/src/views/TopicsPage/index.vue

+      };
+
+      const fetchRemoteBrowsingContentNodeUserData = topic => {
+        if (get(isUserLoggedIn) && props.deviceId) {


props.deviceId is this a way to access the props from within setup?

This is the way to access it.

Importantly, the props object is reactive, but if you destructure const { deviceId } = props then the deviceId will not be reactive.

marcellamaki · 2023-10-12T13:58:53Z

kolibri/plugins/learn/assets/src/views/TopicsPage/index.vue

@@ -827,7 +860,7 @@
        const parent = parentIndex > -1 ? this.contents[parentIndex] : null;
        const more = parent && parent.children && parent.children.more;
        if (more) {
-          if (this.isUserLoggedIn) {
+          if (this.isUserLoggedIn && !this.deviceId) {


I have seen the existence check on the deviceId in a few files now and I don't really know what it's for

If it's not defined then we're not browsing remotely. If it is, we are.

marcellamaki · 2023-10-12T14:00:02Z

kolibri/plugins/learn/assets/src/composables/useDownloadRequests.js

      facility: store.getters.currentFacilityId,
-      status: 'Pending',
+      status: 'PENDING',


important changes here

marcellamaki · 2023-10-12T14:02:26Z

kolibri/plugins/learn/assets/test/views/topic-content.spec.js

+              downloadRequestMap: {
+                [CONTENT_ID]: {
+                  id: CONTENT_ID,
+                  status: 'COMPLETED',


this is the sort of thing in a test that I need to remember for some future date, that I can actually put in a test

pcenov

LGTM, no regression issues observed as well.

rtibbles added 5 commits October 11, 2023 13:53

Allow content requests to be filtered by contentnode_id and contentno…

8be7ef7

…de_id__in.

Fetch download requests for all resources by id filtering.

22ad0a6

Update content node progress fetching logic for remote browsing to filter similarly.

Fetch download request specifically for the displayed resource.

5c8c6df

Clean up reactivity of newly initiated downloads.

278a3cf

Fix improper lookup for download source.

8163f56

rtibbles requested review from MisRob, marcellamaki and pcenov October 11, 2023 20:58

rtibbles added the TODO: needs review Waiting for review label Oct 11, 2023

rtibbles added this to the Kolibri 0.16 Release: Bugs and Regressions milestone Oct 11, 2023

github-actions bot added DEV: backend Python, databases, networking, filesystem... APP: Learn Re: Learn App (content, quizzes, lessons, etc.) SIZE: medium labels Oct 11, 2023

rtibbles added 2 commits October 11, 2023 14:44

Move download request polling logic into composable.

bfb5f0f

Poll for download requests on the topics content page.

65e29c4

rtibbles force-pushed the which_downloads_pacifically branch from e59e480 to 65e29c4 Compare October 11, 2023 21:47

marcellamaki approved these changes Oct 12, 2023

View reviewed changes

pcenov approved these changes Oct 13, 2023

View reviewed changes

rtibbles merged commit f5e25aa into learningequality:release-v0.16.x Oct 13, 2023

rtibbles deleted the which_downloads_pacifically branch October 13, 2023 13:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update download request fetching on topic and topic content pages #11391

Update download request fetching on topic and topic content pages #11391

rtibbles commented Oct 11, 2023

github-actions bot commented Oct 11, 2023 •

edited

Loading

marcellamaki left a comment

marcellamaki Oct 12, 2023

marcellamaki Oct 12, 2023

rtibbles Oct 12, 2023

marcellamaki Oct 12, 2023

rtibbles Oct 12, 2023

marcellamaki Oct 12, 2023

marcellamaki Oct 12, 2023

pcenov left a comment

Update download request fetching on topic and topic content pages #11391

Update download request fetching on topic and topic content pages #11391

Conversation

rtibbles commented Oct 11, 2023

Summary

References

Reviewer guidance

Testing checklist

PR process

Reviewer checklist

github-actions bot commented Oct 11, 2023 • edited Loading

marcellamaki left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pcenov left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 11, 2023 •

edited

Loading