The getDefaultPaymentMethodOption method doesn't check if the default gateway option in order , is currently enabled/not? the selected option may or may not be available depending on changed config.

CommentFileSizeAuthor
#6 2860102-6.patch1.98 KBsumanthkumarc
#4 2860102-4.patch2.09 KBsumanthkumarc

Comments

sumanthkumarc created an issue. See original summary.

sumanthkumarc’s picture

sumanthkumarc commented

Patch will be available shortly.

bojanz’s picture

bojanz commented
Title: The getDefaultPaymentMethodOption method doesn't check if the default gateway option in order , is currently enabled/not? » PaymentInformation::getDefaultPaymentMethodOption doesn't check if the order gateway is still available

Retitling.

sumanthkumarc’s picture

sumanthkumarc commented
Status: Active » Needs work
StatusFileSize
new 2860102-4.patch2.09 KB

attaching an initial patch. Not a finalized one. the gateways array_keys might be passed instead of entire array is one way. Also need to check the methods.

bojanz’s picture

bojanz commented

$options are more important, let's add $payment_gateways after them.

* @param array $payment_gateways

This needs the real typehint: \Drupal\commerce_payment\Entity\PaymentGatewayInterface[]

elseif ($order_payment_gateway && !($order_payment_gateway instanceof SupportsStoredPaymentMethodsInterface) && in_array($order_payment_gateway,$payment_gateways)) {

Might be nicer to do isset($payment_gateways[$order_payment_gateway->id()])

sumanthkumarc’s picture

sumanthkumarc commented
Status: Needs work » Needs review
StatusFileSize
new 2860102-6.patch1.98 KB

Updated patch with the review comments.

sumanthkumarc’s picture

sumanthkumarc commented

also tested with normal gateways without methods and it works fine.

sumanthkumarc’s picture

sumanthkumarc commented

this is a blocker issue for https://www.drupal.org/node/2860111 and https://www.drupal.org/node/2858067.

bojanz’s picture

bojanz commented
Assigned: Unassigned » bojanz
Status: Needs review » Needs work

This protects against a no-longer-available order payment gateway, but it doesn't cover the case where the order has a payment method from a no-longer-available payment gateway.

We need to change our approach here. Instead of modifying getDefaultPaymentMethodOption() we want to make sure $selected_option is not empty.

bojanz’s picture

bojanz commented
Title: PaymentInformation::getDefaultPaymentMethodOption doesn't check if the order gateway is still available » PaymentInformation::buildPaneForm doesn't check if the selected gateway is still available
bojanz’s picture

bojanz commented
Title: PaymentInformation::buildPaneForm doesn't check if the selected gateway is still available » PaymentInformation::getDefaultPaymentMethodOption doesn't check if the selected option is still available

bojanz’s picture

bojanz commented
Status: Needs work » Fixed

Committed a tweaked version of your PR. Thanks!

bojanz’s picture

bojanz commented
Status: Fixed » Needs work

I can still reproduce a crash after disabling a gateway and revisiting checkout. Looks like we also still get payment methods belonging to disabled gateways. Needs another look, and test coverage.

bojanz credited joachim.

bojanz’s picture

bojanz commented

joachim supplied a potential fix in[https://www.drupal.org/project/commerce/issues/2917638#comment-12307962

bojanz’s picture

bojanz commented
Status: Needs work » Fixed

Actually, setting this issue back to Fixed, since the other issue (#2917638: PaymentInformation can crash if the payment gateway becomes unavailable) shows that the remaining bug is not in getDefaultPaymentMethodOption(), it's in the buildPaneForm() method itself. Let's fix it there.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.