diff --git a/api-server/src/common/models/user.js b/api-server/src/common/models/user.js index 38da2a3e3b..d34f95e9d8 100644 --- a/api-server/src/common/models/user.js +++ b/api-server/src/common/models/user.js @@ -108,12 +108,13 @@ function isTheSame(val1, val2) { function getAboutProfile({ username, + usernameDisplay, githubProfile: github, progressTimestamps = [], bio }) { return { - username, + username: usernameDisplay || username, github, browniePoints: progressTimestamps.length, bio @@ -127,7 +128,8 @@ function nextTick(fn) { const getRandomNumber = () => Math.random(); function populateRequiredFields(user) { - user.username = user.username.trim().toLowerCase(); + user.usernameDisplay = user.username.trim(); + user.username = user.usernameDisplay.toLowerCase(); user.email = typeof user.email === 'string' ? user.email.trim().toLowerCase() @@ -724,42 +726,6 @@ export default function initializeUser(User) { ); }; - User.prototype.updateMyUsername = function updateMyUsername(newUsername) { - return Observable.defer(() => { - const isOwnUsername = isTheSame(newUsername, this.username); - if (isOwnUsername) { - return Observable.of(dedent` - ${newUsername} is already associated with this account. - `); - } - return Observable.fromPromise(User.doesExist(newUsername)); - }).flatMap(boolOrMessage => { - if (typeof boolOrMessage === 'string') { - return Observable.of(boolOrMessage); - } - if (boolOrMessage) { - return Observable.of(dedent` - ${newUsername} is already associated with a different account. - `); - } - - const usernameUpdate = new Promise((resolve, reject) => - this.updateAttribute('username', newUsername, err => { - if (err) { - return reject(err); - } - return resolve(); - }) - ); - - return Observable.fromPromise(usernameUpdate).map( - () => dedent` - Your username has been updated successfully. - ` - ); - }); - }; - function prepUserForPublish(user, profileUI) { const { about, diff --git a/api-server/src/common/models/user.json b/api-server/src/common/models/user.json index 8654fb98de..ea2ecec9c9 100644 --- a/api-server/src/common/models/user.json +++ b/api-server/src/common/models/user.json @@ -74,6 +74,9 @@ }, "require": true }, + "usernameDisplay": { + "type": "string" + }, "about": { "type": "string", "default": "" diff --git a/api-server/src/server/boot/settings.js b/api-server/src/server/boot/settings.js index 26de9e4050..952f795296 100644 --- a/api-server/src/server/boot/settings.js +++ b/api-server/src/server/boot/settings.js @@ -173,11 +173,14 @@ function updateMyAbout(req, res, next) { function createUpdateMyUsername(app) { const { User } = app.models; return async function updateMyUsername(req, res, next) { - const { - user, - body: { username } - } = req; - if (username === user.username) { + const { user, body } = req; + const usernameDisplay = body.username.trim(); + const username = usernameDisplay.toLowerCase(); + if ( + username === user.username && + user.usernameDisplay && + usernameDisplay === user.usernameDisplay + ) { return res.json({ type: 'info', message: 'flash.username-used' @@ -192,7 +195,8 @@ function createUpdateMyUsername(app) { }); } - const exists = await User.doesExist(username); + const exists = + username === user.username ? false : await User.doesExist(username); if (exists) { return res.json({ @@ -201,7 +205,7 @@ function createUpdateMyUsername(app) { }); } - return user.updateAttribute('username', username, err => { + return user.updateAttributes({ username, usernameDisplay }, err => { if (err) { res.status(500).json(standardErrorMessage); return next(err); @@ -210,7 +214,7 @@ function createUpdateMyUsername(app) { return res.status(200).json({ type: 'success', message: `flash.username-updated`, - variables: { username: username } + variables: { username: usernameDisplay } }); }); }; diff --git a/api-server/src/server/boot/user.js b/api-server/src/server/boot/user.js index 24ae6213bc..d0ed41a8dc 100644 --- a/api-server/src/server/boot/user.js +++ b/api-server/src/server/boot/user.js @@ -76,6 +76,7 @@ function createReadSessionUser(app) { user: { [user.username]: { ...pick(user, userPropsForSession), + username: user.usernameDisplay || user.username, isEmailVerified: !!user.emailVerified, isGithub: !!user.githubProfile, isLinkedIn: !!user.linkedin, diff --git a/client/src/components/settings/username.tsx b/client/src/components/settings/username.tsx index 671ba1d272..0aedb68df9 100644 --- a/client/src/components/settings/username.tsx +++ b/client/src/components/settings/username.tsx @@ -131,7 +131,9 @@ class UsernameSettings extends Component { submitClicked: false }, () => - this.state.isFormPristine || this.state.characterValidation.error + this.state.isFormPristine || + this.state.characterValidation.error || + username.toLowerCase().trim() === newValue.toLowerCase().trim() ? null : validateUsername(this.state.formValue) ); diff --git a/cypress/integration/settings/username-change.js b/cypress/integration/settings/username-change.js index fd0b8bbd3c..ca13b3a1f9 100644 --- a/cypress/integration/settings/username-change.js +++ b/cypress/integration/settings/username-change.js @@ -148,6 +148,19 @@ describe('Username input field', () => { cy.resetUsername(); }); + it('Should change username with uppercase characters if `Save` button is clicked', () => { + cy.get('@usernameInput') + .clear({ force: true }) + .type('Quincy', { force: true }); + + cy.contains('Username is available'); + + cy.get('@usernameForm').contains('Save').click({ force: true }); + cy.contains('Account Settings for Quincy').should('be.visible'); + + cy.resetUsername(); + }); + it('Should show flash message showing username has been updated', () => { cy.get('@usernameInput') .clear({ force: true }) @@ -195,22 +208,4 @@ describe('Username input field', () => { cy.resetUsername(); }); - it('Should show warning if username includes uppercase characters', () => { - cy.get('@usernameInput') - .clear({ force: true }) - .type('QuincyLarson', { force: true }); - - cy.contains('Username "QuincyLarson" must be lowercase') - .should('be.visible') - .should('have.attr', 'role', 'alert') - .should('have.class', 'alert alert-danger'); - }); - - it('Should not be able to click the `Save` button if username includes uppercase characters', () => { - cy.get('@usernameInput') - .clear({ force: true }) - .type('QuincyLarson', { force: true }); - - cy.get('@usernameForm').contains('Save').should('be.disabled'); - }); }); diff --git a/utils/validate.js b/utils/validate.js index d604c41ab6..dd888a9688 100644 --- a/utils/validate.js +++ b/utils/validate.js @@ -8,20 +8,16 @@ const usernameIsHttpStatusCode = { valid: false, error: 'is a reserved error code' }; -const usernameUpperCase = { valid: false, error: 'must be lowercase' }; const isNumeric = num => !isNaN(num); const validCharsRE = /^[a-zA-Z0-9\-_+]*$/; const isHttpStatusCode = str => isNumeric(str) && parseInt(str, 10) >= 100 && parseInt(str, 10) <= 599; -const isUsernameLowercase = str => { - return str === str.toLowerCase(); -}; + const isValidUsername = str => { if (!validCharsRE.test(str)) return invalidCharError; if (str.length < 3) return usernameTooShort; if (isHttpStatusCode(str)) return usernameIsHttpStatusCode; - if (!isUsernameLowercase(str)) return usernameUpperCase; return validationSuccess; }; @@ -29,10 +25,8 @@ module.exports = { isNumeric, isHttpStatusCode, isValidUsername, - isUsernameLowercase, validationSuccess, usernameTooShort, usernameIsHttpStatusCode, - invalidCharError, - usernameUpperCase + invalidCharError }; diff --git a/utils/validate.test.js b/utils/validate.test.js index ab40a57f9c..24ce35e766 100644 --- a/utils/validate.test.js +++ b/utils/validate.test.js @@ -3,8 +3,7 @@ const { usernameTooShort, validationSuccess, usernameIsHttpStatusCode, - invalidCharError, - usernameUpperCase + invalidCharError } = require('./validate'); function inRange(num, range) { @@ -37,9 +36,6 @@ describe('isValidUsername', () => { expect(isValidUsername('a-b')).toStrictEqual(validationSuccess); expect(isValidUsername('a_b')).toStrictEqual(validationSuccess); }); - it('rejects uppercase characters', () => { - expect(isValidUsername('Quincy')).toStrictEqual(usernameUpperCase); - }); it('rejects all other ASCII characters', () => { const allowedCharactersList = ['-', '_', '+']; @@ -54,7 +50,7 @@ describe('isValidUsername', () => { let expected = invalidCharError; if (allowedCharactersList.includes(char)) expected = validationSuccess; if (inRange(code, numbers)) expected = validationSuccess; - if (inRange(code, upperCase)) expected = usernameUpperCase; + if (inRange(code, upperCase)) expected = validationSuccess; if (inRange(code, lowerCase)) expected = validationSuccess; expect(isValidUsername(base + char)).toStrictEqual(expected); }