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

[YS-124] feat: 참여자 회원 정보 조회 API 구현 #31

Merged
merged 11 commits into from
Jan 14, 2025
Merged

Conversation

Ji-soo708
Copy link
Member

@Ji-soo708 Ji-soo708 commented Jan 14, 2025

💡 작업 내용

  • 참여자 회원 정보 조회 API를 구현했습니다

✅ 셀프 체크리스트

  • PR 제목을 형식에 맞게 작성했나요?
  • 브랜치 전략에 맞는 브랜치에 PR을 올리고 있나요?
  • 테스트는 잘 통과했나요?
  • 빌드에 성공했나요?
  • 본인을 assign 해주세요.
  • 해당 PR에 맞는 label을 붙여주세요.

🙋🏻‍ 확인해주세요

  • Restful 설계에 대해 코멘트를 작성했습니다. 해당 부분 같이 논의하면 좋을 거 같아요~ 👍

🔗 Jira 티켓


https://yappsocks.atlassian.net/browse/YS-124

@Ji-soo708 Ji-soo708 added the ✨ FEATURE 기능 추가 label Jan 14, 2025
@Ji-soo708 Ji-soo708 self-assigned this Jan 14, 2025
@github-actions github-actions bot changed the title feat: 참여자 회원 정보 조회 API 구현 [YS-124] feat: 참여자 회원 정보 조회 API 구현 Jan 14, 2025
Copy link
Member Author

@Ji-soo708 Ji-soo708 left a comment

Choose a reason for hiding this comment

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

이번 API 구현 과정에서, 수정님과 RESTful 설계의 범위에 대해 논의할 필요성이 느껴졌습니다. 🤔

코멘트에서 언급한 것처럼, 저는 REST를 엄격하게 지키는 것과 뷰 기반으로 제공하는 것 모두 장단점이 있다고 생각해요. 수정님의 의견을 듣고, 함께 방향을 정할 수 있으면 좋겠습니다. 미리 리뷰 감사드립니다~

Comment on lines 63 to 71
@PreAuthorize("hasRole('PARTICIPANT')")
@GetMapping("/participant/{memberId}")
@Operation(
summary = "참여자 회원 기본 정보 렌더링",
description = "참여자의 기본 정보를 반환합니다."
)
fun getParticipantInfo(
@PathVariable memberId: Long
): ParticipantInfoResponse {
Copy link
Member Author

@Ji-soo708 Ji-soo708 Jan 14, 2025

Choose a reason for hiding this comment

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

어느정도로 RESTful하게 URI를 설계할지 논의가 필요해보입니다!

REST는 자원을 중심으로 설계되며, 자원을 URI로 식별할 수 있어야 한다는 점에서, /participant/{memberId}와 같은 URI 설계는 해당 memberId를 가지는 특정 회원에 대한 정보 조회 작업을 요청하는 명확한 의미를 전달한다고 생각합니다.

이전에 수정님이 구현하신 실험자 회원 기본 정보 조회 API는 프론트에서 memberId를 전달하지 않고 서버에서 로그인한 회원의 정보를 추출해서 처리하기에 뷰 기반 형식에 더 가깝다고 생각했습니다. 두 API가 동일한 유형의 조회 API이기 때문에, 어느 방식이 더 적합할지에 대해 함께 논의하고 통일하면 좋을 것 같습니다!

개발 편의성이나 프론트엔드 측면에서 뷰 기반 접근 방식이 더 유리할 수도 있을 것 같아 어느정도로 REST하게 갈 지는 수정님의 의견도 들으면서 함께 정하고 싶어요~ 👋

Copy link
Member

@chock-cho chock-cho Jan 14, 2025

Choose a reason for hiding this comment

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

지수님께서 말씀하신 방식이 더 RESTful한 설계 원칙에 부합한다는 점에는 동의해요!
다만, 저는 이번 엔드포인트 설계 자체에 대해 약간 의문이 들었어요.

왜냐하면 '로그인한 사용자 자신의 기본 정보를 반환한다'는 점에서, 기존의 experiment-posts/{postId}처럼 특정 자원을 명시적으로 식별하는 맥락과는 조금 다르다고 생각했기 때문입니다.
participant/{memberId} 에서는 항상 memberId가 고정인 값이 될 테니까요.
이런 경우에는 엔드포인트에서 memberId를 명시하는 것보다는, 서버에서 로그인한 사용자의 정보를 인증 토큰을 통해 추출하는 로직이 더 적합하지 않을까 싶은 개인적인 생각이 듭니다!

지수님께서는 이에 대해 어떻게 생각하시는지 궁금해요! 😊

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는 마이페이지필터링을 위한 자동완성 기능에서 주로 쓰일 걸 생각하면 수정님이 말씀하신 거처럼 자동 추출하는 로직이 더 자연스러울 수 있겠네요!

이 부분은 반영해서 다시 수정하겠습니다. 👍

Comment on lines +8 to +24
@Schema(description = "주소 정보 반환 DTO")
data class AddressInfoResponse(
@Schema(description = "지역")
val region: Region,

@Schema(description = "지역 상세")
val area: Area
) {
companion object {
fun fromDomain(basicAddressInfo: Participant.AddressInfo): AddressInfoResponse {
return AddressInfoResponse(
region = basicAddressInfo.region,
area = basicAddressInfo.area
)
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

자주 사용하게 될 거 같아 별도의 DTO로 추출했습니다. 수정님도 이 방식이 낫다면 해당 PR에서 AddressInfo를 사용하는 API들은 이 DTO를 재사용하도록 리팩터링하겠습니다!

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 +8 to +27
@Schema(description = "공고 등록 시, 자동 입력에 필요한 연구자 정보 반환 DTO")
data class ParticipantInfoResponse(
@Schema(description = "사용자 기본 정보")
val memberInfo: MemberResponse,

@Schema(description = "성별")
val gender: GenderType,

@Schema(description = "생년월일")
val birthDate: LocalDate,

@Schema(description = "기본 주소 정보")
val basicAddressInfo: AddressInfoResponse,

@Schema(description = "추가 주소 정보")
val additionalAddressInfo: AddressInfoResponse?,

@Schema(description = "매칭 선호 타입")
val matchType: MatchType?,
)
Copy link
Member Author

Choose a reason for hiding this comment

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

현재 회원 정보 조회와 관련된 디자인이나 와이어프레임이 없어, 참여자의 모든 정보를 반환하고 있습니다. 추후 필요 없는 필드가 있다면, 그에 맞게 응답을 간소화하여 최적화해 나가는 것이 좋을 것 같습니다!

또한, 실험자의 경우에도 현재 참여자 응답 구조처럼 memberInfo를 추가하여 일관성 있게 처리한다면, API 응답 형식에서 통일성을 유지할 수 있을 거라 기대하는데 어떠신가요??

Copy link
Member Author

@Ji-soo708 Ji-soo708 Jan 14, 2025

Choose a reason for hiding this comment

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

추가적으로 수정님의 경우에는 매칭 선호 타입을 응답에서 반환할 때, preferType으로 하셨더라고요. 필드명만 봤을 때 matchType이나 preferredMatchType이 더 직관적으로 이해될 거라 기대하는데 이 부분에 대해 수정님의 의견을 듣고 싶습니다. 🤔

Copy link
Member

Choose a reason for hiding this comment

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

저는 처음에 '실험자'의 입장에서 '선호 타입'이라는 이름을 그대로 채택해서 preferType 으로 네이밍했었던 것 같은데요.
개발 진척도가 어느 정도 된 상태에서 보니 통일성을 위해 matchType 으로 채택하는 게 좋아보여요!

Copy link
Member Author

Choose a reason for hiding this comment

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

네 좋습니다! 아무래도 프론트에게는 통일된 응답 필드를 주어야해서 해당 PR에서 preferType이라 선언된 부분이 있다면 matchType으로 수정하겠습니다!

@Ji-soo708 Ji-soo708 requested a review from chock-cho January 14, 2025 05:52
Comment on lines +63 to +64
@PreAuthorize("hasRole('PARTICIPANT')")
@GetMapping("/participants/me")
Copy link
Member Author

Choose a reason for hiding this comment

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

해당 회원의 기본 정보를 보는 URI는 이렇게 명하는 거 어떠신지 궁금합니다!

@Ji-soo708 Ji-soo708 requested review from chock-cho and removed request for chock-cho January 14, 2025 12:24
chock-cho
chock-cho previously approved these changes Jan 14, 2025
Copy link
Member

@chock-cho chock-cho left a comment

Choose a reason for hiding this comment

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

LGTM!
개인적으로 지수님과 나누는 코드 리뷰가 고민해볼 만한 주제들이라 엄청 유익한 시간인 것 같습니다 😊
수고 많으셨습니다.

@Ji-soo708 Ji-soo708 merged commit 091678d into dev Jan 14, 2025
2 checks passed
@Ji-soo708 Ji-soo708 deleted the feat/YS-124 branch January 14, 2025 13:01
Ji-soo708 added a commit that referenced this pull request Jan 26, 2025
* feat: add ParticipantInfoResponse

* feat: add fromDomain and toDomain to ParticipantEntity

* feat: add findByMemberId to ParticipantRepository

* feat: add ParticipantException

* refact: delete unused import line

* feat: add GetParticipantInfoUseCase logic

* test: add GetParticipantInfoUseCase Test

* refact: delete memberId PathVariable in controller

* refact: rename preferType to matchType for consistency

* refact: refact getMemberInfo API's URI

* refact: refactor request, response dto
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ FEATURE 기능 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants