diff --git a/jupyterlab_git/git.py b/jupyterlab_git/git.py index 095dd3c9c..2beb69af9 100644 --- a/jupyterlab_git/git.py +++ b/jupyterlab_git/git.py @@ -792,16 +792,27 @@ async def checkout_all(self, top_repo_path): return {"code": code, "command": " ".join(cmd), "message": error} return {"code": code} + async def reset_author(self, top_repo_path): + """ + Reset committer identity in previous commit + """ + cmd = ["git", "commit", "--amend", "--reset-author", "--no-edit"] + code, _, error = await execute(cmd, cwd=top_repo_path) + + if code != 0: + return {"code": code, "command": " ".join(cmd), "message": error} + return {"code": code} + async def commit(self, commit_msg, top_repo_path): """ Execute git commit command & return the result. """ cmd = ["git", "commit", "-m", commit_msg] - code, _, error = await execute(cmd, cwd=top_repo_path) + code, output, error = await execute(cmd, cwd=top_repo_path) if code != 0: return {"code": code, "command": " ".join(cmd), "message": error} - return {"code": code} + return {"code": code, "output": output} async def pull(self, curr_fb_path, auth=None, cancel_on_conflict=False): """ diff --git a/jupyterlab_git/handlers.py b/jupyterlab_git/handlers.py index 8f3472379..644570010 100644 --- a/jupyterlab_git/handlers.py +++ b/jupyterlab_git/handlers.py @@ -375,6 +375,22 @@ async def post(self): self.finish(json.dumps(body)) +class GitResetAuthorHandler(GitHandler): + """ + Handler for 'git commit --amend --reset-author --no-edit'. + """ + + @web.authenticated + async def post(self): + data = self.get_json_body() + top_repo_path = data["top_repo_path"] + body = await self.git.reset_author(top_repo_path) + + if body["code"] != 0: + self.set_status(500) + self.finish(json.dumps(body)) + + class GitCommitHandler(GitHandler): """ Handler for 'git commit -m '. Commits files. @@ -711,6 +727,7 @@ def setup_handlers(web_app): ("/git/changed_files", GitChangedFilesHandler), ("/git/checkout", GitCheckoutHandler), ("/git/clone", GitCloneHandler), + ("/git/reset_author", GitResetAuthorHandler), ("/git/commit", GitCommitHandler), ("/git/config", GitConfigHandler), ("/git/delete_commit", GitDeleteCommitHandler), diff --git a/src/components/GitPanel.tsx b/src/components/GitPanel.tsx index e99173178..8d2c350be 100644 --- a/src/components/GitPanel.tsx +++ b/src/components/GitPanel.tsx @@ -6,7 +6,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 { GitExtension } from '../model'; import { sleep } from '../utils'; import { Git, ILogMessage } from '../tokens'; @@ -28,6 +27,9 @@ import { SuspendModal } from './SuspendModal'; import { Alert } from './Alert'; import { CommandIDs } from '../commandsAndMenu'; +const MISSING_IDENTITY = + 'Your name and email address were configured automatically'; + /** * Interface describing component properties. */ @@ -227,35 +229,20 @@ export class GitPanel extends React.Component { * @returns a promise which commits the files */ commitStagedFiles = async (message: string): Promise => { - let res: boolean; + let res: Response; if (!message) { return; } - try { - res = await this._hasIdentity(this.props.model.pathRepository); - } catch (err) { - this._log({ - severity: 'error', - message: 'Failed to commit changes.' - }); - console.error(err); - showErrorMessage('Fail to commit', err); - return; - } - if (!res) { - this._log({ - severity: 'error', - message: 'Failed to commit changes.' - }); - return; - } this._log({ severity: 'info', message: 'Committing changes...' }); this._suspend(true); try { - await Promise.all([sleep(1000), this.props.model.commit(message)]); + [, res] = await Promise.all([ + sleep(1000), + this.props.model.commit(message) + ]); } catch (err) { this._suspend(false); this._log({ @@ -271,6 +258,10 @@ export class GitPanel extends React.Component { severity: 'success', message: 'Committed changes.' }); + const { output } = await res.json(); + if (output.indexOf(MISSING_IDENTITY) !== -1) { + await this._setIdentity(this.props.model.pathRepository); + } }; /** @@ -570,37 +561,36 @@ export class GitPanel extends React.Component { * @param path - repository path * @returns a promise which returns a success status */ - private async _hasIdentity(path: string): Promise { + private async _setIdentity(path: string): Promise { // If the repository path changes, check the user identity if (path !== this._previousRepoPath) { try { - let res = await this.props.model.config(); - if (res.ok) { - const options: JSONObject = (await res.json()).options; - const keys = Object.keys(options); - - // 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) { - const result = await showDialog({ - title: 'Who is committing?', - body: new GitAuthorForm() - }); - if (!result.button.accept) { - console.log('User refuses to set identity.'); - return false; - } - const identity = result.value; - res = await this.props.model.config({ - 'user.name': identity.name, - 'user.email': identity.email - }); - if (!res.ok) { - console.log(await res.text()); - return false; - } - } - this._previousRepoPath = path; + const result = await showDialog({ + title: 'Who is committing?', + body: new GitAuthorForm() + }); + if (!result.button.accept) { + console.log('User refuses to set identity.'); + return false; + } + const identity = result.value; + let res = await this.props.model.config({ + 'user.name': identity.name, + 'user.email': identity.email + }); + if (!res.ok) { + console.log(await res.text()); + return false; + } + this._suspend(true); + res = await this.props.model.resetAuthor(); + if (!res.ok) { + this._suspend(false); + console.log(await res.text()); + return false; } + this._suspend(false); + this._previousRepoPath = path; } catch (error) { throw new Error('Failed to set your identity. ' + error.message); } diff --git a/src/model.ts b/src/model.ts index c09e0223f..8d66f5ae9 100644 --- a/src/model.ts +++ b/src/model.ts @@ -529,6 +529,40 @@ export class GitExtension implements IGitExtension { return data; } + async resetAuthor(): 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/reset_author', 'POST', { + top_repo_path: path + }); + if (!response.ok) { + return response.json().then((data: any) => { + throw new ServerConnection.ResponseError(response, data.message); + }); + } + + this.refreshStatus(); + this._headChanged.emit(); + + return response; + } catch (err) { + throw new ServerConnection.NetworkError(err); + } + } + /** * Commit all staged file changes. * diff --git a/tests/test-components/GitPanel.spec.tsx b/tests/test-components/GitPanel.spec.tsx index 66bb4dfb4..a498f476e 100644 --- a/tests/test-components/GitPanel.spec.tsx +++ b/tests/test-components/GitPanel.spec.tsx @@ -43,6 +43,9 @@ function MockRequest(url: string, method: string, request: any) { }; response = new Response(JSON.stringify(obj)); break; + case '/git/reset_author': + response = new Response(); + break; default: obj = { message: `No mock implementation for ${url}.` @@ -98,26 +101,13 @@ describe('GitPanel', () => { }); it('should commit when commit message is provided', async () => { - const spy = jest.spyOn(GitModel.prototype, 'commit'); - - // Mock identity look up - const identity = jest - .spyOn(GitModel.prototype, 'config') + const spy = jest.spyOn(GitModel.prototype, 'commit') .mockResolvedValue( - new Response( - JSON.stringify({ - options: { - 'user.name': 'John Snow', - 'user.email': 'john.snow@winteris.com' - } - }), - { status: 201 } - ) - ); + new Response(JSON.stringify({code: 200, output: ""}) + )); const panel = new GitPanel(props); await panel.commitStagedFiles('Initial commit'); - expect(identity).toHaveBeenCalledTimes(1); expect(spy).toHaveBeenCalledTimes(1); expect(spy).toHaveBeenCalledWith('Initial commit'); }); @@ -130,61 +120,23 @@ describe('GitPanel', () => { spy.mockRestore(); }); - it('should prompt for user identity if user.name 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.email': 'john.snow@winteris.com' - } - }), - { status: 201 } - ); - } else { - 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'); + it('should prompt for user identity when git advices to set committer identity', async () => { + const spy = jest.spyOn(GitModel.prototype, 'commit') + .mockResolvedValue( + new Response( + JSON.stringify( + { + code: 200, + output: `Your name and email address were configured automatically based + on your username and hostname. Please check that they are accurate. + You can suppress this message by setting them explicitly: + + git config --global user.name "Your Name" + git config --global user.email you@example.com` + } + ) + ) + ); // Mock identity look up const identity = jest @@ -222,11 +174,12 @@ describe('GitPanel', () => { } }); + const resetAuthorMock = jest.spyOn(GitModel.prototype, 'resetAuthor'); + 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(identity.mock.calls[0]).toEqual([ { 'user.name': 'John Snow', 'user.email': 'john.snow@winteris.com' @@ -234,28 +187,31 @@ describe('GitPanel', () => { ]); expect(spy).toHaveBeenCalledTimes(1); expect(spy).toHaveBeenCalledWith('Initial commit'); + expect(resetAuthorMock).toHaveBeenCalledTimes(1); }); - it('should not commit if no user identity is set and the user rejects the dialog', async () => { - const spy = jest.spyOn(GitModel.prototype, 'commit'); + it('should not reset author if no user identity is set and the user rejects the dialog', async () => { + const spy = jest.spyOn(GitModel.prototype, 'commit') + .mockResolvedValue( + new Response( + JSON.stringify( + { + code: 200, + output: `Your name and email address were configured automatically based + on your username and hostname. Please check that they are accurate. + You can suppress this message by setting them explicitly: + + git config --global user.name "Your Name" + git config --global user.email you@example.com` + } + ) + ) + ); // 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); - }); + const identity = jest.spyOn(GitModel.prototype, 'config') + const resetAuthorMock = jest.spyOn(GitModel.prototype, 'resetAuthor'); + const mock = apputils as jest.Mocked; mock.showDialog.mockResolvedValue({ button: { @@ -272,9 +228,9 @@ describe('GitPanel', () => { const panel = new GitPanel(props); await panel.commitStagedFiles('Initial commit'); - expect(identity).toHaveBeenCalledTimes(1); - expect(identity).toHaveBeenCalledWith(); - expect(spy).not.toHaveBeenCalled(); + expect(spy).toHaveBeenCalledTimes(1); + expect(identity).toHaveBeenCalledTimes(0); + expect(resetAuthorMock).toHaveBeenCalledTimes(0); }); }); });