[조하나] Good-Night-3rd-Hackathon-Backend 제출 #12
Conversation
유효성 검사 미완
객체 전체를 반환
객체 전체 반환 중
기본 CRUD 파일 추가
페이지네이션 적용 전
dlwhsk0
changed the title
| name: 'is_confirm', | ||
| comment: '승인 상태', | ||
| }) | ||
| isConfirm: string; |
There was a problem hiding this comment.
지금 상태에서는 요청 객체에서 enum 값으로 유효성 검사를 하고, 내부 서비스 로직에서는 문자열을 사용하는 것 같아요. 이 방식은 문자열 값을 직접 다루는 과정에서 오타나 일관성 없는 값을 사용하게 될 위험이 있어요.
대신, 데이터베이스에는 varchar(10) 타입으로 저장해서 유연성을 유지하면서도, 내부 로직에서는 Enum을 사용해 타입 안전성과 유지보수성을 확보할 수 있을 것 같아요. 이렇게 하면 코드가 더 명확해지고, 상태 값 관리도 더 편리해지며, 개발자가 실수할 위험도 줄어들 것 같은데, 어떻게 생각하시나요?
| isConfirm: string; | |
| isConfirm: WishStatus; |
| @Query('limit') limit: string = '10', | ||
| @Query('offset') offset: string = '0', | ||
| ): Promise<any> { | ||
| return await this.wishesService.findAll(+confirm, +limit, +offset); |
There was a problem hiding this comment.
현재 confirm을 통해 서버에서 승인 여부를 처리하고 있는데, 이 방법 대신 클라이언트에서 status 값을 직접 전달받아 그에 따라 데이터를 조회하고 응답하는 방식으로 API를 변경하는 게 어떨까요? 이렇게 하면 API가 더 포괄적으로 사용될 수 있고, 다양한 상태에 대한 처리가 유연해질 것 같습니다. 어떻게 생각하실까요?
|
|
||
| // 소원 등록 | ||
| createWish(createWishDto: CreateWishDto) { | ||
| const wish = this.create({ ...createWishDto, isConfirm: '보류됨' }); |
There was a problem hiding this comment.
UTF-8 인코딩을 사용하는 데이터베이스의 경우, 한글은 한글자가 2~3 바이트를 차지하지만 영어는 1바이트만 차지하기때문에, 영어로 데이터를 저장하면 더 작은 바이트 크기를 유지할 수 있을 것 같아요.
| import { CreateWishDto } from '../dto/create-wish.dto'; | ||
|
|
||
| @Injectable() | ||
| export class WishesRepository extends Repository<Wish> { |
There was a problem hiding this comment.
단순 궁금증이에요.WishesRepository, WishesConfirmRepository 나눈 기준이 궁금해요. WishesRepository 에도 승인 관련한 쿼리가 있는 것 같아서 여쭤봐요.
There was a problem hiding this comment.
#12 (comment) 의 WishesConfirm에 대한 고민과 이어지는 부분입니다!
소원 관련해서 필수로 구현해야했던 기능이 아래처럼 5개가 있었는데,
- 소원 등록 2. 소원 삭제 3. 소원 승인/거절 4. 소원 단일 조회 5. 소원 목록 조회
이 중 3번의 소원 승인/거절 기능은 관리자가 사용할 기능이고 관리자 기능을 다른 기능들과 파일을 분리해 작성하면 스웨거 문서화나 추가적인 관리자 기능을 구현할 때 편리할 것 같아서 분리하게 되었어요.
그런데 말씀하신대로 WishesRepository에도 비슷한 조회 기능이 있기도 하고, 파일 구조 측에서 이렇게 쓰이기도 하는지 잘 모르겠어서 끝까지 계속 수정하게 되었던 부분입니다. 이렇게 분리해서 작성하는 점에 대해 어떻게 생각하시나요? 🥺
There was a problem hiding this comment.
아하, 좋은 고민이라고 생각해요.
추후에 public과 admin 모듈을 어떻게 분리할지에 따라 결정될 부분인 것 같습니다.
저라면 위의 경우, 다음과 같이 3개의 모듈로 분리할 것 같아요. 여기서 공통 유틸리티를 포함하는 common 모듈은 편의상 제외할게요.
- persistence (entity, repository)
- public (persistence 의존성을 가짐, service, controller)
- admin (persistence 의존성을 가짐, service, controller)
모노레포 환경에서 모듈이 분리되어 각기 다른 포트로 실행되는 어플리케이션의 경우, repository에서 겹치는 부분이 많았던 경험이 있어요. 그래서 repository를 공통으로 관리하는 것�을 통해 중복 코드를 줄이고, 유지보수를 더 쉽게 할 수 있었어요.
만약 repository 도 분리해서 모듈을 가져간다면 다른 클래스로 구현되는 것이 편할 것 같고, repository 는 공통으로 사용하면서 service 만 public/admin 모듈에서 나누서 가져가는 구조라면 공통으로 사용되어도 좋다고 생각해요.
| async findAll( | ||
| confirm: number, | ||
| limit: number, | ||
| offset: number, | ||
| ): Promise<Wish[]> { | ||
| const queryBuilder = this.createQueryBuilder('wish'); | ||
|
|
||
| if (confirm) { | ||
| // 승인된 경우 | ||
| queryBuilder.where('wish.is_confirm = :isConfirm', { | ||
| isConfirm: '승인됨', | ||
| }); | ||
| } else { | ||
| // 미승인된 경우 | ||
| queryBuilder.where('wish.is_confirm IN (:...isConfirm)', { | ||
| isConfirm: ['보류됨', '거절됨'], | ||
| }); | ||
| } | ||
|
|
||
| return queryBuilder | ||
| .orderBy('wish.createdAt', 'DESC') | ||
| .limit(limit) | ||
| .offset(offset) | ||
| .getMany(); | ||
| } |
There was a problem hiding this comment.
클라이언트로부터 상태값(status) 및 정렬 순서(order)를 받아서 처리하여 중복 코드를 제거하여, 메서드 재사용성을 높여볼 수 있을 것 같아요. 정렬 컬럼도 받아도 좋을 것 같은데, 하나님이 필요하다고 생각되시면 추가해주세요.
위 방식대로 하면 findAll, confirmList 를 분리하지 않고 하나의 메서드를 재사용할 수 있을 것 같아요.
| async findAll( | |
| confirm: number, | |
| limit: number, | |
| offset: number, | |
| ): Promise<Wish[]> { | |
| const queryBuilder = this.createQueryBuilder('wish'); | |
| if (confirm) { | |
| // 승인된 경우 | |
| queryBuilder.where('wish.is_confirm = :isConfirm', { | |
| isConfirm: '승인됨', | |
| }); | |
| } else { | |
| // 미승인된 경우 | |
| queryBuilder.where('wish.is_confirm IN (:...isConfirm)', { | |
| isConfirm: ['보류됨', '거절됨'], | |
| }); | |
| } | |
| return queryBuilder | |
| .orderBy('wish.createdAt', 'DESC') | |
| .limit(limit) | |
| .offset(offset) | |
| .getMany(); | |
| } | |
| async findWishes( | |
| status: ConfirmStatus[], // 상태값 배열로 받기 | |
| order: 'ASC' | 'DESC', // 정렬 순서 | |
| limit: number, | |
| offset: number, | |
| ): Promise<Wish[]> { | |
| const queryBuilder = this.createQueryBuilder('wish'); | |
| this.applyStatusFilter(queryBuilder, status); | |
| return queryBuilder | |
| .orderBy('wish.createdAt', order) | |
| .limit(limit) | |
| .offset(offset) | |
| .getMany(); | |
| } | |
| // 상태 필터를 적용하는 메서드 | |
| private applyStatusFilter(queryBuilder: SelectQueryBuilder<Wish>, status: ConfirmStatus[]): void { | |
| if (status.length > 0) { | |
| queryBuilder.where('wish.is_confirm IN (:...status)', { status }); | |
| } | |
| } |
src/app.module.ts
Outdated
| username: process.env.DB_USERNAME, | ||
| password: process.env.DB_PASSWORD, | ||
| database: process.env.DB_NAME, | ||
| entities: [Wish, Comment], |
There was a problem hiding this comment.
이 설정은 프로젝트 내에 있는 모든 *.entity.ts 또는 *.entity.js 파일들을 자동으로 찾아줘서, 파일 경로를 일일이 지정할 필요 없이, 새로운 엔티티 파일을 추가해도 TypeORM이 알아서 불러오기 때문에 엔티티가 늘어날 때 계속 추가할 필요가 없어요.
entities: [__dirname + '/**/*.entity{.ts,.js}'],| // 댓글 등록 | ||
| async create(createCommentDto: CreateCommentDto) { | ||
| const { wishId, content } = createCommentDto; | ||
| const wish = this.wishesRepository.findById(wishId); |
|
|
@dlwhsk0 하나님, 안녕하세요! 코멘트 드린 부분에 대해서 리뷰 반영이 종료된 것 같아 해당 PR 머지하려고 하는데 괜찮을까요?!😀 외에 궁금하신 부분이 있다면, 더 이야기 나눈 후에 머지하려고 합니다. |
|
@hocaron 네! 파일 구조나 예외 처리는 조언해주신 내용을 참고해서 더 생각해보고 지금 진행 중인 nest 프로젝트에서 팀원들과 얘기하면서 적용해보겠습니다! 이번 해커톤을 참여하고 선우님의 피드백을 받으면서 정말 많은 걸 배운 것 같습니다. 감사합니다. 🥹👍👍 |
Nest.js 로 필수 기능까지 구현했습니다.
소원 승인/거절 기능은 스웨거에서 admin 카테고리로 분리했습니다.