Feature: phone number 및 마케팅 입력 기능 추가 #311
Conversation
|
Caution Review failedThe pull request is closed. Walkthrough관리자 CORS 도메인 추가, User 엔티티에 전화번호/동의/감사 필드 및 빌더 변경, JWT 액세스 토큰에 phoneEntered/marketingAgree 클레임 추가 및 호출부 수정, 리프레시/로그인 흐름과 선택 정보 업데이트 API(컨트롤러/서비스/DTO) 추가, Store 컬럼 길이 확장. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant FE as Frontend
participant AS as Auth Server (User API)
participant Repo as UserRepository
participant Jwt as JwtUtil
participant TS as TokenService
rect rgba(200,230,255,0.25)
note over User,FE: OAuth2 로그인 성공 및 리디렉션
User->>FE: OAuth2 Redirect
FE->>AS: GET /login/success
AS->>Repo: findOrCreate User (신규 시 phoneNumber="", phoneEntered=false, isMarketingAgree=false, createdAt/updatedAt=now)
AS->>Jwt: createAccessToken(tokenCategory, userId, role, phoneEntered, marketingAgree, exp=60min)
Jwt-->>AS: accessToken
AS-->>FE: Redirect with accessToken (cookie refreshToken)
end
sequenceDiagram
autonumber
actor FE
participant AS as Auth Server (User API)
participant Repo as UserRepository
participant TS as TokenService
participant Jwt as JwtUtil
rect rgba(200,255,200,0.22)
note over FE,AS: 리프레시 토큰으로 액세스 재발급
FE->>AS: POST /token/refresh (cookie: refreshToken)
AS->>TS: parse userId from refresh
TS-->>AS: userId
AS->>Repo: findById(userId)
Repo-->>AS: User
AS->>TS: validateToken(refreshToken, userId)
TS-->>AS: valid
AS->>Jwt: createAccessToken(..., phoneEntered, marketingAgree, exp=60min)
Jwt-->>AS: accessToken
AS-->>FE: 200 { accessToken }
end
sequenceDiagram
autonumber
actor User
participant FE as Frontend
participant UC as UserController
participant US as UserService
participant Repo as UserRepository
participant Jwt as JwtUtil
rect rgba(255,240,200,0.22)
note over User,FE: 선택 정보 업데이트 흐름
User->>FE: 입력(phoneNumber, consent)
FE->>UC: PUT /users/optional-info (Authorization: Bearer)
UC->>US: putOptional(userId, phoneNumber, consent)
US->>Repo: existsByPhoneNumberAndIdNot(phoneNumber, userId)
Repo-->>US: false
US->>Repo: findById(userId)
Repo-->>US: User
US->>US: setPhoneNumberAndMarkEntered / setIsMarketingAgree
US->>Jwt: createAccessToken(..., true, consent, exp=60min)
Jwt-->>US: newAccessToken
US-->>UC: newAccessToken
UC-->>FE: 200 { token }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (5)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (14)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/config/security/CorsConfig.java (1)
17-23: setAllowCredentials 중복 호출 제거Line 17과 22에서 중복 호출됩니다. 기능 영향은 없으나 불필요합니다.
권장 변경:
- config.setAllowCredentials(true); // 쿠키 포함 허용nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/dto/UserUpdateRequest.java (1)
7-9: 전화번호 입력 포맷 유연성(하이픈 유무 허용/정규화) 제안FE가 하이픈 없는 숫자만 보내는 경우를 대비해 하이픈 유무 모두 허용하고 서비스에서 표준 포맷으로 정규화하는 편이 운영 안정적입니다.
예시:
- @Pattern(regexp = "^010-\\d{4}-\\d{4}$", message = "휴대폰 번호는 010-0000-0000 형식이어야 합니다.") + @Pattern(regexp = "^010-?\\d{4}-?\\d{4}$", message = "휴대폰 번호는 010-0000-0000 또는 01000000000 형식이어야 합니다.")nowait-app-user-api/src/main/java/com/nowait/applicationuser/security/jwt/JwtUtil.java (1)
26-27: 신규 클레임 노출 범위 재확인(최소권한·최소정보 원칙)
phoneEntered,marketingAgree를 액세스 토큰에 포함하면 FE/서드파티가 파싱할 수 있습니다. 진짜 클라이언트에 필요한 경우만 포함하고, 아니라면 서버 조회로 대체해 토큰 부피/노출을 줄이는 것을 권장합니다.Also applies to: 32-33
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerSignupRequestDto.java (2)
48-49: createdAt/updatedAt 수동 세팅 대신 감사(Auditing) 사용 권장BaseTimeEntity를 사용 중이면 JPA Auditing(@CreatedDate/@LastModifiedDate)로 관리하는 것이 일관·신뢰성 측면에서 낫습니다. DTO→엔티티 매핑에서 현재시각을 박아 넣는 패턴은 테스트/타임존 이슈를 유발할 수 있습니다.
권장 변경:
- .createdAt(LocalDateTime.now()) - .updatedAt(LocalDateTime.now())
43-44: phoneNumber("") 사용 지양 및 null 처리 권장
- ManagerSignupRequestDto.toEntity(...)와 CustomOAuth2UserService에서
.phoneNumber("") → .phoneNumber(null)로 변경// nowait-app-admin-api/src/.../ManagerSignupRequestDto.java:43 - .phoneNumber("") + .phoneNumber(null) // nowait-app-user-api/src/.../CustomOAuth2UserService.java:54 - .phoneNumber("") + .phoneNumber(null)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/OAuth2LoginSuccessHandler.java (1)
47-49: JWT TTL 상수화
OAuth2LoginSuccessHandler에서 accessToken 만료시간(60 × 60 × 1000L)이 하드코딩되어 있습니다. UserService, TokenController 등 여러 호출부에서 동일값을 사용하니, 상수 혹은 설정으로 중앙 관리하세요.nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/CustomOAuth2UserService.java (1)
52-64: 초기값 설정 개선: phoneNumber null 처리·now 변수 재사용·Auditing 활용
.phoneNumber("")대신 null 또는 빌더에서 미설정 (빈 문자열은 유니크·중복 검사 혼선)LocalDateTime.now()를 한 번만 호출해 변수에 할당 후 재사용createdAt/updatedAt수동 설정 제거 (BaseTimeEntity에 JPA Auditing(@CreatedDate/@LastModifiedDate, @EntityListeners) 이미 적용됨)nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/service/UserService.java (2)
32-34:LocalDateTime.now()중복 호출 제거동일 트랜잭션 내 타임스탬프 일관성과 미세 성능을 위해 한 번만 구해 재사용하세요.
- user.setPhoneNumberAndMarkEntered(phoneNumber, LocalDateTime.now()); - user.setIsMarketingAgree(consent, LocalDateTime.now()); + final var now = LocalDateTime.now(); + user.setPhoneNumberAndMarkEntered(phoneNumber, now); + user.setIsMarketingAgree(consent, now);
37-41: 토큰 TTL 하드코딩 제거 및 중앙관리, boolean 플래그 전달 순서 일관화
- UserService, OAuth2LoginSuccessHandler, TokenController 등 여러 곳에서
60 * 60 * 1000L과 같은 만료 시간을 반복 중입니다. application.yml 설정 또는JwtUtil내부static final상수로 통합하세요.Boolean.TRUE.equals(user.getPhoneEntered())및Boolean.TRUE.equals(user.getIsMarketingAgree())인수 순서를 호출부 전역에서 동일하게 유지해야 합니다.nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java (4)
98-101: 닉네임 변경 시 updatedAt 동기화 누락다른 변경 메서드는 updatedAt을 갱신하지만 닉네임은 제외되어 일관성이 깨집니다.
public void updateNickname(String nickname){ this.nickname = nickname; + this.updatedAt = LocalDateTime.now(); }
107-116: 세터에서 수동 시간 주입(ts) 지양서비스 단에서 시간을 주입받는 패턴은 테스트/호출자 책임을 늘립니다. Auditing으로 통일하면 ts 파라미터가 불필요합니다.
리팩터링 방향:
- 메서드 시그니처에서 LocalDateTime ts 제거
- updatedAt 갱신은 Auditing에 위임
52-53: 불리언 필드 네이밍 컨벤션 정리 제안isMarketingAgree(Boolean) 형태는 Lombok 게터가 getIsMarketingAgree()로 생성되어 어색합니다. marketingAgree(boolean) 또는 marketingConsented(boolean)와 같이 단순화하면 가독성이 좋아집니다(토큰 생성부에서도 .isMarketingAgree() 형태 사용 가능).
Also applies to: 55-60
63-66: updatedAt 관리 일원화
BaseTimeEntity에 이미 @EntityListeners(AuditingEntityListener.class)와 @CreatedDate가 설정되어 있으므로, User 엔티티의updatedAt필드는 @LastModifiedDate Auditing 전담으로 전환하고, 빌더/세터에서 수동 설정(.updatedAt(...))을 제거하세요.nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/controller/TokenController.java (1)
49-52: 검증 순서 최적화: DB 조회는 토큰 검증 후로 이동현재는 사용자 조회가 토큰 유효성 검사 전에 실행됩니다. 위조/만료 토큰에 불필요한 DB 부하가 발생합니다. validateToken 통과 후 사용자 로드를 권장합니다.
리팩터링 아이디어:
- try에서 userId/role만 추출
- if (!validateToken) → 즉시 401
- 이후 userRepository.findById 호출
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (12)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/config/security/CorsConfig.java(1 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerSignupRequestDto.java(2 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/CustomOAuth2UserService.java(2 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/oauth/oauth2/OAuth2LoginSuccessHandler.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/security/jwt/JwtUtil.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/controller/TokenController.java(2 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/controller/UserController.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/dto/UserUpdateRequest.java(1 hunks)nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/service/UserService.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/store/entity/Store.java(1 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java(5 hunks)nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/repository/UserRepository.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java (1)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerSignupRequestDto.java (1)
Getter(16-53)
🔇 Additional comments (4)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/config/security/CorsConfig.java (1)
18-18: Apex 도메인(https://nowait-admin.com)도 허용 대상인지 확인 필요www 서브도메인만 추가되어 apex 도메인 접근 시 CORS 실패 가능성이 있습니다. 운영에서 apex를 쓰지 않는지 확인 후 필요하면 함께 허용해주세요.
권장 변경 예:
- config.setAllowedOrigins(List.of("http://localhost:5173","http://localhost:63342", "https://no-wait-fe-nowait-admin-2y5y.vercel.app", "https://nowait-admin.co.kr", "https://www.nowait-admin.com")); + config.setAllowedOrigins(List.of( + "http://localhost:5173", + "http://localhost:63342", + "https://no-wait-fe-nowait-admin-2y5y.vercel.app", + "https://nowait-admin.co.kr", + "https://nowait-admin.com", + "https://www.nowait-admin.com" + ));nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/store/entity/Store.java (1)
43-45: DDL 마이그레이션 동기화 확인(길이 250/500 확장)컬럼 길이 확장은 운영 DB에 ALTER가 필요합니다. Flyway/Liquibase 스크립트가 본 PR/동시 PR에 포함됐는지 확인해주세요. API 입력 검증(요청 DTO/Swagger 제약)도 새 길이에 맞게 조정되었는지 함께 점검이 필요합니다.
Also applies to: 49-51
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/user/dto/ManagerSignupRequestDto.java (1)
40-50: 비밀번호 인코딩 보장 여부 확인
toEntity()에 원문 패스워드가 그대로 전달됩니다. 저장 직전에 반드시 해시(BCrypt 등)됨을 보장하는 서비스/리졸버 레이어가 있는지 확인해주세요. 없다면 이 레이어에서 인코딩을 강제하세요.nowait-app-user-api/src/main/java/com/nowait/applicationuser/token/controller/TokenController.java (1)
56-59: 토큰 페이로드 Boolean 파생값 처리 OKBoolean.TRUE.equals(...)로 NPE 방지하며 새 클레임을 구성한 부분은 적절합니다.
Also applies to: 68-70
...-app-user-api/src/main/java/com/nowait/applicationuser/token/controller/TokenController.java
Show resolved
Hide resolved
...it-app-user-api/src/main/java/com/nowait/applicationuser/user/controller/UserController.java
Show resolved
Hide resolved
nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/dto/UserUpdateRequest.java
Outdated
Show resolved
Hide resolved
| if (userRepository.existsByPhoneNumber(phoneNumber)) { | ||
| throw new IllegalArgumentException("Phone number already in use"); | ||
| } | ||
|
|
There was a problem hiding this comment.
전화번호 중복 검사에서 자기 자신 제외 누락
자기 자신의 기존 번호를 동일하게 제출해도 중복으로 간주되어 예외가 발생합니다. id 제외 쿼리를 사용하세요. 또한 경합 조건을 근본적으로 막기 위해 DB 유니크 인덱스를 병행해야 합니다.
- if (userRepository.existsByPhoneNumber(phoneNumber)) {
+ if (userRepository.existsByPhoneNumberAndIdNot(phoneNumber, userId)) {
throw new IllegalArgumentException("Phone number already in use");
}DB 권장: phone_number 유니크 인덱스 추가 및 DataIntegrityViolationException을 도메인 예외로 매핑.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (userRepository.existsByPhoneNumber(phoneNumber)) { | |
| throw new IllegalArgumentException("Phone number already in use"); | |
| } | |
| // 기존 사용자일 경우, 자기 자신(id)이 아닌 레코드에서만 전화번호 중복 검사 | |
| if (userRepository.existsByPhoneNumberAndIdNot(phoneNumber, userId)) { | |
| throw new IllegalArgumentException("Phone number already in use"); | |
| } |
🤖 Prompt for AI Agents
nowait-app-user-api/src/main/java/com/nowait/applicationuser/user/service/UserService.java
around lines 28-31: the current phone-number uniqueness check treats submitting
the same existing number as a duplicate and throws even when it belongs to the
same user; change the check to exclude the current user's id (use repository
method like existsByPhoneNumberAndIdNot(phoneNumber, id) or add a repository
query that ignores the provided id) so updates that keep the same number don't
raise IllegalArgumentException, and concurrently add a unique constraint/index
on the phone_number column in the database and catch
DataIntegrityViolationException at the service/adapter layer to map it to a
domain-level exception for race-condition safety.
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java
Show resolved
Hide resolved
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/entity/User.java
Show resolved
Hide resolved
| public interface UserRepository extends JpaRepository<User, Long> { | ||
| Optional<User> findByEmail(String email); | ||
| Optional<User> findByNickname(String nickName); | ||
| boolean existsByPhoneNumber(String phoneNumber); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
전화번호 중복 검사: 자기 자신 업데이트 시 오탐 방지 메서드 추가 필요
현재 existsByPhoneNumber만 있으면 사용자가 자신의 번호로 업데이트할 때도 중복으로 판단될 수 있습니다. id를 제외하는 파생 쿼리를 추가하고 서비스에서 이를 사용해주세요. 또한 DB 레벨 유니크 인덱스로 최종 일관성을 보장하는 것을 권장합니다.
적용 예시:
public interface UserRepository extends JpaRepository<User, Long> {
Optional<User> findByEmail(String email);
Optional<User> findByNickname(String nickName);
boolean existsByPhoneNumber(String phoneNumber);
+ boolean existsByPhoneNumberAndIdNot(String phoneNumber, Long id);
}DB 권장: @Column(unique = true) + 마이그레이션으로 phone_number 컬럼에 유니크 인덱스 추가.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| boolean existsByPhoneNumber(String phoneNumber); | |
| public interface UserRepository extends JpaRepository<User, Long> { | |
| Optional<User> findByEmail(String email); | |
| Optional<User> findByNickname(String nickName); | |
| boolean existsByPhoneNumber(String phoneNumber); | |
| boolean existsByPhoneNumberAndIdNot(String phoneNumber, Long id); | |
| } |
🤖 Prompt for AI Agents
In
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/repository/UserRepository.java
around line 14, the repository only exposes existsByPhoneNumber(String) which
will false-positive when a user updates their own phone; add a derived query
method like existsByPhoneNumberAndIdNot(String phoneNumber, Long id) (or the
inverse parameter order used in your project) so service layer can check
uniqueness excluding the current user id, update service update flows to call
the new method when validating phone changes, and add a DB-level unique
constraint for phone_number (e.g., @Column(unique=true) plus a migration to
create a unique index) to guarantee final consistency.
작업 요약
Issue Link
#310
문제점 및 어려움
해결 방안
Reference
Summary by CodeRabbit