Do not bold changes with an undefined change.review value This came up in Polymer 2 testing. Apparently change.review (which has boolean type) can be undefined, while in Polymer 1 it was always set. The rationale is that the default should be "reviewed = true", because something being unreviewed and thus becoming bold is the non-standard case. Change-Id: Ie027d9c8830e0d83b8d104261512269c241f2250
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js index 5d15d60..7d4dc5a 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js +++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
@@ -79,7 +79,7 @@ }, _computeItemNeedsReview(reviewed) { - return !reviewed; + return reviewed === false; }, _computeChangeURL(change) {
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html index df4a442..1d76df2 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html +++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
@@ -274,5 +274,11 @@ assert.equal(element._computeRepoDisplay(change, true), '…/test/repo'); }); + + test('_computeItemNeedsReview', () => { + assert.isFalse(element._computeItemNeedsReview(undefined)); + assert.isFalse(element._computeItemNeedsReview(true)); + assert.isTrue(element._computeItemNeedsReview(false)); + }); }); </script>
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js index 2214d52..d2b17a3 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
@@ -236,7 +236,7 @@ }, _computeItemNeedsReview(account, change, showReviewedState) { - return showReviewedState && !change.reviewed && + return showReviewedState && change.reviewed === false && !change.work_in_progress && this.changeIsOpen(change.status) && (!account || account._account_id != change.owner._account_id);
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html index dce2a55..650ebeb 100644 --- a/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html +++ b/polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
@@ -221,29 +221,38 @@ { _number: 1, status: 'NEW', + reviewed: false, owner: {_account_id: 0}, }, { _number: 2, status: 'MERGED', + reviewed: false, owner: {_account_id: 0}, }, { _number: 3, status: 'ABANDONED', + reviewed: false, owner: {_account_id: 0}, }, { _number: 4, status: 'NEW', + reviewed: false, work_in_progress: true, owner: {_account_id: 0}, }, + { + _number: 5, + status: 'NEW', + owner: {_account_id: 0}, + }, ]; flushAsynchronousOperations(); let elementItems = Polymer.dom(element.root).querySelectorAll( 'gr-change-list-item'); - assert.equal(elementItems.length, 5); + assert.equal(elementItems.length, 6); for (let i = 0; i < elementItems.length; i++) { assert.isFalse(elementItems[i].hasAttribute('needs-review')); } @@ -251,22 +260,24 @@ element.showReviewedState = true; elementItems = Polymer.dom(element.root).querySelectorAll( 'gr-change-list-item'); - assert.equal(elementItems.length, 5); + assert.equal(elementItems.length, 6); assert.isFalse(elementItems[0].hasAttribute('needs-review')); assert.isTrue(elementItems[1].hasAttribute('needs-review')); assert.isFalse(elementItems[2].hasAttribute('needs-review')); assert.isFalse(elementItems[3].hasAttribute('needs-review')); assert.isFalse(elementItems[4].hasAttribute('needs-review')); + assert.isFalse(elementItems[5].hasAttribute('needs-review')); element.account = {_account_id: 42}; elementItems = Polymer.dom(element.root).querySelectorAll( 'gr-change-list-item'); - assert.equal(elementItems.length, 5); + assert.equal(elementItems.length, 6); assert.isFalse(elementItems[0].hasAttribute('needs-review')); assert.isTrue(elementItems[1].hasAttribute('needs-review')); assert.isFalse(elementItems[2].hasAttribute('needs-review')); assert.isFalse(elementItems[3].hasAttribute('needs-review')); assert.isFalse(elementItems[4].hasAttribute('needs-review')); + assert.isFalse(elementItems[5].hasAttribute('needs-review')); }); test('no changes', () => {