Skip to content

Conversation

@bradleybrecher
Copy link
Contributor

Description (*)

Fixed Express checkout to show lastname in the the db for billing and shipping in guest mode. Two files were changed in the Magento\Paypal\Model, Checkout.php and Api\Nvp.php. For Checkout.php, had to comment out the lines that were setting the sipping details to NULL. In Api\Nvp.php , had to change the SHIPTONAME to split up the first and last name so each field can be saved respectively instead of the whole name in firstname.

Fixed Issues (if relevant)

  1. paypal is not giving lastname in express checkout #23061: paypal is not giving lastname in express checkout

Manual testing scenarios (*)

  1. Place a order on magento using express checkout in guest mode
  2. After successful order, check sales_order and sales_order_adress table in db to confirm that order information is correct.

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
Copy link

m2-assistant bot commented Jul 19, 2019

Hi @bradleybrecher. 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 Assistant documentation

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Jul 19, 2019

CLA assistant check
All committers have signed the CLA.

$this->_applyStreetAndRegionWorkarounds($shippingAddress);
// PayPal doesn't provide detailed shipping name fields, so the name will be overwritten
$shippingAddress->addData(['firstname' => $data['SHIPTONAME']]);
$_arraydata = explode(' ', $data['SHIPTONAME'], 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can SHIPTONAME contain more than two names? If so splitting and grabbing the second will result in inaccurate data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@navarr PayPal send the data for the SHIPTONAME as one string(separated by spaces). PayPal requires a first and last name on express checkout so there must be at least 2 names in the string. So what I propose (in my most recent commit) is in the case where there are more than one first and/or last name, make an assumption that it is a first name. In the small chance where there are 2 last names, the admin can edit orders manually rather than always editing the database to insert the name(the initial issue). This issue is discussed further on Adam Wilson's blog.

@pmclain pmclain self-requested a review July 24, 2019 04:51
@pmclain pmclain self-assigned this Jul 24, 2019
Copy link
Contributor

@pmclain pmclain left a comment

Choose a reason for hiding this comment

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

A few syntax requests while the CI builds are still pending.

@pmclain
Copy link
Contributor

pmclain commented Jul 25, 2019

@bradleybrecher Please review failing checks.

Copy link
Contributor

@sidolov sidolov left a comment

Choose a reason for hiding this comment

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

Please, fix the code style issues, you need just to revert a lot of code styles changes made in the previous commits.

@ghost ghost assigned sidolov Sep 23, 2019
@sidolov
Copy link
Contributor

sidolov commented Oct 8, 2019

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

@sidolov sidolov closed this Oct 8, 2019
@m2-assistant
Copy link

m2-assistant bot commented Oct 8, 2019

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

@hubertus2017
Copy link

hubertus2017 commented May 28, 2022

Hmm, still the same problem in M2.4.4 !?!
Edit: Seems to be in bundled Braintree module, not in the express module mentioned here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants