-
Notifications
You must be signed in to change notification settings - Fork 1
[Feat] #660 앱잼팀 랭킹 API 구현 #664
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
Conversation
|
아직 작업 중이신 것 같아서 코드리뷰는 따로 남기지 않았습니다~ |
|
@jher235 나머지 다 구현되면 리뷰어 추가해서 노티 드릴게요 ~~ |
jher235
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!!
리뷰에도 적어뒀는데 top 3, top 10 같은 정보들은 클라가 컨트롤하도록 해도 좋을 것 같아요~
| private static final int TODAY_TEAM_RANK_LIMIT = 10; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
개인적인 생각으로 top3, top10 과 같은 값들을 상수로 관리하는 것보다 클라에서 유연하게 사용할 수 있도록 queryString으로 받는게 좋다고 생각해요. 어떻게 생각하세요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 해당 의견은 동의하긴합니다.
유동적으로 리퀘스트로 입력받으면, 기획쪽에서 변동 사항이 생겼을때도 유동적으로 대처할 수 있을거같네요 👍🏻
| public AppjamRankInfo.RankAggregate findRecentTeamRanks() { | ||
| Pageable latestStampPageable = PageRequest.of(0, RECENT_RANK_LIMIT); | ||
|
|
||
| List<Stamp> latestStamps = stampRepository.findLatestStamps(latestStampPageable); | ||
| if (latestStamps.isEmpty()) { | ||
| return AppjamRankInfo.RankAggregate.empty(); | ||
| } | ||
|
|
||
| List<Long> uploaderUserIds = latestStamps.stream() | ||
| .map(Stamp::getUserId) | ||
| .distinct() | ||
| .toList(); | ||
|
|
||
| List<AppjamUser> uploaderAppjamUsers = appjamUserRepository.findAllByUserIdIn(uploaderUserIds); | ||
|
|
||
| Map<Long, AppjamUser> uploaderAppjamUserByUserId = uploaderAppjamUsers.stream() | ||
| .collect(Collectors.toMap( | ||
| AppjamUser::getUserId, | ||
| Function.identity(), | ||
| (existing, replacement) -> existing | ||
| )); | ||
|
|
||
| return AppjamRankInfo.RankAggregate.of( | ||
| latestStamps, | ||
| uploaderUserIds, | ||
| uploaderAppjamUserByUserId | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹시 해당 메서드의 목적이 뭘까요??
메서드명은 랭킹을 가져오는 것 같았는데 로직을 읽다보니 스탬프 정보를 반환하는 것 같아서 어떤 목적인지 살짝 헷갈린다는 느낌을 받았어요. 반환 DTO 명과 메서드명을 이렇게 두신 이유가 궁금해요.
| @Override | ||
| public List<AppjamTodayRankSource> findTodayUserRankSources(LocalDateTime todayStart, LocalDateTime tomorrowStart) { | ||
| final String sql = String.format(""" | ||
| SELECT | ||
| s.user_id AS user_id, | ||
| (COUNT(*) * :todayPointPerStamp) AS today_points, | ||
| MIN(s.created_at) AS first_certified_at_today | ||
| FROM %s.stamp s | ||
| WHERE s.created_at >= :todayStart | ||
| AND s.created_at < :tomorrowStart | ||
| GROUP BY s.user_id | ||
| ORDER BY today_points DESC, first_certified_at_today ASC | ||
| """, schema); | ||
|
|
||
| Query query = em.createNativeQuery(sql); | ||
| query.setParameter("todayStart", todayStart); | ||
| query.setParameter("tomorrowStart", tomorrowStart); | ||
| query.setParameter("todayPointPerStamp", TODAY_POINT_PER_STAMP); | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| List<Object[]> rows = query.getResultList(); | ||
|
|
||
| return rows.stream() | ||
| .map(row -> new AppjamTodayRankSource( | ||
| ((Number) row[0]).longValue(), | ||
| ((Number) row[1]).longValue(), | ||
| toLocalDateTime(row[2]) | ||
| )) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그리고 시작 시간, 종료 시간 2개를 주입 받으면 굳이 today에 억매일 필요가 없지 않을까요? 그냥 between 등의 네이밍으로 받는게 더 재활용성이 높을 것 같다는 생각이 들었어요.
그리고 혹시 이 부분에선 stamp 마다 점수(==level?)가 다른 부분은 따로 계산을 하진 않나요??
Related issue 🛠
Work Description ✏️
앱잼 랭킹 가장 최근에 미션 인증한 TOP 3 순위 API 구현
팀별
max(stamp.createdAt)기준으로 정렬해서 최신에 스탬프를 업로드한 TOP3 팀을 반환stamp.images가 여러개 있을때는 첫 번째 이미지를imageUrl로 내려주도록 구현appjamUser.getTeamName()으로 사전에 전달받은 앱팀 별명을 함께 반환앱잼 팀 랭킹 TOP 10 순위 조회 API 구현
DB(스탬프) + Redis(ZSET 누적 점수) 를 합쳐서 랭킹 계산
오늘 점수(todayPoints) = stamp를 “오늘(00:00~24:00)” 범위로 필터링해서 유저별 인증 개수 × 1000점
정렬 기준이 총점이 아니라 “오늘 추가 점수” 이고, 동점일 때는 가장 먼저 인증한 팀(최소
createdAt) 이 우선되도록 타이브레이커를 넣었습니다To Reviewers 📢
집계함수나 정렬같은거 처리해주면서
Native Query도 사용하게 됐었고,제 예상보다 복잡한 내용이 좀 있었어서, 해당 부분들 꼼꼼히 봐주시면 감사하겠습니다 🙇🏻♂️
기존
app_service라고 되어있던 패키지명도 통일해서, import문 정도만 변경된거라고 생각해주시면 됩니당코드 보실때
appservice는 닫은 채로 봐주시면 더 편하실거같아요추가로 현재
createAt을 그냥 내려주고 있는데, 솝탬프 인증 시간이 1분 전이 아니고 10분 미만 일 경우에도" 방금 전 " 이라 표시되는게 의도된 설계인지, 기획분에게 크로스 체크 중이여서 해당 내용 답변 받으면 반영하도록 할게요