magento/magento2-page-builder#842: Template Preview Images Incorrectly Saved to Media Directory

bluemwhitew commented 1 year ago

Description (*)

This fixes an issue where Page Builder would save its preview images into the top-level media directory, as opposed to within its designated .template-manager subdirectory. It also contains some minor refactoring.

Story

N/A

Bug

N/A

Task

N/A

Fixed Issues

magento/magento2-page-builder#842: Template Preview Images Incorrectly Saved to Media Directory

Builds

N/A

Related Pull Requests

N/A

Manual Testing Scenarios (*)

[ ] Validate Saved to Directory

Click on any Page Builder-enabled field (doesn't have to be empty)
Click Save as Template
Define a Template Name (such as "Example")
Click Save
Verify that "The current contents of Page Builder has been successfully saved as a template."
Click Apply Template
Inspect the Preview Image path (can be full-size or thumbnail) of the template you just saved
Validate that the image was saved within the .template-manager directory

[ ] Validate Deleted from Directory

Follow Steps 1-8 from the "Validate Saved to Directory" scenario
Browse to Content » Templates
Open the Preview Image in a new tab ("Open Image in New Tab")
Click Delete
Verify that you see, "Are you sure you want to permanently delete template Example" (where "Example" is your Template Name)
Click OK
Verify that you see, "Template successfully deleted."
Reload the tab from Step 3 pointing to the Preview Image
Validate that it now returns a 404

Questions or Comments

Other minor improvements that can be made here include:

amending the messaging from "The current contents of Page Builder has been successfully saved as a template." to "The current contents of Page Builder have been successfully saved as a template."
reducing the complexity of the storePreviewImage method by creating helper methods which handle image naming conventions, etc.
improving instances of str_replace('.jpg', '-thumb.jpg', ...) so that other file formats (such as PNG) can be used, particularly via data patches.

Checklist

[x] Pull request has a meaningful description of its purpose
[x] All commits are accompanied by meaningful commit messages
[ ] All new or changed code is covered with unit/integration tests (if applicable)
[ ] README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
[ ] All automated tests passed successfully (all builds are green)

bluemwhitew commented 1 year ago

@magento run Integration Tests, Unit Tests

magento-automated-testing[bot] commented 1 year ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

bluemwhitew commented 1 year ago

@magento run all tests

magento-automated-testing[bot] commented 1 year ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please re-request them if they don't show in a reasonable amount of time.

bluemwhitew commented 1 year ago

@magento run all tests

magento-automated-testing[bot] commented 1 year ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

bluemwhitew commented 1 year ago

@dhaecker,

Sorry to ping directly, are these test failures already known (not related to this PR)? 🙏🏻

dhaecker commented 1 year ago

@bluemwhitew Hi, long time no see!

Those failures are definitely not expected. Looking at the builds, branches being used, etc., I THINK the issue is that magento/magento2-page-builder/ is not synced with magento-commerce/magento2-page-builder/. Looks like the commits are 1 year behind so a LOT of stuff is missing (like php 8.2 compatibility & whatever else)... I'll ask around & see how this can be fixed

dhaecker commented 1 year ago

@magento run all tests

magento-automated-testing[bot] commented 1 year ago

The requested builds are added to the queue. You should be able to see them here within a few minutes. Please message the #magento-devops slack channel if they don't show in a reasonable amount of time and a representative will look into any issues.

dhaecker commented 1 year ago

@bluemwhitew i think i fixed the issue. magento/magento2-page-builder/develop was out of sync with magento-commerce/magento2-page-builder/develop. it is now in sync. however, you now need to merge in latest develop into your branch. once you do that, i THINK the builds should work

direct message me again if there are more problems (otherwise I might miss it)