-
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
[DDING-000] 폼지 수정내 기간 검증로직 오류 수정 #267
Conversation
Walkthrough이번 변경사항은 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 호출자
participant Service as FacadeCentralFormServiceImpl
participant FormRepo as 폼 저장소
Client->>Service: updateForm(command)
Service->>Service: validateDuplicationDateExcludingSelf(club, startDate, endDate, formId)
alt 중복 없음
Service->>FormRepo: 폼 조회 및 업데이트
else 중복 있음
Service->>Client: 중복 오류 응답
end
sequenceDiagram
participant Caller as 호출자
participant Form as Form 객체
Caller->>Form: isEqualsById(formId)
Form-->>Caller: boolean 결과
Suggested labels
Suggested reviewers
✨ Finishing Touches
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
Documentation and Community
|
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
🧹 Nitpick comments (1)
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (1)
189-203
: 중복 날짜 검증 로직이 개선되었습니다!현재 수정 중인 폼을 제외하고 날짜 중복을 검사하는 새로운 메서드가 추가되어 검증 로직이 정확해졌습니다. 스트림 API를 사용하여 깔끔하게 구현되었습니다.
하나 제안드리고 싶은 점은, 메서드 이름을 더 명확하게 할 수 있을 것 같습니다:
- private void validateDuplicationDateExcludingSelf( + private void validateDateOverlapExcludingSelf(또는
- private void validateDuplicationDateExcludingSelf( + private void validateNoDateOverlapExceptSelf(
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java
(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java
(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (3)
src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java (1)
79-81
: 구현이 명확하고 목적에 부합합니다!ID 비교를 위한 간단하면서도 명확한 메서드입니다.
equals()
를 사용하여 null 안전성도 보장되어 있습니다.src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (2)
68-74
: 매개변수명이 더 간결해졌습니다!
updateFormCommand
에서command
로 변경하여 코드가 더 간결해졌습니다.
176-187
: 메서드 가시성 변경이 적절합니다!
validateDuplicationDate
메서드를 private으로 변경한 것은 내부 구현 세부사항을 적절히 캡슐화한 좋은 변경입니다.
|
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.
확인했습니다!
🚀 작업 내용
🤔 고민했던 내용
💬 리뷰 중점사항
Summary by CodeRabbit