Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 본 PR은 시스템 관리자가 결제 내역을 확인하고 필요시 결제를 취소할 수 있도록 하는 어드민 기능을 도입합니다. 결제 상태 및 기간별 필터링 기능을 포함하여 관리 효율성을 높였으며, 관련 DTO와 서비스 로직을 신규로 구성하여 기존 결제 시스템과 연동하였습니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. 결제 내역 한눈에 보고, 취소 요청도 빠르게 처리. 관리자의 손끝에서, 시스템은 더욱 견고해지네. Footnotes
|
|
Caution Review failedPull request was closed or merged during review Summary by CodeRabbit
Walkthrough관리자용 결제 관리 기능이 추가되었습니다. 결제 목록 조회(필터링·페이징, 선택적 memberId/state/startDate/endDate)와 결제 취소 API가 컨트롤러, 서비스, DTO, 리포지토리 및 통합 테스트와 함께 도입되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as AdminPaymentController
participant Service as AdminPaymentService
participant RepoP as TosspaymentsPaymentRepository
participant RepoR as TosspaymentsPaymentResultRepository
participant Facade as PaymentFacadeService
participant External as TosspaymentsClient
Client->>Controller: GET /api/v1/admin/payments?memberId=&state=&startDate=&endDate=&page...
Controller->>Service: findPayments(memberId,state,startDate,endDate,pageable)
Service->>RepoP: findPaymentsWithFilters(...)
RepoP-->>Service: Page<TosspaymentsPayment>
Service->>RepoR: findByTosspaymentsPaymentIdIn(paymentIds)
RepoR-->>Service: List<TosspaymentsPaymentResult>
Service-->>Controller: AdminPaymentPageResponse
Controller-->>Client: 200 + JSON
Client->>Controller: POST /api/v1/admin/payments/{paymentId}/cancel {cancelReason}
Controller->>Service: cancelPayment(paymentId, request)
Service->>RepoP: findById(paymentId)
RepoP-->>Service: TosspaymentsPayment
Service->>Facade: cancelPayment(cancelRequest)
Facade->>External: cancelPayment(paymentKey, ...)
External-->>Facade: TosspaymentsPaymentResponse
Facade-->>Service: (void)
Service-->>Controller: (void)
Controller-->>Client: 200
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 docstrings
🧪 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces administrative functionality for managing payments, including endpoints for listing all payments, filtering by member ID, and canceling specific payments. It includes new controller and service layers, along with necessary DTOs and repository extensions for filtered pagination. Feedback focuses on optimizing API response payloads by excluding large metadata fields from lists, improving query performance by replacing static JPQL with dynamic query tools like Querydsl, and reducing redundant database lookups during the payment cancellation process.
| String orderId, | ||
| String orderName, | ||
| Long totalAmount, | ||
| String metadata, |
There was a problem hiding this comment.
metadata 필드는 대용량 JSON 데이터를 포함할 수 있어, 목록 조회 API 응답에 매번 포함될 경우 네트워크 페이로드와 메모리 사용량에 부담을 줄 수 있습니다. 목록 조회 시에는 이 필드를 제외하고, 상세 정보가 필요한 경우에만 제공하는 방식을 권장합니다.
Comment on lines
+31
to
+37
| @Query(""" | ||
| SELECT p FROM TosspaymentsPayment p | ||
| WHERE (:memberId IS NULL OR p.memberId = :memberId) | ||
| AND (:state IS NULL OR p.state = :state) | ||
| AND (:startDate IS NULL OR p.createdAt >= :startDate) | ||
| AND (:endDate IS NULL OR p.createdAt <= :endDate) | ||
| """) |
There was a problem hiding this comment.
(:param IS NULL OR p.field = :param) 형태의 JPQL 쿼리는 데이터 양이 많아질 경우 인덱스를 효율적으로 활용하지 못해 성능 저하를 유발할 수 있습니다. 또한, 필터링과 정렬에 사용되는 state 및 createdAt 필드에 인덱스가 없다면 성능 문제가 더욱 심각해질 수 있습니다. Querydsl 등을 활용한 동적 쿼리 최적화와 해당 필드들에 대한 인덱스 추가를 검토해 주세요.
|
|
||
| public void cancelPayment(Long paymentId, AdminCancelPaymentRequest request) { | ||
| TosspaymentsPayment payment = readPaymentById(paymentId); | ||
| paymentFacadeService.cancelPayment(request.toCancelRequest(payment.getPaymentKey())); |
There was a problem hiding this comment.
AdminPaymentService에서 이미 엔티티를 조회했음에도 불구하고, 호출되는 paymentFacadeService.cancelPayment 내부에서 동일한 엔티티를 다시 조회하는 중복이 발생합니다. facade 메서드가 엔티티를 직접 인자로 받도록 개선하여 불필요한 데이터베이스 접근을 줄이는 것이 좋습니다.
Test Results 50 files 50 suites 1m 45s ⏱️ Results for commit 9697f04. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/samhap/kokomen/admin/controller/AdminPaymentController.java`:
- Around line 24-63: Add explicit ADMIN authorization to the admin endpoints:
annotate AdminPaymentController (or each method findPayments,
findPaymentsByMemberId, cancelPayment) with a method-security annotation like
`@PreAuthorize`("hasRole('ADMIN')") and ensure Spring Security is present in
build.gradle; enable method security in your security configuration (e.g., add
`@EnableGlobalMethodSecurity`(prePostEnabled = true) on SecurityConfig) and/or
configure the HttpSecurity to restrict the path /api/v1/admin/** to users with
role ADMIN. Also ensure authentication principal is available where needed by
accepting an Authentication or `@CurrentUser` parameter in service calls if
authorization decisions require user context.
In `@src/main/java/com/samhap/kokomen/admin/service/AdminPaymentService.java`:
- Around line 32-57: The service methods findPayments and findPaymentsByMemberId
currently allow inverted date ranges; add a validation at the start of each (or
extract a private validateDateRange(LocalDateTime start, LocalDateTime end) and
call it) that checks if startDate and endDate are both non-null and
startDate.isAfter(endDate), and if so throw a BadRequestException with a clear
message (e.g., "startDate must be before or equal to endDate"); this enforces
service-layer business validation before calling
tosspaymentsPaymentRepository.findPaymentsWithFilters and returning
toPageResponse.
In
`@src/test/java/com/samhap/kokomen/admin/controller/AdminPaymentControllerTest.java`:
- Around line 89-117: Duplicate responseFields definitions in
AdminPaymentControllerTest cause maintenance pain; extract the repeated
FieldDescriptor array into a single private static final FieldDescriptor[]
(e.g., PAYMENT_PAGE_RESPONSE_FIELDS) defined in the AdminPaymentControllerTest
class and replace both inline responseFields(...) usages in the 전체_결제목록_조회_API
and 유저별_결제목록_조회_API tests with responseFields(PAYMENT_PAGE_RESPONSE_FIELDS) so
both tests reference the shared constant.
- Around line 220-224: The test in AdminPaymentControllerTest only asserts
status.isOk(); extend it to verify the cancellation effect by (1) asserting the
response body contains cancellation info using mockMvc.andExpect(jsonPath(...))
for the expected fields (e.g., "status":"CANCELED" or cancellationTimestamp) and
(2) reloading the Payment from the DB via the PaymentRepository (or the
repository used in the test) using payment.getId() and asserting the entity's
status == CANCELED; update the test method that calls mockMvc.perform(post(...))
to include these additional assertions and any necessary repository
autowiring/cleanup.
In `@src/test/java/com/samhap/kokomen/admin/service/AdminPaymentServiceTest.java`:
- Around line 212-254: The helper method createCancelResponse duplicates test
fixtures present in AdminPaymentControllerTest; extract this into a shared
fixture builder or utility: create a TosspaymentsPaymentResponseFixtureBuilder
(or a common test util class) that can build TosspaymentsPaymentResponse
instances for cancellations, move the logic currently in createCancelResponse
into that builder (exposing a method like
buildCancelledResponse(TosspaymentsPayment) or withPayment(...)), and replace
usages of createCancelResponse in AdminPaymentServiceTest and
AdminPaymentControllerTest to use the new fixture builder to eliminate
duplication.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: d425e573-93a7-4017-8bd7-520bb3a83767
📒 Files selected for processing (10)
src/main/java/com/samhap/kokomen/admin/controller/AdminPaymentController.javasrc/main/java/com/samhap/kokomen/admin/service/AdminPaymentService.javasrc/main/java/com/samhap/kokomen/admin/service/dto/AdminCancelPaymentRequest.javasrc/main/java/com/samhap/kokomen/admin/service/dto/AdminPaymentPageResponse.javasrc/main/java/com/samhap/kokomen/admin/service/dto/AdminPaymentResponse.javasrc/main/java/com/samhap/kokomen/admin/service/dto/AdminPaymentResultResponse.javasrc/main/java/com/samhap/kokomen/payment/repository/TosspaymentsPaymentRepository.javasrc/main/java/com/samhap/kokomen/payment/repository/TosspaymentsPaymentResultRepository.javasrc/test/java/com/samhap/kokomen/admin/controller/AdminPaymentControllerTest.javasrc/test/java/com/samhap/kokomen/admin/service/AdminPaymentServiceTest.java
Comment on lines
+24
to
+63
| @RequiredArgsConstructor | ||
| @RequestMapping("/api/v1/admin/payments") | ||
| @RestController | ||
| public class AdminPaymentController { | ||
|
|
||
| private final AdminPaymentService adminPaymentService; | ||
|
|
||
| @GetMapping | ||
| public ResponseEntity<AdminPaymentPageResponse> findPayments( | ||
| @RequestParam(required = false) PaymentState state, | ||
| @RequestParam(required = false) @DateTimeFormat(iso = ISO.DATE_TIME) LocalDateTime startDate, | ||
| @RequestParam(required = false) @DateTimeFormat(iso = ISO.DATE_TIME) LocalDateTime endDate, | ||
| @PageableDefault(size = 10, sort = "createdAt", direction = Sort.Direction.DESC) Pageable pageable | ||
| ) { | ||
| AdminPaymentPageResponse response = adminPaymentService.findPayments(state, startDate, endDate, pageable); | ||
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @GetMapping("/members/{memberId}") | ||
| public ResponseEntity<AdminPaymentPageResponse> findPaymentsByMemberId( | ||
| @PathVariable Long memberId, | ||
| @RequestParam(required = false) PaymentState state, | ||
| @RequestParam(required = false) @DateTimeFormat(iso = ISO.DATE_TIME) LocalDateTime startDate, | ||
| @RequestParam(required = false) @DateTimeFormat(iso = ISO.DATE_TIME) LocalDateTime endDate, | ||
| @PageableDefault(size = 10, sort = "createdAt", direction = Sort.Direction.DESC) Pageable pageable | ||
| ) { | ||
| AdminPaymentPageResponse response = adminPaymentService.findPaymentsByMemberId( | ||
| memberId, state, startDate, endDate, pageable | ||
| ); | ||
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @PostMapping("/{paymentId}/cancel") | ||
| public ResponseEntity<Void> cancelPayment( | ||
| @PathVariable Long paymentId, | ||
| @RequestBody @Valid AdminCancelPaymentRequest request | ||
| ) { | ||
| adminPaymentService.cancelPayment(paymentId, request); | ||
| return ResponseEntity.ok().build(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) 보안 설정 클래스 후보 확인
fd -i 'security.*\.java' src/main/java
# 2) 관리자 경로 인가 규칙 확인
rg -n --type=java -C3 '/api/v1/admin|authorizeHttpRequests|requestMatchers|antMatchers|hasRole|hasAuthority' src/main/java
# 3) 컨트롤러 레벨 권한 애너테이션 확인
rg -n --type=java -C2 '@PreAuthorize|@Secured|@RolesAllowed' src/main/java/com/samhap/kokomen/admin/controller/AdminPaymentController.javaRepository: samhap-soft/kokomen-backend
Length of output: 1487
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find all security configuration classes
echo "=== Security Config Classes ==="
fd -e java 'Config|Security' src/main/java | grep -i security | head -20
echo ""
echo "=== All Java files in config directory ==="
fd -e java src/main/java/com/samhap/kokomen/config 2>/dev/null || echo "No config directory found"
echo ""
echo "=== WebSecurityConfiguration or similar ==="
find src/main/java -name '*Security*.java' -o -name '*Config*.java' | grep -i security | head -20Repository: samhap-soft/kokomen-backend
Length of output: 102
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 2) Search for authorization rules targeting /api/v1/admin
echo "=== Authorization rules for admin paths ==="
rg -n --type=java 'api/v1/admin|hasRole.*ADMIN|hasAuthority.*ADMIN' src/main/java | head -30
echo ""
echo "=== SecurityFilterChain or HttpSecurity configuration ==="
rg -n --type=java -B2 -A5 'SecurityFilterChain|HttpSecurity|authorizeRequests|authorizeHttpRequests' src/main/java | head -50Repository: samhap-soft/kokomen-backend
Length of output: 403
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 3) Check AdminPaymentController for any authorization annotations
echo "=== AdminPaymentController full content (first 100 lines) ==="
head -100 src/main/java/com/samhap/kokomen/admin/controller/AdminPaymentController.java
echo ""
echo "=== AdminController for comparison ==="
head -50 src/main/java/com/samhap/kokomen/admin/controller/AdminController.javaRepository: samhap-soft/kokomen-backend
Length of output: 4618
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) List all Java files in src/main/java to understand structure
echo "=== Directory structure ==="
find src/main/java -type d -name "*config*" -o -type d -name "*security*" | head -20
echo ""
echo "=== All Java files with 'Config' in name ==="
find src/main/java -name '*Config.java' | head -20
echo ""
echo "=== All Java files with 'Security' in name ==="
find src/main/java -name '*Security*.java' | head -20Repository: samhap-soft/kokomen-backend
Length of output: 703
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 2) Search for class names that indicate security configuration
echo "=== Search for SecurityConfiguration class definition ==="
rg -n --type=java 'class.*Security.*Config|class.*WebSecurity|class.*ConfigurationAdapter|@EnableWebSecurity' src/main/java
echo ""
echo "=== Search for any `@Bean` methods related to security ==="
rg -n --type=java -B2 '@Bean.*Security|SecurityFilterChain|AuthenticationManager|PasswordEncoder' src/main/java | head -50Repository: samhap-soft/kokomen-backend
Length of output: 128
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 3) Check if Spring Security is in dependencies and look for pom/gradle
echo "=== Check for Spring Security in pom.xml ==="
cat pom.xml 2>/dev/null | grep -i security || echo "pom.xml not found or no security dependency"
echo ""
echo "=== Check gradle files ==="
find . -name "build.gradle*" -exec grep -l security {} \; 2>/dev/null || echo "No gradle files with security"
echo ""
echo "=== Look for application.properties or application.yml ==="
find src/main/resources -name "application*" -exec echo "--- {} ---" \; -exec cat {} \; 2>/dev/null | head -100Repository: samhap-soft/kokomen-backend
Length of output: 2298
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 4) Check entire src/main directory structure
echo "=== Main source directories ==="
ls -la src/main/java/com/samhap/kokomen/
echo ""
echo "=== All Java files in entire codebase ==="
find src/main/java -name '*.java' -type f | wc -l
echo ""
echo "=== Look for any interceptor or filter implementations ==="
rg -n --type=java 'implements.*Filter|implements.*Interceptor|extends.*Filter|extends.*Interceptor' src/main/java | head -20Repository: samhap-soft/kokomen-backend
Length of output: 1328
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== WebConfig.java content ==="
cat src/main/java/com/samhap/kokomen/global/config/WebConfig.java
echo ""
echo "=== Check pom.xml for Spring Security dependency ==="
cat pom.xml | grep -A5 -B5 'security\|spring-boot-starter' || echo "pom.xml content not found"Repository: samhap-soft/kokomen-backend
Length of output: 1638
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Auth module structure ==="
find src/main/java/com/samhap/kokomen/auth -type f -name '*.java'
echo ""
echo "=== Check for JWT or token-based authentication ==="
rg -n --type=java 'jwt|token|authenticate|authorization|principal' src/main/java/com/samhap/kokomen/auth | head -30Repository: samhap-soft/kokomen-backend
Length of output: 2600
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== MemberAuthArgumentResolver.java ==="
cat src/main/java/com/samhap/kokomen/global/infrastructure/MemberAuthArgumentResolver.java
echo ""
echo "=== Search for build configuration files ==="
find . -maxdepth 2 -name "build.gradle*" -o -name "pom.xml" -o -name "settings.gradle"
echo ""
echo "=== Check if build.gradle exists ==="
ls -la | grep -E 'build|gradle|pom'Repository: samhap-soft/kokomen-backend
Length of output: 3236
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check for any Interceptor implementations ==="
find src/main/java -name '*Interceptor*.java' -exec cat {} \;
echo ""
echo "=== Search for `@MemberAuth` or custom security annotations ==="
rg -n --type=java '@MemberAuth|@AdminAuth|@RequireAdmin|@RequireRole' src/main/java | head -20Repository: samhap-soft/kokomen-backend
Length of output: 183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check AdminPaymentService for authorization logic ==="
cat src/main/java/com/samhap/kokomen/admin/service/AdminPaymentService.java 2>/dev/null | head -50 || echo "File not found or error"
echo ""
echo "=== Check for any validation in admin methods ==="
rg -n --type=java -B3 -A3 'admin|ADMIN|Admin' src/main/java/com/samhap/kokomen/admin/service/AdminPaymentService.java 2>/dev/null | head -40 || echo "Not found"Repository: samhap-soft/kokomen-backend
Length of output: 3785
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== build.gradle content ==="
cat build.gradle
echo ""
echo "=== Search for spring-security in gradle ==="
grep -i "security" build.gradle || echo "No security dependency found"Repository: samhap-soft/kokomen-backend
Length of output: 2862
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if AdminPaymentController methods have `@Authentication` ==="
rg -n --type=java -B2 -A10 '@GetMapping|@PostMapping' src/main/java/com/samhap/kokomen/admin/controller/AdminPaymentController.java
echo ""
echo "=== Check global exception handling for admin authorization ==="
rg -n --type=java 'admin|Admin|ADMIN' src/main/java/com/samhap/kokomen/global --type=java | grep -i 'exception\|interceptor\|filter' | head -20Repository: samhap-soft/kokomen-backend
Length of output: 2048
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== List all files in global directory ==="
find src/main/java/com/samhap/kokomen/global -type f -name '*.java' | sort
echo ""
echo "=== Check annotation definitions ==="
cat src/main/java/com/samhap/kokomen/global/annotation/Authentication.java 2>/dev/null || echo "Not found"Repository: samhap-soft/kokomen-backend
Length of output: 3509
🏁 Script executed:
#!/bin/bash
set -euo pipefixpipe
echo "=== Check if any other admin controllers have authorization ==="
find src/main/java/com/samhap/kokomen -name '*AdminController.java' -exec echo "--- {} ---" \; -exec grep -n '@Authentication\|@PreAuthorize\|@Secured' {} \;
echo ""
echo "=== Check MemberAuth class to see if it has role information ==="
cat src/main/java/com/samhap/kokomen/global/dto/MemberAuth.javaRepository: samhap-soft/kokomen-backend
Length of output: 127
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if any other admin controllers have authorization ==="
find src/main/java/com/samhap/kokomen -name '*AdminController.java' -exec echo "--- {} ---" \; -exec grep -n '@Authentication\|@PreAuthorize\|@Secured' {} \;
echo ""
echo "=== Check MemberAuth class to see if it has role information ==="
cat src/main/java/com/samhap/kokomen/global/dto/MemberAuth.javaRepository: samhap-soft/kokomen-backend
Length of output: 622
어드민 엔드포인트에 ADMIN 권한 검증이 완전히 누락되어 있습니다.
AdminPaymentController의 모든 메서드에서 권한 제어 메커니즘이 없습니다:
- Spring Security 의존성이 없음 (build.gradle 확인)
@PreAuthorize,@Secured,@RolesAllowed등 인가 애너테이션 부재- 메서드 호출 시
@Authentication파라미터 없음 (인증도 강제되지 않음) - MemberAuth는 memberId만 보유하며 역할 정보 없음
- 전역 보안 필터나 인터셉터로 /api/v1/admin/** 보호 안 됨
현재 상태에서는 /api/v1/admin/payments 경로가 인증되지 않은 사용자 또는 일반 사용자도 접근할 수 있습니다. 민감한 관리자 기능(결제 조회, 취소)을 보호하기 위해 ADMIN 권한 검증이 필수입니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/samhap/kokomen/admin/controller/AdminPaymentController.java`
around lines 24 - 63, Add explicit ADMIN authorization to the admin endpoints:
annotate AdminPaymentController (or each method findPayments,
findPaymentsByMemberId, cancelPayment) with a method-security annotation like
`@PreAuthorize`("hasRole('ADMIN')") and ensure Spring Security is present in
build.gradle; enable method security in your security configuration (e.g., add
`@EnableGlobalMethodSecurity`(prePostEnabled = true) on SecurityConfig) and/or
configure the HttpSecurity to restrict the path /api/v1/admin/** to users with
role ADMIN. Also ensure authentication principal is available where needed by
accepting an Authentication or `@CurrentUser` parameter in service calls if
authorization decisions require user context.
Comment on lines
+32
to
+57
| @Transactional(readOnly = true) | ||
| public AdminPaymentPageResponse findPayments( | ||
| PaymentState state, | ||
| LocalDateTime startDate, | ||
| LocalDateTime endDate, | ||
| Pageable pageable | ||
| ) { | ||
| Page<TosspaymentsPayment> page = tosspaymentsPaymentRepository.findPaymentsWithFilters( | ||
| null, state, startDate, endDate, pageable | ||
| ); | ||
| return toPageResponse(page); | ||
| } | ||
|
|
||
| @Transactional(readOnly = true) | ||
| public AdminPaymentPageResponse findPaymentsByMemberId( | ||
| Long memberId, | ||
| PaymentState state, | ||
| LocalDateTime startDate, | ||
| LocalDateTime endDate, | ||
| Pageable pageable | ||
| ) { | ||
| Page<TosspaymentsPayment> page = tosspaymentsPaymentRepository.findPaymentsWithFilters( | ||
| memberId, state, startDate, endDate, pageable | ||
| ); | ||
| return toPageResponse(page); | ||
| } |
There was a problem hiding this comment.
날짜 범위 역전 입력에 대한 서비스 레벨 검증이 필요합니다.
startDate가 endDate보다 이후인 경우를 현재 허용하고 있어, 잘못된 요청이 정상 응답(빈 목록)으로 처리됩니다. 서비스에서 즉시 BadRequestException으로 차단해 주세요.
🔧 제안 수정안
import com.samhap.kokomen.admin.service.dto.AdminCancelPaymentRequest;
import com.samhap.kokomen.admin.service.dto.AdminPaymentPageResponse;
import com.samhap.kokomen.admin.service.dto.AdminPaymentResponse;
+import com.samhap.kokomen.global.exception.BadRequestException;
import com.samhap.kokomen.global.exception.NotFoundException;
@@
public AdminPaymentPageResponse findPayments(
PaymentState state,
LocalDateTime startDate,
LocalDateTime endDate,
Pageable pageable
) {
+ validateDateRange(startDate, endDate);
Page<TosspaymentsPayment> page = tosspaymentsPaymentRepository.findPaymentsWithFilters(
null, state, startDate, endDate, pageable
);
return toPageResponse(page);
}
@@
public AdminPaymentPageResponse findPaymentsByMemberId(
Long memberId,
PaymentState state,
LocalDateTime startDate,
LocalDateTime endDate,
Pageable pageable
) {
+ validateDateRange(startDate, endDate);
Page<TosspaymentsPayment> page = tosspaymentsPaymentRepository.findPaymentsWithFilters(
memberId, state, startDate, endDate, pageable
);
return toPageResponse(page);
}
+
+ private void validateDateRange(LocalDateTime startDate, LocalDateTime endDate) {
+ if (startDate != null && endDate != null && startDate.isAfter(endDate)) {
+ throw new BadRequestException("startDate는 endDate보다 이후일 수 없습니다.");
+ }
+ }As per coding guidelines "Use @Valid annotation in DTOs for validation, entity-level validation in constructors, business validation in service layer".
📝 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
| @Transactional(readOnly = true) | |
| public AdminPaymentPageResponse findPayments( | |
| PaymentState state, | |
| LocalDateTime startDate, | |
| LocalDateTime endDate, | |
| Pageable pageable | |
| ) { | |
| Page<TosspaymentsPayment> page = tosspaymentsPaymentRepository.findPaymentsWithFilters( | |
| null, state, startDate, endDate, pageable | |
| ); | |
| return toPageResponse(page); | |
| } | |
| @Transactional(readOnly = true) | |
| public AdminPaymentPageResponse findPaymentsByMemberId( | |
| Long memberId, | |
| PaymentState state, | |
| LocalDateTime startDate, | |
| LocalDateTime endDate, | |
| Pageable pageable | |
| ) { | |
| Page<TosspaymentsPayment> page = tosspaymentsPaymentRepository.findPaymentsWithFilters( | |
| memberId, state, startDate, endDate, pageable | |
| ); | |
| return toPageResponse(page); | |
| } | |
| `@Transactional`(readOnly = true) | |
| public AdminPaymentPageResponse findPayments( | |
| PaymentState state, | |
| LocalDateTime startDate, | |
| LocalDateTime endDate, | |
| Pageable pageable | |
| ) { | |
| validateDateRange(startDate, endDate); | |
| Page<TosspaymentsPayment> page = tosspaymentsPaymentRepository.findPaymentsWithFilters( | |
| null, state, startDate, endDate, pageable | |
| ); | |
| return toPageResponse(page); | |
| } | |
| `@Transactional`(readOnly = true) | |
| public AdminPaymentPageResponse findPaymentsByMemberId( | |
| Long memberId, | |
| PaymentState state, | |
| LocalDateTime startDate, | |
| LocalDateTime endDate, | |
| Pageable pageable | |
| ) { | |
| validateDateRange(startDate, endDate); | |
| Page<TosspaymentsPayment> page = tosspaymentsPaymentRepository.findPaymentsWithFilters( | |
| memberId, state, startDate, endDate, pageable | |
| ); | |
| return toPageResponse(page); | |
| } | |
| private void validateDateRange(LocalDateTime startDate, LocalDateTime endDate) { | |
| if (startDate != null && endDate != null && startDate.isAfter(endDate)) { | |
| throw new BadRequestException("startDate는 endDate보다 이후일 수 없습니다."); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/samhap/kokomen/admin/service/AdminPaymentService.java`
around lines 32 - 57, The service methods findPayments and
findPaymentsByMemberId currently allow inverted date ranges; add a validation at
the start of each (or extract a private validateDateRange(LocalDateTime start,
LocalDateTime end) and call it) that checks if startDate and endDate are both
non-null and startDate.isAfter(endDate), and if so throw a BadRequestException
with a clear message (e.g., "startDate must be before or equal to endDate");
this enforces service-layer business validation before calling
tosspaymentsPaymentRepository.findPaymentsWithFilters and returning
toPageResponse.
Comment on lines
+89
to
+117
| responseFields( | ||
| fieldWithPath("data").type(JsonFieldType.ARRAY).description("결제 목록"), | ||
| fieldWithPath("data[].id").type(JsonFieldType.NUMBER).description("결제 ID"), | ||
| fieldWithPath("data[].payment_key").type(JsonFieldType.STRING).description("결제 키"), | ||
| fieldWithPath("data[].member_id").type(JsonFieldType.NUMBER).description("회원 ID"), | ||
| fieldWithPath("data[].order_id").type(JsonFieldType.STRING).description("주문 ID"), | ||
| fieldWithPath("data[].order_name").type(JsonFieldType.STRING).description("주문명"), | ||
| fieldWithPath("data[].total_amount").type(JsonFieldType.NUMBER).description("결제 금액"), | ||
| fieldWithPath("data[].metadata").type(JsonFieldType.STRING).description("메타데이터 (JSON)"), | ||
| fieldWithPath("data[].state").type(JsonFieldType.STRING).description("결제 상태"), | ||
| fieldWithPath("data[].service_type").type(JsonFieldType.STRING).description("서비스 타입"), | ||
| fieldWithPath("data[].created_at").type(JsonFieldType.STRING).description("생성일시"), | ||
| fieldWithPath("data[].updated_at").type(JsonFieldType.STRING).description("수정일시"), | ||
| fieldWithPath("data[].result").type(JsonFieldType.OBJECT).description("결제 결과 상세").optional(), | ||
| fieldWithPath("data[].result.method").type(JsonFieldType.STRING).description("결제 수단").optional(), | ||
| fieldWithPath("data[].result.balance_amount").type(JsonFieldType.NUMBER).description("잔액").optional(), | ||
| fieldWithPath("data[].result.tosspayments_status").type(JsonFieldType.STRING).description("토스페이먼츠 상태").optional(), | ||
| fieldWithPath("data[].result.requested_at").type(JsonFieldType.STRING).description("요청일시").optional(), | ||
| fieldWithPath("data[].result.approved_at").type(JsonFieldType.STRING).description("승인일시").optional(), | ||
| fieldWithPath("data[].result.cancel_reason").type(JsonFieldType.STRING).description("취소 사유").optional(), | ||
| fieldWithPath("data[].result.canceled_at").type(JsonFieldType.STRING).description("취소일시").optional(), | ||
| fieldWithPath("data[].result.cancel_status").type(JsonFieldType.STRING).description("취소 상태").optional(), | ||
| fieldWithPath("data[].result.receipt_url").type(JsonFieldType.STRING).description("영수증 URL").optional(), | ||
| fieldWithPath("data[].result.easy_pay_provider").type(JsonFieldType.STRING).description("간편결제 제공자").optional(), | ||
| fieldWithPath("current_page").type(JsonFieldType.NUMBER).description("현재 페이지"), | ||
| fieldWithPath("total_count").type(JsonFieldType.NUMBER).description("전체 건수"), | ||
| fieldWithPath("total_pages").type(JsonFieldType.NUMBER).description("전체 페이지 수"), | ||
| fieldWithPath("has_next").type(JsonFieldType.BOOLEAN).description("다음 페이지 존재 여부") | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
RestDocs 응답 필드 정의가 중복됩니다.
전체_결제목록_조회_API와 유저별_결제목록_조회_API의 responseFields 정의가 완전히 동일합니다(Lines 89-117, 160-188). 응답 구조 변경 시 두 곳을 수정해야 하므로 유지보수가 어려워집니다.
♻️ 공통 응답 필드를 상수로 추출하는 예시
private static final FieldDescriptor[] PAYMENT_PAGE_RESPONSE_FIELDS = {
fieldWithPath("data").type(JsonFieldType.ARRAY).description("결제 목록"),
fieldWithPath("data[].id").type(JsonFieldType.NUMBER).description("결제 ID"),
// ... 나머지 필드들
fieldWithPath("has_next").type(JsonFieldType.BOOLEAN).description("다음 페이지 존재 여부")
};
// 테스트에서 사용
.andDo(document("admin-findPayments",
queryParameters(...),
responseFields(PAYMENT_PAGE_RESPONSE_FIELDS)
));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/samhap/kokomen/admin/controller/AdminPaymentControllerTest.java`
around lines 89 - 117, Duplicate responseFields definitions in
AdminPaymentControllerTest cause maintenance pain; extract the repeated
FieldDescriptor array into a single private static final FieldDescriptor[]
(e.g., PAYMENT_PAGE_RESPONSE_FIELDS) defined in the AdminPaymentControllerTest
class and replace both inline responseFields(...) usages in the 전체_결제목록_조회_API
and 유저별_결제목록_조회_API tests with responseFields(PAYMENT_PAGE_RESPONSE_FIELDS) so
both tests reference the shared constant.
Comment on lines
+220
to
+224
| // when & then | ||
| mockMvc.perform(post("/api/v1/admin/payments/{paymentId}/cancel", payment.getId()) | ||
| .contentType(MediaType.APPLICATION_JSON) | ||
| .content(objectMapper.writeValueAsString(request))) | ||
| .andExpect(status().isOk()) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
취소 결과에 대한 검증이 부족합니다.
현재 테스트는 status().isOk()만 확인하고 있습니다. 취소 API 호출 후 DB에서 결제 상태가 CANCELED로 변경되었는지, 또는 응답 본문에 취소 정보가 포함되어 있는지 추가 검증을 고려해 주세요.
🧪 상태 변경 검증 추가 예시
// when & then
mockMvc.perform(post("/api/v1/admin/payments/{paymentId}/cancel", payment.getId())
.contentType(MediaType.APPLICATION_JSON)
.content(objectMapper.writeValueAsString(request)))
.andExpect(status().isOk())
.andDo(document("admin-cancelPayment",
...
));
+
+ // verify state change
+ TosspaymentsPayment canceledPayment = tosspaymentsPaymentRepository.findById(payment.getId()).orElseThrow();
+ assertThat(canceledPayment.getState()).isEqualTo(PaymentState.CANCELED);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/test/java/com/samhap/kokomen/admin/controller/AdminPaymentControllerTest.java`
around lines 220 - 224, The test in AdminPaymentControllerTest only asserts
status.isOk(); extend it to verify the cancellation effect by (1) asserting the
response body contains cancellation info using mockMvc.andExpect(jsonPath(...))
for the expected fields (e.g., "status":"CANCELED" or cancellationTimestamp) and
(2) reloading the Payment from the DB via the PaymentRepository (or the
repository used in the test) using payment.getId() and asserting the entity's
status == CANCELED; update the test method that calls mockMvc.perform(post(...))
to include these additional assertions and any necessary repository
autowiring/cleanup.
Comment on lines
+212
to
+254
| private TosspaymentsPaymentResponse createCancelResponse(TosspaymentsPayment payment) { | ||
| return new TosspaymentsPaymentResponse( | ||
| payment.getPaymentKey(), | ||
| PaymentType.NORMAL, | ||
| payment.getOrderId(), | ||
| payment.getOrderName(), | ||
| "mId", | ||
| "KRW", | ||
| "카드", | ||
| payment.getTotalAmount(), | ||
| 0L, | ||
| TosspaymentsStatus.CANCELED, | ||
| LocalDateTime.now(), | ||
| LocalDateTime.now(), | ||
| "transaction_key", | ||
| 9091L, | ||
| 909L, | ||
| 0L, | ||
| 0L, | ||
| true, | ||
| null, | ||
| null, | ||
| null, | ||
| null, | ||
| "KR", | ||
| null, | ||
| Collections.singletonList( | ||
| new TosspaymentsCancel( | ||
| "cancel_transaction_key", | ||
| "테스트 환불", | ||
| 0L, | ||
| LocalDateTime.now(), | ||
| 0L, | ||
| "receipt_key", | ||
| payment.getTotalAmount(), | ||
| 0L, | ||
| 0L, | ||
| "DONE", | ||
| null | ||
| ) | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
createCancelResponse 헬퍼가 컨트롤러 테스트와 중복됩니다.
AdminPaymentControllerTest.java에도 동일한 헬퍼 메서드가 존재합니다. TosspaymentsPaymentFixtureBuilder와 같은 방식으로 취소 응답용 픽스처 빌더(TosspaymentsPaymentResponseFixtureBuilder)를 생성하거나, 공통 테스트 유틸리티 클래스로 추출하는 것을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/test/java/com/samhap/kokomen/admin/service/AdminPaymentServiceTest.java`
around lines 212 - 254, The helper method createCancelResponse duplicates test
fixtures present in AdminPaymentControllerTest; extract this into a shared
fixture builder or utility: create a TosspaymentsPaymentResponseFixtureBuilder
(or a common test util class) that can build TosspaymentsPaymentResponse
instances for cancellations, move the logic currently in createCancelResponse
into that builder (exposing a method like
buildCancelledResponse(TosspaymentsPayment) or withPayment(...)), and replace
usages of createCancelResponse in AdminPaymentServiceTest and
AdminPaymentControllerTest to use the new fixture builder to eliminate
duplication.
unifolio0
added a commit
that referenced
this pull request
Jun 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closed #349
작업 내용
스크린샷
참고 사항