-
Notifications
You must be signed in to change notification settings - Fork 0
Issue: (#166) 약관 동의 API #167
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughMySQL 커넥터의 Gradle 의존성 선언이 변경되었으며, 회원(Member) 엔티티와 온보딩 요청(OnboardingRequest)에 약관 동의 여부(isTermsAgreed) 필드가 추가되었습니다. 온보딩 서비스는 이제 해당 값을 처리하도록 수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant OnboardingService
participant Member
Client->>Controller: OnboardingRequest (isTermsAgreed 포함)
Controller->>OnboardingService: onboarding(request)
OnboardingService->>Member: isTermsAgreed 값 할당
OnboardingService-->>Controller: 온보딩 완료
Controller-->>Client: 응답 반환
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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: 3
🧹 Nitpick comments (2)
build.gradle.kts (1)
38-38: MySQL 커넥터 의존성 업데이트가 올바르지만 버전 명시를 권장합니다.
mysql-connector-java에서mysql-connector-j로의 변경은 올바른 방향입니다. 하지만 버전을 명시하지 않으면 예기치 않은 버전이 사용될 수 있습니다.다음과 같이 명시적인 버전 지정을 고려해보세요:
- runtimeOnly("com.mysql:mysql-connector-j") + runtimeOnly("com.mysql:mysql-connector-j:8.3.0")src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt (1)
53-54: 약관 동의 관련 비즈니스 메서드 추가를 고려해보세요.엔티티 필드 추가는 적절하지만, 약관 동의와 관련된 비즈니스 메서드를 추가하면 도메인 로직을 더 명확하게 표현할 수 있습니다.
다음과 같은 메서드들을 고려해보세요:
fun agreeToTerms() { this.isTermsAgreed = true } fun checkTermsAgreement(): Boolean { return isTermsAgreed }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
build.gradle.kts(1 hunks)src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt(1 hunks)src/main/kotlin/gomushin/backend/member/domain/service/OnboardingService.kt(1 hunks)src/main/kotlin/gomushin/backend/member/dto/request/OnboardingRequest.kt(1 hunks)
| @Column(name = "terms_agreed", nullable = false) | ||
| var isTermsAgreed: Boolean = false, |
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.
💡 Verification agent
🧩 Analysis chain
데이터베이스 마이그레이션 스크립트를 확인해주세요.
새로운 non-nullable 필드가 추가되었으므로 기존 데이터에 대한 마이그레이션이 필요합니다.
다음 스크립트로 마이그레이션 파일이 있는지 확인해보겠습니다:
🏁 Script executed:
#!/bin/bash
# Description: 데이터베이스 마이그레이션 파일 확인
# Expected: terms_agreed 컬럼 추가에 대한 마이그레이션 파일 존재
# Flyway 마이그레이션 파일 검색
fd -e sql . | xargs grep -l "terms_agreed\|isTermsAgreed" || echo "마이그레이션 파일을 찾을 수 없습니다"
# Liquibase 마이그레이션 파일 검색
fd -e xml -e yml -e yaml . | xargs grep -l "terms_agreed\|isTermsAgreed" || echo "Liquibase 마이그레이션 파일을 찾을 수 없습니다"Length of output: 256
데이터베이스 마이그레이션 스크립트 추가 필요
새로 추가된 non-nullable 필드(isTermsAgreed: Boolean = false)에 대한 마이그레이션 파일이 현재 확인되지 않습니다. 기존 테이블에 해당 컬럼을 추가하고 기본값을 설정하려면 아래 사항을 반영해주세요.
• 대상 코드
- 파일:
src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt - 위치: 53–54라인 (
@Column(name = "terms_agreed", nullable = false) var isTermsAgreed: Boolean = false)
• 추가할 마이그레이션 예시 (Flyway SQL)
-- V20250801__add_terms_agreed.sql
ALTER TABLE member
ADD COLUMN terms_agreed BOOLEAN NOT NULL DEFAULT FALSE;• Liquibase 사용 시 예시 (XML)
<changeSet id="20250801-add-terms-agreed" author="yourname">
<addColumn tableName="member">
<column name="terms_agreed" type="BOOLEAN" defaultValueBoolean="false" />
</addColumn>
</changeSet>위와 같이 Flyway 또는 Liquibase 마이그레이션 파일을 작성·등록하여 기존 데이터가 false로 설정되도록 처리해주세요.
🤖 Prompt for AI Agents
In src/main/kotlin/gomushin/backend/member/domain/entity/Member.kt at lines
53-54, you added a new non-nullable Boolean field isTermsAgreed with a default
value false, but there is no corresponding database migration script. To fix
this, create and register a migration file (using Flyway or Liquibase) that adds
the terms_agreed column to the member table with NOT NULL and DEFAULT FALSE,
ensuring existing records get the default value without errors.
| member.birthDate = onboardingRequest.birthDate | ||
| member.role = Role.MEMBER | ||
| member.fcmToken = onboardingRequest.fcmToken | ||
| member.isTermsAgreed = onboardingRequest.isTermsAgreed |
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.
🛠️ Refactor suggestion
약관 동의 필수 검증 로직 추가를 고려해보세요.
약관 동의는 중요한 비즈니스 로직이므로, 단순한 할당보다는 검증 로직을 추가하는 것이 좋겠습니다.
다음과 같은 검증 로직을 고려해보세요:
- member.isTermsAgreed = onboardingRequest.isTermsAgreed
+ if (!onboardingRequest.isTermsAgreed) {
+ throw BadRequestException("sarangggun.member.terms-agreement-required")
+ }
+ member.isTermsAgreed = onboardingRequest.isTermsAgreed📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| member.isTermsAgreed = onboardingRequest.isTermsAgreed | |
| if (!onboardingRequest.isTermsAgreed) { | |
| throw BadRequestException("sarangggun.member.terms-agreement-required") | |
| } | |
| member.isTermsAgreed = onboardingRequest.isTermsAgreed |
🤖 Prompt for AI Agents
In src/main/kotlin/gomushin/backend/member/domain/service/OnboardingService.kt
at line 23, the code directly assigns the terms agreement status without
validation. Add a validation step before assignment to ensure that the user has
agreed to the terms, and if not, throw an appropriate exception or handle the
error to enforce mandatory agreement.
| @Schema(description = "약관 동의 여부", example = "true") | ||
| val isTermsAgreed: Boolean |
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.
🛠️ Refactor suggestion
약관 동의 필드에 validation 추가를 고려해보세요.
약관 동의는 필수 항목일 가능성이 높으므로 validation을 추가하는 것이 좋겠습니다.
다음과 같은 validation을 고려해보세요:
+import jakarta.validation.constraints.AssertTrue
+
@Schema(description = "약관 동의 여부", example = "true")
+@AssertTrue(message = "약관에 동의해야 합니다")
val isTermsAgreed: Boolean또는 커스텀 validation을 사용할 수도 있습니다:
+@ValidTermsAgreement
val isTermsAgreed: Boolean🤖 Prompt for AI Agents
In src/main/kotlin/gomushin/backend/member/dto/request/OnboardingRequest.kt at
lines 19-20, the isTermsAgreed Boolean field lacks validation to ensure it is
true, which is important since agreeing to terms is likely mandatory. Add a
validation annotation such as @AssertTrue to this field to enforce that the user
must agree to the terms. If needed, implement a custom validator to provide more
specific validation logic or error messages.
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.
validation은 프론트엔드에서 처리할거야.
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.
@HoyeongJeon 알겠습니다! 프론트엔드에서 validation을 처리하시는 방향으로 진행하시는군요. 현재 구현된 코드 그대로 두시면 됩니다.
(´•̥̥̥ω•̥̥̥`)
✏️ Learnings added
Learnt from: HoyeongJeon
PR: Gomushim/backend#167
File: src/main/kotlin/gomushin/backend/member/dto/request/OnboardingRequest.kt:19-20
Timestamp: 2025-08-02T07:38:57.362Z
Learning: In the Gomushim backend project, validation for terms agreement (isTermsAgreed) is handled on the frontend rather than adding server-side validation annotations to the OnboardingRequest DTO.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
✅ PR 유형
어떤 변경 사항이 있었나요?
📝 작업 내용
✏️ 관련 이슈
🎸 기타 사항 or 추가 코멘트
만약 약관이 여러개라면 약관 늘리거나 따로 embeddable로 분리하거나 그럴 예정
Summary by CodeRabbit
신규 기능
버그 수정