WordPress.org

Make WordPress Core

Opened 7 weeks ago

Last modified 5 days ago

#49087 reviewing defect (bug)

impossible to set show_admin_bar_front to false in wp_insert_user

Reported by: bilgilabs Owned by: SergeyBiryukov
Milestone: 5.4 Priority: normal
Severity: normal Version: 5.3.2
Component: Users Keywords: has-patch
Focuses: docs Cc:
PR Number:

Description

in wp-includes/user.php line 1752

        $meta['show_admin_bar_front'] = empty( $userdata['show_admin_bar_front'] ) ? 'true' : $userdata['show_admin_bar_front'];

with this expression it seems to me that setting show_admin_bar_front to false is impossible. it should be

        $meta['show_admin_bar_front'] = !isset( $userdata['show_admin_bar_front'] ) ? 'true' : $userdata['show_admin_bar_front'];

Attachments (3)

49087-wp_insert_user-show_admin_bar_front.diff (1.9 KB) - added by valentinbora 7 weeks ago.
49087.diff (1.0 KB) - added by audrasjb 4 weeks ago.
Docs: improve params documentation for wp_insert_user()
49087.2.diff (1.0 KB) - added by garrett-eclipse 4 weeks ago.
Updated patch to be clear 'true'/'false' are string literal and not boolean.

Download all attachments as: .zip

Change History (12)

#1 @SergeyBiryukov
7 weeks ago

  • Component changed from General to Users

#2 @SergeyBiryukov
7 weeks ago

  • Focuses docs added
  • Milestone changed from Awaiting Review to 5.4
  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

Hi there, welcome to WordPress Trac! Thanks for the report.

As seen in edit_user(), the value accepts a string 'true' or 'false', not a boolean true or false, so the code is correct as is.

It could probably be explained better in the documentation, though.

#3 @valentinbora
7 weeks ago

@SergeyBiryukov https://developer.wordpress.org/reference/functions/wp_insert_user/ mentions (string|bool) in the param docs, so the OP's ticket makes sense to a certain extent.

Should we consider updating the code per the patch or updating the docs?

@audrasjb
4 weeks ago

Docs: improve params documentation for wp_insert_user()

#4 follow-up: @audrasjb
4 weeks ago

  • Keywords has-patch added

Hi,

There may be some backward compatibility issues if we decide to transform it into a real boolean. I propose to update the documentation to make it clear this parameter accepts only 'false' or 'true'.

In 49087.diff: Docs: improve show_admin_bar_front parameter documentation for wp_insert_user()

#5 in reply to: ↑ 4 @bilgilabs
4 weeks ago

hello

it's probably a good idea to only make the document clear. the line should say something like

  • @type string $show_admin_bar_front Whether to display the Admin Bar for the user on the site's front end. Default 'true' (string literal not boolean or otherwise), or empty, or no parameter enables it, only 'false' (string literal not boolean or otherwise) disables it.

Replying to audrasjb:

Hi,

There may be some backward compatibility issues if we decide to transform it into a real boolean. I propose to update the documentation to make it clear this parameter accepts only 'false' or 'true'.

In 49087.diff: Docs: improve show_admin_bar_front parameter documentation for wp_insert_user()

@garrett-eclipse
4 weeks ago

Updated patch to be clear 'true'/'false' are string literal and not boolean.

#6 @garrett-eclipse
4 weeks ago

I've refreshed the patch by @audrasjb (thank you) in 49087.2.diff to hopefully clarify that the values should be 'true'/'false' as string literal and not boolean. I hope that clarifies for you as well @bilgilabs.

If that works I think it's ready to move forward.

#7 @audrasjb
4 weeks ago

Right. @garrett-eclipse ’s proposal looks great to me 👍

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


4 weeks ago

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


5 days ago

Note: See TracTickets for help on using tickets.