Add Attention Set UI to reply dialog
Mocks: https://www.gerritcodereview.com/design-docs/attention-set-solution-1-user-interface.html
Screenshots: https://imgur.com/a/Ik5IXGV
Change-Id: I6deb54a71c9b5f7e0bdbd3854f21b72a0be0cf18
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
index f5a0463..c9fdb78 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_html.js
@@ -777,6 +777,7 @@
permitted-labels="[[_change.permitted_labels]]"
draft-comment-threads="[[_draftCommentThreads]]"
project-config="[[_projectConfig]]"
+ server-config="[[_serverConfig]]"
can-be-started="[[_canStartReview]]"
on-send="_handleReplySent"
on-cancel="_handleReplyCancel"
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
index a0e1d71..6524411 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.js
@@ -182,6 +182,7 @@
* @type {{ commentlinks: Array }}
*/
projectConfig: Object,
+ serverConfig: Object,
knownLatestState: String,
underReview: {
type: Boolean,
@@ -250,6 +251,33 @@
type: Boolean,
value: false,
},
+ /**
+ * Is the UI in the state where the user individually modifies attention
+ * set entries?
+ */
+ _attentionModified: {
+ type: Boolean,
+ value: false,
+ },
+ /**
+ * Set of account IDs that currently constitutes the attention set, read
+ * from change.attention_set. Will be updated by the
+ * _computeNewAttention() observer.
+ */
+ _currentAttentionSet: {
+ type: Object,
+ value: () => new Set(),
+ },
+ /**
+ * Set of account IDs that should constitute the attention set after
+ * publishing the votes/comments. Will be initialized with a default (that
+ * matches the default rules that the backend would also apply) by the
+ * _computeNewAttention(_account, _reviewers, change) observer.
+ */
+ _newAttentionSet: {
+ type: Object,
+ value: () => new Set(),
+ },
_sendDisabled: {
type: Boolean,
computed: '_computeSendButtonDisabled(canBeStarted, ' +
@@ -282,6 +310,7 @@
'_changeUpdated(change.reviewers.*, change.owner)',
'_ccsChanged(_ccs.splices)',
'_reviewersChanged(_reviewers.splices)',
+ '_computeNewAttention(_account, _reviewers, change)',
];
}
@@ -503,13 +532,29 @@
this.reporting.time(SEND_REPLY_TIMING_LABEL);
const labels = this.$.labelScores.getLabelValues();
- const obj = {
+ const reviewInput = {
drafts: includeComments ? 'PUBLISH_ALL_REVISIONS' : 'KEEP',
labels,
};
if (startReview) {
- obj.ready = true;
+ reviewInput.ready = true;
+ }
+
+ if (this._attentionModified) {
+ reviewInput.ignore_default_rules = true;
+ reviewInput.add_to_attention_set = [];
+ for (const user of this._newAttentionSet) {
+ if (!this._currentAttentionSet.has(user)) {
+ reviewInput.add_to_attention_set.push(user);
+ }
+ }
+ reviewInput.remove_from_attention_set = [];
+ for (const user of this._currentAttentionSet) {
+ if (!this._newAttentionSet.has(user)) {
+ reviewInput.remove_from_attention_set.push(user);
+ }
+ }
}
if (this.draft != null) {
@@ -518,16 +563,16 @@
message: this.draft,
unresolved: !this._isResolvedPatchsetLevelComment,
};
- obj.comments = {
+ reviewInput.comments = {
[SpecialFilePath.PATCHSET_LEVEL_COMMENTS]: [comment],
};
} else {
- obj.message = this.draft;
+ reviewInput.message = this.draft;
}
}
const accountAdditions = {};
- obj.reviewers = this.$.reviewers.additions().map(reviewer => {
+ reviewInput.reviewers = this.$.reviewers.additions().map(reviewer => {
if (reviewer.account) {
accountAdditions[reviewer.account._account_id] = true;
}
@@ -541,14 +586,14 @@
}
reviewer = this._mapReviewer(reviewer);
reviewer.state = 'CC';
- obj.reviewers.push(reviewer);
+ reviewInput.reviewers.push(reviewer);
}
}
this.disabled = true;
const errFn = this._handle400Error.bind(this);
- return this._saveReview(obj, errFn)
+ return this._saveReview(reviewInput, errFn)
.then(response => {
if (!response) {
// Null or undefined response indicates that an error handler
@@ -610,6 +655,11 @@
return FocusTarget.BODY;
}
+ _isOwner(account, change) {
+ if (!account || !change || !change.owner) return false;
+ return account._account_id === change.owner._account_id;
+ }
+
_handle400Error(response) {
// A call to _saveReview could fail with a server error if erroneous
// reviewers were requested. This is signalled with a 400 Bad Request
@@ -713,6 +763,58 @@
this._reviewers = reviewers;
}
+ _handleAttentionModify() {
+ this._attentionModified = true;
+ }
+
+ _showAttentionSummary(config, attentionModified) {
+ return this._isAttentionSetEnabled(config) && !attentionModified;
+ }
+
+ _showAttentionDetails(config, attentionModified) {
+ return this._isAttentionSetEnabled(config) && attentionModified;
+ }
+
+ _isAttentionSetEnabled(config) {
+ return !!config && !!config.change && config.change.enable_attention_set;
+ }
+
+ _handleAttentionClick(e) {
+ const id = e.target.account._account_id;
+ if (!id) return;
+ if (this._newAttentionSet.has(id)) {
+ this._newAttentionSet.delete(id);
+ } else {
+ this._newAttentionSet.add(id);
+ }
+ // Ensure that Polymer picks up the change.
+ this._newAttentionSet = new Set(this._newAttentionSet);
+ }
+
+ _computeHasNewAttention(account, newAttention) {
+ return newAttention && account && newAttention.has(account._account_id);
+ }
+
+ _computeNewAttention(user, reviewers, change) {
+ if ([user, reviewers, change].includes(undefined)) {
+ return;
+ }
+ this._attentionModified = false;
+ this._currentAttentionSet =
+ new Set(Object.keys(change.attention_set || {})
+ .map(id => parseInt(id)));
+ const newAttention = new Set(this._currentAttentionSet);
+ if (this._isOwner(user, change)) {
+ reviewers.forEach(r => newAttention.add(r._account_id));
+ } else {
+ if (change.owner) {
+ newAttention.add(change.owner._account_id);
+ }
+ }
+ if (user) newAttention.delete(user._account_id);
+ this._newAttentionSet = newAttention;
+ }
+
_accountOrGroupKey(entry) {
return entry.id || entry._account_id;
}
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.js
index 79f38a6..b611e41 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.js
@@ -134,6 +134,45 @@
.preview-formatting {
margin-left: var(--spacing-m);
}
+ .attention-icon {
+ width: 14px;
+ height: 14px;
+ vertical-align: top;
+ position: relative;
+ top: 3px;
+ --iron-icon-height: 24px;
+ --iron-icon-width: 24px;
+ }
+ .attention .edit-attention-button {
+ vertical-align: top;
+ --padding: 0px 4px;
+ }
+ .attention .edit-attention-button iron-icon {
+ color: inherit;
+ }
+ .attention-detail .peopleList {
+ margin-top: var(--spacing-s);
+ }
+ .attention-detail gr-account-label {
+ background-color: var(--background-color-tertiary);
+ border-radius: 10px;
+ padding: 0 var(--spacing-m) 0 var(--spacing-s);
+ margin-right: var(--spacing-m);
+ user-select: none;
+ }
+ .attention-detail gr-account-label:focus {
+ outline: none;
+ }
+ .attention-detail gr-account-label:hover {
+ box-shadow: var(--elevation-level-1);
+ cursor: pointer;
+ }
+ .attention-detail .attentionDetailsTitle {
+ margin-bottom: var(--spacing-s);
+ }
+ .attention-detail .selectUsers {
+ color: var(--deemphasized-text-color);
+ }
</style>
<div class="container" tabindex="-1">
<section class="peopleContainer">
@@ -276,6 +315,82 @@
Saving comments...
</span>
</section>
+ <section
+ hidden$="[[!_showAttentionSummary(serverConfig, _attentionModified)]]"
+ class="attention"
+ >
+ <div>
+ <iron-icon class="attention-icon" icon="gr-icons:attention"></iron-icon>
+ <span hidden$="[[_isOwner(_account, change)]]"
+ >Bring to owner's attention.</span
+ >
+ <span hidden$="[[!_isOwner(_account, change)]]"
+ >Bring to all reviewer's attention.</span
+ >
+ <gr-button
+ class="edit-attention-button"
+ on-click="_handleAttentionModify"
+ link=""
+ position-below=""
+ data-label="Edit"
+ data-action-type="change"
+ data-action-key="edit"
+ title="Edit attention set changes"
+ role="button"
+ tabindex="0"
+ >
+ <iron-icon icon="gr-icons:edit" class=""></iron-icon>
+ Modify
+ </gr-button>
+ </div>
+ </section>
+ <section
+ hidden$="[[!_showAttentionDetails(serverConfig, _attentionModified)]]"
+ class="attention-detail"
+ >
+ <div class="attentionDetailsTitle">
+ <iron-icon class="attention-icon" icon="gr-icons:attention"></iron-icon>
+ <span>Bring to attention of ...</span>
+ <span class="selectUsers">(select users)</span>
+ </div>
+ <div class="peopleList">
+ <div class="peopleListLabel">Owner</div>
+ <gr-account-label
+ account="[[_owner]]"
+ show-attention="[[_computeHasNewAttention(_owner, _newAttentionSet)]]"
+ blurred="[[!_computeHasNewAttention(_owner, _newAttentionSet)]]"
+ hide-hovercard=""
+ on-click="_handleAttentionClick"
+ >
+ </gr-account-label>
+ </div>
+ <div class="peopleList">
+ <div class="peopleListLabel">Reviewers</div>
+ <template is="dom-repeat" items="[[_reviewers]]" as="account">
+ <gr-account-label
+ account="[[account]]"
+ show-attention="[[_computeHasNewAttention(account, _newAttentionSet)]]"
+ blurred="[[!_computeHasNewAttention(account, _newAttentionSet)]]"
+ hide-hovercard=""
+ on-click="_handleAttentionClick"
+ >
+ </gr-account-label>
+ </template>
+ </div>
+ <div class="peopleList">
+ <div class="peopleListLabel">CC</div>
+ <template is="dom-repeat" items="[[_ccs]]" as="account">
+ <gr-account-label
+ account="[[account]]"
+ show-attention="[[_computeHasNewAttention(account, _newAttentionSet)]]"
+ blurred="[[!_computeHasNewAttention(account, _newAttentionSet)]]"
+ hide-hovercard=""
+ on-click="_handleAttentionClick"
+ >
+ </gr-account-label>
+ </template>
+ </div>
+ </section>
<section class="actions">
<div class="left">
<span
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
index 576ee3e..24faa0d 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.html
@@ -90,6 +90,9 @@
element = fixture('basic');
element.change = {
_number: changeNum,
+ owner: {
+ _account_id: 999,
+ },
labels: {
'Verified': {
values: {
@@ -199,6 +202,48 @@
});
});
+ test('modified attention set', done => {
+ const buttonEl = element.shadowRoot.querySelector('.edit-attention-button');
+ MockInteractions.tap(buttonEl);
+ flushAsynchronousOperations();
+
+ stubSaveReview(review => {
+ assert.isTrue(review.ignore_default_rules);
+ assert.deepEqual(review.add_to_attention_set, []);
+ assert.deepEqual(review.remove_from_attention_set, []);
+ done();
+ });
+ MockInteractions.tap(element.shadowRoot.querySelector('.send'));
+ });
+
+ function checkComputeAttention(
+ userId, reviewerIds, ownerId, attSetIds, expectedIds) {
+ const user = {_account_id: userId};
+ const reviewers = reviewerIds.map(id => {
+ return {_account_id: id};
+ });
+ const change = {
+ owner: {_account_id: ownerId},
+ attention_set: {},
+ };
+ attSetIds.forEach(id => change.attention_set[id] = {});
+ element._computeNewAttention(user, reviewers, change);
+ assert.deepEqual(element._newAttentionSet, new Set(expectedIds));
+ }
+
+ test('computeNewAttention', () => {
+ checkComputeAttention(null, [], 999, [], [999]);
+ checkComputeAttention(1, [], 999, [], [999]);
+ checkComputeAttention(1, [], 999, [1], [999]);
+ checkComputeAttention(1, [22], 999, [], [999]);
+ checkComputeAttention(1, [22], 999, [22], [22, 999]);
+ checkComputeAttention(1, [], 1, [], []);
+ checkComputeAttention(1, [], 1, [1], []);
+ checkComputeAttention(1, [22], 1, [], [22]);
+ checkComputeAttention(1, [22, 33], 1, [], [22, 33]);
+ checkComputeAttention(1, [22, 33], 1, [22, 33], [22, 33]);
+ });
+
test('toggle resolved checkbox', done => {
// Async tick is needed because iron-selector content is distributed and
// distributed content requires an observer to be set up.
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
index 844b7f4..f666148 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label.js
@@ -60,6 +60,15 @@
type: Boolean,
value: false,
},
+ blurred: {
+ type: Boolean,
+ value: false,
+ reflectToAttribute: true,
+ },
+ hideHovercard: {
+ type: Boolean,
+ value: false,
+ },
hideAvatar: {
type: Boolean,
value: false,
diff --git a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.js b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.js
index 12df5fe..9e7807a 100644
--- a/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.js
+++ b/polygerrit-ui/app/elements/shared/gr-account-label/gr-account-label_html.js
@@ -65,13 +65,15 @@
</style>
<div class="overlay"></div>
<span>
- <gr-hovercard-account
- account="[[account]]"
- change="[[change]]"
- highlight-attention="[[highlightAttention]]"
- voteable-text="[[voteableText]]"
- >
- </gr-hovercard-account>
+ <template is="dom-if" if="[[!hideHovercard]]">
+ <gr-hovercard-account
+ account="[[account]]"
+ change="[[change]]"
+ highlight-attention="[[highlightAttention]]"
+ voteable-text="[[voteableText]]"
+ >
+ </gr-hovercard-account>
+ </template>
<template
is="dom-if"
if="[[_computeShowAttentionIcon(_config, highlightAttention, account, change)]]"
