Skip to content

Conversation

@Jjiggu
Copy link
Contributor

@Jjiggu Jjiggu commented Sep 5, 2025

작업 요약

  • 대기열 조회 휴대폰번호 필드 추가

Issue Link

#310

문제점 및 어려움

해결 방안

Reference

Summary by CodeRabbit

  • 신기능

    • 대기자 목록/상세에서 사용자 전화번호가 추가로 표시됩니다.
  • 성능 개선

    • 대기자 정보 조회 시 사용자 기본정보를 일괄로 미리 가져오도록 개선해 목록 로딩 속도가 향상되었습니다.
  • API 변경

    • 대기자 응답에 phoneNumber 필드가 추가되었습니다. 기존 연동은 호환되지만, 새 필드 수용을 권장합니다.

@Jjiggu Jjiggu self-assigned this Sep 5, 2025
@Jjiggu Jjiggu added the refactor 리팩토링 label Sep 5, 2025
@coderabbitai
Copy link

coderabbitai bot commented Sep 5, 2025

Walkthrough

대기 사용자 응답 DTO에 phoneNumber 필드를 추가하고, Redis/서비스 경로에서 전화번호를 전달하도록 ReservationService를 수정했으며, 사용자 다건 조회를 위해 UserRepository에 findByIdIn(Collection) 메서드를 추가했습니다.

Changes

Cohort / File(s) Summary
DTO 업데이트
nowait-app-admin-api/.../reservation/dto/WaitingUserResponse.java
public 필드 phoneNumber 추가(스키마 주석 포함). fromEntity에서 사용자 전화번호 매핑. fromRedis 시그니처에 phoneNumber 인자 추가 및 빌더 설정.
서비스 로직 변경
nowait-app-admin-api/.../reservation/service/ReservationService.java
사용자 닉네임 일괄 조회 방식을 findByIdIn으로 변경하고 nicknameMap, phoneNumberMap 구성. WaitingUserResponse.fromRedis 호출부에 phoneNumber 전달. 포맷팅 소폭 변경.
리포지토리 확장
nowait-domain/domain-core-rdb/.../user/repository/UserRepository.java
List<User> findByIdIn(Collection<Long> ids) 메서드 추가(관련 import 포함). 기존 메서드 변경 없음.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Admin as Admin API Caller
    participant RS as ReservationService
    participant UR as UserRepository
    participant R as Redis

    Admin->>RS: 요청(대기 사용자 목록)
    RS->>R: 예약 대기 엔트리 조회
    R-->>RS: 대기 레코드들(userId, ...)

    RS->>UR: findByIdIn(userIds)
    UR-->>RS: List<User>(id, nickname, phoneNumber)

    RS->>RS: nicknameMap/phoneNumberMap 생성
    RS->>RS: WaitingUserResponse.fromRedis(..., phoneNumber, ...)
    RS-->>Admin: 응답(전화번호 포함)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • HyemIin
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/#310-phoneNumber

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot requested a review from HyemIin September 5, 2025 12:12
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java (2)

110-118: 인증/인가 누락: 전체 대기열 조회에 권한 검증이 없습니다

getAllWaitingUserDetails는 MemberDetails 인자를 받지 않고 authorize 호출도 없습니다. 전화번호 등 PII가 포함되므로 권한 검증이 필수입니다.

권장 수정 스케치(요약):

@Transactional(readOnly = true)
public List<WaitingUserResponse> getAllWaitingUserDetails(Long storeId, MemberDetails member) {
  authorize(storeId, member);
  ...
}

엔드포인트/호출부도 함께 업데이트해 주세요.


147-158: executePipelined 호출 시 Serializer 미지정으로 인한 ClassCastException 위험 수정
RedisTemplate.executePipelined 호출에 결과 역직렬화용 StringSerializer를 두번째 인자로 넘기지 않아 raw byte[]가 반환되고, 이후 String으로 캐스팅할 때 ClassCastException이 발생할 수 있습니다.

  • 수정 대상:
    • nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java (147행)
    • nowait-app-user-api/src/main/java/com/nowait/applicationuser/reservation/repository/WaitingUserRedisRepository.java (142행)
    • nowait-app-user-api/src/main/java/com/nowait/applicationuser/reservation/service/ReservationService.java (285행)
      각 executePipelined 호출에 , redisTemplate.getStringSerializer()를 추가하고, 키·필드 직렬화도 redisTemplate.getStringSerializer().serialize(...)로 일관 처리하세요. 테스트(mock)도 변경된 시그니처에 맞춰 갱신해야 합니다.
🧹 Nitpick comments (7)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/repository/UserRepository.java (1)

16-16: 대량 조회 시 시그니처·프로젝션 최적화 제안

  • ids에 중복이 들어올 수 있으므로 Set 사용을 고려하세요.
  • nickname/phoneNumber만 필요하다면 인터페이스 기반 프로젝션으로 불필요한 컬럼 로딩을 줄일 수 있습니다.

가능한 대안 예시:

public interface UserBrief {
  Long getId();
  String getNickname();
  String getPhoneNumber();
}
// Repository
List<UserBrief> findByIdIn(Collection<Long> ids);
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/dto/WaitingUserResponse.java (2)

21-23: 전화번호 PII 노출 – 정책 확인 및 마스킹 고려

관리자 API라도 기본 노출 범위를 최소화하는 게 안전합니다. 필요 권한 확인 및 기본 마스킹(예: 010-****-5678) 또는 옵트인 필드를 검토해 주세요.

권한 체인이 적절한지(엔드포인트에서 관리자 권한 요구) 확인 부탁드립니다.


53-53: Null/포맷 처리

DB에 포맷이 섞여 있을 경우를 대비해 표준 포맷(E.164 또는 일관된 하이픈 규칙)으로 정규화해 넣는 것을 권장합니다. null이면 빌더에서 제외되도록 처리해도 좋습니다.

nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java (4)

129-138: 대량 사용자 프리페치 로직 OK, 소소한 개선 여지

  • userIdLongs에 비정상 값이 섞일 경우 NumberFormatException이 날 수 있습니다. try-catch로 건너뛰거나 filter를 권장합니다.
  • 닉네임/전화번호 두 개의 Map 대신 작은 레코드/DTO 한 개의 Map으로 묶으면 조회·전달이 단순해집니다.

예:

record UserBrief(String nickname, String phoneNumber) {}
Map<String, UserBrief> userMap = new HashMap<>(userList.size());
for (User u : userList) userMap.put(u.getId().toString(), new UserBrief(u.getNickname(), u.getPhoneNumber()));

182-188: 'Unknown' 하드코딩 노출 대신 null 반환/마스킹 권장

API 응답에 "Unknown"이 노출되면 클라이언트 처리에 부담을 줍니다. null을 반환하고 스웨거에 nullable로 표시하거나 서버에서 마스킹된 기본값을 일관되게 내려주세요.

권장 변경:

- String phoneNumber = phoneNumberMap.getOrDefault(userId, "Unknown");
+ String phoneNumber = phoneNumberMap.get(userId); // 없으면 null 반환 → 응답에서 미포함 또는 마스킹 처리
- WaitingUserResponse.fromRedis(reservationId, userId, phoneNumber, partySize, userName, createdAt,
+ WaitingUserResponse.fromRedis(reservationId, userId, phoneNumber, partySize, userName, createdAt,
   calledAt, status, score);

280-283: 빈 catch 블록 제거하고 최소 경고 로그 남기기

예외 삼키면 운영 이슈 파악이 어렵습니다. 적어도 WARN 로그를 남겨주세요.

다음 변경을 권장합니다(클래스에 @slf4j 또는 Logger 추가 전제):

- } catch (Exception ignore) {
- }
+ } catch (Exception e) {
+   log.warn("removeActiveMember failed. storeId={}, userId={}, reservationNumber={}",
+       storeId, userId, reservationNumber, e);
+ }

114-114: System.out.println 제거

운영 로그에 불필요한 출력입니다. 로거 DEBUG 수준으로 전환하거나 삭제하세요.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 20b3512 and 4b5f020.

📒 Files selected for processing (3)
  • nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/dto/WaitingUserResponse.java (2 hunks)
  • nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java (4 hunks)
  • nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/repository/UserRepository.java (2 hunks)
🔇 Additional comments (4)
nowait-domain/domain-core-rdb/src/main/java/com/nowait/domaincorerdb/user/repository/UserRepository.java (1)

3-4: LGTM – 필요한 컬렉션 import 추가

변경 목적에 부합합니다.

nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/dto/WaitingUserResponse.java (2)

67-67: LGTM – 새 필드 매핑 적절

빌더 매핑이 일관됩니다.


61-64: fromRedis 호출부 검증 완료
rg -nP --type=java '\bWaitingUserResponse\.fromRedis\s*\(' -C2 결과 ReservationService 내 호출부에서만 사용되며, 해당 호출문에 phoneNumber 인자가 정상적으로 반영된 것을 확인했습니다. 추가 수정 불필요합니다.

nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/reservation/service/ReservationService.java (1)

9-9: LGTM – HashMap 도입

대량 매핑 용도에 적합합니다.

@Jjiggu Jjiggu merged commit 8708e8a into develop Sep 5, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor 리팩토링

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants