Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: 페이징 오류 처리 & 리뷰 공개, 비공개에 따른 crud와 모아보기, 랜덤 피드 수정 #296

Merged
merged 5 commits into from
Jul 25, 2024

Conversation

yujiyea
Copy link
Collaborator

@yujiyea yujiyea commented Jul 22, 2024

무엇을 위한 PR인가요?(: 뒤 설명추가)

  • 신규 기능 추가 :
  • 버그 수정 : 리뷰 모아보기 페이징 오류 처리
  • 리펙토링 : 리뷰 공개, 비공개에 따른 create, update API 수정, 랜덤 피드, 피드 검색엔진 수정, 리뷰 모아보기 - 인스타뷰 수정
  • 문서 업데이트 :
  • 기타 :

변경사항 및 이유

리뷰 모아보기 - 인스타뷰, 타임라인 뷰의 페이징 처리 오류로 인해 리뷰가 존재하나 존재하지 않음 예외가 발생

작업 내역

  • 리뷰 모아보기 페이징 오류 처리
    • sort가 잘못되어 있었음, reviewId를 고려한다는게 createAt에 의존해버림
  • 리뷰 공개, 비공개 설정할 수 있도록 create, update API 수정
  • 랜덤 피드 API: 공개된 리뷰만 나올 수 있도록 수정
  • 피드 검색 API: 페이징 처리 & 공개된 리뷰만 검색 결과에 나오도록 수정
  • 리뷰 모아보기 - 인스타뷰 API: 내 리뷰를 인스타뷰 하는 것과 타인의 프로필 인스타뷰를 하는 경우 리뷰 공개 여부를 체크하는게 달라지도록 수정

Issue Number

close: #250

어떤 부분에 리뷰어가 집중하면 좋을까요?

deleteReview 함수와 getFeedDetail 함수에 적어둔 TODO에 대해 의견 바랍니다.

zzo3ozz and others added 4 commits July 17, 2024 11:15
hotfix : 내 카테고리는 private 값이여도 확인 가능하게 수정
- 피드 검색 API: 리뷰 공개된 것만 검색
- 랜덤 피드 API: 사용자 공개와 리뷰 공개까지 고려해서 노출
- 리뷰 생성, 수정 API: 공개, 비공개
- 인스타 뷰 API: 내 리뷰 보기용일때는 각 리뷰의 공개 체크가 필요없음, 타인 리뷰 보기 용일 때는 타인의 프로필 공개 여부와 각 리뷰 공개 체크 해야 함
- 페이징 처리: 이전에 sort방식이 잘못됨, reviewId로 정렬을 한번 해야 하는데 createAt만 생각했음
@yujiyea yujiyea added 🐛 Bug Something isn't working 🛠️ Refactor Code Refectoring labels Jul 22, 2024
@yujiyea yujiyea self-assigned this Jul 22, 2024
Copy link
Member

@zzo3ozz zzo3ozz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다~! 코멘트로 의견 달아두었으니 확인 부탁드립니다!

@@ -153,6 +142,7 @@ public void deleteReview(User user, Long reviewId) {
public ReviewDetailResponse getReview(Long reviewId) {
Review review = reviewRepository.findByReviewIdAndStatus(reviewId, BaseEntity.Status.ACTIVE).orElseThrow(()->new NotFoundException(Code.REVIEW_NOT_FOUND));
//TODO: 후에 각 리뷰마다의 공개, 비공개를 확인해서 주는거로 수정하기
//TODO: 해당 함수는 사용자의 공개를 체크하면 안 될 것 같은데?? 리뷰 하나를 조회하는 것으로 분명 앞에서 리뷰가 보였으니까 해당 함수를 사용할 것임
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

다수의 리뷰 조회 시점과 리뷰 1건 조회 시점이 차이가 나고, 그 사이에 비공개 처리가 되어있다면 해당 API 사용 시 비공개 시점인 리뷰가 조회될 수 있을 것 같습니다~!
개인적으로는 한번 더 공개 여부 체크를 하는 것이 좋다고 생각하지만, 개발 의도에 따라 유연하게 적용할 수 있는 문제인 것 같습니다.

우선은 현재 상태로 배포하고, 차후 논의해보면 좋을 듯 해요!

@@ -31,26 +32,40 @@ public List<RandomFeedResponse> getRandomFeed(User user) {
}

@Override
public SearchFeedResponse searchFeed(String keyword, List<Long> hashTags) {
List<Review> searchResult = new ArrayList<>();
public SearchFeedResponse searchFeed(String keyword, List<Long> hashTags, Long cursor) { //TODO: 검색 결과가 랜덤으로 다시 정렬되야 할듯 & 자기 리뷰가 아니여야함
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개인적으로는 랜덤 정렬까지는 굳이 필요 없을 것 같고, 자신의 리뷰는 제외하는 것이 좋을 듯 합니다.

@zzo3ozz zzo3ozz merged commit 042f92c into gusto-umc:develop Jul 25, 2024
@zzo3ozz zzo3ozz mentioned this pull request Jul 25, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working 🛠️ Refactor Code Refectoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants