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

[refactor] 카카오 오픈채팅 api service 분리 #212

Merged
merged 4 commits into from
Jan 30, 2024
Merged

Conversation

pkl0912
Copy link
Contributor

@pkl0912 pkl0912 commented Jan 30, 2024

관련 이슈번호

해결하는 데 얼마나 걸렸나요? (예상 작업 시간 / 실제 작업 시간)

  • 1h~2h

해결하려는 문제가 무엇인가요?

  • 카카오 오픈채팅 로직 service 분리

어떻게 해결했나요?

image
applicationUrl 외에 다른 건 다 designer관련입니다!
근데 offerId 로 application, designer 를 찾는 로직입니다

@pkl0912 pkl0912 added this to the 1차 리팩토링 기간 milestone Jan 30, 2024
@pkl0912 pkl0912 requested review from hellozo0 and KWY0218 January 30, 2024 07:15
@pkl0912 pkl0912 self-assigned this Jan 30, 2024
Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

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

작업하시느라 고생하셨습니다!
코맨트 확인해주세요!

@@ -39,6 +40,7 @@
public class ModelController {

private final ModelService modelService;
private final ModelRetrieveService modelRetrieveService;
Copy link
Member

Choose a reason for hiding this comment

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

p2
현재 ModelController이긴 하지만 해당 API는 OfferController의 성격을 띄고 있습니다.
비록 현재 라우터는 /model/{offerId}/agress 이지만, 이는 추후에 수정할 예정이기 때문에
지금은 어색하더라도 ModelRetrieveService가 아닌 OfferRetrieveService를 통해 정보를 가져오는 것이 좋을 것 같습니다

Comment on lines 11 to 14
@Service
@RequiredArgsConstructor
@Transactional(readOnly = true)
public class ModelRetrieveService {
Copy link
Member

Choose a reason for hiding this comment

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

p2
ModelRetrieveService 가 아닌 OfferRetrieveService 라고 생각하고 리뷰 달겠습니다!

Comment on lines 21 to 22
Long designerId = hairServiceOfferRetrieveService.getDesignerApplicationId(offerId).designerId();
Long applicationId = hairServiceOfferRetrieveService.getDesignerApplicationId(offerId).applicationId();
Copy link
Member

Choose a reason for hiding this comment

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

p1
우선 Offer 엔티티는 designerId와 applicationId를 외래키로 가지고 있습니다.
그래서 각 도메인의 retrieve 서비스를 이용하지 않아도
offerRepository를 통해서 해당 Offer를 가져온다면 designerId와 applicationId를 가지고 올 수 있습니다.

Comment on lines 20 to 23
public DesignerInfoOpenChatDTO getDesignerOpenDetail(final Long userId){
Designer designer = designerJpaRepository.findById(userId).orElseThrow(() -> new NotFoundException(ErrorCode.DESIGNER_NOT_FOUND_EXCEPTION));
return new DesignerInfoOpenChatDTO(designer.getKakaoOpenChatUrl(),designer.getProfileImgUrl(), designer.getHairShop().getName(), designer.getName(), designer.getIntroduction());
}
Copy link
Member

Choose a reason for hiding this comment

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

👍

Long designerId = hairServiceOfferRetrieveService.getDesignerApplicationId(offerId).designerId();
Long applicationId = hairServiceOfferRetrieveService.getDesignerApplicationId(offerId).applicationId();

OpenChatResponse openChatResponse = new OpenChatResponse(hairModelApplicationRetrieveService.getApplicationCaptureUrl(applicationId), designerRetrieveService.getDesignerOpenDetail(designerId).kakaoUrl(), designerRetrieveService.getDesignerOpenChatDetail(designerId));
Copy link
Member

Choose a reason for hiding this comment

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

p2
applicationRetreieveService를 통해서 applicationCaptureUrl을 가져온 것은 좋습니다.
designerResponse를 반환할 때 designerRetrieveService에서 하는 것이 아닌 해당 로직에서 하는 것은 어떠나요?

아래 suggestion 코드의 변수 네이밍은 임의로 정한 것입니다! 코드를 반영하실 땐 적절한 네이밍으로 작성해주세요!

Suggested change
OpenChatResponse openChatResponse = new OpenChatResponse(hairModelApplicationRetrieveService.getApplicationCaptureUrl(applicationId), designerRetrieveService.getDesignerOpenDetail(designerId).kakaoUrl(), designerRetrieveService.getDesignerOpenChatDetail(designerId));
String applicationCaptureUrl = hairModelApplicationRetrieveService.getApplicationCaptureUrl(applicationId);
DesignerInfoOpenChatDTO dto = designerRetrieveService.getDesignerOpenDetail(designerId);
DesignerInfoOpenChatResponse response = new DesignerInfoOpenChatResponse(dto.imgUrl(), dto.shopName(), dto.name(), dto.introduction())
OpenChatResponse openChatResponse = new OpenChatResponse(applicationCaptureUrl, dto.kakaoUrl(), response);
return openChatResponse;

Comment on lines 2 to 4

public record DesignerInfoOpenChatDTO(
String kakaoUrl,
Copy link
Member

Choose a reason for hiding this comment

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

p2
앱잼 때 DTO가 아닌 Dto로 명시하기로 정한 걸로 기억하고 있습니다..!

Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

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

수정사항 변경하느라 고생하셨습니다!
자잘한 코맨트 확인해주세요!

Comment on lines 11 to 14
@Service
@RequiredArgsConstructor
@Transactional(readOnly = true)
public class ModelRetrieveService {
Copy link
Member

Choose a reason for hiding this comment

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

p2
사용하지 않는 파일은 제거해주세요!

Comment on lines 19 to 20
public DesignerInfoOpenChatDto getDesignerOpenDetail(final Long userId){
Designer designer = designerJpaRepository.findById(userId).orElseThrow(() -> new NotFoundException(ErrorCode.DESIGNER_NOT_FOUND_EXCEPTION));
Copy link
Member

Choose a reason for hiding this comment

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

p2
userId 보다 designerId로 하는 것이 더욱 명시적일 것 같습니다!

Comment on lines 24 to 26

public OpenChatResponse getOpenChatInfo(Long userId, Long offerId) {
HairServiceOffer hairServiceOffer = hairServiceOfferJpaRepository.findById(offerId).orElseThrow(() -> new NotFoundException(ErrorCode.NOT_FOUNT_OFFER_EXCEPTION));
Copy link
Member

Choose a reason for hiding this comment

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

p3
해당 파라메터에도 final 을 붙이면 좋을 것 같습니다

@@ -73,6 +74,7 @@ public class ModelService {
private final HairServiceRecordJpaRepository hairServiceRecordJpaRepository;
private final AuthService authService;
private final S3Service s3Service;
private final DesignerRetrieveService designerRetrieveService;
Copy link
Member

Choose a reason for hiding this comment

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

p2
사용하지 않는 서비스는 제거해주세요!
+) import

Copy link
Member

@KWY0218 KWY0218 left a comment

Choose a reason for hiding this comment

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

굳 좋습니다~!

@KWY0218
Copy link
Member

KWY0218 commented Jan 30, 2024

@pkl0912
추가로!! 커밋할 때 앞에 #이슈번호 잊지 말아주세요!!

Copy link
Member

@hellozo0 hellozo0 left a comment

Choose a reason for hiding this comment

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

구우욷... 고생하셨습니다..!!!
Designer랑 Application의 이미지 캡쳐만 가져오는데도 많은..파일 터치가 있네요.... 🫠

@pkl0912 pkl0912 merged commit 1002c00 into develop Jan 30, 2024
1 check passed
@pkl0912 pkl0912 deleted the refactor/#211 branch January 30, 2024 12:48
@KWY0218 KWY0218 removed this from the 1차 리팩토링 기간 milestone Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[refactor] designer service 분리
3 participants