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

[BE-FEAT] 기술 관련 api 만들기 #360

Merged
merged 17 commits into from
Oct 20, 2024
Merged

Conversation

unifolio0
Copy link

@unifolio0 unifolio0 commented Oct 13, 2024

🍄 PR 확인 사항

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

  • API 명세서가 업데이트 혹은 작성이 되어 있나요?

현재 작업은 어떤 이슈를 해결한 것인지 설명해주세요.

Issue Number: #359

  • 전체 기술 리스트 반환 api
  • 단일 기술 정보 반환 api

기존 코드에서 변경된 점이 있다면 설명해주세요. (추가 X)

우선 이 정보가 들어가면 되겠지 하고 짠 코드입니다.
추가로 기존에 존재하던 api와 중복 문제 때문에 임시로 api를 대충 만들었는데 어떻게 할 지 정해야 될 것 같아요

@unifolio0 unifolio0 added the BE_FEAT ✨ 백엔드 새로운 기능 label Oct 13, 2024
@unifolio0 unifolio0 added this to the BE 포켓로그 1차 배포 milestone Oct 13, 2024
@unifolio0 unifolio0 self-assigned this Oct 13, 2024
Copy link
Contributor

@jongmee jongmee left a comment

Choose a reason for hiding this comment

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

구현 잘 해주셨네요 👍 api 엔드포인트랑 메소드명 관련해서 간단하게 리뷰 남겼어요!

@@ -31,14 +37,26 @@ public ApiResponse<List<MoveResponse>> moveListByPokedexNumber(@RequestParam("po
}

@GetMapping("/api/v1/move/{id}")
public ApiResponse<MoveResponse> moveDetails(@PathVariable String id) {
public ApiResponse<MoveResponse> moveDetailsByBattle(@PathVariable String id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이 메소드들(서비스 메소드 포함) moveDetailsInBattle 어때요 ?!

Copy link
Author

Choose a reason for hiding this comment

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

수정했습니다! ☺️☺️

return new ApiResponse<>("기술 정보 불러오기에 성공했습니다.", moveService.findMoveByBattle(id));
}

@GetMapping("/api/v1/move1/{id}")
Copy link
Contributor

Choose a reason for hiding this comment

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

지금 사용되고 있는 move1 대신에 /api/v1/move-dex/{id}/api/v1/move/dex/{id}어떤가요

'기술 도감' 이어서 생각났어요! 배틀 관련 기술 api에 battle 붙이고 싶지만 하위호환성 때문에 그건 안될 거 같고.. 🥲

Copy link
Author

Choose a reason for hiding this comment

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

수정했습니다! ☺️☺️

Copy link
Contributor

@jinchiim jinchiim left a comment

Choose a reason for hiding this comment

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

미아가 이미 리뷰를 남겨줘서 전부 동의하기도 하고
하나만 추가로 남겨봤습니다! ☺️

고생하셨어요!

@unifolio0
Copy link
Author

/noti
폴라, 미아꺼 끝~

Copy link
Contributor

@jongmee jongmee left a comment

Choose a reason for hiding this comment

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

굿💯

Copy link
Contributor

@jinchiim jinchiim left a comment

Choose a reason for hiding this comment

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

굿! 고생하셨어요! 👍🏻

Copy link

@dwax1324 dwax1324 left a comment

Choose a reason for hiding this comment

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

안녕하세요 비토
깔끔하게 잘 구현하셨네요!!
고생하셨어요👏👏

@unifolio0 unifolio0 merged commit fd1f18e into be/develop Oct 20, 2024
1 check passed
@unifolio0 unifolio0 deleted the be/feat/#359-get-move-api branch October 20, 2024 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BE_FEAT ✨ 백엔드 새로운 기능
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants