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

[FEAT] GET /room/code/:code 참여코드로 대기방 조회 API 구현 #5

Merged
merged 8 commits into from
Jan 11, 2022

Conversation

youngkwon02
Copy link
Member

@youngkwon02 youngkwon02 commented Jan 10, 2022

✅ Default Checklist

  • check branch

  • set Labels

  • set Reviewers


📕 Task

  • GET /room/code/:code 참여코드로 대기방 조회 api 작성 완료
  • 참여코드가 올바르지 않은 경우 처리 완료
  • 이미 습관 도전 중인 방인 경우 처리 완료
  • 사용자가 이미 참가중인 방인 경우 처리 완료

💡TODO

  • Access Token을 해석하여 DB에 존재하는 사용자인지 검사 필요

Comment on lines 4 to 6
* @error
* 1. 요청 권한이 없음 (회원가입 되지 않은 사용자이거나, 권한이 없는 회원)
* 2. 참여 코드가 전달되지 않음
Copy link
Contributor

Choose a reason for hiding this comment

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

  • 1번은 checkUser 미들웨어를 사용하면 자동으로 check가 될 것 같아서 없어도 될 것 같아요! (오류가 있다면 미들웨어단에서 걸러줌!)
  • 이미 kick된 유저인 경우는 대기방 참여 API에서 에러처리해주나요?!
  • [참여코드에 일치하는 방이 없는 경우 / 이미 시작된 습관방인 경우 / 이미 참여중인 방인 경우]도 error 케이스에 추가해줘도 좋을 것 같아요!

Copy link
Contributor

Choose a reason for hiding this comment

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

1번 체크유저 참고하겠습니다
2번의 경우 /room/:roomId/enter (습관방 참여) 에서 킥 유저 걸러주는 작업을 하는건지 저도 궁금하네요!
3번의 경우에도 설희와 같은 생각입니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

@xxeol2

  1. 네네!! 어제 미들웨어 코드 올려두신거 보니까 그렇게 될거같아요~~ 감사합니당 :)
  2. 저거는 기획단이랑도 얘기해볼 부분인거같은데, 조회할 때 막아주는게 더 좋을것도 같아요!! 작업해야겠네용
  3. 네!! status 400으로 바꾸고 error 케이스로 처리하겠습니다~~~

}

// 이미 해당 습관에 참여중인 사용자
if (userId == entries[i].userId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

비교연산자 == 가 좋을까요, ===가 좋을까요?! 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

비교해야하는게 같은 userId라서 === 을 안 써도 될거 같은데 혹시 변수 유형까지 고려해야할 케이스가 있을 수 있을까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋ오노,, js 패치가 덜됐네요 ㅋㅋㅋㅋㅋ ㅠㅠ
===이 좋을거 같아요!! 수정하겠습니다~~

Copy link
Member Author

Choose a reason for hiding this comment

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

@junghyun-jacky
일반적으로 userId는 숫자가 와서 비교 되어야 하는데, 엉뚱한 문자가 왔을 때, 타입체킹까지 하지 않는다면 우연히 조건문을 만족하는 경우가 발생할 수도 있겠죠~!
그래서 타입까지 확인하도록 처리하는게 안전해요~~

Comment on lines +49 to +58
const creator = await userDB.getUserById(client, room.creator);
const entries = await roomDB.getEntriesByRoomId(client, room.roomId);
const imageNum = 3;
let profileImgs = [];

for (let i = 0; i < entries.length; i++) {
if (i < imageNum) {
let user = await userDB.getUserById(client, entries[i].userId);
profileImgs.push(user.profileImg);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

getEntriesByRoomId 에서 user테이블을 조인해 profileImg를 가져오는 방법도 있을 것 같아요!
해당 방법의 장점은, DB에 접근하는 횟수를 줄여준다는 점?! (현재는 for문 안에서 DB에 접근을 함)
어떤 방법이 더 좋을지는 저도 잘 모르겠어서, 고민해보면 좋을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

@xxeol2
현재 코드에서는 Join연산이 나을것도 같은데,,, 다른 api에서 roomId로 entry 접근하는 경우가 되게 많을거로 예상되는데
getEntriesByroomId 내부로직이 간결한게 좋을거 같아서 이렇게 작성했습니당~
사실 모든 참여자의 사진이 필요한것도 아니구 해서...?
쫌 더 생각해볼게요~~~ 감사합니당 :0

Copy link
Contributor

@junghyun-jacky junghyun-jacky left a comment

Choose a reason for hiding this comment

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

좋아요~

Copy link
Contributor

@xxeol2 xxeol2 left a comment

Choose a reason for hiding this comment

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

LGTM ! 😀

@xxeol2 xxeol2 merged commit f4d7b19 into develop Jan 11, 2022
@xxeol2 xxeol2 deleted the feature/#3 branch January 11, 2022 07:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ New feature or request 🦋 영권
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants