N°7726 - Fatal error if a newsroom is sent without any message

Finalization of tests

core/datamodel.core.xml

@@ -355,20 +355,30 @@
       }
     }
 
-		if ($bIsAsync === true) {
-		    AsyncSendNewsroom::AddToQueue($this->GetKey(), $oTrigger->GetKey(), $aRecipientsIds, $sMessage, $sTitle, $sUrl, $iObjectId, $sObjectClass);
-    } else {
-      foreach ($aRecipientsIds as $iRecipientId) {
-        $oEvent = Combodo\iTop\Service\Notification\Event\EventNotificationNewsroomService::MakeEventFromAction($this, $iRecipientId, $oTrigger->GetKey(), $sMessage, $sTitle, $sUrl, $iObjectId, $sObjectClass);
-        try {
-          $oEvent->DBInsertNoReload();
-        } catch(CoreCannotSaveObjectException $e) {
-          ExceptionLog::LogException($e);
-          $oEvent = Combodo\iTop\Service\Notification\Event\EventNotificationNewsroomService::MakeEventFromAction($this, $iRecipientId, $oTrigger->GetKey(), Dict::Format('Core:EventNotificationNewsroom:ErrorOnDBInsert'), Dict::Format('Core:EventNotificationNewsroom:ErrorNotificationNotSent'), $sUrl, $iObjectId, $sObjectClass);
-          $oEvent->DBInsertNoReload();
-        }
-      }
-    }
+		try {
+			if ($bIsAsync === true) {
+				AsyncSendNewsroom::AddToQueue($this->GetKey(), $oTrigger->GetKey(), $aRecipientsIds, $sMessage, $sTitle, $sUrl, $iObjectId, $sObjectClass);
+			} else {
+				foreach ($aRecipientsIds as $iRecipientId) {
+					$oEvent = Combodo\iTop\Service\Notification\Event\EventNotificationNewsroomService::MakeEventFromAction($this, $iRecipientId, $oTrigger->GetKey(), $sMessage, $sTitle, $sUrl, $iObjectId, $sObjectClass);
+					$oEvent->DBInsertNoReload();
+				}
+			}
+		} catch (CoreCannotSaveObjectException $e) {
+			ExceptionLog::LogException($e);
+			foreach($aRecipientsIds as $iRecipientId) {
+				$oEvent = Combodo\iTop\Service\Notification\Event\EventNotificationNewsroomService::MakeEventFromAction($this,
+					$iRecipientId,
+					$oTrigger->GetKey(),
+					Dict::S('Core:EventNotificationNewsroom:ErrorOnDBInsert'),
+					Dict::S('Core:EventNotificationNewsroom:ErrorNotificationNotSent'),
+					$sUrl,
+					$iObjectId,
+					$sObjectClass
+				);
+				$oEvent->DBInsertNoReload();
+			}
+		}
 
 		$this->SetNotificationLanguage($sPreviousLanguage, $aPreviousPluginProperties['language_code'] ?? null);
 	}

pages/preferences.php

@@ -24,7 +24,6 @@ use Combodo\iTop\Application\UI\Preferences\BlockShortcuts\BlockShortcuts;
 use Combodo\iTop\Application\WebPage\ErrorPage;
 use Combodo\iTop\Application\WebPage\iTopWebPage;
 use Combodo\iTop\Application\WebPage\WebPage;
-use Combodo\iTop\Controller\Newsroom\iTopNewsroomController;
 use Combodo\iTop\Controller\Notifications\NotificationsCenterController;
 use Combodo\iTop\Service\InterfaceDiscovery\InterfaceDiscovery;
 use Combodo\iTop\Service\Router\Router;
@@ -34,7 +33,6 @@ require_once(APPROOT.'/application/application.inc.php');
 require_once(APPROOT.'/application/startup.inc.php');
 IssueLog::Trace('----- Request: '.utils::GetRequestUri(), LogChannels::WEB_REQUEST);
 
