feat: Admin ticketType, ticketRound 삭제 API - #149 #156
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
📝 WalkthroughSummary by CodeRabbit
Walkthrough관리자용 티켓 라운드/타입 삭제 기능을 추가했다. 각 도메인에 DELETE API 컨트롤러와 트랜잭션 서비스, 삭제 컴포넌트를 도입하고, 타입 일괄 삭제용 리포지토리 메서드를 추가했다. 요청/응답 로깅 필터는 /actuator/health 경로를 로깅에서 제외하도록 조건이 추가되었다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin Client
participant C as AdminTicketRoundController
participant S as AdminTicketRoundService
participant Rv as AdminTicketRoundRetriever
participant TRm as AdminTicketTypeRemover
participant RRm as AdminTicketRoundRemover
Admin->>C: DELETE /api/admin/tickets/rounds/{ticketRoundId}
C->>S: deleteTicketRound(ticketRoundId)
S->>Rv: getTicketRoundById(ticketRoundId)
alt Found
S->>TRm: deleteAllTicketTypeByTicketRoundId(roundId)
S->>RRm: deleteTicketRoundById(roundId)
S-->>C: void
C-->>Admin: 200 OK (BaseResponse)
else Not Found
S-->>C: throw AdminApiException(NOT_FOUND_TICKET_ROUND)
C-->>Admin: Error response
end
sequenceDiagram
autonumber
actor Admin as Admin Client
participant C as AdminTicketTypeController
participant S as AdminTicketTypeService
participant Rt as AdminTicketTypeRetriever
participant Rm as AdminTicketTypeRemover
Admin->>C: DELETE /api/admin/tickets/types/{ticketTypeId}
C->>S: deleteTicketType(ticketTypeId)
S->>Rt: getTicketTypeById(ticketTypeId)
alt Found
S->>Rm: deleteTicketTypeById(ticketTypeId)
S-->>C: void
C-->>Admin: 200 OK (BaseResponse)
else Not Found
S-->>C: throw AdminApiException(NOT_FOUND_TICKET_TYPE)
C-->>Admin: Error response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/main/java/com/permitseoul/permitserver/domain/tickettype/core/repository/TicketTypeRepository.java (1)
25-25: 삭제 건수 반환으로 후속 로직/모니터링에 활용 가능삭제 성공 건수를 반환하면 테스트·모니터링·검증에 유용합니다. 선택 사항이지만 권장합니다.
- void deleteAllByTicketRoundId(final long ticketRoundId); + long deleteAllByTicketRoundId(final long ticketRoundId);src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java (1)
37-40: 구현 적절합니다. 파라미터를 원시형으로 단순화 고려(선택)서비스에서 primitive long만 전달된다면 Long → long으로 단순화해 NPE 여지를 제거할 수 있습니다.
- public TicketType getTicketTypeById(final Long ticketTypeId) { + public TicketType getTicketTypeById(final long ticketTypeId) { return TicketType.fromEntity(ticketTypeRepository.findById(ticketTypeId).orElseThrow(AdminTicketTypeNotFoundException::new)); }src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/api/service/AdminTicketTypeService.java (1)
19-27: 불필요한 도메인 변환 제거로 간결화 가능존재 체크만 필요하므로 도메인 매핑 없이 엔티티 조회 후 바로 삭제하면 비용과 의존을 줄일 수 있습니다.
@Transactional public void deleteTicketType(final long ticketTypeId) { try { - final TicketType ticketType = adminTicketTypeRetriever.getTicketTypeById(ticketTypeId); - adminTicketTypeRemover.deleteTicketTypeById(ticketType.getTicketTypeId()); + // 존재 검증 + adminTicketTypeRetriever.getTicketTypeEntityById(ticketTypeId); + // 삭제 + adminTicketTypeRemover.deleteTicketTypeById(ticketTypeId); } catch (AdminTicketTypeNotFoundException e) { throw new AdminApiException(ErrorCode.NOT_FOUND_TICKET_TYPE); } }src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/api/controller/AdminTicketRoundController.java (1)
23-23: DELETE 작업에는 204 NO_CONTENT 상태 코드 사용 권장.응답 본문이 없는 DELETE 작업에는 200 OK 대신 204 NO_CONTENT가 더 적합합니다.
다음 diff를 적용하여 상태 코드를 변경하세요:
adminTicketRoundService.deleteTicketRound(ticketRoundId); - return ApiResponseUtil.success(SuccessCode.OK); + return ApiResponseUtil.success(SuccessCode.NO_CONTENT);참고:
SuccessCodeenum에NO_CONTENT가 없는 경우 추가가 필요합니다.src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/api/controller/AdminTicketTypeController.java (1)
26-26: DELETE 작업에는 204 NO_CONTENT 상태 코드 사용 권장.응답 본문이 없는 DELETE 작업에는 200 OK 대신 204 NO_CONTENT가 더 적합합니다.
다음 diff를 적용하여 상태 코드를 변경하세요:
adminTicketTypeService.deleteTicketType(ticketTypeId); - return ApiResponseUtil.success(SuccessCode.OK); + return ApiResponseUtil.success(SuccessCode.NO_CONTENT);참고:
SuccessCodeenum에NO_CONTENT가 없는 경우 추가가 필요합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/api/controller/AdminTicketRoundController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/api/service/AdminTicketRoundService.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundRemover.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/api/controller/AdminTicketTypeController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/api/service/AdminTicketTypeService.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRemover.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/tickettype/core/repository/TicketTypeRepository.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/filter/DevRequestResponseLoggingFilter.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/api/controller/AdminTicketRoundController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/api/service/AdminTicketRoundService.java (2)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/exception/AdminTicketRoundNotFoundException.java (1)
AdminTicketRoundNotFoundException(3-4)src/main/java/com/permitseoul/permitserver/domain/admin/ticket/api/service/AdminTicketService.java (1)
Service(38-245)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRemover.java (2)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java (1)
Component(13-41)src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundRemover.java (1)
Component(7-15)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/api/service/AdminTicketTypeService.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/exception/AdminTicketTypeNotFoundException.java (1)
AdminTicketTypeNotFoundException(3-4)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRetriever.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/exception/AdminTicketTypeNotFoundException.java (1)
AdminTicketTypeNotFoundException(3-4)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/api/controller/AdminTicketTypeController.java (1)
src/main/java/com/permitseoul/permitserver/global/response/ApiResponseUtil.java (1)
ApiResponseUtil(7-29)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundRemover.java (1)
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRemover.java (1)
Component(7-19)
🔇 Additional comments (6)
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/core/component/AdminTicketRoundRemover.java (1)
12-14: LGTM — 단순 위임이 명확합니다서비스 계층 트랜잭션 경계 내에서 호출되므로 책임 분리가 적절합니다.
src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/api/service/AdminTicketRoundService.java (1)
21-31: 삭제 순서와 트랜잭션 경계 적절합니다타입 선삭제 → 라운드 삭제 순서가 FK 제약 충돌을 예방하며, @transactional로 원자성 보장됩니다. 그대로 진행해도 무방합니다.
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/api/controller/AdminTicketTypeController.java (2)
20-27: 관리자 권한 검증 확인 필요.관리자 전용 엔드포인트에 인증/인가 검증이 보이지 않습니다. AdminTicketRoundController와 동일하게 보안 필터나 글로벌 설정에서 처리되고 있는지 확인이 필요합니다.
보안 설정 확인은 AdminTicketRoundController 리뷰의 검증 스크립트를 참고하세요.
14-27: 관리자 권한 검증 확인 필요
AdminTicketRoundController와 동일하게, 이 컨트롤러도/api/admin/**경로의 삭제 API를 제공하지만 컨트롤러 레벨에서 인증/인가 어노테이션이 보이지 않습니다.글로벌 Security 설정에서 관리자 경로에 대한 접근 제어가 적절히 구성되어 있는지 확인하세요.
위의
AdminTicketRoundController에서 제공한 검증 스크립트를 실행하여 Security 설정을 확인하세요.src/main/java/com/permitseoul/permitserver/domain/admin/ticketround/api/controller/AdminTicketRoundController.java (2)
17-24: SecurityConfig에서 ADMIN 권한 검증 적용됨
SecurityConfig.filterChain의adminURIList에/api/admin/**엔드포인트가hasRole(UserRole.ADMIN.name())로 보호되고 있어, 컨트롤러에 별도 인증/인가 어노테이션 추가 불필요합니다.
11-24: 컨트롤러 레벨 인증 어노테이션 생략 가능 글로벌 SecurityConfig에서/api/admin/**경로에 대해hasRole(ADMIN)으로 보호하므로 별도의@PreAuthorize어노테이션이 필요 없습니다.Likely an incorrect or invalid review comment.
Comment on lines
+7
to
+19
| @Component | ||
| @RequiredArgsConstructor | ||
| public class AdminTicketTypeRemover { | ||
| private final TicketTypeRepository ticketTypeRepository; | ||
|
|
||
| public void deleteAllTicketTypeByTicketRoundId(final long ticketRoundId) { | ||
| ticketTypeRepository.deleteAllByTicketRoundId(ticketRoundId); | ||
| } | ||
|
|
||
| public void deleteTicketTypeById(final long ticketTypeId) { | ||
| ticketTypeRepository.deleteById(ticketTypeId); | ||
| } | ||
| } |
There was a problem hiding this comment.
트랜잭션 관리 누락
삭제 작업에 @Transactional 어노테이션이 없습니다. 특히 deleteAllTicketTypeByTicketRoundId 메서드는 여러 행을 삭제할 수 있는 bulk 작업이므로, 트랜잭션 없이 실행될 경우 부분 삭제 후 오류 발생 시 롤백이 불가능합니다.
관련 파일인 AdminTicketTypeRetriever는 조회 작업에도 @Transactional(readOnly = true)를 사용하고 있어, 일관성을 위해서도 트랜잭션 관리가 필요합니다.
클래스 레벨에 트랜잭션을 추가하세요:
@Component
@RequiredArgsConstructor
+@Transactional
public class AdminTicketTypeRemover {
private final TicketTypeRepository ticketTypeRepository;📝 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.
Suggested change
| @Component | |
| @RequiredArgsConstructor | |
| public class AdminTicketTypeRemover { | |
| private final TicketTypeRepository ticketTypeRepository; | |
| public void deleteAllTicketTypeByTicketRoundId(final long ticketRoundId) { | |
| ticketTypeRepository.deleteAllByTicketRoundId(ticketRoundId); | |
| } | |
| public void deleteTicketTypeById(final long ticketTypeId) { | |
| ticketTypeRepository.deleteById(ticketTypeId); | |
| } | |
| } | |
| @Component | |
| @RequiredArgsConstructor | |
| @Transactional | |
| public class AdminTicketTypeRemover { | |
| private final TicketTypeRepository ticketTypeRepository; | |
| public void deleteAllTicketTypeByTicketRoundId(final long ticketRoundId) { | |
| ticketTypeRepository.deleteAllByTicketRoundId(ticketRoundId); | |
| } | |
| public void deleteTicketTypeById(final long ticketTypeId) { | |
| ticketTypeRepository.deleteById(ticketTypeId); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRemover.java
around lines 7-19, the deletion methods lack transactional boundaries; annotate
the class with @Transactional (from
org.springframework.transaction.annotation.Transactional) so both
deleteAllTicketTypeByTicketRoundId and deleteTicketTypeById execute within a
transaction (no readOnly), import the annotation, and ensure the class retains
@Component and @RequiredArgsConstructor so bulk deletes can be rolled back on
errors and remain consistent with AdminTicketTypeRetriever's readOnly usage.
Comment on lines
+12
to
+18
| public void deleteAllTicketTypeByTicketRoundId(final long ticketRoundId) { | ||
| ticketTypeRepository.deleteAllByTicketRoundId(ticketRoundId); | ||
| } | ||
|
|
||
| public void deleteTicketTypeById(final long ticketTypeId) { | ||
| ticketTypeRepository.deleteById(ticketTypeId); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
트랜잭션 어노테이션 추가 필요.
삭제 작업은 원자성을 보장하기 위해 트랜잭션 내에서 실행되어야 합니다. 특히 deleteAllTicketTypeByTicketRoundId는 여러 레코드를 삭제할 수 있어 부분 실패 시 데이터 무결성 문제가 발생할 수 있습니다.
다음 diff를 적용하여 트랜잭션 처리를 추가하세요:
+import org.springframework.transaction.annotation.Transactional;
+
@Component
@RequiredArgsConstructor
public class AdminTicketTypeRemover {
private final TicketTypeRepository ticketTypeRepository;
+ @Transactional
public void deleteAllTicketTypeByTicketRoundId(final long ticketRoundId) {
ticketTypeRepository.deleteAllByTicketRoundId(ticketRoundId);
}
+ @Transactional
public void deleteTicketTypeById(final long ticketTypeId) {
ticketTypeRepository.deleteById(ticketTypeId);
}
}🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/admin/tickettype/core/component/AdminTicketTypeRemover.java
around lines 12 to 18, the two delete methods lack transactional boundaries;
wrap the deletion operations in a Spring transaction to ensure atomicity and
automatic rollback on failure by annotating either the class or the two methods
with @Transactional (org.springframework.transaction.annotation.Transactional)
and import that annotation, using default propagation (REQUIRED) and allowing
rollback for runtime exceptions; ensure the annotation is applied to the public
methods that call ticketTypeRepository.deleteAllByTicketRoundId(...) and
ticketTypeRepository.deleteById(...).
Comment on lines
+37
to
+41
| final String uri = req.getRequestURI(); | ||
| if (!uri.startsWith("/actuator/health")) { | ||
| final String jsonLog = HttpReqResLogJsonBuilder.buildJsonLog(req, res, duration); | ||
| discordSender.send(jsonLog); | ||
| } |
There was a problem hiding this comment.
finally 블록에서 Discord 전송 예외가 원인 예외를 가릴 수 있음
finally 내부 전송 실패(RuntimeException 등) 시 원래 처리 예외를 덮어쓰거나 응답 복사가 스킵될 수 있습니다. 전송 로직을 try/catch로 감싸 예외를 격리하세요.
- if (!uri.startsWith("/actuator/health")) {
- final String jsonLog = HttpReqResLogJsonBuilder.buildJsonLog(req, res, duration);
- discordSender.send(jsonLog);
- }
+ if (!uri.startsWith("/actuator/health")) {
+ try {
+ final String jsonLog = HttpReqResLogJsonBuilder.buildJsonLog(req, res, duration);
+ discordSender.send(jsonLog);
+ } catch (Exception ex) {
+ log.warn("Failed to send dev req/res log to Discord. uri={}, error={}", uri, ex.toString());
+ }
+ }📝 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.
Suggested change
| final String uri = req.getRequestURI(); | |
| if (!uri.startsWith("/actuator/health")) { | |
| final String jsonLog = HttpReqResLogJsonBuilder.buildJsonLog(req, res, duration); | |
| discordSender.send(jsonLog); | |
| } | |
| final String uri = req.getRequestURI(); | |
| if (!uri.startsWith("/actuator/health")) { | |
| try { | |
| final String jsonLog = HttpReqResLogJsonBuilder.buildJsonLog(req, res, duration); | |
| discordSender.send(jsonLog); | |
| } catch (Exception ex) { | |
| log.warn("Failed to send dev req/res log to Discord. uri={}, error={}", uri, ex.toString()); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/global/filter/DevRequestResponseLoggingFilter.java
around lines 37 to 41, the discordSender.send call in the finally block can
throw and mask the original exception or prevent response copying; wrap the
discordSender.send(jsonLog) call in its own try/catch to isolate failures, log
the send error (including exception) without rethrowing, and ensure the finally
block always completes the response-copy/cleanup path regardless of discord send
failures.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항