search

five18pm / spree-ccavenue

Spree extension for CCAvenue payment gateway
BSD 3-Clause "New" or "Revised" License
7 stars 10 forks source link

Extension compatibility with new Spree #5

Closed lkananowicz closed 10 years ago

lkananowicz commented 10 years ago

Hello. Can I ask you few questions about the code? I'm trying to rewrite it to enable compatibility with spree 2.3.

Kind Regards Leon

five18pm commented 10 years ago

Yes, please go ahead with your questions

On Fri, Aug 29, 2014 at 2:14 AM, lkananowicz notifications@github.com wrote:

Hello. Can I ask you few questions about the code? I'm trying to rewrite it to enable compatibility with spree 2.3.

Kind Regards Leon

— Reply to this email directly or view it on GitHub https://github.com/five18pm/spree-ccavenue/issues/5.

lkananowicz commented 10 years ago

Thanks.

  1. In Spree::Ccavenue::Transaction model there are methods gateway_order_number generate_transaction_number! Why add additional randomness to order.number provided by Spree. Are there cases where order.number won't be unique?
  2. Do you think it would appropriate to rewrite Spree::Ccavenue::Transaction#initialize_state! method from

raise "Order is not in 'confirm' state. Order should be in 'confirm' state before transaction can be sent to CCAvenue" unless order.confirm? this = self previous = order.ccavenue_transactions.reject{|t| t == this} previous .each {|p| p.cancel!} generate_transaction_number!

to

if order.confirmation_required? && !order.confirm? raise "Order is not in 'confirm' state. Order should be in 'confirm' state before transaction can be sent to CCAvenue" end this = self previous = order.ccavenue_transactions.reject{|t| t == this} previous.each {|p| p.cancel!} self.transaction_number = order.number

Kind Regards Leon

2014-08-29 8:00 GMT+02:00 Chandramohan Rangaswamy notifications@github.com :

Yes, please go ahead with your questions

On Fri, Aug 29, 2014 at 2:14 AM, lkananowicz notifications@github.com wrote:

Hello. Can I ask you few questions about the code? I'm trying to rewrite it to enable compatibility with spree 2.3.

Kind Regards Leon

— Reply to this email directly or view it on GitHub https://github.com/five18pm/spree-ccavenue/issues/5.

— Reply to this email directly or view it on GitHub https://github.com/five18pm/spree-ccavenue/issues/5#issuecomment-53840252 .

five18pm commented 10 years ago

Why add additional randomness to order.number provided by Spree. Are there cases where order.number won't be unique?

CCAvenue requires transaction number to be unique for every transaction. If the initial payment did not go through, then we need a new transaction number to retry payment for the same order.

Your changes to initialize_state! look good