[BE] feat: 하이라이트 추가 및 수정 API 구현 #813

skylar1220 · 2024-10-10T05:24:41Z

resolves [BE] 하이라이트 추가 및 수정 API를 구현한다. #805

🚀 어떤 기능을 구현했나요 ?

하이라이트 추가 및 수정 API 구현

🔥 어떻게 해결했나요 ?

하이라이트 도메인 및 비즈니스 로직을 추가했습니다.
flyway로 테이블도 함께 추가되도록 했습니다.

📝 어떤 부분에 집중해서 리뷰해야 할까요?

하이라이트 수정 요청이 오면 삭제, 저장의 로직이 정상적으로 실행되는지 로직인지 봐주세요.
여러 예외가 추가되었는데 패키지, 메시지 등이 적절한지 봐주세요.
겹겹이 패스츄리 request 구조로 인해 나온 장풍 맞은 스트림들 어떡하죠ㅎ
중복을 확인하기 위해 일단 HighlightKey라는 클래스를 만들었는데 이거 그냥 메서드로 만들지 고민입니당
부족한 검증이 있는지. 반대로 불필요한 검증이 있는지 봐주세요.

📚 참고 자료, 할 말

잘 돌아가는지 확인만 가능하게 테스트 짜서 테스트 리팩토링이 필요합니다..
추가로 해야할 것: 복합 인덱스 동시성, 조회 성능 확인
재귀적 valid 추가는 다른 pr에서 해도 될까요? 기능 완성에 우선 집중하고 싶어서요!

github-actions · 2024-10-10T05:26:04Z

Test Results

126 tests 126 ✅ 4s ⏱️
49 suites 0 💤
49 files 0 ❌

Results for commit 24b706b.

♻️ This comment has been updated with latest results.

Kimprodp

커비 고생많았습니다

우선 1차 코멘트 남겨요 확인 부탁해요~

그리고 우리가 명령에 해당하는 로직(CUD)는 객체 지향적으로 하기로 했던가 했으면 좋겠나 얘기했던 것 같은데, 절차지향적으로 되어 있는 것 같아요.
때문에 검증 로직이 상당히 길어지고 복잡해지는 것 같아요. 객체를 생성하면서 생성자에서 책임을 넘기면 간단하게 검증할 수 있지 않을까요?

우선 코멘트 확인 후 다시 리뷰 할게요~

Kimprodp · 2024-10-10T05:31:55Z

backend/src/main/java/reviewme/highlight/controller/HighlightController.java

