[CMAT-50] feat : 플래너 생성 다시 추가 및 초기 데이터 타입 수정 #32
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Walkthrough이 PR은 회원 프로필 생성 시 플래너 초기화 로직으로 변경하는 내용을 포함합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as PlannerController
participant Service as PlannerCommandService
participant Converter as PlannerConverter
participant Repo as PlannerRepository
Client->>Controller: POST /initPlanner (인증된 회원 전달)
Controller->>Service: initPlanner(member)
Service->>Converter: toInitialPlannerList(member)
Converter-->>Service: 초기 플래너 리스트 반환
Service->>Repo: saveAll(초기 플래너 리스트)
Repo-->>Service: 저장 결과 반환
Service-->>Controller: 성공 응답 전송
Controller-->>Client: ApiResponse 전송
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/UMC/career_mate/domain/planner/converter/PlannerConverter.java (1)
31-45: 초기 플래너 생성 로직 개선이 필요합니다.다음 사항들을 고려해 주시기 바랍니다:
- 빈 문자열("") 대신 의미 있는 기본값을 사용하는 것이 좋습니다.
startTime과endTime이 null인 경우 프론트엔드에서 처리가 필요할 수 있습니다.member파라미터에 대한 null 체크가 없습니다.다음과 같이 개선하는 것을 제안드립니다:
public static Planner toInitialPlanner(Member member){ + if (member == null) { + throw new IllegalArgumentException("멤버 정보가 없습니다."); + } return Planner .builder() - .activityName("") + .activityName("새로운 활동") .startTime(null) .endTime(null) - .specifics("") - .measurable("") - .achievable("") - .relevant("") - .timeBound("") - .otherPlans("") + .specifics("구체적인 계획을 입력해주세요") + .measurable("측정 가능한 목표를 입력해주세요") + .achievable("달성 가능한 목표를 입력해주세요") + .relevant("관련성 있는 목표를 입력해주세요") + .timeBound("목표 기한을 입력해주세요") + .otherPlans("추가 계획을 입력해주세요") .member(member) .build(); }src/main/java/UMC/career_mate/domain/planner/controller/PlannerController.java (1)
23-30: API 문서화가 명확합니다만, 테스트용 API 표시를 개선할 수 있습니다.테스트용 API임을 더 명확하게 표시하고, 프로덕션 환경에서의 사용을 제한하는 것이 좋을 것 같습니다.
@Operation( - summary = "플래너 생성(TEST용)", + summary = "[개발 테스트 전용] 플래너 생성", description = """ + ⚠️ 주의: 이 API는 개발 테스트 목적으로만 사용되어야 합니다. + [프로필 작성 완료시 플래너가 자동 생성되기 때문에, PATCH API를 사용해주시면 되겠습니다] 요청한 멤버에 해당하는 빈 플래너를 생성합니다. ```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/UMC/career_mate/domain/member/service/MemberService.java(1 hunks)src/main/java/UMC/career_mate/domain/planner/controller/PlannerController.java(1 hunks)src/main/java/UMC/career_mate/domain/planner/converter/PlannerConverter.java(1 hunks)src/main/java/UMC/career_mate/domain/planner/service/PlannerCommandService.java(1 hunks)
🔇 Additional comments (4)
src/main/java/UMC/career_mate/domain/planner/service/PlannerCommandService.java (2)
27-31: 멤버 유효성 검사 로직이 적절합니다.null 체크를 통한 방어적 프로그래밍이 잘 구현되어 있습니다.
37-37: PlannerConverter 사용 검증이 필요합니다.toInitialPlannerList 메서드의 구현을 확인하여 초기 플래너가 올바르게 생성되는지 검증이 필요합니다.
✅ Verification successful
다음과 같이 추가 검증을 진행하겠습니다.
PlannerConverter의 toInitialPlannerList 검증 결과
toInitialPlannerList메서드는 입력된 Member 객체를 기반으로 두 개의 초기 Planner 인스턴스를 생성하여 리스트로 반환합니다.- 각 Planner 인스턴스는 내부에서
toInitialPlanner(member)를 통해 동일한 초기값(예, 빈 문자열로 설정된 'relevant', 'timeBound', 'otherPlans' 등과 member 필드 할당)을 갖도록 생성됩니다.- 현재 코드 구현 상으로는 Builder 패턴을 활용하여 올바르게 초기화된 Planner 객체들이 반환되고 있으므로, 초기 Planner가 의도한 대로 구성되는 것으로 보입니다.
- 단, 두 개의 동일한 Planner 객체를 생성하는 것이 실제 요구사항에 부합하는지 최종적으로 팀 내부에서 한번 더 확인해 주시기 바랍니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # PlannerConverter의 toInitialPlannerList 구현 확인 ast-grep --pattern 'class PlannerConverter { $$$ static List<Planner> toInitialPlannerList($_) { $$$ } $$$ }'Length of output: 123
Script:
#!/bin/bash # PlannerConverter 클래스 정의를 포함하는 파일 찾기 rg -n "class PlannerConverter" --type java # toInitialPlannerList 메서드의 구현을 포함하는 코드를 검색 rg -n "static List<Planner> toInitialPlannerList" --type javaLength of output: 369
Script:
#!/bin/bash # PlannerConverter.java 파일에서 toInitialPlannerList 메서드의 구현 세부 내용을 확인합니다. sed -n '40,120p' src/main/java/UMC/career_mate/domain/planner/converter/PlannerConverter.javaLength of output: 1579
src/main/java/UMC/career_mate/domain/member/service/MemberService.java (1)
39-39: 메서드 호출 변경이 적절합니다.PlannerCommandService의 변경된 메서드명을 반영하여 올바르게 수정되었습니다.
src/main/java/UMC/career_mate/domain/planner/controller/PlannerController.java (1)
31-34: 보안 검증이 적절합니다.@LoginMember 어노테이션을 통해 인증된 사용자만 접근할 수 있도록 보안이 적절히 구현되어 있습니다.
Comment on lines
+47
to
52
| public static List<Planner> toInitialPlannerList(Member member){ | ||
| return List.of( | ||
| PlannerConverter.toInitialPlanner(member), | ||
| PlannerConverter.toInitialPlanner(member) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
초기 플래너 리스트 생성 로직 개선이 필요합니다.
현재 구현의 잠재적인 문제점들:
- 플래너 개수가 하드코딩되어 있습니다.
- 두 개의 플래너가 동일한 내용으로 생성됩니다.
List.of()는 불변 리스트를 반환하여 추후 수정이 필요한 경우 문제가 될 수 있습니다.
다음과 같이 개선하는 것을 제안드립니다:
public static List<Planner> toInitialPlannerList(Member member){
- return List.of(
- PlannerConverter.toInitialPlanner(member),
- PlannerConverter.toInitialPlanner(member)
- );
+ List<Planner> planners = new ArrayList<>();
+
+ Planner firstPlanner = PlannerConverter.toInitialPlanner(member);
+ firstPlanner.setActivityName("첫 번째 활동");
+ planners.add(firstPlanner);
+
+ Planner secondPlanner = PlannerConverter.toInitialPlanner(member);
+ secondPlanner.setActivityName("두 번째 활동");
+ planners.add(secondPlanner);
+
+ return planners;
}또한, 초기 플래너의 개수를 설정 파일이나 상수로 관리하는 것을 고려해보세요.
📝 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.
Suggested change
| public static List<Planner> toInitialPlannerList(Member member){ | |
| return List.of( | |
| PlannerConverter.toInitialPlanner(member), | |
| PlannerConverter.toInitialPlanner(member) | |
| ); | |
| } | |
| public static List<Planner> toInitialPlannerList(Member member){ | |
| - return List.of( | |
| - PlannerConverter.toInitialPlanner(member), | |
| - PlannerConverter.toInitialPlanner(member) | |
| - ); | |
| + List<Planner> planners = new ArrayList<>(); | |
| + | |
| + Planner firstPlanner = PlannerConverter.toInitialPlanner(member); | |
| + firstPlanner.setActivityName("첫 번째 활동"); | |
| + planners.add(firstPlanner); | |
| + | |
| + Planner secondPlanner = PlannerConverter.toInitialPlanner(member); | |
| + secondPlanner.setActivityName("두 번째 활동"); | |
| + planners.add(secondPlanner); | |
| + | |
| + return planners; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/main/java/UMC/career_mate/domain/planner/service/PlannerCommandService.java (1)
40-52: 리스트 크기 불일치에 대한 방어 로직이 필요합니다.현재 구현에서는
planners와plannerDTOs의 크기가 다를 경우NoSuchElementException또는 일부 데이터가 무시되는 문제가 발생할 수 있습니다.다음과 같은 방어 로직 추가를 제안합니다:
public void editPlanner(Member member, CreatePlannerListDTO createPlannerListDTO){ if(!plannerRepository.existsByMember(member)){ throw new GeneralException(CommonErrorCode.PLANNER_NOT_EXISTS); } List<Planner> planners = plannerRepository.findByMember(member); List<CreatePlannerDTO> plannerDTOs = createPlannerListDTO.planners(); + if (planners.size() != plannerDTOs.size()) { + throw new GeneralException(CommonErrorCode.INVALID_REQUEST); + } + Iterator<CreatePlannerDTO> dtoIterator = plannerDTOs.iterator(); planners.forEach(planner -> planner.update(dtoIterator.next())); }
🧹 Nitpick comments (1)
src/main/java/UMC/career_mate/domain/planner/service/PlannerCommandService.java (1)
27-38: 구현이 잘 되어있습니다만, 작은 개선사항이 있습니다.메서드의 유효성 검사 순서를 최적화하면 좋을 것 같습니다. null 체크와 존재 여부 체크의 순서를 바꾸면, 불필요한 데이터베이스 조회를 방지할 수 있습니다.
다음과 같이 수정하는 것을 제안합니다:
public void initPlanner(Member member){ + if(member == null){ + throw new GeneralException(CommonErrorCode.NOT_FOUND_BY_MEMBER_ID); + } + if(plannerRepository.existsByMember(member)){ throw new GeneralException(CommonErrorCode.PLANNER_EXISTS); } - if(member == null){ - throw new GeneralException(CommonErrorCode.NOT_FOUND_BY_MEMBER_ID); - } plannerRepository.saveAll(PlannerConverter.toInitialPlannerList(member)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/UMC/career_mate/domain/planner/repository/PlannerRepository.java(0 hunks)src/main/java/UMC/career_mate/domain/planner/service/PlannerCommandService.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/UMC/career_mate/domain/planner/repository/PlannerRepository.java
🔇 Additional comments (1)
src/main/java/UMC/career_mate/domain/planner/service/PlannerCommandService.java (1)
54-63: 구현이 깔끔하고 효율적입니다.존재 여부 확인 후 일괄 삭제하는 방식으로 잘 구현되어 있습니다.
deleteAll을 사용하여 배치 처리를 하는 것이 성능상 좋은 선택입니다.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/UMC/career_mate/domain/planner/converter/PlannerConverter.java (1)
45-50: 🛠️ Refactor suggestion초기 플래너 리스트 생성 로직 개선이 필요합니다.
이전 리뷰에서 지적된 문제점들이 여전히 존재합니다:
- 플래너 개수가 하드코딩되어 있습니다
- 두 플래너가 동일한 내용으로 생성됩니다
- List.of()는 불변 리스트를 반환합니다
추가로 다음과 같은 개선사항을 제안드립니다:
public static final int DEFAULT_INITIAL_PLANNER_COUNT = 2; // 설정 값으로 관리 public static List<Planner> toInitialPlannerList(Member member) { List<Planner> planners = new ArrayList<>(); for (int i = 0; i < DEFAULT_INITIAL_PLANNER_COUNT; i++) { Planner planner = toInitialPlanner(member); planner.setActivityName(String.format("%d번째 활동", i + 1)); planners.add(planner); } return planners; }
🧹 Nitpick comments (1)
src/main/java/UMC/career_mate/domain/planner/service/PlannerQueryService.java (1)
29-31: 로깅 추가를 고려해보세요.예외가 발생하는 상황에서 디버깅을 용이하게 하기 위해 로그를 추가하는 것이 좋을 것 같습니다.
다음과 같이 로그를 추가하는 것을 제안합니다:
if (planners.isEmpty()) { + log.error("플래너를 찾을 수 없습니다. memberId: {}", member.getId()); throw new GeneralException(CommonErrorCode.PLANNER_NOT_EXISTS); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/UMC/career_mate/domain/planner/converter/PlannerConverter.java(1 hunks)src/main/java/UMC/career_mate/domain/planner/service/PlannerQueryService.java(1 hunks)
🔇 Additional comments (1)
src/main/java/UMC/career_mate/domain/planner/service/PlannerQueryService.java (1)
28-32: 예외 처리 로직이 적절히 추가되었습니다.플래너 목록이 비어있는 경우에 대한 예외 처리가 명확하게 구현되었습니다. 프론트엔드 팀에서 더 나은 에러 핸들링이 가능할 것 같습니다.
Comment on lines
+29
to
+43
| public static Planner toInitialPlanner(Member member){ | ||
| return Planner | ||
| .builder() | ||
| .activityName("") | ||
| .startTime(null) | ||
| .endTime(null) | ||
| .specifics("") | ||
| .measurable("") | ||
| .achievable("") | ||
| .relevant("") | ||
| .timeBound("") | ||
| .otherPlans("") | ||
| .member(member) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
초기 플래너 생성 메소드의 개선이 필요합니다.
다음과 같은 개선사항을 제안드립니다:
- 빈 문자열 대신 의미 있는 기본값 설정이 필요합니다.
- member 파라미터에 대한 null 체크가 필요합니다.
- JavaDoc을 추가하여 초기 상태의 목적과 사용법을 문서화해주세요.
+ /**
+ * 회원의 초기 플래너를 생성합니다.
+ * @param member 플래너를 생성할 회원 (null이 아니어야 함)
+ * @return 기본값으로 초기화된 플래너
+ * @throws IllegalArgumentException member가 null인 경우
+ */
public static Planner toInitialPlanner(Member member){
+ if (member == null) {
+ throw new IllegalArgumentException("회원 정보는 필수입니다.");
+ }
return Planner
.builder()
- .activityName("")
+ .activityName("새로운 활동")
.startTime(null)
.endTime(null)
- .specifics("")
- .measurable("")
- .achievable("")
- .relevant("")
- .timeBound("")
- .otherPlans("")
+ .specifics("구체적인 계획을 입력해주세요")
+ .measurable("측정 가능한 목표를 설정해주세요")
+ .achievable("달성 가능한 목표를 설정해주세요")
+ .relevant("관련성 있는 목표를 설정해주세요")
+ .timeBound("기한이 있는 목표를 설정해주세요")
+ .otherPlans("추가 계획을 입력해주세요")
.member(member)
.build();
}📝 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.
Suggested change
| public static Planner toInitialPlanner(Member member){ | |
| return Planner | |
| .builder() | |
| .activityName("") | |
| .startTime(null) | |
| .endTime(null) | |
| .specifics("") | |
| .measurable("") | |
| .achievable("") | |
| .relevant("") | |
| .timeBound("") | |
| .otherPlans("") | |
| .member(member) | |
| .build(); | |
| } | |
| /** | |
| * 회원의 초기 플래너를 생성합니다. | |
| * @param member 플래너를 생성할 회원 (null이 아니어야 함) | |
| * @return 기본값으로 초기화된 플래너 | |
| * @throws IllegalArgumentException member가 null인 경우 | |
| */ | |
| public static Planner toInitialPlanner(Member member){ | |
| if (member == null) { | |
| throw new IllegalArgumentException("회원 정보는 필수입니다."); | |
| } | |
| return Planner | |
| .builder() | |
| .activityName("새로운 활동") | |
| .startTime(null) | |
| .endTime(null) | |
| .specifics("구체적인 계획을 입력해주세요") | |
| .measurable("측정 가능한 목표를 설정해주세요") | |
| .achievable("달성 가능한 목표를 설정해주세요") | |
| .relevant("관련성 있는 목표를 설정해주세요") | |
| .timeBound("기한이 있는 목표를 설정해주세요") | |
| .otherPlans("추가 계획을 입력해주세요") | |
| .member(member) | |
| .build(); | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#️⃣ 요약 설명
프런트가 요청사항으로 부탁하신 플래너 생성 API를 다시 생성했습니다.
초기 데이터 타입도 요청해주셔서 맞게 수정했습니다.
📝 작업 내용
Converter로 새로 생성하도록 했고 아래처럼 서비스 코드를 간소화했습니다.
member가 null인 상태로 서비스 단으로 넘어오는 경우도 있어서 우선 예외 처리했고, 안정화되면 수정하겠습니다.
동작 확인
로컬호스트에서 로그인이 불가능하고, 프런트가 빨리 필요한 것 같아 우선 올렸습니다.
💬 리뷰 요구사항(선택)
코드의 논리적 오류를 살펴봐주시면 감사하겠습니다.
Summary by CodeRabbit