From a60651d52971fd6ae00549fb07079b118a1dee7e Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Thu, 18 Jan 2024 07:49:02 -0800 Subject: [PATCH 1/6] Additional minor updates Signed-off-by: Matt Friedman --- .github/workflows/tests.yml | 4 ++++ composer.json | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index f9a9965..6cc6a2b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -125,6 +125,8 @@ jobs: db: "mysql:5.7" - php: '8.2' db: "mysql:5.7" + - php: '8.3' + db: "mysql:5.7" name: PHP ${{ matrix.php }} - ${{ matrix.db_alias != '' && matrix.db_alias || matrix.db }} @@ -266,6 +268,8 @@ jobs: db: "postgres:14" - php: '8.2' db: "postgres:14" + - php: '8.3' + db: "postgres:14" name: PHP ${{ matrix.php }} - ${{ matrix.db }} diff --git a/composer.json b/composer.json index 5ed16ee..a88adc8 100644 --- a/composer.json +++ b/composer.json @@ -32,7 +32,8 @@ "version-check": { "host": "www.phpbb.com", "directory": "/customise/db/extension/topicprefixes", - "filename": "version_check" + "filename": "version_check", + "ssl": true } } } From c0b90e919e6e5a24ac7bf8df07c3a77cfce4e904 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Thu, 18 Jan 2024 09:42:11 -0800 Subject: [PATCH 2/6] Handle empty prefix tag on form entry in ACP Signed-off-by: Matt Friedman --- controller/admin_controller.php | 6 +++++- tests/controller/add_prefix_test.php | 28 ++++++++++++++++++---------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/controller/admin_controller.php b/controller/admin_controller.php index 2fc85f6..c02b29e 100644 --- a/controller/admin_controller.php +++ b/controller/admin_controller.php @@ -155,7 +155,11 @@ public function add_prefix() $tag = $this->request->variable('prefix_tag', '', true); $prefix = $this->manager->add_prefix($tag, $this->forum_id); - $this->log($prefix['prefix_tag'], 'ACP_LOG_PREFIX_ADDED'); + + if ($prefix) + { + $this->log($prefix['prefix_tag'], 'ACP_LOG_PREFIX_ADDED'); + } } } diff --git a/tests/controller/add_prefix_test.php b/tests/controller/add_prefix_test.php index 09978bd..0fa9d55 100644 --- a/tests/controller/add_prefix_test.php +++ b/tests/controller/add_prefix_test.php @@ -25,21 +25,23 @@ class add_prefix_test extends admin_controller_base */ public function data_add_prefix() { - return array( - array(true, true), - array(true, false), - array(false, false), - ); + return [ + ['', true, true], + ['topic_prefix1', true, true], + ['topic_prefix2', true, false], + ['topic_prefix3', false, false], + ]; } /** * Test add_prefix() * * @dataProvider data_add_prefix + * @param $prefix * @param $submit * @param $valid_form */ - public function test_add_prefix($submit, $valid_form) + public function test_add_prefix($prefix, $submit, $valid_form) { if ($submit) { @@ -59,13 +61,19 @@ public function test_add_prefix($submit, $valid_form) } else { + $valid_prefix = $prefix !== ''; + $this->request->expects(static::once()) + ->method('variable') + ->willReturnMap([ + ['prefix_tag', '', true, \phpbb\request\request_interface::REQUEST, $prefix], + ]); $this->manager->expects(static::once()) ->method('add_prefix') - ->willReturn(['prefix_tag' => 'topic_prefix']); - $this->log->expects(static::once()) + ->willReturn($valid_prefix ? ['prefix_tag' => $prefix] : false); + $this->log->expects($valid_prefix ? static::once() : static::never()) ->method('add') - ->with('admin', static::anything(), static::anything(), 'ACP_LOG_PREFIX_ADDED', static::anything(), ['topic_prefix', 'Test Forum']); - $this->db->expects(static::once()) + ->with('admin', static::anything(), static::anything(), 'ACP_LOG_PREFIX_ADDED', static::anything(), [$prefix, 'Test Forum']); + $this->db->expects($valid_prefix ? static::once() : static::never()) ->method('sql_fetchrow') ->willReturn(['forum_name' => 'Test Forum']); } From c1abae38aafe87f303cdb612baf3da29d719ff26 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Thu, 18 Jan 2024 09:54:08 -0800 Subject: [PATCH 3/6] Clean up trigger error tests Signed-off-by: Matt Friedman --- tests/controller/add_prefix_test.php | 6 +----- tests/controller/delete_prefix_test.php | 12 ++---------- tests/controller/edit_prefix_test.php | 12 ++---------- tests/controller/move_prefix_test.php | 12 ++---------- 4 files changed, 7 insertions(+), 35 deletions(-) diff --git a/tests/controller/add_prefix_test.php b/tests/controller/add_prefix_test.php index 0fa9d55..637e2af 100644 --- a/tests/controller/add_prefix_test.php +++ b/tests/controller/add_prefix_test.php @@ -53,11 +53,7 @@ public function test_add_prefix($prefix, $submit, $valid_form) if (!$valid_form) { - // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions - $exceptionName = version_compare(PHP_VERSION, '8.0', '<') ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; - $errno = version_compare(PHP_VERSION, '8.0', '<') ? E_USER_WARNING : E_WARNING; - $this->expectException($exceptionName); - $this->expectExceptionCode($errno); + $this->setExpectedTriggerError(E_USER_WARNING, $this->language->lang('FORM_INVALID')); } else { diff --git a/tests/controller/delete_prefix_test.php b/tests/controller/delete_prefix_test.php index 0c9ed45..e03dd9e 100644 --- a/tests/controller/delete_prefix_test.php +++ b/tests/controller/delete_prefix_test.php @@ -63,11 +63,7 @@ public function test_delete_prefix($prefix_id, $confirm) ->will(self::throwException(new \OutOfBoundsException())); $this->log->expects(self::never()) ->method('add'); - // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions - $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; - $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; - $this->expectException($exceptionName); - $this->expectExceptionCode($errno); + $this->setExpectedTriggerError(E_USER_WARNING); } else { @@ -83,11 +79,7 @@ public function test_delete_prefix($prefix_id, $confirm) $this->db->expects(static::once()) ->method('sql_fetchrow') ->willReturn(['forum_name' => 'Test Forum']); - // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions - $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; - $errno = PHP_VERSION_ID < 80000 ? E_USER_NOTICE : E_WARNING; - $this->expectException($exceptionName); - $this->expectExceptionCode($errno); + $this->setExpectedTriggerError(E_USER_NOTICE, $this->language->lang('TOPIC_PREFIX_DELETED')); } $this->controller->delete_prefix($prefix_id); diff --git a/tests/controller/edit_prefix_test.php b/tests/controller/edit_prefix_test.php index 04c34e2..7407c45 100644 --- a/tests/controller/edit_prefix_test.php +++ b/tests/controller/edit_prefix_test.php @@ -55,11 +55,7 @@ public function test_edit_prefix($prefix_id, $valid_form, $is_ajax) ->method('get_prefix'); $this->manager->expects(self::never()) ->method('update_prefix'); - // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions - $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; - $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; - $this->expectException($exceptionName); - $this->expectExceptionCode($errno); + $this->setExpectedTriggerError(E_USER_WARNING, $this->language->lang('FORM_INVALID')); } else if ($prefix_id === 0) { @@ -70,11 +66,7 @@ public function test_edit_prefix($prefix_id, $valid_form, $is_ajax) $this->manager->expects(self::once()) ->method('update_prefix') ->will(self::throwException(new \OutOfBoundsException)); - // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions - $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; - $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; - $this->expectException($exceptionName); - $this->expectExceptionCode($errno); + $this->setExpectedTriggerError(E_USER_WARNING); } else { diff --git a/tests/controller/move_prefix_test.php b/tests/controller/move_prefix_test.php index 8ebd2a5..5834a37 100644 --- a/tests/controller/move_prefix_test.php +++ b/tests/controller/move_prefix_test.php @@ -58,21 +58,13 @@ public function test_move_prefix($prefix_id, $direction, $valid_form, $is_ajax) if (!$valid_form) { $prefix_id = 0; - // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions - $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; - $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; - $this->expectException($exceptionName); - $this->expectExceptionCode($errno); + $this->setExpectedTriggerError(E_USER_WARNING, $this->language->lang('FORM_INVALID')); $this->manager->expects(static::never()) ->method('move_prefix'); } else if ($prefix_id === 0) { - // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions - $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; - $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; - $this->expectException($exceptionName); - $this->expectExceptionCode($errno); + $this->setExpectedTriggerError(E_USER_WARNING); $this->manager->expects(static::once()) ->method('move_prefix') ->with(static::equalTo(0), static::stringContains($direction)) From 8083ffd319df3e41372f5dc5d225a490b750c9e2 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Thu, 18 Jan 2024 10:06:33 -0800 Subject: [PATCH 4/6] Fix tests Signed-off-by: Matt Friedman --- tests/controller/add_prefix_test.php | 2 +- tests/controller/delete_prefix_test.php | 2 +- tests/controller/edit_prefix_test.php | 2 +- tests/controller/move_prefix_test.php | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/controller/add_prefix_test.php b/tests/controller/add_prefix_test.php index 637e2af..d36db11 100644 --- a/tests/controller/add_prefix_test.php +++ b/tests/controller/add_prefix_test.php @@ -53,7 +53,7 @@ public function test_add_prefix($prefix, $submit, $valid_form) if (!$valid_form) { - $this->setExpectedTriggerError(E_USER_WARNING, $this->language->lang('FORM_INVALID')); + $this->setExpectedTriggerError(E_USER_WARNING, 'The submitted form was invalid. Try submitting again.'); } else { diff --git a/tests/controller/delete_prefix_test.php b/tests/controller/delete_prefix_test.php index e03dd9e..8b70441 100644 --- a/tests/controller/delete_prefix_test.php +++ b/tests/controller/delete_prefix_test.php @@ -79,7 +79,7 @@ public function test_delete_prefix($prefix_id, $confirm) $this->db->expects(static::once()) ->method('sql_fetchrow') ->willReturn(['forum_name' => 'Test Forum']); - $this->setExpectedTriggerError(E_USER_NOTICE, $this->language->lang('TOPIC_PREFIX_DELETED')); + $this->setExpectedTriggerError(E_USER_NOTICE, 'TOPIC_PREFIX_DELETED'); } $this->controller->delete_prefix($prefix_id); diff --git a/tests/controller/edit_prefix_test.php b/tests/controller/edit_prefix_test.php index 7407c45..581fd3a 100644 --- a/tests/controller/edit_prefix_test.php +++ b/tests/controller/edit_prefix_test.php @@ -55,7 +55,7 @@ public function test_edit_prefix($prefix_id, $valid_form, $is_ajax) ->method('get_prefix'); $this->manager->expects(self::never()) ->method('update_prefix'); - $this->setExpectedTriggerError(E_USER_WARNING, $this->language->lang('FORM_INVALID')); + $this->setExpectedTriggerError(E_USER_WARNING, 'The submitted form was invalid. Try submitting again.'); } else if ($prefix_id === 0) { diff --git a/tests/controller/move_prefix_test.php b/tests/controller/move_prefix_test.php index 5834a37..d5317ed 100644 --- a/tests/controller/move_prefix_test.php +++ b/tests/controller/move_prefix_test.php @@ -58,7 +58,7 @@ public function test_move_prefix($prefix_id, $direction, $valid_form, $is_ajax) if (!$valid_form) { $prefix_id = 0; - $this->setExpectedTriggerError(E_USER_WARNING, $this->language->lang('FORM_INVALID')); + $this->setExpectedTriggerError(E_USER_WARNING, 'The submitted form was invalid. Try submitting again.'); $this->manager->expects(static::never()) ->method('move_prefix'); } From a747b1703ae0a6729837d12fa13b40028cc1ab93 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Thu, 18 Jan 2024 10:15:50 -0800 Subject: [PATCH 5/6] Revert "Clean up trigger error tests" This reverts commit c1abae38aafe87f303cdb612baf3da29d719ff26. --- tests/controller/add_prefix_test.php | 6 +++++- tests/controller/delete_prefix_test.php | 12 ++++++++++-- tests/controller/edit_prefix_test.php | 12 ++++++++++-- tests/controller/move_prefix_test.php | 12 ++++++++++-- 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/tests/controller/add_prefix_test.php b/tests/controller/add_prefix_test.php index d36db11..0fa9d55 100644 --- a/tests/controller/add_prefix_test.php +++ b/tests/controller/add_prefix_test.php @@ -53,7 +53,11 @@ public function test_add_prefix($prefix, $submit, $valid_form) if (!$valid_form) { - $this->setExpectedTriggerError(E_USER_WARNING, 'The submitted form was invalid. Try submitting again.'); + // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions + $exceptionName = version_compare(PHP_VERSION, '8.0', '<') ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; + $errno = version_compare(PHP_VERSION, '8.0', '<') ? E_USER_WARNING : E_WARNING; + $this->expectException($exceptionName); + $this->expectExceptionCode($errno); } else { diff --git a/tests/controller/delete_prefix_test.php b/tests/controller/delete_prefix_test.php index 8b70441..0c9ed45 100644 --- a/tests/controller/delete_prefix_test.php +++ b/tests/controller/delete_prefix_test.php @@ -63,7 +63,11 @@ public function test_delete_prefix($prefix_id, $confirm) ->will(self::throwException(new \OutOfBoundsException())); $this->log->expects(self::never()) ->method('add'); - $this->setExpectedTriggerError(E_USER_WARNING); + // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions + $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; + $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; + $this->expectException($exceptionName); + $this->expectExceptionCode($errno); } else { @@ -79,7 +83,11 @@ public function test_delete_prefix($prefix_id, $confirm) $this->db->expects(static::once()) ->method('sql_fetchrow') ->willReturn(['forum_name' => 'Test Forum']); - $this->setExpectedTriggerError(E_USER_NOTICE, 'TOPIC_PREFIX_DELETED'); + // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions + $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; + $errno = PHP_VERSION_ID < 80000 ? E_USER_NOTICE : E_WARNING; + $this->expectException($exceptionName); + $this->expectExceptionCode($errno); } $this->controller->delete_prefix($prefix_id); diff --git a/tests/controller/edit_prefix_test.php b/tests/controller/edit_prefix_test.php index 581fd3a..04c34e2 100644 --- a/tests/controller/edit_prefix_test.php +++ b/tests/controller/edit_prefix_test.php @@ -55,7 +55,11 @@ public function test_edit_prefix($prefix_id, $valid_form, $is_ajax) ->method('get_prefix'); $this->manager->expects(self::never()) ->method('update_prefix'); - $this->setExpectedTriggerError(E_USER_WARNING, 'The submitted form was invalid. Try submitting again.'); + // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions + $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; + $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; + $this->expectException($exceptionName); + $this->expectExceptionCode($errno); } else if ($prefix_id === 0) { @@ -66,7 +70,11 @@ public function test_edit_prefix($prefix_id, $valid_form, $is_ajax) $this->manager->expects(self::once()) ->method('update_prefix') ->will(self::throwException(new \OutOfBoundsException)); - $this->setExpectedTriggerError(E_USER_WARNING); + // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions + $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; + $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; + $this->expectException($exceptionName); + $this->expectExceptionCode($errno); } else { diff --git a/tests/controller/move_prefix_test.php b/tests/controller/move_prefix_test.php index d5317ed..8ebd2a5 100644 --- a/tests/controller/move_prefix_test.php +++ b/tests/controller/move_prefix_test.php @@ -58,13 +58,21 @@ public function test_move_prefix($prefix_id, $direction, $valid_form, $is_ajax) if (!$valid_form) { $prefix_id = 0; - $this->setExpectedTriggerError(E_USER_WARNING, 'The submitted form was invalid. Try submitting again.'); + // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions + $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; + $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; + $this->expectException($exceptionName); + $this->expectExceptionCode($errno); $this->manager->expects(static::never()) ->method('move_prefix'); } else if ($prefix_id === 0) { - $this->setExpectedTriggerError(E_USER_WARNING); + // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions + $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; + $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; + $this->expectException($exceptionName); + $this->expectExceptionCode($errno); $this->manager->expects(static::once()) ->method('move_prefix') ->with(static::equalTo(0), static::stringContains($direction)) From 9954a9cc73923c9870490f5b39f7ff4383e36756 Mon Sep 17 00:00:00 2001 From: Matt Friedman Date: Thu, 18 Jan 2024 10:24:06 -0800 Subject: [PATCH 6/6] Simplify test code Signed-off-by: Matt Friedman --- tests/controller/add_prefix_test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/controller/add_prefix_test.php b/tests/controller/add_prefix_test.php index 0fa9d55..d239e56 100644 --- a/tests/controller/add_prefix_test.php +++ b/tests/controller/add_prefix_test.php @@ -54,8 +54,8 @@ public function test_add_prefix($prefix, $submit, $valid_form) if (!$valid_form) { // Throws E_WARNING in PHP 8.0+ and E_USER_WARNING in earlier versions - $exceptionName = version_compare(PHP_VERSION, '8.0', '<') ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; - $errno = version_compare(PHP_VERSION, '8.0', '<') ? E_USER_WARNING : E_WARNING; + $exceptionName = PHP_VERSION_ID < 80000 ? \PHPUnit\Framework\Error\Error::class : \PHPUnit\Framework\Error\Warning::class; + $errno = PHP_VERSION_ID < 80000 ? E_USER_WARNING : E_WARNING; $this->expectException($exceptionName); $this->expectExceptionCode($errno); }