opendev/gerrit
Code Issues Proposed changes

Revert "Revert "Show author and committer when relevant""

Browse Source 
This reverts commit 4ed22af4fd.

Reason for revert: Acceptable intermediate solution for Issue 7919 while
awaiting resolution of Issue 10026.

Change-Id: I746dd11219652aa7348e1ba2dbbab0c0445cb66d
This commit is contained in:
Kasper Nilsson
2018-11-12 19:48:12 +00:00
parent 2c3bdf9c55
commit 5486249ce7
4 changed files with 178 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
View File

@@ -139,11 +139,28 @@ limitations under the License.
<gr-account-link account="[[change.owner]]"></gr-account-link>
</span>
</section>
<section class$="[[_computeShowUploaderHide(change)]]">
<section class$="[[_computeShowRoleClass(change, _CHANGE_ROLE.UPLOADER)]]">
<span class="title">Uploader</span>
<span class="value">
<gr-account-link
account="[[_computeShowUploader(change)]]"></gr-account-link>
account="[[_getNonOwnerRole(change, _CHANGE_ROLE.UPLOADER)]]"
></gr-account-link>
</span>
</section>
<section class$="[[_computeShowRoleClass(change, _CHANGE_ROLE.AUTHOR)]]">
<span class="title">Author</span>
<span class="value">
<gr-account-link
account="[[_getNonOwnerRole(change, _CHANGE_ROLE.AUTHOR)]]"
></gr-account-link>
</span>
</section>
<section class$="[[_computeShowRoleClass(change, _CHANGE_ROLE.COMMITTER)]]">
<span class="title">Committer</span>
<span class="value">
<gr-account-link
account="[[_getNonOwnerRole(change, _CHANGE_ROLE.COMMITTER)]]"
></gr-account-link>
</span>
</section>
<section class="assignee">

47
polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
View File

@@ -97,6 +97,18 @@
type: Array,
computed: '_computeParents(change)',
},
/** @type {?} */
_CHANGE_ROLE: {
type: Object,
readOnly: true,
value: {
OWNER: 'owner',
UPLOADER: 'uploader',
AUTHOR: 'author',
COMMITTER: 'committer',
},
},
},
behaviors: [
@@ -299,24 +311,45 @@
return !!change.work_in_progress;
},
_computeShowUploaderHide(change) {
return this._computeShowUploader(change) ? '' : 'hideDisplay';
_computeShowRoleClass(change, role) {
return this._getNonOwnerRole(change, role) ? '' : 'hideDisplay';
},
_computeShowUploader(change) {
/**
* Get the user with the specified role on the change. Returns null if the
* user with that role is the same as the owner.
* @param {!Object} change
* @param {string} role One of the values from _CHANGE_ROLE
* @return {Object|null} either an accound or null.
*/
_getNonOwnerRole(change, role) {
if (!change.current_revision ||
!change.revisions[change.current_revision]) {
return null;
}
const rev = change.revisions[change.current_revision];
if (!rev) { return null; }
if (!rev || !rev.uploader ||
change.owner._account_id === rev.uploader._account_id) {
return null;
if (role === this._CHANGE_ROLE.UPLOADER &&
rev.uploader &&
change.owner._account_id !== rev.uploader._account_id) {
return rev.uploader;
}
return rev.uploader;
if (role === this._CHANGE_ROLE.AUTHOR &&
rev.commit && rev.commit.author &&
change.owner.email !== rev.commit.author.email) {
return rev.commit.author;
}
if (role === this._CHANGE_ROLE.COMMITTER &&
rev.commit && rev.commit.committer &&
change.owner.email !== rev.commit.committer.email) {
return rev.commit.committer;
}
return null;
},
_computeParents(change) {

221
polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
View File

@@ -185,115 +185,130 @@ limitations under the License.
assert.equal(element._computeWebLinks(element.commitInfo).length, 1);
});
test('_computeShowUploader test for uploader', () => {
const change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
owner: {
_account_id: 1019328,
},
revisions: {
rev1: {
_number: 1,
uploader: {
_account_id: 1011123,
},
},
},
current_revision: 'rev1',
status: 'NEW',
labels: {},
mergeable: true,
};
assert.deepEqual(element._computeShowUploader(change),
{_account_id: 1011123});
});
suite('_getNonOwnerRole', () => {
let change;
test('_computeShowUploader test that it does not return uploader', () => {
const change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
owner: {
_account_id: 1011123,
},
revisions: {
rev1: {
_number: 1,
uploader: {
_account_id: 1011123,
setup(() => {
change = {
owner: {
email: 'abc@def',
_account_id: 1019328,
},
revisions: {
rev1: {
_number: 1,
uploader: {
email: 'ghi@def',
_account_id: 1011123,
},
commit: {
author: {email: 'jkl@def'},
committer: {email: 'ghi@def'},
},
},
},
},
current_revision: 'rev1',
status: 'NEW',
labels: {},
mergeable: true,
};
assert.isNotOk(element._computeShowUploader(change));
});
current_revision: 'rev1',
};
});
test('no current_revision makes _computeShowUploader return null', () => {
const change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
owner: {
_account_id: 1011123,
},
revisions: {
rev1: {
_number: 1,
uploader: {
_account_id: 1011123,
},
},
},
status: 'NEW',
labels: {},
mergeable: true,
};
assert.isNotOk(element._computeShowUploader(change));
});
suite('role=uploader', () => {
test('_getNonOwnerRole for uploader', () => {
assert.deepEqual(
element._getNonOwnerRole(change, element._CHANGE_ROLE.UPLOADER),
{email: 'ghi@def', _account_id: 1011123});
});
test('_computeShowUploaderHide test for string which equals true', () => {
const change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
owner: {
_account_id: 1019328,
},
revisions: {
rev1: {
_number: 1,
uploader: {
_account_id: 1011123,
},
},
},
current_revision: 'rev1',
status: 'NEW',
labels: {},
mergeable: true,
};
assert.equal(element._computeShowUploaderHide(change), '');
});
test('_getNonOwnerRole that it does not return uploader', () => {
// Set the uploader email to be the same as the owner.
change.revisions.rev1.uploader._account_id = 1019328;
assert.isNull(element._getNonOwnerRole(change,
element._CHANGE_ROLE.UPLOADER));
});
test('_computeShowUploaderHide test for hideDisplay', () => {
const change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
owner: {
_account_id: 1011123,
},
revisions: {
rev1: {
_number: 1,
uploader: {
_account_id: 1011123,
},
},
},
current_revision: 'rev1',
status: 'NEW',
labels: {},
mergeable: true,
};
assert.equal(
element._computeShowUploaderHide(change), 'hideDisplay');
test('_getNonOwnerRole null for uploader with no current rev', () => {
delete change.current_revision;
assert.isNull(element._getNonOwnerRole(change,
element._CHANGE_ROLE.UPLOADER));
});
test('_computeShowRoleClass show uploader', () => {
assert.equal(element._computeShowRoleClass(
change, element._CHANGE_ROLE.UPLOADER), '');
});
test('_computeShowRoleClass hide uploader', () => {
// Set the uploader email to be the same as the owner.
change.revisions.rev1.uploader._account_id = 1019328;
assert.equal(element._computeShowRoleClass(change,
element._CHANGE_ROLE.UPLOADER), 'hideDisplay');
});
});
suite('role=committer', () => {
test('_getNonOwnerRole for committer', () => {
assert.deepEqual(
element._getNonOwnerRole(change, element._CHANGE_ROLE.COMMITTER),
{email: 'ghi@def'});
});
test('_getNonOwnerRole that it does not return committer', () => {
// Set the committer email to be the same as the owner.
change.revisions.rev1.commit.committer.email = 'abc@def';
assert.isNull(element._getNonOwnerRole(change,
element._CHANGE_ROLE.COMMITTER));
});
test('_getNonOwnerRole null for committer with no current rev', () => {
delete change.current_revision;
assert.isNull(element._getNonOwnerRole(change,
element._CHANGE_ROLE.COMMITTER));
});
test('_getNonOwnerRole null for committer with no commit', () => {
delete change.revisions.rev1.commit;
assert.isNull(element._getNonOwnerRole(change,
element._CHANGE_ROLE.COMMITTER));
});
test('_getNonOwnerRole null for committer with no committer', () => {
delete change.revisions.rev1.commit.committer;
assert.isNull(element._getNonOwnerRole(change,
element._CHANGE_ROLE.COMMITTER));
});
});
suite('role=author', () => {
test('_getNonOwnerRole for author', () => {
assert.deepEqual(
element._getNonOwnerRole(change, element._CHANGE_ROLE.AUTHOR),
{email: 'jkl@def'});
});
test('_getNonOwnerRole that it does not return author', () => {
// Set the author email to be the same as the owner.
change.revisions.rev1.commit.author.email = 'abc@def';
assert.isNull(element._getNonOwnerRole(change,
element._CHANGE_ROLE.AUTHOR));
});
test('_getNonOwnerRole null for author with no current rev', () => {
delete change.current_revision;
assert.isNull(element._getNonOwnerRole(change,
element._CHANGE_ROLE.AUTHOR));
});
test('_getNonOwnerRole null for author with no commit', () => {
delete change.revisions.rev1.commit;
assert.isNull(element._getNonOwnerRole(change,
element._CHANGE_ROLE.AUTHOR));
});
test('_getNonOwnerRole null for author with no author', () => {
delete change.revisions.rev1.commit.author;
assert.isNull(element._getNonOwnerRole(change,
element._CHANGE_ROLE.AUTHOR));
});
});
});
test('_computeParents', () => {

1
polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
View File

@@ -599,6 +599,7 @@ limitations under the License.
};
element._change = {
change_id: 'Iad9dc96274af6946f3632be53b106ef80f7ba6ca',
owner: {email: 'abc@def'},
revisions: {
rev2: {_number: 2, commit: {parents: []}},
rev1: {_number: 1, commit: {parents: []}},