-
 /**
  * Displays the user's changeable preferences
  * @param $oP WebPage The web page used for the output
@@ -271,10 +269,10 @@ JS
 
 		$sNewsroomHtml = '';
 		$sNewsroomHtml .= '<form method="post">';
-		$iNewsroomDisplaySize = (int)appUserPreferences::GetPref('newsroom_display_size', iTopNewsroomController::DEFAULT_NEWSROOM_DISPLAY_SIZE);
+		$iNewsroomDisplaySize = (int)appUserPreferences::GetPref('newsroom_display_size', 7);
 
-		if ($iNewsroomDisplaySize < iTopNewsroomController::DEFAULT_NEWSROOM_MIN_DISPLAY_SIZE) $iNewsroomDisplaySize = iTopNewsroomController::DEFAULT_NEWSROOM_MIN_DISPLAY_SIZE;
-		if ($iNewsroomDisplaySize > iTopNewsroomController::DEFAULT_NEWSROOM_MAX_DISPLAY_SIZE) $iNewsroomDisplaySize = iTopNewsroomController::DEFAULT_NEWSROOM_MAX_DISPLAY_SIZE;
+		if ($iNewsroomDisplaySize < 1) $iNewsroomDisplaySize = 1;
+		if ($iNewsroomDisplaySize > 20) $iNewsroomDisplaySize = 20;
 		$sInput = '<input min="1" max="20" id="newsroom_display_size" type="number" size="2" name="newsroom_display_size" value="'.$iNewsroomDisplaySize.'">';
 		$sIcon = '<i id="newsroom_menu_icon" class="top-right-icon icon-additional-arrow fas fa-bell" style="top: 0;"></i>';
 		$sNewsroomHtml .= Dict::Format('UI:Newsroom:DisplayAtMost_X_Messages', $sInput, $sIcon);
@@ -854,8 +852,6 @@ try {
 				DisplayPreferences($oPage);
 				break;
 			case 'apply_newsroom_preferences':
-
-
 				$iCountProviders = 0;
 				$oUser = UserRights::GetUserObject();
 				/** @var iNewsroomProvider[] $aProviders */
@@ -868,14 +864,14 @@ try {
 				}
 				$bNewsroomEnabled = (MetaModel::GetConfig()->Get('newsroom_enabled') !== false);
 				if ($bNewsroomEnabled && ($iCountProviders > 0)) {
-					$iNewsroomDisplaySize = (int)utils::ReadParam('newsroom_display_size', iTopNewsroomController::DEFAULT_NEWSROOM_DISPLAY_SIZE);
-					if ($iNewsroomDisplaySize < iTopNewsroomController::DEFAULT_NEWSROOM_MIN_DISPLAY_SIZE) {
-						$iNewsroomDisplaySize = iTopNewsroomController::DEFAULT_NEWSROOM_MIN_DISPLAY_SIZE;
+					$iNewsroomDisplaySize = (int)utils::ReadParam('newsroom_display_size', 7);
+					if ($iNewsroomDisplaySize < 1) {
+						$iNewsroomDisplaySize = 1;
 					}
-					if ($iNewsroomDisplaySize > iTopNewsroomController::DEFAULT_NEWSROOM_MAX_DISPLAY_SIZE) {
-						$iNewsroomDisplaySize = iTopNewsroomController::DEFAULT_NEWSROOM_MAX_DISPLAY_SIZE;
+					if ($iNewsroomDisplaySize > 20) {
+						$iNewsroomDisplaySize = 20;
 					}
-					$iCurrentDisplaySize = (int)appUserPreferences::GetPref('newsroom_display_size', iTopNewsroomController::DEFAULT_NEWSROOM_DISPLAY_SIZE);
+					$iCurrentDisplaySize = (int)appUserPreferences::GetPref('newsroom_display_size', $iNewsroomDisplaySize);
 					if ($iCurrentDisplaySize != $iNewsroomDisplaySize) {
 						// Save the preference only if it differs from the current (or default) value
 						appUserPreferences::SetPref('newsroom_display_size', $iNewsroomDisplaySize);

sources/Controller/Newsroom/iTopNewsroomController.php

@@ -48,10 +48,6 @@ use appUserPreferences;
 class iTopNewsroomController extends Controller
 {
 	public const ROUTE_NAMESPACE = 'itopnewsroom';
-	public const DEFAULT_NEWSROOM_DISPLAY_SIZE = 7;
-	public const DEFAULT_NEWSROOM_MIN_DISPLAY_SIZE = 1;
-	public const DEFAULT_NEWSROOM_MAX_DISPLAY_SIZE = 20;
-
 
 	/**
 	 * @return iTopWebPage

tests/php-unit-tests/src/BaseTestCase/ItopDataTestCase.php

@@ -18,6 +18,8 @@ use CMDBObject;
 use CMDBSource;
 use Combodo\iTop\Service\Events\EventService;
 use Contact;
+use CoreException;
+use CoreUnexpectedValue;
 use DBObject;
 use DBObjectSet;
 use DBSearch;
@@ -29,6 +31,9 @@ use lnkContactToFunctionalCI;
 use lnkContactToTicket;
 use lnkFunctionalCIToTicket;
 use MetaModel;
+use MissingQueryArgument;
+use MySQLException;
+use MySQLHasGoneAwayException;
 use Person;
 use PluginManager;
 use Server;
@@ -1439,4 +1444,24 @@ abstract class ItopDataTestCase extends ItopTestCase
 			self::markTestSkipped("Test skipped: module '$sModule' is not present");
 		}
 	}
+
+	/**
+	 * @throws CoreException
+	 * @throws CoreUnexpectedValue
+	 * @throws ArchivedObjectException
+	 * @throws MissingQueryArgument
+	 * @throws MySQLException
+	 * @throws MySQLHasGoneAwayException
+	 * @throws Exception
+	 */
+	protected function AssertUniqueObjectInDB(string $sClass, array $aCriteria, string $sMessage = ''): void
+	{
+		$oSearch = new \DBObjectSearch($sClass);
+		foreach($aCriteria as $sAttCode => $value)
+		{
+			$oSearch->AddCondition($sAttCode, $value);
+		}
+		$oSet = new DBObjectSet($oSearch);
+		$this->assertEquals(1, $oSet->Count(), $sMessage);
+	}
 }

tests/php-unit-tests/unitary-tests/core/ActionNewsroomTest.php

@@ -0,0 +1,160 @@
+<?php
+// Copyright (c) 2010-2024 Combodo SAS
+//
+//   This file is part of iTop.
+//
+//   iTop is free software; you can redistribute it and/or modify
+//   it under the terms of the GNU Affero General Public License as published by
+//   the Free Software Foundation, either version 3 of the License, or
+//   (at your option) any later version.
+//
+//   iTop is distributed in the hope that it will be useful,
+//   but WITHOUT ANY WARRANTY; without even the implied warranty of
+//   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+//   GNU Affero General Public License for more details.
+//
+//   You should have received a copy of the GNU Affero General Public License
+//   along with iTop. If not, see <http://www.gnu.org/licenses/>
+//
+
+namespace Combodo\iTop\Test\UnitTest\Core;
+
+use ArchivedObjectException;
+use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
+use CoreException;
+use CoreUnexpectedValue;
+use DBObjectSearch;
+use DBObjectSet;
+use Exception;
+use MetaModel;
+use MissingQueryArgument;
+use MySQLException;
+use MySQLHasGoneAwayException;
+
+
+class ActionNewsroomTest extends ItopDataTestCase
+{
+	const CREATE_TEST_ORG = true;
+	private int $iTrigger;
+	private int $iRecipientId;
+
+	protected function setUp(): void
+	{
+		parent::setUp();
+		$this->iTrigger = $this->GivenObjectInDB('TriggerOnObjectCreate',
+			array(
+				'description' => '[TEST] TriggerOnObjectCreate',
+				'target_class' => 'UserRequest',
+			)
+		);
+		$this->iRecipientId = $this->GivenPersonInDB(1);
+	}
+
+	private function ActionNewsroomProvider(): array
+	{
+		return [
+			'With Sync ActionNewsroom' => ['bIsAsynchronous' => false],
+			'With Async ActionNewsroom' => ['bIsAsynchronous' => true]
+		];
+	}
+
+	/**
+	 * @throws CoreException
+	 * @throws MissingQueryArgument
+	 * @throws CoreUnexpectedValue
+	 * @throws ArchivedObjectException
+	 * @throws MySQLException
+	 * @throws MySQLHasGoneAwayException
+	 * @throws Exception
+	 */
+	public function testActionNewsroomRecordsEvent()
+	{
+		$iActionNewsroomId = $this->GivenActionNewsroomInDB(false, 'Body of the notification');
+
+		$this->CreateUserRequest(1,
+			[
+				'title'       => '[TEST] ActionNewsroom',
+				'org_id'      => $this->getTestOrgId(),
+				'caller_id'   => $this->iRecipientId,
+				'description' => 'PHPUnit Test',
+			]
+		);
+
+		$this->AssertUniqueObjectInDB(
+			'EventNotificationNewsroom',
+			[
+				'action_id' => $iActionNewsroomId,
+				'message' => 'Body of the notification',
+				'title' => 'Title'
+			]
+		);
+	}
+
+	/**
+	 * @dataProvider ActionNewsroomProvider
+	 * @return void
+	 * @throws Exception
+	 */
+	public function testActionNewsroomRecordsSpecificEventIfAMandatoryFieldIsEmpty(bool $bIsAsynchronous)
+	{
+		$iActionNewsroomId = $this->GivenActionNewsroomInDB($bIsAsynchronous, '$this->service_name$');
+
+		$this->CreateUserRequest(1,
+			[
+				'title'       => '[TEST] ActionNewsroom',
+				'org_id'      => $this->getTestOrgId(),
+				'caller_id'   => $this->iRecipientId,
+				'description' => 'PHPUnit Test',
+				'service_id' => 0
+			]
+		);
+
+		$this->AssertUniqueObjectInDB(
+			'EventNotificationNewsroom',
+			[
+				'action_id' => $iActionNewsroomId,
+				'title' => 'Notification not sent',
+				'message' => 'An error occurred while saving the notification'
+			]
+		);
+	}
+
+	/**
+	 * @throws Exception
+	 */
+	private function GivenActionNewsroomInDB(bool $bIsAsynchronous, string $sMessage): int
+	{
+		$this->GivenObjectInDB('UserLocal', [
+			'login' => 'demo_test_'.uniqid(__CLASS__, true),
+			'password' => 'admin_123',
+			'language' => 'EN US',
+			'profile_list' => ['profileid:'.self::$aURP_Profiles['Administrator']],
+			'contactid' => $this->iRecipientId
+		]);
+
+		return $this->GivenObjectInDB('ActionNewsroom', [
+			'name' => 'ActionNewsroom TriggerOnObjectCreate',
+			'status' => 'test',
+			'test_recipient_id' => $this->iRecipientId,
+			'title' => 'Title',
+			'message' => $sMessage,
+			'recipients' => "SELECT Person WHERE id = $this->iRecipientId",
+			'asynchronous' => $bIsAsynchronous ? 'yes' : 'no',
+			'trigger_list' => [
+				"trigger_id:$this->iTrigger"
+			],
+		]);
+	}
+
+	/**
+	 * @throws Exception
+	 */
+	private function GivenService(string $sName): int
+	{
+		return $this->GivenObjectInDB('Service', [
+				'name' => $sName,
+				'org_id' => $this->getTestOrgId()
+			]
+		);
+	}
+}