| @@ -1,5 +1,6 @@ | ||
| 1 | 1 | <?php |
| 2 | + | |
| 2 | 3 | /** |
| 3 | 4 | * Abstract SEO Manager Base Class |
| 4 | 5 | * |
| 5 | 6 | * Provides common functionality for all SEO managers including database operations, |
| @@ -16,8 +17,13 @@ | ||
| 16 | 17 | namespace ThinkRank\SEO; |
| 17 | 18 | |
| 18 | 19 | use ThinkRank\SEO\Interfaces\SEO_Manager_Interface; |
| 19 | 20 | |
| 21 | +// Prevent direct access | |
| 22 | +if (!defined('ABSPATH')) { | |
| 23 | + exit; | |
| 24 | +} | |
| 25 | + | |
| 20 | 26 | /** |
| 21 | 27 | * Abstract SEO Manager Base Class |
| 22 | 28 | * |
| 23 | 29 | * Implements common functionality for all SEO managers following DRY principles. |
| @@ -51,8 +57,30 @@ | ||
| 51 | 57 | */ |
| 52 | 58 | protected string $manager_type; |
| 53 | 59 | |
| 54 | 60 | /** |
| 61 | + * Why the most recent save_settings() call failed. | |
| 62 | + * | |
| 63 | + * save_settings() returns a bare bool, so the caller that has to tell the | |
| 64 | + * user something ends up printing a generic "failed" string while the real | |
| 65 | + * reason goes only to the error log. Holding it here lets the REST layer | |
| 66 | + * put the actual cause in the response. First failure wins: a rejected | |
| 67 | + * INSERT can cascade across keys, and the first one names the root cause. | |
| 68 | + * | |
| 69 | + * @since 1.32.1 | |
| 70 | + * @var string | |
| 71 | + */ | |
| 72 | + protected string $last_save_error = ''; | |
| 73 | + | |
| 74 | + /** | |
| 75 | + * Machine-readable counterpart to $last_save_error. | |
| 76 | + * | |
| 77 | + * @since 1.32.1 | |
| 78 | + * @var string | |
| 79 | + */ | |
| 80 | + protected string $last_save_error_code = ''; | |
| 81 | + | |
| 82 | + /** | |
| 55 | 83 | * Supported context types |
| 56 | 84 | * |
| 57 | 85 | * @since 1.0.0 |
| 58 | 86 | * @var array |
| @@ -67,9 +95,9 @@ | ||
| 67 | 95 | * @param string $manager_type The manager type identifier |
| 68 | 96 | */ |
| 69 | 97 | public function __construct(string $manager_type) { |
| 70 | 98 | global $wpdb; |
| 71 | - | |
| 99 | + | |
| 72 | 100 | $this->wpdb = $wpdb; |
| 73 | 101 | $this->settings_table = $wpdb->prefix . 'thinkrank_seo_settings'; |
| 74 | 102 | $this->manager_type = sanitize_key($manager_type); |
| 75 | 103 | } |
| @@ -76,8 +104,15 @@ | ||
| 76 | 104 | |
| 77 | 105 | /** |
| 78 | 106 | * Get SEO settings for a specific context |
| 79 | 107 | * |
| 108 | + * The returned array always carries every key the context defines: the | |
| 109 | + * saved rows are merged OVER the context defaults, so callers can read a | |
| 110 | + * key without checking whether it exists. That also means the result never | |
| 111 | + * distinguishes "the user saved this" from "this is the built-in default" — | |
| 112 | + * when a caller needs that distinction (an audit asking whether anything | |
| 113 | + * was configured at all), use get_stored_settings() instead. | |
| 114 | + * | |
| 80 | 115 | * @since 1.0.0 |
| 81 | 116 | * |
| 82 | 117 | * @param string $context_type The context type |
| 83 | 118 | * @param int|null $context_id Optional. Context ID |
| @@ -84,62 +119,103 @@ | ||
| 84 | 119 | * @return array SEO settings array |
| 85 | 120 | */ |
| 86 | 121 | public function get_settings(string $context_type, ?int $context_id = null): array { |
| 87 | 122 | $context_type = sanitize_key($context_type); |
| 88 | - | |
| 123 | + | |
| 89 | 124 | if (!in_array($context_type, $this->get_supported_contexts(), true)) { |
| 90 | 125 | return $this->get_default_settings($context_type); |
| 91 | 126 | } |
| 92 | 127 | |
| 93 | - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- SEO settings require direct database access for real-time data, table name is validated | |
| 94 | - $sql = sprintf( | |
| 95 | - 'SELECT setting_key, setting_value FROM `%s` WHERE context_type = %%s AND context_id IS NULL AND setting_category = %%s AND is_active = 1', | |
| 96 | - $this->settings_table | |
| 97 | - ); | |
| 98 | - if (null === $context_id) { | |
| 99 | - $results = $this->wpdb->get_results( | |
| 100 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 101 | - $this->wpdb->prepare( | |
| 102 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 103 | - $sql, | |
| 104 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Parameters are validated and used as placeholders | |
| 105 | - $context_type, | |
| 106 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- manager_type is validated class property | |
| 107 | - $this->manager_type | |
| 108 | - ), | |
| 109 | - ARRAY_A | |
| 110 | - ); | |
| 111 | - } else { | |
| 112 | - $sql = sprintf( | |
| 113 | - 'SELECT setting_key, setting_value FROM `%s` WHERE context_type = %%s AND context_id = %%d AND setting_category = %%s AND is_active = 1', | |
| 114 | - $this->settings_table | |
| 115 | - ); | |
| 116 | - $results = $this->wpdb->get_results( | |
| 117 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 118 | - $this->wpdb->prepare( | |
| 119 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 120 | - $sql, | |
| 121 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Parameters are validated and used as placeholders | |
| 122 | - $context_type, | |
| 123 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- context_id is validated integer | |
| 124 | - $context_id, | |
| 125 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- manager_type is validated class property | |
| 126 | - $this->manager_type | |
| 127 | - ), | |
| 128 | - ARRAY_A | |
| 129 | - ); | |
| 128 | + // Serve from the object cache when available. This runs on every front-end | |
| 129 | + // request (the_content, thumbnails), so avoiding a DB hit per request matters. | |
| 130 | + $cache_key = $this->get_cache_key($context_type, $context_id); | |
| 131 | + $cached = wp_cache_get($cache_key, 'thinkrank_seo'); | |
| 132 | + if (is_array($cached)) { | |
| 133 | + return $cached; | |
| 130 | 134 | } |
| 131 | 135 | |
| 136 | + // Merge with defaults to ensure all required keys exist | |
| 137 | + $merged = array_merge( | |
| 138 | + $this->get_default_settings($context_type), | |
| 139 | + $this->get_stored_settings($context_type, $context_id) | |
| 140 | + ); | |
| 141 | + | |
| 142 | + // Cache the resolved settings; invalidated on every save via clear_cache(). | |
| 143 | + wp_cache_set($cache_key, $merged, 'thinkrank_seo'); | |
| 144 | + | |
| 145 | + return $merged; | |
| 146 | + } | |
| 147 | + | |
| 148 | + /** | |
| 149 | + * Get ONLY the settings actually saved for a context — no defaults merged. | |
| 150 | + * | |
| 151 | + * get_settings() layers the context defaults under the stored rows, which | |
| 152 | + * makes "has the user configured anything?" unanswerable through it: a site | |
| 153 | + * with zero saved rows still gets back a fully populated array. Any audit | |
| 154 | + * or first-run check that must tell a configured site from an untouched one | |
| 155 | + * has to read the storage layer directly, which is what this exposes. | |
| 156 | + * | |
| 157 | + * Returns an empty array when nothing has been saved for the context, or | |
| 158 | + * when the context type is not supported by this manager. | |
| 159 | + * | |
| 160 | + * Note this is the storage layer, not the settings a manager reports: a | |
| 161 | + * subclass that overrides get_settings() to layer in another source — | |
| 162 | + * Schema_Management_System fills its logo and organization fields from Site | |
| 163 | + * Identity, Social_Meta_Manager has its own override — contributes nothing | |
| 164 | + * here. Read it to ask what the site saved, never to read a value out. | |
| 165 | + * | |
| 166 | + * @since 2.3.1 | |
| 167 | + * | |
| 168 | + * @param string $context_type The context type | |
| 169 | + * @param int|null $context_id Optional. Context ID | |
| 170 | + * @return array Saved settings, keyed by setting key. Empty when nothing is stored. | |
| 171 | + */ | |
| 172 | + public function get_stored_settings(string $context_type, ?int $context_id = null): array { | |
| 173 | + $context_type = sanitize_key($context_type); | |
| 174 | + | |
| 175 | + if (!in_array($context_type, $this->get_supported_contexts(), true)) { | |
| 176 | + return []; | |
| 177 | + } | |
| 178 | + | |
| 179 | + // Convert NULL context_id to 0 for site-wide settings to match save behavior | |
| 180 | + $db_context_id = $context_id === null ? 0 : $context_id; | |
| 181 | + | |
| 182 | + // Cached under its own key so the merged and unmerged views can never be | |
| 183 | + // served for one another. clear_cache() drops both on every save. | |
| 184 | + $cache_key = $this->get_stored_cache_key($context_type, $context_id); | |
| 185 | + $cached = wp_cache_get($cache_key, 'thinkrank_seo'); | |
| 186 | + if (is_array($cached)) { | |
| 187 | + return $cached; | |
| 188 | + } | |
| 189 | + | |
| 190 | + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SEO settings require direct database access for real-time data, table name is validated | |
| 191 | + $sql = sprintf( | |
| 192 | + 'SELECT setting_key, setting_value FROM `%s` WHERE context_type = %%s AND context_id = %%d AND setting_category = %%s AND is_active = 1', | |
| 193 | + $this->settings_table | |
| 194 | + ); | |
| 195 | + | |
| 196 | + // phpcs:ignore PluginCheck.Security.DirectDB.UnescapedDBParameter -- $sql is built from sprintf with validated table name then prepared below. | |
| 197 | + $results = $this->wpdb->get_results( | |
| 198 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SQL is properly prepared with placeholders | |
| 199 | + $this->wpdb->prepare( | |
| 200 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SQL is properly prepared with placeholders | |
| 201 | + $sql, | |
| 202 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- Parameters are validated and used as placeholders | |
| 203 | + $context_type, | |
| 204 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- context_id is validated integer | |
| 205 | + $db_context_id, | |
| 206 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- manager_type is validated class property | |
| 207 | + $this->manager_type | |
| 208 | + ), | |
| 209 | + ARRAY_A | |
| 210 | + ); | |
| 211 | + | |
| 132 | 212 | $settings = []; |
| 133 | - foreach ($results as $row) { | |
| 213 | + foreach ((array) $results as $row) { | |
| 134 | 214 | $value = maybe_unserialize($row['setting_value']); |
| 135 | 215 | |
| 136 | - // Ensure proper data type conversion for common boolean fields | |
| 137 | - if (in_array($row['setting_key'], [ | |
| 138 | - 'enabled', 'auto_generate_schema', 'rich_snippets_optimization', | |
| 139 | - 'performance_tracking', 'auto_deploy', 'validation_on_save', | |
| 140 | - 'rich_snippets_testing', 'organization_schema', 'knowledge_graph' | |
| 141 | - ], true)) { | |
| 216 | + // Ensure proper data type conversion for boolean fields | |
| 217 | + if (in_array($row['setting_key'], $this->boolean_setting_keys(), true)) { | |
| 142 | 218 | // Convert string/numeric boolean representations to actual booleans |
| 143 | 219 | if (is_string($value)) { |
| 144 | 220 | $value = in_array(strtolower($value), ['true', '1', 'yes', 'on'], true); |
| 145 | 221 | } elseif (is_numeric($value)) { |
| @@ -154,10 +230,11 @@ | ||
| 154 | 230 | |
| 155 | 231 | $settings[$row['setting_key']] = $value; |
| 156 | 232 | } |
| 157 | 233 | |
| 158 | - // Merge with defaults to ensure all required keys exist | |
| 159 | - return array_merge($this->get_default_settings($context_type), $settings); | |
| 234 | + wp_cache_set($cache_key, $settings, 'thinkrank_seo'); | |
| 235 | + | |
| 236 | + return $settings; | |
| 160 | 237 | } |
| 161 | 238 | |
| 162 | 239 | /** |
| 163 | 240 | * Save SEO settings for a specific context |
| @@ -171,16 +248,39 @@ | ||
| 171 | 248 | */ |
| 172 | 249 | public function save_settings(string $context_type, ?int $context_id, array $settings): bool { |
| 173 | 250 | $context_type = sanitize_key($context_type); |
| 174 | 251 | |
| 252 | + $this->last_save_error = ''; | |
| 253 | + $this->last_save_error_code = ''; | |
| 254 | + | |
| 175 | 255 | if (!in_array($context_type, $this->get_supported_contexts(), true)) { |
| 176 | - // Unsupported context type - validation failed | |
| 256 | + $this->log_save_failure("unsupported context type '{$context_type}'", 'unsupported_context'); | |
| 177 | 257 | return false; |
| 178 | 258 | } |
| 179 | 259 | |
| 180 | - // Check if settings table exists | |
| 260 | + // Check if settings table exists. This is the failure a user cannot | |
| 261 | + // diagnose from the UI: on hosts where CREATE TABLE failed (e.g. the | |
| 262 | + // 767-byte InnoDB index limit on MySQL 5.6-era servers), every save in | |
| 263 | + // every manager fails with a generic message while option-backed | |
| 264 | + // features keep working — so name the cause loudly. | |
| 265 | + // | |
| 266 | + // Creation is retried on every request, so a table that stays missing | |
| 267 | + // means the database is refusing the statement. Database_Schema records | |
| 268 | + // that refusal; lead with it, because it is the only text here that | |
| 269 | + // names this site's actual problem. | |
| 181 | 270 | if (!$this->ensure_settings_table_exists()) { |
| 182 | - // Settings table creation failed | |
| 271 | + $create_error = \ThinkRank\Database\Database_Schema::get_last_create_failure(); | |
| 272 | + | |
| 273 | + $this->log_save_failure( | |
| 274 | + "settings table '{$this->settings_table}' does not exist. " . | |
| 275 | + ('' !== $create_error | |
| 276 | + ? 'The database refused to create it: ' . $create_error | |
| 277 | + : 'ThinkRank re-attempts creation on every load, so no reactivation is needed. ' . | |
| 278 | + 'If the table never appears, the database is rejecting the CREATE TABLE: check that the ' . | |
| 279 | + 'database user holds the CREATE privilege, and ask your host for the MySQL/MariaDB version, ' . | |
| 280 | + 'as 5.6-era servers cap an index at 767 bytes and reject wider schemas.'), | |
| 281 | + 'settings_table_missing' | |
| 282 | + ); | |
| 183 | 283 | return false; |
| 184 | 284 | } |
| 185 | 285 | |
| 186 | 286 | // Validate settings before saving |
| @@ -185,36 +285,58 @@ | ||
| 185 | 285 | |
| 186 | 286 | // Validate settings before saving |
| 187 | 287 | $validation = $this->validate_settings($settings); |
| 188 | 288 | if (!$validation['valid']) { |
| 189 | - // Settings validation failed - error details available in validation response | |
| 289 | + $this->log_save_failure( | |
| 290 | + 'validation failed: ' . wp_json_encode($validation['errors'] ?? []), | |
| 291 | + 'validation_failed' | |
| 292 | + ); | |
| 190 | 293 | return false; |
| 191 | 294 | } |
| 192 | 295 | |
| 193 | 296 | // Sanitize settings |
| 194 | - $sanitized_settings = $this->sanitize_settings($settings); | |
| 297 | + $sanitized_settings = $this->sanitize_settings($settings, $context_type); | |
| 195 | 298 | |
| 196 | 299 | $success = true; |
| 197 | 300 | foreach ($sanitized_settings as $key => $value) { |
| 198 | - $data = [ | |
| 199 | - 'context_type' => $context_type, | |
| 200 | - 'context_id' => $context_id, | |
| 201 | - 'setting_category' => $this->manager_type, | |
| 202 | - 'setting_key' => sanitize_key($key), | |
| 203 | - 'setting_value' => maybe_serialize($value), | |
| 204 | - 'is_active' => 1, | |
| 205 | - 'updated_at' => current_time('mysql') | |
| 206 | - ]; | |
| 301 | + $sanitized_key = sanitize_key($key); | |
| 302 | + $serialized_value = maybe_serialize($value); | |
| 303 | + $current_time = current_time('mysql'); | |
| 207 | 304 | |
| 208 | - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching -- SEO settings require direct database access for real-time updates | |
| 209 | - $result = $this->wpdb->replace( | |
| 210 | - $this->settings_table, | |
| 211 | - $data, | |
| 212 | - ['%s', '%d', '%s', '%s', '%s', '%d', '%s'] | |
| 305 | + // Convert NULL context_id to 0 for site-wide settings to work with UNIQUE constraint | |
| 306 | + // MySQL treats multiple NULL values as distinct in UNIQUE constraints | |
| 307 | + $db_context_id = $context_id === null ? 0 : $context_id; | |
| 308 | + | |
| 309 | + // Use INSERT ... ON DUPLICATE KEY UPDATE for proper upsert behavior | |
| 310 | + // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- $this->settings_table is a validated class property set from $wpdb->prefix. | |
| 311 | + $sql = $this->wpdb->prepare( | |
| 312 | + // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared | |
| 313 | + "INSERT INTO `{$this->settings_table}` | |
| 314 | + (`context_type`, `context_id`, `setting_category`, `setting_key`, `setting_value`, `is_active`, `created_at`, `updated_at`) | |
| 315 | + VALUES (%s, %d, %s, %s, %s, %d, %s, %s) | |
| 316 | + ON DUPLICATE KEY UPDATE | |
| 317 | + `setting_value` = VALUES(`setting_value`), | |
| 318 | + `is_active` = VALUES(`is_active`), | |
| 319 | + `updated_at` = VALUES(`updated_at`)", | |
| 320 | + $context_type, | |
| 321 | + $db_context_id, | |
| 322 | + $this->manager_type, | |
| 323 | + $sanitized_key, | |
| 324 | + $serialized_value, | |
| 325 | + 1, | |
| 326 | + $current_time, | |
| 327 | + $current_time | |
| 213 | 328 | ); |
| 214 | 329 | |
| 330 | + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SEO settings require direct database access, SQL is properly prepared | |
| 331 | + $result = $this->wpdb->query($sql); | |
| 332 | + | |
| 215 | 333 | if (false === $result) { |
| 216 | - // Database operation failed - error details available in wpdb->last_error | |
| 334 | + $this->log_save_failure( | |
| 335 | + "insert failed for key '{$sanitized_key}'" . | |
| 336 | + ('' !== (string) $this->wpdb->last_error ? ' — ' . $this->wpdb->last_error : ''), | |
| 337 | + 'db_insert_failed' | |
| 338 | + ); | |
| 217 | 339 | $success = false; |
| 218 | 340 | } |
| 219 | 341 | } |
| 220 | 342 | |
| @@ -220,8 +342,31 @@ | ||
| 220 | 342 | |
| 221 | 343 | // Clear relevant caches |
| 222 | 344 | $this->clear_cache($context_type, $context_id); |
| 223 | 345 | |
| 346 | + /** | |
| 347 | + * Fires after a settings category has been written. | |
| 348 | + * | |
| 349 | + * Lets one manager react to another's save — the Schema Manager uses it | |
| 350 | + * to refresh LocalBusiness when Site Identity's Business Info changes, | |
| 351 | + * since those fields live in a different category and never appear in a | |
| 352 | + * schema settings payload (#455). | |
| 353 | + * | |
| 354 | + * @since 2.0.2 | |
| 355 | + * | |
| 356 | + * @param string $manager_type Settings category that was saved. | |
| 357 | + * @param array $settings The sanitized settings that were written. | |
| 358 | + * @param string $context_type Context type. | |
| 359 | + * @param int|null $context_id Context ID. | |
| 360 | + */ | |
| 361 | + do_action( | |
| 362 | + 'thinkrank_seo_settings_saved', | |
| 363 | + $this->manager_type, | |
| 364 | + $sanitized_settings, | |
| 365 | + $context_type, | |
| 366 | + $context_id | |
| 367 | + ); | |
| 368 | + | |
| 224 | 369 | return $success; |
| 225 | 370 | } |
| 226 | 371 | |
| 227 | 372 | /** |
| @@ -235,14 +380,17 @@ | ||
| 235 | 380 | */ |
| 236 | 381 | public function delete_settings(string $context_type, ?int $context_id): bool { |
| 237 | 382 | $context_type = sanitize_key($context_type); |
| 238 | 383 | |
| 239 | - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching -- SEO settings deletion requires direct database access | |
| 384 | + // Convert NULL context_id to 0 for site-wide settings to match save behavior | |
| 385 | + $db_context_id = $context_id === null ? 0 : $context_id; | |
| 386 | + | |
| 387 | + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SEO settings deletion requires direct database access | |
| 240 | 388 | $result = $this->wpdb->delete( |
| 241 | 389 | $this->settings_table, |
| 242 | 390 | [ |
| 243 | 391 | 'context_type' => $context_type, |
| 244 | - 'context_id' => $context_id, | |
| 392 | + 'context_id' => $db_context_id, | |
| 245 | 393 | 'setting_category' => $this->manager_type |
| 246 | 394 | ], |
| 247 | 395 | ['%s', '%d', '%s'] |
| 248 | 396 | ); |
| @@ -266,45 +414,32 @@ | ||
| 266 | 414 | */ |
| 267 | 415 | public function has_settings(string $context_type, ?int $context_id): bool { |
| 268 | 416 | $context_type = sanitize_key($context_type); |
| 269 | 417 | |
| 270 | - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- SEO settings existence check requires direct database access, table name is validated | |
| 271 | - $sql = sprintf( | |
| 272 | - 'SELECT COUNT(*) FROM `%s` WHERE context_type = %%s AND context_id IS NULL AND setting_category = %%s AND is_active = 1', | |
| 273 | - $this->settings_table | |
| 274 | - ); | |
| 275 | - if (null === $context_id) { | |
| 276 | - $count = $this->wpdb->get_var( | |
| 277 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 278 | - $this->wpdb->prepare( | |
| 279 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 280 | - $sql, | |
| 281 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Parameters are validated and used as placeholders | |
| 282 | - $context_type, | |
| 283 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- manager_type is validated class property | |
| 284 | - $this->manager_type | |
| 285 | - ) | |
| 286 | - ); | |
| 287 | - } else { | |
| 288 | - $sql = sprintf( | |
| 289 | - 'SELECT COUNT(*) FROM `%s` WHERE context_type = %%s AND context_id = %%d AND setting_category = %%s AND is_active = 1', | |
| 290 | - $this->settings_table | |
| 291 | - ); | |
| 292 | - $count = $this->wpdb->get_var( | |
| 293 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 294 | - $this->wpdb->prepare( | |
| 295 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 296 | - $sql, | |
| 297 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- Parameters are validated and used as placeholders | |
| 298 | - $context_type, | |
| 299 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- context_id is validated integer | |
| 300 | - $context_id, | |
| 301 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- manager_type is validated class property | |
| 302 | - $this->manager_type | |
| 303 | - ) | |
| 304 | - ); | |
| 305 | - } | |
| 418 | + // Convert NULL context_id to 0 for site-wide settings to match save behavior | |
| 419 | + $db_context_id = $context_id === null ? 0 : $context_id; | |
| 306 | 420 | |
| 421 | + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SEO settings existence check requires direct database access, table name is validated | |
| 422 | + $sql = sprintf( | |
| 423 | + 'SELECT COUNT(*) FROM `%s` WHERE context_type = %%s AND context_id = %%d AND setting_category = %%s AND is_active = 1', | |
| 424 | + $this->settings_table | |
| 425 | + ); | |
| 426 | + | |
| 427 | + // phpcs:ignore PluginCheck.Security.DirectDB.UnescapedDBParameter -- $sql is built from sprintf with validated table name then prepared below. | |
| 428 | + $count = $this->wpdb->get_var( | |
| 429 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SQL is properly prepared with placeholders | |
| 430 | + $this->wpdb->prepare( | |
| 431 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SQL is properly prepared with placeholders | |
| 432 | + $sql, | |
| 433 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- Parameters are validated and used as placeholders | |
| 434 | + $context_type, | |
| 435 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- context_id is validated integer | |
| 436 | + $db_context_id, | |
| 437 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- manager_type is validated class property | |
| 438 | + $this->manager_type | |
| 439 | + ) | |
| 440 | + ); | |
| 441 | + | |
| 307 | 442 | return (int) $count > 0; |
| 308 | 443 | } |
| 309 | 444 | |
| 310 | 445 | /** |
| @@ -318,8 +453,153 @@ | ||
| 318 | 453 | return $this->supported_contexts; |
| 319 | 454 | } |
| 320 | 455 | |
| 321 | 456 | /** |
| 457 | + * String setting keys whose newlines must be preserved on save. | |
| 458 | + * | |
| 459 | + * @var string[] | |
| 460 | + */ | |
| 461 | + private const MULTILINE_STRING_KEYS = ['robots_txt_content']; | |
| 462 | + | |
| 463 | + /** | |
| 464 | + * Keys holding %token% TEMPLATES rather than plain text. | |
| 465 | + * | |
| 466 | + * sanitize_text_field() strips anything matching /%[a-f0-9]{2}/ as a | |
| 467 | + * percent-encoded byte, which silently eats the leading characters of any | |
| 468 | + * token whose first two letters are valid hex — %category_title% becomes | |
| 469 | + * "tegory_title%", %date% becomes "te%". These keys therefore go through | |
| 470 | + * sanitize_template_field() instead. | |
| 471 | + */ | |
| 472 | + private const TEMPLATE_STRING_KEYS = [ | |
| 473 | + 'homepage_title', 'post_title', 'page_title', 'category_title', 'tag_title', | |
| 474 | + 'search_title', 'archive_title', 'author_title', | |
| 475 | + 'homepage_description', 'post_description', 'page_description', | |
| 476 | + 'title_template', 'description_template', | |
| 477 | + 'alt_format', 'title_format', 'caption_format', | |
| 478 | + 'subject_template', | |
| 479 | + ]; | |
| 480 | + | |
| 481 | + /** | |
| 482 | + * REST envelope keys that must never become stored settings. | |
| 483 | + * | |
| 484 | + * Every settings endpoint answers with | |
| 485 | + * {settings, schema, context_type, context_id}. A caller that posts that | |
| 486 | + * whole envelope back as `settings` writes those four keys as rows, and | |
| 487 | + * because get_settings() returns every stored row, they then round-trip | |
| 488 | + * into the next request forever — the Site Identity payload carried ~6KB | |
| 489 | + * of a serialized copy of itself plus its own JSON schema on every save. | |
| 490 | + * They are not settings in any manager, so drop them on the way in. | |
| 491 | + * | |
| 492 | + * @var string[] | |
| 493 | + */ | |
| 494 | + protected const RESERVED_ENVELOPE_KEYS = ['settings', 'schema', 'context_type', 'context_id']; | |
| 495 | + | |
| 496 | + /** | |
| 497 | + * Setting keys this manager stores that its defaults do not name. | |
| 498 | + * | |
| 499 | + * get_default_settings() is the natural allow-list, but it is not complete | |
| 500 | + * in every manager: Site Identity declares 16 defaults while the screens | |
| 501 | + * behind it legitimately store 55 keys, and gating on defaults alone would | |
| 502 | + * stop title formats, breadcrumb configuration and business details from | |
| 503 | + * saving at all. A manager whose defaults are complete overrides nothing. | |
| 504 | + * | |
| 505 | + * @since 2.0.1 | |
| 506 | + * | |
| 507 | + * @return string[] | |
| 508 | + */ | |
| 509 | + protected function additional_setting_keys(): array { | |
| 510 | + return []; | |
| 511 | + } | |
| 512 | + | |
| 513 | + /** | |
| 514 | + * Regular expressions matching key FAMILIES this manager stores. | |
| 515 | + * | |
| 516 | + * For settings whose key set is open by design — the schema manager's | |
| 517 | + * per-entity fields, the sitemap's per-post-type inclusion flags — an | |
| 518 | + * enumerated list would go stale the first time a post type is registered. | |
| 519 | + * Patterns are anchored and deliberately narrow: they must describe a | |
| 520 | + * family the manager owns, never a catch-all. | |
| 521 | + * | |
| 522 | + * @since 2.0.1 | |
| 523 | + * | |
| 524 | + * @return string[] PCRE patterns, delimiters included. | |
| 525 | + */ | |
| 526 | + protected function dynamic_setting_key_patterns(): array { | |
| 527 | + return []; | |
| 528 | + } | |
| 529 | + | |
| 530 | + /** | |
| 531 | + * The setting keys this manager accepts. | |
| 532 | + * | |
| 533 | + * @since 2.0.1 | |
| 534 | + * | |
| 535 | + * @param string $context_type Context the save is for. | |
| 536 | + * @return string[] | |
| 537 | + */ | |
| 538 | + public function get_known_setting_keys(string $context_type = 'site'): array { | |
| 539 | + $keys = array_merge( | |
| 540 | + array_keys($this->get_default_settings($context_type)), | |
| 541 | + $this->additional_setting_keys() | |
| 542 | + ); | |
| 543 | + | |
| 544 | + $keys = array_values(array_unique(array_filter($keys, 'is_string'))); | |
| 545 | + | |
| 546 | + /** | |
| 547 | + * Filters the keys a settings category accepts. | |
| 548 | + * | |
| 549 | + * Shared with Settings_Management_Endpoint so an add-on registering | |
| 550 | + * settings against an existing category declares them once. | |
| 551 | + * | |
| 552 | + * @since 2.0.1 | |
| 553 | + * | |
| 554 | + * @param string[] $keys Accepted setting keys. | |
| 555 | + * @param string $category Settings category (the manager type). | |
| 556 | + * @param string $context_type Context the save is for. | |
| 557 | + */ | |
| 558 | + return apply_filters('thinkrank_known_setting_keys', $keys, $this->manager_type, $context_type); | |
| 559 | + } | |
| 560 | + | |
| 561 | + /** | |
| 562 | + * Whether this manager stores a setting under this key. | |
| 563 | + * | |
| 564 | + * Public counterpart of is_known_setting_key() for callers outside the | |
| 565 | + * save path — the schema upgrade that clears rows written before the | |
| 566 | + * allow-list existed, and tests. | |
| 567 | + * | |
| 568 | + * @since 2.0.1 | |
| 569 | + * | |
| 570 | + * @param string $key Setting key. | |
| 571 | + * @param string $context_type Context to judge it in. | |
| 572 | + * @return bool | |
| 573 | + */ | |
| 574 | + public function accepts_setting_key(string $key, string $context_type = 'site'): bool { | |
| 575 | + return $this->is_known_setting_key(sanitize_key($key), $this->get_known_setting_keys($context_type)); | |
| 576 | + } | |
| 577 | + | |
| 578 | + /** | |
| 579 | + * Whether a key is one this manager stores. | |
| 580 | + * | |
| 581 | + * @since 2.0.1 | |
| 582 | + * | |
| 583 | + * @param string $key Sanitized setting key. | |
| 584 | + * @param array $known Known keys for the context. | |
| 585 | + * @return bool | |
| 586 | + */ | |
| 587 | + protected function is_known_setting_key(string $key, array $known): bool { | |
| 588 | + if (in_array($key, $known, true)) { | |
| 589 | + return true; | |
| 590 | + } | |
| 591 | + | |
| 592 | + foreach ($this->dynamic_setting_key_patterns() as $pattern) { | |
| 593 | + if (preg_match($pattern, $key)) { | |
| 594 | + return true; | |
| 595 | + } | |
| 596 | + } | |
| 597 | + | |
| 598 | + return false; | |
| 599 | + } | |
| 600 | + | |
| 601 | + /** | |
| 322 | 602 | * Sanitize settings array |
| 323 | 603 | * |
| 324 | 604 | * @since 1.0.0 |
| 325 | 605 | * |
| @@ -325,16 +605,38 @@ | ||
| 325 | 605 | * |
| 326 | 606 | * @param array $settings Settings to sanitize |
| 327 | 607 | * @return array Sanitized settings |
| 328 | 608 | */ |
| 329 | - protected function sanitize_settings(array $settings): array { | |
| 609 | + protected function sanitize_settings(array $settings, string $context_type = 'site'): array { | |
| 330 | 610 | $sanitized = []; |
| 611 | + $known = $this->get_known_setting_keys($context_type); | |
| 331 | 612 | |
| 332 | 613 | foreach ($settings as $key => $value) { |
| 333 | 614 | $sanitized_key = sanitize_key($key); |
| 334 | 615 | |
| 616 | + if (in_array($sanitized_key, self::RESERVED_ENVELOPE_KEYS, true)) { | |
| 617 | + continue; | |
| 618 | + } | |
| 619 | + | |
| 620 | + // A key no manager declares is not a setting. Stored, it becomes a | |
| 621 | + // row that get_settings() returns forever, so it round-trips into | |
| 622 | + // every later response and is re-posted by the UI on the next save | |
| 623 | + // — which is how the REST envelope came to be stored (#452). | |
| 624 | + if (!$this->is_known_setting_key($sanitized_key, $known)) { | |
| 625 | + $this->log_unknown_setting_key($sanitized_key); | |
| 626 | + continue; | |
| 627 | + } | |
| 628 | + | |
| 335 | 629 | if (is_string($value)) { |
| 336 | - $sanitized[$sanitized_key] = sanitize_text_field($value); | |
| 630 | + // Multi-line fields must keep their newlines; sanitize_text_field | |
| 631 | + // would flatten them onto a single line. | |
| 632 | + if (in_array($sanitized_key, self::MULTILINE_STRING_KEYS, true)) { | |
| 633 | + $sanitized[$sanitized_key] = sanitize_textarea_field($value); | |
| 634 | + } elseif (in_array($sanitized_key, self::TEMPLATE_STRING_KEYS, true)) { | |
| 635 | + $sanitized[$sanitized_key] = $this->sanitize_template_field($value); | |
| 636 | + } else { | |
| 637 | + $sanitized[$sanitized_key] = sanitize_text_field($value); | |
| 638 | + } | |
| 337 | 639 | } elseif (is_array($value)) { |
| 338 | 640 | $sanitized[$sanitized_key] = $this->sanitize_array_recursive($value); |
| 339 | 641 | } elseif (is_numeric($value)) { |
| 340 | 642 | $sanitized[$sanitized_key] = (float) $value; |
| @@ -348,19 +650,73 @@ | ||
| 348 | 650 | return $sanitized; |
| 349 | 651 | } |
| 350 | 652 | |
| 351 | 653 | /** |
| 654 | + * Record a rejected setting key. | |
| 655 | + * | |
| 656 | + * Dropping silently is the hazard this gate carries: a legitimate key | |
| 657 | + * missing from a manager's declarations would disappear with no trace. On | |
| 658 | + * a debug install it says so; in production it stays quiet, since the | |
| 659 | + * common source is a client posting fields that were never settings. | |
| 660 | + * | |
| 661 | + * @since 2.0.1 | |
| 662 | + * | |
| 663 | + * @param string $key Key that was dropped. | |
| 664 | + * @return void | |
| 665 | + */ | |
| 666 | + private function log_unknown_setting_key(string $key): void { | |
| 667 | + if (!defined('WP_DEBUG') || !WP_DEBUG) { | |
| 668 | + return; | |
| 669 | + } | |
| 670 | + | |
| 671 | + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- debug-only diagnostic; a dropped key is otherwise invisible. | |
| 672 | + error_log(sprintf('ThinkRank [%s]: dropped unknown setting key "%s"', $this->manager_type, $key)); | |
| 673 | + } | |
| 674 | + | |
| 675 | + /** | |
| 676 | + * Sanitize a %token% template while keeping its tokens intact. | |
| 677 | + * | |
| 678 | + * Applies the same protections as sanitize_text_field() — tag stripping, | |
| 679 | + * invalid-UTF8 rejection, control-character and newline removal — but | |
| 680 | + * deliberately omits its percent-encoding strip, which corrupts tokens like | |
| 681 | + * %category_title% and %date%. Templates are only ever rendered into | |
| 682 | + * escaped output, so no percent sequence here reaches a URL context raw. | |
| 683 | + * | |
| 684 | + * @since 1.20.1 | |
| 685 | + * | |
| 686 | + * @param string $value Raw template | |
| 687 | + * @return string Sanitized template | |
| 688 | + */ | |
| 689 | + private function sanitize_template_field(string $value): string { | |
| 690 | + $filtered = wp_check_invalid_utf8($value); | |
| 691 | + | |
| 692 | + if (strpos($filtered, '<') !== false) { | |
| 693 | + $filtered = wp_pre_kses_less_than($filtered); | |
| 694 | + // Wrap in a paragraph so wp_strip_all_tags() sees a complete node. | |
| 695 | + $filtered = wp_strip_all_tags($filtered, false); | |
| 696 | + $filtered = str_replace("<\n", "<\n", $filtered); | |
| 697 | + } | |
| 698 | + | |
| 699 | + // Collapse newlines/tabs to spaces and drop other control characters, | |
| 700 | + // mirroring sanitize_text_field()'s single-line guarantee. | |
| 701 | + $filtered = preg_replace('/[\r\n\t ]+/', ' ', $filtered); | |
| 702 | + $filtered = preg_replace('/[\x00-\x1F\x7F]/u', '', (string) $filtered); | |
| 703 | + | |
| 704 | + return trim((string) $filtered); | |
| 705 | + } | |
| 706 | + | |
| 707 | + /** | |
| 352 | 708 | * Recursively sanitize array values |
| 353 | 709 | * |
| 354 | 710 | * @since 1.0.0 |
| 355 | 711 | * |
| 356 | - * @param array $array Array to sanitize | |
| 712 | + * @param array $input Array to sanitize | |
| 357 | 713 | * @return array Sanitized array |
| 358 | 714 | */ |
| 359 | - private function sanitize_array_recursive(array $array): array { | |
| 715 | + private function sanitize_array_recursive(array $input): array { | |
| 360 | 716 | $sanitized = []; |
| 361 | 717 | |
| 362 | - foreach ($array as $key => $value) { | |
| 718 | + foreach ($input as $key => $value) { | |
| 363 | 719 | $sanitized_key = sanitize_key($key); |
| 364 | 720 | |
| 365 | 721 | if (is_string($value)) { |
| 366 | 722 | $sanitized[$sanitized_key] = sanitize_text_field($value); |
| @@ -395,9 +751,13 @@ | ||
| 395 | 751 | */ |
| 396 | 752 | protected function clear_cache(string $context_type, ?int $context_id): void { |
| 397 | 753 | $cache_key = $this->get_cache_key($context_type, $context_id); |
| 398 | 754 | wp_cache_delete($cache_key, 'thinkrank_seo'); |
| 399 | - | |
| 755 | + | |
| 756 | + // The defaults-free view is cached separately, so a save has to drop it | |
| 757 | + // too or get_stored_settings() keeps answering with the pre-save rows. | |
| 758 | + wp_cache_delete($this->get_stored_cache_key($context_type, $context_id), 'thinkrank_seo'); | |
| 759 | + | |
| 400 | 760 | // Clear related transients |
| 401 | 761 | delete_transient("thinkrank_seo_{$this->manager_type}_{$context_type}_{$context_id}"); |
| 402 | 762 | } |
| 403 | 763 | |
| @@ -409,13 +769,68 @@ | ||
| 409 | 769 | * @param string $context_type The context type |
| 410 | 770 | * @param int|null $context_id Optional. Context ID |
| 411 | 771 | * @return string Cache key |
| 412 | 772 | */ |
| 773 | + /** | |
| 774 | + * Setting keys stored as booleans, so a read hands them back as booleans. | |
| 775 | + * | |
| 776 | + * The database stores them as '1' / '', and a manager whose validator | |
| 777 | + * demands a real boolean will then reject its own stored values — which is | |
| 778 | + * exactly what made every save routed through | |
| 779 | + * Seo_Settings_Manager::save_settings_by_category() fail after it merged | |
| 780 | + * the existing settings back in (#395). Subclasses override this so the | |
| 781 | + * read and the validator cannot drift apart. | |
| 782 | + * | |
| 783 | + * @since 2.0.1 | |
| 784 | + * | |
| 785 | + * @return string[] Keys to coerce to boolean on read. | |
| 786 | + */ | |
| 787 | + protected function boolean_setting_keys(): array { | |
| 788 | + return [ | |
| 789 | + 'enabled', | |
| 790 | + 'auto_generate_schema', | |
| 791 | + 'rich_snippets_optimization', | |
| 792 | + 'performance_tracking', | |
| 793 | + 'auto_deploy', | |
| 794 | + 'validation_on_save', | |
| 795 | + 'rich_snippets_testing', | |
| 796 | + 'organization_schema', | |
| 797 | + 'knowledge_graph', | |
| 798 | + 'add_missing_alt', | |
| 799 | + 'add_missing_title', | |
| 800 | + 'save_alt_to_media', | |
| 801 | + 'auto_fill_on_upload', | |
| 802 | + 'media_alt_overwrite', | |
| 803 | + ]; | |
| 804 | + } | |
| 805 | + | |
| 413 | 806 | protected function get_cache_key(string $context_type, ?int $context_id): string { |
| 414 | - return "seo_settings_{$this->manager_type}_{$context_type}_" . ($context_id ?? 'site'); | |
| 807 | + // Normalise NULL to 0 so reads (which pass NULL for site-wide) and writes | |
| 808 | + // (which pass 0) resolve to the SAME cache entry — otherwise a save would | |
| 809 | + // never invalidate the value a front-end read cached. | |
| 810 | + $db_context_id = $context_id === null ? 0 : $context_id; | |
| 811 | + return "seo_settings_{$this->manager_type}_{$context_type}_{$db_context_id}"; | |
| 415 | 812 | } |
| 416 | 813 | |
| 417 | 814 | /** |
| 815 | + * Cache key for the defaults-free view of a context. | |
| 816 | + * | |
| 817 | + * Deliberately distinct from get_cache_key(): the two views hold different | |
| 818 | + * data (one merged with defaults, one only what was saved), so sharing an | |
| 819 | + * entry would let whichever ran first answer for the other. | |
| 820 | + * | |
| 821 | + * @since 2.3.1 | |
| 822 | + * | |
| 823 | + * @param string $context_type The context type | |
| 824 | + * @param int|null $context_id Optional. Context ID | |
| 825 | + * @return string | |
| 826 | + */ | |
| 827 | + protected function get_stored_cache_key(string $context_type, ?int $context_id): string { | |
| 828 | + $db_context_id = $context_id === null ? 0 : $context_id; | |
| 829 | + return "seo_stored_settings_{$this->manager_type}_{$context_type}_{$db_context_id}"; | |
| 830 | + } | |
| 831 | + | |
| 832 | + /** | |
| 418 | 833 | * Ensure settings table exists |
| 419 | 834 | * |
| 420 | 835 | * @since 1.0.0 |
| 421 | 836 | * |
| @@ -420,16 +835,68 @@ | ||
| 420 | 835 | * @since 1.0.0 |
| 421 | 836 | * |
| 422 | 837 | * @return bool True if table exists or was created successfully |
| 423 | 838 | */ |
| 839 | + /** | |
| 840 | + * Record why a save failed, so "Failed to update … settings" in the UI has | |
| 841 | + * a matching, actionable line in the PHP error log. | |
| 842 | + * | |
| 843 | + * A customer cannot act on the generic message, and neither can support | |
| 844 | + * without this — the missing-table case (wizard blocked after migration, | |
| 845 | + * every settings screen failing) looked identical to a validation problem. | |
| 846 | + * | |
| 847 | + * The reason is also kept on the instance so the REST layer can put it in | |
| 848 | + * the response instead of a fixed string — see get_last_save_error(). | |
| 849 | + * | |
| 850 | + * @since 1.28.0 | |
| 851 | + * | |
| 852 | + * @param string $reason Why the save failed. | |
| 853 | + * @param string $code Optional. Machine-readable failure code. | |
| 854 | + * @return void | |
| 855 | + */ | |
| 856 | + protected function log_save_failure(string $reason, string $code = 'save_failed'): void { | |
| 857 | + if ('' === $this->last_save_error) { | |
| 858 | + $this->last_save_error = $reason; | |
| 859 | + $this->last_save_error_code = $code; | |
| 860 | + } | |
| 861 | + | |
| 862 | + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- deliberate diagnostic; the UI only shows a generic failure message. | |
| 863 | + error_log(sprintf('ThinkRank [%s]: settings save failed — %s', $this->manager_type, $reason)); | |
| 864 | + } | |
| 865 | + | |
| 866 | + /** | |
| 867 | + * Why the last save_settings() call returned false. | |
| 868 | + * | |
| 869 | + * @since 1.32.1 | |
| 870 | + * | |
| 871 | + * @return string Failure reason, or '' if the last save succeeded. | |
| 872 | + */ | |
| 873 | + public function get_last_save_error(): string { | |
| 874 | + return $this->last_save_error; | |
| 875 | + } | |
| 876 | + | |
| 877 | + /** | |
| 878 | + * Machine-readable code for the last save failure. | |
| 879 | + * | |
| 880 | + * One of: unsupported_context, settings_table_missing, validation_failed, | |
| 881 | + * db_insert_failed, save_failed. | |
| 882 | + * | |
| 883 | + * @since 1.32.1 | |
| 884 | + * | |
| 885 | + * @return string Failure code, or '' if the last save succeeded. | |
| 886 | + */ | |
| 887 | + public function get_last_save_error_code(): string { | |
| 888 | + return $this->last_save_error_code; | |
| 889 | + } | |
| 890 | + | |
| 424 | 891 | protected function ensure_settings_table_exists(): bool { |
| 425 | 892 | // Check if table exists |
| 426 | - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching -- Table existence check requires direct database access | |
| 893 | + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching, PluginCheck.Security.DirectDB.UnescapedDBParameter -- Table existence check requires direct database access | |
| 427 | 894 | $table_exists = $this->wpdb->get_var( |
| 428 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 895 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SQL is properly prepared with placeholders | |
| 429 | 896 | $this->wpdb->prepare( |
| 430 | 897 | "SHOW TABLES LIKE %s", |
| 431 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- settings_table is validated class property | |
| 898 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- settings_table is validated class property | |
| 432 | 899 | $this->settings_table |
| 433 | 900 | ) |
| 434 | 901 | ); |
| 435 | 902 | |
| @@ -546,23 +1013,24 @@ | ||
| 546 | 1013 | $settings = $this->get_settings($context_type, $context_id); |
| 547 | 1014 | $export_data['settings'][$context_type . ':' . ($context_id ?? 'site')] = $settings; |
| 548 | 1015 | } else { |
| 549 | 1016 | // Export all settings for this manager type |
| 550 | - // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared -- SEO settings export requires direct database access, table name is validated | |
| 551 | - $sql = sprintf( | |
| 552 | - 'SELECT context_type, context_id, setting_key, setting_value FROM `%s` WHERE setting_category = %%s AND is_active = 1', | |
| 553 | - $this->settings_table | |
| 554 | - ); | |
| 555 | - $results = $this->wpdb->get_results( | |
| 556 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 557 | - $this->wpdb->prepare( | |
| 558 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- SQL is properly prepared with placeholders | |
| 559 | - $sql, | |
| 560 | - // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared -- manager_type is validated class property | |
| 561 | - $this->manager_type | |
| 562 | - ), | |
| 563 | - ARRAY_A | |
| 564 | - ); | |
| 1017 | + // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching,WordPress.DB.PreparedSQL.InterpolatedNotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SEO settings export requires direct database access, table name is validated | |
| 1018 | + $sql = sprintf( | |
| 1019 | + 'SELECT context_type, context_id, setting_key, setting_value FROM `%s` WHERE setting_category = %%s AND is_active = 1', | |
| 1020 | + $this->settings_table | |
| 1021 | + ); | |
| 1022 | + // phpcs:ignore PluginCheck.Security.DirectDB.UnescapedDBParameter -- $sql is built from sprintf with validated table name then prepared below. | |
| 1023 | + $results = $this->wpdb->get_results( | |
| 1024 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SQL is properly prepared with placeholders | |
| 1025 | + $this->wpdb->prepare( | |
| 1026 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- SQL is properly prepared with placeholders | |
| 1027 | + $sql, | |
| 1028 | + // phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared, PluginCheck.Security.DirectDB.UnescapedDBParameter -- manager_type is validated class property | |
| 1029 | + $this->manager_type | |
| 1030 | + ), | |
| 1031 | + ARRAY_A | |
| 1032 | + ); | |
| 565 | 1033 | |
| 566 | 1034 | $grouped_settings = []; |
| 567 | 1035 | foreach ($results as $row) { |
| 568 | 1036 | $key = $row['context_type'] . ':' . ($row['context_id'] ?? 'site'); |
