From db06da5d68861478047d50ef73598ce1c0717b89 Mon Sep 17 00:00:00 2001
From: David Stone <david@nnucomputerwhiz.com>
Date: Tue, 24 Mar 2026 15:59:34 -0600
Subject: [PATCH 1/2] fix: invert visibility priority so hidden wins over
 visible in limitation merge
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

When merging limitations from products or memberships, the visibility
priority was inverted: 'visible' had priority 1 and 'hidden' had priority 0,
meaning 'visible' always won. This caused plugins and themes marked as
'hidden' on a product or membership to remain visible on the sub-site.

Fix: swap the priorities so 'hidden' (1) > 'visible' (0). A restriction
from any source (product, membership, or site) must take effect — if any
source says 'hidden', the item must be hidden.

Also updates the existing test that was asserting the wrong (buggy) behavior,
and adds two regression tests covering the product and membership scenarios
described in issue #234.

Fixes #234
---
 inc/objects/class-limitations.php            |  12 +-
 tests/WP_Ultimo/Objects/Limitations_Test.php | 119 ++++++++++++++++++-
 2 files changed, 128 insertions(+), 3 deletions(-)

diff --git a/inc/objects/class-limitations.php b/inc/objects/class-limitations.php
index 9b584c377..335a00868 100644
--- a/inc/objects/class-limitations.php
+++ b/inc/objects/class-limitations.php
@@ -327,9 +327,17 @@ protected function merge_recursive(array &$array1, array &$array2, $should_sum =
 				} elseif (isset($array1[ $key ]) && is_numeric($array1[ $key ]) && is_numeric($value) && $should_sum && ! $is_unlimited) {
 					$array1[ $key ] = ((int) $array1[ $key ]) + $value;
 				} elseif ('visibility' === $key && isset($array1[ $key ]) && $should_sum) {
+					/*
+					 * Hidden takes priority over visible: if any source restricts
+					 * visibility to 'hidden', the item must be hidden. A product or
+					 * membership that marks a plugin/theme as hidden should always
+					 * override a default of 'visible'.
+					 *
+					 * Priority: hidden (1) > visible (0)
+					 */
 					$key_priority = [
-						'hidden'  => 0,
-						'visible' => 1,
+						'visible' => 0,
+						'hidden'  => 1,
 					];
 
 					$array1[ $key ] = $key_priority[ $value ] > $key_priority[ $array1[ $key ] ] ? $value : $array1[ $key ];
diff --git a/tests/WP_Ultimo/Objects/Limitations_Test.php b/tests/WP_Ultimo/Objects/Limitations_Test.php
index 9c94683d5..1f9f31dd2 100644
--- a/tests/WP_Ultimo/Objects/Limitations_Test.php
+++ b/tests/WP_Ultimo/Objects/Limitations_Test.php
@@ -720,6 +720,10 @@ public function test_merge_recursive_force_enabled(): void {
 
 	/**
 	 * Test merge_recursive with visibility priority.
+	 *
+	 * Hidden must win over visible: if any source restricts visibility to 'hidden',
+	 * the item must remain hidden. This is the fix for issue #234 where plugins/themes
+	 * set as hidden on a product or membership were not being hidden.
 	 */
 	public function test_merge_recursive_visibility_priority(): void {
 		$limitations = new Limitations();
@@ -732,6 +736,7 @@ public function test_merge_recursive_visibility_priority(): void {
 			$method->setAccessible(true);
 		}
 
+		// Case 1: base is 'hidden', incoming is 'visible' — hidden must win.
 		$array1 = [
 			'enabled'    => true,
 			'visibility' => 'hidden',
@@ -743,7 +748,119 @@ public function test_merge_recursive_visibility_priority(): void {
 
 		$method->invokeArgs($limitations, [&$array1, &$array2, true]);
 
-		$this->assertEquals('visible', $array1['visibility']);
+		$this->assertEquals('hidden', $array1['visibility'], 'hidden should win over visible (restriction takes priority)');
+
+		// Case 2: base is 'visible', incoming is 'hidden' — hidden must win.
+		$array3 = [
+			'enabled'    => true,
+			'visibility' => 'visible',
+		];
+		$array4 = [
+			'enabled'    => true,
+			'visibility' => 'hidden',
+		];
+
+		$method->invokeArgs($limitations, [&$array3, &$array4, true]);
+
+		$this->assertEquals('hidden', $array3['visibility'], 'hidden incoming should override visible base');
+
+		// Case 3: both visible — stays visible.
+		$array5 = [
+			'enabled'    => true,
+			'visibility' => 'visible',
+		];
+		$array6 = [
+			'enabled'    => true,
+			'visibility' => 'visible',
+		];
+
+		$method->invokeArgs($limitations, [&$array5, &$array6, true]);
+
+		$this->assertEquals('visible', $array5['visibility'], 'visible + visible should remain visible');
+	}
+
+	/**
+	 * Regression test for issue #234: plugins hidden on a product/membership must be hidden on the site.
+	 *
+	 * When a plugin is set to 'hidden' visibility on a product limitation, merging that product's
+	 * limitations into the site's composite limitations must result in the plugin being hidden.
+	 * Previously the merge kept 'visible' because the priority was inverted.
+	 */
+	public function test_plugin_hidden_on_product_is_hidden_on_site(): void {
+
+		// Product limitation: plugin is explicitly hidden.
+		$product_limitations = new Limitations([
+			'plugins' => [
+				'enabled' => true,
+				'limit'   => [
+					'woocommerce/woocommerce.php' => [
+						'visibility' => 'hidden',
+						'behavior'   => 'default',
+					],
+				],
+			],
+		]);
+
+		// Site starts with empty limitations (no site-level overrides).
+		$site_limitations = new Limitations([]);
+
+		// Simulate the waterfall: merge product limitations first, then site overrides.
+		$composite = $site_limitations->merge($product_limitations);
+
+		$plugin_limit = $composite->plugins->{'woocommerce/woocommerce.php'};
+
+		$this->assertEquals(
+			'hidden',
+			$plugin_limit->visibility,
+			'Plugin set as hidden on product must be hidden in composite limitations (issue #234)'
+		);
+
+		$this->assertTrue(
+			$composite->plugins->allowed('woocommerce/woocommerce.php', 'hidden'),
+			'allowed() with type "hidden" must return true for a hidden plugin'
+		);
+
+		$this->assertFalse(
+			$composite->plugins->allowed('woocommerce/woocommerce.php', 'visible'),
+			'allowed() with type "visible" must return false for a hidden plugin'
+		);
+	}
+
+	/**
+	 * Regression test for issue #234: themes hidden on a membership must be hidden on the site.
+	 */
+	public function test_theme_hidden_on_membership_is_hidden_on_site(): void {
+
+		// Membership limitation: theme is explicitly hidden.
+		$membership_limitations = new Limitations([
+			'themes' => [
+				'enabled' => true,
+				'limit'   => [
+					'twentytwentyfour' => [
+						'visibility' => 'hidden',
+						'behavior'   => 'available',
+					],
+				],
+			],
+		]);
+
+		// Site starts with empty limitations.
+		$site_limitations = new Limitations([]);
+
+		$composite = $site_limitations->merge($membership_limitations);
+
+		$theme_limit = $composite->themes->{'twentytwentyfour'};
+
+		$this->assertEquals(
+			'hidden',
+			$theme_limit->visibility,
+			'Theme set as hidden on membership must be hidden in composite limitations (issue #234)'
+		);
+
+		$this->assertTrue(
+			$composite->themes->allowed('twentytwentyfour', 'hidden'),
+			'allowed() with type "hidden" must return true for a hidden theme'
+		);
 	}
 
 	/**

From 4bb0e7fb4b9ed977d8f6a490281c86622facc8f1 Mon Sep 17 00:00:00 2001
From: David Stone <david@nnucomputerwhiz.com>
Date: Tue, 24 Mar 2026 16:45:17 -0600
Subject: [PATCH 2/2] fix: resolve CI failures on PR #384
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- tests/WP_Ultimo/Objects/Limitations_Test.php: add missing file doc
  comment, class doc comment, and @param tags on all data-provider test
  methods; auto-fix multi-line function call formatting (phpcbf)
- tests/WP_Ultimo/Managers/Limitation_Manager_Test.php: update
  test_merge_visibility_priorities assertion from 'visible' to 'hidden'
  to match the corrected priority logic (hidden wins over visible,
  restrictions take precedence — fix for issue #234)
---
 .../Managers/Limitation_Manager_Test.php      |   4 +-
 tests/WP_Ultimo/Objects/Limitations_Test.php  | 211 +++++++++++-------
 2 files changed, 132 insertions(+), 83 deletions(-)

diff --git a/tests/WP_Ultimo/Managers/Limitation_Manager_Test.php b/tests/WP_Ultimo/Managers/Limitation_Manager_Test.php
index c322ae1bd..5f9f1af7e 100644
--- a/tests/WP_Ultimo/Managers/Limitation_Manager_Test.php
+++ b/tests/WP_Ultimo/Managers/Limitation_Manager_Test.php
@@ -1791,8 +1791,8 @@ public function test_merge_visibility_priorities(): void {
 		$merged = $base->merge($override_data);
 
 		$plugin = $merged->plugins->{'akismet/akismet.php'};
-		// visible has higher priority than hidden
-		$this->assertEquals('visible', $plugin->visibility);
+		// hidden has higher priority than visible — restrictions take precedence (fix for issue #234)
+		$this->assertEquals('hidden', $plugin->visibility);
 	}
 
 	// ---------------------------------------------------------------
diff --git a/tests/WP_Ultimo/Objects/Limitations_Test.php b/tests/WP_Ultimo/Objects/Limitations_Test.php
index 1f9f31dd2..be5c294b5 100644
--- a/tests/WP_Ultimo/Objects/Limitations_Test.php
+++ b/tests/WP_Ultimo/Objects/Limitations_Test.php
@@ -1,10 +1,20 @@
 <?php
+/**
+ * Unit tests for the Limitations object.
+ *
+ * @package WP_Ultimo\Tests\Objects
+ */
 
 namespace WP_Ultimo\Objects;
 
 use WP_UnitTestCase;
 use WP_Ultimo\Objects\Limitations;
 
+/**
+ * Test case for WP_Ultimo\Objects\Limitations.
+ *
+ * @package WP_Ultimo\Tests\Objects
+ */
 class Limitations_Test extends WP_UnitTestCase {
 
 	/**
@@ -75,6 +85,8 @@ public function constructorDataProvider(): array {
 	 * Test constructor with various module data.
 	 *
 	 * @dataProvider constructorDataProvider
+	 * @param array $modules_data           Input module data.
+	 * @param int   $expected_modules_count Expected number of modules.
 	 */
 	public function test_constructor(array $modules_data, int $expected_modules_count): void {
 		$limitations = new Limitations($modules_data);
@@ -125,6 +137,8 @@ public function magicGetterDataProvider(): array {
 	 * Test magic getter method.
 	 *
 	 * @dataProvider magicGetterDataProvider
+	 * @param string $module_name  Name of the module to access.
+	 * @param bool   $should_exist Whether the module should exist.
 	 */
 	public function test_magic_getter(string $module_name, bool $should_exist): void {
 		$limitations = new Limitations();
@@ -200,6 +214,8 @@ public function buildModulesDataProvider(): array {
 	 * Test build_modules method.
 	 *
 	 * @dataProvider buildModulesDataProvider
+	 * @param array $modules_data   Input module data.
+	 * @param int   $expected_count Expected number of built modules.
 	 */
 	public function test_build_modules(array $modules_data, int $expected_count): void {
 		$limitations = new Limitations();
@@ -261,6 +277,9 @@ public function buildMethodDataProvider(): array {
 	 * Test static build method.
 	 *
 	 * @dataProvider buildMethodDataProvider
+	 * @param mixed  $data           Module data (array or JSON string).
+	 * @param string $module_name    Name of the module to build.
+	 * @param bool   $should_succeed Whether the build should succeed.
 	 */
 	public function test_build_method($data, string $module_name, bool $should_succeed): void {
 		$result = Limitations::build($data, $module_name);
@@ -302,6 +321,9 @@ public function existsMethodDataProvider(): array {
 	 * Test exists method.
 	 *
 	 * @dataProvider existsMethodDataProvider
+	 * @param array  $modules_data Input module data.
+	 * @param string $module_name  Name of the module to check.
+	 * @param bool   $should_exist Whether the module should exist.
 	 */
 	public function test_exists_method(array $modules_data, string $module_name, bool $should_exist): void {
 		$limitations = new Limitations($modules_data);
@@ -355,6 +377,8 @@ public function hasLimitationsDataProvider(): array {
 	 * Test has_limitations method.
 	 *
 	 * @dataProvider hasLimitationsDataProvider
+	 * @param array $modules_data Input module data.
+	 * @param bool  $expected     Expected return value.
 	 */
 	public function test_has_limitations(array $modules_data, bool $expected): void {
 		$limitations = new Limitations($modules_data);
@@ -397,6 +421,9 @@ public function isModuleEnabledDataProvider(): array {
 	 * Test is_module_enabled method.
 	 *
 	 * @dataProvider isModuleEnabledDataProvider
+	 * @param array  $modules_data Input module data.
+	 * @param string $module_name  Name of the module to check.
+	 * @param bool   $expected     Expected return value.
 	 */
 	public function test_is_module_enabled(array $modules_data, string $module_name, bool $expected): void {
 		$limitations = new Limitations($modules_data);
@@ -515,6 +542,10 @@ public function mergeMethodDataProvider(): array {
 	 * Test merge method.
 	 *
 	 * @dataProvider mergeMethodDataProvider
+	 * @param array $base_data      Base limitations data.
+	 * @param array $merge_data     Array of limitations data to merge in.
+	 * @param bool  $override       Whether to use override mode.
+	 * @param mixed $expected_value Expected value after merge.
 	 */
 	public function test_merge_method(array $base_data, array $merge_data, bool $override, $expected_value): void {
 		$limitations = new Limitations($base_data);
@@ -789,17 +820,19 @@ public function test_merge_recursive_visibility_priority(): void {
 	public function test_plugin_hidden_on_product_is_hidden_on_site(): void {
 
 		// Product limitation: plugin is explicitly hidden.
-		$product_limitations = new Limitations([
-			'plugins' => [
-				'enabled' => true,
-				'limit'   => [
-					'woocommerce/woocommerce.php' => [
-						'visibility' => 'hidden',
-						'behavior'   => 'default',
+		$product_limitations = new Limitations(
+			[
+				'plugins' => [
+					'enabled' => true,
+					'limit'   => [
+						'woocommerce/woocommerce.php' => [
+							'visibility' => 'hidden',
+							'behavior'   => 'default',
+						],
 					],
 				],
-			],
-		]);
+			]
+		);
 
 		// Site starts with empty limitations (no site-level overrides).
 		$site_limitations = new Limitations([]);
@@ -832,17 +865,19 @@ public function test_plugin_hidden_on_product_is_hidden_on_site(): void {
 	public function test_theme_hidden_on_membership_is_hidden_on_site(): void {
 
 		// Membership limitation: theme is explicitly hidden.
-		$membership_limitations = new Limitations([
-			'themes' => [
-				'enabled' => true,
-				'limit'   => [
-					'twentytwentyfour' => [
-						'visibility' => 'hidden',
-						'behavior'   => 'available',
+		$membership_limitations = new Limitations(
+			[
+				'themes' => [
+					'enabled' => true,
+					'limit'   => [
+						'twentytwentyfour' => [
+							'visibility' => 'hidden',
+							'behavior'   => 'available',
+						],
 					],
 				],
-			],
-		]);
+			]
+		);
 
 		// Site starts with empty limitations.
 		$site_limitations = new Limitations([]);
@@ -910,24 +945,28 @@ public function test_merge_recursive_behavior_priority(): void {
 	 */
 	public function test_merge_null_limit_does_not_overwrite_template_list(): void {
 
-		$plan_limitations = new Limitations([
-			'site_templates' => [
-				'enabled' => true,
-				'mode'    => 'default',
-				'limit'   => [
-					'2' => ['behavior' => 'available'],
-					'3' => ['behavior' => 'pre_selected'],
+		$plan_limitations = new Limitations(
+			[
+				'site_templates' => [
+					'enabled' => true,
+					'mode'    => 'default',
+					'limit'   => [
+						'2' => ['behavior' => 'available'],
+						'3' => ['behavior' => 'pre_selected'],
+					],
 				],
-			],
-		]);
+			]
+		);
 
-		$addon_limitations = new Limitations([
-			'site_templates' => [
-				'enabled' => true,
-				'mode'    => 'default',
-				'limit'   => null,
-			],
-		]);
+		$addon_limitations = new Limitations(
+			[
+				'site_templates' => [
+					'enabled' => true,
+					'mode'    => 'default',
+					'limit'   => null,
+				],
+			]
+		);
 
 		$merged = $plan_limitations->merge($addon_limitations);
 
@@ -942,19 +981,23 @@ public function test_merge_null_limit_does_not_overwrite_template_list(): void {
 	 */
 	public function test_merge_null_limit_overwrites_in_override_mode(): void {
 
-		$base = new Limitations([
-			'users' => [
-				'enabled' => true,
-				'limit'   => 5,
-			],
-		]);
+		$base = new Limitations(
+			[
+				'users' => [
+					'enabled' => true,
+					'limit'   => 5,
+				],
+			]
+		);
 
-		$override = new Limitations([
-			'users' => [
-				'enabled' => true,
-				'limit'   => null,
-			],
-		]);
+		$override = new Limitations(
+			[
+				'users' => [
+					'enabled' => true,
+					'limit'   => null,
+				],
+			]
+		);
 
 		$merged = $base->merge(true, $override);
 
@@ -985,34 +1028,38 @@ public function test_get_empty_to_array_returns_default_states(): void {
 	public function test_checkout_plan_plus_addon_preserves_templates(): void {
 
 		// Plan with specific templates configured
-		$plan_data = new Limitations([
-			'site_templates' => [
-				'enabled' => true,
-				'mode'    => 'choose_available_templates',
-				'limit'   => [
-					'5' => ['behavior' => 'available'],
-					'7' => ['behavior' => 'available'],
-					'9' => ['behavior' => 'not_available'],
+		$plan_data = new Limitations(
+			[
+				'site_templates' => [
+					'enabled' => true,
+					'mode'    => 'choose_available_templates',
+					'limit'   => [
+						'5' => ['behavior' => 'available'],
+						'7' => ['behavior' => 'available'],
+						'9' => ['behavior' => 'not_available'],
+					],
 				],
-			],
-			'disk_space'     => [
-				'enabled' => true,
-				'limit'   => 500,
-			],
-		]);
+				'disk_space'     => [
+					'enabled' => true,
+					'limit'   => 500,
+				],
+			]
+		);
 
 		// Addon product with no template restrictions but some disk space
-		$addon_data = new Limitations([
-			'site_templates' => [
-				'enabled' => true,
-				'mode'    => 'default',
-				'limit'   => null,
-			],
-			'disk_space'     => [
-				'enabled' => true,
-				'limit'   => 100,
-			],
-		]);
+		$addon_data = new Limitations(
+			[
+				'site_templates' => [
+					'enabled' => true,
+					'mode'    => 'default',
+					'limit'   => null,
+				],
+				'disk_space'     => [
+					'enabled' => true,
+					'limit'   => 100,
+				],
+			]
+		);
 
 		// Simulate the validation rule merge
 		$limits = new Limitations([]);
@@ -1037,17 +1084,19 @@ public function test_checkout_plan_plus_addon_preserves_templates(): void {
 	 */
 	public function test_available_site_templates_returns_integers(): void {
 
-		$limitations = new Limitations([
-			'site_templates' => [
-				'enabled' => true,
-				'mode'    => 'choose_available_templates',
-				'limit'   => [
-					'123' => ['behavior' => 'available'],
-					'456' => ['behavior' => 'pre_selected'],
-					'789' => ['behavior' => 'not_available'],
+		$limitations = new Limitations(
+			[
+				'site_templates' => [
+					'enabled' => true,
+					'mode'    => 'choose_available_templates',
+					'limit'   => [
+						'123' => ['behavior' => 'available'],
+						'456' => ['behavior' => 'pre_selected'],
+						'789' => ['behavior' => 'not_available'],
+					],
 				],
-			],
-		]);
+			]
+		);
 
 		$available = $limitations->site_templates->get_available_site_templates();