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

[User] 사용자 프로필 수정 API 구현 #156

Merged
merged 23 commits into from
Jun 10, 2024

Conversation

minahYu
Copy link
Contributor

@minahYu minahYu commented Jun 5, 2024

#️⃣연관된 이슈

📝작업 내용

  • 사용자 프로필 수정 기능을 구현하였습니다.
  • 사용자 controller 테스트 코드를 작성하였습니다.
  • 사용자 프로필 이미지 업로드 기능을 구현하였습니다.

@minahYu minahYu added enhancement 추가 기능 API 상세 api 문서 User 유저 관련기능 labels Jun 5, 2024
@minahYu minahYu added this to the MSA 채팅 서비스 개발 milestone Jun 5, 2024
@minahYu minahYu self-assigned this Jun 5, 2024
Copy link
Contributor

@yudonggeun yudonggeun left a 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")
Copy link
Contributor

Choose a reason for hiding this comment

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

지원하는 media type을 해당 메서드를 호출할 때마다 생성하는데, 처음 어플리케이션 실행시에 초기화해서 사용하는 것은 어떨까요? String 보다는 Enum을 이용하면 더 좋을 것 같아요.

Copy link
Contributor Author

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"),
Copy link
Contributor

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 혹은 한국어로 된 메시지도 좋아보이는데요?

Copy link
Contributor Author

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)
Copy link
Contributor

Choose a reason for hiding this comment

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

System.getProperty를 필드로 사용하는 것은 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋습니다 수정해보도록 하겠습니다😊

Comment on lines 17 to 20

file:
path:
profile-dir: user/src/main/resources/static/images/profileImg
Copy link
Contributor

Choose a reason for hiding this comment

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

kubernetes에 배포하면 해당 디렉터리 구조가 없는데요. 문제 없이 배포했을 때, 동작을 하나요?

Copy link
Contributor Author

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,
Copy link
Contributor

Choose a reason for hiding this comment

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

multipartFile은 dto 와 같은 데이터 전송을 위한 객체이기 때문에 MockMultipartFile을 이용하여 테스트 코드에서 필요한 경우에만 사용하는 것이 좋다고 생각합니다.

reference

https://docs.spring.io/spring-framework/docs/current/javadoc-api/org/springframework/mock/web/MockMultipartFile.html

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 알겠습니다!

Copy link
Member

@minisundev minisundev 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 +343 to +346
part {
"json" mean "회원정보 수정 요청 JSON"
"file" mean "프로필 이미지 파일"
}
Copy link
Member

Choose a reason for hiding this comment

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

오 신기하네요!!

@minahYu minahYu merged commit 90df7ba into kSideProject:dev Jun 10, 2024
1 check passed
@minahYu minahYu deleted the feat/update-profile branch June 10, 2024 06:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API 상세 api 문서 enhancement 추가 기능 User 유저 관련기능
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants