Spm 496#55
Merged
Merged
Conversation
Walkthrough팩토리/창고 중심의 Employee 모델을 부서 기반의 Member 모델로 전환하는 대규모 리팩토링과 인증/권한 모델 확장(워크스페이스를 권한으로 포함), 관련 엔티티/레포지토리 추가·삭제, DTO/이벤트/퍼블리셔 및 보안 설정 수정이 적용되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant JwtFilter as JwtAuthFilter
participant TokenParser
participant Auth as Authentication
participant UserCtrl as UserController
participant UserSvc as UserService
participant MemberRepo as Member Repos
Client->>JwtFilter: HTTP 요청 + JWT
JwtFilter->>TokenParser: 토큰 파싱(userId, role, workspace)
TokenParser-->>JwtFilter: role/workspace → 권한 리스트 [ROLE_<ROLE>, ROLE_<WORKSPACE>]
JwtFilter->>Auth: Authentication 생성(Principal=userId, Authorities)
Auth->>UserCtrl: 요청 전달 (Authentication 포함)
UserCtrl->>UserSvc: getMyProfile(userId, authorities)
UserSvc->>MemberRepo: 멤버 레포 조회(워크스페이스 기반)
MemberRepo-->>UserSvc: member 엔티티
UserSvc-->>UserCtrl: UserLoginResponse
UserCtrl-->>Client: 응답 반환
sequenceDiagram
autonumber
participant Startup
participant ProdRepo as ProductionMemberRepository
participant InvenRepo as InventoryMemberRepository
participant OtherRepos as Other Member Repos
participant Publisher as UserWarmupPublisher
participant Kafka
Note over Startup,Publisher: 애플리케이션 시작 시 워밍업
Startup->>Publisher: publishWarmupOnStartup()
Publisher->>ProdRepo: findAll()
Publisher->>InvenRepo: findAll()
Publisher->>OtherRepos: findAll() (purchase/sales/md/hr/agency)
ProdRepo-->>Publisher: List<ProductionMember>
InvenRepo-->>Publisher: List<InventoryMember>
OtherRepos-->>Publisher: Lists
Publisher->>Publisher: toPayloads(List<? extends BaseMemberEntity>)
Publisher->>Kafka: UserWarmupEvent(packed member lists)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 검토 시 추가 주의점:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (2)📚 Learning: 2025-11-06T07:37:44.411ZApplied to files:
📚 Learning: 2025-10-14T11:21:55.975ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (8)
src/main/java/com/sampoom/user/api/user/event/UserWarmupEvent.java (1)
29-29: 불필요한 빈 줄을 제거하세요.hrMembers 필드 뒤에 불필요한 빈 줄이 있습니다.
다음 diff를 적용하여 빈 줄을 제거하세요:
private List<UserPayload> mdMembers; private List<UserPayload> hrMembers; -src/main/java/com/sampoom/user/common/config/security/SecurityConfig.java (1)
48-61: 프로덕션 환경에서 localhost origin을 제거하세요.CORS 설정에
http://localhost:8082와http://localhost:3000이 포함되어 있습니다. 이는 개발 환경에서는 유용하지만 프로덕션 배포 시 보안 위험이 될 수 있습니다.환경별로 CORS origin을 분리하는 것을 고려하세요:
.cors(cors -> cors.configurationSource(request -> { var corsConfig = new CorsConfiguration(); corsConfig.setAllowedMethods(List.of("GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS")); // 환경 변수나 프로파일에 따라 origins 설정 List<String> allowedOrigins = environment.getActiveProfiles().contains("prod") ? List.of("https://sampoom.store", "https://samsam.autos", "https://sampoom-management-frontend.vercel.app") : List.of("https://sampoom.store", "https://samsam.autos", "https://sampoom-management-frontend.vercel.app", "http://localhost:8082", "http://localhost:3000"); corsConfig.setAllowedOrigins(allowedOrigins); corsConfig.setAllowCredentials(true); corsConfig.setExposedHeaders(List.of("Authorization")); corsConfig.setAllowedHeaders(List.of("Content-Type", "Authorization", "X-Client-Type")); return corsConfig; }))src/main/java/com/sampoom/user/common/entity/BaseMemberEntity.java (2)
64-66: setPosition과 updatePosition 메서드의 역할을 명확히 구분하거나 통합하세요.새로 추가된
setPosition메서드(line 64-66)는 기존updatePosition메서드(line 69-71)와 동일한 동작을 수행합니다. 두 메서드 모두 position 필드를 직접 변경하지만:
updatePosition은 주석에 "더티 체킹"이라고 표시되어 JPA 변경 추적용으로 보입니다setPosition은 주석에 "setter 추가"라고 표시되어 초기 설정용으로 보입니다그러나 구현상 차이가 없어 혼란을 야기할 수 있습니다. 다음 중 하나를 고려해주세요:
- 두 메서드의 사용 목적을 명확히 구분 (예:
setPosition은@PrePersist나 팩토리 메서드에서만 사용,updatePosition은 업데이트 시 사용)- 중복을 제거하고 하나의 메서드로 통합
선택 1을 선택할 경우 적용 가능한 diff:
- public void setPosition(Position position) { + // 엔티티 생성 시에만 사용 (팩토리 메서드 또는 @PrePersist) + protected void setPosition(Position position) { this.position = position; } - // 더티 체킹 + // JPA 더티 체킹을 통한 업데이트 (비즈니스 로직에서 사용) public void updatePosition(Position newPosition) { this.position = newPosition; }
61-66: 엔티티에 public setter를 노출하는 것은 캡슐화를 약화시킬 수 있습니다.
setUserId와setPositionpublic setter는 엔티티의 내부 상태를 외부에서 직접 변경할 수 있게 합니다. 이는 다음과 같은 문제를 야기할 수 있습니다:
- 검증 로직을 우회할 수 있음
- 엔티티의 불변성 제약을 위반할 수 있음
- 비즈니스 규칙을 무시한 상태 변경 가능
더 나은 설계를 위해 다음을 고려해주세요:
- Setter를
protected로 변경하여 상속 클래스와 같은 패키지 내에서만 사용 가능하도록 제한- 팩토리 메서드나 빌더 패턴을 통해 엔티티 생성 시 초기값 설정
- 비즈니스 로직이 필요한 변경은 명확한 의미를 가진 메서드 사용 (예:
updatePosition)src/main/java/com/sampoom/user/api/member/entity/HRMember.java (1)
7-7: 사용하지 않는 static import 제거 권장.
Workspace.HR상수가 이 파일에서 사용되지 않습니다.다음 diff를 적용하여 사용하지 않는 import를 제거하세요:
-import static com.sampoom.user.common.entity.Workspace.HR;src/main/java/com/sampoom/user/api/member/entity/SalesMember.java (1)
6-7: 사용하지 않는 static import 제거 권장.
Workspace.HR와Workspace.SALES상수가 이 파일에서 사용되지 않습니다.다음 diff를 적용하여 사용하지 않는 import를 제거하세요:
-import static com.sampoom.user.common.entity.Workspace.HR; -import static com.sampoom.user.common.entity.Workspace.SALES;src/main/java/com/sampoom/user/api/agency/entity/AgencyEmployee.java (1)
7-7: 사용하지 않는 static import 제거 권장.
Workspace.AGENCY상수가 이 파일에서 사용되지 않습니다.다음 diff를 적용하여 사용하지 않는 import를 제거하세요:
-import static com.sampoom.user.common.entity.Workspace.AGENCY;src/main/java/com/sampoom/user/api/user/kafka/UserWarmupPublisher.java (1)
69-82:toPayloads의workspace인자를 정리해야 합니다.
Workspace workspace인자를 계속 전달하지만 메서드 내부에서는 더 이상 사용하지 않아 값이 항상 버려집니다. 이 상태로 두면 호출부에서 여전히 워크스페이스 상수를 넘기기 때문에 payload에 워크스페이스 정보가 포함된다고 착각하기 쉽습니다. 실제로 필요 없다면 시그니처와 모든 호출부에서 인자를 걷어내 주시고, 필요하다면 빌더에 다시 설정해 주세요. 아래처럼 정리하면 구현 의도가 명확해집니다.-import com.sampoom.user.common.entity.Workspace; @@ - var prodMembers = toPayloads(prodRepo.findAll(), Workspace.PRODUCTION); - var invenMembers = toPayloads(invenRepo.findAll(), Workspace.INVENTORY); - var agencyMembers = toPayloads(agencyRepo.findAll(), Workspace.AGENCY); - var purchaseMembers = toPayloads(purchaseRepo.findAll(), Workspace.PURCHASE); - var salesMembers = toPayloads(salesRepo.findAll(), Workspace.SALES); - var mdMembers = toPayloads(mdRepo.findAll(), Workspace.MD); - var hrMembers = toPayloads(hrRepo.findAll(), Workspace.HR); + var prodMembers = toPayloads(prodRepo.findAll()); + var invenMembers = toPayloads(invenRepo.findAll()); + var agencyMembers = toPayloads(agencyRepo.findAll()); + var purchaseMembers = toPayloads(purchaseRepo.findAll()); + var salesMembers = toPayloads(salesRepo.findAll()); + var mdMembers = toPayloads(mdRepo.findAll()); + var hrMembers = toPayloads(hrRepo.findAll()); @@ - private List<UserWarmupEvent.UserPayload> toPayloads( - List<? extends BaseMemberEntity> members, - Workspace workspace - ) { + private List<UserWarmupEvent.UserPayload> toPayloads( + List<? extends BaseMemberEntity> members + ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/main/java/com/sampoom/user/common/jwt/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (51)
src/main/java/com/sampoom/backend/common/config/web/WebConfig.java(0 hunks)src/main/java/com/sampoom/user/api/agency/entity/AgencyEmployee.java(1 hunks)src/main/java/com/sampoom/user/api/agency/repository/AgencyEmployeeRepository.java(0 hunks)src/main/java/com/sampoom/user/api/auth/entity/AuthUserProjection.java(2 hunks)src/main/java/com/sampoom/user/api/auth/event/AuthUserEvent.java(2 hunks)src/main/java/com/sampoom/user/api/auth/event/AuthWarmupEvent.java(2 hunks)src/main/java/com/sampoom/user/api/auth/kafka/AuthEventHandler.java(1 hunks)src/main/java/com/sampoom/user/api/auth/service/AuthUserProjectionService.java(2 hunks)src/main/java/com/sampoom/user/api/factory/entity/FactoryEmployee.java(0 hunks)src/main/java/com/sampoom/user/api/factory/repository/FactoryEmployeeRepository.java(0 hunks)src/main/java/com/sampoom/user/api/invitation/dto/InvitationCreateRequestDto.java(2 hunks)src/main/java/com/sampoom/user/api/invitation/entity/Invitation.java(2 hunks)src/main/java/com/sampoom/user/api/invitation/service/InvitationService.java(1 hunks)src/main/java/com/sampoom/user/api/member/entity/HRMember.java(1 hunks)src/main/java/com/sampoom/user/api/member/entity/InventoryMember.java(1 hunks)src/main/java/com/sampoom/user/api/member/entity/MDMember.java(1 hunks)src/main/java/com/sampoom/user/api/member/entity/ProductionMember.java(1 hunks)src/main/java/com/sampoom/user/api/member/entity/PurchaseMember.java(1 hunks)src/main/java/com/sampoom/user/api/member/entity/SalesMember.java(1 hunks)src/main/java/com/sampoom/user/api/member/repository/BaseMemberRepository.java(1 hunks)src/main/java/com/sampoom/user/api/member/repository/HRMemberRepository.java(1 hunks)src/main/java/com/sampoom/user/api/member/repository/InventoryMemberRepository.java(1 hunks)src/main/java/com/sampoom/user/api/member/repository/MDMemberRepository.java(1 hunks)src/main/java/com/sampoom/user/api/member/repository/ProductionMemberRepository.java(1 hunks)src/main/java/com/sampoom/user/api/member/repository/PurchaseMemberRepository.java(1 hunks)src/main/java/com/sampoom/user/api/member/repository/SalesMemberRepository.java(1 hunks)src/main/java/com/sampoom/user/api/user/controller/UserController.java(5 hunks)src/main/java/com/sampoom/user/api/user/dto/request/UserUpdateAdminRequest.java(0 hunks)src/main/java/com/sampoom/user/api/user/dto/response/EmployeeStatusResponse.java(0 hunks)src/main/java/com/sampoom/user/api/user/dto/response/UserInfoResponse.java(1 hunks)src/main/java/com/sampoom/user/api/user/dto/response/UserLoginResponse.java(1 hunks)src/main/java/com/sampoom/user/api/user/dto/response/UserUpdateAdminResponse.java(0 hunks)src/main/java/com/sampoom/user/api/user/event/EmployeeUpdatedEvent.java(0 hunks)src/main/java/com/sampoom/user/api/user/event/UserCreatedEvent.java(0 hunks)src/main/java/com/sampoom/user/api/user/event/UserWarmupEvent.java(2 hunks)src/main/java/com/sampoom/user/api/user/internal/dto/LoginRequest.java(0 hunks)src/main/java/com/sampoom/user/api/user/internal/dto/LoginResponse.java(0 hunks)src/main/java/com/sampoom/user/api/user/internal/dto/SignupUser.java(1 hunks)src/main/java/com/sampoom/user/api/user/kafka/UserWarmupPublisher.java(3 hunks)src/main/java/com/sampoom/user/api/user/repository/UserRepository.java(0 hunks)src/main/java/com/sampoom/user/api/user/service/UserInfoService.java(4 hunks)src/main/java/com/sampoom/user/api/user/service/UserService.java(5 hunks)src/main/java/com/sampoom/user/api/warehouse/entity/WarehouseEmployee.java(0 hunks)src/main/java/com/sampoom/user/api/warehouse/repository/WarehouseEmployeeRepository.java(0 hunks)src/main/java/com/sampoom/user/common/config/security/RoleHierarchyConfig.java(1 hunks)src/main/java/com/sampoom/user/common/config/security/SecurityConfig.java(1 hunks)src/main/java/com/sampoom/user/common/entity/BaseMemberEntity.java(2 hunks)src/main/java/com/sampoom/user/common/entity/Role.java(1 hunks)src/main/java/com/sampoom/user/common/entity/Workspace.java(1 hunks)src/main/java/com/sampoom/user/common/jwt/JwtAuthFilter.java(4 hunks)src/main/java/com/sampoom/user/common/response/ErrorStatus.java(2 hunks)
💤 Files with no reviewable changes (14)
- src/main/java/com/sampoom/user/api/user/internal/dto/LoginRequest.java
- src/main/java/com/sampoom/user/api/user/dto/request/UserUpdateAdminRequest.java
- src/main/java/com/sampoom/user/api/user/dto/response/UserUpdateAdminResponse.java
- src/main/java/com/sampoom/user/api/user/internal/dto/LoginResponse.java
- src/main/java/com/sampoom/user/api/user/event/UserCreatedEvent.java
- src/main/java/com/sampoom/user/api/factory/entity/FactoryEmployee.java
- src/main/java/com/sampoom/user/api/user/dto/response/EmployeeStatusResponse.java
- src/main/java/com/sampoom/user/api/factory/repository/FactoryEmployeeRepository.java
- src/main/java/com/sampoom/user/api/warehouse/repository/WarehouseEmployeeRepository.java
- src/main/java/com/sampoom/backend/common/config/web/WebConfig.java
- src/main/java/com/sampoom/user/api/agency/repository/AgencyEmployeeRepository.java
- src/main/java/com/sampoom/user/api/user/repository/UserRepository.java
- src/main/java/com/sampoom/user/api/warehouse/entity/WarehouseEmployee.java
- src/main/java/com/sampoom/user/api/user/event/EmployeeUpdatedEvent.java
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-06T07:37:44.411Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 41
File: src/main/java/com/sampoom/user/api/agency/service/AgencyProjectionService.java:26-26
Timestamp: 2025-11-06T07:37:44.411Z
Learning: In the Sampoom-Management-Backend-User project, the team is performing an incremental refactoring from "agency" to "vendor" terminology. Field names and method calls are being updated first (e.g., AgencyId -> VendorId), while local variable names will be updated in a follow-up change. Do not suggest renaming local variables during this transition period.
Applied to files:
src/main/java/com/sampoom/user/api/agency/entity/AgencyEmployee.javasrc/main/java/com/sampoom/user/api/user/service/UserService.javasrc/main/java/com/sampoom/user/api/user/service/UserInfoService.javasrc/main/java/com/sampoom/user/api/user/event/UserWarmupEvent.javasrc/main/java/com/sampoom/user/api/user/kafka/UserWarmupPublisher.java
📚 Learning: 2025-10-16T10:32:56.782Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 7
File: src/main/java/com/sampoom/backend/user/jwt/JwtAuthFilter.java:34-37
Timestamp: 2025-10-16T10:32:56.782Z
Learning: In JWT authentication filters for the Sampoom Management Backend User service, when userId or role is null in the token claims, log a warning with both values using log.warn and call chain.doFilter(req, res) before returning, rather than throwing an exception. This allows the request to continue without authentication.
Applied to files:
src/main/java/com/sampoom/user/common/config/security/SecurityConfig.javasrc/main/java/com/sampoom/user/common/jwt/JwtAuthFilter.java
📚 Learning: 2025-10-14T11:21:55.975Z
Learnt from: vivivim
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 6
File: src/main/java/com/sampoom/backend/user/service/UserService.java:24-84
Timestamp: 2025-10-14T11:21:55.975Z
Learning: In the com.sampoom.backend.user project, use custom exceptions from com.sampoom.backend.user.common.exception package (BadRequestException, NotFoundException, etc.) instead of Java built-in exceptions (IllegalArgumentException, EntityNotFoundException) for consistent error handling and proper HTTP status code mapping in service layers.
Applied to files:
src/main/java/com/sampoom/user/api/user/service/UserService.javasrc/main/java/com/sampoom/user/api/user/service/UserInfoService.java
📚 Learning: 2025-10-16T10:22:30.658Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 7
File: src/main/java/com/sampoom/backend/user/common/exception/ForbiddenException.java:15-17
Timestamp: 2025-10-16T10:22:30.658Z
Learning: In the com.sampoom.backend.user.common.exception package, exception class constructors accepting ErrorStatus parameter (e.g., ForbiddenException, BadRequestException, UnauthorizedException) do not perform null checks on the errorStatus parameter before calling its getters.
Applied to files:
src/main/java/com/sampoom/user/common/response/ErrorStatus.java
📚 Learning: 2025-11-05T17:46:15.741Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 37
File: src/main/java/com/sampoom/user/api/user/dto/request/UserUpdateRequest.java:10-13
Timestamp: 2025-11-05T17:46:15.741Z
Learning: In the Sampoom Management Backend User project, the `position` field can only be modified by administrators. Regular users updating their own profile via `UserUpdateRequest` should not have access to the `position` field. Only `UserUpdateAdminRequest` (used in admin-only endpoints) should include the `position` field.
Applied to files:
src/main/java/com/sampoom/user/api/user/dto/response/UserInfoResponse.java
🧬 Code graph analysis (10)
src/main/java/com/sampoom/user/api/member/entity/SalesMember.java (5)
src/main/java/com/sampoom/user/api/member/entity/HRMember.java (1)
Entity(9-28)src/main/java/com/sampoom/user/api/member/entity/InventoryMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/MDMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/ProductionMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/PurchaseMember.java (1)
Entity(10-29)
src/main/java/com/sampoom/user/api/member/entity/ProductionMember.java (5)
src/main/java/com/sampoom/user/api/member/entity/HRMember.java (1)
Entity(9-28)src/main/java/com/sampoom/user/api/member/entity/InventoryMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/MDMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/PurchaseMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/SalesMember.java (1)
Entity(9-28)
src/main/java/com/sampoom/user/api/member/entity/InventoryMember.java (5)
src/main/java/com/sampoom/user/api/member/entity/HRMember.java (1)
Entity(9-28)src/main/java/com/sampoom/user/api/member/entity/MDMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/ProductionMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/PurchaseMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/SalesMember.java (1)
Entity(9-28)
src/main/java/com/sampoom/user/api/agency/entity/AgencyEmployee.java (2)
src/main/java/com/sampoom/user/api/member/entity/HRMember.java (1)
Entity(9-28)src/main/java/com/sampoom/user/api/auth/entity/AuthUserProjection.java (1)
Entity(14-38)
src/main/java/com/sampoom/user/api/user/service/UserService.java (3)
src/main/java/com/sampoom/user/api/user/service/UserInfoService.java (1)
Service(31-245)src/main/java/com/sampoom/user/common/jwt/JwtAuthFilter.java (1)
Slf4j(26-135)src/main/java/com/sampoom/user/api/user/controller/UserController.java (1)
Slf4j(30-145)
src/main/java/com/sampoom/user/api/member/entity/MDMember.java (5)
src/main/java/com/sampoom/user/api/member/entity/HRMember.java (1)
Entity(9-28)src/main/java/com/sampoom/user/api/member/entity/InventoryMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/ProductionMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/PurchaseMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/SalesMember.java (1)
Entity(9-28)
src/main/java/com/sampoom/user/api/user/event/UserWarmupEvent.java (4)
src/main/java/com/sampoom/user/api/auth/event/AuthUserEvent.java (2)
Getter(13-37)Getter(21-36)src/main/java/com/sampoom/user/api/auth/event/AuthWarmupEvent.java (2)
Getter(13-36)Getter(23-35)src/main/java/com/sampoom/user/api/user/event/EmployeeUpdatedEvent.java (2)
Getter(13-34)Getter(24-33)src/main/java/com/sampoom/user/api/user/event/UserCreatedEvent.java (2)
Getter(13-34)Getter(24-33)
src/main/java/com/sampoom/user/api/member/entity/PurchaseMember.java (2)
src/main/java/com/sampoom/user/api/member/entity/HRMember.java (1)
Entity(9-28)src/main/java/com/sampoom/user/api/member/entity/InventoryMember.java (1)
Entity(10-29)
src/main/java/com/sampoom/user/api/member/entity/HRMember.java (5)
src/main/java/com/sampoom/user/api/member/entity/InventoryMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/MDMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/ProductionMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/PurchaseMember.java (1)
Entity(10-29)src/main/java/com/sampoom/user/api/member/entity/SalesMember.java (1)
Entity(9-28)
src/main/java/com/sampoom/user/common/jwt/JwtAuthFilter.java (1)
src/main/java/com/sampoom/user/common/jwt/JwtProvider.java (1)
Component(17-66)
🔇 Additional comments (32)
src/main/java/com/sampoom/user/api/auth/kafka/AuthEventHandler.java (1)
26-26: Kafka 컨슈머 그룹 ID 변경 사항 재검토 필요코드베이스에서
@KafkaListener주석 4개를 검토한 결과,AuthEventHandler.java의 그룹 ID만 다음과 같이 이상한 패턴을 보이고 있습니다:
AgencyEventConsumer:vendor-events-usersWarehouseEventHandler:branch-events-usersAuthEventHandler:auth-events-user-prod1← 유일하게 환경 접미사 포함FactoryEventHandler:factory-branch-events-users다른 모든 컨슈머는
{토픽}-users패턴을 따르지만, 이 변경사항은prod1접미사를 추가했습니다. 이는 다음 중 하나를 시사합니다:
- 임시 테스트 변경 (프로덕션 배포 전 제거 필요)
- 의도된 운영 전환 (배포 계획/조율 필수)
- 실수로 포함된 변경사항
다음을 확인하세요:
- 이 변경이 의도된 것인지, PR 설명이나 커밋 메시지에 명시되어 있는지
- 배포 전환 계획이 수립되어 있는지 (기존 컨슈머 그룹의 처리 방안)
- 의도된 변경이라면 명명 규칙을 다른 리스너와 일치하도록 조정해야 하는지 검토
src/main/java/com/sampoom/user/api/user/event/UserWarmupEvent.java (2)
22-28: 다른 마이크로서비스의 이벤트 컨슈머 업데이트를 확인하세요.7개 작업공간별 멤버 리스트로의 변경은 공개 API의 breaking change입니다. 현재 코드베이스에서는 이 이벤트를 구독하는 컨슈머가 발견되지 않습니다.
user-events토픽을 구독하는 다른 마이크로서비스의 컨슈머들이 새로운 이벤트 구조(7개 필드)를 모두 처리하도록 업데이트되었는지 확인하시기 바랍니다.
36-40: 검증 완료: 변경사항이 타당하고 안전함
@NotNull어노테이션 추가와workspace필드 제거가 모두 안전함을 확인했습니다.
- @NotNull 제약 조건: UserWarmupPublisher의
toPayloads()메서드에서userId(e.getUserId())와employeeStatus(e.getStatus())를 항상 설정하므로,null값이 발생할 수 없습니다.- workspace 필드 제거: 의도적이고 타당합니다. 워크스페이스는 이제 이벤트의 List 필드들(prodMembers, invenMembers 등)의 멤버십으로 식별됩니다.
toPayloads()메서드가 workspace 매개변수를 받지만 UserPayload에는 설정하지 않는 설계가 이를 반영합니다.- Breaking change 위험: 없습니다. UserWarmupEvent의 명시적 컨슈머가 없으며, 발행자(UserWarmupPublisher)가 완전히 제어됩니다.
src/main/java/com/sampoom/user/api/user/dto/response/UserInfoResponse.java (1)
20-26: 변경사항이 적절합니다.
workspace필드가AuthUserProjection섹션에 추가되고userName필드가User섹션에 추가되었습니다. 이는 workspace 기반 모델로의 전환과 일치합니다.src/main/java/com/sampoom/user/common/config/security/SecurityConfig.java (1)
40-40: 인가 요구사항 완화에 대한 검토가 필요합니다.
hasAuthority("ROLE_USER")에서authenticated()로 변경되어 특정 권한 없이 인증된 모든 사용자가 접근할 수 있게 되었습니다. 이는ROLE_ADMIN또는 workspace 기반 역할을 가진 사용자도 접근할 수 있다는 의미입니다.RoleHierarchyConfig의 계층 구조에서 모든 workspace 역할이
ROLE_USER를 포함하도록 설정되어 있으므로 이 변경이 의도적인 것으로 보이지만, 실제로 모든 인증된 사용자에게 접근을 허용하는 것이 보안 정책과 일치하는지 확인해야 합니다.src/main/java/com/sampoom/user/api/user/internal/dto/SignupUser.java (1)
14-18: 사용하지 않는 import 제거 및 주석 개선이 적절합니다.사용하지 않는
NotBlankimport가 제거되었고 필드 주석이 더 명확하게 업데이트되었습니다. 코드 정리가 잘 되었습니다.src/main/java/com/sampoom/user/common/response/ErrorStatus.java (2)
35-42: 검증 결과: 제거되지 않은 상수로 인한 컴파일 오류 없음제거된 상수에 대한 검사 결과, 코드베이스에는 존재하지 않는 상수를 참조하는 부분이 없습니다.
NOT_FOUND_AGENCY_NAME은 여전히 ErrorStatus.java에 존재하며 UserService.java에서도 정상적으로 참조되고 있습니다. 검색 결과에 따르면 NOT_FOUND_FACTORY_NAME, NOT_FOUND_WAREHOUSE_NAME 등의 상수는 코드베이스에서 참조되지 않으므로 제거되더라도 문제가 없습니다.Likely an incorrect or invalid review comment.
17-17: 검증 완료: 에러 코드 사용에 문제 없습니다.코드베이스 전체를 검색한 결과:
INVALID_WORKSPACE_TYPE에 대한 참조가 전혀 없습니다- 에러 코드
11401은INVALID_ROLE_TYPE에서만 사용되며 중복이 없습니다INVALID_ROLE_TYPE상수는 코드베이스에서 활발하게 사용되고 있습니다따라서 상수 교체로 인한 컴파일 오류는 발생하지 않습니다.
src/main/java/com/sampoom/user/common/entity/Role.java (1)
4-5: 이 리뷰 의견은 코드베이스에 적용되지 않습니다.열거형 순서 변경에 대한 우려는
ordinal()메서드 사용을 가정합니다. 그러나 검증 결과:
- Role 열거형을 사용하는 모든 JPA 엔티티가
@Enumerated(EnumType.STRING)설정을 사용합니다 (예: AuthUserProjection.java, Invitation.java 등)- 코드베이스에서
ordinal()메서드 호출이 없습니다- 코드베이스에서
EnumType.ORDINAL사용이 없습니다STRING 타입 직렬화는 열거형 상수 이름을 문자열로 저장하므로, 열거형의 순서 변경은 데이터 무결성이나 역직렬화에 영향을 주지 않습니다. 열거형 순서 재정렬은 안전합니다.
Likely an incorrect or invalid review comment.
src/main/java/com/sampoom/user/common/config/security/RoleHierarchyConfig.java (1)
13-26: 역할 계층이 Workspace 열거형과 일치합니다.검증 결과, Workspace 열거형의 7개 값(MD, SALES, INVENTORY, PRODUCTION, PURCHASE, HR, AGENCY)이 모두 일관되게 역할 계층의 ROLE_MD, ROLE_SALES, ROLE_INVENTORY, ROLE_PRODUCTION, ROLE_PURCHASE, ROLE_HR, ROLE_AGENCY와 매핑됩니다. JwtAuthFilter에서
"ROLE_" + workspace.name()패턴으로 권한을 생성하며, UserService에서도 이를 역으로 파싱하는 방식으로 일관성이 유지되고 있습니다.src/main/java/com/sampoom/user/api/auth/event/AuthUserEvent.java (1)
29-30: Kafka 메시지 역직렬화 호환성 확인됨 - 추가 검증 권장조사 결과,
AuthUserEvent컨슈머(AuthUserProjectionService.apply())는 이미workspace필드를 정상적으로 처리하고 있습니다. ObjectMapper는 누락된 필드를 자동으로null로 처리하며,AuthUserProjection.workspace컬럼이 NULL을 허용하므로 기존 메시지와의 호환성은 유지됩니다.다만 다음을 확인하세요:
- 외부 서비스 컨슈머: Kafka
auth-events토픽을 구독하는 다른 서비스나 시스템이 있다면, 이들이workspace필드 누락(NULL)에 대비했는지 검토 필요- 메시지 검증 전략:
@NotNull은 역직렬화 시 적용되지 않으므로, 필드 필수성을 강제해야 한다면apply()메서드에@Valid추가 검토src/main/java/com/sampoom/user/api/member/repository/SalesMemberRepository.java (1)
1-6: LGTM!표준 Spring Data JPA 리포지토리 패턴을 올바르게 따르고 있습니다. BaseMemberRepository를 상속하여 공통 메서드를 재사용하는 구조가 적절합니다.
src/main/java/com/sampoom/user/api/member/repository/InventoryMemberRepository.java (1)
1-6: LGTM!BaseMemberRepository를 확장하는 표준 리포지토리 구현입니다. 구조가 명확하고 일관성 있습니다.
src/main/java/com/sampoom/user/api/member/repository/PurchaseMemberRepository.java (1)
1-6: LGTM!다른 멤버 리포지토리들과 동일한 패턴을 따르고 있어 일관성이 유지됩니다.
src/main/java/com/sampoom/user/api/auth/event/AuthWarmupEvent.java (2)
4-4: LGTM!Workspace enum을 임포트하여 워크스페이스 기반 권한 모델을 지원합니다.
31-31: LGTM!AuthUserPayload에 workspace 필드를 추가하여 워크스페이스 정보를 포함시키는 것은 적절합니다. 이는 PR의 워크스페이스 권한 기능 추가 목표와 일치합니다.
src/main/java/com/sampoom/user/api/member/repository/HRMemberRepository.java (1)
1-6: LGTM!HRMember 엔티티에 대한 표준 리포지토리 구현입니다. BaseMemberRepository의 공통 메서드를 활용하는 구조가 적절합니다.
src/main/java/com/sampoom/user/api/member/repository/ProductionMemberRepository.java (1)
1-6: LGTM!ProductionMember를 위한 리포지토리가 올바르게 구현되었습니다. 다른 멤버 리포지토리들과 일관된 패턴을 유지합니다.
src/main/java/com/sampoom/user/common/entity/BaseMemberEntity.java (1)
15-15: LGTM!BaseEmployeeEntity에서 BaseMemberEntity로 명명 변경이 이루어졌습니다. 이는 팩토리/창고 기반 직원 모델에서 부서 기반 멤버 모델로의 전환을 명확하게 반영합니다.
src/main/java/com/sampoom/user/api/auth/entity/AuthUserProjection.java (1)
26-27: 변경사항 확인 완료.워크스페이스 필드가 올바르게 추가되었으며,
@Enumerated(EnumType.STRING)어노테이션으로 적절하게 매핑되어 있습니다.src/main/java/com/sampoom/user/api/invitation/entity/Invitation.java (1)
38-38: 변경사항 확인 완료.초대 엔티티가 워크스페이스 기반 모델로 올바르게 업데이트되었습니다.
src/main/java/com/sampoom/user/api/member/repository/MDMemberRepository.java (1)
5-6: 새 레포지토리 확인 완료.표준 Spring Data JPA 패턴을 따르며, 다른 멤버 레포지토리들과 일관된 구조를 가지고 있습니다.
src/main/java/com/sampoom/user/api/user/dto/response/UserLoginResponse.java (1)
21-21: 변경사항 확인 완료.워크스페이스 필드가
AuthUserProjection의 변경사항과 일관되게 업데이트되었습니다.src/main/java/com/sampoom/user/api/member/entity/HRMember.java (1)
22-27: 팩토리 메서드 패턴 확인 완료.다른 멤버 엔티티들과 일관된 생성 패턴을 따르고 있습니다.
src/main/java/com/sampoom/user/api/member/entity/SalesMember.java (1)
22-27: 팩토리 메서드 패턴 확인 완료.다른 멤버 엔티티들과 일관된 생성 패턴을 따르고 있습니다.
src/main/java/com/sampoom/user/api/auth/service/AuthUserProjectionService.java (2)
56-56: 워크스페이스 필드 전파 확인 완료.
AuthUserProjection에 워크스페이스 필드가 올바르게 전파되고 있습니다.Also applies to: 64-64, 83-83
59-59: sourceUpdatedAt 매핑 변경 확인 필요.새 프로젝션 생성 시
sourceUpdatedAt이p.getUpdatedAt()에서p.getCreatedAt()으로 변경되었습니다. 반면 업데이트 경로(Line 67)는 여전히p.getUpdatedAt()을 사용합니다. 이 비대칭이 의도된 것인지 확인이 필요합니다.새 프로젝션의 경우
createdAt을 사용하는 것이 맞는지, 아니면updatedAt을 일관되게 사용해야 하는지 검토해주세요.src/main/java/com/sampoom/user/api/agency/entity/AgencyEmployee.java (1)
15-15: BaseMemberEntity 확장 확인 완료.
BaseEmployeeEntity에서BaseMemberEntity로의 리팩토링이 올바르게 적용되었습니다.src/main/java/com/sampoom/user/api/member/repository/BaseMemberRepository.java (1)
11-18: 공통 멤버 조회 헬퍼 추가 👍
userId 기반 조회/존재 여부 헬퍼를 BaseMemberRepository에 올려둬서 각 부서별 Repository 중복이 사라졌습니다. 잘 반영되었습니다.src/main/java/com/sampoom/user/api/invitation/service/InvitationService.java (1)
38-46: 워크스페이스 기반 초대 저장 전환 확인
초대 생성 시 Role 대신 Workspace를 저장하도록 변경된 부분 확인했습니다. Invitation 엔티티/요청 DTO 개편과 일관성이 맞춰졌습니다.src/main/java/com/sampoom/user/api/user/controller/UserController.java (1)
50-56: Authentication 권한으로 프로필 조회 전환 확인
Workspace파라미터 없이 Authentication의 권한 컬렉션을 그대로 서비스에 전달하도록 바뀐 부분이 새로운 멀티 권한 모델과 맞물려 정상 동작합니다. 깔끔하게 정리되었습니다.src/main/java/com/sampoom/user/api/member/entity/ProductionMember.java (1)
7-8: 미사용 static import로 인한 컴파일 오류
Workspace.HR와Workspace.PRODUCTION을 static import했지만 클래스에서 전혀 사용하지 않아 컴파일이 실패합니다. 불필요한 static import를 제거해주세요.-import static com.sampoom.user.common.entity.Workspace.HR; -import static com.sampoom.user.common.entity.Workspace.PRODUCTION;Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/main/java/com/sampoom/user/common/config/security/.DS_Storeis excluded by!**/.DS_Storesrc/main/resources/.DS_Storeis excluded by!**/.DS_Store
📒 Files selected for processing (6)
src/main/java/com/sampoom/user/common/config/security/SecurityConfig.java(1 hunks)src/main/java/com/sampoom/user/common/entity/BaseMemberEntity.java(3 hunks)src/main/java/com/sampoom/user/common/entity/BaseTimeEntity.java(1 hunks)src/main/java/com/sampoom/user/common/entity/SoftDeleteEntity.java(1 hunks)src/main/java/com/sampoom/user/common/jwt/JwtProvider.java(1 hunks)src/main/java/com/sampoom/user/common/response/ErrorStatus.java(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/com/sampoom/user/common/entity/BaseTimeEntity.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/sampoom/user/common/entity/BaseMemberEntity.java
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 41
File: src/main/java/com/sampoom/user/api/agency/service/AgencyProjectionService.java:26-26
Timestamp: 2025-11-06T07:37:44.411Z
Learning: In the Sampoom-Management-Backend-User project, the team is performing an incremental refactoring from "agency" to "vendor" terminology. Field names and method calls are being updated first (e.g., AgencyId -> VendorId), while local variable names will be updated in a follow-up change. Do not suggest renaming local variables during this transition period.
📚 Learning: 2025-10-16T10:22:30.658Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 7
File: src/main/java/com/sampoom/backend/user/common/exception/ForbiddenException.java:15-17
Timestamp: 2025-10-16T10:22:30.658Z
Learning: In the com.sampoom.backend.user.common.exception package, exception class constructors accepting ErrorStatus parameter (e.g., ForbiddenException, BadRequestException, UnauthorizedException) do not perform null checks on the errorStatus parameter before calling its getters.
Applied to files:
src/main/java/com/sampoom/user/common/response/ErrorStatus.java
📚 Learning: 2025-10-16T10:32:56.782Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 7
File: src/main/java/com/sampoom/backend/user/jwt/JwtAuthFilter.java:34-37
Timestamp: 2025-10-16T10:32:56.782Z
Learning: In JWT authentication filters for the Sampoom Management Backend User service, when userId or role is null in the token claims, log a warning with both values using log.warn and call chain.doFilter(req, res) before returning, rather than throwing an exception. This allows the request to continue without authentication.
Applied to files:
src/main/java/com/sampoom/user/common/config/security/SecurityConfig.java
🪛 GitHub Actions: CI (use Gradle version)
src/main/java/com/sampoom/user/common/config/security/SecurityConfig.java
[error] 45-45: cannot find symbol: class CorsConfiguration
[error] 46-46: cannot find symbol: List (used in List.of(...))
[error] 47-47: cannot find symbol: List (used in List.of(...))
[error] 54-54: cannot find symbol: List (used in List.of(...))
[error] 55-55: cannot find symbol: List (used in List.of(...))
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (8)
src/main/java/com/sampoom/user/common/entity/SoftDeleteEntity.java (1)
23-27: 메서드 이름 변경이 완벽하게 적용되었으므로 승인합니다.
reactive()에서reactivation()으로의 메서드 이름 변경이 코드베이스 전체에서 올바르게 적용되었음을 확인했습니다:
- 기존
reactive()메서드는 완전히 제거됨- 새로운
reactivation()메서드가SoftDeleteEntity.java:23에 정의됨BaseMemberEntity.java:53에서 새로운 메서드명으로 호출 중- 레거시 메서드명에 대한 참조가 남아있지 않음
메서드명 변경이 더 명확한 의도를 표현하며, 모든 호출처가 이미 업데이트되어 있으므로 안전합니다.
src/main/java/com/sampoom/user/common/config/security/SecurityConfig.java (1)
36-36: 인증/인가 정책 변경이 안전하게 구현되었습니다.JwtAuthFilter 검증 결과:
- 모든 인증된 요청에서 권한이 적절히 설정됨 (서비스토큰은 SVC_*, 사용자토큰은 ROLE_와 workspace ROLE_)
- 토큰 없는 요청은 47번 라인에서 통과하지만 SecurityConfig의
.authenticated()에서 거부됨SecurityConfig 검증 결과:
- 28번 라인: POST
/internal/**은 별도로SVC_AUTH권한 확인- 29-34번 라인:
permitAll()이 먼저 실행되어 Swagger/invitations 허용- 36번 라인: 나머지 모든 요청은 인증 필수
결론:
.hasAuthority("ROLE_USER")→.authenticated()변경은 올바르며, workspace 기반 권한 모델을 지원합니다. 인증 규칙의 우선순위(특정규칙 → permitAll → 인증필수)도 정확합니다.src/main/java/com/sampoom/user/common/response/ErrorStatus.java (3)
21-21: LGTM!공개키 검증 및 로딩 실패를 처리하는 새로운 에러 상수가 적절하게 추가되었습니다. 메시지가 명확하고 에러 코드 범위도 적절합니다.
35-42: 워크스페이스 기반 NOT_FOUND 에러 상수 마이그레이션 검증 완료.검증 결과, 마이그레이션이 완벽하게 진행되었습니다:
- ✅ 제거된 상수(
NOT_FOUND_FACTORY_NAME,NOT_FOUND_WAREHOUSE_NAME,NOT_FOUND_USER_BY_WORKSPACE,NOT_FOUND_FACTORY_EMPLOYEE,NOT_FOUND_WAREHOUSE_EMPLOYEE,NOT_FOUND_AGENCY_EMPLOYEE)의 잔여 사용처 없음- ✅ 구 에러 코드 범위(104xx/114xx)에 대한 하드코딩된 참조 없음
- ✅ 새로운 에러 코드(13440-13447)가 ErrorStatus.java에만 정의됨
모든 워크스페이스(AGENCY, PRODUCTION, INVENTORY, PURCHASE, SALES, MD, HR)에 대한 NOT_FOUND 상수가 적절하게 정의되었으며, 기존 상수의 모든 사용처가 성공적으로 마이그레이션되었습니다.
13-14: 토큰 검증 상수 통합 및 공개키 검증 마이그레이션 완료 확인됨.검증 결과:
- ✅ 제거된 상수(
TOO_SHORT_SECRET_KEY,BLANK_TOKEN,NULL_TOKEN)의 잔여 참조 없음- ✅ 새로운 상수들이 모두 JwtProvider에서 적절히 사용 중:
NULL_BLANK_TOKEN(라인 58)SHORT_PUBLIC_KEY(라인 45)INVALID_PUBLIC_KEY(라인 26, 33, 52)- ✅ 에러 코드 범위 정상 관리 (12400-12401)
- ✅
BLANK_TOKEN_ROLE,NULL_TOKEN_ROLE은 별도의 목적(토큰 내 권한 정보)으로 구분되어 있음마이그레이션이 완전하고 올바르게 진행되었습니다.
src/main/java/com/sampoom/user/common/jwt/JwtProvider.java (3)
13-17: LGTM - RSA 공개키 처리에 필요한 임포트.RSA 공개키 검증을 위한 필수 임포트들이 정확하게 추가되었습니다.
56-71: 공개키 기반 토큰 검증 구현이 올바릅니다.검증 완료:
setSigningKey(publicKey)를 통한 공개키 검증은 RSA 비대칭 토큰 검증에 적합합니다.ErrorStatus.NULL_BLANK_TOKEN이 올바르게 정의되어 있습니다 (HttpStatus.BAD_REQUEST).- 만료된 토큰과 무효한 토큰에 대한 예외 처리가 적절합니다.
코드 변경 사항이 정상입니다.
38-54: RSA 공개키 검증 로직이 올바르게 구현되었습니다.
- Base64 디코딩 및 X.509 인코딩 스펙 사용이 정확합니다.
- 2048비트 최소 모듈러스 검증은 현재 보안 모범 사례입니다.
ErrorStatus.SHORT_PUBLIC_KEY가 올바르게 정의되어 있으며, 에러 메시지가 검증 로직과 일치합니다.- 예외 처리 로직이 적절합니다.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/main/java/com/sampoom/user/common/response/ErrorStatus.java (1)
16-16: INVALID_ROLE_TYPE과 INVALID_WORKSPACE_TYPE 구분이 추가되었으나, UserService.java에서 잘못 사용 중.이전 리뷰 코멘트에서 지적된 문제를 해결하기 위해
INVALID_ROLE_TYPE(code 11401)과INVALID_WORKSPACE_TYPE(code 12408)를 별도로 정의한 것은 좋습니다.그러나 UserService.java Line 93에서 여전히 워크스페이스 파싱 실패 시
INVALID_ROLE_TYPE을 사용하고 있습니다:try { workspace = req.getWorkspace(); } catch (IllegalArgumentException ex) { throw new BadRequestException(ErrorStatus.INVALID_ROLE_TYPE); // ← 잘못된 사용 }이는 워크스페이스 검증 오류에 "유효하지 않은 권한(role) 타입입니다"라는 메시지를 반환하여 클라이언트를 혼동시킵니다.
UserService.java Line 93을 수정하세요:
try { workspace = req.getWorkspace(); } catch (IllegalArgumentException ex) { - throw new BadRequestException(ErrorStatus.INVALID_ROLE_TYPE); + throw new BadRequestException(ErrorStatus.INVALID_WORKSPACE_TYPE); }Also applies to: 22-22
🧹 Nitpick comments (5)
src/main/java/com/sampoom/user/api/user/service/UserService.java (5)
10-11: 와일드카드 임포트를 명시적 임포트로 변경하세요.와일드카드 임포트(
import com.sampoom.user.api.member.entity.*)는 실제 사용하는 클래스가 불명확하고, 네임스페이스 충돌 가능성이 있어 가독성과 유지보수성을 저하시킵니다.다음과 같이 명시적으로 변경하세요:
-import com.sampoom.user.api.member.entity.*; -import com.sampoom.user.api.member.repository.*; +import com.sampoom.user.api.member.entity.ProductionMember; +import com.sampoom.user.api.member.entity.InventoryMember; +import com.sampoom.user.api.member.entity.PurchaseMember; +import com.sampoom.user.api.member.entity.SalesMember; +import com.sampoom.user.api.member.entity.MDMember; +import com.sampoom.user.api.member.entity.HRMember; +import com.sampoom.user.api.member.repository.ProductionMemberRepository; +import com.sampoom.user.api.member.repository.InventoryMemberRepository; +import com.sampoom.user.api.member.repository.PurchaseMemberRepository; +import com.sampoom.user.api.member.repository.SalesMemberRepository; +import com.sampoom.user.api.member.repository.MDMemberRepository; +import com.sampoom.user.api.member.repository.HRMemberRepository;
141-141: 코드 포맷팅 일관성 유지.
emp=e;에서 대입 연산자 앞뒤로 공백이 누락되었습니다.- emp=e; + emp = e;
75-145: switch 문의 중복 코드 패턴 리팩토링 고려.Lines 111-144의 switch 문에서 각 case는 거의 동일한 패턴을 반복합니다:
- 각 워크스페이스별 Member 객체 생성
save()호출emp변수에 할당이는 유지보수성을 저하시키고 실수 가능성을 높입니다.
팩토리 패턴 또는 전략 패턴을 고려하여 리팩토링할 수 있습니다:
// 예시: 팩토리 메서드 패턴 private BaseMemberEntity createAndSaveMember(Workspace workspace, Long userId, Position position) { return switch (workspace) { case PRODUCTION -> { ProductionMember e = ProductionMember.create(userId, position); yield prodRepo.save(e); } case INVENTORY -> { InventoryMember e = InventoryMember.create(userId, position); yield invenRepo.save(e); } // ... 나머지 케이스들 default -> throw new BadRequestException(ErrorStatus.INVALID_ROLE_TYPE); }; }그런 다음 Lines 111-144를:
emp = createAndSaveMember(workspace, req.getUserId(), req.getPosition());로 단순화할 수 있습니다.
201-250: 하드코딩된 브랜치명을 상수로 관리하세요.Lines 216, 222, 228, 234, 240, 246에서 브랜치명("생산 관리", "재고 관리" 등)이 하드코딩되어 있습니다. 이는 유지보수성을 저하시키고 다국어 지원 시 문제가 될 수 있습니다.
Workspace enum에 브랜치명을 추가하거나, 별도의 상수 클래스에서 관리하는 것을 고려하세요:
// Workspace enum에 추가 public enum Workspace { PRODUCTION("생산 관리"), INVENTORY("재고 관리"), PURCHASE("구매 관리"), SALES("판매 관리"), MD("기준 정보 관리"), HR("인사 관리"), AGENCY(null); // 동적으로 조회 private final String displayName; Workspace(String displayName) { this.displayName = displayName; } public String getDisplayName() { return displayName; } }그런 다음:
- branchName = "생산 관리"; + branchName = workspace.getDisplayName();
283-341: switch 문의 중복 코드 패턴 - switch expression 사용 고려.Lines 292-335의 switch 문은 각 case에서 동일한 패턴(조회 → 업데이트)을 반복합니다.
updateEmployeeStatus메서드(Lines 352-368)에서 사용한 switch expression 패턴을 여기에도 적용하면 더 간결하고 일관성 있는 코드가 됩니다.다음과 같이 리팩토링하세요:
- BaseMemberEntity emp; - switch (workspace) { - case AGENCY -> { - emp = agencyEmpRepo.findByUserId(userId) - .orElseThrow(() -> new NotFoundException(ErrorStatus.NOT_FOUND_EMPLOYEE_AGENCY)); - emp.updatePosition(newPosition); - } - // ... 나머지 케이스들 - default -> throw new BadRequestException(ErrorStatus.INVALID_ROLE_TYPE); - } + BaseMemberEntity emp = switch (workspace) { + case AGENCY -> agencyEmpRepo.findByUserId(userId) + .orElseThrow(() -> new NotFoundException(ErrorStatus.NOT_FOUND_EMPLOYEE_AGENCY)); + case PRODUCTION -> prodRepo.findByUserId(userId) + .orElseThrow(() -> new NotFoundException(ErrorStatus.NOT_FOUND_MEMBER_PRODUCTION)); + case INVENTORY -> invenRepo.findByUserId(userId) + .orElseThrow(() -> new NotFoundException(ErrorStatus.NOT_FOUND_MEMBER_INVENTORY)); + case PURCHASE -> purchaseRepo.findByUserId(userId) + .orElseThrow(() -> new NotFoundException(ErrorStatus.NOT_FOUND_MEMBER_PURCHASE)); + case SALES -> salesRepo.findByUserId(userId) + .orElseThrow(() -> new NotFoundException(ErrorStatus.NOT_FOUND_MEMBER_SALES)); + case MD -> mdRepo.findByUserId(userId) + .orElseThrow(() -> new NotFoundException(ErrorStatus.NOT_FOUND_MEMBER_MD)); + case HR -> hrRepo.findByUserId(userId) + .orElseThrow(() -> new NotFoundException(ErrorStatus.NOT_FOUND_MEMBER_HR)); + default -> throw new BadRequestException(ErrorStatus.INVALID_ROLE_TYPE); + }; + emp.updatePosition(newPosition);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/com/sampoom/user/api/user/service/UserService.java(5 hunks)src/main/java/com/sampoom/user/common/config/security/SecurityConfig.java(1 hunks)src/main/java/com/sampoom/user/common/response/ErrorStatus.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/sampoom/user/common/config/security/SecurityConfig.java
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-10-16T10:22:30.658Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 7
File: src/main/java/com/sampoom/backend/user/common/exception/ForbiddenException.java:15-17
Timestamp: 2025-10-16T10:22:30.658Z
Learning: In the com.sampoom.backend.user.common.exception package, exception class constructors accepting ErrorStatus parameter (e.g., ForbiddenException, BadRequestException, UnauthorizedException) do not perform null checks on the errorStatus parameter before calling its getters.
Applied to files:
src/main/java/com/sampoom/user/common/response/ErrorStatus.java
📚 Learning: 2025-10-14T11:21:55.975Z
Learnt from: vivivim
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 6
File: src/main/java/com/sampoom/backend/user/service/UserService.java:24-84
Timestamp: 2025-10-14T11:21:55.975Z
Learning: In the com.sampoom.backend.user project, use custom exceptions from com.sampoom.backend.user.common.exception package (BadRequestException, NotFoundException, etc.) instead of Java built-in exceptions (IllegalArgumentException, EntityNotFoundException) for consistent error handling and proper HTTP status code mapping in service layers.
Applied to files:
src/main/java/com/sampoom/user/api/user/service/UserService.java
📚 Learning: 2025-11-06T07:37:44.411Z
Learnt from: Lee-Jong-Jin
Repo: 33-Auto/Sampoom-Management-Backend-User PR: 41
File: src/main/java/com/sampoom/user/api/agency/service/AgencyProjectionService.java:26-26
Timestamp: 2025-11-06T07:37:44.411Z
Learning: In the Sampoom-Management-Backend-User project, the team is performing an incremental refactoring from "agency" to "vendor" terminology. Field names and method calls are being updated first (e.g., AgencyId -> VendorId), while local variable names will be updated in a follow-up change. Do not suggest renaming local variables during this transition period.
Applied to files:
src/main/java/com/sampoom/user/api/user/service/UserService.java
🧬 Code graph analysis (1)
src/main/java/com/sampoom/user/api/user/service/UserService.java (3)
src/main/java/com/sampoom/user/api/user/service/UserInfoService.java (1)
Service(31-245)src/main/java/com/sampoom/user/common/jwt/JwtAuthFilter.java (1)
Slf4j(26-135)src/main/java/com/sampoom/user/api/user/controller/UserController.java (1)
Slf4j(30-145)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (2)
src/main/java/com/sampoom/user/common/response/ErrorStatus.java (1)
36-43: 새로운 NOT_FOUND_MEMBER_ 상수들이 잘 정의되었습니다.*이전의 팩토리/창고 기반 모델에서 부서 기반 멤버 모델로의 마이그레이션에 맞춰 에러 상수들이 적절히 업데이트되었습니다. 각 워크스페이스(PRODUCTION, INVENTORY, PURCHASE, SALES, MD, HR, AGENCY)에 대한 NOT_FOUND 에러가 명확하게 정의되어 있으며, 에러 코드도 순차적으로(13440-13447) 잘 할당되었습니다.
src/main/java/com/sampoom/user/api/user/service/UserService.java (1)
176-187: 리뷰 코멘트의 분석이 실제 코드 구조와 일치하지 않습니다.코드 검증 결과:
JWT 토큰 구조:
JwtAuthFilter.java:92에서 확인했듯이 JWT claims에는 단일 workspace만 포함되며, 다중 워크스페이스를 가지지 않습니다.authorities 컬렉션 구성:
JwtAuthFilter.java:117-119에서 정확히 2개의 권한만 추가됩니다:
"ROLE_" + role.name()(예: ROLE_ADMIN)"ROLE_" + workspace.name()(예: ROLE_MD, ROLE_SALES)getMyProfile의 루프 동작: 현재 로직은 의도대로 작동합니다:
- 첫 번째 순회: roleAuthority (ROLE_ADMIN 등) → Workspace enum과 매칭 실패 →
IllegalArgumentException무시- 두 번째 순회: workspaceAuthority (ROLE_MD 등) → Workspace enum과 매칭 성공 → break
토큰-권한 일치: JWT 필터와 getMyProfile 메서드 모두 단일 workspace를 처리하므로 불일치가 없습니다.
현재 코드는 설계 의도와 일치하며, 리뷰 코멘트의 주요 지적 사항들은 코드 분석 오류입니다.
Likely an incorrect or invalid review comment.
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.
📣 Related Issue
📝 Summary
Workspace 권한 추가
Role: ADMIN/USER
Workspace: PRODUCTION/INVENTORY/PURCHASE/SALES/MD/HR/AGENCY
Role, Workspace 복수 권한으로 변경
🙏 Question & PR point
📬 Reference
Summary by CodeRabbit
릴리스 노트
New Features
Refactor
Chores