Make WordPress Core

Opened 11 years ago

Closed 9 years ago

#24255 closed enhancement (fixed)

WP_User::__set doesn't persist custom fields as it claims

Reported by: quickshiftin's profile quickshiftin Owned by: chriscct7's profile chriscct7
Milestone: 4.4 Priority: normal
Severity: normal Version: 3.3
Component: Users Keywords: has-patch needs-refresh
Focuses: Cc:


Looking through WP_User in version 3.5.1 (verified against svn trunk too), the magic methods __get and __set supposedly read and write from custom fields, per their comment blocks.

	 * Magic method for accessing custom fields
	 * @since 3.3.0
	function __get( $key ) {
        // ...

	 * Magic method for setting custom fields
	 * @since 3.3.0
	function __set( $key, $value ) {
        // ...

And sure enough, if you look through the method bodies __get will make a call to get_user_meta

$value = get_user_meta( $this->ID, $key, true );

However reading through the body of __set , it never calls add_user_meta or update_user_meta , but has the misleading comment that it's "for setting custom fields...".

Seems to me like it's missing a critical line:

update_user_meta($this->ID, $key, $value);

Am I missing something, or is this a bug?

Attachments (2)

capabilities.diff (374 bytes) - added by quickshiftin 11 years ago.
24255.patch (455 bytes) - added by johnpbloch 11 years ago.
Update doc block to clarify functionality of WP_User::set()

Download all attachments as: .zip

Change History (10)

11 years ago


#1 @toscho
11 years ago

  • Cc info@… added

#2 @scribu
11 years ago

I think the bug here is that the description for __set() is vague. It should explicitly state that it does _not_ save custom fields to the DB.

It would be interesting to add a save() method, which would go through all the fields and save the changed ones.

Last edited 11 years ago by scribu (previous) (diff)

#3 @quickshiftin
11 years ago

Hmm, good to know.

It does feel a bit lop-sided reading from custom fields, but not storing to them. To me the simple update_user_meta feels appropriate, but a batch write would be nice. It would have to read any fields that were set but not yet read, but perhaps the read could also be batched.

Another neat idea for it would be to call it in the destructor so client code could just assume the write will be done automatically.

Thanks for looking at it.

#4 @SergeyBiryukov
11 years ago

  • Version changed from 3.5.1 to 3.3

11 years ago

Update doc block to clarify functionality of WP_User::set()

#5 @johnpbloch
11 years ago

  • Cc johnpbloch@… added
  • Keywords has-patch added

Added a patch to clarify in the documentation that __set doesn't actually save the data to the database. It would be great to look into a save method that actually applies those changes, but I think that would definitely have to be a new ticket.

#6 @chriscct7
9 years ago

  • Keywords needs-refresh added

#7 @chriscct7
9 years ago

  • Milestone changed from Awaiting Review to 4.4
  • Owner set to chriscct7
  • Status changed from new to assigned
  • Type changed from defect (bug) to enhancement

#8 @wonderboymusic
9 years ago

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

In 34379:

Users: clarify the docs for WP_User::__set to make it clear that it does save data in the database.

Props johnpbloch.
Fixes #24255.

Note: See TracTickets for help on using tickets.