Merged
Conversation
- StoreStatus를 StoreApplication 테이블로 이동
- 스토어 등록 신청 API 추가 - 스토어 등록 신청 정보 조회 API 추가 - 스토어 목록 조회 API 네이밍 변경 - 스토어 이름 중복 체크 API 로직 리팩토링
4 tasks
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughStoreApplication 엔티티와 DB 마이그레이션을 추가하고, Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as SellerStoreController
participant Facade as SellerStoreApplicationFacade
participant S3 as S3Service
participant SellerSvc as SellerService
participant StoreSvc as SellerStoreService
participant AppSvc as SellerStoreApplicationService
participant Repo as StoreApplicationRepository
Client->>Controller: POST /applications (request, profileImage)
Controller->>Facade: registerStoreForSeller(sellerId, request, profileImage)
Facade->>SellerSvc: validateRegisterAvailable(seller)
SellerSvc-->>Facade: validated
Facade->>StoreSvc: findStoreByStoreName(name)
StoreSvc-->>Facade: Optional<Store>
alt profile image present
Facade->>S3: uploadImage(...)
S3-->>Facade: imagePath
else use request.profile
Facade->>Facade: use request.profile
end
Facade->>AppSvc: createStoreApplication(request, profilePath, seller, store)
AppSvc->>Repo: save(storeApplication)
Repo-->>AppSvc: StoreApplication
AppSvc-->>Facade: StoreApplication
Facade->>SellerSvc: updateSellerStatus(seller, PENDING)
SellerSvc-->>Facade: updated
Facade-->>Controller: StoreApplicationDetail
Controller-->>Client: SingleResult<StoreApplicationDetail>
sequenceDiagram
participant Client as Client
participant Controller as SellerStoreController
participant Facade as SellerStoreFacade
participant StoreSvc as SellerStoreService
participant Repo as StoreRepository
Client->>Controller: GET /store-names?storeName=xxx
Controller->>Facade: checkStoreName(storeName)
Facade->>StoreSvc: findStoreByStoreName(storeName)
StoreSvc->>Repo: findByStoreNameAndIsNotDeleted(...)
Repo-->>StoreSvc: Optional<Store>
StoreSvc-->>Facade: Optional<Store>
alt not found
Facade-->>Controller: { available: true, store: null }
else found
Facade-->>Controller: { available: depends on seller linkage, store: SellerStoreDetail }
end
Controller-->>Client: SingleResult<StoreNameCheck>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
- StoreFixture 리팩토링 - SellerUnitTest 리팩토링 및 테스트 추가 - Store & Seller 엔티티 수정
- StoreApplicationFixture 추가 - StoreRepositoryTest 리팩토링 - StoreRepositoryPagingTest 수정
- SellerServiceIntegrationTest 테스트 코드 추가 - SellerStoreServiceIntegrationTest 테스트 코드 추가 - SellerStoreServiceUnitTest 테스트 코드 수정 - SellerStoreApplicationService 테스트 코드 추가 - StoreApplicationRequestFixture 추가
- SellerFacade 테스트 코드 수정 - SellerStoreApplicationFacade 테스트 코드 추가 - SellerStoreFacade 테스트 코드 추가
- StoreApplicationResponseFixture 추가 - StoreApplicationRequestFixture 수정 - SellerController 테스트 코드 수정
- 사용하지 않는 클래스 제거
- StoreStatus를 StoreApplication 테이블로 이동
- 스토어 등록 신청 API 추가 - 스토어 등록 신청 정보 조회 API 추가 - 스토어 목록 조회 API 네이밍 변경 - 스토어 이름 중복 체크 API 로직 리팩토링
- StoreFixture 리팩토링 - SellerUnitTest 리팩토링 및 테스트 추가 - Store & Seller 엔티티 수정
- StoreApplicationFixture 추가 - StoreRepositoryTest 리팩토링 - StoreRepositoryPagingTest 수정
- SellerServiceIntegrationTest 테스트 코드 추가 - SellerStoreServiceIntegrationTest 테스트 코드 추가 - SellerStoreServiceUnitTest 테스트 코드 수정 - SellerStoreApplicationService 테스트 코드 추가 - StoreApplicationRequestFixture 추가
- SellerFacade 테스트 코드 수정 - SellerStoreApplicationFacade 테스트 코드 추가 - SellerStoreFacade 테스트 코드 추가
- StoreApplicationResponseFixture 추가 - StoreApplicationRequestFixture 수정 - SellerController 테스트 코드 수정
- 사용하지 않는 클래스 제거
…n-table' into refactor/seller-store-application-table
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (8)
src/main/java/com/bbangle/bbangle/seller/seller/facade/SellerFacade.java (1)
82-82: TODO 주석을 이슈로 전환하는 것을 고려해보세요.인라인 TODO 주석은 시간이 지나면서 놓칠 수 있습니다. v3에서 계획된 리팩토링이라면 이슈 트래커에 등록하여 추적하는 것이 더 체계적입니다.
이 TODO를 이슈로 전환하거나 리팩토링 계획을 문서화하는 데 도움이 필요하시면 말씀해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/seller/seller/facade/SellerFacade.java` at line 82, Replace the inline TODO comment in SellerFacade (the "// TODO : v3 - Store/Seller/SellerStoreFacade로 이동" comment) with a tracked issue: create an issue in your tracker describing the v3 refactor to move functionality to Store/Seller/SellerStoreFacade, then either remove the inline TODO or replace it with a short comment that includes the newly created issue ID/URL and a brief one-line summary; update any related documentation or planning doc referenced by the issue and link back from the SellerFacade comment so future readers can find the plan.src/main/java/com/bbangle/bbangle/store/seller/service/SellerStoreService.java (1)
33-37: TODO 주석 확인Admin으로 이동 예정인 코드가 표시되어 있습니다. 이 작업을 추적하기 위한 이슈가 있는지 확인해 주세요.
이슈를 생성하여 이 작업을 추적하도록 도와드릴까요?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/store/seller/service/SellerStoreService.java` around lines 33 - 37, The TODO comment "TODO : v3 - Admin으로 이동" in SellerStoreService.registerStore is untracked; verify whether there is an existing issue/ticket for moving this code to Admin v3, and if not create one (title like "Migrate SellerStoreService.registerStore to Admin v3") in the project tracker; then update the TODO comment to include the created issue ID or URL (e.g., "TODO: v3 - Admin으로 이동 - ISSUE-123") so future reviewers can find the task, or remove the TODO if the work is already completed and no migration is needed.src/test/java/com/bbangle/bbangle/store/seller/service/SellerStoreServiceIntegrationTest.java (1)
115-121:Thread.sleep(1)사용에 대한 리뷰
Thread.sleep(1)은 테스트 데이터의createdAt순서를 보장하기 위한 것으로 보입니다. 이 접근 방식은 취약할 수 있으며, 시스템 부하에 따라 간헐적 실패가 발생할 수 있습니다.가능하다면 테스트에서 명시적으로 시간을 제어하거나 (예:
@MockBean Clock), 순서 보장이 필수적이지 않다면 제거를 고려해 주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/store/seller/service/SellerStoreServiceIntegrationTest.java` around lines 115 - 121, The Thread.sleep(1) used after storeRepository.save(StoreFixture.defaultStore(name)) to force createdAt ordering is fragile; replace it by controlling time explicitly (or remove if ordering isn't required). Fix by injecting/mocking the Clock used by your entity/repository (e.g., add `@MockBean` Clock clock in SellerStoreServiceIntegrationTest), advance or return deterministic Instants per iteration, and update StoreFixture.defaultStore to accept/consume that Instant (or set the entity's createdAt explicitly) so saves have predictable timestamps; alternatively, if ordering isn't required, simply remove the Thread.sleep(1) block and rely on deterministic test data.src/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacadeUnitTest.java (1)
43-44: 클래스명이 Java 네이밍 컨벤션을 따르지 않습니다.내부 테스트 클래스명
checkStoreNameTest는 PascalCase인CheckStoreNameTest로 변경하는 것이 좋습니다.♻️ 네이밍 컨벤션 수정
- class checkStoreNameTest { + class CheckStoreNameTest {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacadeUnitTest.java` around lines 43 - 44, Rename the inner test class named checkStoreNameTest to follow Java PascalCase naming (CheckStoreNameTest); update the class declaration and any references/usages (e.g., nested test annotations or reflective references) so the test framework still discovers it, and run tests to verify no compile/import issues after renaming.src/test/java/com/bbangle/bbangle/store/seller/service/SellerStoreApplicationServiceIntegrationTest.java (1)
127-146: 테스트가 삽입 순서에 의존하여 불안정할 수 있습니다.이 테스트는
storeApplication2가storeApplication1보다 늦게 생성되어createdAt이 더 최신이라고 가정합니다. 그러나 두 엔티티가 거의 동시에 저장되면createdAt타임스탬프가 동일할 수 있습니다.StoreApplicationRepositoryTest처럼 명시적으로createdAt을 설정하는 것이 더 안정적입니다.♻️ 명시적 타임스탬프 설정 제안
// given Seller seller = sellerRepository.save(SellerFixture.defaultSeller()); - storeApplicationRepository.saveAndFlush(StoreApplicationFixture.defaultStoreApplication(seller, null)); + StoreApplication storeApplication1 = storeApplicationRepository.saveAndFlush( + StoreApplicationFixture.defaultStoreApplication(seller, null) + ); StoreApplication storeApplication2 = storeApplicationRepository.saveAndFlush( StoreApplicationFixture.defaultStoreApplication(seller, null) ); + // 명시적으로 createdAt 설정하여 순서 보장 + em.createQuery("update StoreApplication s set s.createdAt = :time where s.id = :id") + .setParameter("time", LocalDateTime.of(2026, 1, 1, 0, 0)) + .setParameter("id", storeApplication1.getId()) + .executeUpdate(); + + em.createQuery("update StoreApplication s set s.createdAt = :time where s.id = :id") + .setParameter("time", LocalDateTime.of(2026, 2, 1, 0, 0)) + .setParameter("id", storeApplication2.getId()) + .executeUpdate(); + + em.clear(); + // when Optional<StoreApplication> result = service.findStoreApplicationBySellerId(seller.getId());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/store/seller/service/SellerStoreApplicationServiceIntegrationTest.java` around lines 127 - 146, The test latest_storeApplication relies on insertion order for createdAt and can be flaky; modify the setup so each StoreApplication saved via StoreApplicationFixture.defaultStoreApplication(...) has an explicit createdAt set (e.g., set older timestamp for the first and newer for storeApplication2) before calling storeApplicationRepository.saveAndFlush, or add a builder/fixture overload that accepts createdAt, so the repository returns a deterministically latest entity when service.findStoreApplicationBySellerId(seller.getId()) is invoked.src/test/java/com/bbangle/bbangle/store/seller/controller/SellerStoreControllerTest.java (1)
225-227: 신규getStoreApplication()컨트롤러 테스트가 비어 있습니다.신규 API가 추가된 PR이므로 최소한 성공/미존재(null) 응답 케이스는 채워두는 편이 안전합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/store/seller/controller/SellerStoreControllerTest.java` around lines 225 - 227, The nested test class GetStoreApplicationTest in SellerStoreControllerTest is empty; add two tests for the new controller method getStoreApplication(): one for the success path and one for the non-existent (null) response. Inside the GetStoreApplicationTest class implement e.g. testGetStoreApplication_Success and testGetStoreApplication_NotFound that mock the underlying service/repository used by SellerStoreController#getStoreApplication, perform the controller call (using the same MockMvc/setup pattern used elsewhere in SellerStoreControllerTest), and assert expected HTTP status and response body (200 + expected JSON for success, 404 or 200+null handling for not-found according to controller behavior). Ensure you reference and mock the same service bean used by the controller and follow existing test utilities/fixtures in SellerStoreControllerTest so the new tests compile and run.src/test/java/com/bbangle/bbangle/fixture/store/seller/controller/dto/StoreApplicationResponseFixture.java (1)
34-35: 시간 필드를 한 번만 생성해 픽스처 안정성을 높여주세요.Line 34-35, Line 56-57에서
LocalDateTime.now()를 두 번 호출해createdAt/modifiedAt값이 미세하게 달라질 수 있습니다. 테스트 안정성을 위해 동일 시각을 재사용하는 편이 좋습니다.♻️ 제안 수정안
public static StoreApplicationDetail defaultStoreApplicationDetail(Long storeId) { + LocalDateTime now = LocalDateTime.now(); return StoreApplicationDetail.builder() .storeApplicationId(1L) .sellerId(1L) .storeId(storeId) .name(DEFAULT_STORE_NAME) .introduce(DEFAULT_INTRODUCE) .profile(DEFAULT_PROFILE) .status(StoreApplicationStatus.PENDING) .phoneNumber(DEFAULT_PHONE) .subPhoneNumber(DEFAULT_SUBPHONE) .email(DEFAULT_EMAIL) .originAddress(DEFAULT_ADDRESS) .originAddressDetail(DEFAULT_DETAIL_ADDRESS) - .createdAt(LocalDateTime.now()) - .modifiedAt(LocalDateTime.now()) + .createdAt(now) + .modifiedAt(now) .build(); } @@ public static StoreApplicationDetail defaultStoreApplicationDetail( Long storeId, StoreApplicationStatus status ) { + LocalDateTime now = LocalDateTime.now(); return StoreApplicationDetail.builder() .storeApplicationId(1L) .sellerId(1L) .storeId(storeId) .name(DEFAULT_STORE_NAME) .introduce(DEFAULT_INTRODUCE) .profile(DEFAULT_PROFILE) .status(status) .phoneNumber(DEFAULT_PHONE) .subPhoneNumber(DEFAULT_SUBPHONE) .email(DEFAULT_EMAIL) .originAddress(DEFAULT_ADDRESS) .originAddressDetail(DEFAULT_DETAIL_ADDRESS) - .createdAt(LocalDateTime.now()) - .modifiedAt(LocalDateTime.now()) + .createdAt(now) + .modifiedAt(now) .build(); }Also applies to: 56-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/fixture/store/seller/controller/dto/StoreApplicationResponseFixture.java` around lines 34 - 35, StoreApplicationResponseFixture currently calls LocalDateTime.now() twice for createdAt and modifiedAt (lines where .createdAt(...) and .modifiedAt(...) are set), causing tiny differences and flakiness; fix by capturing a single LocalDateTime (e.g., now or fixedTimestamp) once inside the fixture builder method and reuse that variable for both createdAt and modifiedAt assignments in StoreApplicationResponseFixture (also apply the same change to the other occurrence around lines 56-57) so createdAt and modifiedAt share the exact same timestamp.src/main/java/com/bbangle/bbangle/store/seller/controller/SellerStoreController.java (1)
35-37: 컨트롤러 계층 경계를 Facade로 일원화하는 것을 권장합니다.현재 컨트롤러가
SellerStoreService와 Facade를 혼용하고 있어 책임 경계가 섞여 보입니다. 조회 API도 Facade로 통일하면 정책 변경 시 유지보수가 더 수월해집니다.Also applies to: 67-73
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/store/seller/controller/SellerStoreController.java` around lines 35 - 37, The controller mixes direct service and facade calls: update SellerStoreController to route all store-related operations through SellerStoreFacade instead of calling SellerStoreService directly—replace usages of SellerStoreService in the controller methods (including the read/query endpoints currently using SellerStoreService) with the corresponding SellerStoreFacade methods, remove SellerStoreService injection if no longer needed, and adjust the constructor and any method signatures in SellerStoreController to depend only on SellerStoreFacade and ResponseService so the controller boundary is unified through the facade.
🤖 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/bbangle/bbangle/seller/domain/Seller.java`:
- Around line 90-92: In Seller.updateStatus(CertificationStatus status) prevent
null being assigned to the certificationStatus field by validating the status
parameter and throwing an appropriate runtime exception (e.g.,
IllegalArgumentException or NullPointerException) when it is null; update the
method in the Seller class to perform this null-check before setting
this.certificationStatus to ensure the entity never holds a null status.
In `@src/main/java/com/bbangle/bbangle/seller/seller/service/SellerService.java`:
- Around line 67-70: The updateSellerStatus method currently allows a null
CertificationStatus which can corrupt domain invariants; add a null check at the
start of SellerService.updateSellerStatus and reject null (e.g., throw
IllegalArgumentException or NullPointerException with a clear message) before
calling seller.updateStatus(status); optionally also validate seller is
non-null. This ensures CertificationStatus is never passed as null into
Seller.updateStatus and preserves domain integrity.
In `@src/main/java/com/bbangle/bbangle/store/domain/StoreApplication.java`:
- Around line 46-48: StoreApplication의 상태 필드(status)와 관련 접근자에서 null을 허용하므로 무결성이
깨집니다; 클래스 StoreApplication의 필드 선언에 DB 레벨 제약을 위해 `@Column`(name = "status",
nullable = false) 및 도메인/검증을 위해 javax.validation의 `@NotNull` 또는 jakarta.validation의
`@NotNull을` 추가하고, 생성자와 setStatus(StoreApplicationStatus status) 메서드(및 121-123에
해당하는 다른 상태 접근/변경 메서드)에 null 입력 시 IllegalArgumentException을 던지도록 유효성 검사를 넣어 상태가
항상 non-null이 되게 보장하세요.
In
`@src/main/java/com/bbangle/bbangle/store/seller/controller/dto/StoreApplicationResponse.java`:
- Around line 25-26: Update the Swagger `@Schema` examples for
StoreApplicationResponse's createdAt and modifiedAt to match the actual ISO-8601
response format (include the 'T' separator) so documentation matches runtime
output; specifically edit the `@Schema` example values on the createdAt and
modifiedAt fields in class StoreApplicationResponse to use
"2026-01-01T12:00:00.123456" (or whatever exact serializer format your API
emits) or adjust the example to the precise configured DateTimeFormatter.
In
`@src/main/java/com/bbangle/bbangle/store/seller/controller/mapper/SellerStoreApplicationMapper.java`:
- Around line 17-24: The mapping for store.id can NPE when
StoreApplication.getStore() is null; update the mapper to handle nulls by either
setting nullValuePropertyMappingStrategy =
NullValuePropertyMappingStrategy.SET_TO_NULL on the mapper or by replacing
`@Mapping`(source = "store.id", target = "storeId") with a null-safe mapping: add
a `@Named` helper like getStoreId(Store store) that returns store == null ? null :
store.getId(), annotate it with `@Named` and reference it via `@Mapping`(source =
"store", target = "storeId", qualifiedByName = "getStoreId") for the
toStoreApplicationDetail method (or use an equivalent expression-based null
check).
In
`@src/main/java/com/bbangle/bbangle/store/seller/controller/swagger/SellerStoreApi.java`:
- Line 31: Fix the Korean typo in the API description inside SellerStoreApi:
change the phrase "null이여야" to the correct "null이어야" in the Swagger/API
description string (locate the description in the SellerStoreApi class or its
Swagger annotation/comment and update that text).
In
`@src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java`:
- Around line 58-62: When request.storeId() is present and you load Store via
sellerStoreService.findStore(...) in SellerStoreApplicationFacade, validate that
request.storeName() matches the loaded Store.getName(); if they differ, fail
fast (throw a ValidationException/IllegalArgumentException or return a
validation error) instead of proceeding to save inconsistent data. Update the
code around the Store store = ... block to perform the comparison against
store.getName() and either set the request/storeName to the canonical name or
throw a clear exception (include the storeId and both names in the error
message) so SellerStore creation cannot persist mismatched storeName/storeId
pairs.
- Around line 34-91: The registerStoreForSeller method performs
sellerStoreApplicationService.createStoreApplication(...) and
sellerService.updateSellerStatus(...) in separate transactions, risking partial
commits; wrap both DB updates in a single transactional boundary by making
registerStoreForSeller (or a new facade/service wrapper method) `@Transactional`
(rollbackFor=Exception.class) so createStoreApplication and updateSellerStatus
execute atomically; ensure exception handling does not swallow rollback (rethrow
after S3 cleanup) and keep S3 upload/cleanup logic outside or compensating to
avoid leaving DB changed when file operations fail.
In
`@src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacade.java`:
- Around line 31-35: When a store name is already taken
(sellerService.existsSellerByStoreId(...) returns true) avoid returning full
details; inside the method that builds the StoreNameCheck (the block using
optionalStore.get() and StoreNameCheck.builder()), set available = false and
return either store = null or a minimal DTO (instead of
sellerStoreMapper.toSellerStoreDetail(store)) — for example call a new or
existing summary mapper like sellerStoreMapper.toSellerStoreSummary(store) or
simply omit the store field. Keep full
sellerStoreMapper.toSellerStoreDetail(store) only for the available=true branch.
In
`@src/main/java/com/bbangle/bbangle/store/seller/service/SellerStoreService.java`:
- Around line 27-31: findStoreByStoreName uses storeName.strip() while
selectStoreNameForSeller uses replaceAll("\\s+", ""), causing inconsistent
normalization; decide on a single normalization strategy (e.g., remove all
whitespace) and apply it consistently by replacing strip() in
findStoreByStoreName or switching selectStoreNameForSeller to strip(), or better
extract a shared private method like normalizeStoreName(String) and call that
from both findStoreByStoreName and selectStoreNameForSeller so both use the same
logic.
In
`@src/test/java/com/bbangle/bbangle/seller/seller/service/SellerServiceIntegrationTest.java`:
- Around line 204-216: The test DisplayName and assertion disagree: the test
method not_registered() calls
sellerService.existsSellerByStoreId(notExistStoreId) and asserts
assertThat(result).isFalse(), but the `@DisplayName` says it returns true; update
the DisplayName to reflect the expected false result (e.g., "해당 Store를 등록한 판매자가
없을 경우 false를 반환한다.") or, if the intended behavior is true, change the assertion
to assertThat(result).isTrue(); adjust the text in the `@DisplayName` or the
assertion in SellerServiceIntegrationTest.not_registered() accordingly.
In `@src/test/java/com/bbangle/bbangle/store/domain/StoreUnitTest.java`:
- Around line 68-75: The failing tests call Store.createForSeller with
identifier and introduce parameters swapped; update the calls in the failing
test methods (e.g., fail_create_store_with_invalid_phone and the other
parameterized failure tests) to match the correct parameter order used in
success_create_store_for_seller: pass DEFAULT_INTRODUCE then DEFAULT_IDENTIFIER
(i.e., name, profile, introduce, identifier, phone, ...), ensuring all
occurrences where DEFAULT_IDENTIFIER and DEFAULT_INTRODUCE are inverted are
corrected.
---
Nitpick comments:
In `@src/main/java/com/bbangle/bbangle/seller/seller/facade/SellerFacade.java`:
- Line 82: Replace the inline TODO comment in SellerFacade (the "// TODO : v3 -
Store/Seller/SellerStoreFacade로 이동" comment) with a tracked issue: create an
issue in your tracker describing the v3 refactor to move functionality to
Store/Seller/SellerStoreFacade, then either remove the inline TODO or replace it
with a short comment that includes the newly created issue ID/URL and a brief
one-line summary; update any related documentation or planning doc referenced by
the issue and link back from the SellerFacade comment so future readers can find
the plan.
In
`@src/main/java/com/bbangle/bbangle/store/seller/controller/SellerStoreController.java`:
- Around line 35-37: The controller mixes direct service and facade calls:
update SellerStoreController to route all store-related operations through
SellerStoreFacade instead of calling SellerStoreService directly—replace usages
of SellerStoreService in the controller methods (including the read/query
endpoints currently using SellerStoreService) with the corresponding
SellerStoreFacade methods, remove SellerStoreService injection if no longer
needed, and adjust the constructor and any method signatures in
SellerStoreController to depend only on SellerStoreFacade and ResponseService so
the controller boundary is unified through the facade.
In
`@src/main/java/com/bbangle/bbangle/store/seller/service/SellerStoreService.java`:
- Around line 33-37: The TODO comment "TODO : v3 - Admin으로 이동" in
SellerStoreService.registerStore is untracked; verify whether there is an
existing issue/ticket for moving this code to Admin v3, and if not create one
(title like "Migrate SellerStoreService.registerStore to Admin v3") in the
project tracker; then update the TODO comment to include the created issue ID or
URL (e.g., "TODO: v3 - Admin으로 이동 - ISSUE-123") so future reviewers can find the
task, or remove the TODO if the work is already completed and no migration is
needed.
In
`@src/test/java/com/bbangle/bbangle/fixture/store/seller/controller/dto/StoreApplicationResponseFixture.java`:
- Around line 34-35: StoreApplicationResponseFixture currently calls
LocalDateTime.now() twice for createdAt and modifiedAt (lines where
.createdAt(...) and .modifiedAt(...) are set), causing tiny differences and
flakiness; fix by capturing a single LocalDateTime (e.g., now or fixedTimestamp)
once inside the fixture builder method and reuse that variable for both
createdAt and modifiedAt assignments in StoreApplicationResponseFixture (also
apply the same change to the other occurrence around lines 56-57) so createdAt
and modifiedAt share the exact same timestamp.
In
`@src/test/java/com/bbangle/bbangle/store/seller/controller/SellerStoreControllerTest.java`:
- Around line 225-227: The nested test class GetStoreApplicationTest in
SellerStoreControllerTest is empty; add two tests for the new controller method
getStoreApplication(): one for the success path and one for the non-existent
(null) response. Inside the GetStoreApplicationTest class implement e.g.
testGetStoreApplication_Success and testGetStoreApplication_NotFound that mock
the underlying service/repository used by
SellerStoreController#getStoreApplication, perform the controller call (using
the same MockMvc/setup pattern used elsewhere in SellerStoreControllerTest), and
assert expected HTTP status and response body (200 + expected JSON for success,
404 or 200+null handling for not-found according to controller behavior). Ensure
you reference and mock the same service bean used by the controller and follow
existing test utilities/fixtures in SellerStoreControllerTest so the new tests
compile and run.
In
`@src/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacadeUnitTest.java`:
- Around line 43-44: Rename the inner test class named checkStoreNameTest to
follow Java PascalCase naming (CheckStoreNameTest); update the class declaration
and any references/usages (e.g., nested test annotations or reflective
references) so the test framework still discovers it, and run tests to verify no
compile/import issues after renaming.
In
`@src/test/java/com/bbangle/bbangle/store/seller/service/SellerStoreApplicationServiceIntegrationTest.java`:
- Around line 127-146: The test latest_storeApplication relies on insertion
order for createdAt and can be flaky; modify the setup so each StoreApplication
saved via StoreApplicationFixture.defaultStoreApplication(...) has an explicit
createdAt set (e.g., set older timestamp for the first and newer for
storeApplication2) before calling storeApplicationRepository.saveAndFlush, or
add a builder/fixture overload that accepts createdAt, so the repository returns
a deterministically latest entity when
service.findStoreApplicationBySellerId(seller.getId()) is invoked.
In
`@src/test/java/com/bbangle/bbangle/store/seller/service/SellerStoreServiceIntegrationTest.java`:
- Around line 115-121: The Thread.sleep(1) used after
storeRepository.save(StoreFixture.defaultStore(name)) to force createdAt
ordering is fragile; replace it by controlling time explicitly (or remove if
ordering isn't required). Fix by injecting/mocking the Clock used by your
entity/repository (e.g., add `@MockBean` Clock clock in
SellerStoreServiceIntegrationTest), advance or return deterministic Instants per
iteration, and update StoreFixture.defaultStore to accept/consume that Instant
(or set the entity's createdAt explicitly) so saves have predictable timestamps;
alternatively, if ordering isn't required, simply remove the Thread.sleep(1)
block and rely on deterministic test data.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
src/main/java/com/bbangle/bbangle/seller/domain/Seller.javasrc/main/java/com/bbangle/bbangle/seller/repository/SellerRepository.javasrc/main/java/com/bbangle/bbangle/seller/seller/controller/SellerController.javasrc/main/java/com/bbangle/bbangle/seller/seller/facade/SellerFacade.javasrc/main/java/com/bbangle/bbangle/seller/seller/service/SellerService.javasrc/main/java/com/bbangle/bbangle/store/domain/Store.javasrc/main/java/com/bbangle/bbangle/store/domain/StoreApplication.javasrc/main/java/com/bbangle/bbangle/store/domain/StoreStatus.javasrc/main/java/com/bbangle/bbangle/store/domain/model/StoreApplicationStatus.javasrc/main/java/com/bbangle/bbangle/store/repository/StoreApplicationQueryDSLRepository.javasrc/main/java/com/bbangle/bbangle/store/repository/StoreApplicationRepository.javasrc/main/java/com/bbangle/bbangle/store/repository/StoreApplicationRepositoryImpl.javasrc/main/java/com/bbangle/bbangle/store/seller/controller/SellerStoreController.javasrc/main/java/com/bbangle/bbangle/store/seller/controller/dto/StoreApplicationRequest.javasrc/main/java/com/bbangle/bbangle/store/seller/controller/dto/StoreApplicationResponse.javasrc/main/java/com/bbangle/bbangle/store/seller/controller/dto/StoreResponse.javasrc/main/java/com/bbangle/bbangle/store/seller/controller/mapper/SellerStoreApplicationMapper.javasrc/main/java/com/bbangle/bbangle/store/seller/controller/swagger/SellerStoreApi.javasrc/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.javasrc/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacade.javasrc/main/java/com/bbangle/bbangle/store/seller/service/SellerStoreApplicationService.javasrc/main/java/com/bbangle/bbangle/store/seller/service/SellerStoreService.javasrc/test/java/com/bbangle/bbangle/fixture/seller/domain/SellerFixture.javasrc/test/java/com/bbangle/bbangle/fixture/store/domain/StoreApplicationFixture.javasrc/test/java/com/bbangle/bbangle/fixture/store/domain/StoreFixture.javasrc/test/java/com/bbangle/bbangle/fixture/store/seller/controller/dto/StoreApplicationRequestFixture.javasrc/test/java/com/bbangle/bbangle/fixture/store/seller/controller/dto/StoreApplicationResponseFixture.javasrc/test/java/com/bbangle/bbangle/seller/domain/SellerUnitTest.javasrc/test/java/com/bbangle/bbangle/seller/seller/controller/SellerControllerTest.javasrc/test/java/com/bbangle/bbangle/seller/seller/facade/SellerFacadeIntegrationTest.javasrc/test/java/com/bbangle/bbangle/seller/seller/facade/SellerFacadeUnitTest.javasrc/test/java/com/bbangle/bbangle/seller/seller/service/AccountVerificationServiceIntegrationTest.javasrc/test/java/com/bbangle/bbangle/seller/seller/service/SellerDocumentServiceIntegrationTest.javasrc/test/java/com/bbangle/bbangle/seller/seller/service/SellerServiceIntegrationTest.javasrc/test/java/com/bbangle/bbangle/store/domain/StoreApplicationTest.javasrc/test/java/com/bbangle/bbangle/store/domain/StoreUnitTest.javasrc/test/java/com/bbangle/bbangle/store/repository/StoreApplicationRepositoryTest.javasrc/test/java/com/bbangle/bbangle/store/repository/StoreRepositoryPagingTest.javasrc/test/java/com/bbangle/bbangle/store/repository/StoreRepositoryTest.javasrc/test/java/com/bbangle/bbangle/store/seller/controller/SellerStoreControllerTest.javasrc/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacadeIntegrationTest.javasrc/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacadeUnitTest.javasrc/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacadeIntegrationTest.javasrc/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacadeUnitTest.javasrc/test/java/com/bbangle/bbangle/store/seller/service/SellerStoreApplicationServiceIntegrationTest.javasrc/test/java/com/bbangle/bbangle/store/seller/service/SellerStoreServiceIntegrationTest.javasrc/test/java/com/bbangle/bbangle/store/seller/service/SellerStoreServiceUnitTest.java
💤 Files with no reviewable changes (4)
- src/test/java/com/bbangle/bbangle/store/repository/StoreRepositoryPagingTest.java
- src/main/java/com/bbangle/bbangle/store/domain/StoreStatus.java
- src/test/java/com/bbangle/bbangle/fixture/seller/domain/SellerFixture.java
- src/test/java/com/bbangle/bbangle/store/seller/service/SellerStoreServiceUnitTest.java
src/main/java/com/bbangle/bbangle/seller/seller/service/SellerService.java
Show resolved
Hide resolved
src/main/java/com/bbangle/bbangle/store/seller/controller/dto/StoreApplicationResponse.java
Show resolved
Hide resolved
...in/java/com/bbangle/bbangle/store/seller/controller/mapper/SellerStoreApplicationMapper.java
Show resolved
Hide resolved
src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java
Show resolved
Hide resolved
src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacade.java
Show resolved
Hide resolved
src/main/java/com/bbangle/bbangle/store/seller/service/SellerStoreService.java
Show resolved
Hide resolved
src/test/java/com/bbangle/bbangle/seller/seller/service/SellerServiceIntegrationTest.java
Show resolved
Hide resolved
- ERD 수정
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/resources/flyway/V38__app.sql (1)
2-19: 테이블 구조는 적절해 보이나, 몇 가지 고려 사항이 있습니다.
중복 신청 방지: 동일 판매자가 여러 개의 PENDING 상태 신청서를 제출할 수 있습니다. 비즈니스 로직상 한 판매자당 하나의 대기 중인 신청만 허용해야 한다면 unique constraint 또는 partial index 추가를 검토해 주세요.
store_id 인덱스 부재:
store_id로 조회하는 쿼리 패턴이 있다면 별도 인덱스가 필요할 수 있습니다 (예: 특정 스토어의 수정 신청 이력 조회).💡 중복 방지 인덱스 예시 (필요시)
-- MySQL 8.0.13+ 에서 functional index 사용 가능 -- 또는 application 레벨에서 중복 검증 처리 CREATE UNIQUE INDEX idx_store_application_seller_pending ON store_application(seller_id, (CASE WHEN status = 'PENDING' THEN 1 ELSE NULL END));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/resources/flyway/V38__app.sql` around lines 2 - 19, The schema allows multiple PENDING applications per seller; add a uniqueness constraint/index to prevent more than one PENDING row per seller and also add an index on store_id to speed lookups; update the migration that creates table store_application to create a unique partial/functional index on (seller_id,status='PENDING') (or equivalent conditional unique constraint depending on MySQL version) and add a regular index on store_id, referencing the existing columns seller_id, status and store_id and keeping existing FK constraints fk_seller_store_application and fk_store_store_application intact.documents/bbangle.erd.json (2)
13595-13636: 미사용 인덱스 메타데이터 정리를 권장합니다Line 13595-13636, Line 13655-13666의 빈 이름/중복 인덱스 엔트리는
doc.indexIds에 포함되지 않아 고아 데이터가 됩니다. 추후 ERD 유지보수 혼선을 줄이려면 제거하는 편이 좋습니다.Also applies to: 13655-13666, 13687-13706
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@documents/bbangle.erd.json` around lines 13595 - 13636, Remove the orphan/unused index metadata entries that have empty names or empty indexColumnIds and are not referenced from doc.indexIds — specifically remove entries like "SuTKgXudD--VWvd6CvpPa" and "29nEKYv3IFb8izJ_9H1qb" (and similar blocks at the other ranges noted) from the JSON; keep valid indexes such as "xpZ2LVIuOLb7-bqjmzzS5", and if any entry must be preserved instead of deleted, ensure its id is present in doc.indexIds so it won’t become an orphan.
145-145:memoIds와memoEntities상태를 맞추는 것이 좋습니다Line 145에서
memoIds가 비어 있는데, 파일 하단memoEntities는 잔존합니다. 사용하지 않는 메모라면 함께 정리해 메타데이터 일관성을 맞춰주세요.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@documents/bbangle.erd.json` at line 145, The JSON shows memoIds is an empty array while memoEntities still contains entries; either remove the unused memoEntities or populate memoIds to reflect the keys in memoEntities so the two structures stay consistent—update the memoIds array to include the same memo identifiers present as keys in memoEntities, or delete memoEntities entries that are no longer used (ensure you edit the memoIds and memoEntities fields together so their state matches).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@documents/bbangle.erd.json`:
- Around line 13637-13648: The index "idx_store_application_status_created_at"
(id qDGApj_YnbmvS0hQA2ZZH) currently lists indexColumnIds and seqIndexColumnIds
in the order created_at then status; update both arrays to list the status
column id first and created_at second (i.e., swap the two entries
"RrccG62XXD4s0-FGBPVfn" and "Y__3goBl5vCKjzuy8Mk8M" so status precedes
created_at). Make the same swap for the duplicate/index definition at the other
occurrence noted (lines ~13707-13724) so both indexColumnIds and
seqIndexColumnIds reflect status -> created_at.
- Around line 2235-2254: The store_application object has orphan/duplicate
column metadata: the seqColumnIds array contains IDs not present in columnIds
and there are empty-name/type placeholders and duplicated FK-style columns in
the column definitions; fix by removing any IDs from seqColumnIds that do not
exist in columnIds, or if those IDs are intended, add corresponding complete
column entries (with name and type) into columnIds; detect and remove duplicate
FK columns (keep a single canonical FK column entry) and delete empty
placeholder columns so every ID in seqColumnIds maps to a single valid column
object and every column object referenced has non-empty name/type, ensuring
seqColumnIds and columnIds are fully consistent for store_application.
In `@src/main/resources/flyway/V38__app.sql`:
- Around line 26-27: V38 currently drops the store.status column without
migrating data; before the ALTER TABLE drop in V38__app.sql, add an INSERT ...
SELECT that moves non-null values from store.status into
store_application.status (map the correct key columns such as store.id ->
store_application.store_id and any other required columns), e.g. INSERT INTO
store_application (store_id, status, ...) SELECT id, status, ... FROM store
WHERE status IS NOT NULL; ensure this runs in the same migration (or
transaction) prior to alter table and include any necessary UPSERT/ON CONFLICT
logic to avoid duplicate keys in store_application; verify in production that
store.status contains data before deployment.
---
Nitpick comments:
In `@documents/bbangle.erd.json`:
- Around line 13595-13636: Remove the orphan/unused index metadata entries that
have empty names or empty indexColumnIds and are not referenced from
doc.indexIds — specifically remove entries like "SuTKgXudD--VWvd6CvpPa" and
"29nEKYv3IFb8izJ_9H1qb" (and similar blocks at the other ranges noted) from the
JSON; keep valid indexes such as "xpZ2LVIuOLb7-bqjmzzS5", and if any entry must
be preserved instead of deleted, ensure its id is present in doc.indexIds so it
won’t become an orphan.
- Line 145: The JSON shows memoIds is an empty array while memoEntities still
contains entries; either remove the unused memoEntities or populate memoIds to
reflect the keys in memoEntities so the two structures stay consistent—update
the memoIds array to include the same memo identifiers present as keys in
memoEntities, or delete memoEntities entries that are no longer used (ensure you
edit the memoIds and memoEntities fields together so their state matches).
In `@src/main/resources/flyway/V38__app.sql`:
- Around line 2-19: The schema allows multiple PENDING applications per seller;
add a uniqueness constraint/index to prevent more than one PENDING row per
seller and also add an index on store_id to speed lookups; update the migration
that creates table store_application to create a unique partial/functional index
on (seller_id,status='PENDING') (or equivalent conditional unique constraint
depending on MySQL version) and add a regular index on store_id, referencing the
existing columns seller_id, status and store_id and keeping existing FK
constraints fk_seller_store_application and fk_store_store_application intact.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java (1)
60-64:⚠️ Potential issue | 🟠 Major
storeId사용 시storeName정합성 검증이 필요합니다.
storeId로 조회한 실제 스토어명과 요청storeName이 달라도 현재 흐름에서는 그대로 저장됩니다. 잘못된storeId/storeName조합이 신청서에 남을 수 있습니다.🔧 제안 수정안
Store store = null; if (request.storeId() != null) { store = sellerStoreService.findStore(request.storeId()); + String requestedName = request.storeName() == null ? null : request.storeName().strip(); + if (requestedName == null || !store.getName().equals(requestedName)) { + throw new BbangleException(BbangleErrorCode.INVALID_STORE_NAME); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java` around lines 60 - 64, When request.storeId() is present, after fetching Store store = sellerStoreService.findStore(request.storeId()), validate that store.getName() (or store.name()) matches request.storeName(); if they differ, reject the request by throwing a clear validation exception (e.g., IllegalArgumentException or a domain ValidationException) or return a validation error response instead of proceeding, so incorrect storeId/storeName combinations cannot be persisted; update the logic in SellerStoreApplicationFacade (around request.storeId()/sellerStoreService.findStore()) to perform this comparison and handle the mismatch case consistently.
🧹 Nitpick comments (3)
src/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacadeUnitTest.java (1)
42-63: 테스트 구조가 적절합니다.Store가 존재하지 않을 때
existsSellerByStoreId가 호출되지 않음을 검증하는 것은 좋은 접근입니다. 완전성을 위해sellerStoreMapper.toSellerStoreDetail()도 호출되지 않음을 검증하는 것을 고려해볼 수 있습니다.♻️ 선택적 개선안
// then assertThat(result.available()).isTrue(); assertThat(result.store()).isNull(); verify(sellerService, never()).existsSellerByStoreId(any()); + verify(sellerStoreMapper, never()).toSellerStoreDetail(any());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacadeUnitTest.java` around lines 42 - 63, In the notExist() test of CheckStoreNameTest, also verify that sellerStoreMapper.toSellerStoreDetail(...) is never invoked when the store is absent: after calling sellerStoreFacade.checkStoreName(storeName) add a verify(sellerStoreMapper, never()).toSellerStoreDetail(any()) to ensure the mapper is not used; this complements the existing verify(sellerService, never()).existsSellerByStoreId(any()) assertion and targets the sellerStoreMapper.toSellerStoreDetail symbol.src/test/java/com/bbangle/bbangle/store/domain/StoreUnitTest.java (2)
63-66:null입력 케이스를 파라미터 테스트에 추가하면 검증 범위가 완성됩니다.현재는 잘못된 문자열만 검증하고 있어
null처리 경로가 비어 있습니다. 전화번호/서브전화번호/이메일 모두@NullSource추가를 권장합니다.테스트 케이스 확장 예시
import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullSource; import org.junit.jupiter.params.provider.ValueSource; `@ParameterizedTest` + `@NullSource` `@DisplayName`("스토어 생성 시 잘못된 전화번호로 인해 실패한다") `@ValueSource`(strings = {"12345", "abcd", "", "010-1234-5678"}) void fail_create_store_with_invalid_phone(String invalidPhone) { `@ParameterizedTest` + `@NullSource` `@DisplayName`("스토어 생성 시 잘못된 서브 전화번호로 인해 실패한다") `@ValueSource`(strings = {"12345", "abcd", "", "010-1234-5678"}) void fail_create_store_with_invalid_sub_phone(String invalidPhone) { `@ParameterizedTest` + `@NullSource` `@DisplayName`("스토어 생성 시 잘못된 이메일 형식으로 인해 실패한다") `@ValueSource`(strings = {"test1234", "@gmail", "test@gmail", "test@.com", "test@com", ""}) void fail_create_store_with_invalid_email(String invalidEmail) {Also applies to: 77-80, 91-94
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/store/domain/StoreUnitTest.java` around lines 63 - 66, Add null-case coverage to the parameterized validation tests by annotating the affected test methods with `@NullSource` in addition to their existing `@ValueSource`; specifically update fail_create_store_with_invalid_phone and the analogous methods testing subphone and email (the tests around lines 77-80 and 91-94) to include `@NullSource` so null inputs are exercised by the parameterized tests.
110-110:StoreFixture.defaultStore()초기화 중복은 헬퍼 메서드로 줄일 수 있습니다.중복 제거로 테스트 본문 가독성이 더 좋아집니다.
중복 제거 예시
public class StoreUnitTest { + private Store createDefaultStore() { + return StoreFixture.defaultStore(); + } + `@Test` `@DisplayName`("스토어 상세 정보 수정에 성공한다") void success_update_store() { - Store store = StoreFixture.defaultStore(); + Store store = createDefaultStore(); } `@ParameterizedTest` void fail_update_store_with_invalid_phone(String invalidPhone) { - Store store = StoreFixture.defaultStore(); + Store store = createDefaultStore(); } `@ParameterizedTest` void fail_update_store_with_invalid_sub_phone(String invalidPhone) { - Store store = StoreFixture.defaultStore(); + Store store = createDefaultStore(); } `@ParameterizedTest` void fail_update_store_with_invalid_email(String invalidEmail) { - Store store = StoreFixture.defaultStore(); + Store store = createDefaultStore(); } }Also applies to: 131-131, 150-150, 169-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/store/domain/StoreUnitTest.java` at line 110, Multiple tests in StoreUnitTest repeatedly call StoreFixture.defaultStore(); extract this duplication by creating a single helper (e.g., a private method like defaultStore() or a `@BeforeEach-initialized` field named defaultStore) inside the StoreUnitTest class and replace the inline calls to StoreFixture.defaultStore() in test methods with that helper/field; update tests that currently call StoreFixture.defaultStore() (instances around tests referencing the StoreFixture.defaultStore() call) to use the new helper/field to improve readability and remove repetition.
🤖 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/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java`:
- Around line 75-90: The catch blocks around seller creation call
sellerService.updateSellerStatus(seller, CertificationStatus.NEW) and then call
s3Service.deleteImage(profileImagePath) directly, which can throw and mask the
original exception; wrap the rollback call to
s3Service.deleteImage(profileImagePath) in its own try-catch that logs any
rollback failure (including the exception) but does not replace or swallow the
original caught exception, and ensure the original exception (the
BbangleException or the generic Exception) is rethrown after the protected
rollback attempt.
---
Duplicate comments:
In
`@src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java`:
- Around line 60-64: When request.storeId() is present, after fetching Store
store = sellerStoreService.findStore(request.storeId()), validate that
store.getName() (or store.name()) matches request.storeName(); if they differ,
reject the request by throwing a clear validation exception (e.g.,
IllegalArgumentException or a domain ValidationException) or return a validation
error response instead of proceeding, so incorrect storeId/storeName
combinations cannot be persisted; update the logic in
SellerStoreApplicationFacade (around
request.storeId()/sellerStoreService.findStore()) to perform this comparison and
handle the mismatch case consistently.
---
Nitpick comments:
In `@src/test/java/com/bbangle/bbangle/store/domain/StoreUnitTest.java`:
- Around line 63-66: Add null-case coverage to the parameterized validation
tests by annotating the affected test methods with `@NullSource` in addition to
their existing `@ValueSource`; specifically update
fail_create_store_with_invalid_phone and the analogous methods testing subphone
and email (the tests around lines 77-80 and 91-94) to include `@NullSource` so
null inputs are exercised by the parameterized tests.
- Line 110: Multiple tests in StoreUnitTest repeatedly call
StoreFixture.defaultStore(); extract this duplication by creating a single
helper (e.g., a private method like defaultStore() or a `@BeforeEach-initialized`
field named defaultStore) inside the StoreUnitTest class and replace the inline
calls to StoreFixture.defaultStore() in test methods with that helper/field;
update tests that currently call StoreFixture.defaultStore() (instances around
tests referencing the StoreFixture.defaultStore() call) to use the new
helper/field to improve readability and remove repetition.
In
`@src/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacadeUnitTest.java`:
- Around line 42-63: In the notExist() test of CheckStoreNameTest, also verify
that sellerStoreMapper.toSellerStoreDetail(...) is never invoked when the store
is absent: after calling sellerStoreFacade.checkStoreName(storeName) add a
verify(sellerStoreMapper, never()).toSellerStoreDetail(any()) to ensure the
mapper is not used; this complements the existing verify(sellerService,
never()).existsSellerByStoreId(any()) assertion and targets the
sellerStoreMapper.toSellerStoreDetail symbol.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/bbangle/bbangle/store/seller/controller/swagger/SellerStoreApi.javasrc/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.javasrc/test/java/com/bbangle/bbangle/seller/seller/service/SellerServiceIntegrationTest.javasrc/test/java/com/bbangle/bbangle/store/domain/StoreUnitTest.javasrc/test/java/com/bbangle/bbangle/store/seller/controller/SellerStoreControllerTest.javasrc/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreFacadeUnitTest.java
src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java
Show resolved
Hide resolved
kmindev
reviewed
Feb 27, 2026
- Seller 스토어 등록 가능 여부 검증 메서드 네이밍 수정
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/main/java/com/bbangle/bbangle/seller/domain/Seller.java (1)
90-92:⚠️ Potential issue | 🟡 Minor
updateStatus에서 null 입력을 방어해주세요.이전 리뷰에서 지적된 대로, 도메인 상태 필드에 null이 들어가면 이후 등록 가능 여부 판단 로직이 모호해집니다.
Objects.requireNonNull로 방어하는 것이 안전합니다.수정 제안
+import java.util.Objects; + public void updateStatus(CertificationStatus status) { - this.certificationStatus = status; + this.certificationStatus = Objects.requireNonNull(status, "status must not be null"); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/seller/domain/Seller.java` around lines 90 - 92, The updateStatus method on Seller currently assigns certificationStatus directly; guard against null by calling Objects.requireNonNull(status, "certificationStatus must not be null") at the start of Seller.updateStatus so certificationStatus is never set to null and downstream eligibility checks remain deterministic; update the method body in Seller.updateStatus to perform this null check before assigning to the certificationStatus field.src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java (2)
60-64:⚠️ Potential issue | 🟠 Major
storeId사용 시storeName정합성 검증이 여전히 필요합니다.Line 63에서
Store를 조회하지만 Line 72에서는 요청storeName을 그대로 저장합니다.storeId의 실제 이름과 요청 이름이 다르면 불일치 데이터가 저장됩니다.🔧 제안 수정안
Store store = null; if (request.storeId() != null) { store = sellerStoreService.findStore(request.storeId()); + if (!store.getName().equals(request.storeName().strip())) { + throw new BbangleException(BbangleErrorCode.INVALID_STORE_NAME); + } }Also applies to: 72-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java` around lines 60 - 64, When a request provides request.storeId() and you fetch the Store via sellerStoreService.findStore(...), validate that request.storeName() matches the fetched Store.getName() (or normalize both before compare); if they differ, reject the request by throwing a clear validation exception (e.g., BadRequestException) or return a domain-specific error instead of persisting the mismatched name. Update SellerStoreApplicationFacade around the block that calls sellerStoreService.findStore(...) to perform this equality check and fail fast with a descriptive message mentioning the mismatched storeName and storeId.
75-90:⚠️ Potential issue | 🟠 MajorS3 롤백 실패가 원래 예외를 마스킹할 수 있습니다.
Line 79, Line 89의
deleteImage가 추가 예외를 던지면 최초 실패 원인이 사라질 수 있습니다. 롤백은 보호된 별도 try-catch로 분리하세요.🔧 제안 수정안
} catch (BbangleException e) { sellerService.updateSellerStatus(seller, CertificationStatus.NEW); - if (profileImagePath != null) { - log.warn("Seller 생성 실패로 인한 S3 이미지 롤백: {}", profileImagePath); - s3Service.deleteImage(profileImagePath); - } + rollbackProfileImageQuietly(profileImagePath); throw e; } catch (Exception e) { sellerService.updateSellerStatus(seller, CertificationStatus.NEW); log.error(e.getMessage(), e); - if (profileImagePath != null) { - log.error("Seller 생성 실패로 인한 S3 이미지 롤백: {}", profileImagePath); - s3Service.deleteImage(profileImagePath); - } + rollbackProfileImageQuietly(profileImagePath); throw new BbangleException(BbangleErrorCode.SELLER_CREATION_FAILED); } } + + private void rollbackProfileImageQuietly(String profileImagePath) { + if (profileImagePath == null) return; + try { + s3Service.deleteImage(profileImagePath); + } catch (Exception rollbackEx) { + log.error("S3 이미지 롤백 실패: {}", profileImagePath, rollbackEx); + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java` around lines 75 - 90, The rollback S3 deletion in SellerStoreApplicationFacade currently runs directly in the catch blocks (calling s3Service.deleteImage) which can throw and mask the original exception; wrap the rollback calls in their own try-catch (for both the BbangleException and generic Exception handlers) so any exception from s3Service.deleteImage is caught and logged (e.g., log.warn/error with the rollback error) without replacing or suppressing the original exception, and ensure sellerService.updateSellerStatus(seller, CertificationStatus.NEW) and re-throwing the original exception remain unchanged.
🧹 Nitpick comments (1)
src/main/java/com/bbangle/bbangle/seller/domain/Seller.java (1)
97-100: 조건문에 중괄호를 추가해 주세요.단일 문장 if문에도 중괄호를 사용하면 가독성과 유지보수성이 향상됩니다.
수정 제안
public void validateRegisterAvailable() { - if (this.certificationStatus != CertificationStatus.NEW && this.certificationStatus != CertificationStatus.REJECTED) - throw new BbangleException(BbangleErrorCode.ALREADY_REGISTER_STORE); + if (this.certificationStatus != CertificationStatus.NEW && this.certificationStatus != CertificationStatus.REJECTED) { + throw new BbangleException(BbangleErrorCode.ALREADY_REGISTER_STORE); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/seller/domain/Seller.java` around lines 97 - 100, The if statement in Seller.validateRegisterAvailable lacks braces; update the method (Seller.validateRegisterAvailable) to wrap the throw statement in explicit braces for the if block (keeping the same condition using CertificationStatus.NEW and CertificationStatus.REJECTED) so the throw new BbangleException(BbangleErrorCode.ALREADY_REGISTER_STORE) is enclosed in { ... } to improve readability and safety.
🤖 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/test/java/com/bbangle/bbangle/seller/domain/SellerUnitTest.java`:
- Line 53: Fix the typo in the test comment inside the SellerUnitTest class:
change the comment text "give" to "given" (look for the comment in
SellerUnitTest.java near the test setup/arrange section) so the test section
headers read "given", "when", "then" consistently.
- Line 69: In SellerUnitTest update the test comment "// give" to the correct
"// given" (fix the typo in the test annotation/comment) so the test method in
class SellerUnitTest uses the conventional "given" header; locate the comment in
SellerUnitTest and replace "give" with "given".
---
Duplicate comments:
In `@src/main/java/com/bbangle/bbangle/seller/domain/Seller.java`:
- Around line 90-92: The updateStatus method on Seller currently assigns
certificationStatus directly; guard against null by calling
Objects.requireNonNull(status, "certificationStatus must not be null") at the
start of Seller.updateStatus so certificationStatus is never set to null and
downstream eligibility checks remain deterministic; update the method body in
Seller.updateStatus to perform this null check before assigning to the
certificationStatus field.
In
`@src/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.java`:
- Around line 60-64: When a request provides request.storeId() and you fetch the
Store via sellerStoreService.findStore(...), validate that request.storeName()
matches the fetched Store.getName() (or normalize both before compare); if they
differ, reject the request by throwing a clear validation exception (e.g.,
BadRequestException) or return a domain-specific error instead of persisting the
mismatched name. Update SellerStoreApplicationFacade around the block that calls
sellerStoreService.findStore(...) to perform this equality check and fail fast
with a descriptive message mentioning the mismatched storeName and storeId.
- Around line 75-90: The rollback S3 deletion in SellerStoreApplicationFacade
currently runs directly in the catch blocks (calling s3Service.deleteImage)
which can throw and mask the original exception; wrap the rollback calls in
their own try-catch (for both the BbangleException and generic Exception
handlers) so any exception from s3Service.deleteImage is caught and logged
(e.g., log.warn/error with the rollback error) without replacing or suppressing
the original exception, and ensure sellerService.updateSellerStatus(seller,
CertificationStatus.NEW) and re-throwing the original exception remain
unchanged.
---
Nitpick comments:
In `@src/main/java/com/bbangle/bbangle/seller/domain/Seller.java`:
- Around line 97-100: The if statement in Seller.validateRegisterAvailable lacks
braces; update the method (Seller.validateRegisterAvailable) to wrap the throw
statement in explicit braces for the if block (keeping the same condition using
CertificationStatus.NEW and CertificationStatus.REJECTED) so the throw new
BbangleException(BbangleErrorCode.ALREADY_REGISTER_STORE) is enclosed in { ... }
to improve readability and safety.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/bbangle/bbangle/seller/domain/Seller.javasrc/main/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacade.javasrc/test/java/com/bbangle/bbangle/seller/domain/SellerUnitTest.javasrc/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacadeUnitTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/com/bbangle/bbangle/store/seller/facade/SellerStoreApplicationFacadeUnitTest.java
src/test/java/com/bbangle/bbangle/seller/domain/SellerUnitTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/bbangle/bbangle/seller/domain/SellerUnitTest.java
Outdated
Show resolved
Hide resolved
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.
History
🚀 Major Changes & Explanations
📷 Test Image
💡 ETC
현재 한 판매자가 여러 개의 스토어 등록 신청을 요청할 수 있습니다.
처음에는 Store Application에 save하기 전에 status가 PENDDING인 데이터가 있는지 검사를 하는 로직을 추가할까 했습니다.
어차피 관리자 측에서 동일한 SellerId에 대한 여러 개의 PENDDING 데이터 처리가 가능하기 때문에 구현을 하지 않았는데 구현하는 것이 좋을까요?
Summary by CodeRabbit
새 기능
개선사항
테스트
데이터베이스