Additional entry point for review comments

lib/containers/current-pull-request-container.js

@@ -38,10 +38,10 @@ export default class CurrentPullRequestContainer extends React.Component {
     pushInProgress: PropTypes.bool.isRequired,
 
     workspace: PropTypes.object.isRequired,
-    workingDirectory: PropTypes.string.isRequired,
 
     // Actions
     onOpenIssueish: PropTypes.func.isRequired,
+    onOpenReviews: PropTypes.func.isRequired,
     onCreatePr: PropTypes.func.isRequired,
   }
 
@@ -147,7 +147,6 @@ export default class CurrentPullRequestContainer extends React.Component {
         results={associatedPullRequests.nodes}
         isLoading={false}
         workspace={this.props.workspace}
-        workingDirectory={this.props.workingDirectory}
         endpoint={this.props.endpoint}
         resultFilter={issueish => issueish.getHeadRepositoryID() === this.props.repository.id}
         {...this.controllerProps()}
@@ -176,9 +175,9 @@ export default class CurrentPullRequestContainer extends React.Component {
     return {
       title: 'Checked out pull request',
       onOpenIssueish: this.props.onOpenIssueish,
+      onOpenReviews: this.props.onOpenReviews,
       emptyComponent: this.renderEmptyTile,
       needReviewsButton: true,
-      workingDirectory: this.props.workingDirectory,
     };
   }
 }

lib/containers/issueish-search-container.js

@@ -22,6 +22,7 @@ export default class IssueishSearchContainer extends React.Component {
     // Action methods
     onOpenIssueish: PropTypes.func.isRequired,
     onOpenSearch: PropTypes.func.isRequired,
+    onOpenReviews: PropTypes.func.isRequired,
   }
 
   static defaultProps = {
@@ -116,6 +117,7 @@ export default class IssueishSearchContainer extends React.Component {
       title: this.props.search.getName(),
 
       onOpenIssueish: this.props.onOpenIssueish,
+      onOpenReviews: this.props.onOpenReviews,
       onOpenMore: () => this.props.onOpenSearch(this.props.search),
     };
   }

lib/controllers/issueish-list-controller.js

@@ -4,7 +4,8 @@ import {graphql, createFragmentContainer} from 'react-relay';
 import {EndpointPropType} from '../prop-types';
 import IssueishListView from '../views/issueish-list-view';
 import Issueish from '../models/issueish';
-import ReviewsItem from '../items/reviews-item';
+import {shell, remote} from 'electron';
+const {Menu, MenuItem} = remote;
 import {addEvent} from '../reporter-proxy';
 
 const StatePropType = PropTypes.oneOf(['EXPECTED', 'PENDING', 'SUCCESS', 'ERROR', 'FAILURE']);
@@ -51,12 +52,12 @@ export class BareIssueishListController extends React.Component {
 
     resultFilter: PropTypes.func,
     onOpenIssueish: PropTypes.func.isRequired,
+    onOpenReviews: PropTypes.func.isRequired,
     onOpenMore: PropTypes.func,
 
     emptyComponent: PropTypes.func,
     workspace: PropTypes.object,
     endpoint: EndpointPropType,
-    workingDirectory: PropTypes.string,
     needReviewsButton: PropTypes.bool,
   };
 
@@ -90,21 +91,31 @@ export class BareIssueishListController extends React.Component {
     return null;
   }
 
-  openReviews = async () => {
-    /* istanbul ignore next */
-    if (this.props.results.length < 1) {
-      return;
-    }
-    const result = this.props.results[0];
-    const uri = ReviewsItem.buildURI({
-      host: this.props.endpoint.getHost(),
-      owner: result.repository.owner.login,
-      repo: result.repository.name,
-      number: result.number,
-      workdir: this.props.workingDirectory,
+  openOnGitHub = url => {
+    return new Promise((resolve, reject) => {
+      shell.openExternal(url, {}, err => {
+        if (err) { reject(err); } else {
+          resolve();
+          addEvent('open-issueish-in-browser', {package: 'github', component: this.constructor.name});
+        }
+      });
     });
-    await this.props.workspace.open(uri);
-    addEvent('open-reviews-tab', {package: 'github', from: this.constructor.name});
+  }
+
+  showActionsMenu = /* istanbul ignore next */ issueish => {
+    const menu = new Menu();
+
+    menu.append(new MenuItem({
+      label: 'See reviews',
+      click: () => this.props.onOpenReviews(issueish),
+    }));
+
+    menu.append(new MenuItem({
+      label: 'Open on GitHub',
+      click: () => this.openOnGitHub(issueish.getGitHubURL()),
+    }));
+
+    menu.popup(remote.getCurrentWindow());
   }
 
   render() {
@@ -118,7 +129,9 @@ export class BareIssueishListController extends React.Component {
         needReviewsButton={this.props.needReviewsButton}
         onIssueishClick={this.props.onOpenIssueish}
         onMoreClick={this.props.onOpenMore}
-        openReviews={this.openReviews}
+        openReviews={this.props.onOpenReviews}
+        openOnGitHub={this.openOnGitHub}
+        showActionsMenu={this.showActionsMenu}
         emptyComponent={this.props.emptyComponent}
       />
     );

lib/controllers/issueish-searches-controller.js

@@ -9,6 +9,7 @@ import Search from '../models/search';
 import IssueishSearchContainer from '../containers/issueish-search-container';
 import CurrentPullRequestContainer from '../containers/current-pull-request-container';
 import IssueishDetailItem from '../items/issueish-detail-item';
+import ReviewsItem from '../items/reviews-item';
 import {addEvent} from '../reporter-proxy';
 
 export default class IssueishSearchesController extends React.Component {
@@ -68,6 +69,7 @@ export default class IssueishSearchesController extends React.Component {
           workspace={this.props.workspace}
           workingDirectory={this.props.workingDirectory}
           onOpenIssueish={this.onOpenIssueish}
+          onOpenReviews={this.onOpenReviews}
           onCreatePr={this.props.onCreatePr}
         />
         {this.state.searches.map(search => (
@@ -81,12 +83,26 @@ export default class IssueishSearchesController extends React.Component {
 
             onOpenIssueish={this.onOpenIssueish}
             onOpenSearch={this.onOpenSearch}
+            onOpenReviews={this.onOpenReviews}
           />
         ))}
       </div>
     );
   }
 
+  onOpenReviews = issueish => {
+    const uri = ReviewsItem.buildURI({
+      host: this.props.endpoint.getHost(),
+      owner: this.props.remote.getOwner(),
+      repo: this.props.remote.getRepo(),
+      number: issueish.getNumber(),
+      workdir: this.props.workingDirectory,
+    });
+    return this.props.workspace.open(uri).then(() => {
+      addEvent('open-reviews-tab', {package: 'github', from: this.constructor.name});
+    });
+  }
+
   onOpenIssueish = issueish => {
     return this.props.workspace.open(
       IssueishDetailItem.buildURI({

lib/views/issueish-list-view.js

@@ -26,7 +26,9 @@ export default class IssueishListView extends React.Component {
     needReviewsButton: PropTypes.bool,
     onIssueishClick: PropTypes.func.isRequired,
     onMoreClick: PropTypes.func,
-    openReviews: PropTypes.func,
+    openReviews: PropTypes.func.isRequired,
+    openOnGitHub: PropTypes.func.isRequired,
+    showActionsMenu: PropTypes.func.isRequired,
 
     emptyComponent: PropTypes.func,
     error: PropTypes.object,
@@ -56,7 +58,7 @@ export default class IssueishListView extends React.Component {
   }
 
   renderReviewsButton = () => {
-    if (!this.props.needReviewsButton || this.props.total < 1) {
+    if (!this.props.needReviewsButton || this.props.issueishes.length < 1) {
       return null;
     }
     return (
@@ -70,7 +72,7 @@ export default class IssueishListView extends React.Component {
 
   openReviews = e => {
     e.stopPropagation();
-    this.props.openReviews();
+    this.props.openReviews(this.props.issueishes[0]);
   }
 
   renderIssueish(issueish) {
@@ -94,10 +96,21 @@ export default class IssueishListView extends React.Component {
           displayStyle="short"
           className="github-IssueishList-item github-IssueishList-item--age"
         />
+        <Octicon icon="ellipses"
+          className="github-IssueishList-item github-IssueishList-item--menu"
+          onClick={event => this.showActionsMenu(event, issueish)}
+        />
       </Fragment>
     );
   }
 
+  showActionsMenu(event, issueish) {
+    event.preventDefault();
+    event.stopPropagation();
+
+    this.props.showActionsMenu(issueish);
+  }
+
   renderStatusSummary(statusCounts) {
     if (['success', 'failure', 'pending'].every(kind => statusCounts[kind] === 0)) {
       return <Octicon className="github-IssueishList-item github-IssueishList-item--status" icon="dash" />;

styles/issueish-list-view.less

@@ -63,6 +63,11 @@
       text-align: center;
     }
 
+    &--menu {
+      color: @text-color-subtle;
+      line-height: 1;
+    }
+
   }
 
   &-more {

test/controllers/issueish-list-controller.test.js

@@ -1,10 +1,10 @@
 import React from 'react';
 import {shallow} from 'enzyme';
+import {shell} from 'electron';
 
 import {createPullRequestResult} from '../fixtures/factories/pull-request-result';
 import Issueish from '../../lib/models/issueish';
 import {BareIssueishListController} from '../../lib/controllers/issueish-list-controller';
-import {getEndpoint} from '../../lib/models/endpoint';
 import * as reporterProxy from '../../lib/reporter-proxy';
 
 describe('IssueishListController', function() {
@@ -85,22 +85,40 @@ describe('IssueishListController', function() {
     assert.deepEqual(view.prop('issueishes').map(issueish => issueish.getNumber()), [11, 13, 12]);
   });
 
-  it('opens reviews', async function() {
-    const atomEnv = global.buildAtomEnvironment();
-    sinon.stub(atomEnv.workspace, 'open').resolves();
-    sinon.stub(reporterProxy, 'addEvent');
-    const pr = createPullRequestResult({number: 1337});
-    Object.assign(pr.repository, {owner: {login: 'owner'}, name: 'repo'});
-    const wrapper = shallow(buildApp({
-      workspace: atomEnv.workspace,
-      endpoint: getEndpoint('github.com'),
-      results: [pr],
-    }));
+  describe('openOnGitHub', function() {
+    const url = 'https://github.com/atom/github/pull/2084';
+
+    it('calls shell.openExternal with specified url', async function() {
+      const wrapper = shallow(buildApp());
+      sinon.stub(shell, 'openExternal').callsArg(2);
+
+      await wrapper.instance().openOnGitHub(url);
+      assert.isTrue(shell.openExternal.calledWith(url));
+    });
 
-    await wrapper.find('IssueishListView').prop('openReviews')();
-    assert.isTrue(atomEnv.workspace.open.called);
-    assert.isTrue(reporterProxy.addEvent.calledWith('open-reviews-tab', {package: 'github', from: 'BareIssueishListController'}));
+    it('fires `open-issueish-in-browser` event upon success', async function() {
+      const wrapper = shallow(buildApp());
+      sinon.stub(shell, 'openExternal').callsArg(2);
+      sinon.stub(reporterProxy, 'addEvent');
 
-    atomEnv.destroy();
+      await wrapper.instance().openOnGitHub(url);
+      assert.strictEqual(reporterProxy.addEvent.callCount, 1);
+
+      await assert.isTrue(reporterProxy.addEvent.calledWith('open-issueish-in-browser', {package: 'github', component: 'BareIssueishListController'}));
+    });
+
+    it('handles error when openOnGitHub fails', async function() {
+      const wrapper = shallow(buildApp());
+      sinon.stub(shell, 'openExternal').callsArgWith(2, new Error('oh noes'));
+      sinon.stub(reporterProxy, 'addEvent');
+
+      try {
+        await wrapper.instance().openOnGitHub(url);
+      } catch (err) {
+        assert.strictEqual(err.message, 'oh noes');
+      }
+      assert.strictEqual(reporterProxy.addEvent.callCount, 0);
+    });
   });
+
 });

test/controllers/issueish-searches-controller.test.js

@@ -115,4 +115,29 @@ describe('IssueishSearchesController', function() {
     );
     assert.isTrue(reporterProxy.addEvent.calledWith('open-issueish-in-pane', {package: 'github', from: 'issueish-list'}));
   });
+
+  it('passes a handler to open reviews and reports an event', async function() {
+    sinon.spy(atomEnv.workspace, 'open');
+
+    const wrapper = shallow(buildApp());
+    const container = wrapper.find('IssueishSearchContainer').at(0);
+
+    const issueish = new Issueish({
+      number: 2084,
+      url: 'https://github.com/atom/github/pull/2084',
+      author: {login: 'kuychaco', avatarUrl: 'https://avatars3.githubusercontent.com/u/7910250?v=4'},
+      createdAt: '2019-04-01T10:00:00',
+      repository: {id: '0'},
+      commits: {nodes: []},
+    });
+
+    sinon.stub(reporterProxy, 'addEvent');
+    await container.prop('onOpenReviews')(issueish);
+    assert.isTrue(
+      atomEnv.workspace.open.calledWith(
+        `atom-github://reviews/github.com/atom/github/2084?workdir=${encodeURIComponent(__dirname)}`,
+      ),
+    );
+    assert.isTrue(reporterProxy.addEvent.calledWith('open-reviews-tab', {package: 'github', from: 'IssueishSearchesController'}));
+  });
 });

test/views/issueish-list-view.test.js

@@ -141,11 +141,25 @@ describe('IssueishListView', function() {
       wrapper.find('.github-IssueishList-more a').simulate('click');
       assert.isTrue(onMoreClick.called);
     });
+
+    it('calls its `showActionsMenu` handler when the menu icon is clicked', function() {
+      const issueishes = [allGreen, mixed, allRed];
+      const showActionsMenu = sinon.stub();
+      const wrapper = mount(buildApp({
+        isLoading: false,
+        total: 3,
+        issueishes,
+        showActionsMenu,
+      }));
+
+      wrapper.find('Octicon.github-IssueishList-item--menu').at(1).simulate('click');
+      assert.isTrue(showActionsMenu.calledWith(mixed));
+    });
   });
 
   it('renders review button only if needed', function() {
     const openReviews = sinon.spy();
-    const wrapper = mount(buildApp({total: 1, openReviews}));
+    const wrapper = mount(buildApp({issueishes: [allGreen], openReviews}));
     assert.isFalse(wrapper.find('.github-IssueishList-openReviewsButton').exists());
 
     wrapper.setProps({needReviewsButton: true});