mollie / spree-mollie-gateway

Mollie payments for Spree Commerce.
https://www.mollie.com
16 stars 23 forks source link

Don't create a Mollie customer when the gateway is inactive #90

Closed rvanlieshout closed 4 years ago

rvanlieshout commented 4 years ago

A client had invalid credentials in the Gateway settings which resulted in a Mollie::RequestError. Let's only create a Mollie customer when the gateway is active.

Using an ensure_mollie_customer after_save should also create Mollie customers for users that have been created in the time that is was inactive

rvanlieshout commented 4 years ago

@here, Any update on this one?

vernondegoede commented 4 years ago

Sorry, @rvanlieshout. @Oldharlem Can you review this PR?

rvanlieshout commented 4 years ago

I'll get back to this one a.s.a.p.!

Oldharlem commented 4 years ago

I'll get back to this one a.s.a.p.!

Can I assist in any way to move this pr forward?

rvanlieshout commented 4 years ago

@oldharlem, I’m currently in holiday-mode and don’t have enough equipment with me to code. Feel free to add any code needed. I’ll be back coding in around 2 weeks

rvanlieshout commented 4 years ago

@Oldharlem, Added a commit

Oldharlem commented 4 years ago

Tests have failed 😔 It seems like mollie_customer_id can be a non existent column by the looks of this migration

@vernondegoede can you explain what the one_click_payments mechanism is and why the mollie customer id is omitted when enabled?

I suggest creating the column either way unless there is specific reasoning behind this.

rvanlieshout commented 4 years ago

It does not quite cover the previous issue although the tests dont choke on it anymore.

If you can incorporate the safe navigation operator on the mollie_customer_id&.present? too we are good to go.

Moving it to below the active?-check dit break other specs

Oldharlem commented 4 years ago

It does not quite cover the previous issue although the tests dont choke on it anymore.

If you can incorporate the safe navigation operator on the mollie_customer_id&.present? too we are good to go.

Moving it to below the active?-check dit break other specs

Since your fork does not have the correct secrets the tests break on api interaction. It does seem to pass the general code though.

rvanlieshout commented 4 years ago

That makes sense

Oldharlem commented 4 years ago

We are all set, tests on master are passing. Thanks for your efforts! 👍

rvanlieshout commented 4 years ago

Cheers