[Merge/#413] todo badge pot rename #419
Conversation
Walkthrough배지 선정 로직을 완료한 “서로 다른 사용자 수” 기준으로 개편하고 관련 리포지토리 쿼리를 페이징·카운트 기반으로 변경했습니다. Pot 이름 변경을 위한 PATCH 엔드포인트·DTO·서비스 구현과 Pot 엔터티의 setter가 추가되었고, 일부 DTO/서비스에서 날짜 타입이 String으로 변경되었습니다. 에러 메시지와 컨트롤러의 문서화된 에러 코드 일부가 수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant PotController
participant PotCommandServiceImpl
participant AuthService
participant PotRepository
participant DB
Client->>PotController: PATCH /pots/{id}/rename { potName }
PotController->>PotCommandServiceImpl: updatePotName(id, request)
PotCommandServiceImpl->>AuthService: getCurrentUser()
PotCommandServiceImpl->>PotRepository: findById(id)
PotRepository-->>PotCommandServiceImpl: Pot
PotCommandServiceImpl->>PotCommandServiceImpl: 소유자 검증
PotCommandServiceImpl->>DB: setPotName(...) 저장
PotCommandServiceImpl-->>PotController: 새 이름(String)
PotController-->>Client: ApiResponse<String>
sequenceDiagram
participant Caller
participant BadgeServiceImpl
participant UserTodoRepository
participant BadgeRepository
participant PotMemberRepository
participant PotMemberBadgeRepository
Caller->>BadgeServiceImpl: assignBadgeToTopMembers(potId)
BadgeServiceImpl->>UserTodoRepository: countDistinctUserIdsByPotAndStatus(potId, COMPLETED)
UserTodoRepository-->>BadgeServiceImpl: completedUserCount
alt completedUserCount < 2
BadgeServiceImpl-->>Caller: throw BADGE_INSUFFICIENT_TODO_COUNTS
else
BadgeServiceImpl->>UserTodoRepository: findTop2UserIds(potId, COMPLETED)
UserTodoRepository-->>BadgeServiceImpl: [userId1, userId2]
BadgeServiceImpl->>BadgeRepository: getBadge(1L)
loop for each userId
BadgeServiceImpl->>PotMemberRepository: findByPot_PotIdAndUser_Id(potId, userId)
PotMemberRepository-->>BadgeServiceImpl: PotMember
BadgeServiceImpl->>PotMemberBadgeRepository: save(PotMemberBadge)
end
BadgeServiceImpl-->>Caller: 완료
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ 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: 4
🧹 Nitpick comments (6)
src/main/java/stackpot/stackpot/pot/entity/Pot.java (1)
41-43: potName에 대한 공개 Setter 도입은 도메인 무결성에 취약합니다. 접근 제한 또는 도메인 메서드로 캡슐화 제안외부에서 무제한으로 이름을 변경할 수 있어 길이(255 초과), 공백/빈 문자열, 앞뒤 공백, 금칙어/중복명 검증 등을 우회할 수 있습니다. 최소한 접근 제어를 낮추거나, 유효성 검증을 포함한 도메인 메서드로 캡슐화하는 것을 권장합니다.
적용 예시(선택 1: 접근 제한):
- @Setter + @Setter(AccessLevel.PROTECTED) @Column(nullable = false, length = 255) private String potName;적용 예시(선택 2: 도메인 메서드 추가 — 해당 엔티티 내부에 추가, 서비스는 이 메서드를 사용):
public void rename(String newName) { if (newName == null || newName.isBlank()) { throw new IllegalArgumentException("팟 이름은 필수입니다."); } String trimmed = newName.trim(); if (trimmed.length() > 255) { throw new IllegalArgumentException("팟 이름은 255자 이하여야 합니다."); } this.potName = trimmed; }src/main/java/stackpot/stackpot/pot/dto/CompletedPotRequestDto.java (1)
18-18: potStartDate를 String으로 변경 시 형식/필수값 검증이 누락되었습니다엔티티는 potStartDate가 NOT NULL이며(컬럼 nullable=false), 주석상 형식은 “yyyy.MM”로 보입니다. 필수값과 형식 검증을 DTO에 추가하세요.
적용 예시:
package stackpot.stackpot.pot.dto; import jakarta.validation.constraints.NotBlank; +import jakarta.validation.constraints.Pattern; import lombok.Builder; import lombok.Getter; import lombok.Setter; @@ - private String potStartDate; + @NotBlank(message = "팟 시작일은 필수입니다.") + @Pattern(regexp = "^\\d{4}\\.\\d{2}$", message = "팟 시작일 형식은 yyyy.MM 이어야 합니다.") + private String potStartDate;컨트롤러 메서드에
@Valid를 적용하여 검증을 활성화하는 것도 잊지 마세요.src/main/java/stackpot/stackpot/pot/service/pot/PotCommandService.java (1)
24-24: API 일관성 고려: 반환 타입을 PotResponseDto 또는 void(204)로 통일하는 방안 검토다른 메서드는 대부분 PotResponseDto를 반환합니다. 여기만 String(이름) 반환이라 클라이언트/스웨거 스키마 일관성이 떨어질 수 있습니다.
대안:
- PotResponseDto 반환으로 통일하여 갱신된 엔티티 스냅샷 제공
- 또는 204 No Content로 응답(비동기 UI라면 ETag 재조회 유도)
src/main/java/stackpot/stackpot/pot/repository/PotMemberRepository.java (1)
96-98: 중복/유사 Finder 정리 검토동일 기능을 수행하는
findByPotIdAndUserId(Line 54-56, JPQL 기반)와 새 파생 메서드(findByPot_PotIdAndUser_Id)가 공존합니다. 팀 컨벤션에 맞춰 하나로 정리하거나, 기존 메서드에 @deprecated를 붙여 사용처를 이관하면 혼동을 줄일 수 있습니다.src/main/java/stackpot/stackpot/pot/service/pot/PotCommandServiceImpl.java (1)
274-287: 팟 이름 업데이트 시 입력 정제(공백 트림) 권장DTO의 Bean Validation이 있더라도, 서비스 단에서 최소한의 정제를 해두면 불필요한 공백 저장을 방지할 수 있습니다.
적용 제안:
- pot.setPotName(request.getPotName()); + // 선행 @Valid로 null/blank 방어가 된다는 전제에서 트림 적용 + pot.setPotName(request.getPotName().trim());src/main/java/stackpot/stackpot/todo/repository/UserTodoRepository.java (1)
41-51: 동률 시 결정적 순서 보장을 위한 보조 정렬 추가 제안COUNT가 동일한 사용자가 여러 명일 때 결과 순서가 비결정적일 수 있습니다. 사용자 id 오름차순 등 보조 정렬 키를 추가해 안정성을 확보하는 것을 권장합니다.
적용 제안:
@Query(""" SELECT ut.user.id FROM UserTodo ut WHERE ut.pot.potId = :potId AND ut.status = :status GROUP BY ut.user.id - ORDER BY COUNT(ut.todoId) DESC + ORDER BY COUNT(ut.todoId) DESC, ut.user.id ASC """)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
src/main/java/stackpot/stackpot/apiPayload/code/status/ErrorStatus.java(1 hunks)src/main/java/stackpot/stackpot/badge/controller/PotBadgeMemberController.java(0 hunks)src/main/java/stackpot/stackpot/badge/service/BadgeServiceImpl.java(1 hunks)src/main/java/stackpot/stackpot/pot/controller/PotController.java(1 hunks)src/main/java/stackpot/stackpot/pot/dto/CompletedPotRequestDto.java(1 hunks)src/main/java/stackpot/stackpot/pot/dto/PotNameUpdateRequestDto.java(1 hunks)src/main/java/stackpot/stackpot/pot/entity/Pot.java(1 hunks)src/main/java/stackpot/stackpot/pot/repository/PotMemberRepository.java(1 hunks)src/main/java/stackpot/stackpot/pot/service/pot/PotCommandService.java(2 hunks)src/main/java/stackpot/stackpot/pot/service/pot/PotCommandServiceImpl.java(4 hunks)src/main/java/stackpot/stackpot/todo/repository/UserTodoRepository.java(3 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/stackpot/stackpot/badge/controller/PotBadgeMemberController.java
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/stackpot/stackpot/pot/dto/PotNameUpdateRequestDto.java (2)
src/main/java/stackpot/stackpot/pot/dto/CompletedPotRequestDto.java (1)
Getter(10-24)src/main/java/stackpot/stackpot/pot/dto/PotRequestDto.java (1)
Getter(13-42)
🔇 Additional comments (6)
src/main/java/stackpot/stackpot/pot/repository/PotMemberRepository.java (1)
96-98: 파생 쿼리 메서드 추가, 배지 로직 요구사항과 정합성 양호
countByPot_PotId,findByPot_PotIdAndUser_Id모두 JPA 파생 쿼리 관례에 맞고, 상위 로직(배지 수여)에서 요구하는 데이터 접근 패턴을 충족합니다. 성능 및 가독성 측면에서도 적절합니다.src/main/java/stackpot/stackpot/badge/service/BadgeServiceImpl.java (3)
45-47: 2인 이하 팟에 대한 Early Return 동작 확인 요청멤버가 2명 이하인 경우 이제 아무 작업도 수행하지 않습니다. 이전에는 에러를 던졌다면 클라이언트/운영 관점 동작 변화가 생깁니다. 의도된 정책인지 최종 확인 부탁드립니다.
49-55: 서로 다른 완료 사용자 수 기반 선행 검증, 로직 적정
countDistinctUserIdsByPotAndStatus로 완료 사용자 수를 선행 검증하는 변경은 요구사항(“완료한 사용자 수” 기준)과 일치합니다.
58-62: 페이징 기반 Top2 조회로 확장성 확보
findTop2UserIds(페이징 래핑)로 상위 2명 조회를 수행하는 접근은 향후 Top N 확장에 유리합니다. 사이즈 < 2 시의 예외 처리도 타당합니다.src/main/java/stackpot/stackpot/pot/service/pot/PotCommandServiceImpl.java (1)
195-202: 검증 결과 — potEndDate 타입 일관됨 (추가 수정 불필요)Pot 엔티티의 potEndDate가 String으로 선언되어 있고 updateFields에서 (String)으로 할당하므로, PotCommandServiceImpl에서 String.valueOf(LocalDate.now())로 설정한 것은 타입 불일치가 아닙니다.
수정/검토한 위치:
- src/main/java/stackpot/stackpot/pot/entity/Pot.java — private String potEndDate; 및 updateFields의 case "potEndDate" -> this.potEndDate = (String) value;
- src/main/java/stackpot/stackpot/pot/service/pot/PotCommandServiceImpl.java — updateValues.put("potEndDate", String.valueOf(LocalDate.now()));
- src/main/java/stackpot/stackpot/pot/dto/PotRequestDto.java, CompletedPotRequestDto.java 등 — potStartDate/potEndDate가 String으로 선언되어 있음
src/main/java/stackpot/stackpot/todo/repository/UserTodoRepository.java (1)
30-56: Distinct 집계 + 페이징 TopN 도입 적절
countDistinctUserIdsByPotAndStatus: “서로 다른 완료 사용자 수” 요구사항을 정확히 반영합니다.findTopUserIdsByPotAndStatus+default findTop2UserIds: TopN 확장성 및 재사용성이 좋습니다.
PR 타입(하나 이상의 PR 타입을 선택해주세요)
반영 브랜치
dev -> main
작업 내용
테스트 결과
Summary by CodeRabbit