Open
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughBaseEntity의 Changes
Sequence DiagramsequenceDiagram
participant Client
participant UserService
participant UserSportProfileRepository
participant UserRepository
participant Database
Client->>UserService: getOtherUserProfiles(otherUserProfileId)
UserService->>UserSportProfileRepository: findUserIdById(otherUserProfileId)
UserSportProfileRepository->>Database: SELECT user_id FROM user_sport_profile WHERE id = :id
Database-->>UserSportProfileRepository: user_id
UserSportProfileRepository-->>UserService: user_id (or null)
UserService->>UserRepository: findByIdIncludingDeleted(otherUserId)
UserRepository->>Database: SELECT * FROM user WHERE id = :id
Database-->>UserRepository: User record (including deleted)
UserRepository-->>UserService: User object
UserService->>UserService: validateUserAccess(otherUser)
alt otherUser.deletedAt != null
UserService-->>Client: ErrorCode.WITHDRAWN_USER (403)
else
UserService->>UserService: Check block relationships & restrictions
UserService-->>Client: Access granted / other errors
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt (2)
384-392:⚠️ Potential issue | 🟡 Minor
getOtherUserRecentReviewSummary도 동일한 탈퇴 유저 처리 문제가 있습니다.
getOtherUserRecentReview와 마찬가지로 이 메서드도 탈퇴 유저에 대해 일관된 에러 응답을 반환하도록 수정이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt` around lines 384 - 392, In getOtherUserRecentReviewSummary, mirror the withdrawn-user handling used in getOtherUserRecentReview: after loading otherUser via userRepository.findByIdOrNull in UserService, check the same user withdrawal/status flag or property (the same predicate used in getOtherUserRecentReview) and throw the same CustomException/ErrorCode used there (instead of proceeding); ensure you use the same repository lookups (otherUserProfileRepository, userRepository) and the same exception/ErrorCode identifier to produce a consistent error response for withdrawn users.
347-356:⚠️ Potential issue | 🟡 Minor
getOtherUserRecentReview에서 탈퇴 유저 처리가 누락되었습니다.이 메서드는 여전히
findByIdOrNull을 사용하여@SQLRestriction의 영향을 받습니다. 탈퇴 유저 프로필 조회 시WITHDRAWN_USER대신USER_NOT_FOUND가 반환됩니다.getOtherUserProfiles와 일관성을 위해 동일한 패턴 적용을 고려해 주세요.🔧 일관성 있는 탈퇴 유저 처리 제안
`@Transactional`(readOnly = true) fun getOtherUserRecentReview( userId: String, otherUserProfileId: String, sportCode: String?, request: CommonCursorRequest, ): CursorResponse<UserRecentReviewResponse> { - val otherUserProfileId = userSportProfileRepository.findByIdOrNull(otherUserProfileId) - ?: throw CustomException(ErrorCode.ACTIVE_PROFILE_NOT_FOUND) - val otherUser = userRepository.findByIdOrNull(otherUserProfileId.user.id!!) + val otherUserId = userSportProfileRepository.findUserIdById(otherUserProfileId) ?: throw CustomException(ErrorCode.USER_NOT_FOUND) + val otherUser = userRepository.findByIdIncludingDeleted(otherUserId) + ?: throw CustomException(ErrorCode.USER_NOT_FOUND) + + // 탈퇴 유저 확인 + if (otherUser.deletedAt != null) { + throw CustomException(ErrorCode.WITHDRAWN_USER) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt` around lines 347 - 356, getOtherUserRecentReview에서 탈퇴 유저를 USER_NOT_FOUND로 처리하는 대신 getOtherUserProfiles와 동일한 패턴으로 탈퇴 유저를 명시적으로 감지해 CustomException(ErrorCode.WITHDRAWN_USER)을 던지도록 수정하세요: userSportProfileRepository.findByIdOrNull로 조회한 결과를 로컬 변수(예: foundProfile)로 저장하고, 해당 프로필/연관 user의 탈퇴 플래그(또는 상태 필드)를 검사해 탈퇴이면 CustomException(ErrorCode.WITHDRAWN_USER)을 던지며, 정상 프로필일 때만 userRepository.findByIdOrNull를 사용해 다른 유저 조회를 진행하도록 변경해 파라미터 변수명(현재 otherUserProfileId로 섀도잉된 부분)을 제거하세요.
🧹 Nitpick comments (1)
src/main/kotlin/org/appjam/smashing/domain/user/repository/UserRepository.kt (1)
13-23: 네이티브 쿼리에서user테이블명을 백틱으로 감싸는 것을 권장합니다.
user는 MySQL의 예약어입니다. 현재 코드는 작동하지만, User 엔티티의@SQLDelete주석에서도 동일하게 백틱 없이 사용 중이므로, 데이터베이스 설정이나 버전이 변경될 경우 일관성 있게 처리하기 위해 백틱 사용을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/org/appjam/smashing/domain/user/repository/UserRepository.kt` around lines 13 - 23, The native query in function findByIdIncludingDeleted uses the reserved word user unescaped; update the SQL to wrap the table name in backticks (e.g., `user`) to avoid conflicts with MySQL reserved words and ensure portability, and do the same wherever the table name appears unescaped (for example in the User entity's `@SQLDelete` annotation) so naming is consistent across queries and entity mappings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt`:
- Around line 384-392: In getOtherUserRecentReviewSummary, mirror the
withdrawn-user handling used in getOtherUserRecentReview: after loading
otherUser via userRepository.findByIdOrNull in UserService, check the same user
withdrawal/status flag or property (the same predicate used in
getOtherUserRecentReview) and throw the same CustomException/ErrorCode used
there (instead of proceeding); ensure you use the same repository lookups
(otherUserProfileRepository, userRepository) and the same exception/ErrorCode
identifier to produce a consistent error response for withdrawn users.
- Around line 347-356: getOtherUserRecentReview에서 탈퇴 유저를 USER_NOT_FOUND로 처리하는 대신
getOtherUserProfiles와 동일한 패턴으로 탈퇴 유저를 명시적으로 감지해
CustomException(ErrorCode.WITHDRAWN_USER)을 던지도록 수정하세요:
userSportProfileRepository.findByIdOrNull로 조회한 결과를 로컬 변수(예: foundProfile)로 저장하고,
해당 프로필/연관 user의 탈퇴 플래그(또는 상태 필드)를 검사해 탈퇴이면
CustomException(ErrorCode.WITHDRAWN_USER)을 던지며, 정상 프로필일 때만
userRepository.findByIdOrNull를 사용해 다른 유저 조회를 진행하도록 변경해 파라미터 변수명(현재
otherUserProfileId로 섀도잉된 부분)을 제거하세요.
---
Nitpick comments:
In
`@src/main/kotlin/org/appjam/smashing/domain/user/repository/UserRepository.kt`:
- Around line 13-23: The native query in function findByIdIncludingDeleted uses
the reserved word user unescaped; update the SQL to wrap the table name in
backticks (e.g., `user`) to avoid conflicts with MySQL reserved words and ensure
portability, and do the same wherever the table name appears unescaped (for
example in the User entity's `@SQLDelete` annotation) so naming is consistent
across queries and entity mappings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c6da0f76-28a4-40ea-8650-c668f1aa2300
📒 Files selected for processing (5)
src/main/kotlin/org/appjam/smashing/domain/common/entity/BaseEntity.ktsrc/main/kotlin/org/appjam/smashing/domain/user/repository/UserRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/user/repository/UserSportProfileRepository.ktsrc/main/kotlin/org/appjam/smashing/domain/user/service/UserService.ktsrc/main/kotlin/org/appjam/smashing/global/exception/ErrorCode.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt`:
- Around line 161-165: The two methods getOtherUserRecentReview and
getOtherUserRecentReviewSummary still use findByIdOrNull and skip
validateUserAccess, causing 500s for deleted users; update both to mirror
getOtherUserProfiles by first resolving otherUserId via
userSportProfileRepository.findUserIdById(otherUserProfileId) (throw
USER_NOT_FOUND if null), then load the user with
userRepository.findByIdIncludingDeleted(otherUserId) (throw USER_NOT_FOUND if
null), and then call validateUserAccess(otherUser, ...) so deleted users produce
403 as intended.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4f8ddfe8-64f4-441b-bca5-33c8deb4cd7a
📒 Files selected for processing (1)
src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt
Comment on lines
+161
to
165
| // 다른 유저 정보 탐색 (탈퇴 유저도 조회하기 위해 id 직접 추출) | ||
| val otherUserId = userSportProfileRepository.findUserIdById(otherUserProfileId) | ||
| ?: throw CustomException(ErrorCode.USER_NOT_FOUND) | ||
| val otherUser = userRepository.findByIdIncludingDeleted(otherUserId) | ||
| ?: throw CustomException(ErrorCode.USER_NOT_FOUND) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 다른 유저 관련 메서드에서 findByIdOrNull 사용 현황 확인
# findByIdOrNull으로 다른 유저를 조회하는 패턴 검색
rg -n "otherUser.*findByIdOrNull|findByIdOrNull.*otherUser" src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.ktRepository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 442
🏁 Script executed:
#!/bin/bash
# Check the complete context around getOtherUserProfiles, getOtherUserRecentReview, and getOtherUserRecentReviewSummary
# First, let's see the actual implementations of these methods
echo "=== getOtherUserProfiles (lines 161-165) ==="
sed -n '155,170p' src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt
echo -e "\n=== getOtherUserRecentReview (around lines 353-356) ==="
sed -n '348,365p' src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt
echo -e "\n=== getOtherUserRecentReviewSummary (around lines 389-392) ==="
sed -n '384,405p' src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt
echo -e "\n=== validateUserAccess method that handles withdrawn users ==="
sed -n '640,670p' src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.ktRepository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 3257
🏁 Script executed:
#!/bin/bash
# Check if getOtherUserRecentReview and getOtherUserRecentReviewSummary call validateUserAccess or resolveProfile
echo "=== Complete getOtherUserRecentReview method ==="
sed -n '348,380p' src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt
echo -e "\n=== Complete getOtherUserRecentReviewSummary method ==="
sed -n '384,420p' src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt
echo -e "\n=== Check if resolveProfile calls validateUserAccess ==="
rg -A 10 "fun resolveProfile" src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt | head -30Repository: TEAM-SMASHING/SMASHING-SERVER
Length of output: 3183
탈퇴 유저 조회 로직 개선이 좋으나, 다른 메서드들과의 불일치 해결 필요
getOtherUserProfiles (lines 161-165)의 변경사항은 좋습니다. SQLRestriction을 우회하여 탈퇴 유저도 조회할 수 있도록 개선했고, validateUserAccess에서 탈퇴 유저 검증으로 403 Forbidden을 반환하도록 했습니다.
다만 getOtherUserRecentReview (line 355)와 getOtherUserRecentReviewSummary (line 391)는 여전히 findByIdOrNull을 사용하고 있어, validateUserAccess를 호출하지 않습니다. 이 메서드들에서 탈퇴 유저에 접근하면 500 에러가 발생할 수 있습니다. 동일한 방식으로 개선하여 일관성 있는 처리가 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/org/appjam/smashing/domain/user/service/UserService.kt`
around lines 161 - 165, The two methods getOtherUserRecentReview and
getOtherUserRecentReviewSummary still use findByIdOrNull and skip
validateUserAccess, causing 500s for deleted users; update both to mirror
getOtherUserProfiles by first resolving otherUserId via
userSportProfileRepository.findUserIdById(otherUserProfileId) (throw
USER_NOT_FOUND if null), then load the user with
userRepository.findByIdIncludingDeleted(otherUserId) (throw USER_NOT_FOUND if
null), and then call validateUserAccess(otherUser, ...) so deleted users produce
403 as intended.
kyoooooong
reviewed
Mar 22, 2026
Contributor
kyoooooong
left a comment
kyoooooong
left a comment
There was a problem hiding this comment.
수고 너무너무 많으셨습니다!! 코멘트 한번 확인 부탁드립니다
| @Column(name = "deleted_at", insertable = false, updatable = false, columnDefinition = "DATETIME") | ||
| @Comment("삭제일자") | ||
| lateinit var deletedAt: LocalDateTime | ||
| var deletedAt: LocalDateTime? = null |
Contributor
There was a problem hiding this comment.
이 부분은 모든 엔티티에서 상속하는 부분이라 신중히 바꾸어야할 것같아요! deletedat의 경우 삭제된 이후에 다시 변경될 수 없고, lateinit 처리로 null 처리가 되어있는데 혹시 이부분 이렇게 처리하신 이유가 있으실까요?
Member
Author
There was a problem hiding this comment.
정상 유저를 조회할 때 otherUser.deletedAt을 접근하려 하면 500에러가 발생했기 때문입니다!
에러명은 다음과 같습니다.
lateinit 상태이기 때문에 null인 상태를 가지지도 않는 시점에서 접근을 시도하여 발생하는 에러 같습니다.
따라서 ?= null을 통해 접근이 가능하도록 명시했던 것이었습니다..!
| ) { | ||
| // 1. 차단 관계 확인 | ||
| // 1. 탈퇴 유저 확인 | ||
| if (otherUser.deletedAt != null) { |
Contributor
There was a problem hiding this comment.
혹시 차단, 탈퇴가 각각 다른 팝업이 뜨는 형식이라 이렇게 예외를 나눠두신 걸까요? 아니라면 user의 경우 탈퇴했을 경우 엔티티에 deletedat is null이 걸려 있어 조회가 되지 않을 거라 not found로 내려주어도 좋을 것같아요!
Member
Author
There was a problem hiding this comment.
위의 코드리뷰와 동일한 맥락인데요!
해당 피알을 반영하지 않은 기존 로직에서는 탈퇴 유저 프로필 방문 시, 존재하지 않는 유저를 접근하려고 하여 500에러가 발생했습니다.
이로 인해 차단/신고는 403 에러를 내려줄 수 있는 경우와 다르게 동작하고 있었습니다.
비록 저희의 정책은 각각 다른 팝업이 뜨지 않아도 되지만, 적어도 500 서버 에러는 발생하지 않도록 하는 것이 더 적절하다고 판단하여 해당 PR을 반영하게 되었습니다!
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
📌 Related Issue
#️⃣ 요약 설명
SQLRestriction으로 인해 500 에러가 발생했습니다. 그러나 저희의 정책에 따르면 탈퇴/차단/신고 제재 상태일 때 팝업창을 별도로 띄워야 하여 500 에러를 내리면 안 되는 상황이었습니다.nativeQuery를 활용하여 직접 id를 추출하도록 바꾸었습니다.📝 작업 내용
👍 동작 확인
다른 유저 정보 상세 조회 API- 탈퇴 시 403 Forbidden💬 리뷰 요구사항(선택)