[MERGE] dev -> main #409
[MERGE] dev -> main #409
Conversation
[Refactor/#405] 업무 뱃지 부여 및 기본 역할 새싹으로 수정
Walkthrough
Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as PotBadgeMemberController
participant BadgeService
participant TaskQuery as TaskQueryService
participant Repo as PotMemberBadgeRepository
Client->>Controller: POST /badges/{potId}/task-badge
Controller->>BadgeService: assignTaskBadgeToTopMembers(potId)
BadgeService->>TaskQuery: getTop2TaskCountByPotMemberId(potMemberIds)
TaskQuery-->>BadgeService: List<PotMember>(Top2)
loop for each Top2 member
BadgeService->>Repo: existsByBadgeAndPotMember(potMemberId, badgeId=2)
alt not exists
BadgeService->>Repo: saveAll(new PotMemberBadge)
end
end
BadgeService-->>Controller: void
Controller-->>Client: 200 ApiResponse<Void>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (7)
src/main/java/stackpot/stackpot/notification/repository/FeedCommentNotificationRepository.java (1)
24-24: JPQL 파라미터 바인딩에 @param 누락 — 런타임 바인딩 실패 가능메서드 파라미터에
@Param("userId")가 누락되어 런타임에서 바인딩 실패 가능성이 있습니다(컴파일 시 파라미터 이름 보존 설정이 없어도 주로 발생). 다른 리포지토리들과도 일관되게@Param을 추가해 주세요.- List<NotificationDto.UnReadNotificationDto> findAllUnReadNotificationsByUserId(Long userId); + List<NotificationDto.UnReadNotificationDto> findAllUnReadNotificationsByUserId(@Param("userId") Long userId);src/main/java/stackpot/stackpot/notification/service/NotificationCommandService.java (4)
59-71: 트랜잭션 누락 및 potId 신뢰성 확보 제안
- 저장(save) 동작이 포함되어 있어 메소드에 @transactional 부착을 권장합니다.
- 변환기에 전달하는 potId는 로드한 엔티티에서 유도하여(SSOT) 불일치 가능성을 제거하는 편이 안전합니다.
다음과 같이 수정을 제안합니다:
- public NotificationResponseDto.UnReadNotificationDto createPotApplicationNotification(Long potId, Long applicationId, String userName) { + @Transactional + public NotificationResponseDto.UnReadNotificationDto createPotApplicationNotification(Long potId, Long applicationId, String userName) { @@ - return notificationConverter.toUnReadNotificationDto( - newPan.getId(), potId, userName, "PotApplication", null, newPan.getCreatedAt()); + return notificationConverter.toUnReadNotificationDto( + newPan.getId(), + potApplication.getPot().getId(), + userName, + "PotApplication", + null, + newPan.getCreatedAt());추가 제안(선택): "PotApplication"과 같은 매직 스트링은 NotificationType 등 상수/열거형 기반으로 치환해 오타 리스크를 줄이는 것을 권장합니다.
78-96: 자기 행위 필터 로직의 ID 필드 불일치(getId vs getUserId)로 인한 오동작 가능성
potComment.getParent().getUser().getUserId()와 비교 대상userId(Long)의 타입/의미가 불일치할 여지가 큽니다. 상단 조건은getId()를 쓰고, 하단 조건은getUserId()를 쓰고 있어 동일 사용자 비교가 실패할 수 있습니다. 동일하게getId()로 정규화하는 것이 안전합니다. 또한 트랜잭션 부착 및 potId를 엔티티에서 유도하는 개선을 함께 제안합니다.- public NotificationResponseDto.UnReadNotificationDto createPotCommentNotification(Long potId, Long commentId, Long userId) { + @Transactional + public NotificationResponseDto.UnReadNotificationDto createPotCommentNotification(Long potId, Long commentId, Long userId) { @@ - if (potComment.getParent() != null && potComment.getParent().getUser().getUserId().equals(userId)) { + if (potComment.getParent() != null && potComment.getParent().getUser().getId().equals(userId)) { return null; // 해당 유저가 부모 댓글 생성자인 경우 알림 생성하지 않음 } @@ - return notificationConverter.toUnReadNotificationDto( - newPcn.getId(), potId, potComment.getUser().getNickname(), - "PotComment", potComment.getComment(), newPcn.getCreatedAt()); + return notificationConverter.toUnReadNotificationDto( + newPcn.getId(), + potComment.getPot().getId(), + potComment.getUser().getNickname(), + "PotComment", + potComment.getComment(), + newPcn.getCreatedAt());추가 제안(선택): 현재 “생성하지 않음” 케이스에 대해 null을 반환하고 있는데, 호출부에서 누락 처리 리스크가 있습니다. Optional로의 전환 또는 Result 타입 도입을 고려해 주세요.
103-117: feedId는 엔티티에서 유도하고, 트랜잭션을 부착하세요
- 저장 동작이 있으므로 @transactional 부착 권장
- converter에 전달하는 feedId는 feedLike로부터 유도하여 데이터 불일치 가능성을 제거
- public NotificationResponseDto.UnReadNotificationDto createFeedLikeNotification(Long feedId, Long feedLikeId, Long userId) { + @Transactional + public NotificationResponseDto.UnReadNotificationDto createFeedLikeNotification(Long feedId, Long feedLikeId, Long userId) { @@ - return notificationConverter.toUnReadNotificationDto( - newFln.getId(), feedId, feedLike.getUser().getNickname(), - "FeedLike", null, newFln.getCreatedAt()); + return notificationConverter.toUnReadNotificationDto( + newFln.getId(), + feedLike.getFeed().getId(), + feedLike.getUser().getNickname(), + "FeedLike", + null, + newFln.getCreatedAt());선택 제안: 본 메소드도 “생성하지 않음”에 대해 null을 반환합니다. 호출부 null 체크 누락을 방지하기 위해 Optional 반환으로의 변경을 고려해 주세요.
124-141: 메소드명 및 필드명 일관성 적용:createdFeedCommentNotification→createFeedCommentNotification메소드명을 팀 컨벤션에 맞춰 수정하고, 호출부 2곳도 함께 변경했습니다.
또한 부모 댓글 필터에서getUserId()→getId()로 통일하고,feedId는 엔티티에서 유도하도록 수정했습니다.수정 위치:
NotificationCommandService.javaFeedCommentCommandService.java(호출부 2건)--- a/src/main/java/stackpot/stackpot/notification/service/NotificationCommandService.java @@ - public NotificationResponseDto.UnReadNotificationDto createdFeedCommentNotification(Long feedId, Long commentId, Long userId) { + @Transactional + public NotificationResponseDto.UnReadNotificationDto createFeedCommentNotification(Long commentId, Long userId) { FeedComment feedComment = feedCommentQueryService.selectFeedCommentByCommentId(commentId); if (feedComment.getFeed().getUser().getId().equals(userId)) { return null; // 피드 생성자는 알림 생성하지 않음 } - if (feedComment.getParent() != null && feedComment.getParent().getUser().getUserId().equals(userId)) { + if (feedComment.getParent() != null + && feedComment.getParent().getUser().getId().equals(userId)) { return null; // 부모 댓글 작성자는 알림 생성하지 않음 } @@ - return notificationConverter.toUnReadNotificationDto( - newFcn.getId(), feedId, feedComment.getUser().getNickname(), + return notificationConverter.toUnReadNotificationDto( + newFcn.getId(), + feedComment.getFeed().getId(), + feedComment.getUser().getNickname(), "FeedComment", feedComment.getComment(), newFcn.getCreatedAt());--- a/src/main/java/stackpot/stackpot/feed/service/FeedCommentCommandService.java @@ - NotificationResponseDto.UnReadNotificationDto dto = notificationCommandService.createdFeedCommentNotification( - feedId, feedComment.getId(), user.getId()); + NotificationResponseDto.UnReadNotificationDto dto = notificationCommandService.createFeedCommentNotification( + feedComment.getId(), user.getId()); @@ - NotificationResponseDto.UnReadNotificationDto dto = notificationCommandService.createdFeedCommentNotification( - feedId, feedComment.getId(), user.getId()); + NotificationResponseDto.UnReadNotificationDto dto = notificationCommandService.createFeedCommentNotification( + feedComment.getId(), user.getId());src/main/java/stackpot/stackpot/badge/service/BadgeServiceImpl.java (1)
62-71: Todo 기반 배지 중복 부여 방지 누락PR 목적(중복 배지 부여 방지) 대비, Todo 기반 배지 부여(assignBadgeToTopMembers)에는 중복 체크가 없습니다. Task 기반 로직과 동일하게 중복을 제거하고 bulk save를 권장합니다.
다음과 같이 수정해 주세요:
- // 4. Todo 배지 부여 - Badge badge = getBadge(1L); - for (PotMember potMember : topPotMembers) { - PotMemberBadge potMemberBadge = PotMemberBadge.builder() - .badge(badge) - .potMember(potMember) - .build(); - potMemberBadgeRepository.save(potMemberBadge); - } + // 4. Todo 배지 부여 (중복 방지 + bulk save) + Badge badge = getBadge(1L); + List<PotMemberBadge> newBadges = topPotMembers.stream() + .filter(pm -> !potMemberBadgeRepository.existsByBadgeAndPotMember(pm.getPotMemberId(), badge.getBadgeId())) + .map(pm -> PotMemberBadge.builder().badge(badge).potMember(pm).build()) + .toList(); + if (!newBadges.isEmpty()) { + potMemberBadgeRepository.saveAll(newBadges); + }src/main/java/stackpot/stackpot/task/service/TaskCommandServiceImpl.java (1)
82-88: modifyTask의 참여자 조회가 팟 범위를 벗어날 수 있습니다현재
findAllById(requestedParticipantIds)는 다른 팟의 PotMember도 포함할 수 있어 데이터 무결성 문제가 발생합니다. createTask와 동일하게 potId로 스코프를 제한해 주세요.- List<Long> requestedParticipantIds = request.getParticipants() != null ? request.getParticipants() : List.of(); - List<PotMember> participants = potMemberRepository.findAllById(requestedParticipantIds); + List<Long> requestedParticipantIds = request.getParticipants() != null ? request.getParticipants() : List.of(); + List<PotMember> validParticipants = potMemberRepository.findByPotId(potId); + List<PotMember> participants = validParticipants.stream() + .filter(pm -> requestedParticipantIds.contains(pm.getPotMemberId())) + .collect(Collectors.toList());
♻️ Duplicate comments (1)
src/main/java/stackpot/stackpot/feed/service/FeedCommentCommandService.java (1)
76-76: 메서드 네이밍 일관성(중복 지적)앞선 코멘트와 동일 사안입니다. created → create 일원화 검토 바랍니다.
🧹 Nitpick comments (12)
src/main/java/stackpot/stackpot/pot/repository/PotCommentRepository.java (1)
17-20: 반환 순서 비결정성 — 일관된 정렬 기준 추가 제안현재 ORDER BY가 없어 DB/플랜에 따라 반환 순서가 달라질 수 있습니다. createdAt, id 기준으로 정렬을 추가하면 API 응답 일관성이 좋아집니다.
- "from PotComment pc where pc.pot.potId = :potId") + "from PotComment pc where pc.pot.potId = :potId order by pc.createdAt asc, pc.id asc")src/main/java/stackpot/stackpot/user/service/UserCommandServiceImpl.java (1)
246-246: 닉네임 응답에서 ' 새싹' 접미사 제거 — 컨슈머 영향 점검 필요Line 246에서 NicknameResponseDto에 접미사를 붙이지 않고 원본 닉네임만 반환하도록 변경되었습니다. 프론트/컨버터 레이어 등 다운스트림에서 표시명 구성 시 ‘ 새싹’ 접미사를 기대/추가하는지 확인해 주세요. 또한, 생성 루프가 요약 모델로부터 동일 문자열을 반복 반환할 경우 무한 루프가 될 수 있으니 최대 시도 횟수와 fallback 도입을 고려해 주세요.
src/main/java/stackpot/stackpot/pot/service/potComment/PotCommentCommandService.java (3)
52-52: 알림 시그니처 변경 반영 OK + 자기 자신에 대한 알림 방지 고려신규 시그니처 반영은 타당합니다. 다만 작성자가 Pot 소유자인 경우(자기 자신)에는 불필요한 알림/이벤트를 건너뛰는 것이 UX/비용 측면에서 바람직할 수 있습니다.
예시:
- NotificationResponseDto.UnReadNotificationDto dto = notificationCommandService.createPotCommentNotification( - potId, potComment.getId(), user.getId()); - - applicationEventPublisher.publishEvent(new PotCommentEvent(pot.getUser().getId(), null, dto)); + NotificationResponseDto.UnReadNotificationDto dto = null; + if (!Objects.equals(user.getUserId(), pot.getUser().getUserId())) { + dto = notificationCommandService.createPotCommentNotification(potId, potComment.getId(), user.getId()); + applicationEventPublisher.publishEvent(new PotCommentEvent(pot.getUser().getUserId(), null, dto)); + }
77-79: 대댓글 알림도 자기 자신 대상일 때는 생략 고려대댓글에서도 작성자와 수신자가 동일한 경우 알림 생성과 이벤트 발행을 생략하는 방어 로직을 제안합니다.
- NotificationResponseDto.UnReadNotificationDto dto = notificationCommandService.createPotCommentNotification( - potId, potComment.getId(), user.getId()); - - applicationEventPublisher.publishEvent(new PotCommentEvent(pot.getUser().getId(), parent.getUser().getUserId(), dto)); + NotificationResponseDto.UnReadNotificationDto dto = null; + if (!Objects.equals(user.getUserId(), pot.getUser().getUserId())) { + dto = notificationCommandService.createPotCommentNotification(potId, potComment.getId(), user.getId()); + applicationEventPublisher.publishEvent(new PotCommentEvent(pot.getUser().getUserId(), parent.getUser().getUserId(), dto)); + }작성자/수신자 동일 시 알림 생략이 제품 요구사항과 일치하는지 확인 부탁드립니다.
56-56: ID 접근자 혼용 여부 검토 권장본 파일 전반에서 user.getId()와 user.getUserId()가 혼용됩니다(예: Line 49, 74). 동일 식별자를 가리킨다면 하나의 접근자로 일원화하는 편이 가독성과 유지보수에 유리합니다.
src/main/java/stackpot/stackpot/task/service/TaskQueryService.java (1)
3-3: Task 서비스에서 PotMember 엔티티 직접 의존 — 모듈 경계 결합도 증가TaskQueryService가 pot.mapping 엔티티(PotMember)를 직접 반환/의존하면 모듈 간 결합도가 커지고 변경 전파 비용이 커집니다. 파라미터/반환을 ID 또는 전용 DTO/프로젝션으로 좁혀 의존성 역전을 유지하는 걸 권장합니다.
src/main/java/stackpot/stackpot/badge/repository/PotMemberBadgeRepository.java (1)
21-22: exists 조회는 파생 메서드가 더 간결/효율적입니다JPQL로
CASE WHEN count(...)를 사용하는 대신, 스프링 데이터의 파생 메서드existsBy...네이밍을 활용하면 더 간결하고 일부 DB에서 효율적으로 동작합니다. 또한 프로퍼티 경로를 명시하면 의미도 분명해집니다.아래와 같이 변경을 제안합니다.
- @Query("SELECT CASE WHEN count(b) > 0 THEN true ELSE false END FROM PotMemberBadge b WHERE b.potMember.potMemberId = :potMemberId AND b.badge.badgeId = :badgeId") - boolean existsByBadgeAndPotMember(@Param("potMemberId") Long potMemberId, @Param("badgeId") Long badgeId); + boolean existsByPotMember_PotMemberIdAndBadge_BadgeId(Long potMemberId, Long badgeId);src/main/java/stackpot/stackpot/badge/service/BadgeService.java (1)
8-8: assignTaskBadgeToTopMembers 반환 계약 개선 검토부여 결과가 불투명합니다. 할당된 대상 수나 ID 목록을 반환하면 호출자가 후속 처리/로깅을 하기 수월합니다. 최소한 예외/실패 케이스(BADGE_INSUFFICIENT_TOP_MEMBERS 등)에 대한 계약을 Javadoc으로 명시하세요.
src/main/java/stackpot/stackpot/badge/controller/PotBadgeMemberController.java (1)
37-47: 엔드포인트 경로 일관성 및 응답 설계 개선 제안
- 경로 일관성: 기존 POST 엔드포인트가 "/{potId}"이고 신규는 "/{potId}/task-badge"입니다. 혼란을 줄이기 위해 기존 엔드포인트도 "/{potId}/todo-badge"와 같이 의미가 드러나는 경로로 맞추는 것을 권장합니다. (호환성 영향 검토 필요)
- 성공 응답: 본문이 없는 성공이라면 200 OK + null 페이로드 대신 204 No Content가 의미적으로 더 적합합니다. 현재 전역 ApiResponse를 사용 중이라면 유지해도 무방하나, 장기적으로는 상태 코드로 표현하는 방식을 고려해 주세요.
- @PostMapping("/{potId}") + @PostMapping("/{potId}/todo-badge") - public ResponseEntity<ApiResponse<Void>> assignBadgeToTopMembers(@PathVariable Long potId) { + public ResponseEntity<ApiResponse<Void>> assignBadgeToTopMembers(@PathVariable Long potId) { badgeService.assignBadgeToTopMembers(potId); return ResponseEntity.ok(ApiResponse.onSuccess(null)); }src/main/java/stackpot/stackpot/task/service/TaskQueryServiceImpl.java (1)
75-75: 변환기 메서드 네이밍 일관성(toDto vs toDTO)본 파일 내에서 toDto와 toDTO가 혼용됩니다. 컨버터에서 하나로 통일해 주세요. (예: toDto)
src/main/java/stackpot/stackpot/badge/service/BadgeServiceImpl.java (2)
86-91: 스트림 수집은 toList()로 간결화 가능Java 16+ 환경이라면 Collectors.toList() 대신 toList() 사용으로 간결화 가능합니다.
- .collect(Collectors.toList()); + .toList();
63-64: 하드코딩된 배지 ID(1L, 2L) 상수화 또는 설정화 권장매직 넘버는 가독성과 유지보수성을 해칩니다. 의미 있는 상수(예: BadgeIds.TODO_TOP, BadgeIds.TASK_TOP)나 설정값(예: application.yml)으로 분리해 주세요.
Also applies to: 86-87
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
src/main/java/stackpot/stackpot/badge/controller/PotBadgeMemberController.java(1 hunks)src/main/java/stackpot/stackpot/badge/repository/PotMemberBadgeRepository.java(1 hunks)src/main/java/stackpot/stackpot/badge/service/BadgeService.java(1 hunks)src/main/java/stackpot/stackpot/badge/service/BadgeServiceImpl.java(4 hunks)src/main/java/stackpot/stackpot/feed/converter/FeedCommentConverter.java(2 hunks)src/main/java/stackpot/stackpot/feed/dto/FeedCommentDto.java(0 hunks)src/main/java/stackpot/stackpot/feed/dto/FeedCommentInfoDto.java(0 hunks)src/main/java/stackpot/stackpot/feed/dto/FeedCommentResponseDto.java(0 hunks)src/main/java/stackpot/stackpot/feed/repository/FeedCommentRepository.java(1 hunks)src/main/java/stackpot/stackpot/feed/service/FeedCommandServiceImpl.java(1 hunks)src/main/java/stackpot/stackpot/feed/service/FeedCommentCommandService.java(2 hunks)src/main/java/stackpot/stackpot/notification/converter/NotificationConverter.java(1 hunks)src/main/java/stackpot/stackpot/notification/dto/NotificationDto.java(0 hunks)src/main/java/stackpot/stackpot/notification/dto/NotificationResponseDto.java(0 hunks)src/main/java/stackpot/stackpot/notification/repository/FeedCommentNotificationRepository.java(1 hunks)src/main/java/stackpot/stackpot/notification/repository/FeedLikeNotificationRepository.java(1 hunks)src/main/java/stackpot/stackpot/notification/repository/PotApplicationNotificationRepository.java(1 hunks)src/main/java/stackpot/stackpot/notification/repository/PotCommentNotificationRepository.java(1 hunks)src/main/java/stackpot/stackpot/notification/service/NotificationCommandService.java(7 hunks)src/main/java/stackpot/stackpot/pot/converter/PotCommentConverter.java(2 hunks)src/main/java/stackpot/stackpot/pot/dto/PotCommentDto.java(0 hunks)src/main/java/stackpot/stackpot/pot/dto/PotCommentResponseDto.java(0 hunks)src/main/java/stackpot/stackpot/pot/repository/PotCommentRepository.java(1 hunks)src/main/java/stackpot/stackpot/pot/service/pot/PotCommandServiceImpl.java(0 hunks)src/main/java/stackpot/stackpot/pot/service/potApplication/PotApplicationCommandServiceImpl.java(1 hunks)src/main/java/stackpot/stackpot/pot/service/potComment/PotCommentCommandService.java(2 hunks)src/main/java/stackpot/stackpot/task/controller/TaskController.java(1 hunks)src/main/java/stackpot/stackpot/task/converter/TaskBoardConverter.java(1 hunks)src/main/java/stackpot/stackpot/task/entity/Taskboard.java(1 hunks)src/main/java/stackpot/stackpot/task/repository/TaskRepository.java(2 hunks)src/main/java/stackpot/stackpot/task/service/TaskCommandService.java(1 hunks)src/main/java/stackpot/stackpot/task/service/TaskCommandServiceImpl.java(3 hunks)src/main/java/stackpot/stackpot/task/service/TaskQueryService.java(2 hunks)src/main/java/stackpot/stackpot/task/service/TaskQueryServiceImpl.java(4 hunks)src/main/java/stackpot/stackpot/user/service/UserCommandServiceImpl.java(1 hunks)
💤 Files with no reviewable changes (8)
- src/main/java/stackpot/stackpot/pot/service/pot/PotCommandServiceImpl.java
- src/main/java/stackpot/stackpot/pot/dto/PotCommentDto.java
- src/main/java/stackpot/stackpot/notification/dto/NotificationDto.java
- src/main/java/stackpot/stackpot/notification/dto/NotificationResponseDto.java
- src/main/java/stackpot/stackpot/feed/dto/FeedCommentDto.java
- src/main/java/stackpot/stackpot/feed/dto/FeedCommentInfoDto.java
- src/main/java/stackpot/stackpot/pot/dto/PotCommentResponseDto.java
- src/main/java/stackpot/stackpot/feed/dto/FeedCommentResponseDto.java
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/main/java/stackpot/stackpot/notification/converter/NotificationConverter.java (1)
src/main/java/stackpot/stackpot/notification/dto/NotificationResponseDto.java (1)
NotificationResponseDto(11-25)
src/main/java/stackpot/stackpot/pot/converter/PotCommentConverter.java (1)
src/main/java/stackpot/stackpot/pot/dto/PotCommentResponseDto.java (1)
PotCommentResponseDto(12-64)
src/main/java/stackpot/stackpot/feed/converter/FeedCommentConverter.java (1)
src/main/java/stackpot/stackpot/feed/dto/FeedCommentResponseDto.java (1)
FeedCommentResponseDto(12-64)
🔇 Additional comments (24)
src/main/java/stackpot/stackpot/pot/repository/PotCommentRepository.java (1)
17-20: 프로젝션 인자 및 DTO 생성자 시그니처 일치 확인
- PotCommentDto.PotCommentInfoDto에 선언된 필드 순서/타입
(Long userId, String userName, Long potWriterId, Long commentId, String comment, Long parentId, LocalDateTime createdAt)
가 JPQLselect new … (pc.user.id, pc.user.nickname, pc.pot.user.id, pc.id, pc.comment, pc.parent.id, pc.createdAt)의 인자 순서·타입과 정확히 일치합니다.- Hibernate(및 주요 JPA 구현체)에서는 내부 클래스명에 ‘$’ 표기를 문제없이 지원하므로 별도 수정 불필요합니다.
src/main/java/stackpot/stackpot/pot/service/potApplication/PotApplicationCommandServiceImpl.java (1)
63-64: 알림 생성 API 시그니처 변경 반영 — OK
createPotApplicationNotification(potId, applicationId, nickname)로의 변경이 최신 시그니처와 일치합니다. 컨버터/DTO에서 역할(Role) 제거 흐름과도 일관됩니다.src/main/java/stackpot/stackpot/notification/repository/PotApplicationNotificationRepository.java (1)
15-20: JPQL 프로젝션 Role 제거 — OKUnread 알림 DTO 6-파라미터 시그니처에 맞춰 role 인자를 제거하고 nickname을 3번째 인자로 전달하도록 일관되게 수정되었습니다.
src/main/java/stackpot/stackpot/feed/service/FeedCommandServiceImpl.java (1)
213-215: 좋아요 알림 생성 API 시그니처 변경 반영 — OK
createFeedLikeNotification(feedId, likeId, userId)3-인자 호출로 업데이트되어 알림 레이어의 Role 제거 방향과 일치합니다.src/main/java/stackpot/stackpot/notification/repository/FeedCommentNotificationRepository.java (1)
15-18: JPQL 프로젝션 Role 제거 — OKUnread 알림 DTO 6-파라미터 형태에 맞춰 프로젝션이 갱신되었습니다. 타입/순서 모두 문제 없어 보입니다.
src/main/java/stackpot/stackpot/notification/repository/PotCommentNotificationRepository.java (1)
15-24: JPQL 프로젝션 Role 제거 — OKUnread 알림 DTO의 6-인자 시그니처에 맞춰 role 제거 및 nickname 전달로 정리되었습니다. WHERE 조건도 기존 로직을 유지합니다.
src/main/java/stackpot/stackpot/notification/repository/FeedLikeNotificationRepository.java (1)
16-21: JPQL 프로젝션 Role 제거 — OKUnread 알림 DTO 6-파라미터 시그니처에 맞는 프로젝션으로 수정되어 일관성이 확보되었습니다.
src/main/java/stackpot/stackpot/notification/converter/NotificationConverter.java (1)
29-29: toUnReadNotificationDto 신규 시그니처 일관성 확인 완료
- NotificationCommandService.java(69–71, 93–95, 114–116, 138–140) 내 총 4곳의 호출부가 모두 Role 파라미터 제거된 신규 시그니처(
Long notificationId, Long potOrFeedId, String userName, String type, String content, LocalDateTime createdAt)를 사용 중입니다.- ChatConverter.java에서의
getVegetable()호출은 채팅 메시지에 역할별 이모티콘을 붙이기 위한 의도된 기능으로, 레거시 포맷 잔존 이슈로 보이지 않습니다.추가 수정 없이 그대로 머지하셔도 됩니다.
src/main/java/stackpot/stackpot/pot/service/potComment/PotCommentCommandService.java (1)
56-57: toPotCommentCreateDto 시그니처 변경 반영 적절Role 제거 및 파라미터 순서 변경에 맞춰 호출부가 정확히 수정되었습니다.
src/main/java/stackpot/stackpot/feed/converter/FeedCommentConverter.java (2)
32-34: toFeedCommentCreateDto 시그니처 변경 반영 적절Role 제거 및 파라미터 재배치가 반영되었고, 반환 DTO 스키마와 일치합니다.
44-47: toFeedReplyCommentCreateDto 시그니처 변경 반영 적절대댓글 생성 DTO도 동일한 방식으로 정리되어 일관성이 확보되었습니다.
src/main/java/stackpot/stackpot/feed/service/FeedCommentCommandService.java (2)
55-56: toFeedCommentCreateDto 시그니처 변경 반영 적절호출부 파라미터 순서/타입이 컨버터와 정확히 일치합니다.
80-81: toFeedReplyCommentCreateDto 시그니처 변경 반영 적절대댓글 생성 호출부도 정확히 반영되었습니다.
src/main/java/stackpot/stackpot/pot/converter/PotCommentConverter.java (2)
18-19: toPotCommentCreateDto 시그니처 변경 반영 적절Role 제거 및 파라미터 순서 변경이 DTO 설계와 일치합니다.
30-33: toPotReplyCommentCreateDto 시그니처 변경 반영 적절대댓글 생성 DTO도 동일하게 일관적으로 반영되었습니다.
src/main/java/stackpot/stackpot/task/controller/TaskController.java (1)
42-42: 메서드명 오탈자 수정 완료 —creatTask호출 제거 확인 LGTM
rg -n -C1 'creatTask\s*\('실행 결과, 리포지토리 전역에 더 이상creatTask(호출이 존재하지 않음을 확인했습니다.src/main/java/stackpot/stackpot/task/service/TaskCommandService.java (1)
11-11: creatTask → createTask 리네임 완료 및 잔존 참조 없음 확인
스크립트 검증 결과, 기존 ‘creatTask’ 메서드는 더 이상 사용되지 않으며, 인터페이스·구현체·컨트롤러 호출부 모두 새 시그니처를 정상 반영했습니다. 변경 사항 승인합니다.
- 인터페이스: src/main/java/stackpot/stackpot/task/service/TaskCommandService.java (createTask 선언)
- 구현체: src/main/java/stackpot/stackpot/task/service/TaskCommandServiceImpl.java (createTask 오버라이드)
- 호출부: src/main/java/stackpot/stackpot/task/controller/TaskController.java (taskCommandService.createTask 호출)
src/main/java/stackpot/stackpot/task/entity/Taskboard.java (1)
42-45: Taskboard–User 연관관계: LAZY 전환 시LazyInitializationException주의 & 불변성 반영
- File:
src/main/java/stackpot/stackpot/task/entity/Taskboard.java(Lines 42–45)- 현재
@ManyToOne(fetch = FetchType.LAZY)로 설정되어 있어, 서비스 레이어 트랜잭션 경계 밖에서taskboard.getUser()호출 시LazyInitializationException이 발생할 수 있습니다.- HQL/JPQL 쿼리에서
fetch join user를 사용하거나, DTO 변환 로직이 반드시 트랜잭션 내에서 실행되는지 직접 검증해 주세요.
확인 스크립트(출력이 없는 상태):rg -n -A2 -B2 '(from|join)\s+Taskboard|Taskboard\s+tb|join\s+fetch\s+.*user'- 클래스 레벨
@Setter로 인해user필드에도 setter가 남아 있습니다. 불변성을 지키려면 아래와 같이 해당 필드에만 setter 생성을 차단하세요.- @ManyToOne(fetch = FetchType.LAZY) + @Setter(AccessLevel.NONE) + @ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "user_id", nullable = false) private User user;위 사항을 수동으로 확인 후 반영해 주세요.
src/main/java/stackpot/stackpot/badge/controller/PotBadgeMemberController.java (1)
49-59: 신규 업무 기반 배지 부여 엔드포인트 추가는 적절합니다에러 코드 정의와 책임 위임(서비스 계층) 모두 적절합니다. 서비스 측 중복 부여 방지 로직과 매칭됩니다.
src/main/java/stackpot/stackpot/task/service/TaskQueryServiceImpl.java (1)
57-57: ✅ Java 17 Toolchain 구성 확인됨 – Stream.toList() 사용 가능
프로젝트의build.gradle에서java { toolchain { languageVersion = JavaLanguageVersion.of(17) } }로 설정되어 있어 Java 16+ 기능인Stream.toList()를 안전하게 사용하실 수 있습니다.src/main/java/stackpot/stackpot/badge/service/BadgeServiceImpl.java (2)
36-39: getBadge(Long) 예외 처리 방식 적절존재하지 않는 배지에 대해 명확히 BADGE_NOT_FOUND를 던지는 구현이 적절합니다.
88-89: existsByBadgeAndPotMember 파라미터 순서 검증 완료
리포지토리 메서드는 @query에서 이름 기반(named) 파라미터(:potMemberId,:badgeId)를 사용하므로, 호출 시 순서와 무관하게 올바르게 바인딩됩니다. 현재 코드(existsByBadgeAndPotMember(pm.getPotMemberId(), badge.getBadgeId()))는 정확히 매칭됩니다.src/main/java/stackpot/stackpot/task/service/TaskCommandServiceImpl.java (2)
48-48: 사용자 컨텍스트를 컨버터로 전달하는 변경은 타당합니다Taskboard 생성 시 User를 즉시 바인딩해 별도 set 호출을 제거한 점이 좋습니다.
118-126: createAndSaveTasks의 저장 방식은 적절합니다빌더 사용과 saveAll 호출 흐름이 명확합니다. 상위 호출부에서 빈 목록일 때 호출을 건너뛰는 최적화가 이미 있으므로 성능/안전성 문제 없습니다.
|
|
||
| public interface BadgeService { | ||
| Badge getDefaultBadge(); | ||
| Badge getBadge(Long badgeId); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
getDefaultBadge → getBadge(badgeId) 전환 — 호출부 매직넘버 제거/캡슐화 권장
호출부에서 하드코딩된 ID(예: 1L/2L)를 넘기기보다, 상수/enum(BadgeType 등)로 캡슐화해 의미를 명확히 하세요. 또한 getDefaultBadge() 사용처가 남아있지 않은지 확인 필요합니다.
아래로 잔존 getDefaultBadge 호출을 확인하세요.
🏁 Script executed:
#!/bin/bash
rg -n -C1 $'getDefaultBadge\\s*\\('Length of output: 34
잔존하는 getDefaultBadge 호출 혹은 정의가 완전히 제거됐는지 추가로 확인해 주세요.
🏁 Script executed:
#!/bin/bash
rg -n 'getDefaultBadge'Length of output: 23
getDefaultBadge 호출 완전 제거 확인 및 매직넘버 사용 여부 추가 검증 필요
getDefaultBadge 관련 호출·정의는 코드베이스 전반에서 모두 제거된 것을 확인했습니다.
다음으로, 여전히 getBadge(1L/2L…) 형태로 숫자 리터럴이 사용되고 있지 않은지 검증해 주세요.
🏁 Script executed:
#!/bin/bash
rg -n 'getBadge\s*\(\s*\d+L'Length of output: 244
1L/2L 매직 넘버 상수·enum으로 캡슐화 필요
현재 여전히 하드코딩된 배지 ID를 직접 넘기고 있어 의미가 불명확합니다. 아래 위치를 상수 혹은 BadgeType 등의 enum으로 대체해 가독성과 유지보수성을 높여주세요.
• src/main/java/stackpot/stackpot/badge/service/BadgeServiceImpl.java:63
• src/main/java/stackpot/stackpot/badge/service/BadgeServiceImpl.java:86
예시 수정안:
- Badge badge = getBadge(1L);
+ Badge badge = getBadge(BadgeType.DEFAULT.getId());
- Badge badge = getBadge(2L);
+ Badge badge = getBadge(BadgeType.SPECIAL.getId());또는 메서드 시그니처를
Badge getBadge(BadgeType type);처럼 변경해 호출부에서 getBadge(BadgeType.DEFAULT)로 바로 사용하도록 리팩토링할 수도 있습니다.
🤖 Prompt for AI Agents
In src/main/java/stackpot/stackpot/badge/service/BadgeService.java around line 6
and in src/main/java/stackpot/stackpot/badge/service/BadgeServiceImpl.java at
lines 63 and 86, replace the hardcoded badge ID magic numbers (1L/2L) with a
clear encapsulation: add a BadgeType enum (e.g., DEFAULT, SPECIAL) or public
constants in BadgeService, update the method signature to Badge
getBadge(BadgeType type) (or keep getBadge(Long id) but use constants), change
the implementation to map BadgeType to the corresponding ID and update all
callers to pass BadgeType.DEFAULT / BadgeType.SPECIAL (or the new constants),
and remove any direct 1L/2L usages so that badge meanings are explicit and
maintainable.
| .userId(dto.getUserId()) | ||
| .userName(dto.getUserName() + " " + dto.getRole().getVegetable()) | ||
| .role(dto.getRole()) | ||
| .userName(dto.getUserName() + " 새싹") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
" 새싹" 접미사 하드코딩 중복 — 공통 유틸 사용으로 일원화 권장
Notification/Pot/Feed 컨버터 전반에 동일한 접미사가 반복됩니다. 공통 유틸(UserDisplayName)로 치환해 중복/국제화 이슈를 해소하세요.
치환 예시:
- .userName(dto.getUserName() + " 새싹")
+ .userName(UserDisplayName.formatSeedling(dto.getUserName()))- .userName(userName + " 새싹")
+ .userName(UserDisplayName.formatSeedling(userName))- .userName(userName + " 새싹")
+ .userName(UserDisplayName.formatSeedling(userName))(별도 유틸 클래스는 NotificationConverter 코멘트를 참고하세요. import 추가 필요)
Also applies to: 36-36, 49-49
🤖 Prompt for AI Agents
In src/main/java/stackpot/stackpot/feed/converter/FeedCommentConverter.java
around lines 21, 36 and 49, replace the hardcoded " 새싹" string concatenations
with a shared UserDisplayName utility call (e.g., a method that appends the
seedling suffix) and add the corresponding import; update each occurrence to
call the utility (instead of dto.getUserName() + " 새싹") so the suffix logic is
centralized and internationalization/duplication is avoided.
| @Query("select new stackpot.stackpot.feed.dto.FeedCommentDto$FeedCommentInfoDto(fc.user.id, fc.user.nickname, " + | ||
| "fc.feed.user.id, fc.id, fc.comment, fc.parent.id, fc.createdAt) " + | ||
| "from FeedComment fc where fc.feed.feedId = :feedId") | ||
| List<FeedCommentDto.FeedCommentInfoDto> findAllCommentInfoDtoByFeedId(@Param("feedId") Long feedId); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
fc.parent.id 사용으로 인한 암묵적 inner join — 상위 댓글이 누락됩니다
JPQL select 절에서 fc.parent.id를 참조하면 암묵적 inner join이 발생하여 parent가 null인 상위 댓글(루트 댓글)이 결과에서 제외됩니다. left join으로 수정해 주세요.
- @Query("select new stackpot.stackpot.feed.dto.FeedCommentDto$FeedCommentInfoDto(fc.user.id, fc.user.nickname, " +
- "fc.feed.user.id, fc.id, fc.comment, fc.parent.id, fc.createdAt) " +
- "from FeedComment fc where fc.feed.feedId = :feedId")
+ @Query("select new stackpot.stackpot.feed.dto.FeedCommentDto$FeedCommentInfoDto(fc.user.id, fc.user.nickname, " +
+ "fc.feed.user.id, fc.id, fc.comment, p.id, fc.createdAt) " +
+ "from FeedComment fc left join fc.parent p where fc.feed.feedId = :feedId")🤖 Prompt for AI Agents
In src/main/java/stackpot/stackpot/feed/repository/FeedCommentRepository.java
around lines 17 to 20, the JPQL references fc.parent.id which causes an implicit
inner join and drops root comments with null parent; change the query to perform
an explicit LEFT JOIN on fc.parent (e.g., left join fc.parent p) and select p.id
(instead of fc.parent.id) so comments with null parent are included; update the
select expression to use the joined alias and keep the rest of the projection
and method signature unchanged.
|
|
||
| NotificationResponseDto.UnReadNotificationDto dto = notificationCommandService.createdFeedCommentNotification( | ||
| feedId, feedComment.getId(), user.getId(), user.getRole()); | ||
| feedId, feedComment.getId(), user.getId()); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
메서드 네이밍 일관성: createdFeedCommentNotification → createFeedCommentNotification 고려
Pot 쪽은 createPotCommentNotification, 여기만 createdFeedCommentNotification입니다. 네이밍을 create*로 일원화하면 API 일관성이 좋아집니다.
레포지토리 전역에서 두 네이밍의 혼재 여부를 확인하려면:
🏁 Script executed:
#!/bin/bash
rg -n "createdFeedCommentNotification\(" -A 1
rg -n "createFeedCommentNotification\(" -A 1
rg -n "createPotCommentNotification\(" -A 1Length of output: 2337
메서드 네이밍 통일: createdFeedCommentNotification → createFeedCommentNotification
Feed 쪽은 createdFeedCommentNotification, Pot 쪽은 createPotCommentNotification을 사용 중이라 네이밍 스타일이 일관되지 않습니다. 두 기능 모두 create* 형태로 통일해 API 일관성을 높여주세요.
- 변경 대상 파일 및 위치
src/main/java/stackpot/stackpot/notification/service/NotificationCommandService.java(124~125행): 메서드 선언부src/main/java/stackpot/stackpot/feed/service/FeedCommentCommandService.java(5051, 7576행): 호출부
// NotificationCommandService.java
- public NotificationResponseDto.UnReadNotificationDto createdFeedCommentNotification(Long feedId, Long commentId, Long userId) {
+ public NotificationResponseDto.UnReadNotificationDto createFeedCommentNotification(Long feedId, Long commentId, Long userId) {
// …
}
// FeedCommentCommandService.java
- notificationCommandService.createdFeedCommentNotification(feedId, feedComment.getId(), user.getId());
+ notificationCommandService.createFeedCommentNotification(feedId, feedComment.getId(), user.getId());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| feedId, feedComment.getId(), user.getId()); | |
| notificationCommandService.createFeedCommentNotification(feedId, feedComment.getId(), user.getId()); |
🤖 Prompt for AI Agents
In
src/main/java/stackpot/stackpot/notification/service/NotificationCommandService.java
around lines 124-125 and in
src/main/java/stackpot/stackpot/feed/service/FeedCommentCommandService.java
around lines 50-51 and 75-76, the notification method is inconsistently named
`createdFeedCommentNotification`; rename the method declaration to
`createFeedCommentNotification` and update all call sites in
FeedCommentCommandService to call `createFeedCommentNotification` instead of
`createdFeedCommentNotification`; ensure method signature, visibility and
imports remain unchanged and run a build to fix any remaining references.
| .potOrFeedId(unReadNotificationDto.getPotOrFeedId()) | ||
| .role(unReadNotificationDto.getRole()) | ||
| .userName(unReadNotificationDto.getUserName() + " " + unReadNotificationDto.getRole().getVegetable()) | ||
| .userName(unReadNotificationDto.getUserName() + " 새싹") |
There was a problem hiding this comment.
🛠️ Refactor suggestion
하드코딩된 " 새싹" 접미사 중복 — 공통 유틸로 추출 권장
여러 컨버터에서 동일한 접미사를 직접 더하고 있어 변경/국제화(i18n) 시 관리가 어렵습니다. 공통 유틸로 추출해 단일 책임으로 관리하세요.
적용 제안(해당 라인 치환):
- .userName(unReadNotificationDto.getUserName() + " 새싹")
+ .userName(UserDisplayName.formatSeedling(unReadNotificationDto.getUserName()))- .userName(userName + " 새싹")
+ .userName(UserDisplayName.formatSeedling(userName))유틸 클래스 예시(새 파일):
package stackpot.stackpot.common.util;
public final class UserDisplayName {
private static final String SEEDLING_SUFFIX = " 새싹";
private UserDisplayName() {}
public static String formatSeedling(String userName) {
return userName + SEEDLING_SUFFIX;
}
}각 파일 상단에 import 추가:
- import stackpot.stackpot.common.util.UserDisplayName;
Also applies to: 33-33
🤖 Prompt for AI Agents
In
src/main/java/stackpot/stackpot/notification/converter/NotificationConverter.java
around lines 21 and 33, multiple places concatenate the hardcoded suffix " 새싹"
to user names; extract this into a single utility to avoid duplication and ease
future i18n/changes. Create a new util class (e.g.,
stackpot.stackpot.common.util.UserDisplayName) with a private constant for the
suffix and a public static method formatSeedling(String userName) that returns
userName + SUFFIX, add the import in NotificationConverter, and replace both
occurrences of userName + " 새싹" with UserDisplayName.formatSeedling(userName).
| public Taskboard toTaskboard(Pot pot, MyPotTaskRequestDto.create request, User user) { | ||
| return Taskboard.builder() | ||
| .title(requset.getTitle()) | ||
| .description(requset.getDescription()) | ||
| .deadLine(requset.getDeadline()) | ||
| .status(requset.getTaskboardStatus()) | ||
| .title(request.getTitle()) | ||
| .description(request.getDescription()) | ||
| .deadLine(request.getDeadline()) | ||
| .status(request.getTaskboardStatus()) | ||
| .pot(pot) | ||
| .user(user) | ||
| .build(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Taskboard 생성 시 user null 방지 — 즉시 검증 추가 권장
@JoinColumn(nullable = false) 제약이 있어도 user가 null이면 영속화 시점에야 실패합니다. 컨버터에서 즉시 검증해 조기 실패하도록 하세요.
public class TaskBoardConverter {
public Taskboard toTaskboard(Pot pot, MyPotTaskRequestDto.create request, User user) {
+ java.util.Objects.requireNonNull(user, "user must not be null");
return Taskboard.builder()
.title(request.getTitle())
.description(request.getDescription())
.deadLine(request.getDeadline())
.status(request.getTaskboardStatus())
.pot(pot)
.user(user)
.build();
}또는 Objects.requireNonNull을 사용하려면 import를 추가하세요:
import java.util.Objects;🤖 Prompt for AI Agents
In src/main/java/stackpot/stackpot/task/converter/TaskBoardConverter.java around
lines 26 to 34, the converter builds a Taskboard with user possibly null which
defers failure to persistence; add an immediate null-check for user and fail
fast. Use Objects.requireNonNull(user, "user must not be null") before calling
the builder (and add import java.util.Objects), or explicitly if (user == null)
throw new IllegalArgumentException("user must not be null"); then proceed to
build the Taskboard.
| @Query("SELECT t.potMember " + | ||
| "FROM Task t " + | ||
| "WHERE t.potMember.potMemberId IN :potMemberIds " + | ||
| "GROUP BY t.potMember.potMemberId " + | ||
| "ORDER BY count(t) DESC") | ||
| List<PotMember> getTop2TaskCountByPotMemberId(@Param("potMemberIds") List<Long> potMemberIds, Pageable pageable); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
동점 정렬 기준 부재 — 결과 안정성 위해 보조 정렬 추가 권장
ORDER BY count(t) DESC만으로는 동점 시 결과가 비결정적일 수 있습니다. 식별자 보조 정렬을 추가해 안정성을 확보하세요. 또한 빈 potMemberIds 전달 시 IN () 문제가 생길 수 있으니 서비스에서 사전 가드하시기 바랍니다.
아래처럼 정렬을 보강하세요.
- "ORDER BY count(t) DESC")
+ "ORDER BY count(t) DESC, t.potMember.potMemberId ASC")추가 제안:
- 반환을
PotMember엔티티 대신Long(potMemberId) 또는 프로젝션으로 축소하면 상위 레이어 결합도와 N+1 가능성을 줄일 수 있습니다.
빈 리스트 가드를 누락했는지 확인하려면 서비스 구현에서 해당 메서드 호출부를 점검하세요.
🏁 Script executed:
#!/bin/bash
# TaskQueryServiceImpl 등에서 getTop2TaskCountByPotMemberId 호출부 확인
rg -n -A2 -B2 $'getTop2TaskCountByPotMemberId\\s*\\('Length of output: 2425
보조 정렬 및 빈 입력 컬렉션 가드 추가 권장
ORDER BY count(t) DESC 만으로는 동점 발생 시 정렬 순서가 비결정적일 수 있습니다. 또한, potMemberIds가 빈 리스트일 경우 JPQL의 IN () 절이 SQL 구문 오류나 예기치 않은 동작을 일으킬 수 있으니 호출부에서 가드 처리하세요.
검토 및 수정 대상:
src/main/java/stackpot/stackpot/task/repository/TaskRepository.javasrc/main/java/stackpot/stackpot/task/service/TaskQueryServiceImpl.java
수정 예시:
// TaskRepository.java
- @Query("SELECT t.potMember " +
- "FROM Task t " +
- "WHERE t.potMember.potMemberId IN :potMemberIds " +
- "GROUP BY t.potMember.potMemberId " +
- "ORDER BY count(t) DESC")
+ @Query("SELECT t.potMember " +
+ "FROM Task t " +
+ "WHERE t.potMember.potMemberId IN :potMemberIds " +
+ "GROUP BY t.potMember.potMemberId " +
+ "ORDER BY count(t) DESC, t.potMember.potMemberId ASC")
List<PotMember> getTop2TaskCountByPotMemberId(@Param("potMemberIds") List<Long> potMemberIds, Pageable pageable);// TaskQueryServiceImpl.java
@Override
public List<PotMember> getTop2TaskCountByPotMemberId(List<Long> potMemberIds) {
if (potMemberIds == null || potMemberIds.isEmpty()) {
return Collections.emptyList(); // 빈 리스트 가드
}
Pageable pageable = PageRequest.of(0, 2);
return taskRepository.getTop2TaskCountByPotMemberId(potMemberIds, pageable);
}추가 제안:
- 결과로
PotMember전체 엔티티가 아닌Long(potMemberId) 또는 DTO/프로젝션을 반환해 상위 계층 결합도를 낮추고 N+1 가능성을 줄이세요.
🤖 Prompt for AI Agents
In src/main/java/stackpot/stackpot/task/repository/TaskRepository.java around
lines 29-34 and in
src/main/java/stackpot/stackpot/task/service/TaskQueryServiceImpl.java, add a
deterministic secondary sort to the JPQL query (e.g. ORDER BY count(t) DESC,
t.potMember.potMemberId ASC) to avoid non-deterministic ordering on ties, and in
TaskQueryServiceImpl guard against null/empty potMemberIds by returning
Collections.emptyList() (or short-circuiting before calling the repository) when
the input list is null or empty; optionally consider changing the repository
return type to a projection/ID (Long) or DTO to reduce coupling and N+1 risks.
|
|
||
| @Override | ||
| public MyPotTaskResponseDto creatTask(Long potId, MyPotTaskRequestDto.create request) { | ||
| public MyPotTaskResponseDto createTask(Long potId, MyPotTaskRequestDto.create request) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
createTask 트랜잭션 경계 추가 필요
Taskboard 생성과 Task 일괄 저장을 하나의 원자적 작업으로 보장하려면 @transactional을 권장합니다. modify/delete는 이미 트랜잭션입니다.
- public MyPotTaskResponseDto createTask(Long potId, MyPotTaskRequestDto.create request) {
+ @Transactional
+ public MyPotTaskResponseDto createTask(Long potId, MyPotTaskRequestDto.create request) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public MyPotTaskResponseDto createTask(Long potId, MyPotTaskRequestDto.create request) { | |
| @Transactional | |
| public MyPotTaskResponseDto createTask(Long potId, MyPotTaskRequestDto.create request) { |
🤖 Prompt for AI Agents
In src/main/java/stackpot/stackpot/task/service/TaskCommandServiceImpl.java
around line 39, the createTask method lacks a transactional boundary so
Taskboard creation and bulk Task saves are not atomic; annotate the method (or
the service class) with @Transactional, import
org.springframework.transaction.annotation.Transactional, and ensure the service
is a Spring-managed bean so the annotation is applied; if you need specific
behavior, set propagation/isolation attributes accordingly and keep
modify/delete methods unchanged.
| MyPotTaskResponseDto viewDetailTask(Long potId, Long taskBoardId); | ||
| List<MyPotTaskPreViewResponseDto> getTasksFromDate(Long potId, LocalDate date); | ||
| List<MonthlyTaskDto> getMonthlyTasks(Long potId, int year, int month); | ||
| List<PotMember> getTop2TaskCountByPotMemberId(List<Long> potMemberIds); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Top-2 하드코딩 및 엔티티 반환 시그니처 — 확장성과 경계 분리를 위해 재설계 권장
- 메서드명/계약에 top-2가 박혀 있어 재사용성이 떨어집니다. limit 또는 Pageable을 인자로 받도록 일반화하세요.
- 서비스 계층에서 엔티티(PotMember)를 반환하면 상위 레이어 및 타 모듈과의 결합도가 올라갑니다. ID나 전용 DTO/프로젝션으로 축소 반환을 권장합니다.
- potMemberIds가 빈 리스트일 때 JPA IN () 문법 문제가 발생할 수 있으니 서비스 레이어에서 사전 가드(early return)도 필요합니다.
아래처럼 시그니처를 일반화하고 반환 타입을 ID로 축소하는 방안을 제안합니다.
- List<PotMember> getTop2TaskCountByPotMemberId(List<Long> potMemberIds);
+ List<Long> getTopTaskContributors(List<Long> potMemberIds, int limit);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| List<PotMember> getTop2TaskCountByPotMemberId(List<Long> potMemberIds); | |
| // src/main/java/stackpot/stackpot/task/service/TaskQueryService.java | |
| - List<PotMember> getTop2TaskCountByPotMemberId(List<Long> potMemberIds); | |
| + List<Long> getTopTaskContributors(List<Long> potMemberIds, int limit); |
🤖 Prompt for AI Agents
In src/main/java/stackpot/stackpot/task/service/TaskQueryService.java around
line 16, the method is hardcoded to "top-2" and returns PotMember entities which
tightens coupling and risks JPA IN() errors for empty lists; change the method
signature to accept a limit (int limit) or a Pageable and return lightweight
results (List<Long> ids or a DTO/projection) instead of PotMember entities, add
an early-return guard when potMemberIds is null or empty to avoid IN() with
empty collections, and update the implementation and all callers to use the new
signature and return type.
| @Override | ||
| public List<PotMember> getTop2TaskCountByPotMemberId(List<Long> potMemberIds) { | ||
| Pageable pageable = PageRequest.of(0, 2); | ||
| return taskRepository.getTop2TaskCountByPotMemberId(potMemberIds, pageable); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
상위 2명 조회 쿼리의 정렬·동률 처리 및 반환 일관성 검증 권장
PageRequest.of(0, 2)로 제한하는 것은 적절합니다. 다만 Repository 쿼리가 다음을 보장하는지 확인 바랍니다:
- 작업 수 내림차순 정렬
- 동률 시 결정적(second key) 정렬 기준 존재(예: potMemberId ASC)
- PotMember 중복 제거(distinct)
검증 후 필요 시 정렬 키를 보강해 주세요.
다음 스크립트로 Repository 메서드 정의와 JPQL/정렬을 확인해 주세요:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -A 8 -B 4 "getTop2TaskCountByPotMemberId" src | sed -n '1,200p'Length of output: 4191
상위 2명 조회 쿼리 정렬·동률 처리 및 반환 일관성 보강 필요
PageRequest.of(0, 2)로 제한하는 것은 적절하지만, 현재 Repository의 JPQL은 ORDER BY count(t) DESC만 지정되어 있어 동률(member 간 작업 수가 같은 경우) 시 결과 순서가 예측 불가합니다. 또한, 그룹핑된 결과이긴 하나 중복 여부를 명시적으로 보장하면 더 명확합니다.
– TaskRepository#getTop2TaskCountByPotMemberId 메서드 JPQL 확인
@Query(
"SELECT t.potMember " +
"FROM Task t " +
"WHERE t.potMember.potMemberId IN :potMemberIds " +
"GROUP BY t.potMember.potMemberId " +
"ORDER BY count(t) DESC"
)
List<PotMember> getTop2TaskCountByPotMemberId(@Param("potMemberIds") List<Long> potMemberIds, Pageable pageable);– 개선 제안
• 동률 시 결정적 정렬 키 추가 (예: potMemberId ASC)
• 명시적 DISTINCT 추가로 중복 제거 보강
@Query(
- "SELECT t.potMember " +
- "FROM Task t " +
- "WHERE t.potMember.potMemberId IN :potMemberIds " +
- "GROUP BY t.potMember.potMemberId " +
- "ORDER BY count(t) DESC"
+ "SELECT DISTINCT t.potMember " +
+ "FROM Task t " +
+ "WHERE t.potMember.potMemberId IN :potMemberIds " +
+ "GROUP BY t.potMember.potMemberId " +
+ "ORDER BY count(t) DESC, t.potMember.potMemberId ASC"
)
List<PotMember> getTop2TaskCountByPotMemberId(@Param("potMemberIds") List<Long> potMemberIds, Pageable pageable);🤖 Prompt for AI Agents
In src/main/java/stackpot/stackpot/task/service/TaskQueryServiceImpl.java around
lines 132 to 136 (and the corresponding TaskRepository method), the current JPQL
orders only by count(t) DESC which yields non-deterministic ties and does not
explicitly deduplicate; update the repository query to select distinct
potMember, include a deterministic tiebreaker, and keep the pageable: change
JPQL to "SELECT DISTINCT t.potMember FROM Task t WHERE t.potMember.potMemberId
IN :potMemberIds GROUP BY t.potMember.potMemberId ORDER BY count(t) DESC,
t.potMember.potMemberId ASC" so tied counts are ordered by potMemberId and
duplicates are eliminated.
PR 타입(하나 이상의 PR 타입을 선택해주세요)
반영 브랜치
dev -> main
작업 내용
뱃지 중복 부여 방지, JPQL 쿼리 수정
테스트 결과
Summary by CodeRabbit
신기능
스타일
리팩터