Multisite: removed_user_from_blog hook

[kevinu]

I useful hook would be something like "removed_user_from_blog". As it is now there is not a way to hook in once a user has been removed from a multisite blog.

There could be many use cases for this such as a custom notice to the user that was removed, doing something with the user after they are removed, etc.

[kevinu]

Patch file for changes to add this hook

[kevinu]

Hello! I have created a patch to make this change and attached it to this ticket. When a maintainer gets a chance I would love a review.

[SergeyBiryukov]

Hi there, welcome to WordPress Trac! Thanks for the ticket and the patch.

My first thought was that it might be good idea to have a corresponding added_user_to_blog action, however that turned out to be more complicated than I expected.

Looks like there are some inconsistencies between these related functions:

• In add_user_to_blog()​, the add_user_to_blog action fires immediately after a user is added to a site.
• In remove_user_from_blog()​, the remove_user_from_blog action fires before a user is removed from a site.
• In add_existing_user_to_blog()​, the added_existing_user action fires immediately after a user is added.
• In add_new_user_to_blog()​, no action is fired.

That does not necessarily block introducing a removed_user_from_blog action here, just something to keep in mind.

At a glance, 0001-Add-removed_user_from_blog-action-hook.patch​ seems OK to proceed.

If we decide to address the discrepancies while we're at it, some options could be:

1. Introduce added_new_user in add_new_user_to_blog(), for consistency with added_existing_user.
2. Introduce added_user_to_blog in add_user_to_blog() after the user cache has been cleaned. That would be similar to create_term/created_term actions.
3. Deprecate both add_user_to_blog and remove_user_from_blog actions and introduce new actions instead. Something like:
   • add_user_to_blog_before
   • add_user_to_blog_after
   • remove_user_from_blog_before
   • remove_user_from_blog_after

I'm leaning to options 1 and 2 at the moment.

Somewhat related: #15173

[kevinu]

Hello @SergeyBiryukov, and thank you for the welcoming and your review!

That makes a lot of sense and I think it should definitely be kept somewhat consistent between these four functions. So I have uploaded 49102.2.patch​ which includes making the suggested changes 1 & 2.

While working on it I noticed three other discrepancies I wanted to bring up and and if you feel these should be addressed I would be happy to add it into this patch as well.

1. The already present actions add_user_to_blog and remove_user_from_blog as well as my addition of removed_user_from_blog all use the terms "to_blog" or "from_blog".

Should the action added_existing_user in add_existing_user_to_blog() and added_new_user in add_new_user_to_blog() also share that same

2. All the actions current and suggested provide the arguments of $user_id and $blog_id, (some providing $role as well) except for added_existing_user. This provides the arguments $user_id and $result.

I feel as though the name added_existing_user already implies a success. Should this be changed to be surrounded in an if-statement and provide $blog_id like the others?

3. I noticed that in remove_user_from_blog() and it's action remove_user_from_blog the doc-blocks refer to $blog_id being an (int). But it actually comes through as a string.

We could either update the doc-blocks, or type cast it as (int) like we do to $user_id in that same function perhaps?
If this one needs to be tracked in a separate ticket I could do so.

[davidbaumwald]

With the added actions here, this at least deserves a small call out on the Misc Dev Note, when the time comes.

[pbiron]

Replying to kevinu:

1. The already present actions add_user_to_blog and remove_user_from_blog as well as my addition of removed_user_from_blog all use the terms "to_blog" or "from_blog".

Should the action added_existing_user in add_existing_user_to_blog() and added_new_user in add_new_user_to_blog() also share that same

I think it would be good to a new added_existing_user_to_blog action to add_existing_user_to_blog(), but not change the name (or deprecate) the current added_existing_user action.

I'm on the fence about the name of the new added_new_user action in add_new_user_to_blog(). On the one hand, using the same action name as the existing action in add_existing_user_to_blog() makes sense. But if the suggestion above about adding a new added_existing_user_to_blog action is agreed to, then the question becomes: should both actions be added to add_new_user_to_blog()?

2. All the actions current and suggested provide the arguments of $user_id and $blog_id, (some providing $role as well) except for added_existing_user. This provides the arguments $user_id and $result.

I feel as though the name added_existing_user already implies a success. Should this be changed to be surrounded in an if-statement and provide $blog_id like the others?

For backwards-compat reasons, I don't think it would be good to change that (at least not as part of this ticket...maybe that question deserves a ticket of it's own?).

3. I noticed that in remove_user_from_blog() and it's action remove_user_from_blog the doc-blocks refer to $blog_id being an (int). But it actually comes through as a string.

We could either update the doc-blocks, or type cast it as (int) like we do to $user_id in that same function perhaps?
If this one needs to be tracked in a separate ticket I could do so.

Really? It should be an int in all the places I find that function called...but I haven't done an exhaustive search. Again, that issue is probably worth another ticket.

A few other comments about 49102.2.patch:

1. it uses spaces for indenting, and should use tabs
2. the @since tags for the new actions should just be @since 5.4.0 instead of @since MU (5.4.0). Yes, the current actions have @since MU (3.0.0), but that's because 3.0.0 was also called "MU".
3. the DocBlocks of the functions the new actions have been added to should get @since 5.4.0 tags that mention the new actions.

[davidbaumwald]

This ticket still needs a decision, and with 5.4 Beta 1 landing shortly, this is being moved to Future Release. If any maintainer or committer feels this can be included in 5.4 or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.

Trac ticket: #49102