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

Notifications only check for hook handlers registered with specific types #9325

Closed
hypeJunction opened this Issue Jan 29, 2016 · 5 comments

Comments

Projects
None yet
2 participants
@hypeJunction
Copy link
Contributor

hypeJunction commented Jan 29, 2016

Notifications detects new style hook-based set up using hooks ->hasHandler('prepare', $type). This fails if your handler is registered using the "all" keyword, because hasHandler looks for a specific registration name/type pair.

@mrclay

This comment has been minimized.

Copy link
Member

mrclay commented Jan 29, 2016

This is internal, so what's the concern. For 2.0 here's a PR to document it #9326.

@hypeJunction

This comment has been minimized.

Copy link
Contributor Author

hypeJunction commented Jan 29, 2016

My problem is with the use of hasHandler in notifications service. I want to register a single handler for all 'prepare' hooks, but it's not possible.

@mrclay mrclay changed the title HooksRegistrationService::hasHandler() doesn't look for 'all' keywords Notifications only check for hook handlers registered with specific types Jan 29, 2016

@mrclay

This comment has been minimized.

Copy link
Member

mrclay commented Jan 29, 2016

I hope it's OK I rewrote the description. The problem is we're using handler presence as an API switch (maybe unwisely). If I register a universal menu handler for [prepare, all], then we'll end up switching to the new API possibly in error.

Ideally the API would have just called a hook and used the return value to determine whether to use the new API.

@mrclay

This comment has been minimized.

Copy link
Member

mrclay commented Jan 29, 2016

may be of interest to @juho-jaakkola

@hypeJunction

This comment has been minimized.

Copy link
Contributor Author

hypeJunction commented Jan 29, 2016

The handlers would have to sniff the hook type to decide whether they have
to act upon it. I suppose Calling deprecated handlers would be wiser based
on return.

On Friday, January 29, 2016, Steve Clay notifications@github.com wrote:

may be of interest to @juho-jaakkola https://github.com/juho-jaakkola


Reply to this email directly or view it on GitHub
#9325 (comment).

@mrclay mrclay closed this in 498abdd Feb 9, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment