Scenario:
1) admin/store/orders/create
Create an order from admin interface for a new or existing client (maybe anonymous but I didn't test it), Complete client fields, add products to order, click submit order before adding a shipping method*, then select any weight quote shipping method (at least with weight_quote I tried myself) and apply it to order, save the order.

2) admin/store/orders/[your_created_order_id]/packages/new
Create one package (maybe the same error with multiple packages but I didn't test it).

3) admin/store/orders/[your_created_order_id]/shipments/new
Create a new shipping for this order. After you select your packages in shipping form you'll get the following error:

    Notice: Undefined index: method în uc_shipping_shipment_edit() (line 898 of ubercart/shipping/uc_shipping/uc_shipping.admin.inc).
    Notice: Undefined index: rate în uc_shipping_shipment_edit() (line 909 of ubercart/shipping/uc_shipping/uc_shipping.admin.inc).

*) if you click on submit buton on this form before hitting first form submit button you will get an order with cleared client fields but with a shipping method attached. I'll create another issue for this bug too...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

SilviuChingaru’s picture

The problem exists when you change a order created like above status also. I think that at creation form or something....

SilviuChingaru’s picture

*) if you click on submit buton on this form before hitting first form submit button you will get an order with cleared client fields but with a shipping method attached. I'll create another issue for this bug too...

Issue for this bug: #1413372: Admin order shipping method not set and order data reset when shipping quote applied to order

SilviuChingaru’s picture

Status: Active » Needs review
FileSize
851 bytes

Not sure if it is the best practice but it works...

Status: Needs review » Needs work

The last submitted patch, undefined_method_and_rate-1410302-1.patch, failed testing.

SilviuChingaru’s picture

Priority: Normal » Critical
Status: Needs work » Active

Because it brakes hole order object if not saved in db.

TR’s picture

Status: Active » Needs work

Testbot says the patch is bad, so the proper status is needs work.

SilviuChingaru’s picture

This bug is present in RC4 release also...

TR’s picture

What's your point? This issue is still open (status = needs work), so by definition it hasn't been fixed yet. If you want to help maybe you can fix your patch in #3 so that it doesn't break the Ubercart tests.

SilviuChingaru’s picture

Status: Needs work » Needs review
FileSize
1.13 KB

More elegant solution to fix this...

longwave’s picture

Priority: Critical » Normal
SilviuChingaru’s picture

There is still a problem when you don't specify a shipping method via admin > create order: SAME ERROR:

Notice: Undefined index: method în uc_quote_condition_order_shipping_method() (line 31 of sites/[site name]/modules/ubercart/shipping/uc_quote/uc_quote.rules.inc).
SilviuChingaru’s picture

Status: Active » Needs review

The problem is form generated by the AJAX, it just submits the form instead of using AJAX too to replace $form_state['#order'] or only the shipping method. I'm not to good at AJAX forms so any help will be appreciated. I'll try to figure it out also by myself and submit the right patch.

Here is the problem:

function uc_order_pane_quotes($op, $order, &$form = NULL, &$form_state = NULL) {
  switch ($op) {
    case 'edit-form':
...
      if (!empty($form_state['quote_requested'])) {
        $form['quotes']['quotes'] += uc_quote_build_quote_form($order);

        $form['quotes']['quotes']['add_quote'] = array(
          '#type' => 'submit',
          '#value' => t('Apply to order'),
          '#submit' => array('uc_quote_apply_quote_to_order'),
        );
      }

      return $form;

    case 'edit-theme':
      return drupal_render($form['quotes']);
  }
}

it should be something like this:

function uc_order_pane_quotes($op, $order, &$form = NULL, &$form_state = NULL) {
  switch ($op) {
    case 'edit-form':
...
      if (!empty($form_state['quote_requested'])) {
        $form['quotes']['quotes'] += uc_quote_build_quote_form($order);

        $form['quotes']['quotes']['add_quote'] = array(
          '#type' => 'submit',
          '#value' => t('Apply to order'),
          '#submit' => array('uc_quote_apply_quote_to_order'),
// AJAX CALL INSTEAD OF RELOADING ORDER EDIT PAGE
         '#ajax' => array(
            'callback' => 'uc_quote_order_apply_rate',
          ),
        );
      }

      return $form;

    case 'edit-theme':
      return drupal_render($form['quotes']);
  }
}

where uc_quote_order_apply_rate will be something like this:

/**
 * AJAX callback to render the line items.
 */
function uc_quote_order_apply_rate($form, &$form_state) {
  $commands[] = ajax_command_replace('#order-line-items', drupal_render($form['line_items']));
  $commands[] = ajax_command_prepend('#order-line-items', theme('status_messages'));

  return array('#type' => 'ajax', '#commands' => $commands);
}

and uc_quote_apply_quote_to_order will be:

/**
 * Ajax callback: Manually applies a shipping quote to an order.
 */
function uc_quote_apply_quote_to_order($form, &$form_state) {
  if (isset($form_state['values']['quotes']['quote_option'])) {
    if ($order = &$form_state['order']) {
...
      $form_state['quote_set'] = ($form_state['triggering_element']['#value'] == $form['quotes']['quotes']['add_quote']['#value']);
      $form_state['rebuild'] = TRUE;
      
    }
  }
}

But for some reason is working only if I remove the shipping line item manualy first (press red error button).

SilviuChingaru’s picture

Status: Needs review » Active
SilviuChingaru’s picture

Status: Needs review » Active
FileSize
2 KB

I made a patch that is partially working.
What is working so far:
If you edit an order and click on "Get shipping quotes" then "Apply to order" the order is not reloaded, its total change but the line item form (textfields value title and amount) is not changing so when you click "Submit changes" the line item remain the same if you save the order.
But if you don't save the order is working, it change the order shipping method.

Any help would be greatly appreciated. Thank you.

SilviuChingaru’s picture

Status: Active » Needs review
longwave’s picture

This patch hides the error if the order genuinely does not have a shipping quote attached.

Let's deal with the source of the problem in the other issue (which I closed, but am about to reopen as I realise now it's a separate problem)

TR’s picture

FileSize
1.89 KB

Here's a different patch. I don't like the above because:
1) The informational message really is content. While a #description on the fieldset may have the same appearance, it's semantically incorrect. This patch keeps the original #markup form element.
2) The original code and my new patch handle the case where the customer selected a shipping method that is no longer installed. The above patches don't. That feature was put in there based on real-world experience.
3) The above patch doesn't inform the admin if the customer didn't select a shipping method / didn't pay for shipping. This new patch does.

Status: Needs review » Needs work

The last submitted patch, 1410302.patch, failed testing.

TR’s picture

Status: Needs work » Needs review
FileSize
1.89 KB

Hmm, how did that happen. Revised patch without the syntax error:

SilviuChingaru’s picture

FileSize
1.89 KB
SilviuChingaru’s picture

Sorry for duplicate...

SilviuChingaru’s picture

Great... I'll test it tomorrow and let you know.
Anyway there still is problem from #1413372: Admin order shipping method not set and order data reset when shipping quote applied to order that make admin interface unusable for creating orders which, in a real shop, is very often used...

SilviuChingaru’s picture

FileSize
2.07 KB

I think we should print also the message as error for user that tries to enter shipping info using drupal_set_message and also log error for admins. The patch will look like attached...

SilviuChingaru’s picture

Status: Needs review » Active
SilviuChingaru’s picture

Status: Active » Needs review
SilviuChingaru’s picture

FileSize
2.07 KB

Status: Needs review » Needs work

The last submitted patch, 1410302_1.patch, failed testing.

SilviuChingaru’s picture

Status: Needs work » Needs review
FileSize
2.07 KB

Status: Needs review » Needs work

The last submitted patch, 1410302_1.patch, failed testing.

SilviuChingaru’s picture

FileSize
2.07 KB
SilviuChingaru’s picture

Status: Needs work » Needs review
SilviuChingaru’s picture

FileSize
2.1 KB

Fixed watchdog link.

TR’s picture

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

I committed #19 without the additional drupal_set_mesage() or watchdog(). I don't consider this to be an error condition - this is strictly an informational message at this point in the shipping process. If it's considered an error for an order to reach this point without a shipping quote then that condition needs to be caught and corrected when it happens, not somewhere down the line. For example, at checkout, we already have a setting at admin/store/settings/quotes/settings to prevent checkout if customer didn't select a quote - this is the default, but since it's a setting it can be unchecked as well to allow orders to go through without quotes. If the problem is administrator-created orders don't get shipping quotes added, then we ought to catch that and form_set_error() to prevent the administrator from submitting an order without quotes. Or at least detect that condition and bring up a confirmation page to make sure the administrator meant to do that.

Fixed in 7.x-3.x, marking for backport to 6.x-2.x.

TR’s picture

Status: Patch (to be ported) » Fixed

Fixed in 6.x-2.x as well.

SilviuChingaru’s picture

Yes you are right...

Status: Fixed » Closed (fixed)

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