Add reviewers column to dashboard

Enable the reviewers column only when the attention set feature is
enabled. We want to roll this out as part of the attention set.

Screenshots:
Before: https://imgur.com/a/EGAHRPy
After: https://imgur.com/a/UtBc8H6

Change-Id: Iebaa8387df630b7ecac93b4586f1e8a49990baf5
This commit is contained in:
Dmitrii Filippov 2020-03-18 15:43:56 +01:00 committed by Ben Rohlfs
parent 0a6e7e20fd
commit 0d80c7db93
16 changed files with 114 additions and 67 deletions

@ -1 +1 @@
Subproject commit 47b783ea75036664dd591d2d3f1bcd06b68cdd5e
Subproject commit e26ed31aaf070ff884e96b9a09d39c20437de6cb

View File

@ -29,6 +29,7 @@
'Status',
'Owner',
'Assignee',
'Reviewers',
'Comments',
'Repo',
'Branch',
@ -74,6 +75,7 @@
isColumnEnabled(column, config, experiments) {
if (!config || !config.change) return true;
if (column === 'Comments') return experiments.includes('comments-column');
if (column === 'Reviewers') return !!config.change.enable_attention_set;
return true;
},

View File

@ -65,6 +65,7 @@ suite('gr-change-table-behavior tests', () => {
'Status',
'Owner',
'Assignee',
'Reviewers',
'Comments',
'Repo',
'Branch',
@ -77,6 +78,7 @@ suite('gr-change-table-behavior tests', () => {
'Subject',
'Status',
'Assignee',
'Reviewers',
'Comments',
'Repo',
'Branch',

View File

@ -52,7 +52,8 @@ export const htmlTemplate = html`
white-space: nowrap;
width: 100%;
}
.comments {
.comments,
.reviewers {
white-space: nowrap;
}
.spacer {
@ -104,6 +105,9 @@ export const htmlTemplate = html`
.cell.label {
font-weight: var(--font-weight-normal);
}
.lastChildHidden:last-of-type {
display: none;
}
@media only screen and (max-width: 50em) {
:host {
display: flex;
@ -153,6 +157,14 @@ export const htmlTemplate = html`
<span class="placeholder">--</span>
</template>
</td>
<td class="cell reviewers" hidden\$="[[isColumnHidden('Reviewers', visibleChangeTableColumns)]]">
<div>
<template is="dom-repeat" items="[[change.reviewers.REVIEWER]]" as="reviewer">
<gr-account-link hide-avatar="" hide-status="" account="[[reviewer]]"></gr-account-link><!--
--><span class="lastChildHidden">, </span>
</template>
</div>
</td>
<td class="cell comments" hidden\$="[[isColumnHidden('Comments', visibleChangeTableColumns)]]">
<iron-icon hidden\$="[[!change.unresolved_comment_count]]" icon="gr-icons:comment"></iron-icon>
<span>[[_computeComments(change.unresolved_comment_count)]]</span>

View File

@ -124,6 +124,7 @@ suite('gr-change-list-item tests', () => {
'Status',
'Owner',
'Assignee',
'Reviewers',
'Comments',
'Repo',
'Branch',
@ -149,6 +150,7 @@ suite('gr-change-list-item tests', () => {
'Status',
'Owner',
'Assignee',
'Reviewers',
'Comments',
'Branch',
'Updated',

View File

@ -371,6 +371,7 @@ suite('gr-change-list basic tests', () => {
'Status',
'Owner',
'Assignee',
'Reviewers',
'Comments',
'Repo',
'Branch',
@ -408,6 +409,7 @@ suite('gr-change-list basic tests', () => {
'Status',
'Owner',
'Assignee',
'Reviewers',
'Comments',
'Branch',
'Updated',

View File

@ -105,14 +105,6 @@ class GrDashboardView extends mixinBehaviors( [
];
}
get options() {
return this.listChangesOptionsToHex(
this.ListChangesOption.LABELS,
this.ListChangesOption.DETAILED_ACCOUNTS,
this.ListChangesOption.REVIEWED
);
}
/** @override */
attached() {
super.attached();
@ -240,7 +232,7 @@ class GrDashboardView extends mixinBehaviors( [
queries.push('owner:self limit:1');
}
return this.$.restAPI.getChanges(null, queries, null, this.options)
return this.$.restAPI.getChanges(null, queries)
.then(changes => {
if (checkForNewUser) {
// Last set of results is not meant for dashboard display.

View File

@ -206,8 +206,7 @@ suite('gr-dashboard-view tests', () => {
};
return paramsChangedPromise.then(() => {
assert.isTrue(
getChangesStub.calledWith(
null, ['1', '2', 'owner:self limit:1'], null, element.options));
getChangesStub.calledWith(null, ['1', '2', 'owner:self limit:1']));
});
});
@ -221,9 +220,7 @@ suite('gr-dashboard-view tests', () => {
user: 'user',
};
return paramsChangedPromise.then(() => {
assert.isTrue(
getChangesStub.calledWith(
null, ['1'], null, element.options));
assert.isTrue(getChangesStub.calledWith(null, ['1']));
});
});
});
@ -239,8 +236,7 @@ suite('gr-dashboard-view tests', () => {
return paramsChangedPromise.then(() => {
assert.isTrue(getChangesStub.calledOnce);
assert.deepEqual(
getChangesStub.firstCall.args,
[null, ['1', '2 suffix'], null, element.options]);
getChangesStub.firstCall.args, [null, ['1', '2 suffix']]);
});
});

View File

@ -47,6 +47,7 @@ suite('gr-change-table-editor tests', () => {
'Status',
'Owner',
'Assignee',
'Reviewers',
'Comments',
'Repo',
'Branch',

View File

@ -54,6 +54,10 @@ class GrAccountLabel extends mixinBehaviors( [
type: Boolean,
value: false,
},
hideStatus: {
type: Boolean,
value: false,
},
_serverConfig: {
type: Object,
value: null,

View File

@ -51,8 +51,10 @@ export const htmlTemplate = html`
<span class="text">
<span class="name">
[[_computeName(account, _serverConfig)]]</span>
<template is="dom-if" if="[[account.status]]">
<iron-icon icon="gr-icons:calendar"></iron-icon>
<template is="dom-if" if="[[!hideStatus]]">
<template is="dom-if" if="[[account.status]]">
<iron-icon icon="gr-icons:calendar"></iron-icon>
</template>
</template>
</span>
</span>

View File

@ -42,6 +42,14 @@ class GrAccountLink extends mixinBehaviors( [
return {
voteableText: String,
account: Object,
hideAvatar: {
type: Boolean,
value: false,
},
hideStatus: {
type: Boolean,
value: false,
},
};
}

View File

@ -33,7 +33,11 @@ export const htmlTemplate = html`
</style>
<span>
<a href\$="[[_computeOwnerLink(account)]]" tabindex="-1">
<gr-account-label account="[[account]]" voteable-text="[[voteableText]]"></gr-account-label>
<gr-account-label hide-avatar="[[hideAvatar]]"
hide-status="[[hideStatus]]"
account="[[account]]"
voteable-text="[[voteableText]]">
</gr-account-label>
</a>
</span>
`;

View File

@ -950,49 +950,50 @@ class GrRestApiInterface extends mixinBehaviors( [
* changeInfos.
*/
getChanges(opt_changesPerPage, opt_query, opt_offset, opt_options) {
const options = opt_options || this.listChangesOptionsToHex(
this.ListChangesOption.LABELS,
this.ListChangesOption.DETAILED_ACCOUNTS
);
// Issue 4524: respect legacy token with max sortkey.
if (opt_offset === 'n,z') {
opt_offset = 0;
}
const params = {
O: options,
S: opt_offset || 0,
};
if (opt_changesPerPage) { params.n = opt_changesPerPage; }
if (opt_query && opt_query.length > 0) {
params.q = opt_query;
}
const iterateOverChanges = arr => {
for (const change of (arr || [])) {
this._maybeInsertInLookup(change);
}
};
const req = {
url: '/changes/',
params,
reportUrlAsIs: true,
};
return this._restApiHelper.fetchJSON(req).then(response => {
// Response may be an array of changes OR an array of arrays of
// changes.
if (opt_query instanceof Array) {
// Normalize the response to look like a multi-query response
// when there is only one query.
if (opt_query.length === 1) {
response = [response];
}
for (const arr of response) {
iterateOverChanges(arr);
}
} else {
iterateOverChanges(response);
}
return response;
});
return this.getConfig(false)
.then(config => {
const options = opt_options || this._getChangesOptionsHex(config);
// Issue 4524: respect legacy token with max sortkey.
if (opt_offset === 'n,z') {
opt_offset = 0;
}
const params = {
O: options,
S: opt_offset || 0,
};
if (opt_changesPerPage) { params.n = opt_changesPerPage; }
if (opt_query && opt_query.length > 0) {
params.q = opt_query;
}
return {
url: '/changes/',
params,
reportUrlAsIs: true,
};
})
.then(req => this._restApiHelper.fetchJSON(req))
.then(response => {
const iterateOverChanges = arr => {
for (const change of (arr || [])) {
this._maybeInsertInLookup(change);
}
};
// Response may be an array of changes OR an array of arrays of
// changes.
if (opt_query instanceof Array) {
// Normalize the response to look like a multi-query response
// when there is only one query.
if (opt_query.length === 1) {
response = [response];
}
for (const arr of response) {
iterateOverChanges(arr);
}
} else {
iterateOverChanges(response);
}
return response;
});
}
/**
@ -1034,6 +1035,20 @@ class GrRestApiInterface extends mixinBehaviors( [
});
}
_getChangesOptionsHex(config) {
const options = [
this.ListChangesOption.LABELS,
this.ListChangesOption.DETAILED_ACCOUNTS,
];
if (config && config.change && config.change.enable_attention_set) {
options.push(this.ListChangesOption.DETAILED_LABELS);
} else {
options.push(this.ListChangesOption.REVIEWED);
}
return this.listChangesOptionsToHex(...options);
}
_getChangeOptionsHex(config) {
if (window.DEFAULT_DETAIL_HEXES && window.DEFAULT_DETAIL_HEXES.changePage
&& !(config.receive && config.receive.enable_signed_push)) {

View File

@ -332,10 +332,11 @@ suite('gr-rest-api-interface tests', () => {
});
});
test('legacy n,z key in change url is replaced', () => {
test('legacy n,z key in change url is replaced', async () => {
sandbox.stub(element, 'getConfig', async () => { return {}; });
const stub = sandbox.stub(element._restApiHelper, 'fetchJSON')
.returns(Promise.resolve([]));
element.getChanges(1, null, 'n,z');
await element.getChanges(1, null, 'n,z');
assert.equal(stub.lastCall.args[0].params.S, 0);
});

View File

@ -89,6 +89,9 @@ $_documentContainer.innerHTML = `<dom-module id="gr-change-list-styles">
.star {
width: 30px;
}
.reviewers div {
overflow: hidden;
}
.label, .endpoint {
border-left: 1px solid var(--border-color);
}
@ -118,7 +121,8 @@ $_documentContainer.innerHTML = `<dom-module id="gr-change-list-styles">
@media only screen and (max-width: 100em) {
.assignee,
.branch,
.owner {
.owner,
.reviewers {
max-width: 10rem;
}
}