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

fix: 알림나무 API 수정 - 즐겨찾기 추가, 메시지 읽음 처리 #49

Merged
merged 7 commits into from
Jul 4, 2022

Conversation

kang-jisu
Copy link
Contributor

이슈

작업 내용

  • 기타

    • 알림나무 조회 LoginUser에서 유저 정보 얻어오도록 수정
    • batchNoticeTree private method로 분리해서 전체 갱신, 특정 userId 갱신에 둘다 사용할 수 있도록 수정 ( 알림나무 조회시 알림나무테이블에 데이터가 안만들어진 유저의 경우 그 즉시 batchNoticeTree돌려서 하나 만들도록함)
  • 즐겨찾기

    • 즐겨찾기 추가한 메시지 알림나무 8개 메시지 만들 때 포함되도록 수정
  • 읽음처리

    • NoticeTree.getUnreadMessages에서 지금 읽으려는 messageId가 들어있는지 확인, 있다면 unreadlist -> readlist로 이동하는과정 진행
    • NoticeTreeService.updateNoticeTree메서드 한번 쭉 읽어보고 이해안되시는 부분 있거나 이상하면 알려주세요

기타 사항

  • 일부 테스트가 그 테스트만 돌릴때는 성공하는데 테스트 전체돌리기하면 자꾸 실패떠서 disabled 처리를 또 했습니다 ㅠㅠㅠ 원인을 모르겠어요
  • 알림나무 생성할때 컬렉션을 List->Set로 변경했고 Set으로 중복처리 하려고 Dto hashCode, equals 오버라이딩 한거입니다 (안읽은 메시지, 즐겨찾기 메시지 중복 없애려고)

체크리스트

  • 테스트코드 통과

진짜 대충 구현만 되게 만든거라 이상한부분이 많을 수 있습니다 ㅠㅠ 이해안되는 부분 물어봐주세요

