Refactor/#461 같은 멤버가 등록중인 노래에 같은 부분을 킬링파트로 등록시 추가 데이터를 저장하지 않는 기능 구현 #469

splitCoding · 2023-09-26T12:13:44Z

📝작업 내용

VotingSong -> VotingSongParts -> Member 로의 많은 N+1 쿼리가 예상되기에

boolean existsByVotingSongAndMemberAndStartSecondAndLength(final VotingSong votingSong,
                                                               final Member member,
                                                               final int startSecond,
                                                               final PartLength length);

해당 메서드를 사용해서 확인했습니다.😊

💬리뷰 참고사항

추후에 해당 쿼리에 대한 분석과 인덱스 적용 여부에 대해서 파악해야 됩니다!!😁

멤버가 같은 파트를 등록했을 때는 200(OK), 아닌 경우에는 201(CREATED) 를 사용해서 클라이언트에게 전달하려 했으나 이 부분에 대해서 추후 프론트 팀원들과의 논의가 필요할 것 같습니다!!🫡

#️⃣연관된 이슈

close #461

생성자 변경으로 인해 실패하는 테스트 수정, sql 스크립트에 해당 변경 내용 적용

github-actions · 2023-09-26T12:16:05Z

Unit Test Results

  84 files   84 suites 12s ⏱️
318 tests 318 ✔️ 0 💤 0 ❌
321 runs 321 ✔️ 0 💤 0 ❌

Results for commit 62521fe.

♻️ This comment has been updated with latest results.

somsom13 · 2023-09-27T01:50:03Z

backend/src/main/java/shook/shook/voting_song/application/VotingSongPartService.java

+
+    private Member findMemberThrowIfNotExist(final Long memberId) {
+        return memberRepository.findById(memberId)
+            .orElseThrow(MemberNotExistException::new);


MemberException.MemberNotExistException 과 같이 바꿔주세요~!

somsom13 · 2023-09-27T01:51:59Z

backend/src/main/java/shook/shook/voting_song/application/VotingSongPartService.java

+                final Map<String, String> errorProperties = Map.of("VotingSongId", String.valueOf(votingSongId));
+                return new VotingSongException.VotingSongNotExistException(errorProperties);


💬 예외를 던질 때, 바로 Map을 주입했던 기존 코드에서는 어떤 식으로 예외를 던지는지 한 번 더 들어가봐야 되더라구요. 그래서 변수를 한 번 추출했습니다.

베로 의견) 통일성이 조금 깨지는 것 같습니다.

somsom13 · 2023-09-27T01:52:51Z

backend/src/main/java/shook/shook/voting_song/application/VotingSongPartService.java


+    private boolean isMemberSamePartExist(final VotingSongPart votingSongPart) {
+        return votingSongPartRepository.existsByVotingSongAndMemberAndStartSecondAndLength(
+            votingSongPart.getVotingSong(),


파라미터로 VotingSong을 받아오는 것이 나을 것 같아요. -> VotingSongPart의 getVotingSong NONE으로 설정합시다!

somsom13 · 2023-09-27T01:54:02Z

backend/src/main/java/shook/shook/voting_song/domain/VotingSongPart.java

@@ -48,7 +48,6 @@ public class VotingSongPart {

    @ManyToOne(fetch = FetchType.LAZY)
    @JoinColumn(name = "voting_song_id", foreignKey = @ForeignKey(name = "none"), updatable = false, nullable = false)
-    @Getter(AccessLevel.NONE)
    private VotingSong votingSong;


위 코멘트와 동일한 내용입니다

Cyma-s

간단한 리뷰 남겨봤씁니다!
시간 되실 때 반영해주세요 :) 크지 않은 변경 사항이라 Approve 하겠습니다!

Cyma-s · 2023-09-27T07:34:14Z

backend/src/test/java/shook/shook/voting_song/ui/VotingSongPartControllerTest.java

+            .contentType(ContentType.JSON)
+            .body(request)
+            .header("Authorization", "Bearer " + accessToken)
+            .when().log().all().post("/voting-songs/" + votingSong.getId() + "/parts")


post("/voting-songs/{songId}/parts", votingSong.getId()) 로 적어도 동일하게 작동했던 것 같아요!

Cyma-s · 2023-09-27T07:35:57Z

backend/src/main/java/shook/shook/voting_song/application/VotingSongPartService.java

    }

-    private void voteToExistPart(final VotingSong votingSong, final VotingSongPart votingSongPart) {
+    private boolean isMemberSameVoteExist(final Member member, final VotingSongPart votingSongPart) {


existSameVoteByMember 라는 메서드 이름은 어떤가요?

Cyma-s · 2023-09-27T07:40:39Z

backend/src/main/java/shook/shook/voting_song/ui/openapi/VotingSongPartApi.java

@@ -27,8 +29,15 @@ public interface VotingSongPartApi {
        description = "파트 수집 중인 노래의 id",
        required = true
    )
+    @Parameter(


굿굿 👍👍👍👍👍

Cyma-s · 2023-09-27T07:48:16Z

backend/src/test/java/shook/shook/voting_song/domain/repository/VoteRepositoryTest.java

+        final boolean isVoteExist = voteRepository.existsByMemberAndVotingSongPart(member, votingSongPart);
+
+        //then
+        Assertions.assertThat(isVoteExist).isTrue();


assertthat static import 해주세요!!

Cyma-s · 2023-09-27T07:49:49Z

backend/src/test/java/shook/shook/voting_song/ui/VotingSongPartControllerTest.java

+        RestAssured.given().log().all()
+            .contentType(ContentType.JSON)
+            .body(request)
+            .header("Authorization", "Bearer " + accessToken)


HttpHeaders.AUTHORIZATION 를 사용해보는 건 어떨까요?

Cyma-s · 2023-09-27T07:49:55Z

backend/src/test/java/shook/shook/voting_song/ui/VotingSongPartControllerTest.java

+        RestAssured.given().log().all()
+            .contentType(ContentType.JSON)
+            .body(request)
+            .header("Authorization", "Bearer " + accessToken)


somsom13

베로가 다 리뷰를 남겨주기도 했고 의도한 로직대로 돌아가는 것 같아서 approve 합니다~

somsom13 · 2023-09-27T09:59:54Z

...nd/src/test/java/shook/shook/voting_song/domain/repository/VotingSongPartRepositoryTest.java

    @Autowired
    private VotingSongPartRepository votingSongPartRepository;

    @Autowired
    private VotingSongRepository votingSongRepository;
+    private static VotingSong SAVED_SONG;


이거 위의 필드랑 한 줄만 뛰고, static을 일반 필드보다 위로 올리면 더 깔끔할 것 같습니다 ㅎㅎ

splitCoding added 5 commits September 26, 2023 19:39

refactor: VotingSongPart 에 ManyToOne 관계의 Member 필드 추가

3a31f1c

생성자 변경으로 인해 실패하는 테스트 수정, sql 스크립트에 해당 변경 내용 적용

feat: 멤버가 등록한 같은 파트가 있는지 확인하는 메서드 추가

08a26c3

feat: VotingSongPartService 멤버가 등록한 같은 파트 처리 로직 추가

8e7c0c9

feat: VotingSongPartController 멤버가 등록한 같은 파트 처리 로직, 테스트 추가

1d5ba15

style: 공백 제거

a872f7d

splitCoding added [ 🌙 BE ] 백엔드 크루들의 멋진 개발 이야기 하나둘셋 야! 🔨 Refactor 꾸준한 개선이 더 나은 애플리케이션을 만든다 labels Sep 26, 2023

splitCoding added this to the 노래 등록 인증샷 트워터 홍보를 준비한다. milestone Sep 26, 2023

splitCoding requested review from Cyma-s and somsom13 as code owners September 26, 2023 12:13

splitCoding self-assigned this Sep 26, 2023

splitCoding requested a review from seokhwan-an as a code owner September 26, 2023 12:13

somsom13 reviewed Sep 27, 2023

View reviewed changes

splitCoding added 2 commits September 27, 2023 15:00

refactor: VotingSongPart 의 Member 필드를 삭제하고 Vote 에 멤버를 추가한다.

bfdd629

refactor: VotingSongPartService 로직 변경

768b3f4

Cyma-s approved these changes Sep 27, 2023

View reviewed changes

somsom13 approved these changes Sep 27, 2023

View reviewed changes

splitCoding added 2 commits September 27, 2023 19:15

refactor: 리뷰 반영

62521fe

Merge branch 'main' into refactor/#461

c073a2d

splitCoding merged commit 77b5dd8 into main Sep 27, 2023

splitCoding deleted the refactor/#461 branch September 27, 2023 10:19

splitCoding restored the refactor/#461 branch September 27, 2023 10:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor/#461 같은 멤버가 등록중인 노래에 같은 부분을 킬링파트로 등록시 추가 데이터를 저장하지 않는 기능 구현 #469

Refactor/#461 같은 멤버가 등록중인 노래에 같은 부분을 킬링파트로 등록시 추가 데이터를 저장하지 않는 기능 구현 #469

splitCoding commented Sep 26, 2023

github-actions bot commented Sep 26, 2023 •

edited

Loading

somsom13 Sep 27, 2023

somsom13 Sep 27, 2023

somsom13 Sep 27, 2023

somsom13 Sep 27, 2023

Cyma-s left a comment

Cyma-s Sep 27, 2023

Cyma-s Sep 27, 2023

Cyma-s Sep 27, 2023

Cyma-s Sep 27, 2023

Cyma-s Sep 27, 2023

Cyma-s Sep 27, 2023

somsom13 left a comment

somsom13 Sep 27, 2023

		final Map<String, String> errorProperties = Map.of("VotingSongId", String.valueOf(votingSongId));
		return new VotingSongException.VotingSongNotExistException(errorProperties);

Refactor/#461 같은 멤버가 등록중인 노래에 같은 부분을 킬링파트로 등록시 추가 데이터를 저장하지 않는 기능 구현 #469

Refactor/#461 같은 멤버가 등록중인 노래에 같은 부분을 킬링파트로 등록시 추가 데이터를 저장하지 않는 기능 구현 #469

Conversation

splitCoding commented Sep 26, 2023

📝작업 내용

💬리뷰 참고사항

#️⃣연관된 이슈

github-actions bot commented Sep 26, 2023 • edited Loading

Unit Test Results

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

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

somsom13 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Sep 26, 2023 •

edited

Loading