-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/#3 basic entity #7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
우선 구현하느라 고생하셨습니다.
들여쓰기들이 정리가 되어 있지 않고, 애노테이이션 작성방식도 일관성이 떨어져서 가독성이 좋지 않아 보여요. 코드 작성할 때, 가독성을 고려하면서 작성하면 좋을 것 같아요.
추가로 커밋 메시지 컨벤션을 지켜주세요
feat#3/basic-entity -> feat: (#3) (구현한 내용) ex) basic-entity 작성
들여쓰기 먼저 정리해주시고, 이후에 코멘트 반영해주시면 감사하겠습니다.
src/main/java/org/groomUniv/meet/blinddate/entity/BlindDate.java
Outdated
Show resolved
Hide resolved
| @EnableJpaAuditing // JPA Auditing 사용을 위한 설정 | ||
| public class MeetApplication { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 각 도메인마다 필요할 때, 애노테이션을 지정해서 사용하고 있는데,
차라리 공통패키지에 japAuditing을 활용해서 baseEntity를 만들고, 해당 엔티티 안에서 생성일자, 수정일자를 관리하게 하고
이 baseEntity를 상속해서 사용하는건 어떤가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 겹치는 부분에 대해서는 baseEntity 고려해서 올려보겠습니다 감사합니다
| // JPA Auditing 설명 | ||
| //@CreatedDate: 엔터티가 처음 생성된 날짜를 자동으로 기록합니다. | ||
| // | ||
| //@LastModifiedDate: 엔터티가 마지막으로 수정된 날짜를 자동으로 기록합니다. | ||
| // | ||
| //@CreatedBy: 엔터티를 처음 생성한 사용자를 자동으로 기록합니다. | ||
| // | ||
| //@LastModifiedBy: 엔터티를 마지막으로 수정한 사용자를 자동으로 기록합니다. No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 엔티티에서 jpa Auditing 은 사용되지 않은 것 같아요.
jpa Auditing을 사용한 쪽에 주석을 넣어주시는 게 더 좋을 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아아 네넵 알겠습니다!
| @CreatedBy | ||
| private String createdBy; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스 레벨에 리스너를 등록하셔야 해요.
| @Id@GeneratedValue | ||
| private Long messageId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @Id@GeneratedValue | |
| private Long messageId; | |
| @Id | |
| @GeneratedValue | |
| privat Long messageId; |
애노테이션을 사용할 때는 가독성을 위해서 한줄에 하나씩 사용해주세요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네넵 수정하겟습니다
| @Id@GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long school_id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
애노테이션 사용에서 가독성을 신경써주세요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네넵 수정하겟습니다
| @OneToMany(mappedBy = "school", fetch = FetchType.LAZY, cascade = CascadeType.ALL) | ||
| private List<Member> members = new ArrayList<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
학교가 사라지면, 학생들이 사라지는 건 아니라서, cascade는 빼는게 나을 것 같아요
추가로, cascade 옵션은 사용할 때, 고민을 하고 사용해주세요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아아 네넵
| @CreatedDate | ||
| private LocalDateTime createdAt; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기도 클래스 레벨에 리스너 등록하셔야 정상적으로 동작해요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네넵
| public enum BodyType { | ||
| 마름,보통,퉁퉁,근육질 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| public enum BodyType { | |
| 마름,보통,퉁퉁,근육질 | |
| } | |
| THIN("마름"), | |
| NORMAL("보통"); | |
| private final String description; |
이런식으로 작성하면 가독성이 좋아질 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네넵 모든 이넘타입 그런식으로 수정하겠습니다
| public enum Drinking { | ||
|
|
||
| 안함,가끔,자주 | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
타입들은 가급적이면 영어로 작성하고 설명을 붙이는 방향으로 진행하면 좋을 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네넵
sunwoo1256
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!!
repository에 정의된 method 중 jpa 기본 method에서 사용할 수 있는 것들을 재정의하지 않아도 괜찮을 것 같습니다!
| package org.groomUniv.meet.oauth.enums; | ||
|
|
||
| public enum Style { | ||
| 마름,보통,퉁퉁,근육질 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum BodyType과 중복 되는 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네넵 수정하겠습니다 enum type
| public interface BlindDateChatRepository extends JpaRepository<BlindDateChat, Long> { | ||
|
|
||
| // 채팅 ID로 채팅 검색 | ||
| Optional<BlindDateChat> findByBlindDataChatId(Long blindDataChatId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jpa 기본 메소드 findById()를 사용할 수 있을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네넵 재정의 하지 않고 findById()로 고치겠습니다 감사합니다
| import java.util.Optional; | ||
|
|
||
| public interface MeetingChatRepository extends JpaRepository<MeetingChat, Long> { | ||
| Optional<MeetingChat> findBymeetingChatId(Long meetingChatId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이것도 findById()와 중복 되는 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네넵 재정의 하지 않고 findById()로 고치겠습니다 감사합니다
| public interface MeetingRepository extends JpaRepository<Meeting, Long> { | ||
|
|
||
| //미팅 Id 기반으로 검색 | ||
| Optional<Meeting> findByMeetingId(Long meetingId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서도 재정의하지 않고 findById를 사용하면 괜찮을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네넵 재정의 하지 않고 findById()로 고치겠습니다 감사합니다
Co-authored-by: SungHo Jung <65662139+SeongHo5356@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정하느라 고생하셨습니다.
oAuth 패키지 안에 멤버 도메인 관련 엔티티를 전부 넣은 이유가 있을까요?
생각해보면 멤버는 oAuth에만 쓰이진 않을 것 같아서, oAuth를 커먼 폴더 안에 넣고, oAuth 패키지 안에는 oAuth 관련 사항(토큰 관련 기능 등)들로 채우는 게 나을 것 같아요.
멤버 관련해서는 따로 도메인을 관리하는 패캐지를 만들어서 관리하면 좋을 것 같구요.
|
이거 다 만든거면 머지하셔도 될 것 같아요 |
FEAT: 멤버 관련 정의 #3
엔티티 추가, 레포지토리 생성 및 연결
JpaAuditing 사용으로 createDate createdAt
늦게 올려서 죄송합니다 ㅜㅜ 민준님은 그룹에 초대가 안되셔서 그런가 안보이네용 리뷰어로
ㅎㅇㅌ