From 8dd35ec9d31a214f4cc88cb478d15a0110b53f85 Mon Sep 17 00:00:00 2001 From: Yuri Kuznetsov Date: Fri, 4 Apr 2025 15:58:45 +0300 Subject: [PATCH] email body to plain inprovement --- .../Classes/RecordHooks/Email/BeforeSave.php | 20 +++- application/Espo/Entities/Email.php | 35 +++---- .../Resources/metadata/entityDefs/Email.json | 4 +- application/Espo/Tools/Email/Util.php | 30 +++--- client/src/views/fields/wysiwyg.js | 12 --- composer.json | 3 +- composer.lock | 91 ++++++++++++++++++- tests/unit/Espo/Entities/EmailTest.php | 81 +++++++++++------ 8 files changed, 198 insertions(+), 78 deletions(-) diff --git a/application/Espo/Classes/RecordHooks/Email/BeforeSave.php b/application/Espo/Classes/RecordHooks/Email/BeforeSave.php index ac04165a9d..7fabf2301e 100644 --- a/application/Espo/Classes/RecordHooks/Email/BeforeSave.php +++ b/application/Espo/Classes/RecordHooks/Email/BeforeSave.php @@ -33,13 +33,14 @@ use Espo\Core\Exceptions\BadRequest; use Espo\Core\Record\Hook\SaveHook; use Espo\Entities\Email; use Espo\ORM\Entity; +use Espo\Tools\Email\Util; +use League\HTMLToMarkdown\HtmlConverter; /** * @implements SaveHook */ class BeforeSave implements SaveHook { - public function process(Entity $entity): void { if ( @@ -49,5 +50,22 @@ class BeforeSave implements SaveHook ) { throw new BadRequest("Cannot set send-at if status is not Draft."); } + + $this->processBodyPlain($entity); + } + + private function processBodyPlain(Email $entity): void + { + if (!$entity->isHtml() || !$entity->isAttributeChanged('body')) { + return; + } + + $body = $entity->getBody(); + + if ($body) { + $body = Util::stripHtml($body) ?: null; + } + + $entity->setBodyPlain($body); } } diff --git a/application/Espo/Entities/Email.php b/application/Espo/Entities/Email.php index 47875aad09..2be63a27c6 100644 --- a/application/Espo/Entities/Email.php +++ b/application/Espo/Entities/Email.php @@ -70,6 +70,8 @@ class Email extends Entity public const SAVE_OPTION_IS_BEING_IMPORTED = 'isBeingImported'; public const SAVE_OPTION_IS_JUST_SENT = 'isJustSent'; + private const ATTR_BODY_PLAIN = 'bodyPlain'; + public function get(string $attribute): mixed { if ($attribute === 'subject') { @@ -92,7 +94,7 @@ class Email extends Entity return $this->getReplyToAddressInternal(); } - if ($attribute === 'bodyPlain') { + if ($attribute === self::ATTR_BODY_PLAIN) { return $this->getBodyPlain(); } @@ -172,6 +174,7 @@ class Email extends Entity /** * @deprecated As of v7.4. As the system user ID may be not constant in the future. + * @todo Remove in v10.0. */ public function isManuallyArchived(): bool { @@ -204,7 +207,7 @@ class Email extends Entity public function hasBodyPlain(): bool { - return $this->hasInContainer('bodyPlain') && $this->getFromContainer('bodyPlain'); + return $this->hasInContainer(self::ATTR_BODY_PLAIN) && $this->getFromContainer(self::ATTR_BODY_PLAIN); } /** @@ -223,20 +226,13 @@ class Email extends Entity public function getBodyPlain(): ?string { - if ($this->getFromContainer('bodyPlain')) { - return $this->getFromContainer('bodyPlain'); + if ($this->getFromContainer(self::ATTR_BODY_PLAIN)) { + return $this->getFromContainer(self::ATTR_BODY_PLAIN); } - /** @var string $body */ - $body = $this->get('body') ?? ''; + $body = $this->getBody() ?: ''; - $body = EmailUtil::stripHtml($body); - - if (!$body) { - return null; - } - - return $body; + return EmailUtil::stripHtml($body) ?: null; } public function getBodyPlainForSending(): string @@ -246,9 +242,9 @@ class Email extends Entity public function getBodyForSending(): string { - $body = $this->get('body') ?? ''; + $body = $this->getBody() ?: ''; - if (!empty($body)) { + if ($body && $this->isHtml()) { $attachmentList = $this->getInlineAttachmentList(); foreach ($attachmentList as $attachment) { @@ -273,9 +269,9 @@ class Email extends Entity { $idList = []; - $body = $this->get('body'); + $body = $this->getBody(); - if (empty($body)) { + if (!$body) { return []; } @@ -302,8 +298,7 @@ class Email extends Entity throw new RuntimeException(); } - /** @var Attachment|null $attachment */ - $attachment = $this->entityManager->getEntityById(Attachment::ENTITY_TYPE, $id); + $attachment = $this->entityManager->getRDBRepositoryByClass(Attachment::class)->getById($id); if ($attachment) { $attachmentList[] = $attachment; @@ -372,7 +367,7 @@ class Email extends Entity public function setBodyPlain(?string $bodyPlain): self { - $this->set('bodyPlain', $bodyPlain); + $this->set(self::ATTR_BODY_PLAIN, $bodyPlain); return $this; } diff --git a/application/Espo/Resources/metadata/entityDefs/Email.json b/application/Espo/Resources/metadata/entityDefs/Email.json index 01414e8893..42f0ad0853 100644 --- a/application/Espo/Resources/metadata/entityDefs/Email.json +++ b/application/Espo/Resources/metadata/entityDefs/Email.json @@ -343,11 +343,11 @@ "bodyPlain": { "type": "text", "seeMoreDisabled": true, - "clientReadOnly": true, "customizationDisabled": true, "layoutDefaultSidePanelDisabled": true, "layoutMassUpdateDisabled": true, - "massUpdateDisabled": true + "massUpdateDisabled": true, + "readOnly": true }, "body": { "type": "wysiwyg", diff --git a/application/Espo/Tools/Email/Util.php b/application/Espo/Tools/Email/Util.php index bb060204f6..f193b301ec 100644 --- a/application/Espo/Tools/Email/Util.php +++ b/application/Espo/Tools/Email/Util.php @@ -29,6 +29,8 @@ namespace Espo\Tools\Email; +use League\HTMLToMarkdown\HtmlConverter; + class Util { static public function parseFromName(string $string): string @@ -71,18 +73,19 @@ class Util */ static public function stripHtml(string $string): string { - $breaks = [ - "
", - "
", - "
", - "
", - "<br />", - "<br/>", - "<br>", - ]; + if (!$string) { + return ''; + } - $string = str_ireplace($breaks, "\r\n", $string); - $string = strip_tags($string); + $converter = new HtmlConverter(); + $converter->setOptions([ + 'remove_nodes' => 'img', + 'strip_tags' => true, + ]); + + $string = $converter->convert($string) ?: ''; + + $string = (string) preg_replace('~\R~u', "\r\n", $string); $reList = [ '&(quot|#34);', @@ -111,10 +114,11 @@ class Util ]; foreach ($reList as $i => $re) { - /** @var string $string */ - $string = mb_ereg_replace($re, $replaceList[$i], $string, 'i'); + $string = (string) mb_ereg_replace($re, $replaceList[$i], $string, 'i'); } + + return $string; } diff --git a/client/src/views/fields/wysiwyg.js b/client/src/views/fields/wysiwyg.js index 6ed4d861c3..d00146f6d5 100644 --- a/client/src/views/fields/wysiwyg.js +++ b/client/src/views/fields/wysiwyg.js @@ -860,18 +860,6 @@ class WysiwygFieldView extends TextFieldView { data[this.name] = null; } - if (this.hasBodyPlainField && this.model.has('isHtml')) { - const plainAttribute = this.name + 'Plain'; - - if (data[this.name] === null) { - data[plainAttribute] = null; - } else { - data[plainAttribute] = this.isHtml() ? - this.htmlToPlain(data[this.name]) : - data[this.name]; - } - } - return data; } diff --git a/composer.json b/composer.json index 0bf2a33fb5..ce609a1a0f 100644 --- a/composer.json +++ b/composer.json @@ -53,7 +53,8 @@ "lasserafn/php-initial-avatar-generator": "^4.4", "tholu/php-cidr-match": "^0.4", "league/oauth2-client": "^2.8", - "symfony/mailer": "^6" + "symfony/mailer": "^6", + "league/html-to-markdown": "^5.1" }, "require-dev": { "phpunit/phpunit": "^9.5", diff --git a/composer.lock b/composer.lock index 4c3b7284ad..1106ef61bb 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "cca59898a92b6c9f0723465a59dd8c9f", + "content-hash": "0c7231d8b867aa042b9360424cf0702a", "packages": [ { "name": "async-aws/core", @@ -3047,6 +3047,95 @@ }, "time": "2024-08-09T21:24:39+00:00" }, + { + "name": "league/html-to-markdown", + "version": "5.1.1", + "source": { + "type": "git", + "url": "https://github.com/thephpleague/html-to-markdown.git", + "reference": "0b4066eede55c48f38bcee4fb8f0aa85654390fd" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/thephpleague/html-to-markdown/zipball/0b4066eede55c48f38bcee4fb8f0aa85654390fd", + "reference": "0b4066eede55c48f38bcee4fb8f0aa85654390fd", + "shasum": "" + }, + "require": { + "ext-dom": "*", + "ext-xml": "*", + "php": "^7.2.5 || ^8.0" + }, + "require-dev": { + "mikehaertl/php-shellcommand": "^1.1.0", + "phpstan/phpstan": "^1.8.8", + "phpunit/phpunit": "^8.5 || ^9.2", + "scrutinizer/ocular": "^1.6", + "unleashedtech/php-coding-standard": "^2.7 || ^3.0", + "vimeo/psalm": "^4.22 || ^5.0" + }, + "bin": [ + "bin/html-to-markdown" + ], + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "5.2-dev" + } + }, + "autoload": { + "psr-4": { + "League\\HTMLToMarkdown\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Colin O'Dell", + "email": "colinodell@gmail.com", + "homepage": "https://www.colinodell.com", + "role": "Lead Developer" + }, + { + "name": "Nick Cernis", + "email": "nick@cern.is", + "homepage": "http://modernnerd.net", + "role": "Original Author" + } + ], + "description": "An HTML-to-markdown conversion helper for PHP", + "homepage": "https://github.com/thephpleague/html-to-markdown", + "keywords": [ + "html", + "markdown" + ], + "support": { + "issues": "https://github.com/thephpleague/html-to-markdown/issues", + "source": "https://github.com/thephpleague/html-to-markdown/tree/5.1.1" + }, + "funding": [ + { + "url": "https://www.colinodell.com/sponsor", + "type": "custom" + }, + { + "url": "https://www.paypal.me/colinpodell/10.00", + "type": "custom" + }, + { + "url": "https://github.com/colinodell", + "type": "github" + }, + { + "url": "https://tidelift.com/funding/github/packagist/league/html-to-markdown", + "type": "tidelift" + } + ], + "time": "2023-07-12T21:21:09+00:00" + }, { "name": "league/mime-type-detection", "version": "1.16.0", diff --git a/tests/unit/Espo/Entities/EmailTest.php b/tests/unit/Espo/Entities/EmailTest.php index facb0151e4..94fb765ff0 100644 --- a/tests/unit/Espo/Entities/EmailTest.php +++ b/tests/unit/Espo/Entities/EmailTest.php @@ -29,8 +29,10 @@ namespace tests\unit\Espo\Entities; +use Espo\Entities\Attachment; use Espo\Entities\Email; -use Espo\Tools\Email\Util; +use Espo\ORM\EntityManager; +use Espo\ORM\Repository\RDBRepository; class EmailTest extends \PHPUnit\Framework\TestCase { @@ -39,6 +41,16 @@ class EmailTest extends \PHPUnit\Framework\TestCase */ private $email; + /** + * @var EntityManager + */ + private $entityManager; + + /** + * @var RDBRepository + */ + private $attachmentRepository; + // TODO defs test helper protected $defs = [ 'attributes' => @@ -425,62 +437,75 @@ class EmailTest extends \PHPUnit\Framework\TestCase protected function setUp() : void { - $this->entityManager = $this->getMockBuilder('\Espo\Core\ORM\EntityManager')->disableOriginalConstructor()->getMock(); + $this->entityManager = $this->createMock(EntityManager::class); + + $this->attachmentRepository = $this->createMock(RDBRepository::class); + + $this->entityManager + ->expects($this->any()) + ->method('getRDBRepositoryByClass') + ->willReturnMap([ + [Attachment::class, $this->attachmentRepository], + ]); $this->repository = $this->getMockBuilder('Espo\Core\ORM\Repositories\Database')->disableOriginalConstructor()->getMock(); - $this->entityManager->expects($this->any()) - ->method('getRepository') - ->will($this->returnValue($this->repository)); + $this->entityManager + ->expects($this->any()) + ->method('getRepository') + ->will($this->returnValue($this->repository)); - $this->email = new Email('Email', $this->defs, $this->entityManager); + $this->email = new Email( + entityType: 'Email', + defs: $this->defs, + entityManager: $this->entityManager, + ); } - protected function tearDown() : void - { - $this->entityManager = null; - $this->repository = null; - $this->email = null; - } - - - function testGetInlineAttachments() + public function testGetInlineAttachments(): void { $this->email->set('body', 'test '); - $this->entityManager->expects($this->exactly(1)) - ->method('getEntityById') - ->with('Attachment', 'Id01'); + $this->attachmentRepository + ->expects($this->once()) + ->method('getById') + ->with('Id01'); $this->email->getInlineAttachmentList(); } - function testGetBodyForSending() + public function testGetBodyForSending(): void { - $attachment = - $this->getMockBuilder('Espo\Entities\Attachment')->disableOriginalConstructor()->getMock(); + $attachment = $this->createMock(Attachment::class); $attachment + ->expects($this->any()) ->method('getId') ->willReturn('Id01'); - $this->email->set('body', 'test '); + $this->attachmentRepository + ->expects($this->once()) + ->method('getById') + ->with('Id01') + ->willReturn($attachment); - $this->entityManager->expects($this->any()) - ->method('getEntityById') - ->with('Attachment', 'Id01') - ->will($this->returnValue($attachment)); + $body = 'test '; + + $this->email->setBody($body); + $this->email->setIsHtml(); $body = $this->email->getBodyForSending(); + $this->assertEquals('test ', $body); } public function testBodyPlain(): void { - $this->email->set('body', '
 &'); + $this->email->setBody('
 &'); $bodyPlain = $this->email->getBodyPlain(); - $this->assertEquals("\r\n &", $bodyPlain); + + $this->assertEquals(" \r\n &", $bodyPlain); } public function testBodyPlainWithoutQuotePart(): void