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

Api: ✨ 사용자 정의 카테고리 수정 API #121

Merged
merged 47 commits into from
Jul 4, 2024

Conversation

psychology50
Copy link
Member

작업 이유

  • 사용자는 카테고리 정보(이름, 아이콘)를 수정할 수 있다.

작업 사항

  • 요청 경로 : PATCH /v2/spending-categories/{categoryId}?name=&icon=
    • 쿼리 파라미터로 사용할 DTO는 생성할 때와 조건이 동일하므로 재활용 했습니다.
  • 원래 카테고리 이름 글자수 제한이 15자였는데, 이번에 다시 보니 8글자로 바뀌어 있더라구요. 그래서 반영했습니다.
  • SpendingCategoryManager의 권한 검사 hasPermission()을 사용할 때, 기존에는 pk가 -1인 경우를 예외적으로 처리하고 있습니다. 그런데 이게 특수한 경우라 메서드를 분리했습니다.

리뷰어가 중점적으로 확인해야 하는 부분

  • WebMvcTest말고 RestTemplate를 써봤는데...진짜 별의 별 일을 다 겪어서 통합 테스트에만 3시간을 썼네요. 사용법은 RestTemplate와 동일합니다.
    • 저도 쓰는데 제법 애먹어서, 모르는 부분 질문 주시면 최대한 상세하게 답변 드리겠습니다. 컨트롤러 단위 테스트 시엔 MockMvc를 사용하는 게 적절하고, 통합 테스트에선 TestRestTemplate를 사용해서 Servlet까지 동작 확인할 수 있었습니다.
    • 몇 개는 유틸로 분리해서 편의성을 높여볼까 했는데, 저도 처음 써본 거라 일단 보류 했어요.
    • @SpringBootApplication에서 랜덤 포트 개방이 필수적이라 옵션을 추가했습니다.
  • 처음에 요구사항을 잘못 정의해서 Api: ✨ 카테고리에 등록된 소비 리스트 무한 스크롤 조회 API #120 PR의 커밋을 몇 개 cherrypick으로 가져왔는데, 필요가 없었어요. ㅜㅜ 라인 수만 길어졌네요. 확인하실 필요없는 클래스 적어놓겠습니다.
    • WebConfig.java
    • SpendingCategoryType.java
    • SpendingCategoryTypeConveter.java
    • SpendingCategory.java

발견한 이슈

  • TestRestTemplate를 사용할 때, 쿼리 파라미터에 한글을 전달하면 자꾸 실패해서 찾아보니 uriComponentsBuilder.build().encode().toUri() 이렇게 하면 된다는데 안 되더라구요...^^
    그래서 build를 할 때 false 옵션을 주면 인코딩을 안 하고 전달한다길래 그렇게 처리했습니다.

@psychology50 psychology50 added the enhancement New feature or request label Jul 3, 2024
@psychology50 psychology50 self-assigned this Jul 3, 2024
Copy link
Collaborator

@asn6878 asn6878 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다. 로직은 간단한데 RestTemplate를 몰라서 이것저것 찾아보고 왔네요ㅎㅎㅎ

기존 통합테스트에서 MockMvc를 사용해 진행했었는데, 혹시 RestTemplate을 활용해 Servlet Container까지 실제처럼 구동시켰을 때 테스트 측면에서 얻는 이점이 뭐가있는지 여쭤봐도 될까요??

@psychology50
Copy link
Member Author

고생하셨습니다. 로직은 간단한데 RestTemplate를 몰라서 이것저것 찾아보고 왔네요ㅎㅎㅎ

기존 통합테스트에서 MockMvc를 사용해 진행했었는데, 혹시 RestTemplate을 활용해 Servlet Container까지 실제처럼 구동시켰을 때 테스트 측면에서 얻는 이점이 뭐가있는지 여쭤봐도 될까요??

솔직히 이점보다 그냥 처음 써보느라 죽겠다는 심정밖에 안 들긴 했는데...ㅋㅋㅋㅋㅋㅋㅋ
서블릿 컨테이너를 사용해서, 실제 클라이언트 관점에서 테스트가 가능하다는데 솔직히 체감상 딱히 느껴지는 건 없었어요..ㅎㅎ

@psychology50 psychology50 merged commit a10e6e8 into dev Jul 4, 2024
1 check passed
@psychology50 psychology50 deleted the feat/PW-403-update-custom-category branch July 4, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants