-
Notifications
You must be signed in to change notification settings - Fork 1
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
[Feature] client가 등록한 authorites를 확인할 수 있는 api 만들기 #97
Conversation
📝 WalkthroughWalkthrough이 풀 리퀘스트는 클라이언트의 UUID를 통해 외부 권한(External Authority) 정보를 포함한 클라이언트 세부 정보를 조회할 수 있는 새로운 API 엔드포인트를 추가합니다. 변경 사항은 컨트롤러, 서비스, 리포지토리, DTO, 그리고 타입 파일에 걸쳐 있으며, 클라이언트가 등록한 권한을 확인할 수 있는 기능을 구현합니다. Changes
Assessment against linked issues
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/client/dto/res/expandedClientRes.dto.ts (1)
13-16
: 속성 이름을 복수 형태로 고려해 보세요.
ExternalAuthority
가 배열이므로,externalAuthorities
등 복수 형태의 네이밍을 사용하면 가독성이 더 좋아질 수 있습니다.src/client/client.controller.ts (1)
39-49
: API 문서화 개선이 필요합니다.현재 API 문서화에 다음과 같은 보완이 필요합니다:
- API 설명에 반환되는 권한 정보에 대한 상세 내용 추가
- 클라이언트를 찾을 수 없는 경우에 대한
@ApiForbiddenResponse
추가다음과 같이 Swagger 데코레이터를 추가해주세요:
@ApiOperation({ summary: 'Get clients', - description: 'Get clients with name and uuid', + description: 'UUID로 클라이언트의 상세 정보와 등록된 외부 권한 목록을 조회합니다.', }) @ApiOkResponse({ description: 'The clients have been successfully fetched.', type: [ExpandedClientResDto], }) +@ApiForbiddenResponse({ + description: '클라이언트를 찾을 수 없습니다.', +}) @ApiInternalServerErrorResponse({ description: 'Unknown errors', })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/client/client.controller.ts
(2 hunks)src/client/client.repository.ts
(2 hunks)src/client/client.service.ts
(2 hunks)src/client/dto/res/expandedClientRes.dto.ts
(1 hunks)src/client/types/ExpandedClient.type.ts
(1 hunks)
🔇 Additional comments (8)
src/client/types/ExpandedClient.type.ts (1)
3-7
: 타입 정의가 간결하고 명확합니다.
Prisma.ClientGetPayload를 이용해 관련 데이터를 포함하도록 설계한 부분이 매우 유용합니다.
src/client/dto/res/expandedClientRes.dto.ts (1)
5-11
: AuthorityResDto의 속성이 적절히 문서화되었습니다.
Swagger 문서를 통해 클라이언트에게 필요한 정보를 명확하게 전달할 수 있다는 점이 좋습니다.
src/client/client.service.ts (2)
11-11
: 새로운 타입을 가져오기 위한 import가 적절합니다.
바로 아래 메서드에서 해당 타입을 명확히 활용하고 있으므로 구조가 일관됩니다.
32-40
: 클라이언트를 조회하는 로직이 명확하고 예외 처리가 적절합니다.
ForbiddenException
을 던지는 방식이 직관적이며, 디버그 로깅도 유용하게 활용되었습니다.
src/client/client.repository.ts (2)
12-12
: ExpandedClient 타입을 사용하는 import가 잘 추가되었습니다.
올바른 위치와 네이밍으로 인해 가독성과 유지보수성이 향상됩니다.
58-69
: 권한 포함 조회 로직이 명확하게 구현되었습니다.
include: { ExternalAuthority: true }
옵션을 통해 해당 관계를 직관적으로 가져오고 있으며, 예외 상황을 고려한 반환 타입도 적절합니다.
src/client/client.controller.ts (2)
31-31
: DTO 임포트가 올바르게 추가되었습니다!
확장된 클라이언트 응답 DTO를 사용하여 외부 권한 정보를 포함한 응답을 구조화할 수 있습니다.
50-53
: 🛠️ Refactor suggestion
컨트롤러 레벨에서의 예외 처리가 필요합니다.
서비스 메소드에서 발생할 수 있는 예외에 대한 처리가 컨트롤러 레벨에서 누락되어 있습니다.
다음과 같이 try-catch 블록을 추가하는 것을 권장합니다:
@Get(':uuid')
async getClient(@Param('uuid') uuid: string): Promise<ExpandedClientResDto> {
- return this.clientService.getClient(uuid);
+ try {
+ return await this.clientService.getClient(uuid);
+ } catch (error) {
+ if (error instanceof ForbiddenException) {
+ throw error;
+ }
+ throw new InternalServerErrorException('클라이언트 조회 중 오류가 발생했습니다.');
+ }
}
서비스 메소드의 예외 처리를 확인하기 위해 다음 스크립트를 실행합니다:
✅ Verification successful
Let me search for the service implementation with a broader pattern and also look for any error throwing patterns in the codebase to verify the error handling requirements.
컨트롤러에서 예외 처리가 필요합니다
코드베이스 분석 결과, ClientService의 getClient 메소드는 다음과 같은 예외를 발생시킬 수 있습니다:
- ForbiddenException: 클라이언트를 찾을 수 없는 경우
- InternalServerErrorException: 데이터베이스 오류나 알 수 없는 오류가 발생한 경우
따라서 다음과 같이 컨트롤러에서 예외를 처리하는 것이 필요합니다:
@Get(':uuid')
async getClient(@Param('uuid') uuid: string): Promise<ExpandedClientResDto> {
try {
return await this.clientService.getClient(uuid);
} catch (error) {
if (error instanceof ForbiddenException) {
throw error;
}
throw new InternalServerErrorException('클라이언트 조회 중 오류가 발생했습니다.');
}
}
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify error handling in ClientService.getClient method
# Search for getClient method implementation and its error handling
ast-grep --pattern 'getClient(uuid: string) {
$$$
}'
Length of output: 56
Script:
#!/bin/bash
# Search for getClient method implementation with broader pattern
ast-grep --pattern 'getClient($$$) {
$$$
}'
# Also search for any error throwing patterns in the service
rg "throw\s+new" -A 2 -B 2
Length of output: 22652
Fixes #54
Summary by CodeRabbit
신규 기능
버그 수정