Fix: Bug Inconsistency: Standardize post meta key naming with the wpai_ prefix
#867
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| <?php | ||
| /** | ||
| * Upgrade routines for version 1.3.0. | ||
| * | ||
| * @package WordPress\AI\Admin\Upgrades | ||
| * @since x.x.x | ||
| */ | ||
|
|
||
| declare( strict_types=1 ); | ||
|
|
||
| namespace WordPress\AI\Admin\Upgrades; | ||
|
|
||
| // Exit if accessed directly. | ||
| defined( 'ABSPATH' ) || exit; | ||
|
|
||
| /** | ||
| * Upgrade routine for standardizing meta keys on the `wpai_` prefix. | ||
| * | ||
| * Renames the legacy `ai_generated` and `ai_generated_summary` post meta keys and | ||
| * the `ai_note` comment meta key to their `wpai_`-prefixed equivalents so all | ||
| * plugin-owned meta shares a consistent namespace. | ||
| * | ||
| * @since x.x.x | ||
| * @internal | ||
| */ | ||
| class V1_3_0 extends Abstract_Upgrade { | ||
|
|
||
| /** | ||
| * {@inheritDoc} | ||
| * | ||
| * @since x.x.x | ||
| */ | ||
| public static string $version = '1.3.0'; | ||
|
|
||
| /** | ||
| * {@inheritDoc} | ||
| * | ||
| * Migrates post and comment meta keys from the legacy `ai_` prefix to the | ||
| * `wpai_` prefix. | ||
| * | ||
| * @since x.x.x | ||
| */ | ||
| protected function upgrade(): void { | ||
| $this->rename_post_meta_key( 'ai_generated', 'wpai_generated' ); | ||
| $this->rename_post_meta_key( 'ai_generated_summary', 'wpai_generated_summary' ); | ||
| $this->rename_comment_meta_key( 'ai_note', 'wpai_note' ); | ||
|
Comment on lines
+44
to
+46
Contributor
There was a problem hiding this comment. Since we're using direct DB queries to update meta, may need a cache flush after this runs in order to ensure any cached values are flushed out. Likely not a problem as we're not changing data, just keys but may be better safe than sorry
Contributor
Author
There was a problem hiding this comment. Sure, Will flush the cache to ensure everything works expected.
Contributor
There was a problem hiding this comment.
Contributor
Author
There was a problem hiding this comment. I do not think,
Contributor
There was a problem hiding this comment. Yes,
Contributor
Author
There was a problem hiding this comment. Thanks, also this is a one off thing, so we can go with wp_cache_flush here. |
||
| } | ||
|
|
||
| /** | ||
| * Renames a post meta key for every row that uses it. | ||
| * | ||
| * @since x.x.x | ||
| * | ||
| * @param string $old_key The existing meta key. | ||
| * @param string $new_key The meta key to migrate to. | ||
| */ | ||
| private function rename_post_meta_key( string $old_key, string $new_key ): void { | ||
| global $wpdb; | ||
|
|
||
| $wpdb->update( // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
|
dkotter marked this conversation as resolved.
Outdated
|
||
| $wpdb->postmeta, | ||
| array( 'meta_key' => $new_key ), | ||
| array( 'meta_key' => $old_key ) | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Renames a comment meta key for every row that uses it. | ||
| * | ||
| * @since x.x.x | ||
| * | ||
| * @param string $old_key The existing meta key. | ||
| * @param string $new_key The meta key to migrate to. | ||
| */ | ||
| private function rename_comment_meta_key( string $old_key, string $new_key ): void { | ||
| global $wpdb; | ||
|
|
||
| $wpdb->update( // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching | ||
| $wpdb->commentmeta, | ||
| array( 'meta_key' => $new_key ), | ||
| array( 'meta_key' => $old_key ) | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,7 +59,7 @@ public function register(): void { | |
|
|
||
| register_meta( | ||
| 'comment', | ||
| 'wpai_note', | ||
|
Contributor
There was a problem hiding this comment. Something I hadn't thought about until now, these meta keys we're renaming are set as I would guess there's a low (potentially zero) amount of users doing this but any thoughts on what is a breaking change here? At the very least, we'll need to call this out as a breaking change in our changelog but wondering if there's anything else we should consider here
Contributor
Author
There was a problem hiding this comment.
Yes, there could be breaking change, for eg; Users using/depend on Rest API for data would break, since Another example could be user external writes request with body params as May be we have 2 pathways here,
IMO, we should go with the 1st, just document it as a breaking change in the release doc.
Contributor
There was a problem hiding this comment. Yeah, I think I'm fine with option 1 here. But probably worth us discussing @jeffpaul prior to merging this in
Contributor
Author
Contributor
There was a problem hiding this comment. I've not had a chance to discuss with Jeff but I'm still fine with option 1 here so let's move forward with that Edit: just saw Jeff left a comment on the Issue. I'm fine with the renaming
Contributor
Author
There was a problem hiding this comment. Thanks, @dkotter I think this would be added at the time of release right? So no any changes in PR. Let me know if there is any other feedback on this? |
||
| array( | ||
| 'type' => 'boolean', | ||
| 'single' => true, | ||
|
|
@@ -97,7 +97,7 @@ public function register_abilities(): void { | |
| * Overrides the author fields for AI-generated Notes before they are inserted. | ||
| * | ||
| * Fires via the rest_pre_insert_comment filter. When the REST request includes | ||
| * meta.wpai_note = true on a Note (comment_type "note") created by a user who can | ||
| * edit the target post, replaces the authenticated user's identity with a generic | ||
| * "AI" author so Notes are not attributed to a personal account. | ||
| * | ||
|
|
@@ -114,7 +114,7 @@ public function maybe_set_ai_author( $prepared_comment, \WP_REST_Request $reques | |
|
|
||
| $meta = $request->get_param( 'meta' ); | ||
|
|
||
| if ( ! is_array( $meta ) || empty( $meta['wpai_note'] ) ) { | ||
| return $prepared_comment; | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| <?php | ||
| /** | ||
| * Integration tests for V1_3_0. | ||
| * | ||
| * @package WordPress\AI\Tests\Integration\Admin\Upgrades | ||
| */ | ||
|
|
||
| namespace WordPress\AI\Tests\Integration\Admin\Upgrades; | ||
|
|
||
| use WP_UnitTestCase; | ||
| use WordPress\AI\Admin\Upgrades\V1_3_0; | ||
|
|
||
| /** | ||
| * V1_3_0 test case. | ||
| * | ||
| * @covers \WordPress\AI\Admin\Upgrades\V1_3_0 | ||
| * @since x.x.x | ||
| */ | ||
| class V1_3_0Test extends WP_UnitTestCase { | ||
|
|
||
| /** | ||
| * Tests that run() renames the ai_generated attachment meta key. | ||
| * | ||
| * @since x.x.x | ||
| */ | ||
| public function test_run_renames_generated_meta(): void { | ||
| $attachment_id = self::factory()->post->create( array( 'post_type' => 'attachment' ) ); | ||
| update_post_meta( $attachment_id, 'ai_generated', 1 ); | ||
|
|
||
| ( new V1_3_0( '1.2.0' ) )->run(); | ||
|
|
||
| // Direct SQL updates bypass the in-request meta cache. | ||
| wp_cache_flush(); | ||
|
|
||
| $this->assertSame( '1', get_post_meta( $attachment_id, 'wpai_generated', true ), 'ai_generated should migrate to wpai_generated.' ); | ||
| $this->assertSame( '', get_post_meta( $attachment_id, 'ai_generated', true ), 'Old ai_generated meta should be removed.' ); | ||
| } | ||
|
|
||
| /** | ||
| * Tests that run() renames the ai_generated_summary post meta key. | ||
| * | ||
| * @since x.x.x | ||
| */ | ||
| public function test_run_renames_summary_meta(): void { | ||
| $post_id = self::factory()->post->create(); | ||
| update_post_meta( $post_id, 'ai_generated_summary', 'A summary.' ); | ||
|
|
||
| ( new V1_3_0( '1.2.0' ) )->run(); | ||
|
|
||
| wp_cache_flush(); | ||
|
|
||
| $this->assertSame( 'A summary.', get_post_meta( $post_id, 'wpai_generated_summary', true ), 'ai_generated_summary should migrate to wpai_generated_summary.' ); | ||
| $this->assertSame( '', get_post_meta( $post_id, 'ai_generated_summary', true ), 'Old ai_generated_summary meta should be removed.' ); | ||
| } | ||
|
|
||
| /** | ||
| * Tests that run() renames the ai_note comment meta key. | ||
| * | ||
| * @since x.x.x | ||
| */ | ||
| public function test_run_renames_note_comment_meta(): void { | ||
| $comment_id = self::factory()->comment->create(); | ||
| update_comment_meta( $comment_id, 'ai_note', true ); | ||
|
|
||
| ( new V1_3_0( '1.2.0' ) )->run(); | ||
|
|
||
| wp_cache_flush(); | ||
|
|
||
| $this->assertSame( '1', get_comment_meta( $comment_id, 'wpai_note', true ), 'ai_note should migrate to wpai_note.' ); | ||
| $this->assertSame( '', get_comment_meta( $comment_id, 'ai_note', true ), 'Old ai_note comment meta should be removed.' ); | ||
| } | ||
|
|
||
| /** | ||
| * Tests that run() returns true on success. | ||
| * | ||
| * @since x.x.x | ||
| */ | ||
| public function test_run_returns_success(): void { | ||
| $this->assertTrue( ( new V1_3_0( '1.2.0' ) )->run() ); | ||
| } | ||
|
|
||
| /** | ||
| * Tests that run() skips migration when the version is already current. | ||
| * | ||
| * @since x.x.x | ||
| */ | ||
| public function test_run_skips_when_version_already_current(): void { | ||
| $post_id = self::factory()->post->create(); | ||
| update_post_meta( $post_id, 'ai_generated_summary', 'A summary.' ); | ||
|
|
||
| ( new V1_3_0( '1.3.0' ) )->run(); | ||
|
|
||
| wp_cache_flush(); | ||
|
|
||
| $this->assertSame( 'A summary.', get_post_meta( $post_id, 'ai_generated_summary', true ), 'Old meta should be untouched when the upgrade is skipped.' ); | ||
| $this->assertSame( '', get_post_meta( $post_id, 'wpai_generated_summary', true ), 'New meta should not be written when the upgrade is skipped.' ); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.