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

[BE] 피드백 완료 알람 기능 구현(#771) #772

Merged
merged 12 commits into from
Nov 21, 2024
Merged

[BE] 피드백 완료 알람 기능 구현(#771) #772

merged 12 commits into from
Nov 21, 2024

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 17, 2024

📌 관련 이슈

✨ PR 세부 내용

이전 매칭 완료 알람 기능이 머지되지 않아서 커밋 기록이 함께 있습니다.
849f73d 여기부터 보시면 될 것 같아요.

피드백 작성이 완료되면 상대에게 피드백을 받았다는 알람을 생성해줍니다.
타입은 FEEDBACK_CREATED로 두었어요.

피드백 수정 알람도 추가해야 할지는 조금 고민해봐야 할 것 같네용.
고민했던 부분은 아래 코멘트로 남겨두겠습니다. 답변해주시면 감사하겠읍니다. 👍

@github-actions github-actions bot added BE 백엔드 개발 관련 작업 기능 기능 구현 작업 labels Nov 17, 2024
Copy link
Contributor Author

github-actions bot commented Nov 17, 2024

Test Results

 65 files   65 suites   8s ⏱️
209 tests 191 ✅ 18 💤 0 ❌
224 runs  206 ✅ 18 💤 0 ❌

Results for commit d8c302d.

♻️ This comment has been updated with latest results.

Comment on lines +49 to +73
@Transactional
public void createUrgeAlarm(long revieweeId, long reviewerId, long roomId) {
boolean unReadUrgeAlarmExist = userToUserAlarmReader.existUnReadUrgeAlarm(revieweeId, reviewerId, roomId);
if (unReadUrgeAlarmExist) {
log.warn("리뷰 재촉 알림 생성을 실패했습니다. 리뷰어 ID={},리뷰이 ID={},방 ID={}",
reviewerId, revieweeId, roomId);
throw new CoreaException(ExceptionType.SAME_UNREAD_ALARM_EXIST);
}
CreateUserToUserAlarmInput input = new CreateUserToUserAlarmInput(AlarmActionType.REVIEW_URGE, revieweeId, reviewerId, roomId);
userToUserAlarmWriter.create(input.toEntity());
}

@Transactional
public void createFeedbackAlarm(long deliverId, long receiverId, long roomId) {
try {
CreateUserToUserAlarmInput input = new CreateUserToUserAlarmInput(AlarmActionType.FEEDBACK_CREATED, deliverId, receiverId, roomId);
userToUserAlarmWriter.create(input.toEntity());
} catch (Exception e) {
log.warn("피드백 작성 알림 생성을 실패했습니다. 작성자 ID={},수신자 ID={},방 ID={}",
deliverId, receiverId, roomId);
}
}

@Transactional
public void createMatchingCompletedAlarm(long roomId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이 로직들은 근본적으로 알람 타입만 다르고 똑같은 형태의 메소드들인데요.

타입을 파라미터로 받아 타입만 다른 알람들을 생성해주는 공통 메소드를 만들까 싶어용.
근데 그 타입을 파라미터로 받기 < 이 부분을 외부 서비스에서 처리할지, 아니면 지금처럼 알람 서비스 내부에서 처리할지 고민이 되네요.

예시를 들자면

  1. 외부 서비스에서 타입을 파라미터로 받기
createAlarm(알람 타입, actor Id, 수신자 ID, 방 아이디)

이런 느낌의 메서드를 만들고 FeedbackServiceReviewService에서

createAlarm(FEEDBACK_CREATED ...)
createAlarm(REVIEW_COMPELTE...)

이렇게 사용하는 거고용

  1. 알람 서비스 내부에서 처리

이 경우 지금과 동일하게 외부 서비스에서는 createReviewAlarm을 호출하고,
알람 서비스 내부에서

createReviewAlarm() {
    createAlaram(REVIEW_COMPLETE ....)
}

이런 식으로 가는 겁니다용.
(혹시 더 좋은 아이디어가 있다면 공유 부탁 드려용.)

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.

둘 다 괜찮은 것 같아요!
근데 AlarmType 이나 AlarmActionType 을 사용하는 서비스에서 알 필요는 없다는 생각이 들어 클래스 내부에서 공통 메서드를 분리하는게 좀 더 나을 것 같아요

assertThatCode(() -> developFeedbackService.create(room.getId(), deliver.getId(), createRequest(receiver.getId())))
.doesNotThrowAnyException();
assertThat(matchResult.isReviewerCompletedFeedback()).isTrue();
}

@Transactional
//@Transactional
Copy link
Contributor

Choose a reason for hiding this comment

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

DevelopFeedbackServiceTest, SocialFeedbackServiceTest에서
@Transactional이 붙어있는 테스트 중 현재 주석 처리 해놓은 테스트들은 해당 어노테이션을 지우더라도 돌아가더라고요.

혹시 이 어노테이션을 붙여놓게 된 이유가 있을까요? ㅇ.ㅇ?
첫 작성자는 뽀로로 같던데

Copy link
Contributor

@youngsu5582 youngsu5582 left a comment

Choose a reason for hiding this comment

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

애쉬 코드 짠다고 수고했구용

해당 부분은 초코랑 처음 알림 초안 짤때도 얘기했던 부분인데

현재는 방에 들어가도 자신이 받은 피드백을 볼 수 없습니다.
이때 방으로 향하게 하는게 맞는가? 라는 생각은 들었는데

이는 당장 해결보단 나중에 피드백 개별로 보게 한다던가 향후 변경에 따라 반영이 될 거 같긴 하네요.

+ 추가로, 개발 / 소설 피드백인지는 중요하지 않나용?


둘다 현재 로직에 연관 없으므로 Approve 할게요~

@ashsty
Copy link
Contributor

ashsty commented Nov 18, 2024

현재는 방에 들어가도 자신이 받은 피드백을 볼 수 없습니다. 이때 방으로 향하게 하는게 맞는가? 라는 생각은 들었는데 이는 당장 해결보단 나중에 피드백 개별로 보게 한다던가 향후 변경에 따라 반영이 될 거 같긴 하네요.

이건 정책 변경이 필요할 지 한 번 봐야할 것 같아용.

지금 당장으로는 피드백 알람 클릭하면
피드백 모아보기 탭으로 이동 -> 해당하는 방 열림 -> 해당 방의 첫 번째 피드백 디스플레이

이런 프로세스가 될 것 같네요!

추가로, 개발 / 소설 피드백인지는 중요하지 않나용?

당장은 필요한 값이 아니라고 하네용 👍 차후 수정 필요하면 수정 가능!

Copy link
Contributor

@hjk0761 hjk0761 left a comment

Choose a reason for hiding this comment

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

새로운 알람도 잘 구현해주셨네요 ㅎㅎ
의견에 답변 달아두었고, 이전 PR 에서의 코멘트와 동일해 여기도 마찬가지로 approve 합니다
고생했어요~

Comment on lines +49 to +73
@Transactional
public void createUrgeAlarm(long revieweeId, long reviewerId, long roomId) {
boolean unReadUrgeAlarmExist = userToUserAlarmReader.existUnReadUrgeAlarm(revieweeId, reviewerId, roomId);
if (unReadUrgeAlarmExist) {
log.warn("리뷰 재촉 알림 생성을 실패했습니다. 리뷰어 ID={},리뷰이 ID={},방 ID={}",
reviewerId, revieweeId, roomId);
throw new CoreaException(ExceptionType.SAME_UNREAD_ALARM_EXIST);
}
CreateUserToUserAlarmInput input = new CreateUserToUserAlarmInput(AlarmActionType.REVIEW_URGE, revieweeId, reviewerId, roomId);
userToUserAlarmWriter.create(input.toEntity());
}

@Transactional
public void createFeedbackAlarm(long deliverId, long receiverId, long roomId) {
try {
CreateUserToUserAlarmInput input = new CreateUserToUserAlarmInput(AlarmActionType.FEEDBACK_CREATED, deliverId, receiverId, roomId);
userToUserAlarmWriter.create(input.toEntity());
} catch (Exception e) {
log.warn("피드백 작성 알림 생성을 실패했습니다. 작성자 ID={},수신자 ID={},방 ID={}",
deliverId, receiverId, roomId);
}
}

@Transactional
public void createMatchingCompletedAlarm(long roomId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

둘 다 괜찮은 것 같아요!
근데 AlarmType 이나 AlarmActionType 을 사용하는 서비스에서 알 필요는 없다는 생각이 들어 클래스 내부에서 공통 메서드를 분리하는게 좀 더 나을 것 같아요

Copy link
Contributor

@jcoding-play jcoding-play left a comment

Choose a reason for hiding this comment

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

수고하셨습니다! 👍
선 어프루브 후, 후 조치 갑니당~

@ashsty ashsty merged commit 903a722 into develop Nov 21, 2024
4 checks passed
@ashsty ashsty deleted the feat/#771 branch November 21, 2024 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE 백엔드 개발 관련 작업 기능 기능 구현 작업
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BE] 피드백 완료 알람 기능 구현
4 participants