Problem/Motivation

Hi,
I'm giving a try to proposed submodule commerce_fee here and found that it was not possible to add a fee by payment gateway because a condition to filter by payment gateways was missing.

Proposed resolution

Added Drupal\commerce_payment\Plugin\Commerce\Condition OrderPaymentGateway
It works ok for me with Drupal 8.5.0 and Commerce 8.x-2.5

Remaining tasks

The only thing I think is not correct is that that condition is displayed on payment gateways condition settings too but did not found how to avoid it.

User interface changes

Find a new condition "Limit by payment gateway" under "Order" tab on conditions group.

Thanks!

CommentFileSizeAuthor
#6 commerce_payment-add-order-payment-gateway-condition-2953940-6.patch6.72 KBjsacksick
#3 commerce-orderpaymentgateway-condition-2953940-3.patch2.38 KBpslcbs
#2 commerce-orderpaymentgateway-condition-2953940-2.patch2.32 KBpslcbs
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pslcbs created an issue. See original summary.

pslcbs’s picture

pslcbs CreditAttribution: pslcbs commented
Status: Active » Needs review
FileSize
commerce-orderpaymentgateway-condition-2953940-2.patch2.32 KB
pslcbs’s picture

pslcbs CreditAttribution: pslcbs commented
FileSize
commerce-orderpaymentgateway-condition-2953940-3.patch2.38 KB

Ooops, problem found and solved, sorry.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented
Issue tags: +Needs tests

The only thing I think is not correct is that that condition is displayed on payment gateways condition settings too but did not found how to avoid it.

We currently do not have a way for Commerce condition plugins to say "except this entity type." So I think we are fine by opening a follow up issue and it is just a slight UX bug for later.

We should also make a test for the condition.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs review » Needs work

We actually do have that mechanism: https://github.com/drupalcommerce/commerce/blob/8.x-2.x/modules/promotio...

Setting "needs work" for that and tests.

jsacksick’s picture

jsacksick CreditAttribution: jsacksick at Centarro commented
Status: Needs work » Needs review
FileSize
commerce_payment-add-order-payment-gateway-condition-2953940-6.patch6.72 KB

I added a test, and the event subscriber for removing the condition on payment gateways, and slightly modified evaluate() on the condition.

mglaman’s picture

mglaman
Primary language English
Location WI, USA
CreditAttribution: mglaman at Centarro commented 
+++ b/modules/payment/tests/src/Unit/Plugin/Commerce/Condition/OrderPaymentGatewayTest.php
@@ -0,0 +1,55 @@
+    $mock_builder = $this->getMockBuilder('Drupal\Core\Field\Plugin\Field\FieldType\EntityReferenceItem')

You have prophesize below, but mockbuilder here

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Assigned: Unassigned » bojanz
Issue tags: -Needs tests

Wrapping this up.

We also need a schema entry such as the one order_store has:

commerce.commerce_condition.plugin.order_store:
  type: commerce_condition_configuration
  mapping:
    stores:
      type: sequence
      label: 'Stores'
      orderby: value
      sequence:
        type: string
        label: 'Store'

  • bojanz committed 8c1bfaa on 8.x-2.x authored by jsacksick 
    Issue #2953940 by pslcbs, jsacksick, bojanz: Add an OrderPaymentGateway...
bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented
Status: Needs review » Fixed

And indeed, Prophecy can't mock magic methods. I switched to using ->get() in the condition itself, so that we don't need mockbuilder.

Status: Fixed » Closed (fixed)

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

pslcbs’s picture

pslcbs CreditAttribution: pslcbs commented

Hi,
Sorry but I think something is wrong on #6 because I was getting some errors related to some lines of code on that patch.

I was trying that features with the committed code on my use case (with commerce_fee) with D8.53 and Commerce 2.6 but I was getting a php error:
Drupal\Core\Entity\EntityStorageException: Unable to get a value with a non-numeric delta in a list. in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 829 of /public_html/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php)

As with patch on #3 it was working for me I was looking for code differences between patches and I found that on patch on #6 @jsacksick made some changes on #3 that were generating my trouble.

Changing it, my environment works again as it did.

On file /modules/payment/src/Plugin/Commerce/Condition/OrderPaymentGateway.php

The "wrong" code

if ($order->get('payment_gateway')->isEmpty()) {
      // The payment gateway is not known yet, the condition cannot pass.
      return FALSE;
}
$payment_gateway_id = $order->get('payment_gateway')->get('target_id');

return in_array($payment_gateway_id, $this->configuration['payment_gateways']);

The "good" code:

$order_payment_gateway = $order->get('payment_gateway')->first()->entity ? $order->get('payment_gateway')->first()->entity->id() : NULL;

return in_array($order_payment_gateway, $this->configuration['payment_gateways']);

Moreover, I think the FilterConditionsEventSubscriber.php is not working correctly because I can see the filter "Limit by payment gateway" on commerce payment gateways settings in admin/commerce/config/payment-gateways page.

bojanz’s picture

bojanz CreditAttribution: bojanz at Centarro commented

@pslcbs
If you're reporting a bug, do it in a new issue.
This issue has been closed, and the described functionality has been released as a part of Commerce 2.6.
Neither of the mentioned patches are relevant anymore.

FWIW I don't see a "Limit by payment gateway" on admin/commerce/config/payment-gateways.