Mark getChangeUrlAndSend private The getChangeUrlAndSend method of the REST API interface is used primarily as an internal utility function. However, it could not be marked as private because it's used outside the interface by the change actions handler. Add a new, public method for executing actions and mark the getChangeUrlAndSend method as private. Change-Id: I4dab166b29f0f119b6f41789cfffabf0709ef0ea
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js index d9da343..910b7fc 100644 --- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js +++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions.js
@@ -1192,8 +1192,8 @@ return Promise.resolve(); } const patchNum = revisionAction ? this.latestPatchNum : null; - return this.$.restAPI.getChangeURLAndSend(this.changeNum, method, - patchNum, actionEndpoint, payload, handleError) + return this.$.restAPI.executeChangeAction(this.changeNum, method, + actionEndpoint, patchNum, payload, handleError) .then(response => { cleanupFn.call(this); return response;
diff --git a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html index 1c841f1..67d28f4 100644 --- a/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html +++ b/polygerrit-ui/app/elements/change/gr-change-actions/gr-change-actions_test.html
@@ -1364,7 +1364,7 @@ setup(() => { sandbox.stub(element, 'fetchChangeUpdates') .returns(Promise.resolve({isLatest: true})); - sendStub = sandbox.stub(element.$.restAPI, 'getChangeURLAndSend') + sendStub = sandbox.stub(element.$.restAPI, 'executeChangeAction') .returns(Promise.resolve({})); }); @@ -1373,8 +1373,8 @@ .then(() => { assert.isFalse(onShowAlert.called); assert.isTrue(cleanup.calledOnce); - assert.isTrue(sendStub.calledWith(42, 'DELETE', null, - '/endpoint', payload)); + assert.isTrue(sendStub.calledWith(42, 'DELETE', '/endpoint', + null, payload)); }); }); @@ -1383,8 +1383,8 @@ .then(() => { assert.isFalse(onShowAlert.called); assert.isTrue(cleanup.calledOnce); - assert.isTrue(sendStub.calledWith(42, 'DELETE', 12, '/endpoint', - payload)); + assert.isTrue(sendStub.calledWith(42, 'DELETE', '/endpoint', + 12, payload)); }); }); }); @@ -1394,7 +1394,7 @@ sandbox.stub(element, 'fetchChangeUpdates') .returns(Promise.resolve({isLatest: false})); const sendStub = sandbox.stub(element.$.restAPI, - 'getChangeURLAndSend'); + 'executeChangeAction'); return element._send('DELETE', payload, '/endpoint', true, cleanup) .then(() => { @@ -1408,7 +1408,7 @@ sandbox.stub(element, 'fetchChangeUpdates') .returns(Promise.resolve({isLatest: true})); const sendStub = sandbox.stub(element.$.restAPI, - 'getChangeURLAndSend', + 'executeChangeAction', (num, method, patchNum, endpoint, payload, onErr) => { onErr(); return Promise.resolve(null);
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 cf680d2..4fd47f1 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
@@ -1593,7 +1593,7 @@ saveFileReviewed(changeNum, patchNum, path, reviewed, opt_errFn) { const method = reviewed ? 'PUT' : 'DELETE'; const endpoint = `/files/${encodeURIComponent(path)}/reviewed`; - return this.getChangeURLAndSend(changeNum, method, patchNum, endpoint, + return this._getChangeURLAndSend(changeNum, method, patchNum, endpoint, null, opt_errFn); }, @@ -1697,7 +1697,7 @@ _getFileInRevision(changeNum, path, patchNum, opt_errFn) { const e = `/files/${encodeURIComponent(path)}/content`; const headers = {Accept: 'application/json'}; - return this.getChangeURLAndSend(changeNum, 'GET', patchNum, e, null, + return this._getChangeURLAndSend(changeNum, 'GET', patchNum, e, null, opt_errFn, null, headers); }, @@ -1709,54 +1709,54 @@ _getFileInChangeEdit(changeNum, path) { const e = '/edit/' + encodeURIComponent(path); const headers = {Accept: 'application/json'}; - return this.getChangeURLAndSend(changeNum, 'GET', null, e, null, null, + return this._getChangeURLAndSend(changeNum, 'GET', null, e, null, null, null, headers); }, rebaseChangeEdit(changeNum) { - return this.getChangeURLAndSend(changeNum, 'POST', null, '/edit:rebase'); + return this._getChangeURLAndSend(changeNum, 'POST', null, '/edit:rebase'); }, deleteChangeEdit(changeNum) { - return this.getChangeURLAndSend(changeNum, 'DELETE', null, '/edit'); + return this._getChangeURLAndSend(changeNum, 'DELETE', null, '/edit'); }, restoreFileInChangeEdit(changeNum, restore_path) { const p = {restore_path}; - return this.getChangeURLAndSend(changeNum, 'POST', null, '/edit', p); + return this._getChangeURLAndSend(changeNum, 'POST', null, '/edit', p); }, renameFileInChangeEdit(changeNum, old_path, new_path) { const p = {old_path, new_path}; - return this.getChangeURLAndSend(changeNum, 'POST', null, '/edit', p); + return this._getChangeURLAndSend(changeNum, 'POST', null, '/edit', p); }, deleteFileInChangeEdit(changeNum, path) { const e = '/edit/' + encodeURIComponent(path); - return this.getChangeURLAndSend(changeNum, 'DELETE', null, e); + return this._getChangeURLAndSend(changeNum, 'DELETE', null, e); }, saveChangeEdit(changeNum, path, contents) { const e = '/edit/' + encodeURIComponent(path); - return this.getChangeURLAndSend(changeNum, 'PUT', null, e, contents, null, - 'text/plain'); + return this._getChangeURLAndSend(changeNum, 'PUT', null, e, contents, + null, 'text/plain'); }, // Deprecated, prefer to use putChangeCommitMessage instead. saveChangeCommitMessageEdit(changeNum, message) { const p = {message}; - return this.getChangeURLAndSend(changeNum, 'PUT', null, '/edit:message', + return this._getChangeURLAndSend(changeNum, 'PUT', null, '/edit:message', p); }, publishChangeEdit(changeNum) { - return this.getChangeURLAndSend(changeNum, 'POST', null, + return this._getChangeURLAndSend(changeNum, 'POST', null, '/edit:publish'); }, putChangeCommitMessage(changeNum, message) { const p = {message}; - return this.getChangeURLAndSend(changeNum, 'PUT', null, '/message', p); + return this._getChangeURLAndSend(changeNum, 'PUT', null, '/message', p); }, saveChangeStarred(changeNum, starred) { @@ -2059,7 +2059,7 @@ this._pendingRequests[Requests.SEND_DIFF_DRAFT] = []; } - const promise = this.getChangeURLAndSend(changeNum, method, patchNum, + const promise = this._getChangeURLAndSend(changeNum, method, patchNum, endpoint, body); this._pendingRequests[Requests.SEND_DIFF_DRAFT].push(promise); @@ -2174,7 +2174,7 @@ */ setChangeTopic(changeNum, topic) { const p = {topic}; - return this.getChangeURLAndSend(changeNum, 'PUT', null, '/topic', p) + return this._getChangeURLAndSend(changeNum, 'PUT', null, '/topic', p) .then(this.getResponseObject.bind(this)); }, @@ -2184,7 +2184,7 @@ * parameter. */ setChangeHashtag(changeNum, hashtag) { - return this.getChangeURLAndSend(changeNum, 'POST', null, '/hashtags', + return this._getChangeURLAndSend(changeNum, 'POST', null, '/hashtags', hashtag).then(this.getResponseObject.bind(this)); }, @@ -2268,12 +2268,12 @@ deleteVote(changeNum, account, label) { const e = `/reviewers/${account}/votes/${encodeURIComponent(label)}`; - return this.getChangeURLAndSend(changeNum, 'DELETE', null, e); + return this._getChangeURLAndSend(changeNum, 'DELETE', null, e); }, setDescription(changeNum, patchNum, desc) { const p = {description: desc}; - return this.getChangeURLAndSend(changeNum, 'PUT', patchNum, + return this._getChangeURLAndSend(changeNum, 'PUT', patchNum, '/description', p); }, @@ -2300,11 +2300,11 @@ setAssignee(changeNum, assignee) { const p = {assignee}; - return this.getChangeURLAndSend(changeNum, 'PUT', null, '/assignee', p); + return this._getChangeURLAndSend(changeNum, 'PUT', null, '/assignee', p); }, deleteAssignee(changeNum) { - return this.getChangeURLAndSend(changeNum, 'DELETE', null, '/assignee'); + return this._getChangeURLAndSend(changeNum, 'DELETE', null, '/assignee'); }, probePath(path) { @@ -2323,7 +2323,7 @@ if (opt_message) { payload.message = opt_message; } - return this.getChangeURLAndSend(changeNum, 'POST', null, '/wip', payload) + return this._getChangeURLAndSend(changeNum, 'POST', null, '/wip', payload) .then(response => { if (response.status === 204) { return 'Change marked as Work In Progress.'; @@ -2337,7 +2337,7 @@ * @param {function(?Response, string=)=} opt_errFn */ startReview(changeNum, opt_body, opt_errFn) { - return this.getChangeURLAndSend(changeNum, 'POST', null, '/ready', + return this._getChangeURLAndSend(changeNum, 'POST', null, '/ready', opt_body, opt_errFn); }, @@ -2349,7 +2349,7 @@ deleteComment(changeNum, patchNum, commentID, reason) { const endpoint = `/comments/${commentID}/delete`; const payload = {reason}; - return this.getChangeURLAndSend(changeNum, 'POST', patchNum, endpoint, + return this._getChangeURLAndSend(changeNum, 'POST', patchNum, endpoint, payload).then(this.getResponseObject.bind(this)); }, @@ -2422,7 +2422,7 @@ * @param {Object=} opt_headers * @return {!Promise<!Object>} */ - getChangeURLAndSend(changeNum, method, patchNum, endpoint, opt_payload, + _getChangeURLAndSend(changeNum, method, patchNum, endpoint, opt_payload, opt_errFn, opt_contentType, opt_headers) { return this._changeBaseURL(changeNum, patchNum).then(url => { return this._send({ @@ -2454,6 +2454,22 @@ }, /** + * Execute a change action or revision action on a change. + * @param {number} changeNum + * @param {string} method + * @param {string} endpoint + * @param {string|number|null|undefined} opt_patchNum + * @param {Object=} opt_payload + * @param {?function(?Response, string=)=} opt_errFn + * @return {Promise} + */ + executeChangeAction(changeNum, method, endpoint, opt_patchNum, opt_payload, + opt_errFn) { + return this._getChangeURLAndSend(changeNum, method, opt_patchNum || null, + endpoint, opt_payload, opt_errFn); + }, + + /** * Get blame information for the given diff. * @param {string|number} changeNum * @param {string|number} patchNum @@ -2517,7 +2533,7 @@ }, getMergeable(changeNum) { - return this.getChangeURLAndSend(changeNum, 'GET', null, + return this._getChangeURLAndSend(changeNum, 'GET', null, '/revisions/current/mergeable') .then(this.getResponseObject.bind(this)); },
diff --git a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html index 5ef4c41..86816f6 100644 --- a/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html +++ b/polygerrit-ui/app/elements/shared/gr-rest-api-interface/gr-rest-api-interface_test.html
@@ -725,7 +725,7 @@ suite('draft comments', () => { test('_sendDiffDraftRequest pending requests tracked', () => { const obj = element._pendingRequests; - sandbox.stub(element, 'getChangeURLAndSend', () => mockPromise()); + sandbox.stub(element, '_getChangeURLAndSend', () => mockPromise()); assert.notOk(element.hasPendingDiffDrafts()); element._sendDiffDraftRequest(null, null, null, {}); @@ -747,7 +747,7 @@ suite('_failForCreate200', () => { test('_sendDiffDraftRequest checks for 200 on create', () => { const sendPromise = Promise.resolve(); - sandbox.stub(element, 'getChangeURLAndSend').returns(sendPromise); + sandbox.stub(element, '_getChangeURLAndSend').returns(sendPromise); const failStub = sandbox.stub(element, '_failForCreate200') .returns(Promise.resolve()); return element._sendDiffDraftRequest('PUT', 123, 4, {}).then(() => { @@ -757,7 +757,7 @@ }); test('_sendDiffDraftRequest no checks for 200 on non create', () => { - sandbox.stub(element, 'getChangeURLAndSend') + sandbox.stub(element, '_getChangeURLAndSend') .returns(Promise.resolve()); const failStub = sandbox.stub(element, '_failForCreate200') .returns(Promise.resolve()); @@ -841,33 +841,33 @@ }); test('startWorkInProgress', () => { - sandbox.stub(element, 'getChangeURLAndSend') + sandbox.stub(element, '_getChangeURLAndSend') .returns(Promise.resolve('ok')); element.startWorkInProgress('42'); - assert.isTrue(element.getChangeURLAndSend.calledWith( + assert.isTrue(element._getChangeURLAndSend.calledWith( '42', 'POST', null, '/wip', {})); element.startWorkInProgress('42', 'revising...'); - assert.isTrue(element.getChangeURLAndSend.calledWith( + assert.isTrue(element._getChangeURLAndSend.calledWith( '42', 'POST', null, '/wip', {message: 'revising...'})); }); test('startReview', () => { - sandbox.stub(element, 'getChangeURLAndSend') + sandbox.stub(element, '_getChangeURLAndSend') .returns(Promise.resolve({})); element.startReview('42', {message: 'Please review.'}); - assert.isTrue(element.getChangeURLAndSend.calledWith( + assert.isTrue(element._getChangeURLAndSend.calledWith( '42', 'POST', null, '/ready', {message: 'Please review.'})); }); test('deleteComment', done => { - sandbox.stub(element, 'getChangeURLAndSend').returns(Promise.resolve()); + sandbox.stub(element, '_getChangeURLAndSend').returns(Promise.resolve()); sandbox.stub(element, 'getResponseObject').returns('some response'); element.deleteComment('foo', 'bar', '01234', 'removal reason') .then(response => { assert.equal(response, 'some response'); done(); }); - assert.isTrue(element.getChangeURLAndSend.calledWith( + assert.isTrue(element._getChangeURLAndSend.calledWith( 'foo', 'POST', 'bar', '/comments/01234/delete', {reason: 'removal reason'})); }); @@ -1129,11 +1129,11 @@ }); }); - test('getChangeURLAndSend', () => { + test('_getChangeURLAndSend', () => { element._projectLookup = {1: 'test'}; const sendStub = sandbox.stub(element, '_send') .returns(Promise.resolve()); - return element.getChangeURLAndSend(1, 'POST', 1, '/test').then(() => { + return element._getChangeURLAndSend(1, 'POST', 1, '/test').then(() => { assert.isTrue(sendStub.calledOnce); assert.equal(sendStub.lastCall.args[0].method, 'POST'); assert.equal(sendStub.lastCall.args[0].url, @@ -1161,7 +1161,7 @@ }); test('setChangeTopic', () => { - const sendSpy = sandbox.spy(element, 'getChangeURLAndSend'); + const sendSpy = sandbox.spy(element, '_getChangeURLAndSend'); return element.setChangeTopic(123, 'foo-bar').then(() => { assert.isTrue(sendSpy.calledOnce); assert.deepEqual(sendSpy.lastCall.args[4], {topic: 'foo-bar'}); @@ -1169,7 +1169,7 @@ }); test('setChangeHashtag', () => { - const sendSpy = sandbox.spy(element, 'getChangeURLAndSend'); + const sendSpy = sandbox.spy(element, '_getChangeURLAndSend'); return element.setChangeHashtag(123, 'foo-bar').then(() => { assert.isTrue(sendSpy.calledOnce); assert.equal(sendSpy.lastCall.args[4], 'foo-bar'); @@ -1271,7 +1271,7 @@ }); test('getFileContent', () => { - sandbox.stub(element, 'getChangeURLAndSend') + sandbox.stub(element, '_getChangeURLAndSend') .returns(Promise.resolve({ ok: 'true', headers: {