fix: Handle missing table in DB repair — use activate() with dbDelta - #98
Conversation
The repair notice was showing indefinitely because: 1. is_database_schema_valid() checked only for a missing column, not a missing table — SHOW COLUMNS on a non-existent table returns empty, causing false positives 2. The repair handler called maybe_upgrade() which runs ALTER TABLE, which fails when the table itself doesn't exist Changes: - is_database_schema_valid() now checks SHOW TABLES first, then checks for the opt_in_token column - handle_database_upgrade() now calls MSKD_Activator::activate() which uses dbDelta to create missing tables and all their columns - Removed the direct ALTER TABLE fallback (wrong for missing tables) - Error notice now shows DB error and suggests deactivate/reactivate - Repair notice text updated to mention tables as well as columns
There was a problem hiding this comment.
Pull request overview
Fixes an issue where the “Repair Database Now” flow could never resolve on some sites because the subscribers table was missing entirely, causing schema checks/upgrades to fail and the repair notice to reappear indefinitely.
Changes:
- Added an explicit table-existence check before checking for required columns.
- Switched the repair handler to run
MSKD_Activator::activate()(dbDelta-based) and added post-run schema validation with an error redirect. - Updated admin notice copy and documented the fix in the changelog.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| includes/Admin/class-admin-notices.php | Improves DB repair flow (table existence check, dbDelta-based repair, error notice + redirect). |
| CHANGELOG.md | Documents the fix in the Unreleased changelog. |
Comments suppressed due to low confidence (1)
CHANGELOG.md:18
- Under the
[Unreleased]section there are now multiple### Fixedheadings (one at line 10 and another later at line 16), which makes the changelog harder to scan and deviates from the usual Keep a Changelog structure. Please consolidate all Unreleased fixes under a single### Fixedheading.
## [Unreleased]
### Fixed
- **Database repair notice persisting infinitely** — when clicking "Repair Database Now", if the required database table or column did not exist (or `ALTER TABLE` silently failed), the repair notice was shown on every page load indefinitely. The handler now calls `MSKD_Activator::activate()` (which uses `dbDelta` to create missing tables and columns), verifies the schema afterwards, and — if still failing — shows an actionable error notice with the database error message. The schema check also now correctly detects a missing table (not just a missing column).
### Changed
- **Delete Inactive Subscribers button** now also deletes subscribers with `unsubscribed` status, in addition to `inactive` (unconfirmed). Updated button description, confirmation dialog, and success messages accordingly. Translations updated for Bulgarian and German.
### Fixed
- Fixed undefined variable `$class` (should be `$class_name`) in test bootstrap autoloader, which caused PHP 8.1 test failures due to undefined variable warnings being converted to exceptions.
| // Show error message if the database repair failed. | ||
| // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Just displaying a message. | ||
| if ( isset( $_GET['mskd_db_error'] ) ) { | ||
| // phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Just displaying a message. | ||
| $db_error = sanitize_text_field( wp_unslash( $_GET['mskd_db_error'] ) ); | ||
| add_action( | ||
| 'admin_notices', | ||
| function () use ( $db_error ) { | ||
| ?> | ||
| <div class="notice notice-error"> | ||
| <p> | ||
| <strong><?php esc_html_e( 'Mail System:', 'mail-system-by-katsarov-design' ); ?></strong> | ||
| <?php esc_html_e( 'Database repair failed. One or more required tables or columns could not be created. Please deactivate and reactivate the plugin, or contact your hosting provider if the issue persists.', 'mail-system-by-katsarov-design' ); ?> | ||
| </p> | ||
| <?php if ( '1' !== $db_error ) : ?> | ||
| <p><em><?php echo esc_html( $db_error ); ?></em></p> | ||
| <?php endif; ?> | ||
| </div> | ||
| <?php | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
The new mskd_db_error notice is added before any current_user_can('manage_options') / is_plugin_page() checks, so any admin-area user (or any admin screen) can display a raw DB error just by adding the query arg. Please gate this notice behind the same capability + plugin-page checks used for the upgrade/repair notices, and consider hiding the raw $wpdb->last_error unless the current user can manage options.
| $table = $wpdb->prefix . 'mskd_subscribers'; | ||
|
|
||
| // First check the table itself exists. | ||
| // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Schema check. | ||
| $table_exists = $wpdb->get_var( | ||
| $wpdb->prepare( 'SHOW TABLES LIKE %s', $table ) | ||
| ); | ||
|
|
||
| if ( ! $table_exists ) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
SHOW TABLES LIKE %s treats _ and % as wildcards. Since WordPress table names commonly contain underscores (e.g. wp_mskd_subscribers), this can return a false positive match and also trigger unnecessary DB errors later. Use $wpdb->esc_like( $table ) (and keep it as an exact pattern) before passing it to prepare() so the existence check is literal.
| // Then check the opt_in_token column exists. | ||
| // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Schema check. | ||
| $column_exists = $wpdb->get_results( | ||
| $wpdb->prepare( | ||
| "SHOW COLUMNS FROM {$wpdb->prefix}mskd_subscribers LIKE %s", | ||
| "SHOW COLUMNS FROM {$table} LIKE %s", | ||
| 'opt_in_token' | ||
| ) | ||
| ); |
There was a problem hiding this comment.
The query passed to $wpdb->prepare() interpolates {$table} directly into the SQL string. Elsewhere in the codebase, these cases include a // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared annotation to keep composer phpcs passing. Add the appropriate PHPCS ignore (or refactor to avoid interpolation) so this new schema check doesn’t introduce a lint failure.
| // Run full activation: uses dbDelta to create missing tables and columns. | ||
| \MSKD_Activator::activate(); |
There was a problem hiding this comment.
MSKD_Activator::activate() does more than schema repair (it schedules cron, sets default options, updates the DB version, and calls flush_rewrite_rules()). Running full activation from an admin repair action can have unexpected side effects and extra overhead. Consider extracting a schema-only method (e.g., calling create_tables()/dbDelta without flushing rewrites) for the repair path.
Problem
The "Repair Database Now" notice was showing on every page load indefinitely on some live servers.
Root cause (revealed by live server error):
The table
{prefix}mskd_subscribersdidn't exist at all. Theis_database_schema_valid()method usedSHOW COLUMNSto check for theopt_in_tokencolumn, butSHOW COLUMNSon a non-existent table returns empty results — making it indistinguishable from a missing column. The repair handler then calledmaybe_upgrade()which only runsALTER TABLE, which also fails on a non-existent table. The version was never updated to current, so the repair notice reappeared forever.Changes
is_database_schema_valid()SHOW TABLES LIKEfirst to confirm the table exists before checking for the columnfalsecorrectly for both a missing table and a missing columnhandle_database_upgrade()MSKD_Activator::activate()(which usesdbDelta) instead of resetting the version to 1.0.0 and callingmaybe_upgrade()dbDeltahandles both creating a missing table from scratch and adding missing columns to an existing tableALTER TABLEfallback (incorrect when the table itself is missing)Repair notice text
Testing
opt_in_tokencolumn is missing → should succeed viadbDeltadbDeltadbDeltafails → error notice with DB error message is shown