Only permit registering username when allowed
The gr-account-details form used in settings houses logic for when to
display an editable username. In particular, the username is only
editable when the server config lists "USER_NAME" as an "editable
account field" and a username hasn't already been set.
However, this logic was not used in the gr-registration-dialog form.
Rather, an input for username was always shown, whether or not it was
editable. As a result, if a user tried to fill out a form when the
username could not be saved, it would always result in an error.
Use the same logic as gr-account-details in gr-registration-dialog for
deciding whether the username should be editable.
Bug: Issue 8167
Change-Id: Ia614b5996f0bcfe7fdf785efcebbbde27919e7d3
diff --git a/polygerrit-ui/app/elements/gr-app.html b/polygerrit-ui/app/elements/gr-app.html
index 6a2bfe0..c9e05e4 100644
--- a/polygerrit-ui/app/elements/gr-app.html
+++ b/polygerrit-ui/app/elements/gr-app.html
@@ -220,8 +220,9 @@
view="[[params.view]]"
on-close="_handleKeyboardShortcutDialogClose"></gr-keyboard-shortcuts-dialog>
</gr-overlay>
- <gr-overlay id="registration" with-backdrop>
+ <gr-overlay id="registrationOverlay" with-backdrop>
<gr-registration-dialog
+ id="registrationDialog"
settings-url="[[_settingsUrl]]"
on-account-detail-update="_handleAccountDetailUpdate"
on-close="_handleRegistrationDialogClose">
diff --git a/polygerrit-ui/app/elements/gr-app.js b/polygerrit-ui/app/elements/gr-app.js
index 3b66ae8..2a247b9 100644
--- a/polygerrit-ui/app/elements/gr-app.js
+++ b/polygerrit-ui/app/elements/gr-app.js
@@ -191,7 +191,10 @@
this.async(() => this.set('_showPluginScreen', true), 1);
}
if (this.params.justRegistered) {
- this.$.registration.open();
+ this.$.registrationOverlay.open();
+ this.$.registrationDialog.loadData().then(() => {
+ this.$.registrationOverlay.refit();
+ });
}
this.$.header.unfloat();
},
@@ -272,7 +275,7 @@
_handleRegistrationDialogClose(e) {
this.params.justRegistered = false;
- this.$.registration.close();
+ this.$.registrationOverlay.close();
},
_computeShadowClass(isShadowDom) {
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.html b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.html
index 79c8a3b..7a8cd6f 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.html
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.html
@@ -32,6 +32,16 @@
main {
max-width: 46em;
}
+ :host(.loading) main {
+ display: none;
+ }
+ .loadingMessage {
+ display: none;
+ font-style: italic;
+ }
+ :host(.loading) .loadingMessage {
+ display: block;
+ }
hr {
margin-top: 1em;
margin-bottom: 1em;
@@ -54,9 +64,13 @@
input {
width: 20em;
}
+ section.hide {
+ display: none;
+ }
</style>
<div class="container gr-form-styles">
<header>Please confirm your contact information</header>
+ <div class="loadingMessage">Loading...</div>
<main>
<p>
The following contact information was automatically obtained when you
@@ -73,7 +87,7 @@
bind-value="{{_account.name}}"
disabled="[[_saving]]">
</section>
- <section>
+ <section class$="[[_computeUsernameClass(_usernameMutable)]]">
<div class="title">Username</div>
<input
is="iron-input"
@@ -108,7 +122,7 @@
id="saveButton"
primary
link
- disabled="[[_computeSaveDisabled(_account.name, _account.username, _account.email, _saving)]]"
+ disabled="[[_computeSaveDisabled(_account.name, _account.email, _saving)]]"
on-tap="_handleSave">Save</gr-button>
</footer>
</div>
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.js b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.js
index 051668c..c6cd578 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.js
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog.js
@@ -43,32 +43,56 @@
return {email: null, name: null, username: null};
},
},
+ _usernameMutable: {
+ type: Boolean,
+ computed: '_computeUsernameMutable(_serverConfig, _account.username)',
+ },
+ _loading: {
+ type: Boolean,
+ value: true,
+ observer: '_loadingChanged',
+ },
_saving: {
type: Boolean,
value: false,
},
+ _serverConfig: Object,
},
hostAttributes: {
role: 'dialog',
},
- attached() {
- this.$.restAPI.getAccount().then(account => {
+ loadData() {
+ this._loading = true;
+
+ const loadAccount = this.$.restAPI.getAccount().then(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);
});
+
+ const loadConfig = this.$.restAPI.getConfig().then(config => {
+ this._serverConfig = config;
+ });
+
+ return Promise.all([loadAccount, loadConfig]).then(() => {
+ this._loading = false;
+ });
},
_save() {
this._saving = true;
const promises = [
this.$.restAPI.setAccountName(this.$.name.value),
- this.$.restAPI.setAccountUsername(this.$.username.value),
this.$.restAPI.setPreferredAccountEmail(this.$.email.value || ''),
];
+
+ if (this._usernameMutable) {
+ promises.push(this.$.restAPI.setAccountUsername(this.$.username.value));
+ }
+
return Promise.all(promises).then(() => {
this._saving = false;
this.fire('account-detail-update');
@@ -90,8 +114,21 @@
this.fire('close');
},
- _computeSaveDisabled(name, username, email, saving) {
- return !name || !username || !email || saving;
+ _computeSaveDisabled(name, email, saving) {
+ return !name || !email || saving;
+ },
+
+ _computeUsernameMutable(config, username) {
+ return config.auth.editable_account_fields.includes('USER_NAME') &&
+ !username;
+ },
+
+ _computeUsernameClass(usernameMutable) {
+ return usernameMutable ? '' : 'hide';
+ },
+
+ _loadingChanged() {
+ this.classList.toggle('loading', this._loading);
},
});
})();
diff --git a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.html b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.html
index e4560ff..93a3188 100644
--- a/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.html
+++ b/polygerrit-ui/app/elements/settings/gr-registration-dialog/gr-registration-dialog_test.html
@@ -45,13 +45,13 @@
let sandbox;
let _listeners;
- setup(done => {
+ setup(() => {
sandbox = sinon.sandbox.create();
_listeners = {};
account = {
name: 'name',
- username: 'username',
+ username: null,
email: 'email',
secondary_emails: [
'email2',
@@ -61,8 +61,6 @@
stub('gr-rest-api-interface', {
getAccount() {
- // Once the account is resolved, we can let the test proceed.
- flush(done);
return Promise.resolve(account);
},
setAccountName(name) {
@@ -77,9 +75,15 @@
account.email = email;
return Promise.resolve();
},
+ getConfig() {
+ return Promise.resolve(
+ {auth: {editable_account_fields: ['USER_NAME']}});
+ },
});
element = fixture('basic');
+
+ return element.loadData();
});
teardown(() => {
@@ -136,7 +140,7 @@
// Nothing should be committed yet.
assert.equal(account.name, 'name');
- assert.equal(account.username, 'username');
+ assert.isNotOk(account.username);
assert.equal(account.email, 'email');
// Save and verify new values are committed.
@@ -158,12 +162,22 @@
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));
+ assert.isTrue(compute('', '', false));
+ assert.isTrue(compute('', 'test', false));
+ assert.isTrue(compute('test', '', false));
+ assert.isTrue(compute('test', 'test', true));
+ assert.isFalse(compute('test', 'test', false));
+ });
+
+ test('_computeUsernameMutable', () => {
+ assert.isTrue(element._computeUsernameMutable(
+ {auth: {editable_account_fields: ['USER_NAME']}}, null));
+ assert.isFalse(element._computeUsernameMutable(
+ {auth: {editable_account_fields: ['USER_NAME']}}, 'abc'));
+ assert.isFalse(element._computeUsernameMutable(
+ {auth: {editable_account_fields: []}}, null));
+ assert.isFalse(element._computeUsernameMutable(
+ {auth: {editable_account_fields: []}}, 'abc'));
});
});
</script>