@@ -19,7 +19,7 @@ public class HighlightController {
    @PostMapping("/v2/highlight")
    public ResponseEntity<Void> highlight(@Valid @RequestBody HighlightsRequest request,
                                          @SessionAttribute("reviewRequestCode") String reviewRequestCode) {
-        highlightService.highlight(request);
+        highlightService.highlight(request, reviewRequestCode);


리뷰 컨트롤러처럼 개행을 맞추면 어떨까요?

@GetMapping("/v2/reviews/{id}") public ResponseEntity<ReviewDetailResponse> findReceivedReviewDetail( @PathVariable long id, @SessionAttribute("reviewRequestCode") String reviewRequestCode ) { ReviewDetailResponse response = reviewDetailLookupService.getReviewDetail(id, reviewRequestCode); return ResponseEntity.ok(response); }

Kimprodp · 2024-10-10T05:39:41Z

backend/src/main/java/reviewme/review/repository/AnswerRepository.java

+            where q.id = :questionId
+            """, nativeQuery = true)
+    Set<Long> findIdsByQuestionId(long questionId);
+}


지금 아루의 PR이 올라와 있으니 미리 JPQL로 바꾸죠!

Kimprodp · 2024-10-10T05:43:24Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+
+    public void validate(HighlightsRequest request, long reviewGroupId) {
+        validateAnswerByReviewGroup(request, reviewGroupId);
+        validateQuestionByReviewGroup(request, reviewGroupId);


요청의 뎁스 순서로 하면 validateQuestionByReviewGroup이 먼저 되어야 할 것 같아요

오 맞네요 질문 먼저 검증!

Kimprodp · 2024-10-10T05:48:49Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+        validateLineIndex(request);
+        validateRange(request);
+        validateDuplicate(request);
+    }


validateAnswerByReviewGroup, validateQuestionByReviewGroup, validateAnswerByQuestion

이 부분은 highlight가 연관된 것들을 id로 가지고 있어서 절차적으로 검증할 수 밖에 없는 부분인데요. 리뷰 그룹과 질문으로 답변을 가져오는 것으로 한번에 검증과 동시에 자원을 가져올 수 있지 않을까요? 로직과 쿼리 한 번으로 감당할 수 있을 것 같아요.

리팩토링 pr에서 반영하겠습니다!

Kimprodp · 2024-10-10T05:50:31Z

backend/src/main/java/reviewme/highlight/domain/HighLight.java

+
+    @Column(name = "review_group_id", nullable = false)
+    private long reviewGroupId;
+


굳이 questionId와 reviewId를 가지고 있지 않아도 되지 않을까요?
answerId로 소속은 충분히 알 수 있다고 생각해요,

questionId와 reviewGroupId를 말하는 게 맞겠죠?
새로운 하이라이트가 들어왔을 때 question id = ? and review_group_id = ? 인 조건문으로 기존 데이터 삭제를 진행하기 떄문에 넣었어요~

JOIN해서 DELETE하는 게 좋지 않을까요? 데이터 삭제에만 필요한 것이 DB에 속성으로 등록될 필요가 있을지 고민해보아야겠네용,,

맞네요 answerId로 처리하고 review group id와 question id는 뺐어요!

Kimprodp · 2024-10-10T05:52:02Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+        }
+    }
+
+    private void validateRange(HighlightsRequest request) {


이 검증은 객체 책임으로 가지는 것이 좋을 것 같다는 의견입니다~ (산초 말투)

의견인 것입니다~

서비스적인 속성이 아니네요 반영완!

Kimprodp · 2024-10-10T05:53:44Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+        }
+    }
+
+    private void validateLineIndex(HighlightsRequest request) {


이 부분도 객체의 책임으로 할 수 있는 방법이 없을까요?

(개인적으로 답변을 함께 생성자에 넘겨서 의존하지 않고 answer를 사용하게 하거나 답변에 대한 index 길이 검증 책임을 가지는 클래스가 있어도 될 것 같아요)

일급 컬렉션을 통해도 좋겠네요. 절차지향을 빌렸지만, 그래도 주는 객체지향임을 명심합시다 ⛰️

저도 동의합니다~

리팩토링 pr에서 반영하겠습니다!

donghoony

앞쪽 커밋 보고 일단 RC 올립니다~! 💪🏻

donghoony · 2024-10-10T05:29:36Z

backend/src/main/resources/db/migration/V3__create_highlight_table.sql

+    questionId      BIGINT NOT NULl,
+    review_group_id BIGINT NOT NULl,
+    answer_id       BIGINT NOT NULl,
+    line_index      BIGINT NOT NULl,
+    start_index     BIGINT NOT NULl,
+    end_index       BIGINT NOT NULl,


NULl ???

index는 몇억 단위로 커질 일이 없는데 BIGINT로 하지 않고 그냥 INT로 해도 되지 않을까요?

BIGINT, INT는 DB에서는 반영하되, 어플리케이션에서 long을 int로 바꾸지는 않아도 될 듯해요.

db에 차지하는 크기를 줄이는 목적이니 ddl만 바꾸면 된다는 말이죠? 반영완!

(어플리케이션이 DB에 종속적일 필요가 없다는 맥락에서 바꾸지 않아도 된다는 이야기를 했슴다)

donghoony · 2024-10-10T05:35:37Z

backend/src/main/java/reviewme/highlight/controller/HighlightController.java

+        highlightService.highlight(request, reviewRequestCode);
        return ResponseEntity.ok().build();


커밋 순서가,, 서비스 변경 사항은 포함돼있지 않았네요 😢

아차차😂

donghoony · 2024-10-10T05:37:03Z

backend/src/main/java/reviewme/highlight/repository/HighlightRepository.java

@@ -4,4 +4,6 @@
 import reviewme.highlight.domain.HighLight;

 public interface HighlightRepository extends JpaRepository<HighLight, Long> {
+
+    void deleteByReviewGroupIdAndQuestionId(long reviewGroupId, long questionId);


메서드 이름으로 생성된 쿼리는 1건당 DELETE 쿼리가 날아가는 것으로 알고 있어요.

@Modifying, @Query로 한 번에 다 날아가게 하면 어떨까요?

오오.. 기본 메서드는 건별로!! 반영완!

donghoony · 2024-10-10T05:38:12Z

backend/src/main/java/reviewme/highlight/service/HighlightService.java

+                .stream()
+                .flatMap(highlightRequest -> highlightRequest.lines().stream()
+                        .flatMap(line -> line.ranges().stream()
+                                .map(range -> new HighLight(
+                                        highlightRequest.answerId(), highlightsRequest.questionId(), reviewGroupId,
+                                        line.index(), range.startIndex(), range.endIndex()


for문으로 일단 돌려놓고 어떻게 해볼게요........!!!!!

donghoony · 2024-10-10T05:38:59Z

backend/src/main/java/reviewme/reviewgroup/repository/ReviewGroupRepository.java

@@ -10,5 +10,7 @@ public interface ReviewGroupRepository extends JpaRepository<ReviewGroup, Long>

    Optional<ReviewGroup> findByReviewRequestCode(String reviewRequestCode);

+    Optional<Long> findIdByReviewRequestCode(String reviewRequestCode);


위의 findByReviewRequestCode의 결과에 .getId를 하는 것과 다를 게 있을까요?

지웠어요 반영완!

donghoony · 2024-10-10T05:42:59Z

backend/src/main/java/reviewme/highlight/service/exception/InvalidHighlightRangeException.java

+
+    public InvalidHighlightRangeException(long startIndex, long endIndex) {
+        super("하이라이트 시작 글자 인덱스는 종료 글자 인덱스보다 작아야해요.");
+        log.info("Highlight index is out of bound - startIndex: {}, endIndex: {}", startIndex, endIndex);


out of bound는 주어진 배열보다 더 큰/작은 값에 대한 예외 사항이예요. Invalid highlight range: {} {}로 해도 충분하지 않을까요?

donghoony · 2024-10-10T05:43:50Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightKey.java

+    private final Long answerId;
+    private final Long lineIndex;
+    private final Long startIndex;
+    private final Long endIndex;


donghoony · 2024-10-10T05:44:21Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightKey.java

+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        HighlightKey that = (HighlightKey) o;
+        return answerId.equals(that.answerId) && lineIndex.equals(that.lineIndex) &&
+               startIndex.equals(that.startIndex) && endIndex.equals(that.endIndex);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(answerId, lineIndex, startIndex, endIndex);
+    }


@EqualsAndHashCode(of) 로 해결할 수 없었나용?

@EqualsAndHashCode 만 붙이면 모든 필드에 데헤서 equals 처리해주는군요!

donghoony · 2024-10-10T05:45:29Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightKey.java

+import lombok.Getter;
+
+@Getter
+public class HighlightKey {


객체 도출 좋네요! 이거 @Embedded로 하지 않은 이유가 있나요?

추가로 네이밍도 약간 수정하면 좋겠습니다~ 키는 너무 광범위해용

highlightPosition 어떤가요? (사실 아루가 제안함)

donghoony · 2024-10-10T05:57:19Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightKey.java

+    public HighlightKey(Long answerId, Long lineIndex, Long startIndex, Long endIndex) {
+        this.answerId = answerId;
+        this.lineIndex = lineIndex;
+        this.startIndex = startIndex;
+        this.endIndex = endIndex;
+    }


애초에 begin <= end는 불변식이니 여기에서 검증해도 되지 않을까요?

nayonsoso

커비 수고했어요☺️
원래 누군가 당연하게 하는 일은, 그렇게 되기까지 많은 시간이 걸린다고 해요!
조금씩 다듬어가면 훨씬 더 좋은 코드가 될 것 같아요~

nayonsoso · 2024-10-10T06:11:04Z

backend/src/main/java/reviewme/highlight/service/HighlightService.java

+    private void saveNewHighlight(HighlightsRequest highlightsRequest, long reviewGroupId) {
+        List<HighLight> highLights = highlightsRequest.highlights()
+                .stream()
+                .flatMap(highlightRequest -> highlightRequest.lines().stream()
+                        .flatMap(line -> line.ranges().stream()
+                                .map(range -> new HighLight(
+                                        highlightRequest.answerId(), highlightsRequest.questionId(), reviewGroupId,
+                                        line.index(), range.startIndex(), range.endIndex()
+                                ))
+                        ))
+                .toList();


스트림으로 하나의 라인에 처리하려고 하는 커비의 마음은 알겠지만 읽는 사람 입장에서 잘 읽히지 않네요🥲 적당한 함수로 분리한다면, 더 작은 단위로 쪼갤 수 있고, 함수 이름으로도 의미를 전달할 수 있을 것이라 생각해요~

냅다 짜고, 스트림으로 바꾸고.. 반성합니다😂 다시 해볼게요!

nayonsoso · 2024-10-10T06:28:57Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+        for (HighlightRequest highlight : request.highlights()) {
+            TextAnswer textAnswer = textAnswerRepository.findById(highlight.answerId())
+                    .orElseThrow(() -> new AnswerNotFoundByIdException(highlight.answerId()));
+            long maxLineIndex = textAnswer.getContent().lines().count();


오 이렇게 개행이 된 라인 수를 가져올 수 있군요! 야미~

nayonsoso · 2024-10-10T06:32:47Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+    private void validateQuestionByReviewGroup(HighlightsRequest request, long reviewGroupId) {
+        Set<Long> providedQuestionIds = questionRepository.findIdsByReviewGroupId(reviewGroupId);
+        long submittedQuestionId = request.questionId();
+
+        if (!providedQuestionIds.contains(submittedQuestionId)) {
+            throw new SubmittedQuestionAndProvidedQuestionMismatchException(submittedQuestionId, providedQuestionIds);
+        }
+    }


questionRepository.existByIdAndReviewGroupId() 어떠신가요? 성능도 더 좋을 것 같아요!

직접적으로 연관이 없는 review group을 통해 question을 가져오도록 하는 것이 너무 특정적이고 비즈니스 로직적이지 않나해서
review group의 속성인 template을 가져오고, template을 통해 question을 가져오도록 변경하였습니다!

nayonsoso · 2024-10-10T06:39:33Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+        validateAnswerByReviewGroup(request, reviewGroupId);
+        validateQuestionByReviewGroup(request, reviewGroupId);
+        validateAnswerByQuestion(request);
+        validateLineIndex(request);
+        validateRange(request);
+        validateDuplicate(request);


(개인 취향일 수도 있음)
무엇을 validate___By___ 함수들을 무엇을 검증하는지 더 명확하게 이름을 바꿀 수 있을 것 같아요!

예를 들면
validateAnswerByReviewGroup
-> validateHighlitedAnswerBelongToReviewGroup

validateQuestionByReviewGroup
-> validateHighlitedQuestionBelongToReviewGroup

validateAnswerByQuestion
-> validateHighlightedAnswerBelongToQuestion

(이건 정말정말 개인 취향일 수 있음)
혹은 이 세가지를 묶어서 이런 validation 함수를 만들수도 있겠네요!

validateHighlightedResourceHiarachyRelationship()

(아루 코멘트 차용) validateReviewGroupContainsQuestion처럼 validate + 긍정문의 검증 행위 형식으로 바꿔보았어요!

nayonsoso · 2024-10-10T06:41:16Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+        }
+    }
+
+    private void validateLineIndex(HighlightsRequest request) {


저도 동의합니다~

donghoony

일단 어프룹합니다~ 코멘트는 확인해주세요~!

donghoony · 2024-10-11T01:55:26Z

backend/src/main/java/reviewme/highlight/domain/Highlight.java

+    @Embedded
+    private HighlightPosition highlightPosition;
+
+    public Highlight(long answerId, HighlightPosition highlightPosition) {


(캡슐화 관점) 이 클래스 밖에서 HighlightPosition을 몰라도 되지 않을까요? 내부에서 만들어주는 게 낫지 않을까 싶습니다.

외부에서 예외를 터뜨려 early fail을 의도했다면, 예외를 터뜨리는 책임이 누구인지도 고민해보면 좋습니다. 내부 클래스를 세부적으로 알고 있는 자는 누구인가? 잘 모르는 클래스가 예외를 발생할 책임이 있는가? 등등...

더 빠르게 예외가 발생하지 않나 잠깐 생각했는데 시점의 차이는 없겠네요! 반영완!

예외를 터뜨리는 책임이 누구인지도 고민해보면 좋습니다.

내부 클래스를 세부적으로 알고있는자는 (이전 기준) HighlightService였네요.
하지만 HighlightService보다는 안그래도 필드로 갖고있어서 관련이 높은 Highlight 도메인이 HighlightPosition의 세부사항을 아는 게 맞겠네요.

잘 모르는 클래스가 예외를 발생할 책임이 있는가? 등등...

예외를 터트리는 책임이라...
예외가 발생하는 곳은 HighlightPosition 자체이지만, 타고타고 들어오는 곳인 HighlightService도 예외를 발생한 책임�을 가지게 되었다고 보는 걸까요? 그렇다면 역시 같은 맥락으로 HighlightService가 그 책임을 가지지 않는 것이 적절하겠어요!

예외도 객체입니다.

객체지향에서는 변경의 전파를 최소화해야 합니다.

를 기준으로 생각해본 질문이었습니다~

donghoony · 2024-10-11T01:56:24Z

backend/src/main/java/reviewme/highlight/domain/HighlightPosition.java

+    private void validateRange(long startIndex, long endIndex) {
+        if (startIndex > endIndex) {
+            throw new InvalidHighlightRangeException(startIndex, endIndex);


0 넘어가는 건 이곳에서 검증하지 않나요 ?

검증이 필요하겠네요 추가했습니다~

donghoony · 2024-10-11T01:58:02Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+            for (HighlightedLineRequest line : highlight.lines()) {
+                long submittedLineIndex = line.index();
+                if (maxLineIndex < submittedLineIndex) {
+                    throw new InvalidHighlightLineIndexException(submittedLineIndex, maxLineIndex);
+                }
+            }
+        }
+    }


레벨1, 2에서 어떻게 리팩터링했는지 돌아보면 실마리가 보일거예요~

해볼게요!

donghoony · 2024-10-11T04:32:53Z

backend/src/main/java/reviewme/highlight/domain/HighlightPosition.java

@@ -25,14 +25,20 @@ public class HighlightPosition {
    private long endIndex;

    public HighlightPosition(long lineIndex, long startIndex, long endIndex) {
-        validateRange(startIndex, endIndex);
+        validatePositiveIndexNumber(startIndex, endIndex);


Positive은 0보다 큰 값을 의미합니다. NonNegative로 음이 아닌을 의미하면 어떨까요?

오호 헷갈렸네요 반완!

nayonsoso · 2024-10-11T04:14:49Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+            long maxLineIndex = textAnswer.getContent().lines().count();
+
+            for (HighlightedLineRequest line : highlight.lines()) {
+                long submittedLineIndex = line.index();
+                if (maxLineIndex < submittedLineIndex) {
+                    throw new InvalidHighlightLineIndexException(submittedLineIndex, maxLineIndex);
+                }
+            }


이 if 문 수정이 필요해보여요.
지금 maxLineIndex 는 서술형 답변의 "라인 수(마지막 idx + 1)"을 나타내기 때문에 <= 여야 합니다.
그리고, maxLineIndex 라는 변수명을 answerLineCount 혹은 existingLineNumbers같은 걸로 바꾸는 것도 의미 전달이 더 잘 될 것 같아요.

그리고 테스트 코드에서도 아래 부분 수정해주세요!

long answerLineCount = textAnswer.getContent().lines().count(); HighlightedLineRequest highlightedLineRequest = new HighlightedLineRequest(answerLineCount + 1, List.of()); // ❗️이 부분을 answerLineCount로 바꿔야 경계값이 됩니다. HighlightRequest highlightRequest = new HighlightRequest(textAnswer.getId(), List.of(highlightedLineRequest)); HighlightsRequest highlightsRequest = new HighlightsRequest(questionId, List.of(highlightRequest));

비교대상 두개 모두 index 개념이어야 쉽게 이해갈 것 같아서 providedMaxLineIndex로 해봤는데 어떤가요?
저 경계값은 고쳐야겠네요! 반완!

음~ 좋네용 😋

nayonsoso · 2024-10-11T04:35:41Z

컬비 Test 코드 autoWired 필드에 private 붙여주세요!

nayonsoso

커비 진짜로 수고 많았어요🥹
이번 기능에서 진짜진짜 핵심이 되었던 기능..!
커비 덕분에 가능해졌네요 ㅎㅅㅎ❤️

Kimprodp

코멘트는 확인 후 다음 PR에서 반영 부탁!

Kimprodp · 2024-10-11T06:44:54Z

backend/src/main/java/reviewme/highlight/domain/Highlight.java

+    private long answerId;
+
+    @Embedded
+    private HighlightPosition highlightPosition;


깔끔하다 👍👍👍

Kimprodp · 2024-10-11T06:51:52Z

backend/src/main/java/reviewme/highlight/service/HighlightService.java

+    private final HighlightValidator highlightValidator;
+
+    @Transactional
+    public void highlight(HighlightsRequest request, String reviewRequestCode) {


메서드명 그냥 하이라이트.. 괜찮을까요? 도메인명하고도 동일한데

리팩토링 pr에서 editHighlight로 변경하였습니다~

Kimprodp · 2024-10-11T06:55:06Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+    }
+
+    private void validateReviewGroupContainsQuestion(HighlightsRequest request, long reviewGroupId) {
+        long templateId = reviewGroupRepository.findById(reviewGroupId)


HighlightService에서 찾은 리뷰 그룹의 아이디를 통해서 보낸 id를 또 리뷰 그룹 repo에서 찾는 것은 비효율적인 것 같아요.

Kimprodp · 2024-10-11T06:59:06Z

backend/src/main/java/reviewme/highlight/service/HighlightService.java

+        saveNewHighlight(request);
+    }
+
+    private void deleteOldHighlight(long questionId, long reviewGroupId) {


오래된... 보단 이전 어때요?

beforeHighlight로 수정!

Kimprodp · 2024-10-11T07:26:45Z

backend/src/main/java/reviewme/highlight/service/validator/HighlightValidator.java

+        validateReviewGroupContainsAnswer(request, reviewGroupId);
+        validateQuestionContainsAnswer(request);
+        validateLineIndex(request);
+        // TODO: 중복 요청 검증 추가 예정


여기 부분이 다 리팩터링 예정인가요?

리팩토링 pr에서 제거했어요!

Kimprodp · 2024-10-11T07:27:12Z

backend/src/test/java/reviewme/highlight/service/HighlightServiceTest.java

+        TextAnswer textAnswer1 = new TextAnswer(questionId, "text answer1");
+        TextAnswer textAnswer2 = new TextAnswer(questionId, "text answer2");
+        Review review = reviewRepository.save(new Review(templateId, reviewGroupId, List.of(textAnswer1, textAnswer2)));
+


순서를
리뷰를 등록 하고나서 하이라이트를 저장하는 것이 어떤가요?

Kimprodp · 2024-10-11T07:29:36Z

backend/src/test/java/reviewme/highlight/domain/HighlightPositionTest.java

+    void 하이라이트의_인덱스들이_0보다_작은_경우_예외를_발생한다() {
+        assertThatCode(() -> new HighlightPosition(1, -2, -1))
+                .isInstanceOf(NegativeHighlightIndexException.class);
+


skylar1220 added 12 commits October 10, 2024 10:15

feat: 하이라이트 기능을 위한 도메인, 테이블, 레포지토리 생성

c62884c

refactor: reviewRequestCode도 서비스에 함께 넘기도록 수정

5979829

feat: 하이라이트 수정을 위한 삭제, 저장 로직 구현

fa5aa38

feat: 하이라이트 수정을 위한 검증 로직 구현

b55df2f

db: 하이라이트 조회 성능과 동시성을 위한 복합 인덱스 추가

cbbc950

fix: 리뷰 그룹으로 질문 검증하는 로직 수정

84898cf

refactor: 레포지토리에 기존에 있던 메서드 활용하도록 수정

2b74e5e

fix: 테이블명 오류 수정

eb1459e

refactor: 초기화시 필드 순서 수정

f4418cc

test: 하이라이트 레포지토리 테스트 추가

921cc6e

test: 하이라이트 입력 검증 테스트 추가

b3a58c3

test: 하이라이트 수정 기능 서비스 테스트 추가

a88d090

skylar1220 added ✨ BE 🚀 기능 labels Oct 10, 2024

skylar1220 added this to the 6차 스프린트: 최종장 milestone Oct 10, 2024

skylar1220 requested review from donghoony, nayonsoso and Kimprodp October 10, 2024 05:24

skylar1220 self-assigned this Oct 10, 2024

skylar1220 linked an issue Oct 10, 2024 that may be closed by this pull request

[BE] 하이라이트 추가 및 수정 API를 구현한다. #805

Closed

donghoony changed the title ~~[BE] 하이라이트 추가 및 수정 API 구현~~ [BE] feat: 하이라이트 추가 및 수정 API 구현 Oct 10, 2024

skylar1220 added 2 commits October 10, 2024 14:31

refactor: 생성자 파라미터 및 필드 순서 변경

b8ce456

fix: not null 설정 오타 수정

ad59c37

Kimprodp requested changes Oct 10, 2024

View reviewed changes

donghoony requested changes Oct 10, 2024

View reviewed changes

nayonsoso requested changes Oct 10, 2024

View reviewed changes

skylar1220 added 3 commits October 10, 2024 15:58

refactor: 검증 순서 변경

f87efad

refactor: jpa 기본 메서드로 쿼리 여러개 나가는 부분을 @query로 변경

d3abfd3

style: 개행 수정

95cf051

skylar1220 added 2 commits October 10, 2024 16:00

db: 하이라이트 인덱스 관련 데이터 타입 int로 수정

c700e45

refactor: 하이라이트 표시 위치 관련 속성을 객체로 분리

834d900

skylar1220 force-pushed the be/feat/805-create-edit-highlight branch from 1ab6d99 to 834d900 Compare October 10, 2024 07:21

skylar1220 added 13 commits October 10, 2024 16:22

refactor: JPQL로 변경

16f5d34

refactor: 기존 레포지토리 메서드를 사용하도록 변경

f71d9de

refactor: 메서드 네이밍 수정

4b28bd4

refactor: 하이라이트 위치 속성 객체에 추가 설정

cd6a16c

refactor: 하이라이트 엔티티에서 reviewGroupId, questionId 필드 제거

fbd8fc4

refactor: 기존 하이라이트 제거를 answerId를 통해 하도록 변경

1ce93e6

test: 책임을 이동한 테스트 제거 및 테스트 오류 수정

ba72762

db: 테이블에 questionId, review_group_id 컬럼 제거 반영

1c16507

refactor: 기존 중복 요청 메서드 제거

a7f25b8

refactor: 중복 요청의 경우 하나만 저장하도록 EqualsAndHashCode 재정의

52c350b

refactor: 하이라이트 저장시 순서 보장 적용

7e546aa

refactor: HighlightPosition 검증, 객체 생성 후 하이라이트 객체 생성하도록 변경

55895db

refactor: 엔티티의 equals 조건 id로 복원 및 중복 제거 로직 삭제

aaf1c6c

donghoony approved these changes Oct 11, 2024

View reviewed changes

skylar1220 added 3 commits October 11, 2024 11:05

refactor: HighlightPosition 객체 생성 위치 생성자 내로 이동

560faec

refactor: 예외명 수정

a3a968f

refactor: 하이라이트 인덱스의 검증 추가

c226e21

donghoony reviewed Oct 11, 2024

View reviewed changes

nayonsoso requested changes Oct 11, 2024

View reviewed changes

skylar1220 added 3 commits October 11, 2024 13:38

refactor: 양수 검증 메서드명 수정

132ff57

test: 필드에 접근 제어자 추가

a9ceb36

fix: 하이라이트 줄 번호 계산 로직 및 변수명 수정

24b706b

skylar1220 mentioned this pull request Oct 11, 2024

[BE] 하이라이트 추가 및 수정 로직을 도메인을 활용해 리팩토링한다. #825

Closed

nayonsoso approved these changes Oct 11, 2024

View reviewed changes

Kimprodp approved these changes Oct 11, 2024

View reviewed changes

skylar1220 merged commit ee2c950 into develop Oct 11, 2024
6 checks passed


		@Column(name = "review_group_id", nullable = false)
		private long reviewGroupId;

		highlightService.highlight(request, reviewRequestCode);
		return ResponseEntity.ok().build();

		@@ -10,5 +10,7 @@ public interface ReviewGroupRepository extends JpaRepository<ReviewGroup, Long>

		Optional<ReviewGroup> findByReviewRequestCode(String reviewRequestCode);

		Optional<Long> findIdByReviewRequestCode(String reviewRequestCode);

[BE] feat: 하이라이트 추가 및 수정 API 구현 #813

[BE] feat: 하이라이트 추가 및 수정 API 구현 #813

Conversation

skylar1220 commented Oct 10, 2024 • edited Loading

🚀 어떤 기능을 구현했나요 ?

🔥 어떻게 해결했나요 ?

📝 어떤 부분에 집중해서 리뷰해야 할까요?

📚 참고 자료, 할 말

github-actions bot commented Oct 10, 2024 • edited Loading

Test Results

Kimprodp left a comment • 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

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

Kimprodp Oct 10, 2024 • 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

donghoony 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

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

skylar1220 Oct 10, 2024 • edited by donghoony 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

nayonsoso 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donghoony 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

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

skylar1220 commented Oct 10, 2024 •

edited

Loading

github-actions bot commented Oct 10, 2024 •

edited

Loading

Kimprodp left a comment •

edited

Loading

Kimprodp Oct 10, 2024 •

edited

Loading

skylar1220 Oct 10, 2024 •

edited by donghoony

Loading