feat: Allow display username with uppercase characters (#43667)

* feat: Allow display username with uppercase characters

* fix: ensure user can change username to uppercased version

* fix: ensure that same username in a different case does not require validation
This commit is contained in:
Valeria
2021-11-04 11:18:40 +01:00
committed by GitHub
parent a961b2c032
commit 753ea937ea
8 changed files with 40 additions and 79 deletions

View File

@ -108,12 +108,13 @@ function isTheSame(val1, val2) {
function getAboutProfile({ function getAboutProfile({
username, username,
usernameDisplay,
githubProfile: github, githubProfile: github,
progressTimestamps = [], progressTimestamps = [],
bio bio
}) { }) {
return { return {
username, username: usernameDisplay || username,
github, github,
browniePoints: progressTimestamps.length, browniePoints: progressTimestamps.length,
bio bio
@ -127,7 +128,8 @@ function nextTick(fn) {
const getRandomNumber = () => Math.random(); const getRandomNumber = () => Math.random();
function populateRequiredFields(user) { function populateRequiredFields(user) {
user.username = user.username.trim().toLowerCase(); user.usernameDisplay = user.username.trim();
user.username = user.usernameDisplay.toLowerCase();
user.email = user.email =
typeof user.email === 'string' typeof user.email === 'string'
? user.email.trim().toLowerCase() ? 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) { function prepUserForPublish(user, profileUI) {
const { const {
about, about,

View File

@ -74,6 +74,9 @@
}, },
"require": true "require": true
}, },
"usernameDisplay": {
"type": "string"
},
"about": { "about": {
"type": "string", "type": "string",
"default": "" "default": ""

View File

@ -173,11 +173,14 @@ function updateMyAbout(req, res, next) {
function createUpdateMyUsername(app) { function createUpdateMyUsername(app) {
const { User } = app.models; const { User } = app.models;
return async function updateMyUsername(req, res, next) { return async function updateMyUsername(req, res, next) {
const { const { user, body } = req;
user, const usernameDisplay = body.username.trim();
body: { username } const username = usernameDisplay.toLowerCase();
} = req; if (
if (username === user.username) { username === user.username &&
user.usernameDisplay &&
usernameDisplay === user.usernameDisplay
) {
return res.json({ return res.json({
type: 'info', type: 'info',
message: 'flash.username-used' 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) { if (exists) {
return res.json({ return res.json({
@ -201,7 +205,7 @@ function createUpdateMyUsername(app) {
}); });
} }
return user.updateAttribute('username', username, err => { return user.updateAttributes({ username, usernameDisplay }, err => {
if (err) { if (err) {
res.status(500).json(standardErrorMessage); res.status(500).json(standardErrorMessage);
return next(err); return next(err);
@ -210,7 +214,7 @@ function createUpdateMyUsername(app) {
return res.status(200).json({ return res.status(200).json({
type: 'success', type: 'success',
message: `flash.username-updated`, message: `flash.username-updated`,
variables: { username: username } variables: { username: usernameDisplay }
}); });
}); });
}; };

View File

@ -76,6 +76,7 @@ function createReadSessionUser(app) {
user: { user: {
[user.username]: { [user.username]: {
...pick(user, userPropsForSession), ...pick(user, userPropsForSession),
username: user.usernameDisplay || user.username,
isEmailVerified: !!user.emailVerified, isEmailVerified: !!user.emailVerified,
isGithub: !!user.githubProfile, isGithub: !!user.githubProfile,
isLinkedIn: !!user.linkedin, isLinkedIn: !!user.linkedin,

View File

@ -131,7 +131,9 @@ class UsernameSettings extends Component<UsernameProps, UsernameState> {
submitClicked: false submitClicked: false
}, },
() => () =>
this.state.isFormPristine || this.state.characterValidation.error this.state.isFormPristine ||
this.state.characterValidation.error ||
username.toLowerCase().trim() === newValue.toLowerCase().trim()
? null ? null
: validateUsername(this.state.formValue) : validateUsername(this.state.formValue)
); );

View File

@ -148,6 +148,19 @@ describe('Username input field', () => {
cy.resetUsername(); 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', () => { it('Should show flash message showing username has been updated', () => {
cy.get('@usernameInput') cy.get('@usernameInput')
.clear({ force: true }) .clear({ force: true })
@ -195,22 +208,4 @@ describe('Username input field', () => {
cy.resetUsername(); 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');
});
}); });

View File

@ -8,20 +8,16 @@ const usernameIsHttpStatusCode = {
valid: false, valid: false,
error: 'is a reserved error code' error: 'is a reserved error code'
}; };
const usernameUpperCase = { valid: false, error: 'must be lowercase' };
const isNumeric = num => !isNaN(num); const isNumeric = num => !isNaN(num);
const validCharsRE = /^[a-zA-Z0-9\-_+]*$/; const validCharsRE = /^[a-zA-Z0-9\-_+]*$/;
const isHttpStatusCode = str => const isHttpStatusCode = str =>
isNumeric(str) && parseInt(str, 10) >= 100 && parseInt(str, 10) <= 599; isNumeric(str) && parseInt(str, 10) >= 100 && parseInt(str, 10) <= 599;
const isUsernameLowercase = str => {
return str === str.toLowerCase();
};
const isValidUsername = str => { const isValidUsername = str => {
if (!validCharsRE.test(str)) return invalidCharError; if (!validCharsRE.test(str)) return invalidCharError;
if (str.length < 3) return usernameTooShort; if (str.length < 3) return usernameTooShort;
if (isHttpStatusCode(str)) return usernameIsHttpStatusCode; if (isHttpStatusCode(str)) return usernameIsHttpStatusCode;
if (!isUsernameLowercase(str)) return usernameUpperCase;
return validationSuccess; return validationSuccess;
}; };
@ -29,10 +25,8 @@ module.exports = {
isNumeric, isNumeric,
isHttpStatusCode, isHttpStatusCode,
isValidUsername, isValidUsername,
isUsernameLowercase,
validationSuccess, validationSuccess,
usernameTooShort, usernameTooShort,
usernameIsHttpStatusCode, usernameIsHttpStatusCode,
invalidCharError, invalidCharError
usernameUpperCase
}; };

View File

@ -3,8 +3,7 @@ const {
usernameTooShort, usernameTooShort,
validationSuccess, validationSuccess,
usernameIsHttpStatusCode, usernameIsHttpStatusCode,
invalidCharError, invalidCharError
usernameUpperCase
} = require('./validate'); } = require('./validate');
function inRange(num, range) { function inRange(num, range) {
@ -37,9 +36,6 @@ describe('isValidUsername', () => {
expect(isValidUsername('a-b')).toStrictEqual(validationSuccess); expect(isValidUsername('a-b')).toStrictEqual(validationSuccess);
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', () => { it('rejects all other ASCII characters', () => {
const allowedCharactersList = ['-', '_', '+']; const allowedCharactersList = ['-', '_', '+'];
@ -54,7 +50,7 @@ describe('isValidUsername', () => {
let expected = invalidCharError; let expected = invalidCharError;
if (allowedCharactersList.includes(char)) expected = validationSuccess; if (allowedCharactersList.includes(char)) expected = validationSuccess;
if (inRange(code, numbers)) 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; if (inRange(code, lowerCase)) expected = validationSuccess;
expect(isValidUsername(base + char)).toStrictEqual(expected); expect(isValidUsername(base + char)).toStrictEqual(expected);
} }