Many of "Roles" hooks don't work

[adrianbj]

Short description of the issue

Most of the Roles hooks don't work

These work
Roles::deleteReady
Roles::deleted
Roles::saveReady
Roles::saved

but these don't work
Roles::add
Roles::delete
Roles::save
Roles::added

Expected behavior

They should work :)

Actual behavior

They don't work :) :)

Steps to reproduce the issue

Add this to ready.php and then create a new role.

$this->addHookBefore("Roles::added", function($event) {
    wire('log')->save('testing', 'Roles::added');
});

Nothing will appear in the log file.

This has been confirmed by others: https://processwire.com/talk/topic/17087-roles-hooks-not-working/

Setup/Environment

• ProcessWire version: 3.0.73, 2.8.62, 2.7.3

[adrianbj]

Same goes for the Permissions class actually. I haven't checked all the direct and inherited methods, but I checked "add" and it doesn't work, but "deleted" does.

[ryancramerdesign]

I'm not really sure what to do about those hooks because they are of pretty limited use. I'm thinking maybe I should just remove or deprecate them. That's because in order for some of them to get called (like those you mentioned, except for added), the method call has to originate on $roles (or $permissions, etc.), and/or the Page has to actually be of a specific type, i.e. Role rather than Page as the class name. There are cases where these conditions aren't present, which means the hooks aren't reliable at catching everything for their type. Best bet is to always use the "Pages::" hooks and check for template there, plus it's more efficient to do so. For these PagesType hooks I think we'll want to deprecate them.

[adrianbj]

If they are problematic then I agree they should be removed - the Pages:: versions will be just fine.

[postscript-chris]

Users::added does not work either @ryancramerdesign

[netcarver]

@ryancramerdesign Bumping for consideration.

[ryancramerdesign]

I had previously made these hooks deprecated. But after @netcarver 's bump, I revisited this and decided to go in the opposite direction and instead make the hooks fully functional in all situations. Rather than only being called if the class name matches what it's supposed to be, the hooks will be called if the template is one that is valid for the type. This ensures they will be called in the situations where they might not have been previously.

[netcarver]

Great, thanks Ryan!

[adrianbj]

Awesome to have these working! I just tested a few and they seem to be working as expected!