feat: markdown in email body for mailing and transactional

tests/Email/EmailTextMailRenderingTest.php

@@ -7,6 +7,7 @@ namespace App\Tests\Email;
 use App\Config\EmailTextCatalog;
 use App\Config\EmailTextKey;
 use App\Email\EmailTextRenderer;
+use App\Email\MailBodyRenderer;
 use App\Email\Mailer;
 use App\Repository\EmailTextRepository;
 use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
@@ -42,7 +43,7 @@ class EmailTextMailRenderingTest extends KernelTestCase
         $repository = $this->createMock(EmailTextRepository::class);
         $repository->method('findByKey')->willReturn(null);
 
-        $text = (new EmailTextRenderer($catalog, $repository))
+        $text = (new EmailTextRenderer($catalog, $repository, $container->get(MailBodyRenderer::class)))
             ->render($key, $placeholders)
         ;
 

tests/Email/EmailTextRendererTest.php

@@ -7,6 +7,7 @@ namespace App\Tests\Email;
 use App\Config\EmailTextCatalog;
 use App\Config\EmailTextKey;
 use App\Email\EmailTextRenderer;
+use App\Email\MailBodyRenderer;
 use App\Entity\EmailText;
 use App\Repository\EmailTextRepository;
 use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
@@ -41,7 +42,11 @@ class EmailTextRendererTest extends KernelTestCase
         $this->assertStringContainsString('Skireise', $rendered->bodyHtml);
     }
 
-    public function testMarkupTypedByAnAdminIsEscaped(): void
+    /**
+     * Admins write Markdown, so markup they type is dropped rather than shown back to them
+     * as entities - a mail full of <script> would be nobody's intention either.
+     */
+    public function testMarkupTypedByAnAdminIsStripped(): void
     {
         $rendered = $this->render(
             $this->emailText(body: 'Hallo <script>alert(1)</script>'),
@@ -49,7 +54,7 @@ class EmailTextRendererTest extends KernelTestCase
         );
 
         $this->assertStringNotContainsString('<script>', $rendered->bodyHtml);
-        $this->assertStringContainsString('&lt;script&gt;', $rendered->bodyHtml);
+        $this->assertStringNotContainsString('&lt;script&gt;', $rendered->bodyHtml);
     }
 
     /**
@@ -66,16 +71,53 @@ class EmailTextRendererTest extends KernelTestCase
         $this->assertStringContainsString('&lt;img', $rendered->bodyHtml);
     }
 
-    public function testAsterisksMakeTextBold(): void
+    public function testDoubleAsterisksMakeTextBold(): void
     {
         $rendered = $this->render(
-            $this->emailText(body: 'Das ist *wichtig* und das nicht'),
+            $this->emailText(body: 'Das ist **wichtig** und das nicht'),
             ['destination' => 'Skireise']
         );
 
         $this->assertSame('<p>Das ist <strong>wichtig</strong> und das nicht</p>', $rendered->bodyHtml);
     }
 
+    public function testSingleAsterisksMakeTextItalic(): void
+    {
+        $rendered = $this->render(
+            $this->emailText(body: 'Das ist *anders* gemeint'),
+            ['destination' => 'Skireise']
+        );
+
+        $this->assertSame('<p>Das ist <em>anders</em> gemeint</p>', $rendered->bodyHtml);
+    }
+
+    public function testDashesAndNumbersBecomeLists(): void
+    {
+        $rendered = $this->render(
+            $this->emailText(body: "- Ausweis\n- Bankverbindung"),
+            ['destination' => 'Skireise']
+        );
+
+        $this->assertSame(
+            "<ul>\n<li>Ausweis</li>\n<li>Bankverbindung</li>\n</ul>",
+            $rendered->bodyHtml
+        );
+    }
+
+    /**
+     * The mail sets its own h1 above the body and styles nothing below h3, so an admin's
+     * heading is folded into what is left.
+     */
+    public function testHeadingsAreFoldedIntoTheLevelsTheMailStyles(): void
+    {
+        $rendered = $this->render(
+            $this->emailText(body: "# Ganz gross\n\n#### Ganz klein"),
+            ['destination' => 'Skireise']
+        );
+
+        $this->assertSame("<h2>Ganz gross</h2>\n<h3>Ganz klein</h3>", $rendered->bodyHtml);
+    }
+
     public function testAnEscapedAsteriskStaysLiteral(): void
     {
         $rendered = $this->render(
@@ -94,11 +136,11 @@ class EmailTextRendererTest extends KernelTestCase
     {
         $rendered = $this->render(
             $this->emailText(body: 'Einsatz {destination}'),
-            ['destination' => '*nicht fett*']
+            ['destination' => '**nicht fett**']
         );
 
         $this->assertStringNotContainsString('<strong>', $rendered->bodyHtml);
-        $this->assertStringContainsString('*nicht fett*', $rendered->bodyHtml);
+        $this->assertStringContainsString('**nicht fett**', $rendered->bodyHtml);
     }
 
     public function testAnEmptyPlaceholderFallsBackToADash(): void
@@ -196,14 +238,15 @@ class EmailTextRendererTest extends KernelTestCase
 
         $renderer = new EmailTextRenderer(
             self::getContainer()->get(EmailTextCatalog::class),
-            $repository
+            $repository,
+            self::getContainer()->get(MailBodyRenderer::class)
         );
 
         $rendered = $renderer->renderValues(
             EmailTextKey::APPLICATION_REJECTED,
             'Entwurf {destination}',
             'Überschrift',
-            'Entwurfstext *fett*',
+            'Entwurfstext **fett**',
             ['destination' => 'Skireise']
         );
 
@@ -221,7 +264,8 @@ class EmailTextRendererTest extends KernelTestCase
 
         $renderer = new EmailTextRenderer(
             self::getContainer()->get(EmailTextCatalog::class),
-            $this->createMock(EmailTextRepository::class)
+            $this->createMock(EmailTextRepository::class),
+            self::getContainer()->get(MailBodyRenderer::class)
         );
 
         $rendered = $renderer->renderValues(
@@ -252,7 +296,8 @@ class EmailTextRendererTest extends KernelTestCase
 
         $renderer = new EmailTextRenderer(
             self::getContainer()->get(EmailTextCatalog::class),
-            $repository
+            $repository,
+            self::getContainer()->get(MailBodyRenderer::class)
         );
 
         return $renderer->render(EmailTextKey::APPLICATION_REJECTED, $placeholders);

tests/Email/MailBodyRendererTest.php

@@ -0,0 +1,174 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Email;
+
+use App\Email\MailBodyRenderer;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * The renderer is the only thing standing between what an admin typed and the HTML of a
+ * mail, and between what a teamer typed and that same HTML. Most of what is asserted here
+ * is where that line runs: admin text may become markup, a substituted value may not.
+ */
+class MailBodyRendererTest extends TestCase
+{
+    private MailBodyRenderer $renderer;
+
+    protected function setUp(): void
+    {
+        $this->renderer = new MailBodyRenderer();
+    }
+
+    public function testDoubleAsterisksMakeTextBoldAndSingleOnesItalic(): void
+    {
+        $this->assertSame(
+            '<p><strong>fett</strong> und <em>kursiv</em></p>',
+            $this->renderer->render('**fett** und *kursiv*', [])
+        );
+    }
+
+    public function testDashesBecomeABulletList(): void
+    {
+        $this->assertSame(
+            "<ul>\n<li>Ausweis</li>\n<li>Bankverbindung</li>\n</ul>",
+            $this->renderer->render("- Ausweis\n- Bankverbindung", [])
+        );
+    }
+
+    public function testNumbersBecomeANumberedList(): void
+    {
+        $this->assertSame(
+            "<ol>\n<li>Erstens</li>\n<li>Zweitens</li>\n</ol>",
+            $this->renderer->render("1. Erstens\n2. Zweitens", [])
+        );
+    }
+
+    /**
+     * The mail layout sets its own h1 above the body and styles nothing below h3, so every
+     * heading is folded into that range rather than producing a tag nobody styled.
+     */
+    public function testHeadingsAreFoldedIntoTheLevelsTheMailStyles(): void
+    {
+        $this->assertSame(
+            "<h2>Gross</h2>\n<h2>Mittel</h2>\n<h3>Klein</h3>\n<h3>Winzig</h3>",
+            $this->renderer->render("# Gross\n\n## Mittel\n\n### Klein\n\n###### Winzig", [])
+        );
+    }
+
+    public function testBlankLinesBecomeParagraphsAndSingleBreaksStay(): void
+    {
+        $this->assertSame(
+            "<p>Erste Zeile<br />\nZweite Zeile</p>\n<p>Neuer Absatz</p>",
+            $this->renderer->render("Erste Zeile\nZweite Zeile\n\nNeuer Absatz", [])
+        );
+    }
+
+    public function testBareUrlsAndMailAddressesBecomeLinks(): void
+    {
+        $html = $this->renderer->render('Schreib an [email protected] oder https://ep-reisen.de/team.', []);
+
+        $this->assertStringContainsString('<a href="mailto:[email protected]">[email protected]</a>', $html);
+        // The full stop ends the sentence, it is not part of the address.
+        $this->assertStringContainsString('<a href="https://ep-reisen.de/team">https://ep-reisen.de/team</a>.', $html);
+    }
+
+    public function testAnEscapedAsteriskStaysLiteral(): void
+    {
+        $this->assertSame('<p>Ein Sternchen: *</p>', $this->renderer->render('Ein Sternchen: \*', []));
+    }
+
+    /**
+     * Admins write Markdown, so markup they type is dropped rather than passed through.
+     */
+    public function testMarkupTypedByAnAdminIsStripped(): void
+    {
+        $html = $this->renderer->render('Hallo <b>alle</b> <script>alert(1)</script>', []);
+
+        $this->assertStringNotContainsString('<b>', $html);
+        $this->assertStringNotContainsString('<script', $html);
+    }
+
+    /**
+     * An image in a mailing would be loaded from an address nobody reviewed, so only the
+     * alt text survives.
+     */
+    public function testAnImageIsReducedToItsAltText(): void
+    {
+        $html = $this->renderer->render('![Ein Bild](https://example.org/pixel.png)', []);
+
+        $this->assertStringNotContainsString('<img', $html);
+        $this->assertStringContainsString('Ein Bild', $html);
+    }
+
+    public function testAnUnsafeLinkLosesItsTarget(): void
+    {
+        $html = $this->renderer->render('[klick](javascript:alert(1))', []);
+
+        $this->assertStringNotContainsString('javascript:', $html);
+        $this->assertStringContainsString('klick', $html);
+    }
+
+    public function testPlaceholdersAreSubstituted(): void
+    {
+        $this->assertSame(
+            '<p>Hallo Anna, dein Einsatz Skireise</p>',
+            $this->renderer->render(
+                'Hallo {{vorname}}, dein Einsatz {destination}',
+                ['{{vorname}}' => 'Anna', '{destination}' => 'Skireise']
+            )
+        );
+    }
+
+    /**
+     * The load-bearing case. A teamer's own words go in after the parser has run, so
+     * nothing in them can become a tag, a heading, a list or emphasis - whatever they
+     * happen to contain.
+     */
+    public function testNothingInAPlaceholderValueBecomesMarkup(): void
+    {
+        $html = $this->renderer->render('Kommentar: {kommentar}', [
+            '{kommentar}' => "**nicht fett**\n- keine Liste\n# keine Überschrift <b>x</b>",
+        ]);
+
+        $this->assertStringNotContainsString('<strong>', $html);
+        $this->assertStringNotContainsString('<ul>', $html);
+        $this->assertStringNotContainsString('<h1', $html);
+        $this->assertStringNotContainsString('<h2', $html);
+        $this->assertStringNotContainsString('<b>', $html);
+        $this->assertStringContainsString('**nicht fett**', $html);
+        $this->assertStringContainsString('- keine Liste', $html);
+        $this->assertStringContainsString('&lt;b&gt;x&lt;/b&gt;', $html);
+    }
+
+    /**
+     * A rejection comment arrives as one multi-line value inside a single paragraph.
+     */
+    public function testLineBreaksInsideAPlaceholderValueSurvive(): void
+    {
+        $this->assertSame(
+            "<p>Erste Zeile<br />\nZweite Zeile</p>",
+            $this->renderer->render('{kommentar}', ['{kommentar}' => "Erste Zeile\nZweite Zeile"])
+        );
+    }
+
+    /**
+     * Documented rather than desired: substitution runs over the finished HTML, so a token
+     * an admin put in backticks is replaced like any other. Harmless, but surprising enough
+     * to pin down.
+     */
+    public function testATokenInACodeSpanIsStillSubstituted(): void
+    {
+        $this->assertSame(
+            '<p><code>Skireise</code></p>',
+            $this->renderer->render('`{destination}`', ['{destination}' => 'Skireise'])
+        );
+    }
+
+    public function testAnEmptyBodyRendersAsNothing(): void
+    {
+        $this->assertSame('', $this->renderer->render('', []));
+        $this->assertSame('', $this->renderer->render(null, []));
+    }
+}

tests/Migrations/EmphasisRewriteTest.php

@@ -0,0 +1,84 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Migrations;
+
+use PHPUnit\Framework\TestCase;
+use Symfony\Component\Yaml\Yaml;
+
+/**
+ * Version20260820000000 rewrites the wording of mails that go out to every teamer, and it
+ * runs exactly once with nobody watching, so the patterns it does that with are pinned
+ * here. The migration is not autoloaded (only src/ and tests/ are), hence the require.
+ */
+class EmphasisRewriteTest extends TestCase
+{
+    private string $single;
+    private string $double;
+
+    protected function setUp(): void
+    {
+        require_once __DIR__.'/../../migrations/Version20260820000000.php';
+
+        $reflection = new \ReflectionClass(\DoctrineMigrations\Version20260820000000::class);
+
+        $this->single = $reflection->getConstant('SINGLE_EMPHASIS');
+        $this->double = $reflection->getConstant('DOUBLE_EMPHASIS');
+    }
+
+    /**
+     * @dataProvider upCases
+     */
+    public function testTheUpwardRewrite(string $before, string $after): void
+    {
+        $this->assertSame($after, preg_replace($this->single, '**$1**', $before));
+    }
+
+    /**
+     * @return iterable<string, array{string, string}>
+     */
+    public static function upCases(): iterable
+    {
+        yield 'one word' => ['Das ist *wichtig*', 'Das ist **wichtig**'];
+        yield 'several words' => ['*Dein Einsatz wurde angenommen!*', '**Dein Einsatz wurde angenommen!**'];
+        yield 'around a placeholder' => ['*{destination}*', '**{destination}**'];
+        yield 'already bold' => ['Das ist **wichtig**', 'Das ist **wichtig**'];
+        yield 'escaped asterisk' => ['Ein Sternchen: \*', 'Ein Sternchen: \*'];
+        // Whitespace next to a marker means it is not emphasis - CommonMark's own rule, and
+        // the reason arithmetic and stray asterisks come through untouched.
+        yield 'multiplication' => ['5 * 3 = 15', '5 * 3 = 15'];
+        yield 'spaced asterisks' => ['a * b * c', 'a * b * c'];
+        yield 'bullet at line start' => ["* Ausweis\n* Bankverbindung", "* Ausweis\n* Bankverbindung"];
+        yield 'not across lines' => ["*offen\ngeblieben*", "*offen\ngeblieben*"];
+    }
+
+    public function testTheRewriteIsReversible(): void
+    {
+        $before = 'Das ist *wichtig* und *{destination}* auch';
+
+        $up = preg_replace($this->single, '**$1**', $before);
+
+        $this->assertSame($before, preg_replace($this->double, '*$1*', $up));
+    }
+
+    /**
+     * The delivered defaults were rewritten by hand in the same commit, so nothing should
+     * be left in them for the migration to find. Read through the parser rather than off
+     * the raw file: the header comment documents *kursiv* on purpose.
+     */
+    public function testTheDeliveredDefaultsAreAlreadyMigrated(): void
+    {
+        $texts = Yaml::parseFile(__DIR__.'/../../config/email_texts.yaml')['texts'];
+
+        foreach ($texts as $key => $text) {
+            $body = (string) ($text['body'] ?? '');
+
+            $this->assertSame(
+                $body,
+                preg_replace($this->single, '**$1**', $body),
+                sprintf('Default wording of "%s" still carries single-asterisk emphasis', $key)
+            );
+        }
+    }
+}

tests/Service/Teamer/TeamerMailingServiceTest.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace App\Tests\Service\Teamer;
 
+use App\Email\MailBodyRenderer;
 use App\Email\Mailer;
 use App\Entity\Teamer;
 use App\Entity\User;
@@ -30,9 +31,12 @@ class TeamerMailingServiceTest extends TestCase
         $this->teamerRepository = $this->createMock(TeamerRepository::class);
         $this->mailer = $this->createMock(Mailer::class);
 
+        // The real renderer, not a mock: what the mailing puts in front of a teamer is
+        // Markdown turned into HTML, and getting that wrong is the whole risk here.
         $this->service = new TeamerMailingService(
             $this->teamerRepository,
             $this->mailer,
+            new MailBodyRenderer(),
             $this->createMock(LoggerInterface::class),
             'mailing',
         );
@@ -111,7 +115,7 @@ class TeamerMailingServiceTest extends TestCase
             ->expects($this->exactly(2))
             ->method('createAndSendEmail')
             ->willReturnCallback(function (array $context, array $options) use (&$sent): void {
-                $sent[] = [$options['to'], $options['subject'], $context['message']];
+                $sent[] = [$options['to'], $options['subject'], $context['bodyHtml']];
             })
         ;
 
@@ -123,8 +127,8 @@ class TeamerMailingServiceTest extends TestCase
         ));
 
         $this->assertSame([
-            ['[email protected]', 'Hallo Anna', 'Servus Anna Berg'],
-            ['[email protected]', 'Hallo Bea', 'Servus Bea Ohm'],
+            ['[email protected]', 'Hallo Anna', '<p>Servus Anna Berg</p>'],
+            ['[email protected]', 'Hallo Bea', '<p>Servus Bea Ohm</p>'],
         ], $sent);
     }
 
@@ -210,7 +214,7 @@ class TeamerMailingServiceTest extends TestCase
             ->expects($this->once())
             ->method('createAndSendEmail')
             ->with(
-                ['message' => 'Servus Rita Kern'],
+                ['bodyHtml' => '<p>Servus Rita Kern</p>'],
                 $this->callback(function (array $options): bool {
                     $this->assertSame('[email protected]', $options['to']);
                     $this->assertSame('[Vorschau] Hallo Rita', $options['subject']);