[Feature] - 여행기 정렬 및 날짜별 필터링 추가 #442

eunjungL · 2024-09-23T13:35:03Z

✅ 작업 내용

여행기 좋아요 수 반정규화
여행기 정렬 기능 구현
날짜별 필터링 기능 구현

참고사항

request param이 두개가 되고, 둘 다 옵셔널이 되면서 controller의 오버라이딩이 불가능해졌습니다. 기존에 사용하던 @GetMapping(params=)를 활용한 오버라이딩은 두 개 모두 존재할 때 사용할 수 있습니다. 따라서 컨트롤러에 if 분기 로직이 들어갔는데요, 분기로직을 서비스로 빼는게 나을지 의견 부탁드립니다.

github-actions · 2024-09-23T13:36:24Z

Test Results

31 files 31 suites 8s ⏱️
266 tests 266 ✅ 0 💤 0 ❌
278 runs 278 ✅ 0 💤 0 ❌

Results for commit 531a7b7.

♻️ This comment has been updated with latest results.

hangillee

고생하셨습니다 클로버!
몇가지 코멘트 남겼는데 확인해주세요!

hangillee · 2024-09-23T14:56:53Z

backend/src/main/java/kr/touroot/travelogue/controller/TravelogueController.java

+        if (filter.isNoCondition()) {
+            return ResponseEntity.ok(travelogueFacadeService.findSimpleTravelogues(pageable));
+        }
+        return ResponseEntity.ok(travelogueFacadeService.findSimpleTravelogues(filter, pageable));


저는 필터도 결국 비즈니스 로직이라고 생각해서 서비스 계층으로 내려가는게 더 낫지 않을까라고 생각합니다..

저도 동의합니다!! 필터 조건이 있냐 없냐에 따라 다른 동작을 한다는 것을 컨트롤러가 모르는 것이 더 좋을 것 같아요!!

hangillee · 2024-09-23T15:00:13Z

backend/src/main/java/kr/touroot/travelogue/domain/Travelogue.java

@@ -49,19 +51,24 @@ public class Travelogue extends BaseEntity {
    @Column(nullable = false)
    private String thumbnail;

+    @Column(nullable = false)
+    @ColumnDefault("0")


와우 이런 어노테이션이 있군요..

DB 스키마에 디폴트 값을 설정하는 애너테이션이군요옵.

저희 빈 값으로 Insert가 나가지는 않을 것 같은데 필요할까요?
클로버의 의도가 궁금합니다리 🤔

기존 데이터와 호환을 위해 default 값 설정해줬습니다.
근데 사실 기존 데이터가 0으로 전부 밀어져도 되나 싶긴 하네요.
삭제하는게 좋을지 의견 한 번 주시면 감사하겠습니다.

nak-honest

고생 많았슴다 응로버씌!!

dto 패키지 의존성 관련해서는 구두로 이야기 나누어서 따로 리뷰 남기지는 않았습니다!
리뷰 확인 되면 approve 드릴게요!!

nak-honest · 2024-09-24T00:48:27Z

backend/src/main/java/kr/touroot/travelogue/controller/TravelogueController.java

+        if (filter.isNoCondition()) {
+            return ResponseEntity.ok(travelogueFacadeService.findSimpleTravelogues(pageable));
+        }
+        return ResponseEntity.ok(travelogueFacadeService.findSimpleTravelogues(filter, pageable));


저도 동의합니다!! 필터 조건이 있냐 없냐에 따라 다른 동작을 한다는 것을 컨트롤러가 모르는 것이 더 좋을 것 같아요!!

nak-honest · 2024-09-24T00:52:29Z

backend/src/main/java/kr/touroot/travelogue/domain/Travelogue.java

+    public void increaseLikeCount() {
+        likeCount += LIKE_COUNT_WEIGHT;
+    }
+
+    public void decreaseLikeCount() {
+        likeCount -= LIKE_COUNT_WEIGHT;
+    }


변경 감지를 이용하게 되면서, 서로 다른 트랜잭션에서 race condition 이 발생할 수 있겠네요!
저희 모두 인지는 하고 있으나, 현재 데모데이 요구사항을 지키는 것이 더 우선순위가 높은 것 같아 다음 스프린트 때 해결해 보도록 하시죠!!

인지해두겠습니다 👍

nak-honest · 2024-09-24T01:09:40Z

backend/src/main/java/kr/touroot/travelogue/repository/query/TravelogueQueryRepositoryImpl.java

+            return;
+        }
+
+        if (filter.period() == 8) {


매직 넘버 처리해 주시면 좋을 것 같군요!!

Libienz

클로버 수고하셨습니다 🍀

우선 제 생각에는 코드의 가독성을 조금 더 챙겨볼 수 있을 것 같아 RC 드립니다.
제 생각이 틀릴 수도 있어서 클로버와 의견 나눠보고 싶네요~

Libienz · 2024-09-24T01:17:06Z

backend/src/main/java/kr/touroot/travelogue/domain/Travelogue.java

@@ -49,19 +51,24 @@ public class Travelogue extends BaseEntity {
    @Column(nullable = false)
    private String thumbnail;

+    @Column(nullable = false)
+    @ColumnDefault("0")


DB 스키마에 디폴트 값을 설정하는 애너테이션이군요옵.

저희 빈 값으로 Insert가 나가지는 않을 것 같은데 필요할까요?
클로버의 의도가 궁금합니다리 🤔

Libienz · 2024-09-24T01:54:47Z

backend/src/main/java/kr/touroot/travelogue/domain/Travelogue.java

+    public void increaseLikeCount() {
+        likeCount += LIKE_COUNT_WEIGHT;
+    }
+
+    public void decreaseLikeCount() {
+        likeCount -= LIKE_COUNT_WEIGHT;
+    }


개인적으로는 +1이 좀 더 가독성 있다고 생각하는데 어떻게 생각하시나유?
지금 구현도 크게 위화감 있다고 생각하지 않아서요~ 반드시 변경하기보다 클로버가 마음가는 구현으로 남겨주세요!

1이 가독성 있긴한데 매직넘버라 생각해 분리해뒀습니다.하나로 관리하는게 증가, 삭제 일관성 지키기도 더 쉬울 것 같구요.
일단 유지하겠습니다!

Libienz · 2024-09-24T01:56:29Z

backend/src/main/java/kr/touroot/travelogue/repository/query/TravelogueQueryRepository.java

 import kr.touroot.travelogue.domain.Travelogue;
+import kr.touroot.travelogue.dto.request.TravelogueFilterCondition;
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.Pageable;

 public interface TravelogueQueryRepository {

    Page<Travelogue> findByTitleContaining(String keyword, Pageable pageable);

-    Page<Travelogue> findAllByTag(List<Long> tagFilter, Pageable pageable);
+    Page<Travelogue> findAllByFilter(TravelogueFilterCondition filter, Pageable pageable);
 }


리건에게 드렸던 코멘트랑 비슷한데요..!
dto의 침투 막아주는 것이 좋을 것 같습니다.
당장은 의미없는 매핑일 수 있어도 진행해놓으면 api 명세 수정에 잘 견딜 수 있다고 생각해요!

Libienz · 2024-09-24T02:02:41Z

backend/src/main/java/kr/touroot/travelogue/repository/query/TravelogueQueryRepositoryImpl.java

+    public Page<Travelogue> findAllByFilter(TravelogueFilterCondition filter, Pageable pageable) {
+        JPAQuery<Travelogue> query = jpaQueryFactory.select(travelogue)
+                .from(travelogueTag);
+
+        addTagFilter(query, filter);
+        addPeriodFilter(query, filter);
+
+        List<Travelogue> results = query.orderBy(findSortCondition(pageable.getSort()))


findAllByFilter 메서드 속에 findSortCondition이 있네요!
find로 시작하는 메서드는 레포지토리 조회 수준의 기능을 제공할 것 같은데 OrderSpecifier를 제공하고 있어요.
findSortCondition의 네이밍을 다음 중 하나, 혹은 클로버가 생각하는 다른 이름으로 바꿔보는 것은 어떨까요??

getSortConditionFrom(pageable)

getOrderSpecifier(pageable)

Libienz · 2024-09-24T02:08:47Z

backend/src/main/java/kr/touroot/travelogue/repository/query/TravelogueQueryRepositoryImpl.java

+        if (filter.period() == 8) {
+            query.where(travelogueTag.travelogue.travelogueDays.size().goe(8));
+            return;
+        }


8은 필터링 최대 기간이라는 의미를 가진 상수인 듯 해요..!
상수 추출을 부탁드리려고 했는데.. 조금 더 생각해보니 다른 생각도 드네요 🤔
서비스 속으로 명세가 숨어야 하는 부분이 아닐까요?

클로버의 의견도 공유해주세요~

Libienz · 2024-09-24T02:17:22Z

backend/src/main/java/kr/touroot/travelogue/repository/query/TravelogueQueryRepositoryImpl.java

+        query.where(travelogueTag.tag.id.in(filter.tag()))
+                .groupBy(travelogueTag.travelogue)
+                .having(isSameCountWithFilter(filter.tag()));


isSameCountWithFilter(filter.tag())
위 메서드는 필터 개수를 인자로 받는 것이 (Integer를 파라미터로 받는 것) 훨씬 자연스러운 것 같아요.
filterCount 등으로 메서드 명이 가독성 있게 바뀔 수 있고 메서드내에서 불필요한 접근 연산자 (.)도 제거할 수 있겠군요.

++ filter.tag()를 변수로 추출해서 사용하는 것이 좋아보여요. 여러번 사용되기도 하고 개수등을 뽑아서 넘길 때도 훨씬 편해질 수 있을 것 같습니다
사실

Libienz · 2024-09-24T02:17:58Z

backend/src/main/java/kr/touroot/travelogue/service/TravelogueService.java

-    public Page<Travelogue> findAllByFilter(List<Long> filter, Pageable pageable) {
-        return travelogueQueryRepository.findAllByTag(filter, pageable);
+    public Page<Travelogue> findAllByFilter(TravelogueFilterCondition filter, Pageable pageable) {
+        return travelogueQueryRepository.findAllByFilter(filter, pageable);


👍🏻 👍🏻 추상화 좋군요

eunjungL

뒤늦게 수정이 들어가서 outdated 된 애들은 일단 답변 달지 않았습니다. (거의 반영된 것 같네요.)
다시 한 번 확인해주시면 감사하겠습니다.

eunjungL · 2024-09-24T02:30:47Z

backend/src/main/java/kr/touroot/travelogue/domain/Travelogue.java

@@ -49,19 +51,24 @@ public class Travelogue extends BaseEntity {
    @Column(nullable = false)
    private String thumbnail;

+    @Column(nullable = false)
+    @ColumnDefault("0")


기존 데이터와 호환을 위해 default 값 설정해줬습니다.
근데 사실 기존 데이터가 0으로 전부 밀어져도 되나 싶긴 하네요.
삭제하는게 좋을지 의견 한 번 주시면 감사하겠습니다.

eunjungL · 2024-09-24T02:31:01Z

backend/src/main/java/kr/touroot/travelogue/domain/Travelogue.java

+    public void increaseLikeCount() {
+        likeCount += LIKE_COUNT_WEIGHT;
+    }
+
+    public void decreaseLikeCount() {
+        likeCount -= LIKE_COUNT_WEIGHT;
+    }


인지해두겠습니다 👍

eunjungL · 2024-09-24T02:31:52Z

backend/src/main/java/kr/touroot/travelogue/domain/Travelogue.java

+    public void increaseLikeCount() {
+        likeCount += LIKE_COUNT_WEIGHT;
+    }
+
+    public void decreaseLikeCount() {
+        likeCount -= LIKE_COUNT_WEIGHT;
+    }


1이 가독성 있긴한데 매직넘버라 생각해 분리해뒀습니다.하나로 관리하는게 증가, 삭제 일관성 지키기도 더 쉬울 것 같구요.
일단 유지하겠습니다!

Libienz

DefaultColumn만 수정해주세요~

고생하셨습니다 로버로버클로버

Libienz · 2024-09-24T02:36:57Z

backend/src/main/java/kr/touroot/travelogue/repository/query/TravelogueQueryRepositoryImpl.java

+        if (filter.isEmptyTagCondition()) {
            return;
        }



오호라 고순가?

Libienz · 2024-09-24T02:38:01Z

backend/src/main/java/kr/touroot/travelogue/service/TravelogueFacadeService.java

+    private Page<Travelogue> findSimpleTraveloguesByFilter(TravelogueFilterCondition filter, Pageable pageable) {
+        if (filter.isEmptyCondition()) {
+            return travelogueService.findAll(pageable);
+        }
+
+        return travelogueService.findAllByFilter(filter, pageable);


너무 클린 & 깔끔

nak-honest

저도 리뷰 반영된 것 잘 확인했습니다!!
approve 드립니닷

Libienz · 2024-09-24T07:42:32Z

flyway script 까먹지 마시죠!

hangillee

LGTM GGWP 👍

hangillee

LGTM GGWP 👍

eunjungL added 7 commits September 22, 2024 15:08

feat: Travelogue Entity에 likeCount 필드 추가

aa0cfa1

refactor: Travelogue Entity의 likeCount long 타입으로 변경

0921781

feat: Travelogue 좋아요 생성, 삭제 시 likeCount 변경 추가

baa3a37

refactor: 좋아요 수 조회 시 travelogue 필드 활용하게 변경

7540226

refactor: 필터링 조건 추가에 따른 TravelogueFilterCondition 추가

f4cd65a

test: 여행기 조회 시 좋아요 순 정렬 테스트 추가

2c6a6f1

feat: 여행기 기간 기반으로 필터링 기능 추가

188a27d

eunjungL added the BE label Sep 23, 2024

eunjungL added this to the sprint 5 milestone Sep 23, 2024

eunjungL requested review from hangillee, Libienz and nak-honest September 23, 2024 13:35

eunjungL self-assigned this Sep 23, 2024

hangillee requested changes Sep 23, 2024

View reviewed changes

nak-honest requested changes Sep 24, 2024

View reviewed changes

eunjungL added 2 commits September 24, 2024 10:31

refactor: 정렬 방향 클라이언트의 입력 값으로 결정하게 변경

73768c2

refactor: 검색 최대 기간 매직 넘버 상수화

c3e79ac

eunjungL marked this pull request as draft September 24, 2024 01:52

eunjungL added 2 commits September 24, 2024 10:53

refactor: 필터에 따른 여행기 조회 분기 service로 이동 및 dto, domain 분리

8e23ff7

refactor: FilterCondition 관련 판단 로직 FilterCondition 내부로 응집

f812e0e

Libienz requested changes Sep 24, 2024

View reviewed changes

refactor: 태그 기반 필터링 쿼리 travelogue 기반으로 변경

44428f1

eunjungL marked this pull request as ready for review September 24, 2024 02:25

eunjungL commented Sep 24, 2024

View reviewed changes

Libienz approved these changes Sep 24, 2024

View reviewed changes

refactor: likeCount Column default 제거

ed96678

nak-honest approved these changes Sep 24, 2024

View reviewed changes

hangillee approved these changes Sep 25, 2024

View reviewed changes

eunjungL added 2 commits September 25, 2024 10:24

fix: develop/be 충돌 해결

9959a83

feat: Travelogue likeCount 컬럼 추가 flyway V3 작성

8936f13

hangillee approved these changes Sep 25, 2024

View reviewed changes

eunjungL added 2 commits September 25, 2024 21:02

Merge branch 'develop/be' into feature/be/#410

16d469d

Merge branch 'develop/be' into feature/be/#410

531a7b7

eunjungL merged commit 819fb3c into develop/be Sep 25, 2024
3 checks passed

eunjungL deleted the feature/be/#410 branch September 25, 2024 13:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] - 여행기 정렬 및 날짜별 필터링 추가 #442

[Feature] - 여행기 정렬 및 날짜별 필터링 추가 #442

eunjungL commented Sep 23, 2024

github-actions bot commented Sep 23, 2024 •

edited

Loading

hangillee left a comment

hangillee Sep 23, 2024

nak-honest Sep 24, 2024

hangillee Sep 23, 2024

Libienz Sep 24, 2024

eunjungL Sep 24, 2024

nak-honest left a comment

nak-honest Sep 24, 2024

nak-honest Sep 24, 2024

eunjungL Sep 24, 2024

nak-honest Sep 24, 2024

Libienz left a comment

Libienz Sep 24, 2024

Libienz Sep 24, 2024

eunjungL Sep 24, 2024

Libienz Sep 24, 2024

Libienz Sep 24, 2024

Libienz Sep 24, 2024

Libienz Sep 24, 2024

Libienz Sep 24, 2024

eunjungL left a comment

eunjungL Sep 24, 2024

eunjungL Sep 24, 2024

eunjungL Sep 24, 2024

Libienz left a comment

Libienz Sep 24, 2024

Libienz Sep 24, 2024

nak-honest left a comment

Libienz commented Sep 24, 2024

hangillee left a comment

hangillee left a comment

[Feature] - 여행기 정렬 및 날짜별 필터링 추가 #442

[Feature] - 여행기 정렬 및 날짜별 필터링 추가 #442

Conversation

eunjungL commented Sep 23, 2024

✅ 작업 내용

참고사항

github-actions bot commented Sep 23, 2024 • edited Loading

Test Results

hangillee 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

nak-honest 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

Libienz 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

eunjungL 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

Libienz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nak-honest left a comment

Choose a reason for hiding this comment

Libienz commented Sep 24, 2024

hangillee left a comment

Choose a reason for hiding this comment

hangillee left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 23, 2024 •

edited

Loading