Skip to content
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

Non successful notifications creates an illegal state change from pending / complete -> invalid ( invalidate ) #9

Open
DangerDawson opened this issue Jun 11, 2014 · 1 comment

Comments

@DangerDawson
Copy link

When adyen sends a non successful notification e.g. cancel a payment the following code is triggered:

# Invalidate payments that doesnt receive a successful notification
  def handle!
    if (authorisation? || capture?) && !success?
      payment = Spree::Payment.find_by(response_code: psp_reference)
      if payment && !payment.failed? && !payment.invalid?
        payment.invalidate!
      end
    end
  end

Now if you look at the state machine code in payment.rb ( spree core )

     event :void do
        transition from: [:pending, :completed, :checkout], to: :void
      end
      # when the card brand isnt supported
      event :invalidate do
        transition from: [:checkout], to: :invalid
      end

It looks as though we should be calling void rather than invalidate ? or am I missing something?

@huoxito
Copy link
Contributor

huoxito commented Jul 3, 2014

Hey @DangerDawson sorry totally missed this one

I'm not sure we should call void and not sure about invalidate either tbh. But we should have in mind that void we'll try to hit Adyen and actually void a payment that might not even have succeeded in the first place. I think thats why I decided to go with invalidate here.

I can imagine that decision being very particular to each store though. Perhaps instead of changing the payment state the safer and most efficient thing to do here would be to email store staff to take be aware theres a failing payment.

Either way thanks a ton for noticing the state bug. I'm thinking about leaving that as is right now while we dont come up with a proper solution because at least an exception will be raise and someone should notice that something has gone wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants