Conversation
… feat/WTH-208-사용자-프로필-기능-추가
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough작성자 정보를 Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as UserController
participant UseCase as UpdateProfileImageUseCase
participant FileRepo as FileRepository
participant FileAccess as FileAccessUrlPort
participant UserRepo as UserRepository
Client->>Controller: PUT /users/me/profile-image (FileSaveRequest)
Controller->>UseCase: execute(userId, request)
UseCase->>UserRepo: getById(userId)
UserRepo-->>UseCase: User
UseCase->>FileRepo: findAllByOwnerTypeAndOwnerIdAndStatus(CLUB_MEMBER_PROFILE/USER_PROFILE, userId, UPLOADED)
FileRepo-->>UseCase: [existingFiles]
UseCase->>FileRepo: mark existingFiles as DELETED (save)
UseCase->>FileRepo: save(new File owner=USER_PROFILE, metadata)
FileRepo-->>UseCase: savedFile
UseCase->>FileAccess: resolve(savedFile.storageKey)
FileAccess-->>UseCase: profileImageUrl
UseCase->>UserRepo: update user's profileImageUrl and persist (via user entity mutation)
UseCase-->>Controller: success
Controller-->>Client: 200 USER_PROFILE_IMAGE_UPDATE_SUCCESS
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 Tip You can enable review details to help with troubleshooting, context usage and more.Enable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/kotlin/com/weeth/domain/user/application/usecase/command/UpdateUserProfileUseCase.kt (1)
23-30:⚠️ Potential issue | 🟠 Major
bio미전달 요청이 기존 자기소개를 지워버립니다.
UpdateUserProfileRequest.bio가 기본값null인데 여기서user.update()에 그대로 넘기고 있어서,bio를 보내지 않은 요청도 기존 값을null로 덮어씁니다. 구버전 클라이언트나 부분 수정 시나리오가 있다면 사용자 데이터가 사라집니다. 전체 치환 API면bio를 필수로 바꾸고, 부분 수정 API면 미전달과 명시적null을 구분할 수 있는 요청 모델이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/user/application/usecase/command/UpdateUserProfileUseCase.kt` around lines 23 - 30, UpdateUserProfileRequest.bio being nullable is passed directly into user.update(...) which causes omission of bio in the request to overwrite the existing bio with null; change the call site in UpdateUserProfileUseCase (the user.update(...) invocation) to avoid overwriting when bio was not supplied—either pass the existing value when request.bio is null (e.g., bio = request.bio ?: user.bio) or change the request model to distinguish “not provided” (e.g., Optional/Wrapper) and only include bio in the update when present; update the call to user.update(...) accordingly so UpdateUserProfileRequest.bio no longer erases the stored bio unintentionally.
🧹 Nitpick comments (2)
src/test/kotlin/com/weeth/domain/user/application/usecase/command/UpdateProfileImageUseCaseTest.kt (1)
61-61: 도메인 엔티티는 모킹 대신 실제 객체 검증이 더 적합합니다Line [61]의
mockk<File>(relaxed = true)대신File.createUploaded(...)로 실제 객체를 만들고, 실행 후status가DELETED로 바뀌는지 검증해 주세요. 엔티티 동작 테스트의 신뢰성이 더 높아집니다.As per coding guidelines: Use MockK and springmockk for mocking in Kotlin tests; mock Repository/Reader/Port interfaces, not implementations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/user/application/usecase/command/UpdateProfileImageUseCaseTest.kt` at line 61, In UpdateProfileImageUseCaseTest replace the mocked domain entity existingFile (mockk<File>(relaxed = true)) with a real instance created via File.createUploaded(...) and run the use case, then assert the entity's status field equals DELETED after execution; specifically, instantiate the domain object with File.createUploaded(...) instead of mocking, invoke the UpdateProfileImageUseCase, and verify existingFile.status == DELETED (do not mock domain entities—only mock repository/port interfaces).src/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.kt (1)
23-26: 엔티티 Mock 스텁 확장을 줄이고 Fixture 기반으로 전환해 주세요.
User엔티티 프로퍼티 스텁을 늘리는 방식은 테스트를 구현 세부사항에 강결합시킵니다. 이 테스트는 실제 fixture 객체를 만들고 Reader/Port 인터페이스만 MockK로 모킹하는 형태가 더 안정적입니다.Based on learnings: Applies to **/*Test.kt : Use MockK and springmockk for mocking in Kotlin tests; mock Repository/Reader/Port interfaces, not implementations
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.kt` around lines 23 - 26, The test currently over-mocks the User entity via repeated stubs (every { user.id } returns ..., every { user.name } returns ..., etc.); instead create a real User fixture object (e.g., a test User instance with id=1L, name="테스터", role=Role.USER, profileImageUrl=null) and stop stubbing its properties, and only mock the reader/port interfaces the test depends on (e.g., UserReader/UserPort or repository used by PostMapperTest) with MockK/springmockk to return the fixture when invoked; update PostMapperTest to use that fixture in place of the mocked `user` variable and adjust any mocked calls to target the interface methods (e.g., every { userReader.findById(...) } returns fixture) rather than the entity properties.
🤖 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/com/weeth/domain/user/application/usecase/command/UpdateProfileImageUseCase.kt`:
- Around line 26-31: The UpdateProfileImageUseCase currently calls
FileRepository.findAllByOwnerTypeAndOwnerIdAndStatus directly (using
FileOwnerType.USER_PROFILE, user.id, FileStatus.UPLOADED) — move that
cross-domain read into a new Reader interface (e.g., FileReader with a method
like findAllByOwnerTypeAndOwnerIdAndStatus or findUploadedByOwner) and inject
FileReader into UpdateProfileImageUseCase; replace the direct repository read
with a call to the FileReader, but keep the write mutation (calling markDeleted
on each File entity and persisting with FileRepository.save or the existing
repository delete/save) in the use case so that only reads use the reader and
writes still use FileRepository.
In `@src/main/kotlin/com/weeth/domain/user/domain/entity/User.kt`:
- Around line 71-85: Add a database migration to alter the users table for the
new non-nullable fields on the User entity: create a migration that adds the
columns termsAgreed and privacyAgreed (and any other added columns like
profileImageUrl/bio if not present) and ensure existing rows are backfilled
before enforcing NOT NULL (either by creating the columns with a DEFAULT false
and then dropping the default, or adding as nullable, updating all rows to
false, and then altering to NOT NULL); reference the User class properties
termsAgreed and privacyAgreed when naming/validating the migration and ensure
the migration is included in the deployment pipeline so the schema change is
applied before this code is released.
---
Outside diff comments:
In
`@src/main/kotlin/com/weeth/domain/user/application/usecase/command/UpdateUserProfileUseCase.kt`:
- Around line 23-30: UpdateUserProfileRequest.bio being nullable is passed
directly into user.update(...) which causes omission of bio in the request to
overwrite the existing bio with null; change the call site in
UpdateUserProfileUseCase (the user.update(...) invocation) to avoid overwriting
when bio was not supplied—either pass the existing value when request.bio is
null (e.g., bio = request.bio ?: user.bio) or change the request model to
distinguish “not provided” (e.g., Optional/Wrapper) and only include bio in the
update when present; update the call to user.update(...) accordingly so
UpdateUserProfileRequest.bio no longer erases the stored bio unintentionally.
---
Nitpick comments:
In `@src/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.kt`:
- Around line 23-26: The test currently over-mocks the User entity via repeated
stubs (every { user.id } returns ..., every { user.name } returns ..., etc.);
instead create a real User fixture object (e.g., a test User instance with
id=1L, name="테스터", role=Role.USER, profileImageUrl=null) and stop stubbing its
properties, and only mock the reader/port interfaces the test depends on (e.g.,
UserReader/UserPort or repository used by PostMapperTest) with MockK/springmockk
to return the fixture when invoked; update PostMapperTest to use that fixture in
place of the mocked `user` variable and adjust any mocked calls to target the
interface methods (e.g., every { userReader.findById(...) } returns fixture)
rather than the entity properties.
In
`@src/test/kotlin/com/weeth/domain/user/application/usecase/command/UpdateProfileImageUseCaseTest.kt`:
- Line 61: In UpdateProfileImageUseCaseTest replace the mocked domain entity
existingFile (mockk<File>(relaxed = true)) with a real instance created via
File.createUploaded(...) and run the use case, then assert the entity's status
field equals DELETED after execution; specifically, instantiate the domain
object with File.createUploaded(...) instead of mocking, invoke the
UpdateProfileImageUseCase, and verify existingFile.status == DELETED (do not
mock domain entities—only mock repository/port interfaces).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c5c27ebf-7939-4b8a-98ee-5f939d5cbd79
📒 Files selected for processing (31)
src/main/kotlin/com/weeth/domain/board/application/dto/response/PostDetailResponse.ktsrc/main/kotlin/com/weeth/domain/board/application/dto/response/PostListResponse.ktsrc/main/kotlin/com/weeth/domain/board/application/mapper/PostMapper.ktsrc/main/kotlin/com/weeth/domain/comment/application/dto/response/CommentResponse.ktsrc/main/kotlin/com/weeth/domain/comment/application/mapper/CommentMapper.ktsrc/main/kotlin/com/weeth/domain/dashboard/application/dto/response/DashboardMyInfoResponse.ktsrc/main/kotlin/com/weeth/domain/dashboard/application/dto/response/DashboardPostResponse.ktsrc/main/kotlin/com/weeth/domain/dashboard/application/mapper/DashboardMapper.ktsrc/main/kotlin/com/weeth/domain/dashboard/application/usecase/query/GetDashboardQueryService.ktsrc/main/kotlin/com/weeth/domain/file/domain/enums/FileOwnerType.ktsrc/main/kotlin/com/weeth/domain/schedule/domain/repository/EventReader.ktsrc/main/kotlin/com/weeth/domain/schedule/domain/repository/EventRepository.ktsrc/main/kotlin/com/weeth/domain/user/application/dto/request/AgreeTermsRequest.ktsrc/main/kotlin/com/weeth/domain/user/application/dto/request/UpdateUserProfileRequest.ktsrc/main/kotlin/com/weeth/domain/user/application/dto/response/UserInfo.ktsrc/main/kotlin/com/weeth/domain/user/application/dto/response/UserProfileResponse.ktsrc/main/kotlin/com/weeth/domain/user/application/mapper/UserMapper.ktsrc/main/kotlin/com/weeth/domain/user/application/usecase/command/AgreeTermsUseCase.ktsrc/main/kotlin/com/weeth/domain/user/application/usecase/command/SocialLoginUseCase.ktsrc/main/kotlin/com/weeth/domain/user/application/usecase/command/UpdateProfileImageUseCase.ktsrc/main/kotlin/com/weeth/domain/user/application/usecase/command/UpdateUserProfileUseCase.ktsrc/main/kotlin/com/weeth/domain/user/domain/entity/User.ktsrc/main/kotlin/com/weeth/domain/user/presentation/UserController.ktsrc/main/kotlin/com/weeth/domain/user/presentation/UserResponseCode.ktsrc/test/kotlin/com/weeth/domain/board/application/mapper/PostMapperTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/query/GetPostQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/comment/application/usecase/query/GetCommentQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/dashboard/application/usecase/query/GetDashboardQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/user/application/usecase/command/AgreeTermsUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/user/application/usecase/command/UpdateProfileImageUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/user/domain/entity/UserTest.kt
Comment on lines
+26
to
+31
| fileRepository | ||
| .findAllByOwnerTypeAndOwnerIdAndStatus( | ||
| FileOwnerType.USER_PROFILE, | ||
| user.id, | ||
| FileStatus.UPLOADED, | ||
| ).forEach { it.markDeleted() } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
타 도메인 조회를 Repository로 직접 수행하고 있습니다
Line [26]~Line [31]은 user 도메인 UseCase에서 file 도메인 조회를 FileRepository로 직접 수행합니다. 조회는 Reader 인터페이스로 분리하고, 저장(save)만 Repository 직접 호출로 유지해 주세요.
As per coding guidelines: Cross-domain reads must use Reader interfaces; cross-domain writes call Repository directly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/kotlin/com/weeth/domain/user/application/usecase/command/UpdateProfileImageUseCase.kt`
around lines 26 - 31, The UpdateProfileImageUseCase currently calls
FileRepository.findAllByOwnerTypeAndOwnerIdAndStatus directly (using
FileOwnerType.USER_PROFILE, user.id, FileStatus.UPLOADED) — move that
cross-domain read into a new Reader interface (e.g., FileReader with a method
like findAllByOwnerTypeAndOwnerIdAndStatus or findUploadedByOwner) and inject
FileReader into UpdateProfileImageUseCase; replace the direct repository read
with a call to the FileReader, but keep the write mutation (calling markDeleted
on each File entity and persisting with FileRepository.save or the existing
repository delete/save) in the use case so that only reads use the reader and
writes still use FileRepository.
Comment on lines
+71
to
+85
| @Column(nullable = false) | ||
| var termsAgreed: Boolean = false | ||
| private set | ||
|
|
||
| @Column(nullable = false) | ||
| var privacyAgreed: Boolean = false | ||
| private set | ||
|
|
||
| @Column(length = 200) | ||
| var bio: String? = null | ||
| private set | ||
|
|
||
| @Column(length = 500) | ||
| var profileImageUrl: String? = profileImageUrl | ||
| private set |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Candidate migration/changelog files"
fd -a 'V[0-9].*\.sql$' || true
fd -a '.*(migration|migrations|changelog|flyway|liquibase|schema).*' || true
echo
echo "## Search for new users columns"
rg -n -C2 \
-g '*.sql' -g '*.xml' -g '*.yml' -g '*.yaml' -g '*.json' \
'terms[_]?agreed|privacy[_]?agreed|profile[_]?image[_]?url|\bbio\b' || trueRepository: Team-Weeth/weeth-server
Length of output: 138
users 테이블 마이그레이션 추가 필요
데이터베이스 마이그레이션이 없습니다. termsAgreed, privacyAgreed 컬럼이 nullable = false이므로, 마이그레이션 없이 배포하면 기존 데이터 행에 대해 스키마 적용이 실패합니다. 마이그레이션 파일을 작성하여 새 컬럼을 추가하고 NOT NULL 제약에 대한 기본값 또는 backfill 처리를 포함해야 합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/com/weeth/domain/user/domain/entity/User.kt` around lines 71
- 85, Add a database migration to alter the users table for the new non-nullable
fields on the User entity: create a migration that adds the columns termsAgreed
and privacyAgreed (and any other added columns like profileImageUrl/bio if not
present) and ensure existing rows are backfilled before enforcing NOT NULL
(either by creating the columns with a DEFAULT false and then dropping the
default, or adding as nullable, updating all rows to false, and then altering to
NOT NULL); reference the User class properties termsAgreed and privacyAgreed
when naming/validating the migration and ensure the migration is included in the
deployment pipeline so the schema change is applied before this code is
released.
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/com/weeth/domain/user/domain/entity/User.kt`:
- Around line 137-138: The updateProfileImageUrl method currently sets
profileImageUrl to url?.trim(), which stores purely-whitespace input as an empty
string; change the assignment in User.updateProfileImageUrl to normalize
whitespace-only values to null by trimming the input and converting any
resulting empty string to null so profileImageUrl is either a non-empty trimmed
string or null (preserve handling of an original null input).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d169a8b7-ea04-4820-b0e7-ac4dde7cf1ab
📒 Files selected for processing (2)
src/main/kotlin/com/weeth/domain/user/application/dto/request/UpdateUserProfileRequest.ktsrc/main/kotlin/com/weeth/domain/user/domain/entity/User.kt
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/com/weeth/domain/user/domain/entity/User.kt`:
- Line 84: The constructor currently assigns var profileImageUrl: String? =
profileImageUrl without normalizing blank strings, allowing empty strings into
the DB; apply the same normalization used by updateProfileImageUrl() when
initializing the property (e.g., trim and convert empty/blank strings to null)
so the User entity always stores a normalized nullable profileImageUrl; update
the initialization in the User class to call the same normalization helper or
inline logic to ensure consistency with updateProfileImageUrl().
- Around line 22-40: 생성자에서 공백만 입력된 이름을 허용해 도메인 불변식이 깨지므로 User 클래스 생성자 초기화부(현재
var name: String = name.trim())와 일치하도록 생성 시에도 공백 검증을 추가하세요; 예를 들어 User 생성자에서
name을 먼저 trim()한 뒤 isNotBlank()를 검사하고(조건 불충족 시 IllegalArgumentException 또는
require로 예외 발생) 검증된 trim된 값을 var name에 할당하도록 수정해 update()와 동일한 검증 로직을 적용하세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 65e9eb45-b628-42af-8218-7d21d8e6bdeb
📒 Files selected for processing (1)
src/main/kotlin/com/weeth/domain/user/domain/entity/User.kt
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/kotlin/com/weeth/domain/user/application/mapper/UserMapper.kt (1)
22-41:cardinals = emptyList()하드코딩은 향후 수정이 필요합니다.
toUserProfileResponse와toUserSummaryResponse모두cardinals를 빈 리스트로 하드코딩하고 있습니다. 관련 코드(GetUserQueryService.kt:18)의 TODO 주석("동아리별 정보 추가")을 보면 이것이 의도된 임시 구현임을 알 수 있습니다.명명된 파라미터 사용으로 가독성이 향상된 점은 좋습니다. 다만
cardinals데이터 연동 작업이 누락되지 않도록 추적이 필요합니다.
cardinals데이터를 실제로 연동하는 구현을 도와드릴까요, 아니면 이 작업을 추적할 이슈를 생성해 드릴까요?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/user/application/mapper/UserMapper.kt` around lines 22 - 41, Replace the hardcoded cardinals = emptyList() in toUserProfileResponse and toUserSummaryResponse with a real mapping from the domain user.cardinals collection (or the field added by GetUserQueryService's "동아리별 정보 추가" work): e.g., use user.cardinals?.map { /* map to the appropriate DTO like CardinalResponse or CardinalSummary */ } ?: emptyList() so null is handled; update the mapper function names to reference the actual response DTO type you map to and ensure the mapping logic is applied in both toUserProfileResponse and toUserSummaryResponse instead of the temporary emptyList().src/test/kotlin/com/weeth/domain/board/application/usecase/command/MarkNoticeReadUseCaseTest.kt (1)
50-50:userId상수를 그대로 재사용해주세요.Line 50만
1L로 고정돼 있어서 위의userId값이 바뀌면 fixture ID만 조용히 어긋납니다. 여기서도userId를 넘기는 편이 테스트 의도가 더 명확합니다.♻️ 제안
- val user = UserTestFixture.createActiveUser1(1L) + val user = UserTestFixture.createActiveUser1(userId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/board/application/usecase/command/MarkNoticeReadUseCaseTest.kt` at line 50, Replace the hard-coded fixture ID with the test's userId constant: in MarkNoticeReadUseCaseTest change the call UserTestFixture.createActiveUser1(1L) to use the existing userId variable (e.g., UserTestFixture.createActiveUser1(userId)) so the fixture ID stays in sync with the test's userId and the test intent is clearer.src/test/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCaseTest.kt (1)
382-396: 테스트 헬퍼 함수 사용의 일관성 검토
owner변수는UserTestFixture.createActiveUser1(1L)을 사용하고,otherUser는createUser(2L, Role.USER)를 사용하고 있습니다. 두 방식 모두 동작하지만, 동일 테스트 파일 내에서 일관된 방식을 사용하는 것이 좋습니다.♻️ 일관성을 위한 제안
describe("owner validation") { it("작성자가 아니면 수정 시 예외를 던진다") { - val owner = UserTestFixture.createActiveUser1(1L) + val owner = createUser(1L, Role.USER) val otherUser = createUser(2L, Role.USER)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCaseTest.kt` around lines 382 - 396, The test mixes two user-creation helpers (UserTestFixture.createActiveUser1(1L) for owner and createUser(2L, Role.USER) for otherUser); make them consistent by using the same fixture API for both users—either change otherUser to use the UserTestFixture helper (e.g., UserTestFixture.createActiveUserX(2L) or equivalent) or change owner to use createUser(1L, Role.USER)—update the creation of owner or otherUser so both reference the same helper method (owner, otherUser, UserTestFixture.createActiveUser1, createUser).
🤖 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/com/weeth/domain/club/application/dto/request/UpdateMemberBioRequest.kt`:
- Around line 6-9: The UpdateMemberBioRequest currently declares val bio:
String? = null so omitted fields bind as null and cannot distinguish "missing"
vs "explicitly clear"; if you don't support explicit clearing, make bio required
by changing the property to a non-null type (e.g., val bio: String) and add a
validation annotation such as `@field`:NotNull or `@field`:NotBlank on
UpdateMemberBioRequest to cause a 400 when the field is omitted; alternatively,
if you do support clearing, add an explicit flag (e.g., val clearBio: Boolean? =
null) alongside bio to signal deletion instead of relying on bio == null.
In `@src/main/kotlin/com/weeth/domain/user/domain/entity/User.kt`:
- Around line 78-80: The profileImageUrl property currently only
trims/null-normalizes but does not enforce the 500-char column length, so long
values may violate DB constraints; update the normalization logic used during
construction/update (the code paths or function that currently sets
profileImageUrl) to also enforce length by truncating or rejecting strings
beyond 500 chars (e.g., apply .takeIf { it.length <= 500 } or .take(500) before
assignment) and ensure the same change is applied to the other occurrences
mentioned (lines referenced around profileImageUrl and the similar block at
128-130) so application-level validation matches the `@Column`(length = 500)
constraint.
- Around line 39-40: The SocialLoginUseCase currently forwards authResult.name
?: "" into User.create(), which now rejects blank names; update the
SocialLoginUseCase logic (the method handling authResult in SocialLoginUseCase
where authResult.name is read around the referenced ranges) to detect
missing/blank authResult.name and either (a) substitute a safe fallback (e.g.,
authResult.nickname, provider displayName, or a generated placeholder like
"user_<provider>_<id>") before calling User.create(), or (b) branch to a
name-completion flow that returns a pending-registration state instead of
calling User.create(); ensure you reference authResult.name and User.create()
when implementing the fix.
---
Nitpick comments:
In `@src/main/kotlin/com/weeth/domain/user/application/mapper/UserMapper.kt`:
- Around line 22-41: Replace the hardcoded cardinals = emptyList() in
toUserProfileResponse and toUserSummaryResponse with a real mapping from the
domain user.cardinals collection (or the field added by GetUserQueryService's
"동아리별 정보 추가" work): e.g., use user.cardinals?.map { /* map to the appropriate
DTO like CardinalResponse or CardinalSummary */ } ?: emptyList() so null is
handled; update the mapper function names to reference the actual response DTO
type you map to and ensure the mapping logic is applied in both
toUserProfileResponse and toUserSummaryResponse instead of the temporary
emptyList().
In
`@src/test/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCaseTest.kt`:
- Around line 382-396: The test mixes two user-creation helpers
(UserTestFixture.createActiveUser1(1L) for owner and createUser(2L, Role.USER)
for otherUser); make them consistent by using the same fixture API for both
users—either change otherUser to use the UserTestFixture helper (e.g.,
UserTestFixture.createActiveUserX(2L) or equivalent) or change owner to use
createUser(1L, Role.USER)—update the creation of owner or otherUser so both
reference the same helper method (owner, otherUser,
UserTestFixture.createActiveUser1, createUser).
In
`@src/test/kotlin/com/weeth/domain/board/application/usecase/command/MarkNoticeReadUseCaseTest.kt`:
- Line 50: Replace the hard-coded fixture ID with the test's userId constant: in
MarkNoticeReadUseCaseTest change the call UserTestFixture.createActiveUser1(1L)
to use the existing userId variable (e.g.,
UserTestFixture.createActiveUser1(userId)) so the fixture ID stays in sync with
the test's userId and the test intent is clearer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0312b6fb-dbb3-4931-831a-d1591d129bb0
📒 Files selected for processing (19)
src/main/kotlin/com/weeth/domain/club/application/dto/request/UpdateMemberBioRequest.ktsrc/main/kotlin/com/weeth/domain/club/application/dto/response/ClubMemberProfileResponse.ktsrc/main/kotlin/com/weeth/domain/club/application/mapper/ClubMapper.ktsrc/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.ktsrc/main/kotlin/com/weeth/domain/club/domain/entity/ClubMember.ktsrc/main/kotlin/com/weeth/domain/club/presentation/ClubController.ktsrc/main/kotlin/com/weeth/domain/club/presentation/ClubResponseCode.ktsrc/main/kotlin/com/weeth/domain/dashboard/application/mapper/DashboardMapper.ktsrc/main/kotlin/com/weeth/domain/dashboard/application/usecase/query/GetDashboardQueryService.ktsrc/main/kotlin/com/weeth/domain/user/application/dto/request/UpdateUserProfileRequest.ktsrc/main/kotlin/com/weeth/domain/user/application/dto/response/UserProfileResponse.ktsrc/main/kotlin/com/weeth/domain/user/application/mapper/UserMapper.ktsrc/main/kotlin/com/weeth/domain/user/domain/entity/User.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/command/ManagePostUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/board/application/usecase/command/MarkNoticeReadUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/club/application/usecase/query/GetClubMemberQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/dashboard/application/usecase/query/GetDashboardQueryServiceTest.ktsrc/test/kotlin/com/weeth/domain/user/domain/entity/UserTest.ktsrc/test/kotlin/com/weeth/domain/user/fixture/UserTestFixture.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/kotlin/com/weeth/domain/user/application/dto/request/UpdateUserProfileRequest.kt
- src/main/kotlin/com/weeth/domain/user/application/dto/response/UserProfileResponse.kt
Comment on lines
+78
to
+80
| @Column(length = 500) | ||
| var profileImageUrl: String? = profileImageUrl?.trim()?.takeIf { it.isNotBlank() } | ||
| private set |
There was a problem hiding this comment.
profileImageUrl도 컬럼 길이 검증이 필요합니다.
지금은 trim/null 정규화만 하고 500자 제한은 강제하지 않아서, 너무 긴 URL이 들어오면 스키마 제약을 애플리케이션 레벨에서 보장하지 못합니다. 생성/수정 경로가 같은 정규화 함수를 타면서 길이도 같이 검증하도록 맞춰두는 편이 안전합니다.
🛡️ 제안
`@Column`(length = 500)
- var profileImageUrl: String? = profileImageUrl?.trim()?.takeIf { it.isNotBlank() }
+ var profileImageUrl: String? = normalizeProfileImageUrl(profileImageUrl)
private set
@@
fun updateProfileImageUrl(url: String?) {
- this.profileImageUrl = url?.trim()?.takeIf { it.isNotBlank() }
+ this.profileImageUrl = normalizeProfileImageUrl(url)
}
+
+ private fun normalizeProfileImageUrl(url: String?): String? =
+ url?.trim()?.takeIf { it.isNotBlank() }?.also {
+ require(it.length <= 500) { "프로필 이미지 URL은 500자 이하여야 합니다." }
+ }Also applies to: 128-130
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/kotlin/com/weeth/domain/user/domain/entity/User.kt` around lines 78
- 80, The profileImageUrl property currently only trims/null-normalizes but does
not enforce the 500-char column length, so long values may violate DB
constraints; update the normalization logic used during construction/update (the
code paths or function that currently sets profileImageUrl) to also enforce
length by truncating or rejecting strings beyond 500 chars (e.g., apply .takeIf
{ it.length <= 500 } or .take(500) before assignment) and ensure the same change
is applied to the other occurrences mentioned (lines referenced around
profileImageUrl and the similar block at 128-130) so application-level
validation matches the `@Column`(length = 500) constraint.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/test/kotlin/com/weeth/domain/club/fixture/ClubMemberTestFixture.kt (1)
25-45: 일관성을 위해 다른 팩토리 메서드에도id파라미터 추가 고려
createActiveMember에는id파라미터가 추가되었지만,createWaitingMember와createAdminMember에는 없습니다. 향후 테스트에서 이 메서드들로 생성된 엔티티의 ID가 필요할 수 있으므로, 동일한 패턴을 적용하는 것을 고려해 주세요.♻️ 제안된 변경 사항
fun createWaitingMember( + id: Long = 0L, club: Club = ClubTestFixture.createClub(), user: User = UserTestFixture.createWaitingUser1(), ): ClubMember = ClubMember( club = club, user = user, memberStatus = MemberStatus.WAITING, memberRole = MemberRole.USER, - ) + ).also { if (id != 0L) ReflectionTestUtils.setField(it, "id", id) } fun createAdminMember( + id: Long = 0L, club: Club = ClubTestFixture.createClub(), user: User = UserTestFixture.createAdmin(), ): ClubMember = ClubMember( club = club, user = user, memberStatus = MemberStatus.ACTIVE, memberRole = MemberRole.ADMIN, - ) + ).also { if (id != 0L) ReflectionTestUtils.setField(it, "id", id) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/kotlin/com/weeth/domain/club/fixture/ClubMemberTestFixture.kt` around lines 25 - 45, Add an optional id parameter to createWaitingMember and createAdminMember to match createActiveMember (e.g. id: Long? = null) and pass it into the ClubMember constructor (id = id) so all test factory methods support supplying an explicit entity id for consistency in tests; update the function signatures for createWaitingMember and createAdminMember and their ClubMember construction to include the id parameter.src/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.kt (1)
87-92: 크로스 도메인 읽기에 Reader 인터페이스 사용 권장현재
fileRepository.findAllByOwnerTypeAndOwnerIdAndStatus()는 file 도메인에서 직접 읽기 작업을 수행하고 있습니다. 코딩 가이드라인에 따르면 "크로스 도메인 읽기는 Reader 인터페이스를 사용해야 한다"고 명시되어 있습니다.
FileReader인터페이스를 도입하여 크로스 도메인 의존성을 완화하는 것을 권장합니다.♻️ FileReader 인터페이스 도입 제안
// FileReader 인터페이스 정의 (file 도메인) interface FileReader { fun findAllByOwnerTypeAndOwnerIdAndStatus( ownerType: FileOwnerType, ownerId: Long, status: FileStatus, ): List<File> }class ManageClubMemberUsecase( private val clubRepository: ClubRepository, private val clubMemberRepository: ClubMemberRepository, private val userReader: UserReader, private val clubMemberPolicy: ClubMemberPolicy, + private val fileReader: FileReader, private val fileRepository: FileRepository, private val fileAccessUrlPort: FileAccessUrlPort, ) {- fileRepository + fileReader .findAllByOwnerTypeAndOwnerIdAndStatus( FileOwnerType.CLUB_MEMBER_PROFILE, member.id, FileStatus.UPLOADED, ).forEach { it.markDeleted() }As per coding guidelines: "Cross-domain reads must use Reader interfaces; cross-domain writes call Repository directly"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.kt` around lines 87 - 92, Replace the direct cross-domain read call to fileRepository.findAllByOwnerTypeAndOwnerIdAndStatus with a FileReader interface: add a constructor dependency (e.g., fileReader: FileReader) to ManageClubMemberUsecase, call fileReader.findAllByOwnerTypeAndOwnerIdAndStatus(FileOwnerType.CLUB_MEMBER_PROFILE, member.id, FileStatus.UPLOADED) to fetch files, and then continue to markDeleted() on each returned File; keep using fileRepository only for repository writes if needed. Ensure FileReader defines fun findAllByOwnerTypeAndOwnerIdAndStatus(ownerType: FileOwnerType, ownerId: Long, status: FileStatus): List<File> and update usages in ManageClubMemberUsecase to use fileReader for reads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@src/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.kt`:
- Around line 87-92: Replace the direct cross-domain read call to
fileRepository.findAllByOwnerTypeAndOwnerIdAndStatus with a FileReader
interface: add a constructor dependency (e.g., fileReader: FileReader) to
ManageClubMemberUsecase, call
fileReader.findAllByOwnerTypeAndOwnerIdAndStatus(FileOwnerType.CLUB_MEMBER_PROFILE,
member.id, FileStatus.UPLOADED) to fetch files, and then continue to
markDeleted() on each returned File; keep using fileRepository only for
repository writes if needed. Ensure FileReader defines fun
findAllByOwnerTypeAndOwnerIdAndStatus(ownerType: FileOwnerType, ownerId: Long,
status: FileStatus): List<File> and update usages in ManageClubMemberUsecase to
use fileReader for reads.
In `@src/test/kotlin/com/weeth/domain/club/fixture/ClubMemberTestFixture.kt`:
- Around line 25-45: Add an optional id parameter to createWaitingMember and
createAdminMember to match createActiveMember (e.g. id: Long? = null) and pass
it into the ClubMember constructor (id = id) so all test factory methods support
supplying an explicit entity id for consistency in tests; update the function
signatures for createWaitingMember and createAdminMember and their ClubMember
construction to include the id parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4144376a-ddda-402b-82f1-d0519003618d
📒 Files selected for processing (18)
src/main/kotlin/com/weeth/domain/club/application/dto/request/UpdateMemberBioRequest.ktsrc/main/kotlin/com/weeth/domain/club/application/dto/response/ClubMemberProfileResponse.ktsrc/main/kotlin/com/weeth/domain/club/application/mapper/ClubMapper.ktsrc/main/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUsecase.ktsrc/main/kotlin/com/weeth/domain/club/domain/entity/ClubMember.ktsrc/main/kotlin/com/weeth/domain/club/presentation/ClubController.ktsrc/main/kotlin/com/weeth/domain/club/presentation/ClubResponseCode.ktsrc/main/kotlin/com/weeth/domain/dashboard/application/dto/response/DashboardMyInfoResponse.ktsrc/main/kotlin/com/weeth/domain/dashboard/application/mapper/DashboardMapper.ktsrc/main/kotlin/com/weeth/domain/file/domain/enums/FileOwnerType.ktsrc/main/kotlin/com/weeth/domain/user/application/usecase/command/SocialLoginUseCase.ktsrc/main/kotlin/com/weeth/domain/user/domain/vo/SocialAuthResult.ktsrc/main/kotlin/com/weeth/domain/user/infrastructure/AppleSocialAuthAdapter.ktsrc/main/kotlin/com/weeth/domain/user/infrastructure/KakaoSocialAuthAdapter.ktsrc/main/kotlin/com/weeth/global/auth/kakao/dto/KakaoProfile.ktsrc/test/kotlin/com/weeth/domain/club/application/usecase/command/ManageClubMemberUseCaseTest.ktsrc/test/kotlin/com/weeth/domain/club/fixture/ClubMemberTestFixture.ktsrc/test/kotlin/com/weeth/domain/user/application/usecase/command/SocialLoginUseCaseTest.kt
💤 Files with no reviewable changes (5)
- src/main/kotlin/com/weeth/domain/user/infrastructure/AppleSocialAuthAdapter.kt
- src/test/kotlin/com/weeth/domain/user/application/usecase/command/SocialLoginUseCaseTest.kt
- src/main/kotlin/com/weeth/domain/user/domain/vo/SocialAuthResult.kt
- src/main/kotlin/com/weeth/domain/user/infrastructure/KakaoSocialAuthAdapter.kt
- src/main/kotlin/com/weeth/global/auth/kakao/dto/KakaoProfile.kt
✅ Files skipped from review due to trivial changes (2)
- src/main/kotlin/com/weeth/domain/file/domain/enums/FileOwnerType.kt
- src/main/kotlin/com/weeth/domain/club/application/dto/request/UpdateMemberBioRequest.kt
🚧 Files skipped from review as they are similar to previous changes (5)
- src/main/kotlin/com/weeth/domain/club/application/dto/response/ClubMemberProfileResponse.kt
- src/main/kotlin/com/weeth/domain/club/presentation/ClubResponseCode.kt
- src/main/kotlin/com/weeth/domain/dashboard/application/dto/response/DashboardMyInfoResponse.kt
- src/main/kotlin/com/weeth/domain/club/domain/entity/ClubMember.kt
- src/main/kotlin/com/weeth/domain/club/presentation/ClubController.kt
hyxklee
reviewed
Mar 19, 2026
| import io.swagger.v3.oas.annotations.media.Schema | ||
| import jakarta.validation.constraints.Size | ||
|
|
||
| data class UpdateMemberBioRequest( |
hyxklee
reviewed
Mar 19, 2026
| return CommonResponse.success(ClubResponseCode.MEMBER_FIND_ME_SUCCESS, meInfo) | ||
| } | ||
|
|
||
| @PatchMapping("/{clubId}/members/me/profile-image") |
Contributor
There was a problem hiding this comment.
동아리 정보 수정 관련해서 구현하다보니 프사 삭제가 약간 애매하더라구요 null이면 PATCH 성격에 따라서 수정을 안하는 방향으로 해야하는데, 그럼 사진을 삭제할 방법이 없어서 저는 별도 API를 떼긴 했습니다!
hyxklee
reviewed
Mar 19, 2026
| return CommonResponse.success(ClubResponseCode.MEMBER_PROFILE_IMAGE_UPDATED_SUCCESS) | ||
| } | ||
|
|
||
| @PatchMapping("/{clubId}/members/me/bio") |
Contributor
There was a problem hiding this comment.
아직 뷰가 안나오긴 했지만, Weeth에서는 정보 수정을 여러 API로 나누는게 아니라 여러 정보를 한 API로 받되, PATCH에 맞게 변경된 값만 저장하는 식으로 관리하고 있어요!
그래서 API를 나누기 보단 "동아리 정보 수정" API처럼 null인 경우는 수정 안함, 값이 있는 경우는 수정 이렇게 API를 합쳐야할 것 같아요!
hyxklee
reviewed
Mar 22, 2026
| return CommonResponse.success(UserResponseCode.USER_TERMS_AGREE_SUCCESS) | ||
| } | ||
|
|
||
| @PutMapping("/profile-image") |
hyxklee
approved these changes
Mar 23, 2026
This was referenced Mar 23, 2026
This was referenced Mar 31, 2026
Merged
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.
📌 Summary
사용자 프로필 기능을 추가했습니다.
📝 Changes
What
Why
How
📸 Screenshots / Logs
💡 Reviewer 참고사항
✅ Checklist
Summary by CodeRabbit
새로운 기능
개선 사항