Skip to content

feat: identity#5

Open
ratafa wants to merge 12 commits intomainfrom
feat/identity
Open

feat: identity#5
ratafa wants to merge 12 commits intomainfrom
feat/identity

Conversation

@ratafa
Copy link
Collaborator

@ratafa ratafa commented Sep 8, 2024

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

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

  • identity 도메인 구현

✨ 이 PR에서 핵심적으로 변경된 사항은 무엇일까요?

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

  • identity 도메인에 대한 전반적인 기능을 구현했습니다.
    • repository
    • dao
    • service
    • controller

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

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

  • 없음

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

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

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

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

  • 테스트 코드 작성을 아직 restdocs 양식에 맞게 작성하지 못한 점 양해 부탁드리겠습니다. 추후 변경하겠습니다.

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

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


📝 Assignee를 위한 CheckList

  • 대표 신분 설정 테스트 코드 작성
  • redis 캐시 기능 추가

@ratafa ratafa requested a review from SangMinLeeAI September 8, 2024 10:16
@ratafa ratafa self-assigned this Sep 8, 2024
val identityList = identityRepository.findByUserId(userId);

// 신분의 개수가 1개인 경우 업데이트 로직을 실행하지 않습니다.
if (identityList?.size == 1) return null;
Copy link
Member

Choose a reason for hiding this comment

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

신분이 하나인 경우 예외를 던지는 것도 하나의 방법이 될 것 같습니다.

Comment on lines +31 to +33
identityList?.forEach { identity: Identity ->
identityRepository.updateActiveIdentityList(identity.id, userId)
}
Copy link
Member

Choose a reason for hiding this comment

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

IdentityRepository 내부에 대표 신분을 설정하는 함수를 정의해서 비즈니스 로직과 분리하면 좋을 것 같습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants