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

Post crud task #41

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open

Post crud task #41

wants to merge 34 commits into from

Conversation

chanmin97
Copy link
Contributor


name: Post crud task
about: Post crud task


Name about title labels assignees
Post crud task It's for Post crud task "[PR]" PR @chanmin97

작업 내용:

게시판 CRUD 구현

이번에 공들였던 부분:

  • JpaRepository의 기본 기능을 처음 사용해보며 공부하게 됨
  • 컨트롤러에서 ResponseEntity 를 반환하게 하여 헤더, 바디, Http Status 를 캡슐화 해서 보내도록함
  • TestCode는 어려워서 GPT의 도움을 많이 받고 이후 이해하려고 노력함,, (Mock사용부분)

질문:

  • 일단 예외처리나 엔티티 게터세터 등 잘 모르겠어서 작동하게만 만들어둔 부분이 있음.
  • 도커 사용을 처음하다보니 commit 에러를 겪어서 처음에 뭉탱이로 커밋해버리고 이후 updatePost 부분부터 따로 커밋시작함..
  • FK 만들때 User가 필요해서 간단하게 도메인 만들어뒀는데 커밋을 아예 안했어야 했는지?

제출 전 필수 확인 사항:

  • 빌드가 되는 코드인가요?
  • 버그가 발생하지 않는지 충분히 테스트 해봤나요?

@chanmin97 chanmin97 requested review from kkho9654 and wintiger98 April 7, 2024 13:16
@chanmin97 chanmin97 self-assigned this Apr 7, 2024
@chanmin97 chanmin97 added the PR label Apr 7, 2024
@chanmin97 chanmin97 linked an issue Apr 7, 2024 that may be closed by this pull request
9 tasks
@wintiger98
Copy link
Contributor

User는 빼고 커밋해야할듯

@kkho9654
Copy link
Contributor

kkho9654 commented Apr 7, 2024

  • test코드에 실패에 대한 시나리오도 넣으면 좋을 것 같아
  • dto접미사 빼기

Copy link

@wnso-kim wnso-kim 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 43 to 62
@Test
void getAllPosts() throws Exception {

//given
when(postService.getAllPosts()).thenReturn(Arrays.asList(
new Post(1L, "First post", LocalDateTime.now(), LocalDateTime.now(), 0L, 0L, 0L, new User()),
new Post(2L, "Second post", LocalDateTime.now(), LocalDateTime.now(), 0L, 0L, 0L, new User())
));

MockMvc mockMvc = MockMvcBuilders.standaloneSetup(postController).build();

//when
mockMvc.perform(MockMvcRequestBuilders.get("/api/v1/posts"))
//then
.andExpect(status().isOk())
.andExpect(MockMvcResultMatchers.content().json(objectMapper.writeValueAsString(Arrays.asList(
new Post(1L, "First post", LocalDateTime.now(), LocalDateTime.now(), 0L, 0L, 0L, new User()),
new Post(2L, "Second post", LocalDateTime.now(), LocalDateTime.now(), 0L, 0L, 0L, new User())
))));
}
Copy link

Choose a reason for hiding this comment

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

Test를 빠르게 확인하기 위해, 메소드를 한글명으로 하거나 @DisplayName을 사용해 주세요.
그리고, service에서는 필요한 테스트가 없었나요?

Comment on lines 36 to 43
public Post updatePost(Long id, Post postDetails) {
Post post = postRepository.findById(id)
.orElseThrow(() -> new RuntimeException());

post.setContents(postDetails.getContents());

return postRepository.save(post);
}
Copy link

Choose a reason for hiding this comment

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

팀원의 업데이트 방식과 상이합니다. 경호씨는 dirty check 사용하셨어요!

@Getter @Setter
public class Post {
@Id
@GeneratedValue
Copy link

Choose a reason for hiding this comment

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

ID 생성 전략이 기본 값으로 돼 있어서 MySQL을 사용하신다면 sequence 전략일 겁니다.
경호씨는 다른 전략 사용하셨던데, 테이블마다 전략이 다른게 아니라면 맞춰주세요!

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 51 to 56
@DisplayName("게시글 전부 조회 예외 테스트")
@Test
void 게시글전부조회예외테스트() {
when(postRepository.findAll()).thenThrow(new RuntimeException());
assertThrows(RuntimeException.class, () -> postService.getAllPosts());
}
Copy link
Contributor

Choose a reason for hiding this comment

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

그냥 Runtime 예외보다는 구체적으로 어떤 예외인지 정의해서 예외처리하는 것이 좋을 것 같습니다.

Comment on lines 110 to 121
@Test
void 게시글수정테스트() {
//given
when(postRepository.findById(1L)).thenReturn(Optional.of(post));
when(postRepository.save(any(Post.class))).thenReturn(post);
Post updatedPost = new Post();
//when
updatedPost.setContents("Updated Contents");
Post result = postService.updatePost(1L, updatedPost);
//then
assertThat(result.getContents()).isEqualTo("Updated Contents");
}
Copy link
Contributor

Choose a reason for hiding this comment

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

#47 참고해주서 업데이트 리팩토링 해주세요

Comment on lines 9 to 27
@Entity
@Getter @Setter
public class PostReply {
@Id
@GeneratedValue
private Long id;

private String contents;

private LocalDateTime createdAt;

private LocalDateTime updatedAt;

private Long likes;

@ManyToOne(fetch = FetchType.LAZY)
@JoinColumn(name = "post_id")
private Post post;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

dto가 있는데 왜 entity로 postReply가 있는건가요?

Copy link
Contributor

Choose a reason for hiding this comment

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

경호가 잘못봤대요

Comment on lines 34 to 40
public Post(long l, String firstPost, LocalDateTime now, LocalDateTime now1, long l1, long l2, long l3, User user) {
}


public Post() {

}
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.

수정완료

@wintiger98
Copy link
Contributor

좋아요

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

[서브태스크] 게시판 CRUD
4 participants