-
Notifications
You must be signed in to change notification settings - Fork 592
fix: enhance thumbnail handling for public and private files #1519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 1 commit
6961cbc
ab02845
e939b46
1908ab5
1934f89
f7a1a60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -375,8 +375,30 @@ def directory_listing(self, request, folder_id=None, viewtype=None): | |
.order_by("-modified") | ||
) | ||
file_qs = file_qs.annotate( | ||
thumbnail_name=Subquery(thumbnail_qs.filter(name__contains=f"__{size}_").values_list("name")[:1]), | ||
thumbnailx2_name=Subquery(thumbnail_qs.filter(name__contains=f"__{size_x2}_").values_list("name")[:1]) | ||
# For private files (Filer pattern) | ||
thumbnail_name_filer=Subquery( | ||
thumbnail_qs.filter(name__contains=f"__{size}_").values_list("name")[:1] | ||
), | ||
|
||
thumbnailx2_name_filer=Subquery( | ||
thumbnail_qs.filter(name__contains=f"__{size_x2}_").values_list("name")[ | ||
:1 | ||
] | ||
), | ||
# For public files (easy_thumbnails pattern) | ||
thumbnail_name_easy=Subquery( | ||
thumbnail_qs.filter(name__contains=f".{size}_q85_crop").values_list( | ||
|
||
"name" | ||
)[:1] | ||
), | ||
thumbnailx2_name_easy=Subquery( | ||
thumbnail_qs.filter(name__contains=f".{size_x2}_q85_crop").values_list( | ||
"name" | ||
)[:1] | ||
), | ||
thumbnail_name=Coalesce("thumbnail_name_filer", "thumbnail_name_easy"), | ||
|
||
thumbnailx2_name=Coalesce( | ||
"thumbnailx2_name_filer", "thumbnailx2_name_easy" | ||
), | ||
).select_related("owner") | ||
|
||
try: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,13 +35,14 @@ def tearDown(self): | |
for f in File.objects.all(): | ||
f.delete() | ||
|
||
def create_filer_image(self, owner=None): | ||
def create_filer_image(self, owner=None, is_public=True): | ||
|
||
if owner is None: | ||
owner = self.superuser | ||
file_obj = DjangoFile(open(self.filename, 'rb'), name=self.image_name) | ||
image = Image.objects.create(owner=owner, | ||
original_filename=self.image_name, | ||
file=file_obj) | ||
file=file_obj, | ||
is_public=is_public) | ||
return image | ||
|
||
def test_create_folder_structure(self): | ||
|
@@ -80,7 +81,7 @@ def test_create_clipboard_item(self): | |
self.assertEqual(Clipboard.objects.count(), 1) | ||
|
||
def test_create_icons(self): | ||
image = self.create_filer_image() | ||
image = self.create_filer_image(is_public=False) | ||
image.save() | ||
icons = image.icons | ||
file_basename = os.path.basename(image.file.path) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
import os | ||
|
||
from django.conf import settings | ||
from django.core.files import File as DjangoFile | ||
from django.test import TestCase, override_settings | ||
|
||
from filer.models.filemodels import File | ||
from filer.settings import FILER_IMAGE_MODEL | ||
from filer.utils.loader import load_model | ||
from tests.helpers import create_image, create_superuser | ||
|
||
Image = load_model(FILER_IMAGE_MODEL) | ||
|
||
|
||
def custom_namer(thumbnailer, **kwargs): | ||
path, filename = os.path.split(thumbnailer.name) | ||
return os.path.join(path, f"custom_prefix_{filename}") | ||
|
||
|
||
class ThumbnailNameTests(TestCase): | ||
def setUp(self): | ||
self.superuser = create_superuser() | ||
self.img = create_image() | ||
self.image_name = "test_file.jpg" | ||
self.filename = os.path.join(settings.FILE_UPLOAD_TEMP_DIR, self.image_name) | ||
self.img.save(self.filename, "JPEG") | ||
|
||
def tearDown(self): | ||
os.remove(self.filename) | ||
for f in File.objects.all(): | ||
f.delete() | ||
|
||
def create_filer_image(self, is_public=True): | ||
with open(self.filename, "rb") as f: | ||
file_obj = DjangoFile(f) | ||
image = Image.objects.create( | ||
owner=self.superuser, | ||
original_filename=self.image_name, | ||
file=file_obj, | ||
is_public=is_public, | ||
) | ||
return image | ||
|
||
def test_thumbnailer_class_for_public_files(self): | ||
image = self.create_filer_image(is_public=True) | ||
thumbnailer = image.easy_thumbnails_thumbnailer | ||
name = thumbnailer.get_thumbnail_name({"size": (100, 100)}) | ||
self.assertNotIn("__", name) | ||
|
||
|
||
def test_thumbnailer_class_for_private_files(self): | ||
image = self.create_filer_image(is_public=False) | ||
thumbnailer = image.easy_thumbnails_thumbnailer | ||
name = thumbnailer.get_thumbnail_name({"size": (100, 100)}) | ||
self.assertIn("__", name) | ||
|
||
|
||
@override_settings(THUMBNAIL_NAMER="tests.test_thumbnails.custom_namer") | ||
def test_thumbnail_custom_namer(self): | ||
image = self.create_filer_image(is_public=True) | ||
thumbnailer = image.easy_thumbnails_thumbnailer | ||
name = thumbnailer.get_thumbnail_name({"size": (100, 100)}) | ||
filename = os.path.basename(name) | ||
self.assertTrue(filename.startswith("custom_prefix_")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (complexity): Consider refactoring the repeated subquery filtering into a helper function to reduce duplication and improve maintainability.
Consider refactoring the repeated subquery filtering into a helper function. This reduces duplication and makes it easier to update the filtering criteria in one place. For example:
Then update the annotation like so:
This keeps all functionality intact while reducing the nesting and duplicated logic.