Repository: urlaubsverwaltung/urlaubsverwaltung (java)

Issue:
Unterstützung von reply-to beim Versenden von E-Mails
Wir möchten reply-to unterstützen und zudem "uv.mail.sender*" zu "uv.mail.from*" vereinheitlichen.

Pull request:
Add reply to to mail service and rename sender to from
closes #4310 

Patch:
diff --git a/.examples/with-docker-compose/docker-compose.yaml b/.examples/with-docker-compose/docker-compose.yaml
index 1f0c2f4f8..fb25e7ce8 100644
--- a/.examples/with-docker-compose/docker-compose.yaml
+++ b/.examples/with-docker-compose/docker-compose.yaml
@@ -11,8 +11,10 @@ services:
       SPRING_DATASOURCE_USERNAME: urlaubsverwaltung
       SPRING_DATASOURCE_PASSWORD: urlaubsverwaltung
       # Mail
-      UV_MAIL_SENDER: urlaubsverwaltung@example.org
-      UV_MAIL_SENDERDISPLAYNAME: urlaubsverwaltung
+      UV_MAIL_FROM: urlaubsverwaltung@example.org
+      UV_MAIL_FROMDISPLAYNAME: urlaubsverwaltung
+      UV_MAIL_REPLYTO: urlaubsverwaltung@example.org
+      UV_MAIL_REPLYTODISPLAYNAME: urlaubsverwaltung
       UV_MAIL_APPLICATIONURL: http://localhost:8080
       UV_MAIL_ADMINISTRATOR: admin@example.org
       SPRING_MAIL_HOST: localhost
diff --git a/README.md b/README.md
index 753c0d397..3ef7da509 100644
--- a/README.md
+++ b/README.md
@@ -138,8 +138,10 @@ uv.development.demodata.additional-inactive-user=0
 # mail
 uv.mail.administrator
 uv.mail.application-url
-uv.mail.sender
-uv.mail.senderDisplayName=Urlaubsverwaltung
+uv.mail.from
+uv.mail.fromDisplayName=Urlaubsverwaltung
+uv.mail.replyTo
+uv.mail.replyToDisplayName=Urlaubsverwaltung
 
 # security - openid connect
 uv.security.oidc.post-logout-redirect-uri={baseUrl}
diff --git a/charts/urlaubsverwaltung/README.md b/charts/urlaubsverwaltung/README.md
index 778c417b3..9506a6ec3 100644
--- a/charts/urlaubsverwaltung/README.md
+++ b/charts/urlaubsverwaltung/README.md
@@ -142,8 +142,10 @@ extraEnv: |
       value: "true"
     - name: UV_MAIL_APPLICATION-URL
       value: https://urlaubsverwaltung.example.org
-    - name: UV_MAIL_SENDER
+    - name: UV_MAIL_FROM
       value: urlaubsverwaltung@example.org
+    - name: UV_MAIL_REPLYTO
+      value: no-reply@example.org
     - name: UV_MAIL_ADMINISTRATOR
       value: root@example.org
     - name: MANAGEMENT_HEALTH_MAIL_ENABLED
diff --git a/src/main/java/org/synyx/urlaubsverwaltung/mail/MailProperties.java b/src/main/java/org/synyx/urlaubsverwaltung/mail/MailProperties.java
index 2dea809d2..582703dc5 100644
--- a/src/main/java/org/synyx/urlaubsverwaltung/mail/MailProperties.java
+++ b/src/main/java/org/synyx/urlaubsverwaltung/mail/MailProperties.java
@@ -12,10 +12,17 @@ public class MailProperties {
 
     @Email
     @NotEmpty
-    private String sender;
+    private String from;
 
     @NotEmpty
-    private String senderDisplayName;
+    private String fromDisplayName;
+
+    @Email
+    @NotEmpty
+    private String replyTo;
+
+    @NotEmpty
+    private String replyToDisplayName;
 
     @Email
     @NotEmpty
@@ -24,20 +31,36 @@ public class MailProperties {
     @URL
     private String applicationUrl;
 
-    public String getSender() {
-        return sender;
+    public String getFrom() {
+        return from;
+    }
+
+    public void setFrom(String from) {
+        this.from = from;
+    }
+
+    public String getFromDisplayName() {
+        return fromDisplayName;
+    }
+
+    public void setFromDisplayName(String fromDisplayName) {
+        this.fromDisplayName = fromDisplayName;
+    }
+
+    public String getReplyTo() {
+        return replyTo;
     }
 
-    public void setSender(String sender) {
-        this.sender = sender;
+    public void setReplyTo(String replyTo) {
+        this.replyTo = replyTo;
     }
 
-    public String getSenderDisplayName() {
-        return senderDisplayName;
+    public String getReplyToDisplayName() {
+        return replyToDisplayName;
     }
 
-    public void setSenderDisplayName(String senderDisplayName) {
-        this.senderDisplayName = senderDisplayName;
+    public void setReplyToDisplayName(String replyToDisplayName) {
+        this.replyToDisplayName = replyToDisplayName;
     }
 
     public String getAdministrator() {
diff --git a/src/main/java/org/synyx/urlaubsverwaltung/mail/MailSenderService.java b/src/main/java/org/synyx/urlaubsverwaltung/mail/MailSenderService.java
index 794dbeccf..fd4134f99 100644
--- a/src/main/java/org/synyx/urlaubsverwaltung/mail/MailSenderService.java
+++ b/src/main/java/org/synyx/urlaubsverwaltung/mail/MailSenderService.java
@@ -37,7 +37,7 @@ class MailSenderService {
      * @param subject   mail subject
      * @param text      mail body
      */
-    void sendEmail(String from, @Nullable String recipient, String subject, String text) {
+    void sendEmail(String from, String replyTo, @Nullable String recipient, String subject, String text) {
 
         if (recipient == null || recipient.isBlank()) {
             LOG.warn("Could not send email to empty recipients!");
@@ -46,11 +46,26 @@ class MailSenderService {
 
         final SimpleMailMessage mailMessage = new SimpleMailMessage();
         mailMessage.setFrom(from);
+        mailMessage.setReplyTo(replyTo);
         mailMessage.setTo(recipient);
         mailMessage.setSubject(subject);
         mailMessage.setText(text);
 
-        send(mailMessage);
+        try {
+            mailSender.send(mailMessage);
+
+            if (LOG.isDebugEnabled()) {
+                for (String recipient1 : mailMessage.getTo()) {
+                    LOG.debug("Sent email to {}", recipient1);
+                }
+                LOG.debug("To={}\n\nSubject={}\n\nText={}",
+                    Arrays.toString(mailMessage.getTo()), mailMessage.getSubject(), mailMessage.getText());
+            }
+        } catch (MailException ex) {
+            for (String recipient1 : mailMessage.getTo()) {
+                LOG.error("Sending email to {} failed", recipient1, ex);
+            }
+        }
     }
 
     /**
@@ -62,7 +77,7 @@ class MailSenderService {
      * @param text            mail body
      * @param mailAttachments List of attachments to add to the mail
      */
-    void sendEmail(String from, @Nullable String recipient, String subject, String text, List<MailAttachment> mailAttachments) {
+    void sendEmail(String from, String replyTo, @Nullable String recipient, String subject, String text, List<MailAttachment> mailAttachments) {
 
         if (recipient == null || recipient.isBlank()) {
             LOG.warn("Could not send email to empty recipients!");
@@ -72,8 +87,9 @@ class MailSenderService {
         final MimeMessage mimeMessage = mailSender.createMimeMessage();
         try {
             final MimeMessageHelper helper = new MimeMessageHelper(mimeMessage, true);
-            helper.setTo(recipient);
             helper.setFrom(from);
+            helper.setReplyTo(replyTo);
+            helper.setTo(recipient);
             helper.setSubject(subject);
             helper.setText(text);
 
@@ -85,23 +101,4 @@ class MailSenderService {
         }
         mailSender.send(mimeMessage);
     }
-
-    private void send(SimpleMailMessage message) {
-        try {
-
-            mailSender.send(message);
-
-            if (LOG.isDebugEnabled()) {
-                for (String recipient : message.getTo()) {
-                    LOG.debug("Sent email to {}", recipient);
-                }
-                LOG.debug("To={}\n\nSubject={}\n\nText={}",
-                    Arrays.toString(message.getTo()), message.getSubject(), message.getText());
-            }
-        } catch (MailException ex) {
-            for (String recipient : message.getTo()) {
-                LOG.error("Sending email to {} failed", recipient, ex);
-            }
-        }
-    }
 }
diff --git a/src/main/java/org/synyx/urlaubsverwaltung/mail/MailServiceImpl.java b/src/main/java/org/synyx/urlaubsverwaltung/mail/MailServiceImpl.java
index 5f5828b83..8ff35be0b 100644
--- a/src/main/java/org/synyx/urlaubsverwaltung/mail/MailServiceImpl.java
+++ b/src/main/java/org/synyx/urlaubsverwaltung/mail/MailServiceImpl.java
@@ -15,7 +15,6 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 import static java.lang.invoke.MethodHandles.lookup;
 import static org.slf4j.LoggerFactory.getLogger;
@@ -62,15 +61,17 @@ class MailServiceImpl implements MailService {
             context.setVariable("rightPadder", RightPadder.getInstance());
             context.setVariable("recipient", recipient);
 
-            final String sender = generateMailAddressAndDisplayName(mailProperties.getSender(), mailProperties.getSenderDisplayName());
+            final String from = generateMailAddressAndDisplayName(mailProperties.getFrom(), mailProperties.getFromDisplayName());
+            final String replyTo = generateMailAddressAndDisplayName(mailProperties.getReplyTo(), mailProperties.getReplyToDisplayName());
+
             final String email = recipient.getEmail();
             final String subject = getTranslation(effectiveLocale, mail.getSubjectMessageKey(), mail.getSubjectMessageArguments());
             final String body = emailTemplateEngine.process(mail.getTemplateName(), context);
 
             if (email != null) {
                 mail.getMailAttachments().ifPresentOrElse(
-                    mailAttachments -> mailSenderService.sendEmail(sender, email, subject, body, mailAttachments),
-                    () -> mailSenderService.sendEmail(sender, email, subject, body)
+                    mailAttachments -> mailSenderService.sendEmail(from, replyTo, email, subject, body, mailAttachments),
+                    () -> mailSenderService.sendEmail(from, replyTo, email, subject, body)
                 );
             } else {
                 LOG.debug("Could not send mail to E-Mail-Address of person with id {}, because email is null.", recipient.getId());
@@ -87,7 +88,9 @@ class MailServiceImpl implements MailService {
             recipients.add(new Person(null, null, "Administrator", mailProperties.getAdministrator()));
         }
 
-        return recipients.stream().distinct().collect(Collectors.toList());
+        return recipients.stream()
+            .distinct()
+            .toList();
     }
 
     private String getTranslation(Locale locale, String key, Object... args) {
diff --git a/src/main/resources/application-demodata.yaml b/src/main/resources/application-demodata.yaml
index a1871731a..2cf87420a 100644
--- a/src/main/resources/application-demodata.yaml
+++ b/src/main/resources/application-demodata.yaml
@@ -1,7 +1,8 @@
 uv:
   mail:
+    from: sender@example.org
+    replyTo: no-reply@example.org
     application-url: http://localhost:8080
-    sender: sender@example.org
     administrator: administrators@example.org
   calendar:
     organizer: organizer@example.org
diff --git a/src/main/resources/application.yaml b/src/main/resources/application.yaml
index 3038b3901..c4982f2a8 100644
--- a/src/main/resources/application.yaml
+++ b/src/main/resources/application.yaml
@@ -1,6 +1,7 @@
 uv:
   mail:
-    senderDisplayName: Urlaubsverwaltung
+    fromDisplayName: Urlaubsverwaltung
+    replyToDisplayName: Urlaubsverwaltung
 
 spring:
   web:


Test patch:
diff --git a/src/test/java/org/synyx/urlaubsverwaltung/application/application/ApplicationMailServiceIT.java b/src/test/java/org/synyx/urlaubsverwaltung/application/application/ApplicationMailServiceIT.java
index e1aad69ed..6ac7d55b9 100644
--- a/src/test/java/org/synyx/urlaubsverwaltung/application/application/ApplicationMailServiceIT.java
+++ b/src/test/java/org/synyx/urlaubsverwaltung/application/application/ApplicationMailServiceIT.java
@@ -1225,7 +1225,7 @@ class ApplicationMailServiceIT extends TestContainersBase {
         Address[] from = msg.getFrom();
         assertThat(from).isNotNull();
         assertThat(from.length).isOne();
-        assertThat(from[0]).hasToString(String.format("%s <%s>", mailProperties.getSenderDisplayName(), mailProperties.getSender()));
+        assertThat(from[0]).hasToString(String.format("%s <%s>", mailProperties.getFromDisplayName(), mailProperties.getFrom()));
     }
 
     @Test
diff --git a/src/test/java/org/synyx/urlaubsverwaltung/mail/MailSenderServiceTest.java b/src/test/java/org/synyx/urlaubsverwaltung/mail/MailSenderServiceTest.java
index 0617deab7..63b7b71a2 100644
--- a/src/test/java/org/synyx/urlaubsverwaltung/mail/MailSenderServiceTest.java
+++ b/src/test/java/org/synyx/urlaubsverwaltung/mail/MailSenderServiceTest.java
@@ -48,8 +48,9 @@ class MailSenderServiceTest {
         final String subject = "subject";
         final String body = "text";
         final String from = "from@example.org";
+        final String replyTo = "replyTo@example.org";
 
-        sut.sendEmail(from, recipient, subject, body);
+        sut.sendEmail(from, replyTo, recipient, subject, body);
 
         verify(javaMailSender).send(mailMessageArgumentCaptor.capture());
         final SimpleMailMessage mailMessage = mailMessageArgumentCaptor.getValue();
@@ -63,7 +64,7 @@ class MailSenderServiceTest {
     @NullSource
     @ValueSource(strings = {"", " "})
     void doesNotSendMailForNullRecipients(final String recipient) {
-        sut.sendEmail("from@example.org", recipient, "subject", "text");
+        sut.sendEmail("from@example.org", "replyTo@example.org", recipient, "subject", "text");
         verifyNoInteractions(javaMailSender);
     }
 
@@ -75,11 +76,13 @@ class MailSenderServiceTest {
         final String subject = "subject";
         final String body = "text";
         final String from = "from@example.org";
+        final String replyTo = "replyTo@example.org";
+        final List<MailAttachment> mailAttachments = List.of(new MailAttachment("name", new ByteArrayResource(new byte[]{})));
 
         final MimeMessage msg = new MimeMessage(getInstance(new Properties(), null));
         when(javaMailSender.createMimeMessage()).thenReturn(msg);
 
-        sut.sendEmail(from, recipient, subject, body, List.of(new MailAttachment("name", new ByteArrayResource(new byte[]{}))));
+        sut.sendEmail(from, replyTo, recipient, subject, body, mailAttachments);
 
         verify(javaMailSender).send(mailMessageArgumentCaptor.capture());
         final MimeMessage mailMessage = mailMessageArgumentCaptor.getValue();
@@ -93,7 +96,7 @@ class MailSenderServiceTest {
     @NullSource
     @ValueSource(strings = {"", " "})
     void doesNotSendMailWithAttachmentToNullRecipients(final String recipient) {
-        sut.sendEmail("from@example.org", recipient, "subject", "text", List.of());
+        sut.sendEmail("from@example.org", "replyTo@example.org", recipient, "subject", "text", List.of());
         verifyNoInteractions(javaMailSender);
     }
 
diff --git a/src/test/java/org/synyx/urlaubsverwaltung/mail/MailServiceImplTest.java b/src/test/java/org/synyx/urlaubsverwaltung/mail/MailServiceImplTest.java
index 1d9f596e4..20f9561df 100644
--- a/src/test/java/org/synyx/urlaubsverwaltung/mail/MailServiceImplTest.java
+++ b/src/test/java/org/synyx/urlaubsverwaltung/mail/MailServiceImplTest.java
@@ -44,8 +44,10 @@ class MailServiceImplTest {
     void setUp() {
         when(messageSource.getMessage(any(), any(), any())).thenReturn("subject");
         when(emailTemplateEngine.process(any(String.class), any(Context.class))).thenReturn("emailBody");
-        when(mailProperties.getSender()).thenReturn("no-reply@example.org");
-        when(mailProperties.getSenderDisplayName()).thenReturn("Urlaubsverwaltung");
+        when(mailProperties.getFrom()).thenReturn("from@example.org");
+        when(mailProperties.getFromDisplayName()).thenReturn("Urlaubsverwaltung");
+        when(mailProperties.getReplyTo()).thenReturn("no-reply@example.org");
+        when(mailProperties.getReplyToDisplayName()).thenReturn("Urlaubsverwaltung");
         when(mailProperties.getApplicationUrl()).thenReturn("http://localhost:8080");
         sut = new MailServiceImpl(messageSource, emailTemplateEngine, mailSenderService, mailProperties, userSettingsService);
     }
@@ -69,7 +71,7 @@ class MailServiceImplTest {
 
         sut.send(mail);
 
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", "hans@example.org", "subject", "emailBody");
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", "hans@example.org", "subject", "emailBody");
     }
 
     @Test
@@ -95,8 +97,8 @@ class MailServiceImplTest {
 
         sut.send(mail);
 
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", "hans@example.org", "subject", "emailBody");
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", "franz@example.org", "subject", "emailBody");
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", "hans@example.org", "subject", "emailBody");
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", "franz@example.org", "subject", "emailBody");
     }
 
     @Test
@@ -125,8 +127,8 @@ class MailServiceImplTest {
 
         sut.send(mail);
 
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", "franz@example.org", "subject", "emailBody", List.of(new MailAttachment("fileName", iCal)));
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", "hans@example.org", "subject", "emailBody", List.of(new MailAttachment("fileName", iCal)));
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", "franz@example.org", "subject", "emailBody", List.of(new MailAttachment("fileName", iCal)));
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", "hans@example.org", "subject", "emailBody", List.of(new MailAttachment("fileName", iCal)));
     }
 
     @Test
@@ -155,8 +157,8 @@ class MailServiceImplTest {
 
         sut.send(mail);
 
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", "hans@example.org", "subject", "emailBody", List.of(new MailAttachment("fileName", iCal)));
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", "franz@example.org", "subject", "emailBody", List.of(new MailAttachment("fileName", iCal)));
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", "hans@example.org", "subject", "emailBody", List.of(new MailAttachment("fileName", iCal)));
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", "franz@example.org", "subject", "emailBody", List.of(new MailAttachment("fileName", iCal)));
     }
 
     @Test
@@ -166,7 +168,7 @@ class MailServiceImplTest {
 
         final String subjectMessageKey = "subject.overtime.created";
         final String templateName = "overtime_office";
-        String to = "admin@example.org";
+        final String to = "admin@example.org";
         when(mailProperties.getAdministrator()).thenReturn(to);
 
         final Mail mail = Mail.builder()
@@ -176,7 +178,7 @@ class MailServiceImplTest {
             .build();
         sut.send(mail);
 
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", to, "subject", "emailBody");
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", to, "subject", "emailBody");
     }
 
     @Test
@@ -201,8 +203,8 @@ class MailServiceImplTest {
 
         sut.send(mail);
 
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", "franz@example.org", "subject", "emailBody");
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", "admin@example.org", "subject", "emailBody");
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", "franz@example.org", "subject", "emailBody");
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", "admin@example.org", "subject", "emailBody");
     }
 
     @Test
@@ -224,7 +226,7 @@ class MailServiceImplTest {
 
         sut.send(mail);
 
-        verify(mailSenderService).sendEmail("Urlaubsverwaltung <no-reply@example.org>", "franz@example.org", "subject", "emailBody");
+        verify(mailSenderService).sendEmail("Urlaubsverwaltung <from@example.org>", "Urlaubsverwaltung <no-reply@example.org>", "franz@example.org", "subject", "emailBody");
         verifyNoMoreInteractions(mailSenderService);
     }
 
diff --git a/src/test/resources/application.yaml b/src/test/resources/application.yaml
index 636c6ae19..2b932987c 100644
--- a/src/test/resources/application.yaml
+++ b/src/test/resources/application.yaml
@@ -3,9 +3,11 @@ uv:
     organizer: organizer@example.org
   mail:
     application-url: https://localhost:8080
-    sender: sender@example.org
     administrator: administrators@example.org
-    senderDisplayName: Urlaubsverwaltung
+    from: sender@example.org
+    fromDisplayName: Urlaubsverwaltung
+    replyTo: sender@example.org
+    replyToDisplayName: Urlaubsverwaltung
 
 spring:
   security: