-
Notifications
You must be signed in to change notification settings - Fork 0
[Feature] #38/mypage #42
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
Conversation
WalkthroughThis update introduces user logout and profile update functionalities. It adds API methods, Redux thunks, and UI logic for logging out and updating member profiles, including nickname availability checks. The Next.js config is updated to allow images from a new domain. State synchronization and error handling are improved in profile editing. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EditPage
participant API
participant Redux
User->>EditPage: Submit profile update
EditPage->>API: checkNickname(nickname)
API-->>EditPage: Nickname status
alt Nickname available
EditPage->>Redux: dispatch(updateProfileThunk)
Redux->>API: updateMemberProfile(profile)
API-->>Redux: Success
Redux-->>EditPage: Update state
else Nickname unavailable
EditPage-->>User: Show alert
end
sequenceDiagram
participant User
participant AccountControlSection
participant Redux
participant API
User->>AccountControlSection: Click 'Logout'
AccountControlSection->>Redux: dispatch(logoutThunk)
Redux->>API: logout()
API-->>Redux: Success
Redux-->>AccountControlSection: Clear tokens, update state
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Nitpick comments (5)
src/components/pages/my/accountControlSection.tsx (1)
5-9: Add error handling to logout function.The logout logic is correct, but consider adding error handling to provide user feedback if logout fails.
const handleLogout = () => { - dispatch(logoutThunk()) + dispatch(logoutThunk()).catch((error) => { + console.error('Logout failed:', error) + // Consider showing user-friendly error message + }) }src/store/thunks/memberThunks.ts (1)
40-46: Remove unnecessary try-catch blockThe catch clause only rethrows the error without any additional handling.
Simplify the thunk by removing the try-catch:
export const updateProfileThunk = (profile: Profile) => async (dispatch: AppDispatch) => { - try { - await updateMemberProfile(profile) - dispatch(updateMypageData(profile)) - return profile - } catch (err) { - throw err - } + await updateMemberProfile(profile) + dispatch(updateMypageData(profile)) + return profile }src/api/member.ts (2)
43-49: Remove unnecessary try-catch blocksBoth functions have catch clauses that only rethrow errors without additional handling.
Simplify both functions:
-export const updateMemberProfile = async (profile: Profile) => { - try { - await authApi.post('/member/profile', profile) - } catch (error) { - throw error - } -} +export const updateMemberProfile = async (profile: Profile) => { + await authApi.post('/member/profile', profile) +} -export const checkNickname = async (nickname: string) => { - try { - const response = await authApi.get<checkNicknameResponse>( - '/member/check-nickname', - { - params: { nickname }, - }, - ) - return response.data - } catch (error) { - throw error - } -} +export const checkNickname = async (nickname: string) => { + const response = await authApi.get<checkNicknameResponse>( + '/member/check-nickname', + { + params: { nickname }, + }, + ) + return response.data +}Also applies to: 51-63
65-69: Follow TypeScript interface naming conventionInterface names should use PascalCase.
-interface checkNicknameResponse { +interface CheckNicknameResponse { isMeaningful: boolean isAvailable: boolean isClean: boolean }Also update the usage on line 53:
- const response = await authApi.get<checkNicknameResponse>( + const response = await authApi.get<CheckNicknameResponse>(src/components/pages/my/edit/editPage.tsx (1)
93-97: Complete the nickname validation logicThe TODO comment indicates incomplete validation logic. Currently only checking
isAvailable, but the response also includesisMeaningfulandisCleanfields.Would you like me to implement comprehensive nickname validation using all three response fields?
- // TODO: 닉네임 사용 가능성 체크, response 요소 3개 적절히 판정 - if (!nicknameCheck.isAvailable) { - alert('이미 사용 중인 닉네임입니다.') - return - } + if (!nicknameCheck.isAvailable) { + alert('이미 사용 중인 닉네임입니다.') + return + } + if (!nicknameCheck.isMeaningful) { + alert('의미 있는 닉네임을 입력해주세요.') + return + } + if (!nicknameCheck.isClean) { + alert('부적절한 닉네임입니다.') + return + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
public/mypage-blue.svgis excluded by!**/*.svg
📒 Files selected for processing (9)
next.config.ts(1 hunks)src/api/auth.ts(1 hunks)src/api/member.ts(1 hunks)src/components/pages/my/accountControlSection.tsx(1 hunks)src/components/pages/my/edit/editPage.tsx(5 hunks)src/components/pages/my/myPage.tsx(1 hunks)src/store/slices/memberSlice.ts(1 hunks)src/store/thunks/authThunks.ts(2 hunks)src/store/thunks/memberThunks.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/api/auth.ts (2)
src/store/slices/authSlice.ts (1)
logout(35-38)src/api/instance/authApi.ts (1)
authApi(16-21)
src/components/pages/my/accountControlSection.tsx (2)
src/hooks/utils/useAppDispatch.ts (1)
useAppDispatch(4-4)src/store/thunks/authThunks.ts (1)
logoutThunk(44-52)
src/store/slices/memberSlice.ts (1)
src/types/_shared/profile.ts (1)
Profile(1-8)
src/api/member.ts (2)
src/types/_shared/profile.ts (1)
Profile(1-8)src/api/instance/authApi.ts (1)
authApi(16-21)
src/store/thunks/memberThunks.ts (4)
src/types/_shared/profile.ts (1)
Profile(1-8)src/store/store.ts (1)
AppDispatch(13-13)src/api/member.ts (1)
updateMemberProfile(43-49)src/store/slices/memberSlice.ts (1)
updateMypageData(33-39)
src/components/pages/my/edit/editPage.tsx (5)
src/hooks/utils/useAppSelector.ts (1)
useAppSelector(4-4)src/types/_shared/profile.ts (1)
MBTI(12-28)src/hooks/utils/useAppDispatch.ts (1)
useAppDispatch(4-4)src/api/member.ts (1)
checkNickname(51-63)src/store/thunks/memberThunks.ts (1)
updateProfileThunk(38-47)
🪛 Biome (1.9.4)
src/api/auth.ts
[error] 18-18: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
src/store/thunks/authThunks.ts
[error] 50-50: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
src/api/member.ts
[error] 47-47: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
[error] 61-61: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
src/store/thunks/memberThunks.ts
[error] 39-39: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🔇 Additional comments (4)
next.config.ts (1)
4-6: LGTM! Proper Next.js image configuration.The images configuration correctly allows external images from Kakao's CDN domain, which is appropriate for profile image handling.
src/store/thunks/authThunks.ts (1)
13-13: LGTM! Proper import of logout API function.The import correctly brings in the logout API function for use in the thunk.
src/components/pages/my/accountControlSection.tsx (2)
1-2: LGTM! Proper imports for logout functionality.The imports correctly bring in the Redux dispatch hook and logout thunk.
15-17: LGTM! Proper onClick integration for logout.The onClick handler is correctly integrated into the logout div element.
| export const logout = async () => { | ||
| try { | ||
| await authApi.post('/auth/logout') | ||
| } catch (error) { | ||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove the useless catch clause.
The catch block only rethrows the original error without adding any value. This is unnecessary and adds complexity.
export const logout = async () => {
- try {
- await authApi.post('/auth/logout')
- } catch (error) {
- throw error
- }
+ await authApi.post('/auth/logout')
}📝 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.
| export const logout = async () => { | |
| try { | |
| await authApi.post('/auth/logout') | |
| } catch (error) { | |
| throw error | |
| } | |
| } | |
| export const logout = async () => { | |
| await authApi.post('/auth/logout') | |
| } |
🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🤖 Prompt for AI Agents
In src/api/auth.ts around lines 14 to 20, remove the try-catch block in the
logout function since the catch clause only rethrows the error without any
additional handling. Simplify the function by directly awaiting the authApi.post
call without the try-catch.
| useEffect(() => { | ||
| console.log('mypageData:', mypageData) | ||
| }, [mypageData]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove debugging code.
This useEffect appears to be for debugging purposes and should be removed before merging to production. Additionally, there's already a console.log for mypageData in the existing useEffect on line 39.
- useEffect(() => {
- console.log('mypageData:', mypageData)
- }, [mypageData])📝 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.
| useEffect(() => { | |
| console.log('mypageData:', mypageData) | |
| }, [mypageData]) |
🤖 Prompt for AI Agents
In src/components/pages/my/myPage.tsx between lines 23 and 25, there is a
useEffect hook that logs mypageData for debugging purposes. Remove this
useEffect entirely to avoid redundant console logging since there is already a
console.log for mypageData in the useEffect on line 39.
| export const logoutThunk = () => async (dispatch: AppDispatch) => { | ||
| try { | ||
| await logoutApi() | ||
| dispatch(logout()) | ||
| clearTokens() | ||
| } catch (err) { | ||
| throw err | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove the useless catch clause.
The logout thunk logic is correct - it properly calls the API, dispatches the logout action, and clears tokens. However, the catch block only rethrows the original error without adding any value.
export const logoutThunk = () => async (dispatch: AppDispatch) => {
- try {
- await logoutApi()
- dispatch(logout())
- clearTokens()
- } catch (err) {
- throw err
- }
+ await logoutApi()
+ dispatch(logout())
+ clearTokens()
}🧰 Tools
🪛 Biome (1.9.4)
[error] 50-50: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🤖 Prompt for AI Agents
In src/store/thunks/authThunks.ts around lines 44 to 52, remove the catch block
from the logoutThunk function since it only rethrows the error without adding
any handling or logging. Keep the try block with the await logoutApi(),
dispatch(logout()), and clearTokens() calls, and let any errors naturally
propagate without an explicit catch.
| state.mypageData.nickname = action.payload.nickname | ||
| state.mypageData.gender = action.payload.gender | ||
| state.mypageData.age = action.payload.age | ||
| state.mypageData.mbti = action.payload.mbti |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential type mismatch between Profile and MypageData fields
The Profile type uses enum types for gender (Gender) and age (Age), while MypageData uses string types. This direct assignment could cause type issues.
Consider adding type conversion to ensure compatibility:
- state.mypageData.nickname = action.payload.nickname
- state.mypageData.gender = action.payload.gender
- state.mypageData.age = action.payload.age
- state.mypageData.mbti = action.payload.mbti
+ state.mypageData.nickname = action.payload.nickname
+ state.mypageData.gender = action.payload.gender as string
+ state.mypageData.age = action.payload.age as string
+ state.mypageData.mbti = action.payload.mbti as string🤖 Prompt for AI Agents
In src/store/slices/memberSlice.ts around lines 35 to 38, the code assigns
enum-typed fields from action.payload (Profile) directly to string-typed fields
in state.mypageData, causing a type mismatch. To fix this, convert the enum
values for gender and age to strings before assignment, ensuring type
compatibility between Profile and MypageData fields.
| const [nickname, setNickname] = useState(myPageData?.nickname) | ||
| const [isNicknameEditing, setIsNicknameEditing] = useState(false) | ||
| const [gender, setGender] = useState<string | null>(responseGender) | ||
| const [age, setAge] = useState<string | null>(responseAge) | ||
| const [gender, setGender] = useState<string | null>( | ||
| myPageData?.gender as string, | ||
| ) | ||
| const [age, setAge] = useState<string | null>(myPageData?.age as string) | ||
| const [mbtiBottomSheetOpen, setMbtiBottomSheetOpen] = useState(false) | ||
| const [mbti, setMbti] = useState<MBTI | null>(responseMBTI) | ||
| const [mbti, setMbti] = useState<MBTI | null>(myPageData?.mbti as MBTI) | ||
| const dispatch = useAppDispatch() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Improve null/undefined handling for state initialization
When myPageData is null, the state values will be undefined, which might cause issues with controlled components.
Consider providing default values:
- const [nickname, setNickname] = useState(myPageData?.nickname)
+ const [nickname, setNickname] = useState(myPageData?.nickname || '')
const [isNicknameEditing, setIsNicknameEditing] = useState(false)
- const [gender, setGender] = useState<string | null>(
- myPageData?.gender as string,
- )
- const [age, setAge] = useState<string | null>(myPageData?.age as string)
+ const [gender, setGender] = useState<string | null>(
+ myPageData?.gender || null,
+ )
+ const [age, setAge] = useState<string | null>(myPageData?.age || null)
const [mbtiBottomSheetOpen, setMbtiBottomSheetOpen] = useState(false)
- const [mbti, setMbti] = useState<MBTI | null>(myPageData?.mbti as MBTI)
+ const [mbti, setMbti] = useState<MBTI | null>(
+ myPageData?.mbti ? (myPageData.mbti as MBTI) : 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.
| const [nickname, setNickname] = useState(myPageData?.nickname) | |
| const [isNicknameEditing, setIsNicknameEditing] = useState(false) | |
| const [gender, setGender] = useState<string | null>(responseGender) | |
| const [age, setAge] = useState<string | null>(responseAge) | |
| const [gender, setGender] = useState<string | null>( | |
| myPageData?.gender as string, | |
| ) | |
| const [age, setAge] = useState<string | null>(myPageData?.age as string) | |
| const [mbtiBottomSheetOpen, setMbtiBottomSheetOpen] = useState(false) | |
| const [mbti, setMbti] = useState<MBTI | null>(responseMBTI) | |
| const [mbti, setMbti] = useState<MBTI | null>(myPageData?.mbti as MBTI) | |
| const dispatch = useAppDispatch() | |
| const [nickname, setNickname] = useState(myPageData?.nickname || '') | |
| const [isNicknameEditing, setIsNicknameEditing] = useState(false) | |
| const [gender, setGender] = useState<string | null>( | |
| myPageData?.gender || null, | |
| ) | |
| const [age, setAge] = useState<string | null>(myPageData?.age || null) | |
| const [mbtiBottomSheetOpen, setMbtiBottomSheetOpen] = useState(false) | |
| const [mbti, setMbti] = useState<MBTI | null>( | |
| myPageData?.mbti ? (myPageData.mbti as MBTI) : null | |
| ) | |
| const dispatch = useAppDispatch() |
🤖 Prompt for AI Agents
In src/components/pages/my/edit/editPage.tsx around lines 48 to 56, the state
initialization uses optional chaining on myPageData which can result in
undefined values if myPageData is null, potentially causing issues with
controlled components. To fix this, provide explicit default values for each
state variable when myPageData is null or undefined, ensuring the state is never
initialized with undefined. For example, use empty strings or null as
appropriate defaults instead of relying solely on optional chaining.
| console.log('전역 상태 nickname', myPageData?.nickname) | ||
| console.log('로컬 상태 nickname', nickname) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove console.log statements
Debug logs should not be included in production code.
useEffect(() => {
- console.log('전역 상태 nickname', myPageData?.nickname)
- console.log('로컬 상태 nickname', nickname)
if (myPageData) {📝 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.
| console.log('전역 상태 nickname', myPageData?.nickname) | |
| console.log('로컬 상태 nickname', nickname) | |
| useEffect(() => { | |
| if (myPageData) { |
🤖 Prompt for AI Agents
In src/components/pages/my/edit/editPage.tsx at lines 59 to 60, remove the
console.log statements that output '전역 상태 nickname' and '로컬 상태 nickname' as
debug logs should not be present in production code.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores