feat(mail): support the use of markdown in the email body #726

woowabrie · 2023-09-04T04:29:23Z

Resolves #714

해결하려는 문제가 무엇인가요?

메일 작성 시, 본문에 스타일을 입히고 싶을 경우 마크다운으로 작성 후 html로 변환하여 입력하는 불편함이 있습니다.

어떻게 해결했나요?

일반적인 메일을 작성 시, 메일 전송 전 본문을 html로 변환하는 로직을 추가했습니다.

어떤 부분에 집중하여 리뷰해야 할까요?

MailService의 sendMailsByBcc() 함수를 사용해 메일을 보낼 경우 본문을 마크다운으로 작성을 해야 합니다. 어드민을 사용해 작성하는 경우엔 문제가 없을 것 같은데, 기존에 /api/mail 엔드포인트를 사용해서 메일을 발송하는 경우가 있는지 확인 부탁드립니다.
utils, support 두 패키지가 있는데 이전 논의를 보니 정리가 안된 것 같은데, 패키지 정리를 위한 이슈 하나를 따면 어떨까요?

참고 자료

[github] intellij-markdown

RCA 룰

r: 꼭 반영해 주세요. 적극적으로 고려해 주세요. (Request changes)
c: 웬만하면 반영해 주세요. (Comment)
a: 반영해도 좋고 넘어가도 좋습니다. 그냥 사소한 의견입니다. (Approve)

woowabrie · 2023-09-05T08:06:33Z

local에서 메일을 보내본 결과 잘 동작 하는 것 확인했습니다.
지난 기수 모집 때 사용한 메일 내용을 기반으로 했고, 스크린샷에서 노출이 불필요한 부분은 가렸어요. ㅋㅋ

로컬 테스트를 위해 변경한 내용은 아래와 같습니다.

aws 관련 토큰 추가
메일 본문 상단의 이미지 url 변경 (local 프론트엔드 url을 호출해서...)

woowahan-pjs

고생하셨습니다. 👍
코틀린 스타일로 코드를 작성하는 데 도움이 되는 몇 가지 사항을 리뷰하고 프로젝트 컨벤션을 소개해 드렸습니다.

woowahan-pjs · 2023-09-07T02:36:17Z

src/main/kotlin/apply/application/mail/MailData.kt

+    fun getHtmlBody(): String {
+        return MarkdownProcessor.generateToHtml(body)
+    }


c: 로직이 복잡하지 않고 위임만 하면 MailService에서 직접 호출할 수 있을 것 같은데, 어떻게 생각하세요?

DTO에 Markdown 의존성이 생기는 것에 공감하여 이동하였습니다.

woowahan-pjs · 2023-09-07T02:36:21Z

src/main/kotlin/apply/application/mail/MailService.kt

@@ -119,4 +111,16 @@ class MailService(
            )
        )
    }
+
+    fun createCommonMailBodyFrom(mailData: MailData): String {


c: 보다 간결하고 명확한 함수 이름을 제안해 봐요!

Suggested change

fun createCommonMailBodyFrom(mailData: MailData): String {

fun String.toMarkdownBody(): String {

리턴 되는 문자열이 html이라 toHtmlBody로 변경했어요.

woowahan-pjs · 2023-09-07T02:37:02Z

src/main/kotlin/apply/application/mail/MailService.kt

@@ -119,4 +111,16 @@ class MailService(
            )
        )
    }
+
+    fun createCommonMailBodyFrom(mailData: MailData): String {


r: MailService 외에 사용하는 곳이 없어 보이네요. public이 아닌 private으로 바꾸면 어떨까요?

미리보기 기능을 만들 때 필요해서 public으로 두었습니다.

woowahan-pjs · 2023-09-07T02:37:05Z

src/main/kotlin/apply/infra/mail/FakeMailSender.kt

+
+private val logger = KotlinLogging.logger { }
+
+class FakeMailSender : MailSender {


r: 가짜 객체는 테스트 디렉터리로 이동해야 하지 않을까요?

woowahan-pjs · 2023-09-07T02:37:08Z

src/main/kotlin/apply/infra/mail/FakeMailSender.kt

+
+class FakeMailSender : MailSender {
+    override fun send(toAddress: String, subject: String, body: String) {
+        logger.info("send mail: to - {}, subject - {}", toAddress, subject)


r: 코틀린 스타일로 작성해 볼까요?

logger.debug("hello {}", heavyWorld(13)) // 항상 연산 logger.debug { "hello ${heavyWorld(33)}" } // debug 레벨 로그 남길 때만 연산

woowahan-pjs · 2023-09-07T02:37:19Z

src/main/kotlin/apply/utils/MarkdownProcessor.kt

+object MarkdownProcessor {
+    private val flavour = CommonMarkFlavourDescriptor()
+
+    fun generateToHtml(source: String): String {


a: text 또는 markdownText와 같이 사용 중인 API에서 지정한 매개 변수 이름을 따르는 것은 어떨까요?

woowahan-pjs · 2023-09-07T02:37:23Z

src/main/resources/templates/mail/common.html

@@ -63,7 +63,7 @@
                        padding-right: 8px;
                      "
                    >
-                      <span th:utext="${content}"></span>
+                      [(${content})]


a: 이 문법은 무슨 뜻인가요?

content가 html string이여서 text inlining 을 적용했습니다.

첨부하신 문서는 Thymeleaf 2.1 버전이라서 찾을 수 없네요! 3.x의 기능인 것 같은데, 스프링 부트는 오랫동안 3.x를 사용해 왔더라고요.
https://www.thymeleaf.org/doc/tutorials/3.1/usingthymeleaf.html#expression-inlining

woowahan-pjs · 2023-09-07T02:37:34Z

src/test/kotlin/apply/application/mail/MailServiceTest.kt

+import support.test.IntegrationTest
+
+@IntegrationTest
+@ContextConfiguration(classes = [TestMailConfig::class])


r: 자주 변경될 수 있는 애너테이션은 상단으로 옮기는 것이 좋습니다.

woowahan-pjs · 2023-09-07T02:37:37Z

src/test/kotlin/apply/application/mail/MailServiceTest.kt

+        val body = "안녕하세요. 우아한테크코스입니다.\n" + "\n" +
+            "1주 차 미션은 개발 환경을 세팅하고, GitHub에 과제를 제출하는 등 미션 외에도 추가로 익혀야 하는 부분들이 있어 가벼운 미션으로 준비했어요."


r: 수많은 이스케이프가 필요한 문자열의 경우 3중 따옴표 문자열을 사용하면 더 깔끔하게 표현할 수 있습니다.
https://kotlinlang.org/docs/strings.html#multiline-strings

woowahan-pjs · 2023-09-07T02:37:41Z

src/test/kotlin/apply/application/mail/MailServiceTest.kt

+            val commonMailBody = mailService.createCommonMailBodyFrom(mailData)
+
+            Then("본문이 포함된 HTML로 작성된다.") {
+                println(commonMailBody)


r: 콘솔로 출력하는 대신 테스트 코드를 더 보완하면 어떨까요?

kang-hyungu · 2023-09-08T00:49:51Z

src/main/kotlin/support/Markdown.kt

+private val flavour: MarkdownFlavourDescriptor = CommonMarkFlavourDescriptor()
+
+fun markdownToHtml(markdownText: String): String {
+    val parsedTree = MarkdownParser(flavour).buildMarkdownTreeFromString(markdownText)


c: MarkdownParser도 flavour처럼 객체 하나만 만들어둬도 괜찮을 것 같아요

private val markdownParser = MarkdownParser(flavour)

kang-hyungu · 2023-09-08T00:51:45Z

src/test/kotlin/apply/application/mail/MailServiceTest.kt

+            안녕하세요. 우아한테크코스입니다.
+
+            1주 차 미션은 개발 환경을 세팅하고, GitHub에 과제를 제출하는 등 미션 외에도 추가로 익혀야 하는 부분들이 있어 가벼운 미션으로 준비했어요.
+        """.trimIndent()


c: trimIndent() 처리를 MailData 에서 처리하는건 어떨까요?

trimIndent() 는 테스트 문자열을 멀티라인으로 작성하면서 Indentation이 생겨서 추가했습니다. 검사할 예상 값과 인덴트 차이가 발생해서요!
테스트가 아닌 상황에서는 발생하지 않아서(혹은 인덴트가 있어도 렌더링에는 문제가 없어서) 서비스 코드에 추가하지 않아도 될 것 같아요. ㅎㅎ

woowabrie · 2023-09-08T02:43:32Z

@woowahan-pjs @kang-hyungu
좋은 의견들 감사합니다. ㅎㅎ 코멘트 주신 사항들 적용했어요.
확인 부탁드려요~

woowahan-pjs

메일을 작성할 때 템플릿에 이미 <body>가 있으므로 마크다운을 HTML로 변환할 때 <body>를 추가하지 않는 함수를 하나 더 추가하였습니다.
나머지는 문제 없어요! 수고해 주셔서 감사합니다. 👍

woowahan-pjs · 2023-09-09T07:05:06Z

src/test/kotlin/apply/application/mail/MailServiceIntegrationTest.kt

+import org.springframework.context.annotation.Import
+import support.test.IntegrationTest
+
+@Import(TestMailConfiguration::class)


@ContextConfiguration과 @Import의 차이점은 무엇일까요?

woowabrie added 5 commits August 30, 2023 10:11

feat: add markdown dependency

a82b54d

feat: add MarkdownProcessor using markdown library

734ace7

feat: add function to get html body in MailData

b387be7

refactor: extract function to make mail body

6f37290

feat: use html body when sending common mail

6f0af2a

woowabrie self-assigned this Sep 4, 2023

woowabrie requested review from kang-hyungu, woowahan-pjs and woowahan-neo September 5, 2023 06:38

woowahan-pjs requested changes Sep 7, 2023

View reviewed changes

woowabrie and others added 7 commits September 7, 2023 14:53

refactor: move MarkdownProcessor into support package

6b54028

refactor: make markdownToHtml to top level function

2033dad

refactor: remove Markdown dependency from MailData DTO

68e06be

test: use multiline strings for test data and enhance test match

25a84ef

test: move FakeMailSender to test directory and edit logging level

60ed82a

test: adjust test annotation order

10449cc

Merge branch 'develop' into feature/markdown-to-html

c2f69ae

woowahan-pjs changed the base branch from master to develop September 8, 2023 00:33

kang-hyungu requested changes Sep 8, 2023

View reviewed changes

woowabrie added 2 commits September 8, 2023 11:17

refactor: modify function name using naming convention in project

3f1a85a

refactor: make MarkdownParser to top level property

7ccc79f

woowahan-pjs added 2 commits September 9, 2023 15:14

test(mail): polish test codes

6514b32

feat(mail): implement the function to convert markdown to embedded html

fdca318

woowahan-pjs approved these changes Sep 9, 2023

View reviewed changes

woowahan-pjs changed the title ~~feat(administrator): support markdown for common mail body~~ feat(mail): support the use of markdown in the email body Sep 9, 2023

kang-hyungu approved these changes Sep 13, 2023

View reviewed changes

woowahan-pjs merged commit eb83941 into develop Sep 13, 2023

woowahan-pjs deleted the feature/markdown-to-html branch September 13, 2023 07:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(mail): support the use of markdown in the email body #726

feat(mail): support the use of markdown in the email body #726

woowabrie commented Sep 4, 2023

woowabrie commented Sep 5, 2023

woowahan-pjs left a comment

woowahan-pjs Sep 7, 2023

woowabrie Sep 7, 2023

woowahan-pjs Sep 7, 2023

woowabrie Sep 7, 2023

woowahan-pjs Sep 7, 2023

woowabrie Sep 7, 2023

woowahan-pjs Sep 7, 2023

woowahan-pjs Sep 7, 2023

woowahan-pjs Sep 7, 2023

woowahan-pjs Sep 7, 2023

woowabrie Sep 7, 2023

woowahan-pjs Sep 9, 2023

woowahan-pjs Sep 7, 2023

woowahan-pjs Sep 7, 2023

woowahan-pjs Sep 7, 2023

kang-hyungu Sep 8, 2023

kang-hyungu Sep 8, 2023

woowabrie Sep 8, 2023

woowabrie commented Sep 8, 2023

woowahan-pjs left a comment

woowahan-pjs Sep 9, 2023

	fun createCommonMailBodyFrom(mailData: MailData): String {
	fun String.toMarkdownBody(): String {


		private val logger = KotlinLogging.logger { }

		class FakeMailSender : MailSender {

		val body = "안녕하세요. 우아한테크코스입니다.\n" + "\n" +
		"1주 차 미션은 개발 환경을 세팅하고, GitHub에 과제를 제출하는 등 미션 외에도 추가로 익혀야 하는 부분들이 있어 가벼운 미션으로 준비했어요."

feat(mail): support the use of markdown in the email body #726

feat(mail): support the use of markdown in the email body #726

Conversation

woowabrie commented Sep 4, 2023

해결하려는 문제가 무엇인가요?

어떻게 해결했나요?

어떤 부분에 집중하여 리뷰해야 할까요?

참고 자료

RCA 룰

woowabrie commented Sep 5, 2023

woowahan-pjs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woowabrie commented Sep 8, 2023

woowahan-pjs left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment