Skip to content

20260330 fe checkin redirect prob#346

Merged
discipline24 merged 6 commits into
mainfrom
20260330-FE-CHECKIN-REDIRECT-PROB
Mar 30, 2026
Merged

20260330 fe checkin redirect prob#346
discipline24 merged 6 commits into
mainfrom
20260330-FE-CHECKIN-REDIRECT-PROB

Conversation

@yyunee
Copy link
Copy Markdown
Contributor

@yyunee yyunee commented Mar 30, 2026

  1. 개요
    QR 코드를 스캔하여 접속했을 때 사용자가 로그인되어 있지 않으면 로그인 페이지로 튕기게 되는데, 이때 접속하려던 출석 토큰 정보를 유지하여 로그인 직후 자동으로 출석이 완료되도록 UX를 개선

  2. 주요 변경 사항

ProtectedRoute& AdminRoute
(인증 가드)
인증되지 않은 사용자가 보호된 경로(e.g. /attendance)에 접근 시, 현재의 전체 URL(쿼리 스트링 포함)을 returnUrl 파라미터로 인코딩하여 로그인 페이지로 전달
예: /login?returnUrl=/attendance?token=ABC123XYZ

LoginForm(로그인 처리)
로그인 성공 시 returnUrl 파라미터가 있는지 확인하고, 존재하면 해당 주소로 즉시 리다이렉트합니다. 정보가 없으면 기본값인 홈(/)으로 이동

SignUpForm(회원가입 처리)
회원가입 시에도 returnUrl 정보를 넘겨주어, 회원가입 완료 후 로그인 시에도 출석 페이지로 돌아갈 수 있도록 상태를 보존

useCheckIn커스텀 훅 (자동 출석 처리)
/attendance 페이지 로드 시 URL에 token이 있으면 즉시 백엔드 API 콜을 수행하도록 로직을 강화
중복 방지 로직: 출석 요청이 완료되면 URL에서 토큰을 제거(Clean-up)하여 새로고침 시 중복 요청이 가지 않도록 처리

Summary by CodeRabbit

릴리스 노트

  • 새로운 기능

    • 로그인 후 기존 페이지로 자동 이동하는 반환 URL 기능 추가
  • 버그 수정

    • 모바일 기기에서 뷰포트 높이 계산 개선으로 레이아웃 표시 문제 해결
  • 스타일

    • 피드백 모달의 설명 문구 업데이트 (익명 저장 안내 추가)

@yyunee yyunee requested a review from discipline24 as a code owner March 30, 2026 15:04
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 30, 2026

Walkthrough

뷰포트 높이 대응과 로그인 후 리디렉션 기능을 추가합니다. App에서 CSS 변수 --vh를 설정하는 useEffect를 추가하고, 인증 실패 시 원본 URL을 쿼리 파라미터로 보존하여 로그인 후 해당 페이지로 돌아올 수 있도록 개선합니다. 레이아웃 및 스타일 시트를 --vh 기반 높이 계산으로 업데이트합니다.

Changes

Cohort / File(s) Summary
뷰포트 높이 CSS 변수 설정
frontend/src/App.jsx, frontend/src/index.css
--vh CSS 변수를 동적으로 계산하여 설정하고, 창 크기 변경 시 업데이트하는 useEffect 추가 및 #root의 최소 높이를 CSS 변수 기반으로 설정
레이아웃 및 컨테이너 높이 업데이트
frontend/src/components/Layout.jsx, frontend/src/pages/Home.module.css, frontend/src/pages/LoginAndSignUp.module.css, frontend/src/pages/AdminDashboard.module.css, frontend/src/pages/AdminHome.module.css, frontend/src/pages/Board.module.css, frontend/src/pages/external/External.module.css
다양한 레이아웃 컨테이너의 높이를 calc(var(--vh, 1vh) * 100) 또는 calc(var(--vh, 1vh) * 100 - 90px) 형식으로 변경
사이드바 높이 및 모달 CSS 업데이트
frontend/src/components/Sidebar.jsx, frontend/src/components/Sidebar.module.css
피드백 모달 설명 텍스트를 익명 저장 안내로 업데이트하고, 오버레이·메뉴·모달의 높이 계산을 --vh 기반으로 수정
반환 URL 쿼리 파라미터 처리
frontend/src/components/AdminRoute.jsx, frontend/src/components/protectedRoute.jsx, frontend/src/hooks/useAuthGuard.js
인증 실패 시 현재 페이지 경로를 returnUrl 쿼리 파라미터로 인코딩하여 /login?returnUrl=...로 리디렉션
로그인·가입 폼의 리턴 URL 처리
frontend/src/components/login/LoginForm.jsx, frontend/src/components/signup/SignUpForm.jsx
회원가입 링크와 가입 완료 후 리디렉션 시 returnUrl 쿼리 파라미터를 추가 전달

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • DongEun02
  • gxuoo
  • discipline24

Poem

🐰 뷰포트 높이 변수와 반환 경로,
길을 잃어도 다시 돌아오게,
모바일 화면도 완벽하게 맞춰지고,
로그인 후 원래 자리로 쏙!
작은 배려, 큰 기쁨 ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning 제목이 PR의 주요 변경사항을 정확하게 설명하지 못합니다. 'fe checkin redirect prob'는 너무 모호하고 축약되어 있어 실제 구현 내용을 파악하기 어렵습니다. 제목을 'Add returnUrl parameter to preserve intended destination after login/signup' 또는 'Implement check-in redirect flow with URL preservation' 같이 더 명확하고 구체적으로 개선하세요.
✅ Passed checks (2 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch 20260330-FE-CHECKIN-REDIRECT-PROB

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
frontend/src/components/AdminRoute.jsx (1)

40-40: 의존성 배열로 인한 불필요한 관리자 권한 검사 반복

location.pathnamelocation.search가 의존성 배열에 포함되어, 관리자 페이지 내에서 라우트 변경 시마다 /api/user/details API가 호출됩니다. 관리자 페이지들은 복잡한 필터링 로직을 포함하고 있어(예: AdminMemberManage의 searchQuery, roleFilter 등), URL 변경 시 불필요한 권한 검사가 반복될 수 있습니다.

returnUrl은 리다이렉트 시점에만 필요하므로, 의존성 배열을 단순화하는 것을 권장합니다.

♻️ 제안하는 수정
   useEffect(() => {
     const checkAdmin = async () => {
       try {
         const { data } = await api.get('/api/user/details');
         const role = data?.role;
         setIsAuthorized(isAdminRole(role));
       } catch (error) {
         setIsAuthorized(false);
         const returnUrl = encodeURIComponent(
           location.pathname + location.search
         );
         setRedirectPath(
           error?.status === 401 ? `/login?returnUrl=${returnUrl}` : '/'
         );
       } finally {
         setLoading(false);
       }
     };

     checkAdmin();
-  }, [location.pathname, location.search]);
+  }, []);

useLocation()에서 반환된 location 객체는 항상 현재 위치를 참조하므로, 리다이렉트 시점에 올바른 returnUrl을 계산할 수 있습니다. 빈 의존성 배열을 사용하면 컴포넌트 마운트 시 한 번만 권한 검사가 실행됩니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/components/AdminRoute.jsx` at line 40, The useEffect in
AdminRoute.jsx currently includes location.pathname and location.search in its
dependency array causing repeated admin permission checks on intra-admin route
changes; change the effect to run only on mount by removing those dependencies
(use an empty dependency array) and compute returnUrl from the current location
inside the effect when performing the redirect logic so returnUrl still reflects
the latest location at redirect time; update references to
location.pathname/location.search usage to be inside the effect body and keep
the permission check function (e.g., the fetch to /api/user/details) called only
once on mount.
frontend/src/hooks/useAuthGuard.js (1)

30-30: 의존성 배열로 인한 불필요한 API 호출 가능성 검토 필요

location.pathnamelocation.search가 의존성 배열에 포함되어 있어, 보호된 경로 내에서 네비게이션할 때마다 /api/user/details API가 호출됩니다. 이는 동일한 인증 세션 내에서 불필요한 네트워크 요청을 발생시킬 수 있습니다.

returnUrl 계산은 리다이렉트 시점에만 필요하므로, 의존성 배열에서 location 관련 값을 제거하고 리다이렉트 로직 내에서 현재 위치를 읽는 방식을 고려해 보세요.

♻️ 제안하는 수정
   useEffect(() => {
     const checkAuth = async () => {
       try {
         await api.get('/api/user/details');
       } catch (error) {
         if (error.status === 401) {
           toast.error('로그인 후 이용하실 수 있습니다.');
           const returnUrl = encodeURIComponent(
             location.pathname + location.search
           );
           nav(`/login?returnUrl=${returnUrl}`);
         }
       }
     };

     checkAuth();
-  }, [nav, location.pathname, location.search]);
+  }, [nav]);

location 객체는 useLocation()에서 반환된 참조이므로, 의존성 배열에 포함하지 않아도 리다이렉트 시점에 최신 값을 읽을 수 있습니다.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/hooks/useAuthGuard.js` at line 30, The effect in useAuthGuard.js
is re-running on every path/search change because location.pathname and
location.search are in the dependency array; remove those from the dependency
array (keep nav and other stable deps) and instead compute the returnUrl inside
the redirect logic by reading the current location (from useLocation()) at the
time you perform the redirect (e.g., inside the effect body before calling nav
or redirect). Update references to location.pathname/location.search to be local
reads only when building returnUrl so the API call (fetching /api/user/details)
doesn’t run on every navigation within a protected route.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@frontend/src/components/AdminRoute.jsx`:
- Line 40: The useEffect in AdminRoute.jsx currently includes location.pathname
and location.search in its dependency array causing repeated admin permission
checks on intra-admin route changes; change the effect to run only on mount by
removing those dependencies (use an empty dependency array) and compute
returnUrl from the current location inside the effect when performing the
redirect logic so returnUrl still reflects the latest location at redirect time;
update references to location.pathname/location.search usage to be inside the
effect body and keep the permission check function (e.g., the fetch to
/api/user/details) called only once on mount.

In `@frontend/src/hooks/useAuthGuard.js`:
- Line 30: The effect in useAuthGuard.js is re-running on every path/search
change because location.pathname and location.search are in the dependency
array; remove those from the dependency array (keep nav and other stable deps)
and instead compute the returnUrl inside the redirect logic by reading the
current location (from useLocation()) at the time you perform the redirect
(e.g., inside the effect body before calling nav or redirect). Update references
to location.pathname/location.search to be local reads only when building
returnUrl so the API call (fetching /api/user/details) doesn’t run on every
navigation within a protected route.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fd6941db-c63e-4ba5-aa8a-935cac79a30b

📥 Commits

Reviewing files that changed from the base of the PR and between 96336e3 and b497a36.

📒 Files selected for processing (16)
  • frontend/src/App.jsx
  • frontend/src/components/AdminRoute.jsx
  • frontend/src/components/Layout.jsx
  • frontend/src/components/Sidebar.jsx
  • frontend/src/components/Sidebar.module.css
  • frontend/src/components/login/LoginForm.jsx
  • frontend/src/components/protectedRoute.jsx
  • frontend/src/components/signup/SignUpForm.jsx
  • frontend/src/hooks/useAuthGuard.js
  • frontend/src/index.css
  • frontend/src/pages/AdminDashboard.module.css
  • frontend/src/pages/AdminHome.module.css
  • frontend/src/pages/Board.module.css
  • frontend/src/pages/Home.module.css
  • frontend/src/pages/LoginAndSignUp.module.css
  • frontend/src/pages/external/External.module.css

Copy link
Copy Markdown
Contributor

@discipline24 discipline24 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~

@discipline24 discipline24 merged commit 079c776 into main Mar 30, 2026
1 check passed
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