Skip to content
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 : 알림 기능 구현 #24

Merged
merged 42 commits into from
Nov 28, 2024
Merged

Feature : 알림 기능 구현 #24

merged 42 commits into from
Nov 28, 2024

Conversation

hgh1472
Copy link
Collaborator

@hgh1472 hgh1472 commented Nov 27, 2024

📢 기능 설명

  • 알림 전송
  • 알림 조회
  • 알림 허용
  • 알림 비허용
  • 기기 알림 비허용

추후, JWT 기능 추가 시, 유저 ID 정보를 꺼내오는 로직 추가 필요

연결된 issue

close #4

✅ 체크리스트

  • PR 제목 규칙 잘 지켰는가?
  • 추가/수정사항을 설명하였는가?
  • 이슈넘버를 적었는가?

@hgh1472 hgh1472 self-assigned this Nov 27, 2024
Copy link
Collaborator

@kahyun0255 kahyun0255 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨어요~ 🧚‍♀️


@Getter
public enum NotificationType {
NEW_LETTER("새 편지 도착", "새로운 키워드 편지가 도착했어요."),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이거 주는 메시지는 프론트에서 정한다고 하지 않았나용?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

백엔드측에서 메세지를 보낼 때, 타이틀과 내용을 지정하기 위해서 enum 타입에다가 같이 저장했습니다!

Comment on lines 8 to 9
@RestControllerAdvice
public class NotificationExceptionHandler {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

log 찍을 필요 없나용?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2998abc

추가했습니다!

Copy link
Collaborator

@yeonsu00 yeonsu00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다!

import postman.bottler.notification.exception.NoLetterIdException;

@Getter
public class LetterNotification extends Notification {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

편지 알림은 LetterNotification으로 받고 그냥 신고 알림은 Notification으로 받는 건가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 맞습니다! 편지 관련 알림은 LetterNotification으로 생성되고, 경고나 정지 알림은 Notification으로 생성됩니다.
편지 관련 알림은 알림창에서 바로가기를 제공하기 위해 편지 ID가 따로 필요해서 객체를 구분시켰고, LetterNotification이 이 부분에 대해서만 추가적인 컬럼을 가지기 때문에 도메인 객체에서는 상속을 통해서 관리하도록 했어요!

@PostConstruct
public void init() {
try {
InputStream serviceAccount = new ClassPathResource("bottler-fcm.json").getInputStream();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bottler-fcm.json yml에 넣어놓고
@value로 받아와서 처리해주는거 어떠신가요

fcm:
  내용: bottler-fcm.json

@Value("${위의 경로}")
private String url;
InputStream serviceAccount = new ClassPathResource(url).getInputStream();

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그 방법이 더 좋은 것 같네요! 수정하도록 하겠습니다!

return new Notification(id, type, receiver, createdAt, isRead);
}

private static void validateType(String type) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotificationType 관련 검증이면 가능한 방법이라는 가정 하에 NotificationType 안에 넣는게 좋지 않을까요
약간 이펙티브 자바에서 봤던 느낌?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotificationType인지에 대한 검증은 해당 객체한테 맡기는게 더 괜찮아보이네요!!

public NotificationResponseDTO sendNotification(String type, Long userId, Long letterId) {
Notification notification = Notification.create(type, userId, letterId);
List<Subscription> subscriptions = subscriptionRepository.findByUserId(userId);
subscriptions.forEach(subscription -> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

subscriptions 부분 메서드 분리하는건 어떤거 같나요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 고려해서 수정하도록 해볼게요!

Copy link
Collaborator

@l2yujw l2yujw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨어요~!

Copy link

sonarcloud bot commented Nov 28, 2024

@hgh1472 hgh1472 merged commit 092c2fe into develop Nov 28, 2024
2 checks passed
@hgh1472 hgh1472 deleted the feature/#4-notification branch November 28, 2024 00:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: 알림 기능 추가
4 participants