Merge d883d53 into aec3bce

lib/controllers/issueish-searches-controller.js

@@ -8,6 +8,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 {addEvent} from '../reporter-proxy';
 
 export default class IssueishSearchesController extends React.Component {
   static propTypes = {
@@ -93,7 +94,9 @@ export default class IssueishSearchesController extends React.Component {
         this.props.workingDirectory,
       ),
       {pending: true, searchAllPanes: true},
-    );
+    ).then(() => {
+      addEvent('open-issueish-in-pane', {package: 'github', from: 'issueish-list'});
+    });
   }
 
   onOpenSearch(search) {

lib/controllers/root-controller.js

@@ -26,7 +26,7 @@ import Switchboard from '../switchboard';
 import {WorkdirContextPoolPropType} from '../prop-types';
 import {destroyFilePatchPaneItems, destroyEmptyFilePatchPaneItems, autobind} from '../helpers';
 import {GitError} from '../git-shell-out-strategy';
-import {incrementCounter} from '../reporter-proxy';
+import {incrementCounter, addEvent} from '../reporter-proxy';
 
 export default class RootController extends React.Component {
   static propTypes = {
@@ -471,7 +471,9 @@ export default class RootController extends React.Component {
   acceptOpenIssueish({repoOwner, repoName, issueishNumber}) {
     const uri = IssueishDetailItem.buildURI('https://api.github.com', repoOwner, repoName, issueishNumber);
     this.setState({openIssueishDialogActive: false});
-    this.props.workspace.open(uri);
+    this.props.workspace.open(uri).then(() => {
+      addEvent('open-issueish-in-pane', {package: 'github', from: 'dialog'});
+    });
   }
 
   cancelOpenIssueish() {

lib/items/issueish-detail-item.js

@@ -6,6 +6,7 @@ import {autobind} from '../helpers';
 import {GithubLoginModelPropType, WorkdirContextPoolPropType} from '../prop-types';
 import Repository from '../models/repository';
 import IssueishDetailContainer from '../containers/issueish-detail-container';
+import {addEvent} from '../reporter-proxy';
 
 export default class IssueishDetailItem extends Component {
   static propTypes = {
@@ -98,6 +99,7 @@ export default class IssueishDetailItem extends Component {
           prevState.repo === prev.repo &&
           prevState.issueishNumber === prev.issueishNumber
         ) {
+          addEvent('open-issueish-in-pane', {package: 'github', from: 'issueish-link', target: 'current-tab'});
           return {
             owner,
             repo,

lib/views/github-dotcom-markdown.js

@@ -98,7 +98,9 @@ export class BareGithubDotcomMarkdown extends React.Component {
 
   handleClick(event) {
     if (event.target.dataset.url) {
-      this.props.handleClickEvent(event, event.target.dataset.url);
+      return this.props.handleClickEvent(event, event.target.dataset.url);
+    } else {
+      return null;
     }
   }
 

lib/views/issueish-detail-view.js

@@ -12,6 +12,7 @@ import GithubDotcomMarkdown from '../views/github-dotcom-markdown';
 import PeriodicRefresher from '../periodic-refresher';
 import {EnableableOperationPropType} from '../prop-types';
 import {autobind} from '../helpers';
+import {addEvent} from '../reporter-proxy';
 
 const reactionTypeToEmoji = {
   THUMBS_UP: '👍',
@@ -157,7 +158,8 @@ export class BareIssueishDetailView extends React.Component {
                   state={issueish.state}
                 />
                 <a className="github-IssueishDetailView-headerLink"
-                  href={issueish.url}>{repo.owner.login}/{repo.name}#{issueish.number}
+                  href={issueish.url} onClick={this.recordOpenInBrowserEvent}>
+                  {repo.owner.login}/{repo.name}#{issueish.number}
                 </a>
                 {isPr && <span className="github-IssueishDetailView-headerStatus">
                   <PrStatusesView pullRequest={issueish} displayType="check" />
@@ -253,6 +255,10 @@ export class BareIssueishDetailView extends React.Component {
     this.refresher.refreshNow(true);
   }
 
+  recordOpenInBrowserEvent() {
+    addEvent('open-issueish-in-browser', {package: 'github', from: 'issueish-header'});
+  }
+
   refresh() {
     if (this.state.refreshing) {
       return;

lib/views/issueish-link.js

@@ -5,6 +5,7 @@ import React from 'react';
 import PropTypes from 'prop-types';
 
 import IssueishDetailItem from '../items/issueish-detail-item';
+import {addEvent} from '../reporter-proxy';
 
 // eslint-disable-next-line no-shadow
 export default function IssueishLink({url, children, ...others}) {
@@ -23,26 +24,37 @@ IssueishLink.propTypes = {
 
 // eslint-disable-next-line no-shadow
 export function handleClickEvent(event, url) {
+  event.preventDefault();
+  event.stopPropagation();
   if (!event.shiftKey) {
-    event.preventDefault();
-    event.stopPropagation();
-    openIssueishLinkInNewTab(url, {activate: !(event.metaKey || event.ctrlKey)});
+    return openIssueishLinkInNewTab(url, {activate: !(event.metaKey || event.ctrlKey)});
   } else {
     // Open in browser if shift key held
-    openLinkInBrowser(url);
+    return openLinkInBrowser(url);
   }
 }
 
 // eslint-disable-next-line no-shadow
 export function openIssueishLinkInNewTab(url, options = {}) {
   const uri = getAtomUriForGithubUrl(url);
   if (uri) {
-    openInNewTab(uri, options);
+    return openInNewTab(uri, options);
+  } else {
+    return null;
   }
 }
 
 export function openLinkInBrowser(uri) {
-  shell.openExternal(uri);
+  return new Promise((resolve, reject) => {
+    shell.openExternal(uri, {}, err => {
+      if (err) {
+        reject(err);
+      } else {
+        addEvent('open-issueish-in-browser', {package: 'github', from: 'issueish-link'});
+        resolve();
+      }
+    });
+  });
 }
 
 function getAtomUriForGithubUrl(githubUrl) {
@@ -64,10 +76,7 @@ function getUriForData({hostname, repoOwner, repoName, type, issueishNumber}) {
 }
 
 function openInNewTab(uri, {activate} = {activate: true}) {
-  if (activate) {
-    atom.workspace.open(uri, {activateItem: activate});
-  } else {
-    const item = IssueishDetailItem.opener(uri);
-    atom.workspace.getActivePane().addItem(item);
-  }
+  return atom.workspace.open(uri, {activateItem: activate}).then(() => {
+    addEvent('open-issueish-in-pane', {package: 'github', from: 'issueish-link', target: 'new-tab'});
+  });
 }

test/atom/decoration.test.js

@@ -1,5 +1,4 @@
 import React from 'react';
-import sinon from 'sinon';
 import {mount} from 'enzyme';
 
 import Decoration from '../../lib/atom/decoration';

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

@@ -10,6 +10,7 @@ import Branch from '../../lib/models/branch';
 import BranchSet from '../../lib/models/branch-set';
 import Issueish from '../../lib/models/issueish';
 import {nullOperationStateObserver} from '../../lib/models/operation-state-observer';
+import * as reporterProxy from '../../lib/reporter-proxy';
 
 describe('IssueishSearchesController', function() {
   let atomEnv;
@@ -87,20 +88,22 @@ describe('IssueishSearchesController', function() {
     }
   });
 
-  it('passes a handler to open an issueish pane', async function() {
+  it('passes a handler to open an issueish pane 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(createPullRequestResult({number: 123}));
 
+    sinon.stub(reporterProxy, 'addEvent');
     await container.prop('onOpenIssueish')(issueish);
     assert.isTrue(
       atomEnv.workspace.open.calledWith(
         `atom-github://issueish/https%3A%2F%2Fapi.github.com/atom/github/123?workdir=${encodeURIComponent(__dirname)}`,
         {pending: true, searchAllPanes: true},
       ),
     );
+    assert.isTrue(reporterProxy.addEvent.calledWith('open-issueish-in-pane', {package: 'github', from: 'issueish-list'}));
   });
 });

test/controllers/root-controller.test.js

@@ -1072,5 +1072,15 @@ describe('RootController', function() {
       assert.strictEqual(item.getTitle(), 'owner/repo#123');
       assert.lengthOf(wrapper.update().find('IssueishDetailItem'), 1);
     });
+
+    describe('acceptOpenIssueish', function() {
+      it('records an event', async function() {
+        const wrapper = mount(app);
+        sinon.stub(reporterProxy, 'addEvent');
+        sinon.stub(workspace, 'open').returns(Promise.resolve());
+        await wrapper.instance().acceptOpenIssueish({repoOwner: 'owner', repoName: 'repo', issueishNumber: 123});
+        assert.isTrue(reporterProxy.addEvent.calledWith('open-issueish-in-pane', {package: 'github', from: 'dialog'}));
+      });
+    });
   });
 });

test/items/issueish-detail-item.test.js

@@ -7,6 +7,7 @@ import IssueishDetailItem from '../../lib/items/issueish-detail-item';
 import PaneItem from '../../lib/atom/pane-item';
 import WorkdirContextPool from '../../lib/models/workdir-context-pool';
 import {issueishPaneItemProps} from '../fixtures/props/issueish-pane-props';
+import * as reporterProxy from '../../lib/reporter-proxy';
 
 describe('IssueishDetailItem', function() {
   let atomEnv, subs;
@@ -170,6 +171,17 @@ describe('IssueishDetailItem', function() {
       assert.strictEqual(wrapper.find('IssueishDetailContainer').prop('issueishNumber'), 100);
       assert.isTrue(wrapper.find('IssueishDetailContainer').prop('repository').isAbsent());
     });
+
+    it('records an event after switching', async function() {
+      const wrapper = mount(buildApp({workdirContextPool}));
+      await atomEnv.workspace.open(IssueishDetailItem.buildURI('host.com', 'me', 'original', 1, __dirname));
+
+      wrapper.update();
+
+      sinon.stub(reporterProxy, 'addEvent');
+      await wrapper.find('IssueishDetailContainer').prop('switchToIssueish')('atom', 'github', 2);
+      assert.isTrue(reporterProxy.addEvent.calledWith('open-issueish-in-pane', {package: 'github', from: 'issueish-link', target: 'current-tab'}));
+    });
   });
 
   it('reconstitutes its original URI', async function() {

test/views/changed-files-count-view.test.js

@@ -1,6 +1,5 @@
 import React from 'react';
 import {shallow} from 'enzyme';
-import sinon from 'sinon';
 
 import ChangedFilesCountView from '../../lib/views/changed-files-count-view';
 import * as reporterProxy from '../../lib/reporter-proxy';