composer/iTop
6
0
Fork 0
mirror of https://github.com/Combodo/iTop.git synced 2026-02-13 07:24:13 +01:00
Code Issues Packages Projects Releases Wiki Activity

N°4849 - Enable notification emails grouping in threads in email clients (#275)

Browse Source 
N°4849 - Enable notification emails grouping in threads in email clients (#275)
Co-authored-by: Thomas Casteleyn <thomas.casteleyn@super-visions.com>
This commit is contained in:
Molkobain
2022-03-10 09:31:29 +01:00
committed by GitHub
parent 7ddb47dc83
commit 0f2cbaf186
4 changed files with 191 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

103
core/action.class.inc.php
View File

@@ -200,6 +200,17 @@ abstract class ActionNotification extends Action
*/
class ActionEmail extends ActionNotification
{
/**
* @var string
* @since 3.0.1
*/
const ENUM_HEADER_NAME_MESSAGE_ID = 'Message-ID';
/**
* @var string
* @since 3.0.1
*/
const ENUM_HEADER_NAME_REFERENCES = 'References';
/**
* @inheritDoc
*/
@@ -207,13 +218,13 @@ class ActionEmail extends ActionNotification
{
$aParams = array
(
"category" => "grant_by_profile,core/cmdb,application",
"key_type" => "autoincrement",
"name_attcode" => "name",
"state_attcode" => "",
"reconc_keys" => array('name'),
"db_table" => "priv_action_email",
"db_key_field" => "id",
"category" => "grant_by_profile,core/cmdb,application",
"key_type" => "autoincrement",
"name_attcode" => "name",
"state_attcode" => "",
"reconc_keys" => array('name'),
"db_table" => "priv_action_email",
"db_key_field" => "id",
"db_finalclass_field" => "",
);
MetaModel::Init_Params($aParams);
@@ -416,9 +427,8 @@ class ActionEmail extends ActionNotification
$sBody = MetaModel::ApplyParams($this->Get('body'), $aContextArgs);
$oObj = $aContextArgs['this->object()'];
$sMessageId = sprintf('iTop_%s_%d_%f@%s.openitop.org', get_class($oObj), $oObj->GetKey(), microtime(true /* get as float*/),
MetaModel::GetEnvironmentId());
$sReference = '<'.$sMessageId.'>';
$sMessageId = $this->GenerateIdentifierForHeaders($oObj, static::ENUM_HEADER_NAME_MESSAGE_ID);
$sReference = $this->GenerateIdentifierForHeaders($oObj, static::ENUM_HEADER_NAME_REFERENCES);
}
catch (Exception $e) {
/** @noinspection PhpUnhandledExceptionInspection */
@@ -456,8 +466,7 @@ class ActionEmail extends ActionNotification
$oEmail = new EMail();
if ($this->IsBeingTested())
{
if ($this->IsBeingTested()) {
$oEmail->SetSubject('TEST['.$sSubject.']');
$sTestBody = $sBody;
$sTestBody .= "<div style=\"border: dashed;\">\n";
@@ -467,8 +476,8 @@ class ActionEmail extends ActionNotification
$sTestBody .= "<li>TO: $sTo</li>\n";
$sTestBody .= "<li>CC: $sCC</li>\n";
$sTestBody .= "<li>BCC: $sBCC</li>\n";
$sTestBody .= empty($sFromLabel) ? "<li>From: $sFrom</li>\n": "<li>From: $sFromLabel &lt;$sFrom&gt;</li>\n";
$sTestBody .= empty($sReplyToLabel) ? "<li>Reply-To: $sReplyTo</li>\n": "<li>Reply-To: $sReplyToLabel &lt;$sReplyTo&gt;</li>\n";
$sTestBody .= empty($sFromLabel) ? "<li>From: $sFrom</li>\n" : "<li>From: $sFromLabel &lt;$sFrom&gt;</li>\n";
$sTestBody .= empty($sReplyToLabel) ? "<li>Reply-To: $sReplyTo</li>\n" : "<li>Reply-To: $sReplyToLabel &lt;$sReplyTo&gt;</li>\n";
$sTestBody .= "<li>References: $sReference</li>\n";
$sTestBody .= "</ul>\n";
$sTestBody .= "</p>\n";
@@ -478,9 +487,9 @@ class ActionEmail extends ActionNotification
$oEmail->SetRecipientFrom($sFrom, $sFromLabel);
$oEmail->SetReferences($sReference);
$oEmail->SetMessageId($sMessageId);
}
else
{
// Note: N°4849 We pass the "References" identifier instead of the "Message-ID" on purpose as we want notifications emails to group around the triggering iTop object, not just the users' replies to the notification
$oEmail->SetInReplyTo($sReference);
} else {
$oEmail->SetSubject($sSubject);
$oEmail->SetBody($sBody, 'text/html', $sStyles);
$oEmail->SetRecipientTO($sTo);
@@ -490,6 +499,8 @@ class ActionEmail extends ActionNotification
$oEmail->SetRecipientReplyTo($sReplyTo, $sReplyToLabel);
$oEmail->SetReferences($sReference);
$oEmail->SetMessageId($sMessageId);
// Note: N°4849 We pass the "References" identifier instead of the "Message-ID" on purpose as we want notifications emails to group around the triggering iTop object, not just the users' replies to the notification
$oEmail->SetInReplyTo($sReference);
}
if (isset($aContextArgs['attachments']))
@@ -516,26 +527,64 @@ class ActionEmail extends ActionNotification
{
case EMAIL_SEND_OK:
return "Sent";
case EMAIL_SEND_PENDING:
return "Pending";
case EMAIL_SEND_ERROR:
return "Errors: ".implode(', ', $aErrors);
}
}
}
else
{
if (is_array($this->m_aMailErrors) && count($this->m_aMailErrors) > 0)
{
} else {
if (is_array($this->m_aMailErrors) && count($this->m_aMailErrors) > 0) {
$sError = implode(', ', $this->m_aMailErrors);
}
else
{
} else {
$sError = 'Unknown reason';
}
return 'Notification was not sent: '.$sError;
}
}
/**
* @param \DBObject $oObject
* @param string $sHeaderName {@see \ActionEmail::ENUM_HEADER_NAME_REFERENCES}, {@see \ActionEmail::ENUM_HEADER_NAME_MESSAGE_ID}
*
* @return string The formatted identifier for $sHeaderName based on $oObject
* @throws \Exception
* @since 3.0.1 N°4849
*/
protected function GenerateIdentifierForHeaders(DBObject $oObject, string $sHeaderName): string
{
$sObjClass = get_class($oObject);
$sObjId = $oObject->GetKey();
$sAppName = utils::Sanitize(ITOP_APPLICATION_SHORT, '', utils::ENUM_SANITIZATION_FILTER_VARIABLE_NAME);
switch ($sHeaderName) {
case static::ENUM_HEADER_NAME_MESSAGE_ID:
case static::ENUM_HEADER_NAME_REFERENCES:
// Prefix
$sPrefix = sprintf('%s_%s_%d', $sAppName, $sObjClass, $sObjId);
if ($sHeaderName === static::ENUM_HEADER_NAME_MESSAGE_ID) {
$sPrefix .= sprintf('_%f', microtime(true /* get as float*/));
}
// Suffix
$sSuffix = sprintf('@%s.openitop.org', MetaModel::GetEnvironmentId());
// Identifier
$sIdentifier = $sPrefix.$sSuffix;
if ($sHeaderName === static::ENUM_HEADER_NAME_REFERENCES) {
$sIdentifier = "<$sIdentifier>";
}
return $sIdentifier;
}
// Requested header name invalid
$sErrorMessage = sprintf('%s: Could not generate identifier for header "%s", only %s are supported', static::class, $sHeaderName, implode(' / ', [static::ENUM_HEADER_NAME_MESSAGE_ID, static::ENUM_HEADER_NAME_REFERENCES]));
IssueLog::Error($sErrorMessage, LogChannels::NOTIFICATIONS, [
'Object' => $sObjClass.'::'.$sObjId.' ('.$oObject->GetRawName().')',
'Action' => get_class($this).'::'.$this->GetKey().' ('.$this->GetRawName().')',
]);
throw new Exception($sErrorMessage);
}
}

21
core/email.class.inc.php
View File

@@ -325,20 +325,33 @@ class EMail
// Note: Swift will add the angle brackets for you
// so let's remove the angle brackets if present, for historical reasons
$sId = str_replace(array('<', '>'), '', $sId);
$oMsgId = $this->m_oMessage->getHeaders()->get('Message-ID');
$oMsgId->SetId($sId);
}
public function SetReferences($sReferences)
{
$this->AddToHeader('References', $sReferences);
}
/**
* Set the "In-Reply-To" header to allow emails to group as a conversation in modern mail clients (GMail, Outlook 2016+, ...)
*
* @link https://en.wikipedia.org/wiki/Email#Header_fields
*
* @param string $sMessageId
*
* @since 3.0.1 N°4849
*/
public function SetInReplyTo(string $sMessageId)
{
$this->AddToHeader('In-Reply-To', $sMessageId);
}
public function SetBody($sBody, $sMimeType = 'text/html', $sCustomStyles = null)
{
if (($sMimeType === 'text/html') && ($sCustomStyles !== null))
{
if (($sMimeType === 'text/html') && ($sCustomStyles !== null)) {
$oDomDocument = CssInliner::fromHtml($sBody)->inlineCss($sCustomStyles)->getDomDocument();
HtmlPruner::fromDomDocument($oDomDocument)->removeElementsWithDisplayNone();
$sBody = CssToAttributeConverter::fromDomDocument($oDomDocument)->convertCssToVisualAttributes()->render(); // Adds html/body tags if not already present

8
core/log.class.inc.php
View File

@@ -544,7 +544,13 @@ class LogChannels
{
public const APC = 'apc';
public const CLI = 'CLI';
/**
* @var string
* @since 3.0.1 N°4849
*/
public const NOTIFICATIONS = 'notifications';
public const CLI = 'CLI';
/**
* @var string

91
test/core/ActionEmailTest.php Normal file
View File

@@ -0,0 +1,91 @@
<?php
namespace Combodo\iTop\Test\UnitTest\Core;
use ActionEmail;
use Combodo\iTop\Test\UnitTest\ItopDataTestCase;
use Exception;
use MetaModel;
use utils;
/**
* @runTestsInSeparateProcesses
* @preserveGlobalState disabled
* @backupGlobals disabled
* @covers \ActionEmail
*/
class ActionEmailTest extends ItopDataTestCase
{
/**
* @inheritDoc
*/
const CREATE_TEST_ORG = true;
/** @var \ActionEmail|null Temp ActionEmail created for tests */
protected static $oActionEmail = null;
protected function setUp()
{
parent::setUp();
static::$oActionEmail = MetaModel::NewObject('ActionEmail', [
'name' => 'Test action',
'status' => 'disabled',
'from' => 'unit-test@openitop.org',
'subject' => 'Test subject',
'body' => 'Test body',
]);
static::$oActionEmail->DBInsert();
}
/**
* @covers \ActionEmail::GenerateIdentifierForHeaders
* @dataProvider GenerateIdentifierForHeadersProvider
* @throws \Exception
*/
public function testGenerateIdentifierForHeaders(string $sHeaderName)
{
// Retrieve object
$oObject = MetaModel::GetObject('Organization', $this->getTestOrgId(), true, true);
$sObjClass = get_class($oObject);
$sObjId = $oObject->GetKey();
try {
$sTestedIdentifier = $this->InvokeNonPublicMethod('\ActionEmail', 'GenerateIdentifierForHeaders', static::$oActionEmail, [$oObject, $sHeaderName]);
} catch (Exception $oException) {
$sTestedIdentifier = null;
}
$sAppName = utils::Sanitize(ITOP_APPLICATION_SHORT, '', utils::ENUM_SANITIZATION_FILTER_VARIABLE_NAME);
$sEnvironmentHash = MetaModel::GetEnvironmentId();
switch ($sHeaderName) {
case ActionEmail::ENUM_HEADER_NAME_MESSAGE_ID:
// Note: For this test we can't use the more readable sprintf test as the generated timestamp will never be the same as the one generated during the call of the tested method
// $sTimestamp = microtime(true /* get as float*/);
// $sExpectedIdentifier = sprintf('%s_%s_%d_%f@%s.openitop.org', $sAppName, $sObjClass, $sObjId, $sTimestamp, $sEnvironmentHash);
$this->assertEquals(1, preg_match('/'.$sAppName.'_'.$sObjClass.'_'.$sObjId.'_[\d]+\.[\d]+@'.$sEnvironmentHash.'.openitop.org/', $sTestedIdentifier), "Identifier doesn't match regexp for header $sHeaderName, got $sTestedIdentifier");
break;
case ActionEmail::ENUM_HEADER_NAME_REFERENCES:
$sExpectedIdentifier = '<'.sprintf('%s_%s_%d@%s.openitop.org', $sAppName, $sObjClass, $sObjId, $sEnvironmentHash).'>';
$this->assertEquals($sExpectedIdentifier, $sTestedIdentifier);
break;
default:
$sExpectedIdentifier = null;
$this->assertEquals($sExpectedIdentifier, $sTestedIdentifier);
break;
}
}
public function GenerateIdentifierForHeadersProvider()
{
return [
'Message-ID' => ['Message-ID'],
'References' => ['References'],
'IncorrectHeaderName' => ['IncorrectHeaderName'],
];
}
}