Feat/#134 파트에 댓글 작성 기능 추가 #146

splitCoding · 2023-07-28T03:10:13Z

📝작업 내용

파트별 댓글

파트에 댓글 생성
파트별 댓글 전체 조회
- 정렬 기준 : 최신순

💬리뷰 참고사항

#️⃣연관된 이슈

close #134

github-actions · 2023-07-28T03:11:55Z

Unit Test Results

114 tests 114 ✔️ 4s ⏱️
  34 suites     0 💤
  34 files     0 ❌

Results for commit ee74351.

♻️ This comment has been updated with latest results.

somsom13

고생하셨어요 스플릿 짱짱 👍 👍

somsom13 · 2023-07-28T07:47:25Z

backend/src/main/java/shook/shook/part/domain/PartComments.java

+
+    public List<PartComment> getCommentsInRecentOrder() {
+        return comments.stream()
+            .sorted(Comparator.comparing((PartComment::getCreatedAt)))


recentOrder이 최신순 정렬이 맞다면 createdAt 이 내림차순으로 정렬되어야 하지 않을까요 ?.?
그리고 괄호가 두 번 들어간건 혹시 의도된 것인지 궁금합니다!

Suggested change

.sorted(Comparator.comparing((PartComment::getCreatedAt)))

.sorted(Comparator.comparing(PartComment::getCreatedAt).reversed())

괄호 두번은 전혀 의도되지 않았습니다ㅎㅎ 💩

somsom13 · 2023-07-28T07:52:01Z

backend/src/main/java/shook/shook/part/ui/PartCommentController.java

+import shook.shook.part.application.dto.PartCommentsResponse;
+
+@RequiredArgsConstructor
+@RequestMapping("/songs/{song_id}/parts/{part_id}/comment")


API url은 복수형으로 통일하는 것은 어떨까요 ㅎㅎ
comment -> comments

somsom13 · 2023-07-28T08:03:29Z

backend/src/main/java/shook/shook/part/application/dto/PartCommentsResponse.java

+public class PartCommentsResponse {
+
+    private List<PartCommentResponse> partReplies;
+


PartCommentsResponse 는 내부에 List 하나만 가지는 것 같은데, 그러면 json으로 응답을 받았을 때 불필요한 뎁스가 깊어지는 건 아닐까~~ 라는 생각이 들어요!

++) 프론트엔드 입장에서는 단일 List만 들어온다는걸 API 명세로 알고 있는 경우에는 포장이 없는게 더 편하다고 합니닷

backend/src/main/java/shook/shook/part/domain/PartComment.java

somsom13 · 2023-07-28T08:16:50Z

backend/src/main/java/shook/shook/part/ui/PartCommentController.java

+    ) {
+        final PartCommentsResponse partReplies = partCommentService.findPartReplies(partId);
+
+        return ResponseEntity.ok().body(partReplies);


💬 body 로 따로 response를 담아준 이유가 있는지도 궁금해요~!

메서드 체인닝 가독성 향상에 있어서 큰 장점 가진다고 생각합니다! 😊
ok(response) 보다는 ok().body(response) 가 가독성이 훨씬 좋다고 생각하여 택하였습니다!

이건 나중에 같이 이야기해보고 통일하면 좋을 것 같습니당 ㅎ.ㅎ

아니 제가 이전에는 ok(response) 를 사용했더라구요 😅 ㅎㅎㅎ

저도 익숙한게 ok(response) 인 것 같습니다!!

ok(response)로 적용하도록 하겠습니다!!👍

somsom13 · 2023-07-28T08:20:00Z

backend/src/test/java/shook/shook/part/domain/PartCommentContentTest.java

+    @NullSource
+    @ParameterizedTest(name = "댓글의 내용이 \"{0}\" 일 때")
+    @ValueSource(strings = {"", " "})


👍 👍 👍

somsom13 · 2023-07-28T08:23:54Z

backend/src/test/java/shook/shook/part/domain/PartCommentsTest.java

+        final List<PartComment> repliesInRecentOrder = comments.getCommentsInRecentOrder();
+
+        //then
+        assertThat(repliesInRecentOrder).usingRecursiveComparison().isEqualTo(List.of(early, late));


이 테스트에서도 최신순 정렬이면 late이 먼저 오는게 맞지 않을까요 ?.?

somsom13 · 2023-07-28T08:26:01Z

backend/src/test/java/shook/shook/part/domain/repository/PartCommentRepositoryTest.java

+        assertThat(savedPartComment).isSameAs(partComment);
+        assertThat(partComment.getId()).isNotNull();


💬 검증할 내용이 여러개인 경우에 assertAll 을 사용하는건 혹시 어떻게 생각하시나요?

저도 assertThat 문이 많아져 가독성이 떨어지는 경우 assertAll을 쓰는 편입니다!👍

하지만 개인적으로 검증 문이 2개 존재할 때 있어서 assertAll을 사용해서 묶어서 가독성 향상이 느껴지지는 않는 것 같아서 사용하지 않았습니다 😁

somsom13 · 2023-07-28T08:30:44Z

backend/src/test/java/shook/shook/part/ui/PartCommentControllerTest.java

+        assertThat(response.getPartReplies()).hasSize(1);
+        assertThat(response.getPartReplies().get(0).getContent()).isEqualTo("댓글 내용");


💬 요런 리스트를 검증하는 테스트에서 usingRecursiveComparison 을 사용하지 않고 따로 검증한 이유가 궁금해요!

검증하려는 대상에 @PrePersist 어노테이션에 의존하는 필드가 존재하여 모든 필드가 같은 Response 인스턴스 생성을 직접하기에는 불가한 상황이였습니다!

검증하려는 필드가 content 한개인 상황에서 usingRecursiveComparsion을 사용하여 아래와 같은 메서드로 검증해도 좋다고 생각합니다~

assertThat(response.getPartReplies()) .usingRecursiveComparison() .ignoringFields("createdAt") .isEqualTo(List.of(new PartCommentResponse("댓글 내용", null)));

개인마다의 생각이 다를 테지만 저한테 있어서는 기존 코드가 조금 가독성이 좋다고 생각하여 사용하였습니다!!😊

backend/src/main/java/shook/shook/part/application/PartCommentService.java

reviewer : somsom13

TypeRef 사용에서 jsonPath().getList() 로 변경

seokhwan-an

전체적으로 코드가 엄청깔끔해서 읽기 쉬웠습니다 👍 👍 또 테스트 역시 꼼꼼하게 작성한 것 역시 좋았습니다
간단한 리뷰 몇가지 남겼습니다.

seokhwan-an · 2023-07-30T14:46:57Z

backend/src/main/java/shook/shook/part/application/dto/PartCommentResponse.java

+    private final String content;
+    private final LocalDateTime createdAt;


id는 따로 응답값으로 안보내주는데 의도하신 것인가요?

현재 코드에서 댓글의 수정, 삭제에 대해서 고려를 하지 않아 필요하지 않은 응답이라고 생각해서 추가하지 않았습니다!!

근데 id는 기본 응답에 포함해도 괜찮을 것 같아요!! 추가하겠습니다~ 👍

seokhwan-an · 2023-07-30T14:56:05Z

backend/src/main/java/shook/shook/part/ui/PartCommentController.java

+        @PathVariable(name = "part_id") final Long partId,
+        @RequestBody PartCommentRegisterRequest request


이 부분 @RequestBody뒤에 final이 빠졌습니다!

seokhwan-an · 2023-07-30T15:18:06Z

backend/src/main/java/shook/shook/part/application/dto/PartCommentResponse.java

+    public static PartCommentResponse from(final PartComment partComment) {
+        return new PartCommentResponse(partComment.getContent(), partComment.getCreatedAt());
+    }


정적 팩토리 메소드를 이용하는데 @AllArgsConstructor의 접근제어자가 현재 public으로 되어 있습니다. 이 부분 private으로 변경하면 좋을 것 같아요~

seokhwan-an · 2023-07-30T15:24:04Z

backend/src/test/java/shook/shook/part/ui/PartCommentControllerTest.java

+            .extract()
+            .body()
+            .jsonPath()
+            .getList(".", PartCommentResponse.class);


요청결과를 상태코드만 체크하는 것이 아니라 응답내용까지 체크하는 것 최곱니다 👍 👍

Reviewed-by: seokhwan-an

Cyma-s

잘 구현해주셔서 코드적인 부분보다는 컨벤션 관련 코멘트가 많네요
수고 많으셨습니다 ~~~~

backend/src/main/java/shook/shook/part/application/PartCommentService.java

backend/src/main/java/shook/shook/part/application/dto/PartCommentRegisterRequest.java

backend/src/main/java/shook/shook/part/domain/PartComment.java

Cyma-s · 2023-07-30T14:43:31Z

backend/src/main/java/shook/shook/part/domain/PartComment.java

+    @PrePersist
+    private void prePersist() {
+        createdAt = LocalDateTime.now().truncatedTo(ChronoUnit.MILLIS);
+    }


MILLIS truncate 하는 문제에 대해서는 같이 이야기 해봐야 할 듯 해요 🤔
저번에 문제 났던 부분이 createdAt 테스트 할 때 github flow 의 서버 시간 precision과 달라서 그랬었는데, createdAt 테스트를 따로 할 필요가 있을지도 같이 의논해보면 좋을 것 같습니다 ~ 😄

MILLIS truncate 관련으로 문제가 되었던 부분이 실제 DB 데이터에 시간이

와 같이 밀리초에 대한 정보가 없던 것이였습니다!! 🧐

근데 확인해보니 이 과정은 쿼리에 now() 함수를 사용하면서 생긴 문제였고 api를 사용하여 데이터를 생성시에는

밀리초까지 잘 나오는 것을 확인했습니다!! 👍

이전 createdAt 테스트에서 문제가 되었던 부분은 다시 재차 체크해보았는데 성공할 때도 있고 실패할때도 존재했습니다.
(prev를 저장 후 createdAt 생성까지 밀리초단위에서는 오차가 없는 경우) ( 차이가 있는 경우)
이런 상황에서 isBefore(), isAfter()는 비교하는 시간과 같을 경우를 다르다고 판단하기에 실패하는 경우가 존재합니다!
그래서 추후에 isBetween() 은 검증하려는 시간이 비교하려는 시작과 끝에 포함되는 케이스까지 성공으로 판단하기에 검증 방식을 바꿈으로써 해결했었습니다.

제가 파악하기에는 서버 시간 precision 의 차이보다 위에 문제였던 것 같습니다!!

이 부분에 대한 테스트에 대한 필요에 대해서는 추후 논의 해보면 좋을 것 같습니다!!👍

Cyma-s · 2023-07-30T14:48:59Z

backend/src/main/java/shook/shook/part/ui/PartCommentController.java

+    @PostMapping
+    public ResponseEntity<Void> registerPartComment(
+        @PathVariable(name = "part_id") final Long partId,
+        @RequestBody PartCommentRegisterRequest request


final 이 빠진 것 같아요 😄

Cyma-s · 2023-07-30T14:56:14Z

backend/src/test/java/shook/shook/part/domain/PartCommentContentTest.java

+        //given
+        //when
+        //then
+        Assertions.assertDoesNotThrow(() -> new PartCommentContent("댓글 내용"));


💬 static import 해주시면 제 마음이 편할 거 같아요 😊

Cyma-s · 2023-07-31T04:32:56Z

backend/src/test/java/shook/shook/part/ui/PartCommentControllerTest.java

+    @Autowired
+    private PartCommentRepository partCommentRepository;
+
+    @DisplayName("파트에 댓글 등록시 상태코드 CREATED 를 반환한다.")


💬저번에 DisplayName은 상태코드 숫자를 썼던 것 같은데, 하나로 통일해보는 건 어떠신가요 ☺️

상태코드 숫자로 통일하였습니다!!👍

* config: 프론트엔드 ci 설정 - lint - test - build * style: 주석 삭제 * config: frontend 폴더에서만 trigger 되도록 설정 * config: pull request types 추가 * config: jobs 통합 * config: 프론트엔드 테스트 실패 슬랙 알림 * config: node version lts/hydrogen(18.16.1)

* config: dotenv설치 및 환경변수 설정 * config: msw 라이브러리 설치 및 적용 * refactor: msw 동적 import 로직 간결하게 변경 * config: package-lock 업데이트 및 script에 NODE_ENV 환경변수 * refactor: DefinePlugin common에서 공통적으로 적용 * feat: msw 파일 구조 변경 및 handler 실패시에도 response * config: package-lock.json 최신화 --------- Co-authored-by: cruelladevil <dev.timetravel@gmail.com>

* config: paths 속성 삭제 - 폴더를 기준으로 트리거되던 action들을 무조건 실행하도록 변경 * config: node-version matrix strategy를 env 환경 변수로 변경

* chore: 로고 아이콘 디렉터리 이동 * design: 헤더 높이 theme 추가 * design: 레이아웃 및 헤더 컴포넌트의 박스 사이징 조정 1. 헤더 높이에 맞게 main content 높이 조정 2. 로고 반응형 추가 3. 박스 사이징 시안에 맞게 수정 * design: 레이아웃 min-height 로 변경 컨텐츠가 높이를 넘어갔을 때 자연스럽게 표시되도록 수정 * design: 유튜브 영상 화면비 16:9로 반응형 구현 * design: 토글 그룹 spacing 삭제 * chore: 더미 앨범 자켓 이미지 추가 * design: 킬링파트 등록 페이지 디자인 조정 1. 더미 엘범 자켓 태그추가 2. spacing 세부 조정 3. 슬라이더 / 인터벌 인풋 위치 변경 4. 텍스트 반응형 디자인 추가 * design: theme 미적용된 속성에 theme 일괄 적용 * design: 인터벌 인풋 엑티브 색상 제거 * fix: jest 컴포넌트 랜더링 시 ThemeProvider를 제공하도록 수정 기존 render함수를 래핑한 renderWithTheme 함수 추가 및 적용 * config: 스토리북에 theme관련 설정 추가 1. 스토리북 themeProvider 적용 2. 스토리북 디렉터리 하위에도 tsc 적용되도록 설정 추가 --------- Co-authored-by: 윤정민 <dev.timetravel@gmail.com>

1. script에서 주입하던 NODE_ENV 환경변수 삭제 2. webpack.common.js 파일에서 분기로 처리하던 dotenv path 관련 로직 삭제 3. dev, prod 설정 파일 내 dotenv path 적용

* feat: 총 득표수가 높은 상위 40개 노래 목록 조회 기능 구현 * feat: Song 에 imageUrl 필드 추가 * refactor: BE 코드리뷰 반영 Reviewed-by: seokhwan-an, somsom13, Cyma-s * refactor: BE 코드리뷰 추가 반영

* config: 백엔드 CD 설정 추가 * chore: 백엔드 CD 파일 이름 변경

Reviewed-by: somsom13, Cyma-s

Cyma-s

아프신데도 열심히 해주셨네요
고생하셨습니다~

somsom13

아픈 와중에도 너무 고생하셨어요~~~~ 최고예요!!

seokhwan-an

스플릿 사소한 부분까지 체크하는 것에서 좋았습니다.😄 리뷰 반영된 코드에서 수정할 부분이 없는 것 같습니다. 고생하셨습니다.

feat: 파트에 댓글 작성, 최신 순으로 조회 기능 구현

6a1de24

splitCoding added [ 🌙 BE ] 백엔드 크루들의 멋진 개발 이야기 하나둘셋 야! ✨ Feat 꼼꼼한 기능 구현 중요하죠 labels Jul 28, 2023

splitCoding requested review from Cyma-s, seokhwan-an and somsom13 July 28, 2023 03:10

splitCoding self-assigned this Jul 28, 2023

somsom13 requested changes Jul 28, 2023

View reviewed changes

splitCoding force-pushed the feat/#134 branch from e285210 to 311e9b0 Compare July 29, 2023 07:41

refactor: BE 코드리뷰 반영

2e3d719

reviewer : somsom13

splitCoding force-pushed the feat/#134 branch from 311e9b0 to 2e3d719 Compare July 29, 2023 08:04

splitCoding added 2 commits July 29, 2023 18:26

refactor: RestAssured 응답 본문 추출 로직 변경

1e82457

TypeRef 사용에서 jsonPath().getList() 로 변경

refactor: 사용하지 않는 PartCommentsResponse 제거

e3a4aaf

seokhwan-an requested changes Jul 30, 2023

View reviewed changes

refactor: BE 코드리뷰 반영

47ffc86

Reviewed-by: seokhwan-an

Cyma-s requested changes Jul 31, 2023

View reviewed changes

cruelladevil and others added 13 commits August 2, 2023 13:18

fix: npm ci 실행 step 추가 (#137)

88648ad

fix: working directory 작성 및 if 절 순서 변경 (#138)

6173c60

Fix/#141 폴더 상관없이 항상 CI를 실행하도록 변경 (#142)

3c7f8d5

* config: paths 속성 삭제 - 폴더를 기준으로 트리거되던 action들을 무조건 실행하도록 변경 * config: node-version matrix strategy를 env 환경 변수로 변경

config: 윈도우 환경변수 이슈 해결 (#155)

1c4506d

1. script에서 주입하던 NODE_ENV 환경변수 삭제 2. webpack.common.js 파일에서 분기로 처리하던 dotenv path 관련 로직 삭제 3. dev, prod 설정 파일 내 dotenv path 적용

Feat/#167: 백엔드 CD 설정 추가 (#168)

c63e128

* config: 백엔드 CD 설정 추가 * chore: 백엔드 CD 파일 이름 변경

hotfix: 백엔드 CD pwd 로깅 추가 (#169)

9368f34

config: 입력 받은 브랜치 CD 실행하도록 변경 (#170)

1ec51c9

hotfix: 따옴표 삭제 (#171)

c48251a

config: 실행 권한 root 로 지정 (#173)

cd53f85

Cyma-s and others added 3 commits August 2, 2023 13:18

config: sudo 권한 삭제 (#174)

aa9bae6

fix: application.yml 수정 (#181)

602411e

refactor: BE 코드리뷰 반영

60e4e18

Reviewed-by: somsom13, Cyma-s

splitCoding requested review from Creative-Lee, ukkodeveloper and cruelladevil as code owners August 2, 2023 04:20

Merge branch 'main' into feat/#134

99ca5fd

splitCoding removed request for Creative-Lee, cruelladevil and ukkodeveloper August 2, 2023 04:25

refactor: Repository 어노테이션 추가

ee74351

Cyma-s approved these changes Aug 2, 2023

View reviewed changes

somsom13 approved these changes Aug 2, 2023

View reviewed changes

seokhwan-an approved these changes Aug 2, 2023

View reviewed changes

splitCoding merged commit d045531 into main Aug 2, 2023

splitCoding deleted the feat/#134 branch August 2, 2023 04:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat/#134 파트에 댓글 작성 기능 추가 #146

Feat/#134 파트에 댓글 작성 기능 추가 #146

splitCoding commented Jul 28, 2023

github-actions bot commented Jul 28, 2023 •

edited

Loading

somsom13 left a comment

somsom13 Jul 28, 2023

splitCoding Jul 29, 2023

somsom13 Jul 28, 2023

somsom13 Jul 28, 2023

somsom13 Jul 28, 2023

splitCoding Jul 29, 2023 •

edited

Loading

somsom13 Jul 31, 2023

splitCoding Aug 2, 2023

somsom13 Aug 2, 2023

somsom13 Jul 28, 2023

somsom13 Jul 28, 2023

somsom13 Jul 28, 2023

splitCoding Jul 29, 2023

somsom13 Jul 28, 2023

splitCoding Jul 29, 2023 •

edited

Loading

seokhwan-an left a comment

seokhwan-an Jul 30, 2023

splitCoding Jul 31, 2023

seokhwan-an Jul 30, 2023

seokhwan-an Jul 30, 2023 •

edited

Loading

seokhwan-an Jul 30, 2023

Cyma-s left a comment

Cyma-s Jul 30, 2023

splitCoding Aug 2, 2023 •

edited

Loading

Cyma-s Jul 30, 2023

Cyma-s Jul 30, 2023

Cyma-s Jul 31, 2023

splitCoding Aug 2, 2023

Cyma-s left a comment

somsom13 left a comment

seokhwan-an left a comment •

edited

Loading

	.sorted(Comparator.comparing((PartComment::getCreatedAt)))
	.sorted(Comparator.comparing(PartComment::getCreatedAt).reversed())

		public class PartCommentsResponse {

		private List<PartCommentResponse> partReplies;

		assertThat(savedPartComment).isSameAs(partComment);
		assertThat(partComment.getId()).isNotNull();

		assertThat(response.getPartReplies()).hasSize(1);
		assertThat(response.getPartReplies().get(0).getContent()).isEqualTo("댓글 내용");

		private final String content;
		private final LocalDateTime createdAt;

		@PathVariable(name = "part_id") final Long partId,
		@RequestBody PartCommentRegisterRequest request

Feat/#134 파트에 댓글 작성 기능 추가 #146

Feat/#134 파트에 댓글 작성 기능 추가 #146

Conversation

splitCoding commented Jul 28, 2023

📝작업 내용

💬리뷰 참고사항

#️⃣연관된 이슈

github-actions bot commented Jul 28, 2023 • edited Loading

Unit Test Results

somsom13 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

splitCoding Jul 29, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

splitCoding Jul 29, 2023 • edited Loading

Choose a reason for hiding this comment

seokhwan-an left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seokhwan-an Jul 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cyma-s left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

splitCoding Aug 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Cyma-s left a comment

Choose a reason for hiding this comment

somsom13 left a comment

Choose a reason for hiding this comment

seokhwan-an left a comment • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Jul 28, 2023 •

edited

Loading

splitCoding Jul 29, 2023 •

edited

Loading

splitCoding Jul 29, 2023 •

edited

Loading

seokhwan-an Jul 30, 2023 •

edited

Loading

splitCoding Aug 2, 2023 •

edited

Loading

seokhwan-an left a comment •

edited

Loading