← All changes | includes/api/class-settings-management-endpoint.php +106 -16 2.0.2 → 2.7.0 View file →

		@@ -159,8 +159,39 @@
159	159	return $this->seo_managers[$category];
160	160	}
161	161
162	162	/**
	163	+ * Read a category from whichever store actually owns it.
	164	+ *
	165	+ * The generic store returns `[]` for the eight SEO categories: it looks for
	166	+ * rows whose key carries a `<category>_` prefix, and the rows carry no such
	167	+ * prefix — `social_media` is stored as `social_meta`, `schema_management` as
	168	+ * `schema_management_system`, and their keys are bare (`og_site_name`). So a
	169	+ * direct `Settings_Manager::get_settings()` reports a configured site as
	170	+ * having no settings at all.
	171	+ *
	172	+ * Writes never had the problem, because the write path already falls back to
	173	+ * the owning manager. That asymmetry is what made this invisible from the UI
	174	+ * and dangerous underneath it: the pre-reset rollback snapshotted `[]` and
	175	+ * then defaults were written over live settings, so Reset could not be undone
	176	+ * (#689). Every read goes through here now, so there is one place to be wrong.
	177	+ *
	178	+ * @since 2.7.0
	179	+ *
	180	+ * @param string $category Category key.
	181	+ * @param string $context_type Optional. Context type. Default 'site'.
	182	+ * @param int|null $context_id Optional. Context ID.
	183	+ * @return array The category's stored settings.
	184	+ */
	185	+ private function read_category(string $category, string $context_type = 'site', ?int $context_id = null): array {
	186	+ if ($this->has_seo_manager($category)) {
	187	+ return (array) $this->get_seo_manager($category)->get_settings($context_type, $context_id);
	188	+ }
	189	+
	190	+ return (array) $this->settings_manager->get_settings($category, $context_type, $context_id);
	191	+ }
	192	+
	193	+ /**
163	194	* Register API routes
164	195	*
165	196	* @since 1.0.0
166	197	*/
		@@ -496,10 +527,10 @@
496	527	if (!isset($this->setting_categories[$category])) {
497	528	continue;
498	529	}
499	530
500		- // Get settings for each category using Settings Manager
501		- $category_settings = $this->settings_manager->get_settings($category);
	531	+ // Get settings for each category from the store that owns it.
	532	+ $category_settings = $this->read_category($category);
