Merge pull request #146 from iMattPro/updates

A handful of minor updates

.github/codecov.yml

@@ -0,0 +1,2 @@
+fixes:
+    - "/phpBB3/phpBB/ext/phpbb/ideas/::"

.github/workflows/tests.yml

@@ -113,6 +113,8 @@ jobs:
                       db: "mysql:5.7"
                     - php: '7.2'
                       db: "mysql:5.7"
+                      COVERAGE: 1
+                      db_alias: "mysql:5.7 with Coverage"
                     - php: '7.3'
                       db: "mysql:5.7"
                     - php: '7.4'
@@ -173,12 +175,21 @@ jobs:
                   echo "db=$db" >> $GITHUB_OUTPUT
 
             - name: Setup PHP
+              if: ${{ matrix.COVERAGE != 1 }}
               uses: shivammathur/setup-php@v2
               with:
                   php-version: ${{ matrix.php }}
                   extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, mysqli, sqlite, pdo_sqlite, intl, gd, exif, iconv, sqlsrv, pdo_sqlsrv, ldap
                   coverage: none
 
+            - name: Setup PHP with Coverage
+              if: ${{ matrix.COVERAGE == 1 }}
+              uses: shivammathur/setup-php@v2
+              with:
+                  php-version: ${{ matrix.php }}
+                  extensions: dom, curl, libxml, mbstring, zip, pcntl, pdo, mysqli, sqlite, pdo_sqlite, intl, gd, exif, iconv, sqlsrv, pdo_sqlsrv, ldap
+                  coverage: xdebug
+
             - name: Setup environment for phpBB
               env:
                   DB: ${{steps.database-type.outputs.db}}
@@ -195,14 +206,36 @@ jobs:
               working-directory: ./phpBB3
 
             - name: Setup PHPUnit files
-              run: mkdir -p phpBB/ext/$EXTNAME/.github && cp .github/phpunit* $_
+              env:
+                  DB: ${{steps.database-type.outputs.db}}
+                  COVERAGE: ${{ matrix.COVERAGE != 1 && '0' || '1' }}
+              run: |
+                  if [ $COVERAGE == '1' ]
+                  then
+                      sed -n '1h;1!H;${;g;s/<\/php>/<\/php>\n\t<filter>\n\t\t<whitelist>\n\t\t\t<directory>..\/<\/directory>\n\t\t\t<exclude>\n\t\t\t\t<directory>..\/tests\/<\/directory>\n\t\t\t\t<directory>..\/language\/<\/directory>\n\t\t\t\t<directory>..\/migrations\/<\/directory>\n\t\t\t<\/exclude>\n\t\t<\/whitelist>\n\t<\/filter>/g;p;}' .github/phpunit-$DB-github.xml &> phpBB/ext/$EXTNAME/.github/phpunit-$DB-github.xml
+                  else
+                      mkdir -p phpBB/ext/$EXTNAME/.github && cp .github/phpunit* $_
+                  fi
               working-directory: ./phpBB3
 
             - name: Run unit tests
               env:
                   DB: ${{steps.database-type.outputs.db}}
-              run: phpBB/vendor/bin/phpunit --configuration phpBB/ext/$EXTNAME/.github/phpunit-$DB-github.xml --bootstrap ./tests/bootstrap.php
+                  COVERAGE: ${{ matrix.COVERAGE != 1 && '0' || '1' }}
+              run: |
+                  if [ $COVERAGE == '1' ]
+                  then
+                      phpBB/vendor/bin/phpunit --configuration phpBB/ext/$EXTNAME/.github/phpunit-$DB-github.xml --bootstrap ./tests/bootstrap.php --coverage-clover build/logs/clover.xml
+                  else
+                      phpBB/vendor/bin/phpunit --configuration phpBB/ext/$EXTNAME/.github/phpunit-$DB-github.xml --bootstrap ./tests/bootstrap.php
+                  fi
               working-directory: ./phpBB3
+
+            - name: Send code coverage
+              if: ${{ matrix.COVERAGE == 1 }}
+              uses: codecov/codecov-action@v3
+              with:
+                  files: ./phpBB3/build/logs/clover.xml
     # END MySQL and MariaDB Job
 
     # START PostgreSQL Job

acp/ideas_module.php

@@ -41,6 +41,7 @@ public function main()
 		$request = $phpbb_container->get('request');
 
 		// Get an instance of the admin controller
+		/** @var \phpbb\ideas\controller\admin_controller $admin_controller */
 		$admin_controller = $phpbb_container->get('phpbb.ideas.admin.controller');
 
 		// Add the phpBB Ideas ACP lang file
@@ -58,7 +59,7 @@ public function main()
 			$admin_controller->set_config_options();
 		}
 
-		// Set Ideas forum  options and registered usergroup forum permissions
+		// Set Ideas forum  options and registered user group forum permissions
 		if ($request->is_set_post('ideas_forum_setup'))
 		{
 			$admin_controller->set_ideas_forum_options();

composer.json

@@ -19,7 +19,7 @@
 		{
 			"name": "Callum Macrae",
 			"email": "[email protected]",
-			"homepage": "http://macr.ae/",
+			"homepage": "https://macr.ae/",
 			"role": "Original author"
 		},
 		{
@@ -29,18 +29,18 @@
 		},
 		{
 			"name": "Ruslan Uzdenov",
-			"homepage": "http://www.phpbbguru.net",
+			"homepage": "https://www.phpbbguru.net",
 			"role": "Extensions Development Team"
 		}
 	],
 	"require": {
-		"php": ">=5.4",
+		"php": ">=7.1.3",
 		"composer/installers": "~1.0"
 	},
 	"extra": {
 		"display-name": "phpBB Ideas",
 		"soft-require": {
-			"phpbb/phpbb": ">=3.2.3"
+			"phpbb/phpbb": ">=3.3.0"
 		}
 	}
 }

controller/admin_controller.php

@@ -42,16 +42,13 @@ class admin_controller
 	/** @var string */
 	protected $php_ext;
 
-	/** @var array */
-	protected $cfg_array = array();
-
 	/** @var string */
 	public $u_action;
 
 	/**
 	 * Constructor
 	 *
-	 * @param \phpbb\config\config              $config               Config object
+	 * @param \phpbb\config\config                $config               Config object
 	 * @param \phpbb\db\driver\driver_interface $db                   Database object
 	 * @param \phpbb\language\language          $language             Language object
 	 * @param \phpbb\log\log                    $log                  Log object
@@ -101,11 +98,6 @@ public function set_config_options()
 	{
 		$errors = array();
 
-		// This method is called on submit, so set flag to true initially
-		$submit = true;
-
-		$this->cfg_array = $this->request->variable('config', array('' => ''), true);
-
 		// Check the form for validity
 		if (!check_form_key('acp_phpbb_ideas_settings'))
 		{
@@ -115,36 +107,34 @@ public function set_config_options()
 		// Don't save settings if errors have occurred
 		if (count($errors))
 		{
-			$submit = false;
-
 			$this->template->assign_vars(array(
 				'S_ERROR'	=> true,
 				'ERROR_MSG'	=> implode('<br />', $errors),
 			));
+
+			return;
 		}
 
-		if ($submit)
-		{
-			// Configuration options to list through
-			$display_vars = array(
-				'ideas_forum_id',
-				'ideas_forum_setup',
-			);
-
-			// We go through the display_vars to make sure no one is trying to set variables he/she is not allowed to
-			foreach ($display_vars as $config_name)
-			{
-				if (!isset($this->cfg_array[$config_name]))
-				{
-					continue;
-				}
+		$cfg_array = $this->request->variable('config', array('' => ''));
 
-				$this->config->set($config_name, $this->cfg_array[$config_name]);
+		// Configuration options to list through
+		$display_vars = array(
+			'ideas_forum_id',
+		);
 
+		// We go through the display_vars to make sure no one is trying to set variables he/she is not allowed to
+		foreach ($display_vars as $config_name)
+		{
+			if (!isset($cfg_array[$config_name]))
+			{
+				continue;
 			}
-			$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'ACP_PHPBB_IDEAS_SETTINGS_LOG');
-			trigger_error($this->language->lang('ACP_IDEAS_SETTINGS_UPDATED') . adm_back_link($this->u_action));
+
+			$this->config->set($config_name, $cfg_array[$config_name]);
 		}
+
+		$this->log->add('admin', $this->user->data['user_id'], $this->user->ip, 'ACP_PHPBB_IDEAS_SETTINGS_LOG');
+		trigger_error($this->language->lang('ACP_IDEAS_SETTINGS_UPDATED') . adm_back_link($this->u_action));
 	}
 
 	/**
@@ -156,7 +146,7 @@ public function set_config_options()
 	public function set_ideas_forum_options()
 	{
 		// Check if Ideas forum is selected and apply relevant settings if it is
-		// But display the confirm box first
+		// But display the confirmation box first
 		if (confirm_box(true))
 		{
 			if (empty($this->config['ideas_forum_id']))

controller/base.php

@@ -103,7 +103,7 @@ public function get_entity($entity)
 	 * Check if Ideas is properly configured after installation
 	 * Ideas is available only after forum settings have been set in ACP
 	 *
-	 * @return bool Depending on whether or not the extension is properly configured
+	 * @return bool Depending on whether the extension is properly configured
 	 */
 	public function is_available()
 	{
@@ -135,7 +135,7 @@ protected function assign_template_block_vars($block, $rows)
 				'POINTS'     => $row['idea_votes_up'] - $row['idea_votes_down'], // (not currently implemented)
 				'STATUS'     => $row['idea_status'], // for status icons (not currently implemented)
 				'LOCKED'     => $row['topic_status'] == ITEM_LOCKED,
-				'U_UNAPPROVED_IDEA'	=> (($row['topic_visibility'] == ITEM_UNAPPROVED || $row['topic_visibility'] == ITEM_REAPPROVE) && $this->auth->acl_get('m_approve', $this->config['ideas_forum_id'])) ? append_sid("{$this->root_path}mcp.{$this->php_ext}", 'i=queue&mode=approve_details&t=' . $row['topic_id'], true, $this->user->session_id) : '',
+				'U_UNAPPROVED_IDEA'	=> (($row['topic_visibility'] == ITEM_UNAPPROVED || $row['topic_visibility'] == ITEM_REAPPROVE) && $this->auth->acl_get('m_approve', $this->config['ideas_forum_id'])) ? append_sid("{$this->root_path}mcp.$this->php_ext", 'i=queue&mode=approve_details&t=' . $row['topic_id'], true, $this->user->session_id) : '',
 			));
 		}
 	}
@@ -148,8 +148,8 @@ protected function assign_template_block_vars($block, $rows)
 	protected function display_common_vars()
 	{
 		$this->template->assign_vars([
-			'S_DISPLAY_SEARCHBOX'	=> (bool) $this->auth->acl_get('u_search') && $this->auth->acl_get('f_search', $this->config['ideas_forum_id']) && $this->config['load_search'],
-			'S_SEARCHBOX_ACTION'	=> append_sid("{$this->root_path}search.{$this->php_ext}"),
+			'S_DISPLAY_SEARCHBOX'	=> $this->auth->acl_get('u_search') && $this->auth->acl_get('f_search', $this->config['ideas_forum_id']) && $this->config['load_search'],
+			'S_SEARCHBOX_ACTION'	=> append_sid("{$this->root_path}search.$this->php_ext"),
 			'S_SEARCH_IDEAS_HIDDEN_FIELDS'	=> build_hidden_fields(['fid' => [$this->config['ideas_forum_id']]]),
 
 			'U_SEARCH_MY_IDEAS' 	=> $this->helper->route('phpbb_ideas_list_controller', ['sort' => ext::SORT_MYIDEAS, 'status' => '-1']),

controller/idea_controller.php

@@ -61,7 +61,7 @@ public function idea($idea_id)
 			$params = array_merge($params, array('view' => 'unread'));
 		}
 
-		$url = append_sid(generate_board_url() . "/viewtopic.{$this->php_ext}", $params, false) . ($unread ? '#unread' : '');
+		$url = append_sid(generate_board_url() . "/viewtopic.$this->php_ext", $params, false) . ($unread ? '#unread' : '');
 
 		return new RedirectResponse($url);
 	}
@@ -131,7 +131,7 @@ public function duplicate()
 	/**
 	 * Remove vote action (remove a user's vote from an idea)
 	 *
-	 * @return mixed Array of vote data, an error message, or false if failed
+	 * @return array|false|string Array of vote data, an error message, or false if failed
 	 * @access public
 	 */
 	public function removevote()
@@ -226,7 +226,7 @@ public function implemented()
 	/**
 	 * Vote action (sets an idea's vote)
 	 *
-	 * @return mixed Array of vote data, an error message, or false if failed
+	 * @return array|false|string Array of vote data, an error message, or false if failed
 	 * @access public
 	 */
 	public function vote()

controller/index_controller.php

@@ -48,7 +48,7 @@ public function index()
 			'U_VIEW_LATEST'		=> $this->helper->route('phpbb_ideas_list_controller', ['sort' => ext::SORT_NEW]),
 			'U_VIEW_IMPLEMENTED'=> $this->helper->route('phpbb_ideas_list_controller', ['sort' => ext::SORT_DATE, 'status' => ext::$statuses['IMPLEMENTED']]),
 			'U_POST_ACTION'		=> $this->helper->route('phpbb_ideas_post_controller'),
-			'U_MCP' 			=> ($this->auth->acl_get('m_', $this->config['ideas_forum_id'])) ? append_sid("{$this->root_path}mcp.{$this->php_ext}", "f={$this->config['ideas_forum_id']}&i=main&mode=forum_view", true, $this->user->session_id) : '',
+			'U_MCP' 			=> ($this->auth->acl_get('m_', $this->config['ideas_forum_id'])) ? append_sid("{$this->root_path}mcp.$this->php_ext", "f={$this->config['ideas_forum_id']}&i=main&mode=forum_view", true, $this->user->session_id) : '',
 
 		));
 

controller/list_controller.php

@@ -34,7 +34,7 @@ public function ideas_list($sort)
 
 		// Overwrite the $sort parameter if the url contains a sort query.
 		// This is needed with the sort by options form at the footer of the list.
-		$sort = $this->request->is_set('sort') ? $this->request->variable('sort', ext::SORT_NEW) : $sort;
+		$sort = $this->request->is_set('sort') ? (string) $this->request->variable('sort', ext::SORT_NEW) : $sort;
 
 		// Get additional query values the url may contain
 		$sort_direction = $this->request->variable('sd', 'd');
@@ -82,7 +82,7 @@ public function ideas_list($sort)
 			'SORT_ARY'			=> array(ext::SORT_AUTHOR, ext::SORT_DATE, ext::SORT_SCORE, ext::SORT_TITLE, ext::SORT_TOP, ext::SORT_VOTES),
 			'SORT'				=> $sort,
 			'SORT_DIRECTION'	=> $sort_direction,
-			'U_MCP' 			=> ($this->auth->acl_get('m_', $this->config['ideas_forum_id'])) ? append_sid("{$this->root_path}mcp.{$this->php_ext}", "f={$this->config['ideas_forum_id']}&i=main&mode=forum_view", true, $this->user->session_id) : '',
+			'U_MCP' 			=> ($this->auth->acl_get('m_', $this->config['ideas_forum_id'])) ? append_sid("{$this->root_path}mcp.$this->php_ext", "f={$this->config['ideas_forum_id']}&i=main&mode=forum_view", true, $this->user->session_id) : '',
 
 		));
 

controller/post_controller.php

@@ -39,7 +39,7 @@ public function post()
 			'f'		=> $this->config['ideas_forum_id'],
 		];
 
-		$url = append_sid(generate_board_url() . "/posting.{$this->php_ext}", $params, false);
+		$url = append_sid(generate_board_url() . "/posting.$this->php_ext", $params, false);
 
 		return new RedirectResponse($url);
 	}

event/listener.php

@@ -166,7 +166,7 @@ public function show_idea($event)
 		}
 
 		$points = $idea['idea_votes_up'] - $idea['idea_votes_down'];
-		$can_vote = (bool) ($idea['idea_status'] != ext::$statuses['IMPLEMENTED'] &&
+		$can_vote = ($idea['idea_status'] != ext::$statuses['IMPLEMENTED'] &&
 			$idea['idea_status'] != ext::$statuses['DUPLICATE'] &&
 			$this->auth->acl_get('f_vote', (int) $this->config['ideas_forum_id']) &&
 			$event['topic_data']['topic_status'] != ITEM_LOCKED);
@@ -343,7 +343,7 @@ public function submit_idea_after($event)
 
 		$this->idea->submit($event['data']);
 
-		// Show users who's posts need approval a special message
+		// Show users whose posts need approval a special message
 		if (!$this->auth->acl_get('f_noapprove', $event['data']['forum_id']))
 		{
 			// Using refresh and trigger error because we can't throw http_exceptions from posting.php

ext.php

@@ -58,14 +58,16 @@ public static function status_name($id)
 	 * Check whether the extension can be enabled.
 	 *
 	 * Requires phpBB >= 3.2.3 due to removal of deprecated Twig functions (ie Twig_SimpleFunction)
+	 * Requires phpBB >= 3.3.0 due to use of PHP 7 features
+	 * Requires PHP >= 7.1.0
 	 * Also incompatible with SQLite which does not support SQRT in SQL queries
 	 *
 	 * @return bool
 	 * @access public
 	 */
 	public function is_enableable()
 	{
-		if (phpbb_version_compare(PHPBB_VERSION, '3.2.3', '<'))
+		if (PHP_VERSION_ID < 70100 || phpbb_version_compare(PHPBB_VERSION, '3.3.0', '<'))
 		{
 			return false;
 		}

factory/idea.php

@@ -412,7 +412,7 @@ public function get_voters($id)
 
 		// Process the username for the template now, so it is
 		// ready to use in AJAX responses and DOM injections.
-		$profile_url = append_sid(generate_board_url() . "/memberlist.{$this->php_ext}", array('mode' => 'viewprofile'));
+		$profile_url = append_sid(generate_board_url() . "/memberlist.$this->php_ext", array('mode' => 'viewprofile'));
 		foreach ($rows as &$row)
 		{
 			$row['user'] = get_username_string('full', $row['user_id'], $row['username'], $row['user_colour'], false, $profile_url);

factory/ideas.php

@@ -80,8 +80,8 @@ protected function query_ideas()
 		$this->sql = [];
 
 		$this->sql['SELECT'][] = 't.topic_last_post_time, t.topic_status, t.topic_visibility, i.*';
-		$this->sql['FROM'] = "{$this->table_ideas} i";
-		$this->sql['JOIN'] = "{$this->table_topics} t ON i.topic_id = t.topic_id";
+		$this->sql['FROM'] = "$this->table_ideas i";
+		$this->sql['JOIN'] = "$this->table_topics t ON i.topic_id = t.topic_id";
 		$this->sql['WHERE'][] = 't.forum_id = ' . (int) $this->config['ideas_forum_id'];
 
 		// Only get approved topics for regular users, Moderators will see unapproved topics
@@ -293,6 +293,6 @@ public function delete_orphans()
 	 */
 	public function get_idea_count()
 	{
-		return isset($this->idea_count) ? $this->idea_count : 0;
+		return $this->idea_count ?? 0;
 	}
 }

factory/linkhelper.php

@@ -47,7 +47,7 @@ public function get_idea_link($idea_id, $mode = '', $hash = false)
 	{
 		$params = array('idea_id' => $idea_id);
 		$params = $mode ? array_merge($params, array('mode' => $mode)) : $params;
-		$params = $hash ? array_merge($params, array('hash' => generate_link_hash("{$mode}_{$idea_id}"))) : $params;
+		$params = $hash ? array_merge($params, array('hash' => generate_link_hash("{$mode}_$idea_id"))) : $params;
 
 		return $this->helper->route('phpbb_ideas_idea_controller', $params);
 	}

factory/permission_helper.php

@@ -43,7 +43,7 @@ public function __construct(\phpbb\db\driver\driver_interface $db, $phpbb_root_p
 	}
 
 	/**
-	 * Set the the best permissions for an Ideas forum.
+	 * Set the best permissions for an Ideas forum.
 	 *
 	 * @param int $forum_id A forum id
 	 */

migrations/m11_reparse_old_ideas.php

@@ -74,7 +74,7 @@ public function reparse($current = 0)
 			$current = $reparser->get_max_id();
 		}
 
-		$limit = 50; // lets keep the reparsing conservative
+		$limit = 50; // let's keep the reparsing conservative
 		$start = max(1, $current + 1 - $limit);
 		$end   = max(1, $current);