[Feature] 미팅 전체 취소 api 구현 및 미팅 정책 위반 로직 수정 - #78
Conversation
Walkthrough미팅 전체 취소 요청·투표·만료 처리를 추가하고, 승인 결과에 따라 미팅과 매칭 채팅방을 비활성화하도록 변경했습니다. 빠른 매칭 입장·퇴장 시 채팅방 연결과 미팅 상태 전이를 보강했으며, 비활성 채팅방 접근을 제한했습니다. Changes미팅 취소 및 매칭 흐름
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MeetingController
participant MeetingCancellationService
participant MeetingCancellationVoteRepository
participant ChatRoomService
participant Meeting
Client->>MeetingController: 취소 요청 또는 투표
MeetingController->>MeetingCancellationService: 취소 처리 위임
MeetingCancellationService->>MeetingCancellationVoteRepository: 투표 저장 및 집계
MeetingCancellationService->>ChatRoomService: 채팅방 및 멤버 비활성화
MeetingCancellationService->>Meeting: 취소 상태 반영
MeetingCancellationService-->>MeetingController: 취소 응답 반환
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
manabom/src/test/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationServiceTest.java (1)
124-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win채팅방/멤버 비활성화 경로에 대한 검증 보강 권장
unanimousApprovalCancelsBothMatchedTeams는meeting1/meeting2의cancelByAgreement()호출만 검증합니다.approveCancellation()이 함께 수행하는chatRoomRepository.findByMatch/findByMeeting조회 결과에 대한deactivateChatRoomAndMembers호출(채팅방/멤버 비활성화)은 이 테스트에서 커버되지 않습니다. 이번 PR의 목표 중 하나가 "매칭 성사 후 채팅방 비활성화"이므로, 관련 verify를 추가해 회귀를 방지하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@manabom/src/test/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationServiceTest.java` around lines 124 - 156, 보 unanimousApprovalCancelsBothMatchedTeams 테스트가 meeting 취소뿐 아니라 채팅방과 멤버 비활성화 경로도 검증하도록 보강하세요. approveCancellation()에서 사용하는 chatRoomRepository의 매칭/회의 조회 결과를 설정하고, deactivateChatRoomAndMembers 호출이 두 매칭 팀에 대해 수행되는지 검증하세요.manabom/src/main/java/mannabom_server/manabom/application/meeting/service/MeetingService.java (1)
184-200: 🎯 Functional Correctness | 🔵 Trivial
enterRoomByCode/enterRoomById로직 중복 및saveAndFlush처리 불일치.두 메서드가 취소 대기 검증 → 입장 검증 →
isFastMatchingEntry계산 →addMember→ 채팅방 조인 → 빠른 입장 매칭방 연결 → 응답 생성까지 거의 동일한 로직을 반복하고 있습니다. 공통 private 메서드로 추출하면 유지보수성이 개선되고, 두 진입점이 항상 동일하게 동작함을 보장할 수 있습니다.추가로
enterRoomByCode(Line 188)에만meetingRepository.saveAndFlush(meeting)호출과 "왜 addMember가 반영이 안되지" 주석이 있는데,enterRoomById에는 동일한 flush가 없습니다. 원래 flush가 필요했던 이유(더티 체킹이 반영되지 않는 문제)가 실재한다면enterRoomById에도 동일한 문제가 잠재해 있을 수 있습니다. 두 진입점에서 flush 필요 여부를 명확히 하고 일관되게 처리해 주세요.Also applies to: 215-249
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@manabom/src/main/java/mannabom_server/manabom/application/meeting/service/MeetingService.java` around lines 184 - 200, Extract the shared flow from enterRoomByCode and enterRoomById into one private helper covering cancellation validation, join validation, fast-matching detection, member addition, chat-room joins, and response construction. Ensure both entry points delegate to this helper and use the same meeting persistence behavior, explicitly deciding whether saveAndFlush is required and applying that decision consistently; remove the temporary comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@manabom/src/main/java/mannabom_server/manabom/application/chat/service/ChatRoomService.java`:
- Around line 139-155: Update joinMatchingChatRoom to check duplicate membership
using the member status, limiting the existsBy query to
ChatMemberStatus.ACTIVATE so users with DEACTIVATED membership history can
rejoin; preserve the existing active-member exception and save flow, and add a
database uniqueness safeguard if required to prevent concurrent duplicate
inserts.
In
`@manabom/src/main/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationService.java`:
- Around line 155-165: Update expirePendingRequests() so each expired request is
processed in an independent transaction, preventing an OptimisticLockException
for one request from rolling back other requests. Move the per-request expire
operation into a transactional boundary that can commit or roll back
independently, while preserving the existing count and expiration timestamp
behavior.
In
`@manabom/src/main/java/mannabom_server/manabom/domain/meeting/enums/MeetingCancellationStatus.java`:
- Around line 3-8: Synchronize MeetingCancellationStatus with the
chk_cancellation_request_status database constraint by adding the missing
WITHDRAWN enum value, unless the schema is intentionally changed to remove it;
ensure the JPA enum values and allowed database statuses remain consistent.
---
Nitpick comments:
In
`@manabom/src/main/java/mannabom_server/manabom/application/meeting/service/MeetingService.java`:
- Around line 184-200: Extract the shared flow from enterRoomByCode and
enterRoomById into one private helper covering cancellation validation, join
validation, fast-matching detection, member addition, chat-room joins, and
response construction. Ensure both entry points delegate to this helper and use
the same meeting persistence behavior, explicitly deciding whether saveAndFlush
is required and applying that decision consistently; remove the temporary
comment.
In
`@manabom/src/test/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationServiceTest.java`:
- Around line 124-156: 보 unanimousApprovalCancelsBothMatchedTeams 테스트가 meeting
취소뿐 아니라 채팅방과 멤버 비활성화 경로도 검증하도록 보강하세요. approveCancellation()에서 사용하는
chatRoomRepository의 매칭/회의 조회 결과를 설정하고, deactivateChatRoomAndMembers 호출이 두 매칭 팀에
대해 수행되는지 검증하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: de0f7f96-38d9-4777-899a-18c3ec06efb0
📒 Files selected for processing (27)
manabom/src/main/java/mannabom_server/manabom/application/chat/service/ChatRoomService.javamanabom/src/main/java/mannabom_server/manabom/application/chat/service/ChatService.javamanabom/src/main/java/mannabom_server/manabom/application/meeting/dto/request/MeetingCancellationVoteRequest.javamanabom/src/main/java/mannabom_server/manabom/application/meeting/dto/response/MeetingCancellationResponse.javamanabom/src/main/java/mannabom_server/manabom/application/meeting/dto/response/MeetingCancellationVoteResponse.javamanabom/src/main/java/mannabom_server/manabom/application/meeting/scheduler/MeetingCancellationScheduler.javamanabom/src/main/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationService.javamanabom/src/main/java/mannabom_server/manabom/application/meeting/service/MeetingMatchingService.javamanabom/src/main/java/mannabom_server/manabom/application/meeting/service/MeetingService.javamanabom/src/main/java/mannabom_server/manabom/domain/chat/repository/ChatMemberRepository.javamanabom/src/main/java/mannabom_server/manabom/domain/chat/repository/ChatRoomRepository.javamanabom/src/main/java/mannabom_server/manabom/domain/meeting/entity/Meeting.javamanabom/src/main/java/mannabom_server/manabom/domain/meeting/entity/MeetingCancellationRequest.javamanabom/src/main/java/mannabom_server/manabom/domain/meeting/entity/MeetingCancellationVote.javamanabom/src/main/java/mannabom_server/manabom/domain/meeting/enums/CancellationVoteDecision.javamanabom/src/main/java/mannabom_server/manabom/domain/meeting/enums/MeetingCancellationStatus.javamanabom/src/main/java/mannabom_server/manabom/domain/meeting/enums/MeetingStatus.javamanabom/src/main/java/mannabom_server/manabom/domain/meeting/repository/MeetingCancellationRequestRepository.javamanabom/src/main/java/mannabom_server/manabom/domain/meeting/repository/MeetingCancellationVoteRepository.javamanabom/src/main/java/mannabom_server/manabom/domain/meeting/repository/MeetingMatchRepository.javamanabom/src/main/java/mannabom_server/manabom/infrastructure/security/websocket/StompAuthChannelInterceptor.javamanabom/src/main/java/mannabom_server/manabom/presentation/meeting/controller/MeetingController.javamanabom/src/main/resources/db/migration/V21__add_meeting_cancellation_tables.sqlmanabom/src/main/resources/db/migration/V22__move_meeting_cancellation_to_match.sqlmanabom/src/test/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationServiceTest.javamanabom/src/test/java/mannabom_server/manabom/domain/meeting/MeetingCancellationDomainTest.javamanabom/src/test/java/mannabom_server/manabom/domain/meeting/MeetingMemberLeaveStatusTest.java
There was a problem hiding this comment.
🧹 Nitpick comments (2)
manabom/src/main/resources/db/migration/V23__remove_withdrawn_cancellation_status.sql (1)
4-11: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win테이블 락(Table Lock) 방지를 위한
NOT VALID활용 권장운영 환경에서 제약 조건을 추가할 때 테이블 전체를 스캔하는 동안 쓰기 작업이 차단(Lock)될 수 있습니다. 이를 방지하기 위해
NOT VALID로 제약 조건을 먼저 추가한 뒤VALIDATE CONSTRAINT를 통해 비동기적으로 검증하는 방식을 권장합니다.💡 제안하는 마이그레이션 스크립트 수정안
ALTER TABLE meeting_cancellation_requests ADD CONSTRAINT chk_cancellation_request_status CHECK (status IN ( 'PENDING', 'APPROVED', 'REJECTED', 'EXPIRED' - )); + )) NOT VALID; + +ALTER TABLE meeting_cancellation_requests + VALIDATE CONSTRAINT chk_cancellation_request_status;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@manabom/src/main/resources/db/migration/V23__remove_withdrawn_cancellation_status.sql` around lines 4 - 11, Update the chk_cancellation_request_status constraint in the migration to add it with NOT VALID, then separately validate it using VALIDATE CONSTRAINT so constraint creation does not scan and lock the entire table during the initial ALTER TABLE operation.Source: Linters/SAST tools
manabom/src/main/resources/db/migration/V24__add_unique_active_chat_member.sql (1)
1-3: 🩺 Stability & Availability | 🔵 Trivial인덱스 생성 시 테이블 락(Lock) 발생 주의
운영 환경에서 데이터가 많은 테이블에 일반적인 방식으로 인덱스를 생성하면, 인덱스 생성이 완료될 때까지 해당 테이블의 쓰기 작업(Update, Insert, Delete)이 차단되어 서비스 지연 및 가용성 저하가 발생할 수 있습니다.
서비스에 미치는 영향을 최소화하려면
CONCURRENTLY옵션을 사용하여 쓰기 락을 방지하는 것을 고려해 보세요.
단, Flyway와 같은 데이터베이스 마이그레이션 도구는 기본적으로 트랜잭션 내에서 스크립트를 실행합니다.CONCURRENTLY키워드는 트랜잭션 내부에서 사용할 수 없으므로, 이를 적용하려면 해당 마이그레이션 파일이 트랜잭션 없이(Non-transactional) 실행되도록 별도의 설정이나 파일명 변경이 필요할 수 있습니다.CREATE UNIQUE INDEX CONCURRENTLY uk_chat_members_active_room_user ON chat_members (room_id, user_id) WHERE status = 'ACTIVATE';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@manabom/src/main/resources/db/migration/V24__add_unique_active_chat_member.sql` around lines 1 - 3, Update the uk_chat_members_active_room_user migration to create the partial unique index with the CONCURRENTLY option, and configure this migration to run non-transactionally as required by Flyway. Preserve the existing index name, columns, uniqueness, and status filter.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@manabom/src/main/resources/db/migration/V23__remove_withdrawn_cancellation_status.sql`:
- Around line 4-11: Update the chk_cancellation_request_status constraint in the
migration to add it with NOT VALID, then separately validate it using VALIDATE
CONSTRAINT so constraint creation does not scan and lock the entire table during
the initial ALTER TABLE operation.
In
`@manabom/src/main/resources/db/migration/V24__add_unique_active_chat_member.sql`:
- Around line 1-3: Update the uk_chat_members_active_room_user migration to
create the partial unique index with the CONCURRENTLY option, and configure this
migration to run non-transactionally as required by Flyway. Preserve the
existing index name, columns, uniqueness, and status filter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: abf70e11-ce53-4f00-b90e-4e6999b03786
📒 Files selected for processing (9)
manabom/src/main/java/mannabom_server/manabom/application/chat/service/ChatRoomService.javamanabom/src/main/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationExpirationService.javamanabom/src/main/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationService.javamanabom/src/main/java/mannabom_server/manabom/domain/meeting/repository/MeetingCancellationRequestRepository.javamanabom/src/main/resources/db/migration/V23__remove_withdrawn_cancellation_status.sqlmanabom/src/main/resources/db/migration/V24__add_unique_active_chat_member.sqlmanabom/src/test/java/mannabom_server/manabom/application/chat/service/ChatRoomServiceTest.javamanabom/src/test/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationExpirationServiceTest.javamanabom/src/test/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationServiceTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
- manabom/src/main/java/mannabom_server/manabom/domain/meeting/repository/MeetingCancellationRequestRepository.java
- manabom/src/main/java/mannabom_server/manabom/application/chat/service/ChatRoomService.java
- manabom/src/main/java/mannabom_server/manabom/application/meeting/service/MeetingCancellationService.java
kimjuneon
left a comment
There was a problem hiding this comment.
확인했습니다 아래 부분만 한번 확인해주시면 좋을 것 같아요
| expireIfNecessary(request, now); | ||
|
|
||
| if (request.getStatus() != MeetingCancellationStatus.PENDING) { | ||
| throw new IllegalStateException("이미 종료된 미팅 취소 요청입니다."); | ||
| } |
There was a problem hiding this comment.
이미 만료된 요청은 expireIfNecessary()에서 EXPIRED로 변경되지만 바로 다음 상태 검사에서 PENDING 상태인지를 확인합니다. 이렇게 되면 IllegalStateException이 발생하고 @transactional이 걸려있어 EXPIRED로 변경이 롤백될 것 같습니다!
변경 사항
Summary by CodeRabbit