opendev/gerrit
Code Issues Proposed changes

Merge changes from topic "username" into stable-2.15

Browse Source 
* changes:
  Add username to registration dialog
  Only allow username editing if username is unset
  Allow username editing if permitted by project cfg
This commit is contained in:
David Pursehouse
2017-12-07 04:46:17 +00:00
committed by Gerrit Code Review
parent 791f087c6c 88c0b9d3d4
commit df87a193b8
9 changed files with 268 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
polygerrit-ui/app/elements/core/gr-navigation/gr-navigation.html
View File

@@ -313,6 +313,10 @@ limitations under the License.
}
this._navigate(relativeUrl);
},
getUrlForSettings() {
return this._getUrlFor({view: Gerrit.Nav.View.SETTINGS});
},
};
})(window);
</script>

10
polygerrit-ui/app/elements/core/gr-router/gr-router.js
View File

@@ -262,6 +262,8 @@
if (params.edit) {
url += ',edit';
}
} else if (params.view === Gerrit.Nav.View.SETTINGS) {
url = this._generateSettingsUrl(params);
} else {
throw new Error('Can\'t generate');
}
@@ -269,6 +271,14 @@
return base + url;
},
/**
* @param {!Object} params
* @return {string}
*/
_generateSettingsUrl(params) {
return '/settings';
},
/**
* Given an object of parameters, potentially including a `patchNum` or a
* `basePatchNum` or both, return a string representation of that range. If

19
polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.html
View File

@@ -44,17 +44,28 @@ limitations under the License.
date-str="[[_account.registered_on]]"></gr-date-formatter>
</span>
</section>
<section>
<section id="usernameSection">
<span class="title">Username</span>
<span class="value">[[_account.username]]</span>
<span
hidden$="[[usernameMutable]]"
class="value">[[_username]]</span>
<span
hidden$="[[!usernameMutable]]"
class="value">
<input
is="iron-input"
id="usernameInput"
disabled="[[_saving]]"
on-keydown="_handleKeydown"
bind-value="{{_username}}">
</section>
<section id="nameSection">
<span class="title">Full name</span>
<span
hidden$="[[mutable]]"
hidden$="[[nameMutable]]"
class="value">[[_account.name]]</span>
<span
hidden$="[[!mutable]]"
hidden$="[[!nameMutable]]"
class="value">
<input
is="iron-input"

54
polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.js
View File

@@ -24,21 +24,31 @@
*/
properties: {
mutable: {
usernameMutable: {
type: Boolean,
notify: true,
computed: '_computeMutable(_serverConfig)',
computed: '_computeUsernameMutable(_serverConfig, _account.username)',
},
nameMutable: {
type: Boolean,
notify: true,
computed: '_computeNameMutable(_serverConfig)',
},
hasUnsavedChanges: {
type: Boolean,
notify: true,
computed: '_computeHasUnsavedChanges(_hasNameChange, _hasStatusChange)',
computed: '_computeHasUnsavedChanges(_hasNameChange, ' +
'_hasUsernameChange, _hasStatusChange)',
},
_hasNameChange: {
type: Boolean,
value: false,
},
_hasUsernameChange: {
type: Boolean,
value: false,
},
_hasStatusChange: {
type: Boolean,
value: false,
@@ -54,6 +64,10 @@
/** @type {?} */
_account: Object,
_serverConfig: Object,
_username: {
type: String,
observer: '_usernameChanged',
},
},
observers: [
@@ -71,7 +85,11 @@
}));
promises.push(this.$.restAPI.getAccount().then(account => {
// Provide predefined value for username to trigger computation of
// username mutability.
account.username = account.username || '';
this._account = account;
this._username = account.username;
}));
return Promise.all(promises).then(() => {
@@ -88,6 +106,7 @@
// Set only the fields that have changed.
// Must be done in sequence to avoid race conditions (@see Issue 5721)
return this._maybeSetName()
.then(this._maybeSetUsername.bind(this))
.then(this._maybeSetStatus.bind(this))
.then(() => {
this._hasNameChange = false;
@@ -98,9 +117,15 @@
},
_maybeSetName() {
return this._hasNameChange && this.mutable ?
this.$.restAPI.setAccountName(this._account.name) :
Promise.resolve();
return this._hasNameChange && this.nameMutable ?
this.$.restAPI.setAccountName(this._account.name) :
Promise.resolve();
},
_maybeSetUsername() {
return this._hasUsernameChange && this.usernameMutable ?
this.$.restAPI.setAccountUsername(this._username) :
Promise.resolve();
},
_maybeSetStatus() {
@@ -109,11 +134,17 @@
Promise.resolve();
},
_computeHasUnsavedChanges(name, status) {
return name || status;
_computeHasUnsavedChanges(nameChanged, usernameChanged, statusChanged) {
return nameChanged || usernameChanged || statusChanged;
},
_computeMutable(config) {
_computeUsernameMutable(config, username) {
// Username may not be changed once it is set.
return config.auth.editable_account_fields.includes('USER_NAME') &&
!username;
},
_computeNameMutable(config) {
return config.auth.editable_account_fields.includes('FULL_NAME');
},
@@ -122,6 +153,11 @@
this._hasStatusChange = true;
},
_usernameChanged() {
if (this._loading) { return; }
this._hasUsernameChange = true;
},
_nameChanged() {
if (this._loading) { return; }
this._hasNameChange = true;

97
polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info_test.html
View File

@@ -84,18 +84,18 @@ limitations under the License.
assert.equal(valueOf('Username').textContent, account.username);
});
test('user name render (immutable)', () => {
test('full name render (immutable)', () => {
const section = element.$.nameSection;
const displaySpan = section.querySelectorAll('.value')[0];
const inputSpan = section.querySelectorAll('.value')[1];
assert.isFalse(element.mutable);
assert.isFalse(element.nameMutable);
assert.isFalse(displaySpan.hasAttribute('hidden'));
assert.equal(displaySpan.textContent, account.name);
assert.isTrue(inputSpan.hasAttribute('hidden'));
});
test('user name render (mutable)', () => {
test('full name render (mutable)', () => {
element.set('_serverConfig',
{auth: {editable_account_fields: ['FULL_NAME']}});
@@ -103,32 +103,64 @@ limitations under the License.
const displaySpan = section.querySelectorAll('.value')[0];
const inputSpan = section.querySelectorAll('.value')[1];
assert.isTrue(element.mutable);
assert.isTrue(element.nameMutable);
assert.isTrue(displaySpan.hasAttribute('hidden'));
assert.equal(element.$.nameInput.bindValue, account.name);
assert.isFalse(inputSpan.hasAttribute('hidden'));
});
test('username render (immutable)', () => {
const section = element.$.usernameSection;
const displaySpan = section.querySelectorAll('.value')[0];
const inputSpan = section.querySelectorAll('.value')[1];
assert.isFalse(element.usernameMutable);
assert.isFalse(displaySpan.hasAttribute('hidden'));
assert.equal(displaySpan.textContent, account.username);
assert.isTrue(inputSpan.hasAttribute('hidden'));
});
test('username render (mutable)', () => {
element.set('_serverConfig',
{auth: {editable_account_fields: ['USER_NAME']}});
element.set('_account.username', '');
element.set('_username', '');
const section = element.$.usernameSection;
const displaySpan = section.querySelectorAll('.value')[0];
const inputSpan = section.querySelectorAll('.value')[1];
assert.isTrue(element.usernameMutable);
assert.isTrue(displaySpan.hasAttribute('hidden'));
assert.equal(element.$.usernameInput.bindValue, account.username);
assert.isFalse(inputSpan.hasAttribute('hidden'));
});
suite('account info edit', () => {
let nameChangedSpy;
let usernameChangedSpy;
let statusChangedSpy;
let nameStub;
let usernameStub;
let statusStub;
setup(() => {
nameChangedSpy = sandbox.spy(element, '_nameChanged');
usernameChangedSpy = sandbox.spy(element, '_usernameChanged');
statusChangedSpy = sandbox.spy(element, '_statusChanged');
element.set('_serverConfig',
{auth: {editable_account_fields: ['FULL_NAME']}});
{auth: {editable_account_fields: ['FULL_NAME', 'USER_NAME']}});
nameStub = sandbox.stub(element.$.restAPI, 'setAccountName', name =>
Promise.resolve());
nameStub = sandbox.stub(element.$.restAPI, 'setAccountName',
name => Promise.resolve());
usernameStub = sandbox.stub(element.$.restAPI, 'setAccountUsername',
username => Promise.resolve());
statusStub = sandbox.stub(element.$.restAPI, 'setAccountStatus',
status => Promise.resolve());
});
test('name', done => {
assert.isTrue(element.mutable);
assert.isTrue(element.nameMutable);
assert.isFalse(element.hasUnsavedChanges);
element.set('_account.name', 'new name');
@@ -137,18 +169,40 @@ limitations under the License.
assert.isFalse(statusChangedSpy.called);
assert.isTrue(element.hasUnsavedChanges);
MockInteractions.pressAndReleaseKeyOn(element.$.nameInput, 13);
element.save().then(() => {
assert.isFalse(usernameStub.called);
assert.isTrue(nameStub.called);
assert.isFalse(statusStub.called);
nameStub.lastCall.returnValue.then(() => {
assert.equal(nameStub.lastCall.args[0], 'new name');
done();
});
});
});
assert.isTrue(nameStub.called);
assert.isFalse(statusStub.called);
nameStub.lastCall.returnValue.then(() => {
assert.equal(nameStub.lastCall.args[0], 'new name');
done();
test('username', done => {
element.set('_account.username', '');
element._hasUsernameChange = false;
assert.isTrue(element.usernameMutable);
element.set('_username', 'new username');
assert.isTrue(usernameChangedSpy.called);
assert.isFalse(statusChangedSpy.called);
assert.isTrue(element.hasUnsavedChanges);
element.save().then(() => {
assert.isTrue(usernameStub.called);
assert.isFalse(nameStub.called);
assert.isFalse(statusStub.called);
usernameStub.lastCall.returnValue.then(() => {
assert.equal(usernameStub.lastCall.args[0], 'new username');
done();
});
});
});
test('status', done => {
assert.isTrue(element.mutable);
assert.isFalse(element.hasUnsavedChanges);
element.set('_account.status', 'new status');
@@ -158,6 +212,7 @@ limitations under the License.
assert.isTrue(element.hasUnsavedChanges);
element.save().then(() => {
assert.isFalse(usernameStub.called);
assert.isTrue(statusStub.called);
assert.isFalse(nameStub.called);
statusStub.lastCall.returnValue.then(() => {
@@ -178,16 +233,18 @@ limitations under the License.
nameChangedSpy = sandbox.spy(element, '_nameChanged');
statusChangedSpy = sandbox.spy(element, '_statusChanged');
element.set('_serverConfig',
{auth: {editable_account_fields: ['FULL_NAME']}});
{auth: {editable_account_fields: ['FULL_NAME']}});
nameStub = sandbox.stub(element.$.restAPI, 'setAccountName', name =>
Promise.resolve());
nameStub = sandbox.stub(element.$.restAPI, 'setAccountName',
name => Promise.resolve());
statusStub = sandbox.stub(element.$.restAPI, 'setAccountStatus',
status => Promise.resolve());
sandbox.stub(element.$.restAPI, 'setAccountUsername',
username => Promise.resolve());
});
test('set name and status', done => {
assert.isTrue(element.mutable);
assert.isTrue(element.nameMutable);
assert.isFalse(element.hasUnsavedChanges);
element.set('_account.name', 'new name');
@@ -231,7 +288,7 @@ limitations under the License.
const displaySpan = section.querySelectorAll('.value')[0];
const inputSpan = section.querySelectorAll('.value')[1];
assert.isFalse(element.mutable);
assert.isFalse(element.nameMutable);
assert.isFalse(element.hasUnsavedChanges);

46
polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.html
View File

@@ -16,6 +16,7 @@ limitations under the License.
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../styles/gr-form-styles.html">
<link rel="import" href="../../core/gr-navigation/gr-navigation.html">
<link rel="import" href="../../shared/gr-button/gr-button.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
<link rel="import" href="../../../styles/shared-styles.html">
@@ -37,18 +38,23 @@ limitations under the License.
header {
border-bottom: 1px solid #cdcdcd;
font-family: var(--font-family-bold);
margin-bottom: 1em;
}
header,
main,
footer {
.container {
padding: .5em 1.5em;
}
footer {
display: flex;
justify-content: space-between;
justify-content: flex-end;
}
footer gr-button {
margin-left: 1em;
}
input {
width: 20em;
}
</style>
<main class="gr-form-styles">
<div class="container gr-form-styles">
<header>Please confirm your contact information</header>
<main>
<p>
@@ -64,8 +70,15 @@ limitations under the License.
is="iron-input"
id="name"
bind-value="{{_account.name}}"
disabled="[[_saving]]"
on-keydown="_handleNameKeydown">
disabled="[[_saving]]">
</section>
<section>
<div class="title">Username</div>
<input
is="iron-input"
id="username"
bind-value="{{_account.username}}"
disabled="[[_saving]]">
</section>
<section>
<div class="title">Preferred Email</div>
@@ -78,19 +91,26 @@ limitations under the License.
</template>
</select>
</section>
<hr>
<p>
More configuration options for Gerrit may be found in the
<a on-tap="close" href$="[[_computeSettingsUrl(_account)]]">settings</a>.
</p>
</main>
<footer>
<gr-button
id="saveButton"
primary
disabled="[[_saving]]"
on-tap="_handleSave">Save</gr-button>
<gr-button
id="closeButton"
link
disabled="[[_saving]]"
on-tap="_handleClose">Close</gr-button>
<gr-button
id="saveButton"
primary
link
disabled="[[_computeSaveDisabled(_account.name, _account.username, _account.email, _saving)]]"
on-tap="_handleSave">Save</gr-button>
</footer>
</main>
</div>
<gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
</template>
<script src="gr-registration-dialog.js"></script>

45
polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.js
View File

@@ -31,8 +31,18 @@
properties: {
/** @type {?} */
_account: Object,
_saving: Boolean,
_account: {
type: Object,
value: () => {
// Prepopulate possibly undefined fields with values to trigger
// computed bindings.
return {email: null, name: null, username: null};
},
},
_saving: {
type: Boolean,
value: false,
},
},
hostAttributes: {
@@ -41,22 +51,19 @@
attached() {
this.$.restAPI.getAccount().then(account => {
this._account = account;
// Using Object.assign here allows preservation of the default values
// supplied in the value generating function of this._account, unless
// they are overridden by properties in the account from the response.
this._account = Object.assign({}, this._account, account);
});
},
_handleNameKeydown(e) {
if (e.keyCode === 13) { // Enter
e.stopPropagation();
this._save();
}
},
_save() {
this._saving = true;
const promises = [
this.$.restAPI.setAccountName(this.$.name.value),
this.$.restAPI.setPreferredAccountEmail(this.$.email.value),
this.$.restAPI.setAccountUsername(this.$.username.value),
this.$.restAPI.setPreferredAccountEmail(this.$.email.value || ''),
];
return Promise.all(promises).then(() => {
this._saving = false;
@@ -66,15 +73,25 @@
_handleSave(e) {
e.preventDefault();
this._save().then(() => {
this.fire('close');
});
this._save().then(this.close.bind(this));
},
_handleClose(e) {
e.preventDefault();
this.close();
},
close() {
this._saving = true; // disable buttons indefinitely
this.fire('close');
},
_computeSaveDisabled(name, username, email, saving) {
return !name || !username || !email || saving;
},
_computeSettingsUrl() {
return Gerrit.Nav.getUrlForSettings();
},
});
})();

32
polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.html
View File

@@ -41,13 +41,16 @@ limitations under the License.
suite('gr-registration-dialog tests', () => {
let element;
let account;
let sandbox;
let _listeners;
setup(done => {
sandbox = sinon.sandbox.create();
_listeners = {};
account = {
name: 'name',
username: 'username',
email: 'email',
secondary_emails: [
'email2',
@@ -65,6 +68,10 @@ limitations under the License.
account.name = name;
return Promise.resolve();
},
setAccountUsername(username) {
account.username = username;
return Promise.resolve();
},
setPreferredAccountEmail(email) {
account.email = email;
return Promise.resolve();
@@ -75,6 +82,7 @@ limitations under the License.
});
teardown(() => {
sandbox.restore();
for (const eventType in _listeners) {
if (_listeners.hasOwnProperty(eventType)) {
element.removeEventListener(eventType, _listeners[eventType]);
@@ -119,32 +127,26 @@ limitations under the License.
}).then(done);
});
test('saves name and preferred email', done => {
test('saves account details', done => {
flush(() => {
element.$.name.value = 'new name';
element.$.username.value = 'new username';
element.$.email.value = 'email3';
// Nothing should be committed yet.
assert.equal(account.name, 'name');
assert.equal(account.username, 'username');
assert.equal(account.email, 'email');
// Save and verify new values are committed.
save().then(() => {
assert.equal(account.name, 'new name');
assert.equal(account.username, 'new username');
assert.equal(account.email, 'email3');
}).then(done);
});
});
test('pressing enter saves name', done => {
element.$.name.value = 'entered name';
save(() => {
MockInteractions.pressAndReleaseKeyOn(element.$.name, 13); // 'enter'
}).then(() => {
assert.equal(account.name, 'entered name');
}).then(done);
});
test('email select properly populated', done => {
element._account = {email: 'foo', secondary_emails: ['bar', 'baz']};
flush(() => {
@@ -152,5 +154,15 @@ limitations under the License.
done();
});
});
test('save btn disabled', () => {
const compute = element._computeSaveDisabled;
assert.isTrue(compute('', '', '', false));
assert.isTrue(compute('', 'test', 'test', false));
assert.isTrue(compute('test', '', 'test', false));
assert.isTrue(compute('test', 'test', '', false));
assert.isTrue(compute('test', 'test', 'test', true));
assert.isFalse(compute('test', 'test', 'test', false));
});
});
</script>

58
polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
View File

@@ -543,26 +543,40 @@
});
},
/**
* @param {?Object} obj
*/
_updateCachedAccount(obj) {
// If result of getAccount is in cache, update it in the cache
// so we don't have to invalidate it.
const cachedAccount = this._cache['/accounts/self/detail'];
if (cachedAccount) {
// Replace object in cache with new object to force UI updates.
this._cache['/accounts/self/detail'] =
Object.assign({}, cachedAccount, obj);
}
},
/**
* @param {string} name
* @param {function(?Response, string=)=} opt_errFn
* @param {?=} opt_ctx
*/
setAccountName(name, opt_errFn, opt_ctx) {
return this.send('PUT', '/accounts/self/name', {name}, opt_errFn,
opt_ctx).then(response => {
// If result of getAccount is in cache, update it in the cache
// so we don't have to invalidate it.
const cachedAccount = this._cache['/accounts/self/detail'];
if (cachedAccount) {
return this.getResponseObject(response).then(newName => {
// Replace object in cache with new object to force UI updates.
// TODO(logan): Polyfill for Object.assign in IE
this._cache['/accounts/self/detail'] = Object.assign(
{}, cachedAccount, {name: newName});
});
}
});
return this.send('PUT', '/accounts/self/name', {name}, opt_errFn, opt_ctx)
.then(response => this.getResponseObject(response)
.then(newName => this._updateCachedAccount({name: newName})));
},
/**
* @param {string} username
* @param {function(?Response, string=)=} opt_errFn
* @param {?=} opt_ctx
*/
setAccountUsername(username, opt_errFn, opt_ctx) {
return this.send('PUT', '/accounts/self/username', {username}, opt_errFn,
opt_ctx).then(response => this.getResponseObject(response)
.then(newName => this._updateCachedAccount({username: newName})));
},
/**
@@ -572,19 +586,9 @@
*/
setAccountStatus(status, opt_errFn, opt_ctx) {
return this.send('PUT', '/accounts/self/status', {status},
opt_errFn, opt_ctx).then(response => {
// If result of getAccount is in cache, update it in the cache
// so we don't have to invalidate it.
const cachedAccount = this._cache['/accounts/self/detail'];
if (cachedAccount) {
return this.getResponseObject(response).then(newStatus => {
// Replace object in cache with new object to force UI updates.
// TODO(logan): Polyfill for Object.assign in IE
this._cache['/accounts/self/detail'] = Object.assign(
{}, cachedAccount, {status: newStatus});
});
}
});
opt_errFn, opt_ctx).then(response => this.getResponseObject(response)
.then(newStatus => this._updateCachedAccount(
{status: newStatus})));
},
getAccountStatus(userId) {