[wp-trac] [WordPress Trac] #49102: Multisite: removed_user_from_blog hook

[WordPress Trac]

#49102: Multisite: removed_user_from_blog hook
-----------------------------+-----------------------------
 Reporter:  kevinu           |       Owner:  SergeyBiryukov
     Type:  feature request  |      Status:  reviewing
 Priority:  normal           |   Milestone:  5.4
Component:  Users            |     Version:  5.3.2
 Severity:  minor            |  Resolution:
 Keywords:  needs-dev-note   |     Focuses:  multisite
-----------------------------+-----------------------------

Comment (by pbiron):

 Replying to [comment:5 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
 [https://core.trac.wordpress.org/attachment/ticket/49102/49102.2.patch
 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.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/49102#comment:9>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform