[FEAT] 레포지토리 목록/상세 조회 및 삭제 API 구현#38
Conversation
|
Warning Review limit reached
More reviews will be available in 54 minutes and 41 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough이 PR은 사용자가 등록한 GitHub 레포지토리를 조회, 대시보드 확인, 삭제할 수 있는 REST API를 구현합니다. 엔티티 스키마 확장, 9개의 응답 DTO, QueryDSL 기반 데이터 접근, 서비스 로직, 3개 HTTP 엔드포인트, 통합 테스트, 그리고 분석 요청 흐름과의 통합을 포함합니다. ChangesRepository Management API Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50분 Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/test/java/SeCause/SeCause_be/domain/projectRepository/service/ProjectRepositoryServiceTest.java (1)
129-129: ⚡ Quick win부동소수점 비교는
isCloseTo()를 사용하는 것이 더 안전합니다.
isEqualTo(3.23)는 부동소수점 정밀도 문제로 인해 예상치 못한 테스트 실패를 유발할 수 있습니다. AssertJ의isCloseTo()를 사용하면 허용 오차 범위 내에서 비교할 수 있습니다.♻️ 제안하는 수정안
- assertThat(response.severityBreakdown().getFirst().percentage()).isEqualTo(3.23); + assertThat(response.severityBreakdown().getFirst().percentage()).isCloseTo(3.23, within(0.01));
import static org.assertj.core.data.Percentage.withPercentage;또는import static org.assertj.core.data.Offset.offset;를 추가해within()메서드를 사용할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/SeCause/SeCause_be/domain/projectRepository/service/ProjectRepositoryServiceTest.java` at line 129, In ProjectRepositoryServiceTest replace the exact floating-point equality assertion on response.severityBreakdown().getFirst().percentage() (currently assertThat(...).isEqualTo(3.23)) with a tolerant comparison such as AssertJ's isCloseTo(..., offset(...)) or isCloseTo(..., withPercentage(...)); add the corresponding static import (import static org.assertj.core.data.Offset.offset; or import static org.assertj.core.data.Percentage.withPercentage;) and choose a small offset/percentage (e.g. offset(0.01) or withPercentage(1.0)) to avoid floating-point precision failures.src/main/java/SeCause/SeCause_be/domain/projectRepository/controller/ProjectRepositoryController.java (1)
24-62: ⚖️ Poor tradeoff서비스 메서드 호출 시 매개변수 순서가 일관되지 않습니다.
- Line 31-35:
getRepositories(userId, accountName, keyword)- userId가 첫 번째- Line 46-49:
getRepositoryDashboard(repositoryId, userId)- repositoryId가 첫 번째- Line 60:
deleteRepository(repositoryId, userId)- repositoryId가 첫 번째목록 조회는 사용자 중심, 대시보드/삭제는 리소스 중심이라는 설계 의도는 이해되지만, 향후 유지보수 시 혼란을 줄 수 있습니다. 가능하다면 서비스 계층에서 매개변수 순서를 통일하는 것을 고려해보세요.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/SeCause/SeCause_be/domain/projectRepository/controller/ProjectRepositoryController.java` around lines 24 - 62, Controller calls use inconsistent parameter ordering across getRepositories, getRepositoryDashboard, and deleteRepository; standardize the service API so all methods use the same parameter order (pick one convention, e.g., userId first). Update the service interface and its implementations for getRepositoryDashboard(...) and deleteRepository(...) to accept (userId, repositoryId) to match getRepositories(userId, accountName, keyword), then change the controller calls in ProjectRepositoryController to pass userPrincipal.userId() first for getRepositoryDashboard and deleteRepository; also update any other call sites and tests to the new signature.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/SeCause/SeCause_be/domain/projectRepository/dto/RepositorySummaryResponse.java`:
- Around line 24-53: In RepositorySummaryResponse.of(...) (the static factory
method) protect construction of fullName (currently owner + "/" + name) from
null owner or name by building it null-safely: check owner and name and join
only non-null/ non-empty parts with "/" (or use empty string when both null),
then pass that computed fullName into the RepositorySummaryResponse constructor;
update the method signature usage for fullName so it never becomes "null/..." or
".../null" when owner or name are null.
In
`@src/test/java/SeCause/SeCause_be/domain/projectRepository/controller/ProjectRepositoryControllerTest.java`:
- Around line 33-94: Add a unit test for the missing getRepositories endpoint:
create a UserPrincipal, stub
projectRepositoryService.getRepositories(userPrincipal.userId(), accountName,
keyword) to return a RepositoryListResponse (e.g., empty list), call
projectRepositoryController.getRepositories(userPrincipal, accountName,
keyword), assert response.isSuccess() is true, response.message() equals "레포지토리
목록 조회가 완료됐습니다.", response.result() isSameAs the stubbed RepositoryListResponse,
and verify projectRepositoryService.getRepositories(...) was invoked with the
correct args; place the test alongside the existing
getRepositoryDashboardReturnsSuccessResponse and
deleteRepositoryReturnsSuccessResponse tests.
---
Nitpick comments:
In
`@src/main/java/SeCause/SeCause_be/domain/projectRepository/controller/ProjectRepositoryController.java`:
- Around line 24-62: Controller calls use inconsistent parameter ordering across
getRepositories, getRepositoryDashboard, and deleteRepository; standardize the
service API so all methods use the same parameter order (pick one convention,
e.g., userId first). Update the service interface and its implementations for
getRepositoryDashboard(...) and deleteRepository(...) to accept (userId,
repositoryId) to match getRepositories(userId, accountName, keyword), then
change the controller calls in ProjectRepositoryController to pass
userPrincipal.userId() first for getRepositoryDashboard and deleteRepository;
also update any other call sites and tests to the new signature.
In
`@src/test/java/SeCause/SeCause_be/domain/projectRepository/service/ProjectRepositoryServiceTest.java`:
- Line 129: In ProjectRepositoryServiceTest replace the exact floating-point
equality assertion on response.severityBreakdown().getFirst().percentage()
(currently assertThat(...).isEqualTo(3.23)) with a tolerant comparison such as
AssertJ's isCloseTo(..., offset(...)) or isCloseTo(..., withPercentage(...));
add the corresponding static import (import static
org.assertj.core.data.Offset.offset; or import static
org.assertj.core.data.Percentage.withPercentage;) and choose a small
offset/percentage (e.g. offset(0.01) or withPercentage(1.0)) to avoid
floating-point precision failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bdce35e7-3dac-4341-aaf7-82bb1985afab
📒 Files selected for processing (20)
src/main/java/SeCause/SeCause_be/domain/analysis/service/AnalysisRequestPersistenceService.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/controller/ProjectRepositoryApi.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/controller/ProjectRepositoryController.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/dto/RepositoryAnalysisResponse.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/dto/RepositoryCodeDetailsResponse.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/dto/RepositoryDashboardResponse.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/dto/RepositoryDashboardSummaryResponse.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/dto/RepositoryIssueTypeCountResponse.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/dto/RepositoryListResponse.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/dto/RepositorySeverityBreakdownResponse.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/dto/RepositorySeverityCountResponse.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/dto/RepositorySummaryResponse.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/entity/ProjectRepository.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/repository/ProjectRepositoryRepository.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/repository/ProjectRepositoryRepositoryCustom.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/repository/ProjectRepositoryRepositoryImpl.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/repository/RepositoryDashboardQueryResult.javasrc/main/java/SeCause/SeCause_be/domain/projectRepository/service/ProjectRepositoryService.javasrc/test/java/SeCause/SeCause_be/domain/projectRepository/controller/ProjectRepositoryControllerTest.javasrc/test/java/SeCause/SeCause_be/domain/projectRepository/service/ProjectRepositoryServiceTest.java
dldusgh318
left a comment
There was a problem hiding this comment.
확인했습니다. pr에 대해 1가지 코멘트 전달드립니다
- 말씀하신대로
owner,line_count컬럼이 엔티티에 추가됐는데, 현재 변경사항에는 DB 스키마 변경을 재현할 수 있는 migration/schema 반영이 없어 보입니다.
dev 환경은ddl-auto: validate라서 직접 쿼리를 실행하지 않은 환경에서는 애플리케이션 부팅 시 validation 에러가 날 수 있을 것 같아요.schema.sql또는 마이그레이션 파일로 컬럼 추가를 코드에 포함하는 게 좋을 거 같습니다!
다른 부분들은 리뷰 코멘트 확인 부탁드립니다 수고하셨습니다~!!
| public record RepositoryDashboardSummaryResponse( | ||
| long totalIssues, | ||
| long criticalIssues, | ||
| long highIssues, |
There was a problem hiding this comment.
해당 부분의 high, medium, low 이슈 수는 대시보드 와이어프레임 뷰의 어느 부분에 해당하는걸까요? 찾지못해서 코멘트 남겨둡니다!
There was a problem hiding this comment.
대시보드 상에서는 집계 정보는 사용되지 않는 것 같네요. 수정하도록 하겠습니다!
| result.repositoryId(), | ||
| result.owner(), | ||
| result.name(), | ||
| result.owner() + "/" + result.name(), |
There was a problem hiding this comment.
코드래빗이 달아둔 리뷰가 해당 부분에도 똑같이 있네요! 반영할 때 참고하시면 될 거 같습니다
| projectRepositoryValidator.validateRepositoryOwner(repositoryId, userId); | ||
|
|
||
| RepositoryDashboardQueryResult result = projectRepositoryRepository | ||
| .findRepositoryDashboard(repositoryId, userId) |
There was a problem hiding this comment.
findRepositoryDashboard()에서 소유자검증도 하고있는 걸로 알고있습니다.
validateRepositoryOwner()랑 겹치는 부분이라, validate 로직은 생략해도 괜찮을 거 같습니다
| .mapToLong(Long::longValue) | ||
| .sum(); | ||
|
|
||
| return new RepositoryDashboardResponse( |
There was a problem hiding this comment.
응답 DTO를 조립하는 return 구문이 길어서 각 필드의 의미를 한눈에 파악하기 조금 어려운 것 같습니다!
codeDetails, analysis, summary, issuesByType, severityBreakdown를 private 메서드로 분리하면 getRepositoryDashboard()에서는 전체 응답 흐름만 보이고, 세부 조립 로직은 각 메서드에서 확인할 수 있어 가독성이 좋아질 것 같습니다! 제안이니 가볍게 봐주세용
…feat/36-repository-management
close #36
🔎 개요
사용자가 등록한 GitHub 레포지토리의 목록·대시보드를 조회하고 삭제할 수 있는 API를 구현했습니다.
📝 작업 내용
👀 변경 사항
✅ 체크리스트
Summary by CodeRabbit
출시 노트