-
Notifications
You must be signed in to change notification settings - Fork 6
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
[User] 사용자 프로필 수정 API 구현 #156
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다!!!
} | ||
|
||
private fun isFileExtensionSupported(multipartFile: MultipartFile) { | ||
val supportedExtensions = listOf("image/png", "image/jpeg") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지원하는 media type을 해당 메서드를 호출할 때마다 생성하는데, 처음 어플리케이션 실행시에 초기화해서 사용하는 것은 어떨까요? String 보다는 Enum을 이용하면 더 좋을 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 말씀해 주신 대로 수정하겠습니다!
NOT_ALLOWED(HttpStatus.FORBIDDEN, "4003", "Not allowed"), // 권한이 없는 경우 | ||
|
||
INCORRECT_PASSWORD(HttpStatus.UNAUTHORIZED, "4010", "Incorrect password"), | ||
USER_NOT_FOUND(HttpStatus.NOT_FOUND, "4011", "User not found"), | ||
|
||
EXTENSION_NOT_SUPPORTED(HttpStatus.BAD_REQUEST, "4020", "Extension is not supported"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extension is not supported 메시지가 실패시 client 응답으로 표현되는데 이 표현으로는 어떤 실수를 했는지 파악하기가 힘들어보여요. http에서 사용하는 content type이나 media type을 사용하는 것은 어떨까요?
not supported media type, not supported content type 혹은 한국어로 된 메시지도 좋아보이는데요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
동근님이 말씀해 주신 대로 구체적으로 적어서 한국어로 바꾸는 게 좋을 것 같네요!
TODO("Not yet implemented") | ||
var newPassword: String? = null | ||
var uniqueFileName: String? = null | ||
val profileImgDir = Paths.get(System.getProperty("user.dir")).resolve(profileImgDirPath) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
System.getProperty를 필드로 사용하는 것은 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다 수정해보도록 하겠습니다😊
|
||
file: | ||
path: | ||
profile-dir: user/src/main/resources/static/images/profileImg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kubernetes에 배포하면 해당 디렉터리 구조가 없는데요. 문제 없이 배포했을 때, 동작을 하나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일단 구현할 때는 그 부분을 고려하지 못해서, 해당 부분에 대해서는 공부가 필요할 것 같습니다,,! 공부를 해보고 수정하겠습니다!
@@ -40,6 +48,7 @@ class UserControllerTest( | |||
webContext: WebApplicationContext, | |||
@MockkBean val authClient: AuthClient, | |||
@MockkBean val userService: UserService, | |||
@MockkBean val multipartFile: MultipartFile, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
multipartFile은 dto 와 같은 데이터 전송을 위한 객체이기 때문에 MockMultipartFile
을 이용하여 테스트 코드에서 필요한 경우에만 사용하는 것이 좋다고 생각합니다.
reference
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 알겠습니다!
…t/update-profile
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
너무 수고많으셨어요!!
part { | ||
"json" mean "회원정보 수정 요청 JSON" | ||
"file" mean "프로필 이미지 파일" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 신기하네요!!
#️⃣연관된 이슈
📝작업 내용