-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
fix negative total displaying in cart by displaying zero if total is … #2769
fix negative total displaying in cart by displaying zero if total is … #2769
Conversation
…rt values in views and add tests
expect(subject).to eq(order.display_total.to_html) | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/TrailingWhitespace: Trailing whitespace detected.
if current_order.total.negative? | ||
Spree::Money.new(0, currency: current_order.currency).to_html | ||
else | ||
current_order.display_total.to_html |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/TrailingWhitespace: Trailing whitespace detected.
@@ -156,5 +156,13 @@ def link_to_tracking(shipment, options = {}) | |||
def plural_resource_name(resource_class) | |||
resource_class.model_name.human(count: Spree::I18N_GENERIC_PLURAL) | |||
end | |||
|
|||
def protect_from_negative(current_order) | |||
if current_order.total.negative? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Layout/TrailingWhitespace: Trailing whitespace detected.
Renamed the method - feel free to give some feedback! Thanks! |
I think the best way to solve this issue is to do not allow promotion to apply (or apply it partially) when the order total reach a negative amount, but at the moment I don't know if this is easy to achieve with current promotion implementation. I'm not sure about what to do here. I'll bring this topic into the next core team meeting and will let you know! |
@kennyadsl sounds good. However, it is worth mentioning, I think this is 99% presentation as the checkout still reflects a total of $0.00. |
Yes, checkout could work, but the issue is at data level if I got it correctly. We should not allow that since an order with negative amount makes no sense. |
@kennyadsl I agree, with the caveat that I don't think we should make it difficult to set a product to zero as some store owners might intentionally give a product away. Is #2593 relevant to this issue? Could you kill two birds with one stone by handling this issue at the promotions instead of the orders level? Just a thought. edit: I didn't check to see if the order is still negative post-checkout. It's possible that the checkout is $0.00, but the order total is recorded as negative. |
…less than zero
Refs #2741
If an admin set the promotions in a specific way, it was possible to make the cart display a negative value before checkout. The bug is purely visual as Solidus sets the checkout value to $0.00. This PR simply makes the frontend reflect that behavior in the cart.
Feedback is welcome.