Opened 22 months ago
Closed 22 months ago
#18195 closed defect (bug) (fixed)
Introduce *_metadata_by_mid()
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 3.3 |
| Component: | General | Version: | 3.2.1 |
| Severity: | normal | Keywords: | |
| Cc: | kawauso, kovshenin, sirzooro |
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)
Change History (24)
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?
- 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.
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
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!
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!
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().
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
ryan — 22 months ago
As discussed in IRC, let's make new API not require slashed arguments. Thus we can remove the stripslashes*() calls from the patch.
comment:11
kovshenin — 22 months ago
The new patch implements both functions, the test has been written here in the fourth patch for both functions: #ut21
comment:12
ryan — 22 months 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().
comment:13
ryan — 22 months ago
In [18494]:
comment:14
sirzooro — 22 months ago
- Cc sirzooro added
comment:15
ryan — 22 months ago
In [18501]:
comment:16
ryan — 22 months ago
In [18502]:
comment:17
ryan — 22 months ago
- Resolution set to fixed
- Status changed from new to closed

A patch for update_metadata_by_mid() function