[YS-233] feat: 회원 탈퇴 API 구현 #89
Conversation
… soft-deleted members
…er.deletedAt.isNull condition
Walkthrough이번 PR은 MemberService에 멤버 삭제(deleteMember)와 역할 조회(getMemberRole) 관련 신규 기능을 추가하며, 관련 Use Case와 Gateway, 도메인 모델이 확장되었습니다. 또한, 기존 enum 파일들의 패키지 재구성과 삭제, 새로운 enum 클래스 도입, 멤버의 soft-delete를 위한 deletedAt 필드 추가 등 전반적인 코드 재구조화가 이루어졌습니다. 테스트 코드도 이러한 변경 사항에 맞춰 업데이트되었습니다. Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🔭 Outside diff range comments (3)
src/test/kotlin/com/dobby/backend/application/usecase/member/UpdateParticipantInfoUseCaseTest.kt (1)
25-183: PR 목적에 맞는 포괄적인 테스트 보완이 필요합니다.현재 테스트 케이스들이 PR에서 구현된 주요 기능들을 충분히 검증하지 못하고 있습니다. 다음 시나리오들에 대한 테스트 추가를 고려해주세요:
- 회원 상태가 HOLD로 변경된 경우의 처리
- 이메일 마스킹 처리 검증
- findByIdAndDeletedAtIsNull 조건에 따른 동작 검증
제안드린 테스트 케이스 구현에 도움이 필요하시다면 말씀해 주세요.
src/main/kotlin/com/dobby/backend/application/usecase/member/CreateResearcherUseCase.kt (1)
68-88: 입력 유효성 검사 및 예외 처리 보완 필요
createResearcher메서드에서 다음 사항들을 검증하면 좋을 것 같습니다:
- 이메일 형식 유효성 검사 (
oauthEmail,contactEmail,univEmail)- 필수 필드 null 체크
- 중복 이메일 체크
private fun createResearcher(input: Input): Researcher { + validateEmails(input) + checkDuplicateEmails(input) + val member = Member.newMember( id = idGenerator.generateId(), oauthEmail = input.oauthEmail, contactEmail = input.contactEmail, provider = input.provider, role = RoleType.RESEARCHER, name = input.name, ) // ... } +private fun validateEmails(input: Input) { + require(isValidEmail(input.oauthEmail)) { "유효하지 않은 OAuth 이메일 형식입니다" } + require(isValidEmail(input.contactEmail)) { "유효하지 않은 연락처 이메일 형식입니다" } + require(isValidEmail(input.univEmail)) { "유효하지 않은 대학 이메일 형식입니다" } +} + +private fun checkDuplicateEmails(input: Input) { + require(input.oauthEmail != input.contactEmail) { "OAuth 이메일과 연락처 이메일은 달라야 합니다" } + require(input.oauthEmail != input.univEmail) { "OAuth 이메일과 대학 이메일은 달라야 합니다" } + require(input.contactEmail != input.univEmail) { "연락처 이메일과 대학 이메일은 달라야 합니다" } +}src/test/kotlin/com/dobby/backend/infrastructure/token/JwtTokenProviderTest.kt (1)
43-66: 탈퇴 회원에 대한 토큰 검증 테스트 필요탈퇴한 회원(deletedAt이 null이 아닌 경우)에 대한 토큰 검증 테스트가 누락되어 있습니다.
다음과 같이 테스트 케이스를 추가해주세요:
given("탈퇴한 회원의 JWT 토큰이 주어지고") { val withdrawnMember = Member( id = "1", oauthEmail = "[email protected]", contactEmail = "[email protected]", provider = ProviderType.NAVER, role = RoleType.PARTICIPANT, name = "dobby", status = MemberStatus.HOLD, createdAt = LocalDateTime.now(), updatedAt = LocalDateTime.now(), deletedAt = LocalDateTime.now() ) val authorities = listOf(SimpleGrantedAuthority(withdrawnMember.role?.name ?: "PARTICIPANT")) val authentication = UsernamePasswordAuthenticationToken(withdrawnMember.id, null, authorities) val token = jwtTokenProvider.generateAccessToken(authentication) `when`("해당 토큰을 파싱하면") { then("AuthenticationTokenNotValidException 예외가 발생해야 한다") { assertFailsWith<AuthenticationTokenNotValidException> { jwtTokenProvider.parseAuthentication(token) } } } }
🧹 Nitpick comments (13)
src/main/kotlin/com/dobby/backend/infrastructure/database/repository/ResearcherRepository.kt (1)
8-8: 새로운 메서드 추가가 적절합니다!회원 탈퇴 기능 구현을 위해
deletedAt필드를 확인하는 메서드가 잘 추가되었습니다.다만, 메서드의 목적과 사용 사례를 명확히 하기 위해 KDoc 문서화를 추가하면 좋을 것 같습니다.
+ /** + * memberId로 연구자를 조회하되, 탈퇴하지 않은 회원만 조회합니다. + * + * @param memberId 조회할 회원의 ID + * @return 탈퇴하지 않은 연구자 정보, 없을 경우 null + */ fun findByMemberIdAndMemberDeletedAtIsNull(memberId: String): ResearcherEntity?src/main/kotlin/com/dobby/backend/domain/gateway/member/ResearcherGateway.kt (1)
7-7: 회원 탈퇴 기능에 맞게 적절한 메소드가 추가되었습니다.
findByMemberIdAndMemberDeletedAtIsNull메소드는:
- 탈퇴하지 않은 연구자만 조회하는 요구사항을 잘 반영했습니다
- 기존
findByMemberId메소드와 일관된 네이밍 컨벤션을 따르고 있습니다다만 한 가지 제안드립니다:
메소드 이름이 다소 길어서, 아래와 같이 더 간단한 이름을 고려해보시는 것은 어떨까요?
-fun findByMemberIdAndMemberDeletedAtIsNull(memberId: String): Researcher? +fun findActiveByMemberId(memberId: String): Researcher?src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/MemberGatewayImpl.kt (1)
59-61: 역할 조회 메서드가 잘 구현되었습니다.탈퇴하지 않은 회원의 역할만 조회하도록 구현된 점이 좋습니다. 다만, 다음 사항을 고려해보시면 좋겠습니다:
- 메서드에 대한 문서화 (KDoc)를 추가하여
null을 반환하는 경우를 명확히 설명- 탈퇴한 회원의 역할을 조회하려 할 때의 로깅 추가 고려
다음과 같이 KDoc을 추가하는 것을 제안드립니다:
+ /** + * 탈퇴하지 않은 회원의 역할을 조회합니다. + * @param memberId 회원 ID + * @return 회원의 역할. 회원이 존재하지 않거나 탈퇴한 경우 null + */ override fun findRoleByIdAndDeletedAtIsNull(memberId: String): RoleType? { return memberRepository.findRoleByIdAndDeletedAtIsNull(memberId) }src/main/kotlin/com/dobby/backend/application/usecase/member/DeleteResearcherUseCase.kt (1)
23-25: Output 구조가 직관적입니다
isSuccess 필드만으로 성공 여부를 간단히 전달할 수 있습니다. 다만, 에러 상황에서 원인을 함께 반환해야 하는 요구사항이 있을 경우, 에러 코드나 메시지 필드를 추가하는 방안도 고려할 수 있습니다.src/main/kotlin/com/dobby/backend/presentation/api/dto/request/member/DeleteMemberRequest.kt (1)
7-14: 데이터 클래스 구현이 잘 되었습니다!검증 어노테이션과 스키마 설명이 적절하게 구현되어 있습니다. 다만,
reason필드의 경우reasonType이OTHER일 때만 사용되는 것으로 보이므로, 이를 명확히 하기 위해 스키마 설명을 더 자세히 작성하는 것이 좋겠습니다.다음과 같이 스키마 설명을 수정하는 것을 제안합니다:
- @Schema(description = "기타 탈퇴 사유") + @Schema(description = "기타 탈퇴 사유 (reasonType이 OTHER인 경우에만 사용)")src/main/kotlin/com/dobby/backend/domain/model/member/MemberWithdrawal.kt (1)
6-25: 도메인 모델이 잘 설계되었습니다만, 몇 가지 개선사항이 있습니다.도메인 모델의 기본 구조는 잘 되어있으나, 다음과 같은 개선사항을 제안드립니다:
otherReason필드가reasonType이OTHER일 때만 의미가 있으므로, 이를 검증하는 로직이 필요합니다.createdAt이 미래 시간으로 설정되는 것을 방지하는 것이 좋겠습니다.다음과 같은 검증 로직 추가를 제안합니다:
data class MemberWithdrawal( val memberId: String, val reasonType: WithdrawalReasonType, val otherReason: String? = null, val createdAt: LocalDateTime ) { + init { + require(!(reasonType == WithdrawalReasonType.OTHER && otherReason == null)) { + "reasonType이 OTHER일 경우 otherReason은 필수입니다." + } + require(!createdAt.isAfter(LocalDateTime.now())) { + "생성 시간이 미래일 수 없습니다." + } + } + companion object { fun newWithdrawal( memberId: String,src/main/kotlin/com/dobby/backend/domain/gateway/member/MemberGateway.kt (1)
9-9: 새로운 메서드들이 잘 추가되었습니다만, 문서화가 필요합니다.
deletedAt이 null인 경우만 조회하는 메서드들이 추가되었습니다. 이러한 메서드들의 목적과 동작을 명확히 하기 위해 KDoc 문서화를 추가하는 것이 좋겠습니다.다음과 같은 문서화 추가를 제안합니다:
+ /** + * 삭제되지 않은(deletedAt이 null인) 회원을 ID로 조회합니다. + * @param memberId 회원 ID + * @return 삭제되지 않은 회원 정보, 없을 경우 null + */ fun findByIdAndDeletedAtIsNull(memberId: String): Member? + /** + * 삭제되지 않은(deletedAt이 null인) 회원의 역할을 조회합니다. + * @param memberId 회원 ID + * @return 회원의 역할, 회원이 없거나 삭제된 경우 null + */ fun findRoleByIdAndDeletedAtIsNull(memberId: String): RoleType?Also applies to: 16-16
src/main/kotlin/com/dobby/backend/infrastructure/database/entity/member/MemberWithdrawalEntity.kt (1)
11-13: member_id에 인덱스 추가를 고려해보세요.
member_id필드는 조회 조건으로 자주 사용될 것으로 예상되므로, 성능 최적화를 위해 인덱스 추가를 권장드립니다.다음과 같이 수정해주세요:
@Id + @Index(name = "idx_member_withdrawal_member_id") @Column(name = "member_id", columnDefinition = "CHAR(13)") val memberId: String,src/main/kotlin/com/dobby/backend/domain/model/member/Researcher.kt (1)
54-60: 민감 정보 마스킹 처리 개선 제안빈 문자열로 마스킹하는 것보다 더 명시적인 마스킹 패턴을 사용하는 것이 좋습니다.
다음과 같이 수정하는 것을 고려해보세요:
- univEmail = "", - univName = "", - major = "", + univEmail = "DELETED_${member.id}@masked.com", + univName = "MASKED_UNIV", + major = "MASKED_MAJOR",src/main/kotlin/com/dobby/backend/infrastructure/database/entity/member/MemberEntity.kt (1)
48-49: deletedAt 컬럼 정의를 개선해주세요.
deletedAt컬럼에 대한 상세한 정의가 필요합니다. 인덱스 추가를 고려해보세요.- @Column(name = "deleted_at") + @Column(name = "deleted_at", columnDefinition = "TIMESTAMP") + @Index(name = "idx_member_deleted_at") val deletedAt: LocalDateTime?src/test/kotlin/com/dobby/backend/application/usecase/member/DeleteParticipantUseCaseTest.kt (1)
21-59: 테스트 케이스 보완 제안성공 케이스에서 다음 사항들도 검증하면 좋을 것 같습니다:
- 회원 상태가
HOLD로 변경되는지 확인deletedAt필드가 설정되는지 확인oauthEmail이 마스킹되는지 확인then("isSuccess가 true여야 한다") { output.isSuccess shouldBe true } +then("회원 상태가 HOLD로 변경되어야 한다") { + verify { + memberGateway.save(match { + it.status == MemberStatus.HOLD && + it.deletedAt != null && + it.oauthEmail.endsWith("*****") + }) + } +}src/test/kotlin/com/dobby/backend/application/usecase/auth/FetchGoogleUserInfoUseCaseTest.kt (1)
30-82: 테스트 케이스 보완 제안다음과 같은 추가 테스트 케이스들을 고려해보면 좋을 것 같습니다:
- 탈퇴한 회원(
deletedAt이 null이 아닌 경우)이 로그인을 시도하는 경우- 구글 API 호출 실패 시의 예외 처리
- 토큰 생성 실패 시의 예외 처리
+ given("탈퇴한 회원이 로그인을 시도하는 경우") { + val deletedMember = mockMember.copy( + deletedAt = LocalDateTime.now(), + status = MemberStatus.HOLD + ) + every { memberGateway.findByOauthEmailAndStatus("[email protected]", MemberStatus.ACTIVE) } returns null + + `when`("로그인을 시도하면") { + val result = fetchGoogleUserInfoUseCase.execute(input) + + then("신규 가입으로 처리되어야 한다") { + result.isRegistered shouldBe false + } + } + }src/main/kotlin/com/dobby/backend/application/service/MemberService.kt (1)
79-82: 예외 처리 메시지 개선 필요
getMemberRole메소드에서 발생하는MemberNotFoundException예외에 상세한 메시지가 없어 디버깅이 어려울 수 있습니다.다음과 같이 개선해주세요:
fun getMemberRole(memberId: String): RoleType { return memberGateway.findRoleByIdAndDeletedAtIsNull(memberId) - ?: throw MemberNotFoundException + ?: throw MemberNotFoundException("Member not found with ID: $memberId") }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (84)
src/main/kotlin/com/dobby/backend/application/service/MemberService.kt(3 hunks)src/main/kotlin/com/dobby/backend/application/usecase/auth/FetchGoogleUserInfoUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/auth/FetchNaverUserInfoUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/auth/GenerateTestTokenUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/experiment/CreateExperimentPostUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostsUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/experiment/UpdateExperimentPostUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/CreateParticipantUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/CreateResearcherUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/DeleteParticipantUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/DeleteResearcherUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/GetParticipantInfoUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/application/usecase/member/UpdateParticipantInfoUseCase.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/gateway/email/VerificationGateway.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/gateway/member/MemberGateway.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/gateway/member/MemberWithdrawalGateway.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/gateway/member/ResearcherGateway.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/model/experiment/CustomFilter.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/model/experiment/ExperimentPost.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/model/experiment/TargetGroup.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/model/member/Member.kt(3 hunks)src/main/kotlin/com/dobby/backend/domain/model/member/MemberWithdrawal.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/model/member/Participant.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/model/member/Researcher.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/GenderType.kt(0 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/MemberStatus.kt(0 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/ProviderType.kt(0 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/experiment/TimeSlot.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/member/GenderType.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/member/MemberStatus.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/member/ProviderType.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/member/RoleType.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/member/WithdrawalReasonType.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/experiment/ExperimentPostEntity.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/experiment/TargetGroupEntity.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/member/MemberEntity.kt(4 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/member/MemberWithdrawalEntity.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/entity/member/ParticipantEntity.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/repository/ExperimentPostCustomRepositoryImpl.kt(3 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/repository/MemberRepository.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/repository/MemberWithdrawalRepository.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/repository/ResearcherRepository.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/database/repository/VerificationRepository.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/gateway/email/VerificationGatewayImpl.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/MemberGatewayImpl.kt(3 hunks)src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/MemberWithdrawalGatewayImpl.kt(1 hunks)src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/ResearcherGatewayImpl.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/controller/AuthController.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/controller/EmailController.kt(0 hunks)src/main/kotlin/com/dobby/backend/presentation/api/controller/ExperimentPostController.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/controller/MemberController.kt(2 hunks)src/main/kotlin/com/dobby/backend/presentation/api/dto/request/experiment/CreateExperimentPostRequest.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/dto/request/experiment/RelationInfo.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/dto/request/experiment/UpdateExperimentPostRequest.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/dto/request/member/DeleteMemberRequest.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/dto/request/member/ParticipantSignupRequest.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/dto/request/member/ResearcherSignupRequest.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/dto/response/experiment/ExperimentPostDetailResponse.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/dto/response/member/MemberResponse.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/dto/response/member/ParticipantInfoResponse.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/mapper/AuthMapper.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/mapper/ExperimentPostMapper.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/mapper/MemberMapper.kt(2 hunks)src/test/kotlin/com/dobby/backend/application/usecase/auth/FetchGoogleUserInfoUseCaseTest.kt(2 hunks)src/test/kotlin/com/dobby/backend/application/usecase/auth/FetchNaverUserInfoUseCaseTest.kt(2 hunks)src/test/kotlin/com/dobby/backend/application/usecase/auth/GenerateTestTokenUseCaseTest.kt(3 hunks)src/test/kotlin/com/dobby/backend/application/usecase/auth/GenerateTokenWithRefreshTokenUseCaseTest.kt(2 hunks)src/test/kotlin/com/dobby/backend/application/usecase/experiment/CreateExperimentPostUseCaseTest.kt(5 hunks)src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostApplyMethodUseCaseTest.kt(1 hunks)src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostDetailUseCaseTest.kt(2 hunks)src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostsUseCaseTest.kt(6 hunks)src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetMyExperimentPostsUseCaseTest.kt(1 hunks)src/test/kotlin/com/dobby/backend/application/usecase/experiment/UpdateExperimentPostRecruitStatusUseCaseTest.kt(1 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/CreateParticipantUseCaseTest.kt(1 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/CreateResearcherUseCaseTest.kt(1 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/DeleteParticipantUseCaseTest.kt(1 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/DeleteResearcherUseCaseTest.kt(1 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/GetParticipantInfoUseCaseTest.kt(1 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/GetResearcherInfoUseCaseTest.kt(2 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/UpdateParticipantInfoUseCaseTest.kt(4 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/UpdateResearcherInfoUseCaseTest.kt(4 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/email/GetMatchingExperimentPostsUseCaseTest.kt(2 hunks)src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCaseTest.kt(3 hunks)src/test/kotlin/com/dobby/backend/infrastructure/token/JwtTokenProviderTest.kt(3 hunks)
💤 Files with no reviewable changes (4)
- src/main/kotlin/com/dobby/backend/presentation/api/controller/EmailController.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/MemberStatus.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/GenderType.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/ProviderType.kt
✅ Files skipped from review due to trivial changes (39)
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/experiment/ExperimentPostEntity.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/member/MemberStatus.kt
- src/main/kotlin/com/dobby/backend/presentation/api/mapper/ExperimentPostMapper.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/repository/MemberWithdrawalRepository.kt
- src/main/kotlin/com/dobby/backend/presentation/api/dto/request/experiment/CreateExperimentPostRequest.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/member/GenderType.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/member/ParticipantEntity.kt
- src/main/kotlin/com/dobby/backend/domain/model/member/Participant.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/member/WithdrawalReasonType.kt
- src/main/kotlin/com/dobby/backend/presentation/api/controller/AuthController.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/member/RoleType.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/experiment/TargetGroupEntity.kt
- src/test/kotlin/com/dobby/backend/application/usecase/member/GetParticipantInfoUseCaseTest.kt
- src/main/kotlin/com/dobby/backend/application/usecase/member/UpdateParticipantInfoUseCase.kt
- src/main/kotlin/com/dobby/backend/domain/model/experiment/CustomFilter.kt
- src/main/kotlin/com/dobby/backend/presentation/api/dto/response/member/ParticipantInfoResponse.kt
- src/main/kotlin/com/dobby/backend/presentation/api/dto/request/experiment/RelationInfo.kt
- src/main/kotlin/com/dobby/backend/application/usecase/auth/FetchGoogleUserInfoUseCase.kt
- src/main/kotlin/com/dobby/backend/domain/model/experiment/TargetGroup.kt
- src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostsUseCase.kt
- src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostApplyMethodUseCaseTest.kt
- src/test/kotlin/com/dobby/backend/application/usecase/member/CreateResearcherUseCaseTest.kt
- src/main/kotlin/com/dobby/backend/application/usecase/experiment/CreateExperimentPostUseCase.kt
- src/main/kotlin/com/dobby/backend/presentation/api/dto/response/member/MemberResponse.kt
- src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/experiment/TimeSlot.kt
- src/test/kotlin/com/dobby/backend/application/usecase/experiment/UpdateExperimentPostRecruitStatusUseCaseTest.kt
- src/main/kotlin/com/dobby/backend/presentation/api/dto/request/member/ResearcherSignupRequest.kt
- src/main/kotlin/com/dobby/backend/application/usecase/auth/FetchNaverUserInfoUseCase.kt
- src/main/kotlin/com/dobby/backend/application/usecase/experiment/UpdateExperimentPostUseCase.kt
- src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostDetailUseCaseTest.kt
- src/main/kotlin/com/dobby/backend/domain/model/experiment/ExperimentPost.kt
- src/main/kotlin/com/dobby/backend/presentation/api/mapper/AuthMapper.kt
- src/main/kotlin/com/dobby/backend/application/usecase/member/GetParticipantInfoUseCase.kt
- src/main/kotlin/com/dobby/backend/presentation/api/dto/request/member/ParticipantSignupRequest.kt
- src/test/kotlin/com/dobby/backend/application/usecase/member/CreateParticipantUseCaseTest.kt
- src/test/kotlin/com/dobby/backend/application/usecase/auth/FetchNaverUserInfoUseCaseTest.kt
- src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetMyExperimentPostsUseCaseTest.kt
- src/main/kotlin/com/dobby/backend/presentation/api/controller/ExperimentPostController.kt
- src/main/kotlin/com/dobby/backend/presentation/api/dto/request/experiment/UpdateExperimentPostRequest.kt
🧰 Additional context used
📓 Learnings (3)
src/main/kotlin/com/dobby/backend/infrastructure/gateway/email/VerificationGatewayImpl.kt (1)
Learnt from: chock-cho
PR: YAPP-Github/25th-Web-Team-2-BE#67
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/EmailCodeSendUseCase.kt:44-56
Timestamp: 2025-01-29T16:14:08.389Z
Learning: The VerificationRepository in this codebase implements a robust concurrency control mechanism with:
1. Pessimistic write locking (@Lock(LockModeType.PESSIMISTIC_WRITE)) on findByUnivEmail
2. Proper transaction boundaries using @Modifying annotations
3. Automatic timestamp updates in native queries for code updates
src/main/kotlin/com/dobby/backend/application/service/MemberService.kt (1)
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#81
File: src/main/kotlin/com/dobby/backend/application/usecase/member/ValidateContactEmailForUpdateUseCase.kt:19-22
Timestamp: 2025-02-05T13:26:43.470Z
Learning: The `MemberGateway.findContactEmailByMemberId()` method always returns a non-null String value containing the member's contact email.
src/test/kotlin/com/dobby/backend/application/usecase/member/email/GetMatchingExperimentPostsUseCaseTest.kt (1)
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#77
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/GetMatchingExperimentPostsUseCase.kt:16-18
Timestamp: 2025-02-03T01:27:08.183Z
Learning: In Kotlin, prefer using non-nullable collections (e.g., `List<T>`) with `emptyList()` as default value instead of nullable collections (e.g., `List<T>?`) to improve null safety and reduce the need for null checks.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (38)
src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/ResearcherGatewayImpl.kt (1)
20-24: 구현이 깔끔하고 일관성이 있습니다!기존
findByMemberId메서드와 동일한 패턴을 따르면서도 탈퇴한 회원을 적절히 필터링하는 로직이 잘 구현되어 있습니다. 도메인 모델 변환도 올바르게 처리되어 있습니다.src/main/kotlin/com/dobby/backend/domain/gateway/member/ResearcherGateway.kt (1)
5-10: 인터페이스 구조가 깔끔하고 일관성이 있습니다!게이트웨이 패턴을 잘 활용하여 도메인 로직과 영속성 계층을 깔끔하게 분리했습니다.
src/test/kotlin/com/dobby/backend/application/usecase/member/UpdateParticipantInfoUseCaseTest.kt (1)
12-15: LGTM! enum 클래스들의 패키지 구조가 잘 정리되었습니다.member 관련 enum 클래스들이 적절한 하위 패키지로 이동되어 더 명확한 구조를 가지게 되었습니다.
src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCaseTest.kt (2)
15-19: LGTM!enum 클래스들의 패키지 구조 변경이 잘 반영되었습니다.
49-50: 탈퇴 회원에 대한 테스트 케이스 추가 필요회원 탈퇴 API 구현과 관련하여,
deletedAt필드가 추가되었지만 탈퇴한 회원에게 이메일이 발송되지 않는지 확인하는 테스트 케이스가 누락되었습니다.다음과 같은 테스트 케이스 추가를 제안드립니다:
given("이메일 매칭 발송을 실행할 때") { + `when`("탈퇴한 회원의 실험 공고가 주어지면") { + val contactEmail = "[email protected]" + val withdrawnMember = Member( + id = "3", + role = RoleType.RESEARCHER, + contactEmail = "[email protected]", + oauthEmail = "[email protected]", + name = "탈퇴회원", + provider = ProviderType.NAVER, + status = MemberStatus.HOLD, + createdAt = LocalDateTime.now().minusDays(2), + updatedAt = LocalDateTime.now().minusDays(2), + deletedAt = LocalDateTime.now().minusDays(1) + ) + + val experimentPosts = listOf( + ExperimentPost( + id = "3", + member = withdrawnMember, + // ... other fields same as existing test + ) + ) + val input = SendMatchingEmailUseCase.Input(contactEmail, experimentPosts, LocalDateTime.now()) + + then("이메일이 발송되지 않아야 한다") { + val output = sendMatchingEmailUseCase.execute(input) + output.isSuccess shouldBe true + verify(exactly = 0) { emailGateway.sendEmail(any(), any(), any()) } + } + }Also applies to: 114-115
✅ Verification successful
검증 결과: 탈퇴 회원에 대한 테스트 케이스가 추가되어야 합니다.
현재 테스트 파일인
src/test/kotlin/com/dobby/backend/application/usecase/member/email/SendMatchingEmailUseCaseTest.kt
내에서deletedAt필드가 null 인 경우만 확인되고 있으며, 탈퇴한 회원(즉,deletedAt값이 존재하는 경우)에 대해 이메일 발송이 일어나지 않는지 검증하는 케이스가 누락되어 있습니다. 리뷰어가 제안한 코드 스니펫과 같이 해당 상황을 검증하는 테스트 케이스를 추가해야 합니다.
- 탈퇴 회원의 경우,
deletedAt이 현재 시각보다 이전의 값으로 설정되어 있어야 하며, 이 경우 이메일 발송 메서드가 호출되지 않아야 합니다.- 리뷰 코멘트에서 동일한 검증이 114-115 라인에도 적용되어야 하므로, 해당 테스트 파일의 다른 부분에도 동일한 사례가 누락되지 않았는지 확인이 필요합니다.
src/main/kotlin/com/dobby/backend/infrastructure/database/repository/MemberRepository.kt (3)
4-5: 패키지 구조 개선이 잘 되었습니다!member 관련 enum들을 별도 패키지로 구성한 것이 도메인 응집도를 높이는데 도움이 됩니다.
7-8: Spring Data JPA 어노테이션 추가가 적절합니다.커스텀 쿼리 구현을 위한 필수 어노테이션들이 잘 추가되었습니다.
16-18: 🏁 Script executed:#!/bin/bash # Description: Find usages of findById that might need migration # Search for findById usage in Kotlin files rg "findById\(" --type kotlinLength of output: 1632
src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/MemberGatewayImpl.kt (2)
4-4: 패키지 구조 개선이 잘 이루어졌습니다!member 관련 enum들을 하나의 패키지로 구조화한 것이 좋습니다.
Also applies to: 6-6
21-25: 메서드 이름 변경이 적절합니다.탈퇴한 회원을 필터링하기 위한
findByIdAndDeletedAtIsNull메서드명이 명확하게 의도를 전달합니다. 안전한 null 처리를 위해 safe call operator를 사용한 것도 좋습니다.다음 스크립트를 실행하여 이 메서드를 사용하는 모든 곳에서 탈퇴 회원 필터링이 제대로 적용되었는지 확인해주세요:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: 탈퇴 회원 필터링 적용 여부 확인 # findById 호출이 모두 findByIdAndDeletedAtIsNull로 변경되었는지 확인 rg -A 5 "findById[^A]"Length of output: 7574
검증 결과: 탈퇴 회원 필터링 구현이 의도대로 작동합니다.
검토한 MemberGatewayImpl의 findByIdAndDeletedAtIsNull 메서드는 회원의 탈퇴 여부를 올바르게 확인하도록 구현되어 있으며, safe call operator 사용으로 null 안전성도 확보되었습니다.
또한, 스크립트 실행 결과에서 확인된 findById 호출은 실험(Experiment) 관련 게이트웨이에서 발생한 것으로, 해당 부분은 본 PR의 대상이 아니며 의도된 동작임을 확인했습니다.src/main/kotlin/com/dobby/backend/presentation/api/mapper/MemberMapper.kt (1)
7-7: LGTM!enum 패키지 구조가 도메인 중심 설계를 잘 반영하고 있습니다.
src/main/kotlin/com/dobby/backend/application/usecase/member/DeleteResearcherUseCase.kt (3)
1-2: 패키지 구조가 명확합니다
해당 패키지 네이밍이 기능별로 잘 구분되어 있어, 유지보수 및 확장에 용이할 것으로 보입니다.
3-10: Import 정합성 점검
현재 import 구문들 모두 실제로 사용되는 클래스/인터페이스로 보이며 불필요한 부분이 없습니다.
17-21: Input 데이터 클래스 설계가 간결합니다
memberId, reasonType, reason 필드가 필요한 정보를 명확히 표현하고 있으며, reason 필드를 nullable로 둬 선택적 입력을 허용하는 방식이 적절해 보입니다.src/main/kotlin/com/dobby/backend/infrastructure/database/entity/enums/member/ProviderType.kt (1)
3-5: 열거형 정의가 명확합니다
NAVER, GOOGLE 두 가지 타입만을 간결하게 지원하며, 추후 다른 소셜/외부 연동이 필요하다면 손쉽게 확장 가능합니다.src/main/kotlin/com/dobby/backend/domain/gateway/member/MemberWithdrawalGateway.kt (1)
5-7: Withdrawal 기록용 Gateway 인터페이스가 단순·명확합니다
멤버의 탈퇴 이력을 별도로 관리하려는 설계는 데이터 추적으로 적절해 보입니다. 추가적인 조회 기능이 필요하다면 메서드를 확장하는 방식으로 유지보수가 용이할 것으로 보입니다.src/main/kotlin/com/dobby/backend/domain/gateway/email/VerificationGateway.kt (1)
11-11: 이메일 검증 게이트웨이에 삭제 기능이 적절하게 추가되었습니다!회원 탈퇴 시 이메일 검증 정보를 정리하는 용도로 적절한 메서드가 추가되었습니다.
src/main/kotlin/com/dobby/backend/infrastructure/gateway/member/MemberWithdrawalGatewayImpl.kt (1)
9-18: 구현이 깔끔하고 명확합니다!도메인 모델과 엔티티 간의 변환이 잘 구현되어 있으며, 단일 책임 원칙을 잘 준수하고 있습니다.
src/main/kotlin/com/dobby/backend/application/usecase/auth/GenerateTestTokenUseCase.kt (1)
25-26: 회원 조회 로직이 적절하게 수정되었습니다!삭제된 회원에 대한 토큰 생성을 방지하기 위해
findByIdAndDeletedAtIsNull을 사용하는 것이 올바른 접근입니다.src/main/kotlin/com/dobby/backend/infrastructure/database/repository/VerificationRepository.kt (1)
29-29: 🛠️ Refactor suggestion@Modifying 어노테이션 추가가 필요합니다.
데이터를 수정하는 작업이므로
@Modifying어노테이션을 추가해야 합니다.다음과 같이 수정해주세요:
+ @Modifying fun deleteByUnivEmail(univEmail: String)Likely invalid or redundant comment.
src/main/kotlin/com/dobby/backend/infrastructure/database/entity/member/MemberWithdrawalEntity.kt (1)
8-24: 엔티티 설계가 잘 되어있습니다!필요한 필드들이 적절하게 정의되어 있고, JPA 어노테이션도 올바르게 사용되었습니다.
src/main/kotlin/com/dobby/backend/application/usecase/member/DeleteParticipantUseCase.kt (1)
24-39: 트랜잭션 경계 확인 필요여러 저장소 작업이 순차적으로 실행되고 있습니다. 트랜잭션 실패 시 데이터 일관성이 깨질 수 있습니다.
트랜잭션 어노테이션이 상위 레이어에 제대로 적용되어 있는지 확인해 주세요:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for @Transactional annotations in service layer rg -A 2 "@Transactional.*class.*MemberService"Length of output: 48
🏁 Script executed:
#!/bin/bash # 전체 Kotlin 코드에서 @Transactional 어노테이션을 검색하여 트랜잭션 경계 설정 여부를 확인합니다. rg -A 2 "@Transactional" src/main/kotlinLength of output: 9778
트랜잭션 경계는 상위 레이어에서 보장되고 있음
DeleteParticipantUseCase 내부에 직접 @transactional 어노테이션이 없어도, 관련된 다른 서비스 메서드(MemberService 등)에서 이미 @transactional이 적용되어 있는 것으로 확인되었습니다.
- MemberService.kt 내 여러 메서드에 @transactional 어노테이션이 존재
- 상위 서비스 레이어에서 DeleteParticipantUseCase와 유사한 저장소 작업들이 트랜잭션 내에서 호출되고 있음
따라서, 현재 코드 구조 상 트랜잭션 경계 문제가 발생할 여지가 낮아 보입니다.
src/test/kotlin/com/dobby/backend/application/usecase/auth/GenerateTestTokenUseCaseTest.kt (2)
8-10: 패키지 구조 개선이 잘 되었습니다.enum 클래스들을 member 패키지로 이동하여 도메인 관심사를 명확하게 분리했습니다.
31-31: 회원 조회 로직이 적절히 업데이트되었습니다.
findById에서findByIdAndDeletedAtIsNull로 변경하여 탈퇴하지 않은 회원만 조회하도록 수정했습니다.Also applies to: 43-43
src/main/kotlin/com/dobby/backend/infrastructure/database/entity/member/MemberEntity.kt (1)
61-62: 도메인 변환 메서드가 잘 수정되었습니다.
toDomain()과fromDomain()메서드에deletedAt필드가 적절히 추가되었습니다.Also applies to: 76-77
src/test/kotlin/com/dobby/backend/application/usecase/auth/GenerateTokenWithRefreshTokenUseCaseTest.kt (1)
24-24: 삭제된 회원에 대한 테스트 케이스가 필요합니다.삭제된 회원(
deletedAt이 null이 아닌 경우)이 토큰을 재발급 받으려고 할 때의 케이스를 검증하는 테스트를 추가해주세요.Also applies to: 31-31
src/test/kotlin/com/dobby/backend/application/usecase/member/GetResearcherInfoUseCaseTest.kt (1)
32-33: 삭제된 연구자에 대한 테스트 케이스가 필요합니다.탈퇴한 연구자의 정보를 조회하려고 할 때의 케이스를 검증하는 테스트를 추가해주세요.
src/main/kotlin/com/dobby/backend/presentation/api/dto/response/experiment/ExperimentPostDetailResponse.kt (1)
3-5: LGTM!enum 클래스들의 패키지 구조가 더 명확하게 정리되었습니다.
src/main/kotlin/com/dobby/backend/application/usecase/member/CreateParticipantUseCase.kt (1)
12-14: 패키지 구조 개선이 잘 되었습니다멤버 관련 enum들을 member 패키지로 이동하여 도메인 응집도가 향상되었습니다.
src/test/kotlin/com/dobby/backend/application/usecase/member/email/GetMatchingExperimentPostsUseCaseTest.kt (1)
9-13: 회원 삭제 기능 구현을 위한 변경사항이 적절히 반영되었습니다.Member 객체에 deletedAt 필드가 추가되었고, 관련 enum들이 적절한 패키지로 이동되었습니다.
Also applies to: 43-43
src/main/kotlin/com/dobby/backend/presentation/api/controller/MemberController.kt (1)
134-148: 회원 탈퇴 API가 적절히 구현되었습니다.회원 탈퇴 로직이 다음과 같이 잘 구현되어 있습니다:
- 현재 회원의 ID를 가져옵니다
- 회원의 역할을 확인합니다
- 적절한 입력값으로 변환 후 삭제를 수행합니다
src/test/kotlin/com/dobby/backend/application/usecase/member/UpdateResearcherInfoUseCaseTest.kt (1)
9-11: 테스트 케이스에 회원 삭제 상태 필드가 적절히 추가되었습니다.모든 테스트 케이스의 Member 객체에 deletedAt 필드가 null로 초기화되어 추가되었고, enum 클래스들의 패키지 구조가 개선되었습니다.
Also applies to: 33-33, 104-104, 150-150
src/test/kotlin/com/dobby/backend/application/usecase/experiment/CreateExperimentPostUseCaseTest.kt (1)
14-18: 실험 게시글 생성 테스트에 회원 삭제 상태 필드가 적절히 반영되었습니다.다음 사항들이 잘 구현되어 있습니다:
- Member 객체의 모든 인스턴스에 deletedAt 필드가 추가됨
- enum 클래스들이 도메인별로 적절히 분리되어 재구성됨
- 기존 테스트 케이스의 기능이 그대로 유지됨
Also applies to: 45-45, 107-107, 161-161, 217-217
src/main/kotlin/com/dobby/backend/infrastructure/database/repository/ExperimentPostCustomRepositoryImpl.kt (3)
5-5: LGTM: 멤버 관련 enum 패키지 구조 개선멤버 관련 enum들을 member 패키지로 이동하여 도메인 구조가 명확해졌습니다.
244-244: 탈퇴 회원 필터링 로직 추가 👍
deletedAt.isNull조건을 추가하여 탈퇴한 회원에게 실험 공고 알림이 가지 않도록 처리했습니다.
225-253: 로깅 개선이 잘 되었습니다.쿼리 범위와 결과에 대한 로깅이 상세하게 추가되어 디버깅과 모니터링이 용이해졌습니다.
src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostsUseCaseTest.kt (2)
17-20: LGTM: 멤버 관련 enum 임포트 정리member 패키지로 이동된 enum들의 임포트 경로가 올바르게 수정되었습니다.
76-77: 테스트 케이스에 회원 탈퇴 필드 추가 👍모든 Member 인스턴스에
deletedAt필드가 추가되어 회원 탈퇴 기능이 테스트에 잘 반영되었습니다.Also applies to: 159-160, 239-240, 318-319, 397-398
| @@ -30,7 +34,7 @@ class UpdateParticipantInfoUseCaseTest : BehaviorSpec({ | |||
| id = memberId, | |||
| member = Member(id = memberId, name = "기존 이름", contactEmail = "[email protected]", oauthEmail = "[email protected]", | |||
| provider = ProviderType.NAVER, role = RoleType.PARTICIPANT, status = MemberStatus.ACTIVE, | |||
| createdAt = LocalDateTime.now(), updatedAt = LocalDateTime.now()), | |||
| createdAt = LocalDateTime.now(), updatedAt = LocalDateTime.now(), deletedAt = null), | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
회원 탈퇴 상태에 대한 테스트 케이스 추가가 필요합니다.
Member 클래스에 deletedAt 필드가 추가되었지만, 현재 테스트는 모두 deletedAt이 null인 경우만 다루고 있습니다. 회원 탈퇴 API 구현과 관련하여 다음과 같은 테스트 케이스 추가를 제안드립니다:
- 탈퇴한 회원(deletedAt이 null이 아닌 경우)의 정보 수정 시도
- 탈퇴 상태(MemberStatus.HOLD)인 회원의 정보 수정 시도
Also applies to: 108-108, 152-153
| fun toDeleteMemberUseCaseInput(request: DeleteMemberRequest, roleType: RoleType): Any { | ||
| return when (roleType) { | ||
| RoleType.RESEARCHER -> DeleteResearcherUseCase.Input( | ||
| memberId = getCurrentMemberId(), | ||
| reasonType = request.reasonType, | ||
| reason = request.reason | ||
| ) | ||
| RoleType.PARTICIPANT -> DeleteParticipantUseCase.Input( | ||
| memberId = getCurrentMemberId(), | ||
| reasonType = request.reasonType, | ||
| reason = request.reason | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
타입 안전성 개선 및 코드 중복 제거가 필요합니다.
다음과 같은 개선사항을 제안드립니다:
Any타입은 타입 안전성을 저해할 수 있습니다.memberId,reasonType,reason이 두 케이스에서 중복됩니다.- 다른
RoleType값에 대한 처리가 누락되어 있습니다.
다음과 같이 개선해보시는 건 어떨까요?:
-fun toDeleteMemberUseCaseInput(request: DeleteMemberRequest, roleType: RoleType): Any {
+sealed interface DeleteMemberUseCaseInput {
+ data class Researcher(val input: DeleteResearcherUseCase.Input) : DeleteMemberUseCaseInput
+ data class Participant(val input: DeleteParticipantUseCase.Input) : DeleteMemberUseCaseInput
+}
+
+fun toDeleteMemberUseCaseInput(
+ request: DeleteMemberRequest,
+ roleType: RoleType
+): DeleteMemberUseCaseInput {
+ val baseInput = DeleteMemberBaseInput(
+ memberId = getCurrentMemberId(),
+ reasonType = request.reasonType,
+ reason = request.reason
+ )
+
return when (roleType) {
- RoleType.RESEARCHER -> DeleteResearcherUseCase.Input(
- memberId = getCurrentMemberId(),
- reasonType = request.reasonType,
- reason = request.reason
- )
- RoleType.PARTICIPANT -> DeleteParticipantUseCase.Input(
- memberId = getCurrentMemberId(),
- reasonType = request.reasonType,
- reason = request.reason
- )
+ RoleType.RESEARCHER -> DeleteMemberUseCaseInput.Researcher(
+ DeleteResearcherUseCase.Input(
+ memberId = baseInput.memberId,
+ reasonType = baseInput.reasonType,
+ reason = baseInput.reason
+ )
+ )
+ RoleType.PARTICIPANT -> DeleteMemberUseCaseInput.Participant(
+ DeleteParticipantUseCase.Input(
+ memberId = baseInput.memberId,
+ reasonType = baseInput.reasonType,
+ reason = baseInput.reason
+ )
+ )
+ else -> throw IllegalArgumentException("지원하지 않는 역할 유형입니다: $roleType")
}
}Committable suggestion skipped: line range outside the PR's diff.
| override fun execute(input: Input): Output { | ||
| val researcher = researcherGateway.findByMemberId(input.memberId) | ||
| ?: throw MemberNotFoundException | ||
|
|
||
| verificationGateway.deleteByUnivEmail(researcher.univEmail) | ||
| researcherGateway.save(researcher.withdraw()) | ||
|
|
||
| memberWithdrawalGateway.save( | ||
| MemberWithdrawal.newWithdrawal( | ||
| memberId = input.memberId, | ||
| reasonType = input.reasonType, | ||
| otherReason = input.reason | ||
| ) | ||
| ) | ||
|
|
||
| return Output(isSuccess = true) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
트랜잭션 처리 및 실패 대응 로직 고려
현재 메서드는 researcher 조회, verificationGateway 처리, researcher 저장, withdrawal 기록 저장을 순차적으로 수행합니다. 예기치 않은 예외 발생 시 일부 작업만 처리될 위험이 있어, 트랜잭션 범위를 지정하거나 예외 발생 시 롤백 등의 처리가 필요해 보입니다. 또한 verificationGateway나 researcherGateway에서 예외가 발생하는 경우, isSuccess가 항상 true로 리턴되는 점도 주의해야 합니다.
There was a problem hiding this comment.
서비스 계층에서 트랜잭션으로 묶어주고 있어서 문제가 없어보여
There was a problem hiding this comment.
🧩 Analysis chain
@Ji-soo708 네, 말씀하신대로 서비스 계층에서 트랜잭션을 관리하고 있어서 문제가 없을 것 같습니다. 설명 감사합니다.
(｀･ω･´)ゞ
🏁 Script executed:
#!/bin/bash
# Search for transaction annotations in MemberService
rg -A 5 "@Transactional" "src/main/kotlin/com/dobby/backend/application/service/MemberService.kt"
Length of output: 2113
No additional response needed as I've already acknowledged the user's correct explanation about transaction handling in the service layer.
✏️ Learnings added
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#89
File: src/main/kotlin/com/dobby/backend/application/usecase/member/DeleteResearcherUseCase.kt:27-43
Timestamp: 2025-02-08T14:37:16.353Z
Learning: Transaction handling for member-related operations is managed at the service layer (MemberService), not in individual use cases.
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.
| fun withdraw(): Member = copy( | ||
| name = "ExMember", | ||
| oauthEmail = "Deleted_${id}", | ||
| contactEmail = "Deleted_${id}", | ||
| status = MemberStatus.HOLD, | ||
| updatedAt = LocalDateTime.now(), | ||
| deletedAt = LocalDateTime.now(), | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
상태 전이 검증 로직 추가 필요
현재 상태와 관계없이 HOLD 상태로 전환이 가능합니다. 유효하지 않은 상태 전이를 방지하기 위한 검증이 필요합니다.
다음과 같은 검증 로직 추가를 제안합니다:
fun withdraw(): Member = copy(
+ ).also {
+ require(status == MemberStatus.ACTIVE) {
+ "Only ACTIVE members can be withdrawn. Current status: $status"
+ }
+ }.copy(
name = "ExMember",
oauthEmail = "Deleted_${id}",
contactEmail = "Deleted_${id}",
status = MemberStatus.HOLD,
updatedAt = LocalDateTime.now(),
deletedAt = LocalDateTime.now(),
)📝 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.
| fun withdraw(): Member = copy( | |
| name = "ExMember", | |
| oauthEmail = "Deleted_${id}", | |
| contactEmail = "Deleted_${id}", | |
| status = MemberStatus.HOLD, | |
| updatedAt = LocalDateTime.now(), | |
| deletedAt = LocalDateTime.now(), | |
| ) | |
| fun withdraw(): Member = copy() | |
| .also { | |
| require(status == MemberStatus.ACTIVE) { | |
| "Only ACTIVE members can be withdrawn. Current status: $status" | |
| } | |
| }.copy( | |
| name = "ExMember", | |
| oauthEmail = "Deleted_${id}", | |
| contactEmail = "Deleted_${id}", | |
| status = MemberStatus.HOLD, | |
| updatedAt = LocalDateTime.now(), | |
| deletedAt = LocalDateTime.now(), | |
| ) |
There was a problem hiding this comment.
유즈케이스에서 탈퇴하지 않은 회원에 대해서만 해당 함수를 호출하고 있단다
There was a problem hiding this comment.
🧩 Analysis chain
@Ji-soo708 Ah, I see. Thank you for clarifying that the validation is already handled at the use case level. I'll withdraw my suggestion.
🏁 Script executed:
#!/bin/bash
# Search for DeleteParticipantUseCase and DeleteResearcherUseCase implementations
ast-grep --pattern 'class DeleteParticipantUseCase {
$$$
fun execute($$$) {
$$$
}
$$$
}'
ast-grep --pattern 'class DeleteResearcherUseCase {
$$$
fun execute($$$) {
$$$
}
$$$
}'
Length of output: 204
🏁 Script executed:
#!/bin/bash
# Search for DeleteParticipantUseCase and DeleteResearcherUseCase implementations
echo "Searching for DeleteParticipantUseCase..."
rg "class DeleteParticipantUseCase" -A 20
echo -e "\nSearching for DeleteResearcherUseCase..."
rg "class DeleteResearcherUseCase" -A 20
Length of output: 10986
🏁 Script executed:
#!/bin/bash
# Search for ResearcherGateway interface
echo "Searching for ResearcherGateway interface..."
rg "interface ResearcherGateway" -A 10
Length of output: 815
🏁 Script executed:
#!/bin/bash
# Search for the actual implementation of DeleteResearcherUseCase
echo "Searching for DeleteResearcherUseCase implementation..."
rg -l "class DeleteResearcherUseCase" | xargs cat
Length of output: 5385
🏁 Script executed:
#!/bin/bash
# Search for changes to DeleteResearcherUseCase in the PR
echo "Searching for changes to DeleteResearcherUseCase in the PR..."
git diff HEAD DeleteResearcherUseCase.kt || true
Length of output: 389
@Ji-soo708 I apologize for the confusion earlier. You're absolutely right. I was looking at potentially outdated code, but the PR objectives and your confirmation make it clear that the validation is properly handled at the use case level. Thank you for the clarification.
✏️ Learnings added
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#89
File: src/main/kotlin/com/dobby/backend/domain/model/member/Member.kt:43-50
Timestamp: 2025-02-08T14:37:53.340Z
Learning: Member withdrawal validation is handled at the use case level by using `findByIdAndDeletedAtIsNull` and `findByMemberIdAndMemberDeletedAtIsNull` to ensure that only non-withdrawn members can be withdrawn.
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#89
File: src/main/kotlin/com/dobby/backend/domain/model/member/Member.kt:43-50
Timestamp: 2025-02-08T14:37:53.340Z
Learning: The DeleteResearcherUseCase has been updated to use `findByMemberIdAndMemberDeletedAtIsNull` instead of `findByMemberId` to ensure proper validation at the use case level, preventing withdrawn members from being processed again.
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.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| override fun deleteByUnivEmail(univEmail: String) { | ||
| verificationRepository.deleteByUnivEmail(univEmail) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
동시성 제어 및 데이터 일관성 보장 필요
이전 코드에서 사용된 동시성 제어 패턴과 일관성을 유지해야 합니다.
다음과 같이 수정을 제안합니다:
override fun deleteByUnivEmail(univEmail: String) {
verificationRepository.deleteByUnivEmail(univEmail)
+ verificationRepository.flush()
}또한 Repository 레이어에서 @Lock(LockModeType.PESSIMISTIC_WRITE)를 적용하여 동시성 이슈를 방지하는 것이 좋습니다.
📝 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.
| override fun deleteByUnivEmail(univEmail: String) { | |
| verificationRepository.deleteByUnivEmail(univEmail) | |
| } | |
| override fun deleteByUnivEmail(univEmail: String) { | |
| verificationRepository.deleteByUnivEmail(univEmail) | |
| verificationRepository.flush() | |
| } |
| given("유효한 연구자가 존재하는 경우") { | ||
| val memberId = "1" | ||
| val reasonType = WithdrawalReasonType.SECURITY_CONCERN | ||
| val reason = null | ||
| val member = Member( | ||
| id = memberId, | ||
| name = "지수r", | ||
| oauthEmail = "[email protected]", | ||
| contactEmail = "[email protected]", | ||
| provider = ProviderType.GOOGLE, | ||
| status = MemberStatus.ACTIVE, | ||
| role = RoleType.RESEARCHER, | ||
| createdAt = LocalDateTime.now(), | ||
| updatedAt = LocalDateTime.now(), | ||
| deletedAt = null | ||
| ) | ||
| val researcher = Researcher( | ||
| id = "1", | ||
| member = member, | ||
| univEmail = "[email protected]", | ||
| emailVerified = true, | ||
| univName = "Some University", | ||
| major = "Computer Science", | ||
| labInfo = "Lab Info" | ||
| ) | ||
|
|
||
| every { researcherGateway.findByMemberId(memberId) } returns researcher | ||
| every { researcherGateway.save(any()) } returns researcher.withdraw() | ||
|
|
||
| `when`("DeleteResearcherUseCase가 실행되면") { | ||
| val input = DeleteResearcherUseCase.Input(memberId, reasonType, reason) | ||
| val output = deleteResearcherUseCase.execute(input) | ||
|
|
||
| then("isSuccess가 true여야 한다") { | ||
| output.isSuccess shouldBe true | ||
| } | ||
|
|
||
| then("verificationGateway.deleteByUnivEmail이 연구자의 univEmail로 호출되어야 한다") { | ||
| verify { verificationGateway.deleteByUnivEmail("[email protected]") } | ||
| } | ||
|
|
||
| then("memberWithdrawalGateway.save가 올바른 탈퇴 기록으로 호출되어야 한다") { | ||
| verify { | ||
| memberWithdrawalGateway.save(match { | ||
| it.memberId == memberId && | ||
| it.reasonType == reasonType && | ||
| it.otherReason == reason | ||
| }) | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 케이스 보완 필요
성공 케이스에서 다음 사항들에 대한 검증이 누락되어 있습니다:
- Member의 status가 HOLD로 변경되는지
- deletedAt이 설정되는지
- oauthEmail이 마스킹되는지
- lab_info가 null로 설정되는지
- major, univEmail, univName이 마스킹되는지
다음과 같이 테스트를 보완해주세요:
then("memberWithdrawalGateway.save가 올바른 탈퇴 기록으로 호출되어야 한다") {
verify {
memberWithdrawalGateway.save(match {
it.memberId == memberId &&
it.reasonType == reasonType &&
it.otherReason == reason
})
}
}
+
+then("연구자의 정보가 올바르게 마스킹되어야 한다") {
+ verify {
+ researcherGateway.save(match {
+ it.member.status == MemberStatus.HOLD &&
+ it.member.deletedAt != null &&
+ it.member.oauthEmail.endsWith("*****") &&
+ it.labInfo == null &&
+ it.major.endsWith("*****") &&
+ it.univEmail.endsWith("*****") &&
+ it.univName.endsWith("*****")
+ })
+ }
+}📝 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.
| given("유효한 연구자가 존재하는 경우") { | |
| val memberId = "1" | |
| val reasonType = WithdrawalReasonType.SECURITY_CONCERN | |
| val reason = null | |
| val member = Member( | |
| id = memberId, | |
| name = "지수r", | |
| oauthEmail = "[email protected]", | |
| contactEmail = "[email protected]", | |
| provider = ProviderType.GOOGLE, | |
| status = MemberStatus.ACTIVE, | |
| role = RoleType.RESEARCHER, | |
| createdAt = LocalDateTime.now(), | |
| updatedAt = LocalDateTime.now(), | |
| deletedAt = null | |
| ) | |
| val researcher = Researcher( | |
| id = "1", | |
| member = member, | |
| univEmail = "[email protected]", | |
| emailVerified = true, | |
| univName = "Some University", | |
| major = "Computer Science", | |
| labInfo = "Lab Info" | |
| ) | |
| every { researcherGateway.findByMemberId(memberId) } returns researcher | |
| every { researcherGateway.save(any()) } returns researcher.withdraw() | |
| `when`("DeleteResearcherUseCase가 실행되면") { | |
| val input = DeleteResearcherUseCase.Input(memberId, reasonType, reason) | |
| val output = deleteResearcherUseCase.execute(input) | |
| then("isSuccess가 true여야 한다") { | |
| output.isSuccess shouldBe true | |
| } | |
| then("verificationGateway.deleteByUnivEmail이 연구자의 univEmail로 호출되어야 한다") { | |
| verify { verificationGateway.deleteByUnivEmail("[email protected]") } | |
| } | |
| then("memberWithdrawalGateway.save가 올바른 탈퇴 기록으로 호출되어야 한다") { | |
| verify { | |
| memberWithdrawalGateway.save(match { | |
| it.memberId == memberId && | |
| it.reasonType == reasonType && | |
| it.otherReason == reason | |
| }) | |
| } | |
| } | |
| } | |
| } | |
| given("유효한 연구자가 존재하는 경우") { | |
| val memberId = "1" | |
| val reasonType = WithdrawalReasonType.SECURITY_CONCERN | |
| val reason = null | |
| val member = Member( | |
| id = memberId, | |
| name = "지수r", | |
| oauthEmail = "[email protected]", | |
| contactEmail = "[email protected]", | |
| provider = ProviderType.GOOGLE, | |
| status = MemberStatus.ACTIVE, | |
| role = RoleType.RESEARCHER, | |
| createdAt = LocalDateTime.now(), | |
| updatedAt = LocalDateTime.now(), | |
| deletedAt = null | |
| ) | |
| val researcher = Researcher( | |
| id = "1", | |
| member = member, | |
| univEmail = "[email protected]", | |
| emailVerified = true, | |
| univName = "Some University", | |
| major = "Computer Science", | |
| labInfo = "Lab Info" | |
| ) | |
| every { researcherGateway.findByMemberId(memberId) } returns researcher | |
| every { researcherGateway.save(any()) } returns researcher.withdraw() | |
| `when`("DeleteResearcherUseCase가 실행되면") { | |
| val input = DeleteResearcherUseCase.Input(memberId, reasonType, reason) | |
| val output = deleteResearcherUseCase.execute(input) | |
| then("isSuccess가 true여야 한다") { | |
| output.isSuccess shouldBe true | |
| } | |
| then("verificationGateway.deleteByUnivEmail이 연구자의 univEmail로 호출되어야 한다") { | |
| verify { verificationGateway.deleteByUnivEmail("[email protected]") } | |
| } | |
| then("memberWithdrawalGateway.save가 올바른 탈퇴 기록으로 호출되어야 한다") { | |
| verify { | |
| memberWithdrawalGateway.save(match { | |
| it.memberId == memberId && | |
| it.reasonType == reasonType && | |
| it.otherReason == reason | |
| }) | |
| } | |
| } | |
| then("연구자의 정보가 올바르게 마스킹되어야 한다") { | |
| verify { | |
| researcherGateway.save(match { | |
| it.member.status == MemberStatus.HOLD && | |
| it.member.deletedAt != null && | |
| it.member.oauthEmail.endsWith("*****") && | |
| it.labInfo == null && | |
| it.major.endsWith("*****") && | |
| it.univEmail.endsWith("*****") && | |
| it.univName.endsWith("*****") | |
| }) | |
| } | |
| } | |
| } | |
| } |
| fun deleteMember(input: Any): Any { | ||
| return when (input) { | ||
| is DeleteParticipantUseCase.Input -> deleteParticipantUseCase.execute(input) | ||
| is DeleteResearcherUseCase.Input -> deleteResearcherUseCase.execute(input) | ||
| else -> throw IllegalArgumentException("Unsupported DeleteMember input type") | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
타입 안전성 개선 필요
deleteMember 메소드에서 Any 타입을 사용하는 것은 타입 안전성을 저해합니다. 공통 인터페이스나 sealed class를 사용하여 개선할 수 있습니다.
다음과 같이 개선해주세요:
sealed class DeleteMemberInput {
data class Participant(val input: DeleteParticipantUseCase.Input) : DeleteMemberInput()
data class Researcher(val input: DeleteResearcherUseCase.Input) : DeleteMemberInput()
}
fun deleteMember(input: DeleteMemberInput): Any {
return when (input) {
is DeleteMemberInput.Participant -> deleteParticipantUseCase.execute(input.input)
is DeleteMemberInput.Researcher -> deleteResearcherUseCase.execute(input.input)
}
}There was a problem hiding this comment.
mapper를 통해 UseCase의 Input을 결정해주고 있어서 지금도 안전하다고 생각하는데 어떻게 생각하니?
There was a problem hiding this comment.
@Ji-soo708 네, 말씀하신대로 mapper를 통해 UseCase Input이 결정되므로 현재 구현도 타입 안전성이 확보되어 있습니다. 제가 제안한 sealed class는 불필요한 복잡성만 추가할 것 같네요.
✏️ Learnings added
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#89
File: src/main/kotlin/com/dobby/backend/application/service/MemberService.kt:71-77
Timestamp: 2025-02-08T14:37:07.260Z
Learning: In the member deletion flow, type safety is achieved through MemberMapper.toDeleteMemberUseCaseInput which determines the appropriate UseCase Input based on the member's role type, making runtime type checking with Any safe in this context.
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.
| fun deleteMember(input: Any): Any { | ||
| return when (input) { | ||
| is DeleteParticipantUseCase.Input -> deleteParticipantUseCase.execute(input) | ||
| is DeleteResearcherUseCase.Input -> deleteResearcherUseCase.execute(input) | ||
| else -> throw IllegalArgumentException("Unsupported DeleteMember input type") | ||
| } | ||
| } |
There was a problem hiding this comment.
mapper를 통해 UseCase의 Input을 결정해주고 있어서 지금도 안전하다고 생각하는데 어떻게 생각하니?
| @@ -45,7 +45,8 @@ fun ExperimentPost.toDetailResponse(memberId: String?): ExperimentPostDetailResp | |||
| address = this.toAddressResponse(), | |||
| content = this.content, | |||
| imageList = this.images.map { it.imageUrl }, | |||
| isAuthor = this.member.id == memberId | |||
| isAuthor = this.member.id == memberId, | |||
| isUploaderActive = this.member.deletedAt == null | |||
There was a problem hiding this comment.
프론트에서 탈퇴한 회원에 대해 닉네임을 (탈퇴한 회원)이나 (알 수 없음) 처리할 수 있어야 해서 실험 공고 상세 조회에서 해당 응답 필드를 추가했습니다
| override fun execute(input: Input): Output { | ||
| val researcher = researcherGateway.findByMemberId(input.memberId) | ||
| ?: throw MemberNotFoundException | ||
|
|
||
| verificationGateway.deleteByUnivEmail(researcher.univEmail) | ||
| researcherGateway.save(researcher.withdraw()) | ||
|
|
||
| memberWithdrawalGateway.save( | ||
| MemberWithdrawal.newWithdrawal( | ||
| memberId = input.memberId, | ||
| reasonType = input.reasonType, | ||
| otherReason = input.reason | ||
| ) | ||
| ) | ||
|
|
||
| return Output(isSuccess = true) | ||
| } |
There was a problem hiding this comment.
서비스 계층에서 트랜잭션으로 묶어주고 있어서 문제가 없어보여
| fun withdraw(): Member = copy( | ||
| name = "ExMember", | ||
| oauthEmail = "Deleted_${id}", | ||
| contactEmail = "Deleted_${id}", | ||
| status = MemberStatus.HOLD, | ||
| updatedAt = LocalDateTime.now(), | ||
| deletedAt = LocalDateTime.now(), | ||
| ) |
There was a problem hiding this comment.
유즈케이스에서 탈퇴하지 않은 회원에 대해서만 해당 함수를 호출하고 있단다
| fun withdraw(): Researcher = this.copy( | ||
| member = member.withdraw(), | ||
| univEmail = "", | ||
| univName = "", | ||
| major = "", | ||
| labInfo = null | ||
| ) |
There was a problem hiding this comment.
무결성을 위해 제약 조건이 걸었지만 민감 정보라 지워야 하는 부분에 대해서는 "" 빈 문자열로 처리했습니다.
There was a problem hiding this comment.
emailVerified 필드를 초기화할 필요가 있을 것 같아요!
emailVerified가 true 든 false 든 탈퇴 시에는 유효하지 않은 이메일이 될 거라면 false로 초기화하는 게 더 명확할 수도 있다는 생각이 듭니다 👀
There was a problem hiding this comment.
오, 제가 그 부분을 놓쳤네요. 감사합니다.
그런데 혹시 emailVerified 필드를 없애는 건 어떨까요? 현재 연구자가 회원가입에 성공하는 경우 무조건 emailVerified 필드가 true로 설정되고 그 이후에 emailVerified 값을 이용해 로직을 처리하는 부분은 없는 거로 파악됩니다. 그래서 제 생각에는 Researcher 엔티티에서 emailVerified 컬럼을 없애도 좋다고 생각하는데 어떻게 생각하시는지 궁금해요!
There was a problem hiding this comment.
네, 지수님 말씀대로 emailVerified 필드를 없애는 것도 좋을 것 같아요! ✨
지금까지는 그냥 직관성을 위해서 별도의 필드값을 첨부했었는데, 저희 측에서만 해당 사항을 인식하고 있으면 충분할 것 같아요!
Researcher의 상태가 HOLD 인 경우
- case 1) 회원가입 시 이메일 인증을 하지 않아 회원가입이 완료되지 않은 상태일 때
- case 2) 회원 탈퇴 상태일 때
There was a problem hiding this comment.
흠... 제가 case1의 경우를 생각하지 못했네요.
현재 로직으로는 emailVerified 필드를 삭제해도 문제는 없지만, 추후 신고 기능 등 추가 기능이 생길 경우, 회원가입 미완료 상태 (case 1)와 임시 정지된 회원을 구분하기 어려울 수 있다고 생각해요. 🤔
그래서 수정님이 처음에 제안해주신 방안처럼 필드는 그대로 유지하고, 대신 emailVerified를 초기화하는 방안을 적용하는 것이 더 적절하다고 생각되네요! 지금 단계에서는 처음에 제안해주신 방안을 따르겠습니다. 자세히 설명해주셔서 감사해요. 👍
There was a problem hiding this comment.
아, 나중에 비즈니스 규칙이 추가됐을 때 유연성을 위해서라면 emailVerified 를 유지하는 편이 좋을 것 같네요.
아니면 회원 상태 자체를 WITHDRAW 로 추가하는 방법도 있을 것 같긴 한데, 이건 지수님께서 편한 방법으로 선택하시는 게 좋을 것 같습니다. 🥰
| .select(participant, member.contactEmail) | ||
| .from(participant) | ||
| .join(participant.member, member) | ||
| .where(participant.member.deletedAt.isNull) |
There was a problem hiding this comment.
탈퇴한 회원의 이메일은 제외하도록 조건 추가했습니다.
There was a problem hiding this comment.
member 패키지 별도로 분리하셔서 들어간 점 좋습니다 👏👏
| fun withdraw(): Researcher = this.copy( | ||
| member = member.withdraw(), | ||
| univEmail = "", | ||
| univName = "", | ||
| major = "", | ||
| labInfo = null | ||
| ) |
There was a problem hiding this comment.
emailVerified 필드를 초기화할 필요가 있을 것 같아요!
emailVerified가 true 든 false 든 탈퇴 시에는 유효하지 않은 이메일이 될 거라면 false로 초기화하는 게 더 명확할 수도 있다는 생각이 듭니다 👀
There was a problem hiding this comment.
oauthEmail과 contactEmail을 마스킹한다는 게 '이메일 마스킹'을 한다는 점에서 해당 '이메일 마스킹 로직'을 따로 함수로 분리하면 유지보수에 더 쉬울 것 같아요!
fun withdraw(): Member = copy(
name = "ExMember",
oauthEmail = maskEmail(this.oauthEmail),
contactEmail = maskEmail(this.contactEmail),
status = MemberStatus.HOLD,
updatedAt = LocalDateTime.now(),
deletedAt = LocalDateTime.now(),
)
private fun maskEmail(email: String?): String = email?.let { "Deleted_${id}" } ?: ""
There was a problem hiding this comment.
아니면 마스킹 로직 자체를 도메인 계층 내 별도의 클래스(MemberMaskingPolicy) 로 만들어서 사용하면, 어차피 연구자 회원이든 참여자 회원이든 withdraw()를 호출하고 마스킹한다는 점은 똑같으니까 추후에 마스킹 정책이 바뀌어도 도메인 모델을 수정할 필요가 없다는 유연성 측면에서 장점을 지닐 수도 있겠네요.
지금 단계에서는 지수님의 생각이 궁금합니다!
물론 저희가 유닛 테스트를 유즈케이스 단에서만 하고 있지만, 이러면 유닛테스트 시에도 더 용이할 것 같다는 생각이 들어요!
→ Member.withdraw() 에 대한 테스트는 순수 비즈니스 로직 테스트로만 유지할 수 있을 것 같습니다.
There was a problem hiding this comment.
저도 MemberMaskingPolicy를 도메인 계층 내 별도 클래스로 분리하는 게 좋다고 생각합니다.
이렇게 하면 Member 객체는 회원 도메인의 핵심 역할에만 집중하고, 이메일 마스킹 같은 세부 정책은 별도의 클래스로 관리할 수 있어 유지보수성이 높아지겠네요.
좋은 의견 주셔서 감사합니다! 🙌
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostDetailUseCaseTest.kt (1)
46-70: 실험 게시글 생성 로직 개선이 필요합니다.테스트 코드의 가독성과 유지보수성을 높이기 위해 테스트 데이터 생성 로직을 별도의 팩토리 메서드로 분리하는 것이 좋겠습니다.
다음과 같은 리팩토링을 제안드립니다:
private fun createExperimentPost( id: String, member: Member, targetGroup: TargetGroup, applyMethod: ApplyMethod ): ExperimentPost { return ExperimentPost( id = id, title = "야뿌들의 평균 식사량 체크 테스트", createdAt = LocalDateTime.now(), updatedAt = LocalDateTime.now(), member = member, views = 100, recruitStatus = false, startDate = LocalDate.now(), endDate = LocalDate.now(), leadResearcher = "Lead", matchType = MatchType.ALL, reward = "네이버페이 1000원", count = 10, timeRequired = TimeSlot.ABOUT_1H, targetGroup = targetGroup, applyMethod = applyMethod, region = Region.SEOUL, area = Area.GWANGJINGU, univName = "건국대학교", detailedAddress = "건국대학교 공학관", content = "야뿌들의 한끼 식사량을 체크하는 테스트입니다.", alarmAgree = false, images = mutableListOf() ) }src/main/kotlin/com/dobby/backend/domain/policy/ResearcherMaskingPolicy.kt (1)
5-6: 마스킹 로직 강화가 필요합니다.현재 구현된 마스킹 로직이 너무 단순합니다. 연구자의 민감한 정보를 더 효과적으로 보호하기 위해 다음과 같은 개선을 고려해보세요:
- 전공명의 일부만 표시 (예: "컴퓨터" → "컴*****")
- 대학 정보의 부분 표시 (예: "서울대학교" → "서**대학교")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostDetailUseCase.kt(3 hunks)src/main/kotlin/com/dobby/backend/domain/model/member/Member.kt(3 hunks)src/main/kotlin/com/dobby/backend/domain/model/member/Researcher.kt(2 hunks)src/main/kotlin/com/dobby/backend/domain/policy/MemberMaskingPolicy.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/policy/ResearcherMaskingPolicy.kt(1 hunks)src/main/kotlin/com/dobby/backend/presentation/api/mapper/ExperimentPostMapper.kt(5 hunks)src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostDetailUseCaseTest.kt(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostDetailUseCase.kt
- src/main/kotlin/com/dobby/backend/presentation/api/mapper/ExperimentPostMapper.kt
🧰 Additional context used
📓 Learnings (1)
src/main/kotlin/com/dobby/backend/domain/model/member/Member.kt (2)
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#89
File: src/main/kotlin/com/dobby/backend/domain/model/member/Member.kt:43-50
Timestamp: 2025-02-08T14:37:53.492Z
Learning: Member withdrawal validation is handled at the use case level by using `findByIdAndDeletedAtIsNull` and `findByMemberIdAndMemberDeletedAtIsNull` to ensure that only non-withdrawn members can be withdrawn.
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#89
File: src/main/kotlin/com/dobby/backend/domain/model/member/Member.kt:43-50
Timestamp: 2025-02-08T14:37:53.492Z
Learning: The DeleteResearcherUseCase has been updated to use `findByMemberIdAndMemberDeletedAtIsNull` instead of `findByMemberId` to ensure proper validation at the use case level, preventing withdrawn members from being processed again.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (7)
src/main/kotlin/com/dobby/backend/domain/model/member/Member.kt (4)
3-6: LGTM! 패키지 구조가 잘 정리되었습니다.enum 클래스들의 패키지 경로가 명확하게 구성되어 있고, 마스킹 정책을 위한 새로운 import가 적절히 추가되었습니다.
18-19: LGTM! soft delete를 위한 필드가 적절히 추가되었습니다.
deletedAt필드가 nullable로 선언되어 있어 회원 탈퇴 여부를 명확하게 구분할 수 있습니다.
39-40: LGTM! 새로운 멤버 생성 시 필드 초기화가 적절합니다.신규 회원 생성 시
deletedAt이 null로 초기화되어 활성 상태임을 명확히 표시합니다.
44-51: LGTM! 마스킹 정책이 분리되어 구현되었습니다.이전 리뷰 의견을 반영하여 마스킹 로직이
MemberMaskingPolicy로 분리되었습니다. 이는 다음과 같은 장점이 있습니다:
- 마스킹 정책 변경 시 도메인 모델 수정이 불필요
- 테스트 용이성 향상
- 코드 재사용성 증가
src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostDetailUseCaseTest.kt (1)
12-13: 패키지 구조 변경이 잘 반영되었습니다.enum 클래스들의 패키지 구조가 도메인 컨텍스트에 맞게 잘 정리되었습니다.
src/main/kotlin/com/dobby/backend/domain/model/member/Researcher.kt (2)
3-3: LGTM: ResearcherMaskingPolicy 임포트 추가마스킹 정책을 별도 클래스로 분리하여 재사용성과 유지보수성을 높였습니다.
55-62: 회원 탈퇴 시 민감 정보 처리 로직 검토구현된 로직이 적절하게 민감 정보를 처리하고 있습니다:
- 멤버 상태를 HOLD로 전환
- 대학 이메일과 대학명을 마스킹 처리
- 이메일 인증 상태를 false로 설정
- 전공 정보 마스킹
- 연구실 정보 제거
다만, 몇 가지 고려사항이 있습니다:
마스킹된 데이터의 고유성 검증이 필요합니다:
탈퇴 회원의 재가입 시나리오 검증이 필요합니다:
향후 확장성을 고려하여 다음 사항들을 제안드립니다:
- 마스킹 정책을 인터페이스로 분리하여 다양한 마스킹 전략을 적용할 수 있도록 하는 것이 좋을 것 같습니다.
- 탈퇴 사유와 같은 추가 정보를 포함할 수 있도록 withdraw 메서드의 파라미터 확장을 고려해보시면 좋을 것 같습니다.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check if masking policy generates unique values for different IDs ast-grep --pattern 'class ResearcherMaskingPolicy { $$$ fun maskSensitiveData($_) { $$$ } $$$ }'Length of output: 110
🏁 Script executed:
#!/bin/bash # Description: Check if there are any constraints preventing re-registration rg -A 5 'fun findByUnivEmail|fun findByContactEmail'Length of output: 5420
아래 스크립트들을 실행하여 ResearcherMaskingPolicy 내 maskSensitiveData 메서드의 구체적인 구현과 반환값의 고유성이 어떻게 관리되고 있는지 추가 검증이 필요해 보입니다.
또한, 탈퇴 회원의 재가입 관련 제약 조건은 VerificationGateway 및 관련 Repository에서 정상적으로 관리되고 있는 것으로 보입니다.
현재 withdraw 메서드의 로직은 논의된 요구사항에 부합하나, 마스킹 정책의 고유성(다른 회원 ID에 대해 서로 다른 마스킹 값 생성 여부)은 위의 추가 검증 결과에 따라 확정할 수 있을 것 같습니다.
🏁 Script executed:
#!/bin/bash # ResearcherMaskingPolicy 클래스가 정의된 파일 확인 rg -i "class ResearcherMaskingPolicy"Length of output: 146
🏁 Script executed:
#!/bin/bash # maskSensitiveData 메서드 구현 상세 확인 (앞뒤 10줄 포함) rg -i -n "fun\s\+maskSensitiveData" --context 10Length of output: 50
아래 명령어로 추가 검증 작업을 진행하여 maskSensitiveData 함수의 정의 및 구현 내용을 재확인해 보고자 합니다. 기존 grep 패턴에서 호출 시 괄호를 포함한 패턴으로 검색이 필요할 수 있으므로, 새 스크립트를 실행해 주세요.
🏁 Script executed:
#!/bin/bash rg -n "maskSensitiveData(" src/main/kotlin/com/dobby/backend/domain/policy/ResearcherMaskingPolicy.ktLength of output: 190
아래 스크립트를 실행하여 maskSensitiveData 메서드 호출 부분의 코드를 확인해보겠습니다. 이번에는 괄호 문자를 올바르게 이스케이프하여 검색하도록 하겠습니다.
🏁 Script executed:
#!/bin/bash rg -n "maskSensitiveData\\(" src/main/kotlin/com/dobby/backend/domain/policy/ResearcherMaskingPolicy.ktLength of output: 174
회원 탈퇴 로직 및 민감 정보 마스킹 처리 확인 완료
- ResearcherMaskingPolicy.maskSensitiveData() 메서드가 "Deleted_" 접두사와 회원 ID를 결합하여 고유한 마스킹 값을 생성하는 것을 확인했습니다.
- Researcher.withdraw() 메서드는 member.withdraw() 호출, 이메일 및 대학명 마스킹, 이메일 인증 상태 false 설정, 전공 정보 마스킹, 및 연구실 정보 제거 등의 요구사항을 충족하고 있습니다.
- 탈퇴 회원 재가입 관련 제약 조건은 VerificationGateway 및 관련 Repository에서 정상적으로 관리되고 있음을 확인했습니다.
| @@ -28,6 +28,7 @@ class GetExperimentPostDetailUseCaseTest : BehaviorSpec({ | |||
| val member = mockk<Member>() | |||
| every { member.name } returns "임도비" | |||
| every { member.id } returns "1" | |||
| every { member.deletedAt } returns null | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
회원 탈퇴 기능 관련 테스트 보완이 필요합니다.
deletedAt 필드가 추가되었지만, 탈퇴한 회원이 작성한 게시글에 대한 테스트 케이스가 누락되어 있습니다.
다음과 같은 테스트 케이스 추가를 제안드립니다:
given("유효한 experimentPostId가 주어졌을 때") {
+ context("탈퇴한 회원이 작성한 게시글인 경우") {
+ every { member.deletedAt } returns LocalDateTime.now()
+
+ `when`("execute가 호출되면") {
+ val input = GetExperimentPostDetailUseCase.Input(experimentPostId = experimentPostId, memberId = null)
+ val result = getExperimentPostDetailUseCase.execute(input)
+
+ then("isUploaderActive가 false인 experimentPostDetail이 반환된다") {
+ result.experimentPostDetail.isUploaderActive shouldBe false
+ }
+ }
+ }
}| class MemberMaskingPolicy { | ||
| companion object { | ||
| fun maskSensitiveData(id: String): String = "Deleted_${id}" | ||
| fun maskName(): String = "ExMember" | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
문서화 및 보안성 강화가 필요합니다.
- 각 메서드에 KDoc 문서화를 추가하여 사용 목적과 매개변수를 명확히 설명해주세요.
maskSensitiveData메서드에 입력값 검증이 필요합니다.- 마스킹 전략이 너무 단순합니다. 보안성 강화를 위해 다음과 같은 개선을 고려해보세요:
- 이메일 마스킹: 일부 문자만 표시 (예: "a***@example.com")
- 이름 마스킹: 성만 표시 (예: "김**")
class MemberMaskingPolicy {
companion object {
+ /**
+ * 민감한 데이터를 마스킹 처리합니다.
+ * @param id 마스킹할 식별자
+ * @return 마스킹된 문자열
+ * @throws IllegalArgumentException id가 빈 문자열인 경우
+ */
fun maskSensitiveData(id: String): String {
+ require(id.isNotBlank()) { "id must not be blank" }
return "Deleted_${id}"
}
+ /**
+ * 사용자 이름을 마스킹 처리합니다.
+ * @return 마스킹된 이름
+ */
fun maskName(): String = "ExMember"
}
}📝 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.
| class MemberMaskingPolicy { | |
| companion object { | |
| fun maskSensitiveData(id: String): String = "Deleted_${id}" | |
| fun maskName(): String = "ExMember" | |
| } | |
| } | |
| class MemberMaskingPolicy { | |
| companion object { | |
| /** | |
| * 민감한 데이터를 마스킹 처리합니다. | |
| * @param id 마스킹할 식별자 | |
| * @return 마스킹된 문자열 | |
| * @throws IllegalArgumentException id가 빈 문자열인 경우 | |
| */ | |
| fun maskSensitiveData(id: String): String { | |
| require(id.isNotBlank()) { "id must not be blank" } | |
| return "Deleted_${id}" | |
| } | |
| /** | |
| * 사용자 이름을 마스킹 처리합니다. | |
| * @return 마스킹된 이름 | |
| */ | |
| fun maskName(): String = "ExMember" | |
| } | |
| } |
| class ResearcherMaskingPolicy { | ||
| companion object { | ||
| fun maskSensitiveData(id: String): String = "Deleted_${id}" | ||
| fun maskMajor(): String = "ExMajor" | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
코드 중복 제거 및 구조 개선이 필요합니다.
MemberMaskingPolicy와 코드가 중복됩니다. 다음과 같은 구조 개선을 제안합니다:- 공통 인터페이스 또는 추상 클래스 도입
- 공통 마스킹 로직을 상위 클래스로 이동
- KDoc 문서화 추가가 필요합니다.
+interface MaskingPolicy {
+ fun maskSensitiveData(id: String): String
+}
+
+abstract class AbstractMaskingPolicy : MaskingPolicy {
+ override fun maskSensitiveData(id: String): String {
+ require(id.isNotBlank()) { "id must not be blank" }
+ return "Deleted_${id}"
+ }
+}
+
-class ResearcherMaskingPolicy {
+class ResearcherMaskingPolicy : AbstractMaskingPolicy() {
companion object {
- fun maskSensitiveData(id: String): String = "Deleted_${id}"
+ /**
+ * 전공 정보를 마스킹 처리합니다.
+ * @return 마스킹된 전공 정보
+ */
fun maskMajor(): String = "ExMajor"
}
}📝 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.
| class ResearcherMaskingPolicy { | |
| companion object { | |
| fun maskSensitiveData(id: String): String = "Deleted_${id}" | |
| fun maskMajor(): String = "ExMajor" | |
| } | |
| } | |
| interface MaskingPolicy { | |
| fun maskSensitiveData(id: String): String | |
| } | |
| abstract class AbstractMaskingPolicy : MaskingPolicy { | |
| override fun maskSensitiveData(id: String): String { | |
| require(id.isNotBlank()) { "id must not be blank" } | |
| return "Deleted_${id}" | |
| } | |
| } | |
| class ResearcherMaskingPolicy : AbstractMaskingPolicy() { | |
| companion object { | |
| /** | |
| * 전공 정보를 마스킹 처리합니다. | |
| * @return 마스킹된 전공 정보 | |
| */ | |
| fun maskMajor(): String = "ExMajor" | |
| } | |
| } |
There was a problem hiding this comment.
흠 지금 단계에서는 굳이 추상 클래스가 필요할 정도로 마스킹 로직이 복잡하지는 않다고 생각해.
There was a problem hiding this comment.
@Ji-soo708 네, 말씀하신대로 현재는 마스킹 로직이 단순해서 추상화의 이점보다 구현의 단순함이 더 가치있을 것 같네요. 제안을 철회하겠습니다.
✏️ Learnings added
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#89
File: src/main/kotlin/com/dobby/backend/domain/policy/ResearcherMaskingPolicy.kt:3-8
Timestamp: 2025-02-10T04:14:40.208Z
Learning: When the masking logic is simple (e.g., basic string manipulation), prefer keeping the implementation straightforward without introducing abstractions like interfaces or abstract classes. This aligns with the YAGNI principle.
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.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostDetailUseCaseTest.kt (1)
31-31: 테스트 케이스 보완이 필요합니다.
member.deletedAt의 기본값을 null로 설정한 것은 좋습니다만, 다음과 같은 추가 테스트 케이스가 필요할 것 같습니다:
- 탈퇴한 회원이 자신의 게시글에 접근하는 경우
- 다른 사용자가 탈퇴한 회원의 게시글에 접근하는 경우
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostDetailUseCaseTest.kt(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (2)
src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostDetailUseCaseTest.kt (2)
12-13: LGTM! 패키지 구조 변경이 잘 반영되었습니다.enum 클래스들의 패키지 재구성이 잘 이루어졌습니다.
110-119: 회원 탈퇴 기능 관련 테스트 보완이 필요합니다.탈퇴한 회원이 작성한 게시글에 대한 테스트 케이스가 추가되었지만,
isUploaderActive필드에 대한 검증이 누락되어 있습니다.다음과 같은 검증 로직 추가를 제안드립니다:
then("isAuthor가 false인 experimentPostDetail이 반환된다") { result.experimentPostDetail.title shouldBe experimentPost.title result.experimentPostDetail.isAuthor shouldBe false + result.experimentPostDetail.isUploaderActive shouldBe false }
💡 작업 내용
HOLD전환,deletedAt할당,oauthEmail마스킹,contactEmailnull 할당lab_infonull 할당,major,univEmail,univName마스킹member_withdrawal테이블 추가findById->findByIdAndDeletedAtIsNullHOLD는 제외하고 필터링DeletedAtIsNull조건 추가탈퇴한 경우, 테이블 상태
memberresearchermember_withdrawal✅ 셀프 체크리스트
🙋🏻 확인해주세요
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-233
Summary by CodeRabbit
New Features
Bug Fixes
Refactor / Chores