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

Important fixes for VAT-included stores [2-3-stable] #139

Open
wants to merge 13 commits into
base: 2-3-stable
Choose a base branch
from

Conversation

alepore
Copy link
Member

@alepore alepore commented Nov 29, 2014

This PR enable paypal payments for stores with VAT-included prices (very common in the EU) and should finally solve #129 and #17.

The first fix was already included in 2-2-stable, this just adds a regression spec.

The second one it's a kind of workaround because paypal does not accept negative taxes amounts.
But this is very important because on Spree we have negative taxes when a country without VAT buys from a VAT-included store, like in this example:
screen shot 2014-11-29 at 13 36 31

This is also including all the commits from #138 to get green specs.

@alepore
Copy link
Member Author

alepore commented Nov 29, 2014

@peterfealey @paulgroves @ouechtasvu @Hates @IrvinFan please take a look

@alepore alepore changed the title Important fixes for VAT-included prices [2-3-stable] Important fixes for VAT-included stores [2-3-stable] Nov 29, 2014
@pusewicz
Copy link
Contributor

pusewicz commented Apr 7, 2015

@pusewicz
Copy link
Contributor

pusewicz commented Apr 7, 2015

This should also be merged into master

@alepore
Copy link
Member Author

alepore commented Apr 7, 2015

in the end i didn't use this code but i think it's still relevant.
i'm just asking for some feedback/sanity checking as it's pretty complex/critical stuff :)

@JDutil
Copy link
Member

JDutil commented Apr 7, 2015

I don't use or care about paypal at all. The specs fail terribly on this extension for me as well so I'm not going to be touching anything here until someone fixes them. The changes in the works over at spree/spree#6227 may be relevant to fixing this issue though.

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.

5 participants