← All changes | includes/core/class-settings-manager.php +126 -24 1.10.0 → 2.7.0 View file →

		@@ -19,8 +19,13 @@
19	19
20	20	use ThinkRank\Core\Settings;
21	21	use ThinkRank\SEO\SEO_Settings_Manager;
22	22
	23	+// Prevent direct access
	24	+if (!defined('ABSPATH')) {
	25	+ exit;
	26	+}
	27	+
23	28	/**
24	29	* Centralized Settings Manager Class
25	30	*
26	31	* Provides unified settings management interface that coordinates between
		@@ -47,8 +52,20 @@
47	52	*/
48	53	private SEO_Settings_Manager $seo_settings;
49	54
50	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	+ /**
51	68	* Settings categories mapping
52	69	*
53	70	* @since 1.0.0
54	71	* @var array
		@@ -64,8 +81,10 @@
64	81	'claude_api_key',
65	82	'claude_model',
66	83	'gemini_api_key',
67	84	'gemini_model',
	85	+ 'openrouter_api_key',
	86	+ 'openrouter_model',
68	87	'max_tokens',
69	88	'temperature',
70	89	'cache_duration',
71	90	'max_requests_per_minute',
		@@ -72,9 +91,14 @@
72	91	'enable_logging',
73	92	'debug_mode',
74	93	'api_timeout',
75	94	'retry_attempts',
76		- 'rate_limit_enabled',
	95	+ // 'rate_limit_enabled' used to be listed here, but it has no
	96	+ // entry in Settings::defaults, so Settings::set() rejected it on
	97	+ // every save and no code ever read it. Under the old 70%
	98	+ // threshold that silent rejection was reported as success;
	99	+ // all-or-nothing reporting would now fail every core save that
	100	+ // carried it, so the dead key goes rather than the save (#300).
77	101	'data_retention_days',
78	102	'anonymize_logs',
79	103	'share_usage_data',
80	104	'keep_data_on_uninstall'
		@@ -222,17 +246,19 @@
222	246	// Core settings
223	247	'seo_analytics_enabled',
224	248	'seo_analytics_setup_completed',
225	249
226		- // Google Analytics configuration
	250	+ // Google Analytics configuration. NOTE: the account/property/
	251	+ // data-stream picker that reads AND writes these three keys is
	252	+ // thinkrank-pro's GoogleAnalyticsSettings.js (via this plugin's
	253	+ // settings-management endpoint) — a free-repo grep will find no
	254	+ // consumer. ga_analytics_data_stream_id was once removed as a
	255	+ // "dead key" on that basis, which silently broke the Pro
	256	+ // picker's stream selection persisting across reloads.
227	257	'seo_analytics_google_analytics_property_id',
228	258	'ga_analytics_account_id',
229	259	'ga_analytics_data_stream_id',
230	260
231		- // GA4 Tracking Code Injection (Pro)
232		- 'ga4_auto_inject',
233		- 'ga4_measurement_id',
234		-
235	261	// Search Console configuration
236	262	'search_console_property',
237	263
238	264	// AI features
		@@ -258,9 +284,9 @@
258	284	*
259	285	* @since 1.0.0
260	286	*/
261	287	public function __construct() {
262		- $this->core_settings = new Settings();
	288	+ $this->core_settings = Settings::instance();
263	289	$this->seo_settings = new SEO_Settings_Manager();
264	290	}
265	291
266	292	/**
		@@ -295,17 +321,25 @@
295	321	* @param array $settings Settings to update
296	322	* @param string $category Settings category
297	323	* @param string $context_type Optional. Context type for SEO settings
298	324	* @param int|null $context_id Optional. Context ID for SEO settings
299		- * @return bool Success status
	325	+ * @return bool|null True on success, false on failure, null when this store
	326	+ * does not own the category (nothing was attempted).
300	327	*/
301		- public function update_settings(array $settings, string $category, string $context_type = 'site', ?int $context_id = null): bool {
	328	+ public function update_settings(array $settings, string $category, string $context_type = 'site', ?int $context_id = null): ?bool {
	329	+ // Unknown here means "not this store's category", not "the write
	330	+ // failed" — the caller may still have a dedicated manager that owns it
	331	+ // (#371). Failing closed made those saves report 500 after committing.
302	332	if (!isset($this->settings_categories[$category])) {
303		- return false;
	333	+ return null;
304	334	}
305	335
306	336	$category_config = $this->settings_categories[$category];
307	337
	338	+ // Reset here, not only in the core path: a SEO-category save must not
	339	+ // leave a previous core save's failed keys readable.
	340	+ $this->last_failed_keys = [];
	341	+
308	342	if ($category_config['manager'] === 'core') {
309	343	return $this->update_core_settings_by_category($settings, $category);
310	344	} else {
311	345	return $this->update_seo_settings_by_category($settings, $category, $context_type, $context_id);
		@@ -312,8 +346,23 @@
312	346	}
313	347	}
314	348
315	349	/**
	350	+ * Keys the most recent update_settings() call could not persist.
	351	+ *
	352	+ * Empty on success, and reset at the start of every update_settings()
	353	+ * call. SEO categories persist through their own manager and do not
	354	+ * report per key, so this stays empty for them.
	355	+ *
	356	+ * @since 1.30.0
	357	+ *
	358	+ * @return string[] Setting keys that failed to save.
	359	+ */
	360	+ public function get_last_failed_keys(): array {
	361	+ return $this->last_failed_keys;
	362	+ }
	363	+
	364	+ /**
316	365	* Get all settings across categories
317	366	*
318	367	* @since 1.0.0
319	368	*
		@@ -401,8 +450,23 @@
401	450	* @since 1.0.0
402	451	*
403	452	* @return array Categories information
404	453	*/
	454	+ /**
	455	+ * The setting keys a category defines.
	456	+ *
	457	+ * Exposed so callers can reject keys a category does not define instead of
	458	+ * persisting whatever they are handed (#395).
	459	+ *
	460	+ * @since 2.0.1
	461	+ *
	462	+ * @param string $category Category name.
	463	+ * @return string[] Setting keys, or [] when the category is unknown here.
	464	+ */
	465	+ public function get_category_keys(string $category): array {
	466	+ return $this->settings_categories[$category]['keys'] ?? [];
	467	+ }
	468	+
405	469	public function get_categories(): array {
406	470	$categories = [];
407	471
408	472	foreach ($this->settings_categories as $key => $config) {
		@@ -427,9 +491,9 @@
427	491	public function export_settings(array $categories = []): array {
428	492	$export_data = [
429	493	'metadata' => [
430	494	'export_timestamp' => current_time('mysql'),
431		- 'plugin_version' => '1.0.0',
	495	+ 'plugin_version' => defined('THINKRANK_VERSION') ? THINKRANK_VERSION : '1.0.0',
432	496	'wordpress_version' => get_bloginfo('version'),
433	497	'site_url' => home_url(),
434	498	'exported_categories' => empty($categories) ? array_keys($this->settings_categories) : $categories
435	499	],
		@@ -576,8 +640,16 @@
576	640	* @return array Core settings for category
577	641	*/
578	642	private function get_core_settings_by_category(string $category): array {
579	643	$category_config = $this->settings_categories[$category];
	644	+
	645	+ // Prime the option cache in one query before the loop. Every
	646	+ // thinkrank_* option is autoload=off, so WordPress cannot serve them
	647	+ // from `alloptions` and each Settings->get() below was its own
	648	+ // round-trip — 16 of them on every anonymous front-end request, on
	649	+ // pages that use none of the values (#393).
	650	+ $this->core_settings->prime($category_config['keys']);
	651	+
580	652	$settings = [];
581	653
582	654	foreach ($category_config['keys'] as $key) {
583	655	$settings[$key] = $this->core_settings->get($key);
		@@ -596,24 +668,41 @@
596	668	* @return bool Success status
597	669	*/
598	670	private function update_core_settings_by_category(array $settings, string $category): bool {
599	671	$category_config = $this->settings_categories[$category];
600		- $success_count = 0;
601	672	$total_count = 0;
602	673
	674	+ $this->last_failed_keys = [];
	675	+
	676	+ // Sanitize per field before persisting. This is unconditional: callers
	677	+ // (including the REST write routes, where the client can ask to skip
	678	+ // validation) must not be able to reach Settings::set with unsanitized
	679	+ // values — Settings::set only key-allowlists, it does not sanitize.
	680	+ $settings = $this->core_settings->sanitize_settings($settings);
	681	+
603	682	foreach ($settings as $key => $value) {
604	683	if (in_array($key, $category_config['keys'], true)) {
605	684	$total_count++;
606	685
607		- if ($this->core_settings->set($key, $value)) {
608		- $success_count++;
	686	+ if (!$this->core_settings->set($key, $value)) {
	687	+ $this->last_failed_keys[] = $key;
	688	+
	689	+ // Name the key in the log: the UI can only ever show one
	690	+ // message for the batch, so without this a single dropped
	691	+ // setting is indistinguishable from a healthy save.
	692	+ // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- deliberate diagnostic, see above.
	693	+ error_log(sprintf('ThinkRank [%s]: settings save failed — key \'%s\' was not stored', $category, $key));
609	694	}
610	695	}
611	696	}
612	697
613		- // Consider successful if at least 70% of settings were saved
614		- $success_rate = $total_count > 0 ? ($success_count / $total_count) : 0;
615		- $success = $success_rate >= 0.7;
	698	+ // Every requested key must persist. A partial save used to pass on a 70%
	699	+ // threshold, so a batch could silently drop up to a third of the user's
	700	+ // settings while the UI reported success and the values were simply gone
	701	+ // (#300). Note the write is not transactional: the keys that did save
	702	+ // stay saved, which is why the failed keys are reported rather than just
	703	+ // a bare false.
	704	+ $success = $total_count > 0 && empty($this->last_failed_keys);
616	705
617	706	if ($success) {
618	707	update_option('thinkrank_settings_last_updated', current_time('mysql'));
619	708	}
		@@ -644,11 +733,12 @@
644	733	* @param array $settings Settings to update
645	734	* @param string $category Category name
646	735	* @param string $context_type Context type
647	736	* @param int|null $context_id Context ID
648		- * @return bool Success status
	737	+ * @return bool|null True on success, false on failure, null when the SEO
	738	+ * store does not own the category.
649	739	*/
650		- private function update_seo_settings_by_category(array $settings, string $category, string $context_type, ?int $context_id): bool {
	740	+ private function update_seo_settings_by_category(array $settings, string $category, string $context_type, ?int $context_id): ?bool {
651	741	return $this->seo_settings->save_settings_by_category($context_type, $context_id, $settings, $category);
652	742	}
653	743
654	744	/**
		@@ -714,8 +804,9 @@
714	804
715	805	switch ($key) {
716	806	case 'openai_api_key':
717	807	case 'claude_api_key':
	808	+ case 'openrouter_api_key':
718	809	if (!empty($value) && !is_string($value)) {
719	810	$validation['valid'] = false;
720	811	$validation['errors'][] = "{$key} must be a string";
721	812	}
		@@ -741,11 +832,15 @@
741	832	}
742	833	break;
743	834
744	835	case 'ai_provider':
745		- if (!in_array($value, ['openai', 'claude', 'gemini'], true)) {
	836	+ // '' is legal: it is Settings::AI_PROVIDER_NONE, the state a
	837	+ // fresh install starts in and the one a user returns to by
	838	+ // deselecting their provider (#572).
	839	+ if (!in_array($value, \ThinkRank\Core\Settings::selectable_ai_providers(), true)) {
746	840	$validation['valid'] = false;
747		- $validation['errors'][] = "ai_provider must be 'openai', 'claude', or 'gemini'";
	841	+ $validation['errors'][] = "ai_provider must be empty (no provider) or one of: "
	842	+ . implode(', ', \ThinkRank\Core\Settings::SUPPORTED_AI_PROVIDERS);
748	843	}
749	844	break;
750	845
751	846	case 'dashboard_widgets':
		@@ -788,9 +883,16 @@
788	883	* @param string $category Category name
789	884	* @return bool Success status
790	885	*/
791	886	private function reset_seo_settings(string $category): bool {
792		- // For SEO settings, we would need to implement reset functionality
793		- // in the SEO Settings Manager. For now, return true as placeholder.
794		- return true;
	887	+ try {
	888	+ // Map the settings category to the SEO context type
	889	+ return $this->seo_settings->reset_to_defaults('site');
	890	+ } catch (\Exception $e) {
	891	+ if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
	892	+ // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log -- Debug logging only when WP_DEBUG is enabled.
	893	+ error_log('ThinkRank: Failed to reset SEO settings for category "' . $category . '": ' . $e->getMessage());
	894	+ }
	895	+ return false;
	896	+ }
795	897	}
796	898	}