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

feat : 다른 유저 정보 조회 API 추가 #158

Merged
merged 8 commits into from
Nov 14, 2024
Merged

feat : 다른 유저 정보 조회 API 추가 #158

merged 8 commits into from
Nov 14, 2024

Conversation

sh0723
Copy link
Contributor

@sh0723 sh0723 commented Nov 9, 2024

📌 Related Issue

#152

🚀 Description

  • 하나의 api에 user( desc, email, bjNick)등등 과 해당 user가 속한 그룹리스트에 대한 정보를 담는건 크고 코드 재사용성도 떨어지는 것으로 판단되어 두개로 쪼개서 api를 만들었습니다.

📢 Review Point

  • 기존의 코드들을 최대한 재사용해서 만들었는데 혹시 고쳐야하는 부분 있으면 말해주세요!

📚Etc (선택)

@sh0723 sh0723 self-assigned this Nov 9, 2024
Copy link
Contributor

@s-hwan s-hwan 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
Contributor

@rladmstn rladmstn left a comment

Choose a reason for hiding this comment

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

수고하셨어요!! 코멘트 고려 부탁합니다-!

@@ -167,4 +167,12 @@ public ResponseEntity<Void> editStudyGroupVisibility(@AuthedUser User user,
studyGroupService.editStudyGroupVisibility(user, request);
return ResponseEntity.ok().build();
}

@GetMapping(value = "other-user-group_list")
Copy link
Contributor

Choose a reason for hiding this comment

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

엔드포인트에 이상한 언더바가 끼워져있어요..!

그리고 이 부분은 @PathVariable을 사용하는게 더 좋아보여요! 모르신다면 찾아보길 권장드립니다
Github도 url 보면 사용자 닉네임 들어가듯이 /api/{userId}/group 뭐 이런식으로 쓰이는건데 이 API에서는 이런식의 엔드포인트가 더 적절하지 않을까 싶습니담

@@ -167,4 +167,12 @@ public ResponseEntity<Void> editStudyGroupVisibility(@AuthedUser User user,
studyGroupService.editStudyGroupVisibility(user, request);
return ResponseEntity.ok().build();
}

@GetMapping(value = "other-user-group_list")
@Operation(summary = "그룹 목록 조회 API", description = "방장 여부 상관 없이 유저가 참여하고 있는 그룹 모두 조회")
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] Swagger summary가 위에 현재 유저의 그룹 목록 조회랑 겹칩니다.
로직상 문제는 없지만 프론트 배려를 위해 더 정확한 메세지로 명시해줍시다!

@@ -432,4 +432,47 @@ public void editStudyGroupVisibility(User user, EditGroupVisibilityRequest reque
member.updateVisibility(request.isVisible());
log.info("success to update group visibility ( userId : {} )", user.getId());
}

@Transactional(readOnly = true)
public GetStudyGroupListsResponse getOtherStudyGroupList(Long targetUserId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 뭔가 내 그룹 목록 조회 메소드랑 중복이 많아서 조회 메소드를 하나로 빼서 재사용하는 리팩토링 할 수 있을 것 같은데..

Copy link
Contributor

@hwangjokim hwangjokim left a comment

Choose a reason for hiding this comment

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

아 리뷰 해놓고 서브밋 안함;;

build.gradle Outdated
Comment on lines 62 to 64
// QueryDSL
implementation 'com.querydsl:querydsl-jpa:5.0.0:jakarta'
annotationProcessor "com.querydsl:querydsl-apt:5.0.0:jakarta"
Copy link
Contributor

Choose a reason for hiding this comment

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

QueryDSL 이미 바로 위쪽에 선언되어있어요

Copy link
Contributor

Choose a reason for hiding this comment

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

덜 지워졌어요

@@ -167,4 +167,12 @@ public ResponseEntity<Void> editStudyGroupVisibility(@AuthedUser User user,
studyGroupService.editStudyGroupVisibility(user, request);
return ResponseEntity.ok().build();
}

@GetMapping(value = "other-user-group_list")
Copy link
Contributor

Choose a reason for hiding this comment

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

underscore(_) 말고 dash(-)로 써주세요. 혼용되어있어요

@@ -167,4 +167,12 @@ public ResponseEntity<Void> editStudyGroupVisibility(@AuthedUser User user,
studyGroupService.editStudyGroupVisibility(user, request);
return ResponseEntity.ok().build();
}

@GetMapping(value = "other-user-group_list")
@Operation(summary = "그룹 목록 조회 API", description = "방장 여부 상관 없이 유저가 참여하고 있는 그룹 모두 조회")
Copy link
Contributor

Choose a reason for hiding this comment

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

operation 설명이 list랑 완전히 동일한데, 수정해주세요


@GetMapping(value = "otheruser-info")
@Operation(summary = "타회원정보조회 API")
public ResponseEntity<UserInfoResponse> OtherUserInfo(@AuthedUser User user, @RequestParam Long targetUserId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

함수명은 동사형으로 써주세요

@sh0723
Copy link
Contributor Author

sh0723 commented Nov 13, 2024

피드백 다 반영했슴다~

@@ -127,4 +127,11 @@ public ResponseEntity<Void> checkNickname(@RequestParam String nickname) {
userService.checkNickname(nickname);
return ResponseEntity.ok().build();
}

@GetMapping(value = "otheruser-info")
Copy link
Contributor

Choose a reason for hiding this comment

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

얘도 PathVariable 사용하면 더 좋을 것 같아요-!

@@ -167,4 +167,12 @@ public ResponseEntity<Void> editStudyGroupVisibility(@AuthedUser User user,
studyGroupService.editStudyGroupVisibility(user, request);
return ResponseEntity.ok().build();
}

@GetMapping(value = "/{targetUserId}/list")
Copy link
Contributor

Choose a reason for hiding this comment

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

소사하지만 userId가 좋아보입니다!

@sh0723 sh0723 requested a review from rladmstn November 13, 2024 09:16
@sh0723
Copy link
Contributor Author

sh0723 commented Nov 13, 2024

다시 반영 해 놨슴다~!

Copy link
Contributor

@rladmstn rladmstn 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
Contributor

@hwangjokim hwangjokim left a comment

Choose a reason for hiding this comment

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

Conflict resolve 하고 머지해주세요~~

@sh0723 sh0723 merged commit ec158cc into develop Nov 14, 2024
1 check passed
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.

4 participants