-
Notifications
You must be signed in to change notification settings - Fork 2
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
톡픽 생성과 수정 요청 DTO 분리 #709
톡픽 생성과 수정 요청 DTO 분리 #709
Conversation
Walkthrough
Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 0
🧹 Outside diff range and nitpick comments (2)
src/main/java/balancetalk/talkpick/presentation/TalkPickController.java (1)
Line range hint
1-82
: 아키텍처 관점에서 몇 가지 제안사항이 있습니다.현재 구조는 잘 설계되어 있지만, 다음과 같은 개선사항을 고려해보시면 좋을 것 같습니다:
- 응답 DTO들도 요청 DTO처럼 별도의 패키지로 분리하는 것을 고려해보세요
- API 버저닝을 위한 준비를 해두면 좋을 것 같습니다
- 페이징 관련 파라미터를 상수로 분리하는 것을 고려해보세요
src/main/java/balancetalk/talkpick/dto/TalkPickDto.java (1)
66-76
: Builder 패턴의 Null 안전성 개선이 필요합니다.현재 구현에서는 필수 필드들에 대한 null 체크가 없습니다. 다음과 같은 개선을 제안드립니다:
public TalkPick toEntity(Member member) { + if (member == null || baseFields == null) { + throw new IllegalArgumentException("필수 필드가 null일 수 없습니다."); + } return TalkPick.builder() .member(member) .title(baseFields.getTitle()) .content(baseFields.getContent()) .optionA(baseFields.getOptionA()) .optionB(baseFields.getOptionB()) .sourceUrl(baseFields.getSourceUrl()) .editedAt(LocalDateTime.now()) .build(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/main/java/balancetalk/talkpick/application/TalkPickService.java
(3 hunks)src/main/java/balancetalk/talkpick/dto/TalkPickDto.java
(2 hunks)src/main/java/balancetalk/talkpick/presentation/TalkPickController.java
(4 hunks)
🔇 Additional comments (8)
src/main/java/balancetalk/talkpick/presentation/TalkPickController.java (2)
3-6
: DTO 클래스들의 정적 임포트가 잘 정리되었습니다!
각각의 DTO를 명시적으로 임포트함으로써 코드의 가독성이 향상되었습니다.
41-43
: DTO 분리가 성공적으로 구현되었습니다.
생성과 수정 요청을 위한 DTO를 분리함으로써:
- 각 작업의 의도가 명확해졌습니다
- 수정 시 조회수와 북마크 수가 초기화되는 문제가 해결되었습니다
다음 스크립트로 DTO 사용을 검증하겠습니다:
Also applies to: 64-66
✅ Verification successful
DTO 분리 검증이 성공적으로 완료되었습니다.
- 코드베이스 전체에서
CreateOrUpdateTalkPickRequest
의 사용이 더 이상 존재하지 않습니다. - 서비스 레이어에서
CreateTalkPickRequest
와UpdateTalkPickRequest
가 올바르게 사용되고 있음을 확인했습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that CreateTalkPickRequest and UpdateTalkPickRequest are used correctly throughout the codebase
# Check for any remaining usage of CreateOrUpdateTalkPickRequest
rg "CreateOrUpdateTalkPickRequest"
# Verify the separation of DTOs in service layer
rg -A 5 "class TalkPickService"
Length of output: 1239
src/main/java/balancetalk/talkpick/application/TalkPickService.java (4)
5-5
: import 문이 DTO 분리 목적에 맞게 잘 수정되었습니다.
CreateTalkPickRequest와 UpdateTalkPickRequest를 개별적으로 import하여 각각의 책임을 명확히 분리했습니다.
Also applies to: 8-8
49-51
: 파일 처리 로직이 잘 개선되었습니다.
파일 재배치 로직을 단일 메서드로 통합하여 코드 중복을 제거하고 재사용성을 향상시켰습니다.
87-93
: 수정 로직이 개선되었으나 toEntity 구현 확인이 필요합니다.
UpdateTalkPickRequest를 사용하여 수정 시 view count와 bookmark count가 초기화되는 문제를 해결했습니다. 다만, 이 필드들이 보존되는지 확인이 필요합니다.
39-45
: 생성 로직이 개선되었으나 toEntity 구현 확인이 필요합니다.
파일 처리 로직이 명확해졌고 CreateTalkPickRequest를 사용하여 책임이 분리되었습니다. 다만, view count와 bookmark count가 0으로 초기화되는지 확인이 필요합니다.
src/main/java/balancetalk/talkpick/dto/TalkPickDto.java (2)
Line range hint 26-55
: 생성 요청 DTO의 분리가 잘 이루어졌습니다!
단일 책임 원칙에 맞게 생성 요청만을 담당하도록 분리되었으며, views와 bookmarks를 0으로 초기화하는 것이 적절합니다.
56-81
: 수정 요청 DTO의 분리가 잘 이루어졌습니다!
기존의 문제였던 조회수와 북마크 수가 초기화되는 이슈가 해결되었습니다.
} | ||
private void relocateFiles(List<Long> fileIds, Long talkPickId) { | ||
List<File> files = fileRepository.findAllById(fileIds); | ||
fileHandler.relocateFiles(files, talkPickId, TALK_PICK); |
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.
분리 👍 🥇
Quality Gate passedIssues Measures |
💡 작업 내용
💡 자세한 설명
아래와 같이 톡픽 생성 요청과 수정 요청 DTO를 하나로 통일하면, 수정 요청할 때도 조회수와 북마크 수가 0으로 초기화되는 문제가 발생합니다.
따라서 CreateOrUpdateTalkPickRequest를 생성 요청 DTO와 수정 요청 DTO로 분리하였습니다.
✅ 셀프 체크리스트
closes #704
Summary by CodeRabbit
새로운 기능
버그 수정
문서화