[FIX] 방장이 자신의 방이 아닌 다른 방의 방장 권한을 얻게 되는 버그 수정 #421

novice0840 · 2024-12-03T02:50:59Z

Issue Number

#404

As-Is

A 방의 방장이 B 방에 접속시 방장 권한을 얻는 문제 발생

To-Be

JWT에 담긴 정보에서의 방 번호와 현재 접속한 방 번호를 비교해 일치하지 않는 경우 방장 권한을 주지 않는 방식으로 변경

Check List

테스트가 전부 통과되었나요?
모든 commit이 push 되었나요?
merge할 branch를 확인했나요?
Assignee를 지정했나요?
Label을 지정했나요?

Test Screenshot

(Optional) Additional Description

방장 권한 확인 시 현재 URL를 확인하는 로직이 추가되어 테스트가 깨지는 문제가 발생 (테스트 환경에서는 URL에 방 번호가 없으므로)
useIsMaster를 mocking하는 방식으로 테스트 코드를 변경함

🌸 Storybook 배포 주소

https://woowacourse-teams.github.io/2024-ddangkong/storybook/

🌸 Storybook 배포 주소

https://woowacourse-teams.github.io/2024-ddangkong/storybook/

rbgksqkr

고생하셨습니다 폼에~! 저도 이 버그를 들었었는데 그때 경황이 없어 제대로 확인 못했어요ㅜ 깔끔하게 해결해주셨네요:)
파일 체인지가 적어서 파악하는 데에는 어려움이 없었는데, PR 제목이랑 description을 좀더 구체적으로 적어주시면 수월하게 리뷰할 수 있을 것 같아요~! 의견 몇가지 남겨봤습니다! 답변 부탁드려용

rbgksqkr · 2024-12-03T12:32:22Z

frontend/src/utils/test-utils.tsx

@@ -73,22 +74,4 @@ const customRender = (ui: React.ReactNode, options: CustomRenderOptions = {}) =>
  });
 };

-const customRenderWithMaster = (Component: React.ReactNode) => {


🌸 칭찬 🌸

방장 훅을 분리하면서 이런 커스텀 함수 없이도 방장 여부가 명확해졌네용

rbgksqkr · 2024-12-03T12:37:12Z

frontend/src/hooks/useGetUserInfo.ts

@@ -19,7 +22,7 @@ const useGetUserInfo = (): RoomAndMember => {
    member: {
      memberId: data?.member?.memberId || 0,
      nickname: data?.member?.nickname || '',
-      isMaster: data?.member?.isMaster || false,
+      isMaster: Boolean(data?.member?.isMaster && Number(roomId) === data?.roomId),


🙏 제안 🙏

member는 optional값이 아닌 것 같은데 옵셔널 체이닝을 없애도 괜찮을 것 같아요!

수정하였습니다!

rbgksqkr · 2024-12-03T12:43:47Z

frontend/src/components/layout/Header/Header.test.tsx

+import useIsMaster from '@/hooks/useIsMaster';
+import { customRender } from '@/utils/test-utils';
+
+jest.mock('@/hooks/useIsMaster');


💭 질문 💭

API 응답값으로 isMaster를 판단했는데, useParams로 url을 판단하는 로직이 추가되어 master에 대한 테스트가 꼬였나보네요ㅜ

useParams를 모킹하는 것보다 useIsMaster이라는 커스텀훅을 따로 만들고 모킹한 이유가 궁금합니다!
useParams의 roomId만 모킹해서 msw 응답값이랑 맞추면 테스트 코드를 건드는 작업이 더 줄어들 것으로 생각이 드는데, 어떻게 생각하시나용

useIsMaster을 추가한 이유가 mocking 뿐만 아니라 컴포넌트에서 해당 hook 로직을 단순화하게 위해 만든 부분도 있습니다.

기존에는
const {member: { isMaster } } = useGetUserInfo()
이런식으로 가져와 useGetUserInfo의 내부 데이터 중 isMaster 하나만 있으면 되는데 불필요하게 유저 정보의 전체 데이터를 다 가져오게 되더라고요. 그래서 로직이 다소 불명확하다고 생각해서 레이어를 하나 더 추가해

const isMaster = useIsMaster()
코드를 더 깔끔하게 변경하였습니다

rbgksqkr

코멘트 확인했습니다!! 이유가 명확해서 좋네용 useIsMaster 커스텀훅을 통해 좀더 명확하게 방장 여부를 가져오고 + 반환값을 모킹하여 방장 로직 테스트가 가능해졌군요! 다시 생각해보니 useParams를 모킹하는 것보다 더 직관적이라는 생각도 드네요 굿굿 고생하셨습니다 approve할게요!!

useon

안뇽하십니까 폼에 ~ !!! 큰일 날 수도 있는 버그를 빠르고 깔끔하게 고쳐줘서 감사합니다 👏👏 !!!! 불필요한 코드들이 많이 줄어들어서 너무 좋네요 !!! 머지 전에 충돌이 나는 부분이 있어서 해결 부탁 드립니다 ~ ^.^ !!!

useon · 2024-12-06T18:08:19Z

frontend/src/hooks/useGetUserInfo.ts

@@ -19,7 +22,7 @@ const useGetUserInfo = (): RoomAndMember => {
    member: {
      memberId: data?.member?.memberId || 0,
      nickname: data?.member?.nickname || '',
-      isMaster: data?.member?.isMaster || false,
+      isMaster: Boolean(data?.member?.isMaster && Number(roomId) === data?.roomId),


💭 질문 💭

Number(roomId) === data?.roomId
이 부분이 추가되면서 현재 방의 방장인 경우에만 방장 권한을 가질 수 있게 된 건가요?!

useon · 2024-12-06T18:09:45Z

frontend/src/hooks/useIsMaster.ts

+  const {
+    member: { isMaster },
+  } = useGetUserInfo();
+  return isMaster;


🌸 칭찬 🌸

isMaster만 필요한데 사용자 정보 전부 불러와야 했던 것을 분리했군요 굿굿 !!!

useon · 2024-12-06T18:11:23Z

frontend/src/pages/RoundResultPage/components/NextRoundButton/NextRoundButton.tsx


 import AlertModal from '@/components/AlertModal/AlertModal';
+import Button from '@/components/common/Button/Button';
+import { bottomButtonLayout } from '@/components/common/Button/Button.styled';


🌸 칭찬 🌸

이런 세세한 부분도 챙겨서 바꿔주신 디테일 ,, 머야머야 👍👍👍

useon · 2024-12-06T18:13:52Z

frontend/src/pages/GameResultPage/components/GameResult/FinalButton/FinalButton.test.tsx

    const user = userEvent.setup();
+    (useIsMaster as jest.Mock).mockReturnValue(true);


💭 질문 💭

이렇게 하면 useIsMaster의 리턴 값을 true가 되도록 설정하는 건가요?

rbgksqkr · 2024-12-06T19:05:20Z

지금 생각났는데 자신의 userId와 다른 방에 접속하면 메인 화면으로 보내는 건 어떨까요?

Number(roomId) !== data?.roomId 일 때 navigate('/', {replace: true}); 처럼요!

novice0840 · 2024-12-13T05:14:34Z

지금 생각났는데 자신의 userId와 다른 방에 접속하면 메인 화면으로 보내는 건 어떨까요?

Number(roomId) !== data?.roomId 일 때 navigate('/', {replace: true}); 처럼요!

좋을 것 같긴한데 그럼 한 번 방장이 된 후에 다른 사람에게 초대를 받아 다른 방을 들어간 경우에 방 접속이 안되지 않을까요?

rbgksqkr · 2024-12-13T05:48:13Z

새로 들어가면 쿠키 갱신이 되서 방장이였던 방에선 안되지만 새로 들어간 방에선 될 것 같아요!!! 한번 확인해주시고 반영하면 좋을 것 같습니당

fix: 방이 바뀌면 방장 권한 해체 #404

a834cd2

novice0840 added 🛠 fix 버그 🫧 FE front end labels Dec 3, 2024

novice0840 requested review from rbgksqkr and useon December 3, 2024 02:51

novice0840 self-assigned this Dec 3, 2024

novice0840 linked an issue Dec 3, 2024 that may be closed by this pull request

[FIX] 방장 권한 버그 #404

Closed

1 task

novice0840 added 4 commits December 3, 2024 12:52

refactor: useIsMaster hook 추가 #404

016fc59

refactor: 긴 상대 경로 import 절대 경로로 변경 #404

ddb3bee

refactor: useIsMaster mock test 추가 #404

64ecc8c

fix: conflict 해결 #404

774ee96

rbgksqkr removed the D-0 이제 그냥 머지합니다? label Dec 3, 2024

github-actions bot added the D-3 3일 안에 리뷰 부탁드려요:) label Dec 3, 2024

rbgksqkr reviewed Dec 3, 2024

View reviewed changes

fix: 불필요한 optional chaining 제거 #404

385117b

novice0840 changed the title ~~[FIX] 방장 권한 버그~~ [FIX] 방장이 자신의 방이 아닌 다른 방의 방장 권한을 얻게 되는 버그 수정 Dec 4, 2024

github-actions bot added D-2 2일 안에 리뷰 부탁드려요:) and removed D-3 3일 안에 리뷰 부탁드려요:) labels Dec 4, 2024

rbgksqkr approved these changes Dec 4, 2024

View reviewed changes

useon added D-1 1일 안에 리뷰 부탁드려요:) and removed D-2 2일 안에 리뷰 부탁드려요:) labels Dec 5, 2024

