Hello!

Recently found a problem in ec_recurring.module.

If you have a several schedules (more than one) created and if you assign one schedule to some product, and other schedule to other product and then you process a purchase you will see that wrong schedules assigned to purchased product. Really it's a wrong schedules assigne to products at the moment of product creation in admin.

Problem was found at line 281 on ec_recurring.module file in function ec_recurring_form_alter($form_id, &$form):

Line:

    $slist = array_merge(array(0 => '--'), ec_recurring_get_list($allow_renewals));

do wrong array merging.

To fix you should replace that line with:

    $tlist = ec_recurring_get_list($allow_renewals);
    $slist[0] = "--";

    if (is_array($tlist))
    {
      foreach ($tlist as $key => $value)
      {
        $slist[$key] = $value;
      }
    }

or some shorter commands that do correct array merging.

Module developers please fix ec_recurring.module with better solution.

Thanks!

--
With best regards,
Dmitry Yeskin
President of Web Style Media, LLC
Internet Marketing and Internet Advertising Agency

CommentFileSizeAuthor
#3 ec_recurring - corrected wrong sid values in select box - 151607 - 3.patch860 bytesjbjaaz
#1 ec_recurring - corrected wrong sid values in select box - 151607 - 1.patch599 bytesjbjaaz

Comments

jbjaaz’s picture

jbjaaz commented
Version: 5.x-3.0 » 5.x-3.x-dev
StatusFileSize
new ec_recurring - corrected wrong sid values in select box - 151607 - 1.patch599 bytes

I had the same problem and this fix worked for me.

I optimized your code a little. You don't need if (is_array($tlist)) if statement because ec_recurring_get_list always returns an array.

I attached a modified patch.

sime’s picture

sime
any/any
Location Melbourne
commented
Status: Patch (to be ported) » Needs work

better description of issue state. needs patch.

jbjaaz’s picture

jbjaaz commented
Title: Schedule ID assigned wrong + bug fix » Renewal schedule select box not populated correctly after schedules are removed
Version: 5.x-3.x-dev » 5.x-3.2
Status: Needs work » Needs review
StatusFileSize
new ec_recurring - corrected wrong sid values in select box - 151607 - 3.patch860 bytes

Here's a better description of the problem.

- create some schedules
- delete the schedules
- create a couple more
- create a product
- select a renewal schedule and save the product
- edit the product again and notice that the "renewal schedule" you selected was not saved properly.

It turns out, using array_merge at
$slist = array_merge(array(0 => '--'), ec_recurring_get_list($allow_renewals));
is renumbering the numeric keys.

This is PHP doing it's thing. See example 257 of the array_merge documentation.

So, changing that line to
$slist = array(0 => '--') + ec_recurring_get_list($allow_renewals);
preserves the keys.

A patch for this one-liner is attached.

Much cleaner than the originally proposed code fix :-P

mark matuschka’s picture

mark matuschka commented

+ operator for arrays == sweet!
Good one.

gordon’s picture

gordon commented
Status: Needs review » Closed (duplicate)

duplicate of #163429

Now fixed in all version