Block preview in page builder broken for blocks that do not belong in the default strore

rostilos commented 1 year ago

Description (*)

If we insert in the PageBuilder editor a block which is configured for any store view other than default (or the first available if there is no default), the CMS block content will not be loaded in the preview. This happens because the environment emulation is run for the default store id and if the CMS block is configured for another store view, the html content will not be rendered for it

Task

Fixed Issues (if relevant)

https://github.com/magento/magento2/issues/36383

Manual testing scenarios (*)

Have a magento instance with at least two stores
Create a cms block only visible in the store that is not the default store
Create a cms page in on of the two stores (doesn't matter which one)
Add the cms block you created, in the page through the page builder.
The previews should be displayed correctly

Checklist

[ ] Pull request has a meaningful description of its purpose
[ ] 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)

rostilos 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.

rostilos commented 1 year ago

@magento run Functional Tests B2B Functional Tests CE

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

Failed to run the builds. Please try to re-run them later.

rostilos commented 1 year ago

@magento run Functional Tests B2B

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.

rostilos commented 1 year ago

@magento run Functional Tests CE

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.

rostilos commented 1 year ago

@magento run Functional Tests CE

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.

rostilos commented 1 year ago

@magento run Functional Tests CE

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.

paras89 commented 1 year ago

@rostilos - this solution needs to be refined more since in the current implementation there are some concerns:

The change impacts not just cms blocks but other entities like dynamic blocks etc. too
In case a CMS block is assigned to the default store, the merchant will see the default store's theme preview for the block content. Which may not be a good experience.

A better solution might be to pass back a flag from the front-end to preview controller if the given cms block is not assigned to default store, and only in this case do we pass storeId in second param to Magento\PageBuilder\Model\Stage\Preview::startPreviewMode this way we avoid change behavior where not needed.

rostilos commented 1 year ago

@paras89 I agree, but what about the cases where the selected CMS block is not assigned to the default store, but to the other two? Which value of storeId of the two to which it is assigned should be passed to the controller as a parameter for emulation, the first of the available? Also wanted to ask: would it be better to check whether the CMS block is assigned to default store before sending request to preview controller, and pass the parameter (store_id) only in this case? Or to pass additional block_id parameter to controller and then check which stores it is assigned to?

engcom-Hotel commented 6 months ago

Hello @rostilos,

Thanks for the contribution!

I agree here with @paras89, as per the implementation it will always be considered the Store::DEFAULT_STORE_ID and show a preview related to it which could not be a good experience.

And there is still an open point which has been made by you here:

@paras89 I agree, but what about the cases where the selected CMS block is not assigned to the default store, but to the other two? Which value of storeId of the two to which it is assigned should be passed to the controller as a parameter for emulation, the first of the available?

I think we need to consider PO's suggestion here. But before that can you please elaborate on the below point:

Also wanted to ask: would it be better to check whether the CMS block is assigned to default store before sending request to preview controller, and pass the parameter (store_id) only in this case? Or to pass additional block_id parameter to controller and then check which stores it is assigned to?

Meanwhile moving this PR on hold.

Thanks

engcom-Hotel commented 4 months ago

Hello @rostilos,

Have you had a chance to check this comment https://github.com/magento/magento2-page-builder/pull/833#issuecomment-1870158941?

engcom-Hotel commented 4 months ago

Hello @rostilos,

Thank you for your contribution!

As we have been unable to get any reply from your side for a long time, we are closing this PR for now.

Whenever you are ready to take this up please free to reopen it or ask us to reopen the same. We will be happy to do that.

Thanks again!

magento / magento2-page-builder