fix: crash in the file detail view (#1395)

* fix: crash in the file detail view

In a recent fix, we introduced a regression where we started to access
width and height of a file. However, this will only work for images and
not for normal files like PDF, Docx etc.

In order to fix this, we need to guard and check if the file is actually
an image before trying to access the attributes.

- Github Issue: #1394

Authored-by: Vinit Kumar <[email protected]>

* fix: refactor code and add test to check the functionality and verify fix

* fix: remove logs

filer/templatetags/filer_admin_tags.py

@@ -100,12 +100,7 @@ def file_icon_context(file, detail, width, height):
         'mime_maintype': mime_maintype,
         'mime_type': file.mime_type,
     }
-    # Get download_url and aspect ratio right for detail view
-    if detail:
-        context['download_url'] = file.url
-        if file.width:
-            width, height = 210, ceil(210 / file.width * file.height)
-            context['sidebar_image_ratio'] = file.width / 210
+    height, width, context = get_aspect_ratio_and_download_url(context, detail, file, height, width)
     # returned context if icon is not available
     not_available_context = {
         'icon_url': staticfiles_storage.url('filer/icons/file-missing.svg'),
@@ -168,6 +163,20 @@ def file_icon_context(file, detail, width, height):
     return context
 
 
+def get_aspect_ratio_and_download_url(context, detail, file, height, width):
+    # Get download_url and aspect ratio right for detail view
+    if detail:
+        context['download_url'] = file.url
+        if isinstance(file, BaseImage):
+            # only check for file width, if the file
+            # is actually an image and not on other files
+            # because they don't really have width or height
+            if file.width:
+                width, height = 210, ceil(210 / file.width * file.height)
+                context['sidebar_image_ratio'] = file.width / 210
+    return height, width, context
+
+
 @register.inclusion_tag('admin/filer/templatetags/file_icon.html')
 def file_icon(file, detail=False, size=None):
     """

tests/test_admin.py

@@ -22,7 +22,8 @@
 from filer.models.foldermodels import Folder, FolderPermission
 from filer.models.virtualitems import FolderRoot
 from filer.settings import DEFERRED_THUMBNAIL_SIZES, FILER_IMAGE_MODEL
-from filer.templatetags.filer_admin_tags import file_icon_url
+from filer.templatetags.filer_admin_tags import file_icon_url, get_aspect_ratio_and_download_url
+
 from filer.thumbnail_processors import normalize_subject_location
 from filer.utils.loader import load_model
 from tests.helpers import SettingsOverride, create_folder_structure, create_image, create_superuser
@@ -1678,3 +1679,29 @@ def test_admin_url_params(self):
         })
         request = request_factory.get('/', {'_pick': 'bad_type'})
         self.assertDictEqual(tools.admin_url_params(request), {})
+
+
+class FileIconContextTests(TestCase):
+
+    def test_image_icon_with_size(self):
+        """
+        Image with get an aspect ratio and will be present in context
+        """
+        image = Image.objects.create(name='test.jpg')
+        image._width = 50
+        image._height = 200
+        image.save()
+        context = {}
+        height, width, context = get_aspect_ratio_and_download_url(context=context, detail=True, file=image, height=40, width=40)
+        assert 'sidebar_image_ratio' in context.keys()
+        assert 'download_url' in context.keys()
+
+    def test_file_icon_with_size(self):
+        """
+        File with not get an aspect ratio and will not be present in context
+        """
+        file = File.objects.create(name='test.pdf')
+        context = {}
+        height, width, context = get_aspect_ratio_and_download_url(context=context, detail=True, file=file, height=40, width=40)
+        assert 'sidebar_image_ratio' not in context.keys()
+        assert 'download_url' in context.keys()