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

[FE] 약속 공유 링크 상수 경로 수정 #434

Merged
merged 4 commits into from
Dec 6, 2024
Merged

Conversation

Largopie
Copy link
Contributor

관련 이슈

작업 내용

링크가 https://가 아닌 https:/로 경로가 설정되어 경로를 인식하지 못하는 문제가 있었습니다.
이를 해결하기 위해 origin 경로 자체를 복사하도록 상수를 수정했습니다.

// 수정 전
const LINK = `${window.location.protocol}/${window.location.host}/meeting/${uuid}`;

// 수정 후
const LINK = `${window.location.origin}/meeting/${uuid}`;

특이 사항

리뷰 요구사항 (선택)

@Largopie Largopie added the 🛠️ 픽스 버그를 해결했어요 :) label Nov 12, 2024
@Largopie Largopie added this to the 6차(최종) 데모데이 milestone Nov 12, 2024
@Largopie Largopie self-assigned this Nov 12, 2024
Copy link

github-actions bot commented Nov 12, 2024

Test Results

35 tests   35 ✅  24s ⏱️
 4 suites   0 💤
 1 files     0 ❌

Results for commit b71a372.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@Yoonkyoungme Yoonkyoungme left a comment

Choose a reason for hiding this comment

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

굿👍🏻
현재 URI 경로에 포함된 /meeting/ 부분은 다음 단계에서 제외하는 URI 리팩토링이 필요할 것 같네요!

🚨테스트 통과 못 하는 문제 확인해 주세요🚨

@Largopie
Copy link
Contributor Author

Largopie commented Dec 6, 2024

@Yoonkyoungme @hwinkr
테스트 케이스 통과되지 않는 문제 수정했습니다~
왜 통과 안되었는지는 아래 백로그에 별도로 기록해뒀으니 참고하세용~

https://paper-mass-5ff.notion.site/useCalendar-test-ts-3af80a20c3e24401a8cf649c462dfc18?pvs=4

Copy link
Contributor

@Yoonkyoungme Yoonkyoungme left a comment

Choose a reason for hiding this comment

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

고생하셨어요 낙타🐦‍🔥
노션에 문제 상황과 해결 방법을 잘 정리해 주셨더군요!! 덕분에 맥락을 이해하기 편했습니다😊

act(() => {
moveToNextMonth();
});
// 총 13번의 moveToNextMonth() 함수 실행
Copy link
Contributor

Choose a reason for hiding this comment

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

[P3]

현재 주석에 적힌 13번 반복 호출의 의미를 명확히 수정하거나
테스트 제목에 포함시키는 것을 가볍게 제안드려 봅니다~!

ex)

it('현재 월 기준, 약속 날짜 범위가 1년을 벗어나면 토스트 UI를 활용하여 사용자에게 예외 피드백을 전달한다', async () => {
      const { result } = renderHookWithProvider(useCalendar);

      /* 1년 후 범위를 초과하려면 13번 moveToNextMonth 호출이 필요함
       * 또는
       * 1년이 12달이므로 최소 13번 moveToNextMonth를 호출하면 범위를 초과함
       * 등등
      */

      for (let i = 0; i < 13; i++) {
        ...
      }
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

의견 감사합니다요~ 수정했어요~! 👍

Copy link
Contributor

@hwinkr hwinkr left a comment

Choose a reason for hiding this comment

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

고생하셨습니다! 왜 안되는지 자세하게 정리해 줘서 이해하기 편했습니다 😃

@Largopie Largopie merged commit 07bfecd into develop Dec 6, 2024
5 checks passed
@Largopie Largopie deleted the fix/433-copy-link branch December 6, 2024 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠️ 픽스 버그를 해결했어요 :)
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants