Magento2: Adding a handler to the Magento\Catalog\Helper\Output Class

Created on 21 Nov 2017  路  13Comments  路  Source: magento/magento2

I want to add a custom productAttribute handler to the Magento\Catalog\Helper\Output Class. The changelog says it should be done by using a Plugin (because event catalog_helper_output_construct is removed) but as far as I can see, each available method would add the handler multiple times if I don't check if it is already added (which is no clean solution in my opinion). The only proper solution would be adding it after the __construct method with an after plugin method but plugins can't be used with those methods. Does anyone know how this can be done properly?

Preconditions


  1. Magento 2.1.10
  2. PHP 7.0.25

Steps to reproduce

  1. Add a handler by using a plugin method on the Magento\Catalog\Helper\Output Class
  2. The plugin method gets called multiple times
  3. The handler gets added multiple times

Expected result

  1. A proper formatted attribute

Actual result

  1. The proper formatted attribute has been formatted way to much.

Catalog Fixed in 2.3.x Clear Description Confirmed Format is valid Ready for Work Reproduced on 2.1.x Reproduced on 2.2.x Reproduced on 2.3.x help wanted

Most helpful comment

@okobchenko , we need a method that can be used to "collect" all handlers. The method "getHandlers" is a simple getter and should not be used to keep loading handlers on every call.

The init() method may be an anti-pattern, but using a getter to collect stuff is most certainly not a good practise either.

All 13 comments

@quisse, thank you for your report.
Unfortunately, now your report seems unclear. Please provide more detailed steps to reproduce and actual result.

@quisse means that there is no good place to put the plugin on the Magento\Catalog\Helper\Output class

There are several public methods where we could add the plugin, but all of them will cause the logic to run more than once (every time an attribute is output for example, the handler would be re-added, forcing us to write extra logic to check if it wasn't already - not handy).

In our opinion, there should be an init() method, where the handlers can be added once, in a clean manner.

Previously in Magento 1 there was an event to do this, but for M2 the comment was made that one should use plugins instead. The person who wrote that did obviously not take a good look at the class as there is no good insertion point for the plugin.

@magento-engcom-team As Wouter says. I've also added a link to the class in magento 1.9 where the event is dispatched. We need this back in Magento 2 for properly adding custom productAttribute handlers since there is no other clean way to add those.

Hi @quisse
Please let me know why you can not add plugin to getHandlers method?
@woutersamaey init method is the antipattern . We are trying to cleanup our code from such cases
Dispatching events in the class constructor is prohibited in Magento 2, so we can not revert this event.

I'm closing this ticket as non-issue.

Hi @okorshenko,

I can add a plugin to the getHandlers method. But when adding a handler in the plugin for the getHandlers method, my handler is added everytime the getHandlers method is called. As a quick fix I check if it doesn't exist yet in the array of handlers, but a better fix would be placing the event back, in the __construct method or some other method which gets called only once.

(btw, I've never mentioned that I can't add a plugin around the getHandlers method so closing this issue and labeling it as a non-issue seems a bit short sighted?)

@okobchenko , we need a method that can be used to "collect" all handlers. The method "getHandlers" is a simple getter and should not be used to keep loading handlers on every call.

The init() method may be an anti-pattern, but using a getter to collect stuff is most certainly not a good practise either.

Hi @okorshenko,
Any update/response on this?

Following gist contains the current way of adding an outputHandler. This isn't a good way because the plugin triggers too much.
https://gist.github.com/quisse/b78ee3d121a4c9b8dd141334c8fa27a8

As discussed in https://github.com/magento/magento2/pull/14527 this is an issue which requires major refactoring and a more complex solution

Still willing to create a PR, if someone can push me in the right direction?

@okorshenko

@akaplya Can you point me in the right direction please?

Hi @quisse. Thank you for your report.
The issue has been fixed in magento/magento2#24405 by @thomas-kl1 in 2.3-develop branch
Related commit(s):

The fix will be available with the upcoming 2.3.4 release.

Was this page helpful?
0 / 5 - 0 ratings