Feat/upload 이미지 업로드 구현 #56
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sally0226
reviewed
Sep 8, 2024
sally0226
reviewed
Sep 8, 2024
sally0226
reviewed
Sep 8, 2024
src/user/user.service.ts
Outdated
Comment on lines
51
to
65
| async update( | ||
| id: number, | ||
| updateUserDto: UpdateUserDto, | ||
| profileImage?: Express.Multer.File, | ||
| ) { | ||
| const user = await this.userRepository.findOne(id); | ||
| if (user == undefined) { | ||
| throw new UserNotFoundException(); | ||
| } | ||
|
|
||
| if (profileImage) { | ||
| const profileImageUrl = await this.uploadService.uploadFile(profileImage); | ||
| Object.assign(updateUserDto, { profileImageUrl }); | ||
| } | ||
|
|
There was a problem hiding this comment.
request로부터 받은 이미지 업로드 하는 로직을 분리하는건 어떨까유?
controller에서 서비스 두개 호출
- 사진 업로드 서비스 호출 후url 받기
- 받은 url을 이용해 user update 서비스 호출
그러면 userService.update에서는 파일을 직접받지 않고 사진 url (string)만 받아 처리할 수 있어 간편해질 것 같고
추후에 다른 곳에도 사진 업로드가 필요할 때 편리할 것 같아서유 ~
(아래 형태로 확장 가능)
controller에서 서비스 두개 호출
- 사진 업로드 서비스 호출 후url 받기
- 받은 url을 이용해 ???? 서비스 호출
There was a problem hiding this comment.
나도 이 방식을 선호하는데, 전에 같이 협업하던 프론트분도 이 방식이 불편하다해서,
웹팀한테 물어보니 , 지금 방식으로 해달라고 해서 이렇게 작성하였습니다!
There was a problem hiding this comment.
웅웅!! API를 두개로 나누라는 말은 아니었어~~
컨트롤러 안에서 나누자는 뜻!!
- AS-IS
- user update service에서 1)파일업로드, 2)업로드된 url로 user profile 정보 업데이트 한번에 진행
- TO-BE
- file upload service 호출해서 파일업로드
- user update service에는 string형태로 profile_image 전달
sally0226
reviewed
Sep 8, 2024
src/user/user.controller.ts
Outdated
Comment on lines
69
to
78
| async updateMe( | ||
| @Body() updateUserDto: UpdateUserRequestDto, | ||
| @CurrentUser() user: User, | ||
| @UploadedFile() profileImage?: Express.Multer.File, | ||
| ): Promise<UserResponseDto> { | ||
| const updatedUser = await this.userService.update(user.id, updateUserDto); | ||
| const updatedUser = await this.userService.update( | ||
| user.id, | ||
| updateUserDto, | ||
| profileImage, | ||
| ); |
There was a problem hiding this comment.
예를들면 요런식으로여~~~
async updateMe(
@Body() updateUserDto: UpdateUserRequestDto,
@CurrentUser() user: User,
@UploadedFile() profileImage?: Express.Multer.File,
): Promise<UserResponseDto> {
const uploadedURL: String = await this.fileService.upload(profileImage);
const updatedUser = await this.userService.update(
user.id,
updateUserDto,
uploadedURL,
);
sally0226
approved these changes
Sep 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Type
For what purpose
Key changes
To reviewers
Screenshots