Also remove some code smellstags/10.0.0.68432
@@ -18,9 +18,9 @@ | |||
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. | |||
*/ | |||
import { throwGlobalError } from '../helpers/error'; | |||
import { getJSON, post, postJSON } from '../helpers/request'; | |||
import { getJSON, HttpStatus, parseJSON, post, postJSON } from '../helpers/request'; | |||
import { IdentityProvider, Paging } from '../types/types'; | |||
import { CurrentUser, HomePage, NoticeType, User } from '../types/users'; | |||
import { ChangePasswordResults, CurrentUser, HomePage, NoticeType, User } from '../types/users'; | |||
export function getCurrentUser(): Promise<CurrentUser> { | |||
return getJSON('/api/users/current', undefined, true); | |||
@@ -35,7 +35,14 @@ export function changePassword(data: { | |||
password: string; | |||
previousPassword?: string; | |||
}) { | |||
return post('/api/users/change_password', data).catch(throwGlobalError); | |||
return post('/api/users/change_password', data).catch(async (response) => { | |||
if (response.status === HttpStatus.BadRequest) { | |||
const { result } = await parseJSON(response); | |||
return Promise.reject<ChangePasswordResults>(result); | |||
} | |||
return throwGlobalError(response); | |||
}); | |||
} | |||
export interface UserGroup { |
@@ -24,11 +24,9 @@ import Modal from '../../../components/controls/Modal'; | |||
import { Alert } from '../../../components/ui/Alert'; | |||
import MandatoryFieldMarker from '../../../components/ui/MandatoryFieldMarker'; | |||
import MandatoryFieldsExplanation from '../../../components/ui/MandatoryFieldsExplanation'; | |||
import { throwGlobalError } from '../../../helpers/error'; | |||
import { addGlobalSuccessMessage } from '../../../helpers/globalMessages'; | |||
import { translate } from '../../../helpers/l10n'; | |||
import { parseError } from '../../../helpers/request'; | |||
import { User } from '../../../types/users'; | |||
import { ChangePasswordResults, User } from '../../../types/users'; | |||
interface Props { | |||
isCurrentUser: boolean; | |||
@@ -38,7 +36,7 @@ interface Props { | |||
interface State { | |||
confirmPassword: string; | |||
error?: string; | |||
errorTranslationKey?: string; | |||
newPassword: string; | |||
oldPassword: string; | |||
submitting: boolean; | |||
@@ -61,14 +59,13 @@ export default class PasswordForm extends React.PureComponent<Props, State> { | |||
this.mounted = false; | |||
} | |||
handleError = (response: Response) => { | |||
if (!this.mounted || response.status !== 400) { | |||
return throwGlobalError(response); | |||
} else { | |||
return parseError(response).then( | |||
(errorMsg) => this.setState({ error: errorMsg, submitting: false }), | |||
throwGlobalError | |||
); | |||
handleError = (result: ChangePasswordResults) => { | |||
if (this.mounted) { | |||
if (result === ChangePasswordResults.OldPasswordIncorrect) { | |||
this.setState({ errorTranslationKey: 'user.old_password_incorrect', submitting: false }); | |||
} else if (result === ChangePasswordResults.NewPasswordSameAsOld) { | |||
this.setState({ errorTranslationKey: 'user.new_password_same_as_old', submitting: false }); | |||
} | |||
} | |||
}; | |||
@@ -100,7 +97,7 @@ export default class PasswordForm extends React.PureComponent<Props, State> { | |||
}; | |||
render() { | |||
const { error, submitting, newPassword, confirmPassword } = this.state; | |||
const { errorTranslationKey, submitting, newPassword, confirmPassword } = this.state; | |||
const header = translate('my_profile.password.title'); | |||
return ( | |||
@@ -110,7 +107,7 @@ export default class PasswordForm extends React.PureComponent<Props, State> { | |||
<h2>{header}</h2> | |||
</header> | |||
<div className="modal-body"> | |||
{error && <Alert variant="error">{error}</Alert>} | |||
{errorTranslationKey && <Alert variant="error">{translate(errorTranslationKey)}</Alert>} | |||
<MandatoryFieldsExplanation className="modal-field" /> | |||
@@ -21,6 +21,8 @@ import { shallow } from 'enzyme'; | |||
import * as React from 'react'; | |||
import { changePassword } from '../../../../api/users'; | |||
import { mockUser } from '../../../../helpers/testMocks'; | |||
import { mockEvent, waitAndUpdate } from '../../../../helpers/testUtils'; | |||
import { ChangePasswordResults } from '../../../../types/users'; | |||
import PasswordForm from '../PasswordForm'; | |||
const password = 'new password asdf'; | |||
@@ -38,38 +40,51 @@ it('should handle password change', async () => { | |||
const wrapper = shallowRender({ onClose }); | |||
wrapper.setState({ newPassword: password, confirmPassword: password }); | |||
wrapper.instance().handleChangePassword({ preventDefault: jest.fn() } as any); | |||
wrapper.instance().handleChangePassword(mockEvent({ preventDefault: jest.fn() })); | |||
await new Promise(setImmediate); | |||
await waitAndUpdate(wrapper); | |||
expect(onClose).toHaveBeenCalled(); | |||
}); | |||
it('should handle password change error', async () => { | |||
it('should handle password change error when new password is same as old', async () => { | |||
const wrapper = shallowRender(); | |||
(changePassword as jest.Mock).mockRejectedValue(new Response(undefined, { status: 400 })); | |||
jest.mocked(changePassword).mockRejectedValue(ChangePasswordResults.NewPasswordSameAsOld); | |||
wrapper.setState({ newPassword: password, confirmPassword: password }); | |||
wrapper.instance().mounted = true; | |||
wrapper.instance().handleChangePassword(mockEvent({ preventDefault: jest.fn() })); | |||
await waitAndUpdate(wrapper); | |||
expect(wrapper.state().errorTranslationKey).toBe('user.new_password_same_as_old'); | |||
}); | |||
it('should handle password change error when old password is incorrect', async () => { | |||
const wrapper = shallowRender(); | |||
jest.mocked(changePassword).mockRejectedValue(ChangePasswordResults.OldPasswordIncorrect); | |||
wrapper.setState({ newPassword: password, confirmPassword: password }); | |||
wrapper.instance().mounted = true; | |||
wrapper.instance().handleChangePassword({ preventDefault: jest.fn() } as any); | |||
wrapper.instance().handleChangePassword(mockEvent({ preventDefault: jest.fn() })); | |||
await new Promise(setImmediate); | |||
await waitAndUpdate(wrapper); | |||
expect(wrapper.state('error')).toBe('default_error_message'); | |||
expect(wrapper.state().errorTranslationKey).toBe('user.old_password_incorrect'); | |||
}); | |||
it('should handle form changes', () => { | |||
const wrapper = shallowRender(); | |||
wrapper.instance().handleConfirmPasswordChange({ currentTarget: { value: 'pwd' } } as any); | |||
expect(wrapper.state('confirmPassword')).toBe('pwd'); | |||
wrapper.instance().handleConfirmPasswordChange(mockEvent({ currentTarget: { value: 'pwd' } })); | |||
expect(wrapper.state().confirmPassword).toBe('pwd'); | |||
wrapper.instance().handleNewPasswordChange({ currentTarget: { value: 'pwd' } } as any); | |||
expect(wrapper.state('newPassword')).toBe('pwd'); | |||
wrapper.instance().handleNewPasswordChange(mockEvent({ currentTarget: { value: 'pwd' } })); | |||
expect(wrapper.state().newPassword).toBe('pwd'); | |||
wrapper.instance().handleOldPasswordChange({ currentTarget: { value: 'pwd' } } as any); | |||
expect(wrapper.state('oldPassword')).toBe('pwd'); | |||
wrapper.instance().handleOldPasswordChange(mockEvent({ currentTarget: { value: 'pwd' } })); | |||
expect(wrapper.state().oldPassword).toBe('pwd'); | |||
}); | |||
function shallowRender(props: Partial<PasswordForm['props']> = {}) { |
@@ -24,7 +24,7 @@ import { Alert } from '../../components/ui/Alert'; | |||
import MandatoryFieldMarker from '../../components/ui/MandatoryFieldMarker'; | |||
import MandatoryFieldsExplanation from '../../components/ui/MandatoryFieldsExplanation'; | |||
import { translate } from '../../helpers/l10n'; | |||
import { LoggedInUser } from '../../types/users'; | |||
import { ChangePasswordResults, LoggedInUser } from '../../types/users'; | |||
interface Props { | |||
className?: string; | |||
@@ -76,12 +76,15 @@ export default class ResetPasswordForm extends React.Component<Props, State> { | |||
this.password.focus(); | |||
this.setErrors([translate('user.password_doesnt_match_confirmation')]); | |||
} else { | |||
changePassword({ login: user.login, password, previousPassword }).then( | |||
this.handleSuccessfulChange, | |||
() => { | |||
// error already reported. | |||
} | |||
); | |||
changePassword({ login: user.login, password, previousPassword }) | |||
.then(this.handleSuccessfulChange) | |||
.catch((result: ChangePasswordResults) => { | |||
if (result === ChangePasswordResults.OldPasswordIncorrect) { | |||
this.setErrors([translate('user.old_password_incorrect')]); | |||
} else if (result === ChangePasswordResults.NewPasswordSameAsOld) { | |||
this.setErrors([translate('user.new_password_same_as_old')]); | |||
} | |||
}); | |||
} | |||
}; | |||
@@ -94,9 +97,8 @@ export default class ResetPasswordForm extends React.Component<Props, State> { | |||
{success && <Alert variant="success">{translate('my_profile.password.changed')}</Alert>} | |||
{errors && | |||
errors.map((e, i) => ( | |||
/* eslint-disable-next-line react/no-array-index-key */ | |||
<Alert key={i} variant="error"> | |||
errors.map((e) => ( | |||
<Alert key={e} variant="error"> | |||
{e} | |||
</Alert> | |||
))} |
@@ -22,12 +22,18 @@ import * as React from 'react'; | |||
import { changePassword } from '../../../api/users'; | |||
import { mockLoggedInUser } from '../../../helpers/testMocks'; | |||
import { mockEvent, waitAndUpdate } from '../../../helpers/testUtils'; | |||
import { ChangePasswordResults } from '../../../types/users'; | |||
import { Alert } from '../../ui/Alert'; | |||
import ResetPasswordForm from '../ResetPasswordForm'; | |||
jest.mock('../../../api/users', () => ({ | |||
changePassword: jest.fn().mockResolvedValue({}), | |||
})); | |||
beforeEach(() => { | |||
jest.clearAllMocks(); | |||
}); | |||
it('should trigger on password change prop', () => { | |||
const onPasswordChange = jest.fn(); | |||
const wrapper = shallowRender({ onPasswordChange }); | |||
@@ -50,6 +56,30 @@ it('should not trigger password change', () => { | |||
expect(wrapper.state().errors).toBeDefined(); | |||
}); | |||
it('should not change password if new password is same as old', async () => { | |||
jest.mocked(changePassword).mockRejectedValueOnce(ChangePasswordResults.NewPasswordSameAsOld); | |||
const wrapper = shallowRender(); | |||
wrapper.instance().oldPassword = { value: 'testold' } as HTMLInputElement; | |||
wrapper.instance().password = { value: 'test', focus: () => {} } as HTMLInputElement; | |||
wrapper.instance().passwordConfirmation = { value: 'test' } as HTMLInputElement; | |||
wrapper.instance().handleChangePassword(mockEvent()); | |||
await waitAndUpdate(wrapper); | |||
expect(wrapper.find(Alert)).toMatchSnapshot(); | |||
}); | |||
it('should not change password if old password is incorrect', async () => { | |||
jest.mocked(changePassword).mockRejectedValueOnce(ChangePasswordResults.OldPasswordIncorrect); | |||
const wrapper = shallowRender(); | |||
wrapper.instance().oldPassword = { value: 'testold' } as HTMLInputElement; | |||
wrapper.instance().password = { value: 'test', focus: () => {} } as HTMLInputElement; | |||
wrapper.instance().passwordConfirmation = { value: 'test' } as HTMLInputElement; | |||
wrapper.instance().handleChangePassword(mockEvent()); | |||
await waitAndUpdate(wrapper); | |||
expect(wrapper.find(Alert)).toMatchSnapshot(); | |||
}); | |||
it('should trigger password change', async () => { | |||
const user = mockLoggedInUser(); | |||
const wrapper = shallowRender({ user }); |
@@ -69,3 +69,21 @@ exports[`renders correctly 1`] = ` | |||
</div> | |||
</form> | |||
`; | |||
exports[`should not change password if new password is same as old 1`] = ` | |||
<Alert | |||
key="user.new_password_same_as_old" | |||
variant="error" | |||
> | |||
user.new_password_same_as_old | |||
</Alert> | |||
`; | |||
exports[`should not change password if old password is incorrect 1`] = ` | |||
<Alert | |||
key="user.old_password_incorrect" | |||
variant="error" | |||
> | |||
user.old_password_incorrect | |||
</Alert> | |||
`; |
@@ -89,6 +89,11 @@ export interface UserBase { | |||
name?: string; | |||
} | |||
export const enum ChangePasswordResults { | |||
OldPasswordIncorrect = 'old_password_incorrect', | |||
NewPasswordSameAsOld = 'new_password_same_as_old', | |||
} | |||
export function isUserActive(user: UserBase): user is UserActive { | |||
return user.active !== false && Boolean(user.name); | |||
} |
@@ -2119,6 +2119,8 @@ alert.dismiss=Dismiss this message | |||
# | |||
#------------------------------------------------------------------------------ | |||
user.password_doesnt_match_confirmation=Password doesn't match confirmation. | |||
user.old_password_incorrect=Old password is incorrect | |||
user.new_password_same_as_old=New password must be different from old password | |||
user.login_or_email_used_as_scm_account=Login and email are automatically considered as SCM accounts | |||
user.x_deleted={0} (deleted) | |||