Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings. Walkthrough이력서 기반 면접 질문 생성 기록 목록을 조회하는 기능이 추가되었습니다. 새로운 API 엔드포인트, 다수의 DTO, 리포지토리 쿼리 메서드, 테스트 픽스처 및 문서화가 도입되었으며, 기존 상태 응답 타입이 변경되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller as ResumeBasedInterviewController
participant Service as ResumeBasedInterviewService
participant Repository as ResumeQuestionGenerationRepository
participant Database as DB
Client->>Controller: GET /questions/generations?state=COMPLETED&page=0&size=20
Controller->>Controller: 인증 정보 검증
Controller->>Service: findMyQuestionGenerations(memberId, state, pageable)
Service->>Service: 상태 필터링 결정 (null이면 기본값)
Service->>Repository: findByMemberIdAndState(In)(memberId, states, pageable)
Repository->>Database: SELECT 쿼리 (EntityGraph로 resume/portfolio 함께)
Database-->>Repository: Page<ResumeQuestionGeneration>
Repository-->>Service: 페이지 데이터 반환
Service->>Service: ResumeQuestionGenerationResponse.from() 매핑
Service->>Service: ResumeQuestionGenerationPageResponse.of() 래핑
Service-->>Controller: ResumeQuestionGenerationPageResponse
Controller-->>Client: JSON 응답 (data, currentPage, totalCount, totalPages, hasNext)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 분 Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
Summary of ChangesHello @unifolio0, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 풀 리퀘스트는 사용자가 자신의 이력서 기반 면접 질문 생성 기록을 조회할 수 있는 기능을 추가합니다. 이 기능은 페이지네이션과 상태 필터링을 지원하여 사용자가 특정 상태의 기록을 효율적으로 검색하고 관리할 수 있도록 합니다. 이를 통해 사용자 경험을 개선하고, 질문 생성 과정의 투명성을 높입니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. 데이터 흐름 따라, 기록 쌓여 지혜 되니, 새로운 길 열다. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| log.error("Exception :: state: {}, message: {}, stackTrace: ", HttpStatus.INTERNAL_SERVER_ERROR, | ||
| e.getMessage(), e); |
There was a problem hiding this comment.
로그 메시지의 state 필드에 HttpStatus.INTERNAL_SERVER_ERROR를 출력하고 있어 혼란을 줄 수 있습니다. 예외 객체 e를 로깅하면 메시지와 스택 트레이스가 모두 포함되므로, 로그 메시지를 더 간결하게 수정하는 것을 제안합니다.
log.error("Exception in callbackInterviewProceedBedrockFlow", e);| log.error("Exception :: state: {}, message: {}, stackTrace: ", HttpStatus.INTERNAL_SERVER_ERROR, | ||
| e.getMessage(), e); |
There was a problem hiding this comment.
로그 메시지의 state 필드에 HttpStatus.INTERNAL_SERVER_ERROR를 출력하고 있어 혼란을 줄 수 있습니다. 예외 객체 e를 로깅하면 메시지와 스택 트레이스가 모두 포함되므로, 로그 메시지를 더 간결하게 수정하는 것을 제안합니다. 이 파일의 다른 예외 로깅과 일관성을 맞추는 것이 좋습니다.
log.error("Exception in callbackAnswerFeedbackBedrockFlow", e);| List<ResumeQuestionGenerationState> defaultStates = List.of( | ||
| ResumeQuestionGenerationState.PENDING, | ||
| ResumeQuestionGenerationState.COMPLETED | ||
| ); |
There was a problem hiding this comment.
defaultStates 리스트가 getResumeQuestionGenerations 메소드가 호출될 때마다 생성되고 있습니다. 이 리스트는 변경되지 않으므로 클래스 레벨에서 private static final List<ResumeQuestionGenerationState> DEFAULT_FILTER_STATES = List.of(ResumeQuestionGenerationState.PENDING, ResumeQuestionGenerationState.COMPLETED); 와 같이 상수로 선언하여 재사용하는 것을 고려해보세요. 이렇게 하면 불필요한 객체 생성을 피하고 코드의 의도를 더 명확하게 할 수 있습니다.
Test Results 40 files 40 suites 58s ⏱️ Results for commit 801c6ba. ♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In
`@api/src/main/java/com/samhap/kokomen/interview/service/InterviewProceedBedrockFlowAsyncService.java`:
- Around line 151-152: The log line in InterviewProceedBedrockFlowAsyncService
mistakenly labels a status value as "state": update the log call so labels and
values match by either passing the actual state variable (e.g.,
interviewProceedStateKey or the real state value) in place of
HttpStatus.INTERNAL_SERVER_ERROR when you intend to log state, or change the
label from "state" to "status" if you intend to log the HTTP status; keep the
exception parameter (e) as the last argument and preserve the log.error(...)
signature so message, substituted value, and exception are correctly ordered.
- Around line 288-289: The log line in InterviewProceedBedrockFlowAsyncService
(inside the method that catches exceptions) uses the label "state" but passes an
HttpStatus; update the log template and parameters so labels match values:
replace "state" with "status" or include an identifier like curAnswerId
alongside status; ensure the call to log.error includes the descriptive message,
the correct status (HttpStatus.INTERNAL_SERVER_ERROR) and the exception e
(e.getMessage(), e) in the same order as the template. Locate the log.error in
InterviewProceedBedrockFlowAsyncService.java and adjust the message string and
arguments to keep labels consistent with the passed values.
In
`@api/src/main/java/com/samhap/kokomen/interview/service/ResumeBasedInterviewService.java`:
- Around line 83-95: The helper methods findMemberResume and findMemberPortfolio
currently return null when an ID is provided but the entity isn't found,
silently allowing success; change them to throw a 400-level exception instead
(e.g., ResponseStatusException(HttpStatus.BAD_REQUEST, ...) or your project's
BadRequestException) when resumeId/portfolioId is non-null and
repository.findByIdAndMemberId(...).isEmpty(), and include a clear message like
"resume_id not found or not owned by member" / "portfolio_id not found or not
owned by member" so callers stop processing invalid IDs.
In
`@api/src/test/java/com/samhap/kokomen/global/fixture/interview/ResumeQuestionGenerationFixtureBuilder.java`:
- Around line 46-58: The builder ResumeQuestionGenerationFixtureBuilder.build()
currently allows null for required fields like member and memberResume; either
provide sensible defaults (as other fixture builders do) for member and
memberResume inside the builder or add explicit null-checks (e.g.,
requireNonNull) at the start of build() to fail fast; update the build() method
to validate member and memberResume before constructing ResumeQuestionGeneration
and reference the class/method names
(ResumeQuestionGenerationFixtureBuilder.build, member, memberResume,
ResumeQuestionGeneration) when making the change.
.../main/java/com/samhap/kokomen/interview/service/InterviewProceedBedrockFlowAsyncService.java
Outdated
Show resolved
Hide resolved
.../main/java/com/samhap/kokomen/interview/service/InterviewProceedBedrockFlowAsyncService.java
Outdated
Show resolved
Hide resolved
| private MemberResume findMemberResume(Long memberId, Long resumeId) { | ||
| if (resumeId == null) { | ||
| return null; | ||
| } | ||
| return memberResumeRepository.findByIdAndMemberId(resumeId, memberId).orElse(null); | ||
| } | ||
|
|
||
| private MemberPortfolio findMemberPortfolio(Long memberId, Long portfolioId) { | ||
| if (portfolioId == null) { | ||
| return null; | ||
| } | ||
| return memberPortfolioRepository.findByIdAndMemberId(portfolioId, memberId).orElse(null); | ||
| } |
There was a problem hiding this comment.
잘못된 resume_id/portfolio_id가 조용히 무시됩니다.
ID가 제공됐는데 소유/존재 확인이 실패하면 null로 처리되어 요청이 성공 처리될 수 있습니다. 명시적으로 400을 반환하는 것이 안전합니다.
🔧 수정 제안
- return memberResumeRepository.findByIdAndMemberId(resumeId, memberId).orElse(null);
+ return memberResumeRepository.findByIdAndMemberId(resumeId, memberId)
+ .orElseThrow(() -> new BadRequestException("존재하지 않는 이력서입니다."));
...
- return memberPortfolioRepository.findByIdAndMemberId(portfolioId, memberId).orElse(null);
+ return memberPortfolioRepository.findByIdAndMemberId(portfolioId, memberId)
+ .orElseThrow(() -> new BadRequestException("존재하지 않는 포트폴리오입니다."));🤖 Prompt for AI Agents
In
`@api/src/main/java/com/samhap/kokomen/interview/service/ResumeBasedInterviewService.java`
around lines 83 - 95, The helper methods findMemberResume and
findMemberPortfolio currently return null when an ID is provided but the entity
isn't found, silently allowing success; change them to throw a 400-level
exception instead (e.g., ResponseStatusException(HttpStatus.BAD_REQUEST, ...) or
your project's BadRequestException) when resumeId/portfolioId is non-null and
repository.findByIdAndMemberId(...).isEmpty(), and include a clear message like
"resume_id not found or not owned by member" / "portfolio_id not found or not
owned by member" so callers stop processing invalid IDs.
| public ResumeQuestionGeneration build() { | ||
| ResumeQuestionGeneration generation = new ResumeQuestionGeneration( | ||
| member, | ||
| memberResume, | ||
| memberPortfolio, | ||
| jobCareer != null ? jobCareer : "신입" | ||
| ); | ||
|
|
||
| if (state == ResumeQuestionGenerationState.COMPLETED) { | ||
| generation.complete(); | ||
| } else if (state == ResumeQuestionGenerationState.FAILED) { | ||
| generation.fail(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
필수 필드 기본값 또는 명시적 검증 추가를 고려해 주세요.
member(또는 memberResume)가 도메인에서 필수라면, 현재 builder는 null로 생성되어 테스트가 애매하게 실패할 수 있습니다. 다른 fixture builder처럼 기본값을 넣거나 requireNonNull로 빠른 실패를 유도하는 편이 안전합니다.
🤖 Prompt for AI Agents
In
`@api/src/test/java/com/samhap/kokomen/global/fixture/interview/ResumeQuestionGenerationFixtureBuilder.java`
around lines 46 - 58, The builder ResumeQuestionGenerationFixtureBuilder.build()
currently allows null for required fields like member and memberResume; either
provide sensible defaults (as other fixture builders do) for member and
memberResume inside the builder or add explicit null-checks (e.g.,
requireNonNull) at the start of build() to fail fast; update the build() method
to validate member and memberResume before constructing ResumeQuestionGeneration
and reference the class/method names
(ResumeQuestionGenerationFixtureBuilder.build, member, memberResume,
ResumeQuestionGeneration) when making the change.
1 participant
closed #317
작업 내용
스크린샷
참고 사항