-
Notifications
You must be signed in to change notification settings - Fork 0
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
Refactor #89 UserUseCase 분리 및 리프레시 토큰 수정 #89
The head ref may contain hidden characters: "refactor/#74/UserUsecase-\uBD84\uB9AC"
Conversation
import leets.weeth.global.auth.jwt.application.dto.JwtDto; | ||
|
||
import java.util.List; | ||
import java.util.Map; | ||
|
||
import static leets.weeth.domain.user.application.dto.request.UserRequestDto.refreshRequest; | ||
|
||
public interface UserManageUseCase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생많으셨습니다 어드민 페이지에서 관리할 수 있는 정보(회원조회, 가입 승인, 유저 추방, 관리자 승격 + 강등, 다음 기수도 진행, 비번 초기화)들은 PR에 적어주셨듯이 어드민 기준이기 때문에 userManageUseCase라는 이름과 별도로 구성한 목적에 맞다고 판단이 드는데, 동아리 멤버 전체 조회라던지 내 정보와 관련된 부분도 userManageUseCase로 들어가는 것은 추가적인 고민이 필요해보입니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 조회같은 부분은 manage 말고 UserUseCase에 들어가야 한다고 생각합니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그러면 admin 관련한 메서드만 Manage에 넣도록 하겟습니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨어요!!
import leets.weeth.global.auth.jwt.application.dto.JwtDto; | ||
|
||
import java.util.List; | ||
import java.util.Map; | ||
|
||
import static leets.weeth.domain.user.application.dto.request.UserRequestDto.refreshRequest; | ||
|
||
public interface UserManageUseCase { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 조회같은 부분은 manage 말고 UserUseCase에 들어가야 한다고 생각합니다
확인했습니다 고생하셨어용 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
커멘트 하나 달았는데 확인해보시고 어떻게 생각하시는지 남겨주세요!
|
||
|
||
|
||
List<UserResponseDto.AdminResponse> findAllByAdmin(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사용자가 사용하는 findAllUser이랑 어떤 차이가 있는지 알수있을까요?
어드민이 사용할 수 있는 유저 조회라면 메소드명이 어드민을 조회한다고 생각할 수 있어서 findAllUsersForAdmin로 바꾸는건 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사용자는 현재 활성화된 유저들만 조회가 가능하고 어드민은 탈퇴, 대기 중인 유저 모두 조회가 가능합니당
By에 조건이 들어가는 것이니 어드민으로 조회한다 라는 의미가 명확한 것 같은데 어떠신가용
메서드 명이 자세한 것은 좋지만 너무 길어지면 읽는 것도 어렵고, 현재 메서드는 하위 계층이 아니라 상위 계층에 존재하는 상황이라 어느정도 추상화가 되어야한다고 생각해욤
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니다! 다른 메소드들도 다 괜찮은 위치에 있는거같네요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨어요!
PR 내용
PR 세부사항
관련 스크린샷
주의사항
X
체크 리스트