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-be: 이메일 템플릿 기능 추가 #988

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

github-actions[bot]
Copy link
Contributor

Original issue description

목적

이메일 템플릿 기능 추가

작업 세부사항

  • 특정 단어를 다른 단어로 변환합니다.

참고 사항

아래의 별표줄 밑에 요구사항 ID만 작성해주세요. Prefix 금지!


EMAIL_TEMPLATE_01

closes #987

@github-actions github-actions bot added backend 백엔드 feature 새로운 기능 labels Jan 13, 2025
@github-actions github-actions bot added this to the 스프린트 7.0 milestone Jan 13, 2025
@HyungHoKim00 HyungHoKim00 marked this pull request as ready for review January 13, 2025 09:03
Copy link
Contributor Author

1736759007.661069

Copy link
Member

@cutehumanS2 cutehumanS2 left a comment

Choose a reason for hiding this comment

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

👍

Comment on lines 69 to 76
private String applyTemplate(String content, Club from, Applicant to) {
content = EmailKeyword.APPLICANT_NAME.replace(content, to.getName());
content = EmailKeyword.CLUB_NAME.replace(content, from.getName());
content = EmailKeyword.APPLY_FORM_TITLE.replace(content,
applyFormService.findByDashboard(to.getDashboard()).getTitle());
content = EmailKeyword.PROCESS_NAME.replace(content, to.getProcess().getName());
return content;
}
Copy link
Member

Choose a reason for hiding this comment

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

[가벼운 제안]

  • 계속 String이 새로 생성되는 걸로 판단되는데, StringBuilder/StringBuffer를 쓰는 게 성능이 좀 더 좋지 않을까요? ? 지금은 치환값이 4개이고 변동될 여지도 딱히 없을 거 같아서 반드시 반영하실 필요는 없습니다~ ㅎㅎ
  • 뭔가 치환할 값들을 Map으로 묶어 인자로 전달하고, 루프로 한번에 처리할 수 있을 것 같기도 합니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

적용해 보았습니다! 👍

Copy link
Contributor

@Dobby-Kim Dobby-Kim left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~!

역시 믿고 맡기는 띵오 코드가 참 깔끔하군요

트랜잭션 관련하여 해당 부분만 확인해주세용

Comment on lines +4 to +7
APPLICANT_NAME("{AP_NAME}"),
CLUB_NAME("{CL_NAME}"),
APPLY_FORM_TITLE("{RC_TITLE}"),
PROCESS_NAME("{RC_STEP}"),
Copy link
Contributor

Choose a reason for hiding this comment

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

요부분은 프론트와 협의해서 정해진 정규식인가요?
맞다면 각각의 키워드가 어떤 의미를 가지는지 설명해주실 수 있을까요오

Copy link
Contributor

Choose a reason for hiding this comment

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

맞습니다! 회의때만 말하고 공유사항에 적질 않았네요 ㅎㅎ.. 각각 아래의 줄임말입니다.
지원자 이름 -> APPLICANT_NAME
동아리 이름 -> CLUB_NAME
공고 이름 -> RECRUITMENT_POST_TITLE
모집 단계 -> RECRUITMENT_STEP

import org.springframework.web.multipart.MultipartFile;

@Service
@Transactional(readOnly = true)
Copy link
Contributor

Choose a reason for hiding this comment

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

외부 연동구간이 많은 EmailFacade에서는 의도하여 @Transactional을 사용하지 않은 것으로 알고 있는데 혹시 제가 잘못 알고 있는 걸까요?
이 부분 확인해주시면 감사하겠습니다: >

Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분은 제가 실수했네요 ㅜㅜ. 템플릿이 들어가는 시점에서 데이터의 일관성을 보장하기 위해 추가했었어요. 수정했습니다!

Copy link
Contributor

@Dobby-Kim Dobby-Kim left a comment

Choose a reason for hiding this comment

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

고생하셨습니다 :>

@HyungHoKim00 HyungHoKim00 merged commit e4b7740 into be/develop Jan 15, 2025
10 checks passed
@HyungHoKim00 HyungHoKim00 deleted the 987-be-EMAIL_TEMPLATE_01 branch January 15, 2025 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend 백엔드 feature 새로운 기능
Projects
Status: 완료
Development

Successfully merging this pull request may close these issues.

3 participants