← All changes | includes/core/class-settings-manager.php +131 -19 1.25.0 → 2.10.0 View file →

		@@ -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'
		@@ -229,17 +255,19 @@
229	255	// Core settings
230	256	'seo_analytics_enabled',
231	257	'seo_analytics_setup_completed',
232	258
233		- // Google Analytics configuration
	259	+ // Google Analytics configuration. NOTE: the account/property/
	260	+ // data-stream picker that reads AND writes these three keys is
	261	+ // thinkrank-pro's GoogleAnalyticsSettings.js (via this plugin's
	262	+ // settings-management endpoint) — a free-repo grep will find no
	263	+ // consumer. ga_analytics_data_stream_id was once removed as a
	264	+ // "dead key" on that basis, which silently broke the Pro
	265	+ // picker's stream selection persisting across reloads.
234	266	'seo_analytics_google_analytics_property_id',
235	267	'ga_analytics_account_id',
236	268	'ga_analytics_data_stream_id',
237	269
238		- // GA4 Tracking Code Injection (Pro)
239		- 'ga4_auto_inject',
240		- 'ga4_measurement_id',
241		-
242	270	// Search Console configuration
243	271	'search_console_property',
244	272
245	273	// AI features
		@@ -302,17 +330,25 @@
302	330	* @param array $settings Settings to update
303	331	* @param string $category Settings category
304	332	* @param string $context_type Optional. Context type for SEO settings
305	333	* @param int|null $context_id Optional. Context ID for SEO settings
306		- * @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).
307	336	*/
308		- 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.
309	341	if (!isset($this->settings_categories[$category])) {
310		- return false;
	342	+ return null;
311	343	}
312	344
313	345	$category_config = $this->settings_categories[$category];
314	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	+
315	351	if ($category_config['manager'] === 'core') {
316	352	return $this->update_core_settings_by_category($settings, $category);
317	353	} else {
318	354	return $this->update_seo_settings_by_category($settings, $category, $context_type, $context_id);
		@@ -319,8 +355,23 @@
319	355	}
320	356	}
321	357
322	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	+ /**
323	374	* Get all settings across categories
324	375	*
325	376	* @since 1.0.0
326	377	*
		@@ -408,8 +459,23 @@
408	459	* @since 1.0.0
409	460	*
410	461	* @return array Categories information
411	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	+
412	478	public function get_categories(): array {
413	479	$categories = [];
414	480
415	481	foreach ($this->settings_categories as $key => $config) {
		@@ -583,8 +649,16 @@
583	649	* @return array Core settings for category
584	650	*/
585	651	private function get_core_settings_by_category(string $category): array {
586	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	+
587	661	$settings = [];
588	662
589	663	foreach ($category_config['keys'] as $key) {
590	664	$settings[$key] = $this->core_settings->get($key);
		@@ -603,24 +677,41 @@
603	677	* @return bool Success status
604	678	*/
605	679	private function update_core_settings_by_category(array $settings, string $category): bool {
606	680	$category_config = $this->settings_categories[$category];
607		- $success_count = 0;
608	681	$total_count = 0;
609	682
	683	+ $this->last_failed_keys = [];
	684	+
	685	+ // Sanitize per field before persisting. This is unconditional: callers
	686	+ // (including the REST write routes, where the client can ask to skip
	687	+ // validation) must not be able to reach Settings::set with unsanitized
	688	+ // values — Settings::set only key-allowlists, it does not sanitize.
	689	+ $settings = $this->core_settings->sanitize_settings($settings);
	690	+
610	691	foreach ($settings as $key => $value) {
611	692	if (in_array($key, $category_config['keys'], true)) {
612	693	$total_count++;
613	694
614		- if ($this->core_settings->set($key, $value)) {
615		- $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));
616	703	}
617	704	}
618	705	}
619	706
620		- // Consider successful if at least 70% of settings were saved
621		- $success_rate = $total_count > 0 ? ($success_count / $total_count) : 0;
622		- $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);
623	714
624	715	if ($success) {
625	716	update_option('thinkrank_settings_last_updated', current_time('mysql'));
626	717	}
		@@ -651,11 +742,12 @@
651	742	* @param array $settings Settings to update
652	743	* @param string $category Category name
653	744	* @param string $context_type Context type
654	745	* @param int|null $context_id Context ID
655		- * @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.
656	748	*/
657		- 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 {
658	750	return $this->seo_settings->save_settings_by_category($context_type, $context_id, $settings, $category);
659	751	}
660	752
661	753	/**
		@@ -722,8 +814,10 @@
722	814	switch ($key) {
723	815	case 'openai_api_key':
724	816	case 'claude_api_key':
725	817	case 'openrouter_api_key':
	818	+ case 'openai_compatible_api_key':
	819	+ case 'openai_compatible_model':
726	820	if (!empty($value) && !is_string($value)) {
727	821	$validation['valid'] = false;
728	822	$validation['errors'][] = "{$key} must be a string";
729	823	}
		@@ -728,15 +822,29 @@
728	822	$validation['errors'][] = "{$key} must be a string";
729	823	}
730	824	break;
731	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	+
732	838	case 'max_tokens':
733	839	case 'cache_duration':
734	840	case 'max_requests_per_minute':
	841	+ case 'ai_daily_request_limit':
735	842	case 'seo_score_threshold':
736	843	case 'api_timeout':
737	844	case 'retry_attempts':
738	845	case 'data_retention_days':
	846	+ case 'openai_compatible_timeout':
739	847	if (!is_numeric($value) || $value < 0) {
740	848	$validation['valid'] = false;
741	849	$validation['errors'][] = "{$key} must be a positive number";
742	850	}
		@@ -749,11 +857,15 @@
749	857	}
750	858	break;
751	859
752	860	case 'ai_provider':
753		- 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)) {
754	865	$validation['valid'] = false;
755		- $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);
756	868	}
757	869	break;
758	870
759	871	case 'dashboard_widgets':