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] #286 - 소셜 쿼리 오류 수정 및 닉네임 정규표현식 수정 #287

Merged
merged 2 commits into from
Jun 4, 2024

Conversation

0lynny
Copy link
Member

@0lynny 0lynny commented Jun 4, 2024

🚀PullRequest🚀

📟 관련 이슈

💻 작업 내용

  1. 운영서버에서 유저가 초성으로만 닉네임을 입력하여 서버에러가 발생하였습니다.
    수정 전 정규식은 일반 한글만 가능하도록 되어있었는데 자음 초성, 모음 초성도 가능하도록 정규식을 수정하였습니다. 해당 부분 클라 측에도 전달 예정입니다!

  2. 소셜 API에서 다음과 같은 쿼리 에러가 발생하여서 해당 부분 수정하였습니다.
    Expression #3 of ORDER BY clause is not in SELECT list, references column 'moonshot.krl1_0.idx' which is not in SELECT list; this is incompatible with DISTINCT

해당 에러는 DISTINCT를 사용할 때, ORDER BY절에 나열된 모든 컬럼들이 SELECT 리스트에 포함되어야 하는데 없어서 생긴 문제였습니다. 쿼리문에서 distinct를 제거하고 추후에 distinct를 해주는 방식으로 수정하였습니다. 순서를 보장해주기 위해서 LinkedHashSet을 사용하였습니다 !

📝 리뷰 노트

@0lynny 0lynny requested a review from its-sky June 4, 2024 10:18
@0lynny 0lynny self-assigned this Jun 4, 2024
Copy link
Member

@its-sky its-sky left a comment

Choose a reason for hiding this comment

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

전체적으로 깔끔하게 고치셨네요 수고하셨습니다~

.leftJoin(objective.keyResultList, keyResult).fetchJoin()
.leftJoin(keyResult.taskList, task)
.where(objective.isPublic.eq(true))
.orderBy(objective.heartCount.desc(), objective.id.desc(), keyResult.idx.asc(), task.id.asc())
.orderBy(objective.heartCount.desc(), objective.id.desc(), keyResult.idx.asc(), task.idx.asc())
.limit(10)
.fetch();
}
Copy link
Member

Choose a reason for hiding this comment

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

Repository SRP 지키면서 로직을 작성하신거 너무 잘하셨습니다~

@@ -72,12 +78,12 @@ private OrderSpecifier<?> order(Criteria criteria) {

@Override
public List<Objective> findSocialObjectives() {
return queryFactory.selectFrom(objective).distinct()
.join(objective.user).fetchJoin()
return queryFactory.selectFrom(objective)
Copy link
Member

Choose a reason for hiding this comment

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

Projection으로 DTO로 바로 뽑아내는게 더 효율적이었으면 좋았는데 아쉽네요
그래도 Projection은 성능적으로 fetch join이 안되는 것도 알았으니 하나 배워가시면 좋을거 같습니다~

return objectives.stream()
.filter(objective -> objectiveIds.add(objective.getId()))
.map(SocialOKRResponseDto::of)
.toList();
}
Copy link
Member

Choose a reason for hiding this comment

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

Application 레벨에서 전처리 잘 해주셨네요~
LinkedHashSet을 통해 순서를 보장하며 중복을 제거한 선택 아주 잘하셨습니당~

@0lynny 0lynny merged commit 11e1fd6 into develop Jun 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fix] 소셜 API 버그 수정 및 닉네임 정규식 변경
2 participants