[DDING-89] 동아리 지원 폼지 상세조회 API 구현 #238
Conversation
KoSeonJe
requested review from
wonjunYou,
5uhwann and
Seooooo24
as code owners
|
Caution Review failedThe pull request is closed. Walkthrough이번 변경은 중앙 폼 조회 기능을 확장합니다. API 인터페이스와 컨트롤러에 Changes
Sequence Diagram(s)sequenceDiagram
participant U as 사용자
participant C as CentralFormController
participant S as FacadeCentralFormServiceImpl
participant R as Repository/DB
U->>C: GET /my/forms 또는 /my/forms/{formId} 요청
C->>S: getAllMyForm(User) 또는 getForm(formId) 호출
S->>R: 해당 폼 데이터 질의
R-->>S: 폼 및 폼 필드 데이터 반환
S-->>C: DTO(Query) 반환
C-->>U: 최종 응답(FormListResponse 혹은 FormResponse) 전달
Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🌐 Web search-backed reviews and chat
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepository.java (1)
12-12: 섹션별 조회 메소드가 잘 추가되었습니다.Form과 section을 기준으로 FormField를 필터링하는 새로운 메소드가 적절하게 추가되었습니다. 다만, 메소드의 용도와 사용 사례를 명확히 하기 위해 JavaDoc 주석을 추가하는 것을 제안드립니다.
다음과 같이 문서화를 추가하면 좋을 것 같습니다:
+ /** + * 특정 Form의 특정 섹션에 속한 모든 FormField를 조회합니다. + * + * @param form Form 엔티티 + * @param section 조회할 섹션명 + * @return 해당 Form의 특정 섹션에 속한 FormField 목록 + */ List<FormField> findAllByFormAndSection(Form form, String section);src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormService.java (1)
37-40: 트랜잭션 처리와 문서화 개선이 필요합니다.다음 사항들을 고려해 주세요:
- 읽기 전용 트랜잭션임을 명시적으로 표시하기 위해
@Transactional(readOnly = true)어노테이션을 메소드 레벨에 추가하는 것이 좋습니다.- 메소드의 목적과 동작을 설명하는 JavaDoc 문서화를 추가하면 코드의 가독성이 향상될 것입니다.
다음과 같이 개선해 보세요:
+ /** + * 특정 클럽에 속한 모든 폼을 조회합니다. + * + * @param club 조회할 클럽 + * @return 클럽에 속한 폼 목록 + */ + @Transactional(readOnly = true) @Override public List<Form> getAllByClub(Club club) { return formRepository.findAllByClub(club); }src/main/java/ddingdong/ddingdongBE/common/utils/TimeUtils.java (1)
31-35: 날짜 범위 체크 로직을 더 명확하게 개선할 수 있습니다.현재 구현은 정확하지만, 가독성을 높일 수 있습니다.
다음과 같이 개선해 보세요:
public static boolean isDateInRange(LocalDate nowDate, LocalDate startDate, LocalDate endDate) { if (nowDate == null || startDate == null || endDate == null) { return false; } - return !nowDate.isBefore(startDate) && !nowDate.isAfter(endDate); } + return nowDate.compareTo(startDate) >= 0 && nowDate.compareTo(endDate) <= 0; + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/main/java/ddingdong/ddingdongBE/common/utils/TimeUtils.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/activityreport/service/dto/command/CreateActivityReportCommand.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/activityreport/service/dto/command/UpdateActivityReportCommand.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/controller/CentralFormController.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/FormListResponse.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/FormResponse.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java(4 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepository.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormRepository.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java(4 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FormFieldService.java(0 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/FormService.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/GeneralFormService.java(2 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormListQuery.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormQuery.java(1 hunks)src/main/resources/db/migration/V36__form_add_column_sections.sql(1 hunks)src/test/java/ddingdong/ddingdongBE/common/utils/TimeUtilsTest.java(1 hunks)src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImplTest.java(2 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/form/service/FormFieldService.java
✅ Files skipped from review due to trivial changes (1)
- src/main/resources/db/migration/V36__form_add_column_sections.sql
🔇 Additional comments (19)
src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepository.java (1)
10-10: 기존 메소드가 잘 구현되어 있습니다!Form 엔티티와 관련된 모든 FormField를 조회하는 기본 메소드가 적절하게 구현되어 있습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormRepository.java (1)
10-10: 메소드 구현이 적절합니다!Spring Data JPA 명명 규칙을 잘 따르고 있으며, 클럽별 폼 조회 기능을 명확하게 구현했습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/service/FormService.java (1)
15-15: 서비스 인터페이스 설계가 깔끔합니다!서비스 계층의 명명 규칙을 잘 따르고 있으며, 리포지토리 계층과 일관성 있게 구현되었습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormService.java (1)
18-20: LGTM! 인터페이스 설계가 명확합니다.메소드 시그니처가 명확하고 반환 타입이 적절하게 선택되었습니다.
src/main/java/ddingdong/ddingdongBE/domain/activityreport/service/dto/command/UpdateActivityReportCommand.java (1)
3-3: LGTM! TimeUtils로의 마이그레이션이 올바르게 수행되었습니다.TimeParser에서 TimeUtils로의 변경이 일관성 있게 적용되었습니다.
Also applies to: 25-26
src/main/java/ddingdong/ddingdongBE/domain/activityreport/service/dto/command/CreateActivityReportCommand.java (1)
3-3: 리팩토링이 잘 적용되었습니다!
TimeParser에서TimeUtils로의 클래스명 변경이 일관성 있게 적용되었습니다.Also applies to: 26-27
src/main/java/ddingdong/ddingdongBE/domain/form/service/dto/query/FormQuery.java (1)
42-56: DTO 변환 로직이 깔끔하게 구현되었습니다!
Form과FormField엔티티들을 DTO로 변환하는 로직이 스트림 API를 활용하여 간결하고 명확하게 구현되었습니다.src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java (2)
44-46: 새로운 sections 필드가 적절하게 추가되었습니다.필드가 @column과 @convert 어노테이션으로 올바르게 설정되어 있습니다.
58-58: 생성자와 업데이트 메서드에 sections 필드가 잘 통합되었습니다.sections 필드가 생성자와 update 메서드에 적절하게 추가되었습니다.
Also applies to: 66-66
src/main/java/ddingdong/ddingdongBE/domain/form/controller/CentralFormController.java (2)
48-54: getAllMyForm 메서드가 효율적으로 구현되었습니다.Stream API를 사용하여 응답 변환을 깔끔하게 처리했습니다.
57-60: getForm 메서드가 간결하게 구현되었습니다.정적 팩토리 메서드를 활용하여 응답 변환을 명확하게 처리했습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/controller/dto/response/FormResponse.java (3)
13-28: 응답 DTO가 명확하게 문서화되었습니다.Swagger 어노테이션을 통해 각 필드의 설명과 예시가 잘 문서화되어 있습니다.
31-44: 중첩된 FormFieldListResponse 레코드가 잘 구조화되었습니다.필드 응답에 대한 구조가 명확하며 문서화가 잘 되어있습니다.
58-72: 변환 메서드가 효율적으로 구현되었습니다.Stream API를 활용하여 formFields 변환을 깔끔하게 처리했습니다.
src/main/java/ddingdong/ddingdongBE/domain/form/api/CentralFormApi.java (2)
63-73: 전체조회 API가 잘 정의되었습니다.보안 요구사항과 응답 형식이 명확하게 지정되어 있습니다.
75-83: 상세조회 API가 잘 정의되었습니다.API 문서화가 완벽하며 응답 스키마가 명확합니다.
src/main/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImpl.java (2)
66-73: 클럽의 모든 폼 조회 로직이 잘 구현되었습니다.클럽 사용자의 ID를 통해 클럽을 조회하고, 해당 클럽의 모든 폼을 가져와서 FormListQuery로 변환하는 로직이 깔끔하게 구현되었습니다.
82-85: 폼 활성화 상태 체크 로직이 잘 구현되었습니다.TimeUtils를 활용하여 현재 날짜가 폼의 시작일과 종료일 사이에 있는지 확인하는 로직이 깔끔하게 구현되었습니다.
src/test/java/ddingdong/ddingdongBE/domain/form/service/FacadeCentralFormServiceImplTest.java (1)
203-240: getAllMyForm 테스트가 잘 작성되었습니다.테스트 시나리오가 명확하고, 예상 결과에 대한 검증이 적절하게 이루어졌습니다.
| class TimeUtilsTest { | ||
|
|
||
| @DisplayName("현재 날짜가 기간 내 포함된다면 true를 반환한다.") | ||
| @Test | ||
| void isDateInRange() { | ||
| // given | ||
| LocalDate now = LocalDate.now(); | ||
| LocalDate startDate = now.minusDays(1); | ||
| LocalDate endDate = now.plusDays(1); | ||
| // when | ||
| boolean isActive = TimeUtils.isDateInRange(now, startDate, endDate); | ||
| // then | ||
| Assertions.assertThat(isActive).isTrue(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
테스트 케이스 보완 필요
현재 테스트는 기본적인 긍정 케이스만 검증하고 있습니다. 다음과 같은 추가 테스트 케이스를 고려해 주세요:
- 시작일이 null인 경우
- 종료일이 null인 경우
- 현재 날짜가 시작일과 같은 경우
- 현재 날짜가 종료일과 같은 경우
- 현재 날짜가 범위를 벗어난 경우
예시 테스트 케이스:
@DisplayName("시작일이 null이면 false를 반환한다")
@Test
void isDateInRange_nullStartDate() {
// given
LocalDate now = LocalDate.now();
LocalDate endDate = now.plusDays(1);
// when
boolean isActive = TimeUtils.isDateInRange(now, null, endDate);
// then
Assertions.assertThat(isActive).isFalse();
}| Long formId, | ||
| String title, | ||
| LocalDate startDate, | ||
| LocalDate endData, |
There was a problem hiding this comment.
필드 이름 오타 수정 필요
'endData' 필드명에 오타가 있습니다. 'endDate'로 수정해 주세요.
- LocalDate endData,
+ LocalDate endDate,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| LocalDate endData, | |
| LocalDate endDate, |
| .formId(form.getId()) | ||
| .title(form.getTitle()) | ||
| .startDate(form.getStartDate()) | ||
| .endData(form.getEndDate()) |
There was a problem hiding this comment.
메소드 내 필드 참조 오타 수정 필요
from 메소드에서도 동일한 오타를 수정해 주세요.
- .endData(form.getEndDate())
+ .endDate(form.getEndDate())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .endData(form.getEndDate()) | |
| .endDate(form.getEndDate()) |
| @Schema(description = "지원 폼지 시작일", example = "2001-01-01") | ||
| LocalDate startDate, | ||
| @Schema(description = "지원 폼지 종료일", example = "2001-01-02") | ||
| LocalDate endData, |
There was a problem hiding this comment.
필드명 오타를 수정해 주세요.
endData는 endDate의 오타로 보입니다. 날짜를 나타내는 필드이므로 endDate로 수정하는 것이 적절합니다.
다음과 같이 수정해 주세요:
- LocalDate endData,
+ LocalDate endDate,그리고 from 메소드에서도 동일하게 수정:
- .endData(query.endData())
+ .endDate(query.endDate())Also applies to: 27-27
| @Override | ||
| public FormQuery getForm(Long formId) { | ||
| Form form = formService.getById(formId); | ||
| List<FormField> formFields = formFieldService.findAllByForm(form); | ||
| return FormQuery.of(form, formFields); | ||
| } |
There was a problem hiding this comment.
폼 상세 조회 로직에 권한 검증이 필요합니다.
현재 구현에서는 누구나 formId만 알면 폼을 조회할 수 있습니다. 클럽 소유자만 자신의 폼을 조회할 수 있도록 권한 검증이 필요합니다.
다음과 같이 수정을 제안합니다:
@Override
public FormQuery getForm(Long formId) {
Form form = formService.getById(formId);
+ Club club = form.getClub();
+ validateEqualsClub(club, form);
List<FormField> formFields = formFieldService.findAllByForm(form);
return FormQuery.of(form, formFields);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public FormQuery getForm(Long formId) { | |
| Form form = formService.getById(formId); | |
| List<FormField> formFields = formFieldService.findAllByForm(form); | |
| return FormQuery.of(form, formFields); | |
| } | |
| @Override | |
| public FormQuery getForm(Long formId) { | |
| Form form = formService.getById(formId); | |
| Club club = form.getClub(); | |
| validateEqualsClub(club, form); | |
| List<FormField> formFields = formFieldService.findAllByForm(form); | |
| return FormQuery.of(form, formFields); | |
| } |
| @DisplayName("동아리는 폼지를 상세조회 할 수 있다.") | ||
| @Test | ||
| void getForm() { | ||
| // given | ||
| Form form = fixtureMonkey.giveMeBuilder(Form.class) | ||
| .set("id", 1L) | ||
| .set("title", "제목1") | ||
| .set("club", null) | ||
| .sample(); | ||
| Form form2 = fixtureMonkey.giveMeBuilder(Form.class) | ||
| .set("id", 2L) | ||
| .set("title", "제목2") | ||
| .set("club", null) | ||
| .sample(); | ||
| formService.create(form); | ||
| formService.create(form2); | ||
| // when | ||
| FormQuery formQuery = facadeCentralFormService.getForm(1L); | ||
| // then | ||
| assertThat(formQuery.title()).isEqualTo("제목1"); | ||
| } |
There was a problem hiding this comment.
getForm 테스트에 권한 검증 케이스가 누락되었습니다.
현재 테스트는 기본적인 조회 기능만 검증하고 있습니다. 다음 사항들을 추가로 검증해야 합니다:
- 클럽 소유자가 자신의 폼을 조회하는 케이스
- 다른 클럽의 폼을 조회할 때 예외가 발생하는 케이스
다음과 같이 테스트 케이스 추가를 제안합니다:
@DisplayName("동아리는 폼지를 상세조회 할 수 있다.")
@Test
void getForm() {
// given
+ User user = fixtureMonkey.giveMeBuilder(User.class)
+ .set("id", 1L)
+ .set("Role", Role.CLUB)
+ .set("deletedAt", null)
+ .sample();
+ User savedUser = userRepository.save(user);
+ Club club = fixtureMonkey.giveMeBuilder(Club.class)
+ .set("id", 1L)
+ .set("user", savedUser)
+ .set("score", null)
+ .set("clubMembers", null)
+ .set("deletedAt", null)
+ .sample();
+ clubRepository.save(club);
Form form = fixtureMonkey.giveMeBuilder(Form.class)
.set("id", 1L)
.set("title", "제목1")
- .set("club", null)
+ .set("club", club)
.sample();
Form form2 = fixtureMonkey.giveMeBuilder(Form.class)
.set("id", 2L)
.set("title", "제목2")
- .set("club", null)
+ .set("club", club)
.sample();
formService.create(form);
formService.create(form2);
// when
FormQuery formQuery = facadeCentralFormService.getForm(1L);
// then
assertThat(formQuery.title()).isEqualTo("제목1");
+ assertThat(formQuery.club().getId()).isEqualTo(club.getId());
}
+@DisplayName("동아리는 다른 동아리의 폼지를 상세조회 할 수 없다.")
+@Test
+void getFormWithNonHaveAuthority() {
+ // given
+ User user1 = fixtureMonkey.giveMeBuilder(User.class)
+ .set("id", 1L)
+ .set("Role", Role.CLUB)
+ .set("deletedAt", null)
+ .sample();
+ User savedUser1 = userRepository.save(user1);
+ Club club1 = fixtureMonkey.giveMeBuilder(Club.class)
+ .set("id", 1L)
+ .set("user", savedUser1)
+ .set("score", null)
+ .set("clubMembers", null)
+ .set("deletedAt", null)
+ .sample();
+ clubRepository.save(club1);
+
+ User user2 = fixtureMonkey.giveMeBuilder(User.class)
+ .set("id", 2L)
+ .set("Role", Role.CLUB)
+ .set("deletedAt", null)
+ .sample();
+ User savedUser2 = userRepository.save(user2);
+ Club club2 = fixtureMonkey.giveMeBuilder(Club.class)
+ .set("id", 2L)
+ .set("user", savedUser2)
+ .set("score", null)
+ .set("clubMembers", null)
+ .set("deletedAt", null)
+ .sample();
+ clubRepository.save(club2);
+
+ Form form = fixtureMonkey.giveMeBuilder(Form.class)
+ .set("id", 1L)
+ .set("title", "제목1")
+ .set("club", club1)
+ .sample();
+ formService.create(form);
+
+ // when/then
+ assertThrows(NonHaveAuthority.class, () -> {
+ facadeCentralFormService.getForm(1L);
+ });
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @DisplayName("동아리는 폼지를 상세조회 할 수 있다.") | |
| @Test | |
| void getForm() { | |
| // given | |
| Form form = fixtureMonkey.giveMeBuilder(Form.class) | |
| .set("id", 1L) | |
| .set("title", "제목1") | |
| .set("club", null) | |
| .sample(); | |
| Form form2 = fixtureMonkey.giveMeBuilder(Form.class) | |
| .set("id", 2L) | |
| .set("title", "제목2") | |
| .set("club", null) | |
| .sample(); | |
| formService.create(form); | |
| formService.create(form2); | |
| // when | |
| FormQuery formQuery = facadeCentralFormService.getForm(1L); | |
| // then | |
| assertThat(formQuery.title()).isEqualTo("제목1"); | |
| } | |
| @DisplayName("동아리는 폼지를 상세조회 할 수 있다.") | |
| @Test | |
| void getForm() { | |
| // given | |
| User user = fixtureMonkey.giveMeBuilder(User.class) | |
| .set("id", 1L) | |
| .set("Role", Role.CLUB) | |
| .set("deletedAt", null) | |
| .sample(); | |
| User savedUser = userRepository.save(user); | |
| Club club = fixtureMonkey.giveMeBuilder(Club.class) | |
| .set("id", 1L) | |
| .set("user", savedUser) | |
| .set("score", null) | |
| .set("clubMembers", null) | |
| .set("deletedAt", null) | |
| .sample(); | |
| clubRepository.save(club); | |
| Form form = fixtureMonkey.giveMeBuilder(Form.class) | |
| .set("id", 1L) | |
| .set("title", "제목1") | |
| .set("club", club) | |
| .sample(); | |
| Form form2 = fixtureMonkey.giveMeBuilder(Form.class) | |
| .set("id", 2L) | |
| .set("title", "제목2") | |
| .set("club", club) | |
| .sample(); | |
| formService.create(form); | |
| formService.create(form2); | |
| // when | |
| FormQuery formQuery = facadeCentralFormService.getForm(1L); | |
| // then | |
| assertThat(formQuery.title()).isEqualTo("제목1"); | |
| assertThat(formQuery.club().getId()).isEqualTo(club.getId()); | |
| } | |
| @DisplayName("동아리는 다른 동아리의 폼지를 상세조회 할 수 없다.") | |
| @Test | |
| void getFormWithNonHaveAuthority() { | |
| // given | |
| User user1 = fixtureMonkey.giveMeBuilder(User.class) | |
| .set("id", 1L) | |
| .set("Role", Role.CLUB) | |
| .set("deletedAt", null) | |
| .sample(); | |
| User savedUser1 = userRepository.save(user1); | |
| Club club1 = fixtureMonkey.giveMeBuilder(Club.class) | |
| .set("id", 1L) | |
| .set("user", savedUser1) | |
| .set("score", null) | |
| .set("clubMembers", null) | |
| .set("deletedAt", null) | |
| .sample(); | |
| clubRepository.save(club1); | |
| User user2 = fixtureMonkey.giveMeBuilder(User.class) | |
| .set("id", 2L) | |
| .set("Role", Role.CLUB) | |
| .set("deletedAt", null) | |
| .sample(); | |
| User savedUser2 = userRepository.save(user2); | |
| Club club2 = fixtureMonkey.giveMeBuilder(Club.class) | |
| .set("id", 2L) | |
| .set("user", savedUser2) | |
| .set("score", null) | |
| .set("clubMembers", null) | |
| .set("deletedAt", null) | |
| .sample(); | |
| clubRepository.save(club2); | |
| Form form = fixtureMonkey.giveMeBuilder(Form.class) | |
| .set("id", 1L) | |
| .set("title", "제목1") | |
| .set("club", club1) | |
| .sample(); | |
| formService.create(form); | |
| // when/then | |
| assertThrows(NonHaveAuthority.class, () -> { | |
| facadeCentralFormService.getForm(1L); | |
| }); | |
| } |
|
🚀 작업 내용
Summary by CodeRabbit
New Features
Bug Fixes