Skip to content
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

[#12] 4.04 오늘의 상/하위 종목 API 구현 #30

Merged
merged 8 commits into from
Nov 7, 2024

Conversation

uuuo3o
Copy link
Collaborator

@uuuo3o uuuo3o commented Nov 6, 2024

✅ 주요 작업

  • 한국투자 API 사용에 필요한 access token 발급 로직 구현
  • 상/하위 종목 조회 API에 사용할 DTO 구현
  • 한국투자 API 요청 및 결과값을 받아오는 로직 구현
    • 순위 5개까지만 받아올 수 있도록 구현
  • Controller 구현
  • app module에 추가

💭 고민과 해결과정

  • access token이 필요한 API를 호출할 때마다 서비스 로직에서 access token을 요청하는 상황은 이상하다고 생각한다. 내일 BE 개발 담당자분들과 이야기를 통해 수정이 필요할 것 같다.
  • API 문서를 제대로 읽지 않고 쿼리 파라미터에서 필요하다고 판단한 내용만 전달했다가 결과값이 비어있는 채로 넘어와서 문제가 생겼다. API 문서를 다시 읽고 누락된 쿼리 파라미터 값을 추가해 해결할 수 있었다.
  • .env 파일에서 환경변수를 제대로 가지고 오지 못하는 문제가 발생했다. @nestjs/configConfigService 를 이용해 가지고 오는 방식으로 변경해 해결할 수 있었다.

@uuuo3o uuuo3o requested review from jinddings and sieunie November 6, 2024 08:45
@uuuo3o uuuo3o self-assigned this Nov 6, 2024
KOSPI200 = 'KOSPI200',
}

interface StockApiOutputData {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 인터페이스 다른 파일에 선언하니까 린트 에러 나서 여기 두신건가요?!
저도 서비스에 인터페이스 두긴 했는데 보기가 불편하더라구요... 이거도 논의해보면 좋을 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아니 저 방금 시은님 코드 리뷰에 이 내용 달고 왔는데 같은 생각을 ....ㅎㅎㅎ 나중에 한번 이야기해봐요!!


return response;
} catch (error) {
console.error('API Error Details:', {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 이 부분 깃헙 액션 오류나는데 확인해보셔야 할 것 같아요! console을 없애라네용

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

엇 저한테는 에러는 아니고 warning인데 에러인가요?? 우선 해당 내용 주석 처리하는 방식으로 변경하겠습니다

Github Actions에서 warning으로 처리되어 임시 주석처리
@uuuo3o uuuo3o linked an issue Nov 6, 2024 that may be closed by this pull request
constructor(private readonly topFiveService: TopFiveService) {}

@Get('topfive')
@ApiOperation({ summary: '오늘의 상/하위 종목 조회 API' })
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

swagger 관련 코드를 생각 못했네요..! 빨리 구현하고 저도 해보도록 하겠습니다!

);
return response.data;
} catch (error) {
console.error('API Error Details:', {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

workflow 쓰면 이렇게도 나오네요!
아마 lint에서 console.log 쓰는걸 지적하는 것 같은데 저도 잠깐 찾아보니 nestjs에서 제공하는 로깅 라이브러리가 있다니 사용해보는것도 좋을 것 같습니다!

  private readonly logger = new Logger(AuthService.name);
  constructor(
    @InjectRepository(UserRepository)
    private userRepository: UserRepository,
  ) {}

  async signUp(authCredentialsDto: AuthCredentialsDto) {
    try {
      return this.userRepository.registerUser(authCredentialsDto);
    } catch (error) {
      this.logger.error(`Failed to register user: ${error}`);
      throw error;
    }
  }
  

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 logger 쓰면 되는군요!~ 그러면 다시 수정하겠습니다!! 감사합니다 X)

@uuuo3o uuuo3o merged commit 83d246a into back/main Nov 7, 2024
2 checks passed
@uuuo3o uuuo3o added BE 백엔드 API API 구현 labels Nov 8, 2024
@uuuo3o uuuo3o deleted the feature/api/topfive-#12 branch November 15, 2024 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API 구현 BE 백엔드
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BE] 4.04 오늘의 상/하위 종목 API 구현
3 participants