[FEAT] 비밀번호 변경 및 재설정 - #59
Closed
kkhhmm3103 wants to merge 6 commits into
Closed
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 51 minutes and 54 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough비밀번호 변경 및 재설정 기능을 위한 API 모듈, Zod 검증 스키마, React Query 뮤테이션 훅을 추가했습니다. 설정 프로필 섹션에 비밀번호 변경 폼을 통합하고, API 응답 가로채기 로직을 확장했으며, Toss Payments 타입 정의를 추가했습니다. Changes
Sequence DiagramsequenceDiagram
participant User
participant Settings as Settings Profile<br/>Component
participant Validation as Zod<br/>Validation
participant Mutation as Change Password<br/>Mutation
participant API as API Layer<br/>(baseApi)
participant Server as Auth Server
User->>Settings: 현재 비밀번호, 새 비밀번호 입력
User->>Settings: 변경하기 클릭
Settings->>Validation: changePasswordSchema.parse()<br/>(검증)
alt 검증 실패
Validation-->>Settings: 필드 에러 반환
Settings->>User: 에러 메시지 표시
else 검증 성공
Validation-->>Settings: 통과
Settings->>Mutation: mutateAsync(request)
Mutation->>API: POST /auth/password/change
API->>Server: 요청 전송
alt 성공 (200)
Server-->>API: 응답 반환
API-->>Mutation: data 추출
Mutation-->>Settings: 성공
Settings->>User: 성공 메시지 표시<br/>폼 초기화
else 실패 (4xx/5xx)
Server-->>API: 에러 응답
API-->>Mutation: ApiError 발생
Mutation-->>Settings: 에러 코드 반환
Settings->>User: 매핑된 에러 메시지 표시
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 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}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (6)
src/features/auth-dialog/api/change-password.ts (1)
15-18: 성공 응답의 내부data를 반환하도록 맞추세요.현재
data는 Axios 응답 본문 전체({ status, code, message, data })라서 이 함수의 성공값이null이 아니라 응답 envelope입니다. mutation 결과를 사용하는 쪽에서 계약이 어긋나지 않도록Promise<null>로 명시하고 내부data를 반환하세요. 가능하면 기존src/features/watchlist/api/fetch-watchlist.ts:15-19처럼 공통 응답 스키마 파싱도 함께 맞추면 좋습니다.♻️ 반환 계약 정리 예시
-export async function changePassword(request: ChangePasswordRequest) { +export async function changePassword(request: ChangePasswordRequest): Promise<null> { const { data } = await baseApi.post<ApiResponse<null>>('/auth/password/change', request) - return data + return data.data }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/auth-dialog/api/change-password.ts` around lines 15 - 18, The changePassword function currently returns the full Axios envelope (ApiResponse<null>) instead of the inner payload; update changePassword(request: ChangePasswordRequest) to return Promise<null> and, after calling baseApi.post<ApiResponse<null>>('/auth/password/change', request), return the inner response data (e.g., response.data.data) rather than the whole envelope; ensure the signature and return value match the pattern used in fetch-watchlist (parsing ApiResponse<T> and returning its .data field) so callers receive null on success.index.html (1)
8-8: 결제 SDK 로드를 초기 렌더링 경로에서 분리해 주세요.현재 head의 일반
<script>라 Toss CDN이 느리거나 실패하면 앱 파싱/렌더링이 막힐 수 있습니다. 결제 기능은 특정 버튼 클릭 시에만 필요하므로 최소한defer를 붙이거나, 더 좋게는 결제 요청 시점에 지연 로드하는 방식이 안전합니다.♻️ 최소 변경 예시
- <script src="https://js.tosspayments.com/v1/payment"></script> + <script defer src="https://js.tosspayments.com/v1/payment"></script>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.html` at line 8, The inline loading of the Toss Payments SDK via <script src="https://js.tosspayments.com/v1/payment"></script> blocks initial rendering; change this by either adding defer to that script or, preferably, remove the immediate script tag and implement a lazy loader that injects that src only when payment is needed (for example in the payment button handler such as handlePayClick or payButton click callback). Ensure the loader is idempotent (check for an existing script element or window.TossPayments), returns a promise that resolves when the SDK's script load event fires, and only then invokes the payment initiation logic.src/features/payment/model/payment.query.ts (1)
28-33: 비로그인 상태에서useMySubscriptionQuery자동 실행 가능성
useQuery에enabled조건이 없어 훅이 마운트되는 즉시/payments/me/subscription을 호출합니다. 비로그인 상태에서도 호출되면 401이 발생하고base-api.ts의 refresh 플로우가 불필요하게 동작할 수 있습니다. 인증 상태를 확인하는enabled옵션을 추가하는 것을 권장합니다.♻️ 예시 수정
-export function useMySubscriptionQuery() { +export function useMySubscriptionQuery(options?: { enabled?: boolean }) { return useQuery({ queryKey: PAYMENT_QUERY_KEY.mySubscription, queryFn: getMySubscription, - retry: false + retry: false, + enabled: options?.enabled ?? true }) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/payment/model/payment.query.ts` around lines 28 - 33, useMySubscriptionQuery currently calls useQuery immediately causing /payments/me/subscription to run on mount even when unauthenticated; update the hook to pass an enabled condition that checks authentication before executing. Modify useMySubscriptionQuery to add the enabled option (e.g., enabled: isAuthenticated or isLoggedIn) to the useQuery options so it only runs when the user is logged in; reference the existing queryKey PAYMENT_QUERY_KEY.mySubscription and queryFn getMySubscription and ensure any auth-check utility or state used by base-api.ts (token/refresh logic) is used to derive the enabled flag.src/features/payment/model/payment.schema.ts (2)
8-10: 단일값 enum에 대한 의견
provider: z.enum(['TOSS'])는 사실상 리터럴 상수이므로z.literal('TOSS')가 의도를 더 명확히 드러냅니다. 향후 결제 수단이 추가될 예정이라면 현재z.enum형태를 유지해도 무방합니다.Also applies to: 20-22
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/payment/model/payment.schema.ts` around lines 8 - 10, The provider field currently uses z.enum(['TOSS']) which is a single constant; replace it with z.literal('TOSS') to make the intent explicit (update the provider schema entry and any other identical single-value enums elsewhere in this file, e.g., the other provider occurrences around the bottom of the schema). Keep z.enum for billingCycle and other multi-valued enums unchanged; if you expect multiple providers in the future you may leave z.enum as-is instead of switching to z.literal.
28-28: 날짜 필드에 ISO datetime 검증 적용 검토
requestedAt,approvedAt,subscriptionStartedAt,subscriptionExpiredAt,startedAt,expiredAt모두z.string()으로 선언되어 임의 문자열을 허용합니다. Zod 4의z.iso.datetime()을 사용하면 포맷 검증이 강화되어 백엔드 변경에 의한 회귀를 조기에 잡을 수 있습니다.♻️ 예시 수정
- requestedAt: z.string() + requestedAt: z.iso.datetime()Also applies to: 38-39
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/payment/model/payment.schema.ts` at line 28, Replace the loose z.string() definitions for the date/time fields in the payment schema with Zod's datetime validator so formats are enforced: update the properties requestedAt, approvedAt, subscriptionStartedAt, subscriptionExpiredAt, startedAt, and expiredAt in the schema (and the other occurrences referenced around the second block at lines noted in the review) to use z.string().datetime() (or z.iso.datetime() if using that Zod version) instead of plain z.string(), ensuring any nullable/optional chaining (e.g., .optional()/.nullable()) is preserved.src/features/payment/api/payment.api.ts (1)
19-42: Zod 파싱 실패 시 예외 처리 전략 검토
parse()는 응답 스키마가 맞지 않으면ZodError를 던져 React Query의onError로 전달됩니다. 현재 소비자 쪽(subscribe-button.tsx,payment-success-page.tsx)은 일반alert로만 처리되므로 디버깅이 어렵습니다. 다음 중 하나를 고려해 보세요.
safeParse후 실패 시 로깅을 포함한 명시적 에러를 throw- 공통 헬퍼로 감싸서
ZodError→ 사용자 친화적 메시지/Sentry 전송또한 세 함수 모두
apiResponseSchema(...).parse(response.data).data패턴이 반복되므로 공통 헬퍼(parseApiResponse(schema, response))로 묶으면 중복을 줄일 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/payment/api/payment.api.ts` around lines 19 - 42, The three API functions (createPayment, confirmPayment, getMySubscription) repeat apiResponseSchema(...).parse(response.data).data and currently let ZodError bubble to callers; extract this into a shared helper parseApiResponse(schema, response) that uses schema.safeParse(response.data) and on failure logs the ZodError (and send to Sentry/monitoring) and then throws a clearer error (or rethrows a wrapped Error with context such as function name and endpoint), then update createPayment, confirmPayment, and getMySubscription to call parseApiResponse(createPaymentResponseSchema, response) etc.; ensure the helper returns the .data field on success so callers remain unchanged.
🤖 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/router/app-router.tsx`:
- Line 119: Wrap the payment success route so only authenticated users can
access it: change the Route for path "payment/success" that currently renders
PaymentSuccessPage to render it inside your authentication wrapper (e.g.,
ProtectedRoute or whatever auth wrapper component your app uses). Specifically,
locate the Route with element={<PaymentSuccessPage />} and replace it so the
element is the auth wrapper component rendering PaymentSuccessPage (e.g.,
<ProtectedRoute><PaymentSuccessPage/></ProtectedRoute>), keeping the
failure/public routes unchanged.
In `@src/features/auth-dialog/api/reset-password.ts`:
- Around line 10-13: ResetPasswordRequest is missing the required verification
code field causing the server to never receive the reset code; add a "code:
string" property to the ResetPasswordRequest type definition (the exported
ResetPasswordRequest) so it matches the schema in
src/features/auth-dialog/model/reset-password.schema.ts and the HTTP payload
includes email, newPassword and code when calling the reset endpoint.
- Around line 15-16: The public password reset endpoint used in
resetPassword(request: ResetPasswordRequest) is missing from the refresh-token
interceptor's isAuthRequest exception list, causing 401s from
/auth/password/reset to trigger token refresh and mask the real error; update
the interceptor's exception list (the isAuthRequest logic in baseApi) to include
'/auth/password/reset' or change resetPassword to call the public API client
instead so the refresh flow is not invoked for this endpoint.
In `@src/features/payment/ui/payment-success-page.tsx`:
- Around line 14-54: The page currently requires a user click to run
handleConfirm; instead, invoke the same confirm flow once on component mount by
calling confirmPaymentMutation.mutate with the existing payload (paymentKey,
orderId, amount) inside a useEffect in the PaymentSuccessPage component and
reuse the validation logic from handleConfirm (check
paymentKey/orderId/amountParam and amount numeric >0), and to prevent double
execution in React Strict Mode add a useRef flag (e.g., confirmStartedRef)
checked/set inside the effect so mutate runs only once; keep the existing
onSuccess/onError handlers as-is and still navigate on failure/success.
In `@src/features/payment/ui/subscribe-button.tsx`:
- Around line 1-35: The button only uses isPending (from
useCreatePaymentMutation) which is cleared after mutateAsync resolves, allowing
clicks while requestTossPayment is still in progress; add a local busy state
(e.g., isBusy via useState) in SubscribeButton, set isBusy = true before calling
mutateAsync and keep it true through the call to requestTossPayment (use
try/finally around mutateAsync and requestTossPayment in handleClick to reset
isBusy on completion/error), and update the button disabled prop to
disabled={isPending || isBusy} so the whole payment flow (mutateAsync +
requestTossPayment) is protected from duplicate clicks.
In `@src/features/settings/ui/settings-profile-section.tsx`:
- Around line 185-235: The three password inputs (bound to currentPassword,
newPassword, newPasswordConfirm) need accessible label associations: add a
unique id to each input (e.g. currentPasswordInput, newPasswordInput,
newPasswordConfirmInput) and set the corresponding label's htmlFor to that id so
clicking the label focuses the matching <input>; update the <label> elements for
the inputs that render currentPassword, newPassword, and newPasswordConfirm to
use htmlFor and ensure each input includes the matching id attribute.
- Around line 23-30: When a user logs out, sensitive form state
(isPasswordFormOpen, currentPassword, newPassword, newPasswordConfirm, errors,
successMessage) must be cleared but avoid calling setState inside useEffect;
instead render with derived values that fallback when isLoggedIn is false.
Update the component to compute effective values (e.g. const
effectiveIsPasswordFormOpen = isLoggedIn ? isPasswordFormOpen : false and const
effectiveCurrentPassword = isLoggedIn ? currentPassword : '' etc.) and use those
effective* identifiers in the JSX and handlers so the UI shows cleared fields
for logged-out state without performing setState in an effect; ensure
changePasswordMutation usage and any submit handlers reference the effective
values or guard by isLoggedIn.
In `@src/features/settings/ui/settings-view.tsx`:
- Line 89: Replace the hardcoded numeric prop planId={1} on SubscribeButton with
a centrally defined named constant; create a constant or enum (e.g.,
SUBSCRIPTION_PLAN_ID or PLAN_ID.PRO) exported from a single settings/constants
module, update the SubscribeButton usage to pass that named constant
(SubscribeButton planId={SUBSCRIPTION_PLAN_ID.PRO} or similar), and import the
constant into settings-view.tsx so future plan ID changes are made in one place
rather than scattered literals.
In `@src/shared/type/toss-payments.d.ts`:
- Line 5: Window 인터페이스의 TossPayments 선언이 현재 필수로 되어 있어 guard 검사와 불일치하므로 이를
선택적(optional)으로 변경하세요: locate the Window.TossPayments declaration (symbol
TossPayments) in the toss-payments type file and change its signature to be
optional (e.g. TossPayments?: (clientKey: string) => { ... }) so TypeScript
reflects that the external SDK may be undefined and callers are forced to check
(if (!window.TossPayments)) before use.
---
Nitpick comments:
In `@index.html`:
- Line 8: The inline loading of the Toss Payments SDK via <script
src="https://js.tosspayments.com/v1/payment"></script> blocks initial rendering;
change this by either adding defer to that script or, preferably, remove the
immediate script tag and implement a lazy loader that injects that src only when
payment is needed (for example in the payment button handler such as
handlePayClick or payButton click callback). Ensure the loader is idempotent
(check for an existing script element or window.TossPayments), returns a promise
that resolves when the SDK's script load event fires, and only then invokes the
payment initiation logic.
In `@src/features/auth-dialog/api/change-password.ts`:
- Around line 15-18: The changePassword function currently returns the full
Axios envelope (ApiResponse<null>) instead of the inner payload; update
changePassword(request: ChangePasswordRequest) to return Promise<null> and,
after calling baseApi.post<ApiResponse<null>>('/auth/password/change', request),
return the inner response data (e.g., response.data.data) rather than the whole
envelope; ensure the signature and return value match the pattern used in
fetch-watchlist (parsing ApiResponse<T> and returning its .data field) so
callers receive null on success.
In `@src/features/payment/api/payment.api.ts`:
- Around line 19-42: The three API functions (createPayment, confirmPayment,
getMySubscription) repeat apiResponseSchema(...).parse(response.data).data and
currently let ZodError bubble to callers; extract this into a shared helper
parseApiResponse(schema, response) that uses schema.safeParse(response.data) and
on failure logs the ZodError (and send to Sentry/monitoring) and then throws a
clearer error (or rethrows a wrapped Error with context such as function name
and endpoint), then update createPayment, confirmPayment, and getMySubscription
to call parseApiResponse(createPaymentResponseSchema, response) etc.; ensure the
helper returns the .data field on success so callers remain unchanged.
In `@src/features/payment/model/payment.query.ts`:
- Around line 28-33: useMySubscriptionQuery currently calls useQuery immediately
causing /payments/me/subscription to run on mount even when unauthenticated;
update the hook to pass an enabled condition that checks authentication before
executing. Modify useMySubscriptionQuery to add the enabled option (e.g.,
enabled: isAuthenticated or isLoggedIn) to the useQuery options so it only runs
when the user is logged in; reference the existing queryKey
PAYMENT_QUERY_KEY.mySubscription and queryFn getMySubscription and ensure any
auth-check utility or state used by base-api.ts (token/refresh logic) is used to
derive the enabled flag.
In `@src/features/payment/model/payment.schema.ts`:
- Around line 8-10: The provider field currently uses z.enum(['TOSS']) which is
a single constant; replace it with z.literal('TOSS') to make the intent explicit
(update the provider schema entry and any other identical single-value enums
elsewhere in this file, e.g., the other provider occurrences around the bottom
of the schema). Keep z.enum for billingCycle and other multi-valued enums
unchanged; if you expect multiple providers in the future you may leave z.enum
as-is instead of switching to z.literal.
- Line 28: Replace the loose z.string() definitions for the date/time fields in
the payment schema with Zod's datetime validator so formats are enforced: update
the properties requestedAt, approvedAt, subscriptionStartedAt,
subscriptionExpiredAt, startedAt, and expiredAt in the schema (and the other
occurrences referenced around the second block at lines noted in the review) to
use z.string().datetime() (or z.iso.datetime() if using that Zod version)
instead of plain z.string(), ensuring any nullable/optional chaining (e.g.,
.optional()/.nullable()) is preserved.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2ff6810e-0b09-4617-ab5c-21e9b3029dee
📒 Files selected for processing (19)
.env.exampleindex.htmlsrc/app/router/app-router.tsxsrc/features/auth-dialog/api/change-password.tssrc/features/auth-dialog/api/reset-password.tssrc/features/auth-dialog/model/change-password.schema.tssrc/features/auth-dialog/model/reset-password.schema.tssrc/features/auth-dialog/model/use-change-password-mutation.tssrc/features/auth-dialog/model/use-reset-password-mutation.tssrc/features/payment/api/payment.api.tssrc/features/payment/lib/request-toss-payment.tssrc/features/payment/model/payment.query.tssrc/features/payment/model/payment.schema.tssrc/features/payment/ui/payment-fail-page.tsxsrc/features/payment/ui/payment-success-page.tsxsrc/features/payment/ui/subscribe-button.tsxsrc/features/settings/ui/settings-profile-section.tsxsrc/features/settings/ui/settings-view.tsxsrc/shared/type/toss-payments.d.ts
💤 Files with no reviewable changes (1)
- .env.example
Comment on lines
+23
to
+30
| const [isPasswordFormOpen, setIsPasswordFormOpen] = useState(false) | ||
| const [currentPassword, setCurrentPassword] = useState('') | ||
| const [newPassword, setNewPassword] = useState('') | ||
| const [newPasswordConfirm, setNewPasswordConfirm] = useState('') | ||
| const [errors, setErrors] = useState<PasswordFormErrors>({}) | ||
| const [successMessage, setSuccessMessage] = useState('') | ||
|
|
||
| const changePasswordMutation = useChangePasswordMutation() |
There was a problem hiding this comment.
로그아웃 시 비밀번호 입력 상태를 즉시 지우세요.
isLoggedIn이 false가 되면 폼은 숨겨지지만 isPasswordFormOpen과 비밀번호 입력값은 그대로 남습니다. 같은 컴포넌트 인스턴스에서 다시 로그인하면 이전 사용자의 비밀번호 입력값이 다시 노출될 수 있습니다.
🛡️ 인증 상태 변경 시 민감 상태 초기화 예시
const [errors, setErrors] = useState<PasswordFormErrors>({})
const [successMessage, setSuccessMessage] = useState('')
+ const [previousIsLoggedIn, setPreviousIsLoggedIn] = useState(isLoggedIn)
+
+ if (previousIsLoggedIn !== isLoggedIn) {
+ setPreviousIsLoggedIn(isLoggedIn)
+
+ if (!isLoggedIn) {
+ setIsPasswordFormOpen(false)
+ setCurrentPassword('')
+ setNewPassword('')
+ setNewPasswordConfirm('')
+ setErrors({})
+ setSuccessMessage('')
+ }
+ }Based on learnings, 이 코드베이스에서는 react-hooks/set-state-in-effect 규칙 때문에 prop/state 동기화를 useEffect의 setState로 처리하지 말고 previous value를 state에 두고 render 중 조건부로 조정하는 패턴을 사용해야 합니다.
Also applies to: 181-182
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/settings/ui/settings-profile-section.tsx` around lines 23 - 30,
When a user logs out, sensitive form state (isPasswordFormOpen, currentPassword,
newPassword, newPasswordConfirm, errors, successMessage) must be cleared but
avoid calling setState inside useEffect; instead render with derived values that
fallback when isLoggedIn is false. Update the component to compute effective
values (e.g. const effectiveIsPasswordFormOpen = isLoggedIn ? isPasswordFormOpen
: false and const effectiveCurrentPassword = isLoggedIn ? currentPassword : ''
etc.) and use those effective* identifiers in the JSX and handlers so the UI
shows cleared fields for logged-out state without performing setState in an
effect; ensure changePasswordMutation usage and any submit handlers reference
the effective values or guard by isLoggedIn.
Comment on lines
+185
to
+235
| <label className="mb-1 block text-sm font-medium text-wefin-text"> | ||
| 현재 비밀번호 | ||
| </label> | ||
| <input | ||
| type="password" | ||
| value={currentPassword} | ||
| onChange={(e) => setCurrentPassword(e.target.value)} | ||
| placeholder="현재 비밀번호 입력" | ||
| className={`h-10 w-full max-w-[320px] rounded-lg border bg-white px-3 text-sm text-wefin-text outline-none transition-colors placeholder:text-wefin-subtle focus:border-wefin-mint ${ | ||
| errors.currentPassword ? 'border-red-400' : 'border-wefin-line' | ||
| }`} | ||
| /> | ||
| {errors.currentPassword && ( | ||
| <p className="mt-1 text-xs text-red-500">{errors.currentPassword}</p> | ||
| )} | ||
| </div> | ||
|
|
||
| <div> | ||
| <label className="mb-1 block text-sm font-medium text-wefin-text"> | ||
| 새 비밀번호 | ||
| </label> | ||
| <input | ||
| type="password" | ||
| value={newPassword} | ||
| onChange={(e) => setNewPassword(e.target.value)} | ||
| placeholder="새 비밀번호 입력" | ||
| className={`h-10 w-full max-w-[320px] rounded-lg border bg-white px-3 text-sm text-wefin-text outline-none transition-colors placeholder:text-wefin-subtle focus:border-wefin-mint ${ | ||
| errors.newPassword ? 'border-red-400' : 'border-wefin-line' | ||
| }`} | ||
| /> | ||
| <p className="mt-1 text-xs text-wefin-subtle"> | ||
| 8~20자, 영문과 숫자를 포함해야 합니다. | ||
| </p> | ||
| {errors.newPassword && ( | ||
| <p className="mt-1 text-xs text-red-500">{errors.newPassword}</p> | ||
| )} | ||
| </div> | ||
|
|
||
| <div> | ||
| <label className="mb-1 block text-sm font-medium text-wefin-text"> | ||
| 새 비밀번호 확인 | ||
| </label> | ||
| <input | ||
| type="password" | ||
| value={newPasswordConfirm} | ||
| onChange={(e) => setNewPasswordConfirm(e.target.value)} | ||
| placeholder="새 비밀번호 다시 입력" | ||
| className={`h-10 w-full max-w-[320px] rounded-lg border bg-white px-3 text-sm text-wefin-text outline-none transition-colors placeholder:text-wefin-subtle focus:border-wefin-mint ${ | ||
| errors.newPasswordConfirm ? 'border-red-400' : 'border-wefin-line' | ||
| }`} | ||
| /> |
There was a problem hiding this comment.
비밀번호 입력 label을 실제 input과 연결하세요.
현재 <label>에 htmlFor가 없고 <input>에도 대응되는 id가 없어 보조기술과 label 클릭 포커스가 제대로 동작하지 않습니다. 세 비밀번호 입력 모두 고유 id와 htmlFor를 연결해 주세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/settings/ui/settings-profile-section.tsx` around lines 185 -
235, The three password inputs (bound to currentPassword, newPassword,
newPasswordConfirm) need accessible label associations: add a unique id to each
input (e.g. currentPasswordInput, newPasswordInput, newPasswordConfirmInput) and
set the corresponding label's htmlFor to that id so clicking the label focuses
the matching <input>; update the <label> elements for the inputs that render
currentPassword, newPassword, and newPasswordConfirm to use htmlFor and ensure
each input includes the matching id attribute.
| {active === 'subscription' && ( | ||
| <div className="space-y-4"> | ||
| <SettingsSubscriptionSection /> | ||
| <SubscribeButton planId={1} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: 결제 플랜 ID가 중앙 상수/설정/플랜 데이터에서 관리되는지 확인합니다.
# Expect: planId 숫자 리터럴이 아닌 명명된 플랜 매핑 또는 서버/상태 기반 선택값 사용.
rg -nP -C3 '\b(planId|createPayment|planName|SubscribeButton)\b'Repository: S0L-V/wefin-fe
Length of output: 7458
🏁 Script executed:
# 플랜 관련 상수, 설정, 데이터 모델 찾기
rg -i '(plan|subscription)' --type ts --type tsx -B2 -A2 | grep -E '(const|export|PLAN|Plan|config)' | head -40Repository: S0L-V/wefin-fe
Length of output: 85
🏁 Script executed:
# features/payment 디렉토리 구조 확인
find src/features/payment -type f \( -name "*.ts" -o -name "*.tsx" \) | sortRepository: S0L-V/wefin-fe
Length of output: 375
🏁 Script executed:
# 다른 곳에서 planId 사용되는 모든 위치
rg 'planId' -B3 -A3 --type ts --type tsxRepository: S0L-V/wefin-fe
Length of output: 85
🏁 Script executed:
# 모든 planId 관련 코드 확인 (파일타입 필터 제거)
rg 'planId' -B2 -A2Repository: S0L-V/wefin-fe
Length of output: 3237
🏁 Script executed:
# 플랜 관련 상수나 설정 찾기
rg -i 'plan' src/features/ | grep -iE '(const|export|config|PLAN)' | head -30Repository: S0L-V/wefin-fe
Length of output: 1849
🏁 Script executed:
# subscribe-button.tsx 전체 내용 확인
cat src/features/payment/ui/subscribe-button.tsxRepository: S0L-V/wefin-fe
Length of output: 1069
🏁 Script executed:
# settings-view.tsx 전체 내용 확인
cat src/features/settings/ui/settings-view.tsxRepository: S0L-V/wefin-fe
Length of output: 3580
플랜 ID를 중앙 상수로 분리하세요.
planId={1}이 하드코딩되어 있고, 이 값이 결제 API로 직접 전달됩니다. 플랜이 추가되거나 백엔드 ID가 변경되면 이 값을 여러 위치에서 찾아 수정해야 합니다. 플랜 ID를 명명된 상수(예: PLAN_ID.PRO, SUBSCRIPTION_PLAN_ID)로 정의하여 중앙에서 관리하세요.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/settings/ui/settings-view.tsx` at line 89, Replace the hardcoded
numeric prop planId={1} on SubscribeButton with a centrally defined named
constant; create a constant or enum (e.g., SUBSCRIPTION_PLAN_ID or PLAN_ID.PRO)
exported from a single settings/constants module, update the SubscribeButton
usage to pass that named constant (SubscribeButton
planId={SUBSCRIPTION_PLAN_ID.PRO} or similar), and import the constant into
settings-view.tsx so future plan ID changes are made in one place rather than
scattered literals.
cl-o-lc
approved these changes
Apr 19, 2026
cl-o-lc
left a comment
cl-o-lc
left a comment
Contributor
There was a problem hiding this comment.
Zod 스키마로 API 응답을 검증한 부분과, React Query 컨벤션이 통일된 점이 좋았습니다!
payment-fail-page에서 Toss가 보내주는 code/message 쿼리 파라미터를 그대로 노출시켜서, 디버깅이 용이할 것 같아요 👍
구현 수고하셨습니다! 코멘트 확인 부탁드려요 🏃
Comment on lines
+10
to
+12
| export type ResetPasswordRequest = { | ||
| email: string | ||
| newPassword: string |
Contributor
There was a problem hiding this comment.
ResetPasswordRequest 타입에 code 필드가 누락되어 있습니다!
'newPassword: string` 추가를 해야 기능이 작동할 것 같아요 👀
Comment on lines
+3
to
+8
| type ApiResponse<T> = { | ||
| status: number | ||
| code: string | ||
| message: string | ||
| data: T | ||
| } |
Contributor
There was a problem hiding this comment.
ApiResponse 타입이 파일마다 중복 정의됩니다.
change-password.ts, reset-password.ts 모두 ApiResponse 타입을 각자 선언하고 있는데, src/shared/api/에 공통 타입을 두고 import하면 어떨까요?
추후 리팩토링 단계에서 적용하셔도 됩니다 👍
| @@ -0,0 +1,18 @@ | |||
| import { z } from 'zod' | |||
|
|
|||
| export const changePasswordSchema = z | |||
Contributor
There was a problem hiding this comment.
changePasswordSchema와 resetPasswordSchema가 중복입니다.
두 스키마 모두 동일한 newPassword 규칙(min(8).max(20).regex(...))과 newPasswordConfirm + refine을 가지고 있으니,
공통 스키마를 passwordFieldsSchema로 추출하고 .extend()로 합성하면 좋을 것 같습니다!
(이것도 마찬가지로 리팩토링 단계에 적용하셔도 됩니다!)
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/shared/api/base-api.ts (1)
110-115: 의도된 변경사항으로 확인되며, 로직상 문제 없음.
/auth/password/reset와/auth/email-verifications는 로그인되지 않은 사용자가 호출하는 엔드포인트이므로 401 발생 시 refresh 재시도에서 제외하는 것이 타당합니다./auth/password/change는 목록에 포함되지 않아 인증이 필요한 비밀번호 변경 요청은 정상적으로 refresh 흐름을 타게 되는 점도 올바릅니다.다만
includes기반 문자열 매칭이 5개로 늘어나면서 가독성이 떨어지고, 향후 엔드포인트가 추가될수록 누락/오탐(예: 경로에 유사 문자열이 포함된 다른 API)이 발생하기 쉽습니다. 배열 +some으로 정리해두면 확장이 수월합니다.♻️ 제안 리팩터링
+const AUTH_REQUEST_PATHS = [ + '/auth/login', + '/auth/signup', + '/auth/refresh', + '/auth/password/reset', + '/auth/email-verifications' +] as const + ... - const isAuthRequest = - requestUrl.includes('/auth/login') || - requestUrl.includes('/auth/signup') || - requestUrl.includes('/auth/refresh') || - requestUrl.includes('/auth/password/reset') || - requestUrl.includes('/auth/email-verifications') + const isAuthRequest = AUTH_REQUEST_PATHS.some((path) => requestUrl.includes(path))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/shared/api/base-api.ts` around lines 110 - 115, Replace the long chain of requestUrl.includes(...) with an array-based check to improve readability and reduce false positives: define a const publicAuthPaths = ['/auth/login','/auth/signup','/auth/refresh','/auth/password/reset','/auth/email-verifications'] and then set isAuthRequest = publicAuthPaths.some(p => requestUrl.includes(p)) (or, if you can access the URL pathname, compare against pathname with startsWith/=== for stricter matching); update the code around the isAuthRequest declaration to use these symbols (publicAuthPaths and isAuthRequest).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/shared/api/base-api.ts`:
- Around line 110-115: Replace the long chain of requestUrl.includes(...) with
an array-based check to improve readability and reduce false positives: define a
const publicAuthPaths =
['/auth/login','/auth/signup','/auth/refresh','/auth/password/reset','/auth/email-verifications']
and then set isAuthRequest = publicAuthPaths.some(p => requestUrl.includes(p))
(or, if you can access the URL pathname, compare against pathname with
startsWith/=== for stricter matching); update the code around the isAuthRequest
declaration to use these symbols (publicAuthPaths and isAuthRequest).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b0475ea8-f3c9-40e4-86b9-54ef0bcdc88a
📒 Files selected for processing (2)
src/features/settings/ui/settings-view.tsxsrc/shared/api/base-api.ts
✅ Files skipped from review due to trivial changes (1)
- src/features/settings/ui/settings-view.tsx
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.
📌 PR 설명
설정 페이지에서 비밀번호를 변경할 수 있는 기능을 추가하고,
이메일 인증 기반 비밀번호 재설정 API를 프론트에 연동했습니다.
✅ 완료한 기능 명세
📸 스크린샷
💭 고민과 해결과정
Summary by CodeRabbit
릴리스 노트
새로운 기능
개선