-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Remove area emulation from media-content:sync command #29753
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
Remove area emulation from media-content:sync command #29753
Conversation
…a-content:sync command - remove area emulation
Hi @jmonteros422. Thank you for your contribution
❗ Automated tests can be triggered manually with an appropriate comment:
You can find more information about the builds here ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review. For more details, please, review the Magento Contributor Guide documentation. 🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of Pull Requests happens in the queue order. If you want to speed up the delivery of your contribution, please join the Community Contributions Triage session to discuss the appropriate ticket. 🎥 You can find the recording of the previous Community Contributions Triage on the Magento Youtube Channel ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
@magento run all tests |
Hi @BarnyShergold, thank you for the review.
|
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.
❌ QA Failed
The synchronization works properly only once. But if you clear the media_content_asset
table twice, the Used in will not display any records
Manua testing steps:
- Add some images from media gallery to the content of category, product, page and block
- Clear
media_content_asset
database table - Run
media-content:sync
command - Verify all Used In links are correctly displayed in the asset details
- Clear
media_content_asset
database table one more time - Run
media-content:sync
command - Verify all Used In links are correctly displayed in the asset details
AR: The Used in section is not displayed in image details
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.
Thanks for the PR @jmonteros422 ! Please see my comment
@@ -58,12 +48,7 @@ protected function configure() | |||
protected function execute(InputInterface $input, OutputInterface $output) | |||
{ | |||
$output->writeln('Synchronizing content with assets...'); | |||
$this->state->emulateAreaCode( |
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.
This emulation is used for Magento to generate proper links when searching for content usages. When removing the emulation - the links generation should be fixed
Pull Request state was updated. Re-review required.
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.
Request changes test
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.
Synchronization is not executed for entities that have not been updated since last sync execution, can you please update the entity before performing the second table cleanup and sync
Hi @sivaschenko, thank you for the review.
|
✔️ QA Passed Manual testing steps
The media content synchronization is covered in several scenarios on cucumber studio. There is no need to create a separate for this PR. |
Hi @jmonteros422, thank you for your contribution! |
Description (*)
This PR Removes area emulation from
\Magento\MediaContentSynchronization\Console\Command\Synchronize::execute
Fixed Issues (if relevant)
Testing
media_content_asset
database tablemedia-content:sync
commandQuestions or comments
Contribution checklist (*)