-
Notifications
You must be signed in to change notification settings - Fork 0
✨ feat: 회원 탈퇴 API 구현 (API-028) #57
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
Conversation
- DELETE /api/v1/members/me 엔드포인트 추가 - 회원 삭제 시 연관 데이터(답변, 회고 등) 유지 (ON DELETE SET NULL) - member_id를 Option<i64>로 변경하여 탈퇴 멤버 처리 - 트랜잭션으로 refresh_token과 member 삭제 원자적 처리 - Terraform S3 backend 마이그레이션 (v2) Co-Authored-By: Claude Opus 4.5 <[email protected]>
Terraform Format 🖌
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a member withdrawal API (handler, service, DTO, tests) and OpenAPI docs; makes member-related DB foreign keys nullable (member_id -> Option and on_delete -> SetNull); updates retrospect flows to handle optional member_id; adds AppError::MemberNotFound. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Handler as WithdrawHandler
participant Service as MemberService
participant DB as Database
participant Responder as Responder
Client->>Handler: DELETE /api/v1/members/me (Authorization: Bearer <token>)
Handler->>Handler: extract member_id from token
alt token invalid / parse fails
Handler->>Responder: 401 AUTH4001
else token parsed
Handler->>Service: withdraw(state, member_id)
Service->>DB: BEGIN TRANSACTION
Service->>DB: SELECT member WHERE id = ?
alt no member found
Service->>Responder: 404 MEMBER4042
Service->>DB: ROLLBACK
else member exists
Service->>DB: DELETE FROM refresh_tokens WHERE member_id = ?
Service->>DB: DELETE FROM members WHERE id = ?
Note over DB: FK-related fields set to NULL (ON DELETE SET NULL)
Service->>DB: COMMIT
Service->>Responder: 200 COMMON200 (SuccessWithdrawResponse)
end
end
Responder->>Client: JSON response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
Terraform Format 🖌
|
Terraform Format 🖌
|
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
codes/server/src/domain/retrospect/service.rs (1)
2467-2491:⚠️ Potential issue | 🟠 MajorQuestion ordering breaks when all member_id values are NULL.
If all
member_responserows haveNULLmember_id,member_response_mapbecomes empty andquestion_textsis empty, so category-specific queries return nothing even though responses exist. Add a fallback to derive question order fromall_responses.🛠️ Suggested fix
// member_id별로 그룹화하여 첫 번째 멤버의 응답 세트 확인 let mut member_response_map: HashMap<i64, Vec<i64>> = HashMap::new(); for mr in &first_member_responses { if let Some(member_id) = mr.member_id { member_response_map .entry(member_id) .or_default() .push(mr.response_id); } } - // 첫 번째 멤버의 응답 ID 목록 (오름차순 정렬됨) - let first_member_id = member_response_map.keys().min().copied(); - let question_response_ids: Vec<i64> = first_member_id - .and_then(|mid| member_response_map.get(&mid)) - .cloned() - .unwrap_or_default(); - - // 질문 텍스트 순서를 response_id 순으로 매핑 - let response_map: HashMap<i64, &response::Model> = - all_responses.iter().map(|r| (r.response_id, r)).collect(); - - let question_texts: Vec<String> = question_response_ids - .iter() - .filter_map(|rid| response_map.get(rid).map(|r| r.question.clone())) - .collect(); + // 질문 텍스트 순서를 response_id 순으로 매핑 + let response_map: HashMap<i64, &response::Model> = + all_responses.iter().map(|r| (r.response_id, r)).collect(); + + let question_texts: Vec<String> = if let Some(first_member_id) = + member_response_map.keys().min().copied() + { + member_response_map + .get(&first_member_id) + .into_iter() + .flatten() + .filter_map(|rid| response_map.get(rid).map(|r| r.question.clone())) + .collect() + } else { + let mut seen = HashSet::new(); + all_responses + .iter() + .filter(|r| seen.insert(r.question.clone())) + .map(|r| r.question.clone()) + .collect() + };
🤖 Fix all issues with AI agents
In `@codes/server/src/domain/retrospect/service.rs`:
- Around line 1931-1934: participant_names is built using filter_map which drops
entries with member_id == None, causing withdrawn members to be omitted; change
the logic in the participant_names construction to map each member_retros entry
to a String: if mr.member_id is Some(id) then lookup
member_map.get(&id).cloned().unwrap_or("탈퇴한 멤버".into()) else use "탈퇴한
멤버".into(), so every member_retros produces a name (either the found name or the
placeholder) instead of being filtered out.
- Around line 2255-2258: The code collects member_ids from submitted_members
into member_ids and later builds members_data then calls analyze_retrospective;
add a guard before the analyze_retrospective invocation to check that
members_data (or member_ids) is not empty and return a data-insufficient error
(e.g., Err(...) or a DomainError variant) instead of calling
analyze_retrospective with empty input. Locate the block that computes
member_ids and the subsequent call to analyze_retrospective and insert a
validation that returns early with a clear data-insufficient error when
member_ids.is_empty() (or members_data.is_empty()) to prevent running AI
analysis on empty data.
Terraform Format 🖌
|
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
codes/server/src/domain/retrospect/service.rs (1)
2482-2505:⚠️ Potential issue | 🟠 MajorFix: category filtering breaks when all member_id are NULL.
If allmember_idvalues areNone(withdrawn members),member_response_mapis empty,question_textsbecomes empty, and category queries return empty despite existing responses. Add a fallback that derives question order fromall_responseswhen the map is empty.🛠️ Suggested fix (fallback when no member-response mapping)
- // 질문 텍스트 순서를 response_id 순으로 매핑 - let response_map: HashMap<i64, &response::Model> = - all_responses.iter().map(|r| (r.response_id, r)).collect(); - - let question_texts: Vec<String> = question_response_ids - .iter() - .filter_map(|rid| response_map.get(rid).map(|r| r.question.clone())) - .collect(); + // 질문 텍스트 순서를 response_id 순으로 매핑 + let response_map: HashMap<i64, &response::Model> = + all_responses.iter().map(|r| (r.response_id, r)).collect(); + + let question_texts: Vec<String> = if !question_response_ids.is_empty() { + question_response_ids + .iter() + .filter_map(|rid| response_map.get(rid).map(|r| r.question.clone())) + .collect() + } else { + let mut seen_questions = HashSet::new(); + all_responses + .iter() + .filter(|r| seen_questions.insert(r.question.clone())) + .take(QUESTIONS_PER_RETROSPECT) + .map(|r| r.question.clone()) + .collect() + };
🧹 Nitpick comments (1)
codes/server/src/domain/retrospect/service.rs (1)
1372-1411: Consider whether withdrawn members should appear in detail responses.
filter_mapdropsNoneand missing member rows, so participants can disappear from the API response. If the UI needs visibility of withdrawn members, consider a placeholder name and makingRetrospectMemberItem.member_idoptional.
Terraform Format 🖌
|
Summary
/api/v1/members/me회원 탈퇴 API 추가Changes
member_id를Option<i64>로 변경하여 탈퇴 멤버 처리Test plan
DB Migration Required
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.