Skip to content
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

[14.0] shopinvader_image: improve image metadata handling #1563

Open
wants to merge 5 commits into
base: 14.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion shopinvader_image/__manifest__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
{
"name": "Shopinvader image",
"summary": "Add the export of Image for Shopinvader",
"version": "14.0.1.3.1",
"version": "14.0.1.4.0",
"category": "e-commerce",
"website": "https://github.com/shopinvader/odoo-shopinvader",
"author": "Akretion",
Expand Down
20 changes: 20 additions & 0 deletions shopinvader_image/migrations/14.0.1.4.0/pre-migrate.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Copyright 2023 Camptocamp SA
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl)

import logging

_logger = logging.getLogger(__name__)


def update_shopinvader_tables(cr):
_logger.info("Updating shopinvader_backend table ...")
queries = [
"ALTER TABLE shopinvader_backend "
"ADD COLUMN IF NOT EXISTS image_data_empty_alt_name_allowed BOOLEAN",
]
for query in queries:
cr.execute(query)


def migrate(cr, version):
update_shopinvader_tables(cr)
15 changes: 15 additions & 0 deletions shopinvader_image/models/shopinvader_backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,16 @@
to all images' relative path.
"""

ALT_HELP_TEXT = """
The alt name of an image is used to provide a meaningful description.
The alt name can be configured by image, but it can also be empty.
When it's empty, the product name will be used as a fallback.
When this flag is enabled, the alt name will be empty.
You can then choose on your shop how to populate it.
This helps reducing the amount of data stored in the database
when the alt name is always equal to the product name.
"""


class ShopinvaderBackend(models.Model):
_inherit = "shopinvader.backend"
Expand All @@ -44,3 +54,8 @@ class ShopinvaderBackend(models.Model):
help=CDN_HELP_TEXT,
default=True,
)
image_data_empty_alt_name_allowed = fields.Boolean(
string="Image ALT name can be empty",
help=ALT_HELP_TEXT,
default=False,
)
34 changes: 30 additions & 4 deletions shopinvader_image/models/shopinvader_image_mixin.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,13 @@ def _get_images_store_hash_tuple(self):
self._resize_scales().mapped(lambda r: (r.key, r.size_x, r.size_y))
)
timestamp = self._get_images_store_hash_timestamp()
alt_names = tuple([x.alt_name for x in images])
backend_flags = (
self.backend_id.image_data_include_cdn_url,
self.backend_id.image_data_empty_alt_name_allowed,
)
# TODO: any other bit to consider here?
return resize_scales + public_urls + (timestamp,)
return resize_scales + public_urls + alt_names + backend_flags + (timestamp,)

def _get_image_url_key(self, image_relation):
# You can inherit this method to change the name of the image of
Expand Down Expand Up @@ -126,11 +131,32 @@ def _prepare_data_resize(self, thumbnail, image_relation):
:return: dict
"""
self.ensure_one()
res = {"src": self._get_image_url(thumbnail), "alt": self.name}
if "tag_id" in image_relation._fields:
res["tag"] = image_relation.tag_id.name or ""
res = {
"src": self._get_image_url(thumbnail),
}
alt_name = self._get_image_alt(image_relation.image_id)
if alt_name:
res["alt"] = alt_name
tag = self._get_image_tag(image_relation)
if tag:
res["tag"] = tag
return res

def _get_image_alt(self, image):
alt_name = image.alt_name
# Makes no sense to store the filename as alt as we already have the URL
if alt_name and alt_name != image.name:
return alt_name
if self.backend_id.image_data_empty_alt_name_allowed:
return ""
return self.name

def _get_image_url(self, image):
fname = "url" if self.backend_id.image_data_include_cdn_url else "url_path"
return image[fname]

def _get_image_tag(self, image_relation):
if "tag_id" not in image_relation._fields:
return None
if image_relation.tag_id:
return image_relation.tag_id.name
40 changes: 38 additions & 2 deletions shopinvader_image/tests/test_images.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ def test_basic_images_compute(self):
)
)
)
self.assertIn("tag", img)

def test_basic_images_compute_no_cdn_url(self):
storage_backend = self.shopinvader_variant.image_ids.image_id.backend_id
Expand All @@ -60,7 +59,44 @@ def test_basic_images_compute_no_cdn_url(self):
"/customizable-desk-config_{0.size_x}_{0.size_y}".format(scale)
)
)
self.assertIn("tag", img)

def test_image_metadata(self):
self.shopinvader_variant.invalidate_cache(["images"])
self.shopinvader_variant[0].image_ids[0].image_id.alt_name = "Test Alt Name"
images = self.shopinvader_variant.images
for scale in self.backend.shopinvader_variant_resize_ids:
img = images[0][scale.key]
self.assertEqual(img["alt"], "Test Alt Name")
self.assertNotIn("tag", img)
for scale in self.backend.shopinvader_variant_resize_ids:
img = images[1][scale.key]
# Fallback to product name
self.assertEqual(img["alt"], self.shopinvader_variant.name)
self.assertNotIn("tag", img)
# allow empty alt
self.shopinvader_variant.backend_id.image_data_empty_alt_name_allowed = True
self.shopinvader_variant.invalidate_cache(["images"])
images = self.shopinvader_variant.images
for scale in self.backend.shopinvader_variant_resize_ids:
img = images[1][scale.key]
# NO Fallback to product name
self.assertNotIn("alt", img)
self.shopinvader_variant.invalidate_cache(["images"])
# value tags
self.shopinvader_variant.invalidate_cache(["images"])
# enforce compute to get the tag
# because the field is not there and the hash won't change
self.shopinvader_variant._compute_images_stored()
with mock.patch.object(
type(self.shopinvader_variant), "_get_image_tag"
) as mocked:
mocked.return_value = "Test Tag"
images = self.shopinvader_variant.images
for scale in self.backend.shopinvader_variant_resize_ids:
img = images[0][scale.key]
self.assertEqual(img["tag"], "Test Tag")
img = images[1][scale.key]
self.assertEqual(img["tag"], "Test Tag")

def test_hash_and_compute_flag(self):
variant = self.shopinvader_variant
Expand Down
1 change: 1 addition & 0 deletions shopinvader_image/views/shopinvader_backend_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
widget="many2many_tags"
/>
<field name="image_data_include_cdn_url" />
<field name="image_data_empty_alt_name_allowed" />
</group>
</page>
</field>
Expand Down
Loading