Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

addGrandTotals causes str_contains(): Deprecated functionality Passing null #4202

Closed
Hanmac opened this issue Sep 17, 2024 · 0 comments · Fixed by #4203
Closed

addGrandTotals causes str_contains(): Deprecated functionality Passing null #4202

Hanmac opened this issue Sep 17, 2024 · 0 comments · Fixed by #4203

Comments

@Hanmac
Copy link
Contributor

Hanmac commented Sep 17, 2024

Preconditions (*)

  1. Version: 20.3.0+
  2. Have Some Customer Orders for the Report

Steps to reproduce (*)

  1. Open an Admin Report that has some Totals, but not all columns. Like report_customer/totals

Expected result (*)

  1. [Screenshots, logs or description]
  2. No warnings in system.log

Actual result (*)

  1. [Screenshots, logs or description]
  2. Deprecated functionality: str_contains(): Passing null to parameter #1 ($haystack) of type string is deprecated in app/code/core/Mage/Adminhtml/Block/Report/Grid.php

Explaination

This Grid Column in this Grid: Mage_Adminhtml_Block_Report_Customer_Totals_Grid

$this->addColumn('name', [
'header' => $this->__('Customer Name'),
'sortable' => false,
'index' => 'name'
]);

Causes Problems in this function: Mage_Adminhtml_Block_Report_Grid::addGrandTotals

foreach ($this->getColumns() as $key => $_column) {
if (str_contains($_column->getTotal(), '/')) {
list($t1, $t2) = explode('/', $_column->getTotal());
if ($this->getGrandTotals()->getData($t2) != 0) {
$this->getGrandTotals()->setData(
$key,
(float)$this->getGrandTotals()->getData($t1) / $this->getGrandTotals()->getData($t2)
);
}
}
}

An easy fix probably would be to check hasTotal() first? Ala:
$_column->hasTotal() && str_contains($_column->getTotal(), '/')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants