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

[4981][IMP] product_carousel_image_attachment #151

Open
wants to merge 8 commits into
base: 15.0
Choose a base branch
from

Conversation

AungKoKoLin1997
Copy link
Contributor

Comment on lines 15 to 21
max_resolution = (
"1025x1025" # Use 1025 instead of 1024 to enable the zoom feature
)
quality = int(ICP("base.image_autoresize_quality", 80))
img = ImageProcess(datas, verify_resolution=False)
w, h = img.image.size
nw, nh = map(int, max_resolution.split("x"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
max_resolution = (
"1025x1025" # Use 1025 instead of 1024 to enable the zoom feature
)
quality = int(ICP("base.image_autoresize_quality", 80))
img = ImageProcess(datas, verify_resolution=False)
w, h = img.image.size
nw, nh = map(int, max_resolution.split("x"))
max_resolution = (1025,1025) # Use 1025 instead of 1024 to enable the zoom feature
quality = int(ICP("base.image_autoresize_quality", 80))
img = ImageProcess(datas, verify_resolution=False)
w, h = img.image.size
nw, nh = max_resolution

I thought splitting the text is not necessary.

@kanda999
Copy link
Contributor

@AungKoKoLin1997
The module function is working as expected.

@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 15.0-upd-product_carousel_image_attachment branch from c21c928 to 700d7fa Compare November 25, 2024 08:29
@AungKoKoLin1997 AungKoKoLin1997 force-pushed the 15.0-upd-product_carousel_image_attachment branch from 700d7fa to 46e4045 Compare November 25, 2024 08:39
@AungKoKoLin1997
Copy link
Contributor Author

@yostashiro Please review this PR with the latest changes.


@api.model
def _cron_resize_product_image(self, limit):
images = self.sudo().search([], limit=limit)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AungKoKoLin1997 Would it be possible to filter records by size? There can be so many records unnecessarily processed otherwise. We could live with this if it's hard to do so.

Comment on lines 15 to 18
if len(images) == limit:
self.env.ref(
"product_carousel_image_attachment.resize_product_image"
)._trigger()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this implementation, is it still possible to cancel the process in the middle if we want to? If not, we should just let this method be triggered based on the intervals defined in the scheduled action, as that is more controllable.

Copy link
Member

@yostashiro yostashiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review.

<field name="code">model._cron_resize_product_image(1000)</field>
<field name="user_id" ref="base.user_root" />
<field name="interval_number">1</field>
<field name="interval_type">days</field>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<field name="interval_type">days</field>
<field name="interval_type">hours</field>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants