feat: 소식지 좋아요 기능 구현 #377
feat: 소식지 좋아요 기능 구현 #377
Conversation
Gyuhyeok99
requested review from
lsy1307,
nayonsoso,
whqtker and
wibaek
as code owners
Walkthrough
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/test/java/com/example/solidconnection/news/service/NewsLikeServiceTest.java (1)
23-126: 테스트 코드가 포괄적으로 잘 작성되었습니다다음과 같은 장점들이 있습니다:
- 구조화된 테스트: 중첩 클래스로 기능별 테스트 그룹화
- 시나리오 완전성:
- 좋아요 상태 조회 (좋아요O/좋아요X)
- 좋아요 등록 (성공/이미 좋아요한 경우)
- 좋아요 취소 (성공/좋아요하지 않은 경우)
- 예외 처리 확인: 커스텀 예외와 에러 메시지 검증
- 데이터베이스 상태 검증: 실제 저장/삭제 확인
PR 목표에서 언급하신 "존재하지 않는 소식지" 테스트 케이스 추가를 고려해 보시기 바랍니다.
추가 고려사항:
@Test void 존재하지_않는_소식지에_좋아요_시도시_예외_발생() { // given long nonExistentNewsId = 999L; // when & then assertThatCode(() -> newsLikeService.addNewsLike(user.getId(), nonExistentNewsId)) .isInstanceOf(CustomException.class) .hasMessage(NEWS_NOT_FOUND.getMessage()); }src/main/java/com/example/solidconnection/news/service/NewsLikeService.java (1)
1-55: 전체적으로 견고한 서비스 구현이지만 테스트 케이스 추가를 고려해보세요!현재 구현이 매우 잘 되어 있지만, PR에서 언급한 대로 추가 테스트 케이스를 고려해볼 수 있습니다:
현재 잘 구현된 부분:
- 트랜잭션 경계 설정
- 예외 처리
- 비즈니스 로직 검증
고려해볼 추가 테스트:
- 존재하지 않는 뉴스에 대한 각 메서드 호출
- 동시성 상황에서의 중복 좋아요 처리
존재하지 않는 뉴스에 대한 테스트 케이스를 추가로 작성해드릴까요?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java(1 hunks)src/main/java/com/example/solidconnection/news/controller/NewsController.java(3 hunks)src/main/java/com/example/solidconnection/news/domain/LikedNews.java(1 hunks)src/main/java/com/example/solidconnection/news/dto/LikedNewsResponse.java(1 hunks)src/main/java/com/example/solidconnection/news/repository/LikedNewsRepository.java(1 hunks)src/main/java/com/example/solidconnection/news/service/NewsLikeService.java(1 hunks)src/test/java/com/example/solidconnection/news/service/NewsLikeServiceTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (10)
src/main/java/com/example/solidconnection/news/domain/LikedNews.java (1)
37-40: 생성자 중복 문제 확인이 필요합니다
@AllArgsConstructor가 이미 모든 필드를 포함하는 생성자를 생성하므로, 추가로 정의한 생성자와 중복될 수 있습니다.다음 중 하나를 선택하여 수정하는 것을 권장합니다:
@AllArgsConstructor제거 후 필요한 생성자만 직접 정의- 현재 추가한 생성자 제거 후
@AllArgsConstructor사용-@AllArgsConstructor @NoArgsConstructor(access = AccessLevel.PROTECTED) @Table(uniqueConstraints = { @UniqueConstraint( name = "uk_liked_news_site_user_id_news_id", columnNames = {"site_user_id", "news_id"} ) }) public class LikedNews { // ... 기존 필드들 ... public LikedNews(long newsId, long siteUserId) { this.newsId = newsId; this.siteUserId = siteUserId; } + + public LikedNews(Long id, long newsId, long siteUserId) { + this.id = id; + this.newsId = newsId; + this.siteUserId = siteUserId; + } }Likely an incorrect or invalid review comment.
src/main/java/com/example/solidconnection/common/exception/ErrorCode.java (1)
102-103: 에러 코드 추가가 적절합니다기존 대학 지원 정보 좋아요 기능의 에러 코드(
ALREADY_LIKED_UNIV_APPLY_INFO,NOT_LIKED_UNIV_APPLY_INFO)와 일관성을 유지하여 추가되었습니다. 메시지도 명확하고 HTTP 상태 코드도 적절합니다.src/main/java/com/example/solidconnection/news/dto/LikedNewsResponse.java (1)
3-11: DTO 구조가 깔끔합니다
- 레코드 클래스 사용으로 불변성 보장
- 정적 팩토리 메서드 제공으로 생성 방식 통일
- 필드명이 명확하고 직관적
PR 목표에서 언급하신 대로 응답 구조가 적절한지 확인해 보시기 바랍니다. 현재 구조는 ID와 좋아요 상태를 포함하고 있어 충분해 보입니다.
src/main/java/com/example/solidconnection/news/repository/LikedNewsRepository.java (1)
8-13: 리포지토리 구조가 최적화되어 있습니다PR 목표에서 언급하신 대로 JPA의
exists메서드를 활용하여 성능을 최적화했습니다:
existsByNewsIdAndSiteUserId- 불필요한 find 작업 없이 존재 여부만 확인findByNewsIdAndSiteUserId- 실제 엔티티가 필요한 경우에만 사용- Spring Data JPA 네이밍 컨벤션을 정확히 따름
이는 기존 대학 지원 정보 좋아요 기능 대비 성능 개선된 접근 방식입니다.
src/main/java/com/example/solidconnection/news/controller/NewsController.java (4)
4-4: 의존성 주입이 깔끔하게 구현되었습니다!새로운 좋아요 기능을 위한 의존성들이 적절히 추가되었습니다:
LikedNewsResponseDTO 임포트NewsLikeService서비스 임포트 및 주입기존 패턴과 일관성을 유지하면서 깔끔하게 구현되었네요.
Also applies to: 10-10, 36-36
84-91: 좋아요 상태 조회 엔드포인트가 잘 구현되었습니다!좋아요 상태를 조회하는 GET 엔드포인트가 다음과 같이 적절히 구현되었습니다:
- 적절한 HTTP 메서드 사용 (GET)
- 인증된 사용자 정보 활용
- 서비스 레이어로의 적절한 위임
기존 컨트롤러 패턴과 일관성을 유지하고 있어 좋습니다.
93-100: 좋아요 추가 엔드포인트가 적절히 구현되었습니다!좋아요를 추가하는 POST 엔드포인트가 다음과 같이 잘 구현되었습니다:
- 적절한 HTTP 메서드 사용 (POST)
- 인증된 사용자 정보 활용
- 서비스 레이어로의 적절한 위임
RESTful 원칙을 잘 따르고 있습니다.
102-109: 좋아요 취소 엔드포인트가 적절히 구현되었습니다!좋아요를 취소하는 DELETE 엔드포인트가 다음과 같이 잘 구현되었습니다:
- 적절한 HTTP 메서드 사용 (DELETE)
- 인증된 사용자 정보 활용
- 서비스 레이어로의 적절한 위임
전체적으로 RESTful API 설계 원칙을 잘 따르고 있습니다.
src/main/java/com/example/solidconnection/news/service/NewsLikeService.java (2)
1-22: 서비스 클래스 구조가 깔끔하게 설계되었습니다!새로운
NewsLikeService클래스가 다음과 같이 잘 구현되었습니다:
- 적절한 패키지 구조 및 임포트
- 의존성 주입을 위한
@RequiredArgsConstructor사용- 서비스 레이어 표시를 위한
@Service어노테이션- 필요한 리포지토리들의 명확한 주입
전체적으로 Spring의 모범 사례를 잘 따르고 있습니다.
23-30: 좋아요 상태 조회 메서드가 효율적으로 구현되었습니다!좋아요 상태를 조회하는 메서드가 다음과 같이 적절히 구현되었습니다:
@Transactional(readOnly = true)적용으로 읽기 전용 트랜잭션 최적화- PR에서 언급한 대로
existsById()사용으로 효율적인 존재 확인- 적절한 예외 처리 및 응답 생성
find대신exists메서드를 사용한 것은 성능상 좋은 선택입니다.
src/main/java/com/example/solidconnection/news/service/NewsLikeService.java
Show resolved
Hide resolved
src/main/java/com/example/solidconnection/news/service/NewsLikeService.java
Show resolved
Hide resolved
nayonsoso
left a comment
nayonsoso
left a comment
There was a problem hiding this comment.
깔끔하게 잘 구현하셨네요! 😊
회의에서 나왔던 응답 형식(200, OK)만 반영하시고, 멘션 주세요!
수고하셨습니다~ 🎉🎉
src/main/java/com/example/solidconnection/news/service/NewsLikeService.java
Outdated
Show resolved
Hide resolved
src/test/java/com/example/solidconnection/news/service/NewsLikeServiceTest.java
Show resolved
Hide resolved
whqtker
left a comment
whqtker
left a comment
There was a problem hiding this comment.
고생 많으셨습니다 ~!
- 저도 존재 여부 파악이 목적이라면
find보다exists가 성능 상 유리하다고 알고 있습니다. - 추후 대학 지원 좋아요 여부 확인 함수명을 '대학 지원 정보' 라는 정보가 들어가게끔 수정하면 좋을 거 같습니다 !
현재 post 엔티티에는 post_like 필드가 있어 필드 값으로 바로 게시글 좋아요 개수를 알 수 있습니다. news 엔티티에도 news_like 필드가 있으면 좋을 거 같습니다. 흠 ..... 근데 이 PR에 적절한 내용인지는 고민되네요.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/example/solidconnection/news/dto/LikedNewsResponse.java (1)
3-5: 필드명 개선을 위한 제안
- 문법적 정확성:
isLike보다는isLiked가 더 자연스러운 영어 표현입니다.
- 현재: "좋아요인가?" 보다는 "좋아요를 했는가?"가 더 적절합니다.
다음과 같이 수정하는 것을 제안합니다:
public record LikedNewsResponse( - boolean isLike + boolean isLiked ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/example/solidconnection/news/controller/NewsController.java(3 hunks)src/main/java/com/example/solidconnection/news/dto/LikedNewsResponse.java(1 hunks)src/main/java/com/example/solidconnection/news/service/NewsLikeService.java(1 hunks)src/test/java/com/example/solidconnection/news/service/NewsLikeServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/test/java/com/example/solidconnection/news/service/NewsLikeServiceTest.java
- src/main/java/com/example/solidconnection/news/service/NewsLikeService.java
- src/main/java/com/example/solidconnection/news/controller/NewsController.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
src/main/java/com/example/solidconnection/news/dto/LikedNewsResponse.java (1)
7-9: 팩토리 메서드 구현 확인
일관성 유지: 다른 DTO와의 일관성을 위해 팩토리 메서드를 제공하는 것은 좋은 접근입니다.
- 향후 확장성이나 추가 로직이 필요할 때 유용할 수 있습니다.
단순성: 현재는 단순한 생성자 호출이지만, 코드의 일관성 측면에서 적절합니다.
팩토리 메서드명도 필드명과 일치하도록 수정하는 것을 고려해보세요:
- public static LikedNewsResponse of(boolean isLike) { - return new LikedNewsResponse(isLike); + public static LikedNewsResponse of(boolean isLiked) { + return new LikedNewsResponse(isLiked);
저는 소식지의 경우에는 post보다는 대학지원정보와 더 비슷하다고 생각하여 구현하지 않았습니다! 혹시 기획에서 필요하다고 하면 다른 pr에서 작업하겠습니다!
좋습니다~~ |
There was a problem hiding this comment.
- 불필요한 find는 좋지 않다고 알고 있기에 기존 대학 지원 정보와 달리 JPA의 exist를 활용했는데 괜찮은가요?
좋습니다~ 👍
- 좋아요 상태 확인 함수명
status 는 전체 프로젝트 맥락 상(?) enum 으로 관리되는 무언가를 연상시키는 것 같습니다..!
예를 들어 verifyStatus / exchangeStatus 같은 것들이요.
그래소 boolean 응답임을 명시할 수 있는 isLiked가 포함되면 좋겠습니다.
(제가 그 함수 만든 사람이라 그렇게 생각하는걸수도 있어욧..)
src/test/java/com/example/solidconnection/news/service/NewsLikeServiceTest.java
Outdated
Show resolved
Hide resolved
lsy1307
left a comment
lsy1307
left a comment
There was a problem hiding this comment.
수고하셨습니다! 의문점만 한 가지 있는데 답변해주시면 감사하겠습니다!!
src/main/java/com/example/solidconnection/news/service/NewsLikeService.java
Show resolved
Hide resolved
4 participants
관련 이슈
작업 내용
소식지 좋아요 관련 기능을 추가했습니다!
기존 대학 지원 정보 좋아요 관련 기능을 최대한 참고하며 진행했습니다!
상태확인, 좋아요, 좋아요 취소에 대해 무얼 응답하면 좋을지에 대해 아직 확정이 나지 않아 우선은 모두 id와 좋아요중인지를 응답하는 방식으로 통일해두었습니다!
(사실 전 지금 방식도 괜찮은 거 같긴 한데 의견 남겨주시면 좋을 거 같습니다!)
특이 사항
소식지가 존재하지 않는 경우에 대해서는 따로 테스트 코드를 작성하지 않았습니다!
리뷰 요구사항 (선택)
1. 불필요한 find는 좋지 않다고 알고 있기에 기존 대학 지원 정보와 달리 JPA의 exist를 활용했는데 괜찮은가요?
2. 좋아요 상태 확인 함수명
getNewsLikeStatus로 일단 해두긴 했는데 뭐가 적절한지 모르겠네요 기존 대학 지원 정보에서는 getIsLiked로 되어있었긴 합니다!