Fixes and housekeeping

Signed-off-by: Matt Friedman <maf675@gmail.com>

Author: iMattPro

ext.php

@@ -11,13 +11,13 @@
 namespace phpbb\ideas;
 
 /**
-* This ext class is optional and can be omitted if left empty.
-* However, you can add special (un)installation commands in the
-* methods enable_step(), disable_step() and purge_step(). As it is,
-* these methods are defined in \phpbb\extension\base, which this
-* class extends, but you can overwrite them to give special
-* instructions for those cases.
-*/
+ * This ext class is optional and can be omitted if left empty.
+ * However, you can add special (un)installation commands in the
+ * methods enable_step(), disable_step() and purge_step(). As it is,
+ * these methods are defined in \phpbb\extension\base, which this
+ * class extends, but you can overwrite them to give special
+ * instructions for those cases.
+ */
 class ext extends \phpbb\extension\base
 {
 	public const SORT_AUTHOR = 'author';
@@ -30,45 +30,61 @@ class ext extends \phpbb\extension\base
 	public const SORT_MYIDEAS = 'egosearch';
 	public const SUBJECT_LENGTH = 120;
 	public const NUM_IDEAS = 5;
+	public const NOTIFICATION_TYPE_STATUS = 'phpbb.ideas.notification.type.status';
 
 	/** @var array Idea status names and IDs */
-	public static $statuses = array(
+	public static $statuses = [
 		'NEW'			=> 1,
 		'IN_PROGRESS'	=> 2,
 		'IMPLEMENTED'	=> 3,
 		'DUPLICATE'		=> 4,
 		'INVALID'		=> 5,
-	);
+	];
+
+	/** @var array Cached flipped statuses array */
+	private static $status_names;
 
 	/**
 	 * Return the status name from the status ID.
 	 *
 	 * @param int $id ID of the status.
-	 *
 	 * @return string The status name.
-	 * @static
-	 * @access public
 	 */
 	public static function status_name($id)
 	{
-		return array_flip(self::$statuses)[$id];
+		if (self::$status_names === null)
+		{
+			self::$status_names = array_flip(self::$statuses);
+		}
+
+		return self::$status_names[$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
+	 * Requires PHP >= 7.2.0
 	 *
 	 * @return bool
-	 * @access public
 	 */
 	public function is_enableable()
 	{
-		return !(PHP_VERSION_ID < 70100 ||
-			phpbb_version_compare(PHPBB_VERSION, '3.3.0', '<') ||
-			phpbb_version_compare(PHPBB_VERSION, '4.0.0-dev', '>='));
+		return PHP_VERSION_ID >= 70200
+			&& phpbb_version_compare(PHPBB_VERSION, '3.3.0', '>=')
+			&& phpbb_version_compare(PHPBB_VERSION, '4.0.0-dev', '<');
+	}
+
+	/**
+	 * Handle notification management for extension lifecycle
+	 *
+	 * @param string $method The notification manager method to call
+	 * @return string
+	 */
+	private function handle_notifications($method)
+	{
+		$this->container->get('notification_manager')->$method(self::NOTIFICATION_TYPE_STATUS);
+		return 'notification';
 	}
 
 	/**
@@ -79,15 +95,7 @@ public function is_enableable()
 	 */
 	public function enable_step($old_state)
 	{
-		if ($old_state === false)
-		{
-			$this->container->get('notification_manager')
-				->enable_notifications('phpbb.ideas.notification.type.status');
-
-			return 'notification';
-		}
-
-		return parent::enable_step($old_state);
+		return $old_state === false ? $this->handle_notifications('enable_notifications') : parent::enable_step($old_state);
 	}
 
 	/**
@@ -98,33 +106,17 @@ public function enable_step($old_state)
 	 */
 	public function disable_step($old_state)
 	{
-		if ($old_state === false)
-		{
-			$this->container->get('notification_manager')
-				->disable_notifications('phpbb.ideas.notification.type.status');
-
-			return 'notification';
-		}
-
-		return parent::disable_step($old_state);
+		return $old_state === false ? $this->handle_notifications('disable_notifications') : parent::disable_step($old_state);
 	}
 
 	/**
 	 * Purge notifications for the extension
 	 *
-	 * @param mixed	$old_state
+	 * @param mixed $old_state
 	 * @return bool|string
 	 */
 	public function purge_step($old_state)
 	{
-		if ($old_state === false)
-		{
-			$this->container->get('notification_manager')
-				->purge_notifications('phpbb.ideas.notification.type.status');
-
-			return 'notification';
-		}
-
-		return parent::purge_step($old_state);
+		return $old_state === false ? $this->handle_notifications('purge_notifications') : parent::purge_step($old_state);
 	}
 }

factory/idea.php

@@ -73,7 +73,7 @@ public function set_status($idea_id, $status)
 
 		// Send a notification
 		$method = $this->notification_exists($idea_id) ? 'update_notifications' : 'add_notifications';
-		$this->notification_manager->$method('phpbb.ideas.notification.type.status', [
+		$this->notification_manager->$method(ext::NOTIFICATION_TYPE_STATUS, [
 			'idea_id' => (int) $idea_id,
 			'status'  => (int) $status,
 		]);
@@ -275,7 +275,7 @@ public function delete($id, $topic_id = 0)
 			$this->delete_idea_data($id, $this->table_votes);
 
 			// Delete notifications
-			$this->notification_manager->delete_notifications('phpbb.ideas.notification.type.status', $id);
+			$this->notification_manager->delete_notifications(ext::NOTIFICATION_TYPE_STATUS, $id);
 		}
 
 		return $deleted;
@@ -465,7 +465,7 @@ protected function notification_exists($item_id)
 		$sql = 'SELECT notification_id
 			FROM ' . NOTIFICATIONS_TABLE . '
 			WHERE item_id = ' . (int) $item_id . '
-				AND notification_type_id = ' . $this->notification_manager->get_notification_type_id('phpbb.ideas.notification.type.status');
+				AND notification_type_id = ' . $this->notification_manager->get_notification_type_id(ext::NOTIFICATION_TYPE_STATUS);
 
 		$result = $this->db->sql_query_limit($sql, 1);
 		$row = $this->db->sql_fetchrow($result);

language/en/common.php

@@ -38,7 +38,7 @@
 	'IDEA_DELETED'			=> 'Idea successfully deleted.',
 	'IDEA_LIST'				=> 'Idea List',
 	'IDEA_NOT_FOUND'		=> 'Idea not found',
-	'IDEA_STATUS_CHANGE'	=> 'Your idea <strong>“%1$s”</strong> has been changed to:',
+	'IDEA_STATUS_CHANGE'	=> 'Your idea <strong>“%s”</strong> has been changed to:',
 	'IDEA_STORED_MOD'		=> 'Your idea has been submitted successfully, but it will need to be approved by a moderator before it is publicly viewable. You will be notified when your idea has been approved.<br /><br /><a href="%s">Return to Ideas</a>.',
 	'IDEAS_TITLE'			=> 'phpBB Ideas',
 	'IDEAS_NOT_AVAILABLE'	=> 'Ideas is not available at this time.',

notification/type/status.php

@@ -84,7 +84,7 @@ public function set_additional_services(config $config, helper $helper, idea $id
 	 */
 	public function get_type()
 	{
-		return 'phpbb.ideas.notification.type.status';
+		return ext::NOTIFICATION_TYPE_STATUS;
 	}
 
 	/**

tests/ext_test.php

@@ -69,7 +69,7 @@ private function setup_notification_manager(string $method): void
 
 		$this->notification_manager->expects($this->once())
 			->method($method)
-			->with('phpbb.ideas.notification.type.status');
+			->with(ext::NOTIFICATION_TYPE_STATUS);
 	}
 
 	public function test_is_enableable(): void

tests/ideas/idea_attributes_test.php

@@ -10,6 +10,8 @@
 
 namespace phpbb\ideas\tests\ideas;
 
+use phpbb\ideas\ext;
+
 class idea_attributes_test extends ideas_base
 {
 	/**
@@ -63,7 +65,7 @@ public function test_set_status($idea_id, $status)
 	{
 		$this->notification_manager->expects($this->once())
 			->method('get_notification_type_id')
-			->with('phpbb.ideas.notification.type.status')
+			->with(ext::NOTIFICATION_TYPE_STATUS)
 			->willReturn(1);
 
 		$object = $this->get_idea_object();

tests/notification/status_test.php

@@ -10,6 +10,7 @@
 
 namespace phpbb\ideas\tests\notification\type;
 
+use phpbb\ideas\ext;
 use phpbb\ideas\notification\type\status;
 
 class status_test extends \phpbb_test_case
@@ -89,7 +90,7 @@ protected function setNotificationData(array $data)
 
 	public function test_get_type()
 	{
-		$this->assertEquals('phpbb.ideas.notification.type.status', $this->notification_type->get_type());
+		$this->assertEquals(ext::NOTIFICATION_TYPE_STATUS, $this->notification_type->get_type());
 	}
 
 	public function test_is_available_with_permission()