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

Merged
merged 6 commits into from
Jun 15, 2024

Conversation

AlmondBreez3
Copy link
Member

@AlmondBreez3 AlmondBreez3 commented Jun 11, 2024

🌱 관련 이슈

📌 작업 내용 및 특이사항

  • 이미 기존에 존재하는 전체 회원 조회..이미 사용되고 있는 api이므로 일단 deprecated로 냅두고 /role?role=guest이런식으로 받도록 했습니다
  • updatePaymentStatus에 수정한 REGULAR로 바꾸는 코드는 일단 아직 제대로 된 결제 로직이 없으니까 임시로 만들어 두고 나중에 결제 쪽 할 때 바꾸시면 될듯합니다아
  • 간단한 테스트도 진행했습니다(repository에는 이미 guest, associate는 있어서 regular도 조회할 수 있는 테스트도 구현했습니다!

📝 참고사항

📚 기타

@AlmondBreez3 AlmondBreez3 self-assigned this Jun 11, 2024
@AlmondBreez3 AlmondBreez3 requested a review from a team as a code owner June 11, 2024 14:45
@AlmondBreez3 AlmondBreez3 changed the title feat: 상태별 회원 조회 목록 api구현 및 간단 repository test feat: 상태별 회원 조회 목록 api 구현 Jun 11, 2024
Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

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.

동일 api에서 멤버 역할만 검색 옵션으로 추가하면 될듯 한데 별도 api로 분리하신 이유가 있나요?

Copy link
Member

@Sangwook02 Sangwook02 left a comment

Choose a reason for hiding this comment

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

확인했습니다

@@ -42,6 +43,16 @@ public ResponseEntity<Page<AdminMemberResponse>> getMembers(MemberQueryOption qu
return ResponseEntity.ok().body(response);
}

@Operation(summary = "회원 상태별 목록 조회", description = "정회원, 준회원, 게스트별로 조회합니다.")
@GetMapping("/role")
Copy link
Member

Choose a reason for hiding this comment

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

기존 메서드가 admin 페이지에서 계속 사용되기는 합니다만,
쿼리 파라미터만 추가하면 영향을 주지 않고 수정이 가능할 것 같습니다.

member.verifyDiscord(DISCORD_USERNAME, NICKNAME);
member.verifyBevy();
member.advanceToAssociate();
member.updatePaymentStatus(VERIFIED);
Copy link
Member

Choose a reason for hiding this comment

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

이 메서드는 곧 사라질 예정이라 test에서 사용하지 않는게 좋을것 같습니다~

Copy link
Member Author

Choose a reason for hiding this comment

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

updatePayment말씀하시는거죠? 결제 쪽에서 이거 그대로 사용하실거라고 들어서..혹시 모르니까 그냥 이 테스트는 삭제할까요?

Copy link
Member

Choose a reason for hiding this comment

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

네 updatePaymentStatus 말씀드린거 맞아요
재현님께서 작업하신 내용이 이미 dev에 들어가있어서 conflict가 생겼네요
확인부탁드려요

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다 되고 그거 가져다가 작업하라고 하셔서 기다리고 있던 상태였습니당

@AlmondBreez3
Copy link
Member Author

동일 api에서 멤버 역할만 검색 옵션으로 추가하면 될듯 한데 별도 api로 분리하신 이유가 있나요?

아 그 이미 기존에 사용되던거라서 일단 건드리지 않으려고 한건데 밑에 상욱님이 남겨주신거보면 기존 api를 수정하는 방향으로 가져가도 될것같습니다
수정해서 다시 푸쉬하겠습니다.

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link
Member

@Sangwook02 Sangwook02 left a comment

Choose a reason for hiding this comment

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

확인했습니다!

@@ -35,10 +36,13 @@ public class AdminMemberController {

private final AdminMemberService adminMemberService;

@Operation(summary = "전체 회원 목록 조회", description = "전체 회원 목록을 조회합니다.")
@Operation(summary = "회원 상태별 목록 조회", description = "정회원, 준회원, 게스트별로 조회합니다.")
Copy link
Member

Choose a reason for hiding this comment

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

MemberRole도 있고 MemberStatus도 있으므로 아래와 같이 바꾸는 게 좋을 것 같습니다.
pr 제목도 같이 부탁드립니다~

Suggested change
@Operation(summary = "회원 상태별 목록 조회", description = "정회원, 준회원, 게스트별로 조회합니다.")
@Operation(summary = "회원 역할별 목록 조회", description = "정회원, 준회원, 게스트별로 조회합니다.")

Copy link
Member Author

Choose a reason for hiding this comment

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

앗 알겠습니다~~

member.verifyDiscord(DISCORD_USERNAME, NICKNAME);
member.verifyBevy();
member.advanceToAssociate();
member.updatePaymentStatus(VERIFIED);
Copy link
Member

Choose a reason for hiding this comment

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

네 updatePaymentStatus 말씀드린거 맞아요
재현님께서 작업하신 내용이 이미 dev에 들어가있어서 conflict가 생겼네요
확인부탁드려요

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

Copy link
Member

@Sangwook02 Sangwook02 left a comment

Choose a reason for hiding this comment

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

lgtm

/**
* deprecated
*/
public void updatePaymentStatus(RequirementStatus status) {
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.

이거 merge되면서 문제 생겼나봐요..!! 왜 저기에 들어갔는지 모르겠네요 지우겠습니다

Copy link

Job Summary for Gradle

Check Style and Test to Develop :: build-test
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
gdsc check 8.5 Build Scan published

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

@AlmondBreez3 AlmondBreez3 merged commit 00a7292 into develop Jun 15, 2024
1 check passed
@AlmondBreez3 AlmondBreez3 deleted the feature/344-admin-member-status branch June 26, 2024 12:52
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.

✨ Membership 어드민 회원 조회 API
3 participants