feat: add password reset settings tab / 新增密码修改设置页#928
Open
WenhuaXia wants to merge 1 commit into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
✅ Files skipped from review due to trivial changes (5)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughAdds a Password settings tab with a password-change form, navigation wiring, and locale strings. The new tab validates current, new, and confirmation fields before calling the password-change API, then shows success or error state. ChangesPassword Settings Tab
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 (1)
src/components/settings/view/tabs/password-settings/PasswordSettingsTab.tsx (1)
62-66: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAvoid matching backend errors by display text.
Checking for the exact string
'Current password is incorrect'hard-codes server copy into the client. Any wording change on the API side will bypass the translated branch and start surfacing raw backend text here. Prefer a stable error code/status fromapi.auth.changePasswordand map that tot(...)on the client.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/settings/view/tabs/password-settings/PasswordSettingsTab.tsx` around lines 62 - 66, The password change error handling in PasswordSettingsTab should not branch on the literal backend message text. Update the `api.auth.changePassword`/`PasswordSettingsTab` flow to rely on a stable error code or status from the API response instead of checking for `'Current password is incorrect'`, then map that code to `t('password.error.incorrectOld')` on the client. Keep the fallback behavior for other errors by still using the existing `msg`/`setErrorMessage` path when the code does not match the incorrect-password case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/settings/view/tabs/password-settings/PasswordSettingsTab.tsx`:
- Around line 20-69: Prevent duplicate password-change requests by adding a
synchronous in-flight guard in handleChangePassword inside PasswordSettingsTab.
The issue is that setStatus('saving') updates asynchronously, so a fast
double-click can enter api.auth.changePassword twice before the button disables.
Add a local guard (for example, a ref or similar immediate flag) that is checked
at the start of handleChangePassword and set before awaiting the API call, then
cleared in both success and error paths so only one mutation can run at a time.
---
Nitpick comments:
In `@src/components/settings/view/tabs/password-settings/PasswordSettingsTab.tsx`:
- Around line 62-66: The password change error handling in PasswordSettingsTab
should not branch on the literal backend message text. Update the
`api.auth.changePassword`/`PasswordSettingsTab` flow to rely on a stable error
code or status from the API response instead of checking for `'Current password
is incorrect'`, then map that code to `t('password.error.incorrectOld')` on the
client. Keep the fallback behavior for other errors by still using the existing
`msg`/`setErrorMessage` path when the code does not match the incorrect-password
case.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bb8673a5-1bde-4926-b7d1-c31166c20ce0
📒 Files selected for processing (14)
src/components/settings/types/types.tssrc/components/settings/view/Settings.tsxsrc/components/settings/view/SettingsMainTabs.tsxsrc/components/settings/view/SettingsSidebar.tsxsrc/components/settings/view/tabs/password-settings/PasswordSettingsTab.tsxsrc/i18n/locales/de/settings.jsonsrc/i18n/locales/en/settings.jsonsrc/i18n/locales/it/settings.jsonsrc/i18n/locales/ja/settings.jsonsrc/i18n/locales/ko/settings.jsonsrc/i18n/locales/ru/settings.jsonsrc/i18n/locales/tr/settings.jsonsrc/i18n/locales/zh-CN/settings.jsonsrc/i18n/locales/zh-TW/settings.json
Comment on lines
+20
to
+69
| const handleChangePassword = useCallback(async () => { | ||
| setErrorMessage(''); | ||
|
|
||
| if (!oldPassword || !newPassword || !confirmPassword) { | ||
| setStatus('error'); | ||
| setErrorMessage(t('password.error.fillAllFields')); | ||
| return; | ||
| } | ||
|
|
||
| if (newPassword.length < 6) { | ||
| setStatus('error'); | ||
| setErrorMessage(t('password.error.minLength')); | ||
| return; | ||
| } | ||
|
|
||
| if (newPassword !== confirmPassword) { | ||
| setStatus('error'); | ||
| setErrorMessage(t('password.error.notMatch')); | ||
| return; | ||
| } | ||
|
|
||
| if (oldPassword === newPassword) { | ||
| setStatus('error'); | ||
| setErrorMessage(t('password.error.sameAsOld')); | ||
| return; | ||
| } | ||
|
|
||
| setStatus('saving'); | ||
|
|
||
| try { | ||
| const result = await api.auth.changePassword(oldPassword, newPassword); | ||
| if (result.success) { | ||
| setStatus('success'); | ||
| setOldPassword(''); | ||
| setNewPassword(''); | ||
| setConfirmPassword(''); | ||
| } else { | ||
| setStatus('error'); | ||
| setErrorMessage(result.error || t('password.error.failed')); | ||
| } | ||
| } catch (err: any) { | ||
| setStatus('error'); | ||
| const msg = err.response?.data?.error || err.message || t('password.error.failed'); | ||
| if (msg === 'Current password is incorrect') { | ||
| setErrorMessage(t('password.error.incorrectOld')); | ||
| } else { | ||
| setErrorMessage(msg); | ||
| } | ||
| } | ||
| }, [oldPassword, newPassword, confirmPassword, t]); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Prevent duplicate password-change requests.
setStatus('saving') disables the button only after React re-renders, so a fast double-click can still call api.auth.changePassword(...) twice. Because this mutation is not idempotent, the second request can race the first and leave the tab in an error state even though the password already changed. Add a synchronous in-flight guard around the handler.
Proposed fix
-import { useCallback, useState } from 'react';
+import { useCallback, useRef, useState } from 'react';
@@
const [confirmPassword, setConfirmPassword] = useState('');
const [status, setStatus] = useState<Status>('idle');
const [errorMessage, setErrorMessage] = useState('');
+ const isSubmittingRef = useRef(false);
@@
const handleChangePassword = useCallback(async () => {
+ if (isSubmittingRef.current) {
+ return;
+ }
+
setErrorMessage('');
@@
setStatus('saving');
+ isSubmittingRef.current = true;
try {
const result = await api.auth.changePassword(oldPassword, newPassword);
@@
} catch (err: any) {
setStatus('error');
const msg = err.response?.data?.error || err.message || t('password.error.failed');
if (msg === 'Current password is incorrect') {
setErrorMessage(t('password.error.incorrectOld'));
} else {
setErrorMessage(msg);
}
+ } finally {
+ isSubmittingRef.current = false;
}
}, [oldPassword, newPassword, confirmPassword, t]);📝 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 handleChangePassword = useCallback(async () => { | |
| setErrorMessage(''); | |
| if (!oldPassword || !newPassword || !confirmPassword) { | |
| setStatus('error'); | |
| setErrorMessage(t('password.error.fillAllFields')); | |
| return; | |
| } | |
| if (newPassword.length < 6) { | |
| setStatus('error'); | |
| setErrorMessage(t('password.error.minLength')); | |
| return; | |
| } | |
| if (newPassword !== confirmPassword) { | |
| setStatus('error'); | |
| setErrorMessage(t('password.error.notMatch')); | |
| return; | |
| } | |
| if (oldPassword === newPassword) { | |
| setStatus('error'); | |
| setErrorMessage(t('password.error.sameAsOld')); | |
| return; | |
| } | |
| setStatus('saving'); | |
| try { | |
| const result = await api.auth.changePassword(oldPassword, newPassword); | |
| if (result.success) { | |
| setStatus('success'); | |
| setOldPassword(''); | |
| setNewPassword(''); | |
| setConfirmPassword(''); | |
| } else { | |
| setStatus('error'); | |
| setErrorMessage(result.error || t('password.error.failed')); | |
| } | |
| } catch (err: any) { | |
| setStatus('error'); | |
| const msg = err.response?.data?.error || err.message || t('password.error.failed'); | |
| if (msg === 'Current password is incorrect') { | |
| setErrorMessage(t('password.error.incorrectOld')); | |
| } else { | |
| setErrorMessage(msg); | |
| } | |
| } | |
| }, [oldPassword, newPassword, confirmPassword, t]); | |
| const [confirmPassword, setConfirmPassword] = useState(''); | |
| const [status, setStatus] = useState<Status>('idle'); | |
| const [errorMessage, setErrorMessage] = useState(''); | |
| const isSubmittingRef = useRef(false); | |
| const handleChangePassword = useCallback(async () => { | |
| if (isSubmittingRef.current) { | |
| return; | |
| } | |
| setErrorMessage(''); | |
| if (!oldPassword || !newPassword || !confirmPassword) { | |
| setStatus('error'); | |
| setErrorMessage(t('password.error.fillAllFields')); | |
| return; | |
| } | |
| if (newPassword.length < 6) { | |
| setStatus('error'); | |
| setErrorMessage(t('password.error.minLength')); | |
| return; | |
| } | |
| if (newPassword !== confirmPassword) { | |
| setStatus('error'); | |
| setErrorMessage(t('password.error.notMatch')); | |
| return; | |
| } | |
| if (oldPassword === newPassword) { | |
| setStatus('error'); | |
| setErrorMessage(t('password.error.sameAsOld')); | |
| return; | |
| } | |
| setStatus('saving'); | |
| isSubmittingRef.current = true; | |
| try { | |
| const result = await api.auth.changePassword(oldPassword, newPassword); | |
| if (result.success) { | |
| setStatus('success'); | |
| setOldPassword(''); | |
| setNewPassword(''); | |
| setConfirmPassword(''); | |
| } else { | |
| setStatus('error'); | |
| setErrorMessage(result.error || t('password.error.failed')); | |
| } | |
| } catch (err: any) { | |
| setStatus('error'); | |
| const msg = err.response?.data?.error || err.message || t('password.error.failed'); | |
| if (msg === 'Current password is incorrect') { | |
| setErrorMessage(t('password.error.incorrectOld')); | |
| } else { | |
| setErrorMessage(msg); | |
| } | |
| } finally { | |
| isSubmittingRef.current = false; | |
| } | |
| }, [oldPassword, newPassword, confirmPassword, t]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/settings/view/tabs/password-settings/PasswordSettingsTab.tsx`
around lines 20 - 69, Prevent duplicate password-change requests by adding a
synchronous in-flight guard in handleChangePassword inside PasswordSettingsTab.
The issue is that setStatus('saving') updates asynchronously, so a fast
double-click can enter api.auth.changePassword twice before the button disables.
Add a local guard (for example, a ref or similar immediate flag) that is checked
at the start of handleChangePassword and set before awaiting the API call, then
cleared in both success and error paths so only one mutation can run at a time.
WenhuaXia
force-pushed
the
feat/password-reset-settings
branch
from
9e57ec0 to
6425101
Compare
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.
feat: add password reset settings tab / 新增密码修改设置页
Add a dedicated Password Settings tab in CloudCLI settings UI, allowing users to change their authentication password directly from the web interface.
新增内容：
Files changed:
Summary by CodeRabbit