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 구현 #37

Merged
merged 22 commits into from
Feb 4, 2024

Conversation

Sangwook02
Copy link
Member

🌱 관련 이슈

📌 작업 내용 및 특이사항

  • 어드민 멤버 조회하기 api 구현
  • Querydsl과 Pageable로 검색 조건에 따른 결과 반환하도록 함
  • 응답 dto 구현

📚 기타

  • RepositoryImpl의 switch문을 가능하면 제거하고 싶었으나, 마땅한 방법이 떠오르지 않네요.
    좋은 방법 있으면 남겨주시면 감사하겠습니다!

@Sangwook02 Sangwook02 added the ✨ feature 새로운 기능 추가 및 수정 label Jan 31, 2024
@Sangwook02 Sangwook02 self-assigned this Jan 31, 2024
@Sangwook02 Sangwook02 requested a review from a team as a code owner January 31, 2024 17:27

private BooleanExpression queryOption(String keyword, String type) {
if (keyword != null && type != null) {
return switch (type) {
Copy link
Member

Choose a reason for hiding this comment

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

type 파라미터 처리를 레포지터리 단에서 하게 된다면 멤버 조회하기 API 스펙이 변경된는 경우 레포지터리 레이어가 직접적으로 영향을 받게 됩니다. API 파라미터와 SQL 쿼리 파라미터를 독립적으로 유지할 수 있도록 개선해보세요.

type 파라미터를 enum 타입으로 개선하는 방법을 고민해보거나,
서비스 레이어에서 종속성을 분리해주는 방법을 고민해보시면 좋을듯 합니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

service 단에서 String을 enum으로 바꿔 repository에 전달하도록 수정해봤습니다

Copy link
Member

@uwoobeat uwoobeat left a comment

Choose a reason for hiding this comment

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

좀 고민을 해봤는데 멤버 조회하기 파라미터는 정규화를 치지 않고,
type 파라미터 제거하고 각각의 파라미터를 받게 만들면 어떨까요?
지금 방식에서는 type 파라미터에 대한 의존성이 너무 커져서 별로인 것 같아요.

가령 이름을 검색하고 싶으면
type=name&keyword=이름 이 아니라,
name=이름 과 같이 쿼리하는 것이죠.

이렇게 하면 각각의 파라미터가 스키마를 가지게 되니까, 제약조건 걸기에도 좋을 것 같습니다.

가령 type=phone&keyword=010-1234-5678type=email&[email protected] 같은 경우
keyword의 포맷이 각각 다르기 때문에 검증하기가 어렵지만,
phone={phone}email={email} 같이 파라미터를 받는다면,
phone 파라미터의 값과 email 파라미터의 값 포맷을 깔끔하게 검증할 수 있을 것 같아요.

spotlight에 v2 API 만들어놨는데, 한번 보시고 의견 주세요.

@Sangwook02
Copy link
Member Author

Sangwook02 commented Feb 1, 2024

좀 고민을 해봤는데 멤버 조회하기 파라미터는 정규화를 치지 않고, type 파라미터 제거하고 각각의 파라미터를 받게 만들면 어떨까요? 지금 방식에서는 type 파라미터에 대한 의존성이 너무 커져서 별로인 것 같아요.

가령 이름을 검색하고 싶으면 type=name&keyword=이름 이 아니라, name=이름 과 같이 쿼리하는 것이죠.

이렇게 하면 각각의 파라미터가 스키마를 가지게 되니까, 제약조건 걸기에도 좋을 것 같습니다.

가령 type=phone&keyword=010-1234-5678type=email&[email protected] 같은 경우 keyword의 포맷이 각각 다르기 때문에 검증하기가 어렵지만, phone={phone}email={email} 같이 파라미터를 받는다면, phone 파라미터의 값과 email 파라미터의 값 포맷을 깔끔하게 검증할 수 있을 것 같아요.

spotlight에 v2 API 만들어놨는데, 한번 보시고 의견 주세요.

이렇게 하면 여러 조건을 걸수도 있겠네요.
좋은 것 같습니다!

+) stoplight v2에 type이랑 keyword가 남아있던데, 이 부분은 실수로 남기신거죠?
맞다면 제가 제거하겠습니다

Copy link
Member

@uwoobeat uwoobeat left a comment

Choose a reason for hiding this comment

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

lgtm

import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
Copy link
Member

Choose a reason for hiding this comment

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

@Transactional(readOnly = true) 클래스 레벨


import com.gdschongik.gdsc.domain.member.domain.Member;

public record AdminMemberFindAllResponse(
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public record AdminMemberFindAllResponse(
public record MemberFindAllResponse(

@Sangwook02 Sangwook02 merged commit a2fc43b into develop Feb 4, 2024
1 check passed
@Sangwook02 Sangwook02 deleted the feature/20-get-admin-member branch February 4, 2024 12:55
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.

✨ 어드민 멤버 조회하기 API 구현
3 participants