-
Notifications
You must be signed in to change notification settings - Fork 3
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
base: 15.0
Are you sure you want to change the base?
Conversation
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")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
@AungKoKoLin1997 |
c21c928
to
700d7fa
Compare
700d7fa
to
46e4045
Compare
@yostashiro Please review this PR with the latest changes. |
|
||
@api.model | ||
def _cron_resize_product_image(self, limit): | ||
images = self.sudo().search([], limit=limit) |
There was a problem hiding this comment.
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.
if len(images) == limit: | ||
self.env.ref( | ||
"product_carousel_image_attachment.resize_product_image" | ||
)._trigger() |
There was a problem hiding this comment.
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.
There was a problem hiding this 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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<field name="interval_type">days</field> | |
<field name="interval_type">hours</field> |
4981