19404 fixed newsletter issue

[Dharmeshvaja91]

Description (*)

Fixed Issues (if relevant)

1. When subscribing to newsletter, if email already exists, don't throw exception, but just warn without exception #19404: When subscribing to newsletter, if email already exists, don't throw exception, but just warn without exception

Manual testing scenarios (*)

1. Subscribe for the news latter with already register emial

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)

[magento-engcom-team]

Hi @Dharmeshvaja91. 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-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[sivaschenko]

Thanks for your contribution. Please see the review notes. Also, please note that this issue is already fixed in #19406

[sivaschenko]

I think the throw exception on line 146 should be replaced with this messageManager call. Here several different exceptions are possible (i.e. validateEmailFormat also throws LocalizedException)

[Dharmeshvaja91]

@sivaschenko yes you are right, I will change this too.

[sivaschenko]

@Dharmeshvaja91 thanks for the contribution. Unfortunately, I have to ask you to close this pull request as it is a duplicate. The issue is already fixed in #19406

[Dharmeshvaja91]

@Dharmeshvaja91 thanks for the contribution. Unfortunately, I have to ask you to close this pull request as it is a duplicate. The issue is already fixed in #19406

@sivaschenko thank you for your comments on this i have just checked solution as per this pull request #19406 but still i m getting the same issue.

As per you instruction i have updated my code as well
please update me that should i continue work on this ?.

[sivaschenko]

Your solution (after the fix) looks better, however, the #19406 was created earlier. I propose you to help with #19406 finalization (i.e. comment that you still able to reproduce the issue and link this pull request as an example of fix). We will not be able to merge both pull requests

[Dharmeshvaja91]

@sivaschenko He have closed his pull request, so please accept my pull request.

[magento-engcom-team]

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

[magento-engcom-team]

@Dharmeshvaja91 thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

[magento-engcom-team]

Hi @Dharmeshvaja91. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.