Merge pull request #146 from iMattPro/updates

A handful of minor updates

Author: iMattPro

.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": "callum@phpbb.com",
-			"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);
 	}