[dev to release] 버그 수정

[HongYeseul]

기존 이미지가 있는 일기의 경우 PATCH 되지 않는 버그가 수정되었습니다.

그 외: code-review Action의 경우 가독성을 위해 job 이름이 변경되었습니다.

[jjaegii]

Review the provided code changes and provide feedback.

Good points:

1. The deleteImage method in DiaryImageService.java now correctly deletes the image associated with a diary.
2. The sendImage method in ImageController.java has been improved to handle cases where the main banner URL does not exist.

Improvement suggestions:

1. Code organization: Consider moving the deleteImage and deleteImageAndMapping methods from DiaryImageService.java to a separate service class, e.g., ImageService.java. This would improve code organization and make it easier to manage image-related logic.
2. Commenting: In ImageController.java, add comments to explain the purpose of each method and any complex logic involved.
3. Error handling: In sendImage method, consider adding a try-catch block to handle cases where the main banner URL does not exist.

Proposed changes:

1. Move the deleteImage and deleteImageAndMapping methods from DiaryImageService.java to ImageService.java.
2. Add comments in ImageController.java to explain the purpose of each method.
3. Implement error handling in sendImage method.

Here is an example of how the refactored code could look like:

// ImageService.java
public class ImageService {
 public void deleteImage(Long imageId) {
 // implementation details...
 }

 public void deleteImageAndMapping(Diary diary) {
 // implementation details...
 }
}

// ImageController.java
@RestController
@RequestMapping("/api")
public class ImageController {
 @Autowired
 private ImageService imageService;

 @PostMapping("/diary/{diaryId}")
 public ResponseEntity<?> sendImage(@PathVariable Long diaryId, @RequestPart("image") MultipartFile image) throws IOException {
 // implementation details...
 }

 @DeleteMapping("/diary/{diaryId}")
 public ResponseEntity<?> deleteDiaryImage(@PathVariable Long diaryId) {
 imageService.deleteImageAndMapping(diaryToImageDAO.findByDiaryId(diaryId).orElseThrow());
 return ResponseEntity.ok().build();
 }
}

Note that this is just a suggestion, and the actual implementation may vary depending on the specific requirements of your project.

[jjaegii]

안녕하세요, 리뷰봇입니다. 댓글 기능 PR 주신 것을 확인하고 몇 가지의 사항을 리뷰 해보았습니다.

1. 좋은 점:

   • CommentDeleteResponseDTO 클래스가 필요하지 않다는 점을 검토했습니다.
   • 코드의 단순화와 유지 보수가 용이할 것이라 생각됩니다.

2. 개선할 점:

   • CommentService 클래스 내에서 변수명 일치 문제: update와 delete 메서드에서 첫 번째 매개변수 CommentId가 대문자로 시작하고 있습니다. 이를 소문자로 변경하여 일관성을 유지해야 합니다.
   • CommentService 클래스 내 로직 제안: mapToDTO를 CommentResponseDTO::from으로 코드 재사용을 가능하게 할 수 있습니다.

3. 제안 사항:

   • DiaryImageService 클래스의 deleteImageAndMapping 메서드에서 imageService.deleteImage(diaryImage.getImage().getId()); 부분을 수정하여 더 명확하고 일관된 코드로 변경할 수 있습니다.
   • 코드 리뷰 워크플로우에서 deploy 노드를 code_review 노드로 변경하여, 코드 리뷰가 더 철저하게 이루어지도록 제안합니다.

수고하셨습니다! 👍

[jjaegii]

안녕하세요! 리뷰봇입니다.

1. 좋은 점:

   • 코드의 변수명과 메서드명이 명확하고 일관성을 가지고 있음을 알 수 있습니다.
   • 클래스 설계가 모듈화되어 있어 재사용성이 높고, 유지보수가 용이할 것 같습니다.
   • 에러 처리 및 로깅에 대한 고려가 잘 되어 있으며, 예외 상황을 적절하게 다루고 있습니다.

2. 개선할 점:

   • CommentResponseDTO 클래스의 필요성을 다시 검토해 보아야 할 수도 있습니다. 만약 isDeleted, createdDate, createdAt, depth, childComments 모두가 필수적이지 않다면, 선택적으로 사용하는 것이 좋습니다. 예를 들어, comment.getDepth()가 null일 수 있기 때문에, 이를 처리하는 로직을 추가해야 할 수도 있습니다.

   • CommentService 클래스의 update 메서드에서 parentCommentId 매개변수가 필요하지만, 이를 생략해도 큰 문제는 없을 것 같습니다. 따라서 선택적 매개변수로 변경하여 옵션으로 처리할 수 있게 해보세요.

     public Comment update(String content, Long parentCommentId) {
     return requestDTO.toEntity(member, diary, parentComment, content);
     }

   • DiaryService 클래스의 makeRewardPoint 메서드에서 정수값을 고정하지 않고, 랜덤하게 생성하도록 변경할 수 있습니다. 이를 통해 보다 다양한 포인트를 제공할 수 있을 것입니다.

     private int makeRewardPoint() {
     return new Random().nextInt(100);
     }

3. 제안 사항:

   • CommentResponseDTO 클래스에서 depth 필드를 제거하고, 필요에 따라 선택적으로 사용하도록 변경할 수 있습니다.

     public record CommentResponseDTO(
     boolean deleted,
     String createdDate,
     String createdAt,
     List<CommentResponseDTO> childComments
     ) {
     public static CommentResponseDTO from(Comment comment) {
     return new CommentResponseDTO(comment.isDeleted(), comment.getCreatedAt().format(DateTimeFormatter.ofPattern("yy년 MM월 dd일")), comment.getCreatedAt().format(DateTimeFormatter.ofPattern("HH:mm")), comment.getChildComments().stream().map(CommentResponseDTO::from).collect(Collectors.toList()));
     }
     }

   • DiaryService 클래스의 removeDiaryLikes 메서드에서 삭제 로직을 분리하여 보다 명확하게 작성할 수 있습니다.

     private void removeDiaryLikes(Long diaryId) {
     List<DiaryLike> diaryLikes = diaryLikeDAO.findAllByDiaryId(diaryId);
     for (DiaryLike like : diaryLikes) {
     imageService.deleteImage(like.getImage().getId());
     }
     diaryLikeDAO.deleteAll(diaryLikes);
     }

   • CommentService 클래스의 toEntity 메서드에서 parentCommentDepth 매개변수를 추가하여, comment depth를 계산하고 validate할 수 있습니다.

     public Comment toEntity(Member member, Diary diary, Comment parentComment, int depth) {
     return Comment.builder()
     .member(member)
     .diary(diary)
     .content(content)
     .parentComment(parentComment)
     .depth(depth)
     .build();
     }

   • ImageController 클래스에서 deleteDiaryImage 메서드를 제거하고, 대신 deleteImageAndMapping 메서드에 로직을 통합할 수 있습니다.

     @PostMapping("/diary/{diaryId}")
     public ResponseEntity<?> deleteDiaryImage(
     @PathVariable Long diaryId
     ) {
     imageService.deleteImage(diaryId);
     return ResponseEntity.ok().build();
     }

   • ClientErrorCode 클래스에서 새로운 에러 코드를 추가하여, 댓글의 깊이 제한에 대한 메시지를 제공할 수 있습니다.

     public enum ClientErrorCode implements ErrorCodeModel {
     IMAGE_MAPPING_NOT_FOUND_ERR(400, "IMAGE_MAPPING_NOT_FOUND_ERR", "해당 이미지를 찾을 수 없습니다.(이미지 등록 값 없음)"),
     AUTHOR_MISMATCH_ERR(403, "AUTHOR_MISMATCH_ERR", "작성자가 아닙니다."),
     COMMENT_ALREADY_DELETED_ERR(400, "COMMENT_ALREADY_DELETED_ERR", "이미 삭제된 댓글입니다."),
     COMMENT_DEPTH_EXCEEDED_ERR(400, "COMMENT_DEPTH_EXCEEDED_ERR", "댓글은 대댓글까지만 허용합니다."),
     // 추가 에러 코드
     }

수고하셨습니다! 코드 리뷰를 통해 개선할 수 있는 부분이 있으니, 이를 참고하여 더욱 나은 코드로 발전시켜 주시기 바랍니다.👍

[jjaegii]

안녕하세요! 리뷰봇으로서 이 코드를 검토하면서 몇 가지 의견을 드리려고 합니다.

1. CommentResponseDTO 클래스의 필요성 현재 CommentResponseDTO 클래스에서 isDeleted 필드가 추가되었습니다. 이는 프론트에서 댓글이 삭제되었는지 여부를 나타내는 데 사용될 수 있습니다. 그러나, 이미 CommentDAO에서 deleted 상태를 관리하고 있으며, 이 필드는 중복된 정보일 수 있습니다. 따라서, CommentResponseDTO의 필요성에 대해 다시 고려해 볼 필요가 있습니다.

2. CommentSaveRequestDTO 클래스 내 메서드 수정 toEntity 메서드에서 parentComment과 함께 depth를 인자로 받아 처리하는 것을 확인했습니다. 이는 댓글 트리 구조를 관리하기 위한 중요한 변경입니다. 그러나, Comment 엔티티의 depth 필드는 CommentResponseDTO에서도 사용되는지, 아니면 단순히 서비스 내에서만 사용되는지는 확인해 보아야 합니다.

3. CommentService 클래스 내 로직 제안 calculateCommentDepth 메서드와 validateCommentDepth 메서드를 추가하여 댓글의 깊이를 관리하고 있습니다. 이는 댓글 트리 구조를 유지하기 위한 중요한 구현입니다. 그러나, depth 값을 계산할 때 부모 댓글의 depth 값을 참조하는 방식이 올바른지 여부에 대해 다시 생각해 볼 필요가 있습니다.

4. DiaryService 클래스 내 메서드 수정 makeRewardPoint 메서드에서 랜덤 값 생성 방법을 변경하여 보다 일관된 포인트를 제공하도록 했습니다. 이는 사용자 경험 개선에 기여할 수 있습니다. 그러나, 이 메서드가 실제로 어떤 계산을 수행하는지에 대한 설명이 필요합니다.

5. ImageController 클래스 내 메서드 수정 deleteDiaryImage 메서드는 삭제된 이미지를 처리하는 데 사용되는 것으로 보입니다. 이는 이미지 관리 시스템의 일관성을 유지하기 위해 중요한 구현입니다. 그러나, 이 메서드를 호출할 때 발생할 수 있는 예외 상황을 고려한 에러 처리가 필요합니다.

6. ClientErrorCode 클래스 내 새로운 코드 추가 COMMENT_DEPTH_EXCEEDED_ERR 코드는 댓글 트리 구조를 관리하는 데 사용되는 것으로 보입니다. 이는 프론트에서 댓글 입력 시 오류 메시지를 표시하기 위해 유용할 것입니다. 그러나, 이 에러 코드가 실제로 어디서 사용되는지에 대한 설명이 필요합니다.

이러한 점들을 검토하여 개선할 수 있는 방법을 제안드린 것으로, 코드의 일관성과 유지 보수성을 높일 수 있을 것이라 생각합니다.

감사합니다! 👍