Opened 2 years ago
Last modified 5 days ago
#58763 assigned defect (bug)
Inconsistent add/get/update/delete_post_meta() functions leads to deleting post metadata.
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | major | Version: | |
Component: | General | Keywords: | has-patch has-test-info has-unit-tests early dev-feedback |
Focuses: | Cc: |
Description
The add_post_meta(), delete_post_meta(), and update_post_meta() functions all use wp_is_post_revision() to add/delete/update the metadata of the post, not its revision. That's fine, but the get_post_meta() function does NOT use wp_is_post_revision().
As a consequence, if you use get_post_meta() and update_post_meta() during the saving process for a revision, the post metadata ends up being overwritten/deleted: The get_post_meta() function gets the revision metadata, and then the update_post_meta() function updates the post metadata, NOT the revision metadata.
js.
Change History (23)
This ticket was mentioned in PR #5216 on WordPress/wordpress-develop by @monircoderex.
22 months ago
#3
- Keywords has-patch added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/58763
## Description
This PR used wp_is_post_revision()
function get_post_meta()
function.
#4
@
21 months ago
- Keywords needs-testing added
Needs testing, I wonder if changes that made for #20564 ticket, are affecting outcome for this issue.
#5
follow-up:
↓ 6
@
21 months ago
- Keywords needs-testing-info added
This ticket could use some testing info to make things easier for testers. @jsmoriss would you be able to add them?
#6
in reply to:
↑ 5
@
21 months ago
Replying to nicolefurlan:
This ticket could use some testing info to make things easier for testers. @jsmoriss would you be able to add them?
Off the top of my head, here's some code to define a post metadata key for the post (not the revision), then when a revision is restored, the post metadata is erased because get_post_meta() gets the revision metadata (which does not exist) and update_post_meta() updates the post metadata.
add_action( 'save_post', 'test_save_post', 10, 1 ); function test_save_options( $post_id ) { $post_type = get_post_type( $post_id ); if ( 'revision' !== $post_type ) { add_post_meta( $post_id, '_test_metadata', 'OK' ); // Adds POST metadata once. $val = get_post_meta( $post_id, '_test_metadata', $single = true ); // Value should always be "OK". error_log( $post_id . ' ' . $post_type . ' _test_metadata = ' . $val . ' (value should always be "OK" )' ); if ( empty( $val ) ) { error_log( $post_id . ' ' . $post_type . ' resetting test (by deleting _test_metadata)' ); delete_post_meta( $post_id, '_test_metadata' ); } } else { $val = get_post_meta( $post_id, '_test_metadata', $single = true ); // Retrieves REVISION metadata (which does not exist). update_post_meta( $post_id, '_test_metadata', $val ); // Updates the POST metadata (not the revision metadata). error_log( $post_id . ' ' . $post_type . ' _test_metadata = ' . $val . ' (post metadata value now erased)' ); } }
The error log output looks something like this:
[09-Oct-2023 19:44:46 UTC] 1086 page _test_metadata = OK (value should always be "OK" ) [09-Oct-2023 19:45:27 UTC] 4624 revision _test_metadata = (post metadata value now erased) [09-Oct-2023 19:45:27 UTC] 1086 page _test_metadata = (value should always be "OK" ) [09-Oct-2023 19:45:27 UTC] 1086 page resetting test (by deleting _test_metadata)
js.
#7
@
21 months ago
- Keywords has-testing-info added; needs-testing-info removed
Wonderful. Thank you @jsmoriss!
This ticket was mentioned in Slack in #core-test by nicolefurlan. View the logs.
21 months ago
This ticket was mentioned in PR #5476 on WordPress/wordpress-develop by @felipeelia.
21 months ago
#9
- Keywords has-unit-tests added
This just adds unit tests on top of https://github.com/WordPress/wordpress-develop/pull/5216
Trac ticket: https://core.trac.wordpress.org/ticket/58763
This ticket was mentioned in Slack in #core by oglekler. View the logs.
20 months ago
#11
@
20 months ago
- Milestone changed from 6.4 to 6.5
This ticket was discussed in today's bug scrub.
Per @hellofromTonya:
Because of the change is touching older code, a deep dive into the code's commit history is needed. Why? To understand if it's by design, is it the root cause, when did the bug first start.
It would be great to get this into 6.5 Beta 1 to make sure there are no unknowns or side-effects.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
16 months ago
#13
follow-up:
↓ 17
@
16 months ago
- Keywords early added
- Milestone changed from 6.5 to 6.6
The aim was to commit it before beta 1. Looks like we let it slip to beta 2…
By the way, as unit tests are failing, it's probably better to handle this ticket early
in the 6.6 cycle.
This ticket was mentioned in Slack in #core by nhrrob. View the logs.
13 months ago
#15
@
13 months ago
- Milestone changed from 6.6 to Future Release
With a deeper dive needed and this ticket not really gaining any momentum the last two cycles, moving this to Future Release
.
Once a path forward is determined, the milestone can be updated to a specific cycle.
#17
in reply to:
↑ 13
@
6 weeks ago
- Keywords dev-feedback added; needs-testing removed
Test Report
Description
✅ This report validates that the indicated patch works as expected.
Patch tested: https://github.com/WordPress/wordpress-develop/pull/5476.diff
Environment
- WordPress: 6.9-alpha-60093-src
- PHP: 8.2.28
- Server: nginx/1.27.5
- Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
- Browser: Chrome 136.0.0.0
- OS: Windows 10/11
- Theme: Twenty Twenty-Five 1.2
- MU Plugins: None activated
- Plugins:
- Test Reports 1.2.0
Bug Reproduction
- We can use the code here (note that there is a slight mistake in the function name callback of the
add_action
) - Create a post
- Add a revision
- 🐞 Check PHP logs to observe that the result, unexpectedly is empty
I've also created another version of the code to test this, but its pretty much the same idea
Actual Results
- ✅ Issue resolved with patch.
Additional Notes
Replying to audrasjb:
The aim was to commit it before beta 1. Looks like we let it slip to beta 2…
By the way, as unit tests are failing, it's probably better to handle this ticket
early
in the 6.6 cycle.
I've been running full testing, and I can't really see any errors
I don't entirely understand why this patch has not been integrated already. Both patch, unit tests and the overall testing framework seem legit, and the problem is there. I think that this is ready to be shipped ✅
Supplemental Artifacts
My version of the code
add_action('save_post', 'demo_post_meta_revision_issue', 10, 3);
function demo_post_meta_revision_issue($post_id, $post) {
$meta_key = '_demo_test_metadata';
error_log("--------------------------------");
if ('revision' === $post->post_type) {
$parent_id = wp_get_post_parent_id($post_id);
$log_message = " - Revision ID: $post_id (Parent Post ID: $parent_id)\n";
error_log($log_message);
$meta_value = get_post_meta($post_id, $meta_key, true);
$log_message = " - Retrieved meta value for revision: " . (empty($meta_value) ? '❌ Empty' : '✅ ' . $meta_value) . "\n";
error_log($log_message);
update_post_meta($post_id, $meta_key, $meta_value);
$updated_value = get_post_meta($parent_id, $meta_key, true);
$log_message = " - Updated meta value on parent post: " . (empty($updated_value) ? '❌ Empty (Metadata erased)' : '✅ ' . $updated_value) . "\n";
error_log($log_message);
} else {
if (empty(get_post_meta($post_id, $meta_key, true))) {
add_post_meta($post_id, $meta_key, 'Test Value');
$log_message = " - Post ID: $post_id - Added meta value: Test Value\n";
error_log($log_message);
} else {
$meta_value = get_post_meta($post_id, $meta_key, true);
$log_message = " - Post ID: $post_id - Existing meta value: $meta_value\n";
error_log($log_message);
}
}
}
#18
@
2 weeks ago
Test Report
Description
✅ This report validates that the indicated patch works as expected.
Patch tested: Patch-5476.diff
Environment
- WordPress: 6.9-alpha-60093-src
- PHP: 8.2.28
- Server: nginx/1.27.5
- Database: mysqli (Server: 8.4.5 / Client: mysqlnd 8.2.28)
- Browser: Firefox 139.0
- OS: Windows 10/11
- Theme: Twenty Twenty-Five 1.2
- MU Plugins: None activated
- Plugins:
- Post Meta Logger
- Test Reports 1.2.0
Actual Results
- I used code mentioned here by @jsmoriss .
- Before applying the patch, the PHP log results are:
[07-Jun-2025 11:52:05 UTC] 16 post _test_metadata = OK (value should always be "OK" ) [07-Jun-2025 11:52:05 UTC] 17 revision _test_metadata = (post metadata value now erased) [07-Jun-2025 11:52:41 UTC] 16 post _test_metadata = (value should always be "OK" ) [07-Jun-2025 11:52:41 UTC] 16 post resetting test (by deleting _test_metadata) [07-Jun-2025 11:52:41 UTC] 18 revision _test_metadata = (post metadata value now erased)
- After applying the patch, the ✅ Issue resolved with patch and the PHP log results are:
[07-Jun-2025 12:06:07 UTC] 10 post _test_metadata = OK (value should always be "OK" ) [07-Jun-2025 12:06:53 UTC] 10 post _test_metadata = OK (value should always be "OK" ) [07-Jun-2025 12:06:53 UTC] 11 revision _test_metadata = OK (post metadata value now erased) [07-Jun-2025 12:07:19 UTC] 10 post _test_metadata = OK (value should always be "OK" ) [07-Jun-2025 12:07:20 UTC] 12 revision _test_metadata = OK (post metadata value now erased)
This ticket was mentioned in Slack in #core by sirlouen. View the logs.
13 days ago
@adamsilverstein commented on PR #5476:
7 days ago
#21
We should check for existing uses (existing plugins) that might be expecting to be able to retrieve meta revisions here (in the case of revisioned meta).
@SirLouen commented on PR #5476:
5 days ago
#22
We should check for existing uses (existing plugins) that might be expecting to be able to retrieve meta revisions here (in the case of revisioned meta).
I'm not sure which is your concern here. As you can see here, all plugins simply expect meta from the post, none from revision (prob because it has never been reliable)
I should have added another PR because this is now two years old, I'm starting to doubt if the original submitter is still around. I was expecting that the patch is so trivial that it could be shipped fast. There are multiple versions of code like this and this that prove that this has been missing for ages (and pretty much all the other CRUD post_meta functions already had this)
PS: When you comment here, I don't receive a notification to follow up you (because this is not my PR).
Better comment all concerns directly in Trac.
#23
@
5 days ago
@adamsilverstein I have commented on the PR but if possible, let's continue the conversation here.
This needs patch, and I am also wondering if this ticket needs to be handled early.