-
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] 영어공지 자동 번역 기능 #111
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthrough이 변경 사항은 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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: 1
🧹 Outside diff range and nitpick comments (3)
src/ai/dto/req/translate.dto.ts (1)
19-24
: 변환 로직에 대한 설명 주석이 필요합니다.DeepL API가 왜 'en' 대신 'en-US'를 필요로 하는지 설명하는 주석을 추가하면 좋을 것 같습니다.
다음과 같이 주석을 추가하는 것을 제안합니다:
+ // DeepL API는 영어 번역시 'en' 대신 'en-US' 형식을 요구합니다. @Transform(({ value }) => { if (value === 'en') { return 'en-US'; } return value; })
src/ai/ai.controller.ts (1)
41-41
: 서비스 계층으로의 위임이 적절합니다.비즈니스 로직을 서비스 계층으로 이동시킨 것은 좋은 구조적 변경입니다. 이는:
- 관심사의 분리 원칙을 잘 따름
- 코드의 재사용성과 테스트 용이성 향상
- 향후 DeepL API 통합 시 수정이 용이한 구조
다만, 에러 처리를 위해 try-catch 블록 추가를 고려해보시기 바랍니다:
async translate( @Body() translateDto: TranslateDto, ): Promise<TranslateResDto> { - return this.aiService.translate(translateDto); + try { + return await this.aiService.translate(translateDto); + } catch (error) { + // DeepL API 관련 에러 처리 + throw error; + } }package.json (1)
39-39
: DeepL 의존성 추가 승인
deepl-node
패키지 추가는 영어 공지 자동 번역 기능 구현을 위한 적절한 선택입니다. 현재 계정 설정이 완료되지 않은 상태이지만, 프론트엔드 및 Flutter 테스트를 위한 목업 API 구현에도 활용할 수 있습니다.다만, 다음 사항들을 고려해보시기 바랍니다:
- 버전을
^1.14.0
에서~1.14.0
으로 변경하여 마이너 버전 업데이트로 인한 잠재적 호환성 문제를 방지- 개발 환경에서 목업 API 구현 시 필요한 환경 변수 설정 방법을 README에 문서화
적용하실 경우 다음과 같이 수정해주세요:
- "deepl-node": "^1.14.0", + "deepl-node": "~1.14.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (6)
.vscode/settings.json
(1 hunks)package.json
(1 hunks)src/ai/ai.controller.ts
(1 hunks)src/ai/ai.service.ts
(3 hunks)src/ai/dto/req/translate.dto.ts
(2 hunks)src/ai/dto/res/translateRes.dto.ts
(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- .vscode/settings.json
🔇 Additional comments (7)
src/ai/dto/res/translateRes.dto.ts (2)
2-2
: DeepL 타입 정의 임포트 추가 확인
DeepL API 타입 정의를 사용하여 타입 안정성이 향상되었습니다.
15-15
: lang 속성의 타입 변경 검증 필요
string
에서 deepl.TargetLanguageCode
로의 타입 변경은 적절하나, 이 DTO를 사용하는 기존 코드에 영향을 줄 수 있습니다.
다음 스크립트로 이 DTO를 사용하는 다른 코드들을 확인하세요:
✅ Verification successful
Let me gather more information about how TranslateResDto is used in the identified files.
Let me check the implementation of the translate method to see how the TranslateResDto is constructed.
Let me check one final thing - how the translate method constructs and returns the TranslateResDto.
Let me try a different pattern to find the translate method implementation.
타입 변경이 안전함을 확인했습니다
TranslateResDto
의 lang
속성 타입을 string
에서 deepl.TargetLanguageCode
로 변경하는 것이 안전합니다. 그 이유는:
translate
메서드에서targetLang
이 이미deepl.TargetLanguageCode
타입의 매개변수로 전달되어 사용되고 있습니다.- DTO를 반환할 때
targetLang
을 그대로lang
속성에 할당하고 있어 타입이 일치합니다. - 다른 코드에서는 이 DTO의
lang
속성에 직접 접근하거나 조작하는 패턴이 발견되지 않았습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# TranslateResDto 사용 현황 확인
rg -l "TranslateResDto"
# lang 속성 사용 패턴 검색
ast-grep --pattern 'lang: $_'
Length of output: 135
Script:
#!/bin/bash
# Check how TranslateResDto is used in ai.service.ts and ai.controller.ts
rg "TranslateResDto" src/ai/ai.service.ts src/ai/ai.controller.ts -B 2 -A 2
# Check for any lang property access patterns
rg "\.lang" src/ai/ai.service.ts src/ai/ai.controller.ts -B 2 -A 2
Length of output: 1489
Script:
#!/bin/bash
# Check the translate method implementation in ai.service.ts
rg "translate.*Promise<TranslateResDto>" src/ai/ai.service.ts -A 15
# Also check the full content of translateRes.dto.ts to understand the complete structure
cat src/ai/dto/res/translateRes.dto.ts
Length of output: 459
Script:
#!/bin/bash
# Get the full implementation of the translate method
ast-grep --pattern 'translate($_): Promise<TranslateResDto> {
$$$
}'
Length of output: 72
Script:
#!/bin/bash
# Get the translate method implementation with more context
rg "async translate" src/ai/ai.service.ts -A 20 -B 2
Length of output: 568
src/ai/dto/req/translate.dto.ts (2)
2-2
: 필요한 의존성이 올바르게 추가되었습니다.
DeepL API 통합을 위한 적절한 의존성들이 import 되었습니다.
Also applies to: 4-4
25-25
: 타입 변경이 적절히 이루어졌습니다.
DeepL API의 타입 시스템을 활용하여 타입 안정성이 향상되었습니다. 다만, 지원되는 언어 코드가 올바르게 처리되는지 확인이 필요합니다.
✅ Verification successful
DeepL API 타입 시스템이 올바르게 통합되었습니다
DeepL API의 TargetLanguageCode
타입이 다음과 같이 적절하게 사용되고 있음을 확인했습니다:
- DTO에서 타입 정의 (
translate.dto.ts
,translateRes.dto.ts
) - 서비스 레이어에서 DeepL Translator 인스턴스 생성 및 사용 (
ai.service.ts
)
타입 시스템이 컴파일 시점에서 유효한 언어 코드만 허용하도록 보장하므로 추가 검증이 필요하지 않습니다.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# DeepL API에서 지원하는 언어 코드 사용 확인
rg -A 2 "targetLang.*=.*['\"]" --type ts
Length of output: 44
Script:
#!/bin/bash
# 실제 사용되는 targetLang 값 검색
rg -p "targetLang.*" --type ts
# DeepL 관련 타입 정의 확인
rg -p "deepl.*TargetLanguageCode" --type ts
# 테스트 코드에서 사용되는 언어 코드 확인
rg -p "targetLang.*" --type test
Length of output: 481
Script:
#!/bin/bash
# DeepL 관련 설정 및 초기화 코드 확인
rg -p "deepl.*" -A 5 --type ts
# 테스트 파일 검색 (확장자 기반)
fd -e spec.ts -e test.ts
# 번역 관련 테스트 코드 확인
rg -p "translate.*test" -A 10 --type ts
Length of output: 1673
src/ai/ai.controller.ts (1)
Line range hint 1-40
: 컨트롤러 구조가 잘 설계되어 있습니다!
컨트롤러가 NestJS의 모범 사례를 잘 따르고 있습니다:
- Swagger 문서화가 잘 되어있음
- OAuth2 인증이 적절히 구현됨
- ValidationPipe를 통한 입력 유효성 검사
- 의존성 주입이 올바르게 구현됨
src/ai/ai.service.ts (2)
83-99
: 번역 기능의 적절한 구현 확인
DeepL API를 활용한 번역 기능이 효과적으로 추가되었습니다. 로깅을 통해 함수 호출의 흐름을 추적하는 것도 좋습니다.
20-22
:
Logger 생성자에서 잘못된 옵션 사용
NestJS에서 Logger
클래스의 생성자는 두 번째 인수로 옵션 객체를 받지 않습니다. 'timestamp'
옵션은 지원되지 않습니다.
다음과 같이 수정하십시오:
- private readonly logger = new Logger(AiService.name, {
- timestamp: true,
- });
+ private readonly logger = new Logger(AiService.name);
⛔ Skipped due to learnings
Learnt from: siwonpada
PR: gsainfoteam/ziggle-be#108
File: libs/infoteam-idp/src/infoteam-idp.service.ts:16-18
Timestamp: 2024-10-31T14:13:09.815Z
Learning: In NestJS, the `Logger` 생성자에서 `timestamp` 옵션을 사용하여 경과 시간을 표시할 수 있다.
Fixes #89
Summary by CodeRabbit
New Features
deepl-node
라이브러리 통합.Bug Fixes
Documentation
TranslateDto
및TranslateResDto
의 속성 타입을 더 구체적인 타입으로 변경하여 데이터 검증 강화.