Conversation
Walkthrough조직 삭제 기능에 Soft/Hard 삭제와 소프트 삭제 복구(PATCH)가 추가되었습니다. OrgRequest.Delete DTO는 제거되고 OrgResponse.Delete가 추가되었으며, 서비스·컨트롤러·엔티티·리포지토리·에러코드가 관련 로직으로 확장되었습니다. (사실만 보고) Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant Service
participant Repository
participant Entity
rect rgba(100,200,100,0.5)
Note over Client,Entity: Soft Delete & Restore Flow
Client->>Controller: DELETE /api/org/{orgId}?isHard=false
Controller->>Service: removeOrganizationSoft(userId, orgId)
Service->>Repository: findById(orgId)
Repository-->>Service: Organization(status=ACTIVE)
Service->>Service: 권한 검증 (creator)
Service->>Entity: softDelete() -> status=DELETED
Service->>Repository: save(Organization)
Repository-->>Service: saved
Controller-->>Client: 200/204 (DataResponse<String>)
Client->>Controller: PATCH /api/org/{orgId}/restore
Controller->>Service: restoreOrganization(userId, orgId)
Service->>Repository: findById(orgId)
Repository-->>Service: Organization(status=DELETED)
Service->>Service: 권한 및 상태 검증
Service->>Entity: restoreDelete() -> status=ACTIVE
Service->>Repository: save(Organization)
Service-->>Controller: OrgResponse.Delete(orgId, message)
Controller-->>Client: 200 DataResponse<OrgResponse.Delete>
end
sequenceDiagram
participant Client
participant Controller
participant Service
participant Repository
rect rgba(200,100,100,0.5)
Note over Client,Repository: Hard Delete Flow
Client->>Controller: DELETE /api/org/{orgId}?isHard=true
Controller->>Service: removeOrganization(userId, orgId)
Service->>Repository: findById(orgId)
Repository-->>Service: Organization
Service->>Service: 권한 검증
Service->>Repository: findOrgMemberByOrg(Organization)
Repository-->>Service: List<OrgMember>
Service->>Repository: deleteAll(orgMembers)
Service->>Repository: delete(Organization)
Controller-->>Client: 200/204 (DataResponse<String>)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
시니어 관점 짧은 코멘트:
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`:
- Around line 113-119: The current hard-delete loop uses
orgMemberRepository.deleteAll(orgMembers) which issues individual DELETEs per
OrgMember; replace that with a batch delete to improve performance by calling
orgMemberRepository.deleteAllInBatch(orgMembers) (or the repository's
deleteAllInBatch variant) before orgRepository.delete(organization), ensuring
OrgMember has no JPA lifecycle callbacks like `@PreRemove` that you rely on;
update the call at the site where orgMemberRepository.deleteAll(orgMembers) is
invoked in OrgServiceImpl.java and verify behavior.
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/presentation/docs/OrgControllerDocs.java`:
- Around line 51-55: Remove mapping annotations from the OrgControllerDocs
interface: delete the `@PatchMapping` on restoreOrganization and the
`@DeleteMapping` on the other documented method(s) so only OrgController
implementation declares request mappings; keep only documentation annotations
like `@Operation` and `@ApiResponses` in OrgControllerDocs for consistency with
createOrganization and modifyOrganization. While editing, confirm the
restoreOrganization return type: if restore should not return
OrgResponse.Delete, change the signature in OrgControllerDocs (and align
OrgController) to the correct response DTO (e.g., OrgResponse.Restore or
OrgResponse.Detail) to match intended semantics. Ensure method names
restoreOrganization, createOrganization, modifyOrganization and the
OrgResponse.* types are updated consistently between interface and
implementation.
🧹 Nitpick comments (7)
src/main/java/com/whereyouad/WhereYouAd/domains/organization/application/dto/response/OrgResponse.java (1)
24-28: Record 이름이 용도와 맞지 않아요.
Deleterecord가 실제로는 Soft Delete 복구 응답에 사용되고 있어서, 이름만 보면 "삭제 응답"으로 오해할 수 있어요. 예를 들어Restore나RestoreResponse같은 이름이 더 직관적일 것 같습니다.// 현재: Delete -> 삭제 응답처럼 보임 // 권장: Restore -> 복구 응답임을 명확히 표현♻️ 이름 변경 제안
- //Soft Delete 복구 시 응답값 - public record Delete ( + //Soft Delete 복구 시 응답값 + public record Restore ( Long orgId, String message ) {}src/main/java/com/whereyouad/WhereYouAd/domains/organization/persistence/repository/OrgMemberRepository.java (1)
17-19: Spring Data JPA의 쿼리 메서드 파생 기능을 활용하면 더 간단해져요.현재
@Query어노테이션을 사용하고 있는데, Spring Data JPA는 메서드 이름만으로 쿼리를 자동 생성해줍니다. 더 간결하고 유지보수하기 쉬워요.♻️ 파생 쿼리 메서드 사용 제안
//특정 Organization 에 속한 OrgMember 모두 추출하는 메서드 - `@Query`("select om from OrgMember om where om.organization = :organization") - List<OrgMember> findOrgMemberByOrg(`@Param`(value = "organization") Organization organization); + List<OrgMember> findByOrganization(Organization organization);이렇게 하면 Spring Data JPA가
organization필드를 기준으로 자동으로 쿼리를 생성해줍니다.findOrgMemberByUser와 네이밍 패턴도 일관되게findByOrganization으로 맞출 수 있어요.src/main/java/com/whereyouad/WhereYouAd/domains/organization/application/mapper/OrgConverter.java (1)
23-26: 메서드 구현은 좋은데, 메시지 관리 방식을 고려해보세요.
toRestoredResponse메서드명은 명확하고 좋아요! 다만 두 가지 개선 포인트가 있습니다:
반환 타입
OrgResponse.Delete: 앞서 언급했듯이OrgResponse.Restore로 이름을 바꾸면 이 메서드와도 더 잘 어울립니다.하드코딩된 메시지: 나중에 다국어 지원이나 메시지 일괄 관리가 필요할 때를 대비해서, 상수나
MessageSource로 분리하는 것도 좋아요.// 예시: 상수로 분리 private static final String RESTORE_SUCCESS_MESSAGE = "해당 조직이 활성화 되었습니다";현재 규모에서는 큰 문제가 아니니 참고만 해주세요!
src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java (2)
68-75: 권한 검증 로직이 반복되고 있어요.
findById+ 존재 여부 확인 +ownerUserId검증 패턴이 4개 메서드에서 동일하게 반복됩니다. Private 헬퍼 메서드로 추출하면 코드 중복을 줄이고 유지보수성을 높일 수 있어요.♻️ 공통 로직 추출 제안
/** * 조직을 조회하고, 요청자가 조직 생성자인지 검증합니다. * `@throws` OrgHandler ORG_NOT_FOUND - 조직이 존재하지 않을 때 * `@throws` OrgHandler ORG_FORBIDDEN - 요청자가 조직 생성자가 아닐 때 */ private Organization findOrganizationAndValidateOwner(Long userId, Long orgId) { Organization organization = orgRepository.findById(orgId) .orElseThrow(() -> new OrgHandler(OrgErrorCode.ORG_NOT_FOUND)); if (!organization.getOwnerUserId().equals(userId)) { throw new OrgHandler(OrgErrorCode.ORG_FORBIDDEN); } return organization; }사용 예시:
public OrgResponse.Update modifyOrganization(Long userId, Long orgId, OrgRequest.Update request) { Organization organization = findOrganizationAndValidateOwner(userId, orgId); organization.modifyInfo(request); return OrgConverter.toUpdatedResponse(organization); } public void removeOrganizationSoft(Long userId, Long orgId) { Organization organization = findOrganizationAndValidateOwner(userId, orgId); organization.softDelete(); }이렇게 하면 검증 로직 변경 시 한 곳만 수정하면 되고, 테스트도 더 쉬워집니다!
Also applies to: 84-91, 103-111, 122-130
122-134: Soft Delete 시 이미 삭제된 조직에 대한 검증이 없어요.
restoreOrganization에서는 이미 ACTIVE인 경우를 체크하는데,removeOrganizationSoft에서는 이미 DELETED인 경우를 체크하지 않아요.동작에 문제가 생기진 않지만(같은 값으로 다시 설정하는 것이니까), 일관성 있는 API 동작을 위해 검증을 추가하는 것이 좋을 것 같아요. 클라이언트 입장에서 "이미 삭제된 조직입니다"라는 피드백을 받는 게 더 명확하거든요.
💡 상태 검증 추가 제안
public void removeOrganizationSoft(Long userId, Long orgId) { Organization organization = orgRepository.findById(orgId) .orElseThrow(() -> new OrgHandler(OrgErrorCode.ORG_NOT_FOUND)); if (!organization.getOwnerUserId().equals(userId)) { throw new OrgHandler(OrgErrorCode.ORG_FORBIDDEN); } + if (organization.getStatus() == OrgStatus.DELETED) { + throw new OrgHandler(OrgErrorCode.ORG_ALREADY_DELETED); // 새 에러코드 필요 + } + organization.softDelete(); }
OrgErrorCode에ORG_ALREADY_DELETED추가가 필요합니다.src/main/java/com/whereyouad/WhereYouAd/domains/organization/presentation/docs/OrgControllerDocs.java (1)
52-52: Restore API의 응답 타입 이름이 혼란스러울 수 있습니다.
restoreOrganization메서드가OrgResponse.Delete타입을 반환하는데, "Delete"라는 이름이 복구(restore) 기능과 의미적으로 맞지 않습니다.예를 들어, 나중에 코드를 읽는 개발자가 "왜 복구 API가 Delete 응답을 반환하지?"라고 혼란스러워 할 수 있어요.
OrgResponse.Restore또는OrgResponse.StatusChange와 같이 의도가 명확한 이름을 고려해 보세요.src/main/java/com/whereyouad/WhereYouAd/domains/organization/presentation/OrgController.java (1)
78-82: 컨트롤러의 if-else 분기를 서비스 레이어로 이동하는 것을 고려해 보세요.현재
isHard값에 따라 컨트롤러에서 직접 분기하고 있는데, 이 로직을 서비스 레이어에서 처리하면 컨트롤러가 더 단순해지고 단일 책임 원칙(SRP)에 부합합니다.// 현재 방식 if (isHard) { orgService.removeOrganization(userId, orgId); } else { orgService.removeOrganizationSoft(userId, orgId); } // 대안: 서비스에 위임 orgService.removeOrganization(userId, orgId, isHard);서비스 메서드 하나로 통합하면, 향후 삭제 방식이 추가되거나 정책이 변경될 때 컨트롤러 수정 없이 서비스만 변경하면 됩니다. 다만 현재 구조도 충분히 명확하므로, 이 부분은 선택적 리팩토링입니다.
| //해당 조직에 가입된 모든 회원들의 가입 정보 삭제 | ||
| List<OrgMember> orgMembers = orgMemberRepository.findOrgMemberByOrg(organization); | ||
|
|
||
| orgMemberRepository.deleteAll(orgMembers); | ||
|
|
||
| //조직 실제 삭제 | ||
| orgRepository.delete(organization); |
There was a problem hiding this comment.
Hard Delete 시 성능 개선 포인트가 있어요.
deleteAll(orgMembers)는 내부적으로 각 엔티티마다 개별 DELETE 쿼리를 실행해요. 만약 조직에 멤버가 100명이면 DELETE 쿼리가 100번 나가게 됩니다.
deleteAllInBatch()를 사용하면 단일 쿼리로 처리할 수 있어요:
-- deleteAll: DELETE FROM org_member WHERE id = 1; DELETE FROM org_member WHERE id = 2; ...
-- deleteAllInBatch: DELETE FROM org_member WHERE id IN (1, 2, 3, ...)⚡ 성능 개선 제안
//해당 조직에 가입된 모든 회원들의 가입 정보 삭제
List<OrgMember> orgMembers = orgMemberRepository.findOrgMemberByOrg(organization);
- orgMemberRepository.deleteAll(orgMembers);
+ orgMemberRepository.deleteAllInBatch(orgMembers);
//조직 실제 삭제
orgRepository.delete(organization);단, deleteAllInBatch는 영속성 컨텍스트를 거치지 않으므로 @PreRemove 같은 JPA 라이프사이클 콜백이 호출되지 않아요. OrgMember에 그런 콜백이 없다면 deleteAllInBatch 사용을 권장합니다.
🤖 Prompt for AI Agents
In
`@src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java`
around lines 113 - 119, The current hard-delete loop uses
orgMemberRepository.deleteAll(orgMembers) which issues individual DELETEs per
OrgMember; replace that with a batch delete to improve performance by calling
orgMemberRepository.deleteAllInBatch(orgMembers) (or the repository's
deleteAllInBatch variant) before orgRepository.delete(organization), ensuring
OrgMember has no JPA lifecycle callbacks like `@PreRemove` that you rely on;
update the call at the site where orgMemberRepository.deleteAll(orgMembers) is
invoked in OrgServiceImpl.java and verify behavior.
...java/com/whereyouad/WhereYouAd/domains/organization/presentation/docs/OrgControllerDocs.java
Outdated
Show resolved
Hide resolved
jinnieusLab
left a comment
jinnieusLab
left a comment
There was a problem hiding this comment.
P4: 고생하셨습니다! soft와 hard로 삭제를 나누니 확실히 백엔드 쪽에서도 덜 헷갈릴 것 같고 좋네요!
조직 생성자 외에 요청 시 나오는 에러코드로 통일한 것도 깔끔해서 좋습니다!
kingmingyu
left a comment
kingmingyu
left a comment
There was a problem hiding this comment.
P4: 고생하셨습니다! 응답 값이나 예외 처리도 꼼꼼하게 하신 것 같아요!
src/main/java/com/whereyouad/WhereYouAd/domains/organization/domain/service/OrgServiceImpl.java
Show resolved
Hide resolved
3 participants
📌 관련 이슈
🚀 개요
조직 삭제 API (Soft & Hard) 추가, Soft Delete 복구 API (PATCH) 추가
📄 작업 내용
📸 스크린샷 / 테스트 결과 (선택)
1-1. 해당 조직의 생성자인 id = 2 인 사용자로 로그인, 토큰 획득
1-2 . 해당 토큰으로 조직 Soft delete 요청, DB 정상 반영(status => DELETED)
1-3. Soft Delete 조직 복구, DB 정상 반영(status => ACTIVE)
1-4. org_id = 4 조직에 대해 Hard Delete 요청, DB 정상 반영
===예외 처리===
✅ 체크리스트
🔍 리뷰 포인트 (Review Points)
Summary by CodeRabbit
릴리스 노트
새로운 기능
버그 수정
문서