-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feature/#297 댓글 알림 조회 api, 읽음 상태 변경 api 및 is read 추가 #310
The head ref may contain hidden characters: "Feature/#297_\uB313\uAE00_\uC54C\uB9BC_\uC870\uD68C_API_\uBC0F_is_read_\uCD94\uAC00"
Changes from 5 commits
be9b6c2
ff9c56f
7a4a36f
2f324da
b7ab305
569427b
7f052f4
2841680
aa85fc9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
package com.emmsale.notification.api; | ||
|
||
import com.emmsale.member.domain.Member; | ||
import com.emmsale.notification.application.UpdateNotificationCommandService; | ||
import com.emmsale.notification.application.UpdateNotificationQueryService; | ||
import com.emmsale.notification.application.dto.UpdateNotificationResponse; | ||
import java.util.List; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.http.HttpStatus; | ||
import org.springframework.web.bind.annotation.GetMapping; | ||
import org.springframework.web.bind.annotation.PathVariable; | ||
import org.springframework.web.bind.annotation.PutMapping; | ||
import org.springframework.web.bind.annotation.RequestParam; | ||
import org.springframework.web.bind.annotation.ResponseStatus; | ||
import org.springframework.web.bind.annotation.RestController; | ||
|
||
@RestController | ||
@RequiredArgsConstructor | ||
public class UpdateNotificationApi { | ||
|
||
private final UpdateNotificationQueryService updateNotificationQueryService; | ||
private final UpdateNotificationCommandService updateNotificationCommandService; | ||
|
||
@GetMapping("/update-notifications") | ||
public List<UpdateNotificationResponse> find( | ||
final Member authMember, | ||
@RequestParam("member-id") final Long loginMemberId | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 이해한 바로는 알람 대상자(receiver)의 id를 의미하는 것 같은데, loginMemberId보다는 receiverId는 어떨지 제안드려봅니다! 처음 봤을 때 authMember의 id와 무엇이 다른건지 잘 이해가 안됐어서요...! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 반영 완료했습니다 ! |
||
) { | ||
return updateNotificationQueryService.findAll(authMember, loginMemberId); | ||
} | ||
|
||
@PutMapping("/update-notifications/{update-notifications-id}/read") | ||
@ResponseStatus(HttpStatus.NO_CONTENT) | ||
public void read( | ||
final Member authMember, | ||
@PathVariable("update-notifications-id") final Long notificationId | ||
) { | ||
updateNotificationCommandService.read(authMember, notificationId); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
package com.emmsale.notification.application; | ||
|
||
import com.emmsale.comment.event.UpdateNotificationEvent; | ||
import com.emmsale.notification.domain.UpdateNotification; | ||
import com.emmsale.notification.domain.UpdateNotificationRepository; | ||
import com.emmsale.notification.domain.UpdateNotificationType; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.context.event.EventListener; | ||
import org.springframework.stereotype.Component; | ||
|
||
@Component | ||
@RequiredArgsConstructor | ||
public class NotificationEventListener { | ||
|
||
private final UpdateNotificationRepository updateNotificationRepository; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Service에 있던 Listner가 별도의 Component로 분리가 됬군요. |
||
private final FirebaseCloudMessageClient firebaseCloudMessageClient; | ||
|
||
@EventListener | ||
public void createUpdateNotification(final UpdateNotificationEvent updateNotificationEvent) { | ||
final UpdateNotification updateNotification = new UpdateNotification( | ||
updateNotificationEvent.getReceiverId(), | ||
updateNotificationEvent.getRedirectId(), | ||
UpdateNotificationType.from(updateNotificationEvent.getUpdateNotificationType()), | ||
updateNotificationEvent.getCreatedAt() | ||
); | ||
|
||
final UpdateNotification savedNotification = | ||
updateNotificationRepository.save(updateNotification); | ||
|
||
firebaseCloudMessageClient.sendMessageTo(savedNotification); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,32 +1,37 @@ | ||
package com.emmsale.notification.application; | ||
|
||
import static com.emmsale.member.exception.MemberExceptionType.NOT_MATCHING_TOKEN_AND_LOGIN; | ||
import static com.emmsale.notification.exception.NotificationExceptionType.NOT_FOUND_NOTIFICATION; | ||
|
||
import com.emmsale.member.domain.Member; | ||
import com.emmsale.member.exception.MemberException; | ||
import com.emmsale.notification.domain.UpdateNotification; | ||
import com.emmsale.notification.domain.UpdateNotificationRepository; | ||
import com.emmsale.comment.event.UpdateNotificationEvent; | ||
import com.emmsale.notification.domain.UpdateNotificationType; | ||
import com.emmsale.notification.exception.NotificationException; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.context.event.EventListener; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
@Service | ||
@RequiredArgsConstructor | ||
@Transactional | ||
public class UpdateNotificationCommandService { | ||
|
||
private final UpdateNotificationRepository updateNotificationRepository; | ||
private final FirebaseCloudMessageClient firebaseCloudMessageClient; | ||
|
||
@EventListener | ||
public void createUpdateNotification(final UpdateNotificationEvent updateNotificationEvent) { | ||
final UpdateNotification updateNotification = new UpdateNotification( | ||
updateNotificationEvent.getReceiverId(), | ||
updateNotificationEvent.getRedirectId(), | ||
UpdateNotificationType.from(updateNotificationEvent.getUpdateNotificationType()), | ||
updateNotificationEvent.getCreatedAt() | ||
); | ||
|
||
public void read(final Member authMember, final Long notificationId) { | ||
final UpdateNotification savedNotification = | ||
updateNotificationRepository.save(updateNotification); | ||
updateNotificationRepository.findById(notificationId) | ||
.orElseThrow(() -> new NotificationException(NOT_FOUND_NOTIFICATION)); | ||
|
||
validateSameMember(authMember, savedNotification.getReceiverId()); | ||
|
||
savedNotification.read(); | ||
} | ||
|
||
firebaseCloudMessageClient.sendMessageTo(savedNotification); | ||
private void validateSameMember(final Member authMember, final Long loginMemberId) { | ||
if (authMember.isNotMe(loginMemberId)) { | ||
throw new MemberException(NOT_MATCHING_TOKEN_AND_LOGIN); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
package com.emmsale.notification.application; | ||
|
||
import static com.emmsale.comment.exception.CommentExceptionType.NOT_FOUND_COMMENT; | ||
import static com.emmsale.member.exception.MemberExceptionType.NOT_MATCHING_TOKEN_AND_LOGIN; | ||
|
||
import com.emmsale.comment.domain.Comment; | ||
import com.emmsale.comment.domain.CommentRepository; | ||
import com.emmsale.comment.exception.CommentException; | ||
import com.emmsale.member.domain.Member; | ||
import com.emmsale.member.exception.MemberException; | ||
import com.emmsale.notification.application.dto.UpdateNotificationResponse; | ||
import com.emmsale.notification.domain.UpdateNotification; | ||
import com.emmsale.notification.domain.UpdateNotificationRepository; | ||
import java.util.Comparator; | ||
import java.util.List; | ||
import java.util.stream.Collectors; | ||
import lombok.RequiredArgsConstructor; | ||
import org.springframework.stereotype.Service; | ||
import org.springframework.transaction.annotation.Transactional; | ||
|
||
@Service | ||
@RequiredArgsConstructor | ||
@Transactional(readOnly = true) | ||
public class UpdateNotificationQueryService { | ||
|
||
private final UpdateNotificationRepository updateNotificationRepository; | ||
private final CommentRepository commentRepository; | ||
|
||
public List<UpdateNotificationResponse> findAll( | ||
final Member authMember, | ||
final Long loginMemberId | ||
) { | ||
validateSameMember(authMember, loginMemberId); | ||
|
||
final List<UpdateNotification> notifications = | ||
updateNotificationRepository.findAllByReceiverId(authMember.getId()); | ||
|
||
return notifications.stream() | ||
.sorted(Comparator.comparing(UpdateNotification::getCreatedAt)) | ||
.map(this::convertToResponse) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
private void validateSameMember(final Member authMember, final Long loginMemberId) { | ||
if (authMember.isNotMe(loginMemberId)) { | ||
throw new MemberException(NOT_MATCHING_TOKEN_AND_LOGIN); | ||
} | ||
} | ||
|
||
private UpdateNotificationResponse convertToResponse(final UpdateNotification notification) { | ||
if (notification.isCommentNotification()) { | ||
final Comment savedComment = commentRepository.findById(notification.getReceiverId()) | ||
.orElseThrow(() -> new CommentException(NOT_FOUND_COMMENT)); | ||
|
||
return UpdateNotificationResponse.convertCommentNotification(notification, savedComment); | ||
} | ||
|
||
return UpdateNotificationResponse.convertEventNotification(notification); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
package com.emmsale.notification.application.dto; | ||
|
||
import com.emmsale.comment.domain.Comment; | ||
import com.emmsale.notification.domain.UpdateNotification; | ||
import com.emmsale.notification.domain.UpdateNotificationType; | ||
import com.fasterxml.jackson.annotation.JsonFormat; | ||
import com.fasterxml.jackson.annotation.JsonProperty; | ||
import java.time.LocalDateTime; | ||
import lombok.Getter; | ||
import lombok.RequiredArgsConstructor; | ||
|
||
@Getter | ||
@RequiredArgsConstructor | ||
public class UpdateNotificationResponse { | ||
|
||
private final Long updateNotificationId; | ||
private final Long receiverId; | ||
private final Long redirectId; | ||
@JsonFormat(pattern = "yyyy:MM:dd:HH:mm:ss") | ||
private final LocalDateTime createdAt; | ||
private final UpdateNotificationType type; | ||
@JsonProperty(value = "isRead") | ||
private final boolean isRead; | ||
private final CommentTypeNotification commentTypeNotification; | ||
|
||
@RequiredArgsConstructor | ||
@Getter | ||
public static class CommentTypeNotification { | ||
private final String content; | ||
private final String eventName; | ||
private final String commenterImageUrl; | ||
} | ||
|
||
public static UpdateNotificationResponse convertCommentNotification( | ||
final UpdateNotification notification, | ||
final Comment comment | ||
) { | ||
return new UpdateNotificationResponse( | ||
notification.getId(), notification.getReceiverId(), | ||
notification.getRedirectId(), notification.getCreatedAt(), | ||
notification.getUpdateNotificationType(), notification.isRead(), | ||
hong-sile marked this conversation as resolved.
Show resolved
Hide resolved
|
||
new CommentTypeNotification( | ||
comment.getContent(), | ||
comment.getEvent().getName(), | ||
comment.getMember().getImageUrl() | ||
) | ||
); | ||
} | ||
|
||
public static UpdateNotificationResponse convertEventNotification( | ||
final UpdateNotification notification | ||
) { | ||
return new UpdateNotificationResponse( | ||
notification.getId(), notification.getReceiverId(), | ||
notification.getRedirectId(), notification.getCreatedAt(), | ||
notification.getUpdateNotificationType(), notification.isRead(), | ||
null | ||
); | ||
} | ||
|
||
private boolean getIsRead() { | ||
return isRead; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,9 @@ | ||
package com.emmsale.notification.domain; | ||
|
||
import java.util.List; | ||
import org.springframework.data.jpa.repository.JpaRepository; | ||
|
||
public interface UpdateNotificationRepository extends JpaRepository<UpdateNotification, Long> { | ||
|
||
List<UpdateNotification> findAllByReceiverId(final Long receiverId); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 예외가 발생하는 상황을 고려했을 때
'알림을 조회할 권한이 없습니다'와 같은 식으로 변경하면 예외 상황을 이해하기 좀 더 쉬울 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 아마란스 의견에 동의합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Member 관련 Exception에 굳이 알림이라는 단어가 필요할까라는 생각이 듭니다.
그리고 해당 예외는 저번에 얘기했던 header에 있는 토큰의 member ID는 자원을 찾기 위해서가 아니라 Authorization에서만 사용해야하는 것이 올바르게 사용하는 것이라 생각이 들어서 위와 같은 예외를 하나 만들어두었습니다.
혹시 어떻게 생각하시나요? @amaran-th @hyeonjerry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 이해한게 맞다면 댓글과 같은 리소스의 소유자와 그것을 변경하려는 사용자 사이에 불일치가 일어날 경유 위 예외가 발샹하는 것 같습니다.
그렇다면 저는 "사용자가 일치하지 않습니다" 정도로 포괄적으로 처리할 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 좋네요 수정할게요