502	533	$global_settings[$category] = $category_settings;
503	534
504	535	// Get schema if requested
505	536	if ($include_schema && $this->has_seo_manager($category)) {
		@@ -635,9 +666,9 @@
635	666	// Get updated settings
636	667	$updated_settings = [];
637	668	foreach (array_keys($settings) as $category) {
638	669	if (isset($this->setting_categories[$category])) {
639		- $updated_settings[$category] = $this->settings_manager->get_settings($category);
	670	+ $updated_settings[$category] = $this->read_category($category);
640	671	}
641	672	}
642	673
643	674	return new WP_REST_Response([
		@@ -682,9 +713,9 @@
682	713	);
683	714	}
684	715
685	716	// Get category settings
686		- $category_settings = $this->settings_manager->get_settings($category);
	717	+ $category_settings = $this->read_category($category);
687	718
688	719	// Get schema if requested
689	720	$schema = [];
690	721	if ($include_schema && $this->has_seo_manager($category)) {
		@@ -916,11 +947,16 @@
916	947	// Get updated settings. Read them back from whichever store actually
917	948	// owns the category: the generic store returns [] for the categories it
918	949	// does not know, which would report a successful save as zero settings
919	950	// and hand the UI an empty form to render (#371).
920		- $updated_settings = null === $generic_update && $this->has_seo_manager($category)
921		- ? $this->get_seo_manager($category)->get_settings($context_type, $context_id)
922		- : $this->settings_manager->get_settings($category, $context_type, $context_id);
	951	+ //
	952	+ // Which store *accepted the write* is the wrong question to ask here,
	953	+ // and `sitemap` is the case that proves it: the generic store claims
	954	+ // that write (update_settings() returns true, not null) and then reads
	955	+ // the category back as [], so keying off $generic_update sent the one
	956	+ // read path that had been fixed straight back into the empty store.
	957	+ // Ownership is a property of the category, not of the last write (#689).
	958	+ $updated_settings = $this->read_category($category, $context_type, $context_id);
923	959
924	960	return new WP_REST_Response([
925	961	'success' => true,
926	962	'data' => [
		@@ -1097,9 +1133,9 @@
1097	1133	if (!isset($this->setting_categories[$category])) {
1098	1134	continue;
1099	1135	}
1100	1136
1101		- $export_data[$category] = $this->settings_manager->get_settings($category);
	1137	+ $export_data[$category] = $this->read_category($category);
1102	1138	}
1103	1139
1104	1140	// Never let secrets (API keys, OAuth tokens) leave the site in an
1105	1141	// export file — strip them entirely.
		@@ -1235,9 +1271,9 @@
1235	1271	}
1236	1272
1237	1273	try {
1238	1274	// Check if settings exist and handle overwrite
1239		- $existing_settings = $this->settings_manager->get_settings($category);
	1275	+ $existing_settings = $this->read_category($category);
1240	1276
1241	1277	if (!empty($existing_settings) && !$overwrite_existing) {
1242	1278	$import_results[$category] = [
1243	1279	'success' => false,
		@@ -1310,9 +1346,9 @@
1310	1346	// Create backup data
1311	1347	$backup_data = [];
1312	1348	foreach ($categories as $category) {
1313	1349	if (isset($this->setting_categories[$category])) {
1314		- $backup_data[$category] = $this->settings_manager->get_settings($category);
	1350	+ $backup_data[$category] = $this->read_category($category);
1315	1351	}
1316	1352	}
1317	1353
1318	1354	// Create backup metadata
		@@ -1613,15 +1649,49 @@
1613	1649	* @since 1.0.0
1614	1650	*
1615	1651	* @return bool Permission status
1616	1652	*/
1617		- public function check_read_permissions(): bool {
	1653	+ public function check_read_permissions(WP_REST_Request $request): bool {
1618	1654	// Plugin SEO/AI config is not subscriber-visible — require the same
1619		- // management capability as the write routes.
1620		- return \ThinkRank\Core\Capability_Manager::current_user_can('thinkrank_settings');
	1655	+ // management capability as the write routes, resolved per category so a
	1656	+ // role granted one section can reach that section and no other (#573).
	1657	+ return \ThinkRank\Core\Capability_Manager::current_user_can(
	1658	+ $this->capability_for_request($request)
	1659	+ );
1621	1660	}
1622	1661
1623	1662	/**
	1663	+ * The capability a settings-management request requires.
	1664	+ *
	1665	+ * Category routes belong to the section owning the category; every other
	1666	+ * route on this controller is plugin-wide configuration and stays on
	1667	+ * `thinkrank_settings`. The gate in Role_Manager::gate_rest() reaches the
	1668	+ * same answer through Capability_Manager::capability_for_route() — both are
	1669	+ * kept so neither layer alone is load-bearing.
	1670	+ *
	1671	+ * @since 2.1.3
	1672	+ *
	1673	+ * @param WP_REST_Request $request Request.
	1674	+ * @return string
	1675	+ */
	1676	+ private function capability_for_request(WP_REST_Request $request): string {
	1677	+ // URL params only. get_param() searches the JSON body, the POST body
	1678	+ // and the query string ahead of the route path, so on the routes that
	1679	+ // declare no {category} — /global, /validate, /schema, /export,
	1680	+ // /backup, /restore — it read pure caller input and let a request
	1681	+ // nominate the capability it would be checked against (#582). Reading
	1682	+ // the path is also what Role_Manager::gate_rest() does, so the two
	1683	+ // layers now agree and the claim above is true again.
	1684	+ $category = $request->get_url_params()['category'] ?? null;
	1685	+
	1686	+ if (!is_string($category) || '' === $category) {
	1687	+ return 'thinkrank_settings';
	1688	+ }
	1689	+
	1690	+ return \ThinkRank\Core\Capability_Manager::capability_for_settings_category($category);
	1691	+ }
	1692	+
	1693	+ /**
1624	1694	* Check permissions for managing settings
1625	1695	*
1626	1696	* @since 1.0.0
1627	1697	*
		@@ -1626,10 +1696,12 @@
1626	1696	* @since 1.0.0
1627	1697	*
1628	1698	* @return bool Permission status
1629	1699	*/
1630		- public function check_manage_permissions(): bool {
1631		- return \ThinkRank\Core\Capability_Manager::current_user_can('thinkrank_settings');
	1700	+ public function check_manage_permissions(WP_REST_Request $request): bool {
	1701	+ return \ThinkRank\Core\Capability_Manager::current_user_can(
	1702	+ $this->capability_for_request($request)
	1703	+ );
1632	1704	}
1633	1705
1634	1706	/**
1635	1707	* Check permissions for administrator-only settings operations.
		@@ -2200,9 +2272,27 @@
2200	2272	private function create_settings_snapshot(array $categories, string $label): string {
2201	2273	$backup_data = [];
2202	2274	foreach ($categories as $category) {
2203	2275	if (isset($this->setting_categories[$category])) {
2204		- $backup_data[$category] = $this->settings_manager->get_settings($category);
	2276	+ $backup_data[$category] = $this->read_category($category);
	2277	+ }
	2278	+ }
	2279	+
	2280	+ // A snapshot that captured nothing for a category that does hold settings
	2281	+ // is worse than no snapshot: reset checks only that an id came back, so an
	2282	+ // empty one is accepted as a rollback point and the defaults go over live
	2283	+ // data that can no longer be recovered. That is exactly what #689 was.
	2284	+ //
	2285	+ // Ask the owning manager directly rather than trusting read_category(),
	2286	+ // so this stays a real check if a future edit sends a read back to the
	2287	+ // wrong store instead of quietly agreeing with it.
	2288	+ foreach ($backup_data as $category => $captured) {
	2289	+ if (!empty($captured) || !$this->has_seo_manager($category)) {
	2290	+ continue;
	2291	+ }
	2292	+
	2293	+ if (!empty((array) $this->get_seo_manager($category)->get_settings('site', null))) {
	2294	+ return '';
2205	2295	}
2206	2296	}
2207	2297
2208	2298	$backup_metadata = [