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

Fixed null deprecation array_keys in Multishipping. #4418

Merged
merged 4 commits into from
Dec 19, 2024

Conversation

kiatng
Copy link
Contributor

@kiatng kiatng commented Dec 16, 2024

Description (*)

Uncaught TypeError: array_keys(): Argument #1 ($array) must be of type array, null given in .../app/code/core/Mage/Checkout/Model/Type/Multishipping.php:618

@github-actions github-actions bot added the Component: Core Relates to Mage_Core label Dec 16, 2024
@sreichel
Copy link
Contributor

Reproducable in core?

@kiatng
Copy link
Contributor Author

kiatng commented Dec 18, 2024

Reproducable in core?

Yes, it happens on /checkout/multishipping/success/:

Uncaught TypeError: array_keys(): Argument #1 ($array) must be of type array, null given in .../app/code/core/Mage/Checkout/Model/Type/Multishipping.php:618
Stack trace:
#0 .../app/code/core/Mage/Checkout/Model/Type/Multishipping.php(618): array_keys()
#1 .../app/code/core/Mage/Checkout/controllers/MultishippingController.php(578): Mage_Checkout_Model_Type_Multishipping->getOrderIds()
#2 .../app/code/core/Mage/Core/Controller/Varien/Action.php(419): Mage_Checkout_MultishippingController->successAction()
#3 .../app/code/core/Mage/Core/Controller/Varien/Router/Standard.php(255): Mage_Core_Controller_Varien_Action->dispatch()
#4 .../app/code/core/Mage/Core/Controller/Varien/Front.php(180): Mage_Core_Controller_Varien_Router_Standard->match()
#5 .../app/code/core/Mage/Core/Model/App.php(358): Mage_Core_Controller_Varien_Front->dispatch()
#6 .../app/Mage.php(748): Mage_Core_Model_App->run()
#7 .../index.php(68): Mage::run()

@sreichel
Copy link
Contributor

I am not sure about adding methods for core/session. Better fix it in Mage_Checkout_Model_Type_Multishipping?

@kiatng
Copy link
Contributor Author

kiatng commented Dec 19, 2024

My opinion is fixing it upstream is better, it's fixing at one source. It'll fix 3rd-party code.

@sreichel
Copy link
Contributor

What the reason for clear parameter?

@kiatng
Copy link
Contributor Author

kiatng commented Dec 19, 2024

See

public function getData($key = '', $clear = false)
{
$data = parent::getData($key);
if ($clear && isset($this->_data[$key])) {
unset($this->_data[$key]);
}
return $data;
}

And

$ids = Mage::getSingleton('core/session')->getOrderIds(true);

$clear is traditionally used to unset the $_data.

@sreichel
Copy link
Contributor

sreichel commented Dec 19, 2024

Ignore CS, fixed in #4425

@sreichel sreichel merged commit 2c80954 into OpenMage:main Dec 19, 2024
13 of 14 checks passed
@sreichel sreichel deleted the nd_getOrderIds branch December 19, 2024 09:23
fballiano added a commit to MahoCommerce/maho that referenced this pull request Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Relates to Mage_Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants