스탬프 수정 및 삭제 기능 구현 #115
스탬프 수정 및 삭제 기능 구현 #115
Conversation
|
| public ResponseEntity<ExceptionResponse> handleForbiddenStampToUpdateException( | ||
| final ForbiddenStampToUpdateException exception, final HttpServletRequest request | ||
| ) { | ||
| System.out.println("handleForbiddenStampToUpdateException 들어옴"); |
| getUser(userId); | ||
| getGoal(goalId); |
There was a problem hiding this comment.
조회한 결과를 사용하지 않는 것은 존재 여부만 확인하기 때문일까요?
맞다면, existsById를 통한 검증도 있을 것 같은데 어떻게 생각하시나요?
| } | ||
|
|
||
| private void validUserToUpdate(final Long userId, final Long stampUserId) { | ||
| if (!userId.equals(stampUserId)) { |
There was a problem hiding this comment.
Stamp 내부에 isWriter를 오버로딩한 메서드를 만들면 어떨까요?
혹은 조회한 User를 사용하던가요!
아래 delete와 비슷한 플로우 같은데 어떤 기준으로 서로 다른지 궁금합니다!
| if (updateStamp.changeToDefaultImage()) { | ||
| stamp.deleteImageUrl(); | ||
| } |
There was a problem hiding this comment.
해당 기능은 안드로이드 측과도 논의된 부분일까요?
해당 기능에 대해 이야기해 본 기억이 없는데, 궁금해서 여쭤봅니다!
| public static UpdateStampDto of(final Long stampId, | ||
| final String message, | ||
| final MultipartFile stampImage, | ||
| final boolean changeToDefaultImage) { |
There was a problem hiding this comment.
괄호 앞에 개행 부탁드립니다!
| final boolean changeToDefaultImage) { | |
| final boolean changeToDefaultImage | |
| ) { |
| public void deleteImageUrl() { | ||
| this.stampImageUrl = DEFAULT_STAMP_IMAGE_URL; | ||
| } | ||
|
|
| public ResponseEntity<ReadStampResponse> update( | ||
| @PathVariable("goalId") final Long goalId, | ||
| @PathVariable("stampId") final Long stampId, | ||
| @RequestPart @Valid final UpdateStampRequest request, |
There was a problem hiding this comment.
UpdateStampRequest에 Valid 관련 어노테이션이 없는데, 없어도 뭐가 해주는 기능이 있는 걸까요?
| } | ||
|
|
||
| @Test | ||
| void 수정_요청한_스탬프_메모의_길이가_30자_초과이거나_빈값인_경우_예외를_발생한다() { |
There was a problem hiding this comment.
해당 예외는 각각 다른 상황이라고 생각해, 분리해서 테스트하는 게 가독성 측면과, 명확한 테스트를 위해 더 좋지 않을까 싶은데 어떻게 생각하시나요?
| .with(request -> { | ||
| request.setMethod("PATCH"); | ||
| return request; | ||
| }) |
There was a problem hiding this comment.
파라미터가 있는 경우 multipart()를 통해서 HttpMethod를 전달해주지 못하는군요..!
해당 테스트와 관련한 것은 아닌데, 궁금해서 찾아보다가, 아래래와 같이, Method를 설정해주지 않는 경우 무조건 POST로 요청을 보내는 것을 확인했습니다.
그런데 저희 테스트 중에 아래와 같은 테스트가 있는데, 디버깅을 통해 살펴본 결과 뒤에 HttpMethod.POST는 무시되는 것으로 보입니다. 다행히도 해당 테스트의 요청 메서드가 POST였기에 문제가 없었지만, 명확히 하기 위해 위 방식으로 수정해야 할지 궁금합니다!
| .header("X-API-VERSION", 1) | ||
| .header(HttpHeaders.AUTHORIZATION, 액세스_토큰) | ||
| .with(request -> { | ||
| request.setMethod("PATCH"); |
There was a problem hiding this comment.
별건 아니지만, 실수를 줄이기 위해 아래처럼 수정하면 어떨까요?
| request.setMethod("PATCH"); | |
| request.setMethod(HttpMethod.PATCH.name()); |
7/23
드디어 끝이 났습니다... 구현 난이도에 비해 너무 오래 걸려 죄송합니다..
스탬프 삭제 기능의 경우 HardDelete 구현한 부분이 #110 에 있어 해당 pr 머지 후에 HardDelete로 반영하도록 하겠습니다.
문제상황 생긴 부분에 대한 빠른 피드백 감사합니다.