replace deprecated escaper #38135

Morgy93 · 2023-10-31T07:44:13Z

Description (*)

The getEscaper is deprecated so it has been replaced by dependency injection.

Manual testing scenarios (*)

./vendor/bin/phpunit -c dev/tests/unit/phpunit.xml.dist app/code/Magento/Cms/Test/Unit

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

m2-assistant · 2023-10-31T07:44:16Z

Hi @Morgy93. Thank you for your contribution!
Here are some useful tips on 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
Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

Morgy93 · 2023-10-31T07:45:22Z

@magento run all tests

magento-automated-testing · 2023-10-31T07:45:28Z

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.

Morgy93 · 2023-10-31T09:50:04Z

I fixed the additional phpcs errors that came up.

Morgy93 · 2023-10-31T09:51:04Z

@magento run Functional Tests B2B,Functional Tests CE,Functional Tests EE,Static Tests

magento-automated-testing · 2023-10-31T09:51:11Z

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.

Morgy93 · 2023-11-01T08:10:43Z

@magento run Database Compare,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,Semantic Version Checker

magento-automated-testing · 2023-11-01T08:10:55Z

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.

Morgy93 · 2023-11-01T12:31:58Z

@magento run Sample Data Tests CE,Sample Data Tests B2B,Sample Data Tests EE

magento-automated-testing · 2023-11-01T12:32:08Z

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

Morgy93 · 2023-11-01T12:41:37Z

@magento run Sample Data Tests CE,Sample Data Tests B2B,Sample Data Tests EE

magento-automated-testing · 2023-11-01T12:41:44Z

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

Morgy93 · 2023-11-01T12:43:29Z

What a great developer experience. 🙄

Will leave it as it is until someone gets in touch.

Den4ik

Hi @Morgy93
Thanks for your contribution.
Please follow backward compatibility documentation for adding new constructor params.

Morgy93 · 2023-11-03T08:11:36Z

Hi @Morgy93 Thanks for your contribution. Please follow backward compatibility documentation for adding new constructor params.

Interesting, that's why I see this approach used everywhere.
I don't like it but can understand it.

@Den4ik Changed the files accordingly. Please review.

magento-automated-testing · 2023-11-07T09:15:31Z

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.

hostep · 2023-11-09T16:45:52Z

app/code/Magento/Cms/Ui/Component/Listing/Column/BlockActions.php

@@ -32,24 +32,27 @@ class BlockActions extends Column
    /**
     * @var Escaper
     */
-    private $escaper;
+    protected $escaper;


What's the reason for changing this? I don't immediately see why this would be needed? I think this can be put back to private, no?

Same remark goes for the other class.

@hostep I'm agree with you, but at the same time I don't see any restrictions to use protected access type

Purely because the UrlBuilder was protected as well. I thought that is some kind of coding standard.

I'd be happy to change it to private if you agree to this change.

@Den4ik: Magento encourages composition over inheritance, if we make members protected again, we are pushing people in the direction of using inheritance again.

I would suggest we change it back to private.

@hostep It make sense 👍

@hostep Great. I updated the files accordingly. Please review.

Den4ik

✅ Approved.
@Morgy93 Thanks for contribution, changes looks good for me.

Let's run tests to get things done.
@magento run all tests

app/code/Magento/Cms/Ui/Component/Listing/Column/BlockActions.php

Den4ik · 2023-11-09T18:24:33Z

@magento run all tests

magento-automated-testing · 2023-11-09T18:24:44Z

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.

Den4ik · 2023-11-13T08:50:22Z

@magento run all tests

Run tests again

magento-automated-testing · 2023-11-13T08:50:30Z

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.

Morgy93 · 2023-12-20T10:19:30Z

@magento run all tests

magento-automated-testing · 2023-12-20T10:19:42Z

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.

Morgy93 · 2024-01-18T12:59:05Z

@magento run all tests

magento-automated-testing · 2024-01-18T12:59:14Z

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.

Morgy93 · 2024-01-18T16:04:15Z

Facebook\WebDriver\Exception\NoSuchElementException I don't think that the current errors are related to changes in this PR.

@Den4ik Any idea?

Den4ik · 2024-02-14T07:32:32Z

@magento run all tests

Den4ik · 2024-02-15T05:12:55Z

@magento run WebAPI Tests

Den4ik · 2024-02-15T18:14:53Z

@magento run Functional Tests B2B

Den4ik · 2024-02-16T08:32:39Z

I see that failed tests is not related to PR changes.
@engcom-Bravo can you please proceed this PR

Morgy93 force-pushed the replace-deprecated-escaper branch from f46a30b to fd3664e Compare October 31, 2023 07:45

Morgy93 force-pushed the replace-deprecated-escaper branch from fd3664e to 59600cd Compare October 31, 2023 09:49

engcom-Bravo added the Priority: P2 A defect with this priority could have functionality issues which are not to expectations. label Oct 31, 2023

m2-community-project bot added the Progress: pending review label Oct 31, 2023

Den4ik requested changes Nov 3, 2023

View reviewed changes

m2-community-project bot assigned Den4ik Nov 3, 2023

m2-community-project bot added Progress: needs update and removed Progress: pending review labels Nov 3, 2023

Morgy93 force-pushed the replace-deprecated-escaper branch 2 times, most recently from eff1da9 to 779eabe Compare November 3, 2023 08:09

hostep reviewed Nov 9, 2023

View reviewed changes

m2-community-project bot assigned hostep Nov 9, 2023

m2-community-project bot added Progress: review and removed Progress: needs update labels Nov 9, 2023

hostep removed their assignment Nov 9, 2023

m2-community-project bot assigned hostep Nov 9, 2023

Morgy93 force-pushed the replace-deprecated-escaper branch 3 times, most recently from c41a2a3 to 3a1cefa Compare November 9, 2023 17:32

Den4ik approved these changes Nov 9, 2023

View reviewed changes

m2-community-project bot added Progress: ready for testing and removed Progress: review labels Nov 9, 2023

Den4ik reviewed Nov 9, 2023

View reviewed changes

app/code/Magento/Cms/Ui/Component/Listing/Column/BlockActions.php Show resolved Hide resolved

m2-community-project bot added Progress: review and removed Progress: ready for testing labels Nov 9, 2023

Morgy93 force-pushed the replace-deprecated-escaper branch 3 times, most recently from 0b62db7 to 0a0341c Compare November 10, 2023 07:22

Morgy93 force-pushed the replace-deprecated-escaper branch from 0a0341c to 341601a Compare December 20, 2023 10:19

replace deprecated escaper

e767817

Morgy93 force-pushed the replace-deprecated-escaper branch from 341601a to e767817 Compare January 18, 2024 12:58

Merge branch '2.4-develop' into replace-deprecated-escaper

e2eb948

replace deprecated escaper #38135

Are you sure you want to change the base?

replace deprecated escaper #38135

Conversation

Morgy93 commented Oct 31, 2023

Description (*)

Manual testing scenarios (*)

Contribution checklist (*)

m2-assistant bot commented Oct 31, 2023

Morgy93 commented Oct 31, 2023

magento-automated-testing bot commented Oct 31, 2023

Morgy93 commented Oct 31, 2023

Morgy93 commented Oct 31, 2023

magento-automated-testing bot commented Oct 31, 2023

Morgy93 commented Nov 1, 2023

magento-automated-testing bot commented Nov 1, 2023

Morgy93 commented Nov 1, 2023

magento-automated-testing bot commented Nov 1, 2023

Morgy93 commented Nov 1, 2023

magento-automated-testing bot commented Nov 1, 2023

Morgy93 commented Nov 1, 2023

Den4ik left a comment

Choose a reason for hiding this comment

Morgy93 commented Nov 3, 2023

magento-automated-testing bot commented Nov 7, 2023

hostep Nov 9, 2023

Choose a reason for hiding this comment

Den4ik Nov 9, 2023

Choose a reason for hiding this comment

Morgy93 Nov 9, 2023

Choose a reason for hiding this comment

hostep Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Den4ik Nov 9, 2023

Choose a reason for hiding this comment

Morgy93 Nov 9, 2023

Choose a reason for hiding this comment

Den4ik left a comment

Choose a reason for hiding this comment

Den4ik commented Nov 9, 2023

magento-automated-testing bot commented Nov 9, 2023

Den4ik commented Nov 13, 2023

magento-automated-testing bot commented Nov 13, 2023

Morgy93 commented Dec 20, 2023

magento-automated-testing bot commented Dec 20, 2023

Morgy93 commented Jan 18, 2024

magento-automated-testing bot commented Jan 18, 2024

Morgy93 commented Jan 18, 2024

Den4ik commented Feb 14, 2024

Den4ik commented Feb 15, 2024

Den4ik commented Feb 15, 2024

Den4ik commented Feb 16, 2024

hostep Nov 9, 2023 •

edited

Loading