Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#41697 closed defect (bug) (fixed)

Unused parameter in function is_user_option_local

Reported by: bnap00's profile bnap00 Owned by: spacedmonkey's profile spacedmonkey
Milestone: 4.9 Priority: normal
Severity: normal Version: 3.0
Component: Networks and Sites Keywords: needs-patch
Focuses: multisite Cc:

Description

function is_user_option_local has a parameter for accepting user ID, but is never used in the function.

Attachments (3)

41697.patch (658 bytes) - added by bnap00 6 years ago.
41697-2.patch (578 bytes) - added by bnap00 6 years ago.
41697.diff (1.9 KB) - added by jeremyfelt 6 years ago.

Download all attachments as: .zip

Change History (17)

@bnap00
6 years ago

#1 @bnap00
6 years ago

Also the function is not being used in the entire WordPress code base

#2 @spacedmonkey
6 years ago

  • Keywords has-patch added
  • Version changed from trunk to 3.0

We can't make this change as it would break backwards compatibility. If anyone is using the function is their plugin / theme, they maybe passing a null value there and using blog id.

I would recommend just fixing it, as it is really easy to fix.
There is a question around if we should just deprecate this function, but I would open another ticket to do that.

#3 @bnap00
6 years ago

@spacedmonkey I did not get your comment properly,
should we fix it or not?
if yes, what should be flow?

Last edited 6 years ago by bnap00 (previous) (diff)

#4 @flixos90
6 years ago

I think instead of removing the parameter we should rather apply it correctly: If $user_id is passed, call get_user_by( 'id', $user_id ), otherwise stick with wp_get_current_user().

#5 @spacedmonkey
6 years ago

Fix it. Something like this should work.

function is_user_option_local( $key, $user_id = 0, $blog_id = 0 ) {
    global $wpdb;
    if ( $user_id && is_numeric( $user_id ) ) {
         $current_user = get_user_by( 'id', $user_id );
    } else{   
        $current_user = wp_get_current_user();
    }

@bnap00
6 years ago

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


6 years ago

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


6 years ago

This ticket was mentioned in Slack in #core-multisite by desrosj. View the logs.


6 years ago

#10 @desrosj
6 years ago

FWIW, this function is only called once in the entire .org plugin repository in the removed plugin, presstest.

#11 @spacedmonkey
6 years ago

  • Keywords dev-feedback ms-roadmap added
  • Owner set to spacedmonkey
  • Status changed from new to reviewing

41697-2.patch

Fixes the issue to me. I think it should be merged as a fix.

This function isn't used for anything, it should really be deprecated. @bnap00 do you want another ticket for deprecating this function.

#12 @bnap00
6 years ago

@spacedmonkey That would be great

#13 @jeremyfelt
6 years ago

  • Keywords needs-patch added; has-patch dev-feedback ms-roadmap removed
  • Milestone changed from Awaiting Review to 4.9

We should just deprecate is_user_option_local() on this ticket without making any other changes. It was added in MU (see timeline below) and used for a handful of changesets before it the code using it was removed again. It never actually did anything with $user_id and always deferred to the current user.

@jeremyfelt
6 years ago

#14 @jeremyfelt
6 years ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 41668:

Multisite: Deprecate is_user_option_local().

is_user_option_local() was added during MU development and used for a handful of changesets before the code using it was removed again. It has not been used by MU or core since nor is it widely used elsewhere.

Fixes #41697.
Props bnap00, jeremyfelt.

Note: See TracTickets for help on using tickets.