-
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
Feat: 콜백 단건조회 api 응답값에 boolean isAssignedToSelf, String seniorPhoneNumber 추가 #104
Conversation
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.
테스트 코드가 꼼꼼하네요!
사용자 검증 로직에 대해서 궁금한 점이 있어서 리뷰남겨두었습니다:>
if (callback.getAssignedMemberId().equals(memberId)) { | ||
return new CallbackForSinittoResponse(callback.getId(), callback.getSeniorName(), callback.getPostTime(), callback.getStatus(), callback.getSeniorId(), true, callback.getSenior().getPhoneNumber()); | ||
} |
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.
음 저는 가이드라인 조회에서 사용자 검증할 때, 검증에 실패할 시 예외 처리하고, 성공할 시 response를 보내주는 로직으로 작성해두었는데,
혹시 검증에 실패하더라도 response에 검증여부(false)정보를 담아서 보내야하는 이유가 따로 있을까요?
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.
어라 그러네요; 한번 false 어디에 쓰시는지 물어보겠습니다. 생각해보니 검증 로직만 제대로하면 해결이 되는 문제일 수 도 있겠네요.
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.
저 페이지가 하나의 페이지로 구현하신거라서 이런 응답이 필요하다고 하시네요! true/false
있으면 기존보다 API 호출을 줄일 수 있어서 시니또가 수락하기 할 때 더 빨리 페이지가 갱신되는 효과가 있을거 같습니다.
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.
아래에 조금 더 자세한 설명으로 코멘트 달아놨지만 간단하게 보충하겠습니다!
콜백 상세 조회 페이지가 figma 상으로는 두 페이지(수락 전, 수락 후)지만 실제로는 같은 페이지내에서 현재 시니또가 콜백 요청을 수락했는지 아닌지에 따라 하단메뉴가 달라지는 방식으로 구현됐습니다.
이를 위해서는 현재 조회하고 있는 콜백 요청이 시니또가 수락한 것인지 아닌지에 대한 정보가 필요해 isAssignedToSelf 추가를 요청드렸습니다.
isAssignedToSelf가 false인 경우(현재 시니또가 수락하지 않은 콜백인 경우) 수락 전 페이지가 조회되고, true인 경우(현재 시니또가 수락한 콜백인 경우) 수락 후 페이지가 조회되도록 구현했습니다.
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.
고생하셨습니다 👍
프론트와 논의하신 부분에 대해 궁금한 점이 있어서 코멘트 남겼습니다
public CallbackResponse getCallback(Long callbackId) { | ||
public CallbackForSinittoResponse getCallbackForSinitto(Long memberId, Long callbackId) { | ||
|
||
Callback callback = callbackRepository.findById(callbackId) | ||
.orElseThrow(() -> new NotExistCallbackException("해당 콜백 id에 해당하는 콜백이 없습니다.")); | ||
.orElseThrow(() -> new NotFoundException("해당 콜백 id에 해당하는 콜백이 없습니다.")); | ||
|
||
return new CallbackResponse(callback.getId(), callback.getSeniorName(), callback.getPostTime(), callback.getStatus(), callback.getSeniorId()); | ||
if (callback.getAssignedMemberId().equals(memberId)) { | ||
return new CallbackForSinittoResponse(callback.getId(), callback.getSeniorName(), callback.getPostTime(), callback.getStatus(), callback.getSeniorId(), true, callback.getSenior().getPhoneNumber()); | ||
} | ||
|
||
return new CallbackForSinittoResponse(callback.getId(), callback.getSeniorName(), callback.getPostTime(), callback.getStatus(), callback.getSeniorId(), false, callback.getSenior().getPhoneNumber()); | ||
} | ||
|
||
} |
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.
-
콜백 단건 조회에 대해 해당 콜백을 수락한 시니또가 아님에도 콜백 조회는 가능하게 하고,
대신 프론트상에서 해당 콜백이 조회하고 있는 시니또에게 할당된건지 아닌지를 나타내는 UI를 보여주는 방식으로 유나님께서 구현하신다고 하신건가요? -
콜백 리스트에서는 상태가 대기중인 콜백만 떠서 단건 조회창으로 넘어가도 할당받은 시니또가 없어서 항상 false만 뜰 것 같고, 할당된 콜백은 콜백 리스트에 어차피 뜨지 않아서 단건 조회를 할 일이 없지 않을까요?
이외에 어떤 상황에서 단건 조회를 하는지 궁금합니다!
개인적인 생각으로는 지금 시니또 본인에게 할당된 콜백을 보는 페이지가 ui상에 없어서 프론트에서 해당 페이지를 만들고,
시니또 자신에게 할당된 콜백 리스트를 반환하는 api가 있으면 좋을 것 같습니다!
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.
1번 답
단건조회가 api 이름을 좀 잘못 지은거 같네요. "시니또가 본인이 진행중인 콜백 단건 조회"가 더 정확한 이름같습니다. 좀 혼란을 드린거 같아요!
유나님께 질문 드렸는데 아직 답이 안왔지만 제가 봤을때 이렇게 예상이 되긴한데... 프론트에 물어보고 또 코멘트 남기겠습니다. 진행상황 공유라고 생각해주시면 좋겠네요.!
2번 답
단건 조회 이게 only 시니또가 본인이 진행중인 콜백 단건 조회를 위한건데 위에 은선님 말씀처럼 true, false 말고 "예외"를 던져도 될거 같네요. 그래서 프론트랑 상의해볼 예정입니다.
이런상황에서 기존 단건 조회 API 가 검증(콜백 단건 API를 호출한 시니또가 이 단건 콜백에 할당되어있는지 아닌지)을 안해서 이걸 프론트 단에서 검증을 하니 프론트 페이지 로딩이 좀 오래 걸려서 true, false 추가를 요청하심.
유나님께 도움을 청하였습니다!
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.
유나님 말씀 들어보니 저 페이지가 하나의 페이지로 구현하신거라서 이런 응답이 필요하다고 하시네요! true/false
있으면 기존보다 API 호출을 줄일 수 있어서 시니또가 수락하기 할 때 더 빨리 페이지가 갱신되는 효과가 있을거 같습니다.
제가 화요일에 로딩이 느린것을 보고 이렇게 코드를 수정한거라 디테일한 부분을 잘 모르겠네요 죄송합니다..! 유나님께서 PR 한번 봐주신다고 하셔서 나중에 유나님이 좀 더 자세히 설명해주실거 같아요.
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.
프론트 관련해서 답변 조금 보충하겠습니다.
콜백 상세 조회는 해당 콜백을 수락하지 않은 시니또라도 조회할 수 있는 경우가 있습니다. 상세 조회가 가능한 경우를 정리해보면
- 콜백 요청이 WAITING 상태 일 때(수락한 시니또 없음): 현재 진행중인 콜백 요청이 없는 시니또는 모두 조회가 가능
- 콜백 요청이 IN_PROGRESS 상태 일 때: 해당 콜백 요청을 진행중인 시니또만 조회 가능
콜백 상세 조회(단건 조회) 페이지는 figma 상으로는 위 두 페이지가 있습니다. 두 페이지의 구성과 기능에 공통적인 부분이 많기도 하고 차이점은 하단의 메뉴(버튼과 전화번호 표시) 정도라 figma 상으로는 두 페이지지만 실제로는 같은 페이지내에서 현재 시니또가 콜백 요청을 수락했는지 아닌지에 따라 하단메뉴가 달라지는 방식으로 구현됐습니다.
이를 위해서는 현재 조회하고 있는 콜백 요청이 시니또가 수락한 것인지 아닌지에 대한 정보가 필요한데, 이 값이 없는 경우에는 프론트에서 단건 조회 api와 현재 시니또가 진행중인 콜백 api를 모두 호출해 결과값의 콜백 id를 비교해야합니다. 이 경우, 페이지가 로드될 때마다 2개의 api를 계속해서 호출해 비교해야하는 번거로움과 컴포넌트 갱신에 시간차가 있어 isAssignedToSelf 추가를 요청드렸습니다.
이 값이 true라면(현재 시니또가 수락한 콜백이라면) 오른쪽 페이지가, false라면(현재 시니또가 수락하지 않은 콜백이라면) 왼쪽 페이지가 조회됩니다.
질문에 대한 답변을 정리해보자면
- 콜백 단건 조회에 대해 해당 콜백을 수락한 시니또가 아님에도 콜백 조회는 가능하게 하고,
대신 프론트상에서 해당 콜백이 조회하고 있는 시니또에게 할당된건지 아닌지를 나타내는 UI를 보여주는 방식으로 유나님께서 구현하신다고 하신건가요?
상세 조회 페이지는 콜백 요청을 수락하지 않은 시니또와 해당 콜백을 수락한 시니또 모두 조회할 수 있는 페이지이며, 수락 상태에 따라 하단의 메뉴가 다르게 나타납니다.
- 콜백 리스트에서는 상태가 대기중인 콜백만 떠서 단건 조회창으로 넘어가도 할당받은 시니또가 없어서 항상 false만 뜰 것 같고, 할당된 콜백은 콜백 리스트에 어차피 뜨지 않아서 단건 조회를 할 일이 없지 않을까요?
이외에 어떤 상황에서 단건 조회를 하는지 궁금합니다!
1의 답변처럼 해당 콜백을 수락하지 않았더라도, 현재 진행중인 콜백이 없다면 콜백 상세 조회 페이지에 접근할 수 있습니다.
콜백 상세 조회 페이지의 접근 경로는 콜백 리스트에서 해당 콜백을 선택하는 방법과, 현재 진행중인 콜백요청이 있다면 홈 화면에서 현재 진행중인 콜백 요청으로 이동하는 버튼이 있습니다.
상태가 IN_PROGRESS인 콜백이더라도, 해당 콜백을 수락한 시니또는 조회할 수 있어야하기 때문에 조회한 시니또가 진행중인 콜백인지 아닌지에 대한 정보가 필요합니다!
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.
@JYN523 유나님 친절한 답변 감사드립니다 😃
제가 이해하기로는 콜백 상세페이지를 보는 사람이 '해당 콜백을 할당받은 시니또 or 해당 콜백을 할당받지 않은 시니또'일 때 페이지상 다른 부분이 시니어 번호가 나오냐/안나오냐 차이만 있다. 이를 이용하여 동일한 API를 호출하지만 필드 중 해당하는 boolean값에 의해 프론트에서 조건검증을 하고 시니어 번호 표시 여부를 결정한다. 라고 이해했는데 맞을까요?
동일한 API를 사용하여 처리하는건 프론트 코드 효율측면에서 좋아보여서 괜찮은 것 같습니다! 👍
다만 한가지 걱정되는것이 시니또의 콜백 할당 여부를 백에서 검증 후 프론트로 보내지만 시니어의 전화번호를 보여주냐 마냐의 검증은 프론트에서 서버로부터 전달받은 바디값을 통해 결정하는 것 같아 브라우저에서 검증 바디필드값을 임의로 false-> true로 변경한다면 시니어의 번호를 볼 수 있지 않을까 싶네요..!
'시니어 번호'가 서비스 측면상 예민하거나 보안상 중요한 정보인지 정도에 따라 다르긴 하겠지만 중요도가 높다면 표출여부를 프론트에서 서버로부터 전달받은 필드값을 이용해 조건처리하기 보다는 서버에서 API분리(두번호출x, 시니어번호까지 리턴하는 새로운 API)를 통해 처리하는게 보안상 좋지 않을까요?
물론 기능 구현 단계에서는 어느 방법이던 괜찮을 것 같습니다!! 추후에 한번 리팩토링 할 때 의논해보면 좋을 것 같아요 😃
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.
제가 이해하기로는 콜백 상세페이지를 보는 사람이 '해당 콜백을 할당받은 시니또 or 해당 콜백을 할당받지 않은 시니또'일 때 페이지상 다른 부분이 시니어 번호가 나오냐/안나오냐 차이만 있다. 이를 이용하여 동일한 API를 호출하지만 필드 중 해당하는 boolean값에 의해 프론트에서 조건검증을 하고 시니어 번호 표시 여부를 결정한다. 라고 이해했는데 맞을까요?
넵 맞습니다! 시니어 번호 뿐만 아니라 수락 전에는 "전화걸기 및 수락하기" 버튼이, 수락 후에는 "도움 해결"과"도움 포기" 버튼이 나온다는 차이점이 있습니다.
다만 한가지 걱정되는것이 시니또의 콜백 할당 여부를 백에서 검증 후 프론트로 보내지만 시니어의 전화번호를 보여주냐 마냐의 검증은 프론트에서 서버로부터 전달받은 바디값을 통해 결정하는 것 같아 브라우저에서 검증 바디필드값을 임의로 false-> true로 변경한다면 시니어의 번호를 볼 수 있지 않을까 싶네요..!
api 호출로 받은 값은 react query의 상태관리 시스템으로 관리되는데, 이는 브라우저 개발자 도구 등에서 직접 수정할 수 없습니다. 브라우저상에서 힙 메모리에 저장되고, 제가 따로 스토리지에 저장되도록 하지 않았기 때문에 사용자가 수정할 수 없습니다!
현재에는 true일 때만 전화번호를 보내고, false일 때는 빈 문자열을 보내는 방식이라 보안 쪽에서 크게 문제될 것 같지는 않지만 api를 분리하는 쪽이 조금 더 깔끔할 것 같긴 합니다. 지호님 말씀처럼 리팩토링 시에 다시 한 번 의논해보는게 좋을 것 같네요.
…p3/Team8_BE into Feat/issue-#102
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.
수고하셨습니다:>
return new CallbackForSinittoResponse(callback.getId(), callback.getSeniorName(), callback.getPostTime(), callback.getStatus(), callback.getSeniorId(), true, callback.getSenior().getPhoneNumber()); | ||
} | ||
|
||
return new CallbackForSinittoResponse(callback.getId(), callback.getSeniorName(), callback.getPostTime(), callback.getStatus(), callback.getSeniorId(), false, ""); |
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.
아~ 이해했습니다!
좋은 것 같네요 ㅎㅎ
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.
너무 늦게 홧인했네요 ㅠㅠ 죄송합니다
고생하셨어요~!
* Deploy: PR 생성시 테스트코드 수행 로직 구현 (#94) * deploy: PR 생성시 테스트코드 수행 로직 추가 * feat: jwt.secret키를 깃허브 시크릿에 저장 및 환경변수 설정 * feat: 환경변수 읽어오도록 수정 * feat: 도커를 사용하여 레디스 환경 구축 * Deploy: 깃허브 액션 최신버전으로 업그레이드 (#96) * deploy: deploy.yml 버전 업그레이드 * 깃허브 testcode.yml 버전 업그레이드 * deploy: 가장 최신 버전으로 업그레이드 * deploy: 가장 최신 버전으로 업그레이드 * Fix: 안부전화 Cascade 순환참조 문제 해결 및 관계성 오류 해결 구현 (#100) fix: 안부전화 Cascade 순환참조 문제 해결 및 관계성 오류 해결 * Refactor: 가이드라인 관련 로직 수정 {프론트 요청) (#103) * deploy: 가이드라인 더미데이터 추가 * feat: 콜백과 가이드라인의 시니어가 같지 않을 때 발생하는 예외 구현 * refactor: 타입별 전체 가이드라인 및 특정 가이드라인 조회 시 검증 로직 추가 * refactor: 가이드라인 조회 api 매개변수 수정 * refactor: Reformat code * refactor: 가이드라인 response dto에 id 추가 및 서비스단 수정 * refactor: 중복 주석 삭제 * refactor: 서비스단에서 사용하지 않는 레퍼지토리 선언 삭제 * fix: url 중복 오류해결 (#106) * Refactor: 예외 처리 관련 리팩토링 (#92) * refactor: 콜백 도메인 예외 처리 리팩토링 * refactor: Auth 도메인 예외 처리 리팩토링 * refactor: Guard 도메인 예외 처리 리팩토링 * refactor: GuardGuideline 도메인 예외 처리 리팩토링 * refactor: HelloCall 도메인 예외 처리 리팩토링 * fix: 리팩토링후 콜백 Test 코드 수정안하여 생긴 테스트 실패 수정 * refactor: Member 도메인 예외 처리 리팩토링 * refactor: Point 도메인 예외 처리 리팩토링 * refactor: Review 도메인 예외 처리 리팩토링 * refactor: Sinitto 도메인 예외 처리 리팩토링 * refactor: 사용안되는 exception class, advice class 제거, * refactor: HelloCallPriceService 에서 IllegalArgumentException -> BadRequestException * refactor: 포인트로그 Content 멘트 수정 * refactor: 포인트로그 Content 멘트 최종 수정 * refactor: MultiStatusException->ConflictException * refactor: TokenService의 응답코드 다양화 * refactor: 예외 클래스 이름 더 명확하게 수정 * Feat: 가이드라인 삭제 기능 추가 (#108) * refactor: id 속성 이름 Id로 수정 * feat: 서비스 레이어 가이드라인 삭제 메서드 추가 * feat: 컨트롤러 가이드라인 삭제 api 추가 * refactor: 사용하지 않는 가이드라인 예외클래스 삭제 * refactor: id 속성 코드컨벤션에 맞게 수정 * refactor: 삭제 api를 delete 요청으로 수정 * Feat: 콜백 단건조회 api 응답값에 boolean isAssignedToSelf, String seniorPhoneNumber 추가 (#104) * feat: 콜백 단건 조회 응답 요소 추가 * refactor: 더 명확한 메서드 명으로 수정 getCallback->getCallbackForSinitto * test: 시니또용 콜백 단건 조회 테스트 작성 * chore: 프론트 테스트를 더 명확히하기위해 콜백 더미 데이터에 할당된 시니어 추가 * refactor: 본인에게 할당된 콜백이 아닌것에대한 콜백 상세조회 할때는 시니어 번호 없도록 수정 * test: 테스트 통과하도록 수정 * chore: 더미데이터 개선 * Deploy: 더미데이터 실행 과정 중 initial()과 saveRefreshTokenToRedis() 메서드 실행을 별도의 트랜잭션으로 관리 (#113) deploy: 더미데이터 실행 별도의 트랜잭션에서 수행 * Deploy: 할당되는 콜백 더미 데이터에 각각 `callbackRepository.save(callback{id});` 추가 (#117) chore: 할당되는 콜백 더미 데이터에 각각 callbackRepository.save(callback?); 추가 * Feat: 시니또용 카테고리별 가이드라인 조회 api 추가 및 기존 api 삭제 (프론트 요청) (#114) * refactor: 시니또용 카테고리별 가이드라인 조회 메서드 추가 * refactor: 시니또용 카테고리별 가이드라인 조회 api 추가 * refactor: Swagger @tag 수정 * refactor: 시니또용 가이드라인 조회에서 기존 state 검증로직에 회원이 배정된 시니또인지 검증하는 로직 추가 * refactor: 서비스 레이어 회원 검증 로직 추가를 위해서 memberId 파라미터 추가 * refactor: Reformat Code * Deploy: 로컬과 개발 환경에 따라 카카오 리다이렉트 주소를 다르게 리턴하는 로직 구현 (#120) * feat: 카카오 devRedirectUri 추출 로직 추가 * feat: httpServletRequest를 통해 Referer 또는 Origin을 확인하여 다른 프론트 주소 리다이렉트 * 카카오 로그인 요청 헤더 추출 로직에서 NULL 예외처리 구현 (#122) fix: 카카오 로그인 요청 헤더 추출 로직에서 NULL 예외처리 추가 * Feat: 콜백 단건 조회 로직 개선 (#123) feat: 콜백 단건 조회 로직 개선 - 상태에 따른 검증 로직 추가 - 예외 처리 추가 - 가독성 향상 - 테스트 꼼꼼히 * 더미데이터로 로그인 기능 SSR로 구현 (#128) * feat: 저장 로직 초기세팅 * feat: DummyProperties 추가 * 비밀번호 및 오류 출력 로직 추가 * refactor: 코드 정렬 * feat: 이메일 목록에서 찾는 로직 추가, 에러 메시지 이후에도 목록 유지 * teat: findAllByEmailIn로직 테스트 코드 추가 * feat: css추가 * Feat: 시니또 회원 정보와 계좌 정보 조회 API 및 로직 분리 (#126) * Feat: 서비스 레이어 시니또 계좌정보 조회 메서드 추가 * refactor: 사용하지 않는 api와 관련 로직 삭제 * refactor: 시니어 정보 조회 api 전달 데이터 변경과 로직 수정 * refactor: 계좌정보 삭제 api 삭제 * Feat: Swagger 멘트 수정 * Refactor: SinittoRequest email 속성 삭제 및 Member 엔터티 update 메서드 수정 * Refactor: 중복 url 수정 * Refactor: GuardRequest에서 email 삭제 * Refactor: 계좌 정보가 존재하지 않으면 예외처리 대신 null을 담은 response 반환하도록 수정 * Refactor: Reformat Code * Feat: Member 정보 업데이트 테스트 추가 * Fix: 더미데이터 로그인 SSR input창 좌우여백 조정 (#130) fix: css 수정 --------- Co-authored-by: JIHO LEE <[email protected]> Co-authored-by: eunsoni <[email protected]>
#️⃣ 연관된 이슈
#102
📝 작업 내용
boolean isAssignedToSelf
가 없어서 단건 조회된 콜백이 조회를 요청한 시니또의 것인가 검증하는 작업을 프론트에서 진행하여 웹 페이지 UI 나타나는 속도가 느려지는 이슈가 발생String seniorPhoneNumber
추가된 것도 api 추가 호출을 막아서 웹 페이지 응답 향상을 위함스크린샷 (선택)
💬 리뷰 요구사항(선택)
⏰ 현재 버그
✏ Git Close
close #102