From 31f0cb12b3002f01581db1742063c02f8c3e6e0a Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 30 Apr 2018 15:43:12 -0600 Subject: [PATCH 1/5] Fixed bug in mutation --- perms/reducers/mutation.js | 22 ++++++++----------- .../components/ChangeUsername.js | 9 +++----- .../containers/ChangeUsername.js | 4 ++-- .../talk-plugin-auth/client/translations.yml | 5 +++-- services/users.js | 20 ++++++++++++----- 5 files changed, 32 insertions(+), 28 deletions(-) diff --git a/perms/reducers/mutation.js b/perms/reducers/mutation.js index f063fe1c2..44f66cf88 100644 --- a/perms/reducers/mutation.js +++ b/perms/reducers/mutation.js @@ -14,25 +14,21 @@ module.exports = (user, perm) => { user.password.length > 0 ); - case types.CHANGE_USERNAME: { + case types.CHANGE_USERNAME: + return user.status.username.status === 'REJECTED'; + + case types.SET_USERNAME: { // Only users who have their usernames rejected or those users who // not changed their usernames within 14 days can change their usernames. - const now = moment(); + const deadline = moment().subtract(14, 'days'); return ( - user.status.username.status === 'REJECTED' || - get(user, 'status.username.history', []) - .filter(({ status }) => status === 'CHANGED') - .every(({ created_at }) => - moment(created_at) - .add(14, 'days') - .isAfter(now) - ) + user.status.username.status === 'UNSET' || + get(user, 'status.username.history', []).every(({ created_at }) => + moment(created_at).isBefore(deadline) + ) ); } - case types.SET_USERNAME: - return user.status.username.status === 'UNSET'; - case types.CREATE_COMMENT: case types.CREATE_ACTION: case types.DELETE_ACTION: diff --git a/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsername.js b/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsername.js index df8b3a641..4d6bc41f6 100644 --- a/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsername.js +++ b/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsername.js @@ -50,13 +50,10 @@ class ChangeUsername extends React.Component { saveChanges = async () => { const { newUsername } = this.state.formData; - const { id } = this.props; + const { setUsername } = this.props; try { - await this.props.changeUsername({ - id, - username: newUsername, - }); + await setUsername(newUsername); this.props.notify( 'success', t('talk-plugin-auth.change_username.changed_username_success_msg') @@ -173,7 +170,7 @@ class ChangeUsername extends React.Component { } ChangeUsername.propTypes = { - changeUsername: PropTypes.func.isRequired, + setUsername: PropTypes.func.isRequired, notify: PropTypes.func.isRequired, username: PropTypes.string, emailAddress: PropTypes.string, diff --git a/plugins/talk-plugin-auth/client/profile-settings/containers/ChangeUsername.js b/plugins/talk-plugin-auth/client/profile-settings/containers/ChangeUsername.js index 87e1e18b5..5381ef162 100644 --- a/plugins/talk-plugin-auth/client/profile-settings/containers/ChangeUsername.js +++ b/plugins/talk-plugin-auth/client/profile-settings/containers/ChangeUsername.js @@ -3,10 +3,10 @@ import { bindActionCreators } from 'redux'; import { connect } from 'plugin-api/beta/client/hocs'; import ChangeUsername from '../components/ChangeUsername'; import { notify } from 'coral-framework/actions/notification'; -import { withChangeUsername } from 'plugin-api/beta/client/hocs'; +import { withSetUsername } from 'plugin-api/beta/client/hocs'; const mapDispatchToProps = dispatch => bindActionCreators({ notify }, dispatch); -export default compose(connect(null, mapDispatchToProps), withChangeUsername)( +export default compose(connect(null, mapDispatchToProps), withSetUsername)( ChangeUsername ); diff --git a/plugins/talk-plugin-auth/client/translations.yml b/plugins/talk-plugin-auth/client/translations.yml index 80f0be761..8362a2e0b 100644 --- a/plugins/talk-plugin-auth/client/translations.yml +++ b/plugins/talk-plugin-auth/client/translations.yml @@ -142,8 +142,9 @@ en: cancel: "Cancel" edit: "Edit" changed_password_msg: "Changed Password - Your password has been successfully changed" - change_username: + change_username: change_username_note: "Usernames can be changed every 14 days" + changed_username_success_msg: "Username has been updated" save: "Save" edit_profile: "Edit Profile" cancel: "Cancel" @@ -255,7 +256,7 @@ es: cancel: "Cancelar" edit: "Editar" changed_password_msg: "Contraseña Actualizada - Tu contraseña ha sido exitosamente actualizada" - change_username: + change_username: change_username_note: "El usuario puede ser cambiado cada 14 días" save: "Guardar" edit_profile: "Editar Perfil" diff --git a/services/users.js b/services/users.js index a3301be10..a4cb0645e 100644 --- a/services/users.js +++ b/services/users.js @@ -253,9 +253,19 @@ class Users { }, { 'status.username.status': { $in: ['APPROVED', 'SET'] }, - 'status.username.history.created_at': { - $lte: oldestEditTime, - }, + $or: [ + { + 'status.username.history.created_at': { + $lte: oldestEditTime, + }, + }, + { + 'status.username.history': [], + }, + { + 'status.username.history': { $exists: false }, + }, + ], }, ], }; @@ -286,8 +296,8 @@ class Users { if ( !['UNSET', 'APPROVED', 'SET'].includes(user.status.username.status) || - !user.status.username.history.every(({ created_at }) => - oldestEditTime.isAfter(created_at) + !user.status.username.history.some(({ created_at }) => + moment(created_at).isAfter(oldestEditTime) ) ) { throw new ErrPermissionUpdateUsername(); From 7a0c9c85bc4a3022d1552c33f2a1e4cd42afadd9 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 30 Apr 2018 15:59:35 -0600 Subject: [PATCH 2/5] fixed tests --- test/server/graph/mutations/changeUsername.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/server/graph/mutations/changeUsername.js b/test/server/graph/mutations/changeUsername.js index a5c3daed8..eaff85434 100644 --- a/test/server/graph/mutations/changeUsername.js +++ b/test/server/graph/mutations/changeUsername.js @@ -89,7 +89,7 @@ describe('graph.mutations.changeUsername', () => { expect(res.data.changeUsername.errors).to.have.length(1); expect(res.data.changeUsername.errors[0]).to.have.property( 'translation_key', - 'EDIT_USERNAME_NOT_AUTHORIZED' + 'NOT_AUTHORIZED' ); // Set the user to the desired status. From b42d325cb0c2bbfbb818c05d797e07799d2d9c95 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 30 Apr 2018 16:13:48 -0600 Subject: [PATCH 3/5] fixes for tests --- services/users.js | 2 +- test/server/services/users.js | 86 +++++++++++++++++------------------ 2 files changed, 44 insertions(+), 44 deletions(-) diff --git a/services/users.js b/services/users.js index a4cb0645e..e53df4119 100644 --- a/services/users.js +++ b/services/users.js @@ -296,7 +296,7 @@ class Users { if ( !['UNSET', 'APPROVED', 'SET'].includes(user.status.username.status) || - !user.status.username.history.some(({ created_at }) => + user.status.username.history.some(({ created_at }) => moment(created_at).isAfter(oldestEditTime) ) ) { diff --git a/test/server/services/users.js b/test/server/services/users.js index a7d7475e9..2b9210f83 100644 --- a/test/server/services/users.js +++ b/test/server/services/users.js @@ -304,63 +304,63 @@ describe('services.UsersService', () => { await UsersService[func](user.id, user.username); } }); - }); - if (func === 'setUsername') { - it('should let a user set their username from UNSET', async () => { - const user = mockUsers[0]; + if (func === 'setUsername') { + it('should let a user set their username from UNSET', async () => { + const user = mockUsers[0]; - // Set the user to the desired status. - await UsersService.setUsernameStatus(user.id, 'UNSET'); - await UsersService.setUsername(user.id, 'new_username', null); - }); - - describe('time based', () => { - afterEach(() => { - timekeeper.reset(); + // Set the user to the desired status. + await UsersService.setUsernameStatus(user.id, 'UNSET'); + await UsersService.setUsername(user.id, 'new_username', null); }); - ['SET', 'APPROVED'].forEach(status => { - it(`should not allow users to change their username if it was changed within 14 of today from ${status}`, async () => { - const user = mockUsers[0]; + describe('time based', () => { + afterEach(() => { + timekeeper.reset(); + }); - // Set the user to the desired status. - await UsersService.setUsernameStatus(user.id, status); + ['SET', 'APPROVED'].forEach(status => { + it(`should not allow users to change their username if it was changed within 14 of today from ${status}`, async () => { + const user = mockUsers[0]; - timekeeper.travel( - moment() - .add(5, 'days') - .toDate() - ); + // Set the user to the desired status. + await UsersService.setUsernameStatus(user.id, status); - try { - await UsersService.setUsername(user.id, 'new_username', null); - throw new Error('edit was processed successfully'); - } catch (err) { - expect(err).have.property( - 'translation_key', - 'EDIT_USERNAME_NOT_AUTHORIZED' + timekeeper.travel( + moment() + .add(5, 'days') + .toDate() ); - } - }); - it(`allows users to change their username if it was changed 14 days before today from ${status}`, async () => { - const user = mockUsers[0]; + try { + await UsersService.setUsername(user.id, 'new_username', null); + throw new Error('edit was processed successfully'); + } catch (err) { + expect(err).have.property( + 'translation_key', + 'EDIT_USERNAME_NOT_AUTHORIZED' + ); + } + }); - // Set the user to the desired status. - await UsersService.setUsernameStatus(user.id, status); + it(`allows users to change their username if it was changed 14 days before today from ${status}`, async () => { + const user = mockUsers[0]; - timekeeper.travel( - moment() - .add(15, 'days') - .toDate() - ); + // Set the user to the desired status. + await UsersService.setUsernameStatus(user.id, status); - await UsersService.setUsername(user.id, 'new_username', null); + timekeeper.travel( + moment() + .add(15, 'days') + .toDate() + ); + + await UsersService.setUsername(user.id, 'new_username', null); + }); }); }); - }); - } + } + }); }); describe('#isValidUsername', () => { From ad342c4bb34178750c76474ba32724a8050da501 Mon Sep 17 00:00:00 2001 From: okbel Date: Mon, 30 Apr 2018 19:56:53 -0300 Subject: [PATCH 4/5] Catching if username cant be updated --- .../src/tabs/profile/containers/Profile.js | 9 +++++++++ client/coral-framework/utils/user.js | 16 ++++++++++++++++ .../components/ChangeUsername.js | 17 +++++++++++++---- .../components/ChangeUsernameDialog.js | 17 ++++++++++++++--- 4 files changed, 52 insertions(+), 7 deletions(-) diff --git a/client/coral-embed-stream/src/tabs/profile/containers/Profile.js b/client/coral-embed-stream/src/tabs/profile/containers/Profile.js index 9bc9cc703..0272c1535 100644 --- a/client/coral-embed-stream/src/tabs/profile/containers/Profile.js +++ b/client/coral-embed-stream/src/tabs/profile/containers/Profile.js @@ -54,6 +54,15 @@ const withProfileQuery = withQuery( me { id username + state { + status { + username { + history { + created_at + } + } + } + } } ...${getDefinitionName(TabPanel.fragments.root)} ${getSlotFragmentSpreads(slots, 'root')} diff --git a/client/coral-framework/utils/user.js b/client/coral-framework/utils/user.js index f50fca335..610557542 100644 --- a/client/coral-framework/utils/user.js +++ b/client/coral-framework/utils/user.js @@ -1,4 +1,5 @@ import get from 'lodash/get'; +import moment from 'moment'; /** * getReliability @@ -33,3 +34,18 @@ export const isSuspended = user => { export const isBanned = user => { return get(user, 'state.status.banned.status'); }; + +/** + * canUsernameBeUpdated + * retrieves boolean whether a username can be updated or not + */ + +export const canUsernameBeUpdated = status => { + const oldestEditTime = moment() + .subtract(14, 'days') + .toDate(); + + return !status.username.history.some(({ created_at }) => + moment(created_at).isAfter(oldestEditTime) + ); +}; diff --git a/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsername.js b/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsername.js index 4d6bc41f6..c30a43a29 100644 --- a/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsername.js +++ b/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsername.js @@ -7,6 +7,7 @@ import ChangeUsernameDialog from './ChangeUsernameDialog'; import { t } from 'plugin-api/beta/client/services'; import InputField from './InputField'; import { getErrorMessages } from 'coral-framework/utils'; +import { canUsernameBeUpdated } from 'coral-framework/utils/user'; const initialState = { editing: false, @@ -84,8 +85,13 @@ class ChangeUsername extends React.Component { }; render() { - const { username, emailAddress } = this.props; - const { editing } = this.state; + const { + username, + emailAddress, + root: { me: { state: { status } } }, + notify, + } = this.props; + const { editing, formData, showDialog } = this.state; return (
{editing ? ( @@ -170,6 +178,7 @@ class ChangeUsername extends React.Component { } ChangeUsername.propTypes = { + root: PropTypes.object.isRequired, setUsername: PropTypes.func.isRequired, notify: PropTypes.func.isRequired, username: PropTypes.string, diff --git a/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsernameDialog.js b/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsernameDialog.js index 3ebf6ff02..441a60594 100644 --- a/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsernameDialog.js +++ b/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsernameDialog.js @@ -20,10 +20,19 @@ class ChangeUsernameDialog extends React.Component { confirmChanges = async () => { if (this.formHasError()) { this.showError(); - } else { - await this.props.saveChanges(); - this.props.closeDialog(); + return; } + + if (!this.props.canUsernameBeUpdated) { + this.props.notify( + 'error', + "Username can't be updated. Usernames can be changed every 14 days" + ); + return; + } + + await this.props.saveChanges(); + this.props.closeDialog(); }; formHasError = () => @@ -101,6 +110,8 @@ ChangeUsernameDialog.propTypes = { onChange: PropTypes.func, username: PropTypes.string, formData: PropTypes.object, + canUsernameBeUpdated: PropTypes.bool.isRequired, + notify: PropTypes.func.isRequired, }; export default ChangeUsernameDialog; From cdb392cd0ee62bd0b967dbbc9df240adebd6375b Mon Sep 17 00:00:00 2001 From: okbel Date: Mon, 30 Apr 2018 20:03:01 -0300 Subject: [PATCH 5/5] Adding translations --- .../profile-settings/components/ChangeUsernameDialog.js | 2 +- plugins/talk-plugin-auth/client/translations.yml | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsernameDialog.js b/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsernameDialog.js index 441a60594..321b36926 100644 --- a/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsernameDialog.js +++ b/plugins/talk-plugin-auth/client/profile-settings/components/ChangeUsernameDialog.js @@ -26,7 +26,7 @@ class ChangeUsernameDialog extends React.Component { if (!this.props.canUsernameBeUpdated) { this.props.notify( 'error', - "Username can't be updated. Usernames can be changed every 14 days" + t('talk-plugin-auth.change_username.change_username_attempt') ); return; } diff --git a/plugins/talk-plugin-auth/client/translations.yml b/plugins/talk-plugin-auth/client/translations.yml index 8362a2e0b..7cc0d9267 100644 --- a/plugins/talk-plugin-auth/client/translations.yml +++ b/plugins/talk-plugin-auth/client/translations.yml @@ -144,7 +144,6 @@ en: changed_password_msg: "Changed Password - Your password has been successfully changed" change_username: change_username_note: "Usernames can be changed every 14 days" - changed_username_success_msg: "Username has been updated" save: "Save" edit_profile: "Edit Profile" cancel: "Cancel" @@ -155,6 +154,8 @@ en: bottom_note: "Note: You will not be able to change your username again for 14 days" confirm_changes: "Confirm Changes" username_does_not_match: "Username does not match" + changed_username_success_msg: "Username Changed - Your username has been successfully changed. You will not be able to change your user name for 14 days." + change_username_attempt: "Username can't be updated. Usernames can be changed every 14 days" de: talk-plugin-auth: login: @@ -268,7 +269,8 @@ es: bottom_note: "Nota: No podrás cambiar tu usuario por 14 días" confirm_changes: "Confirmar Cambios" username_does_not_match: "El usuario no coincide" - changed_username_success_msg: "Username Changed - Your username has been successfully changed. You will not be able to change your user name for 14 days." + changed_username_success_msg: "Usuario Actualizado - Tu usuario ha sido exitosamente actualizado. No podrás cambiar el usuario por 14 días." + change_username_attempt: "El usuario no puede ser actualizado. Los usuarios pueden ser cambiados cada 14 días." fr: talk-plugin-auth: login: