fix album list django view filtering by files, add first unit tests f…

…or album django views, move some stuff to utils/tests.py to keep tests DRY

src/albums/filters.py

@@ -3,27 +3,50 @@
 from typing import ClassVar
 
 import django_filters
+from django.db.models import QuerySet
+from django.http import HttpRequest
+from django.utils import timezone
+from files.models import BaseFile
 from ninja import Field
 from utils.filters import ListFilters
 
 from .models import Album
 
 
 class AlbumFilters(ListFilters):
-    """The filters used for the album_list endpoint."""
+    """The filters used for the album_list django-ninja API endpoint."""
 
     files: list[uuid.UUID] = Field(None, alias="files")
 
 
+def get_permitted_files(request: HttpRequest) -> QuerySet[BaseFile]:
+    """Called by AlbumFilter to get files for the albumlist filter multiselect form field."""
+    return BaseFile.permitted_files.get_queryset(user=request.user).all()
+
+
 class AlbumFilter(django_filters.FilterSet):
-    """The Album filter."""
+    """The Album filter used by django-filters."""
+
+    # when filtering by files only show files the user has permission for in the form
+    files = django_filters.filters.ModelMultipleChoiceFilter(
+        field_name="files",
+        queryset=get_permitted_files,
+        method="filter_files",
+    )
+
+    def filter_files(self, queryset: QuerySet[Album], name: str, value: str) -> QuerySet[Album]:
+        """When filtering by files only consider currently active memberships."""
+        # we want AND so loop over files and filter for each,
+        # finally returning only albums containing all the files in value
+        for f in value:
+            queryset = queryset.filter(memberships__basefile__in=[f], memberships__period__contains=timezone.now())
+        return queryset
 
     class Meta:
         """Set model and fields."""
 
         model = Album
         fields: ClassVar[dict[str, list[str]]] = {
-            "files": ["exact"],
             "title": ["exact", "icontains"],
             "description": ["icontains"],
         }

src/albums/tables.py

@@ -19,7 +19,6 @@ class Meta:
             "title",
             "description",
             "owner",
-            "files",
             "active_memberships",
             "historic_memberships",
             "future_memberships",

src/albums/tests.py

@@ -1,4 +1,5 @@
 """Tests for the Album API."""
+from bs4 import BeautifulSoup
 from django.urls import reverse
 from oauth2_provider.models import get_access_token_model
 from oauth2_provider.models import get_application_model
@@ -20,18 +21,7 @@ def test_album_create(
         files: list[str] | None = None,
     ) -> None:
         """Test creating an album."""
-        response = self.client.post(
-            reverse("api-v1-json:album_create"),
-            {
-                "title": title,
-                "description": description,
-                "files": files if files else [],
-            },
-            headers={"authorization": self.curator6.auth},
-            content_type="application/json",
-        )
-        assert response.status_code == 201
-        self.album_uuid = response.json()["bma_response"]["uuid"]
+        self.album_uuid = self.album_create(title=title, description=description, files=files)
 
     def test_album_create_with_files(
         self,
@@ -42,7 +32,7 @@ def test_album_create_with_files(
         self.files = []
         for _ in range(10):
             self.files.append(self.file_upload())
-        self.test_album_create(title=title, description=description, files=self.files)
+        self.album_uuid = self.album_create(title=title, description=description, files=self.files)
 
     def test_album_update(self) -> None:
         """First replace then update."""
@@ -201,3 +191,41 @@ def test_album_list(self) -> None:
         assert response.status_code == 200
         assert len(response.json()["bma_response"]) == 5
         assert response.json()["bma_response"][0]["title"] == "album5"
+
+
+class TestAlbumViews(ApiTestBase):
+    """Unit tests for regular django Album views."""
+
+    def create_albums(self) -> None:
+        """Create som albums for testing."""
+        # upload some files as creator2
+        self.files = []
+        for _ in range(10):
+            self.files.append(self.file_upload())
+        # add them to an album created by creator6
+        self.album_uuid = self.album_create(title="creator2 files", files=self.files)
+        # upload some files as creator3
+        for _ in range(10):
+            self.files.append(self.file_upload(uploader="creator3"))
+        self.album_create(title="creator3 files", files=self.files[10:], creator="curator7")
+
+    def test_album_list_view(self) -> None:
+        """Test the basics of the album list view."""
+        url = reverse("albums:album_list")
+        self.create_albums()
+        self.client.login(username="creator2", password="secret")
+
+        # test listing both albums, no filters
+        response = self.client.get(url)
+        content = response.content.decode()
+        soup = BeautifulSoup(content, "html.parser")
+        rows = soup.select("div.table-container > table > tbody > tr")
+        self.assertEqual(len(rows), 2, "album list does not return 2 albums")
+
+        # test filtering by files to show albums containing a single file
+        url += f"?files={self.files[0]}"
+        response = self.client.get(url)
+        content = response.content.decode()
+        soup = BeautifulSoup(content, "html.parser")
+        rows = soup.select("div.table-container > table > tbody > tr")
+        self.assertEqual(len(rows), 1, "filtering by files does not return 1 album")

src/albums/views.py

@@ -34,7 +34,7 @@ class AlbumListView(SingleTableMixin, FilterView):
     filterset_class = AlbumFilter
     context_object_name = "albums"
 
-    def get_table_data(self) -> QuerySet[Album]:
+    def get_queryset(self) -> QuerySet[Album]:
         """Add membership counts."""
         qs = super().get_queryset()
         active_memberships = Count("memberships", filter=Q(memberships__period__contains=timezone.now()))

src/files/filters.py

@@ -34,7 +34,7 @@ class FileFilter(django_filters.FilterSet):
     not_albums = django_filters.filters.UUIDFilter(field_name="albums", method="filter_albums", exclude=True)
 
     def filter_albums(self, queryset: QuerySet[BaseFile], name: str, value: str) -> QuerySet[BaseFile]:
-        """When filtering for albums only consider currently active memberships."""
+        """When filtering by albums only consider currently active memberships."""
         return queryset.filter(memberships__album__in=[value], memberships__period__contains=timezone.now())
 
     class Meta:

src/files/models.py

@@ -11,6 +11,7 @@
 from django.http import HttpRequest
 from django.urls import reverse
 from guardian.shortcuts import assign_perm
+from guardian.shortcuts import get_objects_for_user
 from guardian.shortcuts import remove_perm
 from polymorphic.managers import PolymorphicQuerySet
 from polymorphic.models import PolymorphicManager
@@ -93,6 +94,20 @@ def unpublish(self) -> int:
         return updated
 
 
+class PermittedFilesManager(PolymorphicManager):
+    """A custom manager which only returns the files the user has access to."""
+
+    def get_queryset(self, user: User) -> models.QuerySet["BaseFile"]:  # type: ignore[valid-type]
+        """Return PUBLISHED files and files where the user has view_basefile perms."""
+        files = super().get_queryset().filter(status="PUBLISHED") | get_objects_for_user(
+            user=user,
+            perms="files.view_basefile",
+            klass=super().get_queryset(),
+        )
+        # do not return duplicates when a file is PUBLISHED and a user also has files.view_basefile
+        return files.distinct()  # type: ignore[no-any-return]
+
+
 class BaseFile(PolymorphicModel):
     """The polymorphic base model inherited by the Picture, Video, Audio, and Document models."""
 
@@ -111,6 +126,8 @@ class Meta:
 
     objects = PolymorphicManager.from_queryset(BaseFileQuerySet)()
 
+    permitted_files = PermittedFilesManager()
+
     uuid = models.UUIDField(
         primary_key=True,
         default=uuid.uuid4,

src/files/tests.py

@@ -623,7 +623,7 @@ def test_file_missing_on_disk(self) -> None:
         self.assertEqual(response.json()["bma_response"]["size_bytes"], 0)
 
 
-class FileAdminTests(ApiTestBase):
+class TestFileAdmin(ApiTestBase):
     """Tests for the FileAdmin."""
 
     def test_file_list_status_code(self) -> None:
@@ -758,7 +758,7 @@ def test_file_list_html(self) -> None:
         )
 
 
-class FileViewTests(ApiTestBase):
+class TestFileViews(ApiTestBase):
     """Unit tests for regular django views."""
 
     def test_file_list_view(self) -> None:  # noqa: PLR0915

src/utils/tests.py

@@ -170,3 +170,25 @@ def file_upload(  # noqa: PLR0913
         if return_full:
             return data
         return data["uuid"]
+
+    def album_create(
+        self,
+        *,
+        title: str = "album title here",
+        description: str = "album description here",
+        files: list[str] | None = None,
+        creator: str = "curator6",
+    ) -> str:
+        """Create an album optionally with some files."""
+        response = self.client.post(
+            reverse("api-v1-json:album_create"),
+            {
+                "title": title,
+                "description": description,
+                "files": files if files else [],
+            },
+            headers={"authorization": getattr(self, creator).auth},
+            content_type="application/json",
+        )
+        assert response.status_code == 201
+        return response.json()["bma_response"]["uuid"]