Skip to content

[FEAT] 회원가입 필수 약관 동의 및 기존 회원 동의 관리 - #429

Open
nuj1min wants to merge 3 commits into
developfrom
feat/428-terms-agreement
Open

nuj1min wants to merge 3 commits into
developfrom
feat/428-terms-agreement

Conversation

@nuj1min

@nuj1min nuj1min commented Nov 2, 2026 •

Copy link
Copy Markdown
Collaborator

📌 PR 설명

  • 이메일 회원가입 및 카카오·네이버 추가 프로필 완료 시 필수 약관 동의 검증을 추가했습니다.
    • 서비스 이용약관(SERVICE)
    • 개인정보 수집·이용 동의(PRIVACY)
  • 약관 전문과 버전을 관리하고, 사용자별 동의 버전 및 동의 시각을 저장하도록 구현했습니다.
  • 현재 약관 조회, 내 동의 상태 조회, 필수 약관 동의 API를 추가했습니다.
  • 로그인 및 토큰 재발급 응답에 termsAgreementRequired를 추가하여 기존 회원의 동의 필요 여부를 확인할 수 있도록 했습니다.
  • 동일 버전의 동의 요청을 반복해도 최초 동의 시각을 유지하도록 처리했습니다.
  • 프론트 연동 및 운영 약관 등록 절차를 문서화했습니다.

🔗 연관된 이슈

Resolves: #428

📝 리뷰 요구사항

  • 이메일·소셜 회원가입 시 필수 동의 누락 및 이전 약관 버전 요청이 정상적으로 차단되는지 확인 부탁드립니다.
  • 기존 회원의 최초 동의와 약관 개정 후 재동의 필요 여부가 올바르게 반환되는지 확인 부탁드립니다.
  • 반복 요청 시 동의 이력이 중복 저장되지 않고 최초 동의 시각이 유지되는지 확인 부탁드립니다.
  • 기존 회원의 동의 화면 이동은 프론트 연동이 필요하며, 이번 변경에서 기존 회원의 전체 API 접근을 차단하지는 않습니다.
  • 배포 전 확정된 약관 전문 두 개를 등록해야 합니다. 미등록 상태에서는 신규 회원가입이 차단됩니다.

💻 테스트 결과

  • API 테스트 완료
  • 정상 동작 확인

Summary by CodeRabbit

  • New Features
    • Added a way to view current service and privacy terms, check agreement status, and submit consent.
    • Email and social sign-up now require consent to current terms. Login responses indicate when agreement is required.
    • Added validation for missing or outdated consent and clear responses when terms are unavailable.
  • Documentation
    • Added Korean guidance on consent flows, validation, and terms management. Existing members are not automatically marked as having agreed.

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Important

  • Trigger review

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.

Changes: Terms consent flow

Layer / File(s) Summary
Terms document and agreement storage
src/main/java/com/yeogido/backend/domain/terms/TermsType.java, src/main/java/com/yeogido/backend/domain/terms/TermsDocument.java, src/main/java/com/yeogido/backend/domain/terms/TermsDocumentRepository.java, src/main/java/com/yeogido/backend/domain/terms/UserTermsAgreement.java, src/main/java/com/yeogido/backend/domain/terms/UserTermsAgreementRepository.java, src/main/resources/db/migration/V40__create_terms_agreement.sql
Adds SERVICE and PRIVACY document types, versioned terms storage, user agreement records, repository queries, and database constraints.
Terms validation and APIs
src/main/java/com/yeogido/backend/domain/terms/TermsDTO.java, src/main/java/com/yeogido/backend/domain/terms/TermsErrorCode.java, src/main/java/com/yeogido/backend/domain/terms/TermsService.java, src/main/java/com/yeogido/backend/domain/terms/TermsController.java, src/main/java/com/yeogido/backend/domain/user/repository/UserRepository.java, src/main/java/com/yeogido/backend/global/config/SecurityConfig.java, src/test/java/com/yeogido/backend/domain/terms/TermsControllerTest.java, src/test/java/com/yeogido/backend/domain/terms/TermsIntegrationTest.java
Adds current-terms lookup, agreement validation and recording, status reporting, and consent endpoints. Tests cover access control, validation, current-version changes, and agreement history.
Signup and authentication integration
src/main/java/com/yeogido/backend/domain/auth/dto/AuthReqDTO.java, src/main/java/com/yeogido/backend/domain/auth/dto/AuthResDTO.java, src/main/java/com/yeogido/backend/domain/auth/service/AuthServiceImpl.java, src/main/java/com/yeogido/backend/domain/auth/service/JwtTokenProvider.java, src/main/java/com/yeogido/backend/domain/auth/converter/AuthConverter.java, src/test/java/com/yeogido/backend/domain/terms/TermsIntegrationTest.java, docs/terms-agreement.md
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.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

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: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning 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 Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title clearly identifies the main change: required terms agreement during signup and agreement management for existing members.
Description check Passed 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 Passed 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 Passed 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):

  1. 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.
  2. 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.
  3. 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.
  4. 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.
  5. 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.
  6. 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:

  • Schema: V40__create_terms_agreement.sql, TermsType, TermsDocument, TermsDocumentRepository, UserTermsAgreement, UserTermsAgreementRepository.
  • Terms domain: TermsService, TermsDTO, TermsErrorCode, TermsController.
  • Signup and login: AuthReqDTO (agreements field), AuthResDTO (termsAgreementRequired), AuthServiceImpl, AuthConverter, JwtTokenProvider.
  • 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.

Share

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.
Recent review info
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: 38a95bd4-d9fb-4578-a623-b810d243d06e
Commits

Reviewing files that changed from the base of the PR and between 8987c2e and 9f8c67b.

Files selected for processing: 20
  • docs/terms-agreement.md
  • src/main/java/com/yeogido/backend/domain/auth/converter/AuthConverter.java
  • src/main/java/com/yeogido/backend/domain/auth/dto/AuthReqDTO.java
  • src/main/java/com/yeogido/backend/domain/auth/dto/AuthResDTO.java
  • src/main/java/com/yeogido/backend/domain/auth/service/AuthServiceImpl.java
  • src/main/java/com/yeogido/backend/domain/auth/service/JwtTokenProvider.java
  • src/main/java/com/yeogido/backend/domain/terms/TermsController.java
  • src/main/java/com/yeogido/backend/domain/terms/TermsDTO.java
  • src/main/java/com/yeogido/backend/domain/terms/TermsDocument.java
  • src/main/java/com/yeogido/backend/domain/terms/TermsDocumentRepository.java
  • src/main/java/com/yeogido/backend/domain/terms/TermsErrorCode.java
  • src/main/java/com/yeogido/backend/domain/terms/TermsService.java
  • src/main/java/com/yeogido/backend/domain/terms/TermsType.java
  • src/main/java/com/yeogido/backend/domain/terms/UserTermsAgreement.java
  • src/main/java/com/yeogido/backend/domain/terms/UserTermsAgreementRepository.java
  • src/main/java/com/yeogido/backend/domain/user/repository/UserRepository.java
  • src/main/java/com/yeogido/backend/global/config/SecurityConfig.java
  • src/main/resources/db/migration/V40__create_terms_agreement.sql
  • src/test/java/com/yeogido/backend/domain/terms/TermsControllerTest.java
  • src/test/java/com/yeogido/backend/domain/terms/TermsIntegrationTest.java

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 nuj1min changed the title feat: 회원가입 필수 약관 동의 및 기존 회원 동의 관리 #428 [FEAT] 회원가입 필수 약관 동의 및 기존 회원 동의 관리 Nov 2, 2026
@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Action performed

Review finished.

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.

@yangjiae12

Copy link
Copy Markdown
Member

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Action not completed

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.

@yangjiae12 yangjiae12 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코멘트 확인 해주세요!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 dto 파일에 요청이랑 응답이 함께 들어가 있는 거 같은데, 기존처럼 분리해주세요!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추가로 패키지 구조도 기존 도메인에 맞춰서 작성 부탁드립니다! Document.from()은 컨버터로 분리하면 될 거 같아요

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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에 반영했으며 전체 테스트 통과했습니다. 현재 아직 푸시 전입니다.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기존 서비스 구조에 맞춰서 인터페이스랑 구현체로 분리해주세요~

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀해 주신 기존 서비스 구조에 맞춰 TermsService는 인터페이스로, TermsServiceImpl은 구현체로 분리했습니다. 호출부는 인터페이스를 참조하고, 구현체의 트랜잭션 설정은 유지했습니다.

로컬 커밋 a8d9413에 반영했으며 전체 테스트 통과했습니다. 현재 아직 푸시 전입니다.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

찬결 님 pr에서도 v40 쓰고 있어서 머지 순서 맞춰서 번호 조정해주세요! 민준 님이 먼저 머지하시면 찬결 님한테 말씀해주시면 될 거 같습니다~

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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가 먼저 반영되도록 머지·배포 순서를 맞추겠습니다.

변경은 로컬 커밋 a8d9413에 반영되어 있으며 아직 푸시 전입니다.

issueAccessToken(user, sessionId),
issueRefreshToken(user, sessionId)
issueRefreshToken(user, sessionId),
termsService.isAgreementRequired(user.getId())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 토큰 발급 과정에서 약관 동의 상태도 함께 조회하고 있는데, 이 부분은 AuthServiceImpl에서 조회해서 응답에 조합하는 방식도 괜찮을 것 같습니다! 문제가 있는 건 아니고 JwtTokenProvider가 토큰 생성에만 집중하도록 하면 좋을 것 같다는 정도여서 구조상 더 적절하다고 판단되시면 반영해주세요~!

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀해 주신 방향이 책임 분리 측면에서 더 적절하다고 판단하여 반영했습니다! JwtTokenProvider는 IssuedTokens로 Access/Refresh Token만 반환하고, AuthServiceImpl에서 약관 동의 상태를 조회한 뒤 AuthConverter를 통해 응답을 조합하도록 변경했습니다. 로그인·소셜 가입 완료·토큰 재발급의 기존 응답 구조와 termsAgreementRequired 동작은 유지했습니다. 로컬 테스트용 토큰 발급 경로도 함께 수정했습니다.

로컬 커밋 a8d9413에 반영했으며 전체 테스트 통과했습니다. 현재 아직 푸시 전입니다.

@yangjiae12 yangjiae12 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

terms-agreement.md 파일도 작성해주셨는데, 저희가 원래 명세는 노션에다 정리했으니까 해당 파일은 지우고 노션에 API 명세서 적는 걸로 해도 괜찮을까요? 아직 노션에 명세서 안 적어주신 거 같아서 말씀드립니다! 그리고 테스트 코드만 실행해보신 거면 437번 pr 머지되고 나서 브랜치 최신화 한 다음에 스웨거로도 테스트 부탁드립니다~

@nuj1min

nuj1min commented Nov 3, 2026

Copy link
Copy Markdown
Collaborator Author

넵! 명세는 기존처럼 노션에서 관리하는 방향으로 맞추겠습니다. docs/terms-agreement.md는 삭제했고, 기존 로컬 개발자료 양식에 맞춰 신규 약관 API 3개와 회원가입·로그인·토큰 재발급 변경 사항을 정리했습니다. 노션에 옮길 수 있도록 복붙용 명세도 준비했습니다. 개발자료 파일은 커밋에 포함하지 않았습니다.

앞서 말씀드린 코드 리뷰 수정(a8d9413)과 이번 문서 삭제(938f8e0)는 푸시했습니다. #437 머지 후 브랜치를 최신화하고 Swagger에서도 테스트하여 결과 공유하겠습니다!

@yangjiae12 yangjiae12 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

브랜치 최신화 해서 스웨거로 실행시켜보고 머지해주세요~ 우선 승인하겠습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEAT] 회원가입 필수 약관 동의 및 기존 회원 동의 관리

2 participants