[Feature/#243] 디스코드 알림 연동 #247

mmihye · 2024-10-15T14:06:50Z

🚩 관련 이슈

close 디스코드 알림 연동하기 #243

📋 구현 기능 명세

회원가입시 알림
500에러 발생시 알림

📌 PR Point

무슨 이유로 어떻게 코드를 변경했는지
로컬일때는 알림 보내지않도록 설정해놨습니다.
어떤 부분에 리뷰어가 집중해야 하는지
이렇게하면 환경 프로필 사용해서 실행시키도록 변경해야하는데 설마 무슨일 발생할까 무섭다...
개발하면서 어떤 점이 궁금했는지
원래 프로필 나눠서 개발하다가 중간에 그냥 안하는걸로 했었던거같은데
프로필 나눌때 따로 또 설정해야할것이 있을까요??

📸 결과물 스크린샷

🛠️ 테스트

테스트

🚀 API Endpoint

sss4920

궁금한거 질문 남겼습니다..! 수고하셨어용

sss4920 · 2024-10-15T14:13:17Z

linkmind/src/main/java/com/app/toaster/external/client/discord/DiscordMessageProvider.java

+        }
+    }
+
+    private DiscordMessage createSingUpMessage() {


옹 좋은데 SignUpMessage()로 오타정도만 나중에 수정해주면 좋을거같네용☺️

sss4920 · 2024-10-15T14:14:37Z

linkmind/src/main/java/com/app/toaster/service/auth/AuthService.java

@@ -48,6 +52,7 @@ public class AuthService {
 	private final PopupManagerRepository popupManagerRepository;

 	private final SlackApi slackApi;


이렇게 되면 SlackApi도 같이 이 pr에서 제거해주실 수 있을까용?🐱

혹시 SlackApi 관련 클래스 모두 삭제하는것을 말하는걸까요...?

아 흠 클래스까지 다 지우기에 좀.. 그렇네용 아까운뎅 ㅜ 일단 보류하는거 어떻게 생각하시나용;?ㅋㅋ 줏대 없어서 죄송합니당😢

sss4920 · 2024-10-15T14:33:54Z

linkmind/src/main/java/com/app/toaster/service/auth/AuthService.java

@@ -76,7 +81,8 @@ public SignInResponseDto signIn(String socialAccessToken, SignInRequestDto reque
 				.socialType(socialType).build();
 			newUser.updateFcmIsAllowed(true); //신규 유저면 true박고
 			userRepository.save(newUser);
-			slackApi.sendSuccess(Success.LOGIN_SUCCESS);
+			// slackApi.sendSuccess(Success.LOGIN_SUCCESS);
+			discordMessageProvider.sendSignUpNotification();


흠 여기서 궁금한게
우리가 회원가입을 할 때 저거 publish 메시지를 하고? 그 publish message하는 것도 트랜잭션에 묶이고 있는데, 해당 publish message가 private 메서드만 호출하고 오류는 private 메서드 내에서 에러가 난다면 그건 회원가입 트랜잭션에 영향이 안가는건지 궁금합니다!

최근 트랜잭션 전파단계에 대해서 공부하긴했었는데 실제로 사례로보니 더 헷갈리는거같네용 ㅠ

엇 그러고 보니 알림에서 에러가 나면 모든로직이 롤백될거같네요...
같은 클래스내 private 메서드면 영향이 없는걸로 아는데 sendSignUpNotification public 메소드라서 영향이 있겠네요...

81f6717 다음과 같이 Try-Catch 문을 사용하여 에러 발생하면 로그찍고넘어가는 걸로 고쳐봤는데 이렇게하면 괜찮을까요? 혹시 좋은방법이나 아이디어있으시면 말씀해주세용

흠.. 일단 로그찍는것도 임시방편으로 나쁘지않은 것 같습니다!!

약간 아이디어는 트랜잭션을 분리시키는 방법이 있을 것 같긴합니다..!
@transactional에서 전파 옵션에서 required_new라는 애가 있는데 사용하면 외부 트랜잭션과 별도의 새로운 트랜잭션을 시작해서 처리하는 것으로 알고있습니당!
대신에 앞에 단계가 처리되고 난 후에 동작해야돼서 after_commit이라는 옵션이 더 필요할 것 같아요!

[관련링크]
https://velog.io/@guswns3371/TransactionalEventListener-%EC%82%AC%EC%9A%A9%EC%8B%9C-%EC%A3%BC%EC%9D%98%ED%95%A0-%EA%B2%83

일단 우리단과 비슷한 경우인 듯합니당.
먼저 우리가 save를 통해서 회원가입을 진행하고?
이 트랜잭션 커밋이후에 작동하도록 after_commit을 진행하는데 이 때, after_commit은 이전 트랜잭션 이후에 추가적인 상황에 해당하므로 required_new라는 애를 통해 새로운 트랜잭션을 파서 처리하면 좋을 것 같습니당! 어떻게 생각하시나욤?!☺️

옹 새로운 방식이네요 한번 공부해보고 적용해보겠습니다!

sss4920

보니까 Enum에도 오타가 있네용;ㅋㅋ(찾아서 ㅈㅅ)

트랜잭션에 대해서 코멘트 달아봤습니다!
만약에 위와 같이 별도의 트랜잭션으로 작동하게 된다면
-> 프로필에 따라서 로컬에서는 작동 안되게!
이 로직 또한 aftercommit 달린쪽에서 Profile을 확인시켜준다면 별도의 트랜잭션이라 회원가입 로직과 별개로 잘 동작할 것 같은데 어떻게 생각하시나용?🐱

sss4920 · 2024-10-16T16:48:52Z

linkmind/src/main/java/com/app/toaster/external/client/discord/DiscordMessageProvider.java

@@ -33,7 +33,7 @@ public void sendNotification(NotificationType type, Exception e, String request)
            try {
                switch (type){
                    case ERROR -> discordSingUpClient.sendMessage(createErrorMessage(e,request));
-                    case SINGUP -> discordSingUpClient.sendMessage(createSingUpMessage());
+                    case SINGUP -> discordSingUpClient.sendMessage(createSignUpMessage());


근데 이걸 봐버렸는데 SIGNUP 킁..ㅋㅋㅋ 못본척하까요

킁... 수정하겠습니다

mmihye · 2024-10-17T14:12:59Z

트랜잭션에 대해서 코멘트 달아봤습니다! 만약에 위와 같이 별도의 트랜잭션으로 작동하게 된다면 -> 프로필에 따라서 로컬에서는 작동 안되게! 이 로직 또한 aftercommit 달린쪽에서 Profile을 확인시켜준다면 별도의 트랜잭션이라 회원가입 로직과 별개로 잘 동작할 것 같은데 어떻게 생각하시나용?🐱

여기서 고민이 되는게 현재 프로필검증을 DiscordMessageProvider에서 하고있는데 aftercommit 달린쪽은 회원가입로직만 있어서 거기서 검증을 하는게 나을지? DiscordMessageProvider 여기서 에러 알림이랑 회원가입알림 한번에 검증할수있도록 하는게 나을지 고민이네용...

sss4920 · 2024-10-18T10:40:10Z

여기서 고민이 되는게 현재 프로필검증을 DiscordMessageProvider에서 하고있는데 aftercommit 달린쪽은 회원가입로직만 있어서 거기서 검증을 하는게 나을지? DiscordMessageProvider 여기서 에러 알림이랑 회원가입알림 한번에 검증할수있도록 하는게 나을지 고민이네용...

제가 질문을 제대로 이해한지는 모르겠지만..! aftercommit, require_new옵션을 디스코드 알림쪽에 작성한거 맞을까용? 알림을 쏘는 로직들은 보통 어떤 로직들이 이미 존재하고, 그 로직들이 종료된 이후에 공통적으로 작동하는 것들이 많아서 DiscordMessageProvider에서 모두 관리하는건 어떻게 생각하세용?

(근데 제가 질문 이해를 잘 못한걸까용? 아니라면 부연설명 조금 부탁드립니다..ㅎ😢)

mmihye · 2024-10-22T13:17:55Z

@sss4920
엇 제가 잘못이해했었네요... 저건 무시해주세요 ㅜㅜㅎㅎ;;

sss4920

수고하셨습니다..! 피드백 반영 속도도 좋은거같아요! 로컬 테스트 끝났으면 머지해도 될 것 같습니다 ㅎㅎ

sss4920 · 2024-10-24T11:49:46Z

linkmind/src/main/java/com/app/toaster/external/client/discord/DiscordMessageProvider.java

    @TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
-    public void sendNotification(NotificationType type, Exception e, String request) {


생각해보니 외부 API니까 쿼리가 날라갈 일이 없으니 따로 Required_new 옵션까진 필요가 없겠네요..!

네네! 그래서 required_new 옵션제외했습니당

mmihye added 3 commits October 10, 2024 22:10

[#243] feat: 디스코드 회원가입 알림

822547f

[#243] feat: 디스코드 회원가입 + 에러 알림

fce6727

[#243] feat: 로컬에서는 에러메시지 전송되지 않도록 수정

83e5387

mmihye requested a review from sss4920 October 15, 2024 14:06

mmihye self-assigned this Oct 15, 2024

sss4920 reviewed Oct 15, 2024

View reviewed changes

mmihye added 2 commits October 16, 2024 23:12

[#243] fix: 오타수정

f947bdf

[#243] fix: 회원가입시 디스코드 알림 에러 예외처리

81f6717

sss4920 reviewed Oct 16, 2024

View reviewed changes

[#243] fix: 오타수정

a0f568f

mmihye and others added 5 commits October 22, 2024 22:18

[#243] feat: TransactionalEventListener 추가

ad8cbd5

Merge branch 'develop' into feature/#243

fcc6b29

[#243] feat: TransactionalEventListener 적용 및 DiscordClient 통합

d8bb06f

Merge remote-tracking branch 'origin/feature/#243' into feature/#243

6cc7f8b

[#243] fix: 필요없는 import 삭제

898fa9d

sss4920 approved these changes Oct 24, 2024

View reviewed changes

mmihye merged commit 23837ad into develop Oct 24, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature/#243] 디스코드 알림 연동 #247

[Feature/#243] 디스코드 알림 연동 #247

mmihye commented Oct 15, 2024

sss4920 left a comment

sss4920 Oct 15, 2024

sss4920 Oct 15, 2024

mmihye Oct 16, 2024

sss4920 Oct 16, 2024

sss4920 Oct 15, 2024

mmihye Oct 16, 2024 •

edited

Loading

sss4920 Oct 16, 2024 •

edited

Loading

mmihye Oct 17, 2024

sss4920 left a comment

sss4920 Oct 16, 2024

mmihye Oct 17, 2024

mmihye commented Oct 17, 2024

sss4920 commented Oct 18, 2024

mmihye commented Oct 22, 2024

sss4920 left a comment

sss4920 Oct 24, 2024

mmihye Oct 24, 2024

		@@ -48,6 +52,7 @@ public class AuthService {
		private final PopupManagerRepository popupManagerRepository;

		private final SlackApi slackApi;

		@TransactionalEventListener(phase = TransactionPhase.AFTER_COMMIT)
		public void sendNotification(NotificationType type, Exception e, String request) {

[Feature/#243] 디스코드 알림 연동 #247

[Feature/#243] 디스코드 알림 연동 #247

Conversation

mmihye commented Oct 15, 2024

🚩 관련 이슈

📋 구현 기능 명세

📌 PR Point

📸 결과물 스크린샷

🛠️ 테스트

🚀 API Endpoint

sss4920 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

mmihye Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

sss4920 Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sss4920 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmihye commented Oct 17, 2024

sss4920 commented Oct 18, 2024

mmihye commented Oct 22, 2024

sss4920 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmihye Oct 16, 2024 •

edited

Loading

sss4920 Oct 16, 2024 •

edited

Loading