Merged
Conversation
Feat/218 관리자 모임 관리 api 연결
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughReplaces client-side dummy admin group/member pages with server-driven implementations, adds typed admin clubs API and endpoint builders, updates UI components to accept structured items and toggle callbacks, and implements loading/error states, validation, and server-backed pagination and detail editing. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Client as Admin Client
participant API as Admin API
participant DB as Database
rect rgba(100,150,200,0.5)
Note over User,API: Group Detail load & edit
User->>Client: Navigate to /admin/groups/:id
Client->>Client: useParams() & useEffect
Client->>API: fetchAdminClubDetail(clubId)
API->>DB: query club detail
DB-->>API: return club detail
API-->>Client: respond with AdminClubDetail
Client->>Client: populate form state
User->>Client: edit & click save
Client->>API: updateAdminClub(clubId, payload)
API->>DB: persist updates
DB-->>API: confirm
API-->>Client: success
Client->>Client: router.refresh() / show success
end
rect rgba(150,200,100,0.5)
Note over User,API: Members pagination
User->>Client: open Members tab / change page
Client->>API: fetchAdminClubActiveMembers(clubId, page, size)
API->>DB: query members w/ pagination
DB-->>API: return members + totalPages
API-->>Client: respond with members
Client->>Client: formatDate/mapRole & render table
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 풀 리퀘스트는 관리자 페이지의 모임 관리 기능을 백엔드 API와 연동하여 실제 데이터를 기반으로 동작하도록 업데이트합니다. 기존에 사용되던 더미 데이터를 제거하고, 모임 목록 조회, 상세 정보 조회 및 수정, 그리고 모임 회원 목록 조회 기능을 API 기반으로 전환하여 관리자 기능의 실제 운영 가능성을 확보했습니다. 이를 통해 관리자는 모임 정보를 효율적으로 관리하고 최신 상태를 유지할 수 있게 됩니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
전반적으로 관리자 모임 관리 페이지를 더미 데이터에서 실제 API 연동으로 전환하는 훌륭한 개선입니다. 그룹 목록, 상세 정보, 회원 목록을 불러오고 그룹 정보를 수정하는 기능이 추가되었네요. 코드 구조도 API, 컴포넌트, 페이지로 잘 분리되어 있습니다.
다만, 리뷰 과정에서 몇 가지 개선점을 발견했습니다.
- 모임 목록 페이지의 검색 기능이 의도와 다르게 동작하는 심각한 문제가 있습니다.
- 회원 목록 페이지의 '상세보기' 링크가 동작하지 않습니다.
- 비동기 작업을 처리하는
useEffect의 일관성이 부족한 부분이 있습니다. - 코드 일관성 및 가독성을 위한 작은 개선 사항들이 있습니다.
각 항목에 대한 구체적인 의견은 코드에 직접 남겼으니 확인 부탁드립니다.
Comment on lines
+206
to
+211
| <Link | ||
| href="#" | ||
| className="text-Gray-7 underline underline-offset-2 hover:opacity-70" | ||
| > | ||
| 상세보기 | ||
| </Link> |
Contributor
There was a problem hiding this comment.
'상세보기' 링크의 href 속성이 "#"로 되어 있어, 클릭해도 아무런 동작을 하지 않습니다. 사용자 상세 페이지로 이동해야 하지만, 현재 API 응답에 사용자 ID가 없어 링크를 구성할 수 없는 것으로 보입니다.
사용자 상세 페이지로 올바르게 라우팅하기 위해서는 API 응답에 사용자 ID와 같은 고유 식별자가 포함되어야 합니다. 백엔드 팀과 협의하여 AdminClubActiveMember 타입에 사용자 ID를 추가하는 것을 강력히 권장합니다.
Comment on lines
+91
to
+119
| useEffect(() => { | ||
| if (Number.isNaN(groupId)) { | ||
| setError("올바르지 않은 모임 ID입니다."); | ||
| setLoading(false); | ||
| return; | ||
| } | ||
|
|
||
| const loadGroupDetail = async () => { | ||
| try { | ||
| setLoading(true); | ||
| setError(""); | ||
| setSuccessMessage(""); | ||
|
|
||
| const data = await fetchAdminClubDetail(groupId); | ||
| setGroup(data); | ||
| setForm(normalizeClubToForm(data)); | ||
| } catch (err) { | ||
| setError( | ||
| err instanceof Error | ||
| ? err.message | ||
| : "모임 상세 정보를 불러오지 못했습니다.", | ||
| ); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| loadGroupDetail(); | ||
| }, [groupId]); |
Contributor
There was a problem hiding this comment.
이 useEffect 훅에는 비동기 작업에 대한 정리(cleanup) 로직이 없습니다. 사용자가 데이터를 로드하는 동안 다른 페이지로 빠르게 이동하면, 컴포넌트가 언마운트된 후에도 상태 업데이트가 시도되어 메모리 누수 및 예기치 않은 오류를 유발할 수 있습니다.
members/page.tsx에서 사용된 것처럼 mounted 플래그나 AbortController를 사용하여 컴포넌트가 언마운트될 때 비동기 작업을 취소하는 정리 함수를 추가해주세요.
| ) : ( | ||
| users.map((u) => ( | ||
| <tr | ||
| key={`${u.nickname}-${u.email}`} |
Contributor
There was a problem hiding this comment.
map 함수에서 key로 u.nickname과 u.email의 조합을 사용하고 있습니다. 이메일(u.email)이 고유하다면, key={u.email}과 같이 더 간단하고 명확하게 키를 설정할 수 있습니다. 만약 API 응답에 사용자 ID와 같은 고유한 값이 있다면 그것을 사용하는 것이 가장 좋습니다.
Suggested change
| key={`${u.nickname}-${u.email}`} | |
| key={u.email} |
| {links.length > 0 ? ( | ||
| <div className="flex flex-col gap-4"> | ||
| {links.map((item, index) => ( | ||
| <div key={index} className="flex gap-4"> |
Contributor
There was a problem hiding this comment.
links.map에서 key로 index를 사용하고 있습니다. 링크 목록이 정적이고 순서가 변경되지 않는다면 문제가 되지 않을 수 있지만, 향후 목록에서 아이템을 추가, 삭제, 또는 재정렬하는 기능이 추가될 경우 React의 재조정(reconciliation) 과정에서 예기치 않은 동작이나 성능 저하를 유발할 수 있습니다.
각 링크에 고유한 ID를 부여하고 그것을 key로 사용하는 것이 가장 이상적입니다. API에서 고유 ID를 제공하지 않는다면, 클라이언트 측에서 임시 고유 ID를 생성하여 사용하는 방법도 고려해볼 수 있습니다.
| {g.ownerEmail} | ||
| </td> | ||
| <td className="pl-[12px] py-0 body_1_2 text-Gray-7"> | ||
| {g.createdAt.slice(0, 10).replace(/-/g, ".")} |
Contributor
There was a problem hiding this comment.
날짜 형식을 인라인으로 처리하고 있습니다 (g.createdAt.slice(0, 10).replace(/-/g, ".")). 이는 코드의 가독성을 해치고, 다른 곳에서 날짜 형식이 변경될 경우 여러 곳을 수정해야 하는 문제를 야기합니다.
members/page.tsx에 있는 formatDate와 같은 유틸리티 함수를 만들어 재사용하는 것을 권장합니다. 이를 통해 코드의 일관성을 유지하고 유지보수성을 높일 수 있습니다.
| key={`${item.label}-${idx}`} | ||
| label={item.label} | ||
| selected={item.selected} | ||
| onClick={() => onToggle?.(item.label)} // 👈 핵심 |
Contributor
There was a problem hiding this comment.
코드 내에 개발 중에 사용된 것으로 보이는 주석(// 👈 핵심)이 남아있습니다. 최종 코드에는 이러한 주석을 제거하여 코드를 깔끔하게 유지하는 것이 좋습니다.
Suggested change
| onClick={() => onToggle?.(item.label)} // 👈 핵심 | |
| onClick={() => onToggle?.(item.label)} |
| title?: string; | ||
| items: string[]; | ||
| items: TargetItem[]; | ||
| onToggle?: (label: string) => void; // 👈 추가 |
Contributor
There was a problem hiding this comment.
코드 내에 개발 중에 사용된 것으로 보이는 주석(// 👈 추가)이 남아있습니다. 최종 코드에는 이러한 주석을 제거하여 코드를 깔끔하게 유지하는 것이 좋습니다.
Suggested change
| onToggle?: (label: string) => void; // 👈 추가 | |
| onToggle?: (label: string) => void; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/app/(admin)/admin/(app)/groups/page.tsx (1)
44-46: MissingloadGroupsin useEffect dependency array.The
loadGroupsfunction is defined outside the effect but not included in the dependency array. This can cause stale closure issues. Consider either movingloadGroupsinside the effect or usinguseCallback.♻️ Proposed fix using inline async function
useEffect(() => { - loadGroups(page); - }, [page]); + const loadData = async () => { + try { + setLoading(true); + setError(""); + const data = await fetchAdminClubs(page - 1, pageSize); + setGroups(data.clubs ?? []); + setTotalPages(Math.max(1, data.totalPages ?? 1)); + } catch (err) { + setError( + err instanceof Error + ? err.message + : "모임 목록을 불러오지 못했습니다.", + ); + setGroups([]); + setTotalPages(1); + } finally { + setLoading(false); + } + }; + loadData(); + }, [page, pageSize]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(admin)/admin/(app)/groups/page.tsx around lines 44 - 46, The useEffect currently calls loadGroups(page) but omits loadGroups from the dependency array, risking a stale closure; fix by either moving the loadGroups implementation into the useEffect as an inline async function that uses page, or memoize loadGroups with useCallback and include loadGroups in the dependency array (e.g., ensure loadGroups and page are listed in useEffect dependencies). Update the file so the effect depends on the stable loadGroups reference or uses the inline loader to preserve correct behavior.src/app/(admin)/admin/(app)/groups/[id]/members/page.tsx (1)
69-72: Club detail is re-fetched on every page change.
fetchAdminClubDetailis called on every pagination change just to getgroupName, which is unlikely to change. Consider fetching it once on mount or separating the concerns.♻️ Suggested optimization
+ // Fetch club detail once on mount + useEffect(() => { + if (!clubId || Number.isNaN(clubId)) return; + fetchAdminClubDetail(clubId).then((detail) => setGroupName(detail.name)); + }, [clubId]); useEffect(() => { if (!clubId || Number.isNaN(clubId)) return; let mounted = true; async function loadData() { try { setLoading(true); setError(null); - const [clubDetail, membersResult] = await Promise.all([ - fetchAdminClubDetail(clubId), - fetchAdminClubActiveMembers(clubId, page - 1, pageSize), - ]); + const membersResult = await fetchAdminClubActiveMembers(clubId, page - 1, pageSize); if (!mounted) return; - setGroupName(clubDetail.name); // ... rest of mapping🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(admin)/admin/(app)/groups/[id]/members/page.tsx around lines 69 - 72, Currently fetchAdminClubDetail(clubId) is being called inside the Promise.all together with fetchAdminClubActiveMembers on every pagination change; move the club detail fetch out so it runs only once (or only when clubId changes) and keep the paginated member fetch for page changes. Concretely, call fetchAdminClubDetail(clubId) separately (e.g., in an effect or a higher-level loader) to set clubDetail/groupName, and then call fetchAdminClubActiveMembers(clubId, page - 1, pageSize) on pagination; update the code that currently destructures const [clubDetail, membersResult] = await Promise.all([...]) to two distinct calls so groupName is not re-fetched on each page.src/app/(admin)/admin/(app)/groups/[id]/page.tsx (1)
18-43: Consider importing from existing constants instead of duplicating.
CATEGORY_OPTIONSduplicatessrc/constants/categories.tsandTARGET_OPTIONSduplicatessrc/types/groups/groups.ts. This creates a maintenance burden and risk of divergence when backend codes change.♻️ Suggested refactor
+import { CATEGORIES } from "@/constants/categories"; +import { PARTICIPANTS, PARTICIPANT_LABEL_TO_TYPE } from "@/types/groups/groups"; -const CATEGORY_OPTIONS = [ - { label: "여행", value: "TRAVEL" }, - // ... rest of options -]; +const CATEGORY_OPTIONS = CATEGORIES; -const TARGET_OPTIONS = [ - { label: "대학생", value: "STUDENT" }, - // ... rest of options -]; +const TARGET_OPTIONS = PARTICIPANTS.map((label) => ({ + label, + value: PARTICIPANT_LABEL_TO_TYPE[label], +}));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(admin)/admin/(app)/groups/[id]/page.tsx around lines 18 - 43, CATEGORY_OPTIONS and TARGET_OPTIONS are duplicated here; replace them by importing the canonical constants from the existing modules and remove the local arrays. Locate the local symbols CATEGORY_OPTIONS and TARGET_OPTIONS in page.tsx, import the matching exports from the shared constants (the categories constant and the group target constant exported by your constants/modules), and use those imports in place of the local arrays; if the shared values use different shape, map/transform them to { label, value } at the import site so all existing consumers of CATEGORY_OPTIONS and TARGET_OPTIONS keep the same shape.
🤖 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)/groups/[id]/members/page.tsx:
- Around line 206-211: The Link element rendering the "상세보기" text currently uses
a placeholder href="#" which doesn't navigate; update the Link in
src/app/(admin)/admin/(app)/groups/[id]/members/page.tsx (the Link with
className "text-Gray-7 underline underline-offset-2 hover:opacity-70" and label
"상세보기") to either point to the actual member detail route (e.g., a dynamic path
using the member id) or remove/replace it with a non-interactive element (like a
span or a disabled button) until the detail page is implemented so it doesn't
act as a dead link.
- Around line 59-61: The effect that begins with useEffect(() => { if (!clubId
|| Number.isNaN(clubId)) return; } ) returns early on an invalid clubId but
leaves loading true and no error set; update that early-return branch to call
setError with a helpful message (e.g., "Invalid club ID") and setLoading(false)
before returning so the UI stops showing the perpetual loading state; adjust
references in the patch to the existing setError and setLoading state setters
used in this component.
In `@src/app/`(admin)/admin/(app)/groups/[id]/page.tsx:
- Around line 239-271: handleSave currently calls updateAdminClub and
router.refresh() but doesn't update the local group/form state, leaving `group`
stale and causing isChanged to be incorrect; after a successful update (inside
the try block, after updateAdminClub and before setting success message) fetch
or use the new data to update the local state by calling the setter(s) that hold
the group and form (e.g., setGroup(...) and setForm(...)) with the updated
payload or the server response so subsequent edits compare against the latest
values; ensure you preserve any server-normalized fields (IDs, timestamps,
profileImageUrl) if you use the payload rather than re-fetching.
In `@src/app/`(admin)/admin/(app)/groups/page.tsx:
- Around line 57-68: filteredGroups currently filters the groups array in
useMemo (function filteredGroups) which only contains the current page of
results, so searches miss items on other pages; update the data flow so the
backend performs the search instead of filtering only the current page: modify
the groups data fetch to accept the keyword (e.g., add a query param in the API
call used to populate groups), pass keyword from the page component to that
fetch, and remove or limit the client-side filtering in useMemo (or apply it
only to the full dataset once server returns matching results); ensure the
components that call the fetch (the hook or loader that provides groups) are
updated to accept and use keyword and that useMemo’s dependency remains [groups,
keyword] if you keep client-side filtering as a fallback.
---
Nitpick comments:
In `@src/app/`(admin)/admin/(app)/groups/[id]/members/page.tsx:
- Around line 69-72: Currently fetchAdminClubDetail(clubId) is being called
inside the Promise.all together with fetchAdminClubActiveMembers on every
pagination change; move the club detail fetch out so it runs only once (or only
when clubId changes) and keep the paginated member fetch for page changes.
Concretely, call fetchAdminClubDetail(clubId) separately (e.g., in an effect or
a higher-level loader) to set clubDetail/groupName, and then call
fetchAdminClubActiveMembers(clubId, page - 1, pageSize) on pagination; update
the code that currently destructures const [clubDetail, membersResult] = await
Promise.all([...]) to two distinct calls so groupName is not re-fetched on each
page.
In `@src/app/`(admin)/admin/(app)/groups/[id]/page.tsx:
- Around line 18-43: CATEGORY_OPTIONS and TARGET_OPTIONS are duplicated here;
replace them by importing the canonical constants from the existing modules and
remove the local arrays. Locate the local symbols CATEGORY_OPTIONS and
TARGET_OPTIONS in page.tsx, import the matching exports from the shared
constants (the categories constant and the group target constant exported by
your constants/modules), and use those imports in place of the local arrays; if
the shared values use different shape, map/transform them to { label, value } at
the import site so all existing consumers of CATEGORY_OPTIONS and TARGET_OPTIONS
keep the same shape.
In `@src/app/`(admin)/admin/(app)/groups/page.tsx:
- Around line 44-46: The useEffect currently calls loadGroups(page) but omits
loadGroups from the dependency array, risking a stale closure; fix by either
moving the loadGroups implementation into the useEffect as an inline async
function that uses page, or memoize loadGroups with useCallback and include
loadGroups in the dependency array (e.g., ensure loadGroups and page are listed
in useEffect dependencies). Update the file so the effect depends on the stable
loadGroups reference or uses the inline loader to preserve correct behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5e6f689-2cda-49e0-8386-3e0db1adc3fc
📒 Files selected for processing (8)
src/app/(admin)/admin/(app)/groups/[id]/members/page.tsxsrc/app/(admin)/admin/(app)/groups/[id]/page.tsxsrc/app/(admin)/admin/(app)/groups/page.tsxsrc/components/base-ui/Admin/groups/Category.tsxsrc/components/base-ui/Admin/groups/Target.tsxsrc/components/base-ui/Admin/groups/TopItem.tsxsrc/lib/api/admin/clubs.tssrc/lib/api/admin/endpoints/clubs.ts
Comment on lines
+59
to
+61
| useEffect(() => { | ||
| if (!clubId || Number.isNaN(clubId)) return; | ||
|
|
There was a problem hiding this comment.
Missing error state when clubId is invalid.
When clubId is NaN, the effect returns early but loading remains true and no error is set. The user will see a perpetual "불러오는 중..." state.
🔧 Proposed fix
useEffect(() => {
- if (!clubId || Number.isNaN(clubId)) return;
+ if (!clubId || Number.isNaN(clubId)) {
+ setError("올바르지 않은 모임 ID입니다.");
+ setLoading(false);
+ return;
+ }
let mounted = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(admin)/admin/(app)/groups/[id]/members/page.tsx around lines 59 -
61, The effect that begins with useEffect(() => { if (!clubId ||
Number.isNaN(clubId)) return; } ) returns early on an invalid clubId but leaves
loading true and no error set; update that early-return branch to call setError
with a helpful message (e.g., "Invalid club ID") and setLoading(false) before
returning so the UI stops showing the perpetual loading state; adjust references
in the patch to the existing setError and setLoading state setters used in this
component.
Comment on lines
+206
to
+211
| <Link | ||
| href="#" | ||
| className="text-Gray-7 underline underline-offset-2 hover:opacity-70" | ||
| > | ||
| 상세보기 | ||
| </Link> |
There was a problem hiding this comment.
Placeholder link with href="#".
The "상세보기" link uses href="#" which doesn't navigate anywhere meaningful. If member detail pages aren't implemented yet, consider disabling the link or removing it temporarily.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(admin)/admin/(app)/groups/[id]/members/page.tsx around lines 206 -
211, The Link element rendering the "상세보기" text currently uses a placeholder
href="#" which doesn't navigate; update the Link in
src/app/(admin)/admin/(app)/groups/[id]/members/page.tsx (the Link with
className "text-Gray-7 underline underline-offset-2 hover:opacity-70" and label
"상세보기") to either point to the actual member detail route (e.g., a dynamic path
using the member id) or remove/replace it with a non-interactive element (like a
span or a disabled button) until the detail page is implemented so it doesn't
act as a dead link.
Comment on lines
+239
to
+271
| const handleSave = async () => { | ||
| if (!form || !isValid || !isChanged) return; | ||
|
|
||
| try { | ||
| setSaving(true); | ||
| setError(""); | ||
| setSuccessMessage(""); | ||
|
|
||
| const payload: UpdateAdminClubRequest = { | ||
| ...form, | ||
| name: form.name.trim(), | ||
| description: form.description.trim(), | ||
| profileImageUrl: form.profileImageUrl.trim(), | ||
| region: form.region.trim(), | ||
| links: form.links | ||
| .map((item) => ({ | ||
| label: item.label.trim(), | ||
| link: item.link.trim(), | ||
| })) | ||
| .filter((item) => item.label !== "" && item.link !== ""), | ||
| }; | ||
|
|
||
| await updateAdminClub(groupId, payload); | ||
| router.refresh(); | ||
| setSuccessMessage("모임 정보가 저장되었습니다."); | ||
| } catch (err) { | ||
| setError( | ||
| err instanceof Error ? err.message : "모임 수정에 실패했습니다.", | ||
| ); | ||
| } finally { | ||
| setSaving(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Form state not updated after successful save.
After updateAdminClub succeeds, router.refresh() is called but the local group state is not updated with the new server data. This means subsequent edits will compare against the stale initial group state, causing isChanged to behave incorrectly.
🔧 Proposed fix
const payload: UpdateAdminClubRequest = {
// ...
};
- await updateAdminClub(groupId, payload);
+ const updatedClub = await updateAdminClub(groupId, payload);
+ setGroup(updatedClub);
+ setForm(normalizeClubToForm(updatedClub));
router.refresh();
setSuccessMessage("모임 정보가 저장되었습니다.");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(admin)/admin/(app)/groups/[id]/page.tsx around lines 239 - 271,
handleSave currently calls updateAdminClub and router.refresh() but doesn't
update the local group/form state, leaving `group` stale and causing isChanged
to be incorrect; after a successful update (inside the try block, after
updateAdminClub and before setting success message) fetch or use the new data to
update the local state by calling the setter(s) that hold the group and form
(e.g., setGroup(...) and setForm(...)) with the updated payload or the server
response so subsequent edits compare against the latest values; ensure you
preserve any server-normalized fields (IDs, timestamps, profileImageUrl) if you
use the payload rather than re-fetching.
[refactor] 댓글 삭제 로직 finally 적용 및 중복 코드 제거
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/app/(admin)/admin/(app)/groups/[id]/page.tsx (1)
261-263:⚠️ Potential issue | 🟠 MajorUpdate local state with the saved club.
The saved response is ignored here, so
groupstays as the pre-save snapshot andisChangedstaystrueafter a successful save.🛠️ Proposed fix
- await updateAdminClub(groupId, payload); + const updatedClub = await updateAdminClub(groupId, payload); + setGroup(updatedClub); + setForm(normalizeClubToForm(updatedClub)); router.refresh(); setSuccessMessage("모임 정보가 저장되었습니다.");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(admin)/admin/(app)/groups/[id]/page.tsx around lines 261 - 263, The save response from updateAdminClub is ignored so local state remains stale; capture the returned saved club (e.g., const saved = await updateAdminClub(groupId, payload)), then update the local group state (call setGroup(saved) or merge saved into current group) and reset the change flag (setIsChanged(false)) before/after calling router.refresh() and setSuccessMessage so the UI reflects the saved data and isChanged becomes false.src/app/(admin)/admin/(app)/groups/[id]/members/page.tsx (2)
59-61:⚠️ Potential issue | 🟡 MinorHandle invalid route params before returning.
This branch exits without clearing
loading, so the page can stay on"불러오는 중..."forever.🛠️ Proposed fix
useEffect(() => { - if (!clubId || Number.isNaN(clubId)) return; + if (!clubId || Number.isNaN(clubId)) { + setError("올바르지 않은 모임 ID입니다."); + setLoading(false); + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(admin)/admin/(app)/groups/[id]/members/page.tsx around lines 59 - 61, The useEffect that checks route param clubId returns early on invalid values but never updates the loading state; update the invalid-params branch in the useEffect (the block surrounding clubId and Number.isNaN(clubId)) to handle invalid route params by clearing loading and handling the error path (e.g., call setLoading(false) and set an error state or redirect) so the page doesn't remain stuck on "불러오는 중...". Ensure you modify the same useEffect where clubId is referenced so the early-return path updates loading via setLoading (and/or setError or router.push).
206-211:⚠️ Potential issue | 🟡 MinorReplace the placeholder member link.
href="#"still renders as an interactive link with no destination. Use the real member detail route or render plain text until that route exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(admin)/admin/(app)/groups/[id]/members/page.tsx around lines 206 - 211, The Link component rendering "상세보기" uses a placeholder href="#" which produces an interactive but non-functional link; update the Link in the members list (the element with text "상세보기" and className "text-Gray-7 underline underline-offset-2 hover:opacity-70") to use the real member detail route (e.g., replace href="#" with the actual route that includes the group and member IDs such as `/groups/${groupId}/members/${memberId}` or the app's equivalent), or if the route does not yet exist, replace the Link with a non-interactive element (e.g., a span/button styled identically) so it no longer behaves like a dead link.
🧹 Nitpick comments (1)
src/app/(admin)/admin/(app)/groups/[id]/members/page.tsx (1)
69-72: Stop refetching club detail on every page change.
groupNameonly depends onclubId, but pagination currently waits on a freshfetchAdminClubDetailcall every time. Splitting that request out prevents redundant traffic and avoids breaking member pagination because of an unrelated detail fetch failure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(admin)/admin/(app)/groups/[id]/members/page.tsx around lines 69 - 72, The current Promise.all refetches fetchAdminClubDetail on every page change; instead call fetchAdminClubDetail(clubId) once and reuse its result (clubDetail) while only refetching fetchAdminClubActiveMembers(clubId, page - 1, pageSize) when pagination changes—i.e., remove fetchAdminClubDetail from the Promise.all, fetch and assign clubDetail separately (once) using the existing fetchAdminClubDetail symbol, then await only fetchAdminClubActiveMembers for page updates (using membersResult) so pagination no longer depends on the club detail request.
🤖 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)/groups/[id]/page.tsx:
- Around line 346-350: The region and link inputs lack accessible names; add
explicit labels or aria attributes and unique ids so screen readers can identify
them. For the region input (where value={form.region} and onChange calls
handleRegionChange), add an id (e.g., id="region-input") and either a visible
<label htmlFor="region-input">Region</label> or aria-label/aria-labelledby on
the input; do the same for both link inputs referenced in the block around lines
375-391 (give each input a unique id and associate a <label> or aria-label, or
use aria-labelledby that points to a descriptive text element). Ensure the label
text is meaningful and update any CSS/layout so the label can be hidden visually
if needed but remains accessible.
In `@src/app/`(admin)/admin/(app)/groups/page.tsx:
- Around line 23-47: The loadGroups function can apply stale async responses
(from fetchAdminClubs) and overwrite state after a newer request; prevent this
by tracking and validating a per-call token or AbortController: when initiating
loadGroups (called by the useEffect watching page and searchKeyword) create a
local requestId or AbortController, store the current token in a ref (e.g.,
currentRequestIdRef) or attach the controller to cancel previous requests, pass
the controller.signal or capture the id in the async closure, and only call
setGroups/setTotalPages/setLoading when the response matches the latest token
(or when the request isn't aborted). Ensure the previous request is cancelled or
ignored in the finally block so stale responses do not update state.
In `@src/lib/api/admin/clubs.ts`:
- Around line 88-93: fetchAdminClubs uses a 1-based page default but calls
ADMIN_CLUBS.list which expects a 0-based page, causing an off-by-one on first
load; update fetchAdminClubs to convert the incoming page to the 0-based
convention before calling ADMIN_CLUBS.list (e.g. compute zeroBased = Math.max(0,
page - 1) and pass that to ADMIN_CLUBS.list) so UI page state and backend page
indexing are consistent.
In `@src/lib/api/admin/endpoints/clubs.ts`:
- Around line 4-5: The URL builder for the list endpoint doesn't encode the
keyword, so special characters like & or = break the query; update the list
function to pass keyword through encodeURIComponent (e.g.,
encodeURIComponent(keyword ?? "")) when constructing the query string so the
keyword is safely encoded in
`${API_BASE_URL}/admin/clubs?page=${page}&size=${size}&keyword=${encodedKeyword}`;
ensure you reference and update the list property used to build the URL.
---
Duplicate comments:
In `@src/app/`(admin)/admin/(app)/groups/[id]/members/page.tsx:
- Around line 59-61: The useEffect that checks route param clubId returns early
on invalid values but never updates the loading state; update the invalid-params
branch in the useEffect (the block surrounding clubId and Number.isNaN(clubId))
to handle invalid route params by clearing loading and handling the error path
(e.g., call setLoading(false) and set an error state or redirect) so the page
doesn't remain stuck on "불러오는 중...". Ensure you modify the same useEffect where
clubId is referenced so the early-return path updates loading via setLoading
(and/or setError or router.push).
- Around line 206-211: The Link component rendering "상세보기" uses a placeholder
href="#" which produces an interactive but non-functional link; update the Link
in the members list (the element with text "상세보기" and className "text-Gray-7
underline underline-offset-2 hover:opacity-70") to use the real member detail
route (e.g., replace href="#" with the actual route that includes the group and
member IDs such as `/groups/${groupId}/members/${memberId}` or the app's
equivalent), or if the route does not yet exist, replace the Link with a
non-interactive element (e.g., a span/button styled identically) so it no longer
behaves like a dead link.
In `@src/app/`(admin)/admin/(app)/groups/[id]/page.tsx:
- Around line 261-263: The save response from updateAdminClub is ignored so
local state remains stale; capture the returned saved club (e.g., const saved =
await updateAdminClub(groupId, payload)), then update the local group state
(call setGroup(saved) or merge saved into current group) and reset the change
flag (setIsChanged(false)) before/after calling router.refresh() and
setSuccessMessage so the UI reflects the saved data and isChanged becomes false.
---
Nitpick comments:
In `@src/app/`(admin)/admin/(app)/groups/[id]/members/page.tsx:
- Around line 69-72: The current Promise.all refetches fetchAdminClubDetail on
every page change; instead call fetchAdminClubDetail(clubId) once and reuse its
result (clubDetail) while only refetching fetchAdminClubActiveMembers(clubId,
page - 1, pageSize) when pagination changes—i.e., remove fetchAdminClubDetail
from the Promise.all, fetch and assign clubDetail separately (once) using the
existing fetchAdminClubDetail symbol, then await only
fetchAdminClubActiveMembers for page updates (using membersResult) so pagination
no longer depends on the club detail request.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e15c89db-2dab-430d-9f3d-e5a57ae84d42
📒 Files selected for processing (5)
src/app/(admin)/admin/(app)/groups/[id]/members/page.tsxsrc/app/(admin)/admin/(app)/groups/[id]/page.tsxsrc/app/(admin)/admin/(app)/groups/page.tsxsrc/lib/api/admin/clubs.tssrc/lib/api/admin/endpoints/clubs.ts
Comment on lines
+346
to
+350
| <input | ||
| type="text" | ||
| value={form.region} | ||
| onChange={(e) => handleRegionChange(e.target.value)} | ||
| className="w-full text-Gray-7 text-[18px] font-normal leading-[135%] tracking-[-0.018px] outline-none bg-transparent" |
There was a problem hiding this comment.
Give these inputs an accessible name.
The region field and both link fields rely on nearby text or placeholders only, so screen readers do not get a stable label for the controls.
🛠️ Proposed fix
<div className="w-full h-[48px] rounded-[8px] border border-Subbrown-4 bg-White px-4 flex items-center">
<input
type="text"
+ aria-label="활동 지역"
value={form.region}
onChange={(e) => handleRegionChange(e.target.value)}
className="w-full text-Gray-7 text-[18px] font-normal leading-[135%] tracking-[-0.018px] outline-none bg-transparent"
/>
@@
<input
type="text"
+ aria-label={`링크 대체 텍스트 ${index + 1}`}
value={item.label ?? ""}
onChange={(e) =>
handleLinkChange(index, "label", e.target.value)
}
placeholder="링크 대체 텍스트 입력(최대 20자)"
className="flex-1 h-[56px] rounded-[8px] border border-Subbrown-4 bg-White px-4 text-[18px] outline-none"
/>
<input
type="text"
+ aria-label={`링크 URL ${index + 1}`}
value={item.link}
onChange={(e) =>
handleLinkChange(index, "link", e.target.value)
}
placeholder="링크 입력(최대 100자)"
className="flex-1 h-[56px] rounded-[8px] border border-Subbrown-4 bg-White px-4 text-[18px] outline-none"
/>Also applies to: 375-391
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(admin)/admin/(app)/groups/[id]/page.tsx around lines 346 - 350, The
region and link inputs lack accessible names; add explicit labels or aria
attributes and unique ids so screen readers can identify them. For the region
input (where value={form.region} and onChange calls handleRegionChange), add an
id (e.g., id="region-input") and either a visible <label
htmlFor="region-input">Region</label> or aria-label/aria-labelledby on the
input; do the same for both link inputs referenced in the block around lines
375-391 (give each input a unique id and associate a <label> or aria-label, or
use aria-labelledby that points to a descriptive text element). Ensure the label
text is meaningful and update any CSS/layout so the label can be hidden visually
if needed but remains accessible.
Comment on lines
+23
to
+47
| const loadGroups = async (targetPage: number, targetKeyword: string) => { | ||
| try { | ||
| setLoading(true); | ||
| setError(""); | ||
|
|
||
| return groups.filter((g) => { | ||
| return ( | ||
| String(g.id).includes(q) || | ||
| g.name.toLowerCase().includes(q) || | ||
| g.ownerEmail.toLowerCase().includes(q) | ||
| const data = await fetchAdminClubs(targetPage, pageSize, targetKeyword); | ||
|
|
||
| setGroups(data.clubs ?? []); | ||
| setTotalPages(Math.max(1, data.totalPages ?? 1)); | ||
| } catch (err) { | ||
| setError( | ||
| err instanceof Error | ||
| ? err.message | ||
| : "모임 목록을 불러오지 못했습니다.", | ||
| ); | ||
| }); | ||
| }, [groups, keyword]); | ||
| setGroups([]); | ||
| setTotalPages(1); | ||
| } finally { | ||
| setLoading(false); | ||
| } | ||
| }; | ||
|
|
||
| const totalPages = Math.max(1, Math.ceil(filtered.length / pageSize)); | ||
| useEffect(() => { | ||
| loadGroups(page, searchKeyword); | ||
| }, [page, searchKeyword]); |
There was a problem hiding this comment.
Discard stale list responses.
A slower earlier request can resolve after a later search or page request and overwrite groups, totalPages, and loading with stale state.
🛠️ Proposed fix
- const loadGroups = async (targetPage: number, targetKeyword: string) => {
- try {
- setLoading(true);
- setError("");
-
- const data = await fetchAdminClubs(targetPage, pageSize, targetKeyword);
-
- setGroups(data.clubs ?? []);
- setTotalPages(Math.max(1, data.totalPages ?? 1));
- } catch (err) {
- setError(
- err instanceof Error
- ? err.message
- : "모임 목록을 불러오지 못했습니다.",
- );
- setGroups([]);
- setTotalPages(1);
- } finally {
- setLoading(false);
- }
- };
-
- useEffect(() => {
- loadGroups(page, searchKeyword);
- }, [page, searchKeyword]);
+ useEffect(() => {
+ let cancelled = false;
+
+ async function loadGroups() {
+ try {
+ setLoading(true);
+ setError("");
+
+ const data = await fetchAdminClubs(page, pageSize, searchKeyword);
+ if (cancelled) return;
+
+ setGroups(data.clubs ?? []);
+ setTotalPages(Math.max(1, data.totalPages ?? 1));
+ } catch (err) {
+ if (cancelled) return;
+
+ setError(
+ err instanceof Error
+ ? err.message
+ : "모임 목록을 불러오지 못했습니다.",
+ );
+ setGroups([]);
+ setTotalPages(1);
+ } finally {
+ if (!cancelled) setLoading(false);
+ }
+ }
+
+ loadGroups();
+
+ return () => {
+ cancelled = true;
+ };
+ }, [page, searchKeyword]);📝 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
| const loadGroups = async (targetPage: number, targetKeyword: string) => { | |
| try { | |
| setLoading(true); | |
| setError(""); | |
| return groups.filter((g) => { | |
| return ( | |
| String(g.id).includes(q) || | |
| g.name.toLowerCase().includes(q) || | |
| g.ownerEmail.toLowerCase().includes(q) | |
| const data = await fetchAdminClubs(targetPage, pageSize, targetKeyword); | |
| setGroups(data.clubs ?? []); | |
| setTotalPages(Math.max(1, data.totalPages ?? 1)); | |
| } catch (err) { | |
| setError( | |
| err instanceof Error | |
| ? err.message | |
| : "모임 목록을 불러오지 못했습니다.", | |
| ); | |
| }); | |
| }, [groups, keyword]); | |
| setGroups([]); | |
| setTotalPages(1); | |
| } finally { | |
| setLoading(false); | |
| } | |
| }; | |
| const totalPages = Math.max(1, Math.ceil(filtered.length / pageSize)); | |
| useEffect(() => { | |
| loadGroups(page, searchKeyword); | |
| }, [page, searchKeyword]); | |
| useEffect(() => { | |
| let cancelled = false; | |
| async function loadGroups() { | |
| try { | |
| setLoading(true); | |
| setError(""); | |
| const data = await fetchAdminClubs(page, pageSize, searchKeyword); | |
| if (cancelled) return; | |
| setGroups(data.clubs ?? []); | |
| setTotalPages(Math.max(1, data.totalPages ?? 1)); | |
| } catch (err) { | |
| if (cancelled) return; | |
| setError( | |
| err instanceof Error | |
| ? err.message | |
| : "모임 목록을 불러오지 못했습니다.", | |
| ); | |
| setGroups([]); | |
| setTotalPages(1); | |
| } finally { | |
| if (!cancelled) setLoading(false); | |
| } | |
| } | |
| loadGroups(); | |
| return () => { | |
| cancelled = true; | |
| }; | |
| }, [page, searchKeyword]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(admin)/admin/(app)/groups/page.tsx around lines 23 - 47, The
loadGroups function can apply stale async responses (from fetchAdminClubs) and
overwrite state after a newer request; prevent this by tracking and validating a
per-call token or AbortController: when initiating loadGroups (called by the
useEffect watching page and searchKeyword) create a local requestId or
AbortController, store the current token in a ref (e.g., currentRequestIdRef) or
attach the controller to cancel previous requests, pass the controller.signal or
capture the id in the async closure, and only call
setGroups/setTotalPages/setLoading when the response matches the latest token
(or when the request isn't aborted). Ensure the previous request is cancelled or
ignored in the finally block so stale responses do not update state.
Comment on lines
+88
to
+93
| export async function fetchAdminClubs( | ||
| page = 1, | ||
| size = 20, | ||
| keyword = "", | ||
| ) { | ||
| const res = await fetch(ADMIN_CLUBS.list(page, size, keyword), { |
There was a problem hiding this comment.
Make fetchAdminClubs use one page-number convention.
This helper defaults to a 1-based page but forwards it to a 0-based endpoint builder. The new groups page passes UI page state straight in here, so the first load can miss the first backend page.
🛠️ One safe fix
export async function fetchAdminClubs(
- page = 1,
+ uiPage = 1,
size = 20,
keyword = "",
) {
- const res = await fetch(ADMIN_CLUBS.list(page, size, keyword), {
+ const page = Math.max(0, uiPage - 1);
+
+ const res = await fetch(ADMIN_CLUBS.list(page, size, keyword), {
method: "GET",
credentials: "include",
cache: "no-store",
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/api/admin/clubs.ts` around lines 88 - 93, fetchAdminClubs uses a
1-based page default but calls ADMIN_CLUBS.list which expects a 0-based page,
causing an off-by-one on first load; update fetchAdminClubs to convert the
incoming page to the 0-based convention before calling ADMIN_CLUBS.list (e.g.
compute zeroBased = Math.max(0, page - 1) and pass that to ADMIN_CLUBS.list) so
UI page state and backend page indexing are consistent.
Comment on lines
+4
to
+5
| list: (page = 0, size = 20, keyword = "") => | ||
| `${API_BASE_URL}/admin/clubs?page=${page}&size=${size}&keyword=${keyword}`, |
There was a problem hiding this comment.
Encode keyword before building the list URL.
Raw search text can split this query at &/=, so terms like R&D stop matching the intended keyword and can mutate the rest of the request.
🛠️ Proposed fix
export const ADMIN_CLUBS = {
- list: (page = 0, size = 20, keyword = "") =>
- `${API_BASE_URL}/admin/clubs?page=${page}&size=${size}&keyword=${keyword}`,
+ list: (page = 0, size = 20, keyword = "") => {
+ const params = new URLSearchParams({
+ page: String(page),
+ size: String(size),
+ keyword,
+ });
+
+ return `${API_BASE_URL}/admin/clubs?${params.toString()}`;
+ },📝 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
| list: (page = 0, size = 20, keyword = "") => | |
| `${API_BASE_URL}/admin/clubs?page=${page}&size=${size}&keyword=${keyword}`, | |
| list: (page = 0, size = 20, keyword = "") => { | |
| const params = new URLSearchParams({ | |
| page: String(page), | |
| size: String(size), | |
| keyword, | |
| }); | |
| return `${API_BASE_URL}/admin/clubs?${params.toString()}`; | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/api/admin/endpoints/clubs.ts` around lines 4 - 5, The URL builder for
the list endpoint doesn't encode the keyword, so special characters like & or =
break the query; update the list function to pass keyword through
encodeURIComponent (e.g., encodeURIComponent(keyword ?? "")) when constructing
the query string so the keyword is safely encoded in
`${API_BASE_URL}/admin/clubs?page=${page}&size=${size}&keyword=${encodedKeyword}`;
ensure you reference and update the list property used to build the URL.
fix: 소셜 로그인 관리 네이버 로그 수정
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📌 개요 (Summary)
🛠️ 변경 사항 (Changes)
📸 스크린샷 (Screenshots)
(UI 변경 사항이 있다면 첨부해주세요)
✅ 체크리스트 (Checklist)
pnpm build)pnpm lint)Summary by CodeRabbit
New Features
Improvements