From 0e2645c9cd6b48d07dc7a8ed05bef104cd9b43a7 Mon Sep 17 00:00:00 2001 From: TallblokeUK Date: Thu, 29 Oct 2026 16:06:03 +0000 Subject: [PATCH 1/3] fix: upgrade a snippets table that missed an upgrade when a save fails on it, and report failed edits --- src/php/Core/DB.php | 25 +++++- src/php/snippet-ops.php | 16 +++- tests/unit/Core/DB_Test.php | 158 ++++++++++++++++++++++++++++++++++++ 3 files changed, 195 insertions(+), 4 deletions(-) create mode 100644 tests/unit/Core/DB_Test.php diff --git a/src/php/Core/DB.php b/src/php/Core/DB.php index 5d90d3e6d..1e403cac6 100644 --- a/src/php/Core/DB.php +++ b/src/php/Core/DB.php @@ -98,13 +98,32 @@ public static function table_exists( string $table_name, bool $refresh = false ) /** * Create the snippet tables, or upgrade them if they already exist + * + * @return bool Whether every table was created or upgraded. */ - public function create_or_upgrade_tables() { + public function create_or_upgrade_tables(): bool { + $success = true; + if ( is_multisite() ) { - $this->create_table( $this->ms_table ); + $success = $this->create_table( $this->ms_table ); } - $this->create_table( $this->table ); + return $this->create_table( $this->table ) && $success; + } + + /** + * Upgrade a table that a write has just failed on because it lacks a column. + * + * A table that missed an upgrade fails every save that writes a column it does not have. + * + * @param string $table_name Table the write failed on. + * + * @return bool Whether the table was upgraded, so the write can be retried. + */ + public static function upgrade_after_unknown_column( string $table_name ): bool { + global $wpdb; + + return false !== stripos( (string) $wpdb->last_error, 'Unknown column' ) && self::create_table( $table_name ); } /** diff --git a/src/php/snippet-ops.php b/src/php/snippet-ops.php index d43f5f056..277b874fa 100644 --- a/src/php/snippet-ops.php +++ b/src/php/snippet-ops.php @@ -810,6 +810,11 @@ function save_snippet( $snippet ): ?Snippet { if ( 0 === $snippet->id ) { $data['created_by'] = $author_id; $result = $wpdb->insert( $table, $data, '%s' ); + + if ( false === $result && Core\DB::upgrade_after_unknown_column( $table ) ) { + $result = $wpdb->insert( $table, $data, '%s' ); + } + if ( false === $result ) { return null; } @@ -828,8 +833,17 @@ function save_snippet( $snippet ): ?Snippet { // Otherwise, update the snippet data. $existing = get_snippet( $snippet->id, $snippet->network ); + $result = $wpdb->update( $table, $data, [ 'id' => $snippet->id ], null, [ '%d' ] ); + + if ( false === $result && Core\DB::upgrade_after_unknown_column( $table ) ) { + $result = $wpdb->update( $table, $data, [ 'id' => $snippet->id ], null, [ '%d' ] ); + } + + if ( false === $result ) { + return null; + } + set_snippet_locked( $snippet->id, $snippet->locked, $snippet->network ); - $wpdb->update( $table, $data, [ 'id' => $snippet->id ], null, [ '%d' ] ); // The row has changed, so the cached list no longer describes it. It is // dropped before the snippet is read back, because everything below — diff --git a/tests/unit/Core/DB_Test.php b/tests/unit/Core/DB_Test.php new file mode 100644 index 000000000..7b7a27d98 --- /dev/null +++ b/tests/unit/Core/DB_Test.php @@ -0,0 +1,158 @@ +shadowed ) { + $wpdb->query( 'DROP TEMPORARY TABLE IF EXISTS `' . code_snippets()->db->table . '`' ); + $this->shadowed = false; + } + + remove_all_filters( 'query' ); + parent::tear_down(); + } + + /** + * Stand in a snippets table that missed the upgrade adding the authorship columns. + * + * A temporary table hides the real one of the same name for this connection only. + * + * @return void + */ + private function shadow_table_without_authorship(): void { + global $wpdb; + + $table = code_snippets()->db->table; + + $wpdb->query( + "CREATE TEMPORARY TABLE `$table` ( + id BIGINT(20) NOT NULL AUTO_INCREMENT, + name TINYTEXT NOT NULL, + description TEXT NOT NULL, + code LONGTEXT NOT NULL, + tags LONGTEXT NOT NULL, + scope VARCHAR(15) NOT NULL DEFAULT 'global', + condition_id BIGINT(20) NOT NULL DEFAULT 0, + priority SMALLINT NOT NULL DEFAULT 10, + active TINYINT(1) NOT NULL DEFAULT 0, + modified DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + revision BIGINT(20) NOT NULL DEFAULT 1, + cloud_id VARCHAR(255) NULL, + PRIMARY KEY (id) + )" + ); + + $this->shadowed = true; + DB::table_exists( $table, true ); + } + + /** + * The names of the columns the current snippets table has. + * + * @return string[] + */ + private function columns(): array { + global $wpdb; + + return $wpdb->get_col( 'SHOW COLUMNS FROM `' . code_snippets()->db->table . '`' ); + } + + /** + * A table that missed the authorship upgrade is upgraded by the first save, which then succeeds. + * + * @return void + */ + public function test_saving_upgrades_a_table_missing_a_column(): void { + $this->shadow_table_without_authorship(); + $this->assertNotContains( 'updated_by', $this->columns() ); + + $suppressed = $GLOBALS['wpdb']->suppress_errors(); + $created = save_snippet( + new Snippet( + [ + 'name' => 'Created on an old table', + 'code' => '// one', + ] + ) + ); + $GLOBALS['wpdb']->suppress_errors( $suppressed ); + + $this->assertNotNull( $created ); + $this->assertGreaterThan( 0, $created->id ); + $this->assertContains( 'created_by', $this->columns() ); + $this->assertContains( 'updated_by', $this->columns() ); + + $created->code = '// two'; + $this->assertNotNull( save_snippet( $created ) ); + $this->assertSame( '// two', get_snippet( $created->id )->code ); + } + + /** + * An edit that cannot be written is reported as failed rather than returning the unchanged snippet. + * + * @return void + */ + public function test_failed_update_returns_null(): void { + $snippet = save_snippet( + new Snippet( + [ + 'name' => 'Cannot be updated', + 'code' => '// before', + ] + ) + ); + $table = code_snippets()->db->table; + + add_filter( + 'query', + static fn( string $query ): string => 0 === strpos( $query, "UPDATE `$table`" ) ? 'UPDATE `cs_missing_table` SET id = 1' : $query + ); + + $snippet->code = '// after'; + + $suppressed = $GLOBALS['wpdb']->suppress_errors(); + $saved = save_snippet( $snippet ); + $GLOBALS['wpdb']->suppress_errors( $suppressed ); + + $this->assertNull( $saved ); + + remove_all_filters( 'query' ); + $this->assertSame( '// before', get_snippet( $snippet->id )->code ); + } + + /** + * Upgrading the tables reports success when they match the current schema. + * + * @return void + */ + public function test_create_or_upgrade_tables_reports_success(): void { + $this->assertTrue( code_snippets()->db->create_or_upgrade_tables() ); + } +} From 5dec20b112a13140bda44fb7089f166fcae02895 Mon Sep 17 00:00:00 2001 From: TallblokeUK Date: Thu, 29 Oct 2026 16:06:13 +0000 Subject: [PATCH 2/3] fix: report a failed database table upgrade instead of always reporting success --- src/php/Settings/settings.php | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/php/Settings/settings.php b/src/php/Settings/settings.php index 62ebd0a16..d1036ff59 100644 --- a/src/php/Settings/settings.php +++ b/src/php/Settings/settings.php @@ -374,14 +374,26 @@ function process_settings_actions( array $input ): ?array { } if ( isset( $input['debug']['database_update'] ) ) { - code_snippets()->db->create_or_upgrade_tables(); + if ( code_snippets()->db->create_or_upgrade_tables() ) { + add_settings_error( + OPTION_NAME, + 'database_update_done', + __( 'Successfully performed database table upgrade.', 'code-snippets' ), + 'updated' + ); + } else { + global $wpdb; - add_settings_error( - OPTION_NAME, - 'database_update_done', - __( 'Successfully performed database table upgrade.', 'code-snippets' ), - 'updated' - ); + add_settings_error( + OPTION_NAME, + 'database_update_failed', + sprintf( + /* translators: %s: database error message */ + __( 'The database table upgrade did not complete: %s', 'code-snippets' ), + $wpdb->last_error + ) + ); + } } if ( isset( $input['debug']['reset_caches'] ) ) { From b9373ff3e66e7f8ef722b836a5d0a441a2fac20b Mon Sep 17 00:00:00 2001 From: Shea Bunge Date: Tue, 3 Nov 2026 17:04:08 +1100 Subject: [PATCH 3/3] Minor formatting adjustments. --- src/php/Core/DB.php | 11 +++------ src/php/Settings/settings.php | 45 ++++++++++++++++------------------- src/php/snippet-ops.php | 13 ++++++---- 3 files changed, 33 insertions(+), 36 deletions(-) diff --git a/src/php/Core/DB.php b/src/php/Core/DB.php index 1e403cac6..0f10c9185 100644 --- a/src/php/Core/DB.php +++ b/src/php/Core/DB.php @@ -102,13 +102,8 @@ public static function table_exists( string $table_name, bool $refresh = false ) * @return bool Whether every table was created or upgraded. */ public function create_or_upgrade_tables(): bool { - $success = true; - - if ( is_multisite() ) { - $success = $this->create_table( $this->ms_table ); - } - - return $this->create_table( $this->table ) && $success; + $network_okay = ! is_multisite() || $this->create_table( $this->ms_table ); + return $this->create_table( $this->table ) && $network_okay; } /** @@ -341,7 +336,7 @@ private static function fetch_snippets_from_table( string $table_name, array $sc : 'AND active <> -1'; $snippets = $wpdb->get_results( - // phpcs:disable WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare + // phpcs:disable WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare $wpdb->prepare( " SELECT id, code, scope, active, priority diff --git a/src/php/Settings/settings.php b/src/php/Settings/settings.php index d1036ff59..a250593d3 100644 --- a/src/php/Settings/settings.php +++ b/src/php/Settings/settings.php @@ -217,7 +217,7 @@ function register_plugin_settings() { // accessible name. Checkboxes render their own labels, and // callback/action fields have no single control to point at. $labelable_types = [ 'text', 'number', 'select' ]; - $field_args = []; + $field_args = []; if ( isset( $field['type'] ) && in_array( $field['type'], $labelable_types, true ) ) { $field_args['label_for'] = $field_object->element_id; @@ -259,7 +259,7 @@ function register_plugin_settings() { * * @param array $field Field definition. * @param array> $settings Current settings values. - * @param array>|null $input Optional raw input values. + * @param array>|null $input Optional raw input values. * * @return bool */ @@ -365,7 +365,7 @@ function process_settings_actions( array $input ): ?array { add_settings_error( OPTION_NAME, 'settings_reset', - __( 'All settings have been reset to their defaults.', 'code-snippets' ), + esc_html__( 'All settings have been reset to their defaults.', 'code-snippets' ), 'updated' ); @@ -378,20 +378,19 @@ function process_settings_actions( array $input ): ?array { add_settings_error( OPTION_NAME, 'database_update_done', - __( 'Successfully performed database table upgrade.', 'code-snippets' ), + esc_html__( 'Successfully performed database table upgrade.', 'code-snippets' ), 'updated' ); } else { global $wpdb; + /* translators: %s: database error message */ + $error_text = __( 'The database table upgrade did not complete: %s', 'code-snippets' ); + add_settings_error( OPTION_NAME, 'database_update_failed', - sprintf( - /* translators: %s: database error message */ - __( 'The database table upgrade did not complete: %s', 'code-snippets' ), - $wpdb->last_error - ) + esc_html( sprintf( $error_text, $wpdb->last_error ) ), ); } } @@ -418,29 +417,27 @@ function process_settings_actions( array $input ): ?array { $refresh_failure = Version_Switch::get_refresh_failure(); if ( $refresh_failure ) { + /* translators: %s: reason the plugin versions could not be listed. */ + $message = __( 'Successfully reset snippets caches, but the available plugin versions could not be listed: %s', 'code-snippets' ); + add_settings_error( OPTION_NAME, 'snippet_caches_reset', - sprintf( - /* translators: %s: reason the plugin versions could not be listed. */ - __( 'Successfully reset snippets caches, but the available plugin versions could not be listed: %s', 'code-snippets' ), - $refresh_failure - ) + esc_html( sprintf( $message, $refresh_failure ) ), ); } else { + /* translators: %d: number of plugin versions available to install. */ + $message = _n( + 'Successfully reset snippets caches. %d plugin version is available to install.', + 'Successfully reset snippets caches. %d plugin versions are available to install.', + count( $available_versions ), + 'code-snippets' + ); + add_settings_error( OPTION_NAME, 'snippet_caches_reset', - sprintf( - /* translators: %d: number of plugin versions available to install. */ - _n( - 'Successfully reset snippets caches. %d plugin version is available to install.', - 'Successfully reset snippets caches. %d plugin versions are available to install.', - count( $available_versions ), - 'code-snippets' - ), - count( $available_versions ) - ), + esc_html( sprintf( $message, count( $available_versions ) ) ), 'updated' ); } diff --git a/src/php/snippet-ops.php b/src/php/snippet-ops.php index 277b874fa..6770200de 100644 --- a/src/php/snippet-ops.php +++ b/src/php/snippet-ops.php @@ -809,10 +809,12 @@ function save_snippet( $snippet ): ?Snippet { // Create a new snippet if the ID is not set. if ( 0 === $snippet->id ) { $data['created_by'] = $author_id; - $result = $wpdb->insert( $table, $data, '%s' ); + $format = '%s'; + + $result = $wpdb->insert( $table, $data, $format ); if ( false === $result && Core\DB::upgrade_after_unknown_column( $table ) ) { - $result = $wpdb->insert( $table, $data, '%s' ); + $result = $wpdb->insert( $table, $data, $format ); } if ( false === $result ) { @@ -833,10 +835,13 @@ function save_snippet( $snippet ): ?Snippet { // Otherwise, update the snippet data. $existing = get_snippet( $snippet->id, $snippet->network ); - $result = $wpdb->update( $table, $data, [ 'id' => $snippet->id ], null, [ '%d' ] ); + $where = [ 'id' => $snippet->id ]; + $where_format = [ '%d' ]; + + $result = $wpdb->update( $table, $data, $where, null, $where_format ); if ( false === $result && Core\DB::upgrade_after_unknown_column( $table ) ) { - $result = $wpdb->update( $table, $data, [ 'id' => $snippet->id ], null, [ '%d' ] ); + $result = $wpdb->update( $table, $data, $where, null, $where_format ); } if ( false === $result ) {