fix: address PR review issues for catalogue images
- Use PNG format instead of JPEG for transparency support - Non-strict mode only resizes if image width > target width - Replace CATALOGUE_BACKGROUND_IMAGES_COLOR with CATALOGUE_BACKGROUND_IMAGES_RGBA (tuple default (0,0,0,0)) - Move all inline imports to top of test file - Add test_resize_non_strict_no_upscale test case - Add comment explaining settings.DEBUG guard for media serving
This commit is contained in:
@@ -146,7 +146,12 @@ MEDIA_URL = "/media/"
|
||||
CATALOGUE_IMAGE_WIDTH = int(os.environ.get("CATALOGUE_IMAGE_WIDTH", "600"))
|
||||
CATALOGUE_IMAGE_HEIGHT = int(os.environ.get("CATALOGUE_IMAGE_HEIGHT", "600"))
|
||||
CATALOGUE_STRICT_DIMENSION = os.environ.get("CATALOGUE_STRICT_DIMENSION", "False").lower() in ("true", "1", "yes")
|
||||
CATALOGUE_BACKGROUND_IMAGES_COLOR = os.environ.get("CATALOGUE_BACKGROUND_IMAGES_COLOR") or None
|
||||
CATALOGUE_BACKGROUND_IMAGES_RGBA = os.environ.get(
|
||||
"CATALOGUE_BACKGROUND_IMAGES_RGBA", "0,0,0,0"
|
||||
)
|
||||
# Parse RGBA string to tuple
|
||||
_rgba_parts = CATALOGUE_BACKGROUND_IMAGES_RGBA.split(",")
|
||||
CATALOGUE_BACKGROUND_IMAGES_RGBA = tuple(int(p.strip()) for p in _rgba_parts)
|
||||
CATALOGUE_MAX_UPLOAD_SIZE = int(os.environ.get("CATALOGUE_MAX_UPLOAD_SIZE", str(5 * 1024 * 1024)))
|
||||
|
||||
# Default primary key field type
|
||||
|
||||
@@ -35,5 +35,7 @@ urlpatterns = [
|
||||
path('api/users/', include('users.urls')),
|
||||
]
|
||||
|
||||
# Serve media files through Django only in development.
|
||||
# In production/staging, media is served by the web server (nginx).
|
||||
if settings.DEBUG:
|
||||
urlpatterns += static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
import os
|
||||
|
||||
from django.conf import settings
|
||||
from PIL import Image
|
||||
|
||||
@@ -8,35 +6,28 @@ def resize_catalogue_image(image_field):
|
||||
width = settings.CATALOGUE_IMAGE_WIDTH
|
||||
height = settings.CATALOGUE_IMAGE_HEIGHT
|
||||
strict = settings.CATALOGUE_STRICT_DIMENSION
|
||||
bg_color = settings.CATALOGUE_BACKGROUND_IMAGES_COLOR
|
||||
bg_color = settings.CATALOGUE_BACKGROUND_IMAGES_RGBA
|
||||
|
||||
img = Image.open(image_field.path)
|
||||
img = img.convert("RGBA")
|
||||
|
||||
if strict:
|
||||
img.thumbnail((width, height), Image.LANCZOS)
|
||||
canvas = Image.new("RGBA", (width, height), (0, 0, 0, 0))
|
||||
if bg_color:
|
||||
canvas = Image.new(
|
||||
"RGBA", (width, height), _hex_to_rgba(bg_color)
|
||||
)
|
||||
img = resize_to_fit(img, width, height)
|
||||
canvas = Image.new("RGBA", (width, height), bg_color)
|
||||
x = (width - img.width) // 2
|
||||
y = (height - img.height) // 2
|
||||
canvas.paste(img, (x, y), img)
|
||||
img = canvas
|
||||
else:
|
||||
new_height = int(img.height * width / img.width)
|
||||
img = img.resize((width, new_height), Image.LANCZOS)
|
||||
if img.width > width:
|
||||
new_height = int(img.height * width / img.width)
|
||||
img = img.resize((width, new_height), Image.LANCZOS)
|
||||
|
||||
if img.mode == "RGBA":
|
||||
background = Image.new("RGB", img.size, (255, 255, 255))
|
||||
background.paste(img, mask=img.split()[3])
|
||||
img = background
|
||||
|
||||
img.save(image_field.path, format="JPEG", quality=85)
|
||||
img.save(image_field.path, format="PNG")
|
||||
|
||||
|
||||
def _hex_to_rgba(hex_color):
|
||||
hex_color = hex_color.lstrip("#")
|
||||
r, g, b = tuple(int(hex_color[i : i + 2], 16) for i in (0, 2, 4))
|
||||
return r, g, b, 255
|
||||
def resize_to_fit(img, max_width, max_height):
|
||||
ratio = min(max_width / img.width, max_height / img.height)
|
||||
new_width = int(img.width * ratio)
|
||||
new_height = int(img.height * ratio)
|
||||
return img.resize((new_width, new_height), Image.LANCZOS)
|
||||
|
||||
@@ -6,8 +6,10 @@ from django.core.files.uploadedfile import SimpleUploadedFile
|
||||
from django.test import override_settings
|
||||
from PIL import Image
|
||||
from rest_framework import status
|
||||
from rest_framework.test import APITestCase
|
||||
from rest_framework.test import APIClient, APITestCase
|
||||
from rest_framework_simplejwt.tokens import RefreshToken
|
||||
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
from ..models.products import Product
|
||||
from .Mixins import LoginMixin
|
||||
|
||||
@@ -32,7 +34,6 @@ class TestCatalogueImageModel(APITestCase, LoginMixin):
|
||||
)
|
||||
|
||||
def test_create_catalogue_image(self):
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
image_file = _create_test_image()
|
||||
catalogue_image = CatalogueImage.objects.create(
|
||||
product=self.product, image=image_file
|
||||
@@ -43,7 +44,6 @@ class TestCatalogueImageModel(APITestCase, LoginMixin):
|
||||
self.assertIsNotNone(catalogue_image.uploaded_at)
|
||||
|
||||
def test_catalogue_image_product_relation(self):
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
image_file = _create_test_image()
|
||||
ci = CatalogueImage.objects.create(
|
||||
product=self.product, image=image_file
|
||||
@@ -51,7 +51,6 @@ class TestCatalogueImageModel(APITestCase, LoginMixin):
|
||||
self.assertIn(ci, self.product.catalogue_images.all())
|
||||
|
||||
def test_catalogue_image_str(self):
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
image_file = _create_test_image()
|
||||
ci = CatalogueImage.objects.create(
|
||||
product=self.product, image=image_file
|
||||
@@ -60,7 +59,6 @@ class TestCatalogueImageModel(APITestCase, LoginMixin):
|
||||
self.assertEqual(str(ci), expected)
|
||||
|
||||
def test_cascade_delete_with_product(self):
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
image_file = _create_test_image()
|
||||
CatalogueImage.objects.create(
|
||||
product=self.product, image=image_file
|
||||
@@ -89,8 +87,6 @@ class TestCatalogueImageAPIPermissions(APITestCase, LoginMixin):
|
||||
email="regular@example.com",
|
||||
password="regularpass",
|
||||
)
|
||||
from rest_framework_simplejwt.tokens import RefreshToken
|
||||
from rest_framework.test import APIClient
|
||||
refresh = RefreshToken.for_user(self.user)
|
||||
self.client = APIClient()
|
||||
self.client.credentials(
|
||||
@@ -109,8 +105,6 @@ class TestCatalogueImageAPIPermissions(APITestCase, LoginMixin):
|
||||
email="regular2@example.com",
|
||||
password="regularpass",
|
||||
)
|
||||
from rest_framework_simplejwt.tokens import RefreshToken
|
||||
from rest_framework.test import APIClient
|
||||
refresh = RefreshToken.for_user(self.user)
|
||||
self.client = APIClient()
|
||||
self.client.credentials(
|
||||
@@ -122,12 +116,9 @@ class TestCatalogueImageAPIPermissions(APITestCase, LoginMixin):
|
||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||
|
||||
def test_delete_catalogue_image_non_admin(self):
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
admin_user = User.objects.create_superuser(
|
||||
username="admin2", email="admin2@example.com", password="adminpass"
|
||||
)
|
||||
from rest_framework_simplejwt.tokens import RefreshToken
|
||||
from rest_framework.test import APIClient
|
||||
admin_refresh = RefreshToken.for_user(admin_user)
|
||||
admin_client = APIClient()
|
||||
admin_client.credentials(
|
||||
@@ -235,7 +226,6 @@ class TestCatalogueImageAPI(APITestCase, LoginMixin):
|
||||
response = self.client.delete(url)
|
||||
self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
|
||||
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
self.assertEqual(CatalogueImage.objects.count(), 0)
|
||||
|
||||
def test_create_catalogue_image_invalid_product(self):
|
||||
@@ -260,15 +250,13 @@ class TestCatalogueImageResize(APITestCase, LoginMixin):
|
||||
)
|
||||
|
||||
def _create_and_get_image_obj(self, width, height, strict=False,
|
||||
bg_color=None):
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
bg_color=(0, 0, 0, 0)):
|
||||
settings_overrides = {
|
||||
"CATALOGUE_IMAGE_WIDTH": 600,
|
||||
"CATALOGUE_IMAGE_HEIGHT": 600,
|
||||
"CATALOGUE_STRICT_DIMENSION": strict,
|
||||
"CATALOGUE_BACKGROUND_IMAGES_RGBA": bg_color,
|
||||
}
|
||||
if bg_color is not None:
|
||||
settings_overrides["CATALOGUE_BACKGROUND_IMAGES_COLOR"] = bg_color
|
||||
|
||||
with override_settings(**settings_overrides):
|
||||
image_file = _create_test_image(width=width, height=height)
|
||||
@@ -306,6 +294,14 @@ class TestCatalogueImageResize(APITestCase, LoginMixin):
|
||||
self.assertEqual(img.width, 600)
|
||||
self.assertEqual(img.height, 600)
|
||||
|
||||
def test_resize_non_strict_no_upscale(self):
|
||||
ci = self._create_and_get_image_obj(
|
||||
width=400, height=300, strict=False
|
||||
)
|
||||
with Image.open(ci.image.path) as img:
|
||||
self.assertEqual(img.width, 400)
|
||||
self.assertEqual(img.height, 300)
|
||||
|
||||
def test_resize_strict_mode_landscape(self):
|
||||
ci = self._create_and_get_image_obj(
|
||||
width=800, height=600, strict=True
|
||||
@@ -324,7 +320,7 @@ class TestCatalogueImageResize(APITestCase, LoginMixin):
|
||||
|
||||
def test_resize_strict_mode_with_color_background(self):
|
||||
ci = self._create_and_get_image_obj(
|
||||
width=800, height=600, strict=True, bg_color="#FFFFFF"
|
||||
width=800, height=600, strict=True, bg_color=(255, 255, 255, 255)
|
||||
)
|
||||
with Image.open(ci.image.path) as img:
|
||||
self.assertEqual(img.width, 600)
|
||||
@@ -357,7 +353,6 @@ class TestProductListingWithImages(APITestCase, LoginMixin):
|
||||
self.assertIn("catalogue_images", product)
|
||||
|
||||
def test_product_list_catalogue_images_urls(self):
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
image_file = _create_test_image()
|
||||
CatalogueImage.objects.create(
|
||||
product=self.product, image=image_file
|
||||
@@ -376,7 +371,6 @@ class TestProductListingWithImages(APITestCase, LoginMixin):
|
||||
)
|
||||
|
||||
def test_product_list_catalogue_images_multiple(self):
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
img1 = _create_test_image(name="img1.png")
|
||||
img2 = _create_test_image(name="img2.png")
|
||||
CatalogueImage.objects.create(product=self.product, image=img1)
|
||||
@@ -402,7 +396,6 @@ class TestProductListingWithImages(APITestCase, LoginMixin):
|
||||
self.assertEqual(product_data["catalogue_images"], [])
|
||||
|
||||
def test_product_detail_includes_catalogue_images(self):
|
||||
from ..models.catalogue_images import CatalogueImage
|
||||
image_file = _create_test_image()
|
||||
CatalogueImage.objects.create(
|
||||
product=self.product, image=image_file
|
||||
|
||||
Reference in New Issue
Block a user