You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This repository does not receive automatic reviews because it has fewer than 10 stars.
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 46fc8765-a412-42ed-ad87-bbe8d5f3d1e4
No actionable comments were generated in the recent review.
Walkthrough
This change adds versioned SERVICE and PRIVACY terms, consent APIs, and agreement history. Email and social signup now require current-version consent. Authentication responses include whether the user must agree to current terms.
Adds current-terms lookup, agreement validation and recording, status reporting, and consent endpoints. Tests cover access control, validation, current-version changes, and agreement history.
Adds required agreements to email and social signup requests, records validated consent, and reports whether current terms agreement is required in authentication responses. Integration tests and documentation describe signup and existing-member flows.
sequenceDiagram
participant AuthServiceImpl
participant TermsService
participant UserTermsAgreementRepository
AuthServiceImpl->>TermsService: Validate submitted agreements
AuthServiceImpl->>TermsService: Record validated agreements for the new user
TermsService->>UserTermsAgreementRepository: Check and save user-document agreements
Loading
Merge Risk: Minimal · previous review 9f8c6
There is no identified code issue blocking merge. Finalized SERVICE and PRIVACY text must still be registered before deployment, as the PR specifies.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
The title clearly identifies the main change: required terms agreement during signup and agreement management for existing members.
Description check
The description follows the required template. It explains the changes, links issue #428, lists review requirements, and reports API testing. The normal behavior checkbox is unchecked, but the descrip…
Linked Issues check
Issue #428 is the only direct linked issue. Its coding requirements, checked against the whole-PR diff (mergeBase 8987c2e to head 9f8c67b):
1. Versioned terms and agreement-history schema. Met. Migra…
Out of Scope Changes check
Every changed file maps to an issue #428 objective or to its required support: - Schema: V40__create_terms_agreement.sql, TermsType, TermsDocument, TermsDocumentRepository, UserTermsAgreement, UserTer…
Full details: Description check
Explanation
The description follows the required template. It explains the changes, links issue #428, lists review requirements, and reports API testing. The normal behavior checkbox is unchecked, but the description is otherwise complete and relevant.
Full details: Linked Issues check
Explanation
Issue #428 is the only direct linked issue. Its coding requirements, checked against the whole-PR diff (mergeBase 8987c2e to head 9f8c67b):
Versioned terms and agreement-history schema. Met. Migration V40 creates terms_document (unique (type, version) and (type, effective_at)) and user_terms_agreement (unique (user_id, terms_document_id)). The JPA entities TermsDocument and UserTermsAgreement match.
Public current-terms lookup and user status or submit APIs. Met. TermsController adds current(), status(), and agree(). SecurityConfig adds /api/v1/terms to PUBLIC_GET_PATHS, so current terms are public.
Required consent and latest-version validation for email and social signup. Met. TermsService.validate requires exactly one agreed entry per TermsType, rejects missing, false, duplicate, or blank entries (REQUIRED_AGREEMENT), and rejects non-current versions (VERSION_MISMATCH). AuthServiceImpl.signUp and completeSocialSignup call validate before the existing signup flow and call record after user creation.
Agreement-needed flag in login and token-refresh responses. Met. AuthResDTO.Token and SocialLogin add termsAgreementRequired. JwtTokenProvider.issueToken sets it from TermsService.isAgreementRequired. AuthConverter passes it through; new social signup returns true.
Tests for existing member, repeated request, and revised terms. Met. TermsIntegrationTest covers idempotent agree with the original agreedAt kept, a new version requiring re-consent with history preserved, existing email and social login, and reissue reflecting current consent. TermsControllerTest covers endpoint access and 400 on missing fields.
Frontend integration and terms-registration guidance. Met. docs/terms-agreement.md is added (+94 lines).
Non-coding items (manual operations, terms text approval) need no code verdict. Marketing consent is excluded, as the issue requires. No backfill occurs; the migration comment forbids it.
Limits: I read TermsService, AuthServiceImpl, and TermsIntegrationTest directly. Other items rely on the change summary. I did not run the test suite, so test pass status is not established here. The PR checklist marks normal-behavior verification as unchecked. That item does not change the coding verdict.
Full details: Out of Scope Changes check
Explanation
Every changed file maps to an issue #428 objective or to its required support:
Locking: UserRepository.findByIdForTermsAgreement, used by TermsService.agree to serialize retries.
Public access: SecurityConfig adds GET /api/v1/terms, required for anonymous current-terms lookup.
Tests and docs: TermsControllerTest, TermsIntegrationTest, docs/terms-agreement.md.
No unrelated feature, refactor, or marketing consent appears in the summary. The docs file states that marketing and location consent are out of scope. The change does not block existing members from the wider API. The issue does not require that, and the PR states it.
Full details: Docstring Coverage
Explanation
Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 9 functions across 18 files. (2 skipped: 2 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Tip
🎯 Customize your risk assessments
Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.
✅ Automatic approvals, with your rules
Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.
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.
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
nuj1min
changed the title
feat: 회원가입 필수 약관 동의 및 기존 회원 동의 관리 #428
[FEAT] 회원가입 필수 약관 동의 및 기존 회원 동의 관리
Nov 2, 2026
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
Already reviewed the last commit. Use @coderabbitai full review to rerun a review of the entire changeset.
Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.
The reason will be displayed to describe this comment to others. Learn more.
리뷰 감사합니다! 요청 DTO는 TermsReqDTO, 응답 DTO는 TermsResDTO로 분리했습니다. 추가로 말씀해 주신 패키지도 기존 도메인 구조에 맞춰 controller, dto, entity, repository, service, enums, exception, converter로 정리했고, Document.from()은 TermsConverter.toDocument()로 옮겼습니다. 기존 API 요청·응답 필드는 유지했습니다.
로컬 커밋 a8d9413에 반영했으며 전체 테스트 통과했습니다. 현재 아직 푸시 전입니다.
The reason will be displayed to describe this comment to others. Learn more.
확인 감사합니다! 최신 develop에 V40__create_friendship.sql이 반영되어 있고, 장소 설명 PR #437에서 V41·V42를 사용 중인 것을 확인하여 약관 마이그레이션을 V43__create_terms_agreement.sql로 변경했습니다. 관련 문서도 수정했으며, V41·V42가 먼저 반영되도록 머지·배포 순서를 맞추겠습니다.
The reason will be displayed to describe this comment to others. Learn more.
현재 토큰 발급 과정에서 약관 동의 상태도 함께 조회하고 있는데, 이 부분은 AuthServiceImpl에서 조회해서 응답에 조합하는 방식도 괜찮을 것 같습니다! 문제가 있는 건 아니고 JwtTokenProvider가 토큰 생성에만 집중하도록 하면 좋을 것 같다는 정도여서 구조상 더 적절하다고 판단되시면 반영해주세요~!
The reason will be displayed to describe this comment to others. Learn more.
말씀해 주신 방향이 책임 분리 측면에서 더 적절하다고 판단하여 반영했습니다! JwtTokenProvider는 IssuedTokens로 Access/Refresh Token만 반환하고, AuthServiceImpl에서 약관 동의 상태를 조회한 뒤 AuthConverter를 통해 응답을 조합하도록 변경했습니다. 로그인·소셜 가입 완료·토큰 재발급의 기존 응답 구조와 termsAgreementRequired 동작은 유지했습니다. 로컬 테스트용 토큰 발급 경로도 함께 수정했습니다.
로컬 커밋 a8d9413에 반영했으며 전체 테스트 통과했습니다. 현재 아직 푸시 전입니다.
The reason will be displayed to describe this comment to others. Learn more.
terms-agreement.md 파일도 작성해주셨는데, 저희가 원래 명세는 노션에다 정리했으니까 해당 파일은 지우고 노션에 API 명세서 적는 걸로 해도 괜찮을까요? 아직 노션에 명세서 안 적어주신 거 같아서 말씀드립니다! 그리고 테스트 코드만 실행해보신 거면 437번 pr 머지되고 나서 브랜치 최신화 한 다음에 스웨거로도 테스트 부탁드립니다~
넵! 명세는 기존처럼 노션에서 관리하는 방향으로 맞추겠습니다. docs/terms-agreement.md는 삭제했고, 기존 로컬 개발자료 양식에 맞춰 신규 약관 API 3개와 회원가입·로그인·토큰 재발급 변경 사항을 정리했습니다. 노션에 옮길 수 있도록 복붙용 명세도 준비했습니다. 개발자료 파일은 커밋에 포함하지 않았습니다.
앞서 말씀드린 코드 리뷰 수정(a8d9413)과 이번 문서 삭제(938f8e0)는 푸시했습니다. #437 머지 후 브랜치를 최신화하고 Swagger에서도 테스트하여 결과 공유하겠습니다!
The reason will be displayed to describe this comment to others. Learn more.
브랜치 최신화 해서 스웨거로 실행시켜보고 머지해주세요~ 우선 승인하겠습니다!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 PR 설명
SERVICE)PRIVACY)termsAgreementRequired를 추가하여 기존 회원의 동의 필요 여부를 확인할 수 있도록 했습니다.🔗 연관된 이슈
Resolves: #428
📝 리뷰 요구사항
💻 테스트 결과
Summary by CodeRabbit