Feature/#170 관리자 관련 api 구현

[llddang]

관련 이슈

• close [BACK] 관리자 관련 API 구현 #170

작업 요약

[BACKEND]

• 이메일과 이름 입력을 통한 관리자 단일 등록 api 구현
• 이메일과 이름이 담긴 엑셀파일을 이용한 관리자 다중 등록 api 구현
• 슈퍼관리자만 가능한 관리자 삭제 api 구현

리뷰 요구 사항

• 리뷰 예상 시간: 20분
• 궁금하신 점이 있다면 언제나 물어봐주세요!
• 코드 스타일 잘못 된 것이 있다면 알려주세용!

[amaran-th]

@Transactional은 Class 단에 붙여주면 하위 메서드에 모두 적용이 됩니다!
그리고 QueryService는 조회만 하는 서비스 클래스이고 CommandService는 수정을 하기 위한 서비스 클래스이니 계정 등록 로직은 CommandService로 분리해서 작성해주시면 좋을 것 같아용

[llddang]

@Transactional을 Class 단에 붙여주지 않은 이유는 사용하지 않는 함수도 있어서 입니다.

file을 이용한 관리자 다중등록일 경우,
일부 실패에 대해 모든 사용자가 등록되지 않는 것보단 성공한 사용자는 등록이 되고 등록되지 않은 사용자의 line number를 알려주는 편이 UX적으로 편리할 것 같습니다.

클래스 명은 바꿨습니다!

[amaran-th]

null 객체를 사용하기보단 Optional 객체를 활용하는 편이 더 좋을 것 같습니다!
그리고 개인 취향이긴 한데 전 else if, else를 쓰는 것보다 메서드 분리를 통해 로직을 단순화 시키는 편을 선호합니당 이건 재량껏 반영해주세요!

    final Optional<Member> member = memberRepository.findByEmail(request.email());
    validateDuplicate(member);
    if(member.isPresent()){
        return saveExistFaculty(request.email(), request.name(), member, encodedPassword);
    }
    ...
}

private void validateDuplicate(Optional<Member> member){
    if (member != null && !member.isDeleted()) {
            throw new AdminAuthException(AdminAuthExceptionType.MEMBER_EMAIL_DUPLICATE);
    }
}

[amaran-th]

레파지토리에 접근하는 모든 서비스에는 @Transactional을 붙여주는 것이 좋습니다!

[amaran-th]

JPA에는 엔티티 조회 시 특정 조건에 해당되는 엔티티만 조회되도록 필터링하도록 설정할 수 있는 기능이 있습니다!
관련 글
이렇게 일일이 isDeleted()를 검증해주지 않아도 됩니당

[amaran-th]

그리고 그렇게 고친다면 아래처럼 로직을 수정하면 좋을 것 같아용

if(member!= null){
    throw new AuthException(AuthExceptionType.MEMBER_EMAIL_DUPLICATE);
}

[amaran-th]

가능한 예외 처리 구문은 앞에 두는 것이 로직 흐름을 읽기 편해서 권장되는 컨벤션입니다!

[llddang]

네 예외 구문이 바로 앞에 있을 경우 로직 흐름을 읽기 쉬워질 것 입니다.

if (memberDetail instanceof StudentMember studentMember) {
    // something...

} else if (memberDetail instanceof FacultyMember facultyMember) {
    // something...
}

이 위쪽 코드를 보면 위와 같이 경우에 따른 서로 다른 행동을 보이고 있습니다.
그리고 이에 해당하지 않는 구문이 else로 예외를 던지고 있는 형상입니다.

이럴 때 if (!memberDetail instanceof FacultyMember || !memberDetail instanceof StudentMember) 를 하고 밑에 똑같은 과정을 적는것보다 위의 방식이 좀더 효율적이라고 생각됩니다.

또한 예외구문을 던지는 것이 함수의 제일 마지막에 있기 때문에 모든 과정이 이루어지지 않았을 때 예외를 던지도록 되어있구나라고 판단할 수 있을 것이라 생각됩니다.
그리고 if 문 안의 내용일 길다면 예외를 위로 올리는 것이 좋을 것 같으나. 현재 if, else-if, 예외 구문이 14줄로 한 화면에 다 보여서 읽는데 무리가 없어보입니다.

[amaran-th]

오타가 있는 것 같습니다!

[amaran-th]

관리자를 제거한다고 수정하는 게 좋을 것 같습니다!

[amaran-th]

혹시 이 부분 왜 이렇게 설정하셨는지 알 수 있을까요?? 엔티티 id는 엔티티의 고유성을 결정하는 식별자라 함부로 수정하지 않는 것이 일반적이라서요~!(id가 다르면 다른 엔티티로 취급하기 때문에)

[llddang]

isDeleted 쪽에 코멘트를 달았던데요. id를 말하는 거겠죠?

AdminAuthQueryService의 saveFaculty 부분에서 사용했습니다.

Member member = new Member(email, name, password, "01000000000", false);

final long memberId = memberRepository.save(member).getId();
member.setId(memberId);

위 경우 새롭게 member를 만드는 것보다 id를 입력해주는 것이 낫겠다고 판단했습니다.