[DDING-83] Form 및 FormField 엔터티 세팅 #229
Conversation
KoSeonJe
requested review from
wonjunYou,
5uhwann and
Seooooo24
as code owners
|
""" Walkthrough이 풀 리퀘스트는 폼(Form) 관련 기능을 위한 새로운 엔티티, 리포지토리, 컨버터를 추가합니다. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: 3
🧹 Nitpick comments (5)
src/main/java/ddingdong/ddingdongBE/common/converter/StringListConverter.java (1)
18-20: 예외 처리 개선 필요현재 JsonProcessingException을 RuntimeException으로 래핑하여 처리하고 있습니다. 이는 예외의 컨텍스트를 잃어버릴 수 있습니다.
커스텀 예외를 생성하여 더 명확한 예외 처리를 하는 것을 추천드립니다:
public class StringListConversionException extends RuntimeException { public StringListConversionException(String message, Throwable cause) { super(message, cause); } }그리고 예외 처리를 다음과 같이 수정:
- throw new RuntimeException(e); + throw new StringListConversionException("JSON 변환 중 오류가 발생했습니다", e);Also applies to: 28-29
src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java (2)
25-26: 제목 길이 제한 필요title 필드에 길이 제한이 없습니다. 데이터베이스 컬럼 크기와 일관성을 유지하기 위해 validation 어노테이션을 추가하는 것이 좋습니다.
@Column(nullable = false) + @Size(max = 255) private String title;
19-39: 감사 필드 추가 권장엔티티의 생성 및 수정 시간을 추적하기 위한 감사 필드가 없습니다.
@EntityListeners(AuditingEntityListener.class)와 함께 다음 필드들을 추가하는 것을 권장합니다:+ @CreatedDate + private LocalDateTime createdAt; + + @LastModifiedDate + private LocalDateTime updatedAt;src/main/resources/db/migration/V34__from_and_field_create_table.sql (2)
1-21: 인덱스 추가 고려조회 성능 향상을 위해 자주 사용되는 필드에 대한 인덱스 추가가 필요할 수 있습니다.
다음과 같은 인덱스 추가를 고려해보세요:
-- form 테이블에 대한 인덱스 CREATE INDEX idx_form_club_id ON form(club_id); -- form_field 테이블에 대한 인덱스 CREATE INDEX idx_form_field_form_id ON form_field(form_id); CREATE INDEX idx_form_field_order ON form_field(order_number);
14-14: 컬럼 크기 최적화 필요
field_type과section컬럼의 크기가 최적화되어 있지 않습니다.- field_type VARCHAR(50) NOT NULL, + field_type VARCHAR(20) NOT NULL, - section VARCHAR(255) NOT NULL, + section VARCHAR(100) NOT NULL,Also applies to: 17-17
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/ddingdong/ddingdongBE/common/converter/StringListConverter.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/entity/FieldType.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/entity/FormField.java(1 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/resources/db/migration/V34__from_and_field_create_table.sql(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormFieldRepository.java
- src/main/java/ddingdong/ddingdongBE/domain/form/entity/FieldType.java
- src/main/java/ddingdong/ddingdongBE/domain/form/repository/FormRepository.java
🔇 Additional comments (1)
src/main/java/ddingdong/ddingdongBE/domain/form/entity/FormField.java (1)
41-42: options 필드 유효성 검사 필요options 리스트에 대한 유효성 검사가 없습니다. 특히 CHECK_BOX나 RADIO 타입의 경우 options가 필수적입니다.
필드 타입에 따른 options의 유효성을 검증하는 로직이 필요합니다. 다음과 같은 검증 메서드를 추가하는 것을 제안합니다:
@PrePersist @PreUpdate private void validateOptions() { if ((fieldType == FieldType.CHECK_BOX || fieldType == FieldType.RADIO) && (options == null || options.isEmpty())) { throw new IllegalStateException("CHECK_BOX와 RADIO 타입은 최소 하나의 옵션이 필요합니다."); } }✅ Verification successful
옵션 필드 유효성 검사 추가 필요 확인
코드베이스 분석 결과, CHECK_BOX와 RADIO 타입에 대한 options 필드의 유효성 검사가 현재 구현되어 있지 않음을 확인했습니다. 리뷰 코멘트에서 제안된
@PrePersist와@PreUpdate를 사용한 검증 로직 추가는 적절한 해결책입니다.
FormField엔티티에 options 필드가 nullable하게 설정되어 있음FieldTypeenum에 CHECK_BOX와 RADIO가 명시적으로 정의되어 있음- 현재 코드베이스에 유사한 검증 로직이 구현되어 있지 않음
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find and show the complete FormField class implementation echo "=== FormField.java content ===" fd "FormField.java" --exec cat {} echo -e "\n=== Looking for FieldType enum ===" fd "FieldType.java" --exec cat {} echo -e "\n=== Searching for existing validation patterns ===" rg "@PrePersist|@PreUpdate" -A 3Length of output: 2192
| return mapper.readValue(data, List.class); | ||
| } catch (JsonProcessingException e) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
List 타입 안정성 개선 필요
mapper.readValue(data, List.class)는 raw type을 사용하고 있어 타입 안정성이 보장되지 않습니다.
다음과 같이 TypeReference를 사용하여 타입 안정성을 보장하는 것이 좋습니다:
- return mapper.readValue(data, List.class);
+ return mapper.readValue(data, new TypeReference<List<String>>() {});📝 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.
| return mapper.readValue(data, List.class); | |
| } catch (JsonProcessingException e) { | |
| return mapper.readValue(data, new TypeReference<List<String>>() {}); | |
| } catch (JsonProcessingException e) { |
| @Column(nullable = false) | ||
| private int order; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
예약어 사용 주의
order는 SQL의 예약어입니다. 이로 인해 예기치 않은 문제가 발생할 수 있습니다.
필드명을 orderNumber 또는 sequence로 변경하는 것을 권장합니다:
@Column(nullable = false)
- private int order;
+ private int orderNumber;📝 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.
| @Column(nullable = false) | |
| private int order; | |
| @Column(nullable = false) | |
| private int orderNumber; |
| question VARCHAR(255) NOT NULL, | ||
| field_type VARCHAR(50) NOT NULL, | ||
| required BOOLEAN NOT NULL, | ||
| `order` INT NOT NULL, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
SQL 예약어 사용 수정 필요
order는 SQL 예약어이므로 백틱으로 감싸더라도 사용을 피하는 것이 좋습니다.
- `order` INT NOT NULL,
+ order_number INT NOT NULL,📝 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.
| `order` INT NOT NULL, | |
| order_number INT NOT NULL, |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/main/resources/db/migration/V34__from_and_field_create_table.sql (2)
1-11: 감사 추적을 위한 타임스탬프 필드 추가 필요테이블 감사(audit)를 위해 다음 필드들의 추가를 고려해주세요:
created_at- 레코드 생성 시간updated_at- 레코드 수정 시간CREATE TABLE form ( id BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY, title VARCHAR(255) NOT NULL, description VARCHAR(255), start_date DATE NOT NULL, end_date DATE NOT NULL, has_interview BOOLEAN NOT NULL, club_id BIGINT, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, CONSTRAINT fk_form_club FOREIGN KEY (club_id) REFERENCES club (id) ON DELETE CASCADE );
13-24: 감사 추적을 위한 타임스탬프 필드 추가 필요form_field 테이블에도 감사(audit) 필드 추가가 필요합니다:
created_at- 레코드 생성 시간updated_at- 레코드 수정 시간CREATE TABLE form_field ( id BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY, question VARCHAR(255) NOT NULL, field_type VARCHAR(50) NOT NULL, required BOOLEAN NOT NULL, `order` INT NOT NULL, section VARCHAR(255) NOT NULL, options TEXT, form_id BIGINT, + created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, CONSTRAINT fk_form_field_form FOREIGN KEY (form_id) REFERENCES form (id) ON DELETE CASCADE );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java(1 hunks)src/main/resources/db/migration/V34__from_and_field_create_table.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (1)
src/main/resources/db/migration/V34__from_and_field_create_table.sql (1)
19-19: SQL 예약어 사용 수정 필요
order는 SQL 예약어이므로 백틱으로 감싸더라도 사용을 피하는 것이 좋습니다.
| package ddingdong.ddingdongBE.domain.form.entity; | ||
|
|
||
| import ddingdong.ddingdongBE.domain.club.entity.Club; | ||
| import jakarta.persistence.Column; | ||
| import jakarta.persistence.Entity; | ||
| import jakarta.persistence.FetchType; | ||
| import jakarta.persistence.GeneratedValue; | ||
| import jakarta.persistence.GenerationType; | ||
| import jakarta.persistence.Id; | ||
| import jakarta.persistence.ManyToOne; | ||
| import java.time.LocalDate; | ||
| import lombok.AccessLevel; | ||
| import lombok.Builder; | ||
| import lombok.Getter; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @Entity | ||
| @NoArgsConstructor(access = AccessLevel.PROTECTED) | ||
| @Getter | ||
| public class Form { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @Column(nullable = false) | ||
| private String title; | ||
|
|
||
| private String description; | ||
|
|
||
| @Column(nullable = false) | ||
| private LocalDate startDate; | ||
|
|
||
| @Column(nullable = false) | ||
| private LocalDate endDate; | ||
|
|
||
| @Column(nullable = false) | ||
| private boolean hasInterview; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| private Club club; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
감사 필드 및 날짜 유효성 검증 추가 필요
- SQL 스키마와 일치하도록 감사 필드를 추가해주세요:
+ @Column(nullable = false, updatable = false)
+ private LocalDateTime createdAt;
+
+ @Column(nullable = false)
+ private LocalDateTime updatedAt;startDate가endDate보다 이전인지 검증하는 로직이 필요합니다. 다음과 같은 검증 메서드 추가를 고려해주세요:
private void validateDates() {
if (startDate.isAfter(endDate)) {
throw new IllegalArgumentException("시작일은 종료일보다 이후일 수 없습니다.");
}
}| @Builder | ||
| private Form(String title, String description, Club club) { | ||
| this.title = title; | ||
| this.description = description; | ||
| this.club = club; | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
필수 필드가 빌더에서 누락됨
다음 필수 필드들이 빌더에 포함되어 있지 않습니다:
- startDate (nullable = false)
- endDate (nullable = false)
- hasInterview (nullable = false)
@Builder
-private Form(String title, String description, Club club) {
+private Form(String title, String description, LocalDate startDate, LocalDate endDate, boolean hasInterview, Club club) {
this.title = title;
this.description = description;
+ this.startDate = startDate;
+ this.endDate = endDate;
+ this.hasInterview = hasInterview;
this.club = club;
+ validateDates();
}📝 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.
| @Builder | |
| private Form(String title, String description, Club club) { | |
| this.title = title; | |
| this.description = description; | |
| this.club = club; | |
| } | |
| } | |
| @Builder | |
| private Form(String title, String description, LocalDate startDate, LocalDate endDate, boolean hasInterview, Club club) { | |
| this.title = title; | |
| this.description = description; | |
| this.startDate = startDate; | |
| this.endDate = endDate; | |
| this.hasInterview = hasInterview; | |
| this.club = club; | |
| validateDates(); | |
| } | |
| } |
|
| question VARCHAR(255) NOT NULL, | ||
| field_type VARCHAR(50) NOT NULL, | ||
| required BOOLEAN NOT NULL, | ||
| `order` INT NOT NULL, |
There was a problem hiding this comment.
코드래빗 ai 말대로 이 부분만 수정해주시면 된다고 생각합니다!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/resources/db/migration/V34__from_and_field_create_table.sql (2)
1-13:club_id컬럼에 인덱스 추가 권장
club_id로 폼을 조회하는 작업이 빈번할 것으로 예상되므로, 조회 성능 향상을 위해 인덱스 추가를 권장드립니다.다음과 같이 인덱스를 추가해 주세요:
CREATE TABLE form ( id BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY, title VARCHAR(255) NOT NULL, description VARCHAR(255), start_date DATE NOT NULL, end_date DATE NOT NULL, has_interview BOOLEAN NOT NULL, club_id BIGINT, created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP NULL, updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP NULL, - CONSTRAINT fk_form_club FOREIGN KEY (club_id) REFERENCES club (id) ON DELETE CASCADE + CONSTRAINT fk_form_club FOREIGN KEY (club_id) REFERENCES club (id) ON DELETE CASCADE, + INDEX idx_club_id (club_id) );
15-28: 인덱스 추가 제안폼 필드 조회 성능 향상을 위해 다음과 같은 인덱스 추가를 제안드립니다:
form_id단일 인덱스(form_id, field_order)복합 인덱스 - 폼별 필드 순서 조회 시 성능 향상다음과 같이 인덱스를 추가해 주세요:
CREATE TABLE form_field ( id BIGINT NOT NULL AUTO_INCREMENT PRIMARY KEY, question VARCHAR(255) NOT NULL, field_type VARCHAR(50) NOT NULL, required BOOLEAN NOT NULL, field_order INT NOT NULL, section VARCHAR(255) NOT NULL, options TEXT, form_id BIGINT, created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP NULL, updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP NULL, - CONSTRAINT fk_form_field_form FOREIGN KEY (form_id) REFERENCES form (id) ON DELETE CASCADE + CONSTRAINT fk_form_field_form FOREIGN KEY (form_id) REFERENCES form (id) ON DELETE CASCADE, + INDEX idx_form_id (form_id), + INDEX idx_form_order (form_id, field_order) );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java(1 hunks)src/main/java/ddingdong/ddingdongBE/domain/form/entity/FormField.java(1 hunks)src/main/resources/db/migration/V34__from_and_field_create_table.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java
- src/main/java/ddingdong/ddingdongBE/domain/form/entity/FormField.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🚀 작업 내용
🤔 고민했던 내용
생각해보면 통계 쪽 구현할 때 정말 힘들 것 같아요... 특히 체크박스.
그래도 일단 테이블은 만들지 않은 것으로 했고, 바꿔야한다면 통계쪽 구현하다 수정하겠습니다.
Summary by CodeRabbit
새로운 기능
기술적 개선