Hi guys,

I have E_ALL turned on and I get loads of notices when I create a new recurring field. This is because the $product variable is set only if you are editing an existing field.

I would suggest that defaults be defined in an else statement:

  if (!empty($feature)) {
    $product = uc_recurring_fee_product_load($feature['pfid']);
  }
  else {
    $product = new stdClass;
    $product->model = '';
    ...
  }

I don't really see a real problem except that some places check fields against 0 with <. That may not always work as expected.

If I find the time, I'll post a patch.

Thank you.
Alexis

CommentFileSizeAuthor
#12 uc_recurring-undefined-vars-props-795094-12.patch16.79 KBtinker
#12 uc_recurring-undefined-vars-props-795094-12-interdiff.txt15.89 KBtinker
#11 uc_recurring-795094-11-undefined_various.patch16.19 KBxurizaemon
#10 uc_recurring-795094-10-undefined_stuff.patch16.17 KBxurizaemon
#9 uc_recurring-795094-squash_undefined_variables-9.patch15.56 KBxurizaemon
#8 uc_recurring-795094-undefined_variables_on_new_product.patch6.21 KBxurizaemon
#7 uc_recurring-795094-undefined_variables_on_new_product.patch5.73 KBxurizaemon
#5 uc_recurring-795094-5.patch1.9 KBagileware

Comments

AlexisWilke’s picture

AlexisWilke commented

I meant to post the list of errors too! 8-)

* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1181.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1181.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1192.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1192.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1209.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1209.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1213.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1213.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1232.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1232.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1242.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1242.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1255.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1255.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1265.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1265.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1275.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1275.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1287.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1287.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1287.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1287.
* notice: Undefined variable: product in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1288.
* notice: Trying to get property of non-object in /usr/clients/www/drupal/sites/all/modules/uc_recurring/uc_recurring.module on line 1288.

univate’s picture

univate commented
Title: Errors when creating a new recurring fee » E_ALL Warnings when creating a new recurring fee
agileware’s picture

agileware commented
Version: 6.x-2.x-dev » 7.x-2.x-dev

A drupal 7 issue too
d7 message:

Notice: Undefined variable: product in uc_recurring_product_feature_form() (line 57 of /sites/all/modules/contrib/uc_recurring/modules/uc_recurring_product/uc_recurring_product.module).
Notice: Trying to get property of non-object in uc_recurring_product_feature_form() (line 57 of /sites/all/modules/contrib/uc_recurring/modules/uc_recurring_product/uc_recurring_product.module).
Notice: Undefined variable: product in uc_recurring_product_feature_form() (line 74 of /sites/all/modules/contrib/uc_recurring/modules/uc_recurring_product/uc_recurring_product.module).
Notice: Trying to get property of non-object in uc_recurring_product_feature_form() (line 74 of /sites/all/modules/contrib/uc_recurring/modules/uc_recurring_product/uc_recurring_product.module).
Notice: Undefined variable: product in uc_recurring_product_feature_form() (line 78 of /sites/all/modules/contrib/uc_recurring/modules/uc_recurring_product/uc_recurring_product.module).
Notice: Trying to get property of non-object in uc_recurring_product_feature_form() (line 78 of /sites/all/modules/contrib/uc_recurring/modules/uc_recurring_product/uc_recurring_product.module).

agileware’s picture

agileware commented
Title: E_ALL Warnings when creating a new recurring fee » Undefined variable and undefined property of non-object when adding a new recurring fee product feature
Status: Active » Needs review
StatusFileSize
new uc_recurring-795094-5.patch1.9 KB

Patch to fix in D7.

agileware’s picture

agileware commented
Version: 7.x-2.x-dev » 6.x-2.x-dev
Status: Needs review » Patch (to be ported)

In drupal 7 this is now included in the meta patch at #992252-21: uc_recurring Drupal 7 port.

Probably should be fixed in drupal 6 too.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
StatusFileSize
new uc_recurring-795094-undefined_variables_on_new_product.patch5.73 KB

move along

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new uc_recurring-795094-undefined_variables_on_new_product.patch6.21 KB

MOVE ALONG

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
StatusFileSize
new uc_recurring-795094-squash_undefined_variables-9.patch15.56 KB

Here's what I've got so far - squashes lots of such messages throughout the new product / make order / view order / charge process. Patch for D6, not via #992252: uc_recurring Drupal 7 port though so you may prefer to backport.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
StatusFileSize
new uc_recurring-795094-10-undefined_stuff.patch16.17 KB

EDIT: Noted in testing that this patch doesn't correctly populate the "number of billing periods" field.

xurizaemon’s picture

xurizaemon
he/him
Location Ōtepoti, Aotearoa 🏝
commented
StatusFileSize
new uc_recurring-795094-11-undefined_various.patch16.19 KB

Fixes also "number of billing periods".

tinker’s picture

tinker commented
Priority: Minor » Normal
StatusFileSize
new uc_recurring-undefined-vars-props-795094-12-interdiff.txt15.89 KB
new uc_recurring-undefined-vars-props-795094-12.patch16.79 KB

@grobot, Thanks for all the hard work identifying undefined variables and properties. Some of your changes point to problems that have me a little worried.

You have added checks for:

  • isset($order->order_id) in uc_recurring_get_fees(). It should never happen that an order does not have an order_id. Maybe some faulty code is passing an order that failed to load and is FALSE? This is not happening for me using Authorize.net CIM so I cannot reproduce. You should probably check your system and see why this is happening. Please let me know your findings.
  • isset(product->order_product_id) Again this should never happen. The order went through checkout and each product *must* have an order_product_id. Maybe this is caused by the previous point?

I have changed uc_recurring_get_fees() to return array() if(!isset($order->order_id)) this should stop errors. You can add a backtrace to that if statement and see what caused it.

Other changes:

  1. Return TRUE or FALSE in uc_recurring_order_process_order()
  2. Rebuilt product feature form to use product and form_state['values'] if they are set. The original version did not use form_state['values'] but since you took the effort to check for product correctly I thought I would fix this entirely.
  3. 'new stdClass' should be 'new stdClass()'
  4. Reverted edits in uc_recurring_order_process_order() as they were probably a cascading fault that is now addressed. Please let me knowif you still get errors here.
  5. $product->order_id = NULL to $product->order_id = $order->order_id in uc_recurring_product_recurring_renewal_pending()
  6. Merged the watchdog messages into previous If() as noted in your todo
  7. Simplified uc_recurring_get_extension() db_query. This should return max 1 result.
  8. Added default $extensions = array() in uc_recurring_get_extension_list() that was missed.
  9. Reverted changes to theme_uc_recurring_admin_table(). $fee->name already comes from $account->name. If it is empty then it's due to the fee->uid = 0 bug. Looking it up again will not help.

Check the interdiff as I may have missed something. All the other changes you made work great.

The full patch can be applied using
git am uc_recurring-undefined-vars-props-795094-12.patch