Skip to content
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

feat: 교육 사항 등록 기능 구현 #37

Merged
merged 9 commits into from
Oct 24, 2023
Merged

feat: 교육 사항 등록 기능 구현 #37

merged 9 commits into from
Oct 24, 2023

Conversation

beomukim
Copy link
Contributor

🖥️ 이런 PR 입니다

교육 사항 등록 기능 구현하였습니다.

CC. 리뷰어

User 클래스에 멘티인지 확인하는 메소드 추가하였습니다. @ddongpuri

테스트 설명

검증로직 예외 발생 테스트

기타

Prefix

PR 코멘트를 작성할 때 항상 Prefix를 붙여주세요.

  • K1: 꼭 반영해주세요 (Request changes)
  • K2: 웬만하면 반영해 주세요 (Comment)
  • K3: 그냥 사소한 의견입니다 (Approve)

@beomukim beomukim added the Feat label Oct 24, 2023
@beomukim beomukim self-assigned this Oct 24, 2023
Copy link
Member

@Juhongseok Juhongseok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코멘트 남겼습니다
고생하셨어요

this.title = title;
this.introduce = introduce;
this.user = user;
this.training = training;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

training에 대한 null체크는 없는건가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Column(name = "training_id")
private Long id;

private String schoolOrOrganization;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

단순 '기관' 이라는 단어로 칭해도 될 거 같아요
피그마에 나온대로 다 작성은 안해도 될듯?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 66 to 69
if (this.role == Role.ROLE_MENTEE) {
return true;
}
return false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (this.role == Role.ROLE_MENTEE) {
return true;
}
return false;
return role.equals(Role.ROLE_MENTEE);

}

@Test
void max_gpa_should_be_greater_than_gpa() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메서드명 한글로

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


@Enumerated(EnumType.STRING)
private Position position;

@Lob
private String introduce;

@Embedded
@ManyToOne
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

내가 가진 여러개의 이력서가 하나의 교육 사항을 공유하나요?
이력서 하나에 교육사항 하나만 작성 가능한지도 궁금합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
return false;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻

Copy link
Contributor

@ddongpuri ddongpuri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다~

@Juhongseok
Copy link
Member

커밋에서 test나 엔티티수정, 검증로직 추가 이런것들은 하나로 합쳐도 좋을 것 같아요

@beomukim beomukim merged commit b2dc262 into develop Oct 24, 2023
1 check passed
@beomukim beomukim deleted the feat/GMMQ-125 branch October 24, 2023 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants