commit 43f8e5f8374da00fdcc4a9ca714208be7bebcc2a
Author: Ben Rohlfs <brohlfs@google.com>
Date:   Fri Oct 9 10:36:29 2020 +0200

    Change when and how attention can be modified in the reply dialog
    
    You cannot change the attention set in the reply dialog anymore
    without making one of these three modifications: vote, comment,
    add/remove reviewer/cc.
    
    The 'Modify' button is disabled, iff the 'Send' button is disabled.
    
    If the 'Modify' button is disabled, then the message is always:
    'No changes to the attention set.'
    
    Change-Id: Ib86a3db823641b2ca518bcbf688610e1e88e943c

diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
index eeefe45..8e0d65c 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog.ts
@@ -899,6 +899,13 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
     return isAttentionSetEnabled(config) && attentionExpanded;
   }
 
+  _computeAttentionButtonTitle(sendDisabled?: boolean) {
+    return sendDisabled
+      ? 'Modify the attention set by adding a comment or use the account ' +
+          'hovercard in the change page.'
+      : 'Edit attention set changes';
+  }
+
   _handleAttentionClick(e: Event) {
     const id = (e.target as GrAccountChip)?.account?._account_id;
     if (!id) return;
@@ -1064,12 +1071,14 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
     return accountIds;
   }
 
-  _isNewAttentionEmpty(
+  _computeShowNoAttentionUpdate(
     config?: ServerInfo,
     currentAttentionSet?: Set<AccountId>,
-    newAttentionSet?: Set<AccountId>
+    newAttentionSet?: Set<AccountId>,
+    sendDisabled?: boolean
   ) {
     return (
+      sendDisabled ||
       this._computeNewAttentionAccounts(
         config,
         currentAttentionSet,
@@ -1080,10 +1089,11 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
 
   _computeDoNotUpdateMessage(
     currentAttentionSet?: Set<AccountId>,
-    newAttentionSet?: Set<AccountId>
+    newAttentionSet?: Set<AccountId>,
+    sendDisabled?: boolean
   ) {
     if (!currentAttentionSet || !newAttentionSet) return '';
-    if (areSetsEqual(currentAttentionSet, newAttentionSet)) {
+    if (sendDisabled || areSetsEqual(currentAttentionSet, newAttentionSet)) {
       return 'No changes to the attention set.';
     }
     if (containsAll(currentAttentionSet, newAttentionSet)) {
@@ -1398,10 +1408,7 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
     labelsChanged?: boolean,
     includeComments?: boolean,
     disabled?: boolean,
-    commentEditing?: boolean,
-    attentionExpanded?: boolean,
-    _currentAttentionSet?: Set<AccountId>,
-    _newAttentionSet?: Set<AccountId>
+    commentEditing?: boolean
   ) {
     if (
       canBeStarted === undefined ||
@@ -1411,10 +1418,7 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
       labelsChanged === undefined ||
       includeComments === undefined ||
       disabled === undefined ||
-      commentEditing === undefined ||
-      attentionExpanded === undefined ||
-      _currentAttentionSet === undefined ||
-      _newAttentionSet === undefined
+      commentEditing === undefined
     ) {
       return undefined;
     }
@@ -1425,29 +1429,7 @@ export class GrReplyDialog extends KeyboardShortcutMixin(
       return false;
     }
     const hasDrafts = includeComments && draftCommentThreads.length;
-    // Changing the attention set is also sufficient to enable the 'Send'
-    // button, but only if the attention set detail section was expanded and
-    // either someone other than the owner was added, or someone other than the
-    // current user was removed.
-    const isNotCurrentUser = (id: AccountId) =>
-      id !== this._account?._account_id;
-    const isNotOwner = (id: AccountId) => id !== this._owner?._account_id;
-    const attentionAdditions = [..._newAttentionSet]
-      .filter(id => !_currentAttentionSet.has(id))
-      .filter(isNotOwner);
-    const attentionRemovals = [..._currentAttentionSet]
-      .filter(id => !_newAttentionSet.has(id))
-      .filter(isNotCurrentUser);
-    const hasAttentionChange =
-      attentionExpanded &&
-      (attentionAdditions.length > 0 || attentionRemovals.length > 0);
-    return (
-      !hasDrafts &&
-      !text.length &&
-      !reviewersMutated &&
-      !labelsChanged &&
-      !hasAttentionChange
-    );
+    return !hasDrafts && !text.length && !reviewersMutated && !labelsChanged;
   }
 
   _computePatchSetWarning(patchNum?: PatchSetNum, labelsChanged?: boolean) {
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
index 1dda0f6..4b0bc24 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_html.ts
@@ -376,16 +376,16 @@ export const htmlTemplate = html`
           <div>
             <template
               is="dom-if"
-              if="[[_isNewAttentionEmpty(serverConfig, _currentAttentionSet, _newAttentionSet)]]"
+              if="[[_computeShowNoAttentionUpdate(serverConfig, _currentAttentionSet, _newAttentionSet, _sendDisabled)]]"
             >
               <span
                 >[[_computeDoNotUpdateMessage(_currentAttentionSet,
-                _newAttentionSet)]]</span
+                _newAttentionSet, _sendDisabled)]]</span
               >
             </template>
             <template
               is="dom-if"
-              if="[[!_isNewAttentionEmpty(serverConfig, _currentAttentionSet, _newAttentionSet)]]"
+              if="[[!_computeShowNoAttentionUpdate(serverConfig, _currentAttentionSet, _newAttentionSet, _sendDisabled)]]"
             >
               <span>Bring to attention of</span>
               <template
@@ -406,12 +406,13 @@ export const htmlTemplate = html`
             <gr-button
               class="edit-attention-button"
               on-click="_handleAttentionModify"
+              disabled="[[_sendDisabled]]"
               link=""
               position-below=""
               data-label="Edit"
               data-action-type="change"
               data-action-key="edit"
-              title="Edit attention set changes"
+              title="[[_computeAttentionButtonTitle(_sendDisabled)]]"
               role="button"
               tabindex="0"
             >
diff --git a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
index 2e05ed7..7af3792 100644
--- a/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
+++ b/polygerrit-ui/app/elements/change/gr-reply-dialog/gr-reply-dialog_test.js
@@ -999,11 +999,18 @@ suite('gr-reply-dialog tests', () => {
     element._reviewers = [makeAccount(), makeAccount()];
     element._ccs = [makeAccount(), makeAccount()];
     element.draftCommentThreads = [];
-    MockInteractions.tap(
-        element.shadowRoot.querySelector('.edit-attention-button'));
+    const modifyButton =
+        element.shadowRoot.querySelector('.edit-attention-button');
+    MockInteractions.tap(modifyButton);
     flush();
 
+    // "Modify" button disabled, because "Send" button is disabled.
+    assert.isFalse(element._attentionExpanded);
+    element.draft = 'a test comment';
+    MockInteractions.tap(modifyButton);
+    flush();
     assert.isTrue(element._attentionExpanded);
+
     let accountLabels = Array.from(element.shadowRoot.querySelectorAll(
         '.attention-detail gr-account-label'));
     assert.equal(accountLabels.length, 5);
@@ -1368,10 +1375,7 @@ suite('gr-reply-dialog tests', () => {
         /* labelsChanged= */ false,
         /* includeComments= */ false,
         /* disabled= */ false,
-        /* commentEditing= */ false,
-        /* attentionExpanded= */ false,
-        /* _currentAttentionSet */ new Set(),
-        /* _newAttentionSet */ new Set()
+        /* commentEditing= */ false
     ));
   });
 
@@ -1386,28 +1390,7 @@ suite('gr-reply-dialog tests', () => {
         /* labelsChanged= */ false,
         /* includeComments= */ false,
         /* disabled= */ false,
-        /* commentEditing= */ false,
-        /* attentionExpanded= */ false,
-        /* _currentAttentionSet */ new Set(),
-        /* _newAttentionSet */ new Set()
-    ));
-  });
-
-  test('_computeSendButtonDisabled_attentionExpanded true', () => {
-    const fn = element._computeSendButtonDisabled.bind(element);
-    // Mock everything false
-    assert.isFalse(fn(
-        /* canBeStarted= */ false,
-        /* draftCommentThreads= */ [],
-        /* text= */ '',
-        /* reviewersMutated= */ false,
-        /* labelsChanged= */ false,
-        /* includeComments= */ false,
-        /* disabled= */ false,
-        /* commentEditing= */ false,
-        /* attentionExpanded= */ true,
-        /* _currentAttentionSet */ new Set(),
-        /* _newAttentionSet */ new Set([1])
+        /* commentEditing= */ false
     ));
   });
 
@@ -1422,10 +1405,7 @@ suite('gr-reply-dialog tests', () => {
         /* labelsChanged= */ false,
         /* includeComments= */ true,
         /* disabled= */ false,
-        /* commentEditing= */ false,
-        /* attentionExpanded= */ false,
-        /* _currentAttentionSet */ new Set(),
-        /* _newAttentionSet */ new Set()
+        /* commentEditing= */ false
     ));
   });
 
@@ -1440,10 +1420,7 @@ suite('gr-reply-dialog tests', () => {
         /* labelsChanged= */ false,
         /* includeComments= */ false,
         /* disabled= */ false,
-        /* commentEditing= */ false,
-        /* attentionExpanded= */ false,
-        /* _currentAttentionSet */ new Set(),
-        /* _newAttentionSet */ new Set()
+        /* commentEditing= */ false
     ));
   });
 
@@ -1458,10 +1435,7 @@ suite('gr-reply-dialog tests', () => {
         /* labelsChanged= */ false,
         /* includeComments= */ false,
         /* disabled= */ false,
-        /* commentEditing= */ false,
-        /* attentionExpanded= */ false,
-        /* _currentAttentionSet */ new Set(),
-        /* _newAttentionSet */ new Set()
+        /* commentEditing= */ false
     ));
   });
 
@@ -1476,10 +1450,7 @@ suite('gr-reply-dialog tests', () => {
         /* labelsChanged= */ false,
         /* includeComments= */ false,
         /* disabled= */ false,
-        /* commentEditing= */ false,
-        /* attentionExpanded= */ false,
-        /* _currentAttentionSet */ new Set(),
-        /* _newAttentionSet */ new Set()
+        /* commentEditing= */ false
     ));
   });
 
@@ -1494,10 +1465,7 @@ suite('gr-reply-dialog tests', () => {
         /* labelsChanged= */ true,
         /* includeComments= */ false,
         /* disabled= */ false,
-        /* commentEditing= */ false,
-        /* attentionExpanded= */ false,
-        /* _currentAttentionSet */ new Set(),
-        /* _newAttentionSet */ new Set()
+        /* commentEditing= */ false
     ));
   });
 
@@ -1512,10 +1480,7 @@ suite('gr-reply-dialog tests', () => {
         /* labelsChanged= */ true,
         /* includeComments= */ false,
         /* disabled= */ true,
-        /* commentEditing= */ false,
-        /* attentionExpanded= */ false,
-        /* _currentAttentionSet */ new Set(),
-        /* _newAttentionSet */ new Set()
+        /* commentEditing= */ false
     ));
     assert.isTrue(fn(
         /* buttonLabel= */ 'Send',
@@ -1525,10 +1490,7 @@ suite('gr-reply-dialog tests', () => {
         /* labelsChanged= */ true,
         /* includeComments= */ false,
         /* disabled= */ false,
-        /* commentEditing= */ true,
-        /* attentionExpanded= */ false,
-        /* _currentAttentionSet */ new Set(),
-        /* _newAttentionSet */ new Set()
+        /* commentEditing= */ true
     ));
   });