Merged
Conversation
- 브랜드, 카테고리, 상품, 상품옵션 수정 기능 추가 - 브랜드, 카테고리, 상품, 상품옵션 삭제 기능 추가 - 각 엔티티에 updateXXX 메서드 추가 - ProductOptionRepository에 deleteByProduct 메서드 추가 - Admin API 엔드포인트 추가 (PATCH, DELETE) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- 주문 이력 보존을 위해 상품 물리 삭제 기능 제거 - 대신 상품 수정 API를 통해 onSale=false로 판매 중단 처리 가능 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
Walkthrough관리자용 브랜드·카테고리·상품·상품 옵션의 PATCH(수정) 및 DELETE(삭제) 엔드포인트와 이를 처리하는 서비스/엔티티/저장소 메서드 및 관련 UpdateRequest DTO들이 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as AdminController
participant Service as AdminService
participant Repo as Repository/DB
Client->>Controller: PATCH /admin/brands/{id} + AdminBrandUpdateRequest
Controller->>Service: updateBrand(id, request)
Service->>Repo: find Brand by id
Repo-->>Service: Brand
Service->>Service: brand.updateBrand(...)
Service->>Repo: save/update Brand
Repo-->>Service: updated Brand
Service-->>Controller: BrandResponse
Controller-->>Client: 200 OK (DataResponse<BrandResponse>)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In
`@src/main/java/com/ongil/backend/domain/admin/controller/AdminController.java`:
- Around line 77-82: The delete path currently risks a DB FK violation when
removing Brand/Category; update the deletion flow to perform a pre-check in
adminService.deleteBrand (and the analogous category delete method) to query for
existing Product(s) (by brandId/categoryId) and/or subCategories before
deleting, and if any exist throw a ValidationException (or your app's
DomainValidationException) with message "연관된 상품/카테고리가 있어 삭제할 수 없습니다";
alternatively, if you prefer cascade deletes, add cascade = CascadeType.REMOVE
(or CascadeType.DELETE) on the Brand/Category `@OneToMany` mappings to Product and
subCategories respectively, or implement a GlobalExceptionHandler method that
handles DataIntegrityViolationException and returns the same friendly
message—choose one approach and apply consistently to Brand, Category, Product
entity mappings and adminService.deleteBrand/deleteCategory call sites.
In
`@src/main/java/com/ongil/backend/domain/admin/dto/request/AdminProductUpdateRequest.java`:
- Around line 9-45: The AdminProductUpdateRequest DTO is missing the onSale
field required to toggle product sale status; add a private Boolean onSale field
to the AdminProductUpdateRequest class with an appropriate `@Schema`(description =
"판매 여부", example = "false") annotation so the API can accept onSale updates (the
class already uses `@Getter/`@NoArgsConstructor so no extra accessor code is
needed).
In `@src/main/java/com/ongil/backend/domain/admin/service/AdminService.java`:
- Around line 186-223: The updateProduct method is not passing the onSale flag
from AdminProductUpdateRequest into product.updateProduct; add an onSale Boolean
to AdminProductUpdateRequest (with getter) if missing, update the call site in
AdminService.updateProduct to pass request.getOnSale() into
product.updateProduct, and ensure the Product.updateProduct signature (and any
usages) is updated to accept and apply the onSale value so the Product's onSale
state is persisted; keep other validations unchanged and return
productConverter.toSimpleResponse(product) as before.
- Around line 162-166: The current logic in AdminService (handling
request.getParentCategoryId()) treats a null parentCategoryId as "do not change"
because Category.updateCategory only updates when parentCategory != null; to
allow explicit removal of a parent, add a clear signal in the request (e.g.,
treat parentCategoryId == 0L or add a boolean like clearParent) and branch: if
request.getParentCategoryId() == 0L (or clearParent==true) set parentCategory =
null and mark that parent should be cleared, else if parentCategoryId != null
load parent via categoryRepository.findById(...); then call
Category.updateCategory with a parameter or overload that can distinguish
"no-op", "set to X", and "clear parent" (or pass an Optional/flag) so
updateCategory can explicitly remove the parent when requested.
In `@src/main/java/com/ongil/backend/domain/category/entity/Category.java`:
- Around line 48-63: The updateCategory method in Category allows setting
parentCategory only when non-null and lacks self-reference protection; change
updateCategory in class Category so it accepts null to unset the parent
(allowing promotion to top-level) and explicitly prevent cycles by rejecting
when parentCategory == this (e.g., throw IllegalArgumentException); keep the
other field updates unchanged and reference updateCategory and parentCategory
when making the fix.
In `@src/main/java/com/ongil/backend/domain/product/entity/Product.java`:
- Around line 141-187: Add an onSale boolean to the admin update flow: extend
AdminProductUpdateRequest with a nullable Boolean onSale, add a corresponding
Boolean onSale parameter to Product.updateProduct(...) and, inside
updateProduct, if (onSale != null) set this.onSale = onSale; then update
AdminService.updateProduct (and any callers) to pass the DTO.getOnSale() into
Product.updateProduct; ensure the symbol names referenced are
AdminProductUpdateRequest, Product.updateProduct, and AdminService.updateProduct
so the new onSale value flows from the DTO to the entity.
In `@src/main/java/com/ongil/backend/domain/product/entity/ProductOption.java`:
- Around line 48-59: The updateProductOption method allows negative stock values
which can violate domain invariants and mislead getStockStatus; change
updateProductOption in ProductOption to validate the incoming stock (e.g.,
require stock >= 0) and either clamp negative values to 0 or throw an
IllegalArgumentException when stock < 0, and also update getStockStatus (if
present) to treat any stock <= 0 as OUT_OF_STOCK to be defensive against
negative values.
🧹 Nitpick comments (6)
src/main/java/com/ongil/backend/domain/product/repository/ProductOptionRepository.java (1)
28-29: 파생 삭제 쿼리(derived delete)는 N+1 DELETE 문을 발생시킵니다Spring Data JPA의
deleteByProduct는 내부적으로 먼저 SELECT로 엔티티를 모두 조회한 후, 건별로 DELETE를 실행합니다. 옵션이 많은 상품의 경우 성능 이슈가 발생할 수 있습니다. 벌크 삭제 쿼리 사용을 권장합니다.♻️ 벌크 삭제 쿼리 제안
- // 특정 상품의 모든 옵션 삭제 - void deleteByProduct(Product product); + // 특정 상품의 모든 옵션 삭제 + `@Modifying` + `@Query`("DELETE FROM ProductOption po WHERE po.product = :product") + void deleteByProduct(`@Param`("product") Product product);
@Modifying과@Queryimport가 필요합니다:import org.springframework.data.jpa.repository.Modifying; import org.springframework.data.jpa.repository.Query; import org.springframework.data.repository.query.Param;src/main/java/com/ongil/backend/domain/admin/dto/request/AdminProductUpdateRequest.java (1)
20-21:price에 대한 음수/0 값 검증 부재관리자 실수로 음수 또는 0 가격이 저장될 수 있습니다.
@Positive등 Bean Validation 어노테이션 추가를 권장합니다.discountRate(Line 36)도 0~100 범위 검증이 필요합니다.🛡️ 검증 어노테이션 추가 제안
+import jakarta.validation.constraints.Max; +import jakarta.validation.constraints.Min; +import jakarta.validation.constraints.Positive; + `@Schema`(description = "가격", example = "29000") + `@Positive` private Integer price; ... `@Schema`(description = "할인율 (%)", example = "10") + `@Min`(0) `@Max`(100) private Integer discountRate;src/main/java/com/ongil/backend/domain/product/entity/Product.java (1)
150-156:price와discountRate가 동시에 전달될 때 할인가가 두 번 계산됩니다.
price블록(Line 150-155)에서 기존discountRate로discountPrice를 먼저 계산하고, 이후discountRate블록(Line 169-176)에서 다시 계산합니다. 최종 결과는 올바르지만 불필요한 중간 계산이 발생합니다.할인가 재계산 로직을 메서드 하단에 한 번만 수행하도록 분리하면 가독성과 유지보수성이 향상됩니다.
Also applies to: 169-177
src/main/java/com/ongil/backend/domain/admin/controller/AdminController.java (1)
70-72: PATCH 엔드포인트에@Valid어노테이션이 누락되어 있습니다.POST 엔드포인트(Line 41, 48, 55, 63)에는
@Valid가 적용되어 있으나 PATCH 엔드포인트에는 빠져 있습니다. 현재 update DTO에 validation 제약 조건이 없어 동작에 영향은 없지만, 향후 DTO에 제약 조건이 추가될 경우를 대비해 일관성 있게@Valid를 붙여 두는 것이 좋습니다.public DataResponse<BrandResponse> updateBrand( `@PathVariable` Long brandId, - `@RequestBody` AdminBrandUpdateRequest request) { + `@Valid` `@RequestBody` AdminBrandUpdateRequest request) {Also applies to: 86-88, 102-104, 111-113
src/main/java/com/ongil/backend/domain/admin/service/AdminService.java (2)
225-243:ProductOptionResponse수동 빌드가createProductOption과 중복됩니다 (DRY 위반).
updateProductOption(Line 236-242)과createProductOption(Line 126-132)에서 동일한ProductOptionResponse빌드 로직이 반복됩니다. 다른 엔티티는Converter를 사용하고 있으므로,ProductConverter에toProductOptionResponse메서드를 추가하여 일관성을 맞추는 것이 좋습니다.Also applies to: 114-133
135-147: 읽기 전용 조회에 대한@Transactional(readOnly = true)미분리.클래스 레벨에
@Transactional이 적용되어 있어 모든 메서드가 쓰기 트랜잭션으로 동작합니다. 현재는 모두 쓰기 연산이므로 문제 없지만, 향후 조회 메서드가 추가될 경우 성능에 영향을 줄 수 있으니 참고해 주세요.Also applies to: 226-243
src/main/java/com/ongil/backend/domain/admin/controller/AdminController.java
Show resolved
Hide resolved
Comment on lines
+9
to
+45
| @Getter | ||
| @NoArgsConstructor | ||
| @Schema(description = "상품 수정 요청") | ||
| public class AdminProductUpdateRequest { | ||
|
|
||
| @Schema(description = "상품명", example = "베이직 코튼 티셔츠") | ||
| private String name; | ||
|
|
||
| @Schema(description = "상품 설명", example = "편안한 착용감의 코튼 티셔츠입니다.") | ||
| private String description; | ||
|
|
||
| @Schema(description = "가격", example = "29000") | ||
| private Integer price; | ||
|
|
||
| @Schema(description = "소재 정보", example = "면 100%") | ||
| private String materialOriginal; | ||
|
|
||
| @Schema(description = "이미지 URL들 (쉼표로 구분)", example = "https://example.com/img1.jpg,https://example.com/img2.jpg") | ||
| private String imageUrls; | ||
|
|
||
| @Schema(description = "사이즈들 (쉼표로 구분)", example = "S,M,L,XL") | ||
| private String sizes; | ||
|
|
||
| @Schema(description = "색상들 (쉼표로 구분)", example = "화이트,블랙,네이비") | ||
| private String colors; | ||
|
|
||
| @Schema(description = "할인율 (%)", example = "10") | ||
| private Integer discountRate; | ||
|
|
||
| @Schema(description = "상품 타입", example = "NORMAL") | ||
| private ProductType productType; | ||
|
|
||
| @Schema(description = "브랜드 ID", example = "1") | ||
| private Long brandId; | ||
|
|
||
| @Schema(description = "카테고리 ID", example = "1") | ||
| private Long categoryId; |
Contributor
There was a problem hiding this comment.
onSale 필드 누락 — PR 목적과 불일치
커밋 메시지에 "주문 이력 보존을 위해 상품의 물리 삭제 기능 제거. 상품 판매 중단은 상품 수정 API를 통해 onSale=false로 처리하도록 변경"이라고 명시되어 있으나, 이 DTO에 onSale (또는 Boolean 타입의 판매 상태) 필드가 없습니다. 현재 상태로는 상품 판매 중단 기능이 동작하지 않습니다.
🐛 onSale 필드 추가 제안
`@Schema`(description = "카테고리 ID", example = "1")
private Long categoryId;
+
+ `@Schema`(description = "판매 여부", example = "true")
+ private Boolean onSale;
}As per coding guidelines, DTO 설계 검증: "누락된 필드 - API 응답에 필요한 필드가 빠져있는지 확인".
📝 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
| @Getter | |
| @NoArgsConstructor | |
| @Schema(description = "상품 수정 요청") | |
| public class AdminProductUpdateRequest { | |
| @Schema(description = "상품명", example = "베이직 코튼 티셔츠") | |
| private String name; | |
| @Schema(description = "상품 설명", example = "편안한 착용감의 코튼 티셔츠입니다.") | |
| private String description; | |
| @Schema(description = "가격", example = "29000") | |
| private Integer price; | |
| @Schema(description = "소재 정보", example = "면 100%") | |
| private String materialOriginal; | |
| @Schema(description = "이미지 URL들 (쉼표로 구분)", example = "https://example.com/img1.jpg,https://example.com/img2.jpg") | |
| private String imageUrls; | |
| @Schema(description = "사이즈들 (쉼표로 구분)", example = "S,M,L,XL") | |
| private String sizes; | |
| @Schema(description = "색상들 (쉼표로 구분)", example = "화이트,블랙,네이비") | |
| private String colors; | |
| @Schema(description = "할인율 (%)", example = "10") | |
| private Integer discountRate; | |
| @Schema(description = "상품 타입", example = "NORMAL") | |
| private ProductType productType; | |
| @Schema(description = "브랜드 ID", example = "1") | |
| private Long brandId; | |
| @Schema(description = "카테고리 ID", example = "1") | |
| private Long categoryId; | |
| `@Getter` | |
| `@NoArgsConstructor` | |
| `@Schema`(description = "상품 수정 요청") | |
| public class AdminProductUpdateRequest { | |
| `@Schema`(description = "상품명", example = "베이직 코튼 티셔츠") | |
| private String name; | |
| `@Schema`(description = "상품 설명", example = "편안한 착용감의 코튼 티셔츠입니다.") | |
| private String description; | |
| `@Schema`(description = "가격", example = "29000") | |
| private Integer price; | |
| `@Schema`(description = "소재 정보", example = "면 100%") | |
| private String materialOriginal; | |
| `@Schema`(description = "이미지 URL들 (쉼표로 구분)", example = "https://example.com/img1.jpg,https://example.com/img2.jpg") | |
| private String imageUrls; | |
| `@Schema`(description = "사이즈들 (쉼표로 구분)", example = "S,M,L,XL") | |
| private String sizes; | |
| `@Schema`(description = "색상들 (쉼표로 구분)", example = "화이트,블랙,네이비") | |
| private String colors; | |
| `@Schema`(description = "할인율 (%)", example = "10") | |
| private Integer discountRate; | |
| `@Schema`(description = "상품 타입", example = "NORMAL") | |
| private ProductType productType; | |
| `@Schema`(description = "브랜드 ID", example = "1") | |
| private Long brandId; | |
| `@Schema`(description = "카테고리 ID", example = "1") | |
| private Long categoryId; | |
| `@Schema`(description = "판매 여부", example = "true") | |
| private Boolean onSale; | |
| } |
🤖 Prompt for AI Agents
In
`@src/main/java/com/ongil/backend/domain/admin/dto/request/AdminProductUpdateRequest.java`
around lines 9 - 45, The AdminProductUpdateRequest DTO is missing the onSale
field required to toggle product sale status; add a private Boolean onSale field
to the AdminProductUpdateRequest class with an appropriate `@Schema`(description =
"판매 여부", example = "false") annotation so the API can accept onSale updates (the
class already uses `@Getter/`@NoArgsConstructor so no extra accessor code is
needed).
Comment on lines
+162
to
+166
| Category parentCategory = null; | ||
| if (request.getParentCategoryId() != null) { | ||
| parentCategory = categoryRepository.findById(request.getParentCategoryId()) | ||
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.CATEGORY_NOT_FOUND)); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# AdminService.java의 updateCategory 메서드 전체 확인
fd "AdminService.java" --type f -x cat {}Repository: IT-Cotato/12th-OnGil-BE
Length of output: 9362
🏁 Script executed:
# Category.java 찾기 및 updateCategory 메서드 확인
fd "Category.java" --type f -x cat {}Repository: IT-Cotato/12th-OnGil-BE
Length of output: 1924
🏁 Script executed:
# ProductOptionResponse 확인 - converter 패턴 사용 여부
rg "ProductOptionResponse" --type java -A 5 | head -60Repository: IT-Cotato/12th-OnGil-BE
Length of output: 6463
🏁 Script executed:
# ProductConverter 전체 내용 확인
fd "ProductConverter.java" --type f -x cat {}Repository: IT-Cotato/12th-OnGil-BE
Length of output: 4821
parentCategoryId가 null일 때 상위 카테고리를 해제할 수 없습니다.
Category.updateCategory 메서드가 if (parentCategory != null) 조건으로만 업데이트하므로, 요청의 parentCategoryId가 null이면 현재 값이 유지됩니다. 이는 PATCH 시맨틱의 "변경하지 않음" 의미로는 맞지만, 하위 카테고리를 최상위 카테고리로 승격하려는 의도와 구분할 수 없습니다.
명시적인 "해제" 신호(예: 0L 또는 전용 필드)를 도입하여 두 의도를 명확히 구분하는 것이 안전합니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/admin/service/AdminService.java`
around lines 162 - 166, The current logic in AdminService (handling
request.getParentCategoryId()) treats a null parentCategoryId as "do not change"
because Category.updateCategory only updates when parentCategory != null; to
allow explicit removal of a parent, add a clear signal in the request (e.g.,
treat parentCategoryId == 0L or add a boolean like clearParent) and branch: if
request.getParentCategoryId() == 0L (or clearParent==true) set parentCategory =
null and mark that parent should be cleared, else if parentCategoryId != null
load parent via categoryRepository.findById(...); then call
Category.updateCategory with a parameter or overload that can distinguish
"no-op", "set to X", and "clear parent" (or pass an Optional/flag) so
updateCategory can explicitly remove the parent when requested.
Comment on lines
+186
to
+223
| // 상품 수정 | ||
| public ProductSimpleResponse updateProduct(Long productId, AdminProductUpdateRequest request) { | ||
| Product product = productRepository.findById(productId) | ||
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.PRODUCT_NOT_FOUND)); | ||
|
|
||
| Brand brand = null; | ||
| if (request.getBrandId() != null) { | ||
| brand = brandRepository.findById(request.getBrandId()) | ||
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.BRAND_NOT_FOUND)); | ||
| } | ||
|
|
||
| Category category = null; | ||
| if (request.getCategoryId() != null) { | ||
| category = categoryRepository.findById(request.getCategoryId()) | ||
| .orElseThrow(() -> new EntityNotFoundException(ErrorCode.CATEGORY_NOT_FOUND)); | ||
|
|
||
| // 상품은 하위 카테고리에만 등록 가능 | ||
| if (category.getParentCategory() == null) { | ||
| throw new ValidationException(ErrorCode.INVALID_CATEGORY); | ||
| } | ||
| } | ||
|
|
||
| product.updateProduct( | ||
| request.getName(), | ||
| request.getDescription(), | ||
| request.getPrice(), | ||
| request.getMaterialOriginal(), | ||
| request.getImageUrls(), | ||
| request.getSizes(), | ||
| request.getColors(), | ||
| request.getDiscountRate(), | ||
| request.getProductType(), | ||
| brand, | ||
| category | ||
| ); | ||
|
|
||
| return productConverter.toSimpleResponse(product); | ||
| } |
Contributor
There was a problem hiding this comment.
updateProduct에서 onSale 필드가 전달되지 않습니다.
PR 목표에 명시된 "상품 판매 중단은 상품 수정 API를 통해 onSale=false로 처리"하는 기능이 현재 구현에서 누락되어 있습니다. AdminProductUpdateRequest에 onSale 필드를 추가하고 product.updateProduct(...) 호출 시 함께 전달해야 합니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/admin/service/AdminService.java`
around lines 186 - 223, The updateProduct method is not passing the onSale flag
from AdminProductUpdateRequest into product.updateProduct; add an onSale Boolean
to AdminProductUpdateRequest (with getter) if missing, update the call site in
AdminService.updateProduct to pass request.getOnSale() into
product.updateProduct, and ensure the Product.updateProduct signature (and any
usages) is updated to accept and apply the onSale value so the Product's onSale
state is persisted; keep other validations unchanged and return
productConverter.toSimpleResponse(product) as before.
Comment on lines
+48
to
+63
|
|
||
| // 카테고리 정보 수정 | ||
| public void updateCategory(String name, String iconUrl, Integer displayOrder, Category parentCategory) { | ||
| if (name != null) { | ||
| this.name = name; | ||
| } | ||
| if (iconUrl != null) { | ||
| this.iconUrl = iconUrl; | ||
| } | ||
| if (displayOrder != null) { | ||
| this.displayOrder = displayOrder; | ||
| } | ||
| if (parentCategory != null) { | ||
| this.parentCategory = parentCategory; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
자기 참조(순환 참조) 방지 로직 누락
parentCategory로 자기 자신을 전달하면 순환 참조가 발생하여 카테고리 트리 조회 시 무한 루프에 빠질 수 있습니다. 또한, parentCategory를 null로 설정하여 최상위 카테고리로 변경하는 것이 현재 null-check 패턴으로는 불가능합니다.
🛡️ 순환 참조 방어 코드 제안
if (parentCategory != null) {
+ if (parentCategory.getId().equals(this.id)) {
+ throw new IllegalArgumentException("자기 자신을 상위 카테고리로 설정할 수 없습니다.");
+ }
this.parentCategory = parentCategory;
}🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/category/entity/Category.java` around
lines 48 - 63, The updateCategory method in Category allows setting
parentCategory only when non-null and lacks self-reference protection; change
updateCategory in class Category so it accepts null to unset the parent
(allowing promotion to top-level) and explicitly prevent cycles by rejecting
when parentCategory == this (e.g., throw IllegalArgumentException); keep the
other field updates unchanged and reference updateCategory and parentCategory
when making the fix.
Comment on lines
+141
to
+187
| public void updateProduct(String name, String description, Integer price, String materialOriginal, | ||
| String imageUrls, String sizes, String colors, Integer discountRate, ProductType productType, | ||
| Brand brand, Category category) { | ||
| if (name != null) { | ||
| this.name = name; | ||
| } | ||
| if (description != null) { | ||
| this.description = description; | ||
| } | ||
| if (price != null) { | ||
| this.price = price; | ||
| // 가격 변경 시 할인가 재계산 | ||
| if (this.discountRate != null && this.discountRate > 0) { | ||
| this.discountPrice = price - (price * this.discountRate / 100); | ||
| } | ||
| } | ||
| if (materialOriginal != null) { | ||
| this.materialOriginal = materialOriginal; | ||
| } | ||
| if (imageUrls != null) { | ||
| this.imageUrls = imageUrls; | ||
| } | ||
| if (sizes != null) { | ||
| this.sizes = sizes; | ||
| } | ||
| if (colors != null) { | ||
| this.colors = colors; | ||
| } | ||
| if (discountRate != null) { | ||
| this.discountRate = discountRate; | ||
| // 할인율 변경 시 할인가 재계산 | ||
| if (this.price != null && discountRate > 0) { | ||
| this.discountPrice = this.price - (this.price * discountRate / 100); | ||
| } else { | ||
| this.discountPrice = null; | ||
| } | ||
| } | ||
| if (productType != null) { | ||
| this.productType = productType; | ||
| } | ||
| if (brand != null) { | ||
| this.brand = brand; | ||
| } | ||
| if (category != null) { | ||
| this.category = category; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's view the complete Product.java file to see all fields and understand the onSale field
fd "Product.java" --path "*/domain/product/entity/*" --exec wc -l {}Repository: IT-Cotato/12th-OnGil-BE
Length of output: 282
🏁 Script executed:
# View the complete Product.java file
fd "Product.java" --path "*/domain/product/entity/*" --exec cat -n {}Repository: IT-Cotato/12th-OnGil-BE
Length of output: 282
🏁 Script executed:
# Search for AdminProductUpdateRequest DTO
fd "AdminProductUpdateRequest.java" --exec cat -n {}Repository: IT-Cotato/12th-OnGil-BE
Length of output: 1678
🏁 Script executed:
# Search for AdminService and its updateProduct method
fd "AdminService.java" --exec cat -n {}Repository: IT-Cotato/12th-OnGil-BE
Length of output: 11126
🏁 Script executed:
# Find and view Product.java entity
find . -name "Product.java" -path "*/product/entity/*" -type fRepository: IT-Cotato/12th-OnGil-BE
Length of output: 135
🏁 Script executed:
# View Product.java - search for field definitions and onSale
fd "Product.java" | head -5 | xargs cat -n | head -250Repository: IT-Cotato/12th-OnGil-BE
Length of output: 6509
🏁 Script executed:
# Search for onSale field in Product entity
rg "onSale" --type java -B 2 -A 2Repository: IT-Cotato/12th-OnGil-BE
Length of output: 5940
🏁 Script executed:
# Search for PR objectives or design requirements mentioning onSale
rg "상품.*판매\|판매.*중단\|onSale" --type java --type md -B 2 -A 2Repository: IT-Cotato/12th-OnGil-BE
Length of output: 49
onSale 필드가 updateProduct 메서드에서 누락되었습니다.
Product 엔티티에 onSale 필드가 정의되어 있고 모든 쿼리에서 onSale = true 조건으로 필터링하고 있으나, AdminProductUpdateRequest DTO와 updateProduct 메서드에 onSale 파라미터가 없어 관리자가 판매 중단을 처리할 수 없습니다. 상품 조회 쿼리들이 onSale = true로 제한되어 있으므로, 이 필드를 통해 상품 가시성을 제어해야 합니다.
수정 필요 항목
- AdminProductUpdateRequest DTO에 onSale 필드 추가
- Product.updateProduct 메서드에 onSale 파라미터 추가
- AdminService.updateProduct 호출 시 onSale 전달
🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/product/entity/Product.java` around
lines 141 - 187, Add an onSale boolean to the admin update flow: extend
AdminProductUpdateRequest with a nullable Boolean onSale, add a corresponding
Boolean onSale parameter to Product.updateProduct(...) and, inside
updateProduct, if (onSale != null) set this.onSale = onSale; then update
AdminService.updateProduct (and any callers) to pass the DTO.getOnSale() into
Product.updateProduct; ensure the symbol names referenced are
AdminProductUpdateRequest, Product.updateProduct, and AdminService.updateProduct
so the new onSale value flows from the DTO to the entity.
src/main/java/com/ongil/backend/domain/product/entity/ProductOption.java
Show resolved
Hide resolved
- 브랜드 삭제 시 해당 브랜드를 사용하는 상품이 있으면 삭제 불가 - 카테고리 삭제 시 해당 카테고리를 사용하는 상품이 있으면 삭제 불가 - 카테고리 삭제 시 하위 카테고리가 있으면 삭제 불가 - 카테고리 수정 시 자기 참조(순환 참조) 방지 - 상품 옵션 수정 시 재고 음수 값 방지 - 관련 에러 코드 추가 (BRAND-002, CATEGORY-003~005, PRODUCT-004) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/main/java/com/ongil/backend/domain/category/entity/Category.java`:
- Around line 60-68: The current self-reference check in Category only compares
parentCategory.getId() to this.id and misses indirect cycles (A→B→A); update the
Category logic that sets parentCategory (e.g., the constructor or
setParentCategory method) to walk the parent chain via
parentCategory.getParentCategory() (using getParentCategory()/getId()) until
null, and if any ancestor's id equals this.id throw the same ValidationException
with ErrorCode.CATEGORY_SELF_REFERENCE; ensure the traversal stops when parent
is null to avoid infinite loops and consider adding this check at the service
layer if parent relations may be lazily unloaded.
🧹 Nitpick comments (2)
src/main/java/com/ongil/backend/domain/product/entity/ProductOption.java (1)
48-65: 음수 재고 방어 로직 추가 확인 — FQN 대신 import 사용 권장음수 재고 방어 로직이 잘 추가되었습니다. 다만,
ValidationException과ErrorCode를 완전한 패키지 경로(FQN)로 참조하고 있습니다. 파일 상단에 import를 추가하면 가독성이 향상됩니다.♻️ import 추가 제안
파일 상단 import 영역에 추가:
import com.ongil.backend.global.common.exception.ValidationException; import com.ongil.backend.global.common.exception.ErrorCode;메서드 내부 수정:
if (stock < 0) { - throw new com.ongil.backend.global.common.exception.ValidationException( - com.ongil.backend.global.common.exception.ErrorCode.INVALID_STOCK - ); + throw new ValidationException(ErrorCode.INVALID_STOCK); }src/main/java/com/ongil/backend/domain/admin/service/AdminService.java (1)
36-47: AdminService의 SRP(단일 책임 원칙) 관련 — 향후 분리 검토 제안현재
AdminService가 브랜드, 카테고리, 상품, 상품 옵션 4개 도메인의 CRUD를 모두 담당하고 있어 클래스 크기와 책임이 계속 커질 수 있습니다. 향후 리팩토링 시AdminBrandService,AdminCategoryService,AdminProductService등으로 분리하면 유지보수성이 향상될 수 있습니다.PR 설명에 리팩토링 계획이 언급되어 있으므로, 현 단계에서는 참고 사항으로 남깁니다. As per coding guidelines, "SOLID 원칙, 스프링 어노테이션, 의존성 주입 패턴, 예외 처리에 중점을 둔다".
Comment on lines
+60
to
+68
| if (parentCategory != null) { | ||
| // 자기 참조(순환 참조) 방지 | ||
| if (parentCategory.getId().equals(this.id)) { | ||
| throw new com.ongil.backend.global.common.exception.ValidationException( | ||
| com.ongil.backend.global.common.exception.ErrorCode.CATEGORY_SELF_REFERENCE | ||
| ); | ||
| } | ||
| this.parentCategory = parentCategory; | ||
| } |
Contributor
There was a problem hiding this comment.
간접 순환 참조(A→B→A) 감지 누락
현재 자기 참조 방지 로직은 직접적인 자기 참조(parentCategory.id == this.id)만 검사합니다. 카테고리 A의 부모를 B로, B의 부모를 A로 설정하는 간접 순환 참조는 감지하지 못합니다. 카테고리 트리 깊이가 얕다면 당장 문제가 되지 않을 수 있지만, 향후 트리 순회 시 무한 루프 위험이 있습니다.
간접 순환을 방지하려면 부모 체인을 따라 올라가며 현재 카테고리가 이미 존재하는지 확인하는 로직이 필요합니다.
♻️ 순환 참조 방어 코드 예시
if (parentCategory != null) {
if (parentCategory.getId().equals(this.id)) {
- throw new com.ongil.backend.global.common.exception.ValidationException(
- com.ongil.backend.global.common.exception.ErrorCode.CATEGORY_SELF_REFERENCE
- );
+ throw new ValidationException(ErrorCode.CATEGORY_SELF_REFERENCE);
+ }
+ // 간접 순환 참조 방지
+ Category ancestor = parentCategory.getParentCategory();
+ while (ancestor != null) {
+ if (ancestor.getId().equals(this.id)) {
+ throw new ValidationException(ErrorCode.CATEGORY_SELF_REFERENCE);
+ }
+ ancestor = ancestor.getParentCategory();
}
this.parentCategory = parentCategory;
}참고: 이 체크가 Entity 내부에서 동작하려면 부모 체인이 이미 fetch된 상태여야 합니다. Lazy 로딩 환경에서는 AdminService 레벨에서 검증하는 것이 더 안전할 수 있습니다.
🤖 Prompt for AI Agents
In `@src/main/java/com/ongil/backend/domain/category/entity/Category.java` around
lines 60 - 68, The current self-reference check in Category only compares
parentCategory.getId() to this.id and misses indirect cycles (A→B→A); update the
Category logic that sets parentCategory (e.g., the constructor or
setParentCategory method) to walk the parent chain via
parentCategory.getParentCategory() (using getParentCategory()/getId()) until
null, and if any ancestor's id equals this.id throw the same ValidationException
with ErrorCode.CATEGORY_SELF_REFERENCE; ensure the traversal stops when parent
is null to avoid infinite loops and consider adding this check at the service
layer if parent relations may be lazily unloaded.
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
1 participant
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.
🔍️ 작업 내용
관리자 기능 추가
✨ 상세 설명
관리자 페이지는 없지만 프론트의 편의를 위해서 기능 추가
🛠️ 추후 리팩토링 및 고도화 계획
📸 스크린샷 (선택)
💬 리뷰 요구사항
Summary by CodeRabbit
새로운 기능