If you try to enable a payment method in a component (instead of in the base rule) you get:
PHP Fatal error: Call to a member function getPluginName() on a non-object in /home/rfay/workspace/commerce/sites/all/modules/commerce/modules/payment/commerce_payment.rules.inc on line 203
This is a result of attempting to traverse too far up the parentage of the rule. I don't even really understand the use of the rule itself in this action, but the attached patch resolves the problem by preventing the traversal past where there are parents.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 1379258-11.payment_rules.patch | 752 bytes | rszrama |
| commerce.payment_method_rule_must_not_traverse_too_far.patch | 596 bytes | rfay |
Comments
Comment #1
rszrama commentedHmm, interesting. With your patch, are you sure it still works properly? As in, will the payment method instance load with all the settings from the action?
Comment #2
rfayI just tried it out with Paypal, and it delivered me to paypal with the correct account.
I do think it's odd that it's even using the rule.. Why would it use the rule, when it's the action (that we're executing) that has the information? It looks like it's just using the rule for a hash to create an id or something.
Comment #3
rszrama commentedThat's what's happening. When we developed it, there was no consistency for Rule IDs, so we had to base it on the name of the Rule and traverse it looking for the action that enables the payment method. I think Rules fixed that bug and we have an open issue to change our ID generation... but honestly I think we'll just wait for 2.x and remove payment gateway configuration from the Rules action altogether (like I did for Commerce Flat Rate for Shipping 2.x).
Comment #4
rfayThat makes sense. However... One rule can contain more than one action that enables a payment gateway, so it doesn't seem right at all. See the rule in http://www.drupalcommerce.org/node/1247#comment-2847, where one rule (but two components as actions) are involved. However, it would certainly be possible for one rule to have two actions. There's no reason for it to cause a fatal.
I believe that this patch is probably OK then as a short-term approach, but it makes the assumption (as the code already did) that the rules "thing" that contains the action only contains one action.
The reason I marked this critical is that it causes a fatal during expected user interaction.
Comment #5
rszrama commentedAhh, ok. I suppose it just needs better documentation, then, as it's an intentional limitation they're bumping against. The Payment module, because of the (at least past) instability of Rule configuration IDs, always only accepts the first action enabling a payment method as valid. It was a limitation of the system at the time, I just don't know if we've documented it on DC.org.
Comment #6
rfayWonder if we could get this in the current round of commits.
Comment #7
rfaycommerce.payment_method_rule_must_not_traverse_too_far.patch queued for re-testing.
Comment #8
tevih commentedI had the same issue and the above patch worked.
Thanks!
Comment #9
rfaycommerce.payment_method_rule_must_not_traverse_too_far.patch queued for re-testing.
Comment #10
vasikei can confirm that i was able to reproduce the issue and the patch save the day.
Comment #11
rszrama commentedI've rewritten this a bit to use the instanceof operator instead of depending on the plugin name like we were. I also wasn't sure that we should still allow a payment method to be enabled if for some odd reason we couldn't determine its ultimate ActionContainer. So instead of just ending the while() loop, I'm returning if there is no parent element, which in turn prevents the payment method from being enabled. This will need a good review.
Comment #12
vasikethe last patch with the last dev made the this issue history.
Comment #13
rszrama commentedThanks for the review. : )