Refactor#99 멤버 조회 방식 변경 및 추가 #99

jj0526 · 2025-01-04T08:59:49Z

PR 내용

멤버 조회 방식 변경 및 추가 (이름 오름차순)

PR 세부사항

이름 오름차순으로 조회하되, 활동중, 승인 대기, 탈퇴, 추방을 베이스로 함
다음 브랜치에서 기수 분리 후 기수 순서대로 조회 예정

주의사항

없습니다

체크 리스트

huncozyboy

고생하셨습니다 !!

huncozyboy · 2025-01-05T07:58:13Z

src/main/java/leets/weeth/domain/user/domain/repository/UserRepository.java

@@ -20,4 +20,5 @@ public interface UserRepository extends JpaRepository<User, Long> {
    boolean existsByTelAndIdIsNot(String tel, Long id);

    List<User> findAllByStatusOrderByName(Status status);
+    List<User> findAllByOrderByNameAsc();


해당 부분이 현재는 단순히 이름으로 정렬된 모든 유저를 반환해주는 로직으로 이해해서 문제는 없을거같은데,
추후에 페이징(Pageable)을 적용해주는 방식도 고민해주셨으면 좋겠습니다

huncozyboy · 2025-01-05T08:00:08Z

src/main/java/leets/weeth/domain/user/application/usecase/UserManageUseCaseImpl.java

+                ACTIVE, 1,
+                WAITING, 2,
+                LEFT, 3,
+                BANNED, 4


우선순위를 하드코딩으로 해서 넣어주는 방식보다는, 추후 상태가 추가되거나 변경될 경우의 유지보수와 코드의 직관성을 고려해서
아래 내용처럼 Enum 내부에 우선순위를 정의하는 방식으로 개선해주는 건 어떨까요 ?

public enum Status {
ACTIVE(1),
WAITING(2),
LEFT(3),
BANNED(4);

좋네요!!

huncozyboy · 2025-01-05T08:03:01Z

src/main/java/leets/weeth/domain/user/presentation/UserAdminController.java

+        return CommonResponse.createSuccess(USER_FIND_ALL_SUCCESS.getMessage(), Map.of(0, usersByCardinal,
+                1, usersByName));


해당 내용도 추후에 기수 테이블을 생성하여 작업할 것을 가정했을때,
위 코멘트와 동일한 이유로
0 -> 기수별 데이터(usersByCardinal)
1 -> 이름순 데이터(usersByName)
을 나타낸다는 점을 숫자 대신 의미를 가진 Enum을 활용해준다면, 유지보수 측면에서 더 효과적일거 같다는 생각이 들었는데 어떻게 생각하시나요 ??

public enum UserDataType {
BY_CARDINAL, // 기수별 데이터
BY_NAME // 이름순 데이터
}

hyxklee

고생하셨습니다!
수정이 필요해보이는 부분들에 코멘트 남겼으니 읽어봐주시면 좋을 것 같아요!
그리고 스크린샷의 경우 해당 API의 특성에 맞는 스크린샷이 아니라고 생각합니다!
정렬된 것이 확인할 수 있게 조금 더 많은 데이터가 포함되게 수정해주시면 좋을 것 같아요!

hyxklee · 2025-01-05T09:30:28Z

src/main/java/leets/weeth/domain/user/presentation/UserAdminController.java

    }

    @PatchMapping
-    @Operation(summary = "가입 신청 승인")
+    @Operation(summary="가입 신청 승인")


해당 부분들은 제거해주시면 좋을 것 같아요!
원복이나 커밋 롤백으루

넵 알겠습니다!

hyxklee · 2025-01-05T09:34:16Z

src/main/java/leets/weeth/domain/user/application/usecase/UserManageUseCaseImpl.java

                .map(mapper::toAdminResponse)
-                .toList();
+                .collect(Collectors.toList());


굳이 toList()로 불변 리스트를 반환해주고 있던 것을 가변성을 추가해서 반환하는 이유가 있을까요?
그대로 반환해서 보여주는 역할이라 리스트에 대한 추가적인 작업은 필요 없을 것 같아서요!

hyxklee · 2025-01-05T09:36:03Z

src/main/java/leets/weeth/domain/user/presentation/UserAdminController.java

@@ -22,40 +25,45 @@ public class UserAdminController {

    @GetMapping("/all")
    @Operation(summary = "어드민용 회원 조회")
-    public CommonResponse<List<AdminResponse>> findAll() {
-        return CommonResponse.createSuccess(USER_FIND_ALL_SUCCESS.getMessage(), userManageUseCase.findAllByAdmin());
+    public CommonResponse<Map<Integer, List<AdminResponse>>> findAll() {


동적으로 정렬 방식을 선택할 수 있게 하려면 쿼리 파라미터로 정렬 방식을 입력받을 수 있어야할 것 같아요!
해당 부분까지 추가 작업 부탁 드릴게요

hyxklee · 2025-01-05T09:36:51Z

src/main/java/leets/weeth/domain/user/presentation/UserAdminController.java

+    public CommonResponse<Map<Integer, List<AdminResponse>>> findAll() {
+        // to do : 추후 기수 분리 후 작업 예정
+        List <AdminResponse> usersByCardinal = new ArrayList<>();
+        List<AdminResponse> usersByName = userManageUseCase.findAllByAdmin();


클라이언트로부터 정렬 방식을 입력을 받으면 유즈케이스 단에서 분기 처리를 해서 같은 리스트를 반환해주면 될 것 같아요!
굳이 컨트롤러 단까지 올라올 필요는 없을 것 같습니다

hyxklee · 2025-01-05T09:38:42Z

src/main/java/leets/weeth/domain/user/presentation/UserAdminController.java

+        List <AdminResponse> usersByCardinal = new ArrayList<>();
+        List<AdminResponse> usersByName = userManageUseCase.findAllByAdmin();
+
+        return CommonResponse.createSuccess(USER_FIND_ALL_SUCCESS.getMessage(), Map.of(0, usersByCardinal,


반환 형식이 기존 API를 따르지 않는 이유가 있을까요?
맵으로 반환을 해주는 이유가 궁금합니다.

요구사항 대로라면 기존 API 반환 형식을 따르고, 정렬 방식만 다르게 정렬해서 리스트를 그대로 반환해주면 될 것 같아서요!

그렇게 수정하겠습니다!

hyxklee · 2025-01-05T13:37:46Z

src/main/java/leets/weeth/domain/user/application/usecase/UserManageUseCaseImpl.java

+        if(orderBy == null || !EnumSet.allOf(UsersOrderBy.class).contains(orderBy)){
+            throw new InvalidUserOrderException();
+        }
+        if(orderBy.equals(NAME_ASCENDING)){


개행 한 줄 부탁드릴게요!

hyxklee · 2025-01-05T13:40:34Z

src/main/java/leets/weeth/domain/user/domain/entity/enums/UsersOrderBy.java

@@ -0,0 +1,6 @@
+package leets.weeth.domain.user.domain.entity.enums;
+
+public enum UsersOrderBy {


쿼리 파라미터의 경우 대문자로 입력 받기 보다는 소문자로 받아주는게 좋을 것 같아요!
URL에 노출되는 부분이라서

음.. enum을 소문자로 관리하기도 좀 그렇긴 한데.. 그냥 대문자로 두는게 나을 수도 잇겟네요

orderBy로 받고있습니다!

jj0526 added 2 commits January 3, 2025 01:57

feat: 이름 오름차순으로 유저의 status에 따라 순서대로 멤버 조회

aae7f5c

refactor: 어드민용 회원 조회를 맵핑으로 변경 / 추후 기수 분리 후 작업 예정

b5c5d24

jj0526 added the refactor label Jan 4, 2025

jj0526 requested review from hyxklee and huncozyboy January 4, 2025 08:59

jj0526 self-assigned this Jan 4, 2025

jj0526 linked an issue Jan 4, 2025 that may be closed by this pull request

Refactor #94 멤버 조회 방식 변경 및 추가 #94

Closed

fix: conflict 해결

fc328a4

huncozyboy reviewed Jan 5, 2025

View reviewed changes

hyxklee reviewed Jan 5, 2025

View reviewed changes

jj0526 added 5 commits January 5, 2025 21:52

feat: 올바르지 않은 유저 조회 순서, status의 예외 추가

d73d118

feat: status의 우선순위 지정

7df8bdc

feat: ENUM 타입인 UsersOrderBy 추가

dead581

refactor: 파라미터를 받아 어드민용 회원 조회 방식 개선

80edf05

refactor: 파라미터를 받아 어드민용 회원 조회 방식으로 변경

68f4e98

hyxklee reviewed Jan 5, 2025

View reviewed changes

jj0526 added 3 commits January 5, 2025 22:42

refactor: toList() 메소드 사용으로 코드 간소화

c2f06c2

style: 가독성 향상을 위해 개행 추가

d2d0dac

fix: conflict 해결

b1c45ee

hyxklee approved these changes Jan 5, 2025

View reviewed changes

jj0526 merged commit 4a1098d into develop Jan 5, 2025
2 checks passed

jj0526 changed the title ~~Refactor#94 멤버 조회 방식 변경 및 추가~~ Refactor#99 멤버 조회 방식 변경 및 추가 Jan 6, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor#99 멤버 조회 방식 변경 및 추가 #99

Refactor#99 멤버 조회 방식 변경 및 추가 #99

jj0526 commented Jan 4, 2025 •

edited by huncozyboy

Loading

huncozyboy left a comment

huncozyboy Jan 5, 2025

huncozyboy Jan 5, 2025 •

edited

Loading

jj0526 Jan 5, 2025

huncozyboy Jan 5, 2025 •

edited

Loading

hyxklee left a comment

hyxklee Jan 5, 2025

jj0526 Jan 5, 2025

hyxklee Jan 5, 2025

hyxklee Jan 5, 2025

hyxklee Jan 5, 2025

hyxklee Jan 5, 2025

jj0526 Jan 5, 2025

hyxklee Jan 5, 2025

hyxklee Jan 5, 2025

hyxklee Jan 5, 2025

jj0526 Jan 5, 2025

		return CommonResponse.createSuccess(USER_FIND_ALL_SUCCESS.getMessage(), Map.of(0, usersByCardinal,
		1, usersByName));

		@@ -0,0 +1,6 @@
		package leets.weeth.domain.user.domain.entity.enums;

		public enum UsersOrderBy {

Refactor#99 멤버 조회 방식 변경 및 추가 #99

Refactor#99 멤버 조회 방식 변경 및 추가 #99

Conversation

jj0526 commented Jan 4, 2025 • edited by huncozyboy Loading

PR 내용

PR 세부사항

관련 스크린샷

주의사항

체크 리스트

huncozyboy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huncozyboy Jan 5, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huncozyboy Jan 5, 2025 • edited Loading

Choose a reason for hiding this comment

hyxklee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jj0526 commented Jan 4, 2025 •

edited by huncozyboy

Loading

huncozyboy Jan 5, 2025 •

edited

Loading

huncozyboy Jan 5, 2025 •

edited

Loading