Social signup / signin #120

ohchanghoon · 2024-01-15T15:07:10Z

Description

소셜 (카카오, 구글, 네이버) 유저 확인 / 회원가입 / 로그인

To Reviewer

첫 작업이니 미리 죄송합니다.

Reference Link

API

Method	Path	설명	작업(추가, 수정, 삭제)
POST	/api/auth/social/check-registration	소셜 유저 프로필 유무 조회	추가
POST	/api/auth/social/signup	소셜 회원가입	추가
POST	/api/auth/social/signin	소셜 로그인	추가

rrgks6221

지금 prettierrc를 관리하지 않아서 각 작업자들 로컬 설정을 따라가서 추후에 prettier, eslint 설정 후에 formatting 한번 돌려야할거같습니다.

rrgks6221 · 2024-01-17T00:55:41Z

migrations/1704208353171-add-modify-user-table-column.ts

@@ -0,0 +1,23 @@
+import { MigrationInterface, QueryRunner, TableColumn } from "typeorm"
+
+export class Migrations1704208353171 implements MigrationInterface {


마이그레이션을 실행할 때 클레스명을 기준으로 raw가 쌓이기떄문에 클레스명은 의미있는 이름이 들어가는게 좋습니다.(현재까지 있는 migrations raw 확인 또는 npm run migrate:show로 확인)
우선 마이그레이션 통합 작업을 진행할테니 놔두셔도 됩니다.

@rrgks6221
이거 클래스명 수동으로 변경하나요? 아니면 생성할때 명령어가 있는지 궁금합니다~

저는 README 에 있는 방법대로 하는데 그러면 클레스명이 파일명과 같아져요

rrgks6221 · 2024-01-17T00:56:48Z

migrations/1704441168745-add-enum-type-in-loginType.ts

+
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(`
+      ALTER TABLE user MODIFY COLUMN login_type ENUM('email', 'KAKAO', 'GOOGLE', 'NAVER') NOT NULL DEFAULT 'email';


email 로그인은 사용하지 않기로 했으니 없어도 되지만 현재 email 값을 사용하는 raw가 있을 수 있기 때문에 놔두시고 migration 통합 때 제가 없애도록 하겠습니다.

rrgks6221 · 2024-01-17T00:59:48Z

src/apis/auth/social/dto/auth-social.dto.ts

+    profilePath: string | null;
+
+    @IsOptional()
+    majorId: number = 1;


majorId 1번이 경우에따라 없을수도 있습니다.

userentity에서 필수값이라 isOptinal() 대신 isInt()로 변경해보았습니다.

auth-social.dto.ts nullable

rrgks6221 · 2024-01-17T01:05:44Z

src/apis/auth/social/types/auth-social.type.ts

+export interface SnsProfileBase {
+  sns_id: string;
+}


외부에서 받는 필드의 interface를 정의하는게 아니라면 가급적 camelCase로 명명하는게 좋을거같아요

rrgks6221 · 2024-01-17T01:07:54Z

src/apis/auth/util/getSnsProfile.ts

+ * @param {string} snsToken SNS에서 발급한 token
+ * @returns {Promise<SnsProfileBase>} SNS profile 데이터
+ */
+export async function getSnsProfile(loginType: string, snsToken: string): Promise<SnsProfileBase> {


Suggested change

export async function getSnsProfile(loginType: string, snsToken: string): Promise<SnsProfileBase> {

export async function getSnsProfile(loginType: UserRole, snsToken: string): Promise<SnsProfileBase | null> {

지금 tsconfig 설정이 느슨해서 타입에러가 안나는데 strictNullCheck: true로 설정할 경우 에러가 나는 정의입니다.

UserRole 대신 UserLoginType 으로 변경하겠습니다.

rrgks6221 · 2024-01-17T01:24:30Z

src/apis/auth/social/auth-social.module.ts

+    }),
+  ],
+  controllers: [AuthSocialController],
+  providers: [AuthSocialService, AuthRegistrationService, AuthService],


AuthService를 사용하고 싶다면 AuthModule에서 AuthService를 exports 하고 AuthSocialModule에서 AuthModule을 import 하면 됩니다. 그러면 EncryptionModule, JwtModule을 제거할 수 있습니다.

https://joorrr.tistory.com/24
module imports관련 참조 링크입니당

@rrgks6221 @hobiJeong
좀 헷갈리는데 맞는 방식인지 검토한번 부탁드립니다!

rrgks6221 · 2024-01-17T01:25:47Z

src/apis/auth/util/getSnsProfile.ts

+    let result: SnsProfileBase;
+
+    switch (loginType) {
+      case 'KAKAO': {


각 케이스에 지정돼있는 string 값을 enum을 사용해서 넣으면 좋겠습니다.

rrgks6221 · 2024-01-17T01:26:57Z

src/apis/auth/social/controllers/auth-social.controller.ts

+import { AuthRegistrationService } from "../service/auth-registration.service";
+import { ApiAuthSocial } from "./auth-social.swagger";
+
+@ApiTags('auth/social')


Suggested change

@ApiTags('auth/social')

@ApiTags('auth-social')

구분자는 -로 해주세요

rrgks6221 · 2024-01-17T01:28:42Z

src/apis/auth/social/controllers/auth-social.controller.ts

+  async checkRegistration(@Body() checkRegistrationRequestBodyDto: CheckRegistrationRequestBodyDto): Promise<boolean> {
+    return await this.authRegistrationService.checkUserRegistered(checkRegistrationRequestBodyDto)
+  }
+
+  @ApiAuthSocial.SignUp({ summary: '소셜 회원가입' })
+  @Post('signup')
+  async signUp(@Body() signUpRequestBodyDto: SignUpRequestBodyDto) {
+    return await this.authSocialService.signUp(signUpRequestBodyDto);
+  }
+
+  @ApiAuthSocial.SignIn({ summary: '소셜 로그인' })
+  @Post('signin')
+  async signIn(@Body() signInRequestBodyDto: SignInRequestBodyDto) {
+    return await this.authSocialService.signIn(signInRequestBodyDto);
+  }


https://eslint.org/docs/latest/rules/no-return-await
이 규칙이 고려되면 좋겠습니다.

todo 예정

rrgks6221 · 2024-01-17T01:29:09Z

package.json

@@ -67,6 +67,7 @@
    "@types/express": "^4.17.17",
    "@types/jest": "^29.5.2",
    "@types/node": "^20.3.1",
+    "@types/node-fetch": "^2.6.9",


http client를 어떤걸 사용할 지 내부적으로 한번 정해야겠군요

일단은 유지하겠습니다

…igration

hobiJeong · 2024-01-19T01:37:44Z

src/apis/auth/social/dto/auth-social.dto.ts

+        format: 'email',
+    })
+    @IsEmail()
+    @IsNullable()


IsNullable 데코레이터는 null이 아닌 경우에만 유효성 검사를 진행하게 만드는 데코레이터이고 타입만 봤을땐 email, name, role은 null이 들어오면 안되는 것 같은데 이렇게 했을 때 null이 들어오면 그냥 통과될 것 같습니다.
의도하신 거라면 타입에 null도 추가해주셔야 할 것 같아용

role을 제외하고 우선 null허용으로 작업하고 추후에 기획이 수정되면 다시 재작업하도록 하겠습니다!

rrgks6221

이게 일단 클라이언트측에서 사용해야하는 부분이라 먼저 내보내고 기획이 제대로 나오면 리펙토링이랑 같이 진행하는게 좋을듯하네요

rrgks6221 · 2024-01-21T11:00:56Z

migrations/1704208353171-add-modify-user-table-column.ts

@@ -0,0 +1,23 @@
+import { MigrationInterface, QueryRunner, TableColumn } from "typeorm"
+
+export class Migrations1704208353171 implements MigrationInterface {


저는 README 에 있는 방법대로 하는데 그러면 클레스명이 파일명과 같아져요

ohchanghoon · 2024-01-21T11:06:32Z

npx typeorm migration:create ./migrations/{file_name} 확인했습니다!

ohchanghoon added the Feat 새로운 기능 추가 label Jan 15, 2024

ohchanghoon self-assigned this Jan 15, 2024

ohchanghoon requested review from rrgks6221 and hobiJeong as code owners January 15, 2024 15:07

rrgks6221 reviewed Jan 17, 2024

View reviewed changes

ohchanghoon added 11 commits January 18, 2024 23:36

feat/#108: add and modify columns in user entity, update login_type m…

dcfc13d

…igration

feat/#108: add and modify columns in user entity, update login_type

1d21ec4

feat/#108: add auth module

ab7f221

feat/#108: add checking for social users

8b9b09a

feat/#108: add signup/signin for social users

fbf5764

feat/#108: add add node-fetch

09d7a75

modify/#108: service import approach within module

ae79d09

refactor/#108: function name

75847dd

modify/#108: validation check

44a1115

refactor/#108: using enum, error handling improved

4a6803d

refactor/#108: adjust types and configurations

d0194be

ohchanghoon force-pushed the feat/#108/social-login branch from f5f7943 to d0194be Compare January 18, 2024 14:37

ohchanghoon requested a review from rrgks6221 January 18, 2024 14:37

hobiJeong reviewed Jan 19, 2024

View reviewed changes

feat/#108: modify type

3e93c5f

rrgks6221 approved these changes Jan 21, 2024

View reviewed changes

ohchanghoon merged commit f7d2175 into develop Jan 23, 2024

ohchanghoon deleted the feat/#108/social-login branch January 23, 2024 08:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Social signup / signin #120

Social signup / signin #120

ohchanghoon commented Jan 15, 2024

rrgks6221 left a comment

rrgks6221 Jan 17, 2024

ohchanghoon Jan 18, 2024

rrgks6221 Jan 21, 2024

rrgks6221 Jan 17, 2024

rrgks6221 Jan 17, 2024

ohchanghoon Jan 18, 2024

ohchanghoon Jan 18, 2024

rrgks6221 Jan 17, 2024

rrgks6221 Jan 17, 2024

ohchanghoon Jan 18, 2024

rrgks6221 Jan 17, 2024

hobiJeong Jan 17, 2024

ohchanghoon Jan 18, 2024

rrgks6221 Jan 17, 2024

rrgks6221 Jan 17, 2024

rrgks6221 Jan 17, 2024

ohchanghoon Jan 18, 2024

rrgks6221 Jan 17, 2024

ohchanghoon Jan 18, 2024

hobiJeong Jan 19, 2024

ohchanghoon Jan 20, 2024

rrgks6221 left a comment

rrgks6221 Jan 21, 2024

ohchanghoon commented Jan 21, 2024

		@@ -0,0 +1,23 @@
		import { MigrationInterface, QueryRunner, TableColumn } from "typeorm"

		export class Migrations1704208353171 implements MigrationInterface {

	export async function getSnsProfile(loginType: string, snsToken: string): Promise<SnsProfileBase> {
	export async function getSnsProfile(loginType: UserRole, snsToken: string): Promise<SnsProfileBase \| null> {

Social signup / signin #120

Social signup / signin #120

Conversation

ohchanghoon commented Jan 15, 2024

Description

To Reviewer

Reference Link

Related Issue Link

API

rrgks6221 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rrgks6221 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ohchanghoon commented Jan 21, 2024