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

시니또용 콜백 페이지 api 및 기능 구현 #88

Merged
merged 8 commits into from
Oct 18, 2024
Merged

Conversation

JYN523
Copy link
Collaborator

@JYN523 JYN523 commented Oct 17, 2024

#️⃣ 연관된 이슈

ex) #이슈번호, - #이슈번호

📝 작업 내용

이번 PR에서 작업한 내용을 간략히 설명해주세요.(이미지 첨부 가능)

  • 시니또용 콜백 리스트 페이지 api 추가 및 무한 스크롤 구현
  • 시니또 콜백 요청 수락/취소/완료 api 추가
  • 시니또용 가이드라인 조회 페이지 api 추가

스크린샷 (선택)

💬 리뷰 요구사항(선택)

리뷰어가 특별히 봐주었으면 하는 부분이 있다면 작성해주세요

ex) 메서드 XXX의 이름을 더 잘 짓고 싶은데 혹시 좋은 명칭이 있을까요?

  1. 시니또가 콜백 상세 페이지에서 가이드라인을 조회할 때 시니어id를 어떻게 전달할 지 고민입니다.
  • 기존 시니또의 가이드라인 조회 주소는 '/call-back/{콜백id}/{가이드라인id}'

  • 가이드라인 조회 시 시니어id가 필요해서 전달해야함.

    a. URL 파라미터(/call-back/{콜백id}/{시니어id}/{가이드라인id}) > 시니어id 노출됨. 사용자가 임의로 URL을 입력 시 콜백 요청에 해당하는 시니어가 아닌 다른 시니어의 가이드라인이 조회될 수 있음.
    b. 로컬 또는 세션 스토리지 > 해당 콜백요청의 시니어 id가 저장되고, 다른 페이지로 이동 후 콜백 가이드라인 조회 페이지에 접속(임의로 주소 직접 입력)하면 다른 콜백요청이더라도 이전에 저장된 시니어Id로 조회되는 문제 발생 (= 콜백 요청에 해당하는 시니어가 아닌 다른 시니어의 가이드라인이 조회될 수 있음)
    c. 콜백 요청 api를 호출해 받은 시니어id로 가이드라인 api 호출 > 이전 페이지(콜백 요청 조회 페이지)에서도 이미 콜백 요청 api를 호출하는데, 가이드라인 페이지에서도 또 호출하게 됨.

    우선은 c.(콜백 요청 api 호출 이후 가이드라인 api 호출)로 구현 했는데, 괜찮을까요? 혹은 더 좋은 방법이 있을까요? (/pages/sinitto/guide-line/SinittoGuideLinePage.ts 참고)

(2,3번은 api 수정에 대한 내용이라 코드리뷰 시 해당 사항 참고부탁드립니다!)

  1. 콜백 요청 조회 api (/api/callbacks/{callbackId})에 전화번호 추가 필요함. (현재는 response에 전화번호가 없어서 임의로 넣어놨습니다)

  2. 콜백 요청 상세 페이지에서 해당 콜백 요청이 시니또가 진행중인 콜백 요청인지 확인하는 방법이 시니또에게 할당된 콜백조회 api (/api/callbacks/sinitto/accepted)로 할당된 콜백를 받아와 현재 페이지의 콜백id를 비교하는 방법 밖에 없는 것 같은데, 콜백 단건 조회 api (/api/callbacks/{callbackId})에서 현재 시니또에게 할당된 것인지 아닌지를 추가하면 좋을 것 같습니다. (현재는 할당된 콜백을 받아와 비교하는 방법으로 구현)

⏰ 현재 버그

✏ Git Close

close #이슈번호

close #58

@JYN523 JYN523 added ✨ Feature 새로운 기능 추가 및 구현하는 경우 📡 API 비동기 통신 코드를 짜는 경우, 백엔드와의 통신하는 경우 labels Oct 17, 2024
@JYN523 JYN523 requested review from Diwoni and Dobbymin October 17, 2024 19:52
@JYN523 JYN523 self-assigned this Oct 17, 2024
@JYN523 JYN523 linked an issue Oct 17, 2024 that may be closed by this pull request
2 tasks
Copy link
Contributor

@Dobbymin Dobbymin left a comment

Choose a reason for hiding this comment

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

바쁘셨을텐데 밤새 고생많으셨습니다..
api 호출 관련 코드에서 어떤 코드는 hook이랑 같이 작성하고, 어떤 코드는 hook이랑 분리해놓아서, 이 부분 조금 통일해서 수정해야할 것 같습니다.
그리고 저희 router 경로 부분은 상수로 만들어 놓은걸로 알고있어서 경로 부분은 상수 경로로 수정해주시면 감사하겠습니다!

@JYN523 JYN523 requested a review from Dobbymin October 18, 2024 03:08
@Diwoni
Copy link
Collaborator

Diwoni commented Oct 18, 2024

가이드라인 조회 시 시니어id가 필요해서 전달해야함.
c. 콜백 요청 api를 호출해 받은 시니어id로 가이드라인 api 호출 > 이전 페이지(콜백 요청 조회 페이지)에서도 이미 콜백 요청 api를 호출하는데, 가이드라인 페이지에서도 또 호출하게 됨.

제 생각에는 나중에 상태 관리 라이브러리로 콜백 요청 시 받은 시니어 id 를 관리해서 가이드라인 페이지에서의 API 재호출을 방지하는 방법도 괜찮을 것 같아요.

고생 많으셨습니다!

@JYN523
Copy link
Collaborator Author

JYN523 commented Oct 18, 2024

가이드라인 조회 시 시니어id가 필요해서 전달해야함.
c. 콜백 요청 api를 호출해 받은 시니어id로 가이드라인 api 호출 > 이전 페이지(콜백 요청 조회 페이지)에서도 이미 콜백 요청 api를 호출하는데, 가이드라인 페이지에서도 또 호출하게 됨.

제 생각에는 나중에 상태 관리 라이브러리로 콜백 요청 시 받은 시니어 id 를 관리해서 가이드라인 페이지에서의 API 재호출을 방지하는 방법도 괜찮을 것 같아요.

고생 많으셨습니다!

전역 상태 관리 라이브러리도 생각해보긴 했는데, a,b처럼 콜백 요청에 해당하는 시니어가 아닌 다른 시니어의 가이드라인이 조회되는 문제가 있을거라고 생각했습니다. a(URI), b(스토리지) 방법과 전역 상태 관리 방법에서 시니어id를 저장하면 현재 콜백 요청에 해당하는 시니어인지 아닌지 검증하는 과정이 없다고 생각해 c(API 재호출) 방법으로 구현했는데요.

지금 다시 생각해보니 콜백id와 시니어id를 모두 관리해서 현재 URI의 콜백id와 비교하는 방식으로 구현하면 될 것 같습니다! 전역 관리로 수정하는 쪽으로 진행할게요.

This was referenced Oct 18, 2024
Copy link
Contributor

@Dobbymin Dobbymin left a comment

Choose a reason for hiding this comment

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

음.. 리뷰에 내용이 있는걸 방금 알았네요..
시니어 아이디가 uri에 노출되는게 큰 문제가 있나요...? 유나님이 생각한 예시를 보니까 스토리지(로컬 or 세션)에 넣으면 뺴기 전까지 상태가 그대로 유지되기 때문에 조금 문제가 생기는 것 같은데, 저는 uri로 넘기는게 맞다고 생각합니다.
사실 제가 전역상태관리 라이브러리로 zustand를 쓰자고 제안했긴 한데, 요즘 또 공부하면서 드는 생각이 '굳이 전역 상태를 외부 라이브러리까지 사용하면서 구현할 이유가 있나? 필요하면 context API 쓰는게..?'
그리고 전역 상태관리를 사용하면 새로고침할때 정보가 다 날라가는데 이때 시니어 id가 날라가면 조금 문제가 생길것 같긴 하네요..
전역 상태관리는 '컴포넌트와 컴포넌트 사이가 멀때 사용하자!' 라는 생각을 가지고 있긴 한데, 누군가가 '이거 전역으로 왜 관리했어요?' 라고 물어봤을 때 타당한 이유와 함께 본인이 생각하는 것을 전달하면 전역 상태관리를 굳이 써도 상관없긴 할 것 같습니다!
아니면 백엔드 분들한테 시니어 id나 콜백 id만 받아올 수 있는 api를 만들어 달라고 요청하고, 그들의 생각을 들어보는 것도 좋은 방법일 것 같네요!
유나님이 적절하게 판단해서 구현하실꺼라 믿고 Approve 해놓겠습니다!

@JYN523
Copy link
Collaborator Author

JYN523 commented Oct 18, 2024

시니어id가 단순 숫자다 보니 노출되어도 큰 문제는 없을 것 같긴하네요.
uri로 넘기면 생기는 문제점은 사용자가 임의로 주소를 입력해 접근했을 때, 콜백 요청에 해당하지 않는 시니어id를 입력해도 가이드라인이 조회가 가능하다는 것이라고 생각합니다. 콜백 조회api를 재호출해서 검증하거나 아예 api를 수정해야 해결할 수 있을 것 같아, 콜백 조회api를 재호출해서 검증하는 방식으로 구현했습니다.

api를 수정하지 않는 한 어떤 방법이든 시니어id가 콜백 요청에 해당하는지 아닌지에 대한 검증 과정은 필요하다고 생각해요. 그렇다면 가이드라인 조회 페이지에서 콜백 요청을 불러오긴 해야하는데, 중복 호출이 너무 많이 발생할 것 같아서 이를 방지하고자 전역 상태 관리로 콜백id와 시니어id를 관리하는 방식을 생각해봤습니다.

페이지 이동이나 새로고침 시 날라가는 문제는 context를 스토리지에 저장하면 유지될텐데 지금 생각해보니 이건 그냥 스토리지에 저장하는 것과 관리말고는 별 차이가 없는 것 같네요. 그리고 스토리지에 저장하면 무결성을 완전히 보장할 수는 없으니 결국 검증이 다시 필요할 것 같네요... 검증을 하기 위해 콜백 조회 api를 호출하면 결국 시니어id를 받아오니 따로 시니어id를 넘기거나 저장할 필요가 없어지는 것 같습니다.

계속 생각해봤는데, api를 수정하거나 추가하는 쪽이 깔끔할 것 같습니다.

  1. uri에 시니어id까지 추가 > 가이드라인 조회 시 콜백id를 추가로 받아서 검증까지 해주는 api
  2. 현재 uri 유지(콜백id, 가이드라인id만) > 가이드라인 조회 시 콜백id와 가이드라인id를 받아 콜백에 해당하는 시니어의 가이드라인을 반환하는 api

이건 백엔드쪽과 상의 후에 수정하겠습니다.

일단 중복호출 되더라도 재호출하는 기존 방식으로 구현한 것으로 merge 하겠습니다. 이쪽 관련 수정사항은 새로 issue 만들어서 진행하겠습니다!

@JYN523 JYN523 merged commit 73aef1c into Weekly Oct 18, 2024
1 check passed
@Dobbymin Dobbymin deleted the Feat/issue-#58 branch October 19, 2024 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📡 API 비동기 통신 코드를 짜는 경우, 백엔드와의 통신하는 경우 ✨ Feature 새로운 기능 추가 및 구현하는 경우
Projects
None yet
Development

Successfully merging this pull request may close these issues.

시니또용 콜백 페이지 기능 구현
3 participants