Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- receiverId, createdAt, redirectId #663
- 공통된 필드는 칼럼으로 가지고 있으므로 jsonData에 저장하지 않기 위해 @JsonIgnore 추가 #663
amaran-th
approved these changes
Oct 3, 2023
| //given | ||
| //member1 -> 안드로이드, member2 -> 안드로이드 백엔드, member3 -> 프론트엔드 | ||
| //안드로이드, 백엔드 태그를 가진 Event 추가 | ||
| //member1, member2만 알림이 감 |
hyeonjerry
approved these changes
Oct 3, 2023
| private final NotificationCommandService notificationCommandService; | ||
|
|
||
| @GetMapping("/notifications") | ||
| public List<NotificationAllResponse> find( |
There was a problem hiding this comment.
이건 제 개인적인 느낌인데 DTO명이 AllResponse라 List로 Response들을 갖고 있을 것 같은 느낌을 받았어요. 혹시 DetailResponse는 어떠신가요?
There was a problem hiding this comment.
그럴 수 있겠네요,, 수정했습니다
hong-sile
approved these changes
Oct 4, 2023
|
|
||
| @RequiredArgsConstructor | ||
| @Getter | ||
| public abstract class NotificationEvent { |
There was a problem hiding this comment.
제가 저번에 드린 의견을 잘 반영해주셨군요!
| import org.springframework.stereotype.Service; | ||
|
|
||
| @Service | ||
| @RequiredArgsConstructor |
There was a problem hiding this comment.
@transactional(readOnly=true)가 빠진 것 같아요.
|
|
||
| final Notification notification = notificationRepository.save( | ||
| new Notification(NotificationType.COMMENT, jsonData) | ||
| new Notification( | ||
| NotificationType.valueOf(notificationEvent.getNotificationType()), |
There was a problem hiding this comment.
오 저는 저번에 Enum 자체를 Event 가 갖게했는데, 여기서 변환하는게 더 낫네요.
의존하는 곳이 잘 줄어든 것 같아요.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#️⃣연관된 이슈
📝작업 내용
기존 UpdateNotification에 있던 API들을 Notification에 새로 만들었습니다.
알림 읽기, 알림 삭제 API 가 UpdateNotification에 있었는데,
Notification으로 변경되어서 해당 API 가 필요해졌습니다.
이전 이슈에서는 JsonData에 모든 값들을 저장했는데,
알림에서 공통적으로 필요한 redirectId, receiverId, createdAt은 JsonData에서 뺐습니다.
공통적으로 필요한 칼럼은 부모
NotificationEvent로 만들고,특정한 알림에 필요한 데이터는 자식 클래스에 만들었습니다.
JsonData는 자식 클래스에 있는 값만을 저장하기 때문에 NotificationEvent에 있는 값들은
JsonIgnore해주었습니다.예상 소요 시간 및 실제 소요 시간
예상 : 10/2
실제 : 10/1