Conversation
# Conflicts: # src/main/java/ddingdong/ddingdongBE/domain/form/entity/Form.java
Walkthrough이번 PR은 여러 도메인 클래스와 DTO에서의 메서드 명 변경, 필드 추가, 그리고 엔티티 간 관계 개선 등을 포함합니다.
Changes
Sequence Diagram(s)sequenceDiagram
participant U as UpdateFormCommand
participant S as FacadeCentralFormServiceImpl
participant F as Form
participant FF as FormField
U->>S: updateForm(updateFormFieldCommands)
S->>F: updateFormFields(updatedFormFields)
F->>FF: 각 FormField 처리 (업데이트/추가/삭제)
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🪧 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
|
Seooooo24
left a comment
Seooooo24
left a comment
There was a problem hiding this comment.
LGTM... 고생 많으셨습니다!
코드 살펴보면서 몰랐던 부분도 많이 알게 됐습니다...ㅎㅎ
KoSeonJe
left a comment
KoSeonJe
left a comment
There was a problem hiding this comment.
제 주관적인 피드백이 조금 들어간 것도 있고, 해결해야할 것도 있는 것 같아요!
알맞게 수정해주시면 될 것 같아요!!
src/main/java/ddingdong/ddingdongBE/domain/formapplication/entity/FormAnswer.java
Show resolved
Hide resolved
src/main/java/ddingdong/ddingdongBE/domain/formapplication/entity/FormApplication.java
Show resolved
Hide resolved
src/main/java/ddingdong/ddingdongBE/domain/formapplication/entity/FormAnswer.java
Show resolved
Hide resolved
| List<FormField> deletedFormFields = this.formFields.stream() | ||
| .filter(formField -> updatedFormFields.stream() | ||
| .filter(updatedFormField -> updatedFormField.getId() != null) | ||
| .noneMatch(updatedField -> updatedField.getId().equals(formField.getId()))) | ||
| .toList(); | ||
| this.formFields.removeAll(deletedFormFields); |
There was a problem hiding this comment.
p4)
가독성을 위해 private 메소드로 분리해도 좋을 것 같아요
| List<FormField> deletedFormFields = this.formFields.stream() | ||
| .filter(formField -> updatedFormFields.stream() | ||
| .filter(updatedFormField -> updatedFormField.getId() != null) | ||
| .noneMatch(updatedField -> updatedField.getId().equals(formField.getId()))) |
There was a problem hiding this comment.
| .noneMatch(updatedField -> updatedField.getId().equals(formField.getId()))) | |
| .noneMatch(updatedField -> updatedField.isEqualsId(formField))) |
p5)
사소하지만 formField 내부에서 처리해줘도 좋을 것 같아요!
| // 삭제될 폼 필드 | ||
| List<FormField> deletedFormFields = this.formFields.stream() | ||
| .filter(formField -> updatedFormFields.stream() | ||
| .filter(updatedFormField -> updatedFormField.getId() != null) |
There was a problem hiding this comment.
| .filter(updatedFormField -> updatedFormField.getId() != null) | |
| .filter(updatedFormField -> updatedFormField.existId()) |
p5)
이것도 사소하지만, formField 내부에서 처리해도 좋을 것 같아요!
Introduce a `deleted_at` column to `form_answer` and `form_application` tables to support soft deletes. Also, update `email` and `phone_number` in `form_application` to enforce non-null constraints with increased length.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/ddingdong/ddingdongBE/domain/formapplication/entity/FormAnswer.java (3)
16-16: LocalDateTime 임포트
deletedAt필드를 처리하기 위해LocalDateTime을 사용하고 있습니다. 타임존 고려가 필요한 경우ZonedDateTime등 다른 타입도 고민해볼 수 있습니다.
37-37: value 컬럼의 TEXT 변환
List<String>데이터를 TEXT 필드로 저장하도록 설정하셨습니다. 대량 데이터가 저장될 수 있으니 DB 성능이나 인덱스 전략을 주의 깊게 살펴보세요.
48-49: deletedAt 필드 컬럼 정의
TIMESTAMP 컬럼으로 선언되었습니다. 시간대가 중요한 서비스라면TIMESTAMP WITH TIME ZONE적용 여부를 검토해보시길 권장합니다.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/ddingdong/ddingdongBE/domain/formapplication/entity/FormAnswer.java(3 hunks)src/main/java/ddingdong/ddingdongBE/domain/formapplication/entity/FormApplication.java(2 hunks)src/main/resources/db/migration/V43__alter_table_form_application_and_form_answer.sql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/ddingdong/ddingdongBE/domain/formapplication/entity/FormApplication.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and analyze
🔇 Additional comments (9)
src/main/resources/db/migration/V43__alter_table_form_application_and_form_answer.sql (4)
1-2: 소프트 삭제 컬럼 추가 (form_answer)
소프트 삭제 구현을 위해deleted_at컬럼이 추가되었습니다. NULL 허용으로 설정되었으니, 삭제 로직이 예상대로 동작하는지 확인해 주세요.
4-5: 소프트 삭제 컬럼 추가 (form_application)
form_application테이블에도 동일하게deleted_at을 추가하여 soft delete 로직을 일관성 있게 유지할 수 있습니다. 문제 없어 보입니다.
7-8:Details
❓ Verification inconclusive
email 컬럼 수정
이메일 컬럼을 255자로 확장하고NOT NULL제약을 추가하였습니다. 기존 데이터 중 null 이나 255자를 초과하는 값이 없는지 확인이 필요합니다.
🏁 Script executed:
#!/bin/bash echo "SELECT COUNT(*) AS NullEmailCount FROM form_application WHERE email IS NULL;" echo "SELECT COUNT(*) AS TooLongEmailCount FROM form_application WHERE LENGTH(email) > 255;"Length of output: 343
이메일 컬럼 수정 – 데이터 무결성 검증 필요
이 변경사항으로 인해 이메일 컬럼이 255자로 확장되고
NOT NULL제약이 추가되었습니다. 다만, 제공된 검증 스크립트는 실제 쿼리 실행 결과가 아닌 쿼리문 텍스트만을 출력했으므로, 실제 데이터베이스 환경에서 아래 쿼리를 실행하여 이메일 컬럼에 null 값이나 255자를 초과하는 값이 없는지 재검증해 주시기 바랍니다.
- 검증할 쿼리:
SELECT COUNT(*) AS NullEmailCount FROM form_application WHERE email IS NULL;SELECT COUNT(*) AS TooLongEmailCount FROM form_application WHERE LENGTH(email) > 255;
10-11:Details
❓ Verification inconclusive
phone_number 컬럼 수정
phone_number컬럼을 255자로 확장하고NOT NULL제약을 추가하였습니다. 기존 데이터 중 null 이나 255자를 초과하는 값이 없는지 확인해 주세요.
🏁 Script executed:
#!/bin/bash echo "SELECT COUNT(*) AS NullPhoneCount FROM form_application WHERE phone_number IS NULL;" echo "SELECT COUNT(*) AS TooLongPhoneCount FROM form_application WHERE LENGTH(phone_number) > 255;"Length of output: 371
전화번호 컬럼 변경 내용 및 검증 요청
form_application테이블의phone_number컬럼이 VARCHAR(255)로 확장되고, NOT NULL 제약이 추가되었습니다.- 변경 전에 기존 데이터에 null 값이나 255자를 초과하는 데이터가 있는지 반드시 확인해 주세요.
- 리뷰에 포함된 아래 SQL 쿼리를 실제 데이터베이스에 실행하여 검증하시기 바랍니다.
SELECT COUNT(*) AS NullPhoneCount FROM form_application WHERE phone_number IS NULL; SELECT COUNT(*) AS TooLongPhoneCount FROM form_application WHERE LENGTH(phone_number) > 255;src/main/java/ddingdong/ddingdongBE/domain/formapplication/entity/FormAnswer.java (5)
7-7: JPA 컬럼 어노테이션 임포트
JPA 컬럼 선언에 필요한 임포트가 추가되었습니다. 별다른 문제 없습니다.
22-23: Hibernate 소프트 삭제용 어노테이션 임포트
소프트 삭제 구현에 필요한@SQLDelete,@SQLRestriction어노테이션이 올바르게 추가되었습니다. JPA 동작과의 충돌 없이 정상 작동하는지 확인해 주세요.
28-29: 소프트 삭제 로직 정의
@SQLDelete와@SQLRestriction으로 삭제 시점을CURRENT_TIMESTAMP로 기록하고, 조회 시deleted_at이NULL인 경우만 반환하도록 설정되었습니다. 이전 리뷰에서 요청되었던deleted_at필드가 잘 반영되어 일관성이 있습니다.
52-57: Builder 매개변수에 deletedAt 추가
deletedAt인자를 포함해 객체를 생성할 수 있게 되어, 소프트 삭제 시점을 주입하거나 테스트하는 데 유연성이 높아졌습니다.
61-61: deletedAt 필드 설정
생성자 내부에서deletedAt필드에 값을 할당하고 있습니다. 특별한 로직이 없어 보이며 정상적인 구현으로 보입니다.
|
(cherry picked from commit 481451e)
3 participants
🚀 작업 내용
폼지 수정 시 지원자 답변 내역 삭제 에러 해결을 위한 로직 개선
에러 원인
폼지 질문 수정 로직 개선
Form,FormField의 양방향 연관관계 설정🤔 고민했던 내용
💬 리뷰 중점사항
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Tests