N°7157 - Add unit test for subscription mechanism and refactor repository methods

2026-02-13 07:24:13 +01:00 · 2024-03-08 12:22:16 +01:00
parent 509df0939e
commit 2aa1385958
4 changed files with 222 additions and 47 deletions
--- a/sources/Controller/Notifications/NotificationsCenterController.php
+++ b/sources/Controller/Notifications/NotificationsCenterController.php
@@ -70,7 +70,7 @@ class NotificationsCenterController extends Controller
 		];

 		// Get all subscribed/unsubscribed actions notifications for the current user
-		$oLnkNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionByContact(\UserRights::GetContactId());
+		$oLnkNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionsByContact(\UserRights::GetContactId());
 		$oActionsNotificationsByTrigger = [];
 		$aSubscribedActionsNotificationsByTrigger = [];
 		while ($oLnkActionsNotifications = $oLnkNotificationsSet->Fetch()) {
@@ -279,7 +279,7 @@ JS
 		$oPage->AddUiBlock($oNotificationsPanel);

 		// Get all subscribed/unsubscribed actions notifications for the current user
-		$oLnkNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionByContact(\UserRights::GetContactId());
+		$oLnkNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionsByContact(\UserRights::GetContactId());
 		$oActionsNotificationsByTrigger = [];
 		$aSubscribedActionsNotificationsByTrigger = [];
 		while ($oLnkActionsNotifications = $oLnkNotificationsSet->Fetch()) {
@@ -390,8 +390,7 @@ JS
 			if ($oActionNotification === null) {
 				throw new \Exception('Invalid action notification');
 			}
-			$oSubscribedActionsNotificationsSearch = \DBObjectSearch::FromOQL("SELECT lnkActionNotificationToContact AS lnk WHERE lnk.action_id = :actionnotification_id AND lnk.contact_id = :contact_id AND lnk.trigger_id = :trigger_id AND lnk.subscribed = '1'");
-			$oSubscribedActionsNotificationsSet = new \DBObjectSet($oSubscribedActionsNotificationsSearch, array(), array('actionnotification_id' => $iActionNotificationKey, 'contact_id' => \UserRights::GetContactId(), 'trigger_id' => $iTriggerKey));
+			$oSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscribedSubscriptionsByTriggerContactAndAction($iTriggerKey, $iActionNotificationKey);
 			if ($oSubscribedActionsNotificationsSet->Count() === 0) {
 				throw new \Exception('You are not subscribed to this channel');
 			}
@@ -443,8 +442,7 @@ JS
 			if ($oActionNotification === null) {
 				throw new \Exception('Invalid action notification');
 			}
-			$oSubscribedActionsNotificationsSearch = \DBObjectSearch::FromOQL("SELECT lnkActionNotificationToContact AS lnk WHERE lnk.action_id = :actionnotification_id AND lnk.contact_id = :contact_id AND lnk.trigger_id = :trigger_id AND lnk.subscribed = '0'");
-			$oSubscribedActionsNotificationsSet = new \DBObjectSet($oSubscribedActionsNotificationsSearch, array(), array('actionnotification_id' => $iActionNotificationKey, 'contact_id' => \UserRights::GetContactId(), 'trigger_id' => $iTriggerKey));
+			$oSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchUnsubscribedSubscriptionsByTriggerContactAndAction($iTriggerKey, $iActionNotificationKey);
 			if ($oSubscribedActionsNotificationsSet->Count() === 0) {
 				throw new \Exception('You are not allow to subscribe to this channel');
 			}
@@ -501,13 +499,13 @@ JS
 			}
 			
 			// Check if we are subscribed to at least 1 channel
-			$oSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionByTriggerContactSubscriptionAndFinalclass($iTriggerId, \UserRights::GetContactId(), '1', $sFinalclass);
+			$oSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionsByTriggerContactSubscriptionAndFinalclass($iTriggerId, \UserRights::GetContactId(), '1', $sFinalclass);
 			if ($oSubscribedActionsNotificationsSet->Count() === 0) {
 				throw new \Exception('You are not subscribed to any channel');
 			}
 			// Check the trigger subscription policy and if we are subscribed to at least 1 channel if necessary
 			if($oTrigger->Get('subscription_policy') === SubscriptionPolicy::ForceAtLeastOneChannel->value) {
-				$oTotalSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionByTriggerContactAndSubscription($iTriggerId, \UserRights::GetContactId(), '1');
+				$oTotalSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionsByTriggerContactAndSubscription($iTriggerId, \UserRights::GetContactId(), '1');
 				if (($oTotalSubscribedActionsNotificationsSet->Count() - $oSubscribedActionsNotificationsSet->Count()) === 0) {
 					throw new \Exception('You can\'t unsubscribe from this channel, you must be subscribed to at least one channel');
 				}
@@ -561,7 +559,7 @@ JS
 			if ($oTrigger === null) {
 				throw new \Exception('Invalid trigger');
 			}
-			$oSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionByTriggerContactSubscriptionAndFinalclass($iTriggerId, \UserRights::GetContactId(), '0', $sFinalclass);
+			$oSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionsByTriggerContactSubscriptionAndFinalclass($iTriggerId, \UserRights::GetContactId(), '0', $sFinalclass);
 			if ($oSubscribedActionsNotificationsSet->Count() === 0) {
 				throw new \Exception('You are not subscribed to any channel');
 			}
--- a/sources/Service/Notification/NotificationsRepository.php
+++ b/sources/Service/Notification/NotificationsRepository.php
@@ -2,12 +2,11 @@

 namespace Combodo\iTop\Service\Notification;

-use BinaryExpression;
 use DBObjectSearch;
 use DBObjectSet;
+use DBSearch;
 use Expression;
-use FieldExpression;
-use VariableExpression;
+use UserRights;

 /**
 * Class NotificationsRepository
@@ -58,17 +57,8 @@ class NotificationsRepository
 	 */
 	public function SearchNotificationsByContact(int $iContactId, array $aNotificationIds = []): DBObjectSet
 	{
-		$oSearch = DBObjectSearch::FromOQL("SELECT EventiTopNotification WHERE contact_id = :contact_id");
-		$aParams = [
-			"contact_id" => $iContactId,
-		];
-
-		if (count($aNotificationIds) > 0) {
-			$oSearch->AddConditionExpression(Expression::FromOQL("{$oSearch->GetClassAlias()}.id IN (:notification_ids)"));
-			$aParams["notification_ids"] = $aNotificationIds;
-		}
-
-		return new DBObjectSet($oSearch, [], $aParams);
+		$oSearch = $this->PrepareSearchForNotificationsByContact($iContactId, $aNotificationIds);
+		return new DBObjectSet($oSearch);
 	}

 	/**
@@ -81,10 +71,10 @@ class NotificationsRepository
 	 */
 	public function SearchNotificationsToMarkAsReadByContact(int $iContactId, array $aNotificationIds = []): DBObjectSet
 	{
-		$oSet = $this->SearchNotificationsByContact($iContactId, $aNotificationIds);
-		$oSet->GetFilter()->AddCondition("read", "=", "no");
+		$oSearch = $this->PrepareSearchForNotificationsByContact($iContactId, $aNotificationIds);
+		$oSearch->AddCondition("read", "no");

-		return $oSet;
+		return new DBObjectSet($oSearch);
 	}

 	/**
@@ -97,10 +87,10 @@ class NotificationsRepository
 	 */
 	public function SearchNotificationsToMarkAsUnreadByContact(int $iContactId, array $aNotificationIds = []): DBObjectSet
 	{
-		$oSet = $this->SearchNotificationsByContact($iContactId, $aNotificationIds);
-		$oSet->GetFilter()->AddCondition("read", "=", "yes");
+		$oSearch = $this->PrepareSearchForNotificationsByContact($iContactId, $aNotificationIds);
+		$oSearch->AddCondition("read", "yes");

-		return $oSet;
+		return new DBObjectSet($oSearch);
 	}

 	/**
@@ -116,6 +106,31 @@ class NotificationsRepository
 		return $this->SearchNotificationsByContact($iContactId, $aNotificationIds);
 	}

+	/**
+	 * @param int $iContactId ID of the contact to retrieve notifications for
+	 * @param array $aNotificationIds Optional IDs of the notifications to retrieve, if omitted all notifications will be retrieved
+	 *
+	 * @internal
+	 * @return \DBSearch
+	 * @throws \OQLException
+	 */
+	protected function PrepareSearchForNotificationsByContact(int $iContactId, array $aNotificationIds = []): DBSearch
+	{
+		$oSearch = DBObjectSearch::FromOQL("SELECT EventiTopNotification WHERE contact_id = :contact_id");
+		$aParams = [
+			"contact_id" => $iContactId,
+		];
+
+		if (count($aNotificationIds) > 0) {
+			$oSearch->AddConditionExpression(Expression::FromOQL("{$oSearch->GetClassAlias()}.id IN (:notification_ids)"));
+			$aParams["notification_ids"] = $aNotificationIds;
+		}
+
+		$oSearch->SetInternalParams($aParams);
+
+		return $oSearch;
+	}
+
 	/**
 	 * Search for subscriptions by contact ID.
 	 *
@@ -123,29 +138,86 @@ class NotificationsRepository
 	 *
 	 * @return DBObjectSet The result set of subscriptions associated with the contact.
 	 */
-	public function SearchSubscriptionByContact(int $iContactId): DBObjectSet
+	public function SearchSubscriptionsByContact(int $iContactId): DBObjectSet
 	{
 		$oSearch = DBObjectSearch::FromOQL("SELECT lnkActionNotificationToContact AS lnk WHERE lnk.contact_id = :contact_id");
-		$oSet = new DBObjectSet($oSearch, array(), array('contact_id' => $iContactId));
+		$oSearch->SetInternalParams([
+			"contact_id" => $iContactId
+		]);

-		return $oSet;
+		return new DBObjectSet($oSearch);
 	}

 	/**
-	 * Searches for a subscription by trigger, contact, and action.
+	 * Searches for subscriptions by trigger, contact, and action; no matter it subscription status.
 	 *
 	 * @param int $iTriggerId The ID of the trigger.
-	 * @param int $iContactId The ID of the contact.
-	 * @param int $iActionID The ID of the action.
+	 * @param int $iActionId The ID of the action.
+	 * @param int|null $iContactId The ID of the contact. If null, current user will be used.
 	 *
 	 * @return DBObjectSet The set of subscriptions matching the given trigger, contact, and action.
 	 */
-	public function SearchSubscriptionByTriggerContactAndAction(int $iTriggerId, int $iContactId, int $iActionID): DBObjectSet
+	public function SearchSubscriptionsByTriggerContactAndAction(int $iTriggerId, int $iActionId, int|null $iContactId = null): DBObjectSet
 	{
-		$oSearch = DBObjectSearch::FromOQL("SELECT lnkActionNotificationToContact AS lnk WHERE lnk.contact_id = :contact_id AND lnk.trigger_id = :trigger_id AND lnk.action_id = :action_id");
-		$oSet = new DBObjectSet($oSearch, array(), array('trigger_id' => $iTriggerId, 'contact_id' => $iContactId, 'action_id' => $iActionID));
+		$oSearch = $this->PrepareSearchForSubscriptionsByTriggerContactAndAction($iTriggerId, $iActionId, $iContactId);
+		return new DBObjectSet($oSearch);
+	}

-		return $oSet;
+	/**
+	 * Searches for subscriptions by trigger, contact, and action; where contact is subscribed.
+	 *
+	 * @param int $iTriggerId The ID of the trigger.
+	 * @param int $iActionId The ID of the action.
+	 * @param int|null $iContactId The ID of the contact. If null, current user will be used.
+	 *
+	 * @return DBObjectSet The set of subscriptions matching the given trigger, contact, and action.
+	 */
+	public function SearchSubscribedSubscriptionsByTriggerContactAndAction(int $iTriggerId, int $iActionId, int|null $iContactId = null): DBObjectSet
+	{
+		$oSearch = $this->PrepareSearchForSubscriptionsByTriggerContactAndAction($iTriggerId, $iActionId, $iContactId);
+		$oSearch->AddCondition("subscribed", "1");
+		return new DBObjectSet($oSearch);
+	}
+
+	/**
+	 * Searches for subscriptions by trigger, contact, and action; where contact is unsubscribed.
+	 *
+	 * @param int $iTriggerId The ID of the trigger.
+	 * @param int $iActionId The ID of the action.
+	 * @param int|null $iContactId The ID of the contact. If null, current user will be used.
+	 *
+	 * @return DBObjectSet The set of subscriptions matching the given trigger, contact, and action.
+	 */
+	public function SearchUnsubscribedSubscriptionsByTriggerContactAndAction(int $iTriggerId, int $iActionId, int $iContactId = null): DBObjectSet
+	{
+		$oSearch = $this->PrepareSearchForSubscriptionsByTriggerContactAndAction($iTriggerId, $iActionId, $iContactId);
+		$oSearch->AddCondition("subscribed", "0");
+		return new DBObjectSet($oSearch);
+	}
+
+	/**
+	 * @param int $iTriggerId The ID of the trigger.
+	 * @param int $iActionId The ID of the action.
+	 * @param int|null $iContactId The ID of the contact. If null, current user will be used.
+	 *
+	 * @internal
+	 * @return \DBSearch Search for subscriptions given tuple of $iTriggerId, $iActionId and $iContactId
+	 * @throws \OQLException
+	 */
+	protected function PrepareSearchForSubscriptionsByTriggerContactAndAction(int $iTriggerId, int $iActionId, int|null $iContactId = null): DBSearch
+	{
+		if (null === $iContactId) {
+			$iContactId = UserRights::GetContactId();
+		}
+
+		$oSearch = DBObjectSearch::FromOQL("SELECT lnkActionNotificationToContact AS lnk WHERE lnk.contact_id = :contact_id AND lnk.trigger_id = :trigger_id AND lnk.action_id = :action_id");
+		$oSearch->SetInternalParams([
+			"contact_id" => $iContactId,
+			"trigger_id" => $iTriggerId,
+			"action_id" => $iActionId,
+		]);
+
+		return $oSearch;
 	}

 	/**
@@ -157,12 +229,16 @@ class NotificationsRepository
 	 *
 	 * @return DBObjectSet A set of subscription objects matching the given parameters.
 	 */
-	public function SearchSubscriptionByTriggerContactAndSubscription(int $iTriggerId, int $iContactId, string $sSubscription): DBObjectSet
+	public function SearchSubscriptionsByTriggerContactAndSubscription(int $iTriggerId, int $iContactId, string $sSubscription): DBObjectSet
 	{
 		$oSearch = DBObjectSearch::FromOQL("SELECT lnkActionNotificationToContact AS lnk WHERE lnk.contact_id = :contact_id AND lnk.trigger_id = :trigger_id AND lnk.subscribed = :subscription");
-		$oSet = new DBObjectSet($oSearch, array(), array('trigger_id' => $iTriggerId, 'contact_id' => $iContactId, 'subscription' => $sSubscription));
+		$oSearch->SetInternalParams([
+			"trigger_id" => $iTriggerId,
+			"contact_id" => $iContactId,
+			"subscription" => $sSubscription]
+		);

-		return $oSet;
+		return new DBObjectSet($oSearch);
 	}


@@ -176,12 +252,17 @@ class NotificationsRepository
 	 *
 	 * @return DBObjectSet A set of subscription objects matching the given parameters.
 	 */
-	public function SearchSubscriptionByTriggerContactSubscriptionAndFinalclass(int $iTriggerId, int $iContactId, int $sSubscription, string $sFinalclass): DBObjectSet
+	public function SearchSubscriptionsByTriggerContactSubscriptionAndFinalclass(int $iTriggerId, int $iContactId, int $sSubscription, string $sFinalclass): DBObjectSet
 	{
 		$oSearch = DBObjectSearch::FromOQL("SELECT lnkActionNotificationToContact AS lnk JOIN ActionNotification AS an ON lnk.action_id = an.id WHERE lnk.contact_id = :contact_id AND lnk.trigger_id = :trigger_id AND lnk.subscribed = :subscription AND an.finalclass = :finalclass");
-		$oSet = new DBObjectSet($oSearch, array(), array('trigger_id' => $iTriggerId, 'contact_id' => $iContactId, 'subscription' => $sSubscription, 'finalclass' => $sFinalclass));
+		$oSearch->SetInternalParams([
+			"trigger_id" => $iTriggerId,
+			"contact_id" => $iContactId,
+			"subscription" => $sSubscription,
+			"finalclass" => $sFinalclass
+		]);

-		return $oSet;
+		return new DBObjectSet($oSearch);
 	}

 	public function GetSearchOQLContactUnsubscribedByTriggerAndAction(): DBObjectSearch
--- a/sources/Service/Notification/NotificationsService.php
+++ b/sources/Service/Notification/NotificationsService.php
@@ -65,7 +65,7 @@ class NotificationsService {
 	public function RegisterSubscription(Trigger $oTrigger, ActionNotification $oActionNotification, Contact $oRecipient): void
 	{
 		// Check if the user is already subscribed to the action notification
-		$oSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionByTriggerContactAndAction($oTrigger->GetKey(), $oRecipient->GetKey(), $oActionNotification->GetKey());
+		$oSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionsByTriggerContactAndAction($oTrigger->GetKey(), $oActionNotification->GetKey(), $oRecipient->GetKey());
 		if ($oSubscribedActionsNotificationsSet->Count() === 0) {
 			// Create a new subscription
 			$oSubscribedActionsNotifications = new lnkActionNotificationToContact();
@@ -104,7 +104,7 @@ class NotificationsService {
 			return true;
 		}
 		// Check if the user is already subscribed to the action notification
-		$oSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionByTriggerContactAndAction($oTrigger->GetKey(), $oRecipient->GetKey(), $oActionNotification->GetKey());
+		$oSubscribedActionsNotificationsSet = NotificationsRepository::GetInstance()->SearchSubscriptionsByTriggerContactAndAction($oTrigger->GetKey(), $oActionNotification->GetKey(), $oRecipient->GetKey());
 		if ($oSubscribedActionsNotificationsSet->Count() === 0) {
 			return true;
 		}
--- a/tests/php-unit-tests/unitary-tests/sources/Service/Notification/NotificationsServiceTest.php
+++ b/tests/php-unit-tests/unitary-tests/sources/Service/Notification/NotificationsServiceTest.php
@@ -0,0 +1,96 @@
+<?php
+
+/*
+ * @copyright   Copyright (C) 2010-2024 Combodo SAS
+ * @license     http://opensource.org/licenses/AGPL-3.0
+ */
+
+
+namespace Combodo\iTop\Test\UnitTest\Service\Notification;
+
+
+use Combodo\iTop\Core\Trigger\Enum\SubscriptionPolicy;
+use Combodo\iTop\Service\Notification\NotificationsRepository;
+use Combodo\iTop\Service\Notification\NotificationsService;
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+
+/**
+ * Class NotificationsServiceTest
+ *
+ * @author Guillaume Lajarige <guillaume.lajarige@combodo.com>
+ * @package Combodo\iTop\Test\UnitTest\Service\Notification
+ * @covers \Combodo\iTop\Service\Notification\NotificationsService
+ */
+class NotificationsServiceTest extends ItopDataTestCase {
+	public const CREATE_TEST_ORG = true;
+
+	/**
+	 * @covers \Combodo\iTop\Service\Notification\NotificationsService::IsSubscribed
+	 * @return void
+	 */
+	public function testIsSubscribed(): void
+	{
+		// Prepare base data
+		/** @var \Contact $oPerson */
+		$oPerson = $this->createObject(\Person::class, [
+			"name" => "Khalo",
+			"first_name" => "Frida",
+			"org_id" => $this->getTestOrgId(),
+		]);
+		$iPersonID = $oPerson->GetKey();
+		/** @var \Location $oLocation */
+		$oLocation = $this->createObject(\Location::class, [
+			"name" => "Test location",
+			"org_id" => $this->getTestOrgId(),
+		]);
+		/** @var \Trigger $oTrigger */
+		$oTrigger = $this->createObject(\TriggerOnObjectCreate::class, [
+			"description" => "Test trigger",
+			"subscription_policy" => SubscriptionPolicy::AllowNoChannel->value,
+		]);
+		$iTriggerID = $oTrigger->GetKey();
+		/** @var \ActionNotification $oActionNotification */
+		$oActionNotification = $this->createObject(\ActionEmail::class, [
+			"name" => "Test action",
+			"from" => "test@test.com",
+			"to" => "SELECT Person WHERE id = $iPersonID",
+			"subject" => "Test subject",
+			"body" => "Test body",
+		]);
+		$iActionNotificationID = $oActionNotification->GetKey();
+
+
+		$oService = NotificationsService::GetInstance();
+
+		// Case 1: Person hasn't received action so far, so it is implicitly subscribed by default
+		// - Assert
+		$this->assertTrue($oService->IsSubscribed($oTrigger, $oActionNotification, $oPerson));
+
+
+		// Case 2: Activate an action, the person should have an explicit subscription
+		// - Prepare
+		$oActionNotification->DoExecute($oTrigger, [
+			"this->object()" => $oLocation,
+			"trigger->object()" => $oTrigger,
+		]);
+
+		// - Assert
+		$iSubscribedCount = NotificationsRepository::GetInstance()->SearchSubscribedSubscriptionsByTriggerContactAndAction($iTriggerID, $iActionNotificationID, $iPersonID)->Count();
+		$this->assertEquals(1, $iSubscribedCount, "There should be 1 explicit subscription");
+		$this->assertTrue($oService->IsSubscribed($oTrigger, $oActionNotification, $oPerson));
+
+
+		// Case 3: Unsubscribe, person should have an explicit unsubscribe
+		// - Prepare
+		$oSubscription = NotificationsRepository::GetInstance()->SearchSubscribedSubscriptionsByTriggerContactAndAction($iTriggerID, $iActionNotificationID, $iPersonID)->Fetch();
+		$oSubscription->Set('subscribed', false);
+		$oSubscription->DBUpdate();
+
+		// - Assert
+		$iSubscribedCount = NotificationsRepository::GetInstance()->SearchSubscribedSubscriptionsByTriggerContactAndAction($iTriggerID, $iActionNotificationID, $iPersonID)->Count();
+		$this->assertEquals(0, $iSubscribedCount, "There should be 0 explicit subscription");
+		$iUnsubscribedCount = NotificationsRepository::GetInstance()->SearchUnsubscribedSubscriptionsByTriggerContactAndAction($iTriggerID, $iActionNotificationID, $iPersonID)->Count();
+		$this->assertEquals(1, $iUnsubscribedCount, "There should be 1 explicit unsubscription");
+		$this->assertFalse($oService->IsSubscribed($oTrigger, $oActionNotification, $oPerson));
+	}
+}