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

Add shipping preference option to RestPurchaseRequest #184

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jlttt
Copy link

@jlttt jlttt commented Nov 15, 2017

Fix #175.

Add shipping_preference option to RestPurchaseRequest which allow to disable the shipping address with value set to NO_SHIPPING.

@SlimDeluxe
Copy link

Any chance of getting this merged anytime soon?

Copy link

@SlimDeluxe SlimDeluxe left a comment

Choose a reason for hiding this comment

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

Tested and it works.
You should probably...

  1. Make the "NO_SHIPPING" a class constant
  2. Add comments, especially for setShippingPreference() so it's clear what exactly it does and what the options are (if there are more)

@jlttt jlttt force-pushed the shippingPreference branch 2 times, most recently from d36db0a to eb29e8c Compare February 7, 2018 21:40
@jlttt
Copy link
Author

jlttt commented Feb 7, 2018

@SlimDeluxe Thanks for your advices. I add comments on supported values for setShippingPreference() and create associated class constants.

@SlimDeluxe
Copy link

@jlttt I tested the RestAuthorizeRequest with your code and it works there also. Since RestPurchaseRequest extends RestAuthorizeRequest, I suggest you move the code to the parent class instead.
BTW I found out you can properly manage the shipping address if you create an "experience profile". Please see #192

@delatbabel
Copy link
Contributor

This will need a manual merge because of the conflicts. I'll get to that when I have sufficient time.

@jlttt jlttt force-pushed the shippingPreference branch from a41dda5 to 456f758 Compare February 22, 2018 21:08
@jlttt
Copy link
Author

jlttt commented Feb 22, 2018

@delatbabel I update the PR and remove the conflicts.

@SlimDeluxe
Copy link

@BrightSkyz just copy the file manually until they merge it.
Otherwise I suggest you set up the "experience profile" I mentioned above. It can be easily done with the official PayPal SDK for PHP.

@digibeuk
Copy link
Contributor

can someone merge this? we also need it

@digibeuk
Copy link
Contributor

digibeuk commented Nov 1, 2018

I would propose to make you changes in : \Omnipay\PayPal\Message\AbstractRestRequest so setting this preference can also be done in other Rest based request like the RestAuthorizeRequest also you did not add a test

@eqxDev
Copy link

eqxDev commented Dec 16, 2019

Is there any update on this?

@PatrickHollweck
Copy link

Any updates on this? Can we merge this? :)

@VanillaChan6571
Copy link

Why the hell is this not merged yet?

@delatbabel
Copy link
Contributor

Usually when I want something done I use words like "please" not "why the hell". However I'm not your mother.

@SlimDeluxe
Copy link

... but it got your attention after more than 5 years :)
Can you please merge this? 😇

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.

Can't find noshipping-option (REST)
7 participants