Fix invalid checkout price issue #65
Conversation
| expect(line_item).to receive(:variant_id).and_return(1001) | ||
|
|
||
| expect(Spree::Price).to receive(:where).with( | ||
| currency: 'EUR', |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| describe 'update_price' do | ||
| let(:price) { double 'price' } | ||
| let(:order) { create :order, currency: 'EUR' } | ||
| let(:line_item) { create :line_item, order_id: order.id, currency: 'EUR' } |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| describe Spree::LineItem do | ||
| describe 'update_price' do | ||
| let(:price) { double 'price' } | ||
| let(:order) { create :order, currency: 'EUR' } |
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| @@ -0,0 +1,24 @@ | |||
| describe Spree::LineItem do | |||
| describe 'update_price' do | |||
| let(:price) { double 'price' } | |||
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| @@ -0,0 +1,24 @@ | |||
| describe Spree::LineItem do | |||
| describe 'update_price' do | |||
There was a problem hiding this comment.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
| @@ -0,0 +1,24 @@ | |||
| describe Spree::LineItem do | |||
There was a problem hiding this comment.
Missing frozen string literal comment.
| @@ -0,0 +1,10 @@ | |||
| Spree::LineItem.class_eval do | |||
There was a problem hiding this comment.
Missing frozen string literal comment.
|
Hi @nmccafferty could you please make these changes against |
|
@krzysiek1507 I had initially tried to make the changes against the master but a fresh bundle install fails, colleague has now confirmed this for me, he gets the same: |
why against when this issue fix for 3-1-stable ?? |
|
@arvindvyas I think we want to fix it also in next versions ;) So we can fix it on master and cherry-pick to previous versions. Do you agree? |
|
@krzysiek1507 Don't you think it is important is fix stable branch first? if you don't fix it for un stable branch it is not that important . |
|
@krzysiek1507 Is there any chance you can merge this in as it really is quite a critical issue in the project? I had tried to follow your request to add to master but the master build would not bundle as I've instructed before. I'll continue working with a patched version until you can sort this out. |
|
@nmccafferty seems they don't care! |
|
@nmccafferty @arvindvyas merged! Sorry for the long wait! |
* Fix invalid checkout price issue * Fix hound review
|
@nmccafferty @arvindvyas btw. |
Fix invalid checkout price issue (#65)
Fix for #64
The issue seems to stem from the call to price_including_vat_for against the variant. This delegates the method onto it's price object, but from the variant it has no way to know whether the price is a EUR / USD object, it instead uses the Spree::Config[:currency] value to determine which price to use and comes back with the USD object instead of the EUR object.
Therefore I expect a change needs to be applied where this info is known, in the line_item.rb method update_price to get back the correct price object and invoke price_including_vat_for directly.