Conversation
…mConnection 관련 처리 로직 추가
|
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:
Walkthrough조직 소유자 변경 API와 관련 계약·엔티티·서비스·문서를 추가하고, 조직 삭제 시 연관 데이터(AIInsightReport/Timeline/OrgInvitation 등) 정리용 리포지토리 메서드를 도입하며, 회원 탈퇴(soft→soft+scheduled hard) 흐름과 스케줄러/하드삭제 실행기를 추가합니다. Changes조직 소유자 양도 및 회원 탈퇴 기능 추가
Sequence Diagram(s)sequenceDiagram
participant Client
participant OrgController
participant OrgService
participant OrgMemberRepository
participant Organization
Client->>OrgController: PATCH /api/org/{orgId}/changeOwner
OrgController->>OrgService: changeOwner(userId, orgId, request)
OrgService->>OrgMemberRepository: find org member for newOwnerUserId
OrgMemberRepository-->>OrgService: OrgMember
OrgService->>Organization: changeOwner(newOwnerUserId)
Organization-->>OrgService: done
OrgService-->>OrgController: void
OrgController-->>Client: 200 OK
sequenceDiagram
participant Client
participant UserController
participant UserService
participant PlatformConnectionRepository
participant OrgService
participant OrgMemberRepository
participant AuthProviderAccountRepository
participant UserRepository
participant S3UploadService
Client->>UserController: DELETE /my
UserController->>UserService: deleteUser(userId)
UserService->>PlatformConnectionRepository: findByUser_Id(userId)
alt platform connections exist
UserService-->>UserController: USER_HAS_PLATFORM_CONNECTION (error)
else
UserService->>OrgService: removeOrganizationSoft(...) when owner and only member
UserService->>OrgMemberRepository: deleteByUserId(userId)
UserService->>AuthProviderAccountRepository: deleteByUserId(userId)
UserService->>UserRepository: softDeleteUser()
UserService->>S3UploadService: deleteImageFromUrl(profileImageUrl) (warn on failure)
UserService-->>UserController: 200 OK
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.java (1)
27-28: ⚡ Quick win탈퇴 검증 용도라면 목록 조회 대신 존재 여부 조회로 바꾸는 게 좋습니다.
Line 27의 메서드는 존재 여부 확인만 필요한 경로에서 엔티티 리스트를 모두 로딩하게 되어 불필요한 DB/메모리 비용이 생깁니다.
existsByUser_Id로 바꾸면 검증 쿼리가 훨씬 가벼워집니다.♻️ 제안 diff
--- a/src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.java +++ b/src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.java @@ - List<PlatformConnection> findByUser_Id(Long userId); + boolean existsByUser_Id(Long userId);--- a/src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/UserService.java +++ b/src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/UserService.java @@ - List<PlatformConnection> platformConnections = platformConnectionRepository.findByUser_Id(userId); - if (!platformConnections.isEmpty()) { + if (platformConnectionRepository.existsByUser_Id(userId)) { throw new UserHandler(UserErrorCode.USER_HAS_PLATFORM_CONNECTION); }As per coding guidelines "JPA 사용 시 N+1 문제나 불필요한 쿼리가 발생하지 않는지 ... 체크하라."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.java` around lines 27 - 28, The repository method findByUser_Id in PlatformConnectionRepository loads full entity lists when only existence is needed; replace it with a boolean existsByUser_Id(Long userId) to avoid fetching entities and reduce DB/memory cost—update the repository interface to declare existsByUser_Id and change any callers that expect List<PlatformConnection> (e.g., user deletion validation paths) to use the boolean result accordingly.src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java (1)
127-137: ⚡ Quick winSwagger 설명 문자열을 Java 17 Text Block으로 정리해 주세요.
현재 프로젝트는 Java 17로 설정되어 있어 Text Block을 충분히 활용할 수 있습니다. 긴 문자열을
+로 연결하면 수정할 때\n누락이나 오타가 쉽게 발생하는데, Text Block을 사용하면 실제 모양 그대로 작성할 수 있어 훨씬 안정적입니다.♻️ 제안 diff
- description = "AccessToken 을 헤더로 받아 현재 로그인한 회원을 탈퇴 처리합니다.\n\n" + - "### 1. 사전 검증 (통과해야 탈퇴 가능)\n" + - "- **광고 플랫폼 연동(PlatformConnection) 존재 여부**: 연동된 광고 플랫폼이 하나라도 남아 있으면 탈퇴가 차단됩니다. 먼저 모든 광고 플랫폼 연동을 해제한 뒤 다시 시도해야 합니다. -> 추후 삭제 API 추가 예정\n\n" + - "- **본인이 소유자인 조직에 다른 멤버 존재**: 본인이 생성자(owner)인 조직에 본인 외 다른 멤버가 남아 있으면 탈퇴가 차단됩니다. `PATCH /api/org/{orgId}/changeOwner` API 로 소유권을 위임한 뒤 재시도해야 합니다.\n\n" + - "### 2. 회원이 속한 워크스페이스(Organization) 처리\n" + - "- **단순 ADMIN / MEMBER 로만 속해있는 조직** → 해당 가입 정보(OrgMember)만 제거됩니다. 조직 자체는 유지됩니다.\n\n" + - "- **회원이 생성자(owner)이고 본인만 속한 조직** → 조직이 Soft Delete 처리되며, 해당 조직의 부수 데이터(보낸 초대장 / 활동 타임라인 / AI 인사이트 리포트)도 함께 정리됩니다. 조직 자체는 복구 가능한 상태(status = DELETED) 로 남습니다.\n\n" + - "### 3. 회원 본인 데이터 정리\n" + - "- 소유자가 아닌채로 속한 조직에서 탈퇴하는 회원 정보가 모두 제거됩니다.\n\n" + - "- 소셜 로그인 사용자의 경우 연결된 소셜계정이 함께 제거됩니다.\n\n" + - "- 회원 본인이 삭제된 후, 프로필 이미지가 S3 에서 삭제됩니다. S3 이미지 삭제 실패는 서버 로그로만 기록됩니다." + description = """ + AccessToken 을 헤더로 받아 현재 로그인한 회원을 탈퇴 처리합니다. + + ### 1. 사전 검증 (통과해야 탈퇴 가능) + - **광고 플랫폼 연동(PlatformConnection) 존재 여부**: 연동된 광고 플랫폼이 하나라도 남아 있으면 탈퇴가 차단됩니다. 먼저 모든 광고 플랫폼 연동을 해제한 뒤 다시 시도해야 합니다. -> 추후 삭제 API 추가 예정 + + - **본인이 소유자인 조직에 다른 멤버 존재**: 본인이 생성자(owner)인 조직에 본인 외 다른 멤버가 남아 있으면 탈퇴가 차단됩니다. `PATCH /api/org/{orgId}/changeOwner` API 로 소유권을 위임한 뒤 재시도해야 합니다. + + ### 2. 회원이 속한 워크스페이스(Organization) 처리 + - **단순 ADMIN / MEMBER 로만 속해있는 조직** → 해당 가입 정보(OrgMember)만 제거됩니다. 조직 자체는 유지됩니다. + + - **회원이 생성자(owner)이고 본인만 속한 조직** → 조직이 Soft Delete 처리되며, 해당 조직의 부수 데이터(보낸 초대장 / 활동 타임라인 / AI 인사이트 리포트)도 함께 정리됩니다. 조직 자체는 복구 가능한 상태(status = DELETED) 로 남습니다. + + ### 3. 회원 본인 데이터 정리 + - 소유자가 아닌채로 속한 조직에서 탈퇴하는 회원 정보가 모두 제거됩니다. + + - 소셜 로그인 사용자의 경우 연결된 소셜계정이 함께 제거됩니다. + + - 회원 본인이 삭제된 후, 프로필 이미지가 S3 에서 삭제됩니다. S3 이미지 삭제 실패는 서버 로그로만 기록됩니다. + """🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java` around lines 127 - 137, Replace the long concatenated description string in UserControllerDocs (the assignment to description) with a Java 17 text block ("""...""") so the multi-line Swagger description is written verbatim; locate the line containing description = "..." and swap the concatenated pieces for a single text block, preserving the existing newlines and content and ensuring indentation is trimmed appropriately for Java text blocks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/application/dto/request/OrgRequest.java`:
- Around line 39-43: The ChangeOwner.record's newOwnerUserId currently only has
`@NotNull`; add a positive-number constraint (e.g., `@Positive` or `@Min`(1)) to
reject 0/negative IDs at validation time so invalid IDs don't reach the service
layer; update the annotation on newOwnerUserId in the ChangeOwner record to
include the chosen positive constraint and an appropriate message.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`:
- Around line 312-314: The soft-delete path in OrgServiceImpl currently calls
cleanupOrganizationRelatedData(orgId) which hard-deletes related entities and
breaks restoreOrganization; remove that call from the soft-delete flow and
instead create a dedicated hard-cleanup method (e.g.,
performHardCleanupForWithdrawal or cleanupForWithdrawal) and call it only from
the explicit withdrawal/terminate flow. Update OrgServiceImpl to: 1) stop
invoking cleanupOrganizationRelatedData during soft deletes, 2) add a new public
method for permanent cleanup used by the withdrawal endpoint/process, and 3)
ensure restoreOrganization only toggles status and does not depend on removed
relations.
- Around line 501-512: The self-transfer check (Objects.equals(userId,
request.newOwnerUserId())) is performed before verifying the requester is the
current owner, causing non-owners to get ORG_OWNER_SAME_AS_BEFORE; move and
re-order the logic so you first load the organization via
orgRepository.findById(...) and validate ownership with
Objects.equals(organization.getOwnerUserId(), userId) (throw
OrgHandler(OrgErrorCode.ORG_FORBIDDEN) if false), and only after that perform
the self-transfer comparison against organization.getOwnerUserId() /
request.newOwnerUserId() and throw
OrgHandler(OrgErrorCode.ORG_OWNER_SAME_AS_BEFORE) when equal.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/UserService.java`:
- Around line 231-233: The code calls deleteProfileImage(profileImageUrl)
immediately after userRepository.deleteById(userId) which may delete S3 content
before the DB transaction commits, causing data inconsistency on rollback;
change this so S3 deletion runs only after a successful transaction commit by
registering a post-commit hook (e.g.,
TransactionSynchronizationManager.registerSynchronization(...) with afterCommit)
or publish a domain event and handle it with an
`@TransactionalEventListener`(phase = TransactionPhase.AFTER_COMMIT) that invokes
deleteProfileImage; apply the same change for the similar block around lines
280-288 to ensure all external S3 deletes happen only after the DB transaction
commits.
- Around line 255-257: The current check in UserService uses
orgMemberRepository.countByOrganizationId(organization.getId()), which counts
all OrgMember records including inactive/cleanup users and can wrongly block a
user’s leave; update the logic to call
orgMemberRepository.countByOrganizationIdAndUserStatus(organization.getId(),
<activeStatus>) (using your existing active/normal UserStatus enum constant) so
only active members are counted before deciding on ownership transfer; ensure
the conditional still checks > 1 and adjust any import/usages if necessary.
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.java`:
- Around line 27-28: The repository method findByUser_Id in
PlatformConnectionRepository loads full entity lists when only existence is
needed; replace it with a boolean existsByUser_Id(Long userId) to avoid fetching
entities and reduce DB/memory cost—update the repository interface to declare
existsByUser_Id and change any callers that expect List<PlatformConnection>
(e.g., user deletion validation paths) to use the boolean result accordingly.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java`:
- Around line 127-137: Replace the long concatenated description string in
UserControllerDocs (the assignment to description) with a Java 17 text block
("""...""") so the multi-line Swagger description is written verbatim; locate
the line containing description = "..." and swap the concatenated pieces for a
single text block, preserving the existing newlines and content and ensuring
indentation is trimmed appropriately for Java text blocks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 656340f3-9458-4f72-a5d2-817c6922c131
📒 Files selected for processing (16)
src/main/java/com/whereyouad/WhereYouAd/domains/ai/persistence/repository/AIInsightReportRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/application/dto/request/OrgRequest.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/exception/code/OrgErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/entity/Organization.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgInvitationRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgMemberRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/presentation/OrgController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/presentation/docs/OrgControllerDocs.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/persistence/repository/PlatformConnectionRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/timeline/persistence/repository/TimelineRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/UserService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/exception/code/UserErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/UserController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/presentation/docs/UserControllerDocs.java
| public record ChangeOwner( | ||
| @Schema(description = "워크스페이스 새 소유자로 지정할 사용자의 DB Id", example = "1") | ||
| @NotNull(message = "새 소유자 Id 는 필수입니다.") | ||
| Long newOwnerUserId | ||
| ) {} |
There was a problem hiding this comment.
newOwnerUserId는 양수 제약까지 추가하는 게 안전합니다.
Line 41의 @NotNull만으로는 0/음수 ID가 통과되어 서비스 계층에서 의미가 덜 맞는 에러로 떨어질 수 있습니다. 요청 검증 단계에서 차단해 주세요.
제안 수정안
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.Positive;
@@
public record ChangeOwner(
`@Schema`(description = "워크스페이스 새 소유자로 지정할 사용자의 DB Id", example = "1")
`@NotNull`(message = "새 소유자 Id 는 필수입니다.")
+ `@Positive`(message = "새 소유자 Id 는 1 이상이어야 합니다.")
Long newOwnerUserId
) {}📝 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.
| public record ChangeOwner( | |
| @Schema(description = "워크스페이스 새 소유자로 지정할 사용자의 DB Id", example = "1") | |
| @NotNull(message = "새 소유자 Id 는 필수입니다.") | |
| Long newOwnerUserId | |
| ) {} | |
| public record ChangeOwner( | |
| `@Schema`(description = "워크스페이스 새 소유자로 지정할 사용자의 DB Id", example = "1") | |
| `@NotNull`(message = "새 소유자 Id 는 필수입니다.") | |
| `@Positive`(message = "새 소유자 Id 는 1 이상이어야 합니다.") | |
| Long newOwnerUserId | |
| ) {} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/application/dto/request/OrgRequest.java`
around lines 39 - 43, The ChangeOwner.record's newOwnerUserId currently only has
`@NotNull`; add a positive-number constraint (e.g., `@Positive` or `@Min`(1)) to
reject 0/negative IDs at validation time so invalid IDs don't reach the service
layer; update the annotation on newOwnerUserId in the ChangeOwner record to
include the chosen positive constraint and an appropriate message.
# Conflicts: # src/main/java/com/whereyouad/WhereYouAd/domains/timeline/persistence/repository/TimelineRepository.java
There was a problem hiding this comment.
P1: 고생하셨습니다! 생각보다 유저와 연관된 엔티티가 많아서 조금 까다로운 것 같습니다. 다만.. 지금 로직이 소프트 딜리트와 하드 딜리트가 섞여있는 것 같습니다..!
제 생각에는..
- Hard Delete를 구현 (DB FK에 맞춰 유저 관련 데이터 전부 삭제)
- Soft Delete를 구현(User status만 Deleted로 변경)
- Soft Delete된 상태가 1달이상 정도가 되면 스케줄러를 통해서 자동으로 HardDelete 메서드를 호출하는 방식
으로 구현하는 것은 어떤가요..?
지금 Hard Delete하려면 유저가 먼저 삭제해야 하는 것들이 너무 많은 것 같아서 프론트 쪽에서 유저를 삭제하면 관련된 조직(소유자인 경우)등 모든 데이터가 삭제된다고 안내하고 관련된 모든 데이터를 DB FK 제약조건을 지켜서 순서대로 관련 데이터를 전부 삭제하면 괜찮을 것 같습니다..! 혹시 Metric_Fact부터 순차적으로 조직까지 삭제하고 PlatformConnection -> PlatformAccount 순서로 삭제하는 방식으로 FK 제약조건 맞추어서 삭제를 진행해도 오류가 날까요...? 이쪽에 뭔가 연관된 엔티티가 많아서 삭제하는 순서가 좀 어려운 것 같습니다..
추가로 만약 User에도 SoftDelete가 생긴다면 기존 쿼리문도 수정해야할 수도 있을 것 같습니다..! (WHERE 절에 USER.Status = ACTIVE 추가)
| continue; | ||
| } | ||
|
|
||
| int memberCount = orgMemberRepository.countByOrganizationIdAndUserStatus(organization.getId(), UserStatus.ACTIVE); |
There was a problem hiding this comment.
P2: 아마 지금 구현하신 것이 Hard Delete 같은데 Soft Delete랑 좀 섞여서 구현된 것 같습니다..!
| } | ||
|
|
||
| // 본인만 속한 조직은 OrgService 의 Soft Delete 로직 호출 (조직 연관된 엔티티 정리 포함) | ||
| orgService.removeOrganizationSoft(userId, organization.getId()); |
There was a problem hiding this comment.
P2: 아마 조직 soft delete된 조직을 한달 이상된 조직에 대해서 삭제와 같이 주기적으로 삭제하는 코드가 없는 걸로 알고 있습니다..! 그래서 유저를 하드 딜리트하고 조직을 소프트 딜리트하게 된다면 해당 조직은 계속 DB에 남게 될 것 같습니다..! 또한 회원에 대해서는 Hard Delete이기 때문에 다시 복구할 방법도 없는 것 같습니다..!
jinnieusLab
left a comment
There was a problem hiding this comment.
P3: 확인했습니다! 음 생각보다 회원 탈퇴가 복잡한 것 같아서 제 의견을 정리해봤습니다..! 백엔드 내부에서 1차 지연 삭제(Soft) -> 2차 물리 삭제(Batch)하는 식으로 가면 될 듯한데, 말씀해주신 대로 작업량이 꽤 될 듯 하네요..
1. 회원 탈퇴 요청
단, 탈퇴하는 유저가 소유자(Owner)라면, 다른 멤버에게 소유권을 이전 후 탈퇴하도록 함.
| 엔티티 | 삭제 여부 | 설명 |
|---|---|---|
| User | Soft Delete (status = DELETED) | 한 달 유예 기간 |
| PlatformConnection | Hard Delete | 유저 개인 인증 정보, 즉시 삭제 |
| OrgMember | Hard Delete | 중간 테이블이니 삭제 |
| RefreshToken | Hard Delete (Redis에서 제거) | 탈퇴 시 기존 JWT 토큰 즉시 무효화하기 |
| OrgInvitation | Hard Delete | 탈퇴 이메일로 pending 중인 조직들 초대 정리 필요 |
만약 유저가 조직 내 유일한 멤버였다면, 조직 관련도 삭제 처리 (Soft Delete)
| 엔티티 | 삭제 여부 | 설명 |
|---|---|---|
| Organization | Soft Delete | status = DELETED |
| PlatformAccount | Soft Delete | |
| AdCampaign ~ AdContent | Soft Delete | |
| ClickLog / MetricFact | 상태 변경 없이 유지 | 상위 엔티티(Project/Campaign) 활성 상태 여부만 체크 |
2. 유저 재가입 한 달 유예 기간 지난 후
스케줄러나 스프링 배치 도입하여 새벽에 작업 진행
- ClickLog/MetricFact: chuck 단위만큼 쪼개서 안전하게 Hard Delete
- Organization ~ AdContent: 연관 관계 역순으로 Hard Delete
- User: 개인정보(이메일, 이름, 비밀번호 등) 마스킹/익명화 처리. (과거 소속되었던 조직의 데이터 무결성을 위해 Hard Delete하지 않음)
ClickLog나 MetricFact의 경우 양이 매우 많다면 일일이 삭제 시 DB 병목이나 락이 생길 수도 있을 듯하여, 삭제 시 스케줄러나 배치를 도입해보는 건 어떨까요?
- UserWithdrawalFacade 등 파사드 패턴 도입
여러 군데에서 로직이 흩어져 있는 걸 모아서 한 번에 관장하는 곳이 필요해보이는데, 파사드 클래스에서 하위에 UserService, OrganizationService, PlatformAuthService 등 주입받아 각자의 도메인 로직만 수행하도록 위임하는 것도 좋을 것 같습니다! (도메인간 결합도 낮추기)
정리해보니 수정이 꽤 오래 걸려서 작업량이 많아질 것 같아서 탈퇴 관련 작업 분배를 한 번 해야 할 듯합니다!
…약을 위한 OrgErrorCode 추가
…t Delete 내부 Timeline, AIInsightReport 제거 로직 추가
… 로직 Hard Delete 로 위임
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgMemberRepository.java (1)
23-26: ⚡ Quick win
@Modifying에clearAutomatically = true추가를 권장합니다.벌크 DELETE 쿼리 실행 후 영속성 컨텍스트에 캐시된
OrgMember엔티티가 자동으로 제거되지 않습니다. 같은 트랜잭션 내에서 삭제된 엔티티를 조회하면 여전히 존재하는 것처럼 보일 수 있어요.예를 들어:
// 1. OrgMember 조회 (영속성 컨텍스트에 캐시됨) List<OrgMember> members = orgMemberRepository.findOrgMemberByUserId(userId); // 2. 벌크 삭제 실행 orgMemberRepository.deleteByUserId(userId); // 3. 다시 조회하면? → clearAutomatically 없으면 캐시된 데이터 반환 가능!💡 제안 수정안
// userId 기준 OrgMember 일괄 Hard Delete (User Hard Delete 정리용) -@Modifying +@Modifying(clearAutomatically = true) `@Query`("DELETE FROM OrgMember om WHERE om.user.id = :userId") void deleteByUserId(`@Param`("userId") Long userId);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgMemberRepository.java` around lines 23 - 26, The bulk DELETE in OrgMemberRepository.deleteByUserId uses `@Modifying` but doesn't set clearAutomatically, so deleted OrgMember entities can remain in the persistence context and appear to still exist within the same transaction; update the `@Modifying` annotation on the deleteByUserId method to include clearAutomatically = true so the persistence context is cleared after the bulk delete and stale OrgMember cache entries are removed.src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/entity/User.java (1)
48-49: ⚡ Quick win
LocalDate대신LocalDateTime사용을 고려해 주세요.탈퇴 시각을
LocalDate로 저장하면 날짜만 기록되고 시/분/초 정보가 유실됩니다. 운영 중 문제 추적이나 감사 로그 분석 시 정확한 탈퇴 시각이 필요할 수 있어요.예를 들어, 같은 날 여러 사용자가 탈퇴했을 때 순서를 파악하거나, 탈퇴 후 일정 시간 내 복구 요청이 들어왔을 때 시간 단위 검증이 필요할 수 있습니다.
💡 제안 수정안
-import java.time.LocalDate; +import java.time.LocalDateTime; `@Column`(nullable = true, name = "deleted_at") -private LocalDate deletedAt; +private LocalDateTime deletedAt;그리고
softDeleteUser()메서드도:public void softDeleteUser() { this.status = UserStatus.DELETED; - this.deletedAt = LocalDate.now(); + this.deletedAt = LocalDateTime.now(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/entity/User.java` around lines 48 - 49, User 엔티티의 deletedAt 필드가 LocalDate로 선언되어 있어 시/분/초 정보가 손실됩니다; User 클래스의 private LocalDate deletedAt를 LocalDateTime으로 변경하고, 관련 getter/setter와 매핑 어노테이션(`@Column` name="deleted_at")은 그대로 유지하되 타입을 LocalDateTime으로 업데이트하세요; 또한 softDeleteUser() 메서드(혹은 관련 서비스/레포지토리에서 삭제 시각을 설정하는 메서드)를 찾아 현재 LocalDate로 할당하는 부분을 LocalDateTime.now() 또는 적절한 시점의 LocalDateTime 값으로 변경해 일관된 타입을 사용하도록 수정하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`:
- Around line 329-358: The removeOrganizationsOwnedBySoftDeletedUser method
omits cleaning up OrgInvitation records and should mirror removeOrganization;
before deleting the Organization, call
orgInvitationRepository.deleteByOrganizationId(orgId) (using the same orgId
variable) to remove related invitations so no orphaned OrgInvitation rows or FK
violations remain; add this call alongside
timelineRepository.deleteByOrganizationId(orgId) and
aiInsightReportRepository.deleteByOrganizationId(orgId) prior to
orgRepository.delete(organization).
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/scheduler/UserDeleteScheduler.java`:
- Around line 40-56: The loop in hardDeleteUsers is held in a single transaction
so failures can rollback prior successful deletes; fix by executing each user
deletion in its own transaction: move the deletion logic currently in
hardDeleteSingleUser into a Spring-managed bean (e.g., UserDeletionService) and
annotate that method with `@Transactional`(propagation = Propagation.REQUIRES_NEW)
(or call hardDeleteSingleUser via the class's proxy/self-injection) and invoke
that per-user from UserDeleteScheduler.hardDeleteUsers so each deletion is
isolated and failure on one user does not roll back others.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/UserService.java`:
- Around line 225-233: The current flow only soft-deletes the User
(user.softDeleteUser()) and defers cleaning OrgMember and AuthProviderAccount to
the scheduler; instead, immediately remove organization memberships and social
auth links before or as part of the soft-delete. Add calls to the appropriate
repositories (e.g., orgMemberRepository.deleteByUserId(user.getId()) or
deleteByEmail(user.getEmail()) and
authProviderAccountRepository.deleteByUserId(user.getId()) or
deleteByEmail(user.getEmail())) right before
refreshTokenRepository.deleteById(...) / user.softDeleteUser(), ensuring
OrgMember and AuthProviderAccount entries are removed immediately on withdrawal.
---
Nitpick comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgMemberRepository.java`:
- Around line 23-26: The bulk DELETE in OrgMemberRepository.deleteByUserId uses
`@Modifying` but doesn't set clearAutomatically, so deleted OrgMember entities can
remain in the persistence context and appear to still exist within the same
transaction; update the `@Modifying` annotation on the deleteByUserId method to
include clearAutomatically = true so the persistence context is cleared after
the bulk delete and stale OrgMember cache entries are removed.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/entity/User.java`:
- Around line 48-49: User 엔티티의 deletedAt 필드가 LocalDate로 선언되어 있어 시/분/초 정보가 손실됩니다;
User 클래스의 private LocalDate deletedAt를 LocalDateTime으로 변경하고, 관련 getter/setter와
매핑 어노테이션(`@Column` name="deleted_at")은 그대로 유지하되 타입을 LocalDateTime으로 업데이트하세요; 또한
softDeleteUser() 메서드(혹은 관련 서비스/레포지토리에서 삭제 시각을 설정하는 메서드)를 찾아 현재 LocalDate로 할당하는
부분을 LocalDateTime.now() 또는 적절한 시점의 LocalDateTime 값으로 변경해 일관된 타입을 사용하도록 수정하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 417ef569-f9fa-4670-ba2b-6b50346bd089
📒 Files selected for processing (11)
src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/exception/code/OrgErrorCode.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgInvitationRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgMemberRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/UserService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/scheduler/UserDeleteScheduler.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/entity/User.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/repository/AuthProviderAccountRepository.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/persistence/repository/UserRepository.java
| // 2) 즉시 정리 단계 | ||
| // 해당 회원 이메일로 발송된 pending OrgInvitation 정리 | ||
| orgInvitationRepository.deleteByEmail(user.getEmail()); | ||
| // JWT RefreshToken 삭제 | ||
| refreshTokenRepository.deleteById(user.getEmail()); | ||
|
|
||
| // 3) 회원 삭제 -> Soft Delete 상태로 변경, 30일 지날 시 스케줄러에서 Hard Delete | ||
| user.softDeleteUser(); | ||
| } |
There was a problem hiding this comment.
회원 탈퇴 직후에도 조직 멤버십/소셜 연동 계정이 남아 요구사항과 어긋납니다.
Line 225~233은 사용자만 Soft Delete 처리하고, OrgMember/AuthProviderAccount 정리는 스케줄러(30일 후)로 미뤄집니다. 현재 PR 목표(탈퇴 시 멤버 관계 정리) 기준이면 탈퇴 즉시 정리되어야 합니다.
제안 diff
// JWT RefreshToken 삭제
refreshTokenRepository.deleteById(user.getEmail());
+ // 탈퇴 즉시 관계 정리
+ orgMemberRepository.deleteByUserId(userId);
+ authProviderAccountRepository.deleteByUserId(userId);
+
// 3) 회원 삭제 -> Soft Delete 상태로 변경, 30일 지날 시 스케줄러에서 Hard Delete
user.softDeleteUser();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/UserService.java`
around lines 225 - 233, The current flow only soft-deletes the User
(user.softDeleteUser()) and defers cleaning OrgMember and AuthProviderAccount to
the scheduler; instead, immediately remove organization memberships and social
auth links before or as part of the soft-delete. Add calls to the appropriate
repositories (e.g., orgMemberRepository.deleteByUserId(user.getId()) or
deleteByEmail(user.getEmail()) and
authProviderAccountRepository.deleteByUserId(user.getId()) or
deleteByEmail(user.getEmail())) right before
refreshTokenRepository.deleteById(...) / user.softDeleteUser(), ensuring
OrgMember and AuthProviderAccount entries are removed immediately on withdrawal.
|
@kingmingyu @jinnieusLab 어느정도 수정 진행해봤습니다... 이전 커밋내역이 섞여서 좀 복잡할 수 있을거 같아서 깃허브로 확인하기보다 feat/#127 브랜치로 체크아웃해서 직접 IDE 로 보시는걸 추천합니다ㅠㅠ
저는 "PlatformConnection, PlatformAccount, 와 광고 관련 (AdCampaign, AdGroup, AdContent, MetricFact, ClickLog)" 를 하나로 묶어서 보고, "Organization 과 OrgInvitation, AIInsightReport, Timeline" 을 하나로 묶어서 보는걸로 생각했습니다. 하다보니 많이 복잡해졌네요...ㅠㅠ 시간 걸려도 괜찮으니 천천히 봐주셔도 됩니다ㅠㅠㅠㅠㅠ +++ |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/scheduler/UserDeleteExecutor.java`:
- Around line 32-47: The hardDeleteSingleUser method in UserDeleteExecutor is
missing cleanup of pending OrgInvitation entries sent to the user's email; add a
call to orgInvitationRepository.deleteByEmail(user.getEmail()) (or equivalent)
before deleting the user and/or after removing org memberships, and if the
repository method doesn't exist implement deleteByEmail(String email) in
OrgInvitationRepository to remove invitations by email; ensure you reference the
orgInvitationRepository field in UserDeleteExecutor and handle null/empty email
defensively (skip call if email is null/blank) and keep the S3 image deletion
behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 50a92fcb-e298-4d9b-833e-436157c3a66b
📒 Files selected for processing (3)
src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/scheduler/UserDeleteExecutor.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/scheduler/UserDeleteScheduler.java
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformServiceImpl.java`:
- Around line 147-151: The disconnectPlatform method in PlatformServiceImpl
currently no-ops and falsely signals success; change it to fail-fast until real
deletion logic is implemented by throwing a clear runtime exception (e.g.,
UnsupportedOperationException or a custom NotImplemented/OperationNotSupported
exception) from disconnectPlatform(Long userId, Long orgId, Long accountId) so
callers receive an explicit failure instead of a silent success; include a
message referencing the missing removal of PlatformConnection/PlatformAccount
and related ad-domain entities to aid future implementation and debugging.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7abcfd9b-9e02-4969-b28e-8566487fd2d1
📒 Files selected for processing (4)
src/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformService.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/domain/service/PlatformServiceImpl.javasrc/main/java/com/whereyouad/WhereYouAd/domains/platform/presentation/PlatformController.javasrc/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/UserService.java
💤 Files with no reviewable changes (1)
- src/main/java/com/whereyouad/WhereYouAd/domains/user/domain/service/UserService.java
jinnieusLab
left a comment
There was a problem hiding this comment.
P4: 고생하셨습니다! 최근에 계속 일이 많았어서 이제 코드 리뷰 남기네요 ㅠㅠ..
유저와 조직 쪽 Soft/Delete 로직이 알맞은 여러 클래스에 잘 맞게 분리해서 코드 작성해주신 것 같아 좋습니다!
말씀해주신대로 작업 분배 해보자면
- 광고 관련 MetricFact, ClickLog Hard Delete (배치나 chunk)
- 광고 관련 (Project ~ AdContent) Hard Delete
- PlatformConnection, PlatformAccount Hard Delete
이렇게 나눠서 맡아보면 괜찮을 것 같습니다!
괜찮습니다! 작업한 내용도 좀 복잡해서 제가 보기에도 복잡했어서 오래 걸릴 수 있었을 것 같아요....
이정도 일 것 같습니다! |
kingmingyu
left a comment
There was a problem hiding this comment.
P4: 고생하셨습니다!! 저도 리뷰가 좀 늦어져서 죄송합니다.. 😢 회원 탈퇴가 생각보다 복잡한 것 같은데 이것저것 많이 고려해서 잘 구현해주신 것 같습니다! 플랫폼 연동 해제 쪽만 잘 구현되면 괜찮을 것 같습니다!
궁금한 점이 있는데 현재 스케줄러가 싱글 스레드로 알고 있는데 모든 작업을 새벽 2시에만 하면 유저나 조직등에 만약 문제가 생기거나 작업이 많아지면 광고 연동 작업이 지연될 것 같습니다. 혹은 반대로 광고 연동이 느려져도 마찬가지로 회원 삭제가 지연될 것 같아서 혹시 조직이나 회원, 초대장에 관련된 스케줄러 작업은 새벽 3시 쯤으로 바꾸는 건 어떨까요??
그리고 유저 삭제 스케줄러를 일주일 중 하루로 정하신 이유가 궁금합니다!
코드 구조가 복잡해서 시간 충분히 걸릴 수 있었을 거 같습니다. 괜찮습니다!!
|
30일 이후 회원은 삭제 처리 의도라면 매일 새벽 3시도 괜찮은 것 같습니다! 하지만 일주일에 한 번 처리하는게 모아서 처리하는 방식이다 보니 효율성은 더 좋을 것 같습니다! 좀 더 괜찮다고 생각하시는 방향으로 해도 좋을 것 같습니다..! |
피드백 반영해서 매일 새벽 3시에 Hard Delete 수정으로 변경했습니다! 머지 진행하겠습니다. |
📌 관련 이슈
🚀 개요
회원 탈퇴 기능과 함께 연관된 엔티티 제거 로직 추가
📄 작업 내용
📸 스크린샷 / 테스트 결과 (선택)
탈퇴 API 테스팅을 위한 회원 가입 진행 (userId=14)




userId=14 인 회원이 조직 생성(owner 가 userId=14 이고 다른 멤버 없는 상태)
userId=14 의 조직 가입 상태 (org_id = 10 은 생성자, org_id = 1, 2, 3 은 MEMBER)
회원 탈퇴 진행 결과 -> 회원 삭제, 조직 Soft Delete, OrgMember 관련 데이터들 모두 삭제
===오류 테스트 (다른 회원이 조직에 속한 경우)===
오류 테스트를 위해 회원, 조직 복구


user_id = 3 이 탈퇴하려는 회원이 owner 인 조직에 MEMBER 로 속해있는 상태
탈퇴 시도 시 오류
✅ 체크리스트
🔍 리뷰 포인트 (Review Points)
===의논점===
Summary by CodeRabbit
New Features
Behavior Changes
Errors / Messages