Conversation
…icationEvent를 listen 하도록 수정 #649
| private static final String UPDATE_NOTIFICATION_COMMENT_TYPE = "COMMENT"; | ||
|
|
||
| private final Long receiverId; | ||
| private final Long redirectId; |
There was a problem hiding this comment.
혹시 redirectId가 댓글의 id인가요?
만약 맞다면 redirectId로 추상화하여 EventNotificationEvent와 필드를 통일한 것도 좋지만 개인적으로 commentId 혹은 eventId와 같이 명시적으로 작성하는게 오해를 불러일으킬 확률을 낮출 수 있을 것 같아요.
There was a problem hiding this comment.
오 맞네여 이제 구분지어줄 필요가 없을 것 같습니다
감사합니다
| private NotificationType type; | ||
|
|
||
| @Column(columnDefinition = "MEDIUMTEXT") | ||
| private String jsonData; |
There was a problem hiding this comment.
json으로 데이터를 저장함으로써 데이터 형식에 상관없이 저장할 수 있게 되었네요👍
There was a problem hiding this comment.
json data로 바꾸신건 야무지네요.
이제 QueryService들을 조금씩 수정을 해야겠군요.
| @@ -17,6 +17,7 @@ drop table if exists kerdy.message; | |||
| drop table if exists kerdy.room; | |||
| drop table if exists kerdy.feed; | |||
| drop table if exists kerdy.image; | |||
| drop table if exists kerdy.notification; | |||
There was a problem hiding this comment.
이제 update_notificaition도 지워야겠네요.
| private NotificationType type; | ||
|
|
||
| @Column(columnDefinition = "MEDIUMTEXT") | ||
| private String jsonData; |
There was a problem hiding this comment.
json data로 바꾸신건 야무지네요.
이제 QueryService들을 조금씩 수정을 해야겠군요.
| @@ -40,6 +49,50 @@ public void createUpdateNotification(final UpdateNotificationEvent updateNotific | |||
| } | |||
| } | |||
|
|
|||
| @Transactional(propagation = Propagation.REQUIRES_NEW) | |||
| @TransactionalEventListener | |||
| public void createCommentNotification(final CommentNotificationEvent commentNotificationEvent) { | |||
There was a problem hiding this comment.
이 둘의 코드가 거의 동일한 것 같아요. 차이라고 한다면,
Notification을 생성할 때, Type과 파라미터 정도겠군요.
이정도로 공통된 부분은 추상화가 가능하지 않을까요?
(이건 의견이라 반영 안하셔도 됩니다.)
There was a problem hiding this comment.
NotificationEvent에서, EventType을 들고있어요.
이를 이용해서 한다면 추상화가 될 것 같아요.
There was a problem hiding this comment.
다음 이슈에서 코드 삭제하면서 한번 고려해보도록 할게요 !
| @Getter | ||
| public class CommentNotificationEvent { | ||
|
|
||
| private static final String UPDATE_NOTIFICATION_COMMENT_TYPE = "COMMENT"; |
There was a problem hiding this comment.
여기에 있는 static field를 단순 String이 아니라 NotificationType을 사용해보는 건 어떨까요?
There was a problem hiding this comment.
CommentNotificationEvent가 굳이 Notification을 알아야할까 라는 생각했어요.
enum을 쓴다고 해서 단지 패키지 의존성만 생길 뿐, 큰 장점을 얻지 못한다고 생각했습니다.
#️⃣연관된 이슈
업무
Comment, Event에 관한 알림 메시지가 요구사항에 의해 달라졌고, 이를 이제 하나의 UpdateNotification에서 관리할 수 없게 됐습니다.
그래서 기존 UpdateNoficiation(댓글, 행사 알림)이 Notification으로 변경됐습니다.
이제 Notification에는 json_data라는 칼럼이 존재하는데 이는 알림에 관한 정보를 담고 있는 Json 형식의 데이터라고 생각하면 됩니다.
이렇게 한 이유는 알림이 추후에 추가될 때마다 테이블을 생성하거나 상속을 사용하는 것이 좋지 않은 설계라고 생각했습니다.
알림에 관해 따로 검색이나 특별한 기능이 없기 때문에 저장 용도 + 목록 조회이므로 이렇게 가능하다고 생각이 들어 구현했습니다
예상 소요 시간 및 실제 소요 시간
예상 소요 시간 9/29
실제 소요 시간 9/27