Skip to content

Post crud task#41

Open
chanmin97 wants to merge 34 commits intomainfrom
post-crud-task
Open

Post crud task#41
chanmin97 wants to merge 34 commits intomainfrom
post-crud-task

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