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

Merged PR #2814 is causing trouble with the MageWorx Advanced Product Options extension #2874

Closed
ADDISON74 opened this issue Jan 1, 2023 · 1 comment · Fixed by #2878
Closed
Assignees
Labels
3rd-party Related to 3rd-party code or issues with customization

Comments

@ADDISON74
Copy link
Contributor

ADDISON74 commented Jan 1, 2023

A Happy New Year! Good health and happiness to everyone in 2023!

This great extension (still available but unsupported) has a feature to create templates for custom options. Specifically, in a grid page in the Backend called "Mage Options Templates" there is the [Add Options Template] button, see the screenshot bellow

add_template

Once I press the button, the browser window loads empty and I get the following error in the exception.log file

2023-01-01T12:52:33+00:00 ERR (3): 
TypeError: Mage_Adminhtml_Block_Widget_Grid_Massaction_Abstract::isConfirmMassAction(): Argument #1 ($itemId) must be of type string, null given, called in /var/www/html/app/code/core/Mage/Adminhtml/Block/Widget/Grid/Massaction/Abstract.php on line 113 and defined in /var/www/html/app/code/core/Mage/Adminhtml/Block/Widget/Grid/Massaction/Abstract.php:369
Stack trace:
#0 /var/www/html/app/code/core/Mage/Adminhtml/Block/Widget/Grid/Massaction/Abstract.php(113): Mage_Adminhtml_Block_Widget_Grid_Massaction_Abstract->isConfirmMassAction(NULL)
#1 /var/www/html/app/code/community/MageWorx/CustomOptions/Block/Adminhtml/Options/Edit/Tab/Product.php(212): Mage_Adminhtml_Block_Widget_Grid_Massaction_Abstract->addItem(NULL, Array)
#2 /var/www/html/app/code/core/Mage/Adminhtml/Block/Widget/Grid.php(650): MageWorx_CustomOptions_Block_Adminhtml_Options_Edit_Tab_Product->_prepareMassaction()
#3 /var/www/html/app/code/core/Mage/Adminhtml/Block/Widget/Grid.php(707): Mage_Adminhtml_Block_Widget_Grid->_prepareMassactionBlock()
#4 /var/www/html/app/code/core/Mage/Adminhtml/Block/Widget/Grid.php(719): Mage_Adminhtml_Block_Widget_Grid->_prepareGrid()
#5 /var/www/html/app/code/core/Mage/Core/Block/Abstract.php(931): Mage_Adminhtml_Block_Widget_Grid->_beforeToHtml()
#6 /var/www/html/app/code/community/MageWorx/CustomOptions/Block/Adminhtml/Options/Edit/Tabs.php(39): Mage_Core_Block_Abstract->toHtml()
#7 /var/www/html/app/code/core/Mage/Core/Block/Abstract.php(931): MageWorx_CustomOptions_Block_Adminhtml_Options_Edit_Tabs->_beforeToHtml()
#8 /var/www/html/app/code/core/Mage/Core/Block/Text/List.php(42): Mage_Core_Block_Abstract->toHtml()
#9 /var/www/html/app/code/core/Mage/Core/Block/Abstract.php(932): Mage_Core_Block_Text_List->_toHtml()
#10 /var/www/html/app/code/core/Mage/Core/Block/Abstract.php(650): Mage_Core_Block_Abstract->toHtml()
#11 /var/www/html/app/code/core/Mage/Core/Block/Abstract.php(594): Mage_Core_Block_Abstract->_getChildHtml('left', true)
#12 /var/www/html/app/design/adminhtml/default/default/template/page.phtml(53): Mage_Core_Block_Abstract->getChildHtml('left')
#13 /var/www/html/app/code/core/Mage/Core/Block/Template.php(257): include('/var/www/html/a...')
#14 /var/www/html/app/code/core/Mage/Core/Block/Template.php(291): Mage_Core_Block_Template->fetchView('adminhtml/defau...')
#15 /var/www/html/app/code/core/Mage/Core/Block/Template.php(304): Mage_Core_Block_Template->renderView()
#16 /var/www/html/app/code/core/Mage/Adminhtml/Block/Template.php(74): Mage_Core_Block_Template->_toHtml()
#17 /var/www/html/app/code/core/Mage/Core/Block/Abstract.php(932): Mage_Adminhtml_Block_Template->_toHtml()
#18 /var/www/html/app/code/core/Mage/Core/Model/Layout.php(580): Mage_Core_Block_Abstract->toHtml()
#19 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Action.php(397): Mage_Core_Model_Layout->getOutput()
#20 /var/www/html/app/code/community/MageWorx/CustomOptions/controllers/Adminhtml/Mageworx/Customoptions/OptionsController.php(60): Mage_Core_Controller_Varien_Action->renderLayout()
#21 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Action.php(428): MageWorx_CustomOptions_Adminhtml_Mageworx_Customoptions_OptionsController->editAction()
#22 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Router/Standard.php(262): Mage_Core_Controller_Varien_Action->dispatch('edit')
#23 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Front.php(188): Mage_Core_Controller_Varien_Router_Standard->match(Object(Mage_Core_Controller_Request_Http))
#24 /var/www/html/app/code/core/Mage/Core/Model/App.php(371): Mage_Core_Controller_Varien_Front->dispatch()
#25 /var/www/html/app/Mage.php(745): Mage_Core_Model_App->run(Array)
#26 /var/www/html/index.php(71): Mage::run('', 'store')
#27 {main}

I am using DDEV so it was easy to switch PHP versions, from 7.0 (skipping the OM condition of using > 7.3) to 8.2. The error did not disappeared. Then I used XDebug and I didn't get any error in the extension files. The error above is written in the exception.log file when the running passes the if statement in the file /app/code/core/Mage/Adminhtml/Block/Widget/Grid/Massaction/Abstract.php, between lines 113 - 115, introduced in PR #2814. If I comment it the page loads as expected, there are no more errors in the log files.

I went further and I replaced in the same file in line 369 string $itemId with $itemId in this method

    protected function isConfirmMassAction(string $itemId): bool
    {
        return in_array($itemId, static::$needsConfirm);
    }

the page loads in browser without any issues. Most likely, the change does not take into account the situation in which the argument could have a null value?

TypeError: Mage_Adminhtml_Block_Widget_Grid_Massaction_Abstract::isConfirmMassAction(): Argument #1 ($itemId) must be of type string, null given, called in /var/www/html/app/code/core/Mage/Adminhtml/Block/Widget/Grid/Massaction/Abstract.php on line 113 and defined in /var/www/html/app/code/core/Mage/Adminhtml/Block/Widget/Grid/Massaction/Abstract.php:369
@elidrissidev elidrissidev added the 3rd-party Related to 3rd-party code or issues with customization label Jan 1, 2023
@sreichel sreichel self-assigned this Jan 1, 2023
@sreichel sreichel mentioned this issue Jan 1, 2023
4 tasks
@sreichel
Copy link
Contributor

sreichel commented Jan 2, 2023

Most likely, the change does not take into account the situation in which the argument could have a null value?

Did not expect that someone passes null as identifier (that is later used as array-key).

@mageworx is this single-line fix possible? Just pass a string instead null?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd-party Related to 3rd-party code or issues with customization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants