From 9f48ec72ab2d17a1aca7a28f5909ceb6b43a0f07 Mon Sep 17 00:00:00 2001 From: Domonkos Kostyal Date: Sun, 14 Dec 2025 15:14:37 +0100 Subject: [PATCH 01/20] collection urls are meaningful, relevant sounds marked dirty when collection publicity is changed, featured sounds implementation has started --- freesound/settings.py | 2 +- fscollections/admin.py | 4 +- fscollections/models.py | 81 +++++++++++++++---- .../templatetags/display_collections.py | 20 +++-- fscollections/tests.py | 73 +++++++++-------- fscollections/urls.py | 16 ++-- fscollections/views.py | 29 ++++--- templates/collections/collection.html | 15 ++-- templates/collections/display_collection.html | 4 +- .../collections/display_featured_sound.html | 9 +++ templates/collections/edit_collection.html | 7 +- .../collections/modal_add_maintainer.html | 2 +- 12 files changed, 175 insertions(+), 87 deletions(-) create mode 100644 templates/collections/display_featured_sound.html diff --git a/freesound/settings.py b/freesound/settings.py index 821174536..a19c26ee8 100644 --- a/freesound/settings.py +++ b/freesound/settings.py @@ -1439,7 +1439,7 @@ # ------------------------------------------------------------------------------- # Collections -ENABLE_COLLECTIONS = False +ENABLE_COLLECTIONS = True MAX_SOUNDS_PER_COLLECTION = 250 # ------------------------------------------------------------------------------- diff --git a/fscollections/admin.py b/fscollections/admin.py index 5f9c5c59a..e47fda79a 100644 --- a/fscollections/admin.py +++ b/fscollections/admin.py @@ -7,9 +7,9 @@ @admin.register(Collection) class CollectionAdmin(admin.ModelAdmin): - fields = ["user", "name", "num_sounds", "public"] + fields = ["user", "name", "num_sounds", "public", "featured_sound_ids"] filter_horizontal = ["sounds"] - list_display = ("name", "user", "num_sounds", "public", "get_sounds") + list_display = ("name", "user", "num_sounds", "public", "get_sounds", "featured_sound_ids") readonly_fields = ["created"] actions = ["make_public", "make_private"] diff --git a/fscollections/models.py b/fscollections/models.py index 165d52b47..39e9d2cbd 100644 --- a/fscollections/models.py +++ b/fscollections/models.py @@ -21,10 +21,11 @@ from urllib.parse import quote from django.contrib.auth.models import User +from django.contrib.postgres.fields import ArrayField from django.db import models -from django.db.models import F, Sum +from django.db.models import Case, F, IntegerField, Sum, Value, When from django.db.models.functions import Greatest -from django.db.models.signals import m2m_changed, post_delete, post_save +from django.db.models.signals import m2m_changed, post_delete, post_save, pre_save from django.dispatch import receiver from django.template.loader import render_to_string from django.urls import reverse @@ -46,6 +47,7 @@ class Collection(models.Model): num_downloads = models.PositiveIntegerField(default=0) public = models.BooleanField(default=False) is_default_collection = models.BooleanField(default=False) + featured_sound_ids = ArrayField(models.IntegerField(), size=3, blank=True, default=list) def __str__(self): return f"{self.name}" @@ -83,21 +85,49 @@ def get_total_collection_sounds_length(self): return result["total_duration"] or 0 def save(self, *args, **kwargs): - self.num_sounds = CollectionSound.objects.filter(collection=self).count() - if self.num_sounds > 0: - # this need to be reviewed, featured_sound feature is not fully developed - csound = CollectionSound.objects.filter(collection=self, status="OK").first() - csound.featured_sound = True - csound.save() + # Update num_sounds count + if self.pk: # Only count if collection already exists + self.num_sounds = CollectionSound.objects.filter(collection=self).count() + + # Auto-populate featured_sound_ids if empty but collection has sounds + if not self.featured_sound_ids and self.num_sounds > 0: + # Get up to 3 approved sound IDs + sound_ids = list( + CollectionSound.objects.filter( + collection=self, + sound__processing_state="OK", + sound__moderation_state="OK" + ).values_list('sound_id', flat=True)[:3] + ) + if sound_ids: + self.featured_sound_ids = sound_ids + super().save(*args, **kwargs) + def get_featured_sounds(self, num_sounds=1): + approved_sounds = self.sounds.filter(processing_state="OK", moderation_state="OK") + approved_sounds = order_sounds_by_featured(approved_sounds, self.featured_sound_ids) + return approved_sounds[:num_sounds] + + +def order_sounds_by_featured(sounds, featured_sound_ids=None): + """Return all sounds with featured sounds moved to the front.""" + if not featured_sound_ids: + return sounds + + ordering = Case( + *[When(id=sound_id, then=Value(i)) for i, sound_id in enumerate(featured_sound_ids)], + default=Value(len(featured_sound_ids)), + output_field=IntegerField() + ) + return sounds.annotate(featured_order=ordering).order_by('featured_order') + class CollectionSound(models.Model): # this model relates collections and sounds user = models.ForeignKey(User, on_delete=models.CASCADE) sound = models.ForeignKey(Sound, on_delete=models.CASCADE) collection = models.ForeignKey(Collection, related_name="collectionsound", on_delete=models.CASCADE) - featured_sound = models.BooleanField(default=False) created = models.DateTimeField(db_index=True, auto_now_add=True) STATUS_CHOICES = ( @@ -124,18 +154,39 @@ def update_collection_num_sounds_bulk_changes(sender, instance, **kwargs): Collection.objects.filter(collectionsound=instance).update(num_sounds=Greatest(F("num_sounds") - 1, 0)) -@receiver(post_save, sender=CollectionSound) -def mark_sound_dirty_on_collection_add(sender, instance, **kwargs): - if instance: - Sound.objects.filter(id=instance.sound_id).update(is_index_dirty=True) +@receiver([post_save, post_delete], sender=CollectionSound) +def remove_not_valid_featured_sounds(sender, instance, **kwargs): + """Remove featured_sound_ids that are no longer part of the collection.""" + if instance and instance.collection_id: + collection = instance.collection + if collection.featured_sound_ids: + # Get current sound IDs in the collection + valid_sound_ids = set( + CollectionSound.objects.filter(collection=collection).values_list('sound_id', flat=True) + ) + # Filter out any featured_sound_ids that are not in the collection + valid_featured_ids = [sid for sid in collection.featured_sound_ids if sid in valid_sound_ids] + if valid_featured_ids != collection.featured_sound_ids: + Collection.objects.filter(id=collection.id).update(featured_sound_ids=valid_featured_ids) -@receiver(post_delete, sender=CollectionSound) -def mark_sound_dirty_on_collection_remove(sender, instance, **kwargs): +@receiver([post_save, post_delete], sender=CollectionSound) +def mark_sound_dirty_on_collection_change(sender, instance, **kwargs): if instance: + print(f"----------coll change: marking {instance.sound} dirty!!!!!!!!!!!!") Sound.objects.filter(id=instance.sound_id).update(is_index_dirty=True) +@receiver(pre_save, sender=Collection) +def mark_sounds_dirty_on_public_change(sender, instance, **kwargs): + if instance and instance.pk: + old_instance = Collection.objects.get(pk=instance.pk) + if old_instance.public != instance.public: + print("----------public changed !!!!!!!!!!!!") + # Update all sounds in this collection + instance.sounds.update(is_index_dirty=True) + + class CollectionDownload(models.Model): user = models.ForeignKey(User, related_name="collection_downloads", on_delete=models.CASCADE) collection = models.ForeignKey(Collection, related_name="collection", on_delete=models.CASCADE) diff --git a/fscollections/templatetags/display_collections.py b/fscollections/templatetags/display_collections.py index cc980783f..5f2272ff1 100644 --- a/fscollections/templatetags/display_collections.py +++ b/fscollections/templatetags/display_collections.py @@ -23,7 +23,6 @@ from django.shortcuts import get_object_or_404 from fscollections.models import Collection -from sounds.models import Sound register = template.Library() @@ -32,9 +31,20 @@ def display_collection(context, collection_id): collection = get_object_or_404(Collection, id=collection_id) request = context.get("request") - try: - sound = Sound.objects.get(collections=collection, collectionsound__featured_sound=True) - except Sound.DoesNotExist: - sound = None + sound = collection.get_featured_sounds().first() tvars = {"collection": collection, "ft_sound": sound, "request": request} return tvars + + +@register.inclusion_tag("collections/display_featured_sound.html", takes_context=True) +def display_featured_sound(context, sound): + """Display a sound with the 'Featured' highlight styling. + + Args: + context: Template context (automatically passed by Django) + sound: Sound object to display with featured styling + + Returns: + dict: Template variables for rendering the featured sound + """ + return {"sound": sound, "request": context.get("request")} diff --git a/fscollections/tests.py b/fscollections/tests.py index 26a1eb0cf..4b55f39a2 100644 --- a/fscollections/tests.py +++ b/fscollections/tests.py @@ -3,6 +3,7 @@ from django.contrib.messages import get_messages from django.test import TestCase from django.urls import reverse +from django.utils.text import slugify from fscollections.models import Collection from utils.test_helpers import create_user_and_sounds @@ -22,6 +23,8 @@ def setUp(self): self.sound1 = sounds[1] self.sound2 = sounds[2] self.collection = Collection.objects.create(user=self.user, name="testcollection") + print(self.collection.id) + print(slugify(self.collection.name)) def test_collections_create_and_delete(self): # User not logged in - redirects to login page @@ -36,7 +39,9 @@ def test_collections_create_and_delete(self): self.assertEqual(resp.status_code, 200) # User logged in, check given collection id works - resp = self.client.get(reverse("collection", args=[self.collection.id])) + # #region agent log + generated_url = reverse("collection", args=[self.collection.id, slugify(self.collection.name)]) + resp = self.client.get(generated_url) self.assertEqual(resp.status_code, 200) # Create collection view @@ -45,12 +50,12 @@ def test_collections_create_and_delete(self): delete_collection = Collection.objects.get(name="tbdcollection") # Delete collection view - resp = self.client.post(reverse("delete-collection", args=[delete_collection.id])) + resp = self.client.post(reverse("delete-collection", args=[delete_collection.id, slugify(delete_collection.name)])) self.assertEqual(resp.status_code, 302) self.assertEqual("/collections/", resp.url) # Test collection URL for collection.id does not exist - resp = self.client.get(reverse("collection", args=[delete_collection.id])) + resp = self.client.get(reverse("collection", args=[delete_collection.id, slugify(delete_collection.name)])) self.assertEqual(resp.status_code, 404) def test_add_remove_sounds_as_user(self): @@ -81,7 +86,7 @@ def test_add_remove_sounds_as_user(self): resp = self.client.post(reverse("add-sound-to-collection", args=[self.sound.id]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) default_collection = Collection.objects.get(user=self.user, name="My bookmarks", is_default_collection=True) - resp = self.client.get(reverse("collection", args=[default_collection.id])) + resp = self.client.get(reverse("collection", args=[default_collection.id, slugify(default_collection.name)])) self.assertEqual(200, resp.status_code) # Test creating a new custom collection using the add_sound_to_collection modal @@ -92,7 +97,7 @@ def test_add_remove_sounds_as_user(self): self.assertEqual(1, new_collection.num_sounds) # Test download collection - resp = self.client.get(reverse("download-collection", args=[self.collection.id])) + resp = self.client.get(reverse("download-collection", args=[self.collection.id, slugify(self.collection.name)])) self.assertEqual(resp.status_code, 200) def test_add_remove_sounds_as_maintainer(self): @@ -131,7 +136,7 @@ def test_add_remove_sounds_as_maintainer(self): self.assertEqual(0, maintainer_test_collection.num_sounds) # Test deleting a collection where request user is maintainer (not valid) - resp = self.client.post(reverse("delete-collection", args=[self.collection.id])) + resp = self.client.post(reverse("delete-collection", args=[self.collection.id, slugify(self.collection.name)])) messages = list(get_messages(resp.wsgi_request)) self.assertTrue(len(messages) > 0) self.assertEqual( @@ -139,7 +144,7 @@ def test_add_remove_sounds_as_maintainer(self): "You're not allowed to delete this collection.In order to delete a collection you must be the owner.", ) self.assertEqual(resp.status_code, 302) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) def test_add_remove_sounds_as_external_user(self): self.client.force_login(self.external_user) @@ -155,14 +160,18 @@ def test_add_remove_sounds_as_external_user(self): self.assertEqual(0, self.collection.num_sounds) # Additionally, test edit URL for external user - resp = self.client.get(reverse("edit-collection", args=[self.collection.id])) + # #region agent log + edit_url = reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + resp = self.client.get(edit_url) + expected_redirect = f"/collections/{self.collection.id}_{slugify(self.collection.name)}/" + self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(expected_redirect, resp.url) # Test deleting a collection as an external user (not owner -> not valid) - resp = self.client.post(reverse("delete-collection", args=[self.collection.id])) + resp = self.client.post(reverse("delete-collection", args=[self.collection.id, slugify(self.collection.name)])) self.assertEqual(resp.status_code, 302) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) def test_edit_collection_as_user(self): # Test the edit collection form submission for different case scenarios @@ -171,14 +180,14 @@ def test_edit_collection_as_user(self): # Test setting an already existing name for a collection (form should be invalid therefore, the name should not change) form_data = {"name": "testcollection", "description": "", "public": False} - resp = self.client.post(reverse("edit-collection", args=[edit_collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[edit_collection.id, slugify(edit_collection.name)]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) edit_collection.refresh_from_db() self.assertEqual("edited-collection", edit_collection.name) # Test setting 'My bookmarks' as the name for a collection (form should be invalid therefore, the name should not change) form_data = {"name": "My bookmarks", "description": "", "public": False} - resp = self.client.post(reverse("edit-collection", args=[edit_collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[edit_collection.id, slugify(edit_collection.name)]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) edit_collection.refresh_from_db() self.assertEqual("edited-collection", edit_collection.name) @@ -186,9 +195,9 @@ def test_edit_collection_as_user(self): # Test creating the default collection and trying to change its name and public parameters (both are static) default_collection = Collection.objects.create(name="My bookmarks", user=self.user, is_default_collection=True) form_data = {"name": "other-name", "description": "", "public": True} - resp = self.client.post(reverse("edit-collection", args=[default_collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[default_collection.id, slugify(default_collection.name)]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{default_collection.id}/", resp.url) + self.assertEqual(f"/collections/{default_collection.id}_{slugify(default_collection.name)}/", resp.url) default_collection.refresh_from_db() self.assertEqual("My bookmarks", default_collection.name) self.assertEqual("", default_collection.description) @@ -202,9 +211,9 @@ def test_edit_collection_as_user(self): "public": False, "maintainers": f"{self.maintainer.id},{self.external_user.id},{self.maintainer.id}", } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) self.collection.refresh_from_db() self.assertEqual(2, self.collection.maintainers.all().count()) # Remove maintainer @@ -214,9 +223,9 @@ def test_edit_collection_as_user(self): "public": False, "maintainers": f"{self.maintainer.id}", } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) self.collection.refresh_from_db() self.assertEqual(1, self.collection.maintainers.all().count()) @@ -228,9 +237,9 @@ def test_edit_collection_as_user(self): "public": False, "collection_sounds": f"{self.sound.id},{self.sound1.id},{self.sound.id}", } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) self.collection.refresh_from_db() self.assertEqual(2, self.collection.num_sounds) # Remove sound @@ -240,9 +249,9 @@ def test_edit_collection_as_user(self): "public": False, "collection_sounds": f"{self.sound.id}", } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) self.collection.refresh_from_db() self.assertEqual(1, self.collection.num_sounds) @@ -256,7 +265,7 @@ def test_edit_collection_as_user(self): ) sounds_ids = ",".join([str(s.id) for s in added_sounds]) form_data = {"name": "testcollection", "description": "", "public": False, "collection_sounds": sounds_ids} - resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) self.collection.refresh_from_db() self.assertEqual(1, self.collection.num_sounds) @@ -275,15 +284,15 @@ def test_edit_collection_as_maintainer(self): "public": False, "collection_sounds": str(self.sound.id), } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) self.collection.refresh_from_db() self.assertEqual(1, self.collection.num_sounds) form_data.pop("collection_sounds") - resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) self.collection.refresh_from_db() self.assertEqual(0, self.collection.num_sounds) @@ -294,9 +303,9 @@ def test_edit_collection_as_maintainer(self): "public": True, "maintainers": str(self.external_user.id), } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) self.collection.refresh_from_db() self.assertEqual("testcollection", self.collection.name) self.assertEqual("", self.collection.description) @@ -304,7 +313,7 @@ def test_edit_collection_as_maintainer(self): self.assertEqual(1, self.collection.maintainers.all().count()) # Test collection deletion as maintainer - resp = self.client.get(reverse("delete-collection", args=[self.collection.id])) + resp = self.client.get(reverse("delete-collection", args=[self.collection.id, slugify(self.collection.name)])) self.assertEqual(resp.status_code, 302) - self.assertEqual(f"/collections/{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) self.assertTrue(Collection.objects.filter(id=self.collection.id).exists()) diff --git a/fscollections/urls.py b/fscollections/urls.py index 664f3486d..7e28d9c05 100644 --- a/fscollections/urls.py +++ b/fscollections/urls.py @@ -4,18 +4,18 @@ urlpatterns = [ path("", views.collections_for_user, name="your-collections"), - path("/", views.collection, name="collection"), + path("_/", views.collection, name="collection"), path("/add/", views.add_sound_to_collection, name="add-sound-to-collection"), path("create/", views.create_collection, name="create-collection"), - path("/edit", views.edit_collection, name="edit-collection"), - path("/delete", views.delete_collection, name="delete-collection"), - path("/download/", views.download_collection, name="download-collection"), - path("/licenses/", views.collection_licenses, name="collection-licenses"), + path("_/edit", views.edit_collection, name="edit-collection"), + path("_/delete", views.delete_collection, name="delete-collection"), + path("_/download/", views.download_collection, name="download-collection"), + path("_/licenses/", views.collection_licenses, name="collection-licenses"), path( - "/addsoundsmodal", + "_/addsoundsmodal", views.add_sounds_modal_for_collection_edit, name="add-sounds-modal-collection", ), - path("/addmaintainersmodal", views.add_maintainer_modal, name="add-maintainers-modal"), - path("/section/stats", views.collection_stats_section, name="collection-stats-section"), + path("_/addmaintainersmodal", views.add_maintainer_modal, name="add-maintainers-modal"), + path("_/section/stats", views.collection_stats_section, name="collection-stats-section"), ] diff --git a/fscollections/views.py b/fscollections/views.py index 820c0e419..828db66f6 100644 --- a/fscollections/views.py +++ b/fscollections/views.py @@ -26,6 +26,7 @@ from django.http import HttpResponse, HttpResponseRedirect, JsonResponse from django.shortcuts import get_object_or_404, render from django.urls import reverse +from django.utils.text import slugify from fscollections.forms import ( CollectionEditForm, @@ -34,7 +35,7 @@ MaintainerForm, SelectCollectionOrNewCollectionForm, ) -from fscollections.models import Collection, CollectionDownload, CollectionDownloadSound, CollectionSound +from fscollections.models import Collection, CollectionDownload, CollectionDownloadSound, CollectionSound, order_sounds_by_featured from sounds.models import Sound from sounds.views import add_sounds_modal_helper from utils.downloads import download_sounds @@ -42,7 +43,7 @@ @login_required -def collection(request, collection_id): +def collection(request, collection_id, collection_name): user = request.user is_owner = False is_maintainer = False @@ -57,6 +58,7 @@ def collection(request, collection_id): # one URL needed to display all collections and one URL to display ONE collection at a time # the collections_for_user can be reused to display ONE collection so give it a thought on full collections display collection_sounds = Sound.objects.prefetch_related("collections").filter(collections=collection) + collection_sounds = order_sounds_by_featured(collection_sounds, collection.featured_sound_ids) paginator = paginate(request, collection_sounds, settings.BOOKMARKS_PER_PAGE) page_sounds = Sound.objects.ordered_ids([sound.id for sound in paginator["page"].object_list]) tvars.update(paginator) @@ -76,7 +78,7 @@ def collections_for_user(request): @login_required -def collection_stats_section(request, collection_id): +def collection_stats_section(request, collection_id, collection_name): # TODO: this tries to imitate the pack_stats_section behaviour despite a lack of comprehension # on cache behaviour, so the stats shown by this are not properly updated if not request.GET.get("ajax"): @@ -152,7 +154,7 @@ def create_collection(request): @login_required -def delete_collection(request, collection_id): +def delete_collection(request, collection_id, collection_name): collection = get_object_or_404(Collection, id=collection_id) if request.method == "POST" and request.user == collection.user: @@ -166,11 +168,11 @@ def delete_collection(request, collection_id): messages.INFO, "You're not allowed to delete this collection.In order to delete a collection you must be the owner.", ) - return HttpResponseRedirect(reverse("collection", args=[collection.id])) + return HttpResponseRedirect(reverse("collection", args=[collection.id, slugify(collection.name)])) @login_required -def edit_collection(request, collection_id): +def edit_collection(request, collection_id, collection_name): collection = get_object_or_404(Collection, id=collection_id) collection_sounds = ",".join([str(s.id) for s in Sound.objects.filter(collections=collection)]) maintainers_query = User.objects.filter(collection_maintainer=collection.id) @@ -182,7 +184,7 @@ def edit_collection(request, collection_id): elif request.user in maintainers_query: is_maintainer = True else: - return HttpResponseRedirect(reverse("collection", args=[collection_id])) + return HttpResponseRedirect(reverse("collection", args=[collection_id, slugify(collection.name)])) current_sounds = list() if request.method == "POST": @@ -195,11 +197,11 @@ def edit_collection(request, collection_id): request.POST, instance=collection, label_suffix="", is_owner=is_owner, is_maintainer=is_maintainer ) else: - return HttpResponseRedirect(reverse("collection", collection_id)) + return HttpResponseRedirect(reverse("collection", args=[collection_id, slugify(collection.name)])) if form.is_valid(): form.save(user_adding_sound=request.user) - return HttpResponseRedirect(reverse("collection", args=[collection.id])) + return HttpResponseRedirect(reverse("collection", args=[collection.id, slugify(collection.name)])) else: # NOTE: in this form's validation, errors are raised for each speific field, so when there is a submission attempt the error # is displayed within it. However, fields containing errors are removed from the clean data but we are still interested in @@ -244,6 +246,7 @@ def edit_collection(request, collection_id): is_maintainer=is_maintainer, ) current_sounds = Sound.objects.bulk_sounds_for_collection(collection_id=collection.id) + current_sounds = order_sounds_by_featured(current_sounds, collection.featured_sound_ids) current_maintainers = User.objects.filter(collection_maintainer=collection.id) form.collection_sound_objects = current_sounds form.collection_maintainers_objects = current_maintainers @@ -259,7 +262,7 @@ def edit_collection(request, collection_id): @login_required -def download_collection(request, collection_id): +def download_collection(request, collection_id, collection_name): collection = get_object_or_404(Collection, id=collection_id) collection_sounds = CollectionSound.objects.filter(collection=collection).values("sound_id") sounds_list = Sound.objects.filter( @@ -279,12 +282,12 @@ def download_collection(request, collection_id): cds.append(CollectionDownloadSound(sound=sound, collection_download=cd, license=sound.license)) CollectionDownloadSound.objects.bulk_create(cds) - licenses_url = reverse("collection-licenses", args=[collection_id]) + licenses_url = reverse("collection-licenses", args=[collection_id, collection_name]) licenses_content = collection.get_attribution(sound_qs=sounds_list) return download_sounds(licenses_url, licenses_content, sounds_list, collection.download_filename) -def collection_licenses(request, collection_id): +def collection_licenses(request, collection_id, collection_name): collection = get_object_or_404(Collection, id=collection_id) attribution = collection.get_attribution() return HttpResponse(attribution, content_type="text/plain") @@ -296,7 +299,7 @@ def add_sounds_modal_for_collection_edit(request, collection_id): return render(request, "sounds/modal_add_sounds.html", tvars) -def add_maintainer_modal(request, collection_id): +def add_maintainer_modal(request, collection_id, collection_name): collection = get_object_or_404(Collection, id=collection_id) form = MaintainerForm() # TODO: the below statements exclude users with whitespaces in their usernames (and they still exist) diff --git a/templates/collections/collection.html b/templates/collections/collection.html index 56b0f6474..8f1f49e0b 100644 --- a/templates/collections/collection.html +++ b/templates/collections/collection.html @@ -2,6 +2,7 @@ {% load static %} {% load bw_templatetags %} {% load display_sound %} +{% load display_collections %} {% load display_user %} {% load util %} @@ -12,15 +13,15 @@
+ data-async-section-content-url="{% url 'collection-stats-section' collection.id collection.name|slugify %}?ajax=1">
{% if is_owner or is_maintainer%} - Edit collection + Edit collection {% endif %} {% if collection.num_sounds > 0%} - Download Collection + Download Collection {% endif %}
@@ -54,10 +55,14 @@
Sounds in this collection
{% if page.object_list %} -
- {% for collectionsound, sound in page_collection_and_sound_objects %} +
+ {% for collectionsound, sound in page_collection_and_sound_objects %}
+ {% if sound.id in collection.featured_sound_ids %} + {% display_featured_sound sound %} + {% else %} {% display_sound_small sound %} + {% endif %}
{% endfor %}
diff --git a/templates/collections/display_collection.html b/templates/collections/display_collection.html index 681d45143..c1bf3c8b7 100644 --- a/templates/collections/display_collection.html +++ b/templates/collections/display_collection.html @@ -15,7 +15,7 @@ {% endif %}
@@ -45,7 +45,7 @@
{{ collection.num_sounds|formatnumber }}
- + {{ collection.num_downloads|formatnumber }}
diff --git a/templates/collections/display_featured_sound.html b/templates/collections/display_featured_sound.html new file mode 100644 index 000000000..9691ebf25 --- /dev/null +++ b/templates/collections/display_featured_sound.html @@ -0,0 +1,9 @@ +{% load display_sound %} + +
+
+ Featured +
+ {% display_sound_small sound %} +
+ diff --git a/templates/collections/edit_collection.html b/templates/collections/edit_collection.html index 09934acd1..175989d63 100644 --- a/templates/collections/edit_collection.html +++ b/templates/collections/edit_collection.html @@ -49,7 +49,7 @@

{{ collection.name }}

{% endif %} {% users_selector form.collection_maintainers_objects%}
- +
@@ -59,7 +59,8 @@

{{ collection.name }}

{{form.collection_sounds.errors}}
{% sounds_selector form.collection_sound_objects max_sounds=max_sounds_per_collection%}
- + +
@@ -71,7 +72,7 @@

{{ collection.name }}

{% endif %} {% if is_owner%} - + {% endif %}
diff --git a/templates/collections/modal_add_maintainer.html b/templates/collections/modal_add_maintainer.html index b723351f8..0749682e7 100644 --- a/templates/collections/modal_add_maintainer.html +++ b/templates/collections/modal_add_maintainer.html @@ -11,7 +11,7 @@

Add maintainer to collection

-
{% csrf_token %} + {% csrf_token %} {{form}} {% if not_found_msg %}
{% bw_icon "notification" %}{{not_found_msg}}
From 0107989a820e83fd41c607495a6fdad1a272e2f3 Mon Sep 17 00:00:00 2001 From: Domonkos Kostyal Date: Mon, 12 Jan 2026 12:40:09 +0100 Subject: [PATCH 02/20] Collections updated, added sort, search, for featured sounds added: edit and tests --- freesound/settings.py | 9 + .../bw-frontend/src/pages/collectionEdit.js | 118 ++++++++++- .../styles/atoms/selectableObject.scss | 11 + fscollections/forms.py | 27 +++ fscollections/models.py | 88 +++++--- .../templatetags/display_collections.py | 25 +-- fscollections/tests.py | 197 +++++++++++++++--- fscollections/urls.py | 16 +- fscollections/views.py | 69 ++++-- templates/collections/collection.html | 52 +++-- templates/collections/display_collection.html | 74 +++---- .../collections/display_featured_sound.html | 9 - .../collections/display_featured_sounds.html | 16 ++ templates/collections/edit_collection.html | 33 ++- .../collections/modal_add_maintainer.html | 2 +- .../modal_add_sound_to_collection.html | 10 +- 16 files changed, 581 insertions(+), 175 deletions(-) delete mode 100644 templates/collections/display_featured_sound.html create mode 100644 templates/collections/display_featured_sounds.html diff --git a/freesound/settings.py b/freesound/settings.py index a19c26ee8..73e38541b 100644 --- a/freesound/settings.py +++ b/freesound/settings.py @@ -1441,6 +1441,15 @@ # Collections ENABLE_COLLECTIONS = True MAX_SOUNDS_PER_COLLECTION = 250 +MAX_FEATURED_SOUNDS_PER_COLLECTION = 250 +COLLECTION_SORT_OPTIONS = { + "Featured first": "featured_order", + "Date added (newest first)": "-collectionsound__created", + "Date added (oldest first)": "collectionsound__created", + "Duration (longest first)": "-duration", + "Duration (shortest first)": "duration", +} +COLLECTION_SORT_DEFAULT = "Featured first" # ------------------------------------------------------------------------------- # Import local settings diff --git a/freesound/static/bw-frontend/src/pages/collectionEdit.js b/freesound/static/bw-frontend/src/pages/collectionEdit.js index 5c9bf425f..02183d3fe 100644 --- a/freesound/static/bw-frontend/src/pages/collectionEdit.js +++ b/freesound/static/bw-frontend/src/pages/collectionEdit.js @@ -1,5 +1,121 @@ import {prepareAddMaintainersModalAndFields} from "../components/collectionsModal" import { prepareAddSoundsModalAndFields } from "../components/addSoundsModal"; +import {updateObjectSelectorDataProperties} from '../components/objectSelector' + + +const updateFeaturedHighlights = (soundSelectorContainer, featuredIds) => { + const allSelectableObjects = soundSelectorContainer.querySelectorAll('.bw-selectable-object'); + allSelectableObjects.forEach(element => { + const checkbox = element.querySelector('input.bw-checkbox'); + const existingLabel = element.querySelector('.featured-label'); + + if (checkbox && featuredIds.includes(checkbox.dataset.objectId)) { + element.classList.add('featured'); + // Add "Featured" label if not already present + if (!existingLabel) { + const label = document.createElement('span'); + label.className = 'featured-label'; + label.textContent = 'Featured'; + element.appendChild(label); + } + } else { + element.classList.remove('featured'); + // Remove "Featured" label if present + if (existingLabel) { + existingLabel.remove(); + } + } + }); +}; + +const prepareFeaturedSoundsButtons = (container) => { + const setButton = container.querySelector('[data-toggle="set-featured-sounds"]'); + const removeButton = container.querySelector('[data-toggle="remove-featured-sounds"]'); + const clearAllButton = container.querySelector('[data-toggle="clear-all-featured-sounds"]'); + if (!setButton || !removeButton) return; + + const referenceButton = setButton || removeButton; + const soundSelectorContainer = referenceButton.closest('.v-spacing-5').querySelector('.bw-object-selector-container[data-type="sounds"]'); + if (!soundSelectorContainer) return; + + // Disable selection-based buttons initially + setButton.disabled = true; + removeButton.disabled = true; + + const featuredSoundsInput = document.getElementById(referenceButton.dataset.featuredSoundsHiddenInputId); + + const getSelectedIds = () => (soundSelectorContainer.dataset.selectedIds || "").split(',').filter(Boolean); + const getFeaturedIds = () => ((featuredSoundsInput && featuredSoundsInput.value) || "").split(',').filter(Boolean); + + // Update clear all button based on whether there are any featured sounds + const updateClearAllButtonState = () => { + if (clearAllButton) { + clearAllButton.disabled = getFeaturedIds().length === 0; + } + }; + + // Add change listeners directly to checkboxes (works alongside existing listeners from addSoundsModal) + const updateButtonStates = () => { + // Check directly from checkboxes since dataset.selectedIds may not be updated yet (debounced) + const hasSelection = soundSelectorContainer.querySelector('input.bw-checkbox:checked') !== null; + setButton.disabled = !hasSelection; + removeButton.disabled = !hasSelection; + }; + + soundSelectorContainer.querySelectorAll('input.bw-checkbox').forEach(checkbox => { + checkbox.addEventListener('change', updateButtonStates); + }); + + updateFeaturedHighlights(soundSelectorContainer, getFeaturedIds()); + updateClearAllButtonState(); + + const updateFeatured = (ids) => { + if (featuredSoundsInput) featuredSoundsInput.value = ids.join(','); + updateFeaturedHighlights(soundSelectorContainer, ids); + updateClearAllButtonState(); + }; + + const clearSelection = () => { + soundSelectorContainer.querySelectorAll('input.bw-checkbox:checked').forEach(cb => { + cb.checked = false; + var parent = cb.closest('.bw-selectable-object'); + if (parent) parent.classList.remove('selected'); + }); + updateObjectSelectorDataProperties(soundSelectorContainer); + setButton.disabled = true; + removeButton.disabled = true; + // Dispatch change event so other listeners (e.g. from addSoundsModal) get notified + const firstCheckbox = soundSelectorContainer.querySelector('input.bw-checkbox'); + if (firstCheckbox) { + firstCheckbox.dispatchEvent(new Event('change', { bubbles: true })); + } + }; + + setButton.addEventListener('click', (e) => { + e.preventDefault(); + const merged = [...new Set([...getFeaturedIds(), ...getSelectedIds()])]; + updateFeatured(merged); + clearSelection(); + }); + + removeButton.addEventListener('click', (e) => { + e.preventDefault(); + const selectedIds = new Set(getSelectedIds()); + const newFeatured = selectedIds.size > 0 + ? getFeaturedIds().filter(id => !selectedIds.has(id)) + : []; + updateFeatured(newFeatured); + clearSelection(); + }); + + if (clearAllButton) { + clearAllButton.addEventListener('click', (e) => { + e.preventDefault(); + updateFeatured([]); + }); + } +}; prepareAddMaintainersModalAndFields(document); -prepareAddSoundsModalAndFields(document); \ No newline at end of file +prepareAddSoundsModalAndFields(document); +prepareFeaturedSoundsButtons(document); \ No newline at end of file diff --git a/freesound/static/bw-frontend/styles/atoms/selectableObject.scss b/freesound/static/bw-frontend/styles/atoms/selectableObject.scss index 79494db3e..7a3fef7d9 100644 --- a/freesound/static/bw-frontend/styles/atoms/selectableObject.scss +++ b/freesound/static/bw-frontend/styles/atoms/selectableObject.scss @@ -10,4 +10,15 @@ border:2px solid $navy-light-grey; transition: border-color 0.3s linear; } + + .featured-label { + position: absolute; + top: 4px; + right: 8px; + font-size: 10px; + font-weight: 600; + color: #f5a623; + text-transform: uppercase; + letter-spacing: 0.5px; + } } \ No newline at end of file diff --git a/fscollections/forms.py b/fscollections/forms.py index a38b1ee87..c09e74625 100644 --- a/fscollections/forms.py +++ b/fscollections/forms.py @@ -59,6 +59,8 @@ class SelectCollectionOrNewCollectionForm(forms.Form): new_collection_name = forms.CharField(label=False, help_text=None, max_length=128, required=False) use_last_collection = forms.BooleanField(widget=forms.HiddenInput(), required=False, initial=False) + + mark_as_featured = forms.BooleanField(label=False, required=False, initial=False) user_collections = None user_available_collections = None user_full_collections = None @@ -139,6 +141,13 @@ def save(self, *args, **kwargs): elif self.user_saving_sound.id in maintainers_list: collection, _ = Collection.objects.get_or_create(name=collection_to_use.name, id=collection_to_use.id) CollectionSound.objects.create(user=self.user_saving_sound, collection=collection, sound=sound, status="OK") + + # Handle mark as featured + if self.cleaned_data.get("mark_as_featured"): + if sound.id not in collection.featured_sound_ids: + collection.featured_sound_ids = collection.featured_sound_ids + [sound.id] + collection.save(update_fields=["featured_sound_ids"]) + return collection def clean(self): @@ -198,6 +207,11 @@ class CollectionEditForm(forms.ModelForm): min_length=1, widget=forms.widgets.HiddenInput(attrs={"id": "maintainers"}), required=False ) + featured_sounds = forms.CharField( + widget=forms.widgets.HiddenInput(attrs={"id": "featured_sounds", "name": "featured_sounds"}), + required=False, + ) + def __init__(self, *args, **kwargs): self.is_owner = kwargs.pop("is_owner", False) self.is_maintainer = kwargs.pop("is_maintainer", False) @@ -297,6 +311,17 @@ def save(self, user_adding_sound): for usr in current_maintainers: maintainer = User.objects.get(id=usr) collection.maintainers.remove(maintainer) + + # Handle featured sounds + featured_sounds_str = self.cleaned_data.get("featured_sounds", "") + if featured_sounds_str: + featured_ids = [int(sid) for sid in featured_sounds_str.split(",") if sid.strip()] + # Only keep featured IDs that are still in the collection + featured_ids = [sid for sid in featured_ids if sid in new_sounds] + collection.featured_sound_ids = featured_ids + else: + collection.featured_sound_ids = [] + collection.save() return collection @@ -329,6 +354,8 @@ def clean(self): # are retrieved from DB to ensure no changes are applied to this attribute. collection_maintainers = list(self.instance.maintainers.values_list("id", flat=True)) cleaned_data["maintainers"] = (",").join(str(x) for x in collection_maintainers) + # Preserve featured_sounds from the original instance (maintainers cannot edit) + cleaned_data["featured_sounds"] = ",".join(str(sid) for sid in self.instance.featured_sound_ids) for field in CollectionEditForm.Meta.fields: if cleaned_data[field] != getattr(self.instance, field): self.add_error(field, forms.ValidationError("You don't have permissions to edit this field")) diff --git a/fscollections/models.py b/fscollections/models.py index 39e9d2cbd..d4530d9f4 100644 --- a/fscollections/models.py +++ b/fscollections/models.py @@ -31,6 +31,7 @@ from django.urls import reverse from django.utils.text import slugify +from freesound import settings from sounds.models import License, Sound @@ -47,7 +48,7 @@ class Collection(models.Model): num_downloads = models.PositiveIntegerField(default=0) public = models.BooleanField(default=False) is_default_collection = models.BooleanField(default=False) - featured_sound_ids = ArrayField(models.IntegerField(), size=3, blank=True, default=list) + featured_sound_ids = ArrayField(models.IntegerField(), size=settings.MAX_FEATURED_SOUNDS_PER_COLLECTION, blank=True, default=list) def __str__(self): return f"{self.name}" @@ -86,41 +87,64 @@ def get_total_collection_sounds_length(self): def save(self, *args, **kwargs): # Update num_sounds count - if self.pk: # Only count if collection already exists + if self.pk: self.num_sounds = CollectionSound.objects.filter(collection=self).count() - - # Auto-populate featured_sound_ids if empty but collection has sounds - if not self.featured_sound_ids and self.num_sounds > 0: - # Get up to 3 approved sound IDs - sound_ids = list( - CollectionSound.objects.filter( - collection=self, - sound__processing_state="OK", - sound__moderation_state="OK" - ).values_list('sound_id', flat=True)[:3] - ) - if sound_ids: - self.featured_sound_ids = sound_ids super().save(*args, **kwargs) - def get_featured_sounds(self, num_sounds=1): - approved_sounds = self.sounds.filter(processing_state="OK", moderation_state="OK") - approved_sounds = order_sounds_by_featured(approved_sounds, self.featured_sound_ids) - return approved_sounds[:num_sounds] - - -def order_sounds_by_featured(sounds, featured_sound_ids=None): - """Return all sounds with featured sounds moved to the front.""" - if not featured_sound_ids: - return sounds - - ordering = Case( - *[When(id=sound_id, then=Value(i)) for i, sound_id in enumerate(featured_sound_ids)], - default=Value(len(featured_sound_ids)), - output_field=IntegerField() - ) - return sounds.annotate(featured_order=ordering).order_by('featured_order') + def get_sounds( + self, + sort_by=None, + limit=None, + include_audio_descriptors=False, + include_similarity_vectors=False, + include_remix_subqueries=False, + ): + """Get sounds for this collection with sorting. + + Args: + sort_by: Sort option key from settings.COLLECTION_SORT_OPTIONS. + Defaults to settings.COLLECTION_SORT_DEFAULT. + limit: Optional limit on number of sounds returned + include_audio_descriptors: Include audio descriptor data + include_similarity_vectors: Include similarity vector data + include_remix_subqueries: Include remix relationship data + + Returns: + Sorted QuerySet of Sound objects + """ + # Validate sort option, defaulting to COLLECTION_SORT_DEFAULT if invalid or None + if sort_by not in settings.COLLECTION_SORT_OPTIONS: + sort_by = settings.COLLECTION_SORT_DEFAULT + + qs = Sound.objects.bulk_sounds_for_collection( + collection_id=self.id, + include_audio_descriptors=include_audio_descriptors, + include_similarity_vectors=include_similarity_vectors, + include_remix_subqueries=include_remix_subqueries, + ) + + # Get the sort field from settings (value is the sort field directly) + sort_field = settings.COLLECTION_SORT_OPTIONS[sort_by] + + # Apply sorting based on sort_by option (featured is when sort_by matches the default) + if sort_field == "featured_order": + # Featured sorting - direct access to self.featured_sound_ids + if self.featured_sound_ids: + ordering = Case( + *[When(id=sound_id, then=Value(i)) for i, sound_id in enumerate(self.featured_sound_ids)], + default=Value(len(self.featured_sound_ids)), + output_field=IntegerField() + ) + qs = qs.annotate(featured_order=ordering).order_by(sort_field) + # If no featured sounds, keep default queryset order + elif sort_field: + # Use the sort field from settings for other sort options + qs = qs.order_by(sort_field) + + if limit: + qs = qs[:limit] + return qs class CollectionSound(models.Model): diff --git a/fscollections/templatetags/display_collections.py b/fscollections/templatetags/display_collections.py index 5f2272ff1..125c539ee 100644 --- a/fscollections/templatetags/display_collections.py +++ b/fscollections/templatetags/display_collections.py @@ -23,6 +23,7 @@ from django.shortcuts import get_object_or_404 from fscollections.models import Collection +from sounds.models import Sound register = template.Library() @@ -31,20 +32,16 @@ def display_collection(context, collection_id): collection = get_object_or_404(Collection, id=collection_id) request = context.get("request") - sound = collection.get_featured_sounds().first() - tvars = {"collection": collection, "ft_sound": sound, "request": request} + if collection.featured_sound_ids: + featured_sounds = collection.get_sounds(sort_by="Featured first", limit=len(collection.featured_sound_ids)) + else: + featured_sounds = collection.get_sounds(sort_by="Date added (newest first)", limit=1) + + tvars = {"collection": collection, "featured_sounds": featured_sounds, "request": request} return tvars -@register.inclusion_tag("collections/display_featured_sound.html", takes_context=True) -def display_featured_sound(context, sound): - """Display a sound with the 'Featured' highlight styling. - - Args: - context: Template context (automatically passed by Django) - sound: Sound object to display with featured styling - - Returns: - dict: Template variables for rendering the featured sound - """ - return {"sound": sound, "request": context.get("request")} +@register.inclusion_tag("collections/display_featured_sounds.html", takes_context=True) +def display_featured_sounds(context, sounds): + return {"featured_sounds": sounds, "request": context.get("request")} + diff --git a/fscollections/tests.py b/fscollections/tests.py index 4b55f39a2..ab3010beb 100644 --- a/fscollections/tests.py +++ b/fscollections/tests.py @@ -5,7 +5,7 @@ from django.urls import reverse from django.utils.text import slugify -from fscollections.models import Collection +from fscollections.models import Collection, CollectionSound from utils.test_helpers import create_user_and_sounds @@ -40,7 +40,7 @@ def test_collections_create_and_delete(self): # User logged in, check given collection id works # #region agent log - generated_url = reverse("collection", args=[self.collection.id, slugify(self.collection.name)]) + generated_url = reverse("collection", args=[slugify(self.collection.name), self.collection.id]) resp = self.client.get(generated_url) self.assertEqual(resp.status_code, 200) @@ -50,12 +50,12 @@ def test_collections_create_and_delete(self): delete_collection = Collection.objects.get(name="tbdcollection") # Delete collection view - resp = self.client.post(reverse("delete-collection", args=[delete_collection.id, slugify(delete_collection.name)])) + resp = self.client.post(reverse("delete-collection", args=[slugify(delete_collection.name), delete_collection.id])) self.assertEqual(resp.status_code, 302) self.assertEqual("/collections/", resp.url) # Test collection URL for collection.id does not exist - resp = self.client.get(reverse("collection", args=[delete_collection.id, slugify(delete_collection.name)])) + resp = self.client.get(reverse("collection", args=[slugify(delete_collection.name), delete_collection.id])) self.assertEqual(resp.status_code, 404) def test_add_remove_sounds_as_user(self): @@ -86,7 +86,7 @@ def test_add_remove_sounds_as_user(self): resp = self.client.post(reverse("add-sound-to-collection", args=[self.sound.id]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) default_collection = Collection.objects.get(user=self.user, name="My bookmarks", is_default_collection=True) - resp = self.client.get(reverse("collection", args=[default_collection.id, slugify(default_collection.name)])) + resp = self.client.get(reverse("collection", args=[slugify(default_collection.name), default_collection.id])) self.assertEqual(200, resp.status_code) # Test creating a new custom collection using the add_sound_to_collection modal @@ -97,7 +97,7 @@ def test_add_remove_sounds_as_user(self): self.assertEqual(1, new_collection.num_sounds) # Test download collection - resp = self.client.get(reverse("download-collection", args=[self.collection.id, slugify(self.collection.name)])) + resp = self.client.get(reverse("download-collection", args=[slugify(self.collection.name), self.collection.id])) self.assertEqual(resp.status_code, 200) def test_add_remove_sounds_as_maintainer(self): @@ -136,7 +136,7 @@ def test_add_remove_sounds_as_maintainer(self): self.assertEqual(0, maintainer_test_collection.num_sounds) # Test deleting a collection where request user is maintainer (not valid) - resp = self.client.post(reverse("delete-collection", args=[self.collection.id, slugify(self.collection.name)])) + resp = self.client.post(reverse("delete-collection", args=[slugify(self.collection.name), self.collection.id])) messages = list(get_messages(resp.wsgi_request)) self.assertTrue(len(messages) > 0) self.assertEqual( @@ -144,7 +144,7 @@ def test_add_remove_sounds_as_maintainer(self): "You're not allowed to delete this collection.In order to delete a collection you must be the owner.", ) self.assertEqual(resp.status_code, 302) - self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) def test_add_remove_sounds_as_external_user(self): self.client.force_login(self.external_user) @@ -161,17 +161,17 @@ def test_add_remove_sounds_as_external_user(self): # Additionally, test edit URL for external user # #region agent log - edit_url = reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + edit_url = reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) resp = self.client.get(edit_url) - expected_redirect = f"/collections/{self.collection.id}_{slugify(self.collection.name)}/" + expected_redirect = f"/collections/{slugify(self.collection.name)}-{self.collection.id}/" self.assertEqual(302, resp.status_code) self.assertEqual(expected_redirect, resp.url) # Test deleting a collection as an external user (not owner -> not valid) - resp = self.client.post(reverse("delete-collection", args=[self.collection.id, slugify(self.collection.name)])) + resp = self.client.post(reverse("delete-collection", args=[slugify(self.collection.name), self.collection.id])) self.assertEqual(resp.status_code, 302) - self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) def test_edit_collection_as_user(self): # Test the edit collection form submission for different case scenarios @@ -180,14 +180,14 @@ def test_edit_collection_as_user(self): # Test setting an already existing name for a collection (form should be invalid therefore, the name should not change) form_data = {"name": "testcollection", "description": "", "public": False} - resp = self.client.post(reverse("edit-collection", args=[edit_collection.id, slugify(edit_collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(edit_collection.name), edit_collection.id]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) edit_collection.refresh_from_db() self.assertEqual("edited-collection", edit_collection.name) # Test setting 'My bookmarks' as the name for a collection (form should be invalid therefore, the name should not change) form_data = {"name": "My bookmarks", "description": "", "public": False} - resp = self.client.post(reverse("edit-collection", args=[edit_collection.id, slugify(edit_collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(edit_collection.name), edit_collection.id]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) edit_collection.refresh_from_db() self.assertEqual("edited-collection", edit_collection.name) @@ -195,9 +195,9 @@ def test_edit_collection_as_user(self): # Test creating the default collection and trying to change its name and public parameters (both are static) default_collection = Collection.objects.create(name="My bookmarks", user=self.user, is_default_collection=True) form_data = {"name": "other-name", "description": "", "public": True} - resp = self.client.post(reverse("edit-collection", args=[default_collection.id, slugify(default_collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(default_collection.name), default_collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{default_collection.id}_{slugify(default_collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(default_collection.name)}-{default_collection.id}/", resp.url) default_collection.refresh_from_db() self.assertEqual("My bookmarks", default_collection.name) self.assertEqual("", default_collection.description) @@ -211,9 +211,9 @@ def test_edit_collection_as_user(self): "public": False, "maintainers": f"{self.maintainer.id},{self.external_user.id},{self.maintainer.id}", } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(2, self.collection.maintainers.all().count()) # Remove maintainer @@ -223,9 +223,9 @@ def test_edit_collection_as_user(self): "public": False, "maintainers": f"{self.maintainer.id}", } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(1, self.collection.maintainers.all().count()) @@ -237,9 +237,9 @@ def test_edit_collection_as_user(self): "public": False, "collection_sounds": f"{self.sound.id},{self.sound1.id},{self.sound.id}", } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(2, self.collection.num_sounds) # Remove sound @@ -249,9 +249,9 @@ def test_edit_collection_as_user(self): "public": False, "collection_sounds": f"{self.sound.id}", } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(1, self.collection.num_sounds) @@ -265,7 +265,7 @@ def test_edit_collection_as_user(self): ) sounds_ids = ",".join([str(s.id) for s in added_sounds]) form_data = {"name": "testcollection", "description": "", "public": False, "collection_sounds": sounds_ids} - resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) self.collection.refresh_from_db() self.assertEqual(1, self.collection.num_sounds) @@ -284,15 +284,15 @@ def test_edit_collection_as_maintainer(self): "public": False, "collection_sounds": str(self.sound.id), } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(1, self.collection.num_sounds) form_data.pop("collection_sounds") - resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(0, self.collection.num_sounds) @@ -303,9 +303,9 @@ def test_edit_collection_as_maintainer(self): "public": True, "maintainers": str(self.external_user.id), } - resp = self.client.post(reverse("edit-collection", args=[self.collection.id, slugify(self.collection.name)]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual("testcollection", self.collection.name) self.assertEqual("", self.collection.description) @@ -313,7 +313,138 @@ def test_edit_collection_as_maintainer(self): self.assertEqual(1, self.collection.maintainers.all().count()) # Test collection deletion as maintainer - resp = self.client.get(reverse("delete-collection", args=[self.collection.id, slugify(self.collection.name)])) + resp = self.client.get(reverse("delete-collection", args=[slugify(self.collection.name), self.collection.id])) self.assertEqual(resp.status_code, 302) - self.assertEqual(f"/collections/{self.collection.id}_{slugify(self.collection.name)}/", resp.url) + self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) self.assertTrue(Collection.objects.filter(id=self.collection.id).exists()) + + def test_edit_featured_sounds_as_user(self): + # Test adding and removing featured sounds as owner + self.client.force_login(self.user) + + # First add sounds to the collection + form_data = { + "name": "testcollection", + "description": "", + "public": False, + "collection_sounds": f"{self.sound.id},{self.sound1.id},{self.sound2.id}", + "featured_sounds": "", + } + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + self.assertEqual(302, resp.status_code) + self.collection.refresh_from_db() + self.assertEqual(3, self.collection.num_sounds) + self.assertEqual([], self.collection.featured_sound_ids) + + # Add featured sounds + form_data["featured_sounds"] = f"{self.sound.id},{self.sound1.id}" + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + self.assertEqual(302, resp.status_code) + self.collection.refresh_from_db() + self.assertEqual([self.sound.id, self.sound1.id], self.collection.featured_sound_ids) + + # Reorder featured sounds + form_data["featured_sounds"] = f"{self.sound1.id},{self.sound.id}" + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + self.assertEqual(302, resp.status_code) + self.collection.refresh_from_db() + self.assertEqual([self.sound1.id, self.sound.id], self.collection.featured_sound_ids) + + # Remove a featured sound + form_data["featured_sounds"] = f"{self.sound.id}" + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + self.assertEqual(302, resp.status_code) + self.collection.refresh_from_db() + self.assertEqual([self.sound.id], self.collection.featured_sound_ids) + + # Clear all featured sounds + form_data["featured_sounds"] = "" + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + self.assertEqual(302, resp.status_code) + self.collection.refresh_from_db() + self.assertEqual([], self.collection.featured_sound_ids) + + def test_edit_featured_sounds_as_maintainer(self): + # Maintainer should not be able to edit featured sounds + self.collection.maintainers.add(self.maintainer) + self.collection.featured_sound_ids = [self.sound.id] + self.collection.save() + + # Add sounds to collection first + CollectionSound.objects.create(user=self.user, sound=self.sound, collection=self.collection, status="OK") + CollectionSound.objects.create(user=self.user, sound=self.sound1, collection=self.collection, status="OK") + self.collection.refresh_from_db() + + self.client.force_login(self.maintainer) + + # Try to modify featured sounds as maintainer (should be ignored) + form_data = { + "name": "testcollection", + "description": "", + "public": False, + "collection_sounds": f"{self.sound.id},{self.sound1.id}", + "featured_sounds": f"{self.sound1.id}", # Try to change featured sounds + } + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + self.assertEqual(302, resp.status_code) + self.collection.refresh_from_db() + # Featured sounds should remain unchanged (original value preserved) + self.assertEqual([self.sound.id], self.collection.featured_sound_ids) + + def test_add_sounds_outside_collection_to_featured(self): + # Sounds not in the collection should not be added to featured_sound_ids + self.client.force_login(self.user) + + # Create a sound that is NOT in the collection + _, _, other_sounds = create_user_and_sounds( + num_sounds=1, count_offset=10, user=self.user, processing_state="OK", moderation_state="OK" + ) + sound_not_in_collection = other_sounds[0] + + # Add only sound and sound1 to the collection, but try to feature sound_not_in_collection + form_data = { + "name": "testcollection", + "description": "", + "public": False, + "collection_sounds": f"{self.sound.id},{self.sound1.id}", + "featured_sounds": f"{self.sound.id},{sound_not_in_collection.id}", + } + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + self.assertEqual(302, resp.status_code) + self.collection.refresh_from_db() + # Only sound.id should be in featured (sound_not_in_collection should be filtered out) + self.assertEqual([self.sound.id], self.collection.featured_sound_ids) + + def test_remove_sounds_that_are_featured(self): + # When a featured sound is removed from collection, it should be removed from featured_sound_ids + self.client.force_login(self.user) + + # Add sounds and set as featured + form_data = { + "name": "testcollection", + "description": "", + "public": False, + "collection_sounds": f"{self.sound.id},{self.sound1.id},{self.sound2.id}", + "featured_sounds": f"{self.sound.id},{self.sound1.id},{self.sound2.id}", + } + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + self.assertEqual(302, resp.status_code) + self.collection.refresh_from_db() + self.assertEqual([self.sound.id, self.sound1.id, self.sound2.id], self.collection.featured_sound_ids) + + # Remove a sound that is featured + form_data["collection_sounds"] = f"{self.sound.id},{self.sound2.id}" + form_data["featured_sounds"] = f"{self.sound.id},{self.sound1.id},{self.sound2.id}" # Still has sound1 in featured + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + self.assertEqual(302, resp.status_code) + self.collection.refresh_from_db() + # sound1 should be removed from featured_sound_ids since it's no longer in the collection + self.assertEqual([self.sound.id, self.sound2.id], self.collection.featured_sound_ids) + + # Remove all sounds + form_data["collection_sounds"] = "" + form_data["featured_sounds"] = f"{self.sound.id},{self.sound2.id}" + resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + self.assertEqual(302, resp.status_code) + self.collection.refresh_from_db() + self.assertEqual([], self.collection.featured_sound_ids) diff --git a/fscollections/urls.py b/fscollections/urls.py index 7e28d9c05..763176f34 100644 --- a/fscollections/urls.py +++ b/fscollections/urls.py @@ -4,18 +4,18 @@ urlpatterns = [ path("", views.collections_for_user, name="your-collections"), - path("_/", views.collection, name="collection"), + path("-/", views.collection, name="collection"), path("/add/", views.add_sound_to_collection, name="add-sound-to-collection"), path("create/", views.create_collection, name="create-collection"), - path("_/edit", views.edit_collection, name="edit-collection"), - path("_/delete", views.delete_collection, name="delete-collection"), - path("_/download/", views.download_collection, name="download-collection"), - path("_/licenses/", views.collection_licenses, name="collection-licenses"), + path("-/edit", views.edit_collection, name="edit-collection"), + path("-/delete", views.delete_collection, name="delete-collection"), + path("-/download/", views.download_collection, name="download-collection"), + path("-/licenses/", views.collection_licenses, name="collection-licenses"), path( - "_/addsoundsmodal", + "-/addsoundsmodal", views.add_sounds_modal_for_collection_edit, name="add-sounds-modal-collection", ), - path("_/addmaintainersmodal", views.add_maintainer_modal, name="add-maintainers-modal"), - path("_/section/stats", views.collection_stats_section, name="collection-stats-section"), + path("-/addmaintainersmodal", views.add_maintainer_modal, name="add-maintainers-modal"), + path("-/section/stats", views.collection_stats_section, name="collection-stats-section"), ] diff --git a/fscollections/views.py b/fscollections/views.py index 828db66f6..4d6f42e22 100644 --- a/fscollections/views.py +++ b/fscollections/views.py @@ -35,7 +35,8 @@ MaintainerForm, SelectCollectionOrNewCollectionForm, ) -from fscollections.models import Collection, CollectionDownload, CollectionDownloadSound, CollectionSound, order_sounds_by_featured +from freesound import settings +from fscollections.models import Collection, CollectionDownload, CollectionDownloadSound, CollectionSound from sounds.models import Sound from sounds.views import add_sounds_modal_helper from utils.downloads import download_sounds @@ -43,7 +44,7 @@ @login_required -def collection(request, collection_id, collection_name): +def collection(request, collection_name, collection_id): user = request.user is_owner = False is_maintainer = False @@ -54,15 +55,35 @@ def collection(request, collection_id, collection_name): is_owner = user == collection.user maintainers = collection.maintainers.all() - tvars = {"collection": collection, "is_owner": is_owner, "is_maintainer": is_maintainer, "maintainers": maintainers} - # one URL needed to display all collections and one URL to display ONE collection at a time - # the collections_for_user can be reused to display ONE collection so give it a thought on full collections display - collection_sounds = Sound.objects.prefetch_related("collections").filter(collections=collection) - collection_sounds = order_sounds_by_featured(collection_sounds, collection.featured_sound_ids) + sort_by = request.GET.get("s", settings.COLLECTION_SORT_DEFAULT) + search_query = request.GET.get("q", "").strip() + collection_sounds = collection.get_sounds(sort_by=sort_by) + + # Filter sounds by search query if provided + if search_query: + collection_sounds = collection_sounds.filter( + Q(original_filename__icontains=search_query) + ).distinct() + paginator = paginate(request, collection_sounds, settings.BOOKMARKS_PER_PAGE) page_sounds = Sound.objects.ordered_ids([sound.id for sound in paginator["page"].object_list]) + + # Get featured sounds on the current page for grouped display (only when sorting by featured) + featured_sounds_on_page = [s for s in page_sounds if s.id in collection.featured_sound_ids] if settings.COLLECTION_SORT_OPTIONS[sort_by] == "featured_order" else [] + + tvars = { + "collection": collection, + "is_owner": is_owner, + "is_maintainer": is_maintainer, + "maintainers": maintainers, + "sort_by": sort_by, + "sort_options": settings.COLLECTION_SORT_OPTIONS, + "featured_sounds_on_page": featured_sounds_on_page, + "search_query": search_query, + } tvars.update(paginator) tvars["page_collection_and_sound_objects"] = zip(paginator["page"].object_list, page_sounds) + return render(request, "collections/collection.html", tvars) @@ -78,7 +99,7 @@ def collections_for_user(request): @login_required -def collection_stats_section(request, collection_id, collection_name): +def collection_stats_section(request, collection_name, collection_id): # TODO: this tries to imitate the pack_stats_section behaviour despite a lack of comprehension # on cache behaviour, so the stats shown by this are not properly updated if not request.GET.get("ajax"): @@ -154,7 +175,7 @@ def create_collection(request): @login_required -def delete_collection(request, collection_id, collection_name): +def delete_collection(request, collection_name, collection_id): collection = get_object_or_404(Collection, id=collection_id) if request.method == "POST" and request.user == collection.user: @@ -168,11 +189,11 @@ def delete_collection(request, collection_id, collection_name): messages.INFO, "You're not allowed to delete this collection.In order to delete a collection you must be the owner.", ) - return HttpResponseRedirect(reverse("collection", args=[collection.id, slugify(collection.name)])) + return HttpResponseRedirect(reverse("collection", args=[slugify(collection.name), collection.id])) @login_required -def edit_collection(request, collection_id, collection_name): +def edit_collection(request, collection_name, collection_id): collection = get_object_or_404(Collection, id=collection_id) collection_sounds = ",".join([str(s.id) for s in Sound.objects.filter(collections=collection)]) maintainers_query = User.objects.filter(collection_maintainer=collection.id) @@ -184,7 +205,7 @@ def edit_collection(request, collection_id, collection_name): elif request.user in maintainers_query: is_maintainer = True else: - return HttpResponseRedirect(reverse("collection", args=[collection_id, slugify(collection.name)])) + return HttpResponseRedirect(reverse("collection", args=[slugify(collection.name), collection_id])) current_sounds = list() if request.method == "POST": @@ -197,11 +218,11 @@ def edit_collection(request, collection_id, collection_name): request.POST, instance=collection, label_suffix="", is_owner=is_owner, is_maintainer=is_maintainer ) else: - return HttpResponseRedirect(reverse("collection", args=[collection_id, slugify(collection.name)])) + return HttpResponseRedirect(reverse("collection", args=[slugify(collection.name), collection_id])) if form.is_valid(): form.save(user_adding_sound=request.user) - return HttpResponseRedirect(reverse("collection", args=[collection.id, slugify(collection.name)])) + return HttpResponseRedirect(reverse("collection", args=[slugify(collection.name), collection.id])) else: # NOTE: in this form's validation, errors are raised for each speific field, so when there is a submission attempt the error # is displayed within it. However, fields containing errors are removed from the clean data but we are still interested in @@ -229,10 +250,11 @@ def edit_collection(request, collection_id, collection_name): ) form._errors = errors_data else: + featured_sounds_str = ",".join([str(sid) for sid in collection.featured_sound_ids]) if is_owner: form = CollectionEditForm( instance=collection, - initial=dict(collection_sounds=collection_sounds, maintainers=collection_maintainers), + initial=dict(collection_sounds=collection_sounds, maintainers=collection_maintainers, featured_sounds=featured_sounds_str), label_suffix="", is_owner=is_owner, is_maintainer=is_maintainer, @@ -240,13 +262,14 @@ def edit_collection(request, collection_id, collection_name): elif is_maintainer: form = CollectionEditFormAsMaintainer( instance=collection, - initial=dict(collection_sounds=collection_sounds, maintainers=collection_maintainers), + initial=dict(collection_sounds=collection_sounds, maintainers=collection_maintainers, featured_sounds=featured_sounds_str), label_suffix="", is_owner=is_owner, is_maintainer=is_maintainer, ) - current_sounds = Sound.objects.bulk_sounds_for_collection(collection_id=collection.id) - current_sounds = order_sounds_by_featured(current_sounds, collection.featured_sound_ids) + sort_by = request.GET.get("s", settings.COLLECTION_SORT_DEFAULT) + current_sounds = collection.get_sounds(sort_by=sort_by) + current_maintainers = User.objects.filter(collection_maintainer=collection.id) form.collection_sound_objects = current_sounds form.collection_maintainers_objects = current_maintainers @@ -256,13 +279,15 @@ def edit_collection(request, collection_id, collection_name): "is_owner": is_owner, "is_maintainer": is_maintainer, "max_sounds_per_collection": settings.MAX_SOUNDS_PER_COLLECTION, + "sort_by": sort_by, + "sort_options": settings.COLLECTION_SORT_OPTIONS, } return render(request, "collections/edit_collection.html", tvars) @login_required -def download_collection(request, collection_id, collection_name): +def download_collection(request, collection_name, collection_id): collection = get_object_or_404(Collection, id=collection_id) collection_sounds = CollectionSound.objects.filter(collection=collection).values("sound_id") sounds_list = Sound.objects.filter( @@ -282,12 +307,12 @@ def download_collection(request, collection_id, collection_name): cds.append(CollectionDownloadSound(sound=sound, collection_download=cd, license=sound.license)) CollectionDownloadSound.objects.bulk_create(cds) - licenses_url = reverse("collection-licenses", args=[collection_id, collection_name]) + licenses_url = reverse("collection-licenses", args=[collection_name, collection_id]) licenses_content = collection.get_attribution(sound_qs=sounds_list) return download_sounds(licenses_url, licenses_content, sounds_list, collection.download_filename) -def collection_licenses(request, collection_id, collection_name): +def collection_licenses(request, collection_name, collection_id): collection = get_object_or_404(Collection, id=collection_id) attribution = collection.get_attribution() return HttpResponse(attribution, content_type="text/plain") @@ -299,7 +324,7 @@ def add_sounds_modal_for_collection_edit(request, collection_id): return render(request, "sounds/modal_add_sounds.html", tvars) -def add_maintainer_modal(request, collection_id, collection_name): +def add_maintainer_modal(request, collection_name, collection_id): collection = get_object_or_404(Collection, id=collection_id) form = MaintainerForm() # TODO: the below statements exclude users with whitespaces in their usernames (and they still exist) diff --git a/templates/collections/collection.html b/templates/collections/collection.html index 8f1f49e0b..ebae28e01 100644 --- a/templates/collections/collection.html +++ b/templates/collections/collection.html @@ -6,22 +6,26 @@ {% load display_user %} {% load util %} -{% block title%}Collections{%endblock title%} -{% block page-title%}{{collection.name}}{%endblock page-title%} +{% block title%}{{collection.name}} - Collection{%endblock title%} +{% block page-title-custom %} +
+

Collection: {{ collection.name }}

+
+{% endblock page-title-custom %} {% block page-content %}
+ data-async-section-content-url="{% url 'collection-stats-section' collection.name|slugify collection.id %}?ajax=1">
{% if is_owner or is_maintainer%} - Edit collection + Edit collection {% endif %} {% if collection.num_sounds > 0%} - Download Collection + Download collection {% endif %}
@@ -53,21 +57,45 @@ {% endif %}
-
Sounds in this collection
+ {% comment %}search and sorting options{% endcomment %} +
+ +
+
+
+ +
+
+
+ Sort by: + +
+
+ +
{% if page.object_list %}
+ {% if sort_by == "Featured first" %} + {% display_featured_sounds featured_sounds_on_page %} + {% endif %} {% for collectionsound, sound in page_collection_and_sound_objects %} + {% if sort_by != "Featured first" or sound.id not in collection.featured_sound_ids %}
- {% if sound.id in collection.featured_sound_ids %} - {% display_featured_sound sound %} - {% else %} {% display_sound_small sound %} - {% endif %}
+ {% endif %} {% endfor %}
- {% else %} - There aren't any sounds in this collection yet 😟 + {% else %} + {% if search_query %} + No sounds found matching "{{ search_query }}" in this collection. Clear search + {% else %} + There aren't any sounds in this collection yet 😟 + {% endif %} {% endif %} {% bw_paginator paginator page current_page request "collection" %}
diff --git a/templates/collections/display_collection.html b/templates/collections/display_collection.html index c1bf3c8b7..a98be38ab 100644 --- a/templates/collections/display_collection.html +++ b/templates/collections/display_collection.html @@ -3,52 +3,52 @@ {% load display_sound %} {% load bw_templatetags %} -
- {% if ft_sound %} -
-
- {% display_sound_small_no_info_no_buttons ft_sound %} +
+
+
+
+ {% for sound in featured_sounds %} +
+ {% display_sound_small_no_info_no_buttons sound %} +
+ {% empty %} +
+ {% endfor %} +
- {% else %} -
- {% endif %} -
- +
+
+ {{ collection.name }} +
-
- {% if collection.description %} - {% with collection.description|striptags|safe as preprocessed_description %} -
= 10%}title="{{ preprocessed_description|truncatewords_html:200|force_escape }}"{% endif %}> - {{ preprocessed_description|truncatewords_html:10 }} -
- {% endwith %} - {% else %} - This collection has no description. - {% endif %} -
-
-
-
-
-
- {% bw_user_avatar collection.user.profile.locations.avatar.S.url collection.user.username 32 %} -
- {{ collection.user | truncate_string:15 }} -
+
+ {{ collection.user.username|truncate_string:15 }}
-
-
+
+
{{ collection.num_sounds|formatnumber }}
-
- - {{ collection.num_downloads|formatnumber }} + {% if collection.num_downloads %} +
+ + {{ collection.num_downloads|formatnumber }}
+ {% endif %}
+
+ {{ collection.created|date:"F jS, Y" }} +
+
+ {% if collection.description %} + {% with collection.description|striptags|safe as preprocessed_description %} +
= 55%}title="{{ preprocessed_description|truncatewords_html:200|force_escape }}"{% endif %}> + {{ preprocessed_description|truncatechars_html:55 }} +
+ {% endwith %} + {% endif %} +
\ No newline at end of file diff --git a/templates/collections/display_featured_sound.html b/templates/collections/display_featured_sound.html deleted file mode 100644 index 9691ebf25..000000000 --- a/templates/collections/display_featured_sound.html +++ /dev/null @@ -1,9 +0,0 @@ -{% load display_sound %} - -
-
- Featured -
- {% display_sound_small sound %} -
- diff --git a/templates/collections/display_featured_sounds.html b/templates/collections/display_featured_sounds.html new file mode 100644 index 000000000..c57d9b6da --- /dev/null +++ b/templates/collections/display_featured_sounds.html @@ -0,0 +1,16 @@ +{% load display_sound %} + +{% if featured_sounds %} +
+
+
Featured
+
+ {% for sound in featured_sounds %} +
+ {% display_sound_small sound %} +
+ {% endfor %} +
+
+
+{% endif %} diff --git a/templates/collections/edit_collection.html b/templates/collections/edit_collection.html index 175989d63..0374eab02 100644 --- a/templates/collections/edit_collection.html +++ b/templates/collections/edit_collection.html @@ -49,18 +49,41 @@

{{ collection.name }}

{% endif %} {% users_selector form.collection_maintainers_objects%}
- +
{{ form.collection_sounds }} - + {{ form.featured_sounds }} +
+ +
+ + + +
+
{{form.collection_sounds.errors}}
{% sounds_selector form.collection_sound_objects max_sounds=max_sounds_per_collection%}
- - + + +
+
+ +
+
+
@@ -72,7 +95,7 @@

{{ collection.name }}

{% endif %} {% if is_owner%} - + {% endif %}
diff --git a/templates/collections/modal_add_maintainer.html b/templates/collections/modal_add_maintainer.html index 0749682e7..362c00bfa 100644 --- a/templates/collections/modal_add_maintainer.html +++ b/templates/collections/modal_add_maintainer.html @@ -11,7 +11,7 @@

Add maintainer to collection

-
{% csrf_token %} + {% csrf_token %} {{form}} {% if not_found_msg %}
{% bw_icon "notification" %}{{not_found_msg}}
diff --git a/templates/collections/modal_add_sound_to_collection.html b/templates/collections/modal_add_sound_to_collection.html index 059e11bbf..678011c42 100644 --- a/templates/collections/modal_add_sound_to_collection.html +++ b/templates/collections/modal_add_sound_to_collection.html @@ -46,7 +46,15 @@

Add sound to collection

{% endif %} {% csrf_token %} - {{ form }} +
+ {{ form.collection }} + +
+ {{ form.new_collection_name }} + {{ form.use_last_collection }}
From 80fa3497294195d88ee763876aefccea166a556d Mon Sep 17 00:00:00 2001 From: Domonkos Kostyal Date: Fri, 6 Mar 2026 17:23:07 +0100 Subject: [PATCH 03/20] edit collections page finish --- freesound/settings.py | 11 +- .../src/components/addSoundsModal.js | 122 ++++++++---- .../bw-frontend/src/components/index.js | 4 + .../src/components/objectSelector.js | 110 ++++++++--- freesound/static/bw-frontend/src/htmx-init.js | 28 +++ freesound/static/bw-frontend/src/index.js | 4 +- .../bw-frontend/src/pages/collectionEdit.js | 179 +++++++----------- .../static/bw-frontend/src/utils/data.js | 28 +-- .../bw-frontend/src/utils/soundStateStore.js | 150 +++++++++++++++ .../bw-frontend/styles/atoms/inputs.scss | 8 + .../styles/atoms/selectableObject.scss | 162 ++++++++++++++-- .../bw-frontend/styles/atoms/selects.scss | 1 + .../bw-frontend/styles/molecules/forms.scss | 14 ++ fscollections/forms.py | 124 ++++++------ ...collectionsound_featured_sound_and_more.py | 23 +++ fscollections/models.py | 75 +------- .../templatetags/display_collections.py | 11 +- fscollections/tests.py | 127 +++++++------ fscollections/urls.py | 16 +- fscollections/views.py | 177 ++++++++--------- general/templatetags/bw_templatetags.py | 7 +- package.json | 1 + sounds/models.py | 33 +++- sounds/templatetags/display_sound.py | 13 ++ sounds/templatetags/sounds_selector.py | 12 +- sounds/tests/test_manager.py | 48 +++++ templates/base.html | 1 + templates/collections/collection.html | 45 +++-- templates/collections/display_collection.html | 6 +- .../collections/display_featured_sounds.html | 16 -- templates/collections/edit_collection.html | 93 +++++---- .../collections/modal_add_maintainer.html | 2 +- templates/molecules/object_selector.html | 6 +- templates/molecules/paginator.html | 2 +- .../sounds/display_sound_with_actions.html | 18 ++ 35 files changed, 1084 insertions(+), 593 deletions(-) create mode 100644 freesound/static/bw-frontend/src/htmx-init.js create mode 100644 freesound/static/bw-frontend/src/utils/soundStateStore.js create mode 100644 fscollections/migrations/0003_remove_collectionsound_featured_sound_and_more.py delete mode 100644 templates/collections/display_featured_sounds.html create mode 100644 templates/sounds/display_sound_with_actions.html diff --git a/freesound/settings.py b/freesound/settings.py index 73e38541b..5d839f861 100644 --- a/freesound/settings.py +++ b/freesound/settings.py @@ -1443,13 +1443,12 @@ MAX_SOUNDS_PER_COLLECTION = 250 MAX_FEATURED_SOUNDS_PER_COLLECTION = 250 COLLECTION_SORT_OPTIONS = { - "Featured first": "featured_order", - "Date added (newest first)": "-collectionsound__created", - "Date added (oldest first)": "collectionsound__created", - "Duration (longest first)": "-duration", - "Duration (shortest first)": "duration", + "featured": ("Featured first", "featured_order"), + "created_desc": ("Date added (newest first)", "-collectionsound__created"), + "created_asc": ("Date added (oldest first)", "collectionsound__created"), + "name": ("Name", "original_filename"), } -COLLECTION_SORT_DEFAULT = "Featured first" +COLLECTION_SORT_DEFAULT = "featured" # ------------------------------------------------------------------------------- # Import local settings diff --git a/freesound/static/bw-frontend/src/components/addSoundsModal.js b/freesound/static/bw-frontend/src/components/addSoundsModal.js index c1256272b..4ad8bf09a 100644 --- a/freesound/static/bw-frontend/src/components/addSoundsModal.js +++ b/freesound/static/bw-frontend/src/components/addSoundsModal.js @@ -1,44 +1,59 @@ -import {dismissModal, handleGenericModal} from "../components/modal" -import {initializeObjectSelector, updateObjectSelectorDataProperties} from '../components/objectSelector' -import {serializedIdListToIntList, combineIdsLists} from "../utils/data" +import { dismissModal, handleGenericModal } from "../components/modal" +import { initializeObjectSelector, updateObjectSelectorDataProperties } from '../components/objectSelector' +import { serializedIdListToIntList, combineIdsLists } from "../utils/data" -const handleAddSoundsModal = (modalId, modalUrl, selectedSoundsDestinationElement, onSoundsSelectedCallback) => { - handleGenericModal(modalUrl, (modalContainer) => { - const inputElement = modalContainer.getElementsByTagName('input')[0]; - inputElement.addEventListener("keypress", function(event) { - if (event.key === "Enter") { + +const openAddSoundsModal = (modalId, modalUrl, url, getExcludeIds, onSoundsConfirmed) => { + handleGenericModal(url, (modalContainer) => { + const inputElement = modalContainer.querySelector('input'); + inputElement.addEventListener('keypress', (event) => { + if (event.key === 'Enter') { event.preventDefault(); const baseUrl = modalUrl.split('?')[0]; - const soundIdsToExclude = combineIdsLists(serializedIdListToIntList(selectedSoundsDestinationElement.dataset.selectedIds), serializedIdListToIntList(selectedSoundsDestinationElement.dataset.unselectedIds)).join(','); - handleAddSoundsModal(modalId, `${baseUrl}?q=${inputElement.value}&exclude=${soundIdsToExclude}`, selectedSoundsDestinationElement, onSoundsSelectedCallback); + const excludeIds = getExcludeIds(); + openAddSoundsModal(modalId, modalUrl, `${baseUrl}?q=${inputElement.value}&exclude=${excludeIds}`, getExcludeIds, onSoundsConfirmed); } }); - const objectSelectorElement = modalContainer.getElementsByClassName('bw-object-selector-container')[0]; + const objectSelectorElement = modalContainer.querySelector('.bw-object-selector-container'); + const addSelectedSoundsButton = modalContainer.querySelector('button'); + addSelectedSoundsButton.disabled = true; initializeObjectSelector(objectSelectorElement, (element) => { - addSelectedSoundsButton.disabled = element.dataset.selectedIds == "" + addSelectedSoundsButton.disabled = element.dataset.selectedIds === ''; }); - const addSelectedSoundsButton = modalContainer.getElementsByTagName('button')[0]; - addSelectedSoundsButton.disabled = true; - addSelectedSoundsButton.addEventListener('click', evt => { - const selectableSoundElements = [...modalContainer.getElementsByClassName('bw-selectable-object')]; - selectableSoundElements.forEach( element => { - const checkbox = element.querySelectorAll('input.bw-checkbox')[0]; - if (checkbox.checked) { - const clonedCheckbox = checkbox.cloneNode(); // Cloning the node will remove the event listeners which refer to the "old" sound selector - delete(clonedCheckbox.dataset.initialized); // This will force re-initialize the element when added to the new sounds selector - clonedCheckbox.checked = false; - checkbox.parentNode.replaceChild(clonedCheckbox, checkbox); - element.classList.remove('selected'); - selectedSoundsDestinationElement.appendChild(element.parentNode); - } - }); - onSoundsSelectedCallback(objectSelectorElement.dataset.selectedIds); + addSelectedSoundsButton.addEventListener('click', () => { + onSoundsConfirmed(modalContainer); dismissModal(modalId); }); }, undefined, true, true); -} +}; + +const handleAddSoundsModal = (modalId, modalUrl, selectedSoundsDestinationElement, onSoundsSelectedCallback) => { + const getExcludeIds = () => combineIdsLists( + serializedIdListToIntList(selectedSoundsDestinationElement.dataset.selectedIds), + serializedIdListToIntList(selectedSoundsDestinationElement.dataset.unselectedIds) + ).join(','); + + const onSoundsConfirmed = (modalContainer) => { + const objectSelectorElement = modalContainer.querySelector('.bw-object-selector-container'); + const selectableSoundElements = [...modalContainer.querySelectorAll('.bw-selectable-object')]; + selectableSoundElements.forEach(element => { + const checkbox = element.querySelector('input.bw-checkbox'); + if (checkbox && checkbox.checked) { + const clonedCheckbox = checkbox.cloneNode(); + delete (clonedCheckbox.dataset.initialized); + clonedCheckbox.checked = false; + checkbox.parentNode.replaceChild(clonedCheckbox, checkbox); + element.classList.remove('selected'); + selectedSoundsDestinationElement.appendChild(element.parentNode); + } + }); + onSoundsSelectedCallback(objectSelectorElement.dataset.selectedIds); + }; + + openAddSoundsModal(modalId, modalUrl, modalUrl, getExcludeIds, onSoundsConfirmed); +}; const prepareAddSoundsModalAndFields = (container) => { const addSoundsButtons = [...container.querySelectorAll(`[data-toggle^="add-sounds-modal"]`)]; @@ -52,7 +67,7 @@ const prepareAddSoundsModalAndFields = (container) => { }); const soundsInput = selectedSoundsDestinationElement.parentNode.parentNode.getElementsByTagName('input')[0]; - if(soundsInput.disabled){ + if (soundsInput.disabled) { addSoundsButton.disabled = true const checkboxes = selectedSoundsDestinationElement.querySelectorAll('span.bw-checkbox-container'); checkboxes.forEach(checkbox => { @@ -62,11 +77,11 @@ const prepareAddSoundsModalAndFields = (container) => { const soundsLabel = selectedSoundsDestinationElement.parentNode.parentNode.getElementsByTagName('label')[0]; const itemCountElementInLabel = soundsLabel === null ? null : soundsLabel.querySelector('#element-count'); - + const maxSounds = selectedSoundsDestinationElement.dataset.maxElements; const maxSoundsHelpText = selectedSoundsDestinationElement.parentNode.parentNode.getElementsByClassName('helptext')[0] - if(maxSounds !== "None"){ - if (soundsInput.value.split(',').length >= maxSounds){ + if (maxSounds !== "None") { + if (soundsInput.value.split(',').length >= maxSounds) { addSoundsButton.disabled = true maxSoundsHelpText.style.display = 'block'; } @@ -83,12 +98,13 @@ const prepareAddSoundsModalAndFields = (container) => { updateObjectSelectorDataProperties(selectedSoundsDestinationElement); const selectedSoundsHiddenInput = document.getElementById(addSoundsButton.dataset.selectedSoundsHiddenInputId); selectedSoundsHiddenInput.value = selectedSoundsDestinationElement.dataset.unselectedIds; - if(maxSounds !== "None" && selectedSoundsHiddenInput.value.split(',').length < maxSounds){ + if (maxSounds !== "None" && selectedSoundsHiddenInput.value.split(',').length < maxSounds) { addSoundsButton.disabled = false; maxSoundsHelpText.style.display = 'none'; } - if (itemCountElementInLabel){ - itemCountElementInLabel.innerHTML = selectedSoundsDestinationElement.children.length} + if (itemCountElementInLabel) { + itemCountElementInLabel.innerHTML = selectedSoundsDestinationElement.children.length + } removeSoundsButton.disabled = true; }); @@ -100,19 +116,41 @@ const prepareAddSoundsModalAndFields = (container) => { const newSoundIds = serializedIdListToIntList(selectedSoundIds); const combinedIds = combineIdsLists(currentSoundIds, newSoundIds); selectedSoundsHiddenInput.value = combinedIds.join(','); - if(maxSounds !== "None" && selectedSoundsHiddenInput.value.split(',').length >= maxSounds){ + if (maxSounds !== "None" && selectedSoundsHiddenInput.value.split(',').length >= maxSounds) { addSoundsButton.disabled = true; maxSoundsHelpText.style.display = 'block'; } - if (itemCountElementInLabel){ - itemCountElementInLabel.innerHTML = selectedSoundsDestinationElement.children.length} + if (itemCountElementInLabel) { + itemCountElementInLabel.innerHTML = selectedSoundsDestinationElement.children.length + } initializeObjectSelector(selectedSoundsDestinationElement, (element) => { removeSoundsButton.disabled = element.dataset.selectedIds == "" }); }); - - }); + + }); }); } -export {prepareAddSoundsModalAndFields}; + +const prepareAddSoundsModalDynamic = (container, getExcludeIds, onSoundsConfirmed) => { + const addSoundsButton = container.querySelector('[data-toggle="add-sounds-modal"]'); + if (!addSoundsButton) return; + + const onConfirmed = (modalContainer) => { + const selectedIds = [...modalContainer.querySelectorAll('.bw-selectable-object')] + .reduce((acc, element) => { + const checkbox = element.querySelector('input.bw-checkbox'); + if (checkbox && checkbox.checked) acc.push(parseInt(checkbox.dataset.objectId, 10)); + return acc; + }, []); + onSoundsConfirmed(selectedIds); + }; + + addSoundsButton.addEventListener('click', (evt) => { + evt.preventDefault(); + openAddSoundsModal('addSoundsModal', addSoundsButton.dataset.modalUrl, addSoundsButton.dataset.modalUrl, getExcludeIds, onConfirmed); + }); +}; + +export { prepareAddSoundsModalAndFields, prepareAddSoundsModalDynamic }; diff --git a/freesound/static/bw-frontend/src/components/index.js b/freesound/static/bw-frontend/src/components/index.js index b77d2089c..ca1a16187 100644 --- a/freesound/static/bw-frontend/src/components/index.js +++ b/freesound/static/bw-frontend/src/components/index.js @@ -17,3 +17,7 @@ import './uiThemeDetector'; import { initializeStuffInContainer } from '../utils/initHelper'; initializeStuffInContainer(document, true, true); + +// Export initializeStuffInContainer globally for htmx integration +// This allows base.html to call it after htmx is loaded +window.initializeStuffInContainer = initializeStuffInContainer; diff --git a/freesound/static/bw-frontend/src/components/objectSelector.js b/freesound/static/bw-frontend/src/components/objectSelector.js index 28bbf9d1e..80f2a5a3e 100644 --- a/freesound/static/bw-frontend/src/components/objectSelector.js +++ b/freesound/static/bw-frontend/src/components/objectSelector.js @@ -6,19 +6,19 @@ const updateObjectSelectorDataProperties = (selectorElement, callback) => { const unselectedIds = []; objectCheckboxes.forEach(checkbox => { if (checkbox.checked) { - selectedIds.push(checkbox.dataset.objectId); + selectedIds.push(checkbox.dataset.objectId); } else { - unselectedIds.push(checkbox.dataset.objectId); - } + unselectedIds.push(checkbox.dataset.objectId); + } }); selectorElement.dataset.selectedIds = selectedIds.join(','); selectorElement.dataset.unselectedIds = unselectedIds.join(','); - if (callback !== undefined){ + if (callback !== undefined) { callback(selectorElement); } } -const debouncedUpdateObjectSelectorDataProperties = debounce(updateObjectSelectorDataProperties, 100, {'trailing': true}) +const debouncedUpdateObjectSelectorDataProperties = debounce(updateObjectSelectorDataProperties, 100, { 'trailing': true }) const initializeObjectSelector = (selectorElement, onChangeCallback) => { @@ -26,14 +26,14 @@ const initializeObjectSelector = (selectorElement, onChangeCallback) => { // Also note that if called multiple times, only the first passed onChangeCallback will remain active const debouncedOnChangeCallback = debounce(onChangeCallback); const selectableObjectElements = [...selectorElement.getElementsByClassName('bw-selectable-object')]; - selectableObjectElements.forEach( element => { + selectableObjectElements.forEach(element => { const checkbox = element.querySelectorAll('input.bw-checkbox')[0]; - if (checkbox.dataset.initialized === undefined){ + if (checkbox && checkbox.dataset.initialized === undefined) { debouncedUpdateObjectSelectorDataProperties(element.parentNode.parentNode); checkbox.dataset.initialized = true; // Avoid re-initializing multiple times the same object checkbox.addEventListener('change', evt => { if (checkbox.checked) { - element.classList.add('selected'); + element.classList.add('selected'); } else { element.classList.remove('selected'); } @@ -44,34 +44,98 @@ const initializeObjectSelector = (selectorElement, onChangeCallback) => { // Configure select all/none buttons const selectAllSelectNoneButtons = selectorElement.parentNode.getElementsByClassName('select-button'); - if (selectAllSelectNoneButtons.length == 2){ + if (selectAllSelectNoneButtons.length == 2) { const selectAllButton = selectAllSelectNoneButtons[0]; const selectNoneButton = selectAllSelectNoneButtons[1]; selectAllButton.addEventListener('click', evt => { selectableObjectElements.forEach(element => { const checkbox = element.querySelectorAll('input.bw-checkbox')[0]; - checkbox.checked = true; - if (checkbox.checked) { - element.classList.add('selected'); - } else { - element.classList.remove('selected'); + if (checkbox) { + checkbox.checked = true; + if (checkbox.checked) { + element.classList.add('selected'); + } else { + element.classList.remove('selected'); + } + debouncedUpdateObjectSelectorDataProperties(element.parentNode.parentNode, debouncedOnChangeCallback); } - debouncedUpdateObjectSelectorDataProperties(element.parentNode.parentNode, debouncedOnChangeCallback); }); }); selectNoneButton.addEventListener('click', evt => { selectableObjectElements.forEach(element => { const checkbox = element.querySelectorAll('input.bw-checkbox')[0]; - checkbox.checked = false; - if (checkbox.checked) { - element.classList.add('selected'); - } else { - element.classList.remove('selected'); + if (checkbox) { + checkbox.checked = false; + if (checkbox.checked) { + element.classList.add('selected'); + } else { + element.classList.remove('selected'); + } + debouncedUpdateObjectSelectorDataProperties(element.parentNode.parentNode, debouncedOnChangeCallback); } - debouncedUpdateObjectSelectorDataProperties(element.parentNode.parentNode, debouncedOnChangeCallback); }); }); } - + } -export {initializeObjectSelector, updateObjectSelectorDataProperties}; + +// --------------------------------------------------------------------------- +// Visual-state helper for .with-actions containers +// --------------------------------------------------------------------------- +const updateActionUI = (container, actionName, isActive) => { + const btn = container.querySelector('[data-action="' + actionName + '"]'); + if (!btn) return; + + btn.classList.toggle('active', isActive); + + const containerClass = btn.dataset.containerActiveClass; + if (containerClass) { + container.classList.toggle(containerClass, isActive); + } + + const activeTitle = btn.dataset.activeTitle; + if (activeTitle) { + if (!btn.dataset.originalTitle) { + btn.dataset.originalTitle = btn.title || ''; + } + btn.title = isActive ? activeTitle : btn.dataset.originalTitle; + } + + const disables = btn.dataset.disables; + if (disables) { + const targetBtn = container.querySelector('[data-action="' + disables + '"]'); + if (targetBtn) { + targetBtn.disabled = isActive; + } + } + + btn.blur(); +}; + +const initializeObjectSelectorActions = (parentElement, store) => { + const containers = parentElement.querySelectorAll('.bw-selectable-object.with-actions'); + containers.forEach(container => { + if (container.dataset.actionsInitialized) return; + container.dataset.actionsInitialized = 'true'; + + const objectId = parseInt(container.dataset.objectId, 10); + + // Restore persisted state for all registered actions + store.actions().forEach(function (entry) { + updateActionUI(container, entry.actionName, store.hasFlag(objectId, entry.flag)); + }); + + // Bind action buttons identified by data-action attribute + container.querySelectorAll('[data-action]').forEach(btn => { + btn.addEventListener('click', evt => { + evt.preventDefault(); + const nowActive = store.toggleAction(objectId, btn.dataset.action); + if (nowActive !== undefined) { + updateActionUI(container, btn.dataset.action, nowActive); + } + }); + }); + }); +}; + +export { initializeObjectSelector, updateObjectSelectorDataProperties, initializeObjectSelectorActions, updateActionUI }; diff --git a/freesound/static/bw-frontend/src/htmx-init.js b/freesound/static/bw-frontend/src/htmx-init.js new file mode 100644 index 000000000..70ace75f4 --- /dev/null +++ b/freesound/static/bw-frontend/src/htmx-init.js @@ -0,0 +1,28 @@ +import htmx from 'htmx.org'; + +window.htmx = htmx; + +/** + * HTMX initialization: CSRF token and reinit of UI components after swaps. + * Expects document.body to have data-csrf-token set by the page template. + */ +(function () { + const csrfToken = document.body.getAttribute('data-csrf-token'); + if (csrfToken) { + document.body.addEventListener('htmx:configRequest', (event) => { + event.detail.headers['X-CSRFToken'] = csrfToken; + }); + } + + // Reinitialize components (players, checkboxes, modals, etc.) when htmx adds new elements to the DOM. + // The htmx:load event is triggered for every new element added to the DOM by HTMX. + document.body.addEventListener('htmx:load', (event) => { + if (window.initializeStuffInContainer) { + const newElement = event.detail.elt; + // Skip document.body since it's already initialized on page load by index.js + if (newElement && newElement !== document.body) { + window.initializeStuffInContainer(newElement, true, false); + } + } + }); +})(); diff --git a/freesound/static/bw-frontend/src/index.js b/freesound/static/bw-frontend/src/index.js index a546a7353..9d7eeddf1 100644 --- a/freesound/static/bw-frontend/src/index.js +++ b/freesound/static/bw-frontend/src/index.js @@ -9,4 +9,6 @@ import 'normalize.css' import '../styles/index.scss' import './components' -import './utils/polyfills' \ No newline at end of file +import './utils/polyfills' + +import './htmx-init' \ No newline at end of file diff --git a/freesound/static/bw-frontend/src/pages/collectionEdit.js b/freesound/static/bw-frontend/src/pages/collectionEdit.js index 02183d3fe..b522d978b 100644 --- a/freesound/static/bw-frontend/src/pages/collectionEdit.js +++ b/freesound/static/bw-frontend/src/pages/collectionEdit.js @@ -1,121 +1,70 @@ -import {prepareAddMaintainersModalAndFields} from "../components/collectionsModal" -import { prepareAddSoundsModalAndFields } from "../components/addSoundsModal"; -import {updateObjectSelectorDataProperties} from '../components/objectSelector' - - -const updateFeaturedHighlights = (soundSelectorContainer, featuredIds) => { - const allSelectableObjects = soundSelectorContainer.querySelectorAll('.bw-selectable-object'); - allSelectableObjects.forEach(element => { - const checkbox = element.querySelector('input.bw-checkbox'); - const existingLabel = element.querySelector('.featured-label'); - - if (checkbox && featuredIds.includes(checkbox.dataset.objectId)) { - element.classList.add('featured'); - // Add "Featured" label if not already present - if (!existingLabel) { - const label = document.createElement('span'); - label.className = 'featured-label'; - label.textContent = 'Featured'; - element.appendChild(label); - } - } else { - element.classList.remove('featured'); - // Remove "Featured" label if present - if (existingLabel) { - existingLabel.remove(); - } - } +import { prepareAddMaintainersModalAndFields } from "../components/collectionsModal" +import { prepareAddSoundsModalDynamic } from "../components/addSoundsModal" +import { initializeObjectSelectorActions } from "../components/objectSelector" +import { SoundStateStore, STATE } from "../utils/soundStateStore" + +const store = new SoundStateStore({ + // collection_sounds is a read-only init element (no name attr, not submitted with the form) + input: document.getElementById('collection_sounds'), + actions: [ + { actionName: 'featured', flag: STATE.FEATURED, input: document.getElementById('featured_sounds') }, + { actionName: 'remove', flag: STATE.REMOVED, input: document.getElementById('removed_sound_ids') }, + { actionName: 'added', flag: STATE.ADDED, input: document.getElementById('added_sound_ids') }, + ], +}); + +// React to state changes: update element count and sync hidden inputs +const countEl = document.getElementById('element-count'); +store.onChange(function (_objectId, flag, _isActive) { + if (countEl && flag & (STATE.REMOVED | STATE.ADDED)) { + countEl.textContent = store.presentCount(); + } + store.syncInputs(); +}); + +const refreshSoundsSection = () => { + const soundsSection = document.getElementById('sounds-section'); + if (!soundsSection) return; + + const searchInput = document.getElementById('edit-collection-search'); + const sortSelect = document.getElementById('sort-select'); + const params = []; + + if (searchInput && searchInput.value) params.push(`q=${encodeURIComponent(searchInput.value)}`); + if (sortSelect && sortSelect.value) params.push(`s=${encodeURIComponent(sortSelect.value)}`); + params.push(`added_sounds=${store.idsWithFlag(STATE.ADDED).join(',')}`); + params.push(`featured_sounds=${store.idsWithFlag(STATE.FEATURED, true).join(',')}`); + + const activePage = soundsSection.querySelector('.bw-pagination_selected'); + if (activePage) params.push(`page=${activePage.textContent.trim()}`); + + const baseUrl = sortSelect.getAttribute('hx-get'); + htmx.ajax('GET', `${baseUrl}?${params.join('&')}`, { + target: '#sounds-section', + select: '#sounds-section', + swap: 'outerHTML', }); }; -const prepareFeaturedSoundsButtons = (container) => { - const setButton = container.querySelector('[data-toggle="set-featured-sounds"]'); - const removeButton = container.querySelector('[data-toggle="remove-featured-sounds"]'); - const clearAllButton = container.querySelector('[data-toggle="clear-all-featured-sounds"]'); - if (!setButton || !removeButton) return; - - const referenceButton = setButton || removeButton; - const soundSelectorContainer = referenceButton.closest('.v-spacing-5').querySelector('.bw-object-selector-container[data-type="sounds"]'); - if (!soundSelectorContainer) return; - - // Disable selection-based buttons initially - setButton.disabled = true; - removeButton.disabled = true; - - const featuredSoundsInput = document.getElementById(referenceButton.dataset.featuredSoundsHiddenInputId); - - const getSelectedIds = () => (soundSelectorContainer.dataset.selectedIds || "").split(',').filter(Boolean); - const getFeaturedIds = () => ((featuredSoundsInput && featuredSoundsInput.value) || "").split(',').filter(Boolean); - - // Update clear all button based on whether there are any featured sounds - const updateClearAllButtonState = () => { - if (clearAllButton) { - clearAllButton.disabled = getFeaturedIds().length === 0; - } - }; - - // Add change listeners directly to checkboxes (works alongside existing listeners from addSoundsModal) - const updateButtonStates = () => { - // Check directly from checkboxes since dataset.selectedIds may not be updated yet (debounced) - const hasSelection = soundSelectorContainer.querySelector('input.bw-checkbox:checked') !== null; - setButton.disabled = !hasSelection; - removeButton.disabled = !hasSelection; - }; - - soundSelectorContainer.querySelectorAll('input.bw-checkbox').forEach(checkbox => { - checkbox.addEventListener('change', updateButtonStates); - }); - - updateFeaturedHighlights(soundSelectorContainer, getFeaturedIds()); - updateClearAllButtonState(); - - const updateFeatured = (ids) => { - if (featuredSoundsInput) featuredSoundsInput.value = ids.join(','); - updateFeaturedHighlights(soundSelectorContainer, ids); - updateClearAllButtonState(); - }; - - const clearSelection = () => { - soundSelectorContainer.querySelectorAll('input.bw-checkbox:checked').forEach(cb => { - cb.checked = false; - var parent = cb.closest('.bw-selectable-object'); - if (parent) parent.classList.remove('selected'); - }); - updateObjectSelectorDataProperties(soundSelectorContainer); - setButton.disabled = true; - removeButton.disabled = true; - // Dispatch change event so other listeners (e.g. from addSoundsModal) get notified - const firstCheckbox = soundSelectorContainer.querySelector('input.bw-checkbox'); - if (firstCheckbox) { - firstCheckbox.dispatchEvent(new Event('change', { bubbles: true })); - } - }; - - setButton.addEventListener('click', (e) => { - e.preventDefault(); - const merged = [...new Set([...getFeaturedIds(), ...getSelectedIds()])]; - updateFeatured(merged); - clearSelection(); - }); - - removeButton.addEventListener('click', (e) => { - e.preventDefault(); - const selectedIds = new Set(getSelectedIds()); - const newFeatured = selectedIds.size > 0 - ? getFeaturedIds().filter(id => !selectedIds.has(id)) - : []; - updateFeatured(newFeatured); - clearSelection(); +prepareAddMaintainersModalAndFields(document); +prepareAddSoundsModalDynamic( + document, + () => store.ids().join(','), + (selectedIds) => { selectedIds.forEach(id => store.add(id)); refreshSoundsSection(); } +); +initializeObjectSelectorActions(document, store); + +// Serialize hidden inputs just before the form is submitted +const collectionForm = document.getElementById('collection-form'); +if (collectionForm) { + collectionForm.addEventListener('submit', function () { + store.syncInputs(); }); +} - if (clearAllButton) { - clearAllButton.addEventListener('click', (e) => { - e.preventDefault(); - updateFeatured([]); - }); +// Re-initialise sound actions after any htmx swap into #sounds-section +document.body.addEventListener('htmx:afterSettle', function (event) { + if (event.detail.target && event.detail.target.id === 'sounds-section') { + initializeObjectSelectorActions(document, store); } -}; - -prepareAddMaintainersModalAndFields(document); -prepareAddSoundsModalAndFields(document); -prepareFeaturedSoundsButtons(document); \ No newline at end of file +}); diff --git a/freesound/static/bw-frontend/src/utils/data.js b/freesound/static/bw-frontend/src/utils/data.js index 030791d49..2c03828de 100644 --- a/freesound/static/bw-frontend/src/utils/data.js +++ b/freesound/static/bw-frontend/src/utils/data.js @@ -1,26 +1,6 @@ -const serializedIdListToIntList = serializedIdList => { - const outputList = []; - if (serializedIdList !== '' && serializedIdList !== undefined){ - serializedIdList.split(',').forEach(splitPart => { - outputList.push(parseInt(splitPart, 10)); - }); - } - return outputList; -} +const serializedIdListToIntList = value => + value ? value.split(',').map(s => parseInt(s, 10)) : []; -const combineIdsLists = (list1, list2) => { - const outputList = []; - list1.forEach(listItem => { - if (outputList.indexOf(listItem) == -1){ - outputList.push(listItem); - } - }); - list2.forEach(listItem => { - if (outputList.indexOf(listItem) == -1){ - outputList.push(listItem); - } - }); - return outputList; -} +const combineIdsLists = (list1, list2) => [...new Set([...list1, ...list2])]; -export {serializedIdListToIntList, combineIdsLists}; \ No newline at end of file +export { serializedIdListToIntList, combineIdsLists }; \ No newline at end of file diff --git a/freesound/static/bw-frontend/src/utils/soundStateStore.js b/freesound/static/bw-frontend/src/utils/soundStateStore.js new file mode 100644 index 000000000..fce36af6f --- /dev/null +++ b/freesound/static/bw-frontend/src/utils/soundStateStore.js @@ -0,0 +1,150 @@ +import { serializedIdListToIntList } from './data' + +// State flags as bitmask constants +const STATE = Object.freeze({ + ADDED: 1 << 0, // 0001 + REMOVED: 1 << 1, // 0010 + FEATURED: 1 << 2, // 0100 +}); + +class SoundStateStore { + /** + * @param {Object} config + * @param {Element} config.input - hidden input for base object IDs + * @param {Array<{actionName: string, flag: number, input?: Element}>} config.actions + */ + constructor(config) { + this._states = new Map(); // id → bitmask + this._actions = new Map(); // actionName → { flag, input } + this._listeners = []; + this._mainInput = null; + + if (config) this._init(config); + } + + // ─── Init ───────────────────────────────────────────────── + + _init(config) { + this._mainInput = config.input || null; + + if (this._mainInput && this._mainInput.value) { + serializedIdListToIntList(this._mainInput.value).forEach(id => this.track(id)); + } + + (config.actions || []).forEach(({ actionName, flag, input = null }) => { + this._actions.set(actionName, { flag, input }); + if (input && input.value) { + serializedIdListToIntList(input.value).forEach(id => this.setFlag(id, flag)); + } + }); + } + + // ─── Registration ───────────────────────────────────────── + + /** Register an existing object with no flags (silent, no notification). */ + track(id) { + if (!this._states.has(id)) this._states.set(id, 0); + return this; + } + + /** Register a newly added object with the ADDED flag and notify listeners. */ + add(id) { + if (!this._states.has(id)) { + this._states.set(id, STATE.ADDED); + this._notify(id, STATE.ADDED, true); + } + return this; + } + + // ─── State mutation ──────────────────────────────────────── + + setFlag(id, flag, active = true) { + if (!this._states.has(id)) return this; + const mask = this._states.get(id); + this._states.set(id, active ? mask | flag : mask & ~flag); + this._notify(id, flag, active); + return this; + } + + toggleFlag(id, flag) { + if (!this._states.has(id)) return false; + const active = !this.hasFlag(id, flag); + this.setFlag(id, flag, active); + return active; + } + + toggleAction(id, actionName) { + const action = this._actions.get(actionName); + return action ? this.toggleFlag(id, action.flag) : undefined; + } + + // ─── Queries ────────────────────────────────────────────── + + hasFlag(id, flag) { + const mask = this._states.has(id) ? this._states.get(id) : 0; + return (mask & flag) !== 0; + } + + /** All tracked object IDs. Pass excludeRemoved=true to omit REMOVED objects. */ + ids(excludeRemoved = false) { + if (!excludeRemoved) return Array.from(this._states.keys()); + return Array.from(this._states.entries()) + .filter(([, mask]) => (mask & STATE.REMOVED) === 0) + .map(([id]) => id); + } + + /** Count of objects where REMOVED is NOT set. */ + presentCount() { + return this.ids(true).length; + } + + /** Object IDs where the given flag is set. Pass excludeRemoved=true to omit REMOVED objects. */ + idsWithFlag(flag, excludeRemoved = false) { + return Array.from(this._states.entries()) + .filter(([, mask]) => (mask & flag) !== 0 && (!excludeRemoved || (mask & STATE.REMOVED) === 0)) + .map(([id]) => id); + } + + // ─── Listeners ──────────────────────────────────────────── + + onChange(listener) { + this._listeners.push(listener); + return this; + } + + _notify(id, flag, active) { + this._listeners.forEach(fn => fn(id, flag, active)); + } + + // ─── Action registry ────────────────────────────────────── + + /** Returns registered actions as [{ actionName, flag }]. */ + actions() { + return Array.from(this._actions.entries(), ([actionName, { flag }]) => ({ actionName, flag })); + } + + // ─── Serialization ──────────────────────────────────────── + + /** + * Writes current state back to all associated hidden inputs. + * ADDED objects are included even if REMOVED so the server keeps returning them. + */ + syncInputs() { + if (this._mainInput) { + this._mainInput.value = this.ids(true).join(','); + } + + this._actions.forEach(({ flag, input }) => { + if (input) { + // ADDED and REMOVED: report faithfully (server reconciles the overlap) + // Other flags (e.g. FEATURED): exclude sounds that are REMOVED + const ids = (flag === STATE.ADDED || flag === STATE.REMOVED) + ? this.idsWithFlag(flag) + : this.idsWithFlag(flag, true); + input.value = ids.join(','); + } + }); + } +} + +export { STATE, SoundStateStore }; diff --git a/freesound/static/bw-frontend/styles/atoms/inputs.scss b/freesound/static/bw-frontend/styles/atoms/inputs.scss index 0393f98c3..f9bb029fd 100644 --- a/freesound/static/bw-frontend/styles/atoms/inputs.scss +++ b/freesound/static/bw-frontend/styles/atoms/inputs.scss @@ -12,6 +12,14 @@ input { width: 100%; } + + &--no-icon { + padding-left: 16px; + + input[type='search'] { + margin-left: 0; + } + } } .input-icon { diff --git a/freesound/static/bw-frontend/styles/atoms/selectableObject.scss b/freesound/static/bw-frontend/styles/atoms/selectableObject.scss index 7a3fef7d9..45d035a67 100644 --- a/freesound/static/bw-frontend/styles/atoms/selectableObject.scss +++ b/freesound/static/bw-frontend/styles/atoms/selectableObject.scss @@ -1,24 +1,162 @@ .bw-selectable-object { - margin:8px; + margin: 8px; padding: 6px; - border:2px solid rgba(0,0,0,0); + border: 2px solid rgba(0, 0, 0, 0); border-radius: 4px; &.selected { background-color: $search-bg; outline-color: $navy-light-grey; - border:2px solid $navy-light-grey; + border: 2px solid $navy-light-grey; transition: border-color 0.3s linear; } - .featured-label { - position: absolute; - top: 4px; - right: 8px; - font-size: 10px; - font-weight: 600; - color: #f5a623; - text-transform: uppercase; - letter-spacing: 0.5px; + + &.with-actions { + display: flex; + flex-direction: column; + + &.marked-for-removal { + + // Disable sound player and all interactions + >div:first-child { + opacity: 0.35; + pointer-events: none; + user-select: none; + } + + // Disable featured button completely + .featured-toggle { + opacity: 0.35; + pointer-events: none; + cursor: not-allowed; + } + } + } +} + +.bw-object-actions { + display: flex; + width: 100%; + padding: 6px 0 0; + gap: 8px; + + .btn-inverse { + flex: 1; + padding: 8px 12px; + font-size: 12px; + text-align: center; + + .label-default { + display: inline; + } + + .label-hover { + display: none; + } + + &:hover { + .label-default { + display: none; + } + + .label-hover { + display: inline; + } + } + } + + .featured-toggle { + + .label-active-default, + .label-active-hover { + display: none; + } + + &.active { + background: $yellow; + border-color: $yellow; + color: $black; + + .label-default, + .label-hover { + display: none; + } + + .label-active-default { + display: inline; + } + + &:hover { + background: $black; + border-color: $navy-light-grey; + color: $white; + + .label-active-default { + display: none; + } + + .label-active-hover { + display: inline; + } + } + } + } + + .featured-toggle:disabled { + opacity: 0.4; + cursor: not-allowed; + pointer-events: none; + } + + .remove-toggle { + flex: 0 0 auto; + padding: 8px 12px; + min-width: 60px; + + // Force 'trash' to stay visible on hover + &:hover { + background: $red; + border-color: $red; + color: $white; + + .label-default { + display: inline; + } + + .label-hover { + display: none; + } + } + + &.active { + background: transparent; + border-color: $navy-light-grey; + color: $navy-grey; + + // Force 'Undo' text + .label-default { + display: none !important; + } + + .label-hover { + display: inline !important; + } + + // Keep 'Undo' on hover + &:hover { + background: $almost-white; + border-color: $navy-grey; + color: $black; + + .label-default { + display: none !important; + } + + .label-hover { + display: inline !important; + } + } + } } } \ No newline at end of file diff --git a/freesound/static/bw-frontend/styles/atoms/selects.scss b/freesound/static/bw-frontend/styles/atoms/selects.scss index c84593b4b..ac704abd1 100644 --- a/freesound/static/bw-frontend/styles/atoms/selects.scss +++ b/freesound/static/bw-frontend/styles/atoms/selects.scss @@ -19,6 +19,7 @@ cursor: pointer; min-width: 115px; text-align: left; + white-space: nowrap; &:focus { outline: none; diff --git a/freesound/static/bw-frontend/styles/molecules/forms.scss b/freesound/static/bw-frontend/styles/molecules/forms.scss index 9eff2cae1..e16c2e8c5 100644 --- a/freesound/static/bw-frontend/styles/molecules/forms.scss +++ b/freesound/static/bw-frontend/styles/molecules/forms.scss @@ -37,6 +37,20 @@ } } } + + .input-wrapper input[type='search'] { + margin-top: 0; + margin-bottom: 0; + border: 0; + background: none; + padding: 0; + border-radius: 0; + + &:focus { + border: 0; + background: none; + } + } textarea { min-height: 150px; diff --git a/fscollections/forms.py b/fscollections/forms.py index c09e74625..6bcd13601 100644 --- a/fscollections/forms.py +++ b/fscollections/forms.py @@ -197,9 +197,13 @@ def clean(self): class CollectionEditForm(forms.ModelForm): - collection_sounds = forms.CharField( - min_length=1, - widget=forms.widgets.HiddenInput(attrs={"id": "collection_sounds", "name": "collection_sounds"}), + added_sounds = forms.CharField( + widget=forms.widgets.HiddenInput(attrs={"id": "added_sound_ids"}), + required=False, + ) + + removed_sounds = forms.CharField( + widget=forms.widgets.HiddenInput(attrs={"id": "removed_sound_ids"}), required=False, ) @@ -218,7 +222,7 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.fields["public"].label = "Visibility" - self.fields["collection_sounds"].help_text = ( + self.fields["added_sounds"].help_text = ( f"You have reached the maximum number of sounds available for a collection ({settings.MAX_SOUNDS_PER_COLLECTION}). " "In order to add new sounds, first remove some of the current ones." ) @@ -232,6 +236,22 @@ def __init__(self, *args, **kwargs): for field in self.fields: self.fields[field].disabled = True + @staticmethod + def _parse_id_set(value): + return {int(i) for i in value.replace(" ", "").split(",") if i.isdigit()} if value else set() + + def clean_added_sounds(self): + return self._parse_id_set(self.cleaned_data.get("added_sounds", "")) + + def clean_removed_sounds(self): + return self._parse_id_set(self.cleaned_data.get("removed_sounds", "")) + + def clean_maintainers(self): + return self._parse_id_set(self.cleaned_data.get("maintainers", "")) + + def clean_featured_sounds(self): + return list(self._parse_id_set(self.cleaned_data.get("featured_sounds", ""))) + def clean(self): cleaned_data = super().clean() if not self.is_owner and not self.is_maintainer: @@ -249,78 +269,53 @@ def clean(self): "This collection name is reserved for your personal default collection. Please choose another one." ), ) - collection_sounds = self.cleaned_data.get("collection_sounds").split(",") - if len(collection_sounds) > settings.MAX_SOUNDS_PER_COLLECTION: + added = cleaned_data.get("added_sounds", set()) + removed = cleaned_data.get("removed_sounds", set()) + # A sound that was added and then removed in the same session cancels out + cancelled = added & removed + cleaned_data["added_sounds"] = added = added - cancelled + cleaned_data["removed_sounds"] = removed = removed - cancelled + net_count = self.instance.num_sounds + len(added) - len(removed) + if net_count > settings.MAX_SOUNDS_PER_COLLECTION: self.add_error( - "collection_sounds", + "added_sounds", forms.ValidationError( f"You have exceeded the maximum number of sounds for a collection ({settings.MAX_SOUNDS_PER_COLLECTION})." ), ) - cleaned_data["collection_sounds"] = collection_sounds[: self.instance.num_sounds] return cleaned_data - def clean_ids_field(self, field): - # this function cleans the sounds and maintainers fields which store temporary changes on the edit URL - new_field = re.sub("[^0-9,]", "", self.cleaned_data[field]) - new_field = re.sub(",+", ",", new_field) - new_field = re.sub("^,+", "", new_field) - new_field = re.sub(",+$", "", new_field) - if len(new_field) > 0: - new_field = {int(usr) for usr in new_field.split(",")} - else: - new_field = set() - return new_field - def save(self, user_adding_sound): - """This method is used to apply the temporary changes from the edit URL to the DB. - Useful for maintainers and sounds, where several objects are added to the Collection attrs. - This way, the server side does not change until the Save Collection button is clicked. + """Apply the pending delta (added/removed sounds and featured list) to the DB. Args: user_adding_sound (User): the user modifying the collection Returns: - collection (Collection): the saved collection with proper modficiations + collection (Collection): the saved collection """ collection = super().save(commit=False) - new_sounds = self.clean_ids_field("collection_sounds") - current_sounds = list(Sound.objects.filter(collections=collection).values_list("id", flat=True)) - for snd in new_sounds: - if snd not in current_sounds: - sound = Sound.objects.get(id=snd) - CollectionSound.objects.create(user=user_adding_sound, sound=sound, collection=collection, status="OK") + sounds_to_add = self.cleaned_data["added_sounds"] + sounds_to_remove = self.cleaned_data["removed_sounds"] + + if sounds_to_add: + CollectionSound.objects.bulk_create( + [CollectionSound(user=user_adding_sound, sound_id=snd, collection=collection, status="OK") + for snd in sounds_to_add], + ignore_conflicts=True, + ) - else: - current_sounds.remove(snd) - - sounds = Sound.objects.filter(id__in=current_sounds) - CollectionSound.objects.filter(collection=collection, sound__in=sounds).delete() - - new_maintainers = set(self.clean_ids_field("maintainers")) - # if the owner of the collection has been added as a maintainer, discard it - if collection.user.id in new_maintainers: - new_maintainers.remove(collection.user.id) - current_maintainers = list(self.instance.maintainers.values_list("id", flat=True)) - for usr in new_maintainers: - if usr not in current_maintainers: - maintainer = User.objects.get(id=usr) - collection.maintainers.add(maintainer) - else: - current_maintainers.remove(usr) - for usr in current_maintainers: - maintainer = User.objects.get(id=usr) - collection.maintainers.remove(maintainer) - - # Handle featured sounds - featured_sounds_str = self.cleaned_data.get("featured_sounds", "") - if featured_sounds_str: - featured_ids = [int(sid) for sid in featured_sounds_str.split(",") if sid.strip()] - # Only keep featured IDs that are still in the collection - featured_ids = [sid for sid in featured_ids if sid in new_sounds] - collection.featured_sound_ids = featured_ids - else: - collection.featured_sound_ids = [] + if sounds_to_remove: + CollectionSound.objects.filter(collection=collection, sound_id__in=sounds_to_remove).delete() + + new_maintainers = self.cleaned_data["maintainers"] + new_maintainers.discard(collection.user.id) + collection.maintainers.set(new_maintainers) + + # Filter featured IDs to only sounds currently in the collection + featured_ids = self.cleaned_data["featured_sounds"] + final_sound_ids = set(Sound.objects.filter(collections=collection).values_list("id", flat=True)) + collection.featured_sound_ids = [sid for sid in featured_ids if sid in final_sound_ids] collection.save() return collection @@ -337,12 +332,12 @@ class Meta: class CollectionEditFormAsMaintainer(CollectionEditForm): class Meta(CollectionEditForm.Meta): - fields = CollectionEditForm.Meta.fields + ["collection_sounds"] + ["maintainers"] + fields = CollectionEditForm.Meta.fields + ["added_sounds", "removed_sounds"] + ["maintainers"] def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) for field in self.fields: - if field != "collection_sounds": + if field not in ("added_sounds", "removed_sounds"): self.fields[field].disabled = True def clean(self): @@ -352,10 +347,9 @@ def clean(self): # and if any change in these is found, an error is raised. To prevent changes in "maintainers" even though it is included # in the form but disabled (to allow the user to view but not to modify the field), the original collection maintainers # are retrieved from DB to ensure no changes are applied to this attribute. - collection_maintainers = list(self.instance.maintainers.values_list("id", flat=True)) - cleaned_data["maintainers"] = (",").join(str(x) for x in collection_maintainers) + cleaned_data["maintainers"] = set(self.instance.maintainers.values_list("id", flat=True)) # Preserve featured_sounds from the original instance (maintainers cannot edit) - cleaned_data["featured_sounds"] = ",".join(str(sid) for sid in self.instance.featured_sound_ids) + cleaned_data["featured_sounds"] = list(self.instance.featured_sound_ids) for field in CollectionEditForm.Meta.fields: if cleaned_data[field] != getattr(self.instance, field): self.add_error(field, forms.ValidationError("You don't have permissions to edit this field")) diff --git a/fscollections/migrations/0003_remove_collectionsound_featured_sound_and_more.py b/fscollections/migrations/0003_remove_collectionsound_featured_sound_and_more.py new file mode 100644 index 000000000..28d43984f --- /dev/null +++ b/fscollections/migrations/0003_remove_collectionsound_featured_sound_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 4.2.19 on 2026-01-21 17:34 + +import django.contrib.postgres.fields +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('fscollections', '0002_alter_collectionsound_unique_together'), + ] + + operations = [ + migrations.RemoveField( + model_name='collectionsound', + name='featured_sound', + ), + migrations.AddField( + model_name='collection', + name='featured_sound_ids', + field=django.contrib.postgres.fields.ArrayField(base_field=models.IntegerField(), blank=True, default=list, size=250), + ), + ] diff --git a/fscollections/models.py b/fscollections/models.py index d4530d9f4..12adae32e 100644 --- a/fscollections/models.py +++ b/fscollections/models.py @@ -23,7 +23,7 @@ from django.contrib.auth.models import User from django.contrib.postgres.fields import ArrayField from django.db import models -from django.db.models import Case, F, IntegerField, Sum, Value, When +from django.db.models import F, Sum from django.db.models.functions import Greatest from django.db.models.signals import m2m_changed, post_delete, post_save, pre_save from django.dispatch import receiver @@ -92,60 +92,6 @@ def save(self, *args, **kwargs): super().save(*args, **kwargs) - def get_sounds( - self, - sort_by=None, - limit=None, - include_audio_descriptors=False, - include_similarity_vectors=False, - include_remix_subqueries=False, - ): - """Get sounds for this collection with sorting. - - Args: - sort_by: Sort option key from settings.COLLECTION_SORT_OPTIONS. - Defaults to settings.COLLECTION_SORT_DEFAULT. - limit: Optional limit on number of sounds returned - include_audio_descriptors: Include audio descriptor data - include_similarity_vectors: Include similarity vector data - include_remix_subqueries: Include remix relationship data - - Returns: - Sorted QuerySet of Sound objects - """ - # Validate sort option, defaulting to COLLECTION_SORT_DEFAULT if invalid or None - if sort_by not in settings.COLLECTION_SORT_OPTIONS: - sort_by = settings.COLLECTION_SORT_DEFAULT - - qs = Sound.objects.bulk_sounds_for_collection( - collection_id=self.id, - include_audio_descriptors=include_audio_descriptors, - include_similarity_vectors=include_similarity_vectors, - include_remix_subqueries=include_remix_subqueries, - ) - - # Get the sort field from settings (value is the sort field directly) - sort_field = settings.COLLECTION_SORT_OPTIONS[sort_by] - - # Apply sorting based on sort_by option (featured is when sort_by matches the default) - if sort_field == "featured_order": - # Featured sorting - direct access to self.featured_sound_ids - if self.featured_sound_ids: - ordering = Case( - *[When(id=sound_id, then=Value(i)) for i, sound_id in enumerate(self.featured_sound_ids)], - default=Value(len(self.featured_sound_ids)), - output_field=IntegerField() - ) - qs = qs.annotate(featured_order=ordering).order_by(sort_field) - # If no featured sounds, keep default queryset order - elif sort_field: - # Use the sort field from settings for other sort options - qs = qs.order_by(sort_field) - - if limit: - qs = qs[:limit] - return qs - class CollectionSound(models.Model): # this model relates collections and sounds @@ -178,7 +124,7 @@ def update_collection_num_sounds_bulk_changes(sender, instance, **kwargs): Collection.objects.filter(collectionsound=instance).update(num_sounds=Greatest(F("num_sounds") - 1, 0)) -@receiver([post_save, post_delete], sender=CollectionSound) +@receiver(post_delete, sender=CollectionSound) def remove_not_valid_featured_sounds(sender, instance, **kwargs): """Remove featured_sound_ids that are no longer part of the collection.""" if instance and instance.collection_id: @@ -194,21 +140,16 @@ def remove_not_valid_featured_sounds(sender, instance, **kwargs): Collection.objects.filter(id=collection.id).update(featured_sound_ids=valid_featured_ids) -@receiver([post_save, post_delete], sender=CollectionSound) -def mark_sound_dirty_on_collection_change(sender, instance, **kwargs): +@receiver(post_save, sender=CollectionSound) +def mark_sound_dirty_on_collection_add(sender, instance, **kwargs): if instance: - print(f"----------coll change: marking {instance.sound} dirty!!!!!!!!!!!!") Sound.objects.filter(id=instance.sound_id).update(is_index_dirty=True) -@receiver(pre_save, sender=Collection) -def mark_sounds_dirty_on_public_change(sender, instance, **kwargs): - if instance and instance.pk: - old_instance = Collection.objects.get(pk=instance.pk) - if old_instance.public != instance.public: - print("----------public changed !!!!!!!!!!!!") - # Update all sounds in this collection - instance.sounds.update(is_index_dirty=True) +@receiver(post_delete, sender=CollectionSound) +def mark_sound_dirty_on_collection_remove(sender, instance, **kwargs): + if instance: + Sound.objects.filter(id=instance.sound_id).update(is_index_dirty=True) class CollectionDownload(models.Model): diff --git a/fscollections/templatetags/display_collections.py b/fscollections/templatetags/display_collections.py index 125c539ee..5f1bd314b 100644 --- a/fscollections/templatetags/display_collections.py +++ b/fscollections/templatetags/display_collections.py @@ -33,15 +33,10 @@ def display_collection(context, collection_id): collection = get_object_or_404(Collection, id=collection_id) request = context.get("request") if collection.featured_sound_ids: - featured_sounds = collection.get_sounds(sort_by="Featured first", limit=len(collection.featured_sound_ids)) + header_sounds = Sound.public.filter(id__in=collection.featured_sound_ids) else: - featured_sounds = collection.get_sounds(sort_by="Date added (newest first)", limit=1) + header_sounds = Sound.public.filter(collections=collection)[:1] - tvars = {"collection": collection, "featured_sounds": featured_sounds, "request": request} + tvars = {"collection": collection, "header_sounds": header_sounds, "request": request} return tvars - -@register.inclusion_tag("collections/display_featured_sounds.html", takes_context=True) -def display_featured_sounds(context, sounds): - return {"featured_sounds": sounds, "request": context.get("request")} - diff --git a/fscollections/tests.py b/fscollections/tests.py index ab3010beb..8c32fca53 100644 --- a/fscollections/tests.py +++ b/fscollections/tests.py @@ -3,7 +3,6 @@ from django.contrib.messages import get_messages from django.test import TestCase from django.urls import reverse -from django.utils.text import slugify from fscollections.models import Collection, CollectionSound from utils.test_helpers import create_user_and_sounds @@ -24,7 +23,6 @@ def setUp(self): self.sound2 = sounds[2] self.collection = Collection.objects.create(user=self.user, name="testcollection") print(self.collection.id) - print(slugify(self.collection.name)) def test_collections_create_and_delete(self): # User not logged in - redirects to login page @@ -40,7 +38,7 @@ def test_collections_create_and_delete(self): # User logged in, check given collection id works # #region agent log - generated_url = reverse("collection", args=[slugify(self.collection.name), self.collection.id]) + generated_url = reverse("collection", args=[self.collection.id]) resp = self.client.get(generated_url) self.assertEqual(resp.status_code, 200) @@ -50,12 +48,12 @@ def test_collections_create_and_delete(self): delete_collection = Collection.objects.get(name="tbdcollection") # Delete collection view - resp = self.client.post(reverse("delete-collection", args=[slugify(delete_collection.name), delete_collection.id])) + resp = self.client.post(reverse("delete-collection", args=[delete_collection.id])) self.assertEqual(resp.status_code, 302) self.assertEqual("/collections/", resp.url) # Test collection URL for collection.id does not exist - resp = self.client.get(reverse("collection", args=[slugify(delete_collection.name), delete_collection.id])) + resp = self.client.get(reverse("collection", args=[delete_collection.id])) self.assertEqual(resp.status_code, 404) def test_add_remove_sounds_as_user(self): @@ -86,7 +84,7 @@ def test_add_remove_sounds_as_user(self): resp = self.client.post(reverse("add-sound-to-collection", args=[self.sound.id]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) default_collection = Collection.objects.get(user=self.user, name="My bookmarks", is_default_collection=True) - resp = self.client.get(reverse("collection", args=[slugify(default_collection.name), default_collection.id])) + resp = self.client.get(reverse("collection", args=[default_collection.id])) self.assertEqual(200, resp.status_code) # Test creating a new custom collection using the add_sound_to_collection modal @@ -97,7 +95,7 @@ def test_add_remove_sounds_as_user(self): self.assertEqual(1, new_collection.num_sounds) # Test download collection - resp = self.client.get(reverse("download-collection", args=[slugify(self.collection.name), self.collection.id])) + resp = self.client.get(reverse("download-collection", args=[self.collection.id])) self.assertEqual(resp.status_code, 200) def test_add_remove_sounds_as_maintainer(self): @@ -136,7 +134,7 @@ def test_add_remove_sounds_as_maintainer(self): self.assertEqual(0, maintainer_test_collection.num_sounds) # Test deleting a collection where request user is maintainer (not valid) - resp = self.client.post(reverse("delete-collection", args=[slugify(self.collection.name), self.collection.id])) + resp = self.client.post(reverse("delete-collection", args=[self.collection.id])) messages = list(get_messages(resp.wsgi_request)) self.assertTrue(len(messages) > 0) self.assertEqual( @@ -144,7 +142,7 @@ def test_add_remove_sounds_as_maintainer(self): "You're not allowed to delete this collection.In order to delete a collection you must be the owner.", ) self.assertEqual(resp.status_code, 302) - self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}/", resp.url) def test_add_remove_sounds_as_external_user(self): self.client.force_login(self.external_user) @@ -161,17 +159,17 @@ def test_add_remove_sounds_as_external_user(self): # Additionally, test edit URL for external user # #region agent log - edit_url = reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + edit_url = reverse("edit-collection", args=[self.collection.id]) resp = self.client.get(edit_url) - expected_redirect = f"/collections/{slugify(self.collection.name)}-{self.collection.id}/" + expected_redirect = f"/collections/{self.collection.id}/" self.assertEqual(302, resp.status_code) self.assertEqual(expected_redirect, resp.url) # Test deleting a collection as an external user (not owner -> not valid) - resp = self.client.post(reverse("delete-collection", args=[slugify(self.collection.name), self.collection.id])) + resp = self.client.post(reverse("delete-collection", args=[self.collection.id])) self.assertEqual(resp.status_code, 302) - self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}/", resp.url) def test_edit_collection_as_user(self): # Test the edit collection form submission for different case scenarios @@ -180,14 +178,14 @@ def test_edit_collection_as_user(self): # Test setting an already existing name for a collection (form should be invalid therefore, the name should not change) form_data = {"name": "testcollection", "description": "", "public": False} - resp = self.client.post(reverse("edit-collection", args=[slugify(edit_collection.name), edit_collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[edit_collection.id]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) edit_collection.refresh_from_db() self.assertEqual("edited-collection", edit_collection.name) # Test setting 'My bookmarks' as the name for a collection (form should be invalid therefore, the name should not change) form_data = {"name": "My bookmarks", "description": "", "public": False} - resp = self.client.post(reverse("edit-collection", args=[slugify(edit_collection.name), edit_collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[edit_collection.id]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) edit_collection.refresh_from_db() self.assertEqual("edited-collection", edit_collection.name) @@ -195,9 +193,9 @@ def test_edit_collection_as_user(self): # Test creating the default collection and trying to change its name and public parameters (both are static) default_collection = Collection.objects.create(name="My bookmarks", user=self.user, is_default_collection=True) form_data = {"name": "other-name", "description": "", "public": True} - resp = self.client.post(reverse("edit-collection", args=[slugify(default_collection.name), default_collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[default_collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{slugify(default_collection.name)}-{default_collection.id}/", resp.url) + self.assertEqual(f"/collections/{default_collection.id}/", resp.url) default_collection.refresh_from_db() self.assertEqual("My bookmarks", default_collection.name) self.assertEqual("", default_collection.description) @@ -211,9 +209,9 @@ def test_edit_collection_as_user(self): "public": False, "maintainers": f"{self.maintainer.id},{self.external_user.id},{self.maintainer.id}", } - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(2, self.collection.maintainers.all().count()) # Remove maintainer @@ -223,9 +221,9 @@ def test_edit_collection_as_user(self): "public": False, "maintainers": f"{self.maintainer.id}", } - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(1, self.collection.maintainers.all().count()) @@ -235,37 +233,39 @@ def test_edit_collection_as_user(self): "name": "testcollection", "description": "", "public": False, - "collection_sounds": f"{self.sound.id},{self.sound1.id},{self.sound.id}", + "added_sounds": f"{self.sound.id},{self.sound1.id},{self.sound.id}", + "removed_sounds": "", } - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(2, self.collection.num_sounds) - # Remove sound + # Remove sound1 form_data = { "name": "testcollection", "description": "", "public": False, - "collection_sounds": f"{self.sound.id}", + "added_sounds": "", + "removed_sounds": f"{self.sound1.id}", } - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(1, self.collection.num_sounds) # Test adding more sounds than permitted - ___, ___, added_sounds = create_user_and_sounds( + ___, ___, extra_sounds = create_user_and_sounds( num_sounds=settings.MAX_SOUNDS_PER_COLLECTION + 1, count_offset=3, user=self.user, processing_state="OK", moderation_state="OK", ) - sounds_ids = ",".join([str(s.id) for s in added_sounds]) - form_data = {"name": "testcollection", "description": "", "public": False, "collection_sounds": sounds_ids} - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + sounds_ids = ",".join([str(s.id) for s in extra_sounds]) + form_data = {"name": "testcollection", "description": "", "public": False, "added_sounds": sounds_ids, "removed_sounds": ""} + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(200, resp.status_code) self.collection.refresh_from_db() self.assertEqual(1, self.collection.num_sounds) @@ -282,17 +282,19 @@ def test_edit_collection_as_maintainer(self): "name": "testcollection", "description": "", "public": False, - "collection_sounds": str(self.sound.id), + "added_sounds": str(self.sound.id), + "removed_sounds": "", } - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(1, self.collection.num_sounds) - form_data.pop("collection_sounds") - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + form_data["added_sounds"] = "" + form_data["removed_sounds"] = str(self.sound.id) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual(0, self.collection.num_sounds) @@ -303,9 +305,9 @@ def test_edit_collection_as_maintainer(self): "public": True, "maintainers": str(self.external_user.id), } - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) - self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}/", resp.url) self.collection.refresh_from_db() self.assertEqual("testcollection", self.collection.name) self.assertEqual("", self.collection.description) @@ -313,9 +315,9 @@ def test_edit_collection_as_maintainer(self): self.assertEqual(1, self.collection.maintainers.all().count()) # Test collection deletion as maintainer - resp = self.client.get(reverse("delete-collection", args=[slugify(self.collection.name), self.collection.id])) + resp = self.client.get(reverse("delete-collection", args=[self.collection.id])) self.assertEqual(resp.status_code, 302) - self.assertEqual(f"/collections/{slugify(self.collection.name)}-{self.collection.id}/", resp.url) + self.assertEqual(f"/collections/{self.collection.id}/", resp.url) self.assertTrue(Collection.objects.filter(id=self.collection.id).exists()) def test_edit_featured_sounds_as_user(self): @@ -327,39 +329,44 @@ def test_edit_featured_sounds_as_user(self): "name": "testcollection", "description": "", "public": False, - "collection_sounds": f"{self.sound.id},{self.sound1.id},{self.sound2.id}", + "added_sounds": f"{self.sound.id},{self.sound1.id},{self.sound2.id}", + "removed_sounds": "", "featured_sounds": "", } - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) self.collection.refresh_from_db() self.assertEqual(3, self.collection.num_sounds) self.assertEqual([], self.collection.featured_sound_ids) + # Subsequent posts only change featured_sounds — no sound additions/removals + form_data["added_sounds"] = "" + form_data["removed_sounds"] = "" + # Add featured sounds form_data["featured_sounds"] = f"{self.sound.id},{self.sound1.id}" - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) self.collection.refresh_from_db() self.assertEqual([self.sound.id, self.sound1.id], self.collection.featured_sound_ids) # Reorder featured sounds form_data["featured_sounds"] = f"{self.sound1.id},{self.sound.id}" - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) self.collection.refresh_from_db() self.assertEqual([self.sound1.id, self.sound.id], self.collection.featured_sound_ids) # Remove a featured sound form_data["featured_sounds"] = f"{self.sound.id}" - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) self.collection.refresh_from_db() self.assertEqual([self.sound.id], self.collection.featured_sound_ids) # Clear all featured sounds form_data["featured_sounds"] = "" - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) self.collection.refresh_from_db() self.assertEqual([], self.collection.featured_sound_ids) @@ -382,10 +389,11 @@ def test_edit_featured_sounds_as_maintainer(self): "name": "testcollection", "description": "", "public": False, - "collection_sounds": f"{self.sound.id},{self.sound1.id}", + "added_sounds": "", + "removed_sounds": "", "featured_sounds": f"{self.sound1.id}", # Try to change featured sounds } - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) self.collection.refresh_from_db() # Featured sounds should remain unchanged (original value preserved) @@ -406,10 +414,11 @@ def test_add_sounds_outside_collection_to_featured(self): "name": "testcollection", "description": "", "public": False, - "collection_sounds": f"{self.sound.id},{self.sound1.id}", + "added_sounds": f"{self.sound.id},{self.sound1.id}", + "removed_sounds": "", "featured_sounds": f"{self.sound.id},{sound_not_in_collection.id}", } - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) self.collection.refresh_from_db() # Only sound.id should be in featured (sound_not_in_collection should be filtered out) @@ -424,27 +433,29 @@ def test_remove_sounds_that_are_featured(self): "name": "testcollection", "description": "", "public": False, - "collection_sounds": f"{self.sound.id},{self.sound1.id},{self.sound2.id}", + "added_sounds": f"{self.sound.id},{self.sound1.id},{self.sound2.id}", + "removed_sounds": "", "featured_sounds": f"{self.sound.id},{self.sound1.id},{self.sound2.id}", } - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) self.collection.refresh_from_db() self.assertEqual([self.sound.id, self.sound1.id, self.sound2.id], self.collection.featured_sound_ids) # Remove a sound that is featured - form_data["collection_sounds"] = f"{self.sound.id},{self.sound2.id}" + form_data["added_sounds"] = "" + form_data["removed_sounds"] = str(self.sound1.id) form_data["featured_sounds"] = f"{self.sound.id},{self.sound1.id},{self.sound2.id}" # Still has sound1 in featured - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) self.collection.refresh_from_db() # sound1 should be removed from featured_sound_ids since it's no longer in the collection self.assertEqual([self.sound.id, self.sound2.id], self.collection.featured_sound_ids) # Remove all sounds - form_data["collection_sounds"] = "" + form_data["removed_sounds"] = f"{self.sound.id},{self.sound2.id}" form_data["featured_sounds"] = f"{self.sound.id},{self.sound2.id}" - resp = self.client.post(reverse("edit-collection", args=[slugify(self.collection.name), self.collection.id]) + "?ajax=1", form_data) + resp = self.client.post(reverse("edit-collection", args=[self.collection.id]) + "?ajax=1", form_data) self.assertEqual(302, resp.status_code) self.collection.refresh_from_db() self.assertEqual([], self.collection.featured_sound_ids) diff --git a/fscollections/urls.py b/fscollections/urls.py index 763176f34..664f3486d 100644 --- a/fscollections/urls.py +++ b/fscollections/urls.py @@ -4,18 +4,18 @@ urlpatterns = [ path("", views.collections_for_user, name="your-collections"), - path("-/", views.collection, name="collection"), + path("/", views.collection, name="collection"), path("/add/", views.add_sound_to_collection, name="add-sound-to-collection"), path("create/", views.create_collection, name="create-collection"), - path("-/edit", views.edit_collection, name="edit-collection"), - path("-/delete", views.delete_collection, name="delete-collection"), - path("-/download/", views.download_collection, name="download-collection"), - path("-/licenses/", views.collection_licenses, name="collection-licenses"), + path("/edit", views.edit_collection, name="edit-collection"), + path("/delete", views.delete_collection, name="delete-collection"), + path("/download/", views.download_collection, name="download-collection"), + path("/licenses/", views.collection_licenses, name="collection-licenses"), path( - "-/addsoundsmodal", + "/addsoundsmodal", views.add_sounds_modal_for_collection_edit, name="add-sounds-modal-collection", ), - path("-/addmaintainersmodal", views.add_maintainer_modal, name="add-maintainers-modal"), - path("-/section/stats", views.collection_stats_section, name="collection-stats-section"), + path("/addmaintainersmodal", views.add_maintainer_modal, name="add-maintainers-modal"), + path("/section/stats", views.collection_stats_section, name="collection-stats-section"), ] diff --git a/fscollections/views.py b/fscollections/views.py index 4d6f42e22..338ba3b75 100644 --- a/fscollections/views.py +++ b/fscollections/views.py @@ -22,12 +22,13 @@ from django.contrib import messages from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User -from django.db.models import Q +from django.db.models import CharField, Q +from django.db.models.functions import Cast from django.http import HttpResponse, HttpResponseRedirect, JsonResponse from django.shortcuts import get_object_or_404, render from django.urls import reverse -from django.utils.text import slugify +from freesound import settings from fscollections.forms import ( CollectionEditForm, CollectionEditFormAsMaintainer, @@ -35,7 +36,6 @@ MaintainerForm, SelectCollectionOrNewCollectionForm, ) -from freesound import settings from fscollections.models import Collection, CollectionDownload, CollectionDownloadSound, CollectionSound from sounds.models import Sound from sounds.views import add_sounds_modal_helper @@ -44,7 +44,7 @@ @login_required -def collection(request, collection_name, collection_id): +def collection(request, collection_id): user = request.user is_owner = False is_maintainer = False @@ -57,20 +57,17 @@ def collection(request, collection_name, collection_id): sort_by = request.GET.get("s", settings.COLLECTION_SORT_DEFAULT) search_query = request.GET.get("q", "").strip() - collection_sounds = collection.get_sounds(sort_by=sort_by) - + collection_sounds = Sound.objects.bulk_sounds_for_collection( + collection_id=collection.id, sort_by=sort_by, featured_sound_ids=collection.featured_sound_ids + ) + # Filter sounds by search query if provided if search_query: - collection_sounds = collection_sounds.filter( - Q(original_filename__icontains=search_query) - ).distinct() - + collection_sounds = collection_sounds.filter(Q(original_filename__icontains=search_query)).distinct() + paginator = paginate(request, collection_sounds, settings.BOOKMARKS_PER_PAGE) page_sounds = Sound.objects.ordered_ids([sound.id for sound in paginator["page"].object_list]) - # Get featured sounds on the current page for grouped display (only when sorting by featured) - featured_sounds_on_page = [s for s in page_sounds if s.id in collection.featured_sound_ids] if settings.COLLECTION_SORT_OPTIONS[sort_by] == "featured_order" else [] - tvars = { "collection": collection, "is_owner": is_owner, @@ -78,7 +75,6 @@ def collection(request, collection_name, collection_id): "maintainers": maintainers, "sort_by": sort_by, "sort_options": settings.COLLECTION_SORT_OPTIONS, - "featured_sounds_on_page": featured_sounds_on_page, "search_query": search_query, } tvars.update(paginator) @@ -99,7 +95,7 @@ def collections_for_user(request): @login_required -def collection_stats_section(request, collection_name, collection_id): +def collection_stats_section(request, collection_id): # TODO: this tries to imitate the pack_stats_section behaviour despite a lack of comprehension # on cache behaviour, so the stats shown by this are not properly updated if not request.GET.get("ajax"): @@ -175,7 +171,7 @@ def create_collection(request): @login_required -def delete_collection(request, collection_name, collection_id): +def delete_collection(request, collection_id): collection = get_object_or_404(Collection, id=collection_id) if request.method == "POST" and request.user == collection.user: @@ -189,105 +185,114 @@ def delete_collection(request, collection_name, collection_id): messages.INFO, "You're not allowed to delete this collection.In order to delete a collection you must be the owner.", ) - return HttpResponseRedirect(reverse("collection", args=[slugify(collection.name), collection.id])) + return HttpResponseRedirect(reverse("collection", args=[collection.id])) @login_required -def edit_collection(request, collection_name, collection_id): +def edit_collection(request, collection_id): collection = get_object_or_404(Collection, id=collection_id) - collection_sounds = ",".join([str(s.id) for s in Sound.objects.filter(collections=collection)]) + initial_sound_ids = ",".join(str(s) for s in Sound.objects.filter(collections=collection).values_list("id", flat=True)) maintainers_query = User.objects.filter(collection_maintainer=collection.id) - collection_maintainers = ",".join([str(u.id) for u in maintainers_query]) - is_owner = False - is_maintainer = False - if request.user == collection.user: - is_owner = True - elif request.user in maintainers_query: - is_maintainer = True - else: - return HttpResponseRedirect(reverse("collection", args=[slugify(collection.name), collection_id])) + collection_maintainers = ",".join(str(u) for u in maintainers_query.values_list("id", flat=True)) + is_owner = request.user == collection.user + is_maintainer = not is_owner and maintainers_query.filter(id=request.user.id).exists() + if not is_owner and not is_maintainer: + return HttpResponseRedirect(reverse("collection", args=[collection_id])) - current_sounds = list() - if request.method == "POST": - if is_owner: - form = CollectionEditForm( - request.POST, instance=collection, label_suffix="", is_owner=is_owner, is_maintainer=is_maintainer - ) - elif is_maintainer: - form = CollectionEditFormAsMaintainer( - request.POST, instance=collection, label_suffix="", is_owner=is_owner, is_maintainer=is_maintainer - ) - else: - return HttpResponseRedirect(reverse("collection", args=[slugify(collection.name), collection_id])) + FormClass = CollectionEditForm if is_owner else CollectionEditFormAsMaintainer + if request.method == "POST": + form = FormClass( + request.POST, instance=collection, label_suffix="", is_owner=is_owner, is_maintainer=is_maintainer + ) if form.is_valid(): form.save(user_adding_sound=request.user) - return HttpResponseRedirect(reverse("collection", args=[slugify(collection.name), collection.id])) + return HttpResponseRedirect(reverse("collection", args=[collection.id])) else: - # NOTE: in this form's validation, errors are raised for each speific field, so when there is a submission attempt the error + # NOTE: in this form's validation, errors are raised for each specific field, so when there is a submission attempt the error # is displayed within it. However, fields containing errors are removed from the clean data but we are still interested in # preserving its value. Therefore, we re-initialize a form according to the user's permissions preserving the field's validated data if so, # and in case of error, we take its value from the POST request. The error messages are then attached to the form so that they're displayed. errors_data = form.errors - new_form_data = dict() - for field in form.fields: - try: - new_form_data.setdefault(field, form.cleaned_data[field]) - except KeyError: - new_form_data.setdefault( - field, - request.POST.get( - field, - ), - ) - if is_owner: - form = CollectionEditForm( - initial=new_form_data, label_suffix="", is_owner=is_owner, is_maintainer=is_maintainer - ) - elif is_maintainer: - form = CollectionEditFormAsMaintainer( - initial=new_form_data, label_suffix="", is_owner=is_owner, is_maintainer=is_maintainer - ) + new_form_data = { + field: form.cleaned_data.get(field, request.POST.get(field)) + for field in form.fields + } + form = FormClass(initial=new_form_data, label_suffix="", is_owner=is_owner, is_maintainer=is_maintainer) form._errors = errors_data else: - featured_sounds_str = ",".join([str(sid) for sid in collection.featured_sound_ids]) - if is_owner: - form = CollectionEditForm( - instance=collection, - initial=dict(collection_sounds=collection_sounds, maintainers=collection_maintainers, featured_sounds=featured_sounds_str), - label_suffix="", - is_owner=is_owner, - is_maintainer=is_maintainer, - ) - elif is_maintainer: - form = CollectionEditFormAsMaintainer( - instance=collection, - initial=dict(collection_sounds=collection_sounds, maintainers=collection_maintainers, featured_sounds=featured_sounds_str), - label_suffix="", - is_owner=is_owner, - is_maintainer=is_maintainer, - ) + featured_sounds_str = ",".join(str(sid) for sid in collection.featured_sound_ids) + form = FormClass( + instance=collection, + initial=dict(maintainers=collection_maintainers, featured_sounds=featured_sounds_str), + label_suffix="", + is_owner=is_owner, + is_maintainer=is_maintainer, + ) sort_by = request.GET.get("s", settings.COLLECTION_SORT_DEFAULT) - current_sounds = collection.get_sounds(sort_by=sort_by) + search_query = request.GET.get("q", "").strip() + + # Merge client-side featured IDs with DB-persisted ones for sorting + client_featured_str = request.GET.get("featured_sounds", "") + client_featured_ids = [int(x) for x in client_featured_str.split(",") if x.strip().isdigit()] + if client_featured_ids: + merged_featured = list(collection.featured_sound_ids) + [ + fid for fid in client_featured_ids if fid not in collection.featured_sound_ids + ] + else: + merged_featured = collection.featured_sound_ids + + current_sounds = Sound.objects.bulk_sounds_for_collection( + collection_id=collection.id, sort_by=sort_by, featured_sound_ids=merged_featured + ) + + # Union in sounds added via the modal (not yet persisted) + added_ids_str = request.GET.get("added_sounds", "") + added_ids = [int(x) for x in added_ids_str.split(",") if x.strip().isdigit()] + if added_ids: + added_qs = Sound.objects.filter(id__in=added_ids, moderation_state="OK", processing_state="OK") + current_sounds = (current_sounds | added_qs).distinct() + + # Filter sounds by search query if provided + if search_query: + current_sounds = current_sounds.annotate( + created_str=Cast("created", output_field=CharField()) + ).filter( + Q(original_filename__icontains=search_query) + | Q(description__icontains=search_query) + | Q(user__username__icontains=search_query) + | Q(created_str__icontains=search_query) + ).distinct() + + # Paginate the sounds + paginator_data = paginate(request, current_sounds, settings.BOOKMARKS_PER_PAGE) + page_sounds = Sound.objects.ordered_ids([sound.id for sound in paginator_data["page"].object_list]) + + form.collection_sound_objects = page_sounds + form.collection_maintainers_objects = maintainers_query + form.featured_sound_ids = merged_featured + + total_sounds_count = paginator_data["paginator"].count - current_maintainers = User.objects.filter(collection_maintainer=collection.id) - form.collection_sound_objects = current_sounds - form.collection_maintainers_objects = current_maintainers tvars = { "form": form, "collection": collection, "is_owner": is_owner, "is_maintainer": is_maintainer, + "initial_sound_ids": initial_sound_ids, "max_sounds_per_collection": settings.MAX_SOUNDS_PER_COLLECTION, "sort_by": sort_by, "sort_options": settings.COLLECTION_SORT_OPTIONS, + "total_sounds_count": total_sounds_count, + "search_query": search_query, } + tvars.update(paginator_data) return render(request, "collections/edit_collection.html", tvars) @login_required -def download_collection(request, collection_name, collection_id): +def download_collection(request, collection_id): collection = get_object_or_404(Collection, id=collection_id) collection_sounds = CollectionSound.objects.filter(collection=collection).values("sound_id") sounds_list = Sound.objects.filter( @@ -307,12 +312,12 @@ def download_collection(request, collection_name, collection_id): cds.append(CollectionDownloadSound(sound=sound, collection_download=cd, license=sound.license)) CollectionDownloadSound.objects.bulk_create(cds) - licenses_url = reverse("collection-licenses", args=[collection_name, collection_id]) + licenses_url = reverse("collection-licenses", args=[collection_id]) licenses_content = collection.get_attribution(sound_qs=sounds_list) return download_sounds(licenses_url, licenses_content, sounds_list, collection.download_filename) -def collection_licenses(request, collection_name, collection_id): +def collection_licenses(request, collection_id): collection = get_object_or_404(Collection, id=collection_id) attribution = collection.get_attribution() return HttpResponse(attribution, content_type="text/plain") @@ -324,7 +329,7 @@ def add_sounds_modal_for_collection_edit(request, collection_id): return render(request, "sounds/modal_add_sounds.html", tvars) -def add_maintainer_modal(request, collection_name, collection_id): +def add_maintainer_modal(request, collection_id): collection = get_object_or_404(Collection, id=collection_id) form = MaintainerForm() # TODO: the below statements exclude users with whitespaces in their usernames (and they still exist) diff --git a/general/templatetags/bw_templatetags.py b/general/templatetags/bw_templatetags.py index 6daee89ba..6dc2e1a1a 100644 --- a/general/templatetags/bw_templatetags.py +++ b/general/templatetags/bw_templatetags.py @@ -181,11 +181,15 @@ def bw_generic_stars(context, rating_0_10): @register.inclusion_tag("molecules/paginator.html", takes_context=True) -def bw_paginator(context, paginator, page, current_page, request, anchor="", non_grouped_number_of_results=-1): +def bw_paginator( + context, paginator, page, current_page, request, anchor="", non_grouped_number_of_results=-1, hx_target="" +): """ Adds pagination context variables for use in displaying first, adjacent and last page links in addition to those created by the object_list generic view. + + Optional hx_target parameter enables htmx AJAX pagination (e.g. hx_target="#sounds-section"). """ if paginator is None: # If paginator object is None, don't go ahead as below calculations will fail. This can happen if show_paginator @@ -254,6 +258,7 @@ def bw_paginator(context, paginator, page, current_page, request, anchor="", non "url_last_page": url_last_page, "anchor": anchor, "non_grouped_number_of_results": non_grouped_number_of_results, + "hx_target": hx_target, } diff --git a/package.json b/package.json index 1e160d65b..88d4813f4 100644 --- a/package.json +++ b/package.json @@ -34,6 +34,7 @@ "classlist-polyfill": "^1.2.0", "core-js": "^3.6.1", "element-closest": "^3.0.1", + "htmx.org": "^1.9.12", "lodash.debounce": "^4.0.8", "lodash.throttle": "^4.1.1", "normalize.css": "^8.0.1", diff --git a/sounds/models.py b/sounds/models.py index b86525115..1858ccb42 100644 --- a/sounds/models.py +++ b/sounds/models.py @@ -38,7 +38,8 @@ from django.contrib.sites.models import Site from django.core.exceptions import ObjectDoesNotExist from django.db import models -from django.db.models import Avg, Exists, F, OuterRef, Prefetch, Sum +from django.db.models import Avg, Exists, F, OuterRef, Prefetch, Subquery, Sum +from django.db.models import Case, IntegerField, Value, When from django.db.models.functions import Greatest, JSONObject from django.db.models.signals import post_delete, post_save, pre_delete from django.dispatch import receiver @@ -533,16 +534,44 @@ def bulk_sounds_for_collection( self, collection_id, limit=None, + sort_by=None, + featured_sound_ids=None, include_audio_descriptors=False, include_similarity_vectors=False, include_remix_subqueries=False, ): + qs = self.bulk_query( include_audio_descriptors=include_audio_descriptors, include_similarity_vectors=include_similarity_vectors, include_remix_subqueries=include_remix_subqueries, ) - qs = qs.filter(moderation_state="OK", processing_state="OK", collections=collection_id).order_by("-created") + qs = qs.filter(moderation_state="OK", processing_state="OK", collections=collection_id) + + # Validate sort option, defaulting to COLLECTION_SORT_DEFAULT if invalid + # If sort_by is None, skip sorting entirely and return all sounds + sort_field = None + if sort_by is not None: + if sort_by not in settings.COLLECTION_SORT_OPTIONS: + sort_by = settings.COLLECTION_SORT_DEFAULT + sort_field = settings.COLLECTION_SORT_OPTIONS[sort_by][1] + + # Apply sorting based on sort_by option + if sort_field == "featured_order": + # Featured sorting using Case/When annotation + # Non-featured sounds are ordered by date added to collection (newest first) + if featured_sound_ids: + ordering = Case( + *[When(id=sound_id, then=Value(i)) for i, sound_id in enumerate(featured_sound_ids)], + default=Value(len(featured_sound_ids)), + output_field=IntegerField() + ) + qs = qs.annotate(featured_order=ordering).order_by(sort_field, "collectionsound__created") + else: + qs = qs.order_by("collectionsound__created") + elif sort_field: + qs = qs.order_by(sort_field) + if limit: qs = qs[:limit] return qs diff --git a/sounds/templatetags/display_sound.py b/sounds/templatetags/display_sound.py index 5c3964cbe..45810f0c6 100644 --- a/sounds/templatetags/display_sound.py +++ b/sounds/templatetags/display_sound.py @@ -364,3 +364,16 @@ def display_sound_small_selectable(context, sound, selected=False): } ) return tvars + + +@register.inclusion_tag("sounds/display_sound_with_actions.html", takes_context=True) +def display_sound_small_with_actions(context, sound, is_featured=False): + """Display sound with featured and remove action toggles below it.""" + context = context.get("original_context", context) # This is to allow passing context in nested inclusion tags + tvars = display_sound_small_no_bookmark_no_ratings(context, sound) + tvars.update( + { + "is_featured": is_featured, + } + ) + return tvars diff --git a/sounds/templatetags/sounds_selector.py b/sounds/templatetags/sounds_selector.py index 95e6d8954..69d4966d7 100644 --- a/sounds/templatetags/sounds_selector.py +++ b/sounds/templatetags/sounds_selector.py @@ -27,18 +27,21 @@ @register.inclusion_tag("molecules/object_selector.html", takes_context=True) -def sounds_selector(context, sounds, max_sounds=None, selected_sound_ids=[], show_select_all_buttons=False): +def sounds_selector(context, sounds, max_sounds=None, selected_sound_ids=[], show_select_all_buttons=False, + featured_sound_ids=[], show_actions=False): if sounds: if not isinstance(sounds[0], Sound): # sounds are passed as a list of sound ids, retrieve the Sound objects from DB sounds = Sound.objects.ordered_ids(sounds) for sound in sounds: sound.selected = sound.id in selected_sound_ids + sound.is_featured = sound.id in featured_sound_ids return { "objects": sounds, "type": "sounds", "show_select_all_buttons": show_select_all_buttons, + "show_actions": show_actions, "original_context": context, # This will be used so a nested inclusion tag can get the original context "max_elements": max_sounds, } @@ -63,3 +66,10 @@ def packs_selector_with_select_buttons(context, packs, selected_pack_ids=[]): "show_select_all_buttons": True, "original_context": context, # This will be used so a nested inclusion tag can get the original context } + + +@register.inclusion_tag("molecules/object_selector.html", takes_context=True) +def sounds_selector_with_actions(context, sounds, featured_sound_ids=[], max_sounds=None): + """Displays sounds with featured and remove toggle buttons below each sound.""" + return sounds_selector(context, sounds, max_sounds=max_sounds, featured_sound_ids=featured_sound_ids, + show_actions=True) diff --git a/sounds/tests/test_manager.py b/sounds/tests/test_manager.py index 1e8374cad..7836c96e2 100644 --- a/sounds/tests/test_manager.py +++ b/sounds/tests/test_manager.py @@ -171,6 +171,54 @@ def test_bulk_sounds_for_pack(self): pack_sound_ids_bulk_query.append(sound.id) self.assertCountEqual(pack_sound_ids, pack_sound_ids_bulk_query) + def test_bulk_sounds_for_collection(self): + # This method uses SoundManager.bulk_query internally to retrieve sounds of a collection. + # We check filtering by collection, only_featured filtering, and sort_by ordering. + from fscollections.models import Collection, CollectionSound + + # Create a collection and add sounds to it + collection = Collection.objects.create(user=self.user, name="Test Collection") + sounds = list(Sound.objects.filter(user=self.user)) + for sound in sounds: + CollectionSound.objects.create(user=self.user, sound=sound, collection=collection) + + # Created sounds are not yet moderated and processed ok, so should return no sounds + self.assertEqual(len(list(Sound.objects.bulk_sounds_for_collection(collection_id=collection.id))), 0) + + # Now we set sounds to moderated and processed ok + collection_sound_ids = [] + for sound in sounds: + sound.moderation_state = "OK" + sound.processing_state = "OK" + sound.save() + collection_sound_ids.append(sound.id) + + # Check that sounds returned by bulk_sounds_for_collection are correct + result_ids = [s.id for s in Sound.objects.bulk_sounds_for_collection(collection_id=collection.id)] + self.assertCountEqual(collection_sound_ids, result_ids) + + # Test featured_sound_ids filtering + featured_ids = collection_sound_ids[:2] + featured_result_ids = [ + s.id for s in Sound.objects.bulk_sounds_for_collection( + collection_id=collection.id, + featured_sound_ids=featured_ids, + sort_by="featured", + ) + ] + # All collection sounds should be returned but featured ones ordered first + self.assertEqual(len(featured_result_ids), len(collection_sound_ids)) + + # Test sort_by ordering with a non-featured sort option + sorted_result = list( + Sound.objects.bulk_sounds_for_collection( + collection_id=collection.id, + sort_by="name", + ) + ) + for i in range(len(sorted_result) - 1): + self.assertLessEqual(sorted_result[i].original_filename, sorted_result[i + 1].original_filename) + class PublicSoundManagerTest(TestCase): fixtures = ["licenses", "sounds"] diff --git a/templates/base.html b/templates/base.html index 7a9266d5f..8276f940b 100644 --- a/templates/base.html +++ b/templates/base.html @@ -71,6 +71,7 @@ document.cookie = "disallowSimultaneousAudioPlayback=no;path=/" {% endif %} const userIsAuthenticated = {{ request.user.is_authenticated|yesno:'true,false' }}; + document.body.setAttribute('data-csrf-token', '{{ csrf_token }}'); {% block extrabody %} diff --git a/templates/collections/collection.html b/templates/collections/collection.html index ebae28e01..d19b7de61 100644 --- a/templates/collections/collection.html +++ b/templates/collections/collection.html @@ -2,7 +2,6 @@ {% load static %} {% load bw_templatetags %} {% load display_sound %} -{% load display_collections %} {% load display_user %} {% load util %} @@ -17,15 +16,15 @@

Collection: {{ collection.name }}

+ data-async-section-content-url="{% url 'collection-stats-section' collection.id %}?ajax=1">
{% if is_owner or is_maintainer%} - Edit collection + Edit collection {% endif %} {% if collection.num_sounds > 0%} - Download collection + Download collection {% endif %}
@@ -59,31 +58,44 @@

Collection: {{ collection.name }}

{% comment %}search and sorting options{% endcomment %}
-
- +
Sort by: - + {% for key, option in sort_options.items %} + {% endfor %}
-
- {% if page.object_list %} +
+ {% if page.object_list %}
- {% if sort_by == "Featured first" %} - {% display_featured_sounds featured_sounds_on_page %} - {% endif %} {% for collectionsound, sound in page_collection_and_sound_objects %} - {% if sort_by != "Featured first" or sound.id not in collection.featured_sound_ids %} + {% if sound.id in collection.featured_sound_ids %} +
+ {% display_sound_small sound %} +
+ {% else %}
{% display_sound_small sound %}
@@ -97,7 +109,8 @@

Collection: {{ collection.name }}

There aren't any sounds in this collection yet 😟 {% endif %} {% endif %} - {% bw_paginator paginator page current_page request "collection" %} + {% bw_paginator paginator page current_page request "collection" hx_target="#collection-sounds-results" %} +
{% if collection.public and collection.num_sounds > 0 %} diff --git a/templates/collections/display_collection.html b/templates/collections/display_collection.html index a98be38ab..e039fecda 100644 --- a/templates/collections/display_collection.html +++ b/templates/collections/display_collection.html @@ -7,7 +7,7 @@
- {% for sound in featured_sounds %} + {% for sound in header_sounds %}
{% display_sound_small_no_info_no_buttons sound %}
@@ -19,7 +19,7 @@
- {{ collection.name }} + {{ collection.name }}
@@ -32,7 +32,7 @@
{% if collection.num_downloads %}
- + {{ collection.num_downloads|formatnumber }}
diff --git a/templates/collections/display_featured_sounds.html b/templates/collections/display_featured_sounds.html deleted file mode 100644 index c57d9b6da..000000000 --- a/templates/collections/display_featured_sounds.html +++ /dev/null @@ -1,16 +0,0 @@ -{% load display_sound %} - -{% if featured_sounds %} -
-
-
Featured
-
- {% for sound in featured_sounds %} -
- {% display_sound_small sound %} -
- {% endfor %} -
-
-
-{% endif %} diff --git a/templates/collections/edit_collection.html b/templates/collections/edit_collection.html index 0374eab02..2b3c97078 100644 --- a/templates/collections/edit_collection.html +++ b/templates/collections/edit_collection.html @@ -17,7 +17,7 @@

{{ collection.name }}

-
{% csrf_token %} + {% csrf_token %} {% if collection.is_default_collection %}
{% bw_icon "notification" %}This is your default collection. You cannot change its name, description or public/private status. @@ -49,54 +49,75 @@

{{ collection.name }}

{% endif %} {% users_selector form.collection_maintainers_objects%}
- - + +
-
- {{ form.collection_sounds }} +
+ {# init-only: no name attr so it is not submitted with the form #} + + {{ form.added_sounds }} + {{ form.removed_sounds }} {{ form.featured_sounds }} -
- -
- - - +
+
+ +
+
+
+ +
+
+
+ + +
+
+
{{form.added_sounds.errors}}
+
+ {% sounds_selector_with_actions form.collection_sound_objects featured_sound_ids=form.featured_sound_ids max_sounds=max_sounds_per_collection %} + {% if not form.collection_sound_objects and search_query %} +
+ No sounds found matching "{{ search_query }}" in this collection. Clear search +
+ {% endif %} +
+ {% bw_paginator paginator page current_page request "sounds-section" hx_target="#sounds-section" %}
-
{{form.collection_sounds.errors}}
- {% sounds_selector form.collection_sound_objects max_sounds=max_sounds_per_collection%} -
- - -
-
- -
-
- - +
+
- +
{% if is_owner or is_maintainer%} {% endif %} - {% if is_owner%} - - {% endif %} +
+ Discard changes + {% if is_owner%} + + {% endif %} +
{% endblock %} diff --git a/templates/collections/modal_add_maintainer.html b/templates/collections/modal_add_maintainer.html index 362c00bfa..b723351f8 100644 --- a/templates/collections/modal_add_maintainer.html +++ b/templates/collections/modal_add_maintainer.html @@ -11,7 +11,7 @@

Add maintainer to collection

-
{% csrf_token %} + {% csrf_token %} {{form}} {% if not_found_msg %}
{% bw_icon "notification" %}{{not_found_msg}}
diff --git a/templates/molecules/object_selector.html b/templates/molecules/object_selector.html index af89d5493..60ca7e93a 100644 --- a/templates/molecules/object_selector.html +++ b/templates/molecules/object_selector.html @@ -6,7 +6,10 @@
{% for object in objects %}
- {% if type == "sounds" %}{% display_sound_small_selectable object object.selected %} + {% if type == "sounds" %} + {% if show_actions %}{% display_sound_small_with_actions object object.is_featured %} + {% else %}{% display_sound_small_selectable object object.selected %} + {% endif %} {% elif type == "packs" %}{% display_pack_small_selectable object object.selected %} {% elif type == "users" %}{% display_user_small_selectable object object.selected %} {% endif %} @@ -14,3 +17,4 @@ {% endfor %}
+ diff --git a/templates/molecules/paginator.html b/templates/molecules/paginator.html index 723022f23..9c19e0f0e 100644 --- a/templates/molecules/paginator.html +++ b/templates/molecules/paginator.html @@ -2,7 +2,7 @@ {% if page_numbers|length > 1 %} -