From bb7ae9653e9e6e911d2e53d313f54d477b4f17ea Mon Sep 17 00:00:00 2001 From: Kasper Nilsson Date: Tue, 12 Dec 2017 15:05:22 -0800 Subject: [PATCH] Fetch more info about edit files The type of a file in a change edit is identified in a header on the getFileInChangeEdit response. This change modifies gr-rest-api to return the bare response, and upgrades the editor view to parse both the header identifying the file type and the response body containing the file content. Bug: Issue 4437 Change-Id: Icdadeafb4189692cd369c74fe026442787173ef8 --- .../edit/gr-editor-view/gr-editor-view.html | 7 ++- .../edit/gr-editor-view/gr-editor-view.js | 17 ++++--- .../gr-editor-view/gr-editor-view_test.html | 44 ++++++++++++++++++- .../gr-rest-api-interface.js | 8 +++- .../gr-rest-api-interface_test.html | 21 +++++++++ 5 files changed, 83 insertions(+), 14 deletions(-) diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.html b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.html index c6cf7ceb4d..3042e7650b 100644 --- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.html +++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.html @@ -97,10 +97,9 @@ limitations under the License.
- - - - + + +
diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js index 52ce94395e..f31608d222 100644 --- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js +++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js @@ -36,6 +36,7 @@ _changeEditDetail: Object, _changeNum: String, _path: String, + _type: String, _content: String, _newContent: String, _saveDisabled: { @@ -81,11 +82,7 @@ const promises = []; promises.push(this._getChangeDetail(this._changeNum)); - promises.push(this._getFileContent(this._changeNum, this._path) - .then(fileContent => { - this._content = fileContent; - this._newContent = fileContent; - })); + promises.push(this._getFileData(this._changeNum, this._path)); return Promise.all(promises); }, @@ -109,8 +106,14 @@ Gerrit.Nav.navigateToChange(this._change, this.EDIT_NAME); }, - _getFileContent(changeNum, path) { - return this.$.restAPI.getFileInChangeEdit(changeNum, path); + _getFileData(changeNum, path) { + return this.$.restAPI.getFileInChangeEdit(changeNum, path).then(res => { + if (!res.ok) { return; } + + this._type = res.type; + this._newContent = res.content; + this._content = res.content; + }); }, _saveEdit() { diff --git a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html index b3bcb22645..05a247acaa 100644 --- a/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html +++ b/polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view_test.html @@ -67,8 +67,11 @@ suite('gr-editor-view tests', () => { test('good params proceed', () => { changeDetailStub.returns(Promise.resolve({})); - const fileStub = sandbox.stub(element, '_getFileContent') - .returns(Promise.resolve('text')); + const fileStub = sandbox.stub(element, '_getFileData', () => { + element._content = 'text'; + element._newContent = 'text'; + element._type = 'application/octet-stream'; + }); const promises = element._paramsChanged( Object.assign({}, mockParams, {view: Gerrit.Nav.View.EDIT})); @@ -84,6 +87,7 @@ suite('gr-editor-view tests', () => { return promises.then(() => { assert.equal(element._content, 'text'); assert.equal(element._newContent, 'text'); + assert.equal(element._type, 'application/octet-stream'); }); }); }); @@ -179,5 +183,41 @@ suite('gr-editor-view tests', () => { assert.isTrue(navigateStub.called); }); }); + + suite('_getFileData', () => { + setup(() => { + element._newContent = 'initial'; + element._content = 'initial'; + element._type = 'initial'; + }); + + test('res.ok', () => { + sandbox.stub(element.$.restAPI, 'getFileInChangeEdit') + .returns(Promise.resolve({ + ok: true, + type: 'text/javascript', + content: 'new content', + })); + + // Ensure no data is set with a bad response. + return element._getFileData('1', 'test/path').then(() => { + assert.equal(element._newContent, 'new content'); + assert.equal(element._content, 'new content'); + assert.equal(element._type, 'text/javascript'); + }); + }); + + test('!res.ok', () => { + sandbox.stub(element.$.restAPI, 'getFileInChangeEdit') + .returns(Promise.resolve({})); + + // Ensure no data is set with a bad response. + return element._getFileData('1', 'test/path').then(() => { + assert.equal(element._newContent, 'initial'); + assert.equal(element._content, 'initial'); + assert.equal(element._type, 'initial'); + }); + }); + }); }); \ No newline at end of file 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 89b5473798..3a80feb427 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 @@ -1333,7 +1333,13 @@ return this.getChangeURLAndSend(changeNum, 'GET', null, e, null, null, null, null, headers).then(res => { if (!res.ok) { return res; } - return this.getResponseObject(res); + + // The file type (used for syntax highlighting) is identified in the + // X-FYI-Content-Type header of the response. + const type = res.headers.get('X-FYI-Content-Type'); + return this.getResponseObject(res).then(content => { + return {content, type}; + }); }); }, 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 2a01032abe..bf238af246 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 @@ -1181,5 +1181,26 @@ limitations under the License. fetchStub.lastCall.args[0], '/projects/gerrit%2Fproject/dashboards/default%3Amain'); }); + + test('getFileInChangeEdit', () => { + sandbox.stub(element, 'getChangeURLAndSend') + .returns(Promise.resolve({ + ok: 'true', + headers: { + get(header) { + if (header === 'X-FYI-Content-Type') { + return 'text/java'; + } + }, + }, + })); + + sandbox.stub(element, 'getResponseObject') + .returns(Promise.resolve('new content')); + + return element.getFileInChangeEdit('1', 'test/path').then(res => { + assert.deepEqual(res, {content: 'new content', type: 'text/java'}); + }); + }); });