OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
868 stars 436 forks source link

Mage_GiftMessage shouldn't be required for Mage_Adminhtml #4263

Open Hanmac opened 2 weeks ago

Hanmac commented 2 weeks ago

Preconditions (*)

  1. should be all versions

Steps to reproduce (*)

  1. have Mage_Giftmessage be disabled via etc/modules
  2. Open an existing Sales/Order

Expected result (*)

  1. Sales/Order does open

Actual result (*)

  1. Exception
Error: Class "Mage_Giftmessage_Helper_Message" not found in /var/www/app/Mage.php:612
Stack trace:
#0 /var/www//app/code/core/Mage/Core/Model/Layout.php(625): Mage::helper()
#1 /var/www/app/code/core/Mage/Core/Block/Abstract.php(1108): Mage_Core_Model_Layout->helper()
#2 /var/www/app/code/core/Mage/Adminhtml/Block/Sales/Order/View/Items/Renderer/Default.php(208): Mage_Core_Block_Abstract->helper()
#3 /var/www/app/design/adminhtml/default/default/template/sales/order/view/items/renderer/default.phtml(29): Mage_Adminhtml_Block_Sales_Order_View_Items_Renderer_Default->canDisplayGiftmessage()
#4 /var/www/app/code/core/Mage/Core/Block/Template.php(273): include('...')
#5 /var/www/app/code/core/Mage/Core/Block/Template.php(310): Mage_Core_Block_Template->fetchView()

Possible Solution

Currently in the Route adminhtml_sales_order_view, the gift_options Block is added there: https://github.com/OpenMage/magento-lts/blob/5a95706f4d03d94f90878d12ac448d1a1e28096d/app/design/adminhtml/default/default/layout/sales.xml#L86-L88

That in itself isn't the problem, because the Template itself checks again: https://github.com/OpenMage/magento-lts/blob/5a95706f4d03d94f90878d12ac448d1a1e28096d/app/design/adminhtml/default/default/template/sales/order/view/giftmessage.phtml#L16 which calls this function https://github.com/OpenMage/magento-lts/blob/5a95706f4d03d94f90878d12ac448d1a1e28096d/app/code/core/Mage/Adminhtml/Block/Sales/Order/View/Giftmessage.php#L279-L284

or a different function for the different Item Renderer https://github.com/OpenMage/magento-lts/blob/5a95706f4d03d94f90878d12ac448d1a1e28096d/app/code/core/Mage/Adminhtml/Block/Sales/Order/View/Items/Renderer/Default.php#L205-L214

The problem there is the direct call to the Helper Mage_GiftMessage_Helper_Message, without checking first if the Module Mage_GiftMessage is enabled.

Probably both these functions should have this?

if (!Mage::helper('core')->isModuleEnabled('Mage_GiftMessage')) {
  return false;
}

I haven't checked all Routes yet if they need to be protected like this

sreichel commented 2 weeks ago

Also see https://www.vianetz.com/de/blog/magento-remove-giftmessage-extension/

Hanmac commented 2 weeks ago

I added a Draft PR, it doesn't address the product attribute, but the templates all have better protection functions now