| @@ -57,8 +57,30 @@ | ||
| 57 | 57 | */ |
| 58 | 58 | protected string $manager_type; |
| 59 | 59 | |
| 60 | 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 | + /** | |
| 61 | 83 | * Supported context types |
| 62 | 84 | * |
| 63 | 85 | * @since 1.0.0 |
| 64 | 86 | * @var array |
| @@ -82,8 +104,15 @@ | ||
| 82 | 104 | |
| 83 | 105 | /** |
| 84 | 106 | * Get SEO settings for a specific context |
| 85 | 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 | + * | |
| 86 | 115 | * @since 1.0.0 |
| 87 | 116 | * |
| 88 | 117 | * @param string $context_type The context type |
| 89 | 118 | * @param int|null $context_id Optional. Context ID |
| @@ -95,14 +124,65 @@ | ||
| 95 | 124 | if (!in_array($context_type, $this->get_supported_contexts(), true)) { |
| 96 | 125 | return $this->get_default_settings($context_type); |
| 97 | 126 | } |
| 98 | 127 | |
| 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; | |
| 134 | + } | |
| 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 | + | |
| 99 | 179 | // Convert NULL context_id to 0 for site-wide settings to match save behavior |
| 100 | 180 | $db_context_id = $context_id === null ? 0 : $context_id; |
| 101 | 181 | |
| 102 | - // Serve from the object cache when available. This runs on every front-end | |
| 103 | - // request (the_content, thumbnails), so avoiding a DB hit per request matters. | |
| 104 | - $cache_key = $this->get_cache_key($context_type, $context_id); | |
| 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); | |
| 105 | 185 | $cached = wp_cache_get($cache_key, 'thinkrank_seo'); |
| 106 | 186 | if (is_array($cached)) { |
| 107 | 187 | return $cached; |
| 108 | 188 | } |
| @@ -129,28 +209,13 @@ | ||
| 129 | 209 | ARRAY_A |
| 130 | 210 | ); |
| 131 | 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', | |
| 139 | - 'auto_generate_schema', | |
| 140 | - 'rich_snippets_optimization', | |
| 141 | - 'performance_tracking', | |
| 142 | - 'auto_deploy', | |
| 143 | - 'validation_on_save', | |
| 144 | - 'rich_snippets_testing', | |
| 145 | - 'organization_schema', | |
| 146 | - 'knowledge_graph', | |
| 147 | - 'add_missing_alt', | |
| 148 | - 'add_missing_title', | |
| 149 | - 'save_alt_to_media', | |
| 150 | - 'auto_fill_on_upload', | |
| 151 | - 'media_alt_overwrite' | |
| 152 | - ], true)) { | |
| 216 | + // Ensure proper data type conversion for boolean fields | |
| 217 | + if (in_array($row['setting_key'], $this->boolean_setting_keys(), true)) { | |
| 153 | 218 | // Convert string/numeric boolean representations to actual booleans |
| 154 | 219 | if (is_string($value)) { |
| 155 | 220 | $value = in_array(strtolower($value), ['true', '1', 'yes', 'on'], true); |
| 156 | 221 | } elseif (is_numeric($value)) { |
| @@ -165,15 +230,11 @@ | ||
| 165 | 230 | |
| 166 | 231 | $settings[$row['setting_key']] = $value; |
| 167 | 232 | } |
| 168 | 233 | |
| 169 | - // Merge with defaults to ensure all required keys exist | |
| 170 | - $merged = array_merge($this->get_default_settings($context_type), $settings); | |
| 234 | + wp_cache_set($cache_key, $settings, 'thinkrank_seo'); | |
| 171 | 235 | |
| 172 | - // Cache the resolved settings; invalidated on every save via clear_cache(). | |
| 173 | - wp_cache_set($cache_key, $merged, 'thinkrank_seo'); | |
| 174 | - | |
| 175 | - return $merged; | |
| 236 | + return $settings; | |
| 176 | 237 | } |
| 177 | 238 | |
| 178 | 239 | /** |
| 179 | 240 | * Save SEO settings for a specific context |
| @@ -187,16 +248,39 @@ | ||
| 187 | 248 | */ |
| 188 | 249 | public function save_settings(string $context_type, ?int $context_id, array $settings): bool { |
| 189 | 250 | $context_type = sanitize_key($context_type); |
| 190 | 251 | |
| 252 | + $this->last_save_error = ''; | |
| 253 | + $this->last_save_error_code = ''; | |
| 254 | + | |
| 191 | 255 | if (!in_array($context_type, $this->get_supported_contexts(), true)) { |
| 192 | - // Unsupported context type - validation failed | |
| 256 | + $this->log_save_failure("unsupported context type '{$context_type}'", 'unsupported_context'); | |
| 193 | 257 | return false; |
| 194 | 258 | } |
| 195 | 259 | |
| 196 | - // 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. | |
| 197 | 270 | if (!$this->ensure_settings_table_exists()) { |
| 198 | - // 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 | + ); | |
| 199 | 283 | return false; |
| 200 | 284 | } |
| 201 | 285 | |
| 202 | 286 | // Validate settings before saving |
| @@ -201,14 +285,17 @@ | ||
| 201 | 285 | |
| 202 | 286 | // Validate settings before saving |
| 203 | 287 | $validation = $this->validate_settings($settings); |
| 204 | 288 | if (!$validation['valid']) { |
| 205 | - // 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 | + ); | |
| 206 | 293 | return false; |
| 207 | 294 | } |
| 208 | 295 | |
| 209 | 296 | // Sanitize settings |
| 210 | - $sanitized_settings = $this->sanitize_settings($settings); | |
| 297 | + $sanitized_settings = $this->sanitize_settings($settings, $context_type); | |
| 211 | 298 | |
| 212 | 299 | $success = true; |
| 213 | 300 | foreach ($sanitized_settings as $key => $value) { |
| 214 | 301 | $sanitized_key = sanitize_key($key); |
| @@ -243,9 +330,13 @@ | ||
| 243 | 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 |
| 244 | 331 | $result = $this->wpdb->query($sql); |
| 245 | 332 | |
| 246 | 333 | if (false === $result) { |
| 247 | - // 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 | + ); | |
| 248 | 339 | $success = false; |
| 249 | 340 | } |
| 250 | 341 | } |
| 251 | 342 | |
| @@ -251,8 +342,31 @@ | ||
| 251 | 342 | |
| 252 | 343 | // Clear relevant caches |
| 253 | 344 | $this->clear_cache($context_type, $context_id); |
| 254 | 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 | + | |
| 255 | 369 | return $success; |
| 256 | 370 | } |
| 257 | 371 | |
| 258 | 372 | /** |
| @@ -364,8 +478,128 @@ | ||
| 364 | 478 | 'subject_template', |
| 365 | 479 | ]; |
| 366 | 480 | |
| 367 | 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 | + /** | |
| 368 | 602 | * Sanitize settings array |
| 369 | 603 | * |
| 370 | 604 | * @since 1.0.0 |
| 371 | 605 | * |
| @@ -371,14 +605,28 @@ | ||
| 371 | 605 | * |
| 372 | 606 | * @param array $settings Settings to sanitize |
| 373 | 607 | * @return array Sanitized settings |
| 374 | 608 | */ |
| 375 | - protected function sanitize_settings(array $settings): array { | |
| 609 | + protected function sanitize_settings(array $settings, string $context_type = 'site'): array { | |
| 376 | 610 | $sanitized = []; |
| 611 | + $known = $this->get_known_setting_keys($context_type); | |
| 377 | 612 | |
| 378 | 613 | foreach ($settings as $key => $value) { |
| 379 | 614 | $sanitized_key = sanitize_key($key); |
| 380 | 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 | + | |
| 381 | 629 | if (is_string($value)) { |
| 382 | 630 | // Multi-line fields must keep their newlines; sanitize_text_field |
| 383 | 631 | // would flatten them onto a single line. |
| 384 | 632 | if (in_array($sanitized_key, self::MULTILINE_STRING_KEYS, true)) { |
| @@ -402,8 +650,30 @@ | ||
| 402 | 650 | return $sanitized; |
| 403 | 651 | } |
| 404 | 652 | |
| 405 | 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 | + /** | |
| 406 | 676 | * Sanitize a %token% template while keeping its tokens intact. |
| 407 | 677 | * |
| 408 | 678 | * Applies the same protections as sanitize_text_field() — tag stripping, |
| 409 | 679 | * invalid-UTF8 rejection, control-character and newline removal — but |
| @@ -438,15 +708,15 @@ | ||
| 438 | 708 | * Recursively sanitize array values |
| 439 | 709 | * |
| 440 | 710 | * @since 1.0.0 |
| 441 | 711 | * |
| 442 | - * @param array $array Array to sanitize | |
| 712 | + * @param array $input Array to sanitize | |
| 443 | 713 | * @return array Sanitized array |
| 444 | 714 | */ |
| 445 | - private function sanitize_array_recursive(array $array): array { | |
| 715 | + private function sanitize_array_recursive(array $input): array { | |
| 446 | 716 | $sanitized = []; |
| 447 | 717 | |
| 448 | - foreach ($array as $key => $value) { | |
| 718 | + foreach ($input as $key => $value) { | |
| 449 | 719 | $sanitized_key = sanitize_key($key); |
| 450 | 720 | |
| 451 | 721 | if (is_string($value)) { |
| 452 | 722 | $sanitized[$sanitized_key] = sanitize_text_field($value); |
| @@ -482,8 +752,12 @@ | ||
| 482 | 752 | protected function clear_cache(string $context_type, ?int $context_id): void { |
| 483 | 753 | $cache_key = $this->get_cache_key($context_type, $context_id); |
| 484 | 754 | wp_cache_delete($cache_key, 'thinkrank_seo'); |
| 485 | 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 | + | |
| 486 | 760 | // Clear related transients |
| 487 | 761 | delete_transient("thinkrank_seo_{$this->manager_type}_{$context_type}_{$context_id}"); |
| 488 | 762 | } |
| 489 | 763 | |
| @@ -495,8 +769,41 @@ | ||
| 495 | 769 | * @param string $context_type The context type |
| 496 | 770 | * @param int|null $context_id Optional. Context ID |
| 497 | 771 | * @return string Cache key |
| 498 | 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 | + | |
| 499 | 806 | protected function get_cache_key(string $context_type, ?int $context_id): string { |
| 500 | 807 | // Normalise NULL to 0 so reads (which pass NULL for site-wide) and writes |
| 501 | 808 | // (which pass 0) resolve to the SAME cache entry — otherwise a save would |
| 502 | 809 | // never invalidate the value a front-end read cached. |
| @@ -504,8 +811,26 @@ | ||
| 504 | 811 | return "seo_settings_{$this->manager_type}_{$context_type}_{$db_context_id}"; |
| 505 | 812 | } |
| 506 | 813 | |
| 507 | 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 | + /** | |
| 508 | 833 | * Ensure settings table exists |
| 509 | 834 | * |
| 510 | 835 | * @since 1.0.0 |
| 511 | 836 | * |
| @@ -510,8 +835,60 @@ | ||
| 510 | 835 | * @since 1.0.0 |
| 511 | 836 | * |
| 512 | 837 | * @return bool True if table exists or was created successfully |
| 513 | 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 | + | |
| 514 | 891 | protected function ensure_settings_table_exists(): bool { |
| 515 | 892 | // Check if table exists |
| 516 | 893 | // phpcs:ignore WordPress.DB.DirectDatabaseQuery.DirectQuery,WordPress.DB.DirectDatabaseQuery.NoCaching, PluginCheck.Security.DirectDB.UnescapedDBParameter -- Table existence check requires direct database access |
| 517 | 894 | $table_exists = $this->wpdb->get_var( |
