Skip to content
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 user can be null #2017

Merged
merged 1 commit into from
Oct 16, 2014
Merged

Order user can be null #2017

merged 1 commit into from
Oct 16, 2014

Conversation

umpirsky
Copy link
Contributor

After #1816 order.user can be null.

So, all code like $order->getUser()-> should be fixed to support this change.

For example https://github.com/Sylius/Sylius/blob/master/src/Sylius/Bundle/PayumBundle/Payum/Be2bill/Action/CapturePaymentUsingBe2billFormAction.php#L70.

umpirsky added a commit to umpirsky/Sylius that referenced this pull request Oct 14, 2014
$details['HIDECLIENTEMAIL'] = 'yes';
$details['CLIENTUSERAGENT'] = $this->httpRequest->headers->get('User-Agent', 'Unknown');
$details['CLIENTIP'] = $this->httpRequest->getClientIp();
$details['CLIENTIDENT'] = $order->getUser()->getId();
$details['CLIENTIDENT'] = $order->getUser() ?: $order->getUser()->getId();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong, it should be probably:

$details['CLIENTIDENT'] = $order->getUser() ? $order->getUser()->getId() : $order->getEmail();

@stloyd
Copy link
Contributor

stloyd commented Oct 15, 2014

👍

@umpirsky
Copy link
Contributor Author

@pjedrzejewski Please review and merge.

@stloyd
Copy link
Contributor

stloyd commented Oct 16, 2014

@Arn0d Can you merge this bugfix?

umpirsky added a commit that referenced this pull request Oct 16, 2014
@umpirsky umpirsky merged commit 59810d5 into Sylius:master Oct 16, 2014
@umpirsky umpirsky deleted the fix/issue-2017 branch October 16, 2014 09:23
@pjedrzejewski
Copy link
Member

👍

pamil pushed a commit to pamil/Sylius that referenced this pull request May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants