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/댓글 수정API 추가 #25

Merged
merged 4 commits into from
Aug 18, 2024
Merged

feat/댓글 수정API 추가 #25

merged 4 commits into from
Aug 18, 2024

Conversation

s-hwan
Copy link
Contributor

@s-hwan s-hwan commented Aug 15, 2024

📌 Related Issue

#24

🚀 Description

댓글 수정 가능 API를 추가했습니다

📢 Review Point

원래 댓글 작성한 user 만 수정할 수 있게 검증한번 해보려고 해서
ModifyCommentReuqest에 User를 받아오려고 했는데

  1. User 정보를 전부 다 가져오는거더라구요..? <- 이게 원래 맞는건가...싶은...
  2. 프론트 단에서 User의 Comment 만 수정할 수 있게 거를 수 있는지..? 궁금하구요..
  3. Comment 에서 따로 userId를 들고 있는데 이거로 검증을 할 수 있는지....?

📚Etc (선택)

  1. 궁금한 점 하나 더..! Comment 도메인에 setter를 추가해버렸는데 괜찮은 문제인지

@s-hwan s-hwan changed the title Feat/#24 feat/댓글 수정API 추가 Aug 15, 2024
Copy link
Contributor

@hwangjokim hwangjokim left a comment

Choose a reason for hiding this comment

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

  1. User 정보를 전부 다 가져오는거더라구요..? <- 이게 원래 맞는건가...싶은...
    이것 컨트롤러들 보면 어노테이션 통해서, API 호출한 유저를 주입받는 부분이 있습니다. 그것 참고 부탁드려요
  2. 프론트 단에서 User의 Comment 만 수정할 수 있게 거를 수 있는지..? 궁금하구요..
    뷰 상으로야 할 수 있지만, 이건 충분히 어뷰징할 수 있어요. 백엔드단에서 검증하지 않으면 누구나 어뷰징해서 남의 댓글을 수정해버리게 됩니다.
  3. Comment 에서 따로 userId를 들고 있는데 이거로 검증을 할 수 있는지....?
    정확히는 우린 스프링을 쓰고있으니 User 객체를 들고있는거고, 이것으로 검증하면 됩니다. 다른 기능 수정 구현된 컨트롤러/서비스쪽 코드 참고하면 될 것으로 예상됩니다 (내가 보진 않음)

@@ -54,4 +55,12 @@ public ResponseEntity<Object> deleteComment(@AuthedUser User user, @RequestParam
commentService.deleteComment(user,commentId);
return ResponseEntity.ok().body("OK");
}
@PostMapping("/modify")
Copy link
Contributor

Choose a reason for hiding this comment

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

[blocker] RestFul API를 위해, "행위"를 URL에 묘사하지 말고, "자원"을 URL에 표기한뒤 HTTP METHOD를 통해 행위를 구분해주세요.
이 경우엔 기본 url이 /api/comment니까, 그냥 @PutMapping만 달아주는게 예시입니다.
다른 API도 보면, URL엔 자원을 표기하고 Post, Get, Delete, Put같은 Method를 통해 API의 행위를 구분하는것을 볼 수 있습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

확인했습니다.. 근데 PostMapping PutMapping 이 여러개 있는경우에는 따로 URL에 써줘야 하는거 같은데 그 경우는 어떤 식으로 표기해줘야하나요?!

Copy link
Contributor

Choose a reason for hiding this comment

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

여러개인 경우엔 URL로 구분해줘야 하는 것 맞고, api에 맞게 적절하게 네이밍 지어주면 됩니다!

@@ -54,4 +55,12 @@ public ResponseEntity<Object> deleteComment(@AuthedUser User user, @RequestParam
commentService.deleteComment(user,commentId);
return ResponseEntity.ok().body("OK");
}
@PostMapping("/modify")
@Operation(summary = "댓글 수정 API")
public ResponseEntity<Object> modifyComment(@Valid @RequestBody ModifyCommentRequest request, Errors errors){
Copy link
Contributor

@hwangjokim hwangjokim Aug 15, 2024

Choose a reason for hiding this comment

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

[q] 이것 String으로 "OK' 리턴하게 되어있는데, Object 말고 String으로 쓰면 안돌아가나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

해보겠읍니다..! 근데 전반적으로 이 컨트롤러의 동작 방식이 이해가 잘 안되는데요
이게 프론트에서 요청을 dto에 담아서 Controller 로 날리는건가요? 그런 후에 service에서 메소드 실행을 하고 프론트에 return을 ResponeEntitiy에 담아 보내는 과정이 맞는지 굼금합니다

Copy link
Contributor

@hwangjokim hwangjokim Aug 17, 2024

Choose a reason for hiding this comment

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

  1. 컨트롤러는 DTO 필드 스키마에 맞게, JSON 형태로 API를 호출합니다. 이것을 컨트롤러가 받습니다
  2. JSON 형태로 날아온 것들, @RequestBody 가 붙은 DTO와 필드를 비교해 일치하면 객체로 만들어줍니다.
  3. 서비스 메서드를 통해 로직을 수행하고, Return값을 받습니다.
  4. 프론트에 "OK" 라는 문자열과 함께, HTTP STATUS CODE 200을 리턴합니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. 컨트롤러는 DTO 필드 스키마에 맞게, JSON 형태로 API를 호출합니다. 이것을 컨트롤러가 받습니다
  2. JSON 형태로 날아온 것들, @RequestBody 가 붙은 DTO와 필드를 비교해 일치하면 객체로 만들어줍니다.
  3. 서비스 메서드를 통해 로직을 수행하고, Return값을 받습니다.
  4. 프론트에 "OK" 라는 문자열과 함께, HTTP STATUS CODE 200을 리턴합니다

지식 +1 감사합니다


comment.setContent(request.content());
comment.setCreatedAt(LocalDateTime.now());
commentRepository.save(comment);
Copy link
Contributor

Choose a reason for hiding this comment

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

[major] 그리고, 업데이트는 save가 아니라 JPA에 있는 Dirty Checking이라는 기능을 적극 이용하는 코드를 쓰면 좋을 것 같아요.
Dirty Checking으로 Update하는것의 개념은..
https://jojoldu.tistory.com/415
https://everydayyy.tistory.com/157

내부 용어가 좀 어려울 수 있는데, 이해 안되는 것 바로바로 질문해주세요

@hwangjokim hwangjokim added the new-feature 기능 추가 label Aug 16, 2024
@sh0723 sh0723 closed this Aug 16, 2024
@sh0723 sh0723 reopened this Aug 16, 2024
@s-hwan
Copy link
Contributor Author

s-hwan commented Aug 16, 2024

테스트 코드 작성해보려했는데 쉽지 않더군요... 일단 피드백 반영해서 커밋했습니다.

Copy link
Contributor

@rladmstn rladmstn left a comment

Choose a reason for hiding this comment

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

review point에 대한 답변은 교수님께서 잘해주셨기에,,

@hwangjokim
Copy link
Contributor

카톡에도 써놨지만, 기록차 남깁니다

Resolved 버튼은 코멘트를 단 사람이 누르는 것으로 하고, Change request를 리뷰어가 달았을 경우엔 코멘트 반영 이후 Re-request 버튼을 눌러주세요!

@s-hwan s-hwan requested review from hwangjokim and rladmstn August 16, 2024 14:47
Copy link
Contributor

@hwangjokim hwangjokim left a comment

Choose a reason for hiding this comment

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

수고하셨습니다

@@ -54,4 +55,12 @@ public ResponseEntity<Object> deleteComment(@AuthedUser User user, @RequestParam
commentService.deleteComment(user,commentId);
return ResponseEntity.ok().body("OK");
}
@PostMapping("/modify")
Copy link
Contributor

Choose a reason for hiding this comment

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

여러개인 경우엔 URL로 구분해줘야 하는 것 맞고, api에 맞게 적절하게 네이밍 지어주면 됩니다!

@@ -54,4 +55,12 @@ public ResponseEntity<Object> deleteComment(@AuthedUser User user, @RequestParam
commentService.deleteComment(user,commentId);
return ResponseEntity.ok().body("OK");
}
@PostMapping("/modify")
@Operation(summary = "댓글 수정 API")
public ResponseEntity<Object> modifyComment(@Valid @RequestBody ModifyCommentRequest request, Errors errors){
Copy link
Contributor

@hwangjokim hwangjokim Aug 17, 2024

Choose a reason for hiding this comment

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

  1. 컨트롤러는 DTO 필드 스키마에 맞게, JSON 형태로 API를 호출합니다. 이것을 컨트롤러가 받습니다
  2. JSON 형태로 날아온 것들, @RequestBody 가 붙은 DTO와 필드를 비교해 일치하면 객체로 만들어줍니다.
  3. 서비스 메서드를 통해 로직을 수행하고, Return값을 받습니다.
  4. 프론트에 "OK" 라는 문자열과 함께, HTTP STATUS CODE 200을 리턴합니다

Comment comment = commentRepository.findById(request.commentId())
.orElseThrow(() -> new CommentValidationException(HttpStatus.NOT_FOUND.value(), "존재하지 않는 댓글 입니다."));
if(!comment.getUser().getId().equals(user.getId()))
throw new UserValidationException("댓글 작성자가 아닙니다.");
Copy link
Contributor

@hwangjokim hwangjokim Aug 17, 2024

Choose a reason for hiding this comment

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

@rladmstn 이거 조금 저도 헷갈리는데, @AuthedUser로 주입받은 User도 영속성 컨텍스트의 범위 안에 들어오나요? 맞다면 getId.equals 없이, 그냥 단순 동등성 비교( == ) 로도 해결이 될 것 같아서요

Copy link
Contributor

Choose a reason for hiding this comment

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

영속성 범위가 아니라서 equals로 해야 비교 가능합니담

Copy link
Contributor

Choose a reason for hiding this comment

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

아.. OSIV땜에 될수도 있나 했는데 아닌가보네요 ㅎㅎ

Copy link
Contributor

@rladmstn rladmstn left a comment

Choose a reason for hiding this comment

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

성장추 드립니다!

@sh0723 sh0723 merged commit 13ab437 into develop Aug 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-feature 기능 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants