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

#784 송금하지 않은 방이 있는 경우 추가 방 참여 제한 #787

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

jinhyeonkwon
Copy link
Contributor

Summary

It closes #784

미정산 문제에 대한 대응 중 하나로, 다른 참여자가 정산을 올렸으나 본인이 송금하지 않은 방이 있다면 1) 추가 방 참여와 2) 새로운 방 생성을 제한합니다.

  1. 방 참여
  • 이미 참여 중인 방을 제외하고, 미송금 여부 확인을 최우선순위로 진행하여 송금하지 않은 방이 있다면 버튼을 비활성화하고 안내 텍스트가 보이게 합니다.
  1. 방 생성
  • 미송금 여부 확인을 최우선순위로 진행하여 송금하지 않은 방이 있다면 버튼을 비활성화하고 안내 텍스트가 보이게 합니다.

중점적으로 피드백 받고 싶은 부분

  1. InfoSection에서는, 정렬된 방 목록 (sortedMyRoom) 에서 혹시 정산 또는 결제 (이 PR은 정산이 진행 중이나 본인이 송금하지 않은 상황에 한정되므로 조건은 다릅니다) 가 이루어지지 않은 방이 있는지를 확인하는 notOver라는 변수가 있습니다. 이 과정을 최대한 보존하면서 아래와 같이 수정하였는데, 관련 질문이 있습니다.
  • sortedMyRoom 대신 정렬만 뺀 myOngoingRoom을 선언하여 사용했습니다. 이것이 적절한 사용인지 궁금합니다. 정렬을 하지 않을 것이라면 이렇게 선언하는 것이 불필요한 중복 변수 선언이 될 수 있는 여지가 있는지 궁금합니다.
  • notOver 대신, 정산 진행 중이나 본인이 송금하지 않은 방이 있는지 알기 위해서, 각 방에 대하여 part 안에서 본인 id와 일치하면서 isSettlement === "send-required" 인 방을 찾아 그 여부를 notPaid라는 변수에 저장하는 로직이 들어가 있습니다. 방 참여 쪽 수정 사항이 들어간 BodyRoomSelection에는 loginInfo를 이미 가져오고 있어서 그대로 사용했으나, 방 생성 쪽 수정 사항이 들어간 Addroom의 index.tsx에서는 새롭게 const loginInfo = useValueRecoilState("loginInfo"); 를 추가하였습니다. 문제가 되지 않을 것으로 판단했으나 적절한지 궁금합니다.
  1. notPaid를 설정하는 과정에서 any type으로 인해 VSCode에서 뜨는 에러를 없애기 위해 arrow function의 매개변수를 item: any 라는 형식으로 썼습니다. (이것을 명시하기 전, 오류가 뜨는 상황에서도 작동은 정상적으로 되었습니다) 이것이 적절한 방법이 아닌 것 같아서, 개선할 방법이 있는지 궁금합니다.

  2. 사소한 부분이나, 비활성화된 버튼의 텍스트가 적절한지 궁금합니다.

Images or Screenshots

image

image

Further Work

  • room을 다루는 작업을 하면서, 주변 분들과 함께 이야기하다가 프론트와 백이 공유하는 통합된 인터페이스가 있으면 좋겠다는 의견이 나왔습니다. TS migration이나 Zod 작업에 이미 들어있는지 잘 모르겠으나, 고려하면 좋을 것 같습니다.

Copy link

netlify bot commented May 14, 2024

Deploy Preview for taxi-dev-preview ready!

Name Link
🔨 Latest commit 2325074
🔍 Latest deploy log https://app.netlify.com/sites/taxi-dev-preview/deploys/668a1bc9c5b9b40008641495
😎 Deploy Preview https://deploy-preview-787--taxi-dev-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

room.part.find((item: any) => item._id === loginInfo?.oid)
.isSettlement === "send-required" && room.isDeparted
); // 다른 사람이 정산을 올렸으나 내가 아직 송금하지 않은 방이 있는지 여부 (추가 입장 제한에 사용)
// item : any 가 좋은 방법인지 모르겠습니다
Copy link
Member

Choose a reason for hiding this comment

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

이번 PR에서 방 타입 정의가 같이 이루어져도 괜찮을 것 같네요!

Copy link
Member

Choose a reason for hiding this comment

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

회의 논의 결과, 백엔드가 TS로 넘어가면 타입 공유를 해서 쓰는걸로 결정했습니다.

@@ -68,6 +69,14 @@ const AddRoom = () => {
useState<boolean>(false);
//#endregion

const myOngoingRoom = myRooms?.ongoing.slice() ?? []; // InfoSection의 sortedMyRoom에서 정렬만 뺐습니다.
Copy link

Choose a reason for hiding this comment

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

useMemo 사용 or loginInfo recoilState 에 추가하는게 좋을 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xMHW @kmc7468 useMemo로 변경했는데 확인해주시면 감사하겠습니다!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

송금하지 않은 방이 있는 경우 추가 방 참여 제한
3 participants