Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MyDownloads bug fixes #11092

Merged
merged 3 commits into from
Aug 15, 2023
Merged

MyDownloads bug fixes #11092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c855bc4
Fix removing download requests to actually remove download request pr…
marcellamaki Aug 11, 2023
360485b
Front end refactoring, to better manage reactivity and page state
marcellamaki Aug 11, 2023
2f0dd63
PR updates to address feedback
marcellamaki Aug 15, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions kolibri/core/content/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
from kolibri.core.content import serializers
from kolibri.core.content.models import ContentDownloadRequest
from kolibri.core.content.models import ContentRemovalRequest
from kolibri.core.content.models import ContentRequestReason
from kolibri.core.content.models import ContentRequestStatus
from kolibri.core.content.permissions import CanManageContent
from kolibri.core.content.tasks import automatic_resource_import
Expand Down Expand Up @@ -1349,7 +1350,9 @@ class ContentRequestViewset(ReadOnlyValuesViewset, CreateModelMixin):
)

def get_queryset(self):
return ContentDownloadRequest.objects.filter(source_id=self.request.user.id)
return ContentDownloadRequest.objects.filter(
source_id=self.request.user.id, reason=ContentRequestReason.UserInitiated
)

def annotate_queryset(self, queryset):
return queryset.annotate(
Expand All @@ -1373,11 +1376,12 @@ def delete(self, request, pk=None):
)

existing_download_request = (
existing_deletion_request
) = ContentRemovalRequest.objects.filter(
id=request_id,
source_id=request.user.id,
).first()
self.get_queryset()
.filter(
id=request_id,
)
.first()
)

if existing_download_request is None:
return Response(
Expand All @@ -1386,7 +1390,8 @@ def delete(self, request, pk=None):
)

existing_deletion_request = ContentRemovalRequest.objects.filter(
id=request_id,
contentnode_id=existing_download_request.contentnode_id,
reason=existing_download_request.reason,
source_id=request.user.id,
).first()

Expand Down
18 changes: 5 additions & 13 deletions kolibri/plugins/learn/assets/src/composables/useDownloadRequests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@

import { getCurrentInstance, reactive, ref } from 'kolibri.lib.vueCompositionApi';
import { ContentRequestResource } from 'kolibri.resources';
import Vue from 'kolibri.lib.vue';
import { createTranslator } from 'kolibri.utils.i18n';
import { get, set } from '@vueuse/core';
import redirectBrowser from 'kolibri.utils.redirectBrowser';
import urls from 'kolibri.urls';
import client from 'kolibri.client';
import Vue from 'kolibri.lib.vue';
import useDevices from './useDevices';

const downloadRequestsTranslator = createTranslator('DownloadRequests', {
Expand Down Expand Up @@ -40,7 +40,9 @@ export default function useDownloadRequests(store) {
function fetchUserDownloadRequests(params) {
return ContentRequestResource.list(params)
.then(downloadRequests => {
set(downloadRequestMap, downloadRequests);
for (const obj of downloadRequests) {
set(downloadRequestMap, obj.id, obj);
}
set(loading, false);
})
.then(store.dispatch('notLoading'));
Expand Down Expand Up @@ -100,17 +102,8 @@ export default function useDownloadRequests(store) {
ContentRequestResource.deleteModel({
id: content.id,
contentnode_id: content.contentnode_id,
}).then(Vue.delete(downloadRequestMap, content.id));
return Promise.resolve();
}

function removeDownloadsRequest(contentList) {
contentList.forEach(content => {
ContentRequestResource.deleteModel({
id: content.id,
contentnode_id: content.contentnode_id,
}).then(Vue.delete(downloadRequestMap, content.id));
});
Vue.delete(downloadRequestMap, content.id);
return Promise.resolve();
}

Expand Down Expand Up @@ -138,7 +131,6 @@ export default function useDownloadRequests(store) {
addDownloadRequest,
loading,
removeDownloadRequest,
removeDownloadsRequest,
downloadRequestsTranslator,
isDownloadingByLearner,
isDownloadedByLearner,
Expand Down
123 changes: 84 additions & 39 deletions kolibri/plugins/learn/assets/src/my_downloads/views/DownloadsList/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
v-model="currentPage"
:itemsPerPage="itemsPerPage"
:totalPageNumber="totalPageNumber"
:numFilteredItems="totalDownloads"
:numFilteredItems="downloads.length"
>
<CoreTable>
<template #headers>
Expand All @@ -16,7 +16,7 @@
class="select-all"
:label="coreString('nameLabel')"
:checked="areAllSelected"
:disabled="!downloads || !Object.keys(downloads).length"
:disabled="!areAnyAvailable"
:style="{ color: $themeTokens.annotation }"
@change="selectAll($event)"
/>
Expand All @@ -25,7 +25,7 @@
<th> {{ coreString('dateAdded') }} </th>
</template>
<template #tbody>
<tbody v-if="!loading">
<tbody v-if="!loading && downloadRequestMap">
<tr
v-for="download in paginatedDownloads"
:key="download.contentnode_id"
Expand All @@ -35,7 +35,6 @@
<KCheckbox
:checked="resourceIsSelected(download)"
class="download-checkbox"
:disabled="nonCompleteStatus(download)"
@change="handleCheckResource(download, $event)"
>
<KLabeledIcon
Expand All @@ -51,6 +50,7 @@
</td>
<td>
<KIcon
v-if="downloadStatusIcon(download)"
:icon="downloadStatusIcon(download)"
:color="download.status === 'PENDING' ? $themeTokens.annotation : null"
class="icon"
Expand All @@ -68,7 +68,7 @@
v-else
:text="coreString('viewAction')"
appearance="flat-button"
:href="genExternalContentURLBackLinkCurrentPage(download)"
:href="genExternalContentURLBackLinkCurrentPage(download.contentnode_id)"
/>
</td>
<td class="resource-action">
Expand All @@ -83,7 +83,7 @@
</tbody>
</template>
</CoreTable>
<p v-if="!loading && (!downloads || !Object.keys(downloads).length)">
<p v-if="!loading && (!downloadRequestMap || !Object.keys(downloadRequestMap).length)">
{{ coreString('noResourcesDownloaded') }}
</p>
</PaginatedListContainerWithBackend>
Expand Down Expand Up @@ -111,8 +111,11 @@
import CoreTable from 'kolibri.coreVue.components.CoreTable';
import commonCoreStrings from 'kolibri.coreVue.mixins.commonCoreStrings';
import PaginatedListContainerWithBackend from 'kolibri-common/components/PaginatedListContainerWithBackend';
import { computed, getCurrentInstance, watch, ref } from 'kolibri.lib.vueCompositionApi';
import { get, set } from '@vueuse/core';
import useContentLink from '../../../composables/useContentLink';
import useLearningActivities from '../../../composables/useLearningActivities';
import useDownloadRequests from '../../../composables/useDownloadRequests';
import SelectionBottomBar from './SelectionBottomBar.vue';
import ConfirmationDeleteModal from './ConfirmationDeleteModal.vue';

Expand All @@ -128,25 +131,70 @@
setup() {
const { genExternalContentURLBackLinkCurrentPage } = useContentLink();
const { getLearningActivityIcon } = useLearningActivities();

const { downloadRequestMap, availableSpace } = useDownloadRequests();
const totalPageNumber = ref(0);
const store = getCurrentInstance().proxy.$store;
const query = computed(() => get(route).query);
const route = computed(() => store.state.route);
const sort = computed(() => query.value.sort);
const pageSizeNumber = computed(() => Number(query.value.page_size || 25));
const activityType = computed(() => query.value.activity || 'all');
const downloads = ref([]);
const sortedFilteredDownloads = () => {
let downloadsToDisplay = [];
marcellamaki marked this conversation as resolved.
Show resolved Hide resolved
if (downloadRequestMap) {
for (const [, value] of Object.entries(downloadRequestMap)) {
downloadsToDisplay.push(value);
}
if (activityType) {
if (activityType.value !== 'all') {
downloadsToDisplay = downloadsToDisplay.filter(download =>
download.metadata.learning_activities.includes(activityType.value)
);
}
}
if (sort) {
switch (sort.value) {
case 'newest':
downloadsToDisplay.sort(
(a, b) => new Date(b.requested_at) - new Date(a.requested_at)
);
break;
case 'oldest':
downloadsToDisplay.sort(
(a, b) => new Date(a.requested_at) - new Date(b.requested_at)
);
break;
case 'smallest':
downloadsToDisplay.sort((a, b) => a.metadata.file_size - b.metadata.file_size);
break;
case 'largest':
downloadsToDisplay.sort((a, b) => b.metadata.file_size - a.metadata.file_size);
break;
default:
// If no valid sort option provided, return unsorted array
break;
}
}
}
set(totalPageNumber, Math.ceil(downloadsToDisplay.length / pageSizeNumber.value));
set(downloads, downloadsToDisplay);
};
sortedFilteredDownloads();
watch(route, () => {
rtibbles marked this conversation as resolved.
Show resolved Hide resolved
sortedFilteredDownloads();
});
return {
downloadRequestMap,
downloads,
getLearningActivityIcon,
sortedFilteredDownloads,
totalPageNumber,
availableSpace,
genExternalContentURLBackLinkCurrentPage,
};
},
props: {
downloads: {
type: Array,
required: true,
},
totalDownloads: {
type: Number,
required: true,
},
totalPageNumber: {
type: Number,
required: true,
},
loading: {
type: Boolean,
required: false,
Expand Down Expand Up @@ -198,38 +246,36 @@
areAllSelected() {
return Object.keys(this.downloads).every(id => this.selectedDownloads.includes(id));
},
areAnyAvailable() {
if (this.downloads && this.downloads.length > 0) {
return this.downloads.filter(download => download.status === 'COMPLETED').length > 0;
}
return false;
},
},
watch: {
selectedDownloads(newVal, oldVal) {
if (newVal.length === 0) {
this.selectedDownloadsSize = 0;
return;
}
const addedDownloads = newVal.filter(id => !oldVal.includes(id));
const removedDownloads = oldVal.filter(id => !newVal.includes(id));
const addedDownloadsSize = addedDownloads.reduce(
(acc, id) => acc + (this.downloads[id] ? this.downloads[id].metadata.file_size : 0),
selectedDownloads(newVal) {
this.selectedDownloadsSize = newVal.reduce(
(acc, object) => acc + object.metadata.file_size,
0
);
const removedDownloadsSize = removedDownloads.reduce(
(acc, id) => acc + (this.downloads[id].metadata.file_size ? this.downloads[id] : 0),
0
);
this.selectedDownloadsSize += addedDownloadsSize - removedDownloadsSize;
},
},
mounted() {
this.areAnyAvailable;
},
methods: {
nonCompleteStatus(download) {
return download.status !== 'COMPLETED';
},
selectAll() {
if (this.areAllSelected) {
this.selectedDownloads = this.selectedDownloads.filter(
resourceId => !Object.keys(this.downloads).includes(resourceId)
download => !this.paginatedDownloads.includes(download)
);
} else {
this.selectedDownloads = this.selectedDownloads.concat(
Object.keys(this.downloads).filter(id => !this.selectedDownloads.includes(id))
this.paginatedDownloads.filter(download => !this.selectedDownloads.includes(download))
);
}
},
Expand All @@ -245,13 +291,12 @@
},
removeResource(download) {
this.$emit('removeResources', [download]);
this.sortedFilteredDownloads();
},
removeResources() {
this.$emit('removeResources', this.resourcesToDelete);
this.selectedDownloads = this.selectedDownloads.filter(
resourceId => !this.resourcesToDelete.includes(resourceId)
);
this.resourcesToDelete = [];
this.sortedFilteredDownloads();
},
getIcon(activities) {
return this.getLearningActivityIcon(activities);
Expand All @@ -278,7 +323,7 @@
message = this.coreString('waitingToDownload');
break;
case 'IN_PROGRESS':
message = this.coreString('inProgress');
message = this.coreString('inProgressLabel');
break;
case 'COMPLETED' && this.now - download.requested_at < 10000:
message = this.coreString('justNow');
Expand Down
Loading