-
Notifications
You must be signed in to change notification settings - Fork 0
[CMAT-45] feat: Planner 2개로 수정 #28
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
Conversation
Walkthrough이 풀 리퀘스트는 플래너(Planner) 관리 시스템의 주요 구조를 변경합니다. 주요 변경 사항은 플래너와 회원 간의 관계를 일대일에서 다대일로 수정하고, 단일 플래너 대신 두 개의 플래너를 동시에 처리할 수 있도록 시스템을 재설계하는 것입니다. 이는 컨트롤러, 서비스, 리포지토리, DTO 등 여러 계층에 걸쳐 광범위한 수정을 포함합니다. Changes
Sequence DiagramsequenceDiagram
participant Client
participant PlannerController
participant PlannerCommandService
participant PlannerConverter
participant PlannerRepository
Client->>PlannerController: editPlanner(CreatePlannerListDTO)
PlannerController->>PlannerCommandService: editPlanner(member, createPlannerListDTO)
PlannerCommandService->>PlannerConverter: toPlannerList(member, createPlannerDTOList)
PlannerConverter-->>PlannerCommandService: List<Planner>
PlannerCommandService->>PlannerRepository: saveAll(planners)
PlannerRepository-->>PlannerCommandService: Saved
PlannerCommandService-->>PlannerController: Success
PlannerController-->>Client: ApiResponse
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Nitpick comments (7)
src/main/java/UMC/career_mate/domain/planner/Planner.java (1)
53-55: 관계 매핑 변경이 적절하게 구현되었습니다.Member와 Planner 간의 관계를 @manytoone으로 변경한 것이 요구사항에 잘 부합합니다. 이를 통해 한 멤버가 여러 개의 플래너를 가질 수 있게 되었습니다.
다만, 다음과 같은 개선사항을 고려해보시면 좋을 것 같습니다:
- Member 필드에
@JoinColumn(nullable = false)를 추가하여 필수 관계임을 명시- 관계의 의미를 명확히 하는 주석 추가
@ManyToOne - @JoinColumn(name = "member_id") + @JoinColumn(name = "member_id", nullable = false) + // 한 멤버는 여러 개의 플래너를 가질 수 있음 private Member member;src/main/java/UMC/career_mate/domain/planner/service/PlannerCommandService.java (2)
16-18: List, Iterator 사용 관련 유의
Iterator와List모두 적절히 사용되고 있습니다. 다만, 컬렉션 처리를 단순화하기 위해 스트림 등을 고려할 수도 있습니다.
52-55: 모든 Planner를 일괄 삭제하는 로직 검토
member에 속한 Planner를 모두 삭제하도록 되어 있습니다. 특정 플래너만 삭제가 필요한 경우가 없을지, 혹은 이후 복구가 필요한 기록은 없는지 고려해 보시는 것이 좋겠습니다.src/main/java/UMC/career_mate/domain/planner/controller/PlannerController.java (1)
31-54: API 문서 예시(JSON) 내부 정보 확인
예시 JSON에서 두 개의 플래너를 처리하는 것을 안내하고 있습니다. 예시 속 필드와 실제 DTO 필드가 일치하는지, 혹은 문서화가 충분히 되어 있는지 다시 한 번 점검해 주세요.src/main/java/UMC/career_mate/domain/planner/repository/PlannerRepository.java (1)
7-7: 다중 Planner 조회 메서드 추가
List<Planner> findByMember(Member member);메서드가 문제없이 동작하도록 관련 테스트 케이스를 작성해 보세요. 이미 존재하는findPlannerByMember와 구분되는 역할임을 명시하면 더욱 좋겠습니다.테스트 코드가 없다면, 작성 방법에 대해 안내나 예시를 드릴 수 있으니 요청해 주세요!
Also applies to: 14-14
src/main/java/UMC/career_mate/domain/planner/dto/request/CreatePlannerListDTO.java (1)
18-23: 생성자 유효성 검사 로직 개선이 필요합니다.현재 구현은 기본적인 검증만 수행하고 있습니다. 다음 사항들도 검증하면 좋을 것 같습니다:
- 리스트 내 요소들의 null 여부
- 중복된 플래너 데이터 여부
public CreatePlannerListDTO { - if (planners == null || planners.size() != 2) { + if (planners == null || planners.size() != 2 || planners.contains(null)) { throw new GeneralException(CommonErrorCode.INVALID_PLANNER_COUNT); } + // 중복 검사 + long uniqueCount = planners.stream() + .map(CreatePlannerDTO::activityName) + .distinct() + .count(); + if (uniqueCount != planners.size()) { + throw new GeneralException(CommonErrorCode.DUPLICATE_PLANNER); } }src/main/java/UMC/career_mate/global/response/exception/code/CommonErrorCode.java (1)
25-25: 에러 메시지를 더 구체적으로 작성하면 좋을 것 같습니다.현재 에러 메시지가 간단하게 작성되어 있습니다. 사용자가 더 쉽게 이해할 수 있도록 구체적인 정보를 추가하면 좋을 것 같습니다.
- INVALID_PLANNER_COUNT(400, "EPL003", "플래너는 2개씩 전송해야 합니다."), + INVALID_PLANNER_COUNT(400, "EPL003", "플래너는 정확히 2개를 전송해야 합니다. 현재 전송된 플래너 수가 올바르지 않습니다."),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/UMC/career_mate/domain/member/service/MemberService.java(2 hunks)src/main/java/UMC/career_mate/domain/planner/Planner.java(1 hunks)src/main/java/UMC/career_mate/domain/planner/controller/PlannerController.java(3 hunks)src/main/java/UMC/career_mate/domain/planner/converter/PlannerConverter.java(3 hunks)src/main/java/UMC/career_mate/domain/planner/dto/request/CreatePlannerListDTO.java(1 hunks)src/main/java/UMC/career_mate/domain/planner/dto/response/PlannerListResponseDTO.java(1 hunks)src/main/java/UMC/career_mate/domain/planner/repository/PlannerRepository.java(1 hunks)src/main/java/UMC/career_mate/domain/planner/service/PlannerCommandService.java(3 hunks)src/main/java/UMC/career_mate/domain/planner/service/PlannerQueryService.java(3 hunks)src/main/java/UMC/career_mate/global/response/exception/code/CommonErrorCode.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/UMC/career_mate/domain/planner/dto/response/PlannerListResponseDTO.java
🔇 Additional comments (9)
src/main/java/UMC/career_mate/domain/planner/service/PlannerCommandService.java (2)
7-7: 불필요한 import 여부 검토 권장
CreatePlannerListDTO를 가져오는 import 구문인데, 실제로 사용되는 코드와 일치하는지 한 번 더 점검해보시면 좋겠습니다.
32-38: 빈 Planner 객체 2개 자동 생성 로직 확인
Planner를 무조건 2개 생성하도록 설정되어 있는데, 절차상 필요한 로직인지 다시 한 번 검토해주세요. 실제 요구사항에 맞춰서 DTO에 기본값을 넣거나, 생성 시 조건 처리가 필요할 수도 있습니다.src/main/java/UMC/career_mate/domain/planner/controller/PlannerController.java (4)
4-5: 생성된 DTO와 응답 DTO 확인
CreatePlannerListDTO와PlannerListResponseDTO모두 새롭게 적용되어 있습니다. 다른 컨트롤러나 서비스와의 DTO 호환성에 문제가 없는지 미리 확인해 주세요.
58-59: 플래너 수정 API 정상 동작 확인
editPlanner메서드가 요구사항에 맞게 여러 Planner를 수정하는 로직을 호출하고 있습니다. 서버와 클라이언트 간에 DTO 구조가 일치하는지 테스트가 필요할 것으로 보입니다.
82-103: 복수 Planner 조회 예시 문서 검토
API 응답으로 여러 Planner를 동시에 반환하는 예시가 잘 제시되어 있습니다. 이 구조가 Front-End와 호환되는지, 실제 응답을 테스트로 확인하면 좋겠습니다.
[approve]
106-106: 플래너 리스트 응답 형태 변경
PlannerListResponseDTO를 반환함으로써 다중 Planner 처리가 가능해졌습니다. 새로운 구조가 요구사항에 부합하는 것으로 보이지만, 예외 상황에서 응답 구조가 제대로 처리되는지 확인해 보세요.src/main/java/UMC/career_mate/domain/planner/dto/request/CreatePlannerListDTO.java (1)
14-16: @Valid 어노테이션의 적절한 사용을 확인했습니다!중첩된 유효성 검사를 위해 @Valid 어노테이션을 올바르게 사용하셨습니다.
src/main/java/UMC/career_mate/domain/planner/converter/PlannerConverter.java (1)
49-56: 리스트 변환 로직이 잘 구현되었습니다.스트림 API를 효율적으로 사용하여 변환 로직을 구현하셨습니다. 코드가 간결하고 이해하기 쉽습니다.
src/main/java/UMC/career_mate/domain/member/service/MemberService.java (1)
39-39: PlannerCommandService의 구현을 확인해주세요.
plannerService.savePlanner(profileMember)가 여러 개의 Planner를 처리할 수 있도록 구현되어 있는지 확인이 필요합니다. PR의 목적에 따라 ManyToOne 관계와 List 형식의 처리가 올바르게 구현되어 있어야 합니다.다음 스크립트를 실행하여 PlannerCommandService의 구현을 확인해보겠습니다:
✅ Verification successful
PlannerCommandService 구현이 올바르게 되어있음을 확인했습니다.
- ManyToOne 관계가 올바르게 설정되어 있습니다.
- List 형식의 플래너 처리가
saveAll을 통해 구현되어 있습니다.- 멤버당 여러 개의 플래너를 저장하고 관리할 수 있는 구조로 되어 있습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: PlannerCommandService의 savePlanner 메소드 구현 확인 # PlannerCommandService 클래스에서 savePlanner 메소드 검색 ast-grep --pattern 'class PlannerCommandService { $$$ savePlanner($$$) { $$$ } $$$ }'Length of output: 99
Script:
#!/bin/bash # Find Planner related service files echo "=== Planner Service Files ===" fd -e java -e kt "Planner.*Service" echo -e "\n=== SavePlanner Method Implementation ===" rg -l "savePlanner" --type java --type kotlin echo -e "\n=== Planner Entity Relationship ===" rg "class Planner" -A 10 --type java --type kotlinLength of output: 6704
Script:
#!/bin/bash echo "=== Complete SavePlanner Implementation ===" rg "savePlanner" -A 20 src/main/java/UMC/career_mate/domain/planner/service/PlannerCommandService.java echo -e "\n=== Member-Planner Relationship ===" rg "@ManyToOne|@OneToMany.*Planner" --type javaLength of output: 2243
| public void savePlanner(Member member){ | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
메서드 시그니처 명칭과 실제 동작 간의 괴리
savePlanner 메서드 이름과 달리 내부적으로 2개의 Planner를 자동으로 생성하고 있습니다. 처음 사용자가 보기에 혼동이 생길 수 있으니, 메서드 명을 initializePlanners 등으로 좀 더 명시적으로 수정하는 방안을 고려해 보세요.
- public void savePlanner(Member member){
+ public void initializePlanners(Member member){📝 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 void savePlanner(Member member){ | |
| public void initializePlanners(Member member){ | |
| public void editPlanner(Member member, CreatePlannerListDTO createPlannerListDTO){ | ||
|
|
||
| List<Planner> planners = plannerRepository.findByMember(member); | ||
| List<CreatePlannerDTO> plannerDTOs = createPlannerListDTO.planners(); | ||
|
|
||
| Iterator<CreatePlannerDTO> dtoIterator = plannerDTOs.iterator(); | ||
| planners.forEach(planner -> planner.update(dtoIterator.next())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
플래너 개수와 DTO 목록 개수 불일치 시 예외 처리 필요
planners와 plannerDTOs의 크기가 다를 경우 dtoIterator.next() 호출 시 예외가 발생할 수 있습니다. 두 리스트 길이가 동일한지를 확인하는 로직을 추가하거나, 안전하게 처리하는 방법을 고민해 보세요.
List<CreatePlannerDTO> plannerDTOs = createPlannerListDTO.planners();
if (planners.size() != plannerDTOs.size()) {
+ throw new GeneralException(CommonErrorCode.INVALID_REQUEST, "DTO 개수와 플래너 개수가 일치하지 않습니다.");
}📝 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 void editPlanner(Member member, CreatePlannerListDTO createPlannerListDTO){ | |
| List<Planner> planners = plannerRepository.findByMember(member); | |
| List<CreatePlannerDTO> plannerDTOs = createPlannerListDTO.planners(); | |
| Iterator<CreatePlannerDTO> dtoIterator = plannerDTOs.iterator(); | |
| planners.forEach(planner -> planner.update(dtoIterator.next())); | |
| public void editPlanner(Member member, CreatePlannerListDTO createPlannerListDTO){ | |
| List<Planner> planners = plannerRepository.findByMember(member); | |
| List<CreatePlannerDTO> plannerDTOs = createPlannerListDTO.planners(); | |
| if (planners.size() != plannerDTOs.size()) { | |
| throw new GeneralException(CommonErrorCode.INVALID_REQUEST, "DTO 개수와 플래너 개수가 일치하지 않습니다."); | |
| } | |
| Iterator<CreatePlannerDTO> dtoIterator = plannerDTOs.iterator(); | |
| planners.forEach(planner -> planner.update(dtoIterator.next())); |
| public PlannerListResponseDTO getPlannerByMember(Member member) { | ||
| List<Planner> planners = plannerRepository.findByMember(member); | ||
| return PlannerConverter.toPlannerListResponseDTO(planners); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
빈 리스트 처리 로직이 필요합니다.
현재 구현에서는 플래너가 없는 경우에 대한 명시적인 처리가 없습니다. 다음과 같이 개선하면 좋을 것 같습니다:
public PlannerListResponseDTO getPlannerByMember(Member member) {
List<Planner> planners = plannerRepository.findByMember(member);
+ if (planners.isEmpty()) {
+ throw new GeneralException(CommonErrorCode.PLANNER_NOT_EXISTS);
+ }
return PlannerConverter.toPlannerListResponseDTO(planners);
}Committable suggestion skipped: line range outside the PR's diff.
src/main/java/UMC/career_mate/domain/planner/converter/PlannerConverter.java
Show resolved
Hide resolved
kyoooooong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고 많으셨습니다 👍👍
jjeongdong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM~!
#️⃣ 요약 설명
기획이 바뀌어 플래너를 2개 작성하도록 수정했습니다.
📝 작업 내용
Planner를 ManyToOne으로 바꾸고, List 형태로 처리하도록 했습니다.(프런트와 사전 합의 완.)
member쪽에서 Planner 비즈니스 로직이 들어가는 부분도 보완해서 Planner에서 처리하도록 했습니다.
@KSLEE19 참고하세용~
백엔드 고수분들은 나머지 코드가 간단해서 충분히 이해하실거라고 생각이 드네용
동작 확인
확인 application.yml이 없어서 못했는데 오류가 없을 것 같긴 합니다.
혹시 논리적인 오류가 있어 보이면 말씀해주세여
💬 리뷰 요구사항(선택)
늘 감사합니다.
Summary by CodeRabbit
릴리즈 노트
새로운 기능
변경 사항
버그 수정
제한 사항