-
Notifications
You must be signed in to change notification settings - Fork 0
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
group api & vote api 추가 #12
base: develop
Are you sure you want to change the base?
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.
migrations 폴더는 삭제해주셔야 될 것 같습니다!
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.
아 맞네요
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.
반영 완료
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.
몇가지 삭제 가능한 부분 확인하여 코멘트 남깁니다!
@@ -26,6 +26,7 @@ export class AuthController { | |||
@ApiOperation({ summary: "카카오 로그인 callback" }) | |||
async kakaoLogin(@Query("code") query: string, @Res() res: Response) { | |||
const token = await this.authService.getKakaoIdToken(query); | |||
console.log(token); |
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.
삭제해도 되는 부분 같습니다!
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.
해당 파일도 삭제하시면 될 것 같습니다.
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.
Oauth2Guard
의 isSignUp
을 모두 false
로 바꿔도 될 것 같습니다.
isSignUp
이 true
일 경우, 카카오 로그인 된 유저가 회원 가입이 되어있지 않을 때 exception
을 보내지 않고 oauthId
를 CurrentUser
로 넘겨주는데 이 옵션은 회원가입 과정에서만 필요한 것으로 확인됩니다!
import { IsInt, Min, Max } from "class-validator"; | ||
|
||
export class PatchRegistrationDto { | ||
@ApiProperty({ description: "닉네임" }) |
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.
description 수정해야 할 것 같습니당
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.
Request DTO의 file UUID 형식 관련 의견입니다. 이 부분이 맛집 관련해서 이렇게 슬랙에서 논의된 적이 있는데 단체 사진에 대해서는 따로 논의되지 않았네요 통일하는게 좋지 않을까 싶습니다!
++ 그리고 추가로 getGroup
, petchGroup
, createGroup
에서 사용하는 GroupDto
에는 File 관련 정보가 담기지 않는데 추가하는게 어떨까요? 사실 이부분이 API 문서 작성 부분에서 언급되었어야 했는데 좀 늦게 논의되어서... Restaurant API와 User API는 File 정보를 포함하고 있습니다.
@ApiProperty({ description: "단체 이미지 파일 uuid" }) | ||
@IsUUID() | ||
@IsNotEmpty() | ||
@IsOptional() | ||
fileUUID: string; |
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.
@ApiProperty({ description: "단체 이미지 파일 uuid" }) | ||
@IsUUID() | ||
@IsNotEmpty() | ||
@IsOptional() | ||
fileUUID: string; |
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.
여기도 마찬가지로
fileUUID
필드 대신에
files: { uuid: string }[]
형태로 바꿔주면 될 것 같습니다!
Related to #{이슈 번호 기입}
체크 리스트
작업 내역
비고