Conversation
Summary of ChangesHello @leegwichan, 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! 이 PR은 사용자가 특정 기관과 관련된 토론 템플릿을 조회할 수 있는 새로운 기능을 도입합니다. 여기에는 새로운 API 엔드포인트, 해당 서비스 및 DTO 계층 구현, 그리고 기능 및 적절한 문서화를 보장하기 위한 철저한 테스트가 포함됩니다. Highlights
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
|
|
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. Walkthrough기관별 템플릿 조회 기능을 추가합니다. REST 컨트롤러, 서비스, DTO(응답 레코드)와 관련 테스트(단위·통합·문서)를 추가하고 도메인 저장소의 스트림 import/포맷을 조정했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 클라이언트
participant Controller as OrganizationController
participant Service as OrganizationService
participant Repository as OrganizationDomainRepository
participant DB as Database
Client->>Controller: GET /api/organizations/templates
activate Controller
Controller->>Service: findAll()
activate Service
Service->>Repository: findAll()
activate Repository
Repository->>DB: 쿼리(organizations + templates)
DB-->>Repository: 조직 목록(템플릿 포함)
Repository-->>Service: List<Organization>
deactivate Repository
Service->>Service: OrganizationResponses.from(...) (매핑: Organization → OrganizationResponse, Template → OrganizationTemplateResponse)
Service-->>Controller: OrganizationResponses
deactivate Service
Controller-->>Client: 200 OK + OrganizationResponses JSON
deactivate Controller
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (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)
🔇 Additional comments (3)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
기관별 토론 템플릿 조회 기능을 구현하신 것을 확인했습니다. 전반적으로 컨트롤러, 서비스, 도메인 리포지토리, DTO 계층으로 잘 분리되어 있으며, 각 계층에 맞는 테스트 코드(통합 테스트, 문서화 테스트)도 충실하게 작성되었습니다. PR 설명에서 요청하신 SQL 검토 관련하여, OrganizationDomainRepository에서 findAll을 두 번 호출하여 데이터를 가져온 후 메모리에서 조합하는 방식은 N+1 문제를 효과적으로 방지하는 좋은 접근법입니다. 코드 전반적으로 잘 작성되었으나, 몇 가지 개선점을 제안드립니다. DTO의 필드명을 더 명확하게 변경하고, 테스트 코드에서 사용하지 않는 import 문을 제거하여 코드 가독성과 유지보수성을 높일 수 있습니다. 자세한 내용은 각 파일에 남긴 리뷰 코멘트를 참고해주세요.
| import java.util.List; | ||
|
|
||
| public record OrganizationResponse( | ||
| String organization, |
There was a problem hiding this comment.
OrganizationResponse 레코드의 필드명 organization은 name으로 변경하는 것이 더 명확해 보입니다. 현재 organization이라는 이름은 Organization 도메인 객체 자체로 오해될 소지가 있으며, 실제로는 기관의 이름을 담고 있기 때문입니다. REST API 응답에서 필드명은 그 의미를 명확하게 전달하는 것이 중요합니다.
이 변경에 따라 src/test/java/com/debatetimer/controller/organization/OrganizationDocumentTest.java 파일의 API 문서 정의도 다음과 같이 수정해야 합니다:
fieldWithPath("organizations[].name").type(STRING).description("기관 명"),| String organization, | |
| String name, |
src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java
Outdated
Show resolved
Hide resolved
Test Results135 files 135 suites 15s ⏱️ Results for commit 7af3de9. ♻️ This comment has been updated with latest results. |
📝 Test Coverage Report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
src/main/java/com/debatetimer/service/organization/OrganizationService.java (1)
14-16: 읽기 전용 트랜잭션 어노테이션 추가 고려
findAll()메서드는 읽기 전용 작업이므로@Transactional(readOnly = true)를 추가하면 성능 최적화에 도움이 됩니다. Hibernate의 dirty checking을 건너뛰고 일부 DB에서는 읽기 전용 커넥션으로 라우팅할 수 있습니다.+import org.springframework.transaction.annotation.Transactional; + @Service @RequiredArgsConstructor public class OrganizationService { private final OrganizationDomainRepository organizationDomainRepository; + @Transactional(readOnly = true) public OrganizationsResponse findAll() { return OrganizationsResponse.from(organizationDomainRepository.findAll()); } }src/test/java/com/debatetimer/service/organization/OrganizationServiceTest.java (1)
13-44: 서비스findAll테스트에서 순서 의존성과 검증 범위 보강을 고려해보면 좋겠습니다.현재 테스트는
response.organizations().get(0/1)인덱스에 의존하고,- 각 조직의 템플릿 “개수”만 검증하고 있어서,
저장/조회 정렬 기준이 바뀌거나, 템플릿이 잘못된 조직에 매핑되더라도 테스트가 충분히 잡지 못할 수 있습니다.
조금 더 견고하게 하려면, 예를 들어
organizations()리스트에서organization/affiliation값을 추출해 기대 값과 비교하고,- 특정 기관 이름으로 필터링해 해당 기관의 템플릿 개수와 템플릿 이름까지 함께 검증
하는 식으로 순서 의존성을 줄이고 매핑 정확성까지 함께 확인하는 리팩터를 추천드립니다.
src/main/java/com/debatetimer/controller/organization/OrganizationController.java (1)
1-20: 엔드포인트 구현이 단순하고 역할 분리가 잘 되어 있습니다.조직 템플릿 조회 책임을 서비스에만 위임하고 DTO를 그대로 감싸서 반환해, 컨트롤러가 아주 얇게 잘 구성되어 있습니다.
추가로, 추후 조직 관련 API가 더 늘어날 것을 염두에 둔다면,
- 클래스에
@RequestMapping("/api/organizations")를 두고- 메서드에는
@GetMapping("/templates")만 선언하는 구조도 URL 관리 측면에서 한 번쯤 고려해볼 만합니다.
src/test/java/com/debatetimer/controller/organization/OrganizationDocumentTest.java (1)
1-67: Rest Docs 스펙이 실제 DTO 구조와 잘 맞습니다.
organizations배열과 그 안의organization,affiliation,iconPath,templates[].name/data필드 정의가 DTO 구조와 정확히 일치해서, 문서 신뢰도가 높아 보입니다.향후 조직 템플릿 예시 응답이 다른 테스트(컨트롤러/서비스)와 더 많이 중복되기 시작하면, 공통 팩토리 메서드나 테스트 픽스처로 분리해 재사용하는 정도만 선택적으로 고려해 보셔도 좋겠습니다.
src/main/java/com/debatetimer/dto/organization/OrganizationsResponse.java (1)
1-17: OrganizationsResponse.from 정적 팩토리가 호출 측 코드를 잘 단순화합니다.도메인
List<Organization>에서 DTO 리스트로 변환하는 책임을 응답 타입 안에 모아 두어서, 서비스 계층에서는OrganizationsResponse.from(...)한 줄로 의도가 드러나서 좋습니다.추가로, 도메인 레벨에서
organizations인자가 절대 null 이 되지 않는다는 보장이 없다면,from/toOrganizationsResponse내부에서 null 을List.of()로 치환해 NPE 를 방어하는 것도 선택적으로 고려해 볼 수 있겠습니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (14)
src/main/resources/static/icon/debate_commission_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/government_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/han_alm_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/hantomak_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/igam_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/kogito_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/kondae_time_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/mcu_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/nogotte_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/osansi_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/seobangjeongto_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/todallae_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/visual_icon.pngis excluded by!**/*.pngsrc/main/resources/static/icon/yuppm_law_track_icon.pngis excluded by!**/*.png
📒 Files selected for processing (14)
src/main/java/com/debatetimer/controller/organization/OrganizationController.java(1 hunks)src/main/java/com/debatetimer/domainrepository/organization/OrganizationDomainRepository.java(1 hunks)src/main/java/com/debatetimer/dto/organization/OrganizationResponse.java(1 hunks)src/main/java/com/debatetimer/dto/organization/OrganizationTemplateResponse.java(1 hunks)src/main/java/com/debatetimer/dto/organization/OrganizationsResponse.java(1 hunks)src/main/java/com/debatetimer/service/organization/OrganizationService.java(1 hunks)src/test/java/com/debatetimer/controller/BaseControllerTest.java(2 hunks)src/test/java/com/debatetimer/controller/BaseDocumentTest.java(2 hunks)src/test/java/com/debatetimer/controller/GlobalControllerTest.java(2 hunks)src/test/java/com/debatetimer/controller/Tag.java(1 hunks)src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java(1 hunks)src/test/java/com/debatetimer/controller/organization/OrganizationDocumentTest.java(1 hunks)src/test/java/com/debatetimer/service/BaseServiceTest.java(2 hunks)src/test/java/com/debatetimer/service/organization/OrganizationServiceTest.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/java/com/debatetimer/controller/organization/OrganizationDocumentTest.java (3)
src/test/java/com/debatetimer/controller/RestDocumentationRequest.java (1)
RestDocumentationRequest(18-75)src/test/java/com/debatetimer/controller/RestDocumentationResponse.java (1)
RestDocumentationResponse(14-40)src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java (1)
Nested(17-40)
⏰ 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 (9)
src/test/java/com/debatetimer/controller/Tag.java (1)
5-10: 조직 API 태그 추가가 기존 Enum 패턴과 일관되게 잘 반영되었습니다.
ORGANIZATION_API("Organization API")상수의 이름과 표시 문자열 포맷이 기존 태그들과 일관적이며, 새로운 조직 관련 API 문서/테스트 구분용으로 사용하기에 적절해 보입니다. 별도의 수정 없이 그대로 사용해도 될 것 같습니다.src/main/java/com/debatetimer/domainrepository/organization/OrganizationDomainRepository.java (1)
25-38: LGTM!포맷팅 변경만 있으며 기능적으로 문제없습니다.
groupingBy와mapping을 사용한 스트림 수집 로직이 적절합니다.src/test/java/com/debatetimer/controller/BaseDocumentTest.java (1)
74-76: LGTM!기존 서비스 mock 패턴과 일관성 있게
OrganizationService가 추가되었습니다.src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java (1)
17-40: LGTM!테스트 구조와 검증 로직이 적절합니다. 두 기관을 생성하고 각각 다른 수의 템플릿을 할당한 후 응답을 검증하는 방식이 명확합니다.
src/test/java/com/debatetimer/controller/GlobalControllerTest.java (1)
48-59: LGTM! 정적 파일 조회 테스트 구성이 좋습니다정적 파일 조회 테스트가 잘 구성되어 있습니다. 파라미터화된 테스트로 여러 파일을 효율적으로 검증하며, 테스트된 정적 파일들(
government_icon.png,han_alm_icon.png)이src/main/resources/static/icon/경로에 모두 존재하므로 테스트가 정상 작동합니다.src/test/java/com/debatetimer/service/BaseServiceTest.java (1)
8-9: 조직 관련 픽스처 제너레이터 주입이 기존 패턴과 잘 맞습니다.다른 엔티티 제너레이터들과 동일한 방식으로
@Autowired필드를 추가해서, 서비스 테스트에서 조직/템플릿 픽스처를 재사용하기 좋습니다.Also applies to: 60-64
src/main/java/com/debatetimer/dto/organization/OrganizationTemplateResponse.java (1)
1-10: 도메인 → DTO 매핑이 단순하고 명확합니다.레코드와
OrganizationTemplate기반 생성자를 함께 두어, 서비스/컨트롤러 계층에서 변환 로직을 공통으로 재사용하기 좋습니다.src/test/java/com/debatetimer/controller/BaseControllerTest.java (1)
19-20: 컨트롤러 테스트에서도 조직 픽스처를 공통으로 사용할 수 있어 좋습니다.서비스 쪽 Base 클래스와 동일한 패턴으로 제너레이터를 주입해서, 조직/템플릿 시나리오를 여러 컨트롤러 테스트에서 일관되게 구성하기 좋아 보입니다.
Also applies to: 62-66
src/main/java/com/debatetimer/dto/organization/OrganizationResponse.java (1)
1-28: Organization → OrganizationResponse 매핑이 일관되고 깔끔합니다.도메인 엔티티에서 이름/소속/아이콘 경로와 템플릿 리스트를 한 번에 변환하도록 묶어 두어서, 서비스/컨트롤러에서 매번 필드를 풀어서 매핑하지 않아도 되어 코드 가독성이 좋아졌습니다.
src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java (3)
21-26: 템플릿 이름 픽스처 오타 가능성
"템플릿1","템플릿2"와 달리"릿플템1"은 오타처럼 보입니다. 실제 의미가 있는 값이 아니라면, 아래처럼 일관되게 맞춰 두면 읽기 쉬울 것 같습니다.- organizationTemplateEntityGenerator.generate(organization2, "릿플템1"); + organizationTemplateEntityGenerator.generate(organization2, "템플릿1");[succinct_optional_refactor -> tag must be from list; use suggest_optional_refactor]
27-31: GET 요청에서contentType설정은 생략 가능GET 요청은 보통 요청 본문이 없으므로,
contentType(ContentType.JSON)은 없어도 동작에는 영향이 없습니다. 필요 없다면 제거해 간결하게 유지해도 좋습니다.- OrganizationsResponse response = given() - .contentType(ContentType.JSON) - .when().get("/api/organizations/templates") + OrganizationsResponse response = given() + .when().get("/api/organizations/templates") .then().statusCode(HttpStatus.OK.value()) .extract().as(OrganizationsResponse.class);
33-37: 인덱스에 의존한 검증 대신 조직 식별자 기반 검증을 권장현재는 0, 1번 인덱스에 특정 기관이 온다고 가정하고 템플릿 개수를 확인하고 있어서, 조회 결과 정렬 기준이 바뀌면 테스트가 깨질 수 있습니다. 기관 이름을 기준으로 필터링해서 검증하면 정렬 변화에 덜 민감한 테스트가 됩니다.
assertAll( () -> assertThat(response.organizations()).hasSize(2), - () -> assertThat(response.organizations().get(0).templates()).hasSize(2), - () -> assertThat(response.organizations().get(1).templates()).hasSize(1) + () -> assertThat(response.organizations().stream() + .filter(org -> org.organization().equals("한앎")) + .findFirst() + .get() + .templates()).hasSize(2), + () -> assertThat(response.organizations().stream() + .filter(org -> org.organization().equals("한모름")) + .findFirst() + .get() + .templates()).hasSize(1) );이렇게 하면 서비스/리포지토리에서 정렬 기준이 바뀌어도 테스트 의도가 그대로 유지됩니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-24T03:12:25.245Z
Learnt from: coli-geonwoo
Repo: debate-timer/debate-timer-be PR: 203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
Applied to files:
src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java
🧬 Code graph analysis (1)
src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java (1)
src/test/java/com/debatetimer/controller/organization/OrganizationDocumentTest.java (1)
Nested(22-66)
⏰ 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 (1)
src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java (1)
14-38: 전반적인 테스트 구조와 시나리오가 PR 목적에 잘 맞습니다DB에 기관·템플릿을 직접 생성한 뒤 실제
/api/organizations/templates응답을 DTO로 역직렬화해서 검증하는 방식이라, 컨트롤러–서비스–리포지토리까지 엔드 투 엔드로 잘 커버하고 있습니다. 네이밍과 계층 구조도 일관적이라 추후 유지보수에 무리가 없어 보입니다.
unifolio0
left a comment
unifolio0
left a comment
There was a problem hiding this comment.
/noti
@leegwichan 사소한 리뷰 1개, 추후 고려하면 좋을 사항 1개 남기고 approve 합니다.
| import com.debatetimer.domain.organization.Organization; | ||
| import java.util.List; | ||
|
|
||
| public record OrganizationsResponse(List<OrganizationResponse> organizations) { |
There was a problem hiding this comment.
TableResponses와 같이 Response를 복수형으로 사용하지 않고 Organization을 복수형으로 한 이유가 있나요?
There was a problem hiding this comment.
해당 부분 놓쳤습니다. OrganizationResponses로 수정했습니다.
There was a problem hiding this comment.
이전엔 아무 생각없었는데 지금 이렇게 보니 이 기능을 백엔드로 가져온 건 기관이 추가될 때마다 배포해야 되는 게 싫어서 백엔드로 가져온 건데 결국 배포해야 되는게 프론트에서 백엔드로 바뀐 것 뿐이네 라는 생각이 드네요. 지금 당장 안 바꿔도 좋지만 나중에라도 S3에 업로드 하는 게 좋을 것 같아요.
There was a problem hiding this comment.
하지만 FE에 들어가는 리소스보다 BE에서 하는 리소스가 더 적지 않을까? 라는 것이 개인적인 생각.
이번에 AWS 프리티어 만료되면 이곳저곳 옮겨다녀야 하는 입장에서 오히려 Git으로 관리되는 범위 내에 있는 것도 좋은 생각 아닐까?
There was a problem hiding this comment.
근원적으로 리팩터링이 시작된 원인을 살펴보면 "템플릿 접수부터 홈페이지 내 반영까지의 주기가 너무 길고 복잡하다" 차원인데
템플릿 접수를 내가 하니까, 접수와 배포 모두 내가 익숙한 접근경로 내에서 하는게 효율적일 수 있다는 생각도 들어요.
그런 차원에서 S3에 접근하지 않더라도 Git으로 관리하는게 아직까지는 더 편한 느낌이에요
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
/noti @coli-geonwoo
리뷰 반영 완료했습니다~
There was a problem hiding this comment.
하지만 FE에 들어가는 리소스보다 BE에서 하는 리소스가 더 적지 않을까? 라는 것이 개인적인 생각.
이번에 AWS 프리티어 만료되면 이곳저곳 옮겨다녀야 하는 입장에서 오히려 Git으로 관리되는 범위 내에 있는 것도 좋은 생각 아닐까?
| import com.debatetimer.domain.organization.Organization; | ||
| import java.util.List; | ||
|
|
||
| public record OrganizationsResponse(List<OrganizationResponse> organizations) { |
There was a problem hiding this comment.
해당 부분 놓쳤습니다. OrganizationResponses로 수정했습니다.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java (1)
21-37: 응답 구조 검증은 충분하지만, 정렬/필드 값까지 확인하면 테스트 안정성이 더 높아질 것 같습니다현재 테스트는 기관 개수와 각 기관의 템플릿 개수만 검증하고 있어서, 서비스/리포지토리 구현에서 정렬 기준이 바뀌거나 매핑 필드가 변경되어도 테스트가 통과할 수 있습니다.
크리티컬한 문제는 아니지만, 예를 들어 아래처럼 대표 필드까지 함께 검증하면 회귀를 잡는 데 더 도움이 될 것 같습니다.
response.organizations().get(0).organization()/affiliation()값이 저장한"한앎"/"한양대"인지response.organizations().get(1).organization()/affiliation()값이"한모름"/"양한대"인지이 정도만 추가해도 매핑/정렬 변경 시 테스트가 바로 깨져서 원인 파악이 쉬워질 것 같습니다.
src/test/java/com/debatetimer/service/organization/OrganizationServiceTest.java (1)
21-43: 행복/빈 케이스 커버리지는 좋고, 테스트 간 DB 상태만 한 번 확인해 두면 좋겠습니다
저장한_모든_기관_템플릿을_반환한다테스트는 컨트롤러 테스트와 마찬가지로 응답 리스트의 순서가 저장 순서(또는 기본 정렬 기준)에 고정되어 있다는 가정 위에서get(0),get(1)을 검증하고 있습니다. 현재 구현 의도와 맞다면 문제는 아니지만, 정렬 기준이 바뀔 수 있는 구조라면 나중에 테스트가 불안정해질 수 있어서 정렬 정책을 명시(예: 리포지토리에서 정렬)해 두거나, 이름 기준으로 찾는 형태로 바꾸는 것도 고려해 볼 수 있습니다.
비어있을_경우_빈_객체를_반환한다테스트는 어떤 데이터도 생성하지 않고findAll()을 호출하는데, 이때 BaseServiceTest 환경에서 각 테스트마다 트랜잭션 롤백 또는 DB 클린업이 확실히 되는지 한 번만 확인해 두시면 좋겠습니다. 그렇지 않다면 이전 테스트에서 생성한 데이터 때문에 이 테스트가 깨질 수 있습니다.현재 구조 자체는 잘 잡혀 있고, 위 내용은 안정성을 조금 더 높이기 위한 정도의 제안입니다.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/com/debatetimer/controller/organization/OrganizationController.java(1 hunks)src/main/java/com/debatetimer/dto/organization/OrganizationResponses.java(1 hunks)src/main/java/com/debatetimer/service/organization/OrganizationService.java(1 hunks)src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java(1 hunks)src/test/java/com/debatetimer/controller/organization/OrganizationDocumentTest.java(1 hunks)src/test/java/com/debatetimer/service/organization/OrganizationServiceTest.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/debatetimer/controller/organization/OrganizationController.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-24T03:12:25.245Z
Learnt from: coli-geonwoo
Repo: debate-timer/debate-timer-be PR: 203
File: src/main/resources/db/migration/V11__add_memberId_into_poll.sql:1-2
Timestamp: 2025-07-24T03:12:25.245Z
Learning: debate-timer 프로젝트에서 poll 관련 테이블들은 아직 운영 환경에 배포되지 않았으므로, NOT NULL 컬럼 추가 시 기존 데이터 백필 전략을 고려할 필요가 없음.
Applied to files:
src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java
🧬 Code graph analysis (2)
src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java (2)
src/test/java/com/debatetimer/controller/organization/OrganizationDocumentTest.java (1)
Nested(22-66)src/test/java/com/debatetimer/service/organization/OrganizationServiceTest.java (1)
Nested(18-44)
src/test/java/com/debatetimer/controller/organization/OrganizationDocumentTest.java (4)
src/test/java/com/debatetimer/controller/RestDocumentationRequest.java (1)
RestDocumentationRequest(18-75)src/test/java/com/debatetimer/controller/RestDocumentationResponse.java (1)
RestDocumentationResponse(14-40)src/test/java/com/debatetimer/controller/organization/OrganizationControllerTest.java (1)
Nested(16-39)src/test/java/com/debatetimer/service/organization/OrganizationServiceTest.java (1)
Nested(18-44)
⏰ 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 (3)
src/main/java/com/debatetimer/service/organization/OrganizationService.java (1)
14-16: 단순 위임 + DTO 변환 구조가 명확하고 적절합니다
OrganizationDomainRepository.findAll()결과를 그대로OrganizationResponses.from(...)으로 변환하는 얇은 서비스 레이어 구성이 목적에 잘 맞고, 추후 캐시나 필터링 로직을 넣기에도 확장성이 좋아 보입니다. 현재 구현은 그대로 유지해도 무방해 보입니다.src/test/java/com/debatetimer/controller/organization/OrganizationDocumentTest.java (1)
25-65: REST Docs 스펙 정의와 예시 데이터 구성이 DTO와 잘 맞습니다
organizations[].organization / affiliation / iconPath / templates[].name / templates[].data필드 정의가 실제 DTO 구조와 일관되고, 설명도 직관적이라 프론트엔드에서 참고하기 좋아 보입니다.OrganizationResponses를 직접 구성해서organizationService.findAll()을 스텁하는 방식도 문서 테스트 목적에 잘 맞고, 컨트롤러/서비스 구현 변경에 상대적으로 덜 민감해서 유지보수성이 좋습니다.- 예시 템플릿 데이터에 실제 인코딩된 문자열을 사용한 것도 API 소비자 입장에서 응답 형식을 이해하는 데 도움이 됩니다.
현재 문서 테스트 구성은 그대로 가져가셔도 충분해 보입니다.
src/main/java/com/debatetimer/dto/organization/OrganizationResponses.java (1)
6-16: DTO 변환 유틸 구조가 단순하고 일관성 있게 잘 정리되어 있습니다
- 도메인 리스트를
OrganizationResponses.from(List<Organization>)로 감싸는 패턴 덕분에 서비스/컨트롤러 단에서 매핑 로직이 드러나지 않고, 응답 스펙 변경 시 이 클래스만 수정하면 되는 구조라 좋습니다.organizations.stream().map(OrganizationResponse::new).toList()로 변환 리스트를 한 번 더 감싸 immutable 리스트로 만들어 둔 것도 사이드 이펙트 방지 측면에서 안전합니다.현재 구현은 그대로 사용하셔도 문제 없고, 이후 필드가 추가되더라도 이 클래스 한 곳에서 대응하기 좋아 보입니다.
| OrganizationTemplateEntity::getOrganizationId, | ||
| mapping(OrganizationTemplateEntity::toDomain, toList())) | ||
| ); | ||
| mapping(OrganizationTemplateEntity::toDomain, toList()) |
There was a problem hiding this comment.
정적 import 쓰지 않는 것이 컨벤션이었던 것 같아서 한번 체크 부탁할게요, 커찬!
| mapping(OrganizationTemplateEntity::toDomain, toList()) | |
| Collectors.mapping(OrganizationTemplateEntity::toDomain, Collectors.toList()) |
There was a problem hiding this comment.
근원적으로 리팩터링이 시작된 원인을 살펴보면 "템플릿 접수부터 홈페이지 내 반영까지의 주기가 너무 길고 복잡하다" 차원인데
템플릿 접수를 내가 하니까, 접수와 배포 모두 내가 익숙한 접근경로 내에서 하는게 효율적일 수 있다는 생각도 들어요.
그런 차원에서 S3에 접근하지 않더라도 Git으로 관리하는게 아직까지는 더 편한 느낌이에요
leegwichan
left a comment
leegwichan
left a comment
There was a problem hiding this comment.
/noti 리뷰 반영 완료했습니다. 머지하도록 하겠습니다.
3 participants
🚩 연관 이슈
closed #222
🗣️ 리뷰 요구사항 (선택)
Summary by CodeRabbit
새로운 기능
테스트
✏️ Tip: You can customize this high-level summary in your review settings.