WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 13 days ago

#49102 reviewing feature request

Multisite: removed_user_from_blog hook

Reported by: kevinu Owned by: SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: minor Version: 5.3.2
Component: Users Keywords:
Focuses: multisite Cc:
PR Number:

Description

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.

Attachments (2)

0001-Add-removed_user_from_blog-action-hook.patch (1.1 KB) - added by kevinu 3 weeks ago.
Patch file for changes to add this hook
49102.2.patch (2.6 KB) - added by kevinu 3 weeks ago.

Download all attachments as: .zip

Change History (8)

#1 @SergeyBiryukov
3 weeks ago

  • Component changed from General to Users

@kevinu
3 weeks ago

Patch file for changes to add this hook

#2 @kevinu
3 weeks ago

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.

#3 @SergeyBiryukov
3 weeks ago

  • Milestone changed from Awaiting Review to 5.4

#4 follow-up: @SergeyBiryukov
3 weeks ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

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:

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
3 weeks ago

#5 in reply to: ↑ 4 @kevinu
3 weeks ago

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

  1. 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?

  1. 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.

This ticket was mentioned in Slack in #core by kevinu. View the logs.


13 days ago

Note: See TracTickets for help on using tickets.