Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Promotion credit destroyed after order completed bug #1000

Closed
wants to merge 1 commit into from

4 participants

@evanwhalen

do not process automatic promotions after an order has been finalized

@cmar

This looks like a good fix for 0.60.4. We'll have to bring it forward to 1.0

@schof
Owner

@evanwhalen thanks for reporting. we'll get this fixed in 1.0 shortly. please log any other known problems with promotions that you find.

@radar

Why is this defined here if it's only used in one spot? Why can't we have it inside the finalized? method and then move it out when we need to.

Oh, we use Order.finalized_states in other locations, but not in the spree core code. Can definitely just have one method otherwise.

Collaborator

Oh I see, it's used there. No worries, leave it as is.

@evanwhalen

Thanks guys, I just have this other pull request at the moment: #998, we'll be sure to send pull requests for any more bugs we find.

@radar radar closed this in f418cad
@radar
Collaborator

I went to apply this work into 1.0, but I can't find where the process_automatic_promotions method has gone to...

@schof schof reopened this
@schof
Owner

These are pull requests filed against 0-60-x. We're not going to apply them to 0-60-x but we're keeping them open until cmar processes them for 1.0. Please defer all promotions pull requests and bugs to him since this is his primary focus the next 72 hours or so.

@radar
Collaborator

Right, @cmar this is yours too.

@cmar

@evanwhalen I ported your code to Spree 1.0. Thanks for the fix.

@schof schof closed this in 7608d82
@fonemstr fonemstr referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 62 additions and 1 deletion.
  1. +9 −1 promo/lib/spree_promo.rb
  2. +53 −0 promo/spec/models/order_spec.rb
View
10 promo/lib/spree_promo.rb
@@ -27,6 +27,14 @@ def possible_promotions
attr_accessible :coupon_code
attr_accessor :coupon_code
before_save :process_coupon_code, :if => "@coupon_code.present?"
+
+ def finalized?
+ self.class.finalized_states.include?(state)
+ end
+
+ def self.finalized_states
+ ["complete", "awaiting_return", "returned"]
+ end
def promotion_credit_exists?(credit)
promotion_credits.reload.detect { |c| c.source_id == credit.id }
@@ -47,7 +55,7 @@ def update_totals(force_adjustment_recalculation=false)
self.payment_total = payments.completed.map(&:amount).sum
self.item_total = line_items.map(&:amount).sum
- process_automatic_promotions
+ process_automatic_promotions unless finalized?
if force_adjustment_recalculation
applicable_adjustments, adjustments_to_destroy = adjustments.partition{|a| a.applicable?}
View
53 promo/spec/models/order_spec.rb
@@ -0,0 +1,53 @@
+require File.dirname(__FILE__) + '/../spec_helper'
+
+describe Order do
+ let(:order) { Order.new }
+
+ describe "finalized?" do
+ let(:finalized_states) { ["complete", "awaiting_return", "returned"] }
+
+ it "should return true" do
+ finalized_states.each do |state|
+ order.state = state
+ order.finalized?.should == true
+ end
+ end
+
+ it "should return false" do
+ (Order.state_machine.states.map(&:name) - Order.finalized_states).each do |state|
+ order.state = state
+ order.finalized?.should == false
+ end
+ end
+
+ end
+
+ describe "update_totals" do
+ after {
+ order.update_totals
+ }
+
+ describe "when order finalized" do
+ before {
+ # finalized? is a method that states whether Order is
+ order.stub(:finalized? => true)
+ }
+
+ it "should not process automatic promotions" do
+ order.should_not_receive(:process_automatic_promotions)
+ end
+
+ end
+
+ describe "when not finalized" do
+ before {
+ order.stub(:finalized? => false)
+ }
+
+ it "should process automatic promotions" do
+ order.should_receive(:process_automatic_promotions)
+ end
+
+ end
+ end
+end
Something went wrong with that request. Please try again.