Add undefined check for computed properties

Same as obervers, in polymer 2 a computed property will run when
at least one dependencies is defined. To match the behavior in v1,
return undefined if not all dependencies are defined.

Bug: Issue 10723
Change-Id: Iaa294d5bc392c6f3970cdbb59213a9d2913e6f64
This commit is contained in:
Tao Zhou
2019-08-12 18:53:02 +02:00
parent 57bce3c662
commit d2fca29792
31 changed files with 390 additions and 40 deletions

View File

@@ -250,6 +250,7 @@
_computeTopicReadOnly(mutable, change) {
return !mutable ||
!change ||
!change.actions ||
!change.actions.topic ||
!change.actions.topic.enabled;
@@ -257,6 +258,7 @@
_computeHashtagReadOnly(mutable, change) {
return !mutable ||
!change ||
!change.actions ||
!change.actions.hashtags ||
!change.actions.hashtags.enabled;
@@ -264,6 +266,7 @@
_computeAssigneeReadOnly(mutable, change) {
return !mutable ||
!change ||
!change.actions ||
!change.actions.assignee ||
!change.actions.assignee.enabled;
@@ -297,7 +300,7 @@
* the push validation.
*/
_computePushCertificateValidation(serverConfig, change) {
if (!serverConfig || !serverConfig.receive ||
if (!change || !serverConfig || !serverConfig.receive ||
!serverConfig.receive.enable_signed_push) {
return null;
}

View File

@@ -455,8 +455,20 @@
},
_computeChangeStatusChips(change, mergeable) {
// Polymer 2: check for undefined
if ([
change,
mergeable,
].some(arg => arg === undefined)) {
// To keep consistent with Polymer 1, we are returning undefined
// if not all dependencies are defined
return undefined;
}
// Show no chips until mergeability is loaded.
if (mergeable === null || mergeable === undefined) { return []; }
if (mergeable === null) {
return [];
}
const options = {
includeDerived: true,
@@ -467,7 +479,8 @@
},
_computeHideEditCommitMessage(loggedIn, editing, change, editMode) {
if (!loggedIn || editing || change.status === this.ChangeStatus.MERGED ||
if (!loggedIn || editing ||
(change && change.status === this.ChangeStatus.MERGED) ||
editMode) {
return true;
}
@@ -974,6 +987,11 @@
},
_computeChangeIdCommitMessageError(commitMessage, change) {
// Polymer 2: check for undefined
if ([commitMessage, change].some(arg => arg === undefined)) {
return undefined;
}
if (!commitMessage) { return CHANGE_ID_ERROR.MISSING; }
// Find the last match in the commit message:
@@ -1028,6 +1046,11 @@
},
_computeReplyButtonLabel(changeRecord, canStartReview) {
// Polymer 2: check for undefined
if ([changeRecord, canStartReview].some(arg => arg === undefined)) {
return undefined;
}
if (canStartReview) {
return 'Start review';
}
@@ -1738,6 +1761,10 @@
},
_computeEditMode(patchRangeRecord, paramsRecord) {
if ([patchRangeRecord, paramsRecord].some(arg => arg === undefined)) {
return undefined;
}
if (paramsRecord.base && paramsRecord.base.edit) { return true; }
const patchRange = patchRangeRecord.base || {};
@@ -1823,7 +1850,7 @@
},
_computeCurrentRevision(currentRevision, revisions) {
return revisions && revisions[currentRevision];
return currentRevision && revisions && revisions[currentRevision];
},
_computeDiffPrefsDisabled(disableDiffPrefs, loggedIn) {

View File

@@ -47,11 +47,21 @@
},
_computeShowWebLink(change, commitInfo, serverConfig) {
// Polymer 2: check for undefined
if ([change, commitInfo, serverConfig].some(arg => arg === undefined)) {
return undefined;
}
const weblink = this._getWeblink(change, commitInfo, serverConfig);
return !!weblink && !!weblink.url;
},
_computeWebLink(change, commitInfo, serverConfig) {
// Polymer 2: check for undefined
if ([change, commitInfo, serverConfig].some(arg => arg === undefined)) {
return undefined;
}
const {url} = this._getWeblink(change, commitInfo, serverConfig) || {};
return url;
},

View File

@@ -121,6 +121,7 @@ limitations under the License.
},
],
};
element.serverConfig = {};
assert.isOk(element._computeShowWebLink(element.change,
element.commitInfo, element.serverConfig));

View File

@@ -157,6 +157,11 @@
},
_computeSchemes(change, patchNum) {
// Polymer 2: check for undefined
if ([change, patchNum].some(arg => arg === undefined)) {
return undefined;
}
for (const rev of Object.values(change.revisions || {})) {
if (this.patchNumEquals(rev._number, patchNum)) {
const fetch = rev.fetch;

View File

@@ -133,6 +133,11 @@
},
_computeDescriptionReadOnly(loggedIn, change, account) {
// Polymer 2: check for undefined
if ([loggedIn, change, account].some(arg => arg === undefined)) {
return undefined;
}
return !(loggedIn && (account._account_id === change.owner._account_id));
},

View File

@@ -847,6 +847,11 @@
},
_computeFilesShown(numFilesShown, files) {
// Polymer 2: check for undefined
if ([numFilesShown, files].some(arg => arg === undefined)) {
return undefined;
}
const previousNumFilesShown = this._shownFiles ?
this._shownFiles.length : 0;

View File

@@ -148,6 +148,11 @@
},
_computePermittedLabelValues(permittedLabels, label) {
// Polymer 2: check for undefined
if ([permittedLabels, label].some(arg => arg === undefined)) {
return undefined;
}
return permittedLabels[label];
},

View File

@@ -82,7 +82,12 @@
return null;
},
_computeLabels(labelRecord) {
_computeLabels(labelRecord, account) {
// Polymer 2: check for undefined
if ([labelRecord, account].some(arg => arg === undefined)) {
return undefined;
}
const labelsObj = labelRecord.base;
if (!labelsObj) { return []; }
return Object.keys(labelsObj).sort().map(key => {

View File

@@ -144,7 +144,7 @@
},
_computeShowReplyButton(message, loggedIn) {
return !!message.message && loggedIn &&
return message && !!message.message && loggedIn &&
!this._computeIsAutomated(message);
},

View File

@@ -117,6 +117,11 @@
},
_computeItems(messages, reviewerUpdates) {
// Polymer 2: check for undefined
if ([messages, reviewerUpdates].some(arg => arg === undefined)) {
return undefined;
}
messages = messages || [];
reviewerUpdates = reviewerUpdates || [];
let mi = 0;

View File

@@ -325,6 +325,11 @@
},
_computeConnectedRevisions(change, patchNum, relatedChanges) {
// Polymer 2: check for undefined
if ([change, patchNum, relatedChanges].some(arg => arg === undefined)) {
return undefined;
}
const connected = [];
let changeRevision;
if (!change) { return []; }

View File

@@ -861,6 +861,19 @@
_computeSendButtonDisabled(buttonLabel, drafts, text, reviewersMutated,
labelsChanged, includeComments, disabled) {
// Polymer 2: check for undefined
if ([
buttonLabel,
drafts,
text,
reviewersMutated,
labelsChanged,
includeComments,
disabled,
].some(arg => arg === undefined)) {
return undefined;
}
if (disabled) { return true; }
if (buttonLabel === ButtonLabels.START_REVIEW) { return false; }
const hasDrafts = includeComments && Object.keys(drafts).length;

View File

@@ -1093,20 +1093,84 @@ limitations under the License.
test('_computeSendButtonDisabled', () => {
const fn = element._computeSendButtonDisabled.bind(element);
assert.isFalse(fn('Start review'));
assert.isTrue(fn('Send', {}, '', false, false, false));
assert.isFalse(fn(
/* buttonLabel= */ 'Start review',
/* drafts= */ {},
/* text= */ '',
/* reviewersMutated= */ false,
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false,
));
assert.isTrue(fn(
/* buttonLabel= */ 'Send',
/* drafts= */ {},
/* text= */ '',
/* reviewersMutated= */ false,
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false,
));
// Mock nonempty comment draft array, with seding comments.
assert.isFalse(fn('Send', {file: ['draft']}, '', false, false, true));
assert.isFalse(fn(
/* buttonLabel= */ 'Send',
/* drafts= */ {file: ['draft']},
/* text= */ '',
/* reviewersMutated= */ false,
/* labelsChanged= */ false,
/* includeComments= */ true,
/* disabled= */ false,
));
// Mock nonempty comment draft array, without seding comments.
assert.isTrue(fn('Send', {file: ['draft']}, '', false, false, false));
assert.isTrue(fn(
/* buttonLabel= */ 'Send',
/* drafts= */ {file: ['draft']},
/* text= */ '',
/* reviewersMutated= */ false,
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false,
));
// Mock nonempty change message.
assert.isFalse(fn('Send', {}, 'test', false, false, false));
assert.isFalse(fn(
/* buttonLabel= */ 'Send',
/* drafts= */ {},
/* text= */ 'test',
/* reviewersMutated= */ false,
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false,
));
// Mock reviewers mutated.
assert.isFalse(fn('Send', {}, '', true, false, false));
assert.isFalse(fn(
/* buttonLabel= */ 'Send',
/* drafts= */ {},
/* text= */ '',
/* reviewersMutated= */ true,
/* labelsChanged= */ false,
/* includeComments= */ false,
/* disabled= */ false,
));
// Mock labels changed.
assert.isFalse(fn('Send', {}, '', false, true, false));
assert.isFalse(fn(
/* buttonLabel= */ 'Send',
/* drafts= */ {},
/* text= */ '',
/* reviewersMutated= */ false,
/* labelsChanged= */ true,
/* includeComments= */ false,
/* disabled= */ false,
));
// Whole dialog is disabled.
assert.isTrue(fn('Send', {}, '', false, true, false, true));
assert.isTrue(fn(
/* buttonLabel= */ 'Send',
/* drafts= */ {},
/* text= */ '',
/* reviewersMutated= */ false,
/* labelsChanged= */ true,
/* includeComments= */ false,
/* disabled= */ true,
));
});
test('_submit blocked when no mutations exist', () => {
@@ -1141,4 +1205,4 @@ limitations under the License.
assert.equal(element.$.pluginMessage.textContent, 'foo');
});
});
</script>
</script>

View File

@@ -199,6 +199,11 @@
},
_computeHiddenCount(reviewers, displayedReviewers) {
// Polymer 2: check for undefined
if ([reviewers, displayedReviewers].some(arg => arg === undefined)) {
return undefined;
}
return reviewers.length - displayedReviewers.length;
},

View File

@@ -94,6 +94,15 @@
},
_computeFilteredThreads(sortedThreads, unresolvedOnly, draftsOnly) {
// Polymer 2: check for undefined
if ([
sortedThreads,
unresolvedOnly,
draftsOnly,
].some(arg => arg === undefined)) {
return undefined;
}
return sortedThreads.filter(c => {
if (draftsOnly) {
return c.hasDraft;

View File

@@ -82,6 +82,15 @@
_computeFetchCommand(revision, preferredDownloadCommand,
preferredDownloadScheme) {
// Polymer 2: check for undefined
if ([
revision,
preferredDownloadCommand,
preferredDownloadScheme,
].some(arg => arg === undefined)) {
return undefined;
}
if (!revision) { return; }
if (!revision || !revision.fetch) { return; }

View File

@@ -75,31 +75,40 @@ limitations under the License.
assert.isUndefined(element._computeFetchCommand({fetch: {}}));
});
test('not all defined', () => {
assert.isUndefined(
element._computeFetchCommand(testRev, undefined, ''));
assert.isUndefined(
element._computeFetchCommand(testRev, '', undefined));
assert.isUndefined(
element._computeFetchCommand(undefined, '', ''));
});
test('insufficiently defined scheme', () => {
assert.isUndefined(
element._computeFetchCommand(testRev, undefined, 'badscheme'));
element._computeFetchCommand(testRev, '', 'badscheme'));
const rev = Object.assign({}, testRev);
rev.fetch = Object.assign({}, testRev.fetch, {nocmds: {commands: {}}});
assert.isUndefined(
element._computeFetchCommand(rev, undefined, 'nocmds'));
element._computeFetchCommand(rev, '', 'nocmds'));
rev.fetch.nocmds.commands.unsupported = 'unsupported';
assert.isUndefined(
element._computeFetchCommand(rev, undefined, 'nocmds'));
element._computeFetchCommand(rev, '', 'nocmds'));
});
test('default scheme and command', () => {
const cmd = element._computeFetchCommand(testRev);
const cmd = element._computeFetchCommand(testRev, '', '');
assert.isTrue(cmd === 'http checkout' || cmd === 'ssh pull');
});
test('default command', () => {
assert.strictEqual(
element._computeFetchCommand(testRev, undefined, 'http'),
element._computeFetchCommand(testRev, '', 'http'),
'http checkout');
assert.strictEqual(
element._computeFetchCommand(testRev, undefined, 'ssh'),
element._computeFetchCommand(testRev, '', 'ssh'),
'ssh pull');
});

View File

@@ -66,6 +66,11 @@
},
_getLinks(switchAccountUrl, path) {
// Polymer 2: check for undefined
if ([switchAccountUrl, path].some(arg => arg === undefined)) {
return undefined;
}
const links = [{name: 'Settings', url: '/settings/'}];
if (switchAccountUrl) {
const replacements = {path};

View File

@@ -80,11 +80,15 @@ limitations under the License.
});
test('switch account', () => {
// Missing params.
assert.isUndefined(element._getLinks());
assert.isUndefined(element._getLinks(null));
// No switch account link.
assert.equal(element._getLinks(null).length, 2);
assert.equal(element._getLinks(null, '').length, 2);
// Unparameterized switch account link.
let links = element._getLinks('/switch-account');
let links = element._getLinks('/switch-account', '');
assert.equal(links.length, 3);
assert.deepEqual(links[1], {
name: 'Switch account',

View File

@@ -181,6 +181,17 @@
},
_computeLinks(defaultLinks, userLinks, adminLinks, topMenus, docBaseUrl) {
// Polymer 2: check for undefined
if ([
defaultLinks,
userLinks,
adminLinks,
topMenus,
docBaseUrl,
].some(arg => arg === undefined)) {
return undefined;
}
const links = defaultLinks.map(menu => {
return {
title: menu.title,

View File

@@ -111,13 +111,24 @@ limitations under the License.
}];
// When no admin links are passed, it should use the default.
assert.deepEqual(element._computeLinks(defaultLinks, [], adminLinks, []),
assert.deepEqual(element._computeLinks(
defaultLinks,
/* userLinks= */[],
adminLinks,
/* topMenus= */[],
/* docBaseUrl= */ '',
),
defaultLinks.concat({
title: 'Browse',
links: adminLinks,
}));
assert.deepEqual(
element._computeLinks(defaultLinks, userLinks, adminLinks, []),
assert.deepEqual(element._computeLinks(
defaultLinks,
userLinks,
adminLinks,
/* topMenus= */[],
/* docBaseUrl= */ ''
),
defaultLinks.concat([
{
title: 'Your',
@@ -126,7 +137,8 @@ limitations under the License.
{
title: 'Browse',
links: adminLinks,
}]));
}])
);
});
test('documentation links', () => {
@@ -168,7 +180,13 @@ limitations under the License.
url: 'https://gerrit/plugins/plugin-manager/static/index.html',
}],
}];
assert.deepEqual(element._computeLinks([], [], adminLinks, topMenus), [{
assert.deepEqual(element._computeLinks(
/* defaultLinks= */ [],
/* userLinks= */ [],
adminLinks,
topMenus,
/* baseDocUrl= */ '',
), [{
title: 'Browse',
links: adminLinks,
},
@@ -198,7 +216,13 @@ limitations under the License.
url: '/plugins/myplugin/index.html',
}],
}];
assert.deepEqual(element._computeLinks([], [], adminLinks, topMenus), [{
assert.deepEqual(element._computeLinks(
/* defaultLinks= */ [],
/* userLinks= */ [],
adminLinks,
topMenus,
/* baseDocUrl= */ '',
), [{
title: 'Browse',
links: adminLinks,
},
@@ -231,7 +255,13 @@ limitations under the License.
url: 'https://gerrit/plugins/plugin-manager/static/create.html',
}],
}];
assert.deepEqual(element._computeLinks([], [], adminLinks, topMenus), [{
assert.deepEqual(element._computeLinks(
/* defaultLinks= */ [],
/* userLinks= */ [],
adminLinks,
topMenus,
/* baseDocUrl= */ '',
), [{
title: 'Browse',
links: adminLinks,
}, {
@@ -262,7 +292,13 @@ limitations under the License.
url: 'https://gerrit/plugins/plugin-manager/static/index.html',
}],
}];
assert.deepEqual(element._computeLinks(defaultLinks, [], [], topMenus), [{
assert.deepEqual(element._computeLinks(
defaultLinks,
/* userLinks= */ [],
/* adminLinks= */ [],
topMenus,
/* baseDocUrl= */ '',
), [{
title: 'Faves',
links: defaultLinks[0].links.concat([{
name: 'Manage',
@@ -287,7 +323,13 @@ limitations under the License.
url: 'https://gerrit/plugins/plugin-manager/static/index.html',
}],
}];
assert.deepEqual(element._computeLinks([], userLinks, [], topMenus), [{
assert.deepEqual(element._computeLinks(
/* defaultLinks= */ [],
userLinks,
/* adminLinks= */ [],
topMenus,
/* baseDocUrl= */ '',
), [{
title: 'Your',
links: userLinks.concat([{
name: 'Manage',
@@ -312,7 +354,13 @@ limitations under the License.
url: 'https://gerrit/plugins/plugin-manager/static/index.html',
}],
}];
assert.deepEqual(element._computeLinks([], [], adminLinks, topMenus), [{
assert.deepEqual(element._computeLinks(
/* defaultLinks= */ [],
/* userLinks= */ [],
adminLinks,
topMenus,
/* baseDocUrl= */ '',
), [{
title: 'Browse',
links: adminLinks.concat([{
name: 'Manage',
@@ -350,4 +398,4 @@ limitations under the License.
assert.equal(element._registerText, 'Sign up');
});
});
</script>
</script>

View File

@@ -794,6 +794,15 @@
},
_formatFilesForDropdown(fileList, patchNum, changeComments) {
// Polymer 2: check for undefined
if ([
fileList,
patchNum,
changeComments,
].some(arg => arg === undefined)) {
return;
}
if (!fileList) { return; }
const dropdownContent = [];
for (const path of fileList) {
@@ -931,6 +940,15 @@
},
_computeCommentSkips(commentMap, fileList, path) {
// Polymer 2: check for undefined
if ([
commentMap,
fileList,
path,
].some(arg => arg === undefined)) {
return undefined;
}
const skips = {previous: null, next: null};
if (!fileList.length) { return skips; }
const pathIndex = fileList.indexOf(path);
@@ -1011,6 +1029,11 @@
},
_computeFileNum(file, files) {
// Polymer 2: check for undefined
if ([file, files].some(arg => arg === undefined)) {
return undefined;
}
return files.findIndex(({value}) => value === file) + 1;
},

View File

@@ -64,6 +64,17 @@
_computeBaseDropdownContent(availablePatches, patchNum, _sortedRevisions,
changeComments, revisionInfo) {
// Polymer 2: check for undefined
if ([
availablePatches,
patchNum,
_sortedRevisions,
changeComments,
revisionInfo,
].some(arg => arg === undefined)) {
return undefined;
}
const parentCounts = revisionInfo.getParentCountMap();
const currentParentCount = parentCounts.hasOwnProperty(patchNum) ?
parentCounts[patchNum] : 1;
@@ -107,6 +118,16 @@
_computePatchDropdownContent(availablePatches, basePatchNum,
_sortedRevisions, changeComments) {
// Polymer 2: check for undefined
if ([
availablePatches,
basePatchNum,
_sortedRevisions,
changeComments,
].some(arg => arg === undefined)) {
return undefined;
}
const dropdownContent = [];
for (const patch of availablePatches) {
const patchNum = patch.num;

View File

@@ -210,6 +210,15 @@
},
_computeSaveDisabled(content, newContent, saving) {
// Polymer 2: check for undefined
if ([
content,
newContent,
saving,
].some(arg => arg === undefined)) {
return undefined;
}
if (saving) {
return true;
}

View File

@@ -149,6 +149,14 @@
},
_computeUsernameMutable(config, username) {
// Polymer 2: check for undefined
if ([
config,
username,
].some(arg => arg === undefined)) {
return undefined;
}
// Username may not be changed once it is set.
return config.auth.editable_account_fields.includes('USER_NAME') &&
!username;

View File

@@ -124,6 +124,14 @@
},
_computeUsernameMutable(config, username) {
// Polymer 2: check for undefined
if ([
config,
username,
].some(arg => arg === undefined)) {
return undefined;
}
return config.auth.editable_account_fields.includes('USER_NAME') &&
!username;
},

View File

@@ -67,6 +67,14 @@
},
_computeAccountTitle(account, tooltip) {
// Polymer 2: check for undefined
if ([
account,
tooltip,
].some(arg => arg === undefined)) {
return undefined;
}
if (!account) { return; }
let result = '';
if (this._computeName(account, this._serverConfig)) {

View File

@@ -68,31 +68,34 @@ limitations under the License.
{
name: 'Andrew Bonventre',
email: 'andybons+gerrit@gmail.com',
}),
}, /* additionalText= */ ''),
'Andrew Bonventre <andybons+gerrit@gmail.com>');
assert.equal(element._computeAccountTitle(
{name: 'Andrew Bonventre'}),
{name: 'Andrew Bonventre'}, /* additionalText= */ ''),
'Andrew Bonventre');
assert.equal(element._computeAccountTitle(
{
email: 'andybons+gerrit@gmail.com',
}),
}, /* additionalText= */ ''),
'Anonymous <andybons+gerrit@gmail.com>');
assert.equal(element._computeShowEmailClass(
{
name: 'Andrew Bonventre',
email: 'andybons+gerrit@gmail.com',
}), '');
}, /* additionalText= */ ''), '');
assert.equal(element._computeShowEmailClass(
{
email: 'andybons+gerrit@gmail.com',
}), 'showEmail');
}, /* additionalText= */ ''), 'showEmail');
assert.equal(element._computeShowEmailClass({name: 'Andrew Bonventre'}),
assert.equal(element._computeShowEmailClass(
{name: 'Andrew Bonventre'},
/* additionalText= */ '',
),
'');
assert.equal(element._computeShowEmailClass(undefined), '');

View File

@@ -173,6 +173,14 @@
},
_computeFullDateStr(dateStr, timeFormat) {
// Polymer 2: check for undefined
if ([
dateStr,
timeFormat,
].some(arg => arg === undefined)) {
return undefined;
}
if (!dateStr) { return ''; }
const date = moment(util.parseDate(dateStr));
if (!date.isValid()) { return ''; }

View File

@@ -118,6 +118,15 @@
},
_computeSaveDisabled(disabled, content, newContent) {
// Polymer 2: check for undefined
if ([
disabled,
content,
newContent,
].some(arg => arg === undefined)) {
return undefined;
}
return disabled || (content === newContent);
},