[25.04.22 / TASK-100] Feature - Leaderboard 도메인 추가 및 조회 API 구현 #27
Conversation
any로 두었을 때 lint 검증 및 실행은 잘 되었으나, 테스트 코드 작성시 에러 나서 내부에 정의
1 similar comment
| """ Walkthrough이번 변경 사항은 리더보드(Leaderboard) 기능의 API 엔드포인트 및 관련 서비스, 저장소, 타입, 테스트 코드 도입에 관한 것입니다. 새로운 라우터와 컨트롤러, 서비스, 저장소 계층이 추가되어 쿼리 파라미터에 따라 유저 또는 포스트 기준의 리더보드 데이터를 조회하고, 이를 DTO 형태로 반환합니다. 데이터베이스 쿼리와 결과 매핑, 입력 파라미터 유효성 검증, Swagger 문서화, 그리고 각 계층별 단위 테스트가 포함되어 있습니다. Changes
Sequence Diagram(s)sequenceDiagram participant Client participant Router participant Controller participant Service participant Repository participant DB Client->>Router: GET /leaderboard/user or /leaderboard/post Router->>Controller: getUserLeaderboard / getPostLeaderboard(req, res, next) Controller->>Service: getUserLeaderboard / getPostLeaderboard(sort, dateRange, limit) Service->>Repository: getUserLeaderboard / getPostLeaderboard(sort, dateRange, limit) Repository->>DB: SQL Query 실행 (파라미터 포함) DB-->>Repository: Raw 데이터 반환 Repository-->>Service: Raw 데이터 전달 Service-->>Controller: DTO 변환된 리더보드 데이터 반환 Controller-->>Client: 200 OK + JSON 응답 Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File ( |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/routes/index.ts (1)
5-16: 🛠️ Refactor suggestion라우터 등록 순서 결정 시 충돌 가능성 점검
현재 모든 서브 라우터를
'/'에 mount 하고 있습니다. 경로가 겹칠 경우(예:/leaderboard와/post/:id) Express 매칭 순서에 따라 예기치 않은 라우터가 먼저 처리될 수 있습니다.
- 기능 확장이 예상된다면
router.use('/leaderboard', LeaderboardRouter);처럼 prefix 를 두어 명확하게 구분하는 방식을 권장합니다.
🧹 Nitpick comments (7)
src/repositories/leaderboard.repository.ts (2)
121-135:postCount분기 로직 및 switch 중복 개선 제안
postCount정렬 요청이post리더보드에서도 가능하지만 현재는 무조건view_diff로 매핑됩니다.
- 의도라면 주석으로 명시하고, 아니라면
post타입에선 예외를 던지거나 별도 매핑을 고려해 주세요.
case 'viewCount'와default가 동일하여 정적 분석 도구가 “불필요한 case” 로 경고합니다. 가독성을 높이려면 하나로 통합하는 편이 좋습니다.- case 'viewCount': - default: - sortCol = 'view_diff'; - break; + default: // 'viewCount' 포함 + sortCol = 'view_diff'; + break;
52-68:SELECT절 주석 한글 ↔ SQL 컬럼명 혼합 사용 가독성한글 주석은 매우 도움이 되지만, 긴 블록 주석이 이어질 경우 SQL 본문 가독성이 떨어질 수 있습니다.
/* ... */보다는--단일 라인 주석을 유지하되, 꼭 필요한 위치(계산 컬럼 상단)로 이동하면 검색·복사 시 편의가 향상됩니다.
큰 문제는 아니므로 참고만 해 주세요.src/routes/leaderboard.router.ts (1)
11-14: 싱글톤 DI 컨테이너 도입 고려라우터 파일에서 직접
new Repository → new Service → new Controller체인을 생성하면 테스트 시 mocking 이 어렵고, 앱 전역에서 중복 인스턴스가 생성될 위험이 있습니다.
앱 규모가 커질 예정이라면 다음과 같은 개선을 권장합니다.
- InversifyJS, Typedi 등 DI 컨테이너 도입
- 또는
app.ts수준에서 한 번만 인스턴스를 생성 후 의존성 주입이렇게 하면 재사용성이 높아지고 테스트 코드에서 의존 객체를 쉽게 대체할 수 있습니다.
src/controllers/leaderboard.controller.ts (1)
1-27: 컨트롤러 구현이 잘 되어 있습니다. 몇 가지 개선사항을 제안합니다.리더보드 컨트롤러 코드는 전반적으로 잘 작성되어 있습니다. 의존성 주입과 비동기 처리, 에러 핸들링이 적절히 구현되어 있습니다. 다음과 같은 개선사항을 고려해보세요:
쿼리 파라미터의 타입 지정을 더 엄격하게 처리하는 것이 좋을 것 같습니다. 현재는 서비스 메서드로 쿼리 파라미터를 그대로 전달하고 있지만, 타입 변환과 유효성 검사를 명시적으로 수행하면 좋겠습니다.
에러 메시지에 좀 더 구체적인 정보를 포함하면 디버깅에 도움이 될 것입니다.
아래와 같이 쿼리 파라미터 처리 부분을 개선할 수 있습니다:
getLeaderboard: RequestHandler = async ( req: Request<object, object, object, GetLeaderboardQuery>, res: Response<LeaderboardResponseDto>, next: NextFunction, ) => { try { const { type, sort, dateRange, limit } = req.query; + // 타입 변환 및 기본값 적용 + const parsedDateRange = dateRange ? Number(dateRange) : undefined; + const parsedLimit = limit ? Number(limit) : undefined; + + if (dateRange && isNaN(parsedDateRange)) { + throw new Error('유효하지 않은 dateRange 값입니다.'); + } + + if (limit && isNaN(parsedLimit)) { + throw new Error('유효하지 않은 limit 값입니다.'); + } - const result = await this.leaderboardService.getLeaderboard(type, sort, dateRange, limit); + const result = await this.leaderboardService.getLeaderboard(type, sort, parsedDateRange, parsedLimit); const response = new LeaderboardResponseDto(true, '리더보드 조회에 성공하였습니다.', result, null); res.status(200).json(response); } catch (error) { - logger.error('리더보드 조회 실패:', error); + logger.error(`리더보드 조회 실패: ${error instanceof Error ? error.message : '알 수 없는 오류'}`, error); next(error); } };src/services/__test__/leaderboard.service.test.ts (1)
1-167: 서비스 테스트가 잘 작성되어 있습니다. 몇 가지 추가 테스트를 고려해보세요.리더보드 서비스 테스트는 핵심 기능을 잘 검증하고 있습니다. 데이터 변환, 기본값 처리, 예외 처리 등이 잘 테스트되어 있습니다.
다음과 같은 추가 테스트 케이스를 고려해보시면 좋겠습니다:
- 잘못된 형식의 파라미터가 전달될 때의 동작 검증
- 각 파라미터별 기본값 테스트(현재는 모든 파라미터를 생략한 경우만 테스트)
- 유효하지 않은 type 값이 전달될 때의 동작 검증
예를 들어, 다음과 같은 테스트를 추가할 수 있습니다:
it('유효하지 않은 type 값이 전달되면 기본값(user)을 사용해야 한다', async () => { const mockRawResult = [ { id: 1, email: 'test@test.com', total_views: 100, total_likes: 50, total_posts: 1, view_diff: 20, like_diff: 10, post_diff: 1, }, ]; repo.getLeaderboard.mockResolvedValue(mockRawResult); const result = await service.getLeaderboard('invalid_type'); expect(repo.getLeaderboard).toHaveBeenCalledWith('invalid_type', 'viewCount', 30, 10); expect(result.users).not.toBeNull(); expect(result.posts).toBeNull(); }); it('각 파라미터별 기본값이 올바르게 적용되어야 한다', async () => { repo.getLeaderboard.mockResolvedValue([]); await service.getLeaderboard(undefined, 'likeCount'); expect(repo.getLeaderboard).toHaveBeenCalledWith('user', 'likeCount', 30, 10); await service.getLeaderboard('post', undefined, 15); expect(repo.getLeaderboard).toHaveBeenCalledWith('post', 'viewCount', 15, 10); await service.getLeaderboard('post', 'likeCount', undefined, 20); expect(repo.getLeaderboard).toHaveBeenCalledWith('post', 'likeCount', 30, 20); });src/services/leaderboard.service.ts (1)
64-84: 인터페이스 위치 재고 필요
RawPostResult와RawUserResult인터페이스는 파일 하단보다는 클래스 정의 전에 위치하는 것이 코드 가독성 측면에서 더 좋을 것 같습니다. 또는 별도의 타입 파일로 분리하는 것도 고려해 볼 수 있습니다.import logger from '@/configs/logger.config'; import { LeaderboardRepository } from '@/repositories/leaderboard.repository'; import { LeaderboardPostType, LeaderboardResponseData, LeaderboardUserType, } from '@/types/dto/responses/leaderboardResponse.type'; + interface RawPostResult { + id: number; + title: string; + slug: string; + total_views: number; + total_likes: number; + view_diff: number; + like_diff: number; + released_at: string; + } + + interface RawUserResult { + id: number; + email: string; + total_views: number; + total_likes: number; + total_posts: number; + view_diff: number; + like_diff: number; + post_diff: number; + } export class LeaderboardService { // ... }src/types/dto/requests/getLeaderboardQuery.type.ts (1)
69-98: DTO 클래스 구현에 강화된 유효성 검증 추가 고려현재
@IsString()데코레이터는 문자열 타입만 검증하고 실제 허용된 값('user', 'post' 등)을 검증하지 않습니다. 열거형 값 검증을 위한@IsEnum()데코레이터 사용을 고려해 보세요. 또한dateRange와limit의 최소/최대 값은 상수로 추출하면 유지보수성이 향상됩니다.import { Transform } from 'class-transformer'; - import { IsNumber, IsOptional, IsString, Max, Min } from 'class-validator'; + import { IsEnum, IsNumber, IsOptional, IsString, Max, Min } from 'class-validator'; import { LeaderboardType, LeaderboardSortType } from './path-to-enums'; + // 상수 정의 + const MIN_RANGE = 1; + const MAX_RANGE = 30; // 타입 정의 부분은 유지... export class GetLeaderboardQueryDto { @IsOptional() - @IsString() + @IsEnum(LeaderboardType, { message: '타입은 "user" 또는 "post"여야 합니다' }) type?: LeaderboardType; @IsOptional() - @IsString() + @IsEnum(LeaderboardSortType, { message: '정렬은 "viewCount", "likeCount", "postCount" 중 하나여야 합니다' }) sort?: LeaderboardSortType; @IsOptional() @IsNumber() @Transform(({ value }) => Number(value)) - @Min(1) - @Max(30) + @Min(MIN_RANGE) + @Max(MAX_RANGE) dateRange?: number; @IsOptional() @IsNumber() @Transform(({ value }) => Number(value)) - @Min(1) - @Max(30) + @Min(MIN_RANGE) + @Max(MAX_RANGE) limit?: number; // 생성자 유지... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/controllers/leaderboard.controller.ts(1 hunks)src/repositories/__test__/leaderboard.repo.test.ts(1 hunks)src/repositories/leaderboard.repository.ts(1 hunks)src/routes/index.ts(2 hunks)src/routes/leaderboard.router.ts(1 hunks)src/services/__test__/leaderboard.service.test.ts(1 hunks)src/services/leaderboard.service.ts(1 hunks)src/types/dto/requests/getLeaderboardQuery.type.ts(1 hunks)src/types/dto/responses/leaderboardResponse.type.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/controllers/leaderboard.controller.ts (3)
src/services/leaderboard.service.ts (1)
LeaderboardService(9-62)src/types/dto/requests/getLeaderboardQuery.type.ts (1)
GetLeaderboardQuery(30-35)src/types/dto/responses/leaderboardResponse.type.ts (1)
LeaderboardResponseDto(125-125)
src/routes/leaderboard.router.ts (5)
src/repositories/leaderboard.repository.ts (1)
LeaderboardRepository(5-155)src/services/leaderboard.service.ts (1)
LeaderboardService(9-62)src/controllers/leaderboard.controller.ts (1)
LeaderboardController(7-27)src/middlewares/validation.middleware.ts (1)
validateRequestDto(8-38)src/types/dto/requests/getLeaderboardQuery.type.ts (1)
GetLeaderboardQueryDto(69-98)
src/services/__test__/leaderboard.service.test.ts (2)
src/services/leaderboard.service.ts (1)
LeaderboardService(9-62)src/repositories/leaderboard.repository.ts (1)
LeaderboardRepository(5-155)
src/repositories/__test__/leaderboard.repo.test.ts (1)
src/repositories/leaderboard.repository.ts (1)
LeaderboardRepository(5-155)
src/types/dto/responses/leaderboardResponse.type.ts (1)
src/types/dto/responses/baseResponse.type.ts (1)
BaseResponseDto(28-40)
🪛 Biome (1.9.4)
src/repositories/leaderboard.repository.ts
[error] 150-150: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🔇 Additional comments (9)
src/repositories/leaderboard.repository.ts (1)
16-18:pool.query파라미터 위치 확인 필요본 쿼리는
$1(dateRange) 과$2(limit) 두 개의 파라미터를 사용하고 있습니다.
pool.query(query, [dateRange, limit])호출 시dateRange와limit의 자료형은 반드시number여야 하며,undefined가 들어갈 경우 런타임 오류가 발생합니다. DTO‧Service 단계에서 기본값을 지정하고 있지만, 리포지토리 진입 시점에 한 번 더 검증(early return) 하면 디버깅이 수월해집니다.src/routes/leaderboard.router.ts (1)
49-50: 검증 미들웨어 위치는 OK — 쿼리 기본값 처리도 확인
validateRequestDto로 쿼리를 객체화한 뒤 기본값은 Service 레이어에서 설정되고 있습니다.
만약 Swagger 에서limit,dateRange파라미터를 생략하고 호출했을 때도 정상 동작하는지 한번 더 확인을 권장드립니다.src/repositories/__test__/leaderboard.repo.test.ts (1)
1-271: 리더보드 리포지토리 테스트가 잘 작성되어 있습니다.리더보드 리포지토리 테스트는 다양한 케이스를 포괄적으로 검증하고 있습니다. 각 테스트 케이스는 명확한 목적을 가지고 있으며, 모킹과 검증 방식이 적절합니다.
특히 다음 부분들이 잘 구현되었습니다:
- SQL 쿼리 구성 검증
- 다양한 정렬 방식 테스트
- 타입 파라미터에 따른 동작 변화 검증
- 에러 발생 시 올바른 예외 던지기 확인
테스트 커버리지가 높고 세분화되어 있어 리포지토리의 동작을 매우 잘 검증하고 있습니다.
src/types/dto/responses/leaderboardResponse.type.ts (1)
1-126: DTO 타입 정의가 잘 되어 있습니다.리더보드 응답 DTO 타입 정의는 명확하게 구조화되어 있으며, Swagger 문서화도 잘 되어 있습니다. 인터페이스와 클래스 정의가 잘 구분되어 있고, 필요한 속성들이 모두 포함되어 있습니다.
다음과 같은 점들이 특히 잘 구현되었습니다:
- 각 필드에 대한 상세한 Swagger 문서화
- 명확한 타입 정의
- 재사용 가능한 컴포넌트 구조
- 베이스 응답 타입을 확장한 구조
BaseResponseDto를 상속받아 일관된 응답 형식을 유지하는 것이 좋은 접근법입니다.
src/services/leaderboard.service.ts (2)
1-8: 코드 구조와 임포트 구성이 명확합니다.import 구문과 필요한 타입들을 잘 정의해두었습니다. 전체적인 구조가 깔끔합니다.
9-10: 의존성 주입 패턴이 적절하게 사용되었습니다.생성자를 통한 의존성 주입 방식을 잘 적용했습니다. 이는 테스트 코드 작성 시에도 유리한 구조입니다.
src/types/dto/requests/getLeaderboardQuery.type.ts (3)
4-28: 타입 정의와 Swagger 문서화가 명확합니다.리더보드 타입과 정렬 방식에 대한 타입 정의와 Swagger 문서화가 잘 되어 있습니다. 각 타입에 대한 설명과 기본값이 명확하게 기술되어 있습니다.
30-35: 인터페이스 정의가 간결하고 명확합니다.인터페이스 정의가 간결하며 앞서 정의한 타입들을 잘 활용하고 있습니다. 모든 필드가 적절하게 선택적(optional)으로 표시되어 있습니다.
37-68: Swagger 문서화가 상세하고 완성도가 높습니다.DTO 클래스에 대한 Swagger 문서화가 상세하게 작성되어 있으며, 앞서 정의한 스키마 컴포넌트를 참조하고 있습니다. 최소값, 최대값 등의 제약조건도 잘 문서화되어 있습니다.
유저 리더보드 조회시 GROUP BY절에 user.email 추가 타입 안정성 강화: DTO에 유효성 검증 추가 및 관련 코드 수정에 따른 테스트 케이스 삭제
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/repositories/leaderboard.repository.ts (2)
88-97:dateRange계산 시 타입 미스매치 가능성
CURRENT_DATE - $1::int표현은 PostgreSQL 에서date - integer → date가 지원되므로 동작은 합니다.
다만buildLeaderboardCteQuery()내부의start_stats는$1::int * INTERVAL '1 day'로INTERVAL을,
여기서는 정수 연산을 사용해 두 곳의 계산 방식이 불일치합니다.동일한 방식을 사용해 혼동을 줄이는 편이 좋습니다.
- WHEN p.released_at >= CURRENT_DATE - $1::int + WHEN p.released_at >= (CURRENT_DATE - $1::int)혹은 두 곳 모두
INTERVAL로 통일하면 명시성이 높아집니다.... >= (CURRENT_DATE - ($1::int * INTERVAL '1 day'))
30-38: 타임존 변환 로직의 성능 저하 우려
AT TIME ZONE 'Asia/Seoul' AT TIME ZONE 'UTC'패턴이 매 행마다 실행되면 인덱스를 무시해 큰 테이블에서 심각한 성능 저하가 발생할 수 있습니다.가능하다면
posts_postdailystatistics의date컬럼은 UTC 로 저장한 뒤,- 쿼리 레벨에서
WHERE date::date <= CURRENT_DATE처럼 변환을 제거하거나,- 파티셔닝 / 인덱스 생성 시 동일한 표현식 인덱스를 구축
하는 방법을 고려해 주세요.
프로덕션 DB 크기가 크다는 PR 설명과 맞물려 병목 가능성이 높습니다.Also applies to: 41-48
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/controllers/leaderboard.controller.ts(1 hunks)src/repositories/__test__/leaderboard.repo.test.ts(1 hunks)src/repositories/leaderboard.repository.ts(1 hunks)src/services/__test__/leaderboard.service.test.ts(1 hunks)src/services/leaderboard.service.ts(1 hunks)src/types/dto/requests/getLeaderboardQuery.type.ts(1 hunks)src/types/dto/responses/leaderboardResponse.type.ts(1 hunks)src/types/index.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- src/types/dto/responses/leaderboardResponse.type.ts
- src/repositories/test/leaderboard.repo.test.ts
- src/services/test/leaderboard.service.test.ts
- src/controllers/leaderboard.controller.ts
- src/services/leaderboard.service.ts
- src/types/dto/requests/getLeaderboardQuery.type.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/repositories/leaderboard.repository.ts (1)
src/types/dto/requests/getLeaderboardQuery.type.ts (2)
LeaderboardType(15-15)LeaderboardSortType(28-28)
🪛 Biome (1.9.4)
src/repositories/leaderboard.repository.ts
[error] 150-151: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
🔇 Additional comments (2)
src/repositories/leaderboard.repository.ts (2)
14-18: 동적 문자열 조합 시 SQL 인젝션 가능성은 낮으나, 타입 세이프티 보장을 위해format()사용 고려현재
sortCol을 문자열 치환 방식으로 ORDER BY 절에 직접 삽입하고 있습니다.
sortCol이 내부 매핑 함수에서만 파생되므로 외부 입력에 의한 실제 인젝션 위험은 거의 없지만,
pg-format과 같은 유틸을 사용하면 “컬럼 명” 에 대한 escape 가 명시적으로 이뤄져 가독성과 안전성을 동시에 확보할 수 있습니다.-import { Pool } from 'pg'; +import { Pool } from 'pg'; +import format from 'pg-format'; ... -const query = `${cteQuery} ${selectQuery} ${fromClause} ${groupOrderClause}`; +const query = format('%s %s %s %s', cteQuery, selectQuery, fromClause, groupOrderClause);장점
- 향후 정렬 컬럼이 늘더라도 실수로 사용자 입력이 섞일 위험을 사전에 차단
- 리뷰어가 SQL 문자열 경계를 더 쉽게 식별 가능
150-151: 기존 리뷰에서 지적된GROUP BY u.email누락 문제 해결 확인
GROUP BY u.id, u.email로 수정되어 PostgreSQL 집계 오류가 더 이상 발생하지 않습니다. 👍🧰 Tools
🪛 Biome (1.9.4)
[error] 150-151: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
There was a problem hiding this comment.
하나의 API 에서 2가지 유형의 응답이 오는 구조라, 이게 사실 RESTFul 하지가 않긴 합니다. 이 부분은 저도 처음에 생각했을때 간과했네요. 당연히 사용자별 조회와 게시글 별 조회 응답이 다를텐데, 깊이 생각하지 못했어요.
사실 저라면 서비스 계층에서 부터 2가지 로직으로 getLeaderboardByUser 와 getLeaderboardByPost 로 분리했을 것 같아요.
라우팅할때 하나로 합칠지, 두 개로 가져갈지 고민하겠지만, 저는 2개로 했을 것 같아요.
이유가 있다면, 하나로 합쳤을때 API 에 "멱등성" 을 지키는 응답을 주지 않고 있어요. 쿼리스트링에 response 형태 자체가 바뀌는건 FE 로서도 많이 부담스러울 것 같아요.
@six-standard 님 의견도 궁금하네요
There was a problem hiding this comment.
좋았던 점
- 구조가, 특히 타입들이 생각보다 널뛰죠? DTO 와 인터페이스의 조화.. 과한 부분들이 있는데 다은님이 아주 F/U 을 잘해주신 것 같아요. 너무 깔끔하고 swagger 에 대한 타입 명시도 너무 좋았어요. 고생 너무 많으셨어요 증맬..
- 테스트 코드 역시 잊지 않고, 계층별 테스트 코드까지!! 고생 많으셨어요 굿굿굿
- 앞으로 조금씩, 하나씩 걷어 낼 수 있다면 바로 잡아봐요!
아쉬운 점
- 코멘트 부분 한 번 체크 부탁드릴게요 :)
- 계층별 테스트 코드가 그렇게 상이하게 안느껴 지는 것 같아요! 아무래도 어쩔 수 없는 부분이 분명하게 있는데, 레포 계층의 테스트 코드 방향을 한 번 체크 해보는 것도 좋을 것 같네요!
- 노션에도 만들어지는 전체 raw SQL 에 대한 내용이 없더라구요! type 이 post 이고 default dataRange 와 default limit 일때, 실제 만들어지는 raw SQL 전체 코드가 있으면 아주 아주 저희가 F/U 하는데 도움이 많이 될 것 같아요!
| @Nuung 제 생각에도 +) 노션 업데이트도 해두겠습니다! 🥹 |
라우터, 타입, 함수 모두 분리 repo 쿼리 주석 삭제 및 파라미터에 따른 날짜 변환을 make_interval로 통일 불필요해진 테스트 코드 삭제
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/repositories/leaderboard.repository.ts (1)
75-76: SQL 주석은 영어로 작성하는 것을 고려해보세요SQL 쿼리 내 한글 주석은 인코딩 이슈를 발생시킬 수 있습니다. 또한 이전 PR 코멘트에서 Nuung님이 언급했듯이, 한글 주석은 통신 과정에서 과도한 바이트를 사용할 수 있습니다. 주석을 영어로 변경하거나 코드 외부로 분리하는 것을 고려해보세요.
🧹 Nitpick comments (6)
src/services/__test__/leaderboard.service.test.ts (1)
128-128: 테스트 데이터의 날짜 값 확인 필요테스트에 사용된
released_at값이 미래 날짜(2025-01-01)로 설정되어 있습니다. 테스트 목적으로는 문제가 없지만, 실제 데이터와 유사하게 과거 날짜를 사용하는 것이 더 명확할 수 있습니다.src/repositories/leaderboard.repository.ts (1)
22-24: 통계 계산 로직에 대한 설명 필요중첩된 COALESCE 표현식이 복잡하게 사용되었습니다. 이런 복잡한 계산 로직에는 주석을 통해 의도를 명확히 설명하면 코드 유지보수성이 향상될 것입니다.
src/types/dto/responses/leaderboardResponse.type.ts (2)
59-61: 배열 타입에 대한 nullable 처리 검토가 필요합니다.Swagger 문서에는
users배열이 nullable로 정의되어 있지만, 실제 인터페이스에서는 단순히LeaderboardUser[]로 정의되어 있습니다. 두 정의 간의 일관성을 유지하기 위해 실제 타입도 nullable로 처리하는 것이 좋을 수 있습니다.export interface UserLeaderboardData { - users: LeaderboardUser[]; + users: LeaderboardUser[] | null; }
134-136: UserLeaderboardData와 동일한 nullable 이슈가 있습니다.Swagger 문서에서는
posts배열을 nullable로 정의했지만, 인터페이스에서는 그렇지 않습니다. 일관성을 위해 수정을 고려해보세요.export interface PostLeaderboardData { - posts: LeaderboardPost[]; + posts: LeaderboardPost[] | null; }src/services/leaderboard.service.ts (1)
56-69: 두 매핑 메서드의 중복 로직을 리팩토링하면 좋을 것 같습니다.
mapRawUserResult와mapRawPostResult메서드가 유사한 패턴을 가지고 있습니다. 제네릭 타입을 활용한 공통 매핑 유틸리티 함수로 리팩토링하는 것이 코드 중복을 줄이는 데 도움이 될 수 있습니다.+ private mapSnakeToCamel<T, R>(items: T[], mapper: (item: T) => R): R[] { + return items.map(mapper); + } private mapRawUserResult(rawResult: RawUserResult[]): UserLeaderboardData { - const users = rawResult.map((user) => ({ + const users = this.mapSnakeToCamel(rawResult, (user) => ({ id: user.id, email: user.email, totalViews: user.total_views, totalLikes: user.total_likes, totalPosts: user.total_posts, viewDiff: user.view_diff, likeDiff: user.like_diff, postDiff: user.post_diff, - })); + })); return { users }; }src/types/dto/requests/getLeaderboardQuery.type.ts (1)
30-41: 인터페이스와 DTO 클래스의 중복 정의가 있습니다.
GetLeaderboardQuery인터페이스와GetLeaderboardQueryDto클래스 간에 중복 정의가 있습니다. 필요한 경우가 아니라면 하나로 통합하여 코드를 간소화하는 것이 좋습니다.DTO 클래스만 유지하고 인터페이스는 필요한 경우가 아니라면 제거하는 것을 고려해보세요. 또는 인터페이스를 유지한다면 DTO 클래스가 해당 인터페이스를 구현하도록 명시적으로 표현하는 것이 좋습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/controllers/leaderboard.controller.ts(1 hunks)src/repositories/__test__/leaderboard.repo.test.ts(1 hunks)src/repositories/leaderboard.repository.ts(1 hunks)src/routes/leaderboard.router.ts(1 hunks)src/services/__test__/leaderboard.service.test.ts(1 hunks)src/services/leaderboard.service.ts(1 hunks)src/types/dto/requests/getLeaderboardQuery.type.ts(1 hunks)src/types/dto/responses/leaderboardResponse.type.ts(1 hunks)src/types/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/types/index.ts
- src/repositories/test/leaderboard.repo.test.ts
🧰 Additional context used
🧠 Learnings (1)
src/services/leaderboard.service.ts (2)
Learnt from: ooheunda PR: Check-Data-Out/velog-dashboard-v2-api#27 File: src/services/leaderboard.service.ts:29-61 Timestamp: 2025-04-22T12:46:28.081Z Learning: LeaderboardService의 mapRawResultToLeaderboardResponseData 메서드에서 `RawPostResult[] | RawUserResult[]` 타입을 사용하는 것이 `unknown[]`보다 타입 안전성 측면에서 더 좋은 접근 방식으로 판단됩니다. Learnt from: ooheunda PR: Check-Data-Out/velog-dashboard-v2-api#27 File: src/services/leaderboard.service.ts:12-27 Timestamp: 2025-04-22T12:15:10.947Z Learning: 리퀘스트 검증은 DTO 레벨의 데코레이터(@IsEnum, @Transform 등)를 사용하여 컨트롤러 단계에서 수행하는 것이 바람직하며, 서비스 레이어의 파라미터에는 TypeScript 타입 좁히기를 적용하여 타입 안전성을 확보하는 것이 좋다. 🧬 Code Graph Analysis (6)
src/routes/leaderboard.router.ts (5)
src/repositories/leaderboard.repository.ts (1)
LeaderboardRepository(6-99)src/services/leaderboard.service.ts (1)
LeaderboardService(10-70)src/controllers/leaderboard.controller.ts (1)
LeaderboardController(11-49)src/middlewares/validation.middleware.ts (1)
validateRequestDto(8-38)src/types/dto/requests/getLeaderboardQuery.type.ts (2)
GetUserLeaderboardQueryDto(100-110)GetPostLeaderboardQueryDto(126-136)
src/controllers/leaderboard.controller.ts (3)
src/services/leaderboard.service.ts (1)
LeaderboardService(10-70)src/types/dto/requests/getLeaderboardQuery.type.ts (2)
GetUserLeaderboardQuery(35-37)GetPostLeaderboardQuery(39-41)src/types/dto/responses/leaderboardResponse.type.ts (2)
UserLeaderboardResponseDto(75-75)PostLeaderboardResponseDto(150-150)
src/services/__test__/leaderboard.service.test.ts (2)
src/services/leaderboard.service.ts (1)
LeaderboardService(10-70)src/repositories/leaderboard.repository.ts (1)
LeaderboardRepository(6-99)
src/types/dto/responses/leaderboardResponse.type.ts (2)
src/types/index.ts (4)
UserLeaderboardData(30-30)UserLeaderboardResponseDto(28-28)PostLeaderboardData(31-31)PostLeaderboardResponseDto(29-29)src/types/dto/responses/baseResponse.type.ts (1)
BaseResponseDto(28-40)
src/repositories/leaderboard.repository.ts (1)
src/types/dto/requests/getLeaderboardQuery.type.ts (2)
UserLeaderboardSortType(15-15)PostLeaderboardSortType(28-28)
src/services/leaderboard.service.ts (4)
src/repositories/leaderboard.repository.ts (1)
LeaderboardRepository(6-99)src/types/dto/requests/getLeaderboardQuery.type.ts (2)
UserLeaderboardSortType(15-15)PostLeaderboardSortType(28-28)src/types/index.ts (4)
UserLeaderboardSortType(16-16)UserLeaderboardData(30-30)PostLeaderboardSortType(17-17)PostLeaderboardData(31-31)src/types/dto/responses/leaderboardResponse.type.ts (2)
UserLeaderboardData(59-61)PostLeaderboardData(134-136)
🔇 Additional comments (23)
src/routes/leaderboard.router.ts (3)
45-49: 새로운 라우트 구조 잘 설계되었습니다!사용자 리더보드 엔드포인트가 RESTful 원칙에 맞게
/leaderboard/user로 잘 구현되어 있습니다. 검증 미들웨어를 통한 쿼리 파라미터 유효성 검사도 적절하게 적용되었습니다.
81-85: 게시물 리더보드 엔드포인트 구현 적절합니다
/leaderboard/post엔드포인트와 각각의 검증 로직 분리가 좋습니다. PR 코멘트에서 언급된 대로 타입 기반 엔드포인트 분리도 잘 적용되었습니다.
15-44: Swagger 문서화 완성도가 높습니다사용자 리더보드 API에 대한 Swagger 문서화가 매우 상세하게 작성되었습니다. 파라미터와 응답 스키마가 명확히 참조되어 있어 API 사용자에게 도움이 될 것입니다.
src/controllers/leaderboard.controller.ts (2)
11-30: 컨트롤러 아키텍처 및 에러 처리 적절합니다컨트롤러가 의존성 주입 패턴을 잘 따르고 있으며, 에러 처리 로직이 적절하게 구현되어 있습니다. 쿼리 파라미터 추출과 서비스 호출 로직도 명확합니다.
32-48: 게시물 리더보드 컨트롤러 메소드 구현 적절합니다게시물 리더보드 조회 핸들러도 사용자 리더보드와 일관된 패턴을 유지하고 있어 코드 일관성이 좋습니다. 타입 안전성과 에러 처리도 잘 구현되어 있습니다.
src/services/__test__/leaderboard.service.test.ts (2)
27-81: 사용자 리더보드 테스트 케이스가 잘 구성되어 있습니다데이터 변환, 쿼리 파라미터 적용, 기본값 처리, 빈 결과 처리, 에러 전파 등 다양한 시나리오를 포괄적으로 테스트하고 있습니다. 테스트 케이스 명세가 명확합니다.
117-204: 게시물 리더보드 테스트도 동일하게 철저합니다게시물 리더보드 관련 테스트 케이스도 다양한 시나리오를 체계적으로 검증하고 있습니다. 모의 객체를 통한 테스트 격리가 잘 이루어졌습니다.
src/repositories/leaderboard.repository.ts (3)
11-12: 정렬 컬럼 설정 로직이 효율적입니다정렬 조건에 따른 SQL 컬럼 매핑이 간결하게 처리되어 있습니다. 삼항 연산자를 사용하여 명확하게 표현되었습니다.
85-86: 데이터 필터링 날짜 로직 검토현재 날짜(
NOW())를 기준으로 이전 데이터를 필터링하는 로직이 타임존 변환과 함께 복잡하게 구성되어 있습니다. 이 로직이 의도한 대로 동작하는지 검증이 필요합니다. 특히 시간대 변환(AT TIME ZONE)이 여러 번 사용되는 경우 예상치 못한 결과가 발생할 수 있습니다.
29-31: GROUP BY 절이 완전합니다이전 리뷰 코멘트에서 지적된
GROUP BY절에u.email이 누락된 문제가 해결되었습니다. 현재는u.id와u.email모두GROUP BY절에 포함되어 있어 PostgreSQL에서 정상 작동할 것입니다.src/types/dto/responses/leaderboardResponse.type.ts (4)
3-76: Swagger 문서화가 상세하고 완벽합니다.Swagger 스키마 정의가 매우 상세하게 작성되어 있어 API 문서화에 큰 도움이 될 것입니다. 특히 각 필드에 대한 설명과 타입 정의가 명확합니다.
35-44: 인터페이스 명명 규칙이 일관적이고 직관적입니다.
LeaderboardUser인터페이스의 속성들이 의미를 명확하게 전달하며, 일관된 네이밍 컨벤션을 따르고 있습니다.totalViews,viewDiff등의 이름이 데이터의 의미를 잘 표현합니다.
75-75: BaseResponseDto 사용 방식이 적절합니다.제네릭을 활용한
BaseResponseDto상속 패턴이 잘 적용되어 있어 코드의 재사용성을 높이고 일관된 응답 구조를 유지하는 데 도움이 됩니다.
110-119: LeaderboardPost 인터페이스 구조가 명확합니다.포스트 리더보드에 필요한 모든 필드가 잘 정의되어 있으며, 특히
releasedAt의 타입을 string으로 정의한 것이 JSON 직렬화를 고려했을 때 적절합니다.src/services/leaderboard.service.ts (5)
13-25: getUserLeaderboard 메서드의 구조가 깔끔합니다.파라미터 기본값 설정, 비동기 처리, 에러 핸들링이 모두 적절히 구현되어 있습니다. 특히 기본값 설정으로 API 사용성을 높인 부분이 좋습니다.
22-24: 에러 로깅 후 재발생 패턴이 적절합니다.에러를 로깅한 후 그대로 다시 throw하는 패턴은 에러 추적성을 높이면서도 상위 계층에서 적절한 처리를 할 수 있게 해줍니다.
41-54: private 메서드 분리를 통한 관심사 분리가 잘 되어 있습니다.
mapRawUserResult메서드를 통해 데이터 매핑 로직을 분리한 것은 코드 가독성과 유지보수성을 높이는 좋은 방법입니다. 특히 snake_case에서 camelCase로의 변환이 명확합니다.
72-81: RawPostResult 인터페이스 정의가 명확합니다.데이터베이스 결과를 나타내는 인터페이스가 snake_case 형식으로 일관되게 정의되어 있어 데이터베이스 스키마와의 매핑이 명확합니다.
83-92: RawUserResult 인터페이스도 적절하게 정의되어 있습니다.사용자 리더보드에 필요한 모든 필드가 포함되어 있고, 실제 데이터베이스 결과 형식과 일치하게 snake_case로 정의되어 있습니다.
src/types/dto/requests/getLeaderboardQuery.type.ts (4)
15-16: 유저 리더보드 정렬 타입 정의가 명확합니다.
UserLeaderboardSortType에 필요한 모든 정렬 옵션이 포함되어 있으며, 문자열 리터럴 유니온 타입으로 정의하여 타입 안전성을 확보했습니다.
65-84: GetLeaderboardQueryDto 클래스의 검증 데코레이터 사용이 적절합니다.class-validator와 class-transformer를 활용한 검증 및 변환 로직이 적절하게 구현되어 있습니다. 특히 빈 문자열을 기본값으로 변환하는 로직이 유용합니다.
100-110: GetUserLeaderboardQueryDto 클래스 상속 구조가 잘 설계되었습니다.
GetLeaderboardQueryDto를 상속받아 공통 필드와 검증 로직을 재사용하면서, 사용자 리더보드에 특화된sort필드를 추가한 구조가 효율적입니다.
126-136: GetPostLeaderboardQueryDto 클래스도 일관된 패턴으로 잘 구현되었습니다.사용자 리더보드 DTO와 동일한 패턴을 따르면서, 포스트 리더보드에 필요한 필드와 검증 로직을 적절히 구현했습니다.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
src/routes/leaderboard.router.ts (3)
11-14: 의존성 인스턴스를 라우터 내부에서 직접 생성하면 테스트·확장이 어려워집니다
라우터가LeaderboardRepository,LeaderboardService,LeaderboardController를 직접 new 해서 의존성을 고정하고 있습니다.
DI 컨테이너(예:tsyringe,inversify) 혹은 외부 팩토리 함수로 주입하면
- 단위 테스트에서 목(mock) 교체가 훨씬 쉬워지고
- 다중 DB 풀 / 설정별 인스턴스 분기가 편해집니다.
-const leaderboardRepository = new LeaderboardRepository(pool); -const leaderboardService = new LeaderboardService(leaderboardRepository); -const leaderboardController = new LeaderboardController(leaderboardService); +const leaderboardRepository = container.resolve(LeaderboardRepository); +const leaderboardService = container.resolve(LeaderboardService); +const leaderboardController= container.resolve(LeaderboardController);()
45-50: 라우터 prefix 중복 제거로 가독성 향상 가능
현재 각 엔드포인트가'/leaderboard/user','/leaderboard/post'처럼 전체 경로를 포함합니다.
router.use('/leaderboard', childRouter)구조로 분리하면 URL 중복을 한 곳에서 관리할 수 있어 유지보수가 수월해집니다.
()
45-50: Swagger 파라미터 스키마가 기본값·예시를 누락하고 있습니다
dateRange,limit의 기본값(30, 10)과 허용 범위(예: 최소 1, 최대 100)를@example,minimum,maximum으로 명시하면 API 소비자가 의도를 더 명확히 알 수 있습니다.
()Also applies to: 81-85
src/controllers/leaderboard.controller.ts (2)
14-30: 중복된 로직을 헬퍼로 추출하여 DRY 원칙 준수
두 핸들러 모두sort/dateRange/limit추출->서비스 호출->DTO 생성 패턴이 같아 중복이 발생합니다.
제네릭 헬퍼로 분리하면 테스트 코드와 메트릭 계측 지점을 한 곳에서 관리할 수 있습니다.
()
26-29: 에러 로깅 시 traceId 등 컨텍스트 정보를 포함하면 디버깅이 쉬워집니다
현재 단순 메시지+error만 기록하고 있습니다.
req.id(morgan-uuid 등)나 파라미터 값을 함께 남기면 운영 환경에서 문제 원인을 빠르게 추적할 수 있습니다.
()Also applies to: 44-47
src/types/dto/responses/leaderboardResponse.type.ts (1)
35-44: 인터페이스 export 누락으로 재사용이 제한됩니다
LeaderboardUser,LeaderboardPost를 외부 모듈에서 사용할 가능성이 높습니다(예: 클라이언트 타입 재사용).
export interface로 변경해 두는 편이 안전합니다.
()Also applies to: 110-119
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/controllers/leaderboard.controller.ts(1 hunks)src/repositories/__test__/leaderboard.repo.test.ts(1 hunks)src/repositories/leaderboard.repository.ts(1 hunks)src/routes/leaderboard.router.ts(1 hunks)src/services/__test__/leaderboard.service.test.ts(1 hunks)src/services/leaderboard.service.ts(1 hunks)src/types/dto/requests/getLeaderboardQuery.type.ts(1 hunks)src/types/dto/responses/leaderboardResponse.type.ts(1 hunks)src/types/index.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/services/test/leaderboard.service.test.ts
- src/repositories/test/leaderboard.repo.test.ts
- src/repositories/leaderboard.repository.ts
- src/types/index.ts
🧰 Additional context used
🧠 Learnings (1)
src/services/leaderboard.service.ts (2)
Learnt from: ooheunda PR: Check-Data-Out/velog-dashboard-v2-api#27 File: src/services/leaderboard.service.ts:29-61 Timestamp: 2025-04-22T12:46:28.081Z Learning: LeaderboardService의 mapRawResultToLeaderboardResponseData 메서드에서 `RawPostResult[] | RawUserResult[]` 타입을 사용하는 것이 `unknown[]`보다 타입 안전성 측면에서 더 좋은 접근 방식으로 판단됩니다. Learnt from: ooheunda PR: Check-Data-Out/velog-dashboard-v2-api#27 File: src/services/leaderboard.service.ts:12-27 Timestamp: 2025-04-22T12:15:10.947Z Learning: 리퀘스트 검증은 DTO 레벨의 데코레이터(@IsEnum, @Transform 등)를 사용하여 컨트롤러 단계에서 수행하는 것이 바람직하며, 서비스 레이어의 파라미터에는 TypeScript 타입 좁히기를 적용하여 타입 안전성을 확보하는 것이 좋다. 🧬 Code Graph Analysis (3)
src/routes/leaderboard.router.ts (5)
src/repositories/leaderboard.repository.ts (1)
LeaderboardRepository(6-99)src/services/leaderboard.service.ts (1)
LeaderboardService(10-70)src/controllers/leaderboard.controller.ts (1)
LeaderboardController(11-49)src/middlewares/validation.middleware.ts (1)
validateRequestDto(8-38)src/types/dto/requests/getLeaderboardQuery.type.ts (2)
GetUserLeaderboardQueryDto(100-110)GetPostLeaderboardQueryDto(126-136)
src/types/dto/responses/leaderboardResponse.type.ts (2)
src/types/index.ts (4)
UserLeaderboardData(30-30)UserLeaderboardResponseDto(28-28)PostLeaderboardData(31-31)PostLeaderboardResponseDto(29-29)src/types/dto/responses/baseResponse.type.ts (1)
BaseResponseDto(28-40)
src/services/leaderboard.service.ts (4)
src/repositories/leaderboard.repository.ts (1)
LeaderboardRepository(6-99)src/types/dto/requests/getLeaderboardQuery.type.ts (2)
UserLeaderboardSortType(15-15)PostLeaderboardSortType(28-28)src/types/index.ts (4)
UserLeaderboardSortType(16-16)UserLeaderboardData(30-30)PostLeaderboardSortType(17-17)PostLeaderboardData(31-31)src/types/dto/responses/leaderboardResponse.type.ts (2)
UserLeaderboardData(59-61)PostLeaderboardData(134-136)
🔇 Additional comments (13)
src/types/dto/responses/leaderboardResponse.type.ts (1)
50-58: Swaggernullable: true지정이 실제 타입 정의와 불일치
users,posts는 빈 배열이더라도 배열 타입입니다.nullable을 false로 맞추는 것이 스키마-타입 일관성을 높입니다.
()Also applies to: 126-134
src/services/leaderboard.service.ts (6)
1-11: 적절한 의존성 주입 패턴을 사용했습니다.리더보드 서비스 클래스가 생성자를 통해 레포지토리를 주입받는 방식은 의존성 주입의 좋은 예시입니다. 이렇게 하면 테스트 시 mock 객체를 쉽게 주입할 수 있고 코드의 결합도를 낮출 수 있습니다.
13-25: getUserLeaderboard 메서드의 기본값 설정과 오류 처리가 잘 구현되었습니다.기본 매개변수 값을 적절히 설정하고 있으며, try-catch 블록으로 오류를 포착하고 로깅한 후 다시 던지는 패턴이 좋습니다. 리더닝에 따라
UserLeaderboardSortType과 같은 명확한 타입을 사용한 것도 훌륭합니다.
27-39: getPostLeaderboard 메서드도 일관된 패턴으로 잘 구현되었습니다.getUserLeaderboard와 동일한 패턴을 따르고 있어 코드의 일관성이 유지되고 있습니다. 기본값 설정, 오류 처리, 로깅 모두 적절합니다.
41-54: mapRawUserResult 메서드의 타입 안전성을 향상시킨 점이 좋습니다.rawResult 매개변수에
RawUserResult[]타입을 명시함으로써 타입 안전성을 확보했습니다. 이는 이전에 논의된unknown[]대신 구체적인 타입을 사용하는 접근 방식과 일치합니다.
56-69: mapRawPostResult 메서드도 일관된 매핑 패턴을 유지합니다.Post 데이터에 대한 매핑도 User 데이터 매핑과 일관된 패턴을 유지하며, 스네이크 케이스에서 카멜 케이스로의 변환이 명확히 이루어져 있습니다.
72-92: RawResult 인터페이스의 명확한 정의가 좋습니다.RawPostResult와 RawUserResult 인터페이스가 명확하게 정의되어 있어 데이터베이스 쿼리 결과와 매핑 작업 사이의 타입 안전성을 보장합니다. 스네이크 케이스로 정의된 프로퍼티 이름은 데이터베이스 컬럼명과 일치하도록 한 것으로 보입니다.
src/types/dto/requests/getLeaderboardQuery.type.ts (6)
1-3: 필요한 데코레이터를 적절히 임포트했습니다.class-validator와 class-transformer 라이브러리에서 필요한 데코레이터를 명확하게 임포트했습니다. 이는 요청 유효성 검증에 필요한 기능들을 활용하기 위한 좋은 시작점입니다.
4-28: Swagger 문서화가 상세하게 작성되었습니다.UserLeaderboardSortType과 PostLeaderboardSortType에 대한 Swagger 문서화가 매우 상세하게 작성되었습니다. 타입 정보, 설명, null 허용 여부, 가능한 열거값, 기본값 등을 명확하게 정의했습니다.
30-41: 인터페이스 계층 구조가 잘 설계되었습니다.기본 GetLeaderboardQuery 인터페이스를 정의하고 이를 확장하는 구체적인 인터페이스(GetUserLeaderboardQuery, GetPostLeaderboardQuery)를 만든 접근 방식이 좋습니다. 이는 코드 중복을 줄이고 일관성을 유지하게 해줍니다.
43-84: GetLeaderboardQueryDto의 유효성 검증이 견고합니다.@IsOptional, @IsNumber, @min, @max 데코레이터를 사용하여 dateRange와 limit 필드에 대한 유효성 검증을 철저히 구현했습니다. 또한 @Transform 데코레이터를 사용하여 빈 문자열을 기본값으로 처리하는 로직이 유용합니다.
86-110: GetUserLeaderboardQueryDto가 적절하게 기본 DTO를 확장합니다.GetLeaderboardQueryDto를 확장함으로써 코드 중복을 피하고, sort 필드에 대한 추가적인 유효성 검증을 제공합니다. @IsEnum 데코레이터를 사용하여 가능한 값을 제한하는 것이 좋습니다.
112-136: GetPostLeaderboardQueryDto도 일관된 패턴을 유지합니다.GetUserLeaderboardQueryDto와 동일한 패턴으로 구현되어 코드의 일관성을 유지하고 있습니다. Post 관련 정렬 옵션에 맞게 @IsEnum 값이 적절히 조정되었습니다.
There was a problem hiding this comment.
좋았던 점
- build 패턴이 들어가고, 이 빌드 배턴이 "CTE(with 구문)" 을 위해서 만들어진 것에 감명받았어요. 저는 이전 PR 에서도 해당 부분이 가장 좋았습니다!
- 테스트 케이스도 꼼꼼하셔서 다른 엣지 케이스도 떠오르지 않네요! 좋아요, 너무 고생많으셨어요.
아쉬운 점
PostLeaderboardSortType으로 enum 형태로 레표 계증에서 sort 를 결정하는데, sortCol 로 더블 체크하는게 꼭 필요할까요?! 또는 아래 처럼 object pattern 으로 빼보는건?! (진짜 이건 사견)
const SORT_COLUMN_MAPPING = { viewCount: 'view_diff', likeCount: 'like_diff', postCount: 'post_diff' } as const;- 테스트 데이터 생성때 복붙 말고 저희 같이 사용하는
createMockQueryResult와 같은 헬퍼 함수를 사용해도 괜찮지 않을까 해요!
🔥 변경 사항
참고사항
startDateViews를 제공하는 대신 서버에서 미리 상승값을 계산해 (viewGrowth처럼) 보내는 것으로 수정하였습니다. (스웨거 또는 노션 참고)값 <= 오늘날짜)과 기준날짜에서 가장 최근값(값 >= 기준날짜)를 사용하도록 했습니다.🏷 관련 이슈
없습니다!
📸 스크린샷 (UI 변경 시 필수)
UI 변경 사항 없습니다!
📌 체크리스트
기능 테스트는 약간 세모입니다...
단위 테스트 커버리지는 100이고, GPT로 더미 데이터를 만들어 넣어서 스웨거로 테스트 해보았고 로컬에선 잘 작동했는데요.
운영 DB는 데이터가 방대해서 어떻게 될 지 확신하기가 두렵네요. 현우님의 이 PR이 머지되고 로컬DB로 데이터 마이그레이션 후 테스트 통과한다면 맘 놓을 수 있을 것 같아요. 다른 테스트 방법이 있다면 알려주시면 감사하겠습니다...😸
Summary by CodeRabbit
Summary by CodeRabbit
신규 기능
/leaderboard/user및/leaderboard/post엔드포인트를 추가하여 사용자 및 게시글 리더보드를 조회할 수 있습니다.문서화
테스트