github-actions bot added D-0 이제 그냥 머지합니다? and removed D-1 1일 안에 리뷰 부탁드려요:) labels Dec 5, 2024

useon requested changes Dec 6, 2024

View reviewed changes

fix: conflict 해결 #404

49de438

novice0840 merged commit 7e136f7 into develop Dec 13, 2024
2 checks passed

novice0840 deleted the fix/#404 branch December 13, 2024 05:16

novice0840 restored the fix/#404 branch December 13, 2024 05:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FIX] 방장이 자신의 방이 아닌 다른 방의 방장 권한을 얻게 되는 버그 수정 #421

[FIX] 방장이 자신의 방이 아닌 다른 방의 방장 권한을 얻게 되는 버그 수정 #421

novice0840 commented Dec 3, 2024 •

edited by rbgksqkr

Loading

rbgksqkr left a comment

rbgksqkr Dec 3, 2024

rbgksqkr Dec 3, 2024

novice0840 Dec 3, 2024

rbgksqkr Dec 3, 2024

novice0840 Dec 3, 2024

rbgksqkr left a comment

useon left a comment

useon Dec 6, 2024

useon Dec 6, 2024

useon Dec 6, 2024

useon Dec 6, 2024

rbgksqkr commented Dec 6, 2024

novice0840 commented Dec 13, 2024

rbgksqkr commented Dec 13, 2024

		const user = userEvent.setup();
		(useIsMaster as jest.Mock).mockReturnValue(true);

[FIX] 방장이 자신의 방이 아닌 다른 방의 방장 권한을 얻게 되는 버그 수정 #421

[FIX] 방장이 자신의 방이 아닌 다른 방의 방장 권한을 얻게 되는 버그 수정 #421

Conversation

novice0840 commented Dec 3, 2024 • edited by rbgksqkr Loading

Issue Number

As-Is

To-Be

Check List

Test Screenshot

(Optional) Additional Description

🌸 Storybook 배포 주소

🌸 Storybook 배포 주소

rbgksqkr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

🌸 칭찬 🌸

Choose a reason for hiding this comment

🙏 제안 🙏

Choose a reason for hiding this comment

Choose a reason for hiding this comment

💭 질문 💭

Choose a reason for hiding this comment

rbgksqkr left a comment

Choose a reason for hiding this comment

useon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

💭 질문 💭

Choose a reason for hiding this comment

🌸 칭찬 🌸

Choose a reason for hiding this comment

🌸 칭찬 🌸

Choose a reason for hiding this comment

💭 질문 💭

rbgksqkr commented Dec 6, 2024

novice0840 commented Dec 13, 2024

rbgksqkr commented Dec 13, 2024

novice0840 commented Dec 3, 2024 •

edited by rbgksqkr

Loading