diff --git a/lib/containers/current-pull-request-container.js b/lib/containers/current-pull-request-container.js index 4878222f79..6bccb589df 100644 --- a/lib/containers/current-pull-request-container.js +++ b/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, }; } } diff --git a/lib/containers/issueish-search-container.js b/lib/containers/issueish-search-container.js index 4566ccec3e..2fc0c083f0 100644 --- a/lib/containers/issueish-search-container.js +++ b/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), }; } diff --git a/lib/controllers/issueish-list-controller.js b/lib/controllers/issueish-list-controller.js index a004632a9f..5903a9ed71 100644 --- a/lib/controllers/issueish-list-controller.js +++ b/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} /> ); diff --git a/lib/controllers/issueish-searches-controller.js b/lib/controllers/issueish-searches-controller.js index 25c8dd48cc..b991fb0a7e 100644 --- a/lib/controllers/issueish-searches-controller.js +++ b/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} /> ))} ); } + 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({ diff --git a/lib/views/issueish-list-view.js b/lib/views/issueish-list-view.js index bf869cebab..8e1d33c9a6 100644 --- a/lib/views/issueish-list-view.js +++ b/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" /> + this.showActionsMenu(event, issueish)} + /> ); } + showActionsMenu(event, issueish) { + event.preventDefault(); + event.stopPropagation(); + + this.props.showActionsMenu(issueish); + } + renderStatusSummary(statusCounts) { if (['success', 'failure', 'pending'].every(kind => statusCounts[kind] === 0)) { return ; diff --git a/styles/issueish-list-view.less b/styles/issueish-list-view.less index 107eea01d9..f79e7b4640 100644 --- a/styles/issueish-list-view.less +++ b/styles/issueish-list-view.less @@ -63,6 +63,11 @@ text-align: center; } + &--menu { + color: @text-color-subtle; + line-height: 1; + } + } &-more { diff --git a/test/controllers/issueish-list-controller.test.js b/test/controllers/issueish-list-controller.test.js index ea1eabe7cb..c5b7ab4e3d 100644 --- a/test/controllers/issueish-list-controller.test.js +++ b/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); + }); }); + }); diff --git a/test/controllers/issueish-searches-controller.test.js b/test/controllers/issueish-searches-controller.test.js index 96f917d3c8..aa93c0c708 100644 --- a/test/controllers/issueish-searches-controller.test.js +++ b/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'})); + }); }); diff --git a/test/views/issueish-list-view.test.js b/test/views/issueish-list-view.test.js index b4a94087ee..f731497282 100644 --- a/test/views/issueish-list-view.test.js +++ b/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});