Admin Order - Email is Now Required - Magento 2.3 #22648

solwininfotech · 2019-05-04T06:17:34Z

Description (*)

Fixed Issues (if relevant)

Admin Order - Email is Now Required - Magento 2.2.6 #22251: Admin Order - Email is Now Required - Magento 2.2.6

Manual testing scenarios (*)

Sales -> Orders
Create New Order
Create New Customer
Select Store View or if Single-Store Mode it brings you to the Order Information Page
Email field is now required

Before Fix:
Customer Email is required in Admin Order Creation.

After fixed Customer Email is not required in Admin Order Creation and once order is created customer email will be created automatically by system.

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)

m2-assistant · 2019-05-04T06:17:36Z

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

magento-cicd2 · 2019-05-04T06:17:36Z

All committers have signed the CLA.

solwininfotech · 2019-05-04T06:46:08Z

@magento-engcom-team give me test instance

magento-engcom-team · 2019-05-04T06:46:11Z

Hi @solwininfotech. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-05-04T06:47:03Z

Hi @solwininfotech, here is your new Magento instance.
Admin access: https://pr-22648.instances.magento-community.engineering/admin
Login: admin Password: 123123q

rogyar · 2019-05-05T09:26:16Z

Approving this one.

Basically, this PR reverts the fix provided in b1eece2#diff-b79fbcef70631d52e90840aaf9a0d94d .

This fix results in confusion since the email auto-generation upon guest order creation from admin panel were considered as a useful feature. And the fix prevents store owners from using the expected behavior. Check the original issue for details.

magento-engcom-team · 2019-05-05T09:27:32Z

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

sdzhepa · 2019-05-07T20:59:12Z

Hello

@solwininfotech, @trucatchtraps, @rogyar @driskell

Thank you for contribution and collaboration!

Unfortunately, these changes will not be approved by Magento internal team.

Changes from b1eece2 were made by the Core team intentionally.
It was based on several reports and escalations/complaints from different clients through Magento Support channel.
The main idea was that specific of their businesses did not allow to accept any auto-generated email addresses in the case when field is empty(previous behavior).

But after reading all argumentation from issue description #22251
I see that could be different cases based on business needs

So, after several internal discussion, I propose the next solution that should cover both cases:

❗ NOTE:

Due to several requests, I want to mention that proposed solution and each requirement were
discussed and approved with Magento Product Manager @chernenm

"It should be a configurable option in Configuration" with next requirements:

The option should be as drop-down element with values Yes/No
The option should be System > Configuration > CUSTOMERS > Customer Configuration section Create New Account Options. Between "Show VAT Number on Storefront" and "Default Email Domain"
Element Title: "Email is required field for Admin order creation"
Hint message under field: "If set YES Email field will be required during Admin order creation for new Customer."
Element scope: store view
Default value = No
The option should affect only Order creation flow for a new customer from Admin side and should not be any impact on other functionality(like Customer creation flow or so)

When option set to "No" Magento should work as it was before b1eece2
When option set to "Yes" Magento should work as now, after

solwininfotech · 2019-05-08T08:57:36Z

@sdzhepa Got your point, we will add the settings in the configuration, thank you for your feedback.

solwininfotech · 2019-05-08T11:56:44Z

@sdzhepa, I have updated the code.
Please view the screens for.

Email required setting in the configuration.
Create order when Email required setting is No
Auto-generated email once "Create order when Email required setting is No".
Email required setting is Yes in the configuration.
Email required validation for customer email address once "Create order when Email required setting is Yes".
Admin entered customer email once "Create order when Email required setting is Yes".

solwininfotech · 2019-05-08T11:57:20Z

@magento-engcom-team give me test instance

magento-engcom-team · 2019-05-08T11:57:22Z

Hi @solwininfotech. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-05-08T11:58:28Z

Hi @solwininfotech, here is your new Magento instance.
Admin access: https://pr-22648.instances.magento-community.engineering/admin
Login: admin Password: 123123q

solwininfotech · 2019-05-08T12:12:26Z

@magento-engcom-team
Testing is done all works fine.

sdzhepa · 2019-05-08T17:38:34Z

@solwininfotech
Thank you for so quick reaction and code changes!

solwininfotech · 2019-05-09T04:57:41Z

Hi @sdzhepa @rogyar

Semantic Version Checker build is filed with minor level because of the following reason.
M071 Constant has been added.
V015 [public] Method has been added.

Can you help me what I need to fix and how or it is okay because of new things are added?

solwininfotech · 2019-05-28T10:08:35Z

@sidolov , Thank you for reviewing this thread let me know I need to update in this.

ghost · 2019-06-19T08:51:38Z

@magento-cicd2 unfortunately, only members of the maintainers team are allowed to unassign developers from the pull request

trucatchtraps · 2019-06-20T22:33:46Z

I finally was able to remove the required * for the email address with a custom module. But now when I try to place an order without an email address, I receive the following errors.

The value of attribute "Email" must be set
Validation is failed.

solwininfotech · 2019-06-26T08:25:16Z

@magento-engcom-team give me test instance

magento-engcom-team · 2019-06-26T08:25:17Z

Hi @solwininfotech. Thank you for your request. I'm working on Magento instance for you

magento-engcom-team · 2019-06-26T08:26:03Z

Hi @solwininfotech, here is your new Magento instance.
Admin access: https://pr-22648.instances.magento-community.engineering/admin
Login: admin Password: 123123q

solwininfotech · 2019-06-26T11:35:24Z

Hi @trucatchtraps

Changes in this branch will be more helpful to you because of this code does not has an error [The value of attribute "Email" must be set Validation is failed.] as well as it has an admin setting that allows you to add an order with or without a customer email address .

Click here for more details.

Thank you.

sidolov · 2019-08-07T19:45:12Z

app/code/Magento/Sales/Block/Adminhtml/Order/Create/Form/Account.php

@@ -39,6 +39,8 @@ class Account extends AbstractForm
     */
    protected $_extensibleDataObjectConverter;

+    const XML_PATH_EMAIL_REQUIRED_CREATE_ORDER = 'customer/create_account/email_required_create_order';


Please, make constant private

Needs a PHPDOC too

sidolov · 2019-08-07T19:45:26Z

app/code/Magento/Sales/Block/Adminhtml/Order/Create/Form/Account.php

+     *
+     * @return bool
+     */
+    public function isEmailRequiredCreateOrder()


Please, make method private

sidolov · 2019-08-07T19:45:32Z

app/code/Magento/Sales/Model/AdminOrder/Create.php

@@ -33,6 +33,7 @@ class Create extends \Magento\Framework\DataObject implements \Magento\Checkout\
     */
    const XML_PATH_DEFAULT_EMAIL_DOMAIN = 'customer/create_account/email_domain';

+    const XML_PATH_EMAIL_REQUIRED_CREATE_ORDER = 'customer/create_account/email_required_create_order';


Please, make constant private

Judging by the current code not obeying such practice I would suggest that change be made in a separate PR aimed at resolving this in the entire class.

Having said that - I notice the PHPDOC is missing for this constant.

@driskell we can't make existing constants as private due to backward compatibility, but the new one should be private.

sidolov · 2019-08-07T19:46:36Z

app/code/Magento/Sales/Model/AdminOrder/Create.php

