Action 'switch_blog' duplicated $new_blog param

[ChriCo]

The action 'switch_blog' which can be found in wp-includes/ms-blogs.php L1660 has two times $new_blog injected, while the Doc Block says:

<?php
* @param int $new_blog New blog ID.
* @param int $new_blog Blog ID.

This is maybe - in this context - correct, since in L1651 we're checking for if ( $new_blog == $blog_id ) before doing the action.

But regarding correctness, it should be $new_blog and $prev_blog_id (as declared after the if-statement) as param and in docBlocks (for generation of documentation).

My patch...

1. renames $blog_id into $prev_blog_id.
2. updates the docBlock for the action 'switch_blog'.
3. update third param in 'switch_blog' to $prev_blog_id.
4. removes the not required $prev_blog_id = $blog_id.

[jeremyfelt]

Thanks, @ChriCo, some consistency in docs and parameters makes sense here. I'm wondering if we can do this with a smaller change.

The $prev_blog_id parameter was populated with $GLOBALS['blog_id'] until [38457]. It now serves less of a purpose because there is no worry of the global being overwritten on the next line.

I think we can remove $prev_blog_id entirely and change both action calls to do_action( 'switch_blog', $new_blog, $blog_id );

[ChriCo]

Replying to jeremyfelt:

I think we can remove $prev_blog_id entirely and change both action calls to do_action( 'switch_blog', $new_blog, $blog_id );

Thanks for your feedback. My intention was to name it in way everyone can understand it and also in terms of automatic generation of documentation out of docblocks. The var name $blog_id is not very exact here. Is it the current, new or previous one?

[ChriCo]

Howdy,

any news here? The patch is working so far, so it needs only an "approved" or "not approved".

[ChriCo]

@jeremyfelt any news here? This is already over a half year ago and just a small "fix" in terms of rename internal used variables and update of a DocBlock of the action switch_blog. Any blocker here?

[SergeyBiryukov]

Hi there! Thanks for the ticket and the patch, sorry it took so long for someone to get back to you.

The proposed changes make sense to me, both the documentation fix and the clarified naming.

Let's do the same in restore_current_blog() for consistency.

[SergeyBiryukov]

In 45794:

Networks and Sites: Improve documentation and variable naming in switch_to_blog() and restore_current_blog().

In switch_to_blog():

• Rename $blog_id to $prev_blog_id for clarity.
• Rename $new_blog to $new_blog_id for consistency.
• Pass $prev_blog_id as a second parameter to switch_blog action, instead of the duplicated $new_blog_id. This only clarifies documentation and does not affect functionality, since the values are equal in the context where the DocBlock is located.

In restore_current_blog():

• Rename $blog to $new_blog_id for clarity.
• Rename $blog_id to $prev_blog_id for clarity.

Props ChriCo, jeremyfelt, SergeyBiryukov.
Fixes #45594.