WordPress.org

Make WordPress Core

Opened 6 months ago

Closed 6 months ago

Last modified 4 months ago

#25745 closed defect (bug) (fixed)

Update user with wp_insert_user reset first and last name

Reported by: sudoku1 Owned by: SergeyBiryukov
Milestone: 3.8 Priority: normal
Severity: normal Version: 2.5
Component: Inline Docs Keywords: has-patch dev-feedback
Focuses: Cc:

Description

As mentioned in the title if you trying to make an update of a user, passing the ID, for instance to change the password it's result in resetting the first and the last name due respectively to line 1334 - 1338 of user.php.

Or the documentation is wrong...

Attachments (1)

25745.patch (1.1 KB) - added by SergeyBiryukov 6 months ago.

Download all attachments as: .zip

Change History (19)

comment:1 mauryaratan6 months ago

  • Keywords close added

As explained in the documentation you should be using wp_update_user instead.

Note: As late as 2.7.1, if you are attempting to update the password for the user, this function will not hash the password, thus failing. Use the wp_update_user function instead.

comment:2 sudoku16 months ago

Cool.
But as I explained in the ticket the problem live in the wp_insert_user function.
Doesn't matter if you try to update the user_pass or first_name or even the user_email field.

(Also the method will not fail if you trying to submit a user_pass its just saved unencrypted).

Please have a look at the function and test it before saying I'm wrong.

Cheers

Last edited 6 months ago by sudoku1 (previous) (diff)

comment:3 sudoku16 months ago

  • Keywords close removed

comment:4 sudoku16 months ago

  • Keywords needs-testing dev-feedback added

comment:5 DrewAPicture6 months ago

  • Keywords reporter-feedback added; needs-testing removed

Can you provide some sample code for what you're trying to do?

In my testing, user_login needs to be specified -- which appears to be another issue entirely -- otherwise it produces a notice, but I don't see any data being reset in the process.

comment:6 sudoku16 months ago

Indeed! Also the mandatory presence of the user_login could be considered as an issue.
BTW:

<?php

$user_id = wp_insert_user(array(
    'user_login' => 'mario',
    'user_email' => 'mario@sudoku.com',
    'first_name' => 'Mario',
    'last_name'  => 'Bianchi',
    ));

echo get_user_meta($user_id, 'first_name', true) . PHP_EOL;
echo get_user_meta($user_id, 'last_name', true);

$user_id = wp_insert_user(array(
    'ID'         => (int) $user_id,
    'user_login' => 'mario', // user_login mandatory???
    'user_email' => 'sudoku@mario.com' // !! Email chaged !!
    ));

if ( ! is_wp_error($user_id) ) {

    $first_name = get_user_meta($user_id, 'first_name', true);
    $last_name  = get_user_meta($user_id, 'last_name', true);

    if ( empty($first_name) OR empty($last_name) ) {
        echo 'Huston, we have a problem!';
    }

}

Tested also with 3.7 (Actually the wp_insert_user function is the same)

Last edited 6 months ago by sudoku1 (previous) (diff)

comment:7 DrewAPicture6 months ago

In my reading of the source for wp_insert_user() it definitely seems like we either need to update the docs to reflect that user_login is required for updating existing users or fix it so only the ID is required.

comment:8 follow-up: sudoku16 months ago

Yes but it's another issue.
You get the point I'm trying to explain with the code sample? Its clear enough?
Cheers

comment:9 in reply to: ↑ 8 DrewAPicture6 months ago

  • Keywords reporter-feedback removed
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Severity changed from major to normal
  • Status changed from new to closed

Replying to sudoku1:

Yes but it's another issue.
You get the point I'm trying to explain with the code sample? Its clear enough?
Cheers

The first and last names getting reset is intended behavior for wp_insert_user() if don't pass them in.

The issue is that you should really be using wp_update_user() here instead. Otherwise, you need to make sure to pass all userdata fields when you're attempting to update a user. wp_update_user() does this for you.

comment:10 sudoku16 months ago

Why is an intended behavior? Which are the benefits in doing that?
In this case is a good thing updating the documentation cuz no UPDATE procedures known in CS will reset the others fields if not given.

And please stop to give me alternative solutions. I know how to do. I followed the Wordpress development until the beginning. I'm just here to report an obvious bug and a bit more of consideration is welcome.

Last edited 6 months ago by sudoku1 (previous) (diff)

comment:11 sudoku16 months ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

comment:12 sudoku16 months ago

  • Keywords needs-docs added

SergeyBiryukov6 months ago

comment:13 SergeyBiryukov6 months ago

  • Component changed from General to Inline Docs
  • Keywords has-patch added; needs-docs removed
  • Milestone set to 3.8
  • Version changed from 3.6.1 to 2.5

The current docs (introduced in [6564]) might create an impression that wp_insert_user() and wp_update_user() are somewhat interchangeable. Related: #16731.

We should make it clear that wp_insert_user() should only be used for inserting a new user, and wp_update_user() should be used for updating an existing user.

25745.patch removes some comments along the lines of [24345].

comment:14 follow-up: sudoku16 months ago

Also the codex documentation needs operations. Absolutely.

The second and the third paragraph says:

Can update a current user or insert a new user based on whether the user's ID is present.

Can be used to update the user's info (see below), set the user's role, and set the user's preference on the use of the rich editor.

That is not true.

Last edited 6 months ago by sudoku1 (previous) (diff)

comment:15 in reply to: ↑ 14 SergeyBiryukov6 months ago

  • Keywords needs-codex added

Replying to sudoku1:

Can update a current user or insert a new user based on whether the user's ID is present.

I've removed that part from Codex. The example there should be updated as well.

comment:16 SergeyBiryukov6 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from reopened to closed

In 25978:

Remove docs suggesting that wp_insert_user() can be used to update an existing user. wp_update_user() should be used instead. fixes #25745.

comment:17 sudoku16 months ago

Thank you guys!

Note: See TracTickets for help on using tickets.