Unable to insert widget with text value which contains }} string #29006

engcom-Kilo · 2020-07-06T12:45:19Z

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Fixes Unable to insert widget with text value which contains }} string #12087

Manual testing scenarios (*)

Go to admin Content -> Pages
Click Add New Page
Set Page Title to Page Title
Expand Content section
In the content field, click Insert Widget button
Choose any widget which has at least one text input, e.g. Catalog Products List
Set widget title: Title}}
Click Insert Widget
Click Save Page
Go in frontend to see the page: /page-title
Make sure that on page exist Widget with title "Title}}"

Questions or comments

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 are green)

m2-assistant · 2020-07-06T12:45:22Z

Hi @engcom-Kilo. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

@magento give me test instance - deploy test instance based on PR changes
@magento give me 2.4-develop instance - deploy vanilla Magento instance

❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE,
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests

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.

engcom-Kilo · 2020-07-06T13:09:18Z

@magento run all tests

engcom-Kilo · 2020-07-07T08:05:28Z

@magento run Static Tests

engcom-Kilo · 2020-07-07T11:41:44Z

@magento run Static Tests

engcom-Kilo · 2020-07-07T14:04:04Z

@magento run all tests

gabrieldagama

Hi @engcom-Kilo, thanks for the pull request, it looks good! Please follow a small comments on it. Also, can we cover it with MFTF tests?

Thanks!

app/code/Magento/Widget/Model/Widget.php

app/code/Magento/Widget/Block/Adminhtml/Widget/Options.php

app/code/Magento/Widget/Model/Widget.php

engcom-Charlie · 2020-08-31T07:54:23Z

@magento run all tests

gabrieldagama

Hi @engcom-Charlie, thanks for the updates here, it looks good! Pleases follow some small comments below, also, can you cover this PR with tests?

Thanks!

gabrieldagama · 2020-09-01T13:07:45Z

app/code/Magento/Widget/Model/Widget.php

@@ -131,7 +143,7 @@ public function getWidgetByClassType($type)
     */
    public function getConfigAsXml($type)
    {
-        return $this->getXmlElementByType($type);
+        return $this->getWidgetByClassType($type);


This seems BIC, changing the return type from null|\Magento\Framework\Simplexml\Element to null|array

@gabrieldagama I changed getXmlElementByType to getWidgetByClassType because such i found in obsolete_methods.

If the getWidgetByClassType is really a substitute for getXmlElementByType let's create a new method and deprecate the old one.

app/code/Magento/Widget/Model/Widget.php

engcom-Charlie · 2020-09-09T09:46:54Z

@magento run all tests

engcom-Charlie · 2020-09-09T10:33:15Z

@magento run all tests

magento-engcom-team · 2020-09-10T10:42:41Z

Hi @gabrieldagama, thank you for the review.
ENGCOM-8132 has been created to process this Pull Request
✳️ @gabrieldagama, could you please add one of the following labels to the Pull Request?

Label	Description
`Auto-Tests: Covered`	All changes in Pull Request is covered by auto-tests
`Auto-Tests: Not Covered`	Changes in Pull Request requires coverage by auto-tests
`Auto-Tests: Not Required`	Changes in Pull Request does not require coverage by auto-tests

engcom-Alfa · 2020-09-10T11:37:36Z

✔️ QA Passed

widget title: Title}}

After: ✔️ The Widget with the title "Title}}" exists on the page

widget title: {{Title

After: ✔️ The Widget with the title "{{Title" exists on the page

…} string #29006

m2-assistant · 2020-09-10T19:47:32Z

Hi @engcom-Kilo, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

magento2/issues/12087: Changes for Widget class.

7f0f260

ghost added the Progress: pending review label Jul 6, 2020

magento-engcom-team added Component: Widget Release Line: 2.4 labels Jul 6, 2020

engcom-Kilo and others added 2 commits July 6, 2020 16:07

magento2/issues/12087: Changes for Adminhtml Widget Options class.

2a48abe

Merge branch '2.4-develop' into issue12087

e0eb4b0

magento2/issues/12087: Fix static test.

cb0b3d4

magento2/issues/12087: Fix static test.

1f84319

sidolov added Priority: P1 Once P0 defects have been fixed, a defect having this priority is the next candidate for fixing. Severity: S2 Major restrictions or short-term circumventions are required until a fix is available. labels Aug 13, 2020

gabrieldagama self-assigned this Aug 14, 2020

gabrieldagama suggested changes Aug 19, 2020

View reviewed changes

app/code/Magento/Widget/Model/Widget.php Outdated Show resolved Hide resolved

ghost added Progress: needs update and removed Progress: pending review labels Aug 19, 2020

gabrieldagama suggested changes Aug 19, 2020

View reviewed changes

Merge branch '2.4-develop' into issue12087

f55eeb4

engcom-Charlie self-assigned this Aug 27, 2020

engcom-Charlie added 2 commits August 28, 2020 16:28

fix widget with special char page tittle

ae3d3df

refactor

a2346f4

gabrieldagama suggested changes Sep 2, 2020

View reviewed changes

engcom-Charlie added 2 commits September 4, 2020 15:46

minor refactor

74325ef

mftf coverage

0ac2e5d

ghost added Progress: review and removed Progress: extended testing labels Sep 8, 2020

engcom-Charlie requested review from sidolov and gabrieldagama September 8, 2020 06:46

add testCaseId

ace8a6f

revert changing the obsolete method

01b7906

engcom-Charlie force-pushed the issue12087 branch from 2233234 to 01b7906 Compare September 9, 2020 10:32

gabrieldagama approved these changes Sep 10, 2020

View reviewed changes

ghost added Progress: ready for testing and removed Progress: review labels Sep 10, 2020

ghost added Progress: testing in progress and removed Progress: ready for testing labels Sep 10, 2020

engcom-Alfa added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Sep 10, 2020

ghost added Progress: extended testing and removed Progress: testing in progress labels Sep 10, 2020

ghost added Progress: accept and removed Progress: extended testing labels Sep 10, 2020

magento-engcom-team mentioned this pull request Sep 10, 2020

Unable to insert widget with text value which contains }} string #12087

Closed

magento-engcom-team pushed a commit that referenced this pull request Sep 10, 2020

ENGCOM-8132: Unable to insert widget with text value which contains }…

1f51e88

…} string #29006

magento-engcom-team merged commit a20a955 into magento:2.4-develop Sep 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unable to insert widget with text value which contains }} string #29006

Unable to insert widget with text value which contains }} string #29006

engcom-Kilo commented Jul 6, 2020 •

edited

Loading

m2-assistant bot commented Jul 6, 2020

engcom-Kilo commented Jul 6, 2020

engcom-Kilo commented Jul 7, 2020

engcom-Kilo commented Jul 7, 2020

engcom-Kilo commented Jul 7, 2020

gabrieldagama left a comment •

edited

Loading

engcom-Charlie commented Aug 31, 2020

gabrieldagama left a comment

gabrieldagama Sep 1, 2020

engcom-Charlie Sep 2, 2020

gabrieldagama Sep 8, 2020

engcom-Charlie commented Sep 9, 2020

engcom-Charlie commented Sep 9, 2020

magento-engcom-team commented Sep 10, 2020

engcom-Alfa commented Sep 10, 2020

m2-assistant bot commented Sep 10, 2020

Unable to insert widget with text value which contains }} string #29006

Unable to insert widget with text value which contains }} string #29006

Conversation

engcom-Kilo commented Jul 6, 2020 • edited Loading

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Jul 6, 2020

engcom-Kilo commented Jul 6, 2020

engcom-Kilo commented Jul 7, 2020

engcom-Kilo commented Jul 7, 2020

engcom-Kilo commented Jul 7, 2020

gabrieldagama left a comment • edited Loading

Choose a reason for hiding this comment

engcom-Charlie commented Aug 31, 2020

gabrieldagama left a comment

Choose a reason for hiding this comment

gabrieldagama Sep 1, 2020

Choose a reason for hiding this comment

engcom-Charlie Sep 2, 2020

Choose a reason for hiding this comment

gabrieldagama Sep 8, 2020

Choose a reason for hiding this comment

engcom-Charlie commented Sep 9, 2020

engcom-Charlie commented Sep 9, 2020

magento-engcom-team commented Sep 10, 2020

engcom-Alfa commented Sep 10, 2020

m2-assistant bot commented Sep 10, 2020

engcom-Kilo commented Jul 6, 2020 •

edited

Loading

gabrieldagama left a comment •

edited

Loading