feat: 이벤트 폼 제출 서비스를 정책에 따라 변경 #1219
Conversation
📝 WalkthroughWalkthrough컨트롤러·서비스·DTO의 이벤트 신청 API 명칭과 타입이 변경되었습니다: Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant Controller as ParticipantEventParticipationController
participant Service as EventParticipationService
participant Domain as EventParticipationDomainService
participant MemberRepo as MemberRepository
Client->>Controller: POST /event-participations/apply\n(EventApplyOnlineRequest)
Controller->>Service: applyOnline(request)
Service->>MemberRepo: findByStudentId(participant.studentId)
MemberRepo-->>Service: Member? (or null)
Service->>Domain: applyOnline(participant, member, afterPartyStatus, event, now)
Domain-->>Service: EventParticipation
Service->>Service: save(EventParticipation)
Service-->>Controller: void
Controller-->>Client: 200 OK
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java (1)
283-291: 빈 목록 처리 누락으로 인한 IndexOutOfBounds 위험
participations.get(0)호출 전 빈 목록 체크가 없습니다. 요청이 비어도 서비스는 안전하게 no-op로 처리하는 편이 낫습니다.private void validateRequestParticipationsSameEvent(List<EventParticipation> participations) { - Event event = participations.get(0).getEvent(); + if (participations.isEmpty()) { + return; // 빈 요청은 무해하게 무시 + } + Event event = participations.get(0).getEvent(); boolean hasDifferentEvent = participations.stream() .anyMatch(participation -> !participation.getEvent().equals(event)); if (hasDifferentEvent) { throw new CustomException(PARTICIPATION_NOT_UPDATABLE_DIFFERENT_EVENT); } }
🧹 Nitpick comments (2)
src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java (2)
321-323: null 허용 인자 처리 확인 + 시간 의존성 주입 제안
- Domain 계층의
applyOnline(...)에서member가 null일 수 있음을 전제로 방어 로직이 있는지 확인해 주세요. 없다면 NPE/분기 오류 위험이 있습니다. 가능하면 파라미터에@Nullable명시 또는 오버로드로 의도를 드러내세요.- 테스트 용이성을 위해
now()대신Clock주입을 권장합니다.패치 예시(해당 범위 내 변경):
- participant, member, request.afterPartyApplicationStatus(), event, now()); + participant, member, request.afterPartyApplicationStatus(), event, now(clock));클래스 외부 변경(참고):
import java.time.Clock; // ... @RequiredArgsConstructor public class EventParticipationService { private final Clock clock; // ... }추가로, 다음 케이스의 단위 테스트 2건을 권장합니다.
- 회원 존재(member != null) + after party 신청 O/X
- 회원 미존재(member == null) 흐름
243-249: 변수명 일관성 정리 제안동일 패턴의 메서드에서
memberByParticipant→member로 통일하면 가독성이 좋아집니다.- Member memberByParticipant = - memberRepository.findByStudentId(participant.getStudentId()).orElse(null); + Member member = + memberRepository.findByStudentId(participant.getStudentId()).orElse(null); - EventParticipation participation = - eventParticipationDomainService.applyManual(participant, memberByParticipant, event); + EventParticipation participation = + eventParticipationDomainService.applyManual(participant, member, event);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-test
🔇 Additional comments (2)
src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java (2)
318-318: 지역 변수명(member) 변경 깔끔합니다.의도 전달이 더 명확해졌습니다.
319-320: participant.getStudentId()의 null 가능성 확인 필요EventApplyRequest/Participant DTO에서 studentId가 Bean Validation(@NotBlank/@NotNull 등)으로 보장되는지 확인하세요. 보장되지 않으면 src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java (applyEventParticipation)에서 Assert.notNull(participant.getStudentId())를 추가하거나 DTO에 @NotBlank/@NotNull을 적용하세요.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/gdschongik/gdsc/domain/event/dto/request/EventApplyOnlineRequest.java (1)
8-11: @Valid 추가·도메인 엔티티 직접 노출 최소화 권장
- participant에 @Valid 추가 권장 (Bean Validation cascade가 필요하면). 참고: 현재 Participant는 @JsonCreator로 생성되며 빌더 내부에서 CustomException으로 검증되므로 @Valid는 필수는 아님. 변경 예시:
public record EventApplyOnlineRequest( @NotNull @Positive Long eventId, - @NotNull Participant participant, + @NotNull @Valid Participant participant, @NotNull AfterPartyApplicationStatus afterPartyApplicationStatus) {}추가 import:
import jakarta.validation.Valid;
요청 DTO가 도메인 엔티티 Participant를 직접 노출하고 있음 — 입력 허용 필드를 화이트리스트로 제한하는 전용 Request DTO(e.g. ParticipantRequest {name, studentId, phone})로 분리하고 컨트롤러/서비스에서 Participant.of(...)로 변환할 것. 위치: src/main/java/com/gdschongik/gdsc/domain/event/dto/request/EventApplyOnlineRequest.java, src/main/java/com/gdschongik/gdsc/domain/event/domain/Participant.java
AfterPartyApplicationStatus는 온라인 신청 흐름에서 클라이언트 입력으로 사용됨(검증: EventParticipationDomainService.applyOnline 및 validateAfterPartyApplicationStatus). 정책상 서버 결정값이어야 하면 API를 단순 플래그(e.g. willAttendAfterParty)로 바꾸고 서버에서 매핑할 것. 위치: src/main/java/com/gdschongik/gdsc/domain/event/domain/service/EventParticipationDomainService.java, src/main/java/com/gdschongik/gdsc/domain/event/domain/AfterPartyApplicationStatus.java
src/main/java/com/gdschongik/gdsc/domain/event/api/ParticipantEventParticipationController.java (2)
25-25: 엔드포인트 명세와 메서드 명이 ‘Online’ 정책을 드러내지 않음외부 계약을 그대로 유지해야 하는 사유가 없다면, 메서드명(및 필요시 경로/문서화)에서 온라인 전용임을 드러내면 API 가독성이 좋아집니다.
권장(메서드명만 변경하는 최소 변경):
- public ResponseEntity<Void> applyEventParticipation(@Valid @RequestBody EventApplyOnlineRequest request) { + public ResponseEntity<Void> applyEventParticipationOnline(@Valid @RequestBody EventApplyOnlineRequest request) {선택(경로/문서도 일치화):
- @PostMapping("/apply/online")
- @operation(summary = "온라인 이벤트 참여 신청 폼 제출", description = "온라인 이벤트 참여 신청 폼을 제출합니다.")
26-26: 중복 제출/예외 매핑 정책 확인서비스로 위임만 하고 있어 컨트롤러 계층에서의 예외 → HTTP 상태 코드 매핑이 보이지 않습니다. 아래 항목이 전역 예외 처리(@ControllerAdvice)로 커버되는지 확인 부탁드립니다:
- 존재하지 않는 이벤트/참가자: 404 Not Found
- 정책 위반/중복 신청: 409 Conflict 또는 422 Unprocessable Entity
- 권한 부족: 403 Forbidden
중복 제출 방지(예: Idempotency-Key 헤더 처리, 멱등 키 기반 저장) 여부도 정책과 일치하는지 확인해 주세요.
필요하시면 전역 예외 핸들러 스켈레톤과 멱등 처리 예시를 드리겠습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/gdschongik/gdsc/domain/event/api/ParticipantEventParticipationController.java(2 hunks)src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java(2 hunks)src/main/java/com/gdschongik/gdsc/domain/event/dto/request/EventApplyOnlineRequest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/gdschongik/gdsc/domain/event/application/EventParticipationService.java
🔇 Additional comments (1)
src/main/java/com/gdschongik/gdsc/domain/event/api/ParticipantEventParticipationController.java (1)
4-4: DTO rename 반영 적절새로운 요청 타입 import가 컨트롤러에 일관되게 반영되었습니다.
|
Lgtm |
| @Transactional | ||
| public void applyEventParticipation(EventApplyRequest request) { | ||
| // TODO: 메서드 및 DTO applyOnline으로 이름 변경 | ||
| public void applyOnline(EventApplyOnlineRequest request) { |
There was a problem hiding this comment.
memberByParticipant 말씀하시는건가요?
Job Summary for GradleCheck Style and Test to Develop :: build-test
|
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
🌱 관련 이슈
📌 작업 내용 및 특이사항
📝 참고사항
📚 기타
Summary by CodeRabbit
리팩터링
스타일