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.

Sign up for GitHub

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

Jump to bottom

[Feat] 인벤토리 API #46

Merged
merged 13 commits into from
Oct 3, 2024
Merged

[Feat] 인벤토리 API #46

merged 13 commits into from
Oct 3, 2024

Conversation

david-parkk
Copy link
Member

@david-parkk david-parkk commented Oct 1, 2024 •
edited
Loading

✏️ 작업 개요

⛳ 작업 분류

  • 인벤토리 조회 API
  • 인벤토리 아이템 추가 API
  • 인벤토리 테스트 코드

🔨 작업 상세 내용

  1. 인벤토리 조회 API, 인벤토리 아이템 추가 API 를 추가하였습니다.
  2. 비즈니스 로직이 정해져 있지 않는 대로 가장 단순한 형태로 구현하였습니다.
  3. 현재 유저와 캐릭터, 인벤토리간에 연관관계가 없고 인벤토리와 아이템만 연관관계를 가지고 있습니다.(생명주기가 다르지만 비즈니스 로직를 생각해봤습니다)

💡 생각해볼 문제

  • Service layer 순환 참조 #43
  • 아직 상점 도메인을 개발하지 않았기 때문에 인벤토리 도메인이 상점 도메인의 일부분 역활을 수행하고 있습니다.
  • 아이템 구입은 엔드포인트를 어떻게 해야할까요? (shops/items or /items ?)
  • 아이템과 인벤토리는 일대다로 해야할까요? 한다면 연관관계가 필요할까요?
  • 그래서 @RequestBody 는 ??

@david-parkk david-parkk added the 기능구현 Good for newcomers label Oct 1, 2024
@david-parkk david-parkk requested a review from kamothi October 1, 2024 16:40
@david-parkk david-parkk self-assigned this Oct 1, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Oct 1, 2024 •
edited
Loading

Test Results

22 tests   22 ✅  3s ⏱️
 5 suites   0 💤
 5 files     0 ❌

Results for commit 55b1214.

♻️ This comment has been updated with latest results.

kamothi
kamothi reviewed Oct 2, 2024
View reviewed changes
Copy link
Member

@kamothi kamothi left a comment

Choose a reason for hiding this comment

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

리뷰 내용에도 담겨 있는데, 유저 <-> 인벤토리 그림이 아닌 캐릭터 <-> 인벤토리가 아닌가 생각이 듭니다. 아이템을 실질적으로 사용하고 관리하는 주체는 캐릭터가 아닌가라는 생각입니다. 만약 캐릭터를 삭제하는 로직이 들어간다면 인벤토리를 모두 이어 받아서 사용할 수 있는가 하면 그것은 아니라는 생각이라. 의견이 궁금합니다.

src/main/java/ku/user/domain/inventory/domain/Inventory.java Outdated
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;

private Long userId;
Copy link
Member

Choose a reason for hiding this comment

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

PR 본문에도 궁금했던 내용인데 유저와 인벤토리가 연결되는 이유를 모르겠습니다. 인벤토리는 캐릭터와 연결되는게 맞는 그림이 아닌가 생각하는데 의도하신 이유가 궁금합니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

잘못생각했습니다. 캐릭터와 관계를 가져야 하는 것이 맞다고 생각합니다

src/main/java/ku/user/domain/inventory/service/InventoryService.java Outdated
public Inventory findByUserId(Long userId){
Optional<Inventory> inventoryOptional = inventoryRepository.findByUserId(userId);
if(inventoryOptional.isEmpty())
throw new RuntimeException("해당하는 인벤토리가 없습니다. 회원가입된 아이디가 아닙니다.");
Copy link
Member

Choose a reason for hiding this comment

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

예외 내용에 두 가지가 들어가는거 같은데 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

현재 개발된 비즈니스 로직에 따르면 유저를 생성하면서 인벤토리를 생성하기 때문에 인벤토리가 없는경우가 직접삭제하거나 회원가입조차되지 않은 경우에 해당합니다
수정이 필요한 부분 같습니다. 관련된 내용으로 캐릭터가 생성될때 인벤토리가 같이 생성되도록 변경해야 합니다.
비즈니스로 로직상으로 회원가입이 된 이후에 해당 API를 호출한다는 점에서 "회원가입된 아이디가 아닙니다" 가 삭제되어야합니다

@david-parkk
Copy link
Member Author

david-parkk commented Oct 2, 2024 •
edited
Loading

약 캐릭터를 삭제하는 로직이 들어간다면 인벤토리를 모두 이어 받아서 사용할 수 있는가 하면 그것은 아니라는 생각이라. 의견이 궁금합니다.

같이 삭제되는 것이 맞다고 생각합니다

@david-parkk david-parkk requested a review from kamothi October 2, 2024 15:38
kamothi
kamothi approved these changes Oct 3, 2024
View reviewed changes
Copy link
Member

@kamothi kamothi left a comment

Choose a reason for hiding this comment

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

저번 로직과 비슷해보이네요. 수고하셨습니다.

@david-parkk david-parkk merged commit 4ac0194 into main Oct 3, 2024
3 checks passed
@kamothi
Copy link
Member

kamothi commented Oct 5, 2024

/쿠타버스 배포

1 similar comment
@kamothi
Copy link
Member

kamothi commented Oct 5, 2024

/쿠타버스 배포

@kamothi
Copy link
Member

kamothi commented Oct 6, 2024

🌎 배포하였습니다.

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

@kamothi kamothi kamothi approved these changes

Assignees

@david-parkk david-parkk

Labels
📡 자동 배포 기능구현 Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@david-parkk @kamothi