This is using:

  • Drupal 7.42
  • Inline Conditions 7.x-1.0-alpha5
  • Commerce Discount 7-x.1.0-alpha and 7.x-1.x-dev

To reproduce:

  1. Create New Discount
  2. Choose Discount Type "Order Discount"
  3. Under "Order discount conditions" -> "Apply To", select "Product(s) and quantity"
  4. Enter any existing product name and set "quantity equals" to any integer
  5. Complete the "Choose offer type" (there appears to be another css bug which I'll open another issue for)"
  6. Save discount
  7. Try to apply discount (it will not)
  8. Edit the reaction rule for the created discount and you'll get the error:
    Warning: call_user_func() expects parameter 1 to be a valid callback, function '_inline_conditions_order_operator_options' not found or invalid function name in RulesDataUI::renderOptionsLabel() (line 155 of /Users/chrisrockwell/Sites/quick-drupal-20151129042144/drupal-7.x/sites/all/modules/rules/ui/ui.data.inc).

There are a couple ways to fix this. I'm going to attach a couple patches in hopes of getting feedback. I prefer patch #2 (commerce-discount-7.x-options-list-function-doesnt-exist-2.patch) because it aligns the options available in the discount with what is used within the rule. However, it's entirely possible that it should be aligned with `commerce_discount_rules_condition_info()`.

CommentFileSizeAuthor
#20 2__drupal_commerce_discount__bash__and_options_list_in_commerce_discount_rules_condition_info___uses_function_that_doesn_t_exist___2625086____Drupal_org.png101.85 KBjoelpittet
#19 commerce_discount-inline_conditions_function_not_available-2625086-18.patch1.09 KBchrisrockwell
#8 commerce_discount-inline_conditions_function_not_available-2625086-8.patch1.24 KBchrisrockwell
#3 commerce_discount-inline_conditions_function_not_available-2625086-2.patch1.77 KBchrisrockwell
#2 commerce_discount-inline_conditions_function_not_available-2625086-1.patch765 byteschrisrockwell
commerce-discount-7.x-options-list-function-doesnt-exist-2.patch1.77 KBchrisrockwell
commerce-discount-7.x-options-list-function-doesnt-exist-1.patch765 byteschrisrockwell

Comments

chrisrockwell created an issue. See original summary.

chrisrockwell’s picture

chrisrockwell commented
StatusFileSize
new commerce_discount-inline_conditions_function_not_available-2625086-1.patch765 bytes

Fixing patches; realized they had .txt when I tried to use drush-patchfile.

chrisrockwell’s picture

chrisrockwell commented
StatusFileSize
new commerce_discount-inline_conditions_function_not_available-2625086-2.patch1.77 KB

And the second one.

chrisrockwell’s picture

chrisrockwell commented

I just realized I didn't include the module_load_include which is required for patch #2; I'll submit another patch shortly.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs work

Thanks for the patch @chrisrockwell.

Here's a review: The nitpicks are for your reference, I'd have fixed them on commit if it was just those.

  1. +++ b/sites/all/modules/commerce_discount/commerce_discount.inline_conditions.inc
@@ -410,11 +410,7 @@ function commerce_order_has_specific_quantity_products_configure($settings) {
+    '#options' => _inline_conditions_order_operator_options(),

+++ b/sites/all/modules/commerce_discount/commerce_discount.rules.inc
@@ -1420,6 +1420,17 @@ function _commerce_discount_operator_options() {
+function _inline_conditions_order_operator_options() {

    Maybe we should use the existing _commerce_discount_operator_options? 

  2. +++ b/sites/all/modules/commerce_discount/commerce_discount.inline_conditions.inc
@@ -650,4 +646,4 @@ function commerce_product_has_specified_terms_configure($settings) {
-}
+} ¶

    nitpick: white space at end of line added. 

  3. +++ b/sites/all/modules/commerce_discount/commerce_discount.rules.inc
@@ -1420,6 +1420,17 @@ function _commerce_discount_operator_options() {
-    '>=' => t('greater than or equal to'),
+    '>=' => t('greater than or eq ual to'),

    Typo added 

  4. +++ b/sites/all/modules/commerce_discount/commerce_discount.rules.inc
@@ -1420,6 +1420,17 @@ function _commerce_discount_operator_options() {
+ * Options list callback for condition

    Nit: all sentences with periods at the end.

Please if you post another patch set the status to "Needs Review". They get more attention with patches. https://www.drupal.org/node/156119#needs-review

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Oh and make the patch relative to the commerce_discount modules. With git diff --relative flag

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Version: 7.x-1.0-alpha7 » 7.x-1.x-dev
chrisrockwell’s picture

chrisrockwell commented
StatusFileSize
new commerce_discount-inline_conditions_function_not_available-2625086-8.patch1.24 KB

Thank you for the feedback @joelpittet. I've made the changes noted in 2,3, and 4. 1 is addressed by the first patch (most up-to-date is in comment #2: https://www.drupal.org/files/issues/commerce_discount-inline_conditions_...). The reason for creating the alternate (#3) is that what is returned by _commerce_discount_operator_options() isn't consistent with the options available when creating the discount.

And thank you also for the tip on --relative - I've been trying to figure that out since the first patch I submitted.

chrisrockwell’s picture

chrisrockwell commented
Status: Needs work » Needs review

The last submitted patch, commerce-discount-7.x-options-list-function-doesnt-exist-1.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Sorry for being a dunce but I can't follow how the patch resolves the problem in the issue summary.

I wonder if there is a reason for not just using the same callback for the options list too.

The last submitted patch, commerce-discount-7.x-options-list-function-doesnt-exist-2.patch, failed testing.

The last submitted patch, 2: commerce_discount-inline_conditions_function_not_available-2625086-1.patch, failed testing.

The last submitted patch, 3: commerce_discount-inline_conditions_function_not_available-2625086-2.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 8: commerce_discount-inline_conditions_function_not_available-2625086-8.patch, failed testing.

chrisrockwell’s picture

chrisrockwell commented
Status: Needs work » Needs review

Hi @joelpittet,

I probably muddied the waters by submitting two different patches for the same issue. They both solve the issue, but I'm not sure that either are the right way to do it.

The first patch updates the 'options list' (http://cgit.drupalcode.org/commerce_discount/tree/commerce_discount.rule...) to use the existing callback. However, the callback that I think you're referring to (http://cgit.drupalcode.org/commerce_discount/tree/commerce_discount.rule...) doesn't return the same options that are used when building the actual discount: http://cgit.drupalcode.org/commerce_discount/tree/commerce_discount.inli...

Are you suggesting we simply update commerce_discount.inline_conditions.inc to use the existing callback? If so, that *sounds* like a good idea to me but I haven't researched enough to know why only those three options (the ones on line 426 of commerce_discount.inline_conditions) were given. If we can safely use all 5, this would bet the best way to go, in my opinion, and we should also use that callback in the form builder.

Because of that....

The second patch just adds the function that doesn't exist, and returns the same options to rules that are available when creating the discount. I went back through a couple versions, and searched a lot of code assuming _inline_conditions_order_operator_options() existed at some point ( I never found it). Then I updated it the form builder to use the same callback.

Does that clear it up at all?

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Fixed

Kinda hairy but I think I understand:)

I'd like to use the same callback actually for both. One reason is you will notice the default operator is '>=' for the commerce_order_has_specific_quantity_products condition. Which isn't available in that callback.

I just turned on the label and changed it to 'Quantity'. Also defaulted the the value to 1 because it's nothing and that's not good UX.

And needed to include the rules.inc file to get access to that function.

I've committed this to dev. Please download and give it a try. Re-open if I made a mistake.

chrisrockwell’s picture

chrisrockwell commented
Status: Fixed » Needs review
StatusFileSize
new commerce_discount-inline_conditions_function_not_available-2625086-18.patch1.09 KB

I hope it's OK to re-open this, I think we missed something: The callback for inline entity conditions doesn't account for the new comparison operators (<=, >=) so they never kick-in as discounts.

To reproduce:

  1. Create a discount Order->Product and Quantity -> use the "greater than or equal to" or "less than or equal to" selections
  2. Try to get the discount to hit

Please see the attached patch.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Issue summary: View changes
StatusFileSize
new 2__drupal_commerce_discount__bash__and_options_list_in_commerce_discount_rules_condition_info___uses_function_that_doesn_t_exist___2625086____Drupal_org.png101.85 KB

We should probably put in a couple test cases to ensure this is working too. Thanks @chrisrockwell. I'll commit that in a moment. Just a quick note, the whitespace should likely be trimmed in your editor, most editors can do this on save for you. Sometimes they don't and it's accident (phpstorm save on same line for instance).

I'm fine re-opening this, it's an obvious oversight. Thanks for the patch!

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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