| @@ -52,8 +52,20 @@ | ||
| 52 | 52 | */ |
| 53 | 53 | private SEO_Settings_Manager $seo_settings; |
| 54 | 54 | |
| 55 | 55 | /** |
| 56 | + * Keys the most recent core-category save could not persist. | |
| 57 | + * | |
| 58 | + * A batch save is all-or-nothing in its reporting but not in its writes, so | |
| 59 | + * a caller that gets false needs to know *which* settings did not make it — | |
| 60 | + * a bare boolean leaves the UI unable to say anything useful (#300). | |
| 61 | + * | |
| 62 | + * @since 1.30.0 | |
| 63 | + * @var string[] | |
| 64 | + */ | |
| 65 | + private array $last_failed_keys = []; | |
| 66 | + | |
| 67 | + /** | |
| 56 | 68 | * Settings categories mapping |
| 57 | 69 | * |
| 58 | 70 | * @since 1.0.0 |
| 59 | 71 | * @var array |
| @@ -71,17 +83,31 @@ | ||
| 71 | 83 | 'gemini_api_key', |
| 72 | 84 | 'gemini_model', |
| 73 | 85 | 'openrouter_api_key', |
| 74 | 86 | 'openrouter_model', |
| 87 | + 'openai_compatible_base_url', | |
| 88 | + 'openai_compatible_api_key', | |
| 89 | + 'openai_compatible_model', | |
| 90 | + 'openai_compatible_timeout', | |
| 91 | + 'openai_compatible_supports_images', | |
| 92 | + 'openai_compatible_json_mode', | |
| 93 | + 'openai_compatible_price_per_million', | |
| 75 | 94 | 'max_tokens', |
| 76 | 95 | 'temperature', |
| 77 | 96 | 'cache_duration', |
| 78 | 97 | 'max_requests_per_minute', |
| 98 | + 'ai_daily_request_limit', | |
| 99 | + 'ai_paused', | |
| 79 | 100 | 'enable_logging', |
| 80 | 101 | 'debug_mode', |
| 81 | 102 | 'api_timeout', |
| 82 | 103 | 'retry_attempts', |
| 83 | - 'rate_limit_enabled', | |
| 104 | + // 'rate_limit_enabled' used to be listed here, but it has no | |
| 105 | + // entry in Settings::defaults, so Settings::set() rejected it on | |
| 106 | + // every save and no code ever read it. Under the old 70% | |
| 107 | + // threshold that silent rejection was reported as success; | |
| 108 | + // all-or-nothing reporting would now fail every core save that | |
| 109 | + // carried it, so the dead key goes rather than the save (#300). | |
| 84 | 110 | 'data_retention_days', |
| 85 | 111 | 'anonymize_logs', |
| 86 | 112 | 'share_usage_data', |
| 87 | 113 | 'keep_data_on_uninstall' |
| @@ -240,12 +266,8 @@ | ||
| 240 | 266 | 'seo_analytics_google_analytics_property_id', |
| 241 | 267 | 'ga_analytics_account_id', |
| 242 | 268 | 'ga_analytics_data_stream_id', |
| 243 | 269 | |
| 244 | - // GA4 Tracking Code Injection (Pro) | |
| 245 | - 'ga4_auto_inject', | |
| 246 | - 'ga4_measurement_id', | |
| 247 | - | |
| 248 | 270 | // Search Console configuration |
| 249 | 271 | 'search_console_property', |
| 250 | 272 | |
| 251 | 273 | // AI features |
| @@ -308,17 +330,25 @@ | ||
| 308 | 330 | * @param array $settings Settings to update |
| 309 | 331 | * @param string $category Settings category |
| 310 | 332 | * @param string $context_type Optional. Context type for SEO settings |
| 311 | 333 | * @param int|null $context_id Optional. Context ID for SEO settings |
| 312 | - * @return bool Success status | |
| 334 | + * @return bool|null True on success, false on failure, null when this store | |
| 335 | + * does not own the category (nothing was attempted). | |
| 313 | 336 | */ |
| 314 | - public function update_settings(array $settings, string $category, string $context_type = 'site', ?int $context_id = null): bool { | |
| 337 | + public function update_settings(array $settings, string $category, string $context_type = 'site', ?int $context_id = null): ?bool { | |
| 338 | + // Unknown here means "not this store's category", not "the write | |
| 339 | + // failed" — the caller may still have a dedicated manager that owns it | |
| 340 | + // (#371). Failing closed made those saves report 500 after committing. | |
| 315 | 341 | if (!isset($this->settings_categories[$category])) { |
| 316 | - return false; | |
| 342 | + return null; | |
| 317 | 343 | } |
| 318 | 344 | |
| 319 | 345 | $category_config = $this->settings_categories[$category]; |
| 320 | 346 | |
| 347 | + // Reset here, not only in the core path: a SEO-category save must not | |
| 348 | + // leave a previous core save's failed keys readable. | |
| 349 | + $this->last_failed_keys = []; | |
| 350 | + | |
| 321 | 351 | if ($category_config['manager'] === 'core') { |
| 322 | 352 | return $this->update_core_settings_by_category($settings, $category); |
| 323 | 353 | } else { |
| 324 | 354 | return $this->update_seo_settings_by_category($settings, $category, $context_type, $context_id); |
| @@ -325,8 +355,23 @@ | ||
| 325 | 355 | } |
| 326 | 356 | } |
| 327 | 357 | |
| 328 | 358 | /** |
| 359 | + * Keys the most recent update_settings() call could not persist. | |
| 360 | + * | |
| 361 | + * Empty on success, and reset at the start of every update_settings() | |
| 362 | + * call. SEO categories persist through their own manager and do not | |
| 363 | + * report per key, so this stays empty for them. | |
| 364 | + * | |
| 365 | + * @since 1.30.0 | |
| 366 | + * | |
| 367 | + * @return string[] Setting keys that failed to save. | |
| 368 | + */ | |
| 369 | + public function get_last_failed_keys(): array { | |
| 370 | + return $this->last_failed_keys; | |
| 371 | + } | |
| 372 | + | |
| 373 | + /** | |
| 329 | 374 | * Get all settings across categories |
| 330 | 375 | * |
| 331 | 376 | * @since 1.0.0 |
| 332 | 377 | * |
| @@ -414,8 +459,23 @@ | ||
| 414 | 459 | * @since 1.0.0 |
| 415 | 460 | * |
| 416 | 461 | * @return array Categories information |
| 417 | 462 | */ |
| 463 | + /** | |
| 464 | + * The setting keys a category defines. | |
| 465 | + * | |
| 466 | + * Exposed so callers can reject keys a category does not define instead of | |
| 467 | + * persisting whatever they are handed (#395). | |
| 468 | + * | |
| 469 | + * @since 2.0.1 | |
| 470 | + * | |
| 471 | + * @param string $category Category name. | |
| 472 | + * @return string[] Setting keys, or [] when the category is unknown here. | |
| 473 | + */ | |
| 474 | + public function get_category_keys(string $category): array { | |
| 475 | + return $this->settings_categories[$category]['keys'] ?? []; | |
| 476 | + } | |
| 477 | + | |
| 418 | 478 | public function get_categories(): array { |
| 419 | 479 | $categories = []; |
| 420 | 480 | |
| 421 | 481 | foreach ($this->settings_categories as $key => $config) { |
| @@ -589,8 +649,16 @@ | ||
| 589 | 649 | * @return array Core settings for category |
| 590 | 650 | */ |
| 591 | 651 | private function get_core_settings_by_category(string $category): array { |
| 592 | 652 | $category_config = $this->settings_categories[$category]; |
| 653 | + | |
| 654 | + // Prime the option cache in one query before the loop. Every | |
| 655 | + // thinkrank_* option is autoload=off, so WordPress cannot serve them | |
| 656 | + // from `alloptions` and each Settings->get() below was its own | |
| 657 | + // round-trip — 16 of them on every anonymous front-end request, on | |
| 658 | + // pages that use none of the values (#393). | |
| 659 | + $this->core_settings->prime($category_config['keys']); | |
| 660 | + | |
| 593 | 661 | $settings = []; |
| 594 | 662 | |
| 595 | 663 | foreach ($category_config['keys'] as $key) { |
| 596 | 664 | $settings[$key] = $this->core_settings->get($key); |
| @@ -609,11 +677,12 @@ | ||
| 609 | 677 | * @return bool Success status |
| 610 | 678 | */ |
| 611 | 679 | private function update_core_settings_by_category(array $settings, string $category): bool { |
| 612 | 680 | $category_config = $this->settings_categories[$category]; |
| 613 | - $success_count = 0; | |
| 614 | 681 | $total_count = 0; |
| 615 | 682 | |
| 683 | + $this->last_failed_keys = []; | |
| 684 | + | |
| 616 | 685 | // Sanitize per field before persisting. This is unconditional: callers |
| 617 | 686 | // (including the REST write routes, where the client can ask to skip |
| 618 | 687 | // validation) must not be able to reach Settings::set with unsanitized |
| 619 | 688 | // values — Settings::set only key-allowlists, it does not sanitize. |
| @@ -622,17 +691,27 @@ | ||
| 622 | 691 | foreach ($settings as $key => $value) { |
| 623 | 692 | if (in_array($key, $category_config['keys'], true)) { |
| 624 | 693 | $total_count++; |
| 625 | 694 | |
| 626 | - if ($this->core_settings->set($key, $value)) { | |
| 627 | - $success_count++; | |
| 695 | + if (!$this->core_settings->set($key, $value)) { | |
| 696 | + $this->last_failed_keys[] = $key; | |
| 697 | + | |
| 698 | + // Name the key in the log: the UI can only ever show one | |
| 699 | + // message for the batch, so without this a single dropped | |
| 700 | + // setting is indistinguishable from a healthy save. | |
| 701 | + // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- deliberate diagnostic, see above. | |
| 702 | + error_log(sprintf('ThinkRank [%s]: settings save failed — key \'%s\' was not stored', $category, $key)); | |
| 628 | 703 | } |
| 629 | 704 | } |
| 630 | 705 | } |
| 631 | 706 | |
| 632 | - // Consider successful if at least 70% of settings were saved | |
| 633 | - $success_rate = $total_count > 0 ? ($success_count / $total_count) : 0; | |
| 634 | - $success = $success_rate >= 0.7; | |
| 707 | + // Every requested key must persist. A partial save used to pass on a 70% | |
| 708 | + // threshold, so a batch could silently drop up to a third of the user's | |
| 709 | + // settings while the UI reported success and the values were simply gone | |
| 710 | + // (#300). Note the write is not transactional: the keys that did save | |
| 711 | + // stay saved, which is why the failed keys are reported rather than just | |
| 712 | + // a bare false. | |
| 713 | + $success = $total_count > 0 && empty($this->last_failed_keys); | |
| 635 | 714 | |
| 636 | 715 | if ($success) { |
| 637 | 716 | update_option('thinkrank_settings_last_updated', current_time('mysql')); |
| 638 | 717 | } |
| @@ -663,11 +742,12 @@ | ||
| 663 | 742 | * @param array $settings Settings to update |
| 664 | 743 | * @param string $category Category name |
| 665 | 744 | * @param string $context_type Context type |
| 666 | 745 | * @param int|null $context_id Context ID |
| 667 | - * @return bool Success status | |
| 746 | + * @return bool|null True on success, false on failure, null when the SEO | |
| 747 | + * store does not own the category. | |
| 668 | 748 | */ |
| 669 | - private function update_seo_settings_by_category(array $settings, string $category, string $context_type, ?int $context_id): bool { | |
| 749 | + private function update_seo_settings_by_category(array $settings, string $category, string $context_type, ?int $context_id): ?bool { | |
| 670 | 750 | return $this->seo_settings->save_settings_by_category($context_type, $context_id, $settings, $category); |
| 671 | 751 | } |
| 672 | 752 | |
| 673 | 753 | /** |
| @@ -734,8 +814,10 @@ | ||
| 734 | 814 | switch ($key) { |
| 735 | 815 | case 'openai_api_key': |
| 736 | 816 | case 'claude_api_key': |
| 737 | 817 | case 'openrouter_api_key': |
| 818 | + case 'openai_compatible_api_key': | |
| 819 | + case 'openai_compatible_model': | |
| 738 | 820 | if (!empty($value) && !is_string($value)) { |
| 739 | 821 | $validation['valid'] = false; |
| 740 | 822 | $validation['errors'][] = "{$key} must be a string"; |
| 741 | 823 | } |
| @@ -740,15 +822,29 @@ | ||
| 740 | 822 | $validation['errors'][] = "{$key} must be a string"; |
| 741 | 823 | } |
| 742 | 824 | break; |
| 743 | 825 | |
| 826 | + case 'openai_compatible_base_url': | |
| 827 | + // An unreachable or dangerous URL is refused with a reason | |
| 828 | + // rather than quietly stored (see Endpoint_URL_Validator). | |
| 829 | + if (!empty($value)) { | |
| 830 | + $validated = \ThinkRank\AI\Endpoint_URL_Validator::validate((string) $value); | |
| 831 | + if (is_wp_error($validated)) { | |
| 832 | + $validation['valid'] = false; | |
| 833 | + $validation['errors'][] = $validated->get_error_message(); | |
| 834 | + } | |
| 835 | + } | |
| 836 | + break; | |
| 837 | + | |
| 744 | 838 | case 'max_tokens': |
| 745 | 839 | case 'cache_duration': |
| 746 | 840 | case 'max_requests_per_minute': |
| 841 | + case 'ai_daily_request_limit': | |
| 747 | 842 | case 'seo_score_threshold': |
| 748 | 843 | case 'api_timeout': |
| 749 | 844 | case 'retry_attempts': |
| 750 | 845 | case 'data_retention_days': |
| 846 | + case 'openai_compatible_timeout': | |
| 751 | 847 | if (!is_numeric($value) || $value < 0) { |
| 752 | 848 | $validation['valid'] = false; |
| 753 | 849 | $validation['errors'][] = "{$key} must be a positive number"; |
| 754 | 850 | } |
| @@ -761,11 +857,15 @@ | ||
| 761 | 857 | } |
| 762 | 858 | break; |
| 763 | 859 | |
| 764 | 860 | case 'ai_provider': |
| 765 | - if (!in_array($value, ['openai', 'claude', 'gemini', 'openrouter'], true)) { | |
| 861 | + // '' is legal: it is Settings::AI_PROVIDER_NONE, the state a | |
| 862 | + // fresh install starts in and the one a user returns to by | |
| 863 | + // deselecting their provider (#572). | |
| 864 | + if (!in_array($value, \ThinkRank\Core\Settings::selectable_ai_providers(), true)) { | |
| 766 | 865 | $validation['valid'] = false; |
| 767 | - $validation['errors'][] = "ai_provider must be 'openai', 'claude', 'gemini', or 'openrouter'"; | |
| 866 | + $validation['errors'][] = "ai_provider must be empty (no provider) or one of: " | |
| 867 | + . implode(', ', \ThinkRank\Core\Settings::SUPPORTED_AI_PROVIDERS); | |
| 768 | 868 | } |
| 769 | 869 | break; |
| 770 | 870 | |
| 771 | 871 | case 'dashboard_widgets': |