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

Set cartItem unit price before giftcard creation #194

Merged
merged 4 commits into from
May 2, 2022

Conversation

jum4
Copy link
Contributor

@jum4 jum4 commented Feb 3, 2022

Related to #193

@codecov
Copy link

codecov bot commented Feb 28, 2022

Codecov Report

Merging #194 (13401d9) into 0.12.x (a70f927) will increase coverage by 1.51%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##             0.12.x     #194      +/-   ##
============================================
+ Coverage     45.75%   47.27%   +1.51%     
  Complexity      501      501              
============================================
  Files            94       94              
  Lines          1567     1576       +9     
============================================
+ Hits            717      745      +28     
+ Misses          850      831      -19     
Impacted Files Coverage Δ
src/Form/Extension/AddToCartTypeExtension.php 60.86% <100.00%> (+60.86%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a70f927...13401d9. Read the comment docs.

@loevgaard
Copy link
Member

@Roshyo: How did this work in 0.11? Why has it changed? Do you know that?

@Roshyo
Copy link
Contributor

Roshyo commented Apr 4, 2022

It stopped working when we started to allow GC with custom amounts.
Previously, we were creating the GC from the OrderItemUnit when placing the order. Now we do it when creating the Order (cart) and the Order Item Unit has no price yet, since it is assigned by Sylius later.
Perhaps an other fix would be to change the GC creation just after this price assignment (from order processor) but not sure if it would raise more problems or not.

@loevgaard loevgaard merged commit 604cad4 into Setono:0.12.x May 2, 2022
@loevgaard
Copy link
Member

Thank you @jum4 and @Roshyo 🎉

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.

3 participants