Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import org.springframework.http.ResponseEntity
import org.springframework.security.core.annotation.AuthenticationPrincipal
import org.springframework.web.bind.annotation.*
import org.yapp.apis.auth.dto.request.SocialLoginRequest
import org.yapp.apis.auth.dto.request.TermsAgreementRequest
import org.yapp.apis.auth.dto.request.TokenRefreshRequest
import org.yapp.apis.auth.dto.response.AuthResponse
import org.yapp.apis.auth.dto.response.UserProfileResponse
Expand Down Expand Up @@ -43,4 +44,13 @@ class AuthController(
val userProfile = authUseCase.getUserProfile(userId)
return ResponseEntity.ok(userProfile)
}

@PutMapping("/terms-agreement")
override fun updateTermsAgreement(
@AuthenticationPrincipal userId: UUID,
@Valid @RequestBody request: TermsAgreementRequest
): ResponseEntity<UserProfileResponse> {
val userProfile = authUseCase.updateTermsAgreement(userId, request.validTermsAgreed())
return ResponseEntity.ok(userProfile)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import org.springframework.http.ResponseEntity
import org.springframework.security.core.annotation.AuthenticationPrincipal
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.PostMapping
import org.springframework.web.bind.annotation.PutMapping
import org.springframework.web.bind.annotation.RequestBody
import org.yapp.apis.auth.dto.request.SocialLoginRequest
import org.yapp.apis.auth.dto.request.TermsAgreementRequest
import org.yapp.apis.auth.dto.request.TokenRefreshRequest
import org.yapp.apis.auth.dto.response.AuthResponse
import org.yapp.apis.auth.dto.response.UserProfileResponse
Expand Down Expand Up @@ -105,4 +107,25 @@ interface AuthControllerApi {
)
@GetMapping("/me")
fun getUserProfile(@AuthenticationPrincipal userId: UUID): ResponseEntity<UserProfileResponse>

@Operation(summary = "Update terms agreement", description = "Updates the user's terms agreement status")
@ApiResponses(
value = [
ApiResponse(
responseCode = "200",
description = "Terms agreement status updated successfully",
content = [Content(schema = Schema(implementation = UserProfileResponse::class))]
),
ApiResponse(
responseCode = "404",
description = "User not found",
content = [Content(schema = Schema(implementation = ErrorResponse::class))]
)
]
)
@PutMapping("/terms-agreement")
fun updateTermsAgreement(
@AuthenticationPrincipal userId: UUID,
@Valid @RequestBody request: TermsAgreementRequest
): ResponseEntity<UserProfileResponse>
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package org.yapp.apis.auth.dto.request

import io.swagger.v3.oas.annotations.media.Schema
import jakarta.validation.constraints.NotNull

@Schema(description = "Request to update terms agreement status")
data class TermsAgreementRequest private constructor(
@Schema(description = "Whether the user agrees to the terms of service", example = "true", required = true)
val termsAgreed: Boolean? = null

Comment on lines +8 to +10
Copy link
Member

Choose a reason for hiding this comment

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

valid 로직이 빠진 것 같아요!!

Copy link
Member Author

Choose a reason for hiding this comment

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

valid validTermsAgreed() 여기있어요!!~


) {
fun validTermsAgreed(): Boolean = termsAgreed!!
Comment on lines +7 to +13
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

null 안전성 문제와 검증 로직을 개선해야 합니다.

현재 구현에는 심각한 문제가 있습니다:

  1. termsAgreed 필드가 nullable이지만 적절한 검증 어노테이션이 없습니다
  2. validTermsAgreed() 메서드에서 !! 연산자로 강제 언래핑하면 NullPointerException이 발생할 수 있습니다

다음과 같이 수정하는 것을 권장합니다:

+import jakarta.validation.constraints.NotNull

 @Schema(description = "Request to update terms agreement status")
-data class TermsAgreementRequest private constructor(
+data class TermsAgreementRequest(
-    @Schema(description = "Whether the user agrees to the terms of service", example = "true", required = true)
-    val termsAgreed: Boolean? = null
+    @field:NotNull(message = "약관 동의 여부는 필수입니다")
+    @Schema(description = "Whether the user agrees to the terms of service", example = "true", required = true)
+    val termsAgreed: Boolean?
-
-
-) {
-    fun validTermsAgreed(): Boolean = termsAgreed!!
-}
+)

그리고 컨트롤러에서는 request.termsAgreed!! 대신 request.termsAgreed (검증이 보장되므로)를 사용하세요.

🤖 Prompt for AI Agents
In apis/src/main/kotlin/org/yapp/apis/auth/dto/request/TermsAgreementRequest.kt
around lines 7 to 13, the termsAgreed field is nullable without validation, and
validTermsAgreed() uses !! which risks NullPointerException. Fix this by making
termsAgreed non-nullable and adding appropriate validation annotations to ensure
it is always provided. Then update validTermsAgreed() to safely return the value
without forced unwrapping. Also, adjust controller code to use
request.termsAgreed directly without !! since validation guarantees non-null.

}
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,22 @@ data class UserProfileResponse(
description = "Social login provider type",
example = "KAKAO"
)
val provider: ProviderType
val provider: ProviderType,

@Schema(
description = "Whether the user has agreed to the terms of service",
example = "false"
)
val termsAgreed: Boolean
) {
companion object {
fun from(userProfileVO: UserProfileVO): UserProfileResponse {
return UserProfileResponse(
id = userProfileVO.id.value,
email = userProfileVO.email.value,
nickname = userProfileVO.nickname,
provider = userProfileVO.provider
provider = userProfileVO.provider,
termsAgreed = userProfileVO.termsAgreed
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ class UserAuthService(
return UserProfileResponse.from(userProfile)
}

fun updateTermsAgreement(userId: UUID, termsAgreed: Boolean): UserProfileResponse {
validateUserExists(userId)
val updatedUserProfile = userDomainService.updateTermsAgreement(userId, termsAgreed)
return UserProfileResponse.from(updatedUserProfile)
}

fun validateUserExists(userId: UUID) {
if (!userDomainService.existsActiveUserByIdAndDeletedAtIsNull(userId)) {
throw AuthException(AuthErrorCode.USER_NOT_FOUND, "User not found: $userId")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,9 @@ class AuthUseCase(
fun getUserProfile(userId: UUID): UserProfileResponse {
return userAuthService.findUserProfileByUserId(userId)
}

@Transactional
fun updateTermsAgreement(userId: UUID, termsAgreed: Boolean): UserProfileResponse {
return userAuthService.updateTermsAgreement(userId, termsAgreed)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,22 @@ import java.util.UUID


data class UpsertUserBookRequest private constructor(
val userId: UUID,
val bookIsbn: String,
val bookTitle: String,
val bookAuthor: String,
val bookPublisher: String,
val bookCoverImageUrl: String,
val status: BookStatus
val userId: UUID? = null,
val bookIsbn: String? = null,
val bookTitle: String? = null,
val bookAuthor: String? = null,
val bookPublisher: String? = null,
val bookCoverImageUrl: String? = null,
val status: BookStatus? = null
) {
fun validUserId(): UUID = userId!!
fun validBookIsbn(): String = bookIsbn!!
fun validBookTitle(): String = bookTitle!!
fun validBookAuthor(): String = bookAuthor!!
fun validBookPublisher(): String = bookPublisher!!
fun validBookCoverImageUrl(): String = bookCoverImageUrl!!
fun validStatus(): BookStatus = status!!

companion object {
fun of(
userId: UUID,
Expand Down
37 changes: 18 additions & 19 deletions apis/src/main/kotlin/org/yapp/apis/book/service/UserBookService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,18 @@ import java.util.UUID
class UserBookService(
private val userBookDomainService: UserBookDomainService
) {
fun upsertUserBook(upsertUserBookRequest: UpsertUserBookRequest): UserBookResponse =
UserBookResponse.from(
userBookDomainService.upsertUserBook(
upsertUserBookRequest.userId,
upsertUserBookRequest.bookIsbn,
upsertUserBookRequest.bookTitle,
upsertUserBookRequest.bookAuthor,
upsertUserBookRequest.bookPublisher,
upsertUserBookRequest.bookCoverImageUrl,
upsertUserBookRequest.status
)
fun upsertUserBook(upsertUserBookRequest: UpsertUserBookRequest): UserBookResponse {
Copy link
Member

Choose a reason for hiding this comment

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

(사소) UpsertUserBookRequest의 필드에도 validation을 하면 좋을 것 같다는 생각이 듭니다!!

val userBookInfoVO = userBookDomainService.upsertUserBook(
upsertUserBookRequest.validUserId(),
upsertUserBookRequest.validBookIsbn(),
upsertUserBookRequest.validBookTitle(),
upsertUserBookRequest.validBookAuthor(),
upsertUserBookRequest.validBookPublisher(),
upsertUserBookRequest.validBookCoverImageUrl(),
upsertUserBookRequest.validStatus()
)
return UserBookResponse.from(userBookInfoVO)
}

fun findAllUserBooks(userId: UUID): List<UserBookResponse> {
val userBooks: List<UserBookInfoVO> = userBookDomainService.findAllUserBooks(userId)
Expand All @@ -39,12 +39,11 @@ class UserBookService(
}

fun findAllByUserIdAndBookIsbnIn(userBooksByIsbnsRequest: UserBooksByIsbnsRequest): List<UserBookResponse> {
return userBookDomainService
.findAllByUserIdAndBookIsbnIn(
userBooksByIsbnsRequest.validUserId(),
userBooksByIsbnsRequest.validIsbns(),
)
.map { UserBookResponse.from(it) }
val userBooks = userBookDomainService.findAllByUserIdAndBookIsbnIn(
userBooksByIsbnsRequest.validUserId(),
userBooksByIsbnsRequest.validIsbns(),
)
return userBooks.map { UserBookResponse.from(it) }
}

private fun findUserBooksByDynamicCondition(
Expand All @@ -53,8 +52,8 @@ class UserBookService(
sort: String?,
pageable: Pageable
): Page<UserBookResponse> {
return userBookDomainService.findUserBooksByDynamicCondition(userId, status, sort, pageable)
.map { UserBookResponse.from(it) }
val page = userBookDomainService.findUserBooksByDynamicCondition(userId, status, sort, pageable)
return page.map { UserBookResponse.from(it) }
}

fun findUserBooksByDynamicConditionWithStatusCounts(
Expand Down
21 changes: 17 additions & 4 deletions domain/src/main/kotlin/org/yapp/domain/user/User.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ data class User private constructor(
val providerType: ProviderType,
val providerId: ProviderId,
val role: Role,
val termsAgreed: Boolean = false,
Copy link

@coderabbitai coderabbitai bot Jul 23, 2025

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

도메인 모델에 약관 동의 필드가 적절히 추가되었습니다.

기본값 false로 설정하여 명시적 동의가 필요하도록 한 것이 올바른 설계입니다.

클래스 문서화 주석(19-22줄)에 새로 추가된 termsAgreed 속성에 대한 설명을 추가하는 것을 권장합니다:

 * @property role The roles of the user (e.g., USER, ADMIN).
+* @property termsAgreed Whether the user has agreed to the terms and conditions.
 * @property createdAt The timestamp when the user was created.
📝 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.

Suggested change
val termsAgreed: Boolean = false,
* @property role The roles of the user (e.g., USER, ADMIN).
* @property termsAgreed Whether the user has agreed to the terms and conditions.
* @property createdAt The timestamp when the user was created.
🤖 Prompt for AI Agents
In domain/src/main/kotlin/org/yapp/domain/user/User.kt at line 31, the new field
termsAgreed is correctly added with a default value of false. To complete this
update, add a description of the termsAgreed property to the class documentation
comments between lines 19 and 22, explaining its purpose as indicating whether
the user has explicitly agreed to the terms.

Copy link
Member

Choose a reason for hiding this comment

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

@minwoo1999 요거 주석 달면 좋을 거 같아요~

Copy link

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

val createdAt: LocalDateTime? = null,
val updatedAt: LocalDateTime? = null,
val deletedAt: LocalDateTime? = null
Expand All @@ -40,13 +41,20 @@ data class User private constructor(
)
}

fun updateTermsAgreement(termsAgreed: Boolean): User {
return this.copy(
termsAgreed = termsAgreed
)
}

companion object {
fun create(
email: String,
nickname: String,
profileImageUrl: String?,
providerType: ProviderType,
providerId: String
providerId: String,
termsAgreed: Boolean = false
): User {
return User(
id = Id.newInstance(UuidGenerator.create()),
Expand All @@ -55,7 +63,8 @@ data class User private constructor(
profileImageUrl = profileImageUrl,
providerType = providerType,
providerId = ProviderId.newInstance(providerId),
role = Role.USER
role = Role.USER,
termsAgreed = termsAgreed
)
}

Expand All @@ -66,7 +75,8 @@ data class User private constructor(
profileImageUrl: String?,
providerType: ProviderType,
providerId: String,
role: Role
role: Role,
termsAgreed: Boolean = false
): User {
return User(
id = Id.newInstance(UuidGenerator.create()),
Expand All @@ -75,7 +85,8 @@ data class User private constructor(
profileImageUrl = profileImageUrl,
providerType = providerType,
providerId = ProviderId.newInstance(providerId),
role = role
role = role,
termsAgreed = termsAgreed
)
}

Expand All @@ -87,6 +98,7 @@ data class User private constructor(
providerType: ProviderType,
providerId: ProviderId,
role: Role,
termsAgreed: Boolean = false,
createdAt: LocalDateTime? = null,
updatedAt: LocalDateTime? = null,
deletedAt: LocalDateTime? = null
Expand All @@ -99,6 +111,7 @@ data class User private constructor(
providerType = providerType,
providerId = providerId,
role = role,
termsAgreed = termsAgreed,
createdAt = createdAt,
updatedAt = updatedAt,
deletedAt = deletedAt
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,12 @@ class UserDomainService(
val restoredUser = userRepository.save(deletedUser.restore())
return UserIdentityVO.newInstance(restoredUser)
}

fun updateTermsAgreement(userId: UUID, termsAgreed: Boolean): UserProfileVO {
val user = userRepository.findById(userId)
?: throw UserNotFoundException(UserErrorCode.USER_NOT_FOUND)

val updatedUser = userRepository.save(user.updateTermsAgreement(termsAgreed))
return UserProfileVO.newInstance(updatedUser)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ data class UserProfileVO private constructor(
val id: User.Id,
val email: User.Email,
val nickname: String,
val provider: ProviderType
val provider: ProviderType,
val termsAgreed: Boolean
) {
init {
require(nickname.isNotBlank()) {"nickname은 비어 있을 수 없습니다."}
Expand All @@ -22,7 +23,8 @@ data class UserProfileVO private constructor(
id = user.id,
email = user.email,
nickname = user.nickname,
provider = user.providerType
provider = user.providerType,
termsAgreed = user.termsAgreed
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ class UserBookDomainService(
}

fun findAllUserBooks(userId: UUID): List<UserBookInfoVO> {
return userBookRepository.findAllByUserId(userId)
.map(UserBookInfoVO::newInstance)
val userBooks = userBookRepository.findAllByUserId(userId)
return userBooks.map { UserBookInfoVO.newInstance(it) }
}

fun findUserBooksByDynamicCondition(
Expand All @@ -46,16 +46,16 @@ class UserBookDomainService(
sort: String?,
pageable: Pageable
): Page<UserBookInfoVO> {
return userBookRepository.findUserBooksByDynamicCondition(userId, status, sort, pageable)
.map(UserBookInfoVO::newInstance)
val page = userBookRepository.findUserBooksByDynamicCondition(userId, status, sort, pageable)
return page.map { UserBookInfoVO.newInstance(it) }
}

fun findAllByUserIdAndBookIsbnIn(userId: UUID, isbns: List<String>): List<UserBookInfoVO> {
if (isbns.isEmpty()) {
return emptyList()
}
return userBookRepository.findAllByUserIdAndBookIsbnIn(userId, isbns)
.map { UserBookInfoVO.newInstance(it) }
val userBooks = userBookRepository.findAllByUserIdAndBookIsbnIn(userId, isbns)
return userBooks.map { UserBookInfoVO.newInstance(it) }
}

fun getUserBookStatusCounts(userId: UUID): UserBookStatusCountsVO {
Expand Down
4 changes: 2 additions & 2 deletions infra/src/main/kotlin/org/yapp/infra/common/BaseTimeEntity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@ abstract class BaseTimeEntity {

@CreatedDate
@Column(name = "created_at", nullable = false, updatable = false)
lateinit var createdAt: LocalDateTime
var createdAt: LocalDateTime = LocalDateTime.now()
protected set

@LastModifiedDate
@Column(name = "updated_at", nullable = false)
lateinit var updatedAt: LocalDateTime
var updatedAt: LocalDateTime = LocalDateTime.now()
protected set

@Column(name = "deleted_at")
Expand Down
Loading