Skip to content

Conversation

@adrian-martinez-interactiv4
Copy link
Contributor

When saving custom layout update xml in CMS page, in production mode, xml is not validated against schema file, due to \Magento\Framework\App\Arguments\ValidationState::isValidationRequiredmethod:

    /**
     * Retrieve current validation state
     *
     * @return boolean
     */
    public function isValidationRequired()
    {
        return $this->_appMode == \Magento\Framework\App\State::MODE_DEVELOPER;
    }

Actual result:
captura de pantalla 2017-10-30 a las 3 46 56
captura de pantalla 2017-10-30 a las 3 47 15

Desired behaviour would be check the layout update xml against the schema, and show error message to the user if it is invalid:
captura de pantalla 2017-10-30 a las 3 58 10

Description

  • \Magento\Cms\Model\Page\DomValidationState has been added, and it is injected from post data processor to force layout update xml validation, only upon cms page save.

When this PR is applied and validation is performed, application may crush due to unhandled exception. This issue is solved by PR #11857

Manual testing scenarios

  1. Enable production mode
  2. Edit or create a CMS page, enter some invalid xml in Layout Update XML textarea
  3. Try to save the page

Contribution 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)
  • All automated tests passed successfully (all builds on Travis CI are green)

@dmanners
Copy link
Contributor

@adrian-martinez-interactiv4 a couple of Test failures to sort out for us please.

@dmanners dmanners self-assigned this Oct 30, 2017
@dmanners dmanners added this to the October 2017 milestone Oct 30, 2017
@adrian-martinez-interactiv4
Copy link
Contributor Author

Changes of this PR included in PR #11857

@adrian-martinez-interactiv4 adrian-martinez-interactiv4 deleted the FR22#CMS-PAGE-VALIDATE-LAYOUT-UPDATE-XML-PRODUCTION-MODE branch October 30, 2017 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants