Merge pull request #14 from iMattPro/updates

Add tests and some minor fixes

.github/codecov.yml

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

.github/workflows/tests.yml

@@ -113,6 +113,10 @@ jobs:
                       db: "mysql:5.7"
                       db_alias: "MyISAM Tests"
                       MYISAM: 1
+                    - php: '8.1'
+                      db: "mysql:5.7"
+                      COVERAGE: 1
+                      db_alias: "mysql:5.7 with Coverage"
                     - php: '8.1'
                       db: "mysql:8.1"
                     - php: '8.2'
@@ -175,12 +179,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, intl, gd, exif, iconv, mysqli, pdo, pdo_mysql, 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}}
@@ -197,14 +210,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

adm/style/acp_pwakit.html

@@ -48,8 +48,8 @@ <h3>{{ lang('WARNING') }}</h3>
 			<dt><label for="pwa_upload">{{ lang('ACP_PWA_IMG_UPLOAD') ~ lang('COLON') }}</label><br /><span>{{ lang('ACP_PWA_IMG_UPLOAD_EXPLAIN', PWA_IMAGES_DIR) }}</span></dt>
 			<dd>
 				<input type="file" accept="image/*" class="inputbox autowidth" id="pwa_upload" name="pwa_upload" />
-				<button class="button2 pwakit-button" id="upload" name="upload">{{ Icon('font', 'upload', lang('PWA_IMG_UPLOAD_BTN')) }}</button>
-				<button class="button2 pwakit-button" id="resync" name="resync">{{ Icon('font', 'sync', lang('PWA_IMG_RESYNC_BTN')) }}</button>
+				<button class="button2 pwakit-button" id="upload" name="upload">{{ Icon('font', 'upload', lang('ACP_PWA_IMG_UPLOAD_BTN')) }}</button>
+				<button class="button2 pwakit-button" id="resync" name="resync">{{ Icon('font', 'sync', lang('ACP_PWA_IMG_RESYNC_BTN')) }}</button>
 			</dd>
 		</dl>
 		<dl>

controller/admin_controller.php

@@ -145,7 +145,7 @@ public function main(string $id, string $mode = ''): void
 	 *
 	 * @return void
 	 */
-	public function display_settings(): void
+	protected function display_settings(): void
 	{
 		$this->template->assign_vars([
 			'SITE_NAME'			=> $this->config->offsetGet('sitename'),
@@ -165,7 +165,7 @@ public function display_settings(): void
 	 *
 	 * @return void
 	 */
-	public function save_settings(): void
+	protected function save_settings(): void
 	{
 		$config_array = [
 			'pwa_bg_color'		=> $this->request->variable('pwa_bg_color', ''),
@@ -177,7 +177,7 @@ public function save_settings(): void
 			$this->validate_hex_color($config_value);
 		}
 
-		if ($this->display_errors())
+		if ($this->has_errors())
 		{
 			return;
 		}
@@ -191,20 +191,28 @@ public function save_settings(): void
 	}
 
 	/**
-	 * Display any errors
+	 * Are there any errors?
 	 *
 	 * @return bool
 	 */
-	public function display_errors(): bool
+	protected function has_errors(): bool
+	{
+		return (bool) count($this->errors);
+	}
+
+	/**
+	 * Display any errors
+	 *
+	 * @return void
+	 */
+	protected function display_errors(): void
 	{
-		$has_errors = (bool) count($this->errors);
+		$has_errors = $this->has_errors();
 
 		$this->template->assign_vars([
 			'S_ERROR'	=> $has_errors,
 			'ERROR_MSG'	=> $has_errors ? implode('<br>', $this->errors) : '',
 		]);
-
-		return $has_errors;
 	}
 
 	/**
@@ -235,7 +243,7 @@ protected function validate_hex_color(string $code): void
 	 *
 	 * @return void
 	 */
-	public function upload(): void
+	protected function upload(): void
 	{
 		try
 		{
@@ -248,7 +256,7 @@ public function upload(): void
 			$this->errors[] = $this->language->lang($e->getMessage());
 		}
 
-		if ($this->display_errors())
+		if ($this->has_errors())
 		{
 			return;
 		}
@@ -261,7 +269,7 @@ public function upload(): void
 	 *
 	 * @return void
 	 */
-	public function delete(): void
+	protected function delete(): void
 	{
 		$path = $this->request->variable('delete', '');
 

helper/upload.php

@@ -38,6 +38,17 @@ public function __construct(files_upload $files_upload, storage $storage)
 		$this->storage = $storage;
 	}
 
+	/**
+	 * Set the file spec storage
+	 *
+	 * @param filespec_storage $file
+	 * @return void
+	 */
+	public function set_file(filespec_storage $file): void
+	{
+		$this->file = $file;
+	}
+
 	/**
 	 * Handle upload
 	 *
@@ -51,7 +62,7 @@ public function upload(): string
 		$this->files_upload->set_allowed_extensions(['png']);
 
 		// Upload file
-		$this->file = $this->files_upload->handle_upload('files.types.form_storage', 'pwa_upload');
+		$this->set_file($this->files_upload->handle_upload('files.types.form_storage', 'pwa_upload'));
 		$this->file->clean_filename('real');
 
 		// Move file to proper location

language/en/acp_pwa.php

@@ -39,7 +39,7 @@
 //
 
 $lang = array_merge($lang, [
-	'ACP_PWA_KIT_SETTINGS_EXPLAIN'	=> 'Here you can manage the members of your Web Application Manifest. You can also preview the touch icons found in <samp>%s</samp>.',
+	'ACP_PWA_KIT_SETTINGS_EXPLAIN'	=> 'Here you can manage the members of your Web Application Manifest. You can also preview the web application icons found in <samp>%s</samp>.',
 	'ACP_PWA_KIT_SITE_NAME_EXPLAIN'	=> 'Used to specify the full name of your web application. This can be configured in General -> Board Settings.',
 	'ACP_PWA_KIT_SHORT_NAME_EXPLAIN'=> 'Used to specify a short name for your web application, which may be used when the full name is too long for the available space. This can be configured in General -> Board Settings.',
 	'ACP_PWA_KIT_LEGEND_IDENTITY'	=> 'Identity &amp; Presentation',
@@ -48,6 +48,7 @@
 	'ACP_PWA_THEME_COLOR_EXPLAIN'	=> 'Used to specify the default colour for your web application’s user interface. This colour may be applied to various browser UI elements, such as the toolbar, address bar, and status bar.',
 	'ACP_PWA_BG_COLOR'				=> 'Background colour (optional)',
 	'ACP_PWA_BG_COLOR_EXPLAIN'		=> 'Used to specify an initial background colour for your web application. This colour appears in the application window before your application’s stylesheets have loaded.',
+	'ACP_PWA_INVALID_COLOR'			=> 'The colour code “<samp>%s</samp>” is not a valid hex code.',
 	'ACP_PWA_KIT_ICONS'				=> 'Web application icons',
 	'ACP_PWA_KIT_ICONS_EXPLAIN'		=> 'PNG image files that represent your web application. Multiple sizes are preferred for compatibility with various devices.',
 	'ACP_PWA_KIT_NO_ICONS'			=> 'No icons are available. Click <strong>Upload</strong> to add new icons or click <strong>Resync</strong> to find existing icons that were previously uploaded.',
@@ -60,7 +61,6 @@
 	'ACP_PWA_IMG_DELETED'			=> '“%s” has been removed.',
 	'ACP_PWA_IMG_DELETE_PATH_ERR'	=> 'Invalid file path provided.',
 	'ACP_PWA_IMG_DELETE_NAME_ERR'	=> 'Invalid characters in filename.',
-	'PWA_IMG_RESYNC_BTN'			=> 'Resync',
-	'PWA_IMG_UPLOAD_BTN'			=> 'Upload',
-	'ACP_PWA_INVALID_COLOR'			=> 'The colour code “<samp>%s</samp>” is not a valid hex code.',
+	'ACP_PWA_IMG_RESYNC_BTN'		=> 'Resync',
+	'ACP_PWA_IMG_UPLOAD_BTN'		=> 'Upload',
 ]);

phpunit.xml.dist

@@ -1,16 +1,27 @@
 <?xml version="1.0" encoding="UTF-8"?>
-
-<phpunit backupGlobals="true"
-         backupStaticAttributes="true"
-         colors="true"
-         convertErrorsToExceptions="true"
-         convertNoticesToExceptions="true"
-         convertWarningsToExceptions="true"
-         processIsolation="false"
-         stopOnFailure="false"
-         verbose="true"
-         bootstrap="../../../../tests/bootstrap.php"
+<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+		 xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd"
+		 backupGlobals="true"
+		 backupStaticAttributes="false"
+		 colors="true"
+		 convertErrorsToExceptions="true"
+		 convertNoticesToExceptions="true"
+		 convertWarningsToExceptions="true"
+		 processIsolation="false"
+		 stopOnFailure="false"
+		 cacheResult="false"
+		 bootstrap="../../../../tests/bootstrap.php"
 >
+	<coverage>
+		<include>
+			<directory suffix=".php">./</directory>
+		</include>
+		<exclude>
+			<directory suffix=".php">./language/</directory>
+			<directory suffix=".php">./migrations/</directory>
+			<directory suffix=".php">./tests/</directory>
+		</exclude>
+	</coverage>
 	<testsuites>
 		<testsuite name="Extension Test Suite">
 			<directory suffix="_test.php">./tests</directory>
@@ -20,15 +31,4 @@
 			<directory suffix="_test.php">./tests/functional/</directory>
 		</testsuite>
 	</testsuites>
-
-	<filter>
-		<whitelist processUncoveredFilesFromWhitelist="true">
-			<directory suffix=".php">./</directory>
-			<exclude>
-				<directory suffix=".php">./language/</directory>
-				<directory suffix=".php">./migrations/</directory>
-				<directory suffix=".php">./tests/</directory>
-			</exclude>
-		</whitelist>
-	</filter>
 </phpunit>

tests/fixtures/storage.xml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8" ?>
+<dataset>
+    <table name="phpbb_storage">
+        <column>file_id</column>
+        <column>file_path</column>
+        <column>storage</column>
+        <column>filesize</column>
+        <row>
+            <value>1</value>
+            <value>foo.png</value>
+            <value>phpbb_pwakit</value>
+            <value>161</value>
+        </row>
+    </table>
+</dataset>