diff --git a/jupyterlab_git/handlers.py b/jupyterlab_git/handlers.py index 987028e1a..977817df9 100644 --- a/jupyterlab_git/handlers.py +++ b/jupyterlab_git/handlers.py @@ -514,6 +514,23 @@ async def post(self): self.finish(json.dumps(response)) +class GitCommitterIdentityHandler(GitHandler): + """ + Handler for checking if user identity is set + """ + + @web.authenticated + async def get(self): + identity_established = False + top_repo_path = self.get_argument("path") + response = await self.git.config(top_repo_path) + if "user.name" in response["options"] and "user.email" in response["options"]: + identity_established = True + if "GIT_COMMITTER_NAME" in os.environ and "GIT_COMMITTER_EMAIL" in os.environ or "EMAIL" in os.environ: + identity_established = True + self.finish(json.dumps({"identityEstablished": identity_established})) + + class GitDiffContentHandler(GitHandler): """ Handler for plain text diffs. Uses git show $REF:$FILE @@ -576,6 +593,7 @@ def setup_handlers(web_app): ("/git/show_top_level", GitShowTopLevelHandler), ("/git/status", GitStatusHandler), ("/git/upstream", GitUpstreamHandler), + ("/git/committer_identity", GitCommitterIdentityHandler), ] # add the baseurl to our paths diff --git a/jupyterlab_git/tests/test_handlers.py b/jupyterlab_git/tests/test_handlers.py index 648fd9a39..03c7f114b 100644 --- a/jupyterlab_git/tests/test_handlers.py +++ b/jupyterlab_git/tests/test_handlers.py @@ -11,6 +11,7 @@ GitLogHandler, GitPushHandler, GitUpstreamHandler, + GitCommitterIdentityHandler, setup_handlers, ) @@ -62,6 +63,71 @@ def test_all_history_handler_localbranch(self, mock_git): } +class TestCommitterIdentityHandler(ServerTest): + @patch("jupyterlab_git.handlers.GitCommitterIdentityHandler.git", spec=Git) + def test_name_and_email_from_config(self, mock_git): + config_options = {"code": 0, "options": {"user.name": "foo", "user.email": "bar"}} + mock_git.config.return_value = maybe_future(config_options) + + response = self.tester.get(["/committer_identity?path=/path/to/repo"]) + payload = response.json() + assert payload == {"identityEstablished": True} + + @patch("jupyterlab_git.handlers.GitCommitterIdentityHandler.git", spec=Git) + def test_name_missing_from_config(self, mock_git): + config_options = {"code": 0, "options": {"user.email": "bar"}} + mock_git.config.return_value = maybe_future(config_options) + + response = self.tester.get(["/committer_identity?path=/path/to/repo"]) + payload = response.json() + assert payload == {"identityEstablished": False} + + @patch("jupyterlab_git.handlers.GitCommitterIdentityHandler.git", spec=Git) + def test_email_missing_from_config(self, mock_git): + config_options = {"code": 0, "options": {"user.name": "bar"}} + mock_git.config.return_value = maybe_future(config_options) + + response = self.tester.get(["/committer_identity?path=/path/to/repo"]) + payload = response.json() + assert payload == {"identityEstablished": False} + + @patch("jupyterlab_git.handlers.GitCommitterIdentityHandler.git", spec=Git) + @patch.dict(os.environ, {"GIT_COMMITTER_NAME": "foo", "GIT_COMMITTER_EMAIL": "bar@xyz.com"}) + def test_committer_name_email_var(self, mock_git): + mock_git.config.return_value = maybe_future({"code": 0, "options": {}}) + + response = self.tester.get(["/committer_identity?path=/path/to/repo"]) + payload = response.json() + assert payload == {"identityEstablished": True} + + @patch("jupyterlab_git.handlers.GitCommitterIdentityHandler.git", spec=Git) + @patch.dict(os.environ, {"GIT_COMMITTER_NAME": "foo"}) + def test_missing_committer_email_var(self, mock_git): + mock_git.config.return_value = maybe_future({"code": 0, "options": {}}) + + response = self.tester.get(["/committer_identity?path=/path/to/repo"]) + payload = response.json() + assert payload == {"identityEstablished": False} + + @patch("jupyterlab_git.handlers.GitCommitterIdentityHandler.git", spec=Git) + @patch.dict(os.environ, {"GIT_COMMITTER_EMAIL": "bar@xyz.com"}) + def test_committer_name_var(self, mock_git): + mock_git.config.return_value = maybe_future({"code": 0, "options": {}}) + + response = self.tester.get(["/committer_identity?path=/path/to/repo"]) + payload = response.json() + assert payload == {"identityEstablished": False} + + @patch("jupyterlab_git.handlers.GitCommitterIdentityHandler.git", spec=Git) + @patch.dict(os.environ, {"EMAIL": "foo@bar.com"}) + def test_committer_name_email_var(self, mock_git): + mock_git.config.return_value = maybe_future({"code": 0, "options": {}}) + + response = self.tester.get(["/committer_identity?path=/path/to/repo"]) + payload = response.json() + assert payload == {"identityEstablished": True} + + class TestBranch(ServerTest): @patch("jupyterlab_git.handlers.GitBranchHandler.git", spec=Git) def test_branch_handler_localbranch(self, mock_git): diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx index 2b1dd9544..e83c2c8d8 100644 --- a/src/components/GitPanel.tsx +++ b/src/components/GitPanel.tsx @@ -3,7 +3,6 @@ import { PathExt } from '@jupyterlab/coreutils'; import { FileBrowserModel } from '@jupyterlab/filebrowser'; import { IRenderMimeRegistry } from '@jupyterlab/rendermime'; import { ISettingRegistry } from '@jupyterlab/settingregistry'; -import { JSONObject } from '@lumino/coreutils'; import Tab from '@material-ui/core/Tab'; import Tabs from '@material-ui/core/Tabs'; import * as React from 'react'; @@ -383,13 +382,13 @@ export class GitPanel extends React.Component< // If the repository path changes, check the user identity if (path !== this._previousRepoPath) { try { - let res = await this.props.model.config(); + let res = await this.props.model.isCommitterIdentitySet(); if (res.ok) { - const options: JSONObject = (await res.json()).options; - const keys = Object.keys(options); + const identityEstablished: boolean = (await res.json()) + .identityEstablished; // If the user name or e-mail is unknown, ask the user to set it - if (keys.indexOf('user.name') < 0 || keys.indexOf('user.email') < 0) { + if (!identityEstablished) { const result = await showDialog({ title: 'Who is committing?', body: new GitAuthorForm() diff --git a/src/model.ts b/src/model.ts index 5b7046141..07e5ab87e 100644 --- a/src/model.ts +++ b/src/model.ts @@ -571,6 +571,39 @@ export class GitExtension implements IGitExtension { } } + async isCommitterIdentitySet(): Promise { + await this.ready; + const path = this.pathRepository; + + if (path === null) { + return Promise.resolve( + new Response( + JSON.stringify({ + code: -1, + message: 'Not in a git repository.' + }) + ) + ); + } + + try { + const response = await httpGitRequest( + `/git/committer_identity?path=${path}`, + 'GET', + null + ); + + if (!response.ok) { + const jsonData = await response.json(); + throw new ServerConnection.ResponseError(response, jsonData.message); + } + + return response; + } catch (err) { + throw new ServerConnection.NetworkError(err); + } + } + /** * Make request to revert changes from selected commit * diff --git a/tests/test-components/GitPanel.spec.tsx b/tests/test-components/GitPanel.spec.tsx index b441e5209..64ef36ece 100644 --- a/tests/test-components/GitPanel.spec.tsx +++ b/tests/test-components/GitPanel.spec.tsx @@ -108,14 +108,11 @@ describe('GitPanel', () => { // Mock identity look up const identity = jest - .spyOn(GitModel.prototype, 'config') + .spyOn(GitModel.prototype, 'isCommitterIdentitySet') .mockResolvedValue( new Response( JSON.stringify({ - options: { - 'user.name': 'John Snow', - 'user.email': 'john.snow@winteris.com' - } + identityEstablished: true }), { status: 201 } ) @@ -136,81 +133,29 @@ describe('GitPanel', () => { spy.mockRestore(); }); - it('should prompt for user identity if user.name is not set', async () => { + it('should prompt for user identity if not set', async () => { const spy = jest.spyOn(GitModel.prototype, 'commit'); - // Mock identity look up - const identity = jest + const config = jest .spyOn(GitModel.prototype, 'config') .mockImplementation(options => { let response: Response = null; - if (options === undefined) { - response = new Response( - JSON.stringify({ - options: { - 'user.email': 'john.snow@winteris.com' - } - }), - { status: 201 } - ); - } else { - response = new Response('', { status: 201 }); - } + response = new Response('', { status: 201 }); return Promise.resolve(response); }); - const mock = apputils as jest.Mocked; - mock.showDialog.mockResolvedValue({ - button: { - accept: true, - caption: '', - className: '', - displayType: 'default', - iconClass: '', - iconLabel: '', - label: '' - }, - value: { - name: 'John Snow', - email: 'john.snow@winteris.com' - } - }); - - const panel = new GitPanel(props); - await panel.commitStagedFiles('Initial commit'); - expect(identity).toHaveBeenCalledTimes(2); - expect(identity.mock.calls[0]).toHaveLength(0); - expect(identity.mock.calls[1]).toEqual([ - { - 'user.name': 'John Snow', - 'user.email': 'john.snow@winteris.com' - } - ]); - expect(spy).toHaveBeenCalledTimes(1); - expect(spy).toHaveBeenCalledWith('Initial commit'); - }); - - it('should prompt for user identity if user.email is not set', async () => { - const spy = jest.spyOn(GitModel.prototype, 'commit'); // Mock identity look up const identity = jest - .spyOn(GitModel.prototype, 'config') - .mockImplementation(options => { - let response: Response = null; - if (options === undefined) { - response = new Response( - JSON.stringify({ - options: { - 'user.name': 'John Snow' - } - }), - { status: 201 } - ); - } else { - response = new Response('', { status: 201 }); - } - return Promise.resolve(response); - }); + .spyOn(GitModel.prototype, 'isCommitterIdentitySet') + .mockResolvedValue( + new Response( + JSON.stringify({ + identityEstablished: false + }), + { status: 201 } + ) + ); + const mock = apputils as jest.Mocked; mock.showDialog.mockResolvedValue({ button: { @@ -230,9 +175,9 @@ describe('GitPanel', () => { const panel = new GitPanel(props); await panel.commitStagedFiles('Initial commit'); - expect(identity).toHaveBeenCalledTimes(2); - expect(identity.mock.calls[0]).toHaveLength(0); - expect(identity.mock.calls[1]).toEqual([ + expect(identity).toHaveBeenCalledTimes(1); + expect(config).toHaveBeenCalledTimes(1); + expect(config.mock.calls[0]).toEqual([ { 'user.name': 'John Snow', 'user.email': 'john.snow@winteris.com' @@ -247,21 +192,16 @@ describe('GitPanel', () => { // Mock identity look up const identity = jest - .spyOn(GitModel.prototype, 'config') - .mockImplementation(options => { - let response: Response = null; - if (options === undefined) { - response = new Response( - JSON.stringify({ - options: {} - }), - { status: 201 } - ); - } else { - response = new Response('', { status: 201 }); - } - return Promise.resolve(response); - }); + .spyOn(GitModel.prototype, 'isCommitterIdentitySet') + .mockResolvedValue( + new Response( + JSON.stringify({ + identityEstablished: false + }), + { status: 201 } + ) + ); + const mock = apputils as jest.Mocked; mock.showDialog.mockResolvedValue({ button: { @@ -279,7 +219,6 @@ describe('GitPanel', () => { const panel = new GitPanel(props); await panel.commitStagedFiles('Initial commit'); expect(identity).toHaveBeenCalledTimes(1); - expect(identity).toHaveBeenCalledWith(); expect(spy).not.toHaveBeenCalled(); }); });