opendev/gerrit
Code Issues Proposed changes

Do not bold changes with an undefined change.review value

Browse Source 
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
This commit is contained in:
Ben Rohlfs
2019-07-01 09:16:47 +02:00
parent d33b21ca57
commit 65513b0171
4 changed files with 22 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.js
View File

@@ -79,7 +79,7 @@
},
_computeItemNeedsReview(reviewed) {
return !reviewed;
return reviewed === false;
},
_computeChangeURL(change) {

6
polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item_test.html
View File

@@ -274,5 +274,11 @@ limitations under the License.
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>

2
polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list.js
View File

@@ -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);

17
polygerrit-ui/app/elements/change-list/gr-change-list/gr-change-list_test.html
View File

@@ -221,29 +221,38 @@ limitations under the License.
{
_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 @@ limitations under the License.
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', () => {