Skip to content

Commit

Permalink
fix: Compatibility with custom images (#1454)
Browse files Browse the repository at this point in the history
* Made the filer check command compatible with custom image models

* Use the final image model's app label to determine the expand view URL

* Prepare the image expand URL in the admin code and pass it through to the template via context

* Added tests for the new expand link handling

* Added runs with a custom image model to the test matrix for GitHub actions

* Added packaging to requirements
  • Loading branch information
W1ldPo1nter authored Mar 22, 2024
1 parent b0f39ab commit fa385bd
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 17 deletions.
7 changes: 5 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ jobs:
strategy:
fail-fast: false
matrix:
python-version: ['3.8', '3.9', '3.10', '3.11' ]
python-version: ['3.8', '3.9', '3.10', '3.11']
requirements-file: [
django-3.2.txt,
django-4.0.txt,
django-4.1.txt,
django-4.2.txt,
django-5.0.txt,
]
custom-image-model: [false, true]
exclude:
- requirements-file: django-5.0.txt
python-version: 3.8
Expand All @@ -41,8 +42,10 @@ jobs:
python -m pip install --upgrade pip
pip install -r tests/requirements/${{ matrix.requirements-file }}
python setup.py install
- name: Enable the custom image model
run: echo "CUSTOM_IMAGE=custom_image.Image" >> $GITHUB_ENV
if: ${{ matrix.custom-image-model }}
- name: Run coverage
run: coverage run setup.py test

- name: Upload Coverage to Codecov
uses: codecov/codecov-action@v1
23 changes: 17 additions & 6 deletions filer/admin/fileadmin.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import mimetypes

from django import forms
from django.contrib.admin.templatetags.admin_urls import admin_urlname
from django.contrib.admin.utils import unquote
from django.contrib.staticfiles.storage import staticfiles_storage
from django.http import Http404, HttpResponse, HttpResponseRedirect
Expand All @@ -19,10 +20,14 @@
from .. import settings
from ..models import BaseImage, File
from ..settings import DEFERRED_THUMBNAIL_SIZES
from ..utils.loader import load_model
from .permissions import PrimitivePermissionAwareModelAdmin
from .tools import AdminContext, admin_url_params_encoded, popup_status


Image = load_model(settings.FILER_IMAGE_MODEL)


class FileAdminChangeFrom(forms.ModelForm):
class Meta:
model = File
Expand Down Expand Up @@ -123,12 +128,18 @@ def response_change(self, request, obj):

def render_change_form(self, request, context, add=False, change=False,
form_url='', obj=None):
info = self.model._meta.app_label, self.model._meta.model_name
extra_context = {'show_delete': True,
'history_url': 'admin:%s_%s_history' % info,
'is_popup': popup_status(request),
'filer_admin_context': AdminContext(request)}
context.update(extra_context)
context.update({
'show_delete': True,
'history_url': admin_urlname(self.opts, 'history'),
'expand_image_url': None,
'is_popup': popup_status(request),
'filer_admin_context': AdminContext(request),
})
if obj and obj.mime_maintype == 'image' and obj.file.exists():
if 'svg' in obj.mime_type:
context['expand_image_url'] = reverse(admin_urlname(Image._meta, 'expand'), args=(obj.pk,))
else:
context['expand_image_url'] = obj.file.url
return super().render_change_form(
request=request, context=context, add=add, change=change,
form_url=form_url, obj=obj)
Expand Down
2 changes: 1 addition & 1 deletion filer/admin/imageadmin.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def get_urls(self):
return super().get_urls() + [
path("expand/<int:file_id>",
self.admin_site.admin_view(self.expand_view),
name=f"filer_{self.model._meta.model_name}_expand_view")
name=f"{self.opts.app_label}_{self.opts.model_name}_expand")
]

def expand_view(self, request, file_id):
Expand Down
4 changes: 2 additions & 2 deletions filer/management/commands/filer_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from PIL import UnidentifiedImageError

from filer import settings as filer_settings
from filer.utils.loader import load_model


class Command(BaseCommand):
Expand Down Expand Up @@ -130,10 +131,9 @@ def image_dimensions(self, options):
import easy_thumbnails
from easy_thumbnails.VIL import Image as VILImage

from filer.models.imagemodels import Image
from filer.utils.compatibility import PILImage

no_dimensions = Image.objects.filter(
no_dimensions = load_model(filer_settings.FILER_IMAGE_MODEL).objects.filter(
Q(_width=0) | Q(_width__isnull=True)
)
self.stdout.write(f"trying to set dimensions on {no_dimensions.count()} files")
Expand Down
5 changes: 2 additions & 3 deletions filer/templates/admin/filer/tools/detail_info.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,8 @@
<span class="icon fa fa-download filer-icon filer-icon-download"></span>&nbsp;
<span>{% translate "Download" %}</span>
</a>
{% if original.mime_maintype == 'image' %}
<a href="{% if 'svg' in original.mime_type %}{% url 'admin:filer_image_expand_view' original.pk %}{% else %}{{ original.file.url }}{% endif %}"
target="_blank" rel="noopener noreferrer" class="button">
{% if expand_image_url %}
<a href="{{ expand_image_url }}" target="_blank" rel="noopener noreferrer" class="button">
<span>{% translate "Expand" %}</span>&nbsp;
<span class="icon filer-icon filer-icon-expand fa fa-expand"></span>
</a>
Expand Down
1 change: 1 addition & 0 deletions tests/requirements/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,4 @@ django-app-helper>=3.3.1
coverage
isort
flake8
packaging
33 changes: 31 additions & 2 deletions tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.conf import settings
from django.contrib import admin
from django.contrib.admin import helpers
from django.contrib.admin.templatetags.admin_urls import admin_urlname
from django.contrib.auth import get_user_model
from django.contrib.auth.models import Permission
from django.contrib.messages import ERROR, get_messages
Expand Down Expand Up @@ -327,7 +328,7 @@ def test_detail_view_missing_file(self):
image._height = 200
image.save()

url = reverse(f'admin:{image.__class__._meta.app_label}_image_change', kwargs={
url = reverse(admin_urlname(Image._meta, 'change'), kwargs={
'object_id': image.pk,
})

Expand All @@ -338,8 +339,36 @@ def test_detail_view_missing_file(self):
self.assertContains(response, 'height="210"')
self.assertContains(response, 'alt="File is missing"')

def test_image_expand_link_in_change_view(self):
files = [
# Files can use the same contents for this test - it's the mime type that counts
File.objects.create(owner=self.superuser, original_filename='some-file.txt', file=self.file_object.file),
Image.objects.create(owner=self.superuser, original_filename='some-image.jpg'), # missing file
Image.objects.create(owner=self.superuser, original_filename='some-image.jpg', file=self.file_object.file),
Image.objects.create(owner=self.superuser, original_filename='some-image.svg', file=self.file_object.file),
]
test_set = [
(files[0], 'text/plain', None),
(files[1], 'image/jpeg', None),
(files[2], 'image/jpeg', files[2].file.url),
(files[3], 'image/svg+xml', reverse(admin_urlname(Image._meta, 'expand'), args=(files[3].pk,))),
]
for file, mime_type, expected_url in test_set:
file.mime_type = mime_type
file.save()
models = [File]
if isinstance(file, Image):
models.append(Image)
for model in models:
response = self.client.get(reverse(admin_urlname(model._meta, 'change'),
kwargs={'object_id': file.pk}))
if expected_url:
self.assertContains(response, f'href="{expected_url}"')
else:
self.assertNotContains(response, 'filer-icon-expand')

def test_image_expand_view(self):
url = reverse("admin:filer_image_expand_view", kwargs={
url = reverse(admin_urlname(Image._meta, 'expand'), kwargs={
'file_id': self.file_object.pk
})
original_url = self.file_object.url
Expand Down
6 changes: 5 additions & 1 deletion tests/test_filer_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@

from filer import settings as filer_settings
from filer.models.filemodels import File
from filer.models.imagemodels import Image
from filer.settings import FILER_IMAGE_MODEL
from filer.utils.loader import load_model
from tests.helpers import create_image


Image = load_model(FILER_IMAGE_MODEL)


class FilerCheckTestCase(TestCase):

svg_file_string = """<?xml version="1.0" encoding="UTF-8" standalone="no"?>
Expand Down

0 comments on commit fa385bd

Please sign in to comment.