-
Notifications
You must be signed in to change notification settings - Fork 2
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
[OING-336] feat: 푸쉬 알림 메세지 변경 및 미션 업로드 시 노티 발송 #258
Conversation
Test Results 51 files 51 suites 12s ⏱️ Results for commit 1fb81ef. ♻️ This comment has been updated with latest results. |
Code Coverage
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다! isNewPostMadeMissionUnlocked
의 엣지케이스에 대해 생각해봤는데 잘 처리한 거 같습니다.
코드 리팩터링 관련해서 코멘트 남겼는데 확인해주세요~
.setAndroidConfig(FCMNotificationUtil.buildAndroidConfig()) | ||
.build(); | ||
fcmNotificationService.sendMulticastMessage(missionUnlockedMessage); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
onPostCreatedEvent
에서 사용자가 글을 작성했을 때, 가족이 미션 키를 획득했을 때 두 가지 경우에 대해 모두 처리하고 있는데, 이 두 가지 경우를 각 메서드로 분리해서 책임을 명확히 하면 더 좋을 거 같아요!
Quality Gate passedIssues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 수고하셨습니다~
* feat: update pick notification * feat: send notification if mission unlocked * feat: send survival noti only on survival post * refactor: move mission message to another method
❓ 기능 추가 배경
�- 푸쉬 알림 컨텐츠 변경 및 경로 추가
➕ 추가/변경된 기능
🥺 리뷰어에게 하고싶은 말
백엔드 PR 엄청 오랜만이네요 ㄷㄷ
따로 일별로 미션 푸쉬 발송 여부를 디비에 쌓는것도 이상해보여서 로직과 같이 바로 "이전 게시물은 조건 미만족 && 새 게시물 포함하면 미션 조건 만족" 일때 푸쉬 발송하게 해놨는데.. 이론상은 맞는데 혹시 엣지케이스 떠오르는거 있으면 코멘트 부탁드립니다.
🔗 참조 or 관련된 이슈
https://no5ing.atlassian.net/browse/OING-336