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

Jacoco 설정 및 카카오페이 결제준비 Service, Repository Test 생성 #3

Merged
merged 6 commits into from
Nov 23, 2023

Conversation

binarywoo27
Copy link
Contributor

@binarywoo27 binarywoo27 commented Nov 22, 2023

개요

Jacoco 설정 및 단건결제(카카오페이 결제준비) Service, Repository Test 생성

PR 유형

어떤 변경 사항이 있나요? ✅

content
새로운 기능 추가
버그 수정
CSS 등 사용자 UI 디자인 변경
코드에 영향을 주지 않는 변경사항(오타 수정, 탭 사이즈 변경, 변수명 변경)
코드 리팩토링
주석 추가 및 수정
문서 수정
테스트 추가, 테스트 리팩토링
빌드 부분 혹은 패키지 매니저 수정
파일 혹은 폴더명 수정
파일 혹은 폴더 삭제

스크린 샷

PR Checklist

PR이 다음 요구 사항을 충족하는지 확인하세요.

  • local에서 실행되는 것을 확인 했나요?
  • 변경 사항에 대해 테스트를 통과했나요?
  • 설계와 관련된 변경 사항을 공유했나요? (api 스펙, entity 등)

@binarywoo27 binarywoo27 requested review from nowgnas and qwerty1434 and removed request for nowgnas November 22, 2023 10:49
Copy link
Contributor

@qwerty1434 qwerty1434 left a comment

Choose a reason for hiding this comment

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

코멘트 남긴부분 수정한 뒤 merge 부탁드립니다~


@Service
public class PaymentService {}
@RequiredArgsConstructor
public class PaymentService {
Copy link
Contributor

Choose a reason for hiding this comment

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

트랜잭션이 빠져있습니다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

수정하였습니다

* @param dto
* @return
*/
public KakaopayReadyResponseDto payReady(KakaopayReadyRequestDto dto) {
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 메서드는 두 개의 역할을 가지고 있는 것 같습니다.
또한 트랜잭션 안에서 외부 호출이면서 오랜 시간이 소요될 가능성이 있는 restTemplate통신을 진행하는 게 좋지 못해 보입니다

Copy link
Contributor Author

@binarywoo27 binarywoo27 Nov 23, 2023

Choose a reason for hiding this comment

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

외부API 호출 로직을 분리하여 수정하였습니다

Copy link
Contributor

@qwerty1434 qwerty1434 left a comment

Choose a reason for hiding this comment

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

수정 확인했습니다~

@qwerty1434 qwerty1434 merged commit 5d5f295 into develop Nov 23, 2023
1 check passed
@binarywoo27 binarywoo27 deleted the LF1-302-single-payment branch December 29, 2023 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants