Merge pull request #2587 from atom/consent-is-important

Accept and cancel buttons on Git identity panel

Author: smashwilson

lib/controllers/git-tab-controller.js

@@ -3,7 +3,6 @@ import path from 'path';
 import React from 'react';
 import PropTypes from 'prop-types';
 import {TextBuffer} from 'atom';
-import {CompositeDisposable} from 'event-kit';
 
 import GitTabView from '../views/git-tab-view';
 import UserStore from '../models/user-store';
@@ -87,11 +86,6 @@ export default class GitTabController extends React.Component {
       login: this.props.loginModel,
       config: this.props.config,
     });
-
-    this.subs = new CompositeDisposable(
-      this.usernameBuffer.onDidStopChanging(this.setUsername),
-      this.emailBuffer.onDidStopChanging(this.setEmail),
-    );
   }
 
   static getDerivedStateFromProps(props, state) {
@@ -142,6 +136,8 @@ export default class GitTabController extends React.Component {
 
         toggleIdentityEditor={this.toggleIdentityEditor}
         closeIdentityEditor={this.closeIdentityEditor}
+        setLocalIdentity={this.setLocalIdentity}
+        setGlobalIdentity={this.setGlobalIdentity}
         openInitializeDialog={this.props.openInitializeDialog}
         openFiles={this.props.openFiles}
         discardWorkDirChangesForPaths={this.props.discardWorkDirChangesForPaths}
@@ -371,18 +367,26 @@ export default class GitTabController extends React.Component {
 
   closeIdentityEditor = () => this.setState({editingIdentity: false})
 
-  setUsername = () => {
+  setLocalIdentity = () => this.setIdentity({});
+
+  setGlobalIdentity = () => this.setIdentity({global: true});
+
+  async setIdentity(options) {
     const newUsername = this.usernameBuffer.getText();
-    if (newUsername !== this.props.username) {
-      this.props.repository.setConfig('user.name', newUsername, {global: true});
+    const newEmail = this.emailBuffer.getText();
+
+    if (newUsername.length > 0 || options.global) {
+      await this.props.repository.setConfig('user.name', newUsername, options);
+    } else {
+      await this.props.repository.unsetConfig('user.name');
     }
-  }
 
-  setEmail = () => {
-    const newEmail = this.emailBuffer.getText();
-    if (newEmail !== this.props.email) {
-      this.props.repository.setConfig('user.email', newEmail, {global: true});
+    if (newEmail.length > 0 || options.global) {
+      await this.props.repository.setConfig('user.email', newEmail, options);
+    } else {
+      await this.props.repository.unsetConfig('user.email');
     }
+    this.closeIdentityEditor();
   }
 
   restoreFocus() {

lib/views/git-identity-view.js

@@ -7,8 +7,11 @@ export default class GitIdentityView extends React.Component {
     // Model
     usernameBuffer: PropTypes.object.isRequired,
     emailBuffer: PropTypes.object.isRequired,
+    canWriteLocal: PropTypes.bool.isRequired,
 
     // Action methods
+    setLocal: PropTypes.func.isRequired,
+    setGlobal: PropTypes.func.isRequired,
     close: PropTypes.func.isRequired,
   };
 
@@ -19,19 +22,29 @@ export default class GitIdentityView extends React.Component {
           Git Identity
         </h1>
         <p className="github-GitIdentity-explanation">
-          Please set the username and email address that you wish to use to
-          author git commits.
+          Please set the username and email address that you wish to use to author git commits. This will write to the
+          <code>user.name</code> and <code>user.email</code> values in your git configuration at the chosen scope.
         </p>
         <div className="github-GitIdentity-text">
           <AtomTextEditor mini placeholderText="name" buffer={this.props.usernameBuffer} />
           <AtomTextEditor mini placeholderText="email address" buffer={this.props.emailBuffer} />
         </div>
         <div className="github-GitIdentity-buttons">
+          <button className="btn" onClick={this.props.close}>
+            Cancel
+          </button>
+          <button
+            className="btn btn-primary"
+            title="Configure git for this repository"
+            onClick={this.props.setLocal}
+            disabled={!this.props.canWriteLocal}>
+            Use for this repository
+          </button>
           <button
             className="btn btn-primary"
-            onClick={this.props.close}
-            disabled={this.props.usernameBuffer.isEmpty() || this.props.emailBuffer.isEmpty()}>
-            Continue
+            title="Configure git globally for your operating system user account"
+            onClick={this.props.setGlobal}>
+            Use for all repositories
           </button>
         </div>
       </div>

lib/views/git-tab-view.js

@@ -54,6 +54,8 @@ export default class GitTabView extends React.Component {
     tooltips: PropTypes.object.isRequired,
 
     toggleIdentityEditor: PropTypes.func.isRequired,
+    setLocalIdentity: PropTypes.func.isRequired,
+    setGlobalIdentity: PropTypes.func.isRequired,
     closeIdentityEditor: PropTypes.func.isRequired,
     openInitializeDialog: PropTypes.func.isRequired,
     abortMerge: PropTypes.func.isRequired,
@@ -267,6 +269,9 @@ export default class GitTabView extends React.Component {
       <GitIdentityView
         usernameBuffer={this.props.usernameBuffer}
         emailBuffer={this.props.emailBuffer}
+        canWriteLocal={this.props.repository.isPresent()}
+        setLocal={this.props.setLocalIdentity}
+        setGlobal={this.props.setGlobalIdentity}
         close={this.props.closeIdentityEditor}
       />
     );

styles/git-identity.less

@@ -26,6 +26,8 @@
   }
 
   &-buttons {
-    //
+    .btn {
+      margin: @component-padding/2;
+    }
   }
 }

test/controllers/git-tab-controller.test.js

@@ -230,6 +230,51 @@ describe('GitTabController', function() {
       assert.strictEqual(wrapper.find('GitTabView').prop('emailBuffer'), emailBuffer);
       assert.strictEqual(emailBuffer.getText(), 'changed+@email.com');
     });
+
+    it('sets repository-local identity', async function() {
+      const repository = await buildRepository(await cloneRepository('three-files'));
+      const setConfig = sinon.stub(repository, 'setConfig');
+
+      const wrapper = mount(await buildApp(repository));
+
+      wrapper.find('GitTabView').prop('usernameBuffer').setText('changed');
+      wrapper.find('GitTabView').prop('emailBuffer').setText('changed@email.com');
+
+      await wrapper.find('GitTabView').prop('setLocalIdentity')();
+
+      assert.isTrue(setConfig.calledWith('user.name', 'changed', {}));
+      assert.isTrue(setConfig.calledWith('user.email', 'changed@email.com', {}));
+    });
+
+    it('sets account-global identity', async function() {
+      const repository = await buildRepository(await cloneRepository('three-files'));
+      const setConfig = sinon.stub(repository, 'setConfig');
+
+      const wrapper = mount(await buildApp(repository));
+
+      wrapper.find('GitTabView').prop('usernameBuffer').setText('changed');
+      wrapper.find('GitTabView').prop('emailBuffer').setText('changed@email.com');
+
+      await wrapper.find('GitTabView').prop('setGlobalIdentity')();
+
+      assert.isTrue(setConfig.calledWith('user.name', 'changed', {global: true}));
+      assert.isTrue(setConfig.calledWith('user.email', 'changed@email.com', {global: true}));
+    });
+
+    it('unsets config values when empty', async function() {
+      const repository = await buildRepository(await cloneRepository('three-files'));
+      const unsetConfig = sinon.stub(repository, 'unsetConfig');
+
+      const wrapper = mount(await buildApp(repository));
+
+      wrapper.find('GitTabView').prop('usernameBuffer').setText('');
+      wrapper.find('GitTabView').prop('emailBuffer').setText('');
+
+      await wrapper.find('GitTabView').prop('setLocalIdentity')();
+
+      assert.isTrue(unsetConfig.calledWith('user.name'));
+      assert.isTrue(unsetConfig.calledWith('user.email'));
+    });
   });
 
   describe('abortMerge()', function() {

test/fixtures/props/git-tab-props.js

@@ -101,6 +101,8 @@ export async function gitTabViewProps(atomEnv, repository, overrides = {}) {
 
     toggleIdentityEditor: () => {},
     closeIdentityEditor: () => {},
+    setLocalIdentity: () => {},
+    setGlobalIdentity: () => {},
     openInitializeDialog: () => {},
     abortMerge: () => {},
     commit: () => {},

test/views/git-identity-view.test.js

@@ -10,6 +10,9 @@ describe('GitIdentityView', function() {
       <GitIdentityView
         usernameBuffer={new TextBuffer()}
         emailBuffer={new TextBuffer()}
+        canWriteLocal={true}
+        setLocal={() => {}}
+        setGlobal={() => {}}
         close={() => {}}
         {...override}
       />
@@ -30,27 +33,37 @@ describe('GitIdentityView', function() {
     assert.strictEqual(getEditor('email address').prop('buffer'), emailBuffer);
   });
 
-  it('disables the "Continue" button if the name is blank', function() {
-    const usernameBuffer = new TextBuffer();
-    const wrapper = mount(buildApp({usernameBuffer}));
+  it('disables the local repo button when canWriteLocal is false', function() {
+    const wrapper = mount(buildApp({canWriteLocal: false}));
 
-    assert.isTrue(wrapper.find('.btn').prop('disabled'));
+    assert.isTrue(wrapper.find('.btn').filterWhere(each => /this repository/.test(each.text())).prop('disabled'));
   });
 
-  it('disables the "Continue" button if the email is blank', function() {
-    const emailBuffer = new TextBuffer();
-    const wrapper = mount(buildApp({emailBuffer}));
+  it('triggers a callback when "Use for this repository" is clicked', function() {
+    const setLocal = sinon.spy();
+    const wrapper = mount(buildApp({setLocal}));
+
+    wrapper.find('.btn').filterWhere(each => /this repository/.test(each.text())).simulate('click');
+
+    assert.isTrue(setLocal.called);
+  });
+
+  it('triggers a callback when "Use for all repositories" is clicked', function() {
+    const setGlobal = sinon.spy();
+    const wrapper = mount(buildApp({setGlobal}));
+
+    wrapper.find('.btn').filterWhere(each => /all repositories/.test(each.text())).simulate('click');
 
-    assert.isTrue(wrapper.find('.btn').prop('disabled'));
+    assert.isTrue(setGlobal.called);
   });
 
-  it('triggers a callback when "Continue" is clicked', function() {
+  it('triggers a callback when "Cancel" is clicked', function() {
     const usernameBuffer = new TextBuffer({text: 'Me'});
     const emailBuffer = new TextBuffer({text: 'me@email.com'});
     const close = sinon.spy();
     const wrapper = mount(buildApp({usernameBuffer, emailBuffer, close}));
 
-    wrapper.find('.btn').simulate('click');
+    wrapper.find('.btn').filterWhere(each => /Cancel/.test(each.text())).simulate('click');
 
     assert.isTrue(close.called);
   });