PATCH Delete previous promotion and assign a new one properly.

What steps will reproduce the problem?
1. Its not reproductible by the web interface.

What version of the product are you using? On what operating system?
Trunk. Ubuntu 7.10

Please provide any additional information below.

Problem 1:

Do that:
self.order_line_items.delete(self.promotion_line_item) if
self.promotion_line_item

instead of
self.promotion_line_item.destroy if self.promotion_line_item

Its not a good thing to nuke an object inside a collection, the collection
has the delete method (Things start to behave strange until the object is
reloaded).

Problem 2:

Assign an object:
self.promotion = promo

instead of just its id:
self.promotion_id = promo.id

when possible.

Same reason, it behaves wrong unless reloaded and in some situation. (if
you use an object then an id, the relation isn't even saved and doesn't
give any error).

These errors happens when testing the set_promo_code method alone, and
doesn't happen using the web interface because on checkout the controller
reloads all objects again. (I think that a model method should not rely on
the app flow to do its job and work properly)

The following three tests illustrates the problem 2, very odd. Its really a
generic problem, and happens in other places too.

  # Always use the object and nothing more is needed.
  def test_associate_using_object_all_times
    a = Order.new
    a.order_status_code = order_status_codes(:cart)
    a.save
    assert_equal a.order_status_code, order_status_codes(:cart)
    a.order_status_code = order_status_codes(:to_charge)
    a.save
    assert_equal a.order_status_code, order_status_codes(:to_charge)
  end

  # When using the object then the id, the parent need to be reloaded first
or it will simply ignore the change.
  def test_associate_using_object_then_id
    a = Order.new
    a.order_status_code = order_status_codes(:cart)
    a.save
    assert_equal a.order_status_code, order_status_codes(:cart)
    a.order_status_code_id = 2
    a.save
    assert_not_equal a.order_status_code, order_status_codes(:to_charge)
    a.reload
    a.order_status_code_id = 2
    a.save
    assert_equal a.order_status_code, order_status_codes(:to_charge)
  end

  # When using the id then the object, nothing more is needed.
  def test_associate_using_id_then_object
    a = Order.new
    a.order_status_code_id = 1
    a.save
    assert_equal a.order_status_code, order_status_codes(:cart)
    a.order_status_code = order_status_codes(:to_charge)
    a.save
    assert_equal a.order_status_code, order_status_codes(:to_charge)
  end

Original issue reported on code.google.com by edmundo...@gmail.com on 28 Jun 2008 at 6:18

Attachments:

delete_previous_promotion_properly.patch

be090129 / substruct

PATCH Delete previous promotion and assign a new one properly. #102