PolyGerrit: Fix undefined name in reviewers list if you had a anon name
If a user had no name then it would show up blank in the reviewers table
when searching it would bring up undefined <undefined>. This change fixes it
by using a fallback name.
Change-Id: I403a35b1b72280c9928c63139725f73ef46cf3fd
diff --git a/polygerrit-ui/app/behaviors/gr-anonymous-name-behavior/gr-anonymous-name-behavior.html b/polygerrit-ui/app/behaviors/gr-anonymous-name-behavior/gr-anonymous-name-behavior.html
index 329611f..10065af 100644
--- a/polygerrit-ui/app/behaviors/gr-anonymous-name-behavior/gr-anonymous-name-behavior.html
+++ b/polygerrit-ui/app/behaviors/gr-anonymous-name-behavior/gr-anonymous-name-behavior.html
@@ -24,8 +24,16 @@
/** @polymerBehavior Gerrit.AnonymousNameBehavior */
Gerrit.AnonymousNameBehavior = {
- getAnonymousName(config) {
- if (config && config.user &&
+ /**
+ * enableEmail when true enables to fallback to using email if
+ * the account name is not avilable.
+ */
+ getUserName(config, account, enableEmail) {
+ if (account && account.name) {
+ return account.name;
+ } else if (enableEmail && account && account.email) {
+ return account.email;
+ } else if (config && config.user &&
config.user.anonymous_coward_name !== 'Anonymous Coward') {
return config.user.anonymous_coward_name;
}
diff --git a/polygerrit-ui/app/behaviors/gr-anonymous-name-behavior/gr-anonymous-name-behavior_test.html b/polygerrit-ui/app/behaviors/gr-anonymous-name-behavior/gr-anonymous-name-behavior_test.html
new file mode 100644
index 0000000..e4a409b
--- /dev/null
+++ b/polygerrit-ui/app/behaviors/gr-anonymous-name-behavior/gr-anonymous-name-behavior_test.html
@@ -0,0 +1,84 @@
+<!DOCTYPE html>
+<!--
+Copyright (C) 2017 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+
+<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">
+<title>gr-anonymous-name-behavior</title>
+
+<script src="../../bower_components/webcomponentsjs/webcomponents.min.js"></script>
+<script src="../../bower_components/web-component-tester/browser.js"></script>
+<link rel="import" href="../../test/common-test-setup.html"/>
+<link rel="import" href="gr-anonymous-name-behavior.html">
+
+<test-fixture id="basic">
+ <template>
+ <test-element-anon></test-element-anon>
+ </template>
+</test-fixture>
+
+<script>
+ suite('gr-anonymous-name-behavior tests', () => {
+ let element;
+ // eslint-disable-next-line no-unused-vars
+ const config = {
+ user: {
+ anonymous_coward_name: 'Anonymous Coward',
+ },
+ };
+
+ suiteSetup(() => {
+ // Define a Polymer element that uses this behavior.
+ Polymer({
+ is: 'test-element-anon',
+ behaviors: [
+ Gerrit.AnonymousNameBehavior,
+ ],
+ });
+ });
+
+ setup(() => {
+ element = fixture('basic');
+ });
+
+ test('test for it to return name', () => {
+ const account = {
+ name: 'test-user',
+ };
+ assert.deepEqual(element.getUserName(config, account, true), 'test-user');
+ });
+
+ test('test for it to return email', () => {
+ const account = {
+ email: 'test-user@test-url.com',
+ };
+ assert.deepEqual(element.getUserName(config, account, true),
+ 'test-user@test-url.com');
+ });
+
+ test('test for it not to Anonymous Coward as the anon name', () => {
+ assert.deepEqual(element.getUserName(config, null, true), 'Anonymous');
+ });
+
+ test('test for the config returning the anon name', () => {
+ const config = {
+ user: {
+ anonymous_coward_name: 'Test Anon',
+ },
+ };
+ assert.deepEqual(element.getUserName(config, null, true), 'Test Anon');
+ });
+ });
+</script>
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html
index 096794a..4931ff1 100644
--- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html
+++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.html
@@ -14,10 +14,11 @@
limitations under the License.
-->
+<link rel="import" href="../../../behaviors/gr-anonymous-name-behavior/gr-anonymous-name-behavior.html">
<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../../shared/gr-autocomplete/gr-autocomplete.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
-<link rel="import" href="../../../styles/shared-styles.html">
<dom-module id="gr-account-entry">
<template>
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js
index 7db943ca..3626b86 100644
--- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js
+++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry.js
@@ -25,6 +25,7 @@
properties: {
borderless: Boolean,
change: Object,
+ _config: Object,
filter: Function,
placeholder: String,
/**
@@ -52,6 +53,16 @@
},
},
+ behaviors: [
+ Gerrit.AnonymousNameBehavior,
+ ],
+
+ attached() {
+ this.$.restAPI.getConfig().then(cfg => {
+ this._config = cfg;
+ });
+ },
+
get focusStart() {
return this.$.input.focusStart;
},
@@ -77,6 +88,10 @@
this.$.input.focus();
},
+ _accountOrAnon(reviewer) {
+ return this.getUserName(this._config, reviewer, false);
+ },
+
_makeSuggestion(reviewer) {
let name;
let value;
@@ -85,7 +100,8 @@
};
if (reviewer.account) {
// Reviewer is an account suggestion from getChangeSuggestedReviewers.
- name = reviewer.account.name + ' <' + reviewer.account.email + '>' +
+ const reviewerName = this._accountOrAnon(reviewer.account);
+ name = reviewerName + ' <' + reviewer.account.email + '>' +
generateStatusStr(reviewer.account);
value = reviewer;
} else if (reviewer.group) {
@@ -94,7 +110,8 @@
value = reviewer;
} else if (reviewer._account_id) {
// Reviewer is an account suggestion from getSuggestedAccounts.
- name = reviewer.name + ' <' + reviewer.email + '>' +
+ const reviewerName = this._accountOrAnon(reviewer);
+ name = reviewerName + ' <' + reviewer.email + '>' +
generateStatusStr(reviewer);
value = {account: reviewer, count: 1};
}
@@ -110,7 +127,9 @@
return xhr.then(reviewers => {
if (!reviewers) { return []; }
- if (!this.filter) { return reviewers.map(this._makeSuggestion); }
+ if (!this.filter) {
+ return reviewers.map(this._makeSuggestion.bind(this));
+ }
return reviewers
.filter(this.filter)
.map(this._makeSuggestion.bind(this));
diff --git a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html
index 55213c1..a4dacc7 100644
--- a/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html
+++ b/polygerrit-ui/app/elements/change/gr-account-entry/gr-account-entry_test.html
@@ -46,6 +46,15 @@
status: opt_status,
};
};
+ let _nextAccountId2 = 0;
+ const makeAccount2 = function(opt_status) {
+ const accountId2 = ++_nextAccountId2;
+ return {
+ _account_id: accountId2,
+ email: 'email ' + accountId2,
+ status: opt_status,
+ };
+ };
let owner;
let existingReviewer1;
@@ -98,6 +107,7 @@
test('_makeSuggestion formats account or group accordingly', () => {
let account = makeAccount();
+ const account2 = makeAccount2();
let suggestion = element._makeSuggestion({account});
assert.deepEqual(suggestion, {
name: account.name + ' <' + account.email + '>',
@@ -117,6 +127,13 @@
value: {account, count: 1},
});
+ element._config = {
+ user: {
+ anonymous_coward_name: 'Anonymous Coward',
+ },
+ };
+ assert.deepEqual(element._accountOrAnon(account2), 'Anonymous');
+
account = makeAccount('OOO');
suggestion = element._makeSuggestion({account});
diff --git a/polygerrit-ui/app/elements/change/gr-message/gr-message.html b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
index cbbe828..ae43edf 100644
--- a/polygerrit-ui/app/elements/change/gr-message/gr-message.html
+++ b/polygerrit-ui/app/elements/change/gr-message/gr-message.html
@@ -150,7 +150,6 @@
</span>
<gr-account-label
account="[[author]]"
- show-anonymous
hide-avatar></gr-account-label>
</div>
<template is="dom-if" if="[[message.message]]">
diff --git a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js
index e07437b..c147456 100644
--- a/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js
+++ b/polygerrit-ui/app/elements/core/gr-account-dropdown/gr-account-dropdown.js
@@ -93,13 +93,7 @@
},
_accountName(account) {
- if (account && account.name) {
- return account.name;
- } else if (account && account.email) {
- return account.email;
- }
-
- return this.getAnonymousName(this.config);
+ return this.getUserName(this.config, account, true);
},
});
})();
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.html b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.html
index 3811a91..21f4c3e 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.html
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.html
@@ -15,11 +15,11 @@
-->
<link rel="import" href="../../../behaviors/gr-anonymous-name-behavior/gr-anonymous-name-behavior.html">
-<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/gr-tooltip-behavior/gr-tooltip-behavior.html">
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../../styles/shared-styles.html">
<link rel="import" href="../gr-avatar/gr-avatar.html">
<link rel="import" href="../gr-rest-api-interface/gr-rest-api-interface.html">
-<link rel="import" href="../../../styles/shared-styles.html">
<dom-module id="gr-account-label">
<template>
@@ -49,7 +49,7 @@
image-size="[[avatarImageSize]]"></gr-avatar>
</template>
<span class="text">
- <span>[[_computeName(account, _serverConfig, showAnonymous)]]</span>
+ <span>[[_computeName(account, _serverConfig)]]</span>
<span hidden$="[[!_computeShowEmail(showEmail, account)]]">
[[_computeEmailStr(account)]]
</span>
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
index 0346b9b..2dee2f6 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
@@ -44,10 +44,6 @@
type: Boolean,
value: false,
},
- showAnonymous: {
- type: Boolean,
- value: false,
- },
_serverConfig: {
type: Object,
value: null,
@@ -60,27 +56,19 @@
],
ready() {
- if (this.showAnonymous) {
- this.$.restAPI.getConfig()
- .then(config => { this._serverConfig = config; });
- }
+ this.$.restAPI.getConfig()
+ .then(config => { this._serverConfig = config; });
},
- _computeName(account, config, showAnonymous) {
- if (account && account.name) {
- return account.name;
- }
- if (showAnonymous) {
- return this.getAnonymousName(config);
- }
- return '';
+ _computeName(account, config) {
+ return this.getUserName(config, account, false);
},
_computeAccountTitle(account) {
- if (!account || (!account.name && !account.email)) { return; }
+ if (!account) { return; }
let result = '';
- if (account.name) {
- result += account.name;
+ if (this._computeName(account, this._serverConfig)) {
+ result += this._computeName(account, this._serverConfig);
}
if (account.email) {
result += ' <' + account.email + '>';
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.html b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.html
index 7095be3..731c9b7 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_test.html
@@ -43,6 +43,11 @@
getLoggedIn() { return Promise.resolve(false); },
});
element = fixture('basic');
+ element._config = {
+ user: {
+ anonymous_coward_name: 'Anonymous Coward',
+ },
+ };
});
test('null guard', () => {
@@ -67,6 +72,12 @@
{name: 'Andrew Bonventre'}),
'Andrew Bonventre');
+ assert.equal(element._computeAccountTitle(
+ {
+ email: 'andybons+gerrit@gmail.com',
+ }),
+ 'Anonymous <andybons+gerrit@gmail.com>');
+
assert.equal(element._computeShowEmail(true,
{
name: 'Andrew Bonventre',
@@ -93,12 +104,12 @@
suite('_computeName', () => {
test('not showing anonymous', () => {
const account = {name: 'Wyatt'};
- assert.deepEqual(element._computeName(account, null, false), 'Wyatt');
+ assert.deepEqual(element._computeName(account, null), 'Wyatt');
});
test('showing anonymous but no config', () => {
const account = {};
- assert.deepEqual(element._computeName(account, null, true),
+ assert.deepEqual(element._computeName(account, null),
'Anonymous');
});
@@ -109,7 +120,7 @@
},
};
const account = {};
- assert.deepEqual(element._computeName(account, config, true),
+ assert.deepEqual(element._computeName(account, config),
'Anonymous');
});
@@ -120,7 +131,7 @@
},
};
const account = {};
- assert.deepEqual(element._computeName(account, config, true),
+ assert.deepEqual(element._computeName(account, config),
'TestAnon');
});
});
diff --git a/polygerrit-ui/app/test/index.html b/polygerrit-ui/app/test/index.html
index 5426f45..634dfd6 100644
--- a/polygerrit-ui/app/test/index.html
+++ b/polygerrit-ui/app/test/index.html
@@ -151,6 +151,7 @@
'docs-url-behavior/docs-url-behavior_test.html',
'keyboard-shortcut-behavior/keyboard-shortcut-behavior_test.html',
'rest-client-behavior/rest-client-behavior_test.html',
+ 'gr-anonymous-name-behavior/gr-anonymous-name-behavior_test.html',
'gr-change-table-behavior/gr-change-table-behavior_test.html',
'gr-patch-set-behavior/gr-patch-set-behavior_test.html',
'gr-path-list-behavior/gr-path-list-behavior_test.html',