- 메시지함에서 즐겨찾기한 메시지(페이징처리) 조회와 구분
- 테스트
- 안읽은 메시지, 즐겨찾기 메시지 중복 제거되도록 컬렉션 Set으로 변경
- 메시지 읽음여부 수정하는 messageService.updateReadMessage에 noticeTreeService.updateNoticeTree 추가
- NoticeTree의 안읽은 메시지 리스트에 지금 읽고있는 messageId가 포함되고있을 경우 읽은 리스트로 이동
@kang-jisu kang-jisu added 💡feature API feature 🌲칭찬 칭찬 📬메시지함 메시지함 labels Jul 3, 2022
@kang-jisu kang-jisu self-assigned this Jul 3, 2022
@kang-jisu kang-jisu added this to To do in BeTree via automation Jul 3, 2022
@kang-jisu kang-jisu requested a review from suubinkim July 3, 2022 16:42
@@ -67,13 +69,21 @@ public void batchNoticeTree() {
List<Message> unreadMessages = messageRepository.findByUserIdAndAlreadyRead(user.getId(), false);

// 안읽은 메시지 먼저 8개 리스트에 넣음
Copy link
Member

Choose a reason for hiding this comment

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

이건 그냥 갑자기 생각난건데 저희 안 읽은 것 중에 최신순으로 가져오기 그런 것도 얘기했었나요??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

아니요 8개이상일때 어떤거가져올지는 랜덤으로 선정해야할 것 같은데 일단은 메시지고르는 로직 다 생략하고 개수만맞췄어요 !

@ApiResponse(code = 400, message = "[C001]관리자만 사용 가능한 API"),
})
@GetMapping("/api/notices/batch")
public ResponseEntity<Void> batchNoticeTree(@ApiIgnore @LoginUser LoginUserDto loginUser) {
List<String> adminEmails = new ArrayList<>();
adminEmails.add("bi0425@naver.com");
adminEmails.add("happy01234@kakao.com");
Copy link
Member

Choose a reason for hiding this comment

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

저는 그냥 스케줄러 돌리는 걸로 생각했는데 이렇게 api로 만들고 나면 실행 과정이 어떻게 되는건가용?!?!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

스케줄러돌리는 코드를 지금 당장 구현하기가 어려울거같아서 그냥 수동api로 일단만들었고 나중에 다시 적용해야할것같아요 일단은 저희가 api 요청해서 갱신하는거로 동작해요 ..!

@@ -42,8 +42,12 @@ public NoticeTree(Long id, String unreadMessages, String readMessages, Long user
this.userId = userId;
}

public void updateMessages(String unreadMessages) {
public void resetMessages(String unreadMessages) {
this.unreadMessages = unreadMessages;
this.readMessages = "";
}
Copy link
Member

Choose a reason for hiding this comment

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

여기 컬럼에서 readMessages 만 값을 ""로 해준거는 어떤 차이예요!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

메시지목록을 , 구분자로1,2,3 이런식으로 저장하는데 알림나무 생성하고 처음에는 읽은메시지목록이 없어서 ""로 뒀습니당 null로했더니 나중에 readmessage.split("")할때 null오류나서 좀 불편해서 이렇게처리했어여!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reset에서는 unread에 1,2,3,4 이런식으로 번호8개 스트링, read에 빈 스트링 ""가 채워지고
update에서는 만약 1번메시지 읽었다면 "1,2,3,4,5,6,7,8" "" -> "2,3,4,5,6,7,8" "1" 이렇게 바뀌니까 인자로받은값 바로 대입시켜줬어요 !
이렇게 string으로하는게 좀 애매하긴한데 list이나 list 을 디비에 저장하려면 Enumarated인가?? 좀 복잡한 처리 해야되는거같아서 그냥 이렇게 구현해봤습니당..!

.collect(Collectors.joining(","));

// 이미 존재하는 경우 변경
Optional<NoticeTree> saveNoticeTree = noticeTreeRepository.findByUserId(userId);
Copy link
Member

Choose a reason for hiding this comment

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

여기서 null 이면 exception 뜨나요..!? 검색해봤는데 맘에 들게 딱 안나와서요 ㅠ..ㅎ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

optional로받을때 null이면 예외가 안생기려고 쓰는거라 안떠요!
saveNoticeTree.isPresent()가 false로 나와요 이거물어보신거맞나요?_? Optional.empty()랑 같은값으로 생성돼요!

Copy link
Member

Choose a reason for hiding this comment

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

아항 Optional.empty()랑 같은 값이군요 넵 !!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

☺️


// 비트리 제공 메시지로 8개까지 다시 채움
long remainCount = 8 - noticeTreeMessages.size();
for (long i = 1; i <= remainCount; i++) {
Copy link
Member

Choose a reason for hiding this comment

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

궁금한게 여기서 remainCount가 0이면 그냥 for문 안 돌고 넘어가나요?!?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 8개보다 적을때만 채우는거라서 일단 그렇게구현되게 했어요!!

@suubinkim
Copy link
Member

지수님 저는 딱히 이상한 부분 없고 테스트만 잘 돌아가면 나중에 놓친 부분 발견하면 그때 수정하고 해도 될 것같습니당.. 프론트랑 붙여보면 나오지 않을까 싶어용 .. 역시 주말동안 뚝딱하셨네요 넘 고생하셨어요 👍👍

@kang-jisu
Copy link
Contributor Author

kang-jisu commented Jul 4, 2022

확인감사합니당 😍 코멘트 남겨주신거처럼 알림나무 스케쥴러적용이나 메시지 선정 로직, 알림나무테이블 레디스 사용 ? 이런것들은 다른 필수적인것들 구현끝나면 다시 생각해보는거로 해요!!
for문 돌리는거나 이런부분 진짜 고민잘안하고 구현만되게 짠거라 나중에 편하게 리팩토링해주셔도 됩니당..!!🙇‍♀️

@kang-jisu kang-jisu merged commit bad64c1 into YAPP-Github:develop Jul 4, 2022
BeTree automation moved this from To do to Done Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌲칭찬 칭찬 💡feature API feature 📬메시지함 메시지함
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants