Conversation
Test Results 18 files 18 suites 6s ⏱️ Results for commit 2943b22. ♻️ This comment has been updated with latest results. |
| return String.format(message, args); | ||
| } | ||
| } | ||
| } No newline at end of file |
| @@ -0,0 +1,3 @@ | |||
| package com.gamzabat.algohub.feature.notification.enums; | |||
|
|
|||
| public enum NotificationType { PROBLEM, STUDY_GROUP, COMMENT } No newline at end of file | |||
| List<Notification> findAllByUser(User user); | ||
|
|
||
| List<Notification> findAllByUserAndType(User user, NotificationType type); | ||
|
|
There was a problem hiding this comment.
findAllByUserAndType() 메서드 쓰게 되면 위에 있는 findAllByUser() 사용 안하게 되지 않나요?! 확인 후에 사용 안되는 메서드면 제거해주세요!
There was a problem hiding this comment.
전체 notification을 다 가져와야하는 경우가 있어서 위에 findAllByUser도 냅둬야합니다!
| case NotificationCategory.NEW_MEMBER_REQUESTED -> setting.isNewMember(); | ||
| case NotificationCategory.NEW_GROUP_JOINED -> setting.isNewMember(); |
There was a problem hiding this comment.
이 친구들 새로운 가입자 알림에 대해 꺼져있음 아예 안오게 해야할까요??
뭔가 새로운 가입 요청이나 새로운 그룹 가입 했다는 알림은 선택사항 없이 받지 않나 싶어서요!
혹시 이미 프론트나 기획쪽과 이야기 된 내용이 아니라면, 비즈니스 로직의 내용인 것 같아서 의견들을 들어 봐야 할 것 같아요
There was a problem hiding this comment.
음 이부분은 아직 기능개발중인 부분에 대한거 짤 때 미리 만들어둔거라서 얘기된 바는 없어요! 이후에 개발되면서 프론트와 얘기해서 정하면 될거같아요!
There was a problem hiding this comment.
이거는 채널톡에 안건을 올려주시면 좋을 것 같아요! 아마 다음 코타 때 회의를 해볼 것 같네요!
이번 PR에서는 일단 머지하고 나중에 회의 결과로 나온 부분을 반영하는 방향으로 구현합시다
junggyo1020
left a comment
junggyo1020
left a comment
There was a problem hiding this comment.
기능 구현하시느라 고생하셨습니다!!
주된 의논사항은 논의가 완료된 것 같아서, Category로만 대응하는 방식에 대해 제 의견만 남겨드리면..
만약 카테고리만 사용해 관리하면, 클라이언트 측에서 스터디 관련 모든 카테고리를 알고 있어야 해서 새로 카테고리가 추가될 때마다 모든 세부 카테고리에 대한 클라이언트 코드 수정이 필요할 것 같아요. 카테고리 관련 부분은 나중에 추가될 여지도 있어 보여서요..!
그래서 저는 type으로 한번 묶어주는게 더 좋은 방법인 것 같다는 의견입니다 :)
넵 의견 감사드려요! |
4 participants
📌 Related Issue
be-40
🚀 Description
📢 Review Point
type을 새로 안만들고 Category로만 하고싶었는데 방법이 잘 떠오르진 않아서 type을 새로 만들었습니다
좋은 방법있으면 말해주세요!
📚Etc (선택)