[임동민] Good-Night-3rd-Hackathon-Backend 제출 #5
Conversation
dongmin115
changed the title
There was a problem hiding this comment.
고생 많으셨습니다. 프로젝트 구조는 크게 군더더기 없어보여요.
하지만 프로젝트가 소규모이기 때문에 관리할 부분이 많이 없기도 한데요.
여기서 고려할 점은 재사용성을 고려해 유틸성 함수를 추가하거나, 프로젝트 규모가 커져서 새로운 기능이 많아질 때 어떻게 대응할까 인데, 이건 오답은 있지만 뚜렷한 정답은 없어서 좀 더 프로젝트를 해보시면서 상황에 맞는 방법을 찾아보시면 좋을 것 같아요.
혹시 제가 남긴 코멘트에 대해 이해가 안되는 것이 있거나 질문이 있으시면 편하게 코멘트 남겨주시거나 DM 남겨주세요.
| const mockCommentRepository = { | ||
| find: jest.fn(), | ||
| findOne: jest.fn(), | ||
| save: jest.fn(), | ||
| createQueryBuilder: jest.fn().mockReturnThis(), | ||
| where: jest.fn().mockReturnThis(), | ||
| andWhere: jest.fn().mockReturnThis(), | ||
| orderBy: jest.fn().mockReturnThis(), | ||
| skip: jest.fn().mockReturnThis(), | ||
| take: jest.fn().mockReturnThis(), | ||
| getMany: jest.fn(), | ||
| update: jest.fn(), | ||
| getOne: jest.fn(), | ||
| create: jest.fn(), | ||
| }; |
There was a problem hiding this comment.
DB는 보통 mock DB를 쓰기보단 실 DB(우리의 경우엔 mysql)을 쓰곤 합니다.
docker-compose.yml에 테스트용 DB도 넣어주시고 테스트할 때 테스트 DB쪽 환경변수를 읽도록 한 뒤, 매 테스트마다 사용한 DB 데이터의 흔적이 안남도록 잘 처리해주시면 됩니다.
"Nest 테스트 DB" 정도로 구글 검색하시면 예제가 많이 나올거에요.
| enum: ['승인됨', '보류됨', '거절됨'], | ||
| default: '보류됨', | ||
| }) | ||
| is_confirm: string; |
There was a problem hiding this comment.
const CONFIRMATION_CODES = ['승인됨', '보류됨', '거절됨'] as const;
type ConfirmationCode = (typeof CONFIRMATION_CODES)[number];
...
@Column({
type: 'enum',
enum: CONFIRMATION_CODES,
default: '보류됨',
})
is_confirm: ConfirmationCode;이건 어떤가요? String 리터럴을 사용한 방법인데요. 런타임에선 string과 똑같지만, 우리가 개발하는 단계에서는 타입 안정성을 보장해주는 방식입니다.
승인 코드를 재활용할 수 있어 좋고 좀 더 type-safe 해질 것 같아요.
src/wishes/wishes.service.ts
Outdated
| async findAll( | ||
| isConfirmed?: string, | ||
| keyword?: string, | ||
| category?: string, | ||
| page: number = 1, | ||
| limit: number = 10, | ||
| ): Promise<Wish[]> { | ||
| const query = this.wishRepository.createQueryBuilder('wish'); | ||
|
|
||
| // 쿼리파라미터 값에 따라 필터링 | ||
| if (isConfirmed === 'true') { | ||
| query.andWhere('wish.is_confirm = :isConfirmed', { | ||
| isConfirmed: '승인됨', | ||
| }); | ||
| } else if (isConfirmed === 'false') { | ||
| query.andWhere('wish.is_confirm = :isConfirmed', { | ||
| isConfirmed: '거절됨', | ||
| }); | ||
| } else if (isConfirmed === 'pending') { | ||
| query.andWhere('wish.is_confirm = :isConfirmed', { | ||
| isConfirmed: '보류됨', | ||
| }); | ||
| } else if (isConfirmed) { | ||
| throw new BadRequestException('올바르지 않은 값입니다.'); | ||
| } |
There was a problem hiding this comment.
문제가 없다면 isConfirmed의 값을 검증&매핑하는 곳을 Controller로 옮기면 어떨까요?
그리고 findAll에서 isConfirmed: '승인됨' | '거절됨' | '보류됨'으로 타입을 변경하고요.
(여기서 아까 정의한 ConfirmationCode 를 써볼 수 있겠군요)
findAll에서는 무조건 보장된 값만 가지고 놀 수 있으니 type-safe하기도 하고, 관심사도 분리되어 좋을 것 같아요.
그러고 나면 남는 건 isConfirmed를 그대로 집어 넣는 아래 코드겠네요:
query.andWhere('wish.is_confirm = :isConfirmed', { isConfirmed });
| query | ||
| .where('comment.wishId = :wishId', { wishId }) | ||
| .andWhere('comment.deleted_at IS NULL') | ||
| .orderBy('comment.createdAt', 'DESC'); | ||
|
|
||
| // 페이지네이션 | ||
| const offset = (page - 1) * limit; | ||
| query.skip(offset).take(limit); | ||
|
|
||
| return query.getMany(); |
There was a problem hiding this comment.
소소한 nitpicking인데 이렇게 줄여볼 수 있겠네요
return this.commentRepository.createQueryBuilder('comment')
.where('comment.wishId = :wishId', { wishId })
.andWhere('comment.deleted_at IS NULL')
.orderBy('comment.createdAt', 'DESC')
.skip((page - 1) * limit)
.take(limit)
.getMany();- arrow 함수 표현식으로 바꾸면 return도 제거할 수 있습니다. 기존 function과 완전히 동일하진 않지만 이렇게도 가능합니다.
백엔드를 처음 접하여 부족한 점이 많습니다. 적극적인 코멘트 부탁드립니다!! 🙏🏻🙏🏻
Nest 로 작업했습니다.
추가기능 포함하여 모든 기능구현 완료하였습니다.