Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(mail): change to send mail to members only #753

Merged
merged 10 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/main/kotlin/apply/application/EvaluationDtos.kt
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,11 @@ data class EvaluationTargetData(

data class MailTargetResponse(
val email: String,
val name: String? = null
val name: String? = null,
val id: Long,
) {
constructor(memberResponse: MemberResponse) : this(memberResponse.email, memberResponse.name)
constructor(member: Member) : this(member.email, member.name)
constructor(memberResponse: MemberResponse) : this(memberResponse.email, memberResponse.name, memberResponse.id)
constructor(member: Member) : this(member.email, member.name, member.id)
}

data class EvaluationItemScoreData(
Expand Down
10 changes: 4 additions & 6 deletions src/main/kotlin/apply/application/MailTargetService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import apply.domain.evaluationtarget.EvaluationStatus
import apply.domain.evaluationtarget.EvaluationTarget
import apply.domain.evaluationtarget.EvaluationTargetRepository
import apply.domain.member.MemberRepository
import apply.domain.member.findAllByEmailIn
import org.springframework.stereotype.Service
import org.springframework.transaction.annotation.Transactional

Expand All @@ -17,13 +16,12 @@ class MailTargetService(
fun findMailTargets(evaluationId: Long, evaluationStatus: EvaluationStatus? = null): List<MailTargetResponse> {
val memberIds = findEvaluationTargets(evaluationId, evaluationStatus).map { it.memberId }
return memberRepository.findAllById(memberIds)
.map { MailTargetResponse(it.email, it.name) }
.map { MailTargetResponse(it) }
}

fun findAllByEmails(emails: List<String>): List<MailTargetResponse> {
val members = memberRepository.findAllByEmailIn(emails)
val anonymousEmails = emails - members.map { it.email }
return members.map { MailTargetResponse(it) } + anonymousEmails.map { MailTargetResponse(it) }
fun findAllByMemberIds(memberIds: List<Long>): List<MailTargetResponse> {
val members = memberRepository.findAllById(memberIds)
return members.map { MailTargetResponse(it) }
}

private fun findEvaluationTargets(evaluationId: Long, evaluationStatus: EvaluationStatus?): List<EvaluationTarget> {
Expand Down
2 changes: 1 addition & 1 deletion src/main/kotlin/apply/application/mail/MailData.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ data class MailData(
var sender: String = "",

@field:NotEmpty
var recipients: List<String> = emptyList(),
var recipients: List<Long> = emptyList(),

@field:NotNull
var sentTime: LocalDateTime = LocalDateTime.now(),
Expand Down
2 changes: 1 addition & 1 deletion src/main/kotlin/apply/application/mail/MailService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class MailService(
@Async
fun sendMailsByBcc(request: MailData, files: Map<String, ByteArrayResource>) {
val body = generateMailBody(request)
val recipients = request.recipients + mailProperties.username
val recipients = memberRepository.findAllById(request.recipients).map { it.email } + mailProperties.username
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: IN 절과 관련하여 재밌는 내용이 있네요.

얼마전에 Hibernate 를 사용하는 시스템에서 메모리 부족으로 Full GC 연속 발생 -> API 응답이 극단적으로 저하되는 현상이 발생한 적이 있습니다. Memory Leak 은 아니지만, 그래도 메모리를 매우 많이 사용하고, 그게 아니더라도 성능샹 이득이 있으므로, 아래 Hibernate Property를 무조건 켜주는게 좋지 않을까 생각이 됩니다. Hibernate 5.2.18 부터 지원합니다.

대신에 sql에 다음과 같이 들어갈 수 있는데, where in (1,2,3,1,1,1) DBA 분들께도 이런 옵션 때문에 중복되어서 뒤에 같은 ID가 나타난다는 것을 사전에 공유하면 좋겠습니다.


// TODO: 성공과 실패를 분리하여 히스토리 관리
val succeeded = mutableListOf<String>()
Expand Down
2 changes: 1 addition & 1 deletion src/main/kotlin/apply/config/DatabaseInitializer.kt
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ class DatabaseInitializer(
subject = "[우아한테크코스] 프리코스를 진행하는 목적과 사전 준비",
body = "안녕하세요.",
sender = "[email protected]",
recipients = listOf("[email protected]", "[email protected]", "[email protected]", "[email protected]"),
recipients = listOf(1L, 2L, 3L, 4L),
sentTime = createLocalDateTime(2020, 11, 5, 10)
)
)
Expand Down
6 changes: 3 additions & 3 deletions src/main/kotlin/apply/domain/mail/MailHistory.kt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package apply.domain.mail

import support.domain.BaseEntity
import support.domain.StringToListConverter
import support.domain.StringToLongListConverter
import java.time.LocalDateTime
import javax.persistence.Column
import javax.persistence.Convert
Expand All @@ -21,9 +21,9 @@ class MailHistory(
val sender: String,

@Column(nullable = false)
@Convert(converter = StringToListConverter::class)
@Convert(converter = StringToLongListConverter::class)
@Lob
val recipients: List<String>,
val recipients: List<Long>,

@Column(nullable = false)
val sentTime: LocalDateTime = LocalDateTime.now(),
Expand Down
16 changes: 4 additions & 12 deletions src/main/kotlin/apply/ui/admin/mail/MailForm.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import org.springframework.core.io.ByteArrayResource
import support.views.BindingFormLayout
import support.views.NO_NAME
import support.views.addSortableColumn
import support.views.createEnterBox
import support.views.createErrorSmallButton
import support.views.createNormalButton
import support.views.createUpload
Expand All @@ -43,6 +42,7 @@ class MailForm(
private val mailTargetsGrid: Grid<MailTargetResponse> = createMailTargetsGrid(mailTargets)
private val recipientFilter: Component = createRecipientFilter()
private val fileUpload: Upload = createFileUpload()
private var mailData: MailData? = null

init {
add(subject, sender, recipientFilter, mailTargetsGrid, body, fileUpload)
Expand All @@ -60,20 +60,11 @@ class MailForm(

private fun createRecipientFilter(): Component {
return HorizontalLayout(
createTargetEnterBox(),
createIndividualLoadButton(),
createGroupLoadButton()
).apply { defaultVerticalComponentAlignment = FlexComponent.Alignment.END }
}

private fun createTargetEnterBox(): Component {
return createEnterBox("받는사람") {
if (it.isNotBlank()) {
refreshGrid { mailTargets.add(MailTargetResponse(it, NO_NAME)) }
}
}
}

private fun createIndividualLoadButton(): Button {
return createNormalButton("개별 불러오기") {
IndividualMailTargetDialog(memberService) {
Expand Down Expand Up @@ -133,15 +124,16 @@ class MailForm(
return null
}
return bindDefaultOrNull()?.apply {
recipients = mailTargets.map { it.email }.toList()
recipients = mailTargets.map { it.id }.toList()
attachments = uploadFile
}
}

override fun fill(data: MailData) {
mailData = data
fillDefault(data)
toReadOnlyMode()
refreshGrid { mailTargets.addAll(mailTargetService.findAllByEmails(data.recipients)) }
refreshGrid { mailTargets.addAll(mailTargetService.findAllByMemberIds(data.recipients)) }
}

private fun toReadOnlyMode() {
Expand Down
18 changes: 12 additions & 6 deletions src/main/kotlin/support/domain/StringToListConverter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,24 @@ package support.domain

import javax.persistence.AttributeConverter
import javax.persistence.Converter
import kotlin.reflect.KClass

@Converter
class StringToListConverter : AttributeConverter<List<String>, String> {
override fun convertToDatabaseColumn(recipients: List<String>): String {
return recipients.joinToString(COMMA)
abstract class StringToListConverter<T : Any>(
private val type: KClass<T>,
private val transform: (String) -> T,
) : AttributeConverter<List<T>, String> {
override fun convertToDatabaseColumn(attribute: List<T>): String {
return attribute.joinToString(COMMA)
}

override fun convertToEntityAttribute(dbData: String): List<String> {
return dbData.split(COMMA)
override fun convertToEntityAttribute(dbData: String): List<T> {
return dbData.split(COMMA).map { transform(it) }
}

companion object {
private const val COMMA: String = ","
}
}

@Converter
class StringToLongListConverter : StringToListConverter<Long>(Long::class, String::toLong)
6 changes: 3 additions & 3 deletions src/test/kotlin/apply/MailHistoryFixtures.kt.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ import java.time.LocalDateTime
private const val SUBJECT: String = "메일제목"
private const val BODY: String = "메일 본문 입니다."
private const val SENDER: String = "[email protected]"
private val RECIPIENTS: List<String> = listOf("[email protected]", "[email protected]")
private val RECIPIENTS: List<Long> = listOf(1L, 2L)
private val SENT_TIME: LocalDateTime = LocalDateTime.now()

fun createMailHistory(
subject: String = SUBJECT,
body: String = BODY,
sender: String = SENDER,
recipients: List<String> = RECIPIENTS,
recipients: List<Long> = RECIPIENTS,
sentTime: LocalDateTime = SENT_TIME,
id: Long = 0L
): MailHistory {
Expand All @@ -25,7 +25,7 @@ fun createMailData(
subject: String = SUBJECT,
body: String = BODY,
sender: String = SENDER,
recipients: List<String> = RECIPIENTS,
recipients: List<Long> = RECIPIENTS,
sentTime: LocalDateTime = SENT_TIME,
id: Long = 0L
): MailData {
Expand Down
26 changes: 15 additions & 11 deletions src/test/kotlin/apply/application/MailTargetServiceTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import apply.domain.evaluationtarget.EvaluationStatus.PENDING
import apply.domain.evaluationtarget.EvaluationStatus.WAITING
import apply.domain.evaluationtarget.EvaluationTargetRepository
import apply.domain.member.MemberRepository
import apply.domain.member.findAllByEmailIn
import io.kotest.core.spec.style.BehaviorSpec
import io.kotest.matchers.collections.shouldBeEmpty
import io.kotest.matchers.collections.shouldContainExactlyInAnyOrder
Expand Down Expand Up @@ -69,7 +68,7 @@ class MailTargetServiceTest : BehaviorSpec({

Then("평가 대상자의 이름 및 이메일을 확인할 수 있다") {
actual shouldHaveSize 1
actual[0] shouldBe MailTargetResponse(member.email, member.name)
actual[0] shouldBe MailTargetResponse(member)
}
}
}
Expand All @@ -88,7 +87,7 @@ class MailTargetServiceTest : BehaviorSpec({

Then("평가 대상자의 이름 및 이메일을 확인할 수 있다") {
actual shouldHaveSize 1
actual[0] shouldBe MailTargetResponse(member.email, member.name)
actual[0] shouldBe MailTargetResponse(member)
}
}
}
Expand All @@ -107,7 +106,7 @@ class MailTargetServiceTest : BehaviorSpec({

Then("평가 대상자의 이름 및 이메일을 확인할 수 있다") {
actual shouldHaveSize 1
actual[0] shouldBe MailTargetResponse(member.email, member.name)
actual[0] shouldBe MailTargetResponse(member)
}
}
}
Expand Down Expand Up @@ -141,16 +140,21 @@ class MailTargetServiceTest : BehaviorSpec({
}
}

Given("특정 이메일을 가진 회원이 없는 경우") {
val email = "[email protected]"
// TODO[#754]: 탈퇴한 회원의 경우 default 정보가 노출된다.
Given("메일 이력을 통해 회원 id 목록을 확인할 수 있는 경우") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a: 탈퇴한 회원이 구분되지 않아 테스트 코드 수정이 필요해 보이네요. 병합 후 #754 에서 수정하면 될 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이후 작업을 놓치지 않게 하기위해 아래 주석을 추가해뒀습니다.

// TODO[#754]: 탈퇴한 회원의 경우 default 정보가 노출된다.

val members = listOf(
createMember(id = 1L),
createMember(id = 2L),
createMember(id = 3L)
)

every { memberRepository.findAllByEmailIn(any()) } returns emptyList()
every { memberRepository.findAllById(any()) } returns members

When("해당 이메일로 이메일 정보를 조회하면") {
val actual = mailTargetService.findAllByEmails(listOf(email))
When("회원 id를 사용해 메일 수신자 정보를 조회하면") {
val actual = mailTargetService.findAllByMemberIds(listOf(1L, 2L, 3L, 4L))

Then("이름이 비어있는 것을 확인할 수 있다") {
actual[0] shouldBe MailTargetResponse(email, null)
Then("현재 회원인 메일 수신자만 확인할 수 있다") {
actual shouldHaveSize 3
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class EvaluationTargetRestControllerTest : RestControllerTest() {
@EnumSource(names = ["PASS", "FAIL", "WAITING"])
@ParameterizedTest
fun `메일 발송 대상(합격자)들의 이메일 정보를 조회한다`(enumStatus: EvaluationStatus) {
val responses = listOf(MailTargetResponse("[email protected]", "김로키"))
val responses = listOf(MailTargetResponse("[email protected]", "김로키", 1L))
every { mailTargetService.findMailTargets(any(), any()) } returns responses

mockMvc.get("/api/recruitments/{recruitmentId}/evaluations/{evaluationId}/targets/emails", 1L, 1L) {
Expand Down