magento/magento2#: Captcha. Improvement. Replace deprecated addError with addErrorMessage. #24340

atwixfirster · 2019-08-28T13:24:13Z

Description (*)

Pull request replaces deprecated addError method with a new one - addErrorMessage in the Magento_Captcha module.

Fixed Issues (if relevant)

magento/magento2#<issue_number>: Issue title

Manual testing scenarios (*)

...
...

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)

Thank you!

m2-assistant · 2019-08-28T13:24:15Z

Hi @atwixfirster. 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.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

southerncomputer · 2019-08-28T18:04:31Z

Why not replace all AddSuccess, AddError, and AddException calls to new format?

ihor-sviziev

Hi @atwixfirster,
Could you review and fix failing static tests?

app/code/Magento/Captcha/Observer/CheckUserLoginObserver.php

ihor-sviziev · 2019-08-29T12:23:51Z

@magento run all

sidolov · 2019-08-29T15:18:25Z

@magento combine 24334 24284

magento-engcom-team · 2019-08-29T15:18:27Z

Hi @sidolov. Thank you for your request. I'm working on combining the pull requests for you

magento-engcom-team · 2019-08-29T15:20:18Z

@atwixfirster all pull requests have been successfully combined together:

merged magento/magento2#: Replace deprecated Magento\Customer\Model\Customer::isConfirmationRequired method #24334: magento/magento2#: Replace deprecated Magento\Customer\Model\Customer::isConfirmationRequired method
merged magento/magento2#: Replace deprecated addError, addSuccess, addException methods in Magento/Customer/Controller/Account/Confirmation.php #24284: magento/magento2#: Replace deprecated addError, addSuccess, addException methods in Magento/Customer/Controller/Account/Confirmation.php

dmytro-ch

@atwixfirster , could you please also check the issue mentioned id the "child" PR #24334.
Thank you!

ihor-sviziev · 2019-09-02T04:53:44Z

@magento run all tests

ihor-sviziev

Hi @atwixfirster,
Could you review my comments?
Also there is some issues with tests for this PR, I think we can ignore these issues

ihor-sviziev · 2019-09-02T19:50:48Z

app/code/Magento/Customer/Model/Customer.php

@@ -486,7 +492,8 @@ public function getAddressById($addressId)
     * Getting customer address object from collection by identifier
     *
     * @param int $addressId
-     * @return Address
+     * @return \Magento\Framework\DataObject


Isn’t it returns Address object? Looks like incorrect change

Yes, it returns Address object.

ihor-sviziev · 2019-09-02T19:53:23Z

app/code/Magento/Customer/Model/Customer.php

@@ -537,7 +545,8 @@ public function getAddresses()
    /**
     * Retrieve all customer attributes
     *
-     * @return Attribute[]
+     * @return array


It’s better to keep original return statement there. Or it was incorrect?

Yes, the origin return statement is correct.

Thanks, @ihor-sviziev

ihor-sviziev · 2019-09-02T19:54:27Z

app/code/Magento/Customer/Model/ResourceModel/Customer.php

     */
-    protected $_validatorFactory;
+    protected $dateTime;


Why did you moved it above?

Sort in alphabetical order ;) I can revert this change if you request it. Thanks

It’s better to keep only needed changes in order to prevent conflicts, so please revert it.

yes. Now I understand this. Thank you

ihor-sviziev · 2019-09-02T19:56:13Z

app/code/Magento/Customer/Model/Session.php

@@ -175,9 +182,11 @@ public function __construct(
            $cookieMetadataFactory,
            $appState
        );
+        $this->_eventManager->dispatch('customer_session_init', ['customer_session' => $this]);


Why you move it above?

Sort in alphabetical order ;)

I was wrong in this case... Thank you for notice

It’s better to keep only needed changes in order to prevent conflicts, so please revert it.

magento-engcom-team · 2019-09-03T10:19:40Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-5753 has been created to process this Pull Request

engcom-Alfa · 2019-09-12T09:29:44Z

app/code/Magento/Customer/Controller/Account/Confirmation.php

 use Magento\Customer\Model\Url;
+use Magento\Framework\App\Action\HttpGetActionInterface as HttpGetActionInterface;
+use Magento\Framework\App\Action\HttpPosttActionInterface as HttpPostActionInterface;


You made spelling mistake in HttpPosttActionInterface . Please, delete the extra letter t

Good catch 👍

Good catch, @engcom-Alfa !

Thank you!

engcom-Alfa

You made spelling mistake in HttpPosttActionInterface . Please, delete the extra letter t

…with addErrorMessage. magento/magento2#: Replace deprecated Magento\Customer\Model\Customer::isConfirmationRequired method magento/magento2#: Replace deprecated addError, addSuccess, addException methods in Magento/Customer/Controller/Account/Confirmation.php

atwixfirster · 2019-09-12T09:43:51Z

You made spelling mistake in HttpPosttActionInterface . Please, delete the extra letter t

fixed

magento-engcom-team · 2019-09-12T09:58:41Z

Hi @ihor-sviziev, thank you for the review.
ENGCOM-5753 has been created to process this Pull Request

engcom-Alfa · 2019-09-13T07:26:23Z

✔️ QA Passed

…ted addError with addErrorMessage. #24340

m2-assistant · 2019-09-14T18:49:42Z

Hi @atwixfirster, 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.

magento-engcom-team added Component: Captcha Release Line: 2.3 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Aug 28, 2019

atwixfirster force-pushed the captcha-replace-deprecated-addError branch from 1a49cf8 to 45de309 Compare August 28, 2019 14:50

ihor-sviziev requested changes Aug 28, 2019

View reviewed changes

ghost assigned ihor-sviziev Aug 28, 2019

ghost added the Progress: needs update label Aug 28, 2019

ihor-sviziev added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Aug 28, 2019

atwixfirster force-pushed the captcha-replace-deprecated-addError branch 2 times, most recently from 8469797 to ee06632 Compare August 29, 2019 06:03

ihor-sviziev requested changes Aug 29, 2019

View reviewed changes

app/code/Magento/Captcha/Observer/CheckUserLoginObserver.php Show resolved Hide resolved

atwixfirster force-pushed the captcha-replace-deprecated-addError branch from ee06632 to 0a31fd4 Compare August 29, 2019 06:50

edenduong mentioned this pull request Aug 29, 2019

magento/magento2#: Replace deprecated methods in back-end controllers of Magento/Customer #24285

Merged

4 tasks

sidolov mentioned this pull request Aug 29, 2019

Resolve "Send Payment Failed Email Copy To" field has no validate email issue24312 #24313

Merged

4 tasks

dmytro-ch self-assigned this Aug 29, 2019

dmytro-ch requested changes Aug 29, 2019

View reviewed changes

ihor-sviziev requested changes Sep 2, 2019

View reviewed changes

atwixfirster force-pushed the captcha-replace-deprecated-addError branch from d490c6e to 5de9fdf Compare September 3, 2019 05:36

engcom-Alfa self-assigned this Sep 12, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels Sep 12, 2019

engcom-Alfa reviewed Sep 12, 2019

View reviewed changes

engcom-Alfa requested changes Sep 12, 2019

View reviewed changes

ghost added Progress: needs update and removed Progress: testing in progress labels Sep 12, 2019

atwixfirster force-pushed the captcha-replace-deprecated-addError branch from 3281416 to 41a35bd Compare September 12, 2019 09:41

atwixfirster requested review from kokoc and paliarush as code owners September 12, 2019 09:41

engcom-Alfa requested a review from ihor-sviziev September 12, 2019 09:56

ihor-sviziev approved these changes Sep 12, 2019

View reviewed changes

ghost added Progress: ready for testing Progress: testing in progress and removed Progress: needs update Progress: ready for testing labels Sep 12, 2019

ghost added Progress: extended testing and removed Progress: testing in progress labels Sep 13, 2019

magento#24340: Static test fix.

8cbc374

ghost added Progress: accept and removed Progress: extended testing labels Sep 13, 2019

magento-engcom-team pushed a commit that referenced this pull request Sep 14, 2019

ENGCOM-5753: magento/magento2#: Captcha. Improvement. Replace depreca…

5dd29c8

…ted addError with addErrorMessage. #24340

magento-engcom-team merged commit 8cbc374 into magento:2.3-develop Sep 14, 2019

magento-engcom-team added this to the Release: 2.3.4 milestone Sep 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

magento/magento2#: Captcha. Improvement. Replace deprecated addError with addErrorMessage. #24340

magento/magento2#: Captcha. Improvement. Replace deprecated addError with addErrorMessage. #24340

atwixfirster commented Aug 28, 2019

m2-assistant bot commented Aug 28, 2019

southerncomputer commented Aug 28, 2019

ihor-sviziev left a comment

ihor-sviziev commented Aug 29, 2019

sidolov commented Aug 29, 2019

magento-engcom-team commented Aug 29, 2019

magento-engcom-team commented Aug 29, 2019

dmytro-ch left a comment

ihor-sviziev commented Sep 2, 2019

ihor-sviziev left a comment •

edited

Loading

ihor-sviziev Sep 2, 2019

atwixfirster Sep 3, 2019

ihor-sviziev Sep 2, 2019

atwixfirster Sep 3, 2019

ihor-sviziev Sep 2, 2019

atwixfirster Sep 3, 2019

ihor-sviziev Sep 3, 2019

atwixfirster Sep 3, 2019

ihor-sviziev Sep 2, 2019

atwixfirster Sep 3, 2019 •

edited

Loading

ihor-sviziev Sep 3, 2019

atwixfirster Sep 3, 2019

magento-engcom-team commented Sep 3, 2019

engcom-Alfa Sep 12, 2019

ihor-sviziev Sep 12, 2019

atwixfirster Sep 12, 2019

engcom-Alfa left a comment

atwixfirster commented Sep 12, 2019

magento-engcom-team commented Sep 12, 2019

engcom-Alfa commented Sep 13, 2019

m2-assistant bot commented Sep 14, 2019

magento/magento2#: Captcha. Improvement. Replace deprecated addError with addErrorMessage. #24340

magento/magento2#: Captcha. Improvement. Replace deprecated addError with addErrorMessage. #24340

Conversation

atwixfirster commented Aug 28, 2019

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Aug 28, 2019

southerncomputer commented Aug 28, 2019

ihor-sviziev left a comment

Choose a reason for hiding this comment

ihor-sviziev commented Aug 29, 2019

sidolov commented Aug 29, 2019

magento-engcom-team commented Aug 29, 2019

magento-engcom-team commented Aug 29, 2019

dmytro-ch left a comment

Choose a reason for hiding this comment

ihor-sviziev commented Sep 2, 2019

ihor-sviziev left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

atwixfirster Sep 3, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magento-engcom-team commented Sep 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

engcom-Alfa left a comment

Choose a reason for hiding this comment

atwixfirster commented Sep 12, 2019

magento-engcom-team commented Sep 12, 2019

engcom-Alfa commented Sep 13, 2019

m2-assistant bot commented Sep 14, 2019

ihor-sviziev left a comment •

edited

Loading

atwixfirster Sep 3, 2019 •

edited

Loading