Fix: AbstractPicture.get_size for portrait images + changelog + tests (…

…#100)

* Fix AbstractPicture.get_size for portrait images

We input a 1:2 image and set JUST a width limit. The output was a tiny image stretched to our original limit. and was getting out a bizarrely tiny image. This was due to two problems.
 - Only involve the Golden Ratio when you're cropping!
 - Use it in the right direction. When you have a portrait image, you need to inverse the ratio.

* - add CHANGELOG.rst entry
- add test for portrait image
- add test for missing picture

Co-authored-by: Oli Warner <oli@thepcspy.com>

CHANGELOG.rst

@@ -12,6 +12,12 @@ Changelog
 * Added support for python 3.9 and 3.10
 
 
+3.0.1 (unreleased)
+=================
+
+* Fixed issue which caused wrong size calculations
+
+
 3.0.0 (2020-09-02)
 ==================
 

djangocms_picture/models.py

@@ -249,15 +249,24 @@ def get_size(self, width=None, height=None):
             width = self.width
             height = self.height
 
-        # calculate height when not given according to the
-        # golden ratio or fallback to the picture size
-        if not height and width:
-            height = int(width / PICTURE_RATIO)
-        elif not width and height:
-            width = int(height * PICTURE_RATIO)
-        elif not width and not height and self.picture:
-            width = self.picture.width
-            height = self.picture.height
+        if self.picture:
+            # calculate height when not given according to the
+            # golden ratio or fallback to the picture size
+            if crop:
+                if not height and width:
+                    if self.picture.width > self.picture.height:
+                        height = int(width / PICTURE_RATIO)
+                    else:
+                        height = int(width * PICTURE_RATIO)
+
+                elif not width and height:
+                    if self.picture.width > self.picture.height:
+                        width = int(height * PICTURE_RATIO)
+                    else:
+                        width = int(height / PICTURE_RATIO)
+
+            width = width or self.picture.width
+            height = height or self.picture.height
 
         options = {
             'size': (width, height),

tests/helpers.py

@@ -24,14 +24,14 @@ def create_image(mode="RGB", size=(800, 600)):
     return image
 
 
-def get_image(image_name="test_file.jpg"):
+def get_image(image_name="test_file.jpg", size=(800, 600)):
     """
     Creates and stores an image to the file system using PILImage
 
     :param image_name: the name for the file (default "test_file.jpg")
     :returns: dict {name, image, path}
     """
-    image = create_image()
+    image = create_image(size=size)
     image_path = os.path.join(
         mkdtemp(),
         image_name,
@@ -65,14 +65,14 @@ def get_file(file_name="test_file.pdf"):
     }
 
 
-def get_filer_image(image_name="test_file.jpg"):
+def get_filer_image(image_name="test_file.jpg", size=(800, 600)):
     """
     Creates and stores an image to filer and returns it
 
     :param image_name: the name for the file (default "test_file.jpg")
     :returns: filer image instance
     """
-    image = get_image(image_name)
+    image = get_image(image_name, size)
     filer_file = File(
         open(image.get("path"), "rb"),
         name=image.get("name"),

tests/test_models.py

@@ -25,7 +25,7 @@ def setUp(self):
         )
         self.picture = Picture.objects.create(
             template="default",
-            picture=get_filer_image(),
+            picture=get_filer_image(size=(800, 600)),
             width=720,
             height=480,
             alignment=get_alignment()[0][0],
@@ -36,6 +36,19 @@ def setUp(self):
             link_target=LINK_TARGET[0][0],
             link_attributes="{'data-type', 'picture'}",
         )
+        self.picture_portrait = Picture.objects.create(
+            template="default",
+            picture=get_filer_image(size=(600, 800)),
+            width=480,
+            height=720,
+            alignment=get_alignment()[0][0],
+            caption_text="some caption",
+            attributes="{'data-type', 'picture'}",
+            link_url="http://www.divio.com",
+            link_page=self.page,
+            link_target=LINK_TARGET[0][0],
+            link_attributes="{'data-type', 'picture'}",
+        )
         self.external_picture = 'https://www.google.com/images/logo.png'
 
     def tearDown(self):
@@ -54,7 +67,7 @@ def test_settings(self):
 
     def test_picture_instance(self):
         instance = Picture.objects.all()
-        self.assertEqual(instance.count(), 1)
+        self.assertEqual(instance.count(), 2)
         instance = Picture.objects.first()
         self.assertEqual(instance.template, "default")
         self.assertEqual(instance.picture.label, "test_file.jpg")
@@ -102,15 +115,25 @@ def test_clean(self):
 
     def test_get_size(self):
         instance = self.picture
+        instance_portrait = self.picture_portrait
         self.assertEqual(
             instance.get_size(),
             {"size": (800, 600), "crop": False, "upscale": False},
         )
         instance.use_crop = True
+        instance_portrait.use_crop = True
         self.assertEqual(
             instance.get_size(),
             {"size": (800, 600), "crop": True, "upscale": False},
         )
+        self.assertEqual(
+            instance_portrait.get_size(width=1000),
+            {'size': (1000, 1618), 'crop': True, 'upscale': False},
+        )
+        self.assertEqual(
+            instance_portrait.get_size(height=1000),
+            {'size': (618, 1000), 'crop': True, 'upscale': False},
+        )
         instance.use_upscale = True
         self.assertEqual(
             instance.get_size(),
@@ -146,6 +169,11 @@ def test_get_size(self):
             instance.get_size(),
             {'size': (200, 200), 'crop': False, 'upscale': False},
         )
+        instance.picture = None
+        self.assertEqual(
+            instance.get_size(),
+            {'size': (200, 200), 'crop': False, 'upscale': False},
+        )
 
     def test_get_link(self):
         instance = self.picture