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

[FEATURE] reply 기능 구현 완료 #34

Merged
merged 6 commits into from
Oct 15, 2023
Merged

[FEATURE] reply 기능 구현 완료 #34

merged 6 commits into from
Oct 15, 2023

Conversation

lgsok00
Copy link
Collaborator

@lgsok00 lgsok00 commented Oct 14, 2023

댓글 등록 / 조회 / 삭제 기능

@lgsok00 lgsok00 changed the title reply 기능 구현 완료 [FEATURE] reply 기능 구현 완료 Oct 14, 2023
@lgsok00 lgsok00 added the enhancement New feature or request label Oct 14, 2023
Copy link
Member

@donsonioc2010 donsonioc2010 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 26 to 42
private final ReplyService replyService;

@PostMapping
public void createReply(@RequestBody ReplyRequestDto requestDto) {
replyService.createReply(requestDto);
}

@GetMapping("/{articleId}")
public List<ReplyRequestDto> getReplyList(@PathVariable Long articleId, HttpServletRequest request) {
return replyService.getReplyList(articleId, LanguageType.findTypeByRequest(request));
}

@DeleteMapping("/{replyId}")
public ResponseEntity<String> deleteReply(@PathVariable Long replyId) {
replyService.deleteReply(replyId);
return ResponseEntity.ok("삭제처리가 완료되었습니다.");
}
Copy link
Member

Choose a reason for hiding this comment

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

수정은 없는게 의도가 된 부분일까요?
Controller Mapping은 잘했습니다. 👍

많이 좋아졌네요.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

수정은 일부러 안넣었습니다. 감사합니다 !

}

@GetMapping("/{articleId}")
public List<ReplyRequestDto> getReplyList(@PathVariable Long articleId, HttpServletRequest request) {
Copy link
Member

Choose a reason for hiding this comment

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

ReplyRequestDto가 클래스 명칭인데 Response를 해당 DTO로 하고있네요.
Naming은 좀 더 어울리게 수정이 필요할것 같네요.


@NotNull
@Enumerated(EnumType.STRING)
private ReplyStatus isDeleted;
Copy link
Member

Choose a reason for hiding this comment

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

컬럼명칭이 replyStatus가 더 어울릴 것 같은 느낌이 드네요

@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

@OneToMany(mappedBy = "reply")
Copy link
Member

Choose a reason for hiding this comment

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

fetch타입은 명시해주시기 바랍니다.

Comment on lines 73 to 81
@Builder
public Reply(Long id, LanguageType orgReplyLanguage, ReplyStatus isDeleted,
User createdUser, User updatedUser) {
this.id = id;
this.orgReplyLanguage = orgReplyLanguage;
this.isDeleted = isDeleted;
this.createdUser = createdUser;
this.updatedUser = updatedUser;
}
Copy link
Member

Choose a reason for hiding this comment

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

위에 @Builder가 명시되어있는데, 또 명시한 이유가 있을까요?

둘중 하나는 날려버리는게 좋지 않을까 하네요.

Comment on lines +35 to +36
// .title(translateService.naverPapagoTextTranslate(requestDto.getOrgArticleLanguage(), languageType.getLanguageCode(), requestDto.getTitle()))
// .content(translateService.naverPapagoHtmlTranslate(requestDto.getOrgArticleLanguage(), languageType.getLanguageCode(), requestDto.getContent()))
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
Collaborator Author

Choose a reason for hiding this comment

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

이부분은 댓글 로직과 관련이 없어서 일단 막아놨습니다.
지금 pr이 merge되면 Article쪽 service 메서드 합치면서 수정하겠습니다.

Comment on lines +69 to +70
getTranslatePapagoTextArticleContent(requestDto.getOrgArticleLanguage().getLanguageCode(), languageType.getLanguageCode(), requestDto.getTitle()),
getTranslatePapagoHTMLArticleContent(requestDto.getOrgArticleLanguage().getLanguageCode(), languageType.getLanguageCode(), requestDto.getContent()));
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
Collaborator Author

Choose a reason for hiding this comment

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

다른 pr로 게시글 쪽 번역 부분 다시 수정하겠습니다!

@Transactional(readOnly = true)
public List<ReplyRequestDto> getReplyList(Long articleId, LanguageType language) {
Article article = articleRepository.findById(articleId).orElseThrow(() -> ArticleContentNotFoundException.EXCEPTION);
List<Reply> replyList = replyRepository.findAllByArticleAndDeleted(article, false);
Copy link
Member

Choose a reason for hiding this comment

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

?

public interface ReplyRepository extends JpaRepository<Reply, Long> {
List<Reply> findAllByArticleAndDeleted(Article article, boolean isDeleted);
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
Member

@donsonioc2010 donsonioc2010 left a comment

Choose a reason for hiding this comment

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

잘했어요, 이제 좋아요만 남았나요?

@donsonioc2010 donsonioc2010 merged commit 5869220 into dev Oct 15, 2023
1 check passed
@donsonioc2010 donsonioc2010 deleted the feat/28-reply branch October 15, 2023 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants