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] 3차 세미나 과제 코드입니다. #6

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

choyeongju
Copy link
Member

@choyeongju choyeongju commented May 1, 2024

Related Issue 📌

  • close [week2] 세미나 3차 과제 _ 블로그 API #5

  • POST API

    • Request Body Validation
    • 블로그 소유X 사용자가, 해당 블로그에 글 작성 시, 예외처리
  • GET API

    SuccessStatusResponse와 ErrorResponse 적절히 변경해서 Data 담아보내기

  • API 명세서


Description ✔️

API 명세서

🔥API 명세서(클릭)


To Reviewers

  • API 명세가 올바르게 작성된 것인지 궁금합니다
  • 권한이 없는 멤버를 확인하기 위해 validation 코드를 작성하였으나, 적용이 되지 않는 것 같습니다 💦
    추후 다시 한 번 확인해서 코드를 수정하도록 하겠습니다!!
  • blog 관리 부분과 post 관리 부분을 분리해서 구현하는게 코드의 가독성이나 유지보수가 올라갈 것 같다는 생각이 들었습니다.

@softmoca
Copy link
Member

softmoca commented May 3, 2024

영주님 중간고사 시험 시간과 겹쳐 세미나 참석 못하셨는데 과제 하시느라 너무 고생 많으셨습니다~!!

우선 API 명세서에 필요한 정보들은 잘 들어 있는것 같습니다 !
이후 validation 로직을 작성 하시면 reponse 값으로 나오는 종류가 추가 될꺼라 고려하셔서 추가해주시면 좋을것 같습니다 !

세미나를 참석하지 못하시구 독학으로 이번 과제를 진행 하셔서 validation 적용을 못하신것 같은데 다른 OP분들 레파지토리 클론 받으셔서 파일 하나하나 함수 하나하나 로직 흐름을 따라가 보시면 도움이 될것 같습니다 !

고생하셨습니다 !

@JungYoonShin
Copy link
Member

영주님 넘 수고하셨습니다~!~!
✅ API 명세서 표에서 단순히 POST 이렇게만 제목이 써있는데, 포스트 생성 또는 포스트 등록 등과 같이 더 명확하게 써있어도 좋을 것 같아요!

Comment on lines +20 to +24
@Transactional
public String createMember(
MemberCreateDto memberCreate
) { //포스트맨에서 POST로 멤버 생성 가능
Member member = memberRepository.save(Member.create(memberCreate.name(), memberCreate.part(), memberCreate.age()));
Copy link
Member

@JungYoonShin JungYoonShin May 6, 2024

Choose a reason for hiding this comment

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

여기에는 @transactional이 달려있는데 BlogService에 create메소드에는 없는 것 같아요!

Comment on lines +35 to +37
public String getContents() {
return this.content;
}
Copy link
Member

Choose a reason for hiding this comment

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

List<>처럼 여러개를 반환하는 게 아니니까 getContent()로 해도 괜찮지 않을까 싶습니다!

Copy link
Contributor

@sohyundoh sohyundoh left a comment

Choose a reason for hiding this comment

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

과제하느라 고생하셨습니다!

몇가지 코멘트를 남겨두었으니 확인하면 좋을 것 같아용!
계속해서 디벨롭하면 좋은 코드가 되겠네요!

화이팅입니다!

Comment on lines +45 to +46
@OneToMany(mappedBy = "blog", cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER)
private List<Post> posts = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

FetchType을 Eager로 설정한 이유가 있을까요!?

Copy link
Member Author

Choose a reason for hiding this comment

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

다시 한 번 복습하다 보니 fetch = FetchType.LAZY 로 설정하는 것이 좋겠다는 것을 알게되었네요..! 감사합니다아~!

@@ -0,0 +1,40 @@
package org.sopt.homework3.domain;

import jakarta.persistence.*;
Copy link
Contributor

Choose a reason for hiding this comment

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

import할 때 이렇게 종종 5개 정도 같은 라이브러리에서 불러오다 보면 *로 변환되게 되는데요!(인텔리제이에서 자동으로 해줍니다)
그런데 간혹가다 클래스의 이름이 동일해 라이브러리를 많이 참조할 경우 혼란이 될 수 있습니다!
와일드카드 import를 방지하는 방법이 담긴 블로그를 공유드립니다 -!

https://dev-kani.tistory.com/39


@Transactional
public void deleteBlog(Long blogId) {
Blog blog = findById(blogId);
Copy link
Contributor

Choose a reason for hiding this comment

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

여기에 적혀있는 FindById는 아래에 있는 findBlogById를 참조하는 것일까요!?
메서드가 없어서 물어봅니다! 푸시하기 전 최종으로 build해보고 API 테스트를 진행한 다음에 푸시해야 최종 코드가 진행될 것 같아요!

Comment on lines +35 to +36
Member member = memberRepository.findById(memberId).orElseThrow(
()->new EntityNotFoundException("ID에 해당하는 사용자가 존재하지 않습니다."));
Copy link
Contributor

Choose a reason for hiding this comment

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

위에 있는 findById메서드를 사용해도 좋을 것 같아요!

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.

[week2] 세미나 3차 과제 _ 블로그 API
4 participants