commerce_line_item_form_alter may lead to this core issue: "Warning : array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController" (#1803048: Warning : array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController) as it doesn't check the value given to commerce_order_load() correctly.
In some cases (in mine for anonymous users loading the page) it provides an array with 1 booelan element instead of the order ID to load:
[0] => false
So I'd suggest further checking the value before providing the parameter. See Mr.
commerce_line_item.module, Line 106 ff.
/**
* Implements hook_form_alter().
*
* Alter the views form with functionality specific to line items.
* This form currently only supports line items from a single order, and it
* determines which order the line items are for based on a Views argument.
*/
function commerce_line_item_form_alter(&$form, &$form_state, $form_id) {
$line_item_form = FALSE;
// Is this a views form?
if (strpos($form_id, 'views_form_') === 0) {
$view = $form_state['build_info']['args'][0];
// Does the view contain one of the line item edit fields?
foreach ($view->field as $field_name => $field) {
if ($field instanceof commerce_line_item_handler_field_edit_delete || $field instanceof commerce_line_item_handler_field_edit_quantity) {
$line_item_form = TRUE;
}
}
}
// This is not the form we are looking for.
if (!$line_item_form) {
return;
}
// Require the existence of an order_id argument (and its value).
if (empty($view->argument['order_id']) || empty($view->argument['order_id']->value)) {
return;
}
$form['#attached']['css'][] = drupal_get_path('module', 'commerce_line_item') . '/theme/commerce_line_item.theme.css';
$form['#attached']['js'][] = drupal_get_path('module', 'commerce_line_item') . '/commerce_line_item.js';
$form['#submit'][] = 'commerce_line_item_line_item_views_form_submit';
// Wrap the form in a div so we can add CSS and javascript behaviors to it.
$form['#prefix'] = '<div class="commerce-line-item-views-form">';
$form['#suffix'] = '</div>';
// Add an additional class to the actions wrapper.
$form['actions']['#attributes']['class'][] = 'commerce-line-item-actions';
// Load the order from the Views argument.
$order = commerce_order_load($view->argument['order_id']->value[0]);
$form_state['order'] = $order;
}
[1] => Array
(
[file] => /sites/all/modules/contrib/commerce/modules/order/commerce_order.module
[line] => 782
[function] => entity_load
[args] => Array
(
[0] => commerce_order
[1] => Array
(
[0] => false
)[2] => Array
(
)[3] =>
))
[2] => Array
(
[file] => /sites/all/modules/contrib/commerce/modules/order/commerce_order.module
[line] => 751
[function] => commerce_order_load_multiple
[args] => Array
(
[0] => Array
(
[0] => false
)[1] => Array
(
))
)
[3] => Array
(
[file] => /sites/all/modules/contrib/commerce/modules/line_item/commerce_line_item.module
[line] => 143
[function] => commerce_order_load
[args] => Array
(
[0] => false
))
[4] => Array
(
[file] => /includes/module.inc
[line] => 1171
[function] => commerce_line_item_form_alter
[args] => Array
Issue fork commerce-3236711
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
anybodyI'm still not sure what this is caused by in the end, but I'm sure we should harden the code to never load an order from a broken order id parameter. :)
Please review the MR.
Comment #4
rszrama commentedNo clue how to reproduce, but the cart form still works fine after applying this. 😅
Comment #5
rszrama commentedComment #7
rszrama commentedActually, apparently introducing this change causes test failures due to the same errors ... they just appear further down the stream for some reason. I think I'm just gonna revert and leave this be for now. Let's reopen this if we can find a way to reproduce it reliably.
Comment #8
anybodyHi @rszrama,
thank you very very much! :)
I'll try to find out, what caused this. I guess it was a rule or view...
But from having a look at the code I still think it makes a lot of sense and we should check how it can lead to errors down the stream.
Compare
So from my perspective the patch can only harden the logics?
Simply doesn't make sense to call commerce_order_load() with a non-numeric value? And in a form alter hook we shouldn't expect the value to be not empty, I guess...
Can I see the test results online?
Comment #9
rszrama commentedYeah, that's what was so weird ... it looked like as safe a change as possible. Tests were specifically failing against PHP 5:
https://www.drupal.org/pift-ci-job/2221760
Comment #10
anybodyThat is so strange ;D
Especially that the error pops up that was the original issue for me which this patch should fix...
The reason for other failing tests is that
is empty, which may happen if the condition kicks out a valid value. Can a commerce order (especially in tests) ever have a zero ID? So that the empty() check breaks it?
But honestly, I don't really get it yet...
Comment #11
anybodyAnother follow-up, propably based on the same root cause:
#3332380: $form_state['order'] is empty on cart submit
Comment #12
jsacksick commentedD7 is EOL, so closing this outdated issue.