@@ -2029,7 +2030,30 @@ protected function _validate()
     */
    protected function _getNewCustomerEmail()
    {
-        return $this->getData('account/email');
+        $email_required = $this->_scopeConfig->getValue(


Please, avoid underscore in the variable naming

As above, this isn’t obeyed by the remaining code and likely require refactor of unrelated code, and so would increase scope of the PR. I’d suggest to pick this up in a separate PR that resolves this for all properties in the class.

@driskell it's not a big deal to rename variables in the modern IDE, it requires an additional commit to make code style better.

Could it be a problem that it is protected and not private? I worry further change here might delay the PR merge due to unrelated opportunistic change. As it stands I’m happy to help with the other changes but this one feels like it needs a little more thought and a separately considered and reversible changeset so this feature isn’t lost should an issue be raised.

sidolov · 2019-08-13T20:55:05Z

app/code/Magento/Sales/Model/AdminOrder/Create.php

@@ -33,6 +33,7 @@ class Create extends \Magento\Framework\DataObject implements \Magento\Checkout\
     */
    const XML_PATH_DEFAULT_EMAIL_DOMAIN = 'customer/create_account/email_domain';

+    const XML_PATH_EMAIL_REQUIRED_CREATE_ORDER = 'customer/create_account/email_required_create_order';


@driskell we can't make existing constants as private due to backward compatibility, but the new one should be private.

sidolov · 2019-08-13T20:57:06Z

app/code/Magento/Sales/Model/AdminOrder/Create.php

@@ -2029,7 +2030,30 @@ protected function _validate()
     */
    protected function _getNewCustomerEmail()
    {
-        return $this->getData('account/email');
+        $email_required = $this->_scopeConfig->getValue(


@driskell it's not a big deal to rename variables in the modern IDE, it requires an additional commit to make code style better.

solwininfotech · 2019-08-16T13:35:43Z

Thank you for updated, I am working on this.

sidolov · 2019-09-04T16:57:09Z

@solwininfotech , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

m2-assistant · 2019-09-04T16:57:10Z

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

trucatchtraps · 2019-09-04T22:41:51Z

So will these changes be implemented in a new version?

solwininfotech · 2019-09-06T07:27:07Z

Hi @sidolov
How are you?
Not sure after remote upstream Magento 2 git repo, my this branch was stopped working, I tried all the possible cases but it was not worked so I have done all the changes in a new branch and created a pull request for that. Is that possible you can review it. Here is the branch pull request #24479.

Thank you.

Admin Order - Email is Now Required - Magento 2.3

7e0be95

magento-engcom-team added Component: Sales Release Line: 2.3 labels May 4, 2019

rogyar self-assigned this May 5, 2019

rogyar approved these changes May 5, 2019

View reviewed changes

ghost added the Progress: ready for testing label May 5, 2019

VasylShvorak self-assigned this May 7, 2019

ghost added Progress: testing in progress and removed Progress: ready for testing labels May 7, 2019

sdzhepa added the Progress: needs update label May 7, 2019

ghost removed the Progress: testing in progress label May 7, 2019

sdzhepa self-assigned this May 7, 2019

solwininfotech added 2 commits May 8, 2019 16:22

Added Email Required Setting In Admin

1243b8a

Removed ScopeInterface

b9e7d1d

sdzhepa removed the Progress: needs update label May 9, 2019

sdzhepa requested a review from sidolov May 10, 2019 15:21

magento-cicd2 unassigned VasylShvorak Jun 19, 2019

rogyar removed their assignment Aug 7, 2019

sidolov requested changes Aug 7, 2019

View reviewed changes

ghost assigned sidolov Aug 7, 2019

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

sidolov requested changes Aug 13, 2019

View reviewed changes

sidolov closed this Sep 4, 2019

swnsma mentioned this pull request Sep 9, 2019

22251 admin order email is now required 1 #24479

Merged

4 tasks

trucatchtraps mentioned this pull request Mar 6, 2020

Admin Order - Email is Now Required - Magento 2.2.6 #22251

Closed

Admin Order - Email is Now Required - Magento 2.3 #22648

Admin Order - Email is Now Required - Magento 2.3 #22648

Uh oh!

Conversation

solwininfotech commented May 4, 2019

Description (*)

Fixed Issues (if relevant)

Manual testing scenarios (*)

Contribution checklist (*)

Uh oh!

m2-assistant bot commented May 4, 2019

Uh oh!

magento-cicd2 commented May 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solwininfotech commented May 4, 2019

Uh oh!

magento-engcom-team commented May 4, 2019

Uh oh!

magento-engcom-team commented May 4, 2019

Uh oh!

rogyar commented May 5, 2019

Uh oh!

magento-engcom-team commented May 5, 2019

Uh oh!

sdzhepa commented May 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

solwininfotech commented May 8, 2019

Uh oh!

solwininfotech commented May 8, 2019

Uh oh!

solwininfotech commented May 8, 2019

Uh oh!

magento-engcom-team commented May 8, 2019

Uh oh!

magento-engcom-team commented May 8, 2019

Uh oh!

solwininfotech commented May 8, 2019

Uh oh!

sdzhepa commented May 8, 2019

Uh oh!

solwininfotech commented May 9, 2019

Uh oh!

solwininfotech commented May 28, 2019

Uh oh!

ghost commented Jun 19, 2019

Uh oh!

trucatchtraps commented Jun 20, 2019

Uh oh!

solwininfotech commented Jun 26, 2019

Uh oh!

magento-engcom-team commented Jun 26, 2019

Uh oh!

magento-engcom-team commented Jun 26, 2019

Uh oh!

solwininfotech commented Jun 26, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

driskell Aug 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

magento-cicd2 commented May 4, 2019 •

edited

Loading

sdzhepa commented May 7, 2019 •

edited

Loading

driskell Aug 8, 2019 •

edited

Loading