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

feat : 공지 읽음 표시 로직 추가 및 response 수정 #151

Merged
merged 6 commits into from
Nov 13, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.gamzabat.algohub.feature.notice.domain;

import com.gamzabat.algohub.feature.user.domain.User;

import jakarta.persistence.Entity;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.JoinColumn;
import jakarta.persistence.ManyToOne;
import lombok.Builder;
import lombok.Getter;
import lombok.NoArgsConstructor;

@Entity
@Getter
@NoArgsConstructor
public class NoticeRead {
Copy link
Contributor

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.

정답-!

@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "notice_id")
private Notice notice;
@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "user_id")
private User user;

@Builder
public NoticeRead(Notice notice, User user) {
this.notice = notice;
this.user = user;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,17 @@ public record GetNoticeResponse(String author,
Long noticeId,
String noticeContent,
String noticeTitle,
String createAt) {
String createAt,
boolean isRead) {

public static GetNoticeResponse toDTO(Notice notice) {
public static GetNoticeResponse toDTO(Notice notice, boolean isRead) {
return GetNoticeResponse.builder()
.author(notice.getAuthor().getNickname())
.noticeId(notice.getId())
.noticeTitle(notice.getTitle())
.noticeContent(notice.getContent())
.createAt(DateFormatUtil.formatDate(notice.getCreatedAt().toLocalDate()))
.isRead(isRead)
.build();

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package com.gamzabat.algohub.feature.notice.repository;

import org.springframework.data.jpa.repository.JpaRepository;

import com.gamzabat.algohub.feature.notice.domain.Notice;
import com.gamzabat.algohub.feature.notice.domain.NoticeRead;
import com.gamzabat.algohub.feature.user.domain.User;

public interface NoticeReadRepository extends JpaRepository<NoticeRead, Long> {
boolean existsByNoticeAndUser(Notice notice, User user);
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@
import com.gamzabat.algohub.feature.group.studygroup.repository.GroupMemberRepository;
import com.gamzabat.algohub.feature.group.studygroup.repository.StudyGroupRepository;
import com.gamzabat.algohub.feature.notice.domain.Notice;
import com.gamzabat.algohub.feature.notice.domain.NoticeRead;
import com.gamzabat.algohub.feature.notice.dto.CreateNoticeRequest;
import com.gamzabat.algohub.feature.notice.dto.GetNoticeResponse;
import com.gamzabat.algohub.feature.notice.dto.UpdateNoticeRequest;
import com.gamzabat.algohub.feature.notice.exception.NoticeValidationException;
import com.gamzabat.algohub.feature.notice.repository.NoticeCommentRepository;
import com.gamzabat.algohub.feature.notice.repository.NoticeReadRepository;
import com.gamzabat.algohub.feature.notice.repository.NoticeRepository;
import com.gamzabat.algohub.feature.user.domain.User;

Expand All @@ -38,6 +40,7 @@ public class NoticeService {
private final NoticeCommentRepository noticeCommentRepository;
private final StudyGroupRepository studyGroupRepository;
private final GroupMemberRepository groupMemberRepository;
private final NoticeReadRepository noticeReadRepository;

@Transactional
public void createNotice(@AuthedUser User user, CreateNoticeRequest request) {
Expand Down Expand Up @@ -67,13 +70,16 @@ public GetNoticeResponse getNotice(@AuthedUser User user, Long noticeId) {
if (!groupMemberRepository.existsByUserAndStudyGroup(user, notice.getStudyGroup()))
throw new StudyGroupValidationException(HttpStatus.FORBIDDEN.value(), "참여하지 않은 스터디 그룹 입니다.");

readNotice(user, notice);
Copy link
Contributor

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.

네이밍 때문에 머리 터질 것 같아요.. 저도 찝찝한 이름이지만 더 생각이 안나서 :(

Copy link
Contributor

Choose a reason for hiding this comment

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

markNoticeAsRead 같은거..?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 굳 👍


log.info("success to get notice");
return GetNoticeResponse.builder()
.author(notice.getAuthor().getNickname())
.noticeId(notice.getId())
.noticeTitle(notice.getTitle())
.noticeContent(notice.getContent())
.createAt(DateFormatUtil.formatDate(notice.getCreatedAt().toLocalDate()))
.isRead(noticeReadRepository.existsByNoticeAndUser(notice, user))
Copy link
Contributor

Choose a reason for hiding this comment

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

이건 무조건 true 아닌가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

공지글 하나 조회니 그러겠네요! 수정해야겠어요 감삼다

.build();
}

Expand All @@ -85,7 +91,9 @@ public List<GetNoticeResponse> getNoticeList(@AuthedUser User user, Long studyGr
throw new GroupMemberValidationException(HttpStatus.FORBIDDEN.value(), "참여하지 않은 스터디 그룹입니다");

List<Notice> list = noticeRepository.findAllByStudyGroup(studyGroup);
List<GetNoticeResponse> result = list.stream().map(GetNoticeResponse::toDTO).toList();
List<GetNoticeResponse> result = list.stream().map(
notice -> GetNoticeResponse.toDTO(notice, noticeReadRepository.existsByNoticeAndUser(notice, user))
).toList();
log.info("success to get notice list");
return result;
}
Expand Down Expand Up @@ -121,4 +129,12 @@ private void validateStudyGroupExists(Notice notice) {
.orElseThrow(() -> new StudyGroupValidationException(HttpStatus.BAD_REQUEST.value(), "존재하지 않는 스터디 그룹입니다"));
}

private void readNotice(User user, Notice notice) {
if (!noticeReadRepository.existsByNoticeAndUser(notice, user)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

참고차 코멘트입니다

사실 이렇게 ifNonExist -> Insert 구조일때 타이밍 이슈가 생길수 있습니다.
서버가 좀 느린 상황에서 유저가 공지를 광클한다던지.. 하면 행이 두 개씩 생길 수 있겠네요

그래서 엄밀해야하는 상황에서는 유저, 노티스 묶어서 Unique index를 걸고, @SQLInsert로 INSERT IGNORE 쿼리를 설정한다던가.. save 쿼리에 try-catch를 건다거나 해야합니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

이 코멘트에 대한 질문인데 ..! 서버가 좀 느린 상황에서 광클을 하면 똑같은 요청이 두세번 가게 되고 그에 대해 똑같은 NoticeRead객체가 저장될 수 있으니 까지는 이해했는데
그 아래에 대한 내용 설명해주실 수 있나요 ㅜ.ㅜ

Copy link
Contributor

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.

어우 이제봤네요 늦어져서 죄송합니담

만약에 그럼 Notice 생성 됐을 때 모든 멤버들에 대해 일괄적으로 NoticeRead 객체를 생성한다고 가정한다면,
읽음 여부가 boolean 필드로 관리가 될텐데 이럴 때는 광클 했을 때 동일한 이슈 안생기나요??
insert일 때만 타이밍 이슈가 치명적으로 발생하는건가요?

noticeReadRepository.save(
NoticeRead.builder().notice(notice).user(user).build()
);
}
log.info("success to read notice. userId: {}, noticeId: {}", user.getId(), notice.getId());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,13 @@
import com.gamzabat.algohub.feature.group.studygroup.repository.GroupMemberRepository;
import com.gamzabat.algohub.feature.group.studygroup.repository.StudyGroupRepository;
import com.gamzabat.algohub.feature.notice.domain.Notice;
import com.gamzabat.algohub.feature.notice.domain.NoticeRead;
import com.gamzabat.algohub.feature.notice.dto.CreateNoticeRequest;
import com.gamzabat.algohub.feature.notice.dto.GetNoticeResponse;
import com.gamzabat.algohub.feature.notice.dto.UpdateNoticeRequest;
import com.gamzabat.algohub.feature.notice.exception.NoticeValidationException;
import com.gamzabat.algohub.feature.notice.repository.NoticeCommentRepository;
import com.gamzabat.algohub.feature.notice.repository.NoticeReadRepository;
import com.gamzabat.algohub.feature.notice.repository.NoticeRepository;
import com.gamzabat.algohub.feature.notice.service.NoticeService;
import com.gamzabat.algohub.feature.user.domain.User;
Expand All @@ -52,6 +54,8 @@ public class NoticeServiceTest {
GroupMemberRepository groupMemberRepository;
@Mock
private NoticeCommentRepository noticeCommentRepository;
@Mock
private NoticeReadRepository noticeReadRepository;
@Captor
private ArgumentCaptor<Notice> noticeCaptor;

Expand Down Expand Up @@ -180,6 +184,7 @@ void getNoticeSuccess_1() {
assertThat(response.noticeTitle()).isEqualTo("title");
assertThat(response.createAt()).isEqualTo(DateFormatUtil.formatDate(LocalDateTime.now().toLocalDate()));
assertThat(response.noticeId()).isEqualTo(1000L);
verify(noticeReadRepository, times(1)).save(any(NoticeRead.class));
}

@Test
Expand Down Expand Up @@ -241,6 +246,7 @@ void getNoticeListSuccess_1() {
for (int i = 0; i < 20; i++) {
assertThat(result.get(i).noticeContent()).isEqualTo("content" + i);
assertThat(result.get(i).noticeTitle()).isEqualTo("title" + i);
assertThat(result.get(i).isRead()).isFalse();
}
}

Expand Down
Loading