Merge pull request #89 from iMattPro/Fixins

Refine module auth

Author: iMattPro

migrations/add_webpush.php

@@ -74,6 +74,7 @@ public function update_data(): array
 				'module_basename'	=> '\phpbb\webpushnotifications\acp\wpn_acp_module',
 				'module_langname'	=> 'ACP_WEBPUSH_EXT_SETTINGS',
 				'module_mode'		=> 'webpush',
+				//'module_auth'		=> 'ext_phpbb/webpushnotifications && acl_a_server', //Accidentally omitted this, fix is in another migration
 				'after'				=> 'ACP_JABBER_SETTINGS',
 			]]],
 		];

migrations/fix_acp_module_auth.php

@@ -14,6 +14,20 @@
 
 class fix_acp_module_auth extends migration
 {
+	public function effectively_installed()
+	{
+		$sql = 'SELECT module_id
+			FROM ' . $this->table_prefix . "modules
+			WHERE module_class = 'acp'
+				AND module_langname = 'ACP_WEBPUSH_EXT_SETTINGS'
+				AND module_auth = 'ext_phpbb/webpushnotifications && acl_a_server'";
+		$result = $this->db->sql_query($sql);
+		$module_id = $this->db->sql_fetchfield('module_id');
+		$this->db->sql_freeresult($result);
+
+		return $module_id !== false;
+	}
+
 	public static function depends_on()
 	{
 		return ['\phpbb\webpushnotifications\migrations\add_webpush'];
@@ -30,8 +44,8 @@ public function set_acp_module_auth()
 	{
 		$phpbb_modules_table = $this->table_prefix . 'modules';
 		$sql = 'UPDATE ' . $phpbb_modules_table . "
-				SET module_auth = '" . $this->db->sql_escape('ext_phpbb/webpushnotifications && acl_a_server') . "'
-				WHERE module_langname = '" . $this->db->sql_escape('ACP_WEBPUSH_EXT_SETTINGS')  . "'";
+			SET module_auth = '" . $this->db->sql_escape('ext_phpbb/webpushnotifications && acl_a_server') . "'
+			WHERE module_langname = '" . $this->db->sql_escape('ACP_WEBPUSH_EXT_SETTINGS')  . "'";
 		$this->db->sql_query($sql);
 	}
 }

notification/method/webpush.php

@@ -371,7 +371,7 @@ public function get_ucp_template_data(helper $controller_helper, form_helper $fo
 			{
 				$subscriptions[] = [
 					'endpoint'			=> $subscription['endpoint'],
-					'expirationTime'	=> $subscription['expiration_time'],
+					'expirationTime'	=> (int) $subscription['expiration_time'],
 				];
 			}
 		}

tests/event/listener_test.php

@@ -244,14 +244,63 @@ public function test_load_template_data($user_id, $method_data, $subscriptions,
 
 		$this->template->expects($expected ? self::once() : self::never())
 			->method('assign_vars')
-			->with([
-				'NOTIFICATIONS_WEBPUSH_ENABLE'	=> true,
-				'U_WEBPUSH_SUBSCRIBE'			=> $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_subscribe_controller'),
-				'U_WEBPUSH_UNSUBSCRIBE'			=> $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_unsubscribe_controller'),
-				'VAPID_PUBLIC_KEY'				=> $this->config['wpn_webpush_vapid_public'],
-				'U_WEBPUSH_WORKER_URL'			=> $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_worker_controller'),
-				'SUBSCRIPTIONS'					=> $subscriptions,
-				'WEBPUSH_FORM_TOKENS'			=> $this->form_helper->get_form_tokens(\phpbb\webpushnotifications\ucp\controller\webpush::FORM_TOKEN_UCP),
+			->withConsecutive([
+				$this->callback(function($arg) use ($subscriptions) {
+					$expectedValues = [
+						'NOTIFICATIONS_WEBPUSH_ENABLE'	=> true,
+						'U_WEBPUSH_SUBSCRIBE'			=> $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_subscribe_controller'),
+						'U_WEBPUSH_UNSUBSCRIBE'			=> $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_unsubscribe_controller'),
+						'VAPID_PUBLIC_KEY'				=> $this->config['wpn_webpush_vapid_public'],
+						'U_WEBPUSH_WORKER_URL'			=> $this->controller_helper->route('phpbb_webpushnotifications_ucp_push_worker_controller'),
+						'SUBSCRIPTIONS'					=> $subscriptions,
+						'WEBPUSH_FORM_TOKENS'			=> $this->form_helper->get_form_tokens(\phpbb\webpushnotifications\ucp\controller\webpush::FORM_TOKEN_UCP),
+					];
+
+					// Check all required keys exist first
+					$missingKeys = array_diff(array_keys($expectedValues), array_keys($arg));
+					if (!empty($missingKeys))
+					{
+						$this->fail("Expected key(s) '" . implode("', '", $missingKeys) . "' missing from argument array");
+					}
+
+					// Handle WEBPUSH_FORM_TOKENS separately
+					if (isset($arg['WEBPUSH_FORM_TOKENS']))
+					{
+						$tokenArg = $arg['WEBPUSH_FORM_TOKENS'];
+						$tokenExpected = $expectedValues['WEBPUSH_FORM_TOKENS'];
+
+						// Check creation_time separately, allow for 1 second discrepancies during test run
+						$timeDiff = abs($tokenArg['creation_time'] - $tokenExpected['creation_time']);
+						if ($timeDiff > 1)
+						{
+							$this->fail(sprintf(
+								"Creation time difference too large. Expected: %d, Actual: %d",
+								$tokenExpected['creation_time'],
+								$tokenArg['creation_time']
+							));
+						}
+						// Remove creation_time after checking to allow other fields comparison
+						unset($tokenArg['creation_time'], $tokenExpected['creation_time']);
+						$arg['WEBPUSH_FORM_TOKENS'] = $tokenArg;
+						$expectedValues['WEBPUSH_FORM_TOKENS'] = $tokenExpected;
+					}
+
+					// Compare values individually
+					foreach ($expectedValues as $key => $value)
+					{
+						if ($arg[$key] !== $value)
+						{
+							$this->fail(sprintf(
+								"Mismatch for key '%s'. Expected: %s, Actual: %s",
+								$key,
+								var_export($value, true),
+								var_export($arg[$key], true)
+							));
+						}
+					}
+
+					return true;
+				})
 			]);
 
 		$dispatcher = new \phpbb\event\dispatcher();
@@ -390,8 +439,8 @@ public function test_validate_pwa_options($validate, $cfg_array, $expected_error
 		$config_name = key($cfg_array);
 		$config_definition = ['validate' => $validate];
 
-		$pwa_icon_small = isset($cfg_array['pwa_icon_small']) ? $cfg_array['pwa_icon_small'] : '';
-		$pwa_icon_large = isset($cfg_array['pwa_icon_large']) ? $cfg_array['pwa_icon_large'] : '';
+		$pwa_icon_small = $cfg_array['pwa_icon_small'] ?? '';
+		$pwa_icon_large = $cfg_array['pwa_icon_large'] ?? '';
 
 		[$small_image_name, $small_image_ext] = $pwa_icon_small ? explode('.', $pwa_icon_small, 2) : ['', ''];
 		[$large_image_name, $large_image_ext] = $pwa_icon_large ? explode('.', $pwa_icon_large, 2) : ['', ''];