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

fix : user-show 관심사 이동 #159

Merged
merged 9 commits into from
Sep 15, 2024
Merged

fix : user-show 관심사 이동 #159

merged 9 commits into from
Sep 15, 2024

Conversation

GaBaljaintheroom
Copy link
Contributor

😋 작업한 내용

  • user-show API들이 user-api, show-api에 흩어져 있어서 구조가 모호해지는 느낌이 있어서 user-show API 를 show-api에 몰아넣었습니다.

🙏 PR Point

  • user domain 모듈에서는 User, ArtistSubscription, GenreSubscription, InterestShow 등등의 사용자가 공연 도메인과 연관된 엔티티가 있습니다. 이때 user가 회원탈퇴 할 때 하나의 트랜잭션에서 hard delete를 실행하려면 같은 모듈에 있어야함으로 그대로 두었고, user-show의 관심사 API를 show-api에 패키지를 나눠 관심사를 모아두었습니다. (EX : 기존은 사용자가 구독, 구독 취소, 구독 목록 API 등은 show-api에 있지만, 구독한 개수 API는 user-api 모듈에 있어 관심사가 모호하게 흩어진 느낌이 있기 때문)
  • 따라서 show-api에 UserShowController을 만들어서 관심사를 모아두었습니다.
  • user-show를 show-api 에서 요청을 받고, user-artist 혹은 user-genre도 show 처럼 API가 많아지거나 하면 이처럼 변경하면 어떨까요?
  • user-show 관련 API는 show-api 모듈에 있고, user-show 관련 도메인은 user 모듈에 있는데 괜찮을까요? (저는 괜찮다고 생각하는데 구조가 이상하다 생각하신다면 말씀해주세용)

👍 관련 이슈


@GaBaljaintheroom GaBaljaintheroom added the fix fix feature label Sep 12, 2024
@GaBaljaintheroom GaBaljaintheroom self-assigned this Sep 12, 2024
Copy link
Contributor

@devmizz devmizz left a comment

Choose a reason for hiding this comment

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

코드레벨에서 피드백할 건 거의 없어서 Approve 눌러 놓을게요 :)
위에서 얘기한 것들만 확인해주세욥

@GaBaljaintheroom GaBaljaintheroom changed the base branch from temp-develop to develop September 14, 2024 17:13
@GaBaljaintheroom GaBaljaintheroom merged commit 32af59e into develop Sep 15, 2024
1 check passed
@GaBaljaintheroom GaBaljaintheroom deleted the fix/structrues branch September 15, 2024 16:34
GaBaljaintheroom added a commit that referenced this pull request Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix fix feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants