Skip to content

04.13 최신화 - #259

Merged
yhi9839 merged 6 commits into
mainfrom
dev
Apr 15, 2026
Merged

yhi9839 merged 6 commits into
mainfrom
dev

Conversation

@shinwokkang

@shinwokkang shinwokkang commented Apr 13, 2026 •

Copy link
Copy Markdown
Contributor

📌 개요 (Summary)

  • 변경 사항에 대한 간략한 요약을 적어주세요.
  • 관련 이슈가 있다면 링크를 걸어주세요 (예: [fix] cicd / build 문제 #123).

🛠️ 변경 사항 (Changes)

  • 새로운 기능 추가
  • 버그 수정
  • 코드 리팩토링
  • 문서 업데이트
  • 기타 (설명: )

📸 스크린샷 (Screenshots)

(UI 변경 사항이 있다면 첨부해주세요)

✅ 체크리스트 (Checklist)

  • 빌드가 성공적으로 수행되었나요? (pnpm build)
  • 린트 에러가 없나요? (pnpm lint)
  • 불필요한 콘솔 로그나 주석을 제거했나요?

Summary by CodeRabbit

Release Notes

  • New Features

    • Global login modal now unified across the application for a seamless authentication experience.
  • Bug Fixes

    • Improved modal scroll behavior to eliminate page jumping when modals open.
  • Refactor

    • Strengthened admin area authentication verification.
    • Updated admin login page with clearer guidance messaging.

@shinwokkang shinwokkang self-assigned this Apr 13, 2026
@vercel

vercel Bot commented Apr 13, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
check-mo Ready Ready Preview, Comment Apr 13, 2026 1:56am
checkmo Ready Ready Preview, Comment Apr 13, 2026 1:56am

@coderabbitai

coderabbitai Bot commented Apr 13, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

The changes refactor login modal rendering into a centralized GlobalLoginModal component, extract scroll-locking into a reusable hook supporting nested modals, add client-side admin authorization checks, and remove per-page modal instances across the application. The admin login page is simplified to a static notice.

Changes

Cohort / File(s) Summary
Admin Authentication
src/app/(admin)/admin/(app)/layout.tsx, src/app/(admin)/admin/(app)/page.tsx
Introduced client-side auth checks via useEffect calling GET /members/me/login-status, added loading/authorized state management, and auto-redirects to /admin/login on auth failure. Admin index page converted to client-side redirect via useRouter.replace().
Admin Login Simplification
src/app/(admin)/admin/(auth)/login/page.tsx
Removed login form, state management (id, pw, loading, toastMsg), and POST handler; replaced with static centered notice ("관리자 기능은 로그인이 필요합니다!").
Global Login Modal
src/app/layout.tsx, src/components/base-ui/Login/GlobalLoginModal.tsx
Added new GlobalLoginModal component that conditionally renders a single shared LoginModal instance based on auth store state; integrated into RootLayout for app-wide access.
Page-level Modal Removal
src/app/(main)/page.tsx, src/app/(main)/stories/..., src/app/groups/page.tsx, src/components/layout/BottomNav.tsx, src/components/layout/Header.tsx
Removed LoginModal imports, eliminated per-page conditional rendering and closeLoginModal destructuring; retained openLoginModal() calls for auth-gated actions.
Scroll Lock Refactoring
src/hooks/useScrollLock.ts, src/components/base-ui/Login/LoginModal.tsx, src/components/base-ui/Admin/ConfirmModal.tsx, src/components/base-ui/Bookcase/bookid/BookshelfDeleteConfirmModal.tsx, src/components/common/ConfirmModal.tsx, src/components/common/ReportModal.tsx, src/components/layout/BookSelectModal.tsx, src/components/layout/SearchModal.tsx
Introduced centralized useScrollLock(lock?) hook with module-scoped counter for nested modal support; migrated all modals from inline document.body.style.overflow mutations to hook-based management. Removed old local useScrollLock hook from src/components/base-ui/Login/hooks/.
Admin Story Page
src/app/(admin)/admin/(app)/stories/[id]/page.tsx
Removed LoginModal import and conditional rendering; retained useAuthStore() destructuring for openLoginModal() in like/follow handlers.

Sequence Diagram

sequenceDiagram
    participant Client as Client / Browser
    participant AdminLayout as Admin Layout<br/>(useEffect)
    participant API as Backend API<br/>/members/me/login-status
    participant Router as Next.js Router
    participant AuthRoutes as Auth Routes<br/>(/login, /signup)

    Client->>AdminLayout: Navigate to /admin/*
    rect rgba(100, 150, 200, 0.5)
        Note over AdminLayout: Check if route is auth-exempt
        alt Is /login or /signup
            AdminLayout->>AdminLayout: Set authorized = true
            AdminLayout->>Client: Render children
        else Is protected route
            AdminLayout->>AdminLayout: Show loading state
            AdminLayout->>API: GET /members/me/login-status<br/>(credentials: include)
            API-->>AdminLayout: Response { isSuccess, result.admin }
            rect rgba(200, 100, 100, 0.5)
                alt Success && admin === true
                    AdminLayout->>AdminLayout: Set authorized = true
                    AdminLayout->>Client: Render children
                else Failure or !admin
                    AdminLayout->>Router: router.push(/admin/login)
                    Router-->>Client: Redirect to /admin/login
                end
            end
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • [fx] 스크롤 잠금 버그 수정 #258: Centralized scroll-lock implementation with lock count and global login modal component—directly parallels this refactoring of modal rendering and scroll behavior.
  • 03/17 최신화 #205: Modifies admin authentication pages (src/app/(admin)/admin/(auth)/login/page.tsx) and admin client-side routing flow.
  • 배포 최신화 #141: Added admin layout and page components that this PR now extends with client-side auth checks and router-based redirects.

Suggested labels

♻️ refactor, 🔐 auth

Suggested reviewers

  • hongik-luke

Poem

🐰 A modal hops from page to page no more,
Now Global and centered, living at the core!
Scroll-locks aligned with nested care,
Admin gates now guarded with flair! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.26% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title '04.13 최신화' (April 13 update) is vague and generic, using non-descriptive terms that do not convey meaningful information about the actual changes. Update the title to clearly describe the main changes, such as 'Refactor admin auth flow and consolidate login modal to global component' or similar to reflect the key modifications.
✅ Passed checks (1 passed)
Check name Status Explanation
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 dev

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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request centralizes the login modal into a global component and introduces a useScrollLock hook with a counter to manage body scrolling across nested modals. It also implements admin authorization checks within the admin layout. Key feedback includes a critical concern regarding the removal of the admin login form, which prevents users from logging in. Additionally, the reviewer suggests using server-side redirects for the admin index, correcting path matching logic in the admin layout, and utilizing getComputedStyle to ensure accurate restoration of the body's overflow state.

Comment on lines +31 to 33
<div className="mt-[80px] text-black subhead_1">
관리자 기능은 로그인이 필요합니다!
</div>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

관리자 로그인 페이지에서 기존의 로그인 폼과 처리 로직(handleLogin)이 모두 삭제되었습니다. 현재는 메시지만 출력되고 있어 사용자가 실제로 로그인을 수행할 수 없는 상태입니다. 관리자 기능을 사용하기 위해 로그인이 필수적이므로, 로그인 폼을 복구하거나 로그인을 수행할 수 있는 수단(예: 로그인 모달 호출 버튼)을 제공해야 합니다.

Comment on lines +9 to +11
useEffect(() => {
router.replace("/admin/users");
}, [router]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Next.js의 서버 컴포넌트에서 제공하는 redirect 대신 클라이언트 사이드에서 useEffect와 router.replace를 사용하여 리다이렉트를 처리하도록 변경되었습니다. 단순한 페이지 리다이렉트의 경우, 서버 사이드에서 redirect를 사용하는 것이 클라이언트에서의 추가적인 렌더링 사이클을 방지하고 사용자 경험 측면에서 더 효율적입니다. 특별한 이유가 없다면 기존의 서버 사이드 리다이렉트 방식을 유지하는 것을 권장합니다.

Comment on lines 26 to 28
const isAuthPage =
pathname.startsWith("/login") ||
pathname.startsWith("/signup");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

isAuthPage 판별 시 pathname.startsWith("/login")을 사용하고 있으나, 현재 파일 위치와 경로 구조상 관리자 로그인 경로는 /admin/login일 가능성이 높습니다. 이 경우 조건문이 항상 false를 반환하게 됩니다. 실제 경로에 맞춰 /admin/login 등으로 수정하거나, 이 레이아웃이 해당 페이지를 포함하지 않는다면 불필요한 로직을 정리하는 것이 좋습니다.

Suggested change
const isAuthPage =
pathname.startsWith("/login") ||
pathname.startsWith("/signup");
const isAuthPage =
pathname.startsWith("/admin/login") ||
pathname.startsWith("/admin/signup");


if (lockCount === 0) {
// 최초 1회 잠금 시 원래의 인라인 스타일 저장
originalOverflow = document.body.style.overflow;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

document.body.style.overflow는 인라인 스타일만 가져옵니다. 만약 CSS 클래스 등을 통해 body에 기본 overflow 스타일이 지정되어 있다면 이를 정확히 캡처하지 못할 수 있습니다. window.getComputedStyle(document.body).overflow를 사용하여 현재 계산된 스타일을 저장하는 것이 더 안전합니다.

Suggested change
originalOverflow = document.body.style.overflow;
originalOverflow = window.getComputedStyle(document.body).overflow;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

스타일 지정을 따로 하지 않았습니다.
해당 부분은 문제가 없습니다. 추후에 스타일이 지정되면 스타일을 넣겠습니다.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/app/`(admin)/admin/(app)/layout.tsx:
- Around line 7-12: The current LoginStatusResponse type and the admin check
consume /members/me/login-status as { result: { admin: boolean } } which
mismatches the shared member/login-status contract and can misclassify valid
sessions; update the code to use the shared member/login-status type/service (or
change the backend contract to include a permission-aware response) and replace
the inline LoginStatusResponse with the canonical shared type, then update the
admin check where you reference the response (the check that inspects
result.admin) to read the correct property shape provided by the shared
service/endpoint (or call the permission-capable endpoint instead) so the admin
redirect logic uses the correct field names and types.
- Around line 26-28: The isAuthPage check currently uses
pathname.startsWith("/login") and "/signup", which doesn't match the actual
route (/admin/login); update the isAuthPage logic in layout.tsx (the const
isAuthPage and the duplicate check near the redirect logic) to only test
pathname.startsWith("/admin/login") so the auth bypass correctly recognizes the
admin login page and avoids running the auth guard on it.

In `@src/app/`(admin)/admin/(auth)/login/page.tsx:
- Around line 7-33: The page currently only renders a notice and must provide a
real login action: add a visible call-to-action (e.g., a "Sign in" button and a
"Forgot password" link) inside the rendered section so users can authenticate or
recover access; the button should either invoke the app's existing global login
modal opener (e.g., call openAuthModal(), showAuthModal(), or dispatch an
auth-modal action) or navigate to the canonical auth route that triggers the
modal (for example /login?modal=1 or /api/auth/signin), and the forgot-password
link should point to the password-recovery route; place these controls next to
the existing Image and the div with the text so the admin flow is not a
dead-end.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3bc196e6-635d-431a-aab1-273041ef19b1

📥 Commits

Reviewing files that changed from the base of the PR and between 72b4514 and 540cede.

📒 Files selected for processing (21)
  • src/app/(admin)/admin/(app)/layout.tsx
  • src/app/(admin)/admin/(app)/page.tsx
  • src/app/(admin)/admin/(app)/stories/[id]/page.tsx
  • src/app/(admin)/admin/(auth)/login/page.tsx
  • src/app/(main)/page.tsx
  • src/app/(main)/stories/[id]/page.tsx
  • src/app/(main)/stories/page.tsx
  • src/app/groups/page.tsx
  • src/app/layout.tsx
  • src/components/base-ui/Admin/ConfirmModal.tsx
  • src/components/base-ui/Bookcase/bookid/BookshelfDeleteConfirmModal.tsx
  • src/components/base-ui/Login/GlobalLoginModal.tsx
  • src/components/base-ui/Login/LoginModal.tsx
  • src/components/base-ui/Login/hooks/useScrollLock.ts
  • src/components/common/ConfirmModal.tsx
  • src/components/common/ReportModal.tsx
  • src/components/layout/BookSelectModal.tsx
  • src/components/layout/BottomNav.tsx
  • src/components/layout/Header.tsx
  • src/components/layout/SearchModal.tsx
  • src/hooks/useScrollLock.ts
💤 Files with no reviewable changes (1)
  • src/components/base-ui/Login/hooks/useScrollLock.ts

Comment on lines +7 to +12
type LoginStatusResponse = {
isSuccess: boolean;
result?: {
admin: boolean;
};
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Authorization check is using a mismatched API shape.

/members/me/login-status is being parsed as { result: { admin: boolean } } and checked at Line 55, but the shared member/login-status contract in this codebase is not shaped that way. This can make valid sessions fail admin checks and redirect incorrectly.
Use a permission-capable endpoint (or align backend contract) and shared types/service to avoid contract drift.

Also applies to: 53-59

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

In `@src/app/`(admin)/admin/(app)/layout.tsx around lines 7 - 12, The current
LoginStatusResponse type and the admin check consume /members/me/login-status as
{ result: { admin: boolean } } which mismatches the shared member/login-status
contract and can misclassify valid sessions; update the code to use the shared
member/login-status type/service (or change the backend contract to include a
permission-aware response) and replace the inline LoginStatusResponse with the
canonical shared type, then update the admin check where you reference the
response (the check that inspects result.admin) to read the correct property
shape provided by the shared service/endpoint (or call the permission-capable
endpoint instead) so the admin redirect logic uses the correct field names and
types.

Comment on lines 26 to 28
const isAuthPage =
pathname.startsWith("/login") ||
pathname.startsWith("/signup");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# 1) Locate admin login/signup pages and admin layouts
fd -t f 'page.tsx|layout.tsx' src/app | rg -n '/\(admin\)/admin/'

# 2) Show current bypass matcher and redirect lines in this layout
rg -n -C2 'startsWith\\("/login"\\)|startsWith\\("/signup"\\)|router\\.replace\\("/admin/login"\\)' src/app

Repository: checkmo2025/FE

Length of output: 733


🏁 Script executed:

head -60 src/app/\(admin\)/admin/\(app\)/layout.tsx

Repository: checkmo2025/FE

Length of output: 1404


🏁 Script executed:

fd -t f 'signup.*page.tsx' src/app

Repository: checkmo2025/FE

Length of output: 40


Auth-page bypass matcher uses incorrect paths and will not work for the actual login route.

Bypass checks pathname.startsWith("/login") and pathname.startsWith("/signup"), but the actual login page is at /admin/login (under the (auth) route group). When users navigate to /admin/login, the pathname will be /admin/login, which does not match the bypass condition. This causes the auth check to run on the login page itself, leading to a redirect loop or authentication failures.

Update lines 26–28 to match the actual route paths:

const isAuthPage =
  pathname.startsWith("/admin/login");

(No /admin/signup page exists in the codebase, so only the login path needs to be checked.)

Also applies to: 49-49

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

In `@src/app/`(admin)/admin/(app)/layout.tsx around lines 26 - 28, The isAuthPage
check currently uses pathname.startsWith("/login") and "/signup", which doesn't
match the actual route (/admin/login); update the isAuthPage logic in layout.tsx
(the const isAuthPage and the duplicate check near the redirect logic) to only
test pathname.startsWith("/admin/login") so the auth bypass correctly recognizes
the admin login page and avoids running the auth guard on it.

Comment on lines +7 to 33
<main
className="
min-h-screen
flex items-center justify-center px-4
bg-[#826656]
"
>
<section
className="
w-full max-w-[456px] h-[276px]
rounded-[8px]
bg-white
flex flex-col items-center justify-center
text-center
"
>
<Image
src="/Vector.svg"
alt="CheckMo Logo"
width={76}
height={46}
className="w-[76.607px] h-[46.815px] shrink-0"
/>

<div className="mt-[80px] text-black subhead_1">
관리자 기능은 로그인이 필요합니다!
</div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Add a real login action here.

Users redirected to /admin/login now only see a notice, but this page no longer gives them any way to authenticate or recover to a route that can open the global login modal. That makes the admin flow dead-end for logged-out users.

💡 Minimal fix
+"use client";
+import { useAuthStore } from "@/store/useAuthStore";
 import Image from "next/image";
 
 export default function AdminLoginPage() {
+  const { openLoginModal } = useAuthStore();
+
   return (
     <main
       className="
         min-h-screen
         flex items-center justify-center px-4
@@
 
         <div className="mt-[80px] text-black subhead_1">
           관리자 기능은 로그인이 필요합니다!
         </div>
+
+        <button
+          type="button"
+          onClick={openLoginModal}
+          className="mt-4 rounded-[8px] bg-primary-3 px-4 py-2 text-white"
+        >
+          로그인하기
+        </button>
       </section>
     </main>
   );
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<main
className="
min-h-screen
flex items-center justify-center px-4
bg-[#826656]
"
>
<section
className="
w-full max-w-[456px] h-[276px]
rounded-[8px]
bg-white
flex flex-col items-center justify-center
text-center
"
>
<Image
src="/Vector.svg"
alt="CheckMo Logo"
width={76}
height={46}
className="w-[76.607px] h-[46.815px] shrink-0"
/>
<div className="mt-[80px] text-black subhead_1">
관리자 기능은 로그인이 필요합니다!
</div>
"use client";
import { useAuthStore } from "@/store/useAuthStore";
import Image from "next/image";
export default function AdminLoginPage() {
const { openLoginModal } = useAuthStore();
return (
<main
className="
min-h-screen
flex items-center justify-center px-4
bg-[`#826656`]
"
>
<section
className="
w-full max-w-[456px] h-[276px]
rounded-[8px]
bg-white
flex flex-col items-center justify-center
text-center
"
>
<Image
src="/Vector.svg"
alt="CheckMo Logo"
width={76}
height={46}
className="w-[76.607px] h-[46.815px] shrink-0"
/>
<div className="mt-[80px] text-black subhead_1">
관리자 기능은 로그인이 필요합니다!
</div>
<button
type="button"
onClick={openLoginModal}
className="mt-4 rounded-[8px] bg-primary-3 px-4 py-2 text-white"
>
로그인하기
</button>
</section>
</main>
);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/app/`(admin)/admin/(auth)/login/page.tsx around lines 7 - 33, The page
currently only renders a notice and must provide a real login action: add a
visible call-to-action (e.g., a "Sign in" button and a "Forgot password" link)
inside the rendered section so users can authenticate or recover access; the
button should either invoke the app's existing global login modal opener (e.g.,
call openAuthModal(), showAuthModal(), or dispatch an auth-modal action) or
navigate to the canonical auth route that triggers the modal (for example
/login?modal=1 or /api/auth/signin), and the forgot-password link should point
to the password-recovery route; place these controls next to the existing Image
and the div with the text so the admin flow is not a dead-end.

@yhi9839
yhi9839 merged commit de0e556 into main Apr 15, 2026
4 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Jul 6, 2026

This branch was successfully deployed

2 active deployments
Preview – check-mo — 540cedec Deployed Apr 13, 2026 by vercel[bot]
Preview – checkmo — 540cedec Deployed Apr 13, 2026 by vercel[bot]
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