작업 작은행동 알림 저장 api, listener 생성 #37
Conversation
| import org.springframework.stereotype.Service | ||
| import org.springframework.transaction.annotation.Transactional | ||
|
|
||
| @Service |
There was a problem hiding this comment.
@Transactional을 클래스 레벨에 선언하는건 어때?!
read-only인 경우에만 메서드 레벨에 선언하고!
There was a problem hiding this comment.
모든 메서드에 적용할 필요 없을 것 같아서 메서드단에 붙여줬어! 저장할 때만 설정하려구
| @PostMapping("/create") | ||
| fun createNotifications( |
There was a problem hiding this comment.
알림은 작업쪽에서 발행하는 이벤트를 받아서 등록하는 플로우로 생각했는데, 직접적으로 클라이언트가 푸쉬알림을 등록하는 경우가 있을까?
There was a problem hiding this comment.
그 나중에 할게요? 누르면 알림 재발송 설정하는거 할 때 필요할 것 같앙
There was a problem hiding this comment.
그 때도 task로 거쳐서 보내야 할 것 같아!
task 쪽에 리마인더 저장 여부를 저장하려구
| private val pushNotificationRepository: PushNotificationRepository, | ||
| ) { | ||
| @Transactional | ||
| fun upsert(pushNotification: PushNotification) = pushNotificationRepository.save(pushNotification.toEntity()) |
There was a problem hiding this comment.
- 함수 네이밍이 Repository에 의존적인 것 같아.
Service니까 알람을 등록하는 의미가 명확하게 드러나는 함수 이름을 쓰는건 어떨까? - 이 메서드의 의도는 알림 생성에 관련된것 같은데
upsert를 쓴 이유가 궁금해!
There was a problem hiding this comment.
- 음.. 그럼 뭐라고 하는 게 좋을까..?? 추천해줄 수 있어?? 이 메서드에서 알림 등록 + 내용 업데이트까지 포함할거야
- 나중에 업데이트 되는 부분도 포함하려고 upsert라고 했어 활성화 여부 같은 거
There was a problem hiding this comment.
-
직관적으로
createNotification는 어때?
https://cocobi.tistory.com/27
위의 글 한번 참고해봐도 좋을 것 같아! -
create랑 update랑 한 메서드 안에서 다루는 것이 괜찮을까? 리포지토리에서 같은 메서드를 부를 순 있지만 서비스에서는 구분하는 것이 낫지 않을까 생각되네!
There was a problem hiding this comment.
어처피 같은 Jpa 메소드 사용해서 create, update을 한 메소드에서 하게 구현하는 건 상관없을 것 같아. 그래서 이름을 create이라고 하기에는 update의 의미를 포함하지 않아서 upsert를 사용한거였어~!
#31
Proposed Changes
Code Review Point
(리뷰어가 중점적으로 보면 좋을 부분을 적어주세요.)