Skip to content
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

Broken CMS block preview when using identifier in pagebuilder #812

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

rvr-robert
Copy link

Description (*)

When adding a CMS block in a CMS page using the block identifier instead of the block ID, pagebuilder displays following error : "Block with ID: [block identifier] doesn't exist." instead of showing preview of the CMS block.

The block renders properly on front-end.
There is no way to add a block with his identifier directly from pagebuilder. This issue would only happen if a developer executes a data patch that generates CMS content or directly edit the content in the database.

Bug

  • <2838> Broken CMS block preview when using identifier in pagebuilder

Fixed Issues (if relevant)

  1. Broken CMS block preview when using identifier in pagebuilder magento2#35070: Broken CMS block preview when using identifier in pagebuilder

Manual testing scenarios (*)

  1. Create a CMS Block with any content
  2. Create a CMS Page and add previously created CMS Block in the page with the PageBuilder Block component
  3. Open cms_page table
  4. Find the created page
  5. Replace block_id value with the block identifier in the content cell. Example for a block with identifier contact-us-banner :
  6. {{widget type="Magento\Cms\Block\Widget\Block" template="widget/static_block/default.phtml" block_id="contact-us-banner" type_name="CMS Static Block"}}
  7. Apply changes
  8. Refresh pagebuilder page to see error

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)

@rvr-robert
Copy link
Author

@magento run all test

@magento-automated-testing
Copy link

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

@engcom-Hotel
Copy link
Collaborator

@magento run all tests

Copy link

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.

@engcom-Hotel engcom-Hotel self-requested a review January 10, 2024 09:59
Copy link
Collaborator

@engcom-Hotel engcom-Hotel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @rvr-robert,

Thanks for the collaboration!

Please update the PR branch with the latest changes in the master branch. Then we will pick this PR for review. It seems the failures are due to this only.

Thanks

@engcom-Hotel
Copy link
Collaborator

@magento run all tests

@engcom-Hotel
Copy link
Collaborator

Hello @rvr-robert @jeremycharron,

Thanks for the contribution!

Can you please help us understand the need to update the database in order to reproduce the issue? I think this is not the correct way to produce an issue. Can we reproduce the issue without this?

Thanks

@jeremycharron
Copy link

Hello @rvr-robert @jeremycharron,

Thanks for the contribution!

Can you please help us understand the need to update the database in order to reproduce the issue? I think this is not the correct way to produce an issue. Can we reproduce the issue without this?

Thanks

Hi @engcom-Hotel
The real concern would be when you are creating a CMS block and assigning it to a page programmatically (in a data patch for example). You'd want the created page to use the same block identifier on every environment the patch is ran.
Editing the database directly is just a quick way to replicate the issue without writing a fictional data patch.

Thanks

@engcom-Hotel
Copy link
Collaborator

@magento run all tests

Copy link
Collaborator

@engcom-Hotel engcom-Hotel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @rvr-robert,

Thanks for the contribution!

Please refer to the below review comment and fix those. Also please look into the failed tests.

Thanks

['block_id', 'identifier'],
[
['eq' => $params['block_id']],
['eq' => $params['block_id']]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me a typo, please review this condition ['eq' => $params['block_id']] is repeating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Changes Requested
Development

Successfully merging this pull request may close these issues.

3 participants