-
Notifications
You must be signed in to change notification settings - Fork 0
[FEAT] 질문이 생성된 PR만 반환되도록 수정 #87
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughPR 처리 상태를 나타내는 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant API as API Controller
participant Facade as RepositoryFacadeService
participant Service as RepositoryService
participant Repo as PullRequestDomainRepository
participant DB as JPA
rect rgba(200,220,255,0.12)
note over API,DB: PR 생성 (초기 상태: PROCESSING)
User->>API: PR 생성 요청
API->>Service: PullRequestCreateCommand.toDomainEntity()
Service->>DB: save(PullRequest { recordStatus=PENDING, processingStatus=PROCESSING })
DB-->>Service: 저장 완료
end
rect rgba(200,255,200,0.12)
note over DB: 비동기 분석 완료
DB->>DB: updateAnalyzeResult(summary, detail)
DB->>DB: processingStatus = DONE
DB->>DB: repository.plusPrCount()
end
rect rgba(255,240,200,0.12)
note over API,DB: PR 목록 조회 (처리된 PR만)
User->>API: PR 목록 요청
API->>Facade: findAllPullRequests...
Facade->>Service: getProcessedPullRequests(...)
Service->>Repo: findProcessedUserPullRequestsOrderByMergedAt(..., ProcessingStatus.DONE)
Repo->>DB: findByUserIdAndProcessingStatus(..., DONE)
DB-->>Repo: Page<PullRequestEntity>
Repo-->>Service: PullRequests
Service-->>API: PullRequests (DTO에 processingStatus 포함)
API-->>User: 응답
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
(해당 없음) 📜 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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)
✨ 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 (
|
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 821ae0c. ♻️ This comment has been updated with latest results. |
📝 Test Coverage Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
gss-domain/src/main/java/com/devoops/jpa/entity/github/pr/PullRequestEntity.java (1)
67-83: 도메인→엔티티 매핑 시 널 안전성 확보processingStatus가 null이면 저장 시 제약 위반이 납니다. 기본값(PROCESSING)으로 대체하세요.
적용 Diff:
public static PullRequestEntity from(PullRequest pullRequest) { - return new PullRequestEntity( + ProcessingStatus safeProcessing = + java.util.Objects.requireNonNullElse(pullRequest.getProcessingStatus(), ProcessingStatus.PROCESSING); + return new PullRequestEntity( pullRequest.getId(), pullRequest.getRepositoryId(), pullRequest.getUserId(), pullRequest.getTitle(), pullRequest.getDescription(), pullRequest.getTag(), pullRequest.getSummary(), pullRequest.getSummaryDetail(), pullRequest.getExternalId(), pullRequest.getPullRequestUrl(), pullRequest.getRecordStatus(), - pullRequest.getProcessingStatus(), + safeProcessing, pullRequest.getMergedAt() ); }gss-api-app/src/main/java/com/devoops/dto/response/PullRequestReadResponse.java (4)
22-25: 사용자 노출 문구 오탈자 수정: '라밸' → '라벨'Swagger 문서의 한글 라벨 오탈자입니다. 사용자 노출 품질 이슈이므로 수정 권장합니다.
다음 변경 적용을 제안합니다.
- @Schema(description = "회고 라밸", example = "feat") + @Schema(description = "회고 라벨", example = "feat")
46-49: 배열 필드의 OpenAPI 스키마 표기 개선categories는 List인데 단일 Schema와 중괄호 예시를 사용하고 있습니다. ArraySchema로 예시를 JSON 배열로 표기하는 것이 정확합니다.
- @NotNull - @Schema(description = "회고 카테고리 목록", example = "{성능, 안전성, 실용성, 가독성}") - List<String> categories, + @NotNull + @ArraySchema(arraySchema = @Schema(description = "회고 카테고리 목록"), schema = @Schema(example = "성능")) + List<String> categories,또는 간단히:
- @Schema(description = "회고 카테고리 목록", example = "{성능, 안전성, 실용성, 가독성}") + @Schema(description = "회고 카테고리 목록", example = "[\"성능\",\"안전성\",\"실용성\",\"가독성\"]")
55-72: PullRequestReadResponse.summary의 Bean Validation 재고
PullRequestReadResponse는 PROCESSING 단계 조회에도 from()으로 호출되므로 summary가 빈 문자열일 경우 @notblank에 의해 검증 오류가 발생합니다. summary 필드의 @notblank를 제거하거나, PROCESSING 전용 응답 DTO를 별도 정의해 사용하세요.
26-33: 스키마 예시값 조정
processingStatus예시를"DONE"으로,recordStatus예시를"PENDING"으로 변경하세요.- @Schema(description = "질문 생성 상태", example = "PROCESSING") + @Schema(description = "질문 생성 상태", example = "DONE") - @Schema(description = "기록 상태", example = "PROGRESS") + @Schema(description = "기록 상태", example = "PENDING")[must] enum 정의에 맞춘 예시로 문서 혼선 방지
gss-domain/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestDomainRepositoryImpl.java (2)
59-65: 동일 이슈: 사용자 기준 조회에서도.get()제거 필요위와 동일한 이유로
.get()대신.getContent().stream()사용이 필요합니다.- return pullRequestRepository.findByUserIdAndProcessingStatus(userId, ProcessingStatus.DONE, pageable) - .get() + return pullRequestRepository + .findByUserIdAndProcessingStatus(userId, ProcessingStatus.DONE, pageable) + .getContent().stream() .map(PullRequestEntity::toDomainEntity) .collect(Collectors.collectingAndThen(Collectors.toList(), PullRequests::new));
78-86: 분석 완료 처리의 멱등성 보장 및 상태 전이 동기화 필요updateAnalyzedResult에서 저장소 prCount를 증가시키지만, 처리 상태 전이(→ DONE) 확인/보장이 보이지 않습니다. 재시도 시 prCount 중복 증가 위험이 있고, DONE 전이 없이 카운트만 증가할 수도 있습니다.
- 처리 전 상태가 DONE이 아닐 때만 prCount 증가.
- 같은 트랜잭션에서 processingStatus를 DONE으로 전이(엔티티 메서드로 캡슐화 권장).
- 재호출 시 추가 증가가 일어나지 않도록 멱등성 확보.
검증 스크립트:
#!/bin/bash # PullRequestEntity.updateAnalyzeResult가 processingStatus를 DONE으로 전이하는지 확인 rg -nP --type=java -C3 'class\s+PullRequestEntity\b.*|updateAnalyzeResult\s*\(|processingStatus|ProcessingStatus\.DONE'또한, GithubRepositoryEntity에 낙관적 락(@Version)이 없다면 동시 증가 시 갱신 손실 위험이 있습니다.
#!/bin/bash rg -nP --type=java -C2 'class\s+GithubRepositoryEntity\b|@Version|prCount'멱등 증가를 위해 DB 레벨
pr_count = pr_count + 1업데이트 또는 @Version 기반 재시도 패턴을 권장합니다.
🧹 Nitpick comments (17)
gss-domain/src/main/java/com/devoops/domain/entity/github/pr/ProcessingStatus.java (2)
5-7: 오류/재시도 상태 추가 고려 (FAILED 등).질문 생성 실패·타임아웃 시 영구 PROCESSING에 머무를 수 있습니다. 운영/재처리를 위해 FAILED(및 필요 시 RETRYING) 상태를 추가하는 것을 권장합니다. 클라이언트 노출 정책은 DONE만 반환 유지하되, 내부 대시보드/재시도 큐에선 FAILED를 활용하면 좋습니다.
가능한 최소 변경 예:
public enum ProcessingStatus { - PROCESSING, - DONE, + PROCESSING, + DONE, + FAILED, ; }
6-7: 불필요한 세미콜론 제거.enum 본문에 필드/메서드가 없어서 세미콜론은 생략 가능합니다. 가독성 측면에서 제거 권장(기능 변화 없음).
- DONE, - ; + DONEgss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java (1)
75-81: 처리 완료(PR DONE)만 조회하도록 변경 — 목적에 부합, LGTM.퍼사드 레벨에서 처리완료 PR만 노출하도록 단일화된 점 좋습니다. API 스펙(응답 설명)에 “DONE만 반환”을 명시하면 클라이언트 혼선을 줄일 수 있습니다. 통합 테스트로 PROCESSING PR이 응답에 포함되지 않음을 검증하는 케이스 추가를 권장합니다.
gss-api-app/src/test/java/com/devoops/controller/repository/RepositoryControllerTest.java (2)
31-33: 테스트 제목 문자열과 타임스탬프가 불일치합니다.
pr3는 1분 전 생성이지만 제목은 "3분전 PR"입니다. 혼동 방지를 위해 제목을 맞춰주세요.- PullRequest pr3 = pullRequestGenerator.generate("3분전 PR", RecordStatus.PENDING, ProcessingStatus.DONE, repo, now.minusMinutes(1L)); + PullRequest pr3 = pullRequestGenerator.generate("1분전 PR", RecordStatus.PENDING, ProcessingStatus.DONE, repo, now.minusMinutes(1L));
25-43: PROCESSING PR 제외 검증 케이스 추가 제안.현재 상태코드만 검증합니다. DONE 3건 + PROCESSING 1건을 준비한 뒤 응답 목록이 3건만 포함되는지(또는 총계/페이지 메타데이터)까지 확인하는 API 테스트를 추가하면 요구사항을 더 정확히 커버할 수 있습니다.
gss-api-app/src/test/java/com/devoops/service/pullrequests/PullRequestServiceTest.java (1)
29-35: DB 인덱스/제약 권장(운영 성능/일관성)processing_status를 WHERE 절로 자주 사용하게 되므로 (repository_id, processing_status) 복합 인덱스를 고려해 주세요. 또한 기본값 PROCESSING, NOT NULL 제약으로 데이터 일관성을 보장하는 마이그레이션이 필요합니다.
gss-api-app/src/test/java/com/devoops/service/question/QuestionServiceTest.java (1)
69-73: 테스트 가독성 니트픽: 질문 제목 중복다중 업데이트 케이스에서 question2/3의 제목이 모두 "질문1"입니다. 구분 편의를 위해 서로 다른 제목을 권장합니다.
- Question question2 = questionGenerator.generate(pullRequest, "질문1"); - Question question3 = questionGenerator.generate(pullRequest, "질문1"); + Question question2 = questionGenerator.generate(pullRequest, "질문2"); + Question question3 = questionGenerator.generate(pullRequest, "질문3");gss-domain/src/main/java/com/devoops/service/repository/RepositoryService.java (2)
44-47: 소유권 검증 후 조회: 안전성 OK. 방어적 필터 이중화 고려validateOwn으로 소유자 확인은 적절합니다. 가능하다면 레포 계층에서도 userId를 함께 조건에 포함해 다중 테넌트 방어를 이중화해 주세요. readOnly 트랜잭션도 부여 권장.
적용 Diff(권장):
+ @Transactional(readOnly = true) public PullRequests getProcessedPullRequestsByRepository(User user, long repositoryId, int size, int page) { validateOwn(user, repositoryId); return pullRequestRepository.findProcessedPullRequestsByRepositoryIdOrderByMergedAt(repositoryId, size, page); }
40-42: 정렬/페이지 일관성 및 readOnly 트랜잭션 적용 확인됨
findProcessedUserPullRequestsOrderByMergedAt/findProcessedPullRequestsByRepositoryIdOrderByMergedAt메서드는
PageRequest.of(page, size, Sort.by(Sort.Direction.DESC, "mergedAt"))로 구현되어 있어 DESC 정렬과 0-based 페이지 인덱스를 사용합니다.PullRequestDomainRepositoryImpl에는 이미@Transactional(readOnly = true)가 적용되어 있어 쿼리 비용 최적화가 보장됩니다.- 서비스 계층
getProcessedPullRequests에 동일 어노테이션을 추가하는 것은 선택적 개선사항입니다.gss-domain/src/testFixtures/java/com/devoops/generator/PullRequestGenerator.java (1)
16-17: 필드 주입 → 생성자 주입 전환 권장(테스트 픽스처라도 일관성 유지)불변성·테스트 용이성을 위해 생성자 주입을 권장합니다.
권장 예시:
- @Autowired - private PullRequestDomainRepository pullRequestDomainRepository; + private final PullRequestDomainRepository pullRequestDomainRepository; + + @Autowired + public PullRequestGenerator(PullRequestDomainRepository pullRequestDomainRepository) { + this.pullRequestDomainRepository = pullRequestDomainRepository; + }gss-domain/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestJpaRepository.java (2)
3-16: 작업 부하 대비 인덱스 설계 필요(성능 영향 큼)쿼리 패턴상 아래 복합 인덱스를 권장합니다: (repository_id, processing_status, merged_at DESC), (user_id, processing_status, merged_at DESC).
DDL 예시(마이그레이션에 추가):
CREATE INDEX idx_pr_repo_status_mergedat ON pull_requests (repository_id, processing_status, merged_at DESC); CREATE INDEX idx_pr_user_status_mergedat ON pull_requests (user_id, processing_status, merged_at DESC);
5-5: 불필요한 import 제거java.util.List가 사용되지 않습니다. 정리해 주세요.
gss-domain/src/main/java/com/devoops/jpa/entity/github/pr/PullRequestEntity.java (2)
89-93: 분석 결과 업데이트 시 상태 전이: OK. 멱등성 고려 가능동일 요약으로 재갱신될 수 있으므로 변경사항이 없을 땐 skip하도록 최적화할 수 있습니다(선택).
22-25: 인덱스 선언을 JPA에도 반영(선택)DDL과 별개로 JPA 메타에도 인덱스를 표현하면 가독성이 올라갑니다.
예시:
@Table( name = "pull_requests", indexes = { @Index(name = "idx_pr_repo_status_mergedat", columnList = "repositoryId,processingStatus,mergedAt"), @Index(name = "idx_pr_user_status_mergedat", columnList = "userId,processingStatus,mergedAt") } )gss-domain/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestDomainRepositoryImpl.java (1)
49-65: 중복 매핑 로직 DRY화두 조회 메서드의 페이지 → PullRequests 변환이 동일합니다. private 헬퍼로 추출해 중복 제거하면 가독성 및 변경 용이성이 개선됩니다.
예:
private PullRequests toDomainPullRequests(Page<PullRequestEntity> page) { return page.getContent().stream() .map(PullRequestEntity::toDomainEntity) .collect(Collectors.collectingAndThen(Collectors.toList(), PullRequests::new)); }gss-api-app/src/test/java/com/devoops/service/repository/RepositoryServiceTest.java (2)
178-201: 테스트 설명과 데이터 불일치(네이밍/예시) — 가독성 니트픽
nowPr의 설명 문자열이 "1분전 PR"이지만 실제 시간값은 now입니다. 테스트 의미에 영향은 없으나 혼동 소지가 있어 정리 권장합니다.- PullRequest nowPr = pullRequestGenerator.generate("1분전 PR", RecordStatus.PENDING, ProcessingStatus.DONE, repo2, now); + PullRequest nowPr = pullRequestGenerator.generate("현재 PR", RecordStatus.PENDING, ProcessingStatus.DONE, repo2, now);
54-77: 엣지 케이스 테스트 추가 제안: DONE이 하나도 없는 경우모든 PR이 PROCESSING일 때 빈 결과가 반환되는지(예: getProcessedPullRequestsByRepository)도 한 케이스로 보강하면 회귀를 막을 수 있습니다.
예시:
@Test void 질문_생성_완료_PR_없으면_빈_목록을_반환한다() { LocalDateTime now = LocalDateTime.now(); User user = userGenerator.generate("김건우"); GithubRepository repo = repoGenerator.generate(user, "레포"); pullRequestGenerator.generate("최근 PR", RecordStatus.PENDING, ProcessingStatus.PROCESSING, repo, now); assertThat(repositoryService.getProcessedPullRequestsByRepository(user, repo.getId(), 10, 0) .getValues()).isEmpty(); }Also applies to: 101-130, 227-257, 259-290
📜 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 (20)
gss-api-app/src/main/java/com/devoops/dto/response/PullRequestReadResponse.java(3 hunks)gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java(1 hunks)gss-api-app/src/test/java/com/devoops/controller/question/QuestionControllerTest.java(2 hunks)gss-api-app/src/test/java/com/devoops/controller/repository/RepositoryControllerTest.java(3 hunks)gss-api-app/src/test/java/com/devoops/service/answerranking/AnswerRankingServiceTest.java(5 hunks)gss-api-app/src/test/java/com/devoops/service/facade/PullRequestFacadeServiceTest.java(2 hunks)gss-api-app/src/test/java/com/devoops/service/facade/QuestionFacadeServiceTest.java(5 hunks)gss-api-app/src/test/java/com/devoops/service/pullrequests/PullRequestServiceTest.java(2 hunks)gss-api-app/src/test/java/com/devoops/service/question/QuestionServiceTest.java(5 hunks)gss-api-app/src/test/java/com/devoops/service/repository/RepositoryServiceTest.java(7 hunks)gss-domain/src/main/java/com/devoops/command/request/PullRequestCreateCommand.java(2 hunks)gss-domain/src/main/java/com/devoops/domain/entity/github/pr/ProcessingStatus.java(1 hunks)gss-domain/src/main/java/com/devoops/domain/entity/github/pr/PullRequest.java(1 hunks)gss-domain/src/main/java/com/devoops/domain/repository/github/pr/PullRequestDomainRepository.java(1 hunks)gss-domain/src/main/java/com/devoops/jpa/entity/github/pr/PullRequestEntity.java(5 hunks)gss-domain/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestDomainRepositoryImpl.java(3 hunks)gss-domain/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestJpaRepository.java(2 hunks)gss-domain/src/main/java/com/devoops/service/repository/RepositoryService.java(1 hunks)gss-domain/src/testFixtures/java/com/devoops/generator/PullRequestGenerator.java(3 hunks)gss-mcp-app/src/test/java/com/devoops/service/webhook/WebhookFacadeServiceTest.java(1 hunks)
⏰ 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: build-and-push
🔇 Additional comments (25)
gss-api-app/src/test/java/com/devoops/service/answerranking/AnswerRankingServiceTest.java (5)
10-10: ProcessingStatus import 추가 — OK.
39-41: fixture 시그니처 변경 반영 — OK.
54-55: fixture 시그니처 변경 반영 — OK.
76-76: fixture 시그니처 변경 반영 — OK.
97-100: fixture 시그니처 변경 반영 — OK.gss-api-app/src/test/java/com/devoops/controller/repository/RepositoryControllerTest.java (2)
4-4: ProcessingStatus import 추가 — OK.
53-53: NOT OWNER 케이스 — 변경 취지와 무관하지만 수정 반영은 적절합니다.gss-api-app/src/test/java/com/devoops/service/facade/QuestionFacadeServiceTest.java (4)
8-8: ProcessingStatus 임포트 추가 OK테스트 목적과 변경 방향에 부합합니다.
56-59: 시그니처 변경 반영 LGTM중복 생성 로직에서도 DONE 전달이 일관적입니다.
75-78: 시그니처 변경 반영 LGTM레코드 상태 전이(PENDING→PROGRESS) 시나리오와 충돌 없습니다.
96-96: 시그니처 변경 반영 LGTM대량 업데이트 시나리오에서도 처리 상태 지정이 일관됩니다.
gss-api-app/src/test/java/com/devoops/service/facade/PullRequestFacadeServiceTest.java (1)
8-8: ProcessingStatus 임포트 추가 OKgss-api-app/src/test/java/com/devoops/service/pullrequests/PullRequestServiceTest.java (1)
6-6: ProcessingStatus 임포트 추가 OKgss-api-app/src/test/java/com/devoops/controller/question/QuestionControllerTest.java (1)
8-8: ProcessingStatus 임포트 추가 OKgss-api-app/src/test/java/com/devoops/service/question/QuestionServiceTest.java (5)
9-9: ProcessingStatus 임포트 추가 OK
38-38: 시그니처 변경 반영 LGTM
53-53: 시그니처 변경 반영 LGTM
69-69: 시그니처 변경 반영 LGTM
100-100: 시그니처 변경 반영 LGTMgss-domain/src/testFixtures/java/com/devoops/generator/PullRequestGenerator.java (2)
3-3: ProcessingStatus 의존성 추가: 방향성 OK테스트 픽스처가 처리 상태를 명시하도록 한 점 좋습니다.
19-25: PullRequestGenerator의 4-매개변수 generate 호출부가 존재하지 않아 오버로드/널 가드 추가 불필요Likely an incorrect or invalid review comment.
gss-domain/src/main/java/com/devoops/domain/repository/github/pr/PullRequestDomainRepository.java (1)
13-18: 기존 메서드명 호출부 없음 확인
rg스크립트 실행 결과findPullRequestsByRepositoryIdOrderByMergedAt및findUserPullRequestsOrderByMergedAt호출이 코드베이스에 존재하지 않음을 확인했습니다.gss-api-app/src/test/java/com/devoops/service/repository/RepositoryServiceTest.java (3)
79-99: DONE 필터링 검증 테스트 적절함PROCESSING이 포함된 데이터셋에서 DONE만 반환되는 동작을 깔끔히 검증합니다. 👍
169-172: 비소유자 접근 차단 동작 검증 적절함존재 은닉을 위해 REPOSITORY_NOT_FOUND로 매핑한 정책이 테스트로 보장됩니다.
203-225: 회원 전체 목록에서 DONE만 정렬 검증 케이스 적절함요구사항과 일치하는 결과 순서를 검증하고 있습니다. 👍
| User user = userGenerator.generate("김건우"); | ||
| GithubRepository repo = repoGenerator.generate(user, "건우의 레포"); | ||
| PullRequest pr1 = pullRequestGenerator.generate("PR1", RecordStatus.PENDING, repo, LocalDateTime.now()); | ||
| PullRequest pr1 = pullRequestGenerator.generate("PR1", RecordStatus.PENDING, ProcessingStatus.DONE, repo, LocalDateTime.now()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
시그니처 변경 반영 LGTM + 교차 시나리오 테스트 제안
이 컨트롤러는 회고 수정 API를 다루지만, 본 PR의 핵심(처리 완료 PR만 노출)을 시스템 관점에서 담보하려면 PR 목록/조회 컨트롤러에 PROCESSING 배제 E2E 테스트를 추가해 주세요.
🤖 Prompt for AI Agents
gss-api-app/src/test/java/com/devoops/controller/question/QuestionControllerTest.java
around line 36: 현재 회고 수정 컨트롤러 테스트는 시그니처 변경 반영은 되었으나 시스템 관점에서 "처리 중(PROCESSING)인
PR 제외" 보장을 위한 교차 시나리오가 부족합니다; 테스트에 PROCESSING 상태의 PullRequest를
생성(pullRequestGenerator)하고, DONE·PENDING 등 다른 상태의 PR도 생성한 뒤 PR 목록 및 단건 조회 API를
호출하여 응답에 PROCESSING 상태 PR이 포함되지 않는지(그리고 DONE 등 허용된 상태만 노출되는지) 단언(assert)하도록 E2E
스타일 테스트를 추가하세요; 필요하면 Mock 인증/권한과 repo 연결을 설정하고, 생성한 PR들의 상태별 노출 여부를 각각 검증하는
케이스들을 추가해 누락 없이 검증되도록 구현하세요.
gss-api-app/src/test/java/com/devoops/service/facade/PullRequestFacadeServiceTest.java
Show resolved
Hide resolved
gss-api-app/src/test/java/com/devoops/service/facade/QuestionFacadeServiceTest.java
Show resolved
Hide resolved
| PullRequest pullRequest = pullRequestGenerator.generate( | ||
| "최초 PR", | ||
| RecordStatus.PENDING, | ||
| ProcessingStatus.DONE, | ||
| repo, | ||
| LocalDateTime.now() | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
시그니처 변경 반영 LGTM + 서비스 레벨 필터 테스트 제안
레코드 상태 업데이트 테스트는 적절합니다. 추가로 “서비스 계층에서 PR 목록/조회 시 PROCESSING 제외”를 검증하는 테스트를 보강해 주세요(예: getProcessed… 계열 메서드가 DONE만 반환).
🤖 Prompt for AI Agents
In
gss-api-app/src/test/java/com/devoops/service/pullrequests/PullRequestServiceTest.java
around lines 29 to 35, add/extend a test that verifies the service-level
filtering excludes PROCESSING and returns only DONE: use the existing
pullRequestGenerator to create multiple PullRequest instances with different
ProcessingStatus values (e.g., PROCESSING and DONE) tied to the same repo,
persist them as needed, call the service method under test (the getProcessed...
/ list/lookup method), and assert that the returned collection contains only the
DONE items (check size and each item's ProcessingStatus == DONE). Ensure test
setup covers repository and record status as required and uses the service API
rather than repository calls to validate the service-layer filter.
gss-domain/src/main/java/com/devoops/command/request/PullRequestCreateCommand.java
Show resolved
Hide resolved
gss-domain/src/main/java/com/devoops/domain/entity/github/pr/PullRequest.java
Show resolved
Hide resolved
gss-domain/src/main/java/com/devoops/jpa/entity/github/pr/PullRequestEntity.java
Show resolved
Hide resolved
...main/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestDomainRepositoryImpl.java
Show resolved
Hide resolved
| Page<PullRequestEntity> findByRepositoryIdAndProcessingStatus(long repositoryId, ProcessingStatus processingStatus, Pageable pageable); | ||
|
|
||
| Page<PullRequestEntity> findByUserId(long userId, Pageable pageable); | ||
| Page<PullRequestEntity> findByUserIdAndProcessingStatus(long userId, ProcessingStatus status, Pageable pageable); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
처리 상태 기반 조회 메서드 전환: OK. 정렬 일관성 확보 필요
Pageable에 mergedAt DESC 정렬을 강제하거나 도메인 레포에서 기본 정렬을 고정하세요. 기존 응답 순서가 바뀌면 UI 회귀가 납니다. 또한 제거된 메서드 사용처가 없는지 확인해 주세요.
점검 스크립트:
🏁 Script executed:
#!/bin/bash
# 제거된 메서드/시그니처 사용처 확인
rg -nP --type=java 'findByRepositoryId\('
rg -nP --type=java 'findByUserId\('Length of output: 2447
PullRequest 조회 메서드 정렬 일관성 확보: mergedAt DESC 고정 필요
findByRepositoryIdAndProcessingStatus 및 findByUserIdAndProcessingStatus 호출 시 항상 mergedAt DESC 정렬을 적용하세요. Pageable 파라미터에 Sort 조건을 추가하거나, 엔티티에 @OrderBy("mergedAt DESC")를 설정해 UI 회귀를 방지해야 합니다.
🤖 Prompt for AI Agents
In
gss-domain/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestJpaRepository.java
around lines 13-16, the two finder methods lack a fixed sort and must always
return results ordered by mergedAt DESC; either ensure callers always pass a
Pageable that includes Sort.by(Sort.Direction.DESC, "mergedAt") or (preferred
for global consistency) add @OrderBy("mergedAt DESC") on the PullRequestEntity
mapping for the mergedAt field so the repository queries always return mergedAt
DESC regardless of the incoming Pageable.
gss-mcp-app/src/test/java/com/devoops/service/webhook/WebhookFacadeServiceTest.java
Show resolved
Hide resolved
There was a problem hiding this 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
♻️ Duplicate comments (1)
gss-domain/src/main/java/com/devoops/command/request/PullRequestCreateCommand.java (1)
47-49: 이전 리뷰 피드백 반영 완료 — recordStatus가 도메인에 전달됩니다toDomainEntity에서 호출 측의 recordStatus를 그대로 매핑하고 있어, 과거 “항상 PENDING으로 고정” 문제는 해소되었습니다. 굿.
🧹 Nitpick comments (4)
gss-domain/src/main/java/com/devoops/command/request/PullRequestCreateCommand.java (4)
22-34: 편의 생성자 인자 순서(mergedAt, tag) 불일치 — 일관성 있게 정렬 권장레코드 컴포넌트는 (… mergedAt, tag) 순서지만, 편의 생성자는 (… tag, mergedAt) 순서입니다. 타입이 달라 컴파일 오류는 없지만 API 일관성이 떨어집니다. 호환성 영향이 작다면 아래처럼 순서를 맞추는 것을 권장합니다.
- public PullRequestCreateCommand( + public PullRequestCreateCommand( long repositoryId, long userId, String title, String description, String pullRequestUrl, long externalId, - String tag, - LocalDateTime mergedAt + LocalDateTime mergedAt, + String tag ) { - this(repositoryId, userId, title, description, "", "", pullRequestUrl, externalId, RecordStatus.PENDING, - mergedAt, tag); + this(repositoryId, userId, title, description, "", "", pullRequestUrl, externalId, RecordStatus.PENDING, + mergedAt, tag); }추가로, 기본 recordStatus를 PENDING으로 강제하는 정책을 Javadoc으로 명시해 오용을 줄이는 것도 좋습니다.
+ /** + * 신규 PR 생성 시 recordStatus는 기본적으로 PENDING이며, + * 처리 상태는 toDomainEntity()에서 PROCESSING으로 시작합니다. + */ public PullRequestCreateCommand(
9-20: CreateCommand에 summary/summaryDetail 노출 필요성 재검토요약은 AI 생성 후 갱신되는 값이라면, 생성 시점 커맨드에서 해당 필드를 받지 않도록 축소(별도 UpdateCommand로 분리)하거나 “생성 시 무시됨”을 명확히 문서화하는 편이 안전합니다.
48-48: 처리 상태 기반 조회 최적화(인덱스/필터링) 제안API가 DONE만 반환한다면 DB 컬럼(processing_status)에 인덱스를 추가하고, JPA 쿼리/스펙에서 명시적으로 WHERE processing_status = 'DONE'을 강제하세요. 대량 데이터 시 효과가 큽니다. 실패/타임아웃 상태(FAILED/TIMEOUT)도 함께 정의하면 운영 가시성이 좋아집니다.
36-51: toDomainEntity 호출과 PullRequest 생성자 인자 순서 일치 확인
Lombok @requiredargsconstructor로 생성된 생성자는 호출부의 13개 인자 순서와 완벽히 일치합니다. 안정성 향상을 위해 빌더나 명명된 정적 팩토리 메서드 도입을 선택적 리팩토링으로 고려하세요.
📜 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 (5)
gss-api-app/src/test/java/com/devoops/service/facade/PullRequestFacadeServiceTest.java(3 hunks)gss-domain/src/main/java/com/devoops/command/request/PullRequestCreateCommand.java(2 hunks)gss-domain/src/main/java/com/devoops/jpa/entity/github/pr/PullRequestEntity.java(6 hunks)gss-domain/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestDomainRepositoryImpl.java(3 hunks)gss-domain/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestJpaRepository.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- gss-api-app/src/test/java/com/devoops/service/facade/PullRequestFacadeServiceTest.java
- gss-domain/src/main/java/com/devoops/jpa/entity/github/pr/PullRequestEntity.java
- gss-domain/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestJpaRepository.java
- gss-domain/src/main/java/com/devoops/jpa/repository/github/pr/PullRequestDomainRepositoryImpl.java
⏰ 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: build-and-push
🚩 연관 이슈
close #86
🔂 변경 내역
Summary by CodeRabbit
신기능
변경 사항