diff --git a/api/funkwhale_api/music/filters.py b/api/funkwhale_api/music/filters.py index fbea3735a267188485bfe236ecbd39ae56ae2e39..752422e75e64aae20f19bc46ab00cd4678c220d6 100644 --- a/api/funkwhale_api/music/filters.py +++ b/api/funkwhale_api/music/filters.py @@ -2,6 +2,7 @@ from django.db.models import Count from django_filters import rest_framework as filters +from funkwhale_api.common import fields from . import models @@ -28,6 +29,39 @@ class ArtistFilter(ListenableMixin): } +class ImportBatchFilter(filters.FilterSet): + q = fields.SearchFilter(search_fields=[ + 'submitted_by__username', + 'source', + ]) + + class Meta: + model = models.ImportBatch + fields = { + 'status': ['exact'], + 'source': ['exact'], + 'submitted_by': ['exact'], + } + + +class ImportJobFilter(filters.FilterSet): + q = fields.SearchFilter(search_fields=[ + 'batch__submitted_by__username', + 'source', + ]) + + class Meta: + model = models.ImportJob + fields = { + 'batch': ['exact'], + 'batch__status': ['exact'], + 'batch__source': ['exact'], + 'batch__submitted_by': ['exact'], + 'status': ['exact'], + 'source': ['exact'], + } + + class AlbumFilter(ListenableMixin): listenable = filters.BooleanFilter(name='_', method='filter_listenable') diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index b5f69eb1db866225fca4cc147e71a08901194dd8..b9ecfc50dcd982e109e369cc611656beba08c4b6 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -6,6 +6,7 @@ from funkwhale_api.activity import serializers as activity_serializers from funkwhale_api.federation import utils as federation_utils from funkwhale_api.federation.models import LibraryTrack from funkwhale_api.federation.serializers import AP_CONTEXT +from funkwhale_api.users.serializers import UserBasicSerializer from . import models @@ -90,6 +91,7 @@ class TrackSerializerNested(LyricsMixin): files = TrackFileSerializer(many=True, read_only=True) album = SimpleAlbumSerializer(read_only=True) tags = TagSerializer(many=True, read_only=True) + class Meta: model = models.Track fields = ('id', 'mbid', 'title', 'artist', 'files', 'album', 'tags', 'lyrics') @@ -108,6 +110,7 @@ class AlbumSerializerNested(serializers.ModelSerializer): class ArtistSerializerNested(serializers.ModelSerializer): albums = AlbumSerializerNested(many=True, read_only=True) tags = TagSerializer(many=True, read_only=True) + class Meta: model = models.Artist fields = ('id', 'mbid', 'name', 'albums', 'tags') @@ -121,18 +124,43 @@ class LyricsSerializer(serializers.ModelSerializer): class ImportJobSerializer(serializers.ModelSerializer): track_file = TrackFileSerializer(read_only=True) + class Meta: model = models.ImportJob - fields = ('id', 'mbid', 'batch', 'source', 'status', 'track_file', 'audio_file') + fields = ( + 'id', + 'mbid', + 'batch', + 'source', + 'status', + 'track_file', + 'audio_file') read_only_fields = ('status', 'track_file') class ImportBatchSerializer(serializers.ModelSerializer): - jobs = ImportJobSerializer(many=True, read_only=True) + submitted_by = UserBasicSerializer(read_only=True) + class Meta: model = models.ImportBatch - fields = ('id', 'jobs', 'status', 'creation_date', 'import_request') - read_only_fields = ('creation_date',) + fields = ( + 'id', + 'submitted_by', + 'source', + 'status', + 'creation_date', + 'import_request') + read_only_fields = ( + 'creation_date', 'submitted_by', 'source') + + def to_representation(self, instance): + repr = super().to_representation(instance) + try: + repr['job_count'] = instance.job_count + except AttributeError: + # Queryset was not annotated + pass + return repr class TrackActivitySerializer(activity_serializers.ModelSerializer): diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 224d085b6dd897642c5e84007bc456f8af029487..890cc2578c3ffa4b78994d3425bbdc2d0bb014d5 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -11,6 +11,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.conf import settings from django.db import models, transaction from django.db.models.functions import Length +from django.db.models import Count from django.http import StreamingHttpResponse from django.urls import reverse from django.utils.decorators import method_decorator @@ -99,14 +100,14 @@ class ImportBatchViewSet( mixins.RetrieveModelMixin, viewsets.GenericViewSet): queryset = ( - models.ImportBatch.objects.all() - .prefetch_related('jobs__track_file') - .order_by('-creation_date')) + models.ImportBatch.objects + .select_related() + .order_by('-creation_date') + .annotate(job_count=Count('jobs')) + ) serializer_class = serializers.ImportBatchSerializer permission_classes = (permissions.DjangoModelPermissions, ) - - def get_queryset(self): - return super().get_queryset().filter(submitted_by=self.request.user) + filter_class = filters.ImportBatchFilter def perform_create(self, serializer): serializer.save(submitted_by=self.request.user) @@ -119,13 +120,30 @@ class ImportJobPermission(HasModelPermission): class ImportJobViewSet( mixins.CreateModelMixin, + mixins.ListModelMixin, viewsets.GenericViewSet): - queryset = (models.ImportJob.objects.all()) + queryset = (models.ImportJob.objects.all().select_related()) serializer_class = serializers.ImportJobSerializer permission_classes = (ImportJobPermission, ) + filter_class = filters.ImportJobFilter - def get_queryset(self): - return super().get_queryset().filter(batch__submitted_by=self.request.user) + @list_route(methods=['get']) + def stats(self, request, *args, **kwargs): + qs = models.ImportJob.objects.all() + filterset = filters.ImportJobFilter(request.GET, queryset=qs) + qs = filterset.qs + qs = qs.values('status').order_by('status') + qs = qs.annotate(status_count=Count('status')) + + data = {} + for row in qs: + data[row['status']] = row['status_count'] + + for s, _ in models.IMPORT_STATUS_CHOICES: + data.setdefault(s, 0) + + data['count'] = sum([v for v in data.values()]) + return Response(data) def perform_create(self, serializer): source = 'file://' + serializer.validated_data['audio_file'].name @@ -136,7 +154,8 @@ class ImportJobViewSet( ) -class TrackViewSet(TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): +class TrackViewSet( + TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): """ A simple ViewSet for viewing and editing accounts. """ diff --git a/api/tests/music/test_api.py b/api/tests/music/test_api.py index 606720e133928ecda0d2bf22e9440e6b1c9eeb07..cc6fe644b6a4acb7a654453332021dfdc3803013 100644 --- a/api/tests/music/test_api.py +++ b/api/tests/music/test_api.py @@ -181,30 +181,6 @@ def test_can_import_whole_artist( assert job.source == row['source'] -def test_user_can_query_api_for_his_own_batches( - superuser_api_client, factories): - factories['music.ImportJob']() - job = factories['music.ImportJob']( - batch__submitted_by=superuser_api_client.user) - url = reverse('api:v1:import-batches-list') - - response = superuser_api_client.get(url) - results = response.data - assert results['count'] == 1 - assert results['results'][0]['jobs'][0]['mbid'] == job.mbid - - -def test_user_cannnot_access_other_batches( - superuser_api_client, factories): - factories['music.ImportJob']() - job = factories['music.ImportJob']() - url = reverse('api:v1:import-batches-list') - - response = superuser_api_client.get(url) - results = response.data - assert results['count'] == 0 - - def test_user_can_create_an_empty_batch(superuser_api_client, factories): url = reverse('api:v1:import-batches-list') response = superuser_api_client.post(url) diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 95c56d914893fb775a2325515b06662f19a32859..5863e7b07217733e4b26121c81d51fb4509b1bf4 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -128,3 +128,46 @@ def test_can_create_import_from_federation_tracks( assert batch.jobs.count() == 5 for i, job in enumerate(batch.jobs.all()): assert job.library_track == lts[i] + + +def test_can_list_import_jobs(factories, superuser_api_client): + job = factories['music.ImportJob']() + url = reverse('api:v1:import-jobs-list') + response = superuser_api_client.get(url) + + assert response.status_code == 200 + assert response.data['results'][0]['id'] == job.pk + + +def test_import_job_stats(factories, superuser_api_client): + job1 = factories['music.ImportJob'](status='pending') + job2 = factories['music.ImportJob'](status='errored') + + url = reverse('api:v1:import-jobs-stats') + response = superuser_api_client.get(url) + expected = { + 'errored': 1, + 'pending': 1, + 'finished': 0, + 'skipped': 0, + 'count': 2, + } + assert response.status_code == 200 + assert response.data == expected + + +def test_import_job_stats_filter(factories, superuser_api_client): + job1 = factories['music.ImportJob'](status='pending') + job2 = factories['music.ImportJob'](status='errored') + + url = reverse('api:v1:import-jobs-stats') + response = superuser_api_client.get(url, {'batch': job1.batch.pk}) + expected = { + 'errored': 0, + 'pending': 1, + 'finished': 0, + 'skipped': 0, + 'count': 1, + } + assert response.status_code == 200 + assert response.data == expected diff --git a/changes/changelog.d/171.enhancement b/changes/changelog.d/171.enhancement new file mode 100644 index 0000000000000000000000000000000000000000..14e9f6d9bb4f35bd0c5a933670d9c2f9513fb024 --- /dev/null +++ b/changes/changelog.d/171.enhancement @@ -0,0 +1,2 @@ +Import job and batch API and front-end have been improved with better performance, +pagination and additional filters (#171) diff --git a/front/src/components/library/import/BatchDetail.vue b/front/src/components/library/import/BatchDetail.vue index c7894fcc0249c4f27f58e6c6f29327b52f3929f5..b73c8cf8257599e87cc21aea05ce669f58aaac45 100644 --- a/front/src/components/library/import/BatchDetail.vue +++ b/front/src/components/library/import/BatchDetail.vue @@ -4,31 +4,80 @@ <div :class="['ui', 'centered', 'active', 'inline', 'loader']"></div> </div> <div v-if="batch" class="ui vertical stripe segment"> - <div :class=" - ['ui', - {'active': batch.status === 'pending'}, - {'warning': batch.status === 'pending'}, - {'error': batch.status === 'errored'}, - {'success': batch.status === 'finished'}, - 'progress']"> - <div class="bar" :style="progressBarStyle"> - <div class="progress"></div> + <table class="ui very basic table"> + <tbody> + <tr> + <td> + <strong>{{ $t('Import batch') }}</strong> + </td> + <td> + #{{ batch.id }} + </td> + </tr> + <tr> + <td> + <strong>{{ $t('Launch date') }}</strong> + </td> + <td> + <human-date :date="batch.creation_date"></human-date> + </td> + </tr> + <tr v-if="batch.user"> + <td> + <strong>{{ $t('Submitted by') }}</strong> + </td> + <td> + <username :username="batch.user.username" /> + </td> + </tr> + <tr v-if="stats"> + <td><strong>{{ $t('Pending') }}</strong></td> + <td>{{ stats.pending }}</td> + </tr> + <tr v-if="stats"> + <td><strong>{{ $t('Skipped') }}</strong></td> + <td>{{ stats.skipped }}</td> + </tr> + <tr v-if="stats"> + <td><strong>{{ $t('Errored') }}</strong></td> + <td>{{ stats.errored }}</td> + </tr> + <tr v-if="stats"> + <td><strong>{{ $t('Finished') }}</strong></td> + <td>{{ stats.finished }}/{{ stats.count}}</td> + </tr> + </tbody> + </table> + <div class="ui inline form"> + <div class="fields"> + <div class="ui field"> + <label>{{ $t('Search') }}</label> + <input type="text" v-model="jobFilters.search" placeholder="Search by source..." /> + </div> + <div class="ui field"> + <label>{{ $t('Status') }}</label> + <select class="ui dropdown" v-model="jobFilters.status"> + <option :value="null">{{ $t('Any') }}</option> + <option :value="'pending'">{{ $t('Pending') }}</option> + <option :value="'errored'">{{ $t('Errored') }}</option> + <option :value="'finished'">{{ $t('Success') }}</option> + <option :value="'skipped'">{{ $t('Skipped') }}</option> + </select> + </div> </div> - <div v-if="batch.status === 'pending'" class="label">Importing {{ batch.jobs.length }} tracks...</div> - <div v-if="batch.status === 'finished'" class="label">Imported {{ batch.jobs.length }} tracks!</div> </div> - <table class="ui unstackable table"> + <table v-if="jobResult" class="ui unstackable table"> <thead> <tr> - <i18next tag="th" path="Job ID"/> - <i18next tag="th" path="Recording MusicBrainz ID"/> - <i18next tag="th" path="Source"/> - <i18next tag="th" path="Status"/> - <i18next tag="th" path="Track"/> + <th>{{ $t('Job ID') }}</th> + <th>{{ $t('Recording MusicBrainz ID') }}</th> + <th>{{ $t('Source') }}</th> + <th>{{ $t('Status') }}</th> + <th>{{ $t('Track') }}</th> </tr> </thead> <tbody> - <tr v-for="job in batch.jobs"> + <tr v-for="job in jobResult.results"> <td>{{ job.id }}</th> <td> <a :href="'https://www.musicbrainz.org/recording/' + job.mbid" target="_blank">{{ job.mbid }}</a> @@ -45,29 +94,64 @@ </td> </tr> </tbody> + <tfoot class="full-width"> + <tr> + <th> + <pagination + v-if="jobResult && jobResult.results.length > 0" + @page-changed="selectPage" + :compact="true" + :current="jobFilters.page" + :paginate-by="jobFilters.paginateBy" + :total="jobResult.count" + ></pagination> + </th> + <th v-if="jobResult && jobResult.results.length > 0"> + {{ $t('Showing results {%start%}-{%end%} on {%total%}', {start: ((jobFilters.page-1) * jobFilters.paginateBy) + 1 , end: ((jobFilters.page-1) * jobFilters.paginateBy) + jobResult.results.length, total: jobResult.count})}} + <th> + <th></th> + <th></th> + <th></th> + </tr> + </tfoot> </table> - </div> </div> </template> <script> +import _ from 'lodash' import axios from 'axios' import logger from '@/logging' - -const FETCH_URL = 'import-batches/' +import Pagination from '@/components/Pagination' export default { props: ['id'], + components: { + Pagination + }, data () { return { isLoading: true, batch: null, - timeout: null + stats: null, + jobResult: null, + timeout: null, + jobFilters: { + status: null, + source: null, + search: '', + paginateBy: 25, + page: 1 + } } }, created () { - this.fetchData() + let self = this + this.fetchData().then(() => { + self.fetchJobs() + self.fetchStats() + }) }, destroyed () { if (this.timeout) { @@ -78,9 +162,9 @@ export default { fetchData () { var self = this this.isLoading = true - let url = FETCH_URL + this.id + '/' + let url = 'import-batches/' + this.id + '/' logger.default.debug('Fetching batch "' + this.id + '"') - axios.get(url).then((response) => { + return axios.get(url).then((response) => { self.batch = response.data self.isLoading = false if (self.batch.status === 'pending') { @@ -90,21 +174,58 @@ export default { ) } }) - } - }, - computed: { - progress () { - return this.batch.jobs.filter(j => { - return j.status !== 'pending' - }).length * 100 / this.batch.jobs.length }, - progressBarStyle () { - return 'width: ' + parseInt(this.progress) + '%' + fetchStats () { + var self = this + let url = 'import-jobs/stats/' + axios.get(url, {params: {batch: self.id}}).then((response) => { + let old = self.stats + self.stats = response.data + self.isLoading = false + if (!_.isEqual(old, self.stats)) { + self.fetchJobs() + self.fetchData() + } + if (self.batch.status === 'pending') { + self.timeout = setTimeout( + self.fetchStats, + 5000 + ) + } + }) + }, + fetchJobs () { + let params = { + batch: this.id, + page_size: this.jobFilters.paginateBy, + page: this.jobFilters.page, + q: this.jobFilters.search + } + if (this.jobFilters.status) { + params.status = this.jobFilters.status + } + if (this.jobFilters.source) { + params.source = this.jobFilters.source + } + let self = this + axios.get('import-jobs/', {params}).then((response) => { + self.jobResult = response.data + }) + }, + selectPage: function (page) { + this.jobFilters.page = page } + }, watch: { id () { this.fetchData() + }, + jobFilters: { + handler () { + this.fetchJobs() + }, + deep: true } } } diff --git a/front/src/components/library/import/BatchList.vue b/front/src/components/library/import/BatchList.vue index 324c3990a1494892435162bb52aa78fb516478c1..bf5a0ca47599e79b56559563aca8291fb3aff0e4 100644 --- a/front/src/components/library/import/BatchList.vue +++ b/front/src/components/library/import/BatchList.vue @@ -2,76 +2,144 @@ <div v-title="'Import Batches'"> <div class="ui vertical stripe segment"> <div v-if="isLoading" :class="['ui', 'centered', 'active', 'inline', 'loader']"></div> - <button - class="ui left floated labeled icon button" - @click="fetchData(previousLink)" - :disabled="!previousLink"><i class="left arrow icon"></i><i18next path="Previous"/></button> - <button - class="ui right floated right labeled icon button" - @click="fetchData(nextLink)" - :disabled="!nextLink"><i18next path="Next"/><i class="right arrow icon"></i></button> - <div class="ui hidden clearing divider"></div> + <div class="ui inline form"> + <div class="fields"> + <div class="ui field"> + <label>{{ $t('Search') }}</label> + <input type="text" v-model="filters.search" placeholder="Search by submitter, source..." /> + </div> + <div class="ui field"> + <label>{{ $t('Status') }}</label> + <select class="ui dropdown" v-model="filters.status"> + <option :value="null">{{ $t('Any') }}</option> + <option :value="'pending'">{{ $t('Pending') }}</option> + <option :value="'errored'">{{ $t('Errored') }}</option> + <option :value="'finished'">{{ $t('Success') }}</option> + </select> + </div> + <div class="ui field"> + <label>{{ $t('Import source') }}</label> + <select class="ui dropdown" v-model="filters.source"> + <option :value="null">{{ $t('Any') }}</option> + <option :value="'shell'">{{ $t('CLI') }}</option> + <option :value="'api'">{{ $t('API') }}</option> + <option :value="'federation'">{{ $t('Federation') }}</option> + </select> + </div> + </div> + </div> <div class="ui hidden clearing divider"></div> - <table v-if="results.length > 0" class="ui unstackable table"> + <table v-if="result && result.results.length > 0" class="ui unstackable table"> <thead> <tr> - <i18next tag="th" path="ID"/> - <i18next tag="th" path="Launch date"/> - <i18next tag="th" path="Jobs"/> - <i18next tag="th" path="Status"/> + <th>{{ $t('ID') }}</th> + <th>{{ $t('Launch date') }}</th> + <th>{{ $t('Jobs') }}</th> + <th>{{ $t('Status') }}</th> + <th>{{ $t('Source') }}</th> + <th>{{ $t('Submitted by') }}</th> </tr> </thead> <tbody> - <tr v-for="result in results"> - <td>{{ result.id }}</th> + <tr v-for="obj in result.results"> + <td>{{ obj.id }}</th> <td> - <router-link :to="{name: 'library.import.batches.detail', params: {id: result.id }}"> - {{ result.creation_date }} + <router-link :to="{name: 'library.import.batches.detail', params: {id: obj.id }}"> + <human-date :date="obj.creation_date"></human-date> </router-link> </td> - <td>{{ result.jobs.length }}</td> + <td>{{ obj.job_count }}</td> <td> <span - :class="['ui', {'yellow': result.status === 'pending'}, {'red': result.status === 'errored'}, {'green': result.status === 'finished'}, 'label']">{{ result.status }}</span> - </td> - </tr> - </tbody> - </table> - </div> + :class="['ui', {'yellow': obj.status === 'pending'}, {'red': obj.status === 'errored'}, {'green': obj.status === 'finished'}, 'label']">{{ obj.status }} + </span> + </td> + <td>{{ obj.source }}</td> + <td><template v-if="obj.submitted_by">{{ obj.submitted_by.username }}</template></td> + </tr> + </tbody> + <tfoot class="full-width"> + <tr> + <th> + <pagination + v-if="result && result.results.length > 0" + @page-changed="selectPage" + :compact="true" + :current="filters.page" + :paginate-by="filters.paginateBy" + :total="result.count" + ></pagination> + </th> + <th v-if="result && result.results.length > 0"> + {{ $t('Showing results {%start%}-{%end%} on {%total%}', {start: ((filters.page-1) * filters.paginateBy) + 1 , end: ((filters.page-1) * filters.paginateBy) + result.results.length, total: result.count})}} + <th> + <th></th> + <th></th> + <th></th> + </tr> + </tfoot> + </table> + </div> </div> </template> <script> import axios from 'axios' import logger from '@/logging' - -const BATCHES_URL = 'import-batches/' +import Pagination from '@/components/Pagination' export default { - components: {}, + components: { + Pagination + }, data () { return { - results: [], + result: null, isLoading: false, - nextLink: null, - previousLink: null + filters: { + status: null, + source: null, + search: '', + paginateBy: 25, + page: 1 + } } }, created () { - this.fetchData(BATCHES_URL) + this.fetchData() }, methods: { - fetchData (url) { + fetchData () { + let params = { + page_size: this.filters.paginateBy, + page: this.filters.page, + q: this.filters.search + } + if (this.filters.status) { + params.status = this.filters.status + } + if (this.filters.source) { + params.source = this.filters.source + } var self = this this.isLoading = true logger.default.time('Loading import batches') - axios.get(url, {}).then((response) => { - self.results = response.data.results - self.nextLink = response.data.next - self.previousLink = response.data.previous + axios.get('import-batches/', {params}).then((response) => { + self.result = response.data logger.default.timeEnd('Loading import batches') self.isLoading = false }) + }, + selectPage: function (page) { + this.filters.page = page + } + }, + watch: { + filters: { + handler () { + this.fetchData() + }, + deep: true } } }