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

[edenduong]

Description (*)

1. Resolve "Send Payment Failed Email Copy To" field has no validate email #24312: "Send Payment Failed Email Copy To" field has no validate email

Solution: Add "validate-emails" in app/code/Magento/Checkout/etc/adminhtml/system.xml

Fixed Issues (if relevant)

1. "Send Payment Failed Email Copy To" field has no validate email #24312: "Send Payment Failed Email Copy To" field has no validate email
2. Many field has no validate number in Checkout settings #24307: Many field has no validate number in Checkout settings
3. No validate "Proxy Port" in Paypal Module (Paypal Express, Paypal Payflow -Link, Pro, Advanced) #24304: No validate "Proxy Port" in Paypal Module (Paypal Express, Paypal Payflow -Link, Pro, Advanced)
4. No validate "Day" in PayPal Express Checkout Settings #24302: No validate "Day" in PayPal Express Checkout Settings

Manual testing scenarios (*)

1. Go to backend
2. Store->Configurations
3. Sales->Checkout
4. In "Payment Failed Email"->"Send Payment Failed Email Copy To", fill "test"
5. Save

Expected result

1. Can not save because it is not email

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]

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

[magento-engcom-team]

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

[dmytro-ch]

Hi @edenduong,
please, combine the pull requests with such improvements in one single pull request in order to make it easier to process these PRs by Community Engineering Team.
#24313
#24309
#24303
#24305
Also, feel free to create pull requests for minor fixes and obvious improvements without opening the issues.

Thank you!

[edenduong]

Hi @edenduong,
please, combine the pull requests with such improvements in one single pull request in order to make it easier to process these PRs by Community Engineering Team.
#24313
#24309
#24303
#24305
Also, feel free to create pull requests for minor fixes and obvious improvements without opening the issues.

Thank you!

Thank you for your suggestion. But I think They are different Validation and different datatype and belong different module so I don't think we should merge it to one PR.

[dmytro-ch]

@edenduong, this approach looks like cheating.
Please combine the mentioned pull requests with minor changes in one PR.
Otherwise, I will have to mark them as cleanup.

Thank you!

[swnsma]

@magento combine 24313 24309 24303 24305

[dmytro-ch]

@magento combine 24309 24303 24305

Could be a reason why combine does not work.

[swnsma]

@magento combine 24309 24303 24305

PR's need to be merged.

[sidolov]

@magento combine 24309 24303 24305

[engcom-Alfa]

Hi @edenduong

It would be nice if you added description (for ex. "Please enter at least 0 and at most 65535") under "Proxy Port" input fields, because in validation for proxy you used digits-range-0-65535. This will be clearer.

@edenduong Could you take a look, please?

Thanks!

[dmytro-ch]

@edenduong, these pull requests are still in progress.
We will check them and merge if it is reasonable.

Thank you!

[edenduong]

Hi @edenduong

It would be nice if you added description (for ex. "Please enter at least 0 and at most 65535") under "Proxy Port" input fields, because in validation for proxy you used digits-range-0-65535. This will be clearer.

@edenduong Could you take a look, please?

Thanks!

@engcom-Alfa : Thank for your feedback. I have changed the source code. Please check it again. Thanks.
@dmytro-ch: Ok . You should ask your colleague changed and it will be combined soon. It is not fair if they aren't combined and they are "improvement", only change Method name. They are Clean Up.

[sidolov]

@edenduong thank you for the input! PRs #24340 #24334 #24284 are combined to one now.
Regarding the achievement - it still may be reconsidered before the delivery to the mainline branch, we are trying to be fair with all Magento contributors, thank you for your collaboration!

[dmytro-ch]

It is not fair if they aren't combined and they are "improvement", only change Method name. They are Clean Up.

@edenduong, If you check the source code of those PRs, you will see that such refactoring requires much more efforts than it looks at first. (e.g. #24197, #24340)
Please be polite and respect other contributors.

Thank you!

[magento-engcom-team]

Hi @dmytro-ch, thank you for the review.
ENGCOM-5710 has been created to process this Pull Request

[engcom-Alfa]

✔️ QA Passed

[m2-assistant]

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