WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 2 months ago

#21864 closed defect (bug) (fixed)

PHPDoc incorrect with the return value of add_*_meta functions

Reported by: mark8barnes Owned by: SergeyBiryukov
Milestone: 3.9 Priority: normal
Severity: normal Version: 3.4.2
Component: Options, Meta APIs Keywords: has-patch commit
Focuses: docs Cc:

Description

The PHPDocs for add_comment_meta, add_post_meta and add_user_meta all say that they return true on success. In fact, each calls add_metadata, which returns the metadata_id on success.

In addition, update_metadata currently returns the metadata_id on success if the metadata did not already exist, but true on success if the metadata did already exist (the PHPDoc says simply that it is true on success).

The patch corrects the documentation, and additionally ensures consistency with the return value for update_metadata.

Attachments (2)

return_meta_id.patch (2.4 KB) - added by mark8barnes 20 months ago.
21864.patch (3.3 KB) - added by SergeyBiryukov 2 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 mark8barnes20 months ago

  • Cc mark@… added

comment:2 follow-up: nacin20 months ago

The documentation changes definitely make sense. The return value for add_metadata() came in [18445] specifically so add_meta() could return the meta ID. Not sure if there's any need though for update_metadata() to bother to return the meta ID. Even more so, not sure if there's any need for the add_*_meta() functions to return the meta ID, though that's probably been out in the wild long enough to be considered functionality.

comment:3 in reply to: ↑ 2 mark8barnes20 months ago

Replying to nacin:

Not sure if there's any need though for update_metadata() to bother to return the meta ID.

I can't think of a use case, and the existing system does have the advantage of different returns depending on whether the metadata was updated or added. But if you want to retain that, the documentation needs changing. Yet for consistency, I think returning the id would be better.

Even more so, not sure if there's any need for the add_*_meta() functions to return the meta ID, though that's probably been out in the wild long enough to be considered functionality.

Metadata IDs are quite hard to get out of WordPress (there's no function to get the metadata ID from a given key and value, for example). But if you want to be 100% sure you're accessing or deleting the right metadata, then you need them. If I had my way, I'd be adding functions that gave access to metadata ids, not removing them! (In my case I needed the meta_id after adding metadata so that I could include it in a AJAX return, and the metadata could then be further manipulated by the user. Without the meta_id they could only add metadata via AJAX, they couldn't add then edit it via AJAX.)

comment:4 DrewAPicture7 months ago

  • Keywords dev-feedback added

@nacin: Is there a decision one way or the other on changing the default return in update_metadata() from true to $meta_id?

comment:5 nacin3 months ago

  • Component changed from Inline Docs to Options and Meta
  • Focuses docs added

comment:6 ircbot3 months ago

This ticket was mentioned in IRC in #wordpress-dev by tw2113. View the logs.

comment:7 DrewAPicture2 months ago

  • Milestone changed from Awaiting Review to 3.6
  • Resolution set to fixed
  • Status changed from new to closed

Looks like the @returns were changed/fixed in [24490], excluding the suggested return change for update_metadata().

Feel free to open a separate ticket proposing the return change on update_metadata(), and that's more than just documentation.

comment:8 SergeyBiryukov2 months ago

  • Keywords commit added; needs-testing dev-feedback removed
  • Milestone changed from 3.6 to 3.9
  • Resolution fixed deleted
  • Status changed from closed to reopened

21864.patch is a follow-up to [24490]. Reflects that update_*_meta() functions can return a meta ID.

SergeyBiryukov2 months ago

comment:9 SergeyBiryukov2 months ago

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

In 27191:

Correct return values for update_metadata() and related functions.

fixes #21864.

Note: See TracTickets for help on using tickets.