Add GraphQL mutations for Reset password for MyAccount #27876

Usik2203 · 2020-04-16T15:13:56Z

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Customer :: MyAccount :: Reset password for MyAccount

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)

m2-assistant · 2020-04-16T15:13:59Z

Hi @Usik2203. 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

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

rogyar

Hi @Usik2203. Good job. Please, check my comments below.

rogyar · 2020-04-16T17:30:44Z

app/code/Magento/CustomerGraphQl/Model/Resolver/RequestPasswordResetEmail.php

+    /**
+     * @var AccountManagementInterface
+     */
+    protected $customerAccountManagement;


Please, use the private scope for properties that are not supposed to be inherited.

It also applies to all other cases

rogyar · 2020-04-16T17:39:05Z

app/code/Magento/CustomerGraphQl/Model/Resolver/RequestPasswordResetEmail.php

+                $args['email'],
+                AccountManagement::EMAIL_RESET
+            );
+        } catch (\Exception $e) {


Just curious. Is there some unhandled exception in the flow? If it is so, we should not simply return false but throw LocalizedException instead.

But the main question is, do we have an unhandled exception somewhere in the middle?

Added LocalizedException .
Yes, we have exceptions inside $this->customerAccountManagement->resetPassword() and $this->customerAccountManagement->initiatePasswordReset() these exceptions related with

Checks if customer account is for this email

Checks match token

Checks length of password

rogyar · 2020-04-16T17:42:54Z

app/code/Magento/CustomerGraphQl/Model/Resolver/ResetPassword.php

+                $args['newPassword']
+            );
+        } catch (\Exception $e) {
+            return false;


Please, read my notice about catching the exception above.

app/code/Magento/CustomerGraphQl/etc/graphql/di.xml

rogyar · 2020-04-21T08:05:33Z

dev/tests/api-functional/testsuite/Magento/GraphQl/Customer/RequestPasswordResetEmailTest.php

+    }
+
+    /**
+     * @expectedException \Exception


Please, do not use exception annotations like expectedException and expectedExceptionMessage. This approach is deprecated in PHPUnit. Feel free to use the corresponding methods instead (self::expectException, self::expectExceptionMessage)

rogyar · 2020-04-21T08:06:43Z

Also, may a ask you to squash all your commit into a single one and perform the force-push to this branch, please?

Thank you

Usik2203 · 2020-04-22T07:43:32Z

Hi @rogyar
I have applied your recomendation about squashing commits.
Thank you.

lenaorobei · 2020-05-12T18:09:02Z

@magento run WebAPI Tests

magento-engcom-team · 2020-05-12T18:28:40Z

Hi @slavvka, thank you for the review.
ENGCOM-7473 has been created to process this Pull Request

… MyAccount #27876

m2-assistant · 2020-05-18T20:13:39Z

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

ghost added the Progress: pending review label Apr 16, 2020

magento-engcom-team added Component: CustomerGraphQl Release Line: 2.4 Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Apr 16, 2020

Usik2203 added the WIP Work In Progress label Apr 16, 2020

Usik2203 marked this pull request as draft April 16, 2020 15:15

Usik2203 changed the title ~~Feature 259 Add GraphQL mutations for Reset password for MyAccount~~ [WIP]Feature 259 Add GraphQL mutations for Reset password for MyAccount Apr 16, 2020

Usik2203 requested a review from rogyar April 16, 2020 15:16

Usik2203 mentioned this pull request Apr 16, 2020

[WIP] Feature 259 Add GraphQL mutations for Reset password for MyAccount #27873

Closed

4 tasks

rogyar self-assigned this Apr 16, 2020

rogyar requested changes Apr 16, 2020

View reviewed changes

ghost added Progress: needs update and removed Progress: pending review labels Apr 16, 2020

rogyar self-requested a review April 21, 2020 07:48

rogyar requested changes Apr 21, 2020

View reviewed changes

rogyar added Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: test coverage Award: category of expertise labels Apr 21, 2020

Add GraphQL coverage for Reset password for MyAccount

1e6c6e0

Usik2203 force-pushed the feature/259 branch from aa258d7 to 1e6c6e0 Compare April 21, 2020 10:10

rogyar changed the title ~~[WIP]Feature 259 Add GraphQL mutations for Reset password for MyAccount~~ Feature 259 Add GraphQL mutations for Reset password for MyAccount Apr 23, 2020

rogyar marked this pull request as ready for review April 23, 2020 08:52

rogyar approved these changes Apr 23, 2020

View reviewed changes

slavvka approved these changes May 12, 2020

View reviewed changes

ghost removed the Progress: needs update label May 12, 2020

ghost added Progress: ready for testing Progress: testing in progress and removed Progress: ready for testing labels May 12, 2020

ghost added Progress: accept and removed Progress: testing in progress labels May 12, 2020

slavvka pushed a commit that referenced this pull request May 18, 2020

ENGCOM-7473: Feature 259 Add GraphQL mutations for Reset password for…

aa01211

… MyAccount #27876

slavvka merged commit 14db001 into magento:2.4.0-develop May 18, 2020

keharper mentioned this pull request Jun 7, 2020

GraphQL: Document password change mutations magento/devdocs#7364

Closed

8 tasks

nrkapoor linked an issue Jun 9, 2020 that may be closed by this pull request

Customer :: MyAccount :: Reset password for MyAccount #28521

Closed

nrkapoor added the Project: GraphQL label Jun 11, 2020

atwixfirster mentioned this pull request Jul 1, 2020

magento/magento2#: Add i18 translations to requestPasswordResetEmail and resetPassword GraphQl mutations #28950

Open

5 tasks

This was referenced Aug 14, 2020

[Issue] magento/magento2#: Add i18 translations for Reset password GraphQL mutations #29548

Closed

[Issue] magento/magento2#: Add i18 translations to requestPasswordResetEmail and resetPassword GraphQl mutations #29658

Open

m2-assistant bot mentioned this pull request Aug 24, 2023

[Issue] magento/magento2#: GraphQl mutation. Additional test coverage for customer storeConfig settings. #37915

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add GraphQL mutations for Reset password for MyAccount #27876

Add GraphQL mutations for Reset password for MyAccount #27876

Usik2203 commented Apr 16, 2020 •

edited by slavvka

Loading

m2-assistant bot commented Apr 16, 2020

rogyar left a comment

rogyar Apr 16, 2020

rogyar Apr 16, 2020

Usik2203 Apr 17, 2020

rogyar Apr 16, 2020

Usik2203 Apr 17, 2020 •

edited

Loading

rogyar Apr 16, 2020

Usik2203 Apr 17, 2020

rogyar Apr 21, 2020

Usik2203 Apr 21, 2020

rogyar commented Apr 21, 2020

Usik2203 commented Apr 22, 2020

lenaorobei commented May 12, 2020

magento-engcom-team commented May 12, 2020

m2-assistant bot commented May 18, 2020

Add GraphQL mutations for Reset password for MyAccount #27876

Add GraphQL mutations for Reset password for MyAccount #27876

Conversation

Usik2203 commented Apr 16, 2020 • edited by slavvka Loading

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

m2-assistant bot commented Apr 16, 2020

rogyar left a 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

Usik2203 Apr 17, 2020 • 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

rogyar commented Apr 21, 2020

Usik2203 commented Apr 22, 2020

lenaorobei commented May 12, 2020

magento-engcom-team commented May 12, 2020

m2-assistant bot commented May 18, 2020

Usik2203 commented Apr 16, 2020 •

edited by slavvka

Loading

Usik2203 Apr 17, 2020 •

edited

Loading