[BE] issue344: 리뷰 리팩토링 #348

sc0116 · 2022-09-14T09:11:28Z

요약

리뷰 리팩토링

세부사항

리뷰 관련 파일 스터디룸 패키지 내부로 이동 및 리팩토링

close #344

tco0427

수고하셨습니다..!!
패키지 이동하면서 StudyRoom 의 메소드를 활용한 부분이 인상깊네요!

backend/src/main/java/com/woowacourse/moamoa/studyroom/service/ReviewService.java

backend/src/main/java/com/woowacourse/moamoa/studyroom/domain/StudyRoom.java

backend/src/main/java/com/woowacourse/moamoa/studyroom/service/ReviewService.java

backend/src/test/java/com/woowacourse/moamoa/studyroom/domain/StudyRoomTest.java

backend/src/test/java/com/woowacourse/moamoa/studyroom/domain/review/ReviewTest.java

jaejae-yoo

짱구 고생하셨습니다! 👍
ReviewControllerTest와 SearchingReviewControllerTest 테스트가 깨지는 데 확인부탁드려요 ~

backend/src/main/java/com/woowacourse/moamoa/studyroom/domain/StudyRoom.java

backend/src/main/java/com/woowacourse/moamoa/studyroom/controller/ReviewController.java

verus-j · 2022-09-28T11:12:54Z

backend/src/main/java/com/woowacourse/moamoa/studyroom/domain/StudyRoom.java

@@ -48,6 +51,13 @@ public Article write(final Accessor accessor, final String title, final String c
        throw new NotParticipatedMemberException();
    }

+    public Review writeReview(final Accessor accessor, final String content) {
+        if (!isPermittedAccessor(accessor)) {
+            throw new NotParticipatedMemberException();


아마 StudyRoom에 UneditableArticleException이 있을 건데 이거와 합치는게 좋을 듯 합니다. 당연히 이름도 Article을 빼서 적절한 이름으로 변경해야겠죠 ㅎㅎ

앗 합치는건 좋은데 리뷰 작성시 예외 발생하는 경우에 "수정할 수 없는 아티클 예외"를 사용하는게 맞나용?

verus-j · 2022-09-28T11:23:19Z

backend/src/main/java/com/woowacourse/moamoa/studyroom/domain/review/Review.java

+    public void updateContent(final AssociatedStudy associatedStudy, final Reviewer reviewer, final String content) {
+        validateReview(associatedStudy, reviewer);
+        this.content = content;
    }

-    public void updateContent(final Reviewer reviewer, final String content) {
-        validateReviewer(reviewer);
-        this.content = content;
+    public void delete(final AssociatedStudy associatedStudy, final Reviewer reviewer) {
+        validateReview(associatedStudy, reviewer);
+        deleted = true;


Article처럼 Accessor를 도입해보는 것도 좋을 것 같습니다.

기존 메서드 유지하면서 Accessor 사용한 메서드 추가하고 테스트 통과한거 확인한 뒤 지우는 절차를 밟았는데 재밌네요!

verus-j · 2022-09-28T11:28:34Z

backend/src/main/java/com/woowacourse/moamoa/studyroom/service/ReviewService.java

+        memberRepository.findById(memberId)
+                .orElseThrow(MemberNotFoundException::new);


StudyRoom에서 작성 가능한 사용자를 체크하는데 member가 DB에 존재하는지 확인할 필요가 없을 것 같습니다.

StudyRoom에서는 스터디에 참여한 회원인가를 확인하고, 저 부분은 해당 id의 멤버가 존재하냐를 확인하는거라 다르긴 하지만 지금 생각해보면 굳이 회원이 존재하는지 확인안해도 될 것 같네요! 저 로직을 타려면 굳이굳이 힘들게 포스트맨으로 보내야만 탈 것 같네용...

verus-j · 2022-09-28T11:29:11Z

backend/src/main/java/com/woowacourse/moamoa/studyroom/service/ReviewService.java

+    private final MemberRepository memberRepository;
+    private final StudyRoomRepository studyRoomRepository;
+
+    public Long writeReview(final Long memberId, final Long studyId, final WriteReviewRequest writeReviewRequest) {


WriteReviewRequest와 EditingReviewRequest가 동일하기 때문에 하나로 합치는게 일단은 좋을 것 같습니다.

verus-j · 2022-09-28T11:31:58Z

backend/src/main/java/com/woowacourse/moamoa/studyroom/service/ReviewService.java

+        final Review review = reviewRepository.findById(reviewId)
+                .orElseThrow(ReviewNotFoundException::new);
+
+        review.updateContent(new AssociatedStudy(studyId), new Reviewer(memberId), editingReviewRequest.getContent());


Article과 비슷한 구조로 만들 수 있을 것 같습니다. Review가 Accessor를 사용해 수정하게 되면, StudyRoom, Member를 DB에 조회할 필요가 없을 것 같습니다.

Accessor를 사용하도록 수정했는데요! 그래도 해당 스터디나 회원이 있는지 여부는 확인해야 하는거 아닌가요??

verus-j · 2022-09-28T11:34:17Z

backend/src/test/java/com/woowacourse/moamoa/studyroom/domain/StudyRoomTest.java

+        assertDoesNotThrow(() -> sut.writeReview(accessor, "content"));
+    }
+
+    @DisplayName("스터디에 참여하지 않은 사용자는 리뷰를 작성할 수 없다.")


작성자 ID는 맞으나 스터디 ID가 다른 케이스도 다뤄야할 것 같습니다.

작성에 있어서는 두 가지가 같은 의미 아닌가요?? 만약 수정, 삭제에서 작성자는 맞지만 스터디가 다른 경우를 말씀하시는거면 ReviewTest에 작성되어있습니다!(삭제만 있고 수정은 없어서 추가했습니다)

2022-moamoa · 2022-10-05T07:14:25Z

Analysis Details

7 Issues

0 Bugs
0 Vulnerabilities
7 Code Smells

Coverage and Duplications

No coverage information (69.50% Estimated after merge)
No duplication information (0.40% Estimated after merge)

Project ID: woowacourse-teams_2022-moamoa_AYKvd_z4VbW_bWBvgn13

View in SonarQube

tco0427

충돌 해결하느라 고생하셨습니다..!!

sc0116 added 3 commits September 14, 2022 16:17

refactor: review 관련 파일 studyroom 패키지로 이동

6d9568b

refactor: review 생성하는 경우 Study 대신 StudyRoom 사용하도록 변경

cd28911

feat: 리뷰 수정, 삭제 시 해당 스터디에 작성된 리뷰인지 검증하는 로직 추가

5b5f0cf

sc0116 added 🖥 backend New backend feature 🛠 refactoring Refactor code labels Sep 14, 2022

sc0116 added this to the Milestone 5 milestone Sep 14, 2022

sc0116 requested review from verus-j, tco0427 and jaejae-yoo September 14, 2022 09:11

sc0116 self-assigned this Sep 14, 2022

sc0116 linked an issue Sep 14, 2022 that may be closed by this pull request

[BE] 리뷰 리팩토링 #344

Closed

This comment has been minimized.

Sign in to view

tco0427 removed this from the Milestone 5 milestone Sep 14, 2022

refactor: 테스트 람다 코드 수정

d104222

This comment has been minimized.

Sign in to view

tco0427 approved these changes Sep 15, 2022

View reviewed changes

Merge branch 'develop' into refactor/344-review-move-the-package

ef46ac0

This comment has been minimized.

Sign in to view

chore: Merge 충돌 해결

5d914b5

jaejae-yoo requested changes Sep 17, 2022

View reviewed changes

backend/src/main/java/com/woowacourse/moamoa/studyroom/domain/StudyRoom.java Show resolved Hide resolved

backend/src/main/java/com/woowacourse/moamoa/studyroom/controller/ReviewController.java Outdated Show resolved Hide resolved

refactor: githubId에서 memberId로 대체하면서 생긴 dao 문제 및 테스트 오류 해결

71d2404

This comment has been minimized.

Sign in to view

refactor: EOF 해결

0ee7d0d

This comment has been minimized.

Sign in to view

verus-j reviewed Sep 28, 2022

View reviewed changes

verus-j requested changes Sep 28, 2022

View reviewed changes

sc0116 added 2 commits September 29, 2022 13:49

refactor: 리뷰 수정 및 삭제 시 Accessor 사용하도록 변경

82ef993

refactor: 리뷰 수정 및 삭제 시 기존 로직 제거

bfc6c1a

style: dto명 변경 및 동일한 로직의 dto 제거

49d4cbb

This comment has been minimized.

Sign in to view

jaejae-yoo approved these changes Sep 29, 2022

View reviewed changes

sc0116 added 2 commits October 4, 2022 13:31

refactor: ReviewService에서 필요없는 의존성 제거

1ef5c7a

test: 테스트 코드에서 사용하지 않는 부분 제거

a4bf8e4

sc0116 force-pushed the refactor/344-review-move-the-package branch from 8720320 to a4bf8e4 Compare October 5, 2022 06:41

sc0116 added 2 commits October 5, 2022 15:55

Merge 충돌 해결

967ffb7

test: 테스트 코드 수정

7e7a61c

tco0427 approved these changes Oct 5, 2022

View reviewed changes

verus-j approved these changes Oct 5, 2022

View reviewed changes

verus-j merged commit 4cb0804 into develop Oct 5, 2022

tco0427 deleted the refactor/344-review-move-the-package branch October 6, 2022 10:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] issue344: 리뷰 리팩토링 #348

[BE] issue344: 리뷰 리팩토링 #348

sc0116 commented Sep 14, 2022 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

tco0427 left a comment

This comment has been minimized.

jaejae-yoo left a comment

This comment has been minimized.

This comment has been minimized.

verus-j Sep 28, 2022

sc0116 Sep 29, 2022

verus-j Sep 28, 2022

sc0116 Sep 29, 2022

verus-j Sep 28, 2022

sc0116 Sep 29, 2022

verus-j Sep 28, 2022

sc0116 Sep 29, 2022

verus-j Sep 28, 2022

sc0116 Sep 29, 2022

verus-j Sep 28, 2022

sc0116 Sep 29, 2022

This comment has been minimized.

2022-moamoa bot commented Oct 5, 2022

tco0427 left a comment

		memberRepository.findById(memberId)
		.orElseThrow(MemberNotFoundException::new);

[BE] issue344: 리뷰 리팩토링 #348

[BE] issue344: 리뷰 리팩토링 #348

Conversation

sc0116 commented Sep 14, 2022 • edited Loading

요약

세부사항

This comment has been minimized.

This comment has been minimized.

tco0427 left a comment

Choose a reason for hiding this comment

This comment has been minimized.

jaejae-yoo left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

2022-moamoa bot commented Oct 5, 2022

Analysis Details

7 Issues

Coverage and Duplications

tco0427 left a comment

Choose a reason for hiding this comment

sc0116 commented Sep 14, 2022 •

edited

Loading