From fdbd3a5784203614df3d1082ef650e4754e46bf4 Mon Sep 17 00:00:00 2001 From: Fabian Helfer Date: Tue, 30 Jun 2026 11:43:51 +0200 Subject: [PATCH 1/2] [Bugfix] Mail 047999: Add supportsRecipient to MailPlaceholder to fix empty resolve in bcc/cc Mails --- .../class.ilMailTemplateContextAdapter.php | 14 +++ .../Mail/classes/Service/MailService.php | 4 - .../ILIAS/Mail/classes/class.ilMail.php | 14 +-- .../classes/class.ilMailTemplateContext.php | 99 ++++++++++++++++++- ...MailTemplatePlaceholderToEmptyResolver.php | 35 ------- components/ILIAS/Mail/tests/bootstrap.php | 2 +- ...TemplatePlaceholderToEmptyResolverTest.php | 99 +++++++++++++++++++ components/ILIAS/Mail/tests/ilMailTest.php | 1 - 8 files changed, 213 insertions(+), 55 deletions(-) delete mode 100755 components/ILIAS/Mail/classes/class.ilMailTemplatePlaceholderToEmptyResolver.php create mode 100644 components/ILIAS/Mail/tests/ilMailTemplatePlaceholderToEmptyResolverTest.php diff --git a/components/ILIAS/Mail/classes/Mustache/class.ilMailTemplateContextAdapter.php b/components/ILIAS/Mail/classes/Mustache/class.ilMailTemplateContextAdapter.php index 6aea8e41eccc..72e4b09f38a1 100755 --- a/components/ILIAS/Mail/classes/Mustache/class.ilMailTemplateContextAdapter.php +++ b/components/ILIAS/Mail/classes/Mustache/class.ilMailTemplateContextAdapter.php @@ -74,6 +74,20 @@ public function __get(string $name): string } } + if ($this->recipient === null) { + foreach ($this->contexts as $context) { + if (!$context->requiresRecipientByPlaceholderName($name)) { + continue; + } + + if ($context->supportsConditionByPlaceholderName($name)) { + return ''; + } + + return $name; + } + } + return ''; } } diff --git a/components/ILIAS/Mail/classes/Service/MailService.php b/components/ILIAS/Mail/classes/Service/MailService.php index 7c87b70eb88a..71b74dafae6b 100755 --- a/components/ILIAS/Mail/classes/Service/MailService.php +++ b/components/ILIAS/Mail/classes/Service/MailService.php @@ -73,10 +73,6 @@ public function placeholderResolver(): \ilMailTemplatePlaceholderResolver ); } - public function placeholderToEmptyResolver(): \ilMailTemplatePlaceholderToEmptyResolver - { - return new \ilMailTemplatePlaceholderToEmptyResolver(); - } public function mustacheFactory(): \ilMustacheFactory { diff --git a/components/ILIAS/Mail/classes/class.ilMail.php b/components/ILIAS/Mail/classes/class.ilMail.php index c07b6a3d9b11..5f7a741f1f78 100755 --- a/components/ILIAS/Mail/classes/class.ilMail.php +++ b/components/ILIAS/Mail/classes/class.ilMail.php @@ -73,7 +73,6 @@ public function __construct( private ?int $mail_obj_ref_id = null, private ?ilObjUser $actor = null, private ?ilMailTemplatePlaceholderResolver $placeholder_resolver = null, - private ?ilMailTemplatePlaceholderToEmptyResolver $placeholder_to_empty_resolver = null, ?Conductor $legal_documents = null, ?MailSignatureService $signature_service = null, ) { @@ -102,7 +101,6 @@ public function __construct( $this->table_mail_saved = 'mail_saved'; $this->setSaveInSentbox(false); $this->placeholder_resolver = $placeholder_resolver ?? $DIC->mail()->placeholderResolver(); - $this->placeholder_to_empty_resolver = $placeholder_to_empty_resolver ?? $DIC->mail()->placeholderToEmptyResolver(); $this->legal_documents = $legal_documents ?? $DIC['legalDocuments']; $this->signature_service = $signature_service ?? $DIC->mail()->signature(); } @@ -563,7 +561,7 @@ private function sendInternalMail( private function replacePlaceholders( string $message, - int $usrId = 0 + ?int $usrId = null ): string { try { if ($this->context_id) { @@ -572,7 +570,7 @@ private function replacePlaceholders( $context = new ilMailTemplateGenericContext(); } - $user = $usrId > 0 ? $this->getUserInstanceById($usrId) : null; + $user = ($usrId !== null && $usrId > 0) ? $this->getUserInstanceById($usrId) : null; $message = $this->placeholder_resolver->resolve( $context, $message, @@ -591,10 +589,6 @@ private function replacePlaceholders( return $message; } - private function replacePlaceholdersEmpty(string $message): string - { - return $this->placeholder_to_empty_resolver->resolve($message); - } private function distributeMail(MailDeliveryData $mail_data): bool { @@ -650,7 +644,7 @@ private function sendMailWithReplacedEmptyPlaceholder( $this->sendChanneledMails( $mail_data, $recipients, - $this->replacePlaceholdersEmpty($mail_data->getMessage()), + $this->replacePlaceholders($mail_data->getMessage()), ); } @@ -1125,7 +1119,7 @@ public function sendMail( $externalMailRecipientsBcc, $mail_data->getSubject(), $mail_data->isUsePlaceholder() ? - $this->replacePlaceholders($mail_data->getMessage(), 0) : + $this->replacePlaceholders($mail_data->getMessage()) : $mail_data->getMessage(), $mail_data->getAttachments() ); diff --git a/components/ILIAS/Mail/classes/class.ilMailTemplateContext.php b/components/ILIAS/Mail/classes/class.ilMailTemplateContext.php index 93e72450fcbd..716d450d7ab5 100755 --- a/components/ILIAS/Mail/classes/class.ilMailTemplateContext.php +++ b/components/ILIAS/Mail/classes/class.ilMailTemplateContext.php @@ -18,8 +18,8 @@ declare(strict_types=1); -use OrgUnit\PublicApi\OrgUnitUserService; use OrgUnit\User\ilOrgUnitUser; +use OrgUnit\PublicApi\OrgUnitUserService; abstract class ilMailTemplateContext { @@ -53,7 +53,16 @@ abstract public function getTitle(): string; abstract public function getDescription(): string; /** - * @return array{mail_salutation: array{placeholder: string, label: string, supportsNestedPlaceholders?: true}, first_name: array{placeholder: string, label: string}, last_name: array{placeholder: string, label: string}, login: array{placeholder: string, label: string}, title: array{placeholder: string, label: string, supportsCondition: true}, firstname_lastname_superior: array{placeholder: string, label: string}, ilias_url: array{placeholder: string, label: string}, installation_name: array{placeholder: string, label: string}} + * @return array{ + * mail_salutation: array{placeholder: string, label: string, supportsNestedPlaceholders?: true}, + * first_name: array{placeholder: string, label: string}, + * last_name: array{placeholder: string, label: string}, + * login: array{placeholder: string, label: string}, + * title: array{placeholder: string, label: string, supportsCondition: true}, + * firstname_lastname_superior: array{placeholder: string, label: string}, + * ilias_url: array{placeholder: string, label: string}, + * installation_name: array{placeholder: string, label: string} + * } */ private function getGenericPlaceholders(): array { @@ -61,36 +70,44 @@ private function getGenericPlaceholders(): array 'mail_salutation' => [ 'placeholder' => 'MAIL_SALUTATION', 'label' => $this->getLanguage()->txt('mail_nacc_salutation'), + 'requiresRecipient' => true, 'supportsNestedPlaceholders' => true, ], 'first_name' => [ 'placeholder' => 'FIRST_NAME', 'label' => $this->getLanguage()->txt('firstname'), + 'requiresRecipient' => true, ], 'last_name' => [ 'placeholder' => 'LAST_NAME', 'label' => $this->getLanguage()->txt('lastname'), + 'requiresRecipient' => true, ], 'login' => [ 'placeholder' => 'LOGIN', 'label' => $this->getLanguage()->txt('mail_nacc_login'), + 'requiresRecipient' => true, ], 'title' => [ 'placeholder' => 'TITLE', 'label' => $this->getLanguage()->txt('mail_nacc_title'), + 'requiresRecipient' => true, 'supportsCondition' => true, ], 'firstname_lastname_superior' => [ 'placeholder' => 'FIRSTNAME_LASTNAME_SUPERIOR', 'label' => $this->getLanguage()->txt('mail_firstname_last_name_superior'), + 'requiresRecipient' => true, ], 'ilias_url' => [ 'placeholder' => 'ILIAS_URL', 'label' => $this->getLanguage()->txt('mail_nacc_ilias_url'), + 'requiresRecipient' => false, ], 'installation_name' => [ 'placeholder' => 'INSTALLATION_NAME', 'label' => $this->getLanguage()->txt('mail_nacc_installation_name'), + 'requiresRecipient' => false, ], ]; } @@ -111,7 +128,13 @@ public function getNestedPlaceholders(): array } /** - * @return list> + * @return array */ final public function getPlaceholders(): array { @@ -122,10 +145,78 @@ final public function getPlaceholders(): array } /** - * @return list> + * @return array */ abstract public function getSpecificPlaceholders(): array; + public function requiresRecipientByPlaceholderName(string $placeholder_name): bool + { + $found = $this->findPlaceholderByMustacheName($placeholder_name); + if ($found === null) { + return false; + } + + return $this->placeholderDefinitionRequiresRecipient($found['key'], $found['definition']); + } + + public function supportsConditionByPlaceholderName(string $placeholder_name): bool + { + $found = $this->findPlaceholderByMustacheName($placeholder_name); + if ($found === null) { + return false; + } + + return isset($found['definition']['supportsCondition']) && + $found['definition']['supportsCondition']; + } + + /** + * @return array{key: string, definition: array{ + * placeholder: string, + * label: string, + * requiresRecipient?: bool, + * supportsCondition?: bool, + * supportsNestedPlaceholders?: bool + * }}|null + */ + private function findPlaceholderByMustacheName(string $placeholder_name): ?array + { + foreach ($this->getPlaceholders() as $key => $placeholder_definition) { + if (strtoupper($placeholder_definition['placeholder']) !== strtoupper($placeholder_name)) { + continue; + } + + return [ + 'key' => (string) $key, + 'definition' => $placeholder_definition, + ]; + } + + return null; + } + + /** + * @param array{ + * placeholder: string, + * label: string, + * requiresRecipient?: bool, + * supportsCondition?: bool, + * supportsNestedPlaceholders?: bool + * } $placeholder_definition + */ + private function placeholderDefinitionRequiresRecipient(string $key, array $placeholder_definition): bool + { + return $placeholder_definition['requiresRecipient'] ?? + (array_key_exists($key, $this->getGenericPlaceholders()) && + !in_array($key, ['ilias_url', 'installation_name'], true)); + } + /** * @param array $context_parameters */ diff --git a/components/ILIAS/Mail/classes/class.ilMailTemplatePlaceholderToEmptyResolver.php b/components/ILIAS/Mail/classes/class.ilMailTemplatePlaceholderToEmptyResolver.php deleted file mode 100755 index d8cfe4d969d8..000000000000 --- a/components/ILIAS/Mail/classes/class.ilMailTemplatePlaceholderToEmptyResolver.php +++ /dev/null @@ -1,35 +0,0 @@ - - */ -class ilMailTemplatePlaceholderToEmptyResolver -{ - public function resolve( - string $message, - ): string { - return preg_replace( - "/({{)(\w+)(}})/", - "$2", - $message - ); - } -} diff --git a/components/ILIAS/Mail/tests/bootstrap.php b/components/ILIAS/Mail/tests/bootstrap.php index fad9a3dad4a7..f9757f080b0d 100755 --- a/components/ILIAS/Mail/tests/bootstrap.php +++ b/components/ILIAS/Mail/tests/bootstrap.php @@ -19,4 +19,4 @@ declare(strict_types=1); require_once __DIR__ . '/../../../../vendor/composer/vendor/autoload.php'; -require_once __DIR__ . '/ilMailBaseTest.php'; +require_once __DIR__ . '/ilMailBaseTestCase.php'; diff --git a/components/ILIAS/Mail/tests/ilMailTemplatePlaceholderToEmptyResolverTest.php b/components/ILIAS/Mail/tests/ilMailTemplatePlaceholderToEmptyResolverTest.php new file mode 100644 index 000000000000..7884cd709174 --- /dev/null +++ b/components/ILIAS/Mail/tests/ilMailTemplatePlaceholderToEmptyResolverTest.php @@ -0,0 +1,99 @@ +getMockBuilder(ilLanguage::class) + ->disableOriginalConstructor() + ->onlyMethods(['txt', 'loadLanguageModule']) + ->getMock(); + $lng->method('txt')->willReturnArgument(0); + $this->setGlobalVariable('lng', $lng); + ilDatePresentation::setLanguage($lng); + + $env_helper = $this->createMock(ilMailEnvironmentHelper::class); + $env_helper->method('getClientId')->willReturn('phpunit_client'); + $env_helper->method('getHttpPath')->willReturn('https://ilias.example/'); + + $lng_helper = $this->createMock(ilMailLanguageHelper::class); + $lng_helper->method('getCurrentLanguage')->willReturn($lng); + + $context = new class ( + new OrgUnitUserService(), + $env_helper, + new ilMailUserHelper(), + $lng_helper + ) extends ilMailTemplateContext { + public function getId(): string + { + return 'phpunit_context'; + } + + public function getTitle(): string + { + return 'phpunit'; + } + + public function getDescription(): string + { + return 'phpunit'; + } + + public function getSpecificPlaceholders(): array + { + return [ + 'course_title' => [ + 'placeholder' => 'COURSE_TITLE', + 'label' => 'Course Title', + ], + ]; + } + + public function resolveSpecificPlaceholder( + string $placeholder_id, + array $context_parameters, + ilObjUser $recipient = null + ): string { + if ('course_title' === $placeholder_id) { + return 'My Course'; + } + + return ''; + } + }; + + $resolver = new ilMailTemplatePlaceholderResolver(new Mustache_Engine()); + $message = 'Hello {{FIRST_NAME}} {{LAST_NAME}}, welcome to {{COURSE_TITLE}} at {{ILIAS_URL}}{{INSTALLATION_NAME}}'; + + $resolved = $resolver->resolve($context, $message, null, ['ref_id' => 123]); + + $this->assertStringContainsString('FIRST_NAME', $resolved); + $this->assertStringContainsString('LAST_NAME', $resolved); + $this->assertStringNotContainsString('{{FIRST_NAME}}', $resolved); + $this->assertStringNotContainsString('{{LAST_NAME}}', $resolved); + $this->assertStringContainsString('My Course', $resolved); + $this->assertStringContainsString('https://ilias.example/', $resolved); + $this->assertStringContainsString('phpunit_client', $resolved); + } +} diff --git a/components/ILIAS/Mail/tests/ilMailTest.php b/components/ILIAS/Mail/tests/ilMailTest.php index c96096262c83..3ea060f4bd6f 100755 --- a/components/ILIAS/Mail/tests/ilMailTest.php +++ b/components/ILIAS/Mail/tests/ilMailTest.php @@ -612,7 +612,6 @@ static function (string $login): int { $this->getMockBuilder(ilObjUser::class)->disableOriginalConstructor()->getMock(), $this->getMockBuilder(ilMailTemplatePlaceholderResolver::class)->disableOriginalConstructor()->getMock(), null, - null, $this->getMockBuilder(MailSignatureService::class)->disableOriginalConstructor()->getMock(), ); } From 60cd111b3b76061a2cc262126ef37f0f21319e76 Mon Sep 17 00:00:00 2001 From: Fabian Helfer Date: Wed, 1 Jul 2026 10:48:54 +0200 Subject: [PATCH 2/2] [Bugfix] Mail 047999: Remove supoortsCondition & Update PHPDocs withh requireRecipient --- .../class.ilMailTemplateContextAdapter.php | 10 +---- .../classes/class.ilMailTemplateContext.php | 38 +++++-------------- 2 files changed, 12 insertions(+), 36 deletions(-) diff --git a/components/ILIAS/Mail/classes/Mustache/class.ilMailTemplateContextAdapter.php b/components/ILIAS/Mail/classes/Mustache/class.ilMailTemplateContextAdapter.php index 72e4b09f38a1..8b5db46733d8 100755 --- a/components/ILIAS/Mail/classes/Mustache/class.ilMailTemplateContextAdapter.php +++ b/components/ILIAS/Mail/classes/Mustache/class.ilMailTemplateContextAdapter.php @@ -76,15 +76,9 @@ public function __get(string $name): string if ($this->recipient === null) { foreach ($this->contexts as $context) { - if (!$context->requiresRecipientByPlaceholderName($name)) { - continue; + if ($context->requiresRecipientByPlaceholderName($name)) { + return $name; } - - if ($context->supportsConditionByPlaceholderName($name)) { - return ''; - } - - return $name; } } diff --git a/components/ILIAS/Mail/classes/class.ilMailTemplateContext.php b/components/ILIAS/Mail/classes/class.ilMailTemplateContext.php index 716d450d7ab5..64f0e07d40ea 100755 --- a/components/ILIAS/Mail/classes/class.ilMailTemplateContext.php +++ b/components/ILIAS/Mail/classes/class.ilMailTemplateContext.php @@ -54,15 +54,15 @@ abstract public function getDescription(): string; /** * @return array{ - * mail_salutation: array{placeholder: string, label: string, supportsNestedPlaceholders?: true}, - * first_name: array{placeholder: string, label: string}, - * last_name: array{placeholder: string, label: string}, - * login: array{placeholder: string, label: string}, - * title: array{placeholder: string, label: string, supportsCondition: true}, - * firstname_lastname_superior: array{placeholder: string, label: string}, + * mail_salutation: array{placeholder: string, label: string, requiresRecipient: true, supportsNestedPlaceholders: true}, + * first_name: array{placeholder: string, label: string, requiresRecipient: true}, + * last_name: array{placeholder: string, label: string, requiresRecipient: true}, + * login: array{placeholder: string, label: string, requiresRecipient: true}, + * title: array{placeholder: string, label: string, requiresRecipient: true}, + * firstname_lastname_superior: array{placeholder: string, label: string, requiresRecipient: true}, * ilias_url: array{placeholder: string, label: string}, * installation_name: array{placeholder: string, label: string} - * } + * } */ private function getGenericPlaceholders(): array { @@ -92,7 +92,6 @@ private function getGenericPlaceholders(): array 'placeholder' => 'TITLE', 'label' => $this->getLanguage()->txt('mail_nacc_title'), 'requiresRecipient' => true, - 'supportsCondition' => true, ], 'firstname_lastname_superior' => [ 'placeholder' => 'FIRSTNAME_LASTNAME_SUPERIOR', @@ -132,7 +131,6 @@ public function getNestedPlaceholders(): array * placeholder: string, * label: string, * requiresRecipient?: bool, - * supportsCondition?: bool, * supportsNestedPlaceholders?: bool * }> */ @@ -149,7 +147,6 @@ final public function getPlaceholders(): array * placeholder: string, * label: string, * requiresRecipient?: bool, - * supportsCondition?: bool, * supportsNestedPlaceholders?: bool * }> */ @@ -162,18 +159,7 @@ public function requiresRecipientByPlaceholderName(string $placeholder_name): bo return false; } - return $this->placeholderDefinitionRequiresRecipient($found['key'], $found['definition']); - } - - public function supportsConditionByPlaceholderName(string $placeholder_name): bool - { - $found = $this->findPlaceholderByMustacheName($placeholder_name); - if ($found === null) { - return false; - } - - return isset($found['definition']['supportsCondition']) && - $found['definition']['supportsCondition']; + return $this->placeholderDefinitionRequiresRecipient($found['definition']); } /** @@ -181,7 +167,6 @@ public function supportsConditionByPlaceholderName(string $placeholder_name): bo * placeholder: string, * label: string, * requiresRecipient?: bool, - * supportsCondition?: bool, * supportsNestedPlaceholders?: bool * }}|null */ @@ -206,15 +191,12 @@ private function findPlaceholderByMustacheName(string $placeholder_name): ?array * placeholder: string, * label: string, * requiresRecipient?: bool, - * supportsCondition?: bool, * supportsNestedPlaceholders?: bool * } $placeholder_definition */ - private function placeholderDefinitionRequiresRecipient(string $key, array $placeholder_definition): bool + private function placeholderDefinitionRequiresRecipient(array $placeholder_definition): bool { - return $placeholder_definition['requiresRecipient'] ?? - (array_key_exists($key, $this->getGenericPlaceholders()) && - !in_array($key, ['ilias_url', 'installation_name'], true)); + return $placeholder_definition['requiresRecipient'] ?? false; } /**