-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Improve command catalog:images:resize #23005
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
Improve command catalog:images:resize #23005
Conversation
- Only resize images that are actually in use. - Improve code on several places
Hi @tdgroot. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
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.
Hi @tdgroot,
thank you for your contribution!
Could you please also check and fix the automated tests failures.
) { | ||
parent::__construct(); | ||
$this->resize = $resize; | ||
$this->appState = $appState; | ||
$this->objectManager = $objectManager; | ||
$this->progressBarFactory = $progressBarFactory; |
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.
Unfortunately, according to the Contribution Guide, we may not remove constructor params.
The new constructor parameters should be optional in order to avoid backward compatibility issues.
Please check the Contribution Guide (section Adding a constructor parameter) for more details.
To preserve the constructor signature:
- Remove type hinting for unused parameters to remove dependency on their type.
- Add
@SuppressWarnings(PHPMD.UnusedFormalParameter)
for unused parameters.
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.
Thank you for the review! I updated the changeset, fixed coding style errors and fixed the BIC.
Hi @dmytro-ch, thank you for the review. |
✔️ QA Passed Steps to reproduce:
Before: After: Note: I unassigned the issue #22808 because PR changes do not fix this problem! Thanks! |
Hi @tdgroot, thank you for your contribution! |
Description (*)
The resizer uses table
catalog_product_entity_media_gallery
as source table. These records contain the paths to the actual images. The problem is, these records can exist, even when they are not being used by any products. I added an inner join to tablecatalog_product_entity_media_gallery_value
, to make sure that there's a link between the image and a product.This improves performance and is less error prone, because the orphan images might not have a physical presence on the filesystem. Now the latter is an other issue, but hardening the resizer won't do any harm.
Fixed Issues
Manual testing scenarios (*)
catalog_product_entity_media_gallery
.bin/magento catalog:images:resize
.Contribution checklist (*)