WordPress.org

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#18195 closed defect (bug) (fixed)

Introduce *_metadata_by_mid()

Reported by: ryan Owned by:
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.2.1
Component: General Keywords:
Focuses: Cc:

Description

get_metadata_by_mid() was recently introduced. Let's do the same for update and delete. These can then be used instead of update_meta() and delete_meta() in places like XML-RPC.

Attachments (7)

update_metadata_by_mid.patch (1.6 KB) - added by kovshenin 3 years ago.
A patch for update_metadata_by_mid() function
update_metadata_by_mid2.patch (1.4 KB) - added by kovshenin 3 years ago.
Another version of the update_metadata_by_mid() function
update_metadata_by_mid3.2.patch (2.2 KB) - added by kovshenin 3 years ago.
Updating to serialize meta value if needed.
update_metadata_by_mid3.3.patch (2.2 KB) - added by kovshenin 3 years ago.
Runs an is_string check against the meta key
update_metadata_by_mid3.patch (2.7 KB) - added by kovshenin 3 years ago.
Using $wpdb->update(), clearing caches
update_delete_metadata_by_mid.patch (3.9 KB) - added by kovshenin 3 years ago.
Removing expected slashes and implementing the delete function
update_delete_metadata_by_mid.2.patch (4.3 KB) - added by kovshenin 3 years ago.
Adding some actions before and after update/delete by mid

Download all attachments as: .zip

Change History (24)

comment:1 kawauso3 years ago

  • Cc kawauso added

kovshenin3 years ago

A patch for update_metadata_by_mid() function

comment:2 kovshenin3 years ago

Hey all, I attached the update_metadata_by_mid function patch, ran a few tests on post, user and comment, seems to work as expected although I'm not sure I've done the right thing by using the update_metadata after fetching the meta data by id, and maybe I should have utilized the get_metadata_by_mid function instead of the SQL query. What do you think?

kovshenin3 years ago

Another version of the update_metadata_by_mid() function

comment:3 kovshenin3 years ago

  • Cc kovshenin added

The second version is slightly easier by utilizing the get_metadata_by_mid function instead of fetching the meta via SQL. Works well for posts, comments and users.

comment:4 ryan3 years ago

mid2.patch looks good. In order to replace existing uses of update_meta(), we may need to add an optional $meta_key argument to the end. If present, the key is updated in addition to the value.

Have a go at delete_metadata_by_mid() as well, if you're interested.

Once we have both update and delete, update_meta() and delete_meta() can be updated to call the new by_mid functions.

We also need to add test cases here: http://unit-tests.trac.wordpress.org/browser/wp-testcase/test_includes_meta.php

And possibly add delete_meta() and update_meta() tests here: http://unit-tests.trac.wordpress.org/browser/wp-testcase/test_includes_post.php

comment:5 kovshenin3 years ago

@ryan, gotcha, will do!

comment:6 kovshenin3 years ago

Here's another go at update_metadata_by_mid, since the update_metadata does not implement meta key updates I used an SQL statement instead that can update both the meta value and the key if present in the arguments list as suggested by @ryan. Figuring out the test cases for this now... Let me know your thoughts. Thanks!

kovshenin3 years ago

Updating to serialize meta value if needed.

kovshenin3 years ago

Runs an is_string check against the meta key

comment:7 kovshenin3 years ago

Alright, sorry for flooding here, just uploaded another tiny patch for the function. I've also written a short test case right here http://unit-tests.trac.wordpress.org/ticket/21 Let me know if that's good enough and I'll continue on to the delete function. Thanks!

comment:8 ryan3 years ago

Looking good. Some suggestions: Use $wpdb->update() instead of $wpdb->query( $wpdb->prepare() ). Clear the cache after update. See the wp_cache_delete()/clean_user_cache() code in update_metadata().

kovshenin3 years ago

Using $wpdb->update(), clearing caches

comment:9 kovshenin3 years ago

Ryan, thanks for your suggestions. Implemented both in the latest attachment. Sorry for the mess with the filenames, still getting used to it, forgot to tick the "replace previous" file in the previous two times. The test case has been modified to test for caching too. Let me know your thoughts, thanks!

comment:10 ryan3 years ago

As discussed in IRC, let's make new API not require slashed arguments. Thus we can remove the stripslashes*() calls from the patch.

kovshenin3 years ago

Removing expected slashes and implementing the delete function

comment:11 kovshenin3 years ago

The new patch implements both functions, the test has been written here in the fourth patch for both functions: #ut21

comment:12 ryan3 years ago

Looking good. These should also do the update[d]_*_meta and the delete[d]_*_meta actions, just like in update_metadata() and delete_metadata().

kovshenin3 years ago

Adding some actions before and after update/delete by mid

comment:13 ryan3 years ago

In [18494]:

update_metadata_by_mid() and delete_metadata_by_mid(). Props kovshenin. see #18195

comment:14 sirzooro3 years ago

  • Cc sirzooro added

comment:15 ryan3 years ago

In [18501]:

Use *_metadata_by_mid() API in set_custom_fields(). Handle slashing when checking caps for key. see #18195

comment:16 ryan3 years ago

In [18502]:

Use *_metadata_by_mid() API when updating post meta in admin ajax. Fix slashing. see #18195

comment:17 ryan3 years ago

  • Resolution set to fixed
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.