search

webjuice / spree-pp-website-standard

Paypal Website Payments Standard Extension for Spree, the open source shopping cart
BSD 3-Clause "New" or "Revised" License
2 stars 0 forks source link

refreshing payment page causes many Payments to get attached to Order #1

Open berkes opened 12 years ago

berkes commented 12 years ago

In the controller_decorator, the GET edit is overridden. In that override we create and attach a new payment on each call:

      payment = Spree::Payment.new
      payment.amount = @order.total
     #...
      @order.payments << payment

If you go back in the statemachine, or refresh that page, new payments will be added to the order. This violates the RESTfull "rules" where a get should never have a side-effect.

Should this paypal extension not simply insert a step in the checkout Order statemachine and act on that, instead of this?

I realize that this is a lot of work, but the core checkoutcontroller is rather inflexible, and violates REST already.

tomash commented 12 years ago

"I realize that this is a lot of work, but the core checkoutcontroller is rather inflexible, and violates REST already."

I think you've kind of summed up why it wasn't our (mine?) concern when developing this extension ;)

berkes commented 12 years ago

Would you prefer if I change the Payment.new into a Payment.find || Payment.new?

tomash commented 12 years ago

No idea. I plan to set-up a decent test suite for this extension (hopefully this week) before further modifications, should make work and decisions a lot easier in the future :)