Merge branch '171-import-jobs' into 'develop'

Resolve "Dedicated API endpoint for import jobs" Closes #171 See merge request funkwhale/funkwhale!156
2018-04-22 13:20:07 +00:00 · 2018-04-22 13:20:07 +00:00 · ad43d160bd
parent 5f6b1ae6e5 6a67bc6fac
commit ad43d160bd
8 changed files with 400 additions and 109 deletions
--- 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')

--- 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):
--- 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.
    """
--- 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)
--- 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
--- a/changes/changelog.d/171.enhancement
+++ 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)
--- 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
    }
  }
 }
--- 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 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>
-      <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
    }
  }
 }