fix: 게시글 폼 공통화 및 차록 태그 수정하기 지원#245
Open
FLYLIKEB wants to merge 1 commit into
Open
Conversation
- PostForm 공통 컴포넌트 추출 (NewPost/EditPost 동일 폼 공유) - EditPost에 차록 태그 UI 추가 (기존 taggedNotes 초기화 포함) - 차록 검색 null-safe 처리 (teaName undefined 시 오류 수정) - 검색 결과 없음 메시지 추가 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughPostForm 컴포넌트를 신규 추가하여 글 작성/편집 폼을 통합하고, NewPost와 EditPost 페이지를 리팩토링했습니다. PostForm은 카테고리, 제목, 내용, 이미지, 태그, 고정 및 스폰서 옵션 등 모든 필드를 관리합니다. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/pages/EditPost.tsx (1)
72-76: 타입 어노테이션이 부정확합니다
post.taggedNotes는Pick<Note, 'id' | 'teaName' | 'overallRating' | 'createdAt'>[]타입이지만,Note타입으로 명시되어 있습니다. 런타임 오류는 없지만 타입 정확성을 위해 수정하는 것이 좋습니다.♻️ 타입 어노테이션 수정 제안
- const initialTaggedNotes = (post.taggedNotes ?? []).map((n: Note) => ({ + const initialTaggedNotes = (post.taggedNotes ?? []).map((n) => ({ id: n.id, teaName: n.teaName, overallRating: n.overallRating, }));타입 추론이 자동으로 동작하므로 명시적 타입 어노테이션을 제거하면 됩니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/EditPost.tsx` around lines 72 - 76, The explicit type annotation "(n: Note)" on the mapped post.taggedNotes is inaccurate because post.taggedNotes is a Pick<Note, 'id' | 'teaName' | 'overallRating' | 'createdAt'>[]; remove the explicit "(n: Note)" annotation in the initialTaggedNotes mapping so TypeScript can infer the correct narrower type from post.taggedNotes (refer to initialTaggedNotes and post.taggedNotes in EditPost.tsx).src/pages/NewPost.tsx (1)
22-28:isPinned처리가EditPost와 일관성이 없습니다
EditPost에서는isAdmin ? values.isPinned : undefined로 관리자 권한을 명시적으로 확인하지만,NewPost에서는values.isPinned || undefined만 사용합니다.PostForm에서 UI가 관리자에게만 표시되지만, 클라이언트 측에서도 일관된 방어적 코딩을 권장합니다.♻️ 일관된 권한 체크 적용
+ const { user, isAdmin } = useAuth(); - const { user } = useAuth(); const [isSubmitting, setIsSubmitting] = useState(false); // ... handleSubmit 내부 const post = await postsApi.create({ ...values, - isPinned: values.isPinned || undefined, + isPinned: isAdmin ? values.isPinned : undefined, sponsorNote: values.isSponsored ? values.sponsorNote.trim() || undefined : undefined,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pages/NewPost.tsx` around lines 22 - 28, NewPost should enforce the same admin-only pin behavior as EditPost: replace the current isPinned: values.isPinned || undefined when calling postsApi.create with the same admin guard used in EditPost (e.g., isPinned: isAdmin ? values.isPinned : undefined), making sure to reference the same isAdmin source (prop/context/hook) used in EditPost so the client enforces admin-only pinning consistently; update NewPost component before calling postsApi.create to use that guarded expression.src/components/PostForm.tsx (1)
106-114: 에러 발생 시 사용자 피드백 누락
.catch(() => {})로 에러를 무시하고 있어 사용자가 노트 로드 실패를 인지하지 못합니다. 최소한 토스트 알림을 표시하는 것이 좋습니다.♻️ 에러 처리 개선 제안
useEffect(() => { if (!notePickerOpen || myNotes.length > 0 || !user) return; notesApi .getAll(user.id, undefined, undefined, undefined, undefined, undefined, 1, 100) .then((notes) => setMyNotes(notes.map((n) => ({ id: n.id, teaName: n.teaName, overallRating: n.overallRating }))), ) - .catch(() => {}); + .catch(() => toast.error('차록을 불러오는 데 실패했습니다.')); }, [notePickerOpen, user]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/PostForm.tsx` around lines 106 - 114, In PostForm.tsx the notesApi.getAll call swallows errors with .catch(() => {}), so update the catch to surface failures to the user and log the error: call the app's toast/error UI (e.g., toast.error or showToast) with a clear message like "Failed to load notes" and include/log the caught error, and still avoid leaving the component state inconsistent; if no global toast helper exists, set a local error state (e.g., notesLoadError) and render a visible message in the note picker. Ensure you modify the .catch on notesApi.getAll (referencing notePickerOpen, myNotes, user, setMyNotes) rather than removing the overall useEffect.
🤖 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/pages/NewPost.tsx`:
- Around line 14-17: The current NewPost component calls navigate('/login', {
replace: true }) during render when user is falsy; move that redirect into a
useEffect inside NewPost so navigation runs after render: add useEffect(() => {
if (!user) navigate('/login', { replace: true }); }, [user, navigate]) and
render a null or loading placeholder while user is undefined to avoid calling
navigate during render.
---
Nitpick comments:
In `@src/components/PostForm.tsx`:
- Around line 106-114: In PostForm.tsx the notesApi.getAll call swallows errors
with .catch(() => {}), so update the catch to surface failures to the user and
log the error: call the app's toast/error UI (e.g., toast.error or showToast)
with a clear message like "Failed to load notes" and include/log the caught
error, and still avoid leaving the component state inconsistent; if no global
toast helper exists, set a local error state (e.g., notesLoadError) and render a
visible message in the note picker. Ensure you modify the .catch on
notesApi.getAll (referencing notePickerOpen, myNotes, user, setMyNotes) rather
than removing the overall useEffect.
In `@src/pages/EditPost.tsx`:
- Around line 72-76: The explicit type annotation "(n: Note)" on the mapped
post.taggedNotes is inaccurate because post.taggedNotes is a Pick<Note, 'id' |
'teaName' | 'overallRating' | 'createdAt'>[]; remove the explicit "(n: Note)"
annotation in the initialTaggedNotes mapping so TypeScript can infer the correct
narrower type from post.taggedNotes (refer to initialTaggedNotes and
post.taggedNotes in EditPost.tsx).
In `@src/pages/NewPost.tsx`:
- Around line 22-28: NewPost should enforce the same admin-only pin behavior as
EditPost: replace the current isPinned: values.isPinned || undefined when
calling postsApi.create with the same admin guard used in EditPost (e.g.,
isPinned: isAdmin ? values.isPinned : undefined), making sure to reference the
same isAdmin source (prop/context/hook) used in EditPost so the client enforces
admin-only pinning consistently; update NewPost component before calling
postsApi.create to use that guarded expression.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 56f8388b-ad5c-4d25-88e6-a41aab373df4
📒 Files selected for processing (3)
src/components/PostForm.tsxsrc/pages/EditPost.tsxsrc/pages/NewPost.tsx
Comment on lines
14
to
17
| if (!user) { | ||
| navigate('/login', { replace: true }); | ||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
렌더링 중 navigate() 호출은 React 안티패턴입니다
컴포넌트 렌더링 단계에서 navigate()를 직접 호출하면 React의 렌더링 사이클과 충돌하여 경고나 예기치 않은 동작이 발생할 수 있습니다. useEffect로 이동하거나 상위에서 라우트 가드로 처리해야 합니다.
🐛 useEffect로 리다이렉트 처리 제안
export function NewPost() {
const navigate = useNavigate();
const { user } = useAuth();
const [isSubmitting, setIsSubmitting] = useState(false);
- if (!user) {
- navigate('/login', { replace: true });
- return null;
- }
+ useEffect(() => {
+ if (!user) {
+ navigate('/login', { replace: true });
+ }
+ }, [user, navigate]);
+
+ if (!user) {
+ return null;
+ }📝 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
| if (!user) { | |
| navigate('/login', { replace: true }); | |
| return null; | |
| } | |
| export function NewPost() { | |
| const navigate = useNavigate(); | |
| const { user } = useAuth(); | |
| const [isSubmitting, setIsSubmitting] = useState(false); | |
| useEffect(() => { | |
| if (!user) { | |
| navigate('/login', { replace: true }); | |
| } | |
| }, [user, navigate]); | |
| if (!user) { | |
| return null; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pages/NewPost.tsx` around lines 14 - 17, The current NewPost component
calls navigate('/login', { replace: true }) during render when user is falsy;
move that redirect into a useEffect inside NewPost so navigation runs after
render: add useEffect(() => { if (!user) navigate('/login', { replace: true });
}, [user, navigate]) and render a null or loading placeholder while user is
undefined to avoid calling navigate during render.
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
1 participant
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
PostForm공통 컴포넌트 추출 — NewPost/EditPost가 동일한 폼 공유, 하나만 수정하면 양쪽 반영teaNameundefined일 때.toLowerCase()오류 수정)Test plan
Summary by CodeRabbit
새로운 기능