From f1a1b93ee54b0946f7f2fd3027935dd9265f2d8c Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Wed, 23 May 2018 19:52:47 +0200
Subject: [PATCH 1/9] See #228: serializer logic

---
 api/funkwhale_api/common/serializers.py | 74 ++++++++++++++++++++
 api/tests/common/test_serializers.py    | 89 +++++++++++++++++++++++++
 2 files changed, 163 insertions(+)
 create mode 100644 api/funkwhale_api/common/serializers.py
 create mode 100644 api/tests/common/test_serializers.py

diff --git a/api/funkwhale_api/common/serializers.py b/api/funkwhale_api/common/serializers.py
new file mode 100644
index 000000000..7e214d7db
--- /dev/null
+++ b/api/funkwhale_api/common/serializers.py
@@ -0,0 +1,74 @@
+from rest_framework import serializers
+
+
+class ActionSerializer(serializers.Serializer):
+    """
+    A special serializer that can operate on a list of objects
+    and apply actions on it.
+    """
+
+    action = serializers.CharField(required=True)
+    objects = serializers.JSONField(required=True)
+    filters = serializers.DictField(required=False)
+    actions = None
+    filterset_class = None
+
+    def __init__(self, *args, **kwargs):
+        self.queryset = kwargs.pop('queryset')
+        if self.actions is None:
+            raise ValueError(
+                'You must declare a list of actions on '
+                'the serializer class')
+
+        for action in self.actions:
+            handler_name = 'handle_{}'.format(action)
+            assert hasattr(self, handler_name), (
+                '{} miss a {} method'.format(
+                    self.__class__.__name__, handler_name)
+            )
+        super().__init__(self, *args, **kwargs)
+
+    def validate_action(self, value):
+        if value not in self.actions:
+            raise serializers.ValidationError(
+                '{} is not a valid action. Pick one of {}.'.format(
+                    value, ', '.join(self.actions)
+                )
+            )
+        return value
+
+    def validate_objects(self, value):
+        qs = None
+        if value == 'all':
+            return self.queryset.all().order_by('id')
+        if type(value) in [list, tuple]:
+            return self.queryset.filter(pk__in=value).order_by('id')
+
+        raise serializers.ValidationError(
+            '{} is not a valid value for objects. You must provide either a '
+            'list of identifiers or the string "all".'.format(value))
+
+    def validate(self, data):
+        if not self.filterset_class or 'filters' not in data:
+            # no additional filters to apply, we just skip
+            return data
+
+        qs_filterset = self.filterset_class(
+            data['filters'], queryset=data['objects'])
+        try:
+            assert qs_filterset.form.is_valid()
+        except (AssertionError, TypeError):
+            raise serializers.ValidationError('Invalid filters')
+        data['objects'] = qs_filterset.qs
+        return data
+
+    def save(self):
+        handler_name = 'handle_{}'.format(self.validated_data['action'])
+        handler = getattr(self, handler_name)
+        result = handler(self.validated_data['objects'])
+        payload = {
+            'updated': self.validated_data['objects'].count(),
+            'action': self.validated_data['action'],
+            'result': result,
+        }
+        return payload
diff --git a/api/tests/common/test_serializers.py b/api/tests/common/test_serializers.py
new file mode 100644
index 000000000..075e957f6
--- /dev/null
+++ b/api/tests/common/test_serializers.py
@@ -0,0 +1,89 @@
+import django_filters
+
+from funkwhale_api.common import serializers
+from funkwhale_api.users import models
+
+
+class TestActionFilterSet(django_filters.FilterSet):
+    class Meta:
+        model = models.User
+        fields = ['is_active']
+
+
+class TestSerializer(serializers.ActionSerializer):
+    actions = ['test']
+    filterset_class = TestActionFilterSet
+
+    def handle_test(self, objects):
+        return {'hello': 'world'}
+
+
+def test_action_serializer_validates_action():
+    data = {'objects': 'all', 'action': 'nope'}
+    serializer = TestSerializer(data, queryset=models.User.objects.none())
+
+    assert serializer.is_valid() is False
+    assert 'action' in serializer.errors
+
+
+def test_action_serializer_validates_objects():
+    data = {'objects': 'nope', 'action': 'test'}
+    serializer = TestSerializer(data, queryset=models.User.objects.none())
+
+    assert serializer.is_valid() is False
+    assert 'objects' in serializer.errors
+
+
+def test_action_serializers_objects_clean_ids(factories):
+    user1 = factories['users.User']()
+    user2 = factories['users.User']()
+
+    data = {'objects': [user1.pk], 'action': 'test'}
+    serializer = TestSerializer(data, queryset=models.User.objects.all())
+
+    assert serializer.is_valid() is True
+    assert list(serializer.validated_data['objects']) == [user1]
+
+
+def test_action_serializers_objects_clean_all(factories):
+    user1 = factories['users.User']()
+    user2 = factories['users.User']()
+
+    data = {'objects': 'all', 'action': 'test'}
+    serializer = TestSerializer(data, queryset=models.User.objects.all())
+
+    assert serializer.is_valid() is True
+    assert list(serializer.validated_data['objects']) == [user1, user2]
+
+
+def test_action_serializers_save(factories, mocker):
+    handler = mocker.spy(TestSerializer, 'handle_test')
+    user1 = factories['users.User']()
+    user2 = factories['users.User']()
+
+    data = {'objects': 'all', 'action': 'test'}
+    serializer = TestSerializer(data, queryset=models.User.objects.all())
+
+    assert serializer.is_valid() is True
+    result = serializer.save()
+    assert result == {
+        'updated': 2,
+        'action': 'test',
+        'result': {'hello': 'world'},
+    }
+    handler.assert_called_once()
+
+
+def test_action_serializers_filterset(factories):
+    user1 = factories['users.User'](is_active=False)
+    user2 = factories['users.User'](is_active=True)
+
+    data = {
+        'objects': 'all',
+        'action': 'test',
+        'filters': {'is_active': True},
+    }
+    serializer = TestSerializer(data, queryset=models.User.objects.all())
+
+    assert serializer.is_valid() is True
+    assert list(serializer.validated_data['objects']) == [user2]

From ba4b6f6ba609ad9db5043d623e2198e6e9d0161b Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Wed, 23 May 2018 21:50:23 +0200
Subject: [PATCH 2/9] See #228: now use our new action logic for library track
 import

---
 api/funkwhale_api/common/serializers.py       | 24 +++++++------
 api/funkwhale_api/federation/serializers.py   | 31 ++++++++++++++++-
 api/funkwhale_api/federation/views.py         | 19 +++++++++--
 api/funkwhale_api/music/serializers.py        | 22 ------------
 api/funkwhale_api/music/views.py              | 16 ---------
 api/tests/common/test_serializers.py          | 11 ++++++
 api/tests/federation/test_views.py            | 34 +++++++++++++++++++
 api/tests/music/test_views.py                 | 18 ----------
 .../federation/LibraryTrackTable.vue          |  7 ++--
 9 files changed, 109 insertions(+), 73 deletions(-)

diff --git a/api/funkwhale_api/common/serializers.py b/api/funkwhale_api/common/serializers.py
index 7e214d7db..62d9c567e 100644
--- a/api/funkwhale_api/common/serializers.py
+++ b/api/funkwhale_api/common/serializers.py
@@ -49,17 +49,19 @@ class ActionSerializer(serializers.Serializer):
             'list of identifiers or the string "all".'.format(value))
 
     def validate(self, data):
-        if not self.filterset_class or 'filters' not in data:
-            # no additional filters to apply, we just skip
-            return data
+        if self.filterset_class and 'filters' in data:
+            qs_filterset = self.filterset_class(
+                data['filters'], queryset=data['objects'])
+            try:
+                assert qs_filterset.form.is_valid()
+            except (AssertionError, TypeError):
+                raise serializers.ValidationError('Invalid filters')
+            data['objects'] = qs_filterset.qs
 
-        qs_filterset = self.filterset_class(
-            data['filters'], queryset=data['objects'])
-        try:
-            assert qs_filterset.form.is_valid()
-        except (AssertionError, TypeError):
-            raise serializers.ValidationError('Invalid filters')
-        data['objects'] = qs_filterset.qs
+        data['count'] = data['objects'].count()
+        if data['count'] < 1:
+            raise serializers.ValidationError(
+                'No object matching your request')
         return data
 
     def save(self):
@@ -67,7 +69,7 @@ class ActionSerializer(serializers.Serializer):
         handler = getattr(self, handler_name)
         result = handler(self.validated_data['objects'])
         payload = {
-            'updated': self.validated_data['objects'].count(),
+            'updated': self.validated_data['count'],
             'action': self.validated_data['action'],
             'result': result,
         }
diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py
index 51561e222..77cb25096 100644
--- a/api/funkwhale_api/federation/serializers.py
+++ b/api/funkwhale_api/federation/serializers.py
@@ -10,8 +10,11 @@ from rest_framework import serializers
 from dynamic_preferences.registries import global_preferences_registry
 
 from funkwhale_api.common import utils as funkwhale_utils
-
+from funkwhale_api.common import serializers as common_serializers
+from funkwhale_api.music import models as music_models
+from funkwhale_api.music import tasks as music_tasks
 from . import activity
+from . import filters
 from . import models
 from . import utils
 
@@ -806,3 +809,29 @@ class CollectionSerializer(serializers.Serializer):
         if self.context.get('include_ap_context', True):
             d['@context'] = AP_CONTEXT
         return d
+
+
+class LibraryTrackActionSerializer(common_serializers.ActionSerializer):
+    actions = ['import']
+    filterset_class = filters.LibraryTrackFilter
+
+    @transaction.atomic
+    def handle_import(self, objects):
+        batch = music_models.ImportBatch.objects.create(
+            source='federation',
+            submitted_by=self.context['submitted_by']
+        )
+        for lt in objects:
+            job = music_models.ImportJob.objects.create(
+                batch=batch,
+                library_track=lt,
+                mbid=lt.mbid,
+                source=lt.url,
+            )
+            funkwhale_utils.on_commit(
+                music_tasks.import_job_run.delay,
+                import_job_id=job.pk,
+                use_acoustid=False,
+            )
+
+        return {'batch': {'id': batch.pk}}
diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py
index 06a2cd040..01bc02fdf 100644
--- a/api/funkwhale_api/federation/views.py
+++ b/api/funkwhale_api/federation/views.py
@@ -15,7 +15,7 @@ from rest_framework.serializers import ValidationError
 
 from funkwhale_api.common import preferences
 from funkwhale_api.common import utils as funkwhale_utils
-from funkwhale_api.music.models import TrackFile
+from funkwhale_api.music import models as music_models
 from funkwhale_api.users.permissions import HasUserPermission
 
 from . import activity
@@ -148,7 +148,9 @@ class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet):
     def list(self, request, *args, **kwargs):
         page = request.GET.get('page')
         library = actors.SYSTEM_ACTORS['library'].get_actor_instance()
-        qs = TrackFile.objects.order_by('-creation_date').select_related(
+        qs = music_models.TrackFile.objects.order_by(
+            '-creation_date'
+        ).select_related(
             'track__artist',
             'track__album__artist'
         ).filter(library_track__isnull=True)
@@ -307,3 +309,16 @@ class LibraryTrackViewSet(
         'fetched_date',
         'published_date',
     )
+
+    @list_route(methods=['post'])
+    def action(self, request, *args, **kwargs):
+        queryset = models.LibraryTrack.objects.filter(
+            local_track_file__isnull=True)
+        serializer = serializers.LibraryTrackActionSerializer(
+            request.data,
+            queryset=queryset,
+            context={'submitted_by': request.user}
+        )
+        serializer.is_valid(raise_exception=True)
+        result = serializer.save()
+        return response.Response(result, status=200)
diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py
index c77983a40..b72bb8c4a 100644
--- a/api/funkwhale_api/music/serializers.py
+++ b/api/funkwhale_api/music/serializers.py
@@ -250,28 +250,6 @@ class TrackActivitySerializer(activity_serializers.ModelSerializer):
         return 'Audio'
 
 
-class SubmitFederationTracksSerializer(serializers.Serializer):
-    library_tracks = serializers.PrimaryKeyRelatedField(
-        many=True,
-        queryset=LibraryTrack.objects.filter(local_track_file__isnull=True),
-    )
-
-    @transaction.atomic
-    def save(self, **kwargs):
-        batch = models.ImportBatch.objects.create(
-            source='federation',
-            **kwargs
-        )
-        for lt in self.validated_data['library_tracks']:
-            models.ImportJob.objects.create(
-                batch=batch,
-                library_track=lt,
-                mbid=lt.mbid,
-                source=lt.url,
-            )
-        return batch
-
-
 class ImportJobRunSerializer(serializers.Serializer):
     jobs = serializers.PrimaryKeyRelatedField(
         many=True,
diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py
index 24a9cbbcd..b42ad1c35 100644
--- a/api/funkwhale_api/music/views.py
+++ b/api/funkwhale_api/music/views.py
@@ -449,22 +449,6 @@ class SubmitViewSet(viewsets.ViewSet):
             data, request, batch=None, import_request=import_request)
         return Response(import_data)
 
-    @list_route(methods=['post'])
-    @transaction.non_atomic_requests
-    def federation(self, request, *args, **kwargs):
-        serializer = serializers.SubmitFederationTracksSerializer(
-            data=request.data)
-        serializer.is_valid(raise_exception=True)
-        batch = serializer.save(submitted_by=request.user)
-        for job in batch.jobs.all():
-            funkwhale_utils.on_commit(
-                tasks.import_job_run.delay,
-                import_job_id=job.pk,
-                use_acoustid=False,
-            )
-
-        return Response({'id': batch.id}, status=201)
-
     @transaction.atomic
     def _import_album(self, data, request, batch=None, import_request=None):
         # we import the whole album here to prevent race conditions that occurs
diff --git a/api/tests/common/test_serializers.py b/api/tests/common/test_serializers.py
index 075e957f6..563676556 100644
--- a/api/tests/common/test_serializers.py
+++ b/api/tests/common/test_serializers.py
@@ -87,3 +87,14 @@ def test_action_serializers_filterset(factories):
 
     assert serializer.is_valid() is True
     assert list(serializer.validated_data['objects']) == [user2]
+
+
+def test_action_serializers_validates_at_least_one_object():
+    data = {
+        'objects': 'all',
+        'action': 'test',
+    }
+    serializer = TestSerializer(data, queryset=models.User.objects.none())
+
+    assert serializer.is_valid() is False
+    assert 'non_field_errors' in serializer.errors
diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py
index 10237ed9f..1b0de5754 100644
--- a/api/tests/federation/test_views.py
+++ b/api/tests/federation/test_views.py
@@ -418,3 +418,37 @@ def test_can_filter_pending_follows(factories, superuser_api_client):
 
     assert response.status_code == 200
     assert len(response.data['results']) == 0
+
+
+def test_library_track_action_import(
+        factories, superuser_api_client, mocker):
+    lt1 = factories['federation.LibraryTrack']()
+    lt2 = factories['federation.LibraryTrack'](library=lt1.library)
+    lt3 = factories['federation.LibraryTrack']()
+    lt4 = factories['federation.LibraryTrack'](library=lt3.library)
+    mocker.patch('funkwhale_api.music.tasks.import_job_run')
+
+    payload = {
+        'objects': 'all',
+        'action': 'import',
+        'filters': {
+            'library': lt1.library.uuid
+        }
+    }
+    url = reverse('api:v1:federation:library-tracks-action')
+    response = superuser_api_client.post(url, payload, format='json')
+    batch = superuser_api_client.user.imports.latest('id')
+    expected = {
+        'updated': 2,
+        'action': 'import',
+        'result': {
+            'batch': {'id': batch.pk}
+        }
+    }
+
+    imported_lts = [lt1, lt2]
+    assert response.status_code == 200
+    assert response.data == expected
+    assert batch.jobs.count() == 2
+    for i, job in enumerate(batch.jobs.all()):
+        assert job.library_track == imported_lts[i]
diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py
index 38366442f..9328ba329 100644
--- a/api/tests/music/test_views.py
+++ b/api/tests/music/test_views.py
@@ -249,24 +249,6 @@ def test_serve_updates_access_date(factories, settings, api_client):
     assert track_file.accessed_date > now
 
 
-def test_can_create_import_from_federation_tracks(
-        factories, superuser_api_client, mocker):
-    lts = factories['federation.LibraryTrack'].create_batch(size=5)
-    mocker.patch('funkwhale_api.music.tasks.import_job_run')
-
-    payload = {
-        'library_tracks': [l.pk for l in lts]
-    }
-    url = reverse('api:v1:submit-federation')
-    response = superuser_api_client.post(url, payload)
-
-    assert response.status_code == 201
-    batch = superuser_api_client.user.imports.latest('id')
-    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')
diff --git a/front/src/components/federation/LibraryTrackTable.vue b/front/src/components/federation/LibraryTrackTable.vue
index d8ee48bf2..ad8b801a6 100644
--- a/front/src/components/federation/LibraryTrackTable.vue
+++ b/front/src/components/federation/LibraryTrackTable.vue
@@ -157,10 +157,11 @@ export default {
       let self = this
       self.isImporting = true
       let payload = {
-        library_tracks: this.checked
+        objects: this.checked,
+        action: 'import'
       }
-      axios.post('/submit/federation/', payload).then((response) => {
-        self.importBatch = response.data
+      axios.post('/federation/library-tracks/action/', payload).then((response) => {
+        self.importBatch = response.data.result.batch
         self.isImporting = false
         self.fetchData()
       }, error => {

From c6b5c71d268dc65329bd945c84f980cebb48c91a Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Wed, 23 May 2018 22:55:33 +0200
Subject: [PATCH 3/9] Upgraded to latest vue to benefit from slot scope

---
 front/package.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/front/package.json b/front/package.json
index 8844e8bee..3dec9c257 100644
--- a/front/package.json
+++ b/front/package.json
@@ -33,7 +33,7 @@
     "raven-js": "^3.22.3",
     "semantic-ui-css": "^2.2.10",
     "showdown": "^1.8.6",
-    "vue": "^2.3.3",
+    "vue": "^2.5.16",
     "vue-lazyload": "^1.1.4",
     "vue-masonry": "^0.10.16",
     "vue-router": "^2.3.1",

From fc88f72a71a45255114fb5dfcfc2eeb2599ba54a Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Thu, 24 May 2018 00:05:00 +0200
Subject: [PATCH 4/9] See #228: generic action table component

---
 front/src/components/common/ActionTable.vue | 150 ++++++++++++++++++++
 1 file changed, 150 insertions(+)
 create mode 100644 front/src/components/common/ActionTable.vue

diff --git a/front/src/components/common/ActionTable.vue b/front/src/components/common/ActionTable.vue
new file mode 100644
index 000000000..b9822ca4d
--- /dev/null
+++ b/front/src/components/common/ActionTable.vue
@@ -0,0 +1,150 @@
+<template>
+  <table class="ui compact very basic single line unstackable table">
+    <thead>
+      <tr v-if="actions.length > 0 && objectsData.count > 0">
+        <th colspan="1000">
+          <div class="ui small form">
+            <div class="ui inline fields">
+              <div class="field">
+                <label>{{ $t('Actions') }}</label>
+                <select class="ui dropdown" v-model="currentAction">
+                  <option v-for="action in actions" :value="action[0]">
+                    {{ action[1] }}
+                  </option>
+                </select>
+              </div>
+              <div class="field">
+                <div
+                  @click="launchAction"
+                  :disabled="checked.length === 0"
+                  :class="['ui', {disabled: checked.length === 0}, {'loading': actionLoading}, 'button']">
+                  {{ $t('Go') }}</div>
+              </div>
+              <div class="count field">
+                <span v-if="selectAll">{{ $t('{% count %} on {% total %} selected', {count: objectsData.count, total: objectsData.count}) }}</span>
+                <span v-else>{{ $t('{% count %} on {% total %} selected', {count: checked.length, total: objectsData.count}) }}</span>
+                <template v-if="checked.length === objectsData.results.length">
+                  <a @click="selectAll = true" v-if="!selectAll">
+                    {{ $t('Select all {% total %} elements', {total: objectsData.count}) }}
+                  </a>
+                  <a @click="selectAll = false" v-else>
+                    {{ $t('Select only current page') }}
+                  </a>
+                </template>
+              </div>
+            </div>
+            <div v-if="actionErrors.length > 0" class="ui negative message">
+              <div class="header">{{ $t('Error while applying action') }}</div>
+              <ul class="list">
+                <li v-for="error in actionErrors">{{ error }}</li>
+              </ul>
+            </div>
+            <div v-if="actionResult" class="ui positive message">
+              <p>{{ $t('Action {% action %} was launched successfully on {% count %} objects.', {action: actionResult.action, count: actionResult.updated}) }}</p>
+              <slot name="action-success-footer" :result="actionResult">
+              </slot>
+            </div>
+          </div>
+        </th>
+      </tr>
+      <tr>
+        <th>
+          <div class="ui checkbox">
+            <input
+              type="checkbox"
+              @change="toggleCheckAll"
+              :checked="objectsData.results.length === checked.length"><label>&nbsp;</label>
+          </div>
+        </th>
+        <slot name="header-cells"></slot>
+      </tr>
+    </thead>
+    <tbody>
+      <tr v-for="obj in objectsData.results">
+        <td class="collapsing">
+          <input
+            type="checkbox"
+            @change="toggleCheck(obj.id)"
+            :checked="checked.indexOf(obj.id) > -1"><label>&nbsp;</label>
+          </div>
+        </td>
+        <slot name="row-cells" :obj="obj"></slot>
+      </tr>
+    </tbody>
+  </table>
+</template>
+<script>
+import axios from 'axios'
+
+export default {
+  props: {
+    actionUrl: {type: String, required: true},
+    objectsData: {type: Object, required: true},
+    actions: {type: Array, required: true, default: () => { return [] }},
+    filters: {type: Object, required: false, default: () => { return {} }}
+  },
+  components: {},
+  data () {
+    let d = {
+      checked: [],
+      actionLoading: false,
+      actionResult: null,
+      actionErrors: [],
+      currentAction: null,
+      selectAll: false
+    }
+    if (this.actions.length > 0) {
+      d.currentAction = this.actions[0][0]
+    }
+    return d
+  },
+  methods: {
+    toggleCheckAll () {
+      if (this.checked.length === this.objectsData.results.length) {
+        // we uncheck
+        this.checked = []
+      } else {
+        this.checked = this.objectsData.results.map(t => { return t.id })
+      }
+    },
+    toggleCheck (id) {
+      if (this.checked.indexOf(id) > -1) {
+        // we uncheck
+        this.selectAll = false
+        this.checked.splice(this.checked.indexOf(id), 1)
+      } else {
+        this.checked.push(id)
+      }
+    },
+    launchAction () {
+      let self = this
+      self.actionLoading = true
+      self.result = null
+      let payload = {
+        action: this.currentAction,
+        filters: this.filters
+      }
+      if (this.selectAll) {
+        payload.objects = 'all'
+      } else {
+        payload.objects = this.checked
+      }
+      axios.post(this.actionUrl, payload).then((response) => {
+        self.actionResult = response.data
+        self.actionLoading = false
+      }, error => {
+        self.actionLoading = false
+        self.actionErrors = error.backendErrors
+      })
+    }
+  }
+}
+</script>
+<style scoped>
+.count.field {
+  font-weight: normal;
+}
+.ui.form .inline.fields {
+  margin: 0;
+}
+</style>

From f12fe0047f3c46c143d64f2840d1be206b417914 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Thu, 24 May 2018 00:05:14 +0200
Subject: [PATCH 5/9] See #228: use action table component for federation
 library tracks

---
 .../federation/LibraryTrackTable.vue          | 193 +++++++-----------
 1 file changed, 73 insertions(+), 120 deletions(-)

diff --git a/front/src/components/federation/LibraryTrackTable.vue b/front/src/components/federation/LibraryTrackTable.vue
index ad8b801a6..551fb992a 100644
--- a/front/src/components/federation/LibraryTrackTable.vue
+++ b/front/src/components/federation/LibraryTrackTable.vue
@@ -16,89 +16,63 @@
         </div>
       </div>
     </div>
-    <table v-if="result" class="ui compact very basic single line unstackable table">
-      <thead>
-        <tr>
-          <th>
-            <div class="ui checkbox">
-              <input
-                type="checkbox"
-                @change="toggleCheckAll"
-                :checked="result.results.length === checked.length"><label>&nbsp;</label>
-            </div>
-          </th>
-          <i18next tag="th" path="Title"/>
-          <i18next tag="th" path="Artist"/>
-          <i18next tag="th" path="Album"/>
-          <i18next tag="th" path="Published date"/>
-          <i18next tag="th" v-if="showLibrary" path="Library"/>
-        </tr>
-      </thead>
-      <tbody>
-        <tr v-for="track in result.results">
-          <td class="collapsing">
-            <div v-if="!track.local_track_file" class="ui checkbox">
-              <input
-                type="checkbox"
-                @change="toggleCheck(track.id)"
-                :checked="checked.indexOf(track.id) > -1"><label>&nbsp;</label>
-            </div>
-            <div v-else class="ui label">
-              <i18next path="In library"/>
-            </div>
-          </td>
-          <td>
-            <span :title="track.title">{{ track.title|truncate(30) }}</span>
-          </td>
-          <td>
-            <span :title="track.artist_name">{{ track.artist_name|truncate(30) }}</span>
-          </td>
-          <td>
-            <span :title="track.album_title">{{ track.album_title|truncate(20) }}</span>
-          </td>
-          <td>
-            <human-date :date="track.published_date"></human-date>
-          </td>
-          <td v-if="showLibrary">
-            {{ track.library.actor.domain }}
-          </td>
-        </tr>
-      </tbody>
-      <tfoot class="full-width">
-        <tr>
-          <th>
-            <pagination
-            v-if="result && result.results.length > 0"
-            @page-changed="selectPage"
-            :compact="true"
-            :current="page"
-            :paginate-by="paginateBy"
-            :total="result.count"
-            ></pagination>
+    <action-table
+      v-if="result"
+      :objects-data="result"
+      :actions="[['import', $t('Import')]]"
+      :action-url="'federation/library-tracks/action/'"
+      :filters="actionFilters">
+      <template slot="header-cells">
+        <th>{{ $t('Status') }}</th>
+        <th>{{ $t('Title') }}</th>
+        <th>{{ $t('Artist') }}</th>
+        <th>{{ $t('Album') }}</th>
+        <th>{{ $t('Published date') }}</th>
+        <th v-if="showLibrary">{{ $t('Library') }}</th>
+      </template>
+      <template slot="action-success-footer" slot-scope="scope">
+        <router-link
+          v-if="scope.result.action === 'import'"
+          :to="{name: 'library.import.batches.detail', params: {id: scope.result.result.batch.id }}">
+          {{ $t('Import #{% id %} launched', {id: scope.result.result.batch.id}) }}
+        </router-link>
+      </template>
+      <template slot="row-cells" slot-scope="scope">
+        <td>
+          <span v-if="scope.obj.local_track_file" class="ui basic green label">{{ $t('In library') }}</span>
+          <span v-else class="ui basic yellow label">{{ $t('Not imported') }}</span>
+        </td>
+        <td>
+          <span :title="scope.obj.title">{{ scope.obj.title|truncate(30) }}</span>
+        </td>
+        <td>
+          <span :title="scope.obj.artist_name">{{ scope.obj.artist_name|truncate(30) }}</span>
+        </td>
+        <td>
+          <span :title="scope.obj.album_title">{{ scope.obj.album_title|truncate(20) }}</span>
+        </td>
+        <td>
+          <human-date :date="scope.obj.published_date"></human-date>
+        </td>
+        <td v-if="showLibrary">
+          {{ scope.obj.library.actor.domain }}
+        </td>
+      </template>
+    </action-table>
+    <div>
+      <pagination
+        v-if="result && result.results.length > 0"
+        @page-changed="selectPage"
+        :compact="true"
+        :current="page"
+        :paginate-by="paginateBy"
+        :total="result.count"
+        ></pagination>
 
-          </th>
-          <th v-if="result && result.results.length > 0">
-            {{ $t('Showing results {%start%}-{%end%} on {%total%}', {start: ((page-1) * paginateBy) + 1 , end: ((page-1) * paginateBy) + result.results.length, total: result.count})}}
-          <th>
-            <button
-              @click="launchImport"
-              :disabled="checked.length === 0 || isImporting"
-              :class="['ui', 'green', {loading: isImporting}, 'button']">
-              {{ $t('Import {%count%} tracks', {'count': checked.length}) }}
-            </button>
-            <router-link
-              v-if="importBatch"
-              :to="{name: 'library.import.batches.detail', params: {id: importBatch.id }}">
-              {{ $t('Import #{% id %} launched', {id: importBatch.id}) }}
-            </router-link>
-          </th>
-          <th></th>
-          <th></th>
-          <th></th>
-          <th v-if="showLibrary"></th>
-        </tr>
-      </tfoot>
-    </table>
+      <span v-if="result && result.results.length > 0">
+        {{ $t('Showing results {%start%}-{%end%} on {%total%}', {start: ((page-1) * paginateBy) + 1 , end: ((page-1) * paginateBy) + result.results.length, total: result.count})}}
+      </span>
+    </div>
   </div>
 </template>
 
@@ -107,6 +81,7 @@ import axios from 'axios'
 import _ from 'lodash'
 
 import Pagination from '@/components/Pagination'
+import ActionTable from '@/components/common/ActionTable'
 
 export default {
   props: {
@@ -114,7 +89,8 @@ export default {
     showLibrary: {type: Boolean, default: false}
   },
   components: {
-    Pagination
+    Pagination,
+    ActionTable
   },
   data () {
     return {
@@ -123,9 +99,6 @@ export default {
       page: 1,
       paginateBy: 25,
       search: '',
-      checked: {},
-      isImporting: false,
-      importBatch: null,
       importedFilter: null
     }
   },
@@ -153,47 +126,26 @@ export default {
         self.errors = error.backendErrors
       })
     },
-    launchImport () {
-      let self = this
-      self.isImporting = true
-      let payload = {
-        objects: this.checked,
-        action: 'import'
-      }
-      axios.post('/federation/library-tracks/action/', payload).then((response) => {
-        self.importBatch = response.data.result.batch
-        self.isImporting = false
-        self.fetchData()
-      }, error => {
-        self.isImporting = false
-        self.errors = error.backendErrors
-      })
-    },
-    toggleCheckAll () {
-      if (this.checked.length === this.result.results.length) {
-        // we uncheck
-        this.checked = []
-      } else {
-        this.checked = this.result.results.filter(t => {
-          return t.local_track_file === null
-        }).map(t => { return t.id })
-      }
-    },
-    toggleCheck (id) {
-      if (this.checked.indexOf(id) > -1) {
-        // we uncheck
-        this.checked.splice(this.checked.indexOf(id), 1)
-      } else {
-        this.checked.push(id)
-      }
-    },
     selectPage: function (page) {
       this.page = page
     }
   },
+  computed: {
+    actionFilters () {
+      var currentFilters = {
+        q: this.search
+      }
+      if (this.filters) {
+        return _.merge(currentFilters, this.filters)
+      } else {
+        return currentFilters
+      }
+    }
+  },
   watch: {
     search (newValue) {
       if (newValue.length > 0) {
+        this.page = 1
         this.fetchData()
       }
     },
@@ -201,6 +153,7 @@ export default {
       this.fetchData()
     },
     importedFilter () {
+      this.page = 1
       this.fetchData()
     }
   }

From 4f8db661fa80c3b1c38fdbbcbc84f64b0fe644c6 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Thu, 24 May 2018 18:33:26 +0200
Subject: [PATCH 6/9] See #228: now expose library track status in API

---
 api/funkwhale_api/federation/filters.py     | 16 ++++++++------
 api/funkwhale_api/federation/serializers.py | 13 ++++++++++++
 api/funkwhale_api/federation/views.py       |  2 +-
 api/tests/federation/test_serializers.py    | 23 +++++++++++++++++++++
 4 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/api/funkwhale_api/federation/filters.py b/api/funkwhale_api/federation/filters.py
index 7a388ff12..1d93f68b9 100644
--- a/api/funkwhale_api/federation/filters.py
+++ b/api/funkwhale_api/federation/filters.py
@@ -24,7 +24,7 @@ class LibraryFilter(django_filters.FilterSet):
 
 class LibraryTrackFilter(django_filters.FilterSet):
     library = django_filters.CharFilter('library__uuid')
-    imported = django_filters.CharFilter(method='filter_imported')
+    status = django_filters.CharFilter(method='filter_status')
     q = fields.SearchFilter(search_fields=[
         'artist_name',
         'title',
@@ -32,11 +32,15 @@ class LibraryTrackFilter(django_filters.FilterSet):
         'library__actor__domain',
     ])
 
-    def filter_imported(self, queryset, field_name, value):
-        if value.lower() in ['true', '1', 'yes']:
-            queryset = queryset.filter(local_track_file__isnull=False)
-        elif value.lower() in ['false', '0', 'no']:
-            queryset = queryset.filter(local_track_file__isnull=True)
+    def filter_status(self, queryset, field_name, value):
+        if value == 'imported':
+            return queryset.filter(local_track_file__isnull=False)
+        elif value == 'not_imported':
+            return queryset.filter(
+                local_track_file__isnull=True
+            ).exclude(import_jobs__status='pending')
+        elif value == 'import_pending':
+            return queryset.filter(import_jobs__status='pending')
         return queryset
 
     class Meta:
diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py
index 77cb25096..59e93a9b1 100644
--- a/api/funkwhale_api/federation/serializers.py
+++ b/api/funkwhale_api/federation/serializers.py
@@ -296,6 +296,7 @@ class APILibraryCreateSerializer(serializers.ModelSerializer):
 
 class APILibraryTrackSerializer(serializers.ModelSerializer):
     library = APILibrarySerializer()
+    status = serializers.SerializerMethodField()
 
     class Meta:
         model = models.LibraryTrack
@@ -314,8 +315,20 @@ class APILibraryTrackSerializer(serializers.ModelSerializer):
             'title',
             'library',
             'local_track_file',
+            'status',
         ]
 
+    def get_status(self, o):
+        try:
+            if o.local_track_file is not None:
+                return 'imported'
+        except music_models.TrackFile.DoesNotExist:
+            pass
+        for job in o.import_jobs.all():
+            if job.status == 'pending':
+                return 'import_pending'
+        return 'not_imported'
+
 
 class FollowSerializer(serializers.Serializer):
     id = serializers.URLField(max_length=500)
diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py
index 01bc02fdf..1350ec731 100644
--- a/api/funkwhale_api/federation/views.py
+++ b/api/funkwhale_api/federation/views.py
@@ -296,7 +296,7 @@ class LibraryTrackViewSet(
         'library__actor',
         'library__follow',
         'local_track_file',
-    )
+    ).prefetch_related('import_jobs')
     filter_class = filters.LibraryTrackFilter
     serializer_class = serializers.APILibraryTrackSerializer
     ordering_fields = (
diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py
index f298c61f5..fcf2ba1b6 100644
--- a/api/tests/federation/test_serializers.py
+++ b/api/tests/federation/test_serializers.py
@@ -699,3 +699,26 @@ def test_api_library_create_serializer_save(factories, r_mock):
     assert library.tracks_count == 10
     assert library.actor == actor
     assert library.follow == follow
+
+
+def test_tapi_library_track_serializer_not_imported(factories):
+    lt = factories['federation.LibraryTrack']()
+    serializer = serializers.APILibraryTrackSerializer(lt)
+
+    assert serializer.get_status(lt) == 'not_imported'
+
+
+def test_tapi_library_track_serializer_imported(factories):
+    tf = factories['music.TrackFile'](federation=True)
+    lt = tf.library_track
+    serializer = serializers.APILibraryTrackSerializer(lt)
+
+    assert serializer.get_status(lt) == 'imported'
+
+
+def test_tapi_library_track_serializer_import_pending(factories):
+    job = factories['music.ImportJob'](federation=True, status='pending')
+    lt = job.library_track
+    serializer = serializers.APILibraryTrackSerializer(lt)
+
+    assert serializer.get_status(lt) == 'import_pending'

From eded32c2e8271cfffb9575a123f0caabdcc851f9 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Thu, 24 May 2018 18:53:12 +0200
Subject: [PATCH 7/9] See #228: more performante federation import launch via
 API

---
 api/funkwhale_api/federation/serializers.py | 12 ++++++------
 api/funkwhale_api/music/tasks.py            |  7 +++++++
 api/tests/federation/test_views.py          |  4 +++-
 api/tests/music/test_tasks.py               |  9 +++++++++
 4 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py
index 59e93a9b1..6ffffaa9a 100644
--- a/api/funkwhale_api/federation/serializers.py
+++ b/api/funkwhale_api/federation/serializers.py
@@ -834,17 +834,17 @@ class LibraryTrackActionSerializer(common_serializers.ActionSerializer):
             source='federation',
             submitted_by=self.context['submitted_by']
         )
+        jobs = []
         for lt in objects:
-            job = music_models.ImportJob.objects.create(
+            job = music_models.ImportJob(
                 batch=batch,
                 library_track=lt,
                 mbid=lt.mbid,
                 source=lt.url,
             )
-            funkwhale_utils.on_commit(
-                music_tasks.import_job_run.delay,
-                import_job_id=job.pk,
-                use_acoustid=False,
-            )
+            jobs.append(job)
+
+        music_models.ImportJob.objects.bulk_create(jobs)
+        music_tasks.import_batch_run.delay(import_batch_id=batch.pk)
 
         return {'batch': {'id': batch.pk}}
diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py
index e5426904a..993456c27 100644
--- a/api/funkwhale_api/music/tasks.py
+++ b/api/funkwhale_api/music/tasks.py
@@ -173,6 +173,13 @@ def import_job_run(self, import_job, replace=False, use_acoustid=False):
         raise
 
 
+@celery.app.task(name='ImportBatch.run')
+@celery.require_instance(models.ImportBatch, 'import_batch')
+def import_batch_run(import_batch):
+    for job_id in import_batch.jobs.order_by('id').values_list('id', flat=True):
+        import_job_run.delay(import_job_id=job_id)
+
+
 @celery.app.task(name='Lyrics.fetch_content')
 @celery.require_instance(models.Lyrics, 'lyrics')
 def fetch_content(lyrics):
diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py
index 1b0de5754..04a419aed 100644
--- a/api/tests/federation/test_views.py
+++ b/api/tests/federation/test_views.py
@@ -426,7 +426,8 @@ def test_library_track_action_import(
     lt2 = factories['federation.LibraryTrack'](library=lt1.library)
     lt3 = factories['federation.LibraryTrack']()
     lt4 = factories['federation.LibraryTrack'](library=lt3.library)
-    mocker.patch('funkwhale_api.music.tasks.import_job_run')
+    mocked_run = mocker.patch(
+        'funkwhale_api.music.tasks.import_batch_run.delay')
 
     payload = {
         'objects': 'all',
@@ -452,3 +453,4 @@ def test_library_track_action_import(
     assert batch.jobs.count() == 2
     for i, job in enumerate(batch.jobs.all()):
         assert job.library_track == imported_lts[i]
+    mocked_run.assert_called_once_with(import_batch_id=batch.pk)
diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py
index 26cb9453e..dfe649be0 100644
--- a/api/tests/music/test_tasks.py
+++ b/api/tests/music/test_tasks.py
@@ -47,6 +47,15 @@ def test_set_acoustid_on_track_file_required_high_score(factories, mocker):
     assert track_file.acoustid_track_id is None
 
 
+def test_import_batch_run(factories, mocker):
+    job = factories['music.ImportJob']()
+    mocked_job_run = mocker.patch(
+        'funkwhale_api.music.tasks.import_job_run.delay')
+    tasks.import_batch_run(import_batch_id=job.batch.pk)
+
+    mocked_job_run.assert_called_once_with(import_job_id=job.pk)
+
+
 def test_import_job_can_run_with_file_and_acoustid(
         artists, albums, tracks, preferences, factories, mocker):
     preferences['providers_acoustid__api_key'] = 'test'

From 6586b2b73d6d094142a0d99edc6e750e4b5d925f Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Thu, 24 May 2018 20:07:14 +0200
Subject: [PATCH 8/9] See #228: smarter action table with shift-click select

---
 front/src/components/common/ActionTable.vue   | 101 ++++++++++++---
 .../src/components/common/DangerousButton.vue |  13 +-
 .../federation/LibraryTrackTable.vue          | 115 ++++++++++--------
 3 files changed, 159 insertions(+), 70 deletions(-)

diff --git a/front/src/components/common/ActionTable.vue b/front/src/components/common/ActionTable.vue
index b9822ca4d..718e57b19 100644
--- a/front/src/components/common/ActionTable.vue
+++ b/front/src/components/common/ActionTable.vue
@@ -1,29 +1,42 @@
 <template>
   <table class="ui compact very basic single line unstackable table">
     <thead>
-      <tr v-if="actions.length > 0 && objectsData.count > 0">
+      <tr v-if="actions.length > 0">
         <th colspan="1000">
           <div class="ui small form">
             <div class="ui inline fields">
               <div class="field">
                 <label>{{ $t('Actions') }}</label>
-                <select class="ui dropdown" v-model="currentAction">
-                  <option v-for="action in actions" :value="action[0]">
-                    {{ action[1] }}
+                <select class="ui dropdown" v-model="currentActionName">
+                  <option v-for="action in actions" :value="action.name">
+                    {{ action.label }}
                   </option>
                 </select>
               </div>
               <div class="field">
                 <div
+                  v-if="!selectAll"
                   @click="launchAction"
                   :disabled="checked.length === 0"
                   :class="['ui', {disabled: checked.length === 0}, {'loading': actionLoading}, 'button']">
                   {{ $t('Go') }}</div>
+                <dangerous-button
+                  v-else :class="['ui', {disabled: checked.length === 0}, {'loading': actionLoading}, 'button']"
+                  confirm-color="green"
+                  color=""
+                  @confirm="launchAction">
+                  {{ $t('Go') }}
+                  <p slot="modal-header">{{ $t('Do you want to launch action "{% action %}" on {% total %} elements?', {action: currentActionName, total: objectsData.count}) }}
+                  <p slot="modal-content">
+                    {{ $t('This may affect a lot of elements, please double check this is really what you want.')}}
+                  </p>
+                  <p slot="modal-confirm">{{ $t('Launch') }}</p>
+                </dangerous-button>
               </div>
               <div class="count field">
                 <span v-if="selectAll">{{ $t('{% count %} on {% total %} selected', {count: objectsData.count, total: objectsData.count}) }}</span>
                 <span v-else>{{ $t('{% count %} on {% total %} selected', {count: checked.length, total: objectsData.count}) }}</span>
-                <template v-if="checked.length === objectsData.results.length">
+                <template v-if="checkable.length === checked.length">
                   <a @click="selectAll = true" v-if="!selectAll">
                     {{ $t('Select all {% total %} elements', {total: objectsData.count}) }}
                   </a>
@@ -53,18 +66,20 @@
             <input
               type="checkbox"
               @change="toggleCheckAll"
-              :checked="objectsData.results.length === checked.length"><label>&nbsp;</label>
+              :disabled="checkable.length === 0"
+              :checked="checkable.length > 0 && checked.length === checkable.length"><label>&nbsp;</label>
           </div>
         </th>
         <slot name="header-cells"></slot>
       </tr>
     </thead>
-    <tbody>
-      <tr v-for="obj in objectsData.results">
+    <tbody v-if="objectsData.count > 0">
+      <tr v-for="(obj, index) in objectsData.results">
         <td class="collapsing">
           <input
             type="checkbox"
-            @change="toggleCheck(obj.id)"
+            :disabled="checkable.indexOf(obj.id) === -1"
+            @click="toggleCheck($event, obj.id, index)"
             :checked="checked.indexOf(obj.id) > -1"><label>&nbsp;</label>
           </div>
         </td>
@@ -90,38 +105,60 @@ export default {
       actionLoading: false,
       actionResult: null,
       actionErrors: [],
-      currentAction: null,
-      selectAll: false
+      currentActionName: null,
+      selectAll: false,
+      lastCheckedIndex: -1
     }
     if (this.actions.length > 0) {
-      d.currentAction = this.actions[0][0]
+      d.currentActionName = this.actions[0].name
     }
     return d
   },
   methods: {
     toggleCheckAll () {
-      if (this.checked.length === this.objectsData.results.length) {
+      this.lastCheckedIndex = -1
+      if (this.checked.length === this.checkable.length) {
         // we uncheck
         this.checked = []
       } else {
-        this.checked = this.objectsData.results.map(t => { return t.id })
+        this.checked = this.checkable.map(i => { return i })
       }
     },
-    toggleCheck (id) {
+    toggleCheck (event, id, index) {
+      let self = this
+      let affectedIds = [id]
+      let newValue = null
       if (this.checked.indexOf(id) > -1) {
         // we uncheck
         this.selectAll = false
-        this.checked.splice(this.checked.indexOf(id), 1)
+        newValue = false
       } else {
-        this.checked.push(id)
+        newValue = true
       }
+      if (event.shiftKey && this.lastCheckedIndex > -1) {
+        // we also add inbetween ids to the list of affected ids
+        let idxs = [index, this.lastCheckedIndex]
+        idxs.sort((a, b) => a - b)
+        let objs = this.objectsData.results.slice(idxs[0], idxs[1] + 1)
+        affectedIds = affectedIds.concat(objs.map((o) => { return o.id }))
+      }
+      affectedIds.forEach((i) => {
+        let checked = self.checked.indexOf(i) > -1
+        if (newValue && !checked && self.checkable.indexOf(i) > -1) {
+          return self.checked.push(i)
+        }
+        if (!newValue && checked) {
+          self.checked.splice(self.checked.indexOf(i), 1)
+        }
+      })
+      this.lastCheckedIndex = index
     },
     launchAction () {
       let self = this
       self.actionLoading = true
       self.result = null
       let payload = {
-        action: this.currentAction,
+        action: this.currentActionName,
         filters: this.filters
       }
       if (this.selectAll) {
@@ -132,11 +169,39 @@ export default {
       axios.post(this.actionUrl, payload).then((response) => {
         self.actionResult = response.data
         self.actionLoading = false
+        self.$emit('action-launched', response.data)
       }, error => {
         self.actionLoading = false
         self.actionErrors = error.backendErrors
       })
     }
+  },
+  computed: {
+    currentAction () {
+      let self = this
+      return this.actions.filter((a) => {
+        return a.name === self.currentActionName
+      })[0]
+    },
+    checkable () {
+      let objs = this.objectsData.results
+      let filter = this.currentAction.filterCheckable
+      if (filter) {
+        objs = objs.filter((o) => {
+          return filter(o)
+        })
+      }
+      return objs.map((o) => { return o.id })
+    }
+  },
+  watch: {
+    objectsData: {
+      handler () {
+        this.checked = []
+        this.selectAll = false
+      },
+      deep: true
+    }
   }
 }
 </script>
diff --git a/front/src/components/common/DangerousButton.vue b/front/src/components/common/DangerousButton.vue
index 690291d5b..52fcdca61 100644
--- a/front/src/components/common/DangerousButton.vue
+++ b/front/src/components/common/DangerousButton.vue
@@ -13,7 +13,7 @@
       </div>
       <div class="actions">
         <div class="ui cancel button"><i18next path="Cancel"/></div>
-        <div :class="['ui', 'confirm', color, 'button']" @click="confirm">
+        <div :class="['ui', 'confirm', confirmButtonColor, 'button']" @click="confirm">
           <slot name="modal-confirm"><i18next path="Confirm"/></slot>
         </div>
       </div>
@@ -28,7 +28,8 @@ export default {
   props: {
     action: {type: Function, required: false},
     disabled: {type: Boolean, default: false},
-    color: {type: String, default: 'red'}
+    color: {type: String, default: 'red'},
+    confirmColor: {type: String, default: null, required: false}
   },
   components: {
     Modal
@@ -38,6 +39,14 @@ export default {
       showModal: false
     }
   },
+  computed: {
+    confirmButtonColor () {
+      if (this.confirmColor) {
+        return this.confirmColor
+      }
+      return this.color
+    }
+  },
   methods: {
     confirm () {
       this.showModal = false
diff --git a/front/src/components/federation/LibraryTrackTable.vue b/front/src/components/federation/LibraryTrackTable.vue
index 551fb992a..43b52c835 100644
--- a/front/src/components/federation/LibraryTrackTable.vue
+++ b/front/src/components/federation/LibraryTrackTable.vue
@@ -10,55 +10,63 @@
           <label>{{ $t('Import status') }}</label>
           <select class="ui dropdown" v-model="importedFilter">
             <option :value="null">{{ $t('Any') }}</option>
-            <option :value="true">{{ $t('Imported') }}</option>
-            <option :value="false">{{ $t('Not imported') }}</option>
+            <option :value="'imported'">{{ $t('Imported') }}</option>
+            <option :value="'not_imported'">{{ $t('Not imported') }}</option>
+            <option :value="'import_pending'">{{ $t('Import pending') }}</option>
           </select>
         </div>
       </div>
     </div>
-    <action-table
-      v-if="result"
-      :objects-data="result"
-      :actions="[['import', $t('Import')]]"
-      :action-url="'federation/library-tracks/action/'"
-      :filters="actionFilters">
-      <template slot="header-cells">
-        <th>{{ $t('Status') }}</th>
-        <th>{{ $t('Title') }}</th>
-        <th>{{ $t('Artist') }}</th>
-        <th>{{ $t('Album') }}</th>
-        <th>{{ $t('Published date') }}</th>
-        <th v-if="showLibrary">{{ $t('Library') }}</th>
-      </template>
-      <template slot="action-success-footer" slot-scope="scope">
-        <router-link
-          v-if="scope.result.action === 'import'"
-          :to="{name: 'library.import.batches.detail', params: {id: scope.result.result.batch.id }}">
-          {{ $t('Import #{% id %} launched', {id: scope.result.result.batch.id}) }}
-        </router-link>
-      </template>
-      <template slot="row-cells" slot-scope="scope">
-        <td>
-          <span v-if="scope.obj.local_track_file" class="ui basic green label">{{ $t('In library') }}</span>
-          <span v-else class="ui basic yellow label">{{ $t('Not imported') }}</span>
-        </td>
-        <td>
-          <span :title="scope.obj.title">{{ scope.obj.title|truncate(30) }}</span>
-        </td>
-        <td>
-          <span :title="scope.obj.artist_name">{{ scope.obj.artist_name|truncate(30) }}</span>
-        </td>
-        <td>
-          <span :title="scope.obj.album_title">{{ scope.obj.album_title|truncate(20) }}</span>
-        </td>
-        <td>
-          <human-date :date="scope.obj.published_date"></human-date>
-        </td>
-        <td v-if="showLibrary">
-          {{ scope.obj.library.actor.domain }}
-        </td>
-      </template>
-    </action-table>
+    <div class="dimmable">
+      <div v-if="isLoading" class="ui active inverted dimmer">
+          <div class="ui loader"></div>
+      </div>
+      <action-table
+        v-if="result"
+        @action-launched="fetchData"
+        :objects-data="result"
+        :actions="actions"
+        :action-url="'federation/library-tracks/action/'"
+        :filters="actionFilters">
+        <template slot="header-cells">
+          <th>{{ $t('Status') }}</th>
+          <th>{{ $t('Title') }}</th>
+          <th>{{ $t('Artist') }}</th>
+          <th>{{ $t('Album') }}</th>
+          <th>{{ $t('Published date') }}</th>
+          <th v-if="showLibrary">{{ $t('Library') }}</th>
+        </template>
+        <template slot="action-success-footer" slot-scope="scope">
+          <router-link
+            v-if="scope.result.action === 'import'"
+            :to="{name: 'library.import.batches.detail', params: {id: scope.result.result.batch.id }}">
+            {{ $t('Import #{% id %} launched', {id: scope.result.result.batch.id}) }}
+          </router-link>
+        </template>
+        <template slot="row-cells" slot-scope="scope">
+          <td>
+            <span v-if="scope.obj.status === 'imported'" class="ui basic green label">{{ $t('In library') }}</span>
+            <span v-else-if="scope.obj.status === 'import_pending'" class="ui basic yellow label">{{ $t('Import pending') }}</span>
+            <span v-else class="ui basic label">{{ $t('Not imported') }}</span>
+          </td>
+          <td>
+            <span :title="scope.obj.title">{{ scope.obj.title|truncate(30) }}</span>
+          </td>
+          <td>
+            <span :title="scope.obj.artist_name">{{ scope.obj.artist_name|truncate(30) }}</span>
+          </td>
+          <td>
+            <span :title="scope.obj.album_title">{{ scope.obj.album_title|truncate(20) }}</span>
+          </td>
+          <td>
+            <human-date :date="scope.obj.published_date"></human-date>
+          </td>
+          <td v-if="showLibrary">
+            {{ scope.obj.library.actor.domain }}
+          </td>
+        </template>
+      </action-table>
+    </div>
     <div>
       <pagination
         v-if="result && result.results.length > 0"
@@ -113,7 +121,7 @@ export default {
         'q': this.search
       }, this.filters)
       if (this.importedFilter !== null) {
-        params.imported = this.importedFilter
+        params.status = this.importedFilter
       }
       let self = this
       self.isLoading = true
@@ -140,14 +148,21 @@ export default {
       } else {
         return currentFilters
       }
+    },
+    actions () {
+      return [
+        {
+          name: 'import',
+          label: this.$t('Import'),
+          filterCheckable: (obj) => { return obj.status === 'not_imported' }
+        }
+      ]
     }
   },
   watch: {
     search (newValue) {
-      if (newValue.length > 0) {
-        this.page = 1
-        this.fetchData()
-      }
+      this.page = 1
+      this.fetchData()
     },
     page () {
       this.fetchData()

From c547c7e14bb8e6813a8b9c69b219ce866579515a Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Thu, 24 May 2018 20:11:53 +0200
Subject: [PATCH 9/9] Fix #164 and #228: new action table component on
 federated tracks pages

---
 changes/changelog.d/164.enhancement | 2 ++
 changes/changelog.d/228.feature     | 3 +++
 2 files changed, 5 insertions(+)
 create mode 100644 changes/changelog.d/164.enhancement
 create mode 100644 changes/changelog.d/228.feature

diff --git a/changes/changelog.d/164.enhancement b/changes/changelog.d/164.enhancement
new file mode 100644
index 000000000..ceea6c2b8
--- /dev/null
+++ b/changes/changelog.d/164.enhancement
@@ -0,0 +1,2 @@
+Can now import a whole remote library at once thanks to new Action Table
+component (#164)
diff --git a/changes/changelog.d/228.feature b/changes/changelog.d/228.feature
new file mode 100644
index 000000000..548c1927e
--- /dev/null
+++ b/changes/changelog.d/228.feature
@@ -0,0 +1,3 @@
+New action table component for quick and efficient batch actions (#228)
+This is implemented on the federated tracks pages, but will be included
+in other pages as well depending on the feedback.