gr-request/gr-ajax cleanup (gr-diff-comment) Bug: Issue 3988 Change-Id: I0c100a0b77a256c1eb34f94e8fff0515add5dea1
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html index 82b0faf..9462408 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment-thread/gr-diff-comment-thread_test.html
@@ -124,11 +124,24 @@ suite('comment action tests', function() { var element; - var server; setup(function() { stub('gr-rest-api-interface', { getLoggedIn: function() { return Promise.resolve(false); }, + saveDiffDraft: function() { return Promise.resolve({ + ok: true, + text: function() { return Promise.resolve(')]}\'\n' + + JSON.stringify({ + id: '7afa4931_de3d65bd', + path: '/path/to/file.txt', + line: 5, + in_reply_to: 'baf0414d_60047215', + updated: '2015-12-21 02:01:10.850000000', + message: 'Done' + })); + }, + })}, + deleteDiffDraft: function() { return Promise.resolve({ok: true}); }, }); element = fixture('withComment'); element.comments = [{ @@ -142,35 +155,6 @@ updated: '2015-12-08 19:48:33.843000000', }]; flushAsynchronousOperations(); - - server = sinon.fakeServer.create(); - // Eat any requests made by elements in this suite. - server.respondWith( - 'PUT', - '/changes/41/1/drafts', - [ - 201, - {'Content-Type': 'application/json'}, - ')]}\'\n' + JSON.stringify({ - id: '7afa4931_de3d65bd', - path: '/path/to/file.txt', - line: 5, - in_reply_to: 'baf0414d_60047215', - updated: '2015-12-21 02:01:10.850000000', - message: 'Done' - }), - ] - ); - - server.respondWith( - 'DELETE', - '/changes/41/1/drafts/baf0414d_60047215', - [ - 204, - {}, - '', - ] - ); }); test('reply', function(done) { @@ -210,7 +194,6 @@ var commentEl = element.$$('gr-diff-comment'); assert.ok(commentEl); commentEl.addEventListener('done', function() { - server.respond(); var drafts = element._orderedComments.filter(function(c) { return c.__draft == true; }); @@ -236,7 +219,6 @@ Polymer.dom(element.root).querySelectorAll('gr-diff-comment')[1]; assert.ok(draftEl); draftEl.addEventListener('comment-discard', function() { - server.respond(); var drafts = element.comments.filter(function(c) { return c.__draft == true; });
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html index a6a8c4d..fd35ad7 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -19,7 +19,7 @@ <link rel="import" href="../../shared/gr-button/gr-button.html"> <link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html"> <link rel="import" href="../../shared/gr-linked-text/gr-linked-text.html"> -<link rel="import" href="../../shared/gr-request/gr-request.html"> +<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html"> <dom-module id="gr-diff-comment"> <template> @@ -147,6 +147,7 @@ </div> </div> </div> + <gr-rest-api-interface id="restAPI"></gr-rest-api-interface> </template> <script src="gr-diff-comment.js"></script> </dom-module>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js index 81449b8..b39295a 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.js
@@ -72,22 +72,33 @@ save: function() { this.comment.message = this._editDraft; this.disabled = true; - var endpoint = this._restEndpoint(this.comment.id); - this._send('PUT', endpoint).then(function(req) { + this._xhrPromise = this._saveDraft(this.comment).then(function(response) { this.disabled = false; - var comment = req.response; - comment.__draft = true; - // Maintain the ephemeral draft ID for identification by other - // elements. - if (this.comment.__draftID) { - comment.__draftID = this.comment.__draftID; + if (!response.ok) { + alert('Your draft couldn’t be saved. Check the console and contact ' + + 'the PolyGerrit team for assistance.'); + return response.text().then(function(text) { + console.error(text); + }); } - this.comment = comment; - this.editing = false; + return this.$.restAPI.getResponseObject(response).then(function(obj) { + var comment = obj; + comment.__draft = true; + // Maintain the ephemeral draft ID for identification by other + // elements. + if (this.comment.__draftID) { + comment.__draftID = this.comment.__draftID; + } + this.comment = comment; + this.editing = false; + + return obj; + }.bind(this)); }.bind(this)).catch(function(err) { alert('Your draft couldn’t be saved. Check the console and contact ' + 'the PolyGerrit team for assistance.'); this.disabled = false; + console.error(err.message); }.bind(this)); }, @@ -179,18 +190,27 @@ throw Error('Cannot discard a non-draft comment.'); } this.disabled = true; - var commentID = this.comment.id; - if (!commentID) { + if (!this.comment.id) { this.fire('comment-discard'); return; } - this._send('DELETE', this._restEndpoint(commentID)).then(function(req) { + + this._xhrPromise = + this._deleteDraft(this.comment).then(function(response) { + this.disabled = false; + if (!response.ok) { + alert('Your draft couldn’t be deleted. Check the console and ' + + 'contact the PolyGerrit team for assistance.'); + return response.text().then(function(text) { + console.error(text); + }); + } this.fire('comment-discard'); }.bind(this)).catch(function(err) { - alert('Your draft couldn’t be deleted. Check the console and ' + - 'contact the PolyGerrit team for assistance.'); + alert('Your draft couldn’t be deleted. Check the console and contact ' + + 'the PolyGerrit team for assistance.'); this.disabled = false; - }.bind(this)); + }.bind(this));; }, _preventDefaultAndBlur: function(e) { @@ -198,26 +218,13 @@ Polymer.dom(e).rootTarget.blur(); }, - _send: function(method, url) { - var xhr = document.createElement('gr-request'); - var opts = { - method: method, - url: url, - }; - if (method == 'PUT' || method == 'POST') { - opts.body = this.comment; - } - this._xhrPromise = xhr.send(opts); - return this._xhrPromise; + _saveDraft: function(draft) { + return this.$.restAPI.saveDiffDraft(this.changeNum, this.patchNum, draft); }, - _restEndpoint: function(id) { - var path = '/changes/' + this.changeNum + '/revisions/' + - this.patchNum + '/drafts'; - if (id) { - path += '/' + id; - } - return path; + _deleteDraft: function(draft) { + return this.$.restAPI.deleteDiffDraft(this.changeNum, this.patchNum, + draft); }, }); })();
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html index c95a9a4..a333e14 100644 --- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html +++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment_test.html
@@ -42,6 +42,9 @@ suite('gr-diff-comment tests', function() { var element; setup(function() { + stub('gr-rest-api-interface', { + getAccount: function() { return Promise.resolve(null); }, + }); element = fixture('basic'); element.comment = { author: { @@ -93,9 +96,30 @@ suite('gr-diff-comment draft tests', function() { var element; - var server; setup(function() { + stub('gr-rest-api-interface', { + getAccount: function() { return Promise.resolve(null); }, + saveDiffDraft: function() { + return Promise.resolve({ + ok: true, + text: function() { + return Promise.resolve( + ')]}\'\n{' + + '"id": "baf0414d_40572e03",' + + '"path": "/path/to/file",' + + '"line": 5,' + + '"updated": "2015-12-08 21:52:36.177000000",' + + '"message": "saved!"' + + '}' + ); + }, + }); + }, + removeChangeReviewer: function() { + return Promise.resolve({ok: true}); + }, + }); element = fixture('draft'); element.changeNum = 42; element.patchNum = 1; @@ -106,43 +130,6 @@ path: '/path/to/file', line: 5, }; - - server = sinon.fakeServer.create(); - server.respondWith( - 'PUT', - '/changes/42/revisions/1/drafts', - [ - 201, - {'Content-Type': 'application/json'}, - ')]}\'\n{' + - '"id": "baf0414d_40572e03",' + - '"path": "/path/to/file",' + - '"line": 5,' + - '"updated": "2015-12-08 21:52:36.177000000",' + - '"message": "created!"' + - '}' - ] - ); - - server.respondWith( - 'PUT', - /\/changes\/42\/revisions\/1\/drafts\/.+/, - [ - 200, - {'Content-Type': 'application/json'}, - ')]}\'\n{' + - '"id": "baf0414d_40572e03",' + - '"path": "/path/to/file",' + - '"line": 5,' + - '"updated": "2015-12-08 21:52:36.177000000",' + - '"message": "saved!"' + - '}' - ] - ); - }); - - teardown(function() { - server.restore(); }); function isVisible(el) { @@ -224,14 +211,10 @@ assert.isTrue(element.disabled, 'Element should be disabled when creating draft.'); - server.respond(); - - element._xhrPromise.then(function(req) { + element._xhrPromise.then(function(draft) { assert.isFalse(element.disabled, 'Element should be enabled when done creating draft.'); - assert.equal(req.status, 201); - assert.equal(req.url, '/changes/42/revisions/1/drafts'); - assert.equal(req.response.message, 'created!'); + assert.equal(draft.message, 'saved!'); assert.isFalse(element.editing); }).then(function() { MockInteractions.tap(element.$$('.edit')); @@ -240,15 +223,11 @@ MockInteractions.tap(element.$$('.save')); assert.isTrue(element.disabled, 'Element should be disabled when updating draft.'); - server.respond(); - element._xhrPromise.then(function(req) { + element._xhrPromise.then(function(draft) { assert.isFalse(element.disabled, 'Element should be enabled when done updating draft.'); - assert.equal(req.status, 200); - assert.equal(req.url, - '/changes/42/revisions/1/drafts/baf0414d_40572e03'); - assert.equal(req.response.message, 'saved!'); + assert.equal(draft.message, 'saved!'); assert.isFalse(element.editing); done(); });
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js index 0765ec8..252c321 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface.js
@@ -465,6 +465,31 @@ return this._changeBaseURL(changeNum, opt_patchNum) + endpoint; }, + saveDiffDraft: function(changeNum, patchNum, draft) { + return this._sendDiffDraftRequest('PUT', changeNum, patchNum, draft); + }, + + deleteDiffDraft: function(changeNum, patchNum, draft) { + return this._sendDiffDraftRequest('DELETE', changeNum, patchNum, draft); + }, + + _sendDiffDraftRequest: function(method, changeNum, patchNum, draft) { + var url = this.getChangeActionURL(changeNum, patchNum, '/drafts'); + var body; + switch(method) { + case 'PUT': + body = draft; + break; + case 'DELETE': + url += '/' + draft.id; + break; + default: + throw Error('Unsupported HTTP method: ' + method); + } + + return this.send(method, url, body); + }, + _changeBaseURL: function(changeNum, opt_patchNum) { var v = '/changes/' + changeNum; if (opt_patchNum) {