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

[OING-379] feat: 알림 목록 조회 기능 추가 #280

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from

Conversation

Kwon770
Copy link
Collaborator

@Kwon770 Kwon770 commented Dec 27, 2024

❓ 기능 추가 배경


�알림 이력 화면에서 최근 한 달간 발생한 알림을 조회하기 위해, 관련 서버 기능을 추가했습니다.

➕ 추가/변경된 기능


  • 유저 알림 이력 테이블 추가
  • 유저 알림 데이터 구조 상 시스템에서 발생한 알림에 대한 통일성을 위해 시스템 계정 추가 (현재는 로그인 불가하고, 데이터 상으로만 존재)
  • 댓글과 미션 해금 알림에 대한 이력 적재 이벤트 등록
  • 추가한 기능에 대한 테스트 코드 작성

🥺 리뷰어에게 하고싶은 말


알림 이력 기능에 대해 기획적으로 논의해야할 부분이 있어, 우선 필수적인 코드들을 우선 작성하고 PR을 올립니다.
전체 회의에서 추가 논의되어야 할 부분들은 다음과 같습니다.

  • 푸시알림과 알림이력은 서로다른 개념인가요? 직관적으로 봤을때는 동일한 개념으로 보이지만, 푸시알림에만 존재하는 알림 종류가 있고 알림이력에만 존재하는 알림 종류가 있기 때문에, 한 번 짚고 넘어가는 것이 좋을 것 같습니다.
  • 메인화면으로 이동하는 것과 신버전 출시시 업데이트 페이지로 가기위한 딥링크가 존재할까요? 그리고 프론트 입장에서 딥링크 주소로 null이 입력될경우, 앱이 어떻게 동작할까요? 특정 알림의 경우 이 부분에 대한 명시가 되어있지 않아, 확인이 필요합니다.
  • 생일 안내 알림의 경우, 몇 시에 발송되는 것이 좋을까요?
  • 신버전 출시에 대한 알림 발송은 어떻게 트리거되도록 해야할까요? 앱 버전 관리에 있어서도 불편한 점이 있어, 관리자 콘솔을 만들어 이와 관련된 불편한 점을 한 번에 해결할 수 있을 것 같습니다.

🔗 참조 or 관련된 이슈


https://no5ing.atlassian.net/browse/OING-379

@Kwon770 Kwon770 added the ✨ FEATURE 기능 추가 label Dec 27, 2024
@Kwon770 Kwon770 self-assigned this Dec 27, 2024
@github-actions github-actions bot changed the title feat: 알림 목록 조회 기능 추가 [OING-379] feat: 알림 목록 조회 기능 추가 Dec 27, 2024
Copy link

Test Results

 54 files   54 suites   12s ⏱️
170 tests 170 ✅ 0 💤 0 ❌
171 runs  171 ✅ 0 💤 0 ❌

Results for commit ef42eea.

Copy link

Code Coverage

File Coverage [59.26%]
UserNotificationHistoryService.java 100% 🍏
ProfileStyle.java 100% 🍏
BaseEntity.java 100% 🍏
NotificationResponse.java 91.55% 🍏
MemberService.java 87.85% 🍏
UserNotificationHistory.java 87.1% 🍏
MemberBridgeImpl.java 76%
CreateUserNotificationHistoryDTO.java 70.97%
UserNotificationHistoryController.java 54.55%
FamilyNotificationEventListener.java 46.35%
DailyNotificationJob.java 6.46%
Total Project Coverage 52.77% 🍏

Copy link
Member

@Ji-soo708 Ji-soo708 left a comment

Choose a reason for hiding this comment

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

추가 논의되어야 하는 부분들이 많아 스타일적인 거에 대해서만 코멘트 남겼습니다! 복잡한 기능이라 구현할 부분이 많았는데 수고하셨어요~~

@Index(name = "user_notification_history_idx1", columnList = "sender_member_id"),
@Index(name = "user_notification_history_idx2", columnList = "receiver_member_id")
})
public class UserNotificationHistory extends BaseEntity {
Copy link
Member

Choose a reason for hiding this comment

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

저희 플젝에서 회원은 Member로 나타내고 있어서 통일성을 위해 User 대신 MemberNotificationHistory로 바꾸는 건 어떠신가요??

@Schema(description = "알림 스타일", example = "BIRTHDAY")
NotificationStyle style,
@Schema(description = "발송자 프로필 스타일", example = "BIRTHDAY")
ProfileStyle sencerProfileStyle,
Copy link
Member

Choose a reason for hiding this comment

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

오타가 있어요! senderProfileStyle로 바꿔주세요~

Comment on lines +81 to +93
@Scheduled(cron = "0 0 13 * * *", zone = "Asia/Seoul") // 1:00 PM
@SchedulerLock(name = "NextWeekBirthdayNotificationSchedule", lockAtMostFor = "PT30S", lockAtLeastFor = "PT30S")
public void sendNextWeekBirthdayNotification() {
LocalDate nextWeek = LocalDate.now().plusWeeks(1);
List<Member> birthdayMembers = memberService.findBirthdayMembersByDate(nextWeek);

birthdayMembers.forEach(sender -> {
List<String> receiverMemberIds = memberService.getFamilyMembersIdsByFamilyId(sender.getFamilyId());
receiverMemberIds.remove(sender.getId());

userNotificationHistoryService.appendNextWeekBirthdayNotiHistory(sender.getName(), sender.getId(), receiverMemberIds);
});
}
Copy link
Member

Choose a reason for hiding this comment

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

아래 기능들과 내부 로직이 비슷한데 아래처럼 공통 로직을 추출하는 건 어떠신가요??

@Scheduled(cron = "0 0 13 * * *", zone = "Asia/Seoul") 
@SchedulerLock(name = "NextWeekBirthdayNotificationSchedule", lockAtMostFor = "PT30S", lockAtLeastFor = "PT30S")
public void sendNextWeekBirthdayNotification() {
    sendBirthdayNotification(LocalDate.now().plusWeeks(1), "nextWeek");
}

@Scheduled(cron = "0 0 13 * * *", zone = "Asia/Seoul")
@SchedulerLock(name = "TomorrowBirthdayNotificationSchedule", lockAtMostFor = "PT30S", lockAtLeastFor = "PT30S")
public void sendTomorrowBirthdayNotification() {
    sendBirthdayNotification(LocalDate.now().plusDays(1), "tomorrow");
}

@Scheduled(cron = "0 0 13 * * *", zone = "Asia/Seoul")
@SchedulerLock(name = "TodayBirthdayNotificationSchedule", lockAtMostFor = "PT30S", lockAtLeastFor = "PT30S")
public void sendTodayBirthdayNotification() {
    sendBirthdayNotification(LocalDate.now(), "today");
}

private void sendBirthdayNotification(LocalDate targetDate, String notificationType) {
     List<Member> birthdayMembers = memberService.findBirthdayMembersByDate(targetDate);

     birthdayMembers.forEach(sender -> {
         List<String> receiverMemberIds = memberService.getFamilyMembersIdsByFamilyId(sender.getFamilyId());
         receiverMemberIds.remove(sender.getId());

         if (notificationType.equals("nextWeek")) {
             userNotificationHistoryService.appendNextWeekBirthdayNotiHistory(sender.getName(), sender.getId(), receiverMemberIds);
         } else if (notificationType.equals("tomorrow")) {
             userNotificationHistoryService.appendTomorrowBirthdayNotiHistory(sender.getName(), sender.getId(), receiverMemberIds);
         } else if (notificationType.equals("today")) {
             userNotificationHistoryService.appendTodayBirthdayNotiHistory(sender.getName(), sender.getId(), receiverMemberIds);
         }
     });
 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ FEATURE 기능 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants