From afc8f22516921e9ed320ccea6da245d730beaa27 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Mon, 21 Oct 2019 09:25:36 +0200
Subject: [PATCH] Fix tag exclusion in custom radios (#950)

---
 api/funkwhale_api/radios/filters.py |  6 +++--
 api/tests/radios/test_radios.py     | 38 +++++++++++++++++++++++++++++
 changes/changelog.d/950.bugfix      |  1 +
 3 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100644 changes/changelog.d/950.bugfix

diff --git a/api/funkwhale_api/radios/filters.py b/api/funkwhale_api/radios/filters.py
index a92dbae88..f3abe22e0 100644
--- a/api/funkwhale_api/radios/filters.py
+++ b/api/funkwhale_api/radios/filters.py
@@ -30,7 +30,7 @@ def run(filters, **kwargs):
 
     if final_query:
         candidates = candidates.filter(final_query)
-    return candidates.order_by("pk")
+    return candidates.order_by("pk").distinct()
 
 
 def validate(filter_config):
@@ -100,7 +100,9 @@ class GroupFilter(RadioFilter):
             conf = collections.ChainMap(filter_config, kwargs)
             query = f.get_query(candidates, **conf)
             if filter_config.get("not", False):
-                query = ~query
+                # query = ~query *should* work but it doesn't (see #950)
+                # The line below generate a proper subquery
+                query = ~Q(pk__in=candidates.filter(query).values_list("pk", flat=True))
 
             if not final_query:
                 final_query = query
diff --git a/api/tests/radios/test_radios.py b/api/tests/radios/test_radios.py
index 040217aac..2aa60c36a 100644
--- a/api/tests/radios/test_radios.py
+++ b/api/tests/radios/test_radios.py
@@ -282,3 +282,41 @@ def test_session_radio_get_queryset_ignore_filtered_track_album_artist(
     radio.start_session(user=cf.user)
 
     assert radio.get_queryset() == [valid_track]
+
+
+def test_get_choices_for_custom_radio_exclude_artist(factories):
+    included_artist = factories["music.Artist"]()
+    excluded_artist = factories["music.Artist"]()
+    included_uploads = factories["music.Upload"].create_batch(
+        5, track__artist=included_artist
+    )
+    factories["music.Upload"].create_batch(5, track__artist=excluded_artist)
+
+    session = factories["radios.CustomRadioSession"](
+        custom_radio__config=[
+            {"type": "artist", "ids": [included_artist.pk]},
+            {"type": "artist", "ids": [excluded_artist.pk], "not": True},
+        ]
+    )
+    choices = session.radio.get_choices(filter_playable=False)
+
+    expected = [u.track.pk for u in included_uploads]
+    assert list(choices.values_list("id", flat=True)) == expected
+
+
+def test_get_choices_for_custom_radio_exclude_tag(factories):
+    included_uploads = factories["music.Upload"].create_batch(
+        5, track__set_tags=["rap"]
+    )
+    factories["music.Upload"].create_batch(5, track__set_tags=["rock", "rap"])
+
+    session = factories["radios.CustomRadioSession"](
+        custom_radio__config=[
+            {"type": "tag", "names": ["rap"]},
+            {"type": "tag", "names": ["rock"], "not": True},
+        ]
+    )
+    choices = session.radio.get_choices(filter_playable=False)
+
+    expected = [u.track.pk for u in included_uploads]
+    assert list(choices.values_list("id", flat=True)) == expected
diff --git a/changes/changelog.d/950.bugfix b/changes/changelog.d/950.bugfix
new file mode 100644
index 000000000..c1195739e
--- /dev/null
+++ b/changes/changelog.d/950.bugfix
@@ -0,0 +1 @@
+Fix tag exclusion in custom radios (#950)