Skip to content

Add a 'priority' parameter to observers #173

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

Closed
wants to merge 1 commit into from
Closed

Add a 'priority' parameter to observers #173

wants to merge 1 commit into from

Conversation

hectorj
Copy link

@hectorj hectorj commented Dec 23, 2012

Allow to add a "priority" parameter to observers, to manage in which order they will be called.
(there is a conception choice to make : order by priority DESC or ASC)
(default priority : 0)

if($a['priority']==$b['priority']){
return 0;
} else {
return $a['priority']<$b['priority'] ? -1 : 1;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is where you choose if you order by DESC priority or ASC.
return $a['priority']<$b['priority'] ? -1 : 1; => ASC
return $a['priority']>$b['priority'] ? -1 : 1; => DESC

@magento-team
Copy link
Contributor

@hectorj
The idea of introduction of priority to event observers has been already discussed internally and rejected. Here is some grounding:

  • dependency on priority occurs only when even data is being modified by observers, which doesn't happen in most cases
  • specifying priority promotes quick-fixes rather than designing events and event arguments in such a way that there would be no dependency on the execution order of event handlers

Rejecting the contribution and closing the ticket.

magento-team pushed a commit that referenced this pull request Mar 23, 2015
okorshenko pushed a commit to isitnikov/magento2 that referenced this pull request Aug 10, 2016
[Support] Portdown MAGETWO-45339 down to M2.0.x branch
VitaliyBoyko pushed a commit to VitaliyBoyko/magento2 that referenced this pull request Nov 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants