Make Mage_GiftMessage optional in templates

[Hanmac]

Description (*)

This makes all templates use functions in their blocks to check isModuleEnabled('Mage_GiftMessage') first

Related Pull Requests

Fixed Issues (if relevant)

1. Fixes OpenMage/magento-lts#4263

Manual testing scenarios (*)

1. ...
2. ...

Questions or comments

Contribution checklist (*)

• [x] Pull request has a meaningful description of its purpose
• [x] All commits are accompanied by meaningful commit messages
• [x] All automated tests passed successfully (all builds are green)
• [x] Add yourself to contributors list

[sreichel]

Nice!

For the template you edit, can you please check that this is at top ...

/**
 * @see Mage_Some_Block_Name
 * @var Mage_Some_Block_Name $this
 */

[Hanmac]

Yeah, that annoyed me when i was looking at the templates

Does it need @see if it already has @var this?

[sreichel]

No. var is more importan.

[sreichel]

Great. :)

Can you please add return types to newly added methods?

[Hanmac]

canDisplayGiftmessage() could be existing methods, but I still touched them, so they should be updated?

[sreichel]

To not break anything i'd not change existing methods.

[Hanmac]

To not break anything i'd not change existing methods.

imo, if getLayout->getBlock returns something else than Mage_Core_Block_Abstract, their code is already broken, they just don't know it yet

[sreichel]

Thats true, but we cant change mehtod signatures that are possibly overridden/extended.

[sreichel]

Great :)

[sreichel]

What do think about to use isModuleOutputEnabled to show template parts?

[Hanmac]

What do think about to use isModuleOutputEnabled to show template parts?

What exactly do you mean?

is it about the move of $this->helper('giftmessage/message')->getIsMessagesAvailable('order_item', $parentItem) into Block logic?

[sreichel]

Moving that logic into blocks is great. :)

I think about if its usefull to replace isModuleEnabled checks with isModuleOutputEnabled - to can disable Giftmessage functionallity from backend.

[sreichel]

Can you try to create an order with a downloadable product?

Uncaught Error: Class 'Mage_Giftmessage_Helper_Message'

[Hanmac]

Can you try to create an order with a downloadable product?

Uncaught Error: Class 'Mage_Giftmessage_Helper_Message'

I don't have a working shop right now, (vacation)

But can you give me a stacktrace to fix that?

[sreichel]

Fatal error:  Uncaught Error: Class 'Mage_Giftmessage_Helper_Message' not found in /var/www/html/app/Mage.php:616
Stack trace:
#0 /var/www/html/app/code/core/Mage/Core/Model/Layout.php(627): Mage::helper('giftmessage/mes...')
#1 /var/www/html/app/code/core/Mage/Core/Block/Abstract.php(1105): Mage_Core_Model_Layout->helper('giftmessage/mes...')
#2 /var/www/html/app/code/core/Mage/Adminhtml/Block/Sales/Order/Create/Items/Grid.php(132): Mage_Core_Block_Abstract->helper('giftmessage/mes...')
#3 /var/www/html/app/design/adminhtml/default/default/template/sales/order/create/items/grid.phtml(427): Mage_Adminhtml_Block_Sales_Order_Create_Items_Grid->isGiftMessagesAvailable()
#4 /var/www/html/app/code/core/Mage/Core/Block/Template.php(272): include('/var/www/html/a...')
#5 /var/www/html/app/code/core/Mage/Core/Block/Template.php(309): Mage_Core_Block_Template->fetchView('adminhtml/defau...')
#6 /var/www/html/app/code/core/Mage/Core/Block/Template.php(322): Mage_Core_Block_Template->renderView()
#7 /var/www/html/app/code/core/Mage/A in /var/www/html/app/Mage.php on line 616

Better enjoy your vacation :)

[sreichel]

Very nice. Thanks for fixing phpstan issues too.

First i thought it would be small change ... but no. :)

Lets wait for https://github.com/OpenMage/magento-lts/pull/4323. It could replace all Mage::helper('core')->isModuleOutputEnabled()