Add a comment tab for robot comments

Change-Id: Ic720a7e153cfb0c7648a438185644ae66584666a
2020-01-27 20:25:21 +01:00
parent 216932151e
commit b41e20e8e9
6 changed files with 293 additions and 20 deletions
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.html
@@ -612,6 +612,7 @@ limitations under the License.
              title$="[[_computeTotalCommentCounts(_change.unresolved_comment_count, _changeComments)]]">
            <span>Comment Threads</span></gr-tooltip-content>
        </paper-tab>
+        <paper-tab class="robotComments">Findings</paper-tab>
      </paper-tabs>
      <template is="dom-if" if="[[_isSelectedView(_currentView,
        _commentTabs.CHANGE_LOG)]]">
@@ -634,6 +635,17 @@ limitations under the License.
            change="[[_change]]"
            change-num="[[_changeNum]]"
            logged-in="[[_loggedIn]]"
+            only-show-robot-comments-with-human-reply
+            on-thread-list-modified="_handleReloadDiffComments"></gr-thread-list>
+      </template>
+      <template is="dom-if" if="[[_isSelectedView(_currentView,
+        _commentTabs.ROBOT_COMMENTS)]]">
+        <gr-thread-list
+            threads="[[_robotCommentThreads]]"
+            change="[[_change]]"
+            change-num="[[_changeNum]]"
+            logged-in="[[_loggedIn]]"
+            hide-toggle-buttons
            on-thread-list-modified="_handleReloadDiffComments"></gr-thread-list>
      </template>
    </div>
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.js
@@ -62,6 +62,7 @@
  const CommentTabs = {
    CHANGE_LOG: 0,
    COMMENT_THREADS: 1,
+    ROBOT_COMMENTS: 2,
  };

  const CHANGE_DATA_TIMING_LABEL = 'ChangeDataLoaded';
@@ -137,6 +138,11 @@
          computed: '_computeDiffPrefsDisabled(disableDiffPrefs, _loggedIn)',
        },
        _commentThreads: Array,
+        _robotCommentThreads: {
+          type: Array,
+          computed: '_computeRobotCommentThreads(_commentThreads,'
+            + ' _selectedRevision)',
+        },
        /** @type {?} */
        _serverConfig: {
          type: Object,
@@ -574,6 +580,15 @@
      return false;
    }

+    _computeRobotCommentThreads(commentThreads, selectedRevision) {
+      if (!commentThreads || !selectedRevision) return [];
+      return commentThreads.filter(thread => {
+        const comments = thread.comments || [];
+        return comments.length && comments[0].robot_id && (comments[0].patch_set
+          === selectedRevision._number);
+      });
+    }
+
    _handleReloadCommentThreads() {
      // Get any new drafts that have been saved in the diff view and show
      // in the comment thread view.
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view_test.html
@@ -68,6 +68,197 @@ limitations under the License.
      COMMENT_THREADS: 1,
    };

+    const THREADS = [
+      {
+        comments: [
+          {
+            __path: '/COMMIT_MSG',
+            author: {
+              _account_id: 1000000,
+              name: 'user',
+              username: 'user',
+            },
+            patch_set: 4,
+            id: 'ecf0b9fa_fe1a5f62',
+            line: 5,
+            updated: '2018-02-08 18:49:18.000000000',
+            message: 'test',
+            unresolved: true,
+          },
+          {
+            id: '503008e2_0ab203ee',
+            path: '/COMMIT_MSG',
+            line: 5,
+            in_reply_to: 'ecf0b9fa_fe1a5f62',
+            updated: '2018-02-13 22:48:48.018000000',
+            message: 'draft',
+            unresolved: false,
+            __draft: true,
+            __draftID: '0.m683trwff68',
+            __editing: false,
+            patch_set: '2',
+          },
+        ],
+        patchNum: 4,
+        path: '/COMMIT_MSG',
+        line: 5,
+        rootId: 'ecf0b9fa_fe1a5f62',
+        start_datetime: '2018-02-08 18:49:18.000000000',
+      },
+      {
+        comments: [
+          {
+            __path: 'test.txt',
+            author: {
+              _account_id: 1000000,
+              name: 'user',
+              username: 'user',
+            },
+            patch_set: 3,
+            id: '09a9fb0a_1484e6cf',
+            side: 'PARENT',
+            updated: '2018-02-13 22:47:19.000000000',
+            message: 'Some comment on another patchset.',
+            unresolved: false,
+          },
+        ],
+        patchNum: 3,
+        path: 'test.txt',
+        rootId: '09a9fb0a_1484e6cf',
+        start_datetime: '2018-02-13 22:47:19.000000000',
+        commentSide: 'PARENT',
+      },
+      {
+        comments: [
+          {
+            __path: '/COMMIT_MSG',
+            author: {
+              _account_id: 1000000,
+              name: 'user',
+              username: 'user',
+            },
+            patch_set: 2,
+            id: '8caddf38_44770ec1',
+            line: 4,
+            updated: '2018-02-13 22:48:40.000000000',
+            message: 'Another unresolved comment',
+            unresolved: true,
+          },
+        ],
+        patchNum: 2,
+        path: '/COMMIT_MSG',
+        line: 4,
+        rootId: '8caddf38_44770ec1',
+        start_datetime: '2018-02-13 22:48:40.000000000',
+      },
+      {
+        comments: [
+          {
+            __path: '/COMMIT_MSG',
+            author: {
+              _account_id: 1000000,
+              name: 'user',
+              username: 'user',
+            },
+            patch_set: 2,
+            id: 'scaddf38_44770ec1',
+            line: 4,
+            updated: '2018-02-14 22:48:40.000000000',
+            message: 'Yet another unresolved comment',
+            unresolved: true,
+          },
+        ],
+        patchNum: 2,
+        path: '/COMMIT_MSG',
+        line: 4,
+        rootId: 'scaddf38_44770ec1',
+        start_datetime: '2018-02-14 22:48:40.000000000',
+      },
+      {
+        comments: [
+          {
+            id: 'zcf0b9fa_fe1a5f62',
+            path: '/COMMIT_MSG',
+            line: 6,
+            updated: '2018-02-15 22:48:48.018000000',
+            message: 'resolved draft',
+            unresolved: false,
+            __draft: true,
+            __draftID: '0.m683trwff68',
+            __editing: false,
+            patch_set: '2',
+          },
+        ],
+        patchNum: 4,
+        path: '/COMMIT_MSG',
+        line: 6,
+        rootId: 'zcf0b9fa_fe1a5f62',
+        start_datetime: '2018-02-09 18:49:18.000000000',
+      },
+      {
+        comments: [
+          {
+            __path: '/COMMIT_MSG',
+            author: {
+              _account_id: 1000000,
+              name: 'user',
+              username: 'user',
+            },
+            patch_set: 4,
+            id: 'rc1',
+            line: 5,
+            updated: '2019-02-08 18:49:18.000000000',
+            message: 'test',
+            unresolved: true,
+            robot_id: 'rc1',
+          },
+        ],
+        patchNum: 4,
+        path: '/COMMIT_MSG',
+        line: 5,
+        rootId: 'rc1',
+        start_datetime: '2019-02-08 18:49:18.000000000',
+      },
+      {
+        comments: [
+          {
+            __path: '/COMMIT_MSG',
+            author: {
+              _account_id: 1000000,
+              name: 'user',
+              username: 'user',
+            },
+            patch_set: 4,
+            id: 'rc2',
+            line: 5,
+            updated: '2019-03-08 18:49:18.000000000',
+            message: 'test',
+            unresolved: true,
+            robot_id: 'rc2',
+          },
+          {
+            __path: '/COMMIT_MSG',
+            author: {
+              _account_id: 1000000,
+              name: 'user',
+              username: 'user',
+            },
+            patch_set: 4,
+            id: 'c2_1',
+            line: 5,
+            updated: '2019-03-08 18:49:18.000000000',
+            message: 'test',
+            unresolved: true,
+          },
+        ],
+        patchNum: 4,
+        path: '/COMMIT_MSG',
+        line: 5,
+        rootId: 'rc2',
+        start_datetime: '2019-03-08 18:49:18.000000000',
+      },
+    ];
+
    setup(() => {
      sandbox = sinon.sandbox.create();
      stub('gr-endpoint-decorator', {
@@ -472,6 +663,30 @@ limitations under the License.
      });
    });

+    suite('Findings comment tab', () => {
+      setup(done => {
+        element._commentThreads = THREADS;
+        element._selectedRevision = {_number: 4};
+        flush(() => {
+          done();
+        });
+      });
+      test('only robot comments are rendered', () => {
+        assert.equal(element._robotCommentThreads.length, 2);
+        assert.equal(element._robotCommentThreads[0].comments[0].robot_id,
+            'rc1');
+        assert.equal(element._robotCommentThreads[1].comments[0].robot_id,
+            'rc2');
+      });
+      test('changing patchsets resets robot comments', done => {
+        element._selectedRevision = {_number: 3};
+        flush(() => {
+          assert.equal(element._robotCommentThreads.length, 0);
+          done();
+        });
+      });
+    });
+
    test('reply button is not visible when logged out', () => {
      assert.equal(getComputedStyle(element.$.replyBtn).display, 'none');
      element._loggedIn = true;
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.html
@@ -60,18 +60,20 @@ limitations under the License.
        display: block
      }
    </style>
-    <div class="header">
-      <div class="toggleItem">
-        <paper-toggle-button
-            id="unresolvedToggle"
-            checked="{{_unresolvedOnly}}"></paper-toggle-button>
-          Only unresolved threads</div>
-      <div class$="toggleItem draftToggle [[_computeShowDraftToggle(loggedIn)]]">
-        <paper-toggle-button
-            id="draftToggle"
-            checked="{{_draftsOnly}}"></paper-toggle-button>
-          Only threads with drafts</div>
-    </div>
+    <template is="dom-if" if="[[!hideToggleButtons]]">
+      <div class="header">
+        <div class="toggleItem">
+          <paper-toggle-button
+              id="unresolvedToggle"
+              checked="{{_unresolvedOnly}}"></paper-toggle-button>
+            Only unresolved threads</div>
+        <div class$="toggleItem draftToggle [[_computeShowDraftToggle(loggedIn)]]">
+          <paper-toggle-button
+              id="draftToggle"
+              checked="{{_draftsOnly}}"></paper-toggle-button>
+            Only threads with drafts</div>
+      </div>
+    </template>
    <div id="threads">
      <template is="dom-if" if="[[!threads.length]]">
        There are no inline comment threads on any diff for this change.
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list.js
@@ -41,7 +41,8 @@
        _filteredThreads: {
          type: Array,
          computed: '_computeFilteredThreads(_sortedThreads, ' +
-            '_unresolvedOnly, _draftsOnly)',
+            '_unresolvedOnly, _draftsOnly,' +
+            'onlyShowRobotCommentsWithHumanReply)',
        },
        _unresolvedOnly: {
          type: Boolean,
@@ -51,6 +52,16 @@
          type: Boolean,
          value: false,
        },
+        /* Boolean properties used must default to false if passed as attribute
+        by the parent */
+        onlyShowRobotCommentsWithHumanReply: {
+          type: Boolean,
+          value: false,
+        },
+        hideToggleButtons: {
+          type: Boolean,
+          value: false,
+        },
      };
    }

@@ -97,12 +108,14 @@
          });
    }

-    _computeFilteredThreads(sortedThreads, unresolvedOnly, draftsOnly) {
+    _computeFilteredThreads(sortedThreads, unresolvedOnly, draftsOnly,
+        onlyShowRobotCommentsWithHumanReply) {
      // Polymer 2: check for undefined
      if ([
        sortedThreads,
        unresolvedOnly,
        draftsOnly,
+        onlyShowRobotCommentsWithHumanReply,
      ].some(arg => arg === undefined)) {
        return undefined;
      }
@@ -124,8 +137,8 @@
              humanReplyToRobotComment = true;
            }
          });
-          if (robotComment) {
-            return humanReplyToRobotComment ? c : false;
+          if (robotComment && onlyShowRobotCommentsWithHumanReply) {
+            return humanReplyToRobotComment;
          }
          return c;
        }
--- a/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-thread-list/gr-thread-list_test.html
@@ -43,6 +43,7 @@ limitations under the License.
    setup(() => {
      sandbox = sinon.sandbox.create();
      element = fixture('basic');
+      element.onlyShowRobotCommentsWithHumanReply = true;
      element.threads = [
        {
          comments: [
@@ -314,14 +315,15 @@ limitations under the License.
    });

    test('toggle unresolved only shows unresolved comments', () => {
-      MockInteractions.tap(element.$.unresolvedToggle);
+      MockInteractions.tap(element.shadowRoot.querySelector(
+          '#unresolvedToggle'));
      flushAsynchronousOperations();
      assert.equal(Polymer.dom(element.root)
          .querySelectorAll('gr-comment-thread').length, 5);
    });

    test('toggle drafts only shows threads with draft comments', () => {
-      MockInteractions.tap(element.$.draftToggle);
+      MockInteractions.tap(element.shadowRoot.querySelector('#draftToggle'));
      flushAsynchronousOperations();
      assert.equal(Polymer.dom(element.root)
          .querySelectorAll('gr-comment-thread').length, 2);
@@ -329,8 +331,9 @@ limitations under the License.

    test('toggle drafts and unresolved only shows threads with drafts and ' +
        'publicly unresolved ', () => {
-      MockInteractions.tap(element.$.draftToggle);
-      MockInteractions.tap(element.$.unresolvedToggle);
+      MockInteractions.tap(element.shadowRoot.querySelector('#draftToggle'));
+      MockInteractions.tap(element.shadowRoot.querySelector(
+          '#unresolvedToggle'));
      flushAsynchronousOperations();
      assert.equal(Polymer.dom(element.root)
          .querySelectorAll('gr-comment-thread').length, 2);
@@ -348,5 +351,18 @@ limitations under the License.
          'ecf0b9fa_fe1a5f62');
      assert.equal(dispatchSpy.lastCall.args[0].detail.path, '/COMMIT_MSG');
    });
+
+    suite('findings tab', () => {
+      setup(done => {
+        element.hideToggleButtons = true;
+        flush(() => {
+          done();
+        });
+      });
+      test('toggle buttons are hidden', () => {
+        assert.equal(element.shadowRoot.querySelector('.header').style.display,
+            'none');
+      });
+    });
  });
 </script>