When onNotify and onReturn are called at the same time we get an outdated order and it is places twice.

CommentFileSizeAuthor
#9 3043227-9.patch9.71 KBfacine
#5 commerce_sermepa-order_notification-3043227-5.patch9.25 KBfacine
#2 commerce_sermepa-order_notification-3043227-2.patch4.12 KBfacine

Comments

facine created an issue. See original summary.

facine’s picture

facine commented
StatusFileSize
new commerce_sermepa-order_notification-3043227-2.patch4.12 KB

Attach a temporal workaround.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented 
  1. +++ b/src/Plugin/Commerce/PaymentGateway/Sermepa.php
@@ -278,6 +292,21 @@ class Sermepa extends OffsitePaymentGatewayBase implements HasPaymentInstruction
+    $controller = $this->entityTypeManager->getStorage($order->getEntityTypeId());

    This is the storage, not the controller 

  2. +++ b/src/Plugin/Commerce/PaymentGateway/Sermepa.php
@@ -278,6 +292,21 @@ class Sermepa extends OffsitePaymentGatewayBase implements HasPaymentInstruction
+    $updated_order = $controller->load($order->id());

    I think the combo resetCache+load is loadUnchanged 

  3. +++ b/src/Plugin/Commerce/PaymentGateway/Sermepa.php
@@ -278,6 +292,21 @@ class Sermepa extends OffsitePaymentGatewayBase implements HasPaymentInstruction
+    if ($updated_order->getState()->getId() == 'completed' && $order->getState()->getId() == 'draft') {

    Are we sure about the draft state being the only one relevant? or is it just an order state change.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Related issues: +#3003454: The ::onNotify and ::onReturn methods can collide causing a race condition
facine’s picture

facine commented
Status: Active » Needs review
StatusFileSize
new commerce_sermepa-order_notification-3043227-5.patch9.25 KB

After talk about this issue, I've rewrite all the patch.

facine’s picture

facine commented 
+++ b/src/Plugin/Commerce/PaymentGateway/Sermepa.php
@@ -262,21 +276,37 @@ class Sermepa extends OffsitePaymentGatewayBase implements HasPaymentInstruction
-    // Check if the payment has been processed.
-    $payments = $this->entityTypeManager->getStorage('commerce_payment')->getQuery()
-      ->condition('state', 'authorization')
-      ->condition('payment_gateway', $this->entityId)
-      ->condition('order_id', $order->id())
-      ->execute();

This should be checked with the remote_id so we move this to the processRequest method.

pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Priority: Normal » Critical
Status: Needs review » Needs work
pcambra’s picture

pcambra
he/him
Primary language Spanish
commented
Issue tags: +Release blocker
facine’s picture

facine commented
Status: Needs work » Needs review
StatusFileSize
new 3043227-9.patch9.71 KB

@pcambra I've updated the patch to works with the last dev branch and adding reference to #3043180: The changes made to the order on the onNotify method are not applied on the onReturn method so we could modify this again when this issues is fixed in Commerce core.

  • facine committed d7793dd on 8.x-2.x 
    Issue #3043227 by facine, pcambra: Order notification mail are sent...
facine’s picture

facine commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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