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

♻️ refactor(api): change item-detail response values #171

Closed
wants to merge 10 commits into from
Closed

♻️ refactor(api): change item-detail response values #171

wants to merge 10 commits into from

Conversation

siyeonSon
Copy link
Member

@siyeonSon siyeonSon commented Jul 5, 2023 •
edited

@siyeonSon
✨ feat: create UserDetailResponseDto
a0c30b2
@siyeonSon
✨ feat: remove musicApp from UserResponseDto
be17e76
@siyeonSon
♻️ refactor: replace UserResponseDto with UserDetailResponseDto
64f2525
@siyeonSon
🐛 fix: remove musicApp parameter in ItemControllerTest
daa5123
@siyeonSon
Merge branch 'main' into refactor/item-detail
93ba204 
# Conflicts:
#	backend/streetdrop-api/src/main/java/com/depromeet/domains/user/dto/response/UserResponseDto.java
#	backend/streetdrop-api/src/main/java/com/depromeet/domains/user/service/UserService.java
#	backend/streetdrop-api/src/test/java/unit/user/service/UserServiceTest.java
@siyeonSon
🐛 chore: resolve branch conflict
7d1dd3a
@siyeonSon
🐛 fix: resolve test fail
0021371
@siyeonSon siyeonSon added 🥈d-1 With in 1 Day ♻️refactor Refactor code. labels Jul 5, 2023
@siyeonSon siyeonSon self-assigned this Jul 5, 2023
@siyeonSon
Copy link
Member Author

siyeonSon commented Jul 5, 2023 •
edited

고민이 있어요!

image

상세 보기 화면에서 바로 듣기 버튼을 눌렀을 때, 사용자가 어떤 음악 앱을 사용하는지에 대한 정보가 필요해요.
iOS에서 앱을 삭제하기 전까지 사용자가 선택한 음악 앱 정보를 캐시로 가지고 있을 수 있다고 알고 있어요. 그래서 음악 앱 정보를 상세 조회 API에서 제공하지 않고 있어요.

로그인 기능 추가나 iOS 이외에도 웹으로 확장 등 다양한 상황들을 고려하면 사용자의 음악 앱 정보를 넘겨주어야겠다는 생각이 듭니다.

따라서 응답 값이 다음과 같이 변할 것 같아요.

  • 기존
{
  "items": [
		{
			"itemId": 1,
			"user": {
						"userId": 1 
						"nickname" : "시연",
						"profileImage" : "링크형식",
			},
			"location": {
						"address": "성동구 성수1가 1동" 
			},
			"music": {
						"title": "하입보이",
						"artist": "뉴진스",
						"albumImage": "링크형식"
						"genre": [
								"Rock",
								"K-pop"
						]
			},
			"content": "홍대입구 가려면 어떻게 가야해요?",
			"createdAt": "yyyy-MM-dd HH:mm:ss:SSS",
			"itemLikeCount": 26,
			"isLiked": false
		}
  ]
}
  • 변경
{
  "items": [
		{
			"itemId": 1,
			"me": {
						"musicApp" : "youtubemusic"
			},
			"writer": {
						"userId": 1 
						"nickname" : "시연",
						"profileImage" : "링크형식",
			},
			"location": {
						"address": "성동구 성수1가 1동" 
			},
			"music": {
						"title": "하입보이",
						"artist": "뉴진스",
						"albumImage": "링크형식"
						"genre": [
								"Rock",
								"K-pop"
						]
			},
			"content": "홍대입구 가려면 어떻게 가야해요?",
			"createdAt": "yyyy-MM-dd HH:mm:ss:SSS",
			"itemLikeCount": 26,
			"isLiked": false
		}
  ]
}

상세 조회 로직이 전반적으로 수정될 것 같아서 아직 변경은 하지 않았어요.
혹시 이런 식으로 변경하는 게 적절한지 의견이 궁금해요!

yunyoung1819
yunyoung1819 approved these changes Jul 7, 2023
View reviewed changes
Copy link
Collaborator

@yunyoung1819 yunyoung1819 left a comment •
edited

Choose a reason for hiding this comment

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

음 writer가 작성자 외에 '작곡가'라는 뜻도 있고 아래 응답값에 music이 있어서 처음 보면 작곡가로 혼동이 될 수도 있을 것 같아요.
내 정보에도 아직 뮤직 앱 하나 밖에 없어서 로그인이나 추후 기능이 붙었을 때 추가될 수 있는 값들도 한번 논의해보면 좋을 것 같아요.
다른 유저 정보에는 뮤직 앱이 들어갈 필요가 없기 때문에 내 정보(me)와 다른 유저 정보를 분리하는 것 자체는 좋은 것 같습니다. 👍

@seonghun-dev
Copy link
Collaborator

로그인 기능 추가나 iOS 이외에도 웹으로 확장 등 다양한 상황들을 고려하면 사용자의 음악 앱 정보를 넘겨주어야겠다는 생각이 듭니다.

  • 로그인 부분에 음악 앱 정보를 넘겨주는 부분과 어떠한 관련이 있을까요?
  • IOS 이외에서도 웹으로 확정해서 사용한다고 가정했을때, 기본적으로 Vue나 React를 활용하게 되는데 웹에서도 기본적으로 클라데이터에서 props등으로 상태를 가지고 유저 데이터가 표현이 될 것 같습니다. 만약 값을 변경해주게되면 React Query등을 이용해서 post 요청 이후 자동으로 get을 통해서 값 갱신이 가능해서 데이터 최신성등을 유지할 수 있을 것 같습니다.
  • 유저의 음악 앱 정보 자체가 동적 정보라기 보다는 많이 변경이 되지 않는 정적정보의 느낌이기 때문에 저희가 로그인 이후 데이터를 동기화 시키는 부분에서만 활용을 하고, 지속적으로 보내줄 필요는 없을 것 같습니다. 클라측에서도 기존 클라이언트 값을 가지고 전략패턴 활용해서 구현하는것이 더 효율적일 것 같습니다.

@siyeonSon
Copy link
Member Author

두 분의 의견을 토대로 추가 수정 없이 merge하겠습니다!

@siyeonSon
Merge branch 'main' into refactor/item-detail
0f14a2a 
# Conflicts:
#	backend/streetdrop-api/src/main/java/com/depromeet/domains/user/service/UserService.java
#	backend/streetdrop-api/src/test/java/unit/security/provider/SecurityUserDetailsTest.java
#	backend/streetdrop-domain/src/main/java/com/depromeet/user/User.java
@siyeonSon siyeonSon changed the title ♻️ refactor: change item-detail response values ♻️ refactor(api): change item-detail response values Jul 11, 2023
@siyeonSon
Merge branch 'main' into refactor/item-detail
2dc1031 
# Conflicts:
#	backend/streetdrop-api/src/main/java/com/depromeet/domains/user/dto/response/UserResponseDto.java
#	backend/streetdrop-api/src/test/java/unit/user/controller/UserControllerTest.java
@siyeonSon siyeonSon changed the base branch from main to dev July 14, 2023 16:39
@siyeonSon
Copy link
Member Author

merge하기에 버전이 많이 바뀌어서 우선 close 후에 다른 PR에서 작업하겠습니다!

@siyeonSon siyeonSon closed this Jul 15, 2023
@seonghun-dev seonghun-dev deleted the refactor/item-detail branch September 22, 2023 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seonghun-dev seonghun-dev seonghun-dev approved these changes

@yunyoung1819 yunyoung1819 yunyoung1819 approved these changes

Assignees

@siyeonSon siyeonSon

Labels
🥈d-1 With in 1 Day ♻️refactor Refactor code.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Refactor item-detail response values
3 participants
@siyeonSon @seonghun-dev @yunyoung1819