Hello,
some notice errors are created when anonymous user redirects from checkout to registration page(using Commerce Checkout Redirect).

Notice: Undefined index: in theme_commerce_checkout_progress_list() (line 173 of /blah/blah/sites/all/modules/commerce(contrib)/commerce_checkout_progress/commerce_checkout_progress.module)
Notice: Undefined index: in theme_commerce_checkout_progress_list() (line 176 of /var/www/fussycloud_master/sites/all/modules/commerce(contrib)/commerce_checkout_progress/commerce_checkout_progress.module)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jcisio’s picture

Status: Active » Postponed (maintainer needs more info)

It looks like the block is display neither in the cart nor a checkout page. If anyone uses Commerce Checkout Redirect and has the same problem, please help.

FiNeX’s picture

I'm experiencing the same issue without "Commerce Checkout Redirect"

brycesenz’s picture

I am also experiencing this issue without "Commerce Checkout Redirect"

brycesenz’s picture

Status: Needs work » Postponed (maintainer needs more info)

Ok, the issue seems to be that in some cases, the variable '$current_page' (passed by the $variables array) is coming into the function uninitialized. I'm still trying to trace this back to figure out where it's all going wrong.

For others debugging the issue, I get this most often when I go to my shopping cart page while it is empty, then refresh the page on my browser.

*UPDATE*
The fact that $current_page is sometimes passed in uninitialized may be a bigger bug (I'm not sure of the maintainers intentions here).

But changing the following in "theme_commerce_checkout_progress_list()" removes the warnings for me:

  foreach ($items as $page_id => $page) {
    $class = array();

    if (isset($items[$current_page]['prev_page'])) {
      $prev_page = $items[$current_page]['prev_page'];
    }
    else {
      $prev_page = FALSE;
    }

    if (isset($items[$current_page]['next_page'])) {
      $next_page = $items[$current_page]['next_page'];
    }
    else {
      $next_page = FALSE;
    }

    if ($page_id === $current_page) {
      $class[] = 'active';
      // Active page and next pages should not be linked.
      $visited = FALSE;
    }
    if ($page_id === $prev_page) {
      $class[] = 'previous';
    }
    if ($page_id === $next_page) {
      $class[] = 'next';
    }
brycesenz’s picture

Status: Postponed (maintainer needs more info) » Needs work

Changing status.

jcisio’s picture

Status: Postponed (maintainer needs more info) » Active

Could you submit #4 as a patch please? It'd be easier for review.

Simon Georges’s picture

Status: Active » Needs review
FileSize
887 bytes

On the "/cart" page, $current_page is NULL, maybe there are other parts of the module to adapt, but so far, the attached patch fixes the notices for me.

jcisio’s picture

I didn't test, but line 37, there is:
$_GET['q'] = 'cart'
Does changing it to
$_GET['q'] == 'cart'
work?

Simon Georges’s picture

@jcisio, I just tested, and although it's probably a good idea to change it, changing it is not sufficient.

My patch is probably not needed if you find the root cause, I just don't have the time at the moment :-(

Simon Georges’s picture

@jcisio, actually, the reason is I haven't checked the "commerce_checkout_progress_cart" box, therefore, the test is always false for me and $page_id not initialised...

jcisio’s picture

Hmm, if you didn't check it, then the block should not appear on the cart page.

Otherwise, the block visibility settings are overriden. In that case, we should remove the check for it (at the same line).

aromka’s picture

Setting "Include a Cart link as the first step" in checkout config fixed it for me.

Simon Georges’s picture

Status: Needs review » Needs work
GiorgosK’s picture

Title: Notices when user is about to register » Notices after upgrading to latest dev of commerce

I think the problem is more general after upgrading to latest dev of commerce (and this module) and visiting an EMPTY cart notices appear. if cart has items no notices

but as #12 pointed out going to admin/commerce/config/checkout and checking "Include a Cart link as the first step" makes notices go away

rei’s picture

confirmed #12 & #14

agoradesign’s picture

If you neither want to include the cart link, nor get the error messages, you've to exclude the cart page from the block's visibility settings. Just go to admin/structure/block/manage/commerce_checkout_progress/indication/configure and check all pages except the cart

sinasalek’s picture

#12 fixed it for me, tx

jmoruzi’s picture

I get the same error messages, but it occurs when updating the quantity of items in the cart.

#12 and #14 made the errors go away for me as well. This hasn't really solved the problem though has it? Why is the error occurring?

jsacksick’s picture

Status: Needs work » Needs review
FileSize
870 bytes

Ok $page_id is not initialized when you're in the cart page unless you check the misnamed "Include a Cart link as the first step", So i added a check on the page_id and renamed "Include a Cart link as the first step" to "Display the cart step".

jcisio’s picture

Looks like a simple fix. Anyone can test please?

PS: I don't understand why the label is said to be misnamed.

jsacksick’s picture

PS: I don't understand why the label is said to be misnamed.

@jcisio: Because the code actually checks the variable at two places and doesn't add the cart step until you check the "Include a Cart link as the first step".
First in commerce_checkout_progress_block_view().


   if (empty($page_id) && variable_get('commerce_checkout_progress_cart', FALSE) && $_GET['q'] = 'cart') {
      $page_id = 'cart';
    }

And in commerce_checkout_progress_get_items :


  if (variable_get('commerce_checkout_progress_cart', FALSE) && module_exists('commerce_cart')) {
    reset($items);
    $first = key($items);
    $items[$first]['prev_page'] = 'cart';
    $items = array_reverse($items);
    $items['cart'] = array(
      'title' => t('Cart'),
      'weight' => -60,
      'status_cart' => TRUE,
      'href' => 'cart',
      'prev_page' => NULL,
      'next_page' => $first,
    );
    $items = array_reverse($items);
  }

That's probably what the checkbox actually means, Add/don't the cart step ? I don't know

jcisio’s picture

Yes, but for me "Include a Cart link as the first step" == "Display the cart step", at least that's what it does. I'm not a native English speaker, BTW. If others confirm, we can change it.

jsacksick’s picture

You're probably right, I'm not a native English speaker either, let's just add the !empty fix and keep the current text. I'm also wondering if we should add an alter hook at the end of commerce_checkout_progress_get_items() to allow alteration by other modules. What do you think ? I'll probably create an other issue with a patch.

jsacksick’s picture

By the way this patch doesn't only remove the cart step, it removes completly the checkout progress on the cart page, I'm not 100% sure that's what you want, but IMO it doesn't make any sense to display a checkout progress when the current step is not displayed.

jcisio’s picture

I has been a long time since I last used this module... However, one question: did you look at comment #8? It didn't fix the problem (per #9) but does it change anything with your patch? IMO #8 is an error and must be fixed.

jsacksick’s picture

#8 is indeed an error and should be fixed, I think this check is useless.
However, concerning my patch, as I said, It will remove the checkout progress on the cart page unless you check the "Include a Cart link as the first step" option.
We can keep it like this or we can try to fix the current implementation and display the next steps but it doesn't make any sense IMO to display the next steps and not the current, what do you think ?

jsacksick’s picture

eviegas’s picture

At line 173: dprint_r($items);

Array
(
    [checkout] => Array
        (
            [title] => Checkout
            [weight] => 0
            [page_id] => checkout
            [name] => Checkout
            [help] => 
            [status_cart] => 1
            [buttons] => 1
            [back_value] => Go back
            [submit_value] => Continue to next step
            [prev_page] => 
            [next_page] => shipping
            [has_item] => 1
        )
...
    if (isset($items[$current_page]['prev_page']) && $page_id === $items[$current_page]['prev_page']) {
        $class[] = 'previous';
    }
    if (isset($items[$current_page]['next_page']) && $page_id === $items[$current_page]['next_page']) {
        $class[] = 'next';
    }
jcisio’s picture

Status: Needs review » Fixed

I'm not sure what #28 means, but I committed #27 last week http://drupalcode.org/project/commerce_checkout_progress.git/commit/3c7a5cf. Either I forgot to comment here or my comment was lost.

Status: Fixed » Closed (fixed)

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

a.milkovsky’s picture

Status: Closed (fixed) » Active

#7
Works for me

jcisio’s picture

Status: Active » Closed (fixed)

Please don't open closed issue to say that a patch works.

dasjo’s picture

As stated in #31, the patch from #7 solved the problem for me as well.