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

[SWEP-51] Session 테이블 수정 및 마이그레이션 추가 #57

Merged
merged 2 commits into from
Jan 21, 2025

Conversation

GodUser1005
Copy link
Collaborator

@GodUser1005 GodUser1005 commented Jan 21, 2025

Sweepic Server PR List

close #55

⚒️develop의 최신 커밋을 pull 받았나요?

  • 최신 커밋 업데이트

🔍️ 이 PR을 통해 해결하려는 문제가 무엇인가요?

어떤 기능을 구현한건지, 이슈 대응이라면 어떤 이슈인지 PR이 열리게 된 계기와 목적을 Reviewer 들이 쉽게 이해할 수 있도록 적어 주세요
일감 백로그 링크나 다이어그램, 피그마를 첨부해도 좋아요

  • 로그인 기능 추가시 Session에서 타입이 맞지않아 발생 하는 오류 해결
    schema.prisma에서 정의한 Session 모델과 @quixo/prisma-session-store 모듈의 PrismaSessionStore() 생성자에서 요구하는 Session에 대한 모델이 달라서 타입오류가 발생

    image image

✨ 이 PR에서 핵심적으로 변경된 사항은 무엇일까요? (핵심 작업 내용)

문제를 해결하면서 주요하게 변경된 사항들을 적어 주세요

  • schema.prisma에서 Session 모델 수정
    image

    • 기존의 sid에 없었던 @unique 어노테이션 추가
    • Session의 모든 칼럼을 not null로 설정
    • user와의 연결성 해제
  • schema.prisma에서 User 모델 수정
    image

    • 기존에 있었던 session Session[] 부분을 제거(Session과 User와의 외래키로 연결된 부분 삭제)
    • Session 테이블은 다른 테이블과 아무런 연관관계 없이 단독으로 유지됨
  • 위와 같이 바꾼 이유는 @quixo3/prisma-session-store를 이용하려면 아래와 같이 Session 테이블의 형태를 유지해줘야 타입에러가 발생하지 않음
    image
    아래는 공식문서에서의 내용입니다.
    https://www.npmjs.com/package/@quixo3/prisma-session-store

    예를 들어
    image
    공식문서상에는 sid에 @unique 어노테이션 들어가있지만 안들어간 상태로 만들경우
    yarn prisma generate를 통해 prisma client를 만들때
    여기서 만들어진 Session의 타입으로 prisma client의 타입을 정의 하게되고
    이러한 타입의 정의는 node_modules/.prisma/client/index.d.ts 에 생성된다.

    image

    위의 사진처럼 node_modules/.prisma/client/index.d.ts 파일 안에
    Prisma에 의해 자동 생성된 타입인 SessionWhereUniqueInput 이라는 타입이 정의 되어있는데
    prisma-session-store에서 요구하는 건 sid?: string 인데 자동생성된 prisma client는 sid?: StringFilter<"Session"> | string 이렇게 정의가 되어있다.

    그러나 @unique 어노테이션을 넣으면
    image
    다음과 같이 sid?: string으로 타입이 되어있기 때문에 이는 prisma-sesssion-store가 요구하는 타입과 맞아
    yarn compile을 통과하게 된다.

    이처럼 공식문서가 요구하는 Session 모델과 다를 경우 타입에러가 발생하며
    이를 막기위해 schema.prisma의 session 모델을 수정했다.

🤚 동작 확인

기능을 실행했을 때 정상 동작하는지 여부를 확인하고 스크린 샷을 올려주세요

  • yarn compile 확인
    image

🔖 핵심 변경 사항 외에 추가적으로 변경된 부분이 있나요?

없으면 "없음" 이라고 기재해 주세요

  • migration 진행을 위해 migrations 에 변경사항 추가

🙏 Reviewer 분들이 이런 부분을 신경써서 봐 주시면 좋겠어요

개발 과정에서 다른 분들의 의견은 어떠한지 궁금했거나 크로스 체크가 필요하다고 느껴진 코드가 있다면 남겨주세요

  • 현재 마이그레이션은 저의 로컬에서 적용하였고 모든 리뷰어들이 승인을 하면 해당 마이그레이션 내용을 RDS에 deploy 하겠습니다!

🩺 이 PR에서 테스트 혹은 검증이 필요한 부분이 있을까요?

테스트가 필요한 항목이나 테스트 코드가 추가되었다면 함께 적어주세요

📌 PR 진행 시 이러한 점들을 참고해 주세요

  • Reviewer 분들은 코드 리뷰 시 좋은 코드의 방향을 제시하되, 코드 수정을 강제하지 말아 주세요.
  • Reviewer 분들은 좋은 코드를 발견한 경우, 칭찬과 격려를 아끼지 말아 주세요.
  • Review는 특수한 케이스가 아니면 Reviewer로 지정된 시점 기준으로 2일 이내에 진행해 주세요.
  • Comment 작성 시 Prefix로 P1, P2, P3 를 적어 주시면 Assignee가 보다 명확하게 Comment에 대해 대응할 수 있어요
    • P1 : 꼭 반영해 주세요 (Request Changes) - 이슈가 발생하거나 취약점이 발견되는 케이스 등
    • P2 : 반영을 적극적으로 고려해 주시면 좋을 것 같아요 (Comment)
    • P3 : 이런 방법도 있을 것 같아요~ 등의 사소한 의견입니다 (Chore)


📝 Assignee를 위한 CheckList

  • To-Do Item

@GodUser1005 GodUser1005 added the 🐛 FIX 버그 수정 label Jan 21, 2025
@GodUser1005 GodUser1005 self-assigned this Jan 21, 2025
@GodUser1005 GodUser1005 linked an issue Jan 21, 2025 that may be closed by this pull request
Copy link
Collaborator

@Socializedistp Socializedistp left a comment

Choose a reason for hiding this comment

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

확인했습니다

@jjiinaaa
Copy link
Contributor

[P1] 어떤 문제가 발생했는지, 원인이 뭐였는지도 PR 내용에 있으면 좋겠습니다.

@GodUser1005
Copy link
Collaborator Author

[P1] 어떤 문제가 발생했는지, 원인이 뭐였는지도 PR 내용에 있으면 좋겠습니다.

핵심 사항에 추가로 원인과 해결예시를 좀더 자세하게 설명해두었습니다

Copy link
Contributor

@jjiinaaa jjiinaaa left a comment

Choose a reason for hiding this comment

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

LGTM 👍
완벽합니다! 이해가 잘되네요~

Copy link
Collaborator

@codie0226 codie0226 left a comment

Choose a reason for hiding this comment

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

확인했습니다!

Copy link
Collaborator

@jonaeunnn jonaeunnn left a comment

Choose a reason for hiding this comment

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

좋아요 ~

Copy link
Collaborator

@asjasj3964 asjasj3964 left a comment

Choose a reason for hiding this comment

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

새롭게 알게 된 내용이 많네요 타입 에러 해결하시느라 수고하셨습니다!!

@GodUser1005
Copy link
Collaborator Author

마이그레이션 적용 시켰습니다

@GodUser1005 GodUser1005 merged commit 645b740 into develop Jan 21, 2025
1 check passed
@GodUser1005 GodUser1005 deleted the fix/SWEP-51 branch January 21, 2025 14:20
GodUser1005 added a commit to GodUser1005/sweepic-Server that referenced this pull request Jan 23, 2025
[SWEP-51] Session 테이블 수정 및 마이그레이션 추가
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 FIX 버그 수정
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SWEP-51] Session 테이블 수정
6 participants