-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Replaced exception with messageManager error message due to user error and not application error #19406
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Hi @woutersamaey. Thank you for your contribution
For more details, please, review the Magento Contributor Assistant documentation |
7ba7c6e
to
93576d1
Compare
Fixed a typo, squashed commits and force pushed it. |
…r and not application error Fixed typo Added a missing space for “codacy”
93576d1
to
4412505
Compare
Added an extra space for codacy + squashed and force pushed it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@woutersamaey looks good now 👍 You fixed "}else" before I finished a comment :)
Hi @orlangur, thank you for the review. |
Hurray! |
Codacity does not like my else statement, but I don't really see a way around it. What's next @orlangur ? |
@woutersamaey TBH, this is pretty stupid rule and Codacy may be ignored in general. |
@woutersamaey i still able to reproduce the issue after apply your solution |
@Dharmeshvaja91 Okay, I saw you got a handle on this in #19404. Will leave it to you then. |
@woutersamaey Ok thank you, I will take care of it. |
Replaced exception with messageManager error message due to user error and not application error
Description (*)
When subscribing to the newsletter with an existing email address, an exception is thrown, then later caught, added as error message + logged. Since this is a user error, it should not be logged. Our log files don't need these kinds of messages.
Fixed Issues (if relevant)
Prevent the message "This email address is already subscribed." from appearing in the logs.
Manual testing scenarios (*)
Contribution checklist (*)