-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
$order->getCustomer() returns NULL for registered customer #25268
Comments
Hi @qsolutions-pl. Thank you for your report.
Please make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, please, add a comment to the issue:
For more details, please, review the Magento Contributor Assistant documentation. @qsolutions-pl do you confirm that you were able to reproduce the issue on vanilla Magento instance following steps to reproduce?
|
Hi @itsPranith. Thank you for working on this issue.
|
Any update on this? |
✅ Confirmed by @itsPranith Issue Available: @itsPranith, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself. |
Hi @sudheer-gajjala. Thank you for working on this issue.
|
@qsolutions-pl in this event we are not getting customer object so instead of $order->getCustomer() use like $order->getCustomerFirstname() for customer first name |
honestly @mohith227 that is not an answer nor a solution, I need customer object to fetch other customer data like:
|
Hi @rajeebkumar-sahoo. Thank you for working on this issue.
|
Hi @qsolutions-pl @itsPranith @rajeebkumar-sahoo, Unfortunately adding new method to existing functionality will break backward compatibility, more details: https://devdocs.magento.com/guides/v2.3/contributor-guide/backward-compatible-development/ Why do you think order model should have method "getCustomer"? As for me it shoudn't as it's not his responsibility. It should just contain getCustomerId method or something like this, and it is: magento2/app/code/Magento/Sales/Model/Order.php Lines 2744 to 2747 in bff0d38
If you need to have customer - you can get it from customer repository. I would mark it as not an issue |
Hi @ihor-sviziev, The call to "$order->getCustomer()" as mentioned in the issue does not raise any PHP error saying that "method does not exist on object" but it returns NULL. So the method already exists and the functionality provided on "Order" model class as a magic method like The solution to this could be either we provide logic on "__call" by overriding it in "\Magento\Sales\Model\Order" or the way I did it by providing a direct function. I choose the second method because overriding "__call" would be only possible by introducing static text comparison like "if ($method === 'getCustomer')", which is not appropriate. |
In this case would be better to update phpdoc block that will show that
return type for getCustomer() could be also null. Is it sounds reasonable
to you?
…On Thu, 28 Nov 2019 at 05:09, Rajeeb Kumar Sahoo ***@***.***> wrote:
Hi @ihor-sviziev <https://github.com/ihor-sviziev>,
The call to "$order->getCustomer()" as mentioned in the issue does not
raise any PHP error saying that "method does not exist on object" but it
returns NULL. So the method already exists and the functionality provided
on "Order" model class as a magic method like
***@***.*** <https://github.com/method> \Magento\Customer\Model\Customer
getCustomer()". And the magic method gets resolved in
"\Magento\Framework\DataObject::__call" where it doesn't find any key named
"customer" and hence returns NULL which violets the method signature
defined like ***@***.*** <https://github.com/method>
\Magento\Customer\Model\Customer getCustomer()".
The solution to this could be either we provide logic on "__call" by
overriding it in "\Magento\Sales\Model\Order" or the way I did it by
providing a direct function. I choose the second method because overriding
"__call" would be only possible by introducing static text comparison like
"if ($method === 'getCustomer')", which is not appropriate.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#25268?email_source=notifications&email_token=AAOJOUK7OBY4N4TFE47BLL3QV4Y5ZA5CNFSM4JETGHJ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFLJUEY#issuecomment-559323667>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAOJOUMVMDI774WMXNI6RA3QV4Y5ZANCNFSM4JETGHJQ>
.
|
HI @ihor-sviziev @qsolutions-pl @itsPranith, Yes, that sounds reasonable and also we should mark the magic method as deprecated so that with the next minor release we could remove that altogether. like: |
Replacing to something like will be good enough: |
Actually we can see "phpDocumentor/phpDocumentor#1604" if we could or will go with the above solution. |
It wasn't fixed and AFAIK major IDEs are not supporting this syntax. |
Hi @engcom-Hotel. Thank you for working on this issue.
|
✅ Confirmed by @engcom-Hotel Issue Available: @engcom-Hotel, You will be automatically unassigned. Contributors/Maintainers can claim this issue to continue. To reclaim and continue work, reassign the ticket to yourself. |
Hi @sertlab. Thank you for working on this issue.
|
@engcom-Hotel i think we expected result should be changed, that magic method should highlight that it could return null. This is not direct responsibility of order object to retrieve customer. In client code customer could be retrieved by customerId. |
@ihor-sviziev thanks for contributing! I have updated the description. |
Thanks for opening this issue! |
3 similar comments
Thanks for opening this issue! |
Thanks for opening this issue! |
Thanks for opening this issue! |
Hi @qsolutions-pl. Thank you for your report.
The fix will be available with the upcoming 2.4.0 release. |
Preconditions (*)
Steps to reproduce (*)
sales_order_save_after
eventExpected result (*)
getCustomer
should return customer object, as defined in phpdoc block OR phpdoc block should be changed to highlight thatNULL
could be returnedActual result (*)
getCustomer
always returnsNULL
, but it's not highlighted in phpdoc block that it could return nullAdditional info
Method
getCustomer
will return customer only in case if someone added it to Order object, in other cases it doesn't exists. In case if you need to get customer from the Order object you need to do following get customer ID from the order objectWhere
$this->customerRepository
is instance of\Magento\Customer\Api\CustomerRepositoryInterface
that retrieved via constructorThe text was updated successfully, but these errors were encountered: