Clean up gr-file-list
+ Use gr-rest-api-interface instead of gr-ajax and gr-request.
+ Use strict equality operators.
+ Clean up tests.
+ Add hover state to non-header rows.
Change-Id: I8589dc7f9466d61928add8f2587af2b8a2a56497
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
index 7d69d7e..d94a828 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.html
@@ -16,9 +16,8 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/keyboard-shortcut-behavior.html">
-<link rel="import" href="../../../behaviors/rest-client-behavior.html">
-<link rel="import" href="../../shared/gr-ajax/gr-ajax.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-file-list">
<template>
@@ -49,6 +48,9 @@
visibility: hidden;
width: 1.25em;
}
+ .row:not(.header):hover {
+ background-color: #f5fafd;
+ }
.row[selected] {
background-color: #ebf5fb;
}
@@ -112,14 +114,6 @@
}
}
</style>
- <gr-ajax id="filesXHR"
- url="[[_computeFilesURL(changeNum, patchNum)]]"
- on-response="_handleResponse"></gr-ajax>
- <gr-ajax id="reviewedXHR"
- url="[[_computeReviewedURL(changeNum, patchNum)]]"
- last-response="{{_reviewed}}"></gr-ajax>
- </gr-ajax>
-
<div class="row header">
<div class="positionIndicator"></div>
<div class="reviewed" hidden$="[[!_loggedIn]]" hidden></div>
@@ -128,7 +122,7 @@
<div class="comments">Comments</div>
<div class="stats">Stats</div>
</div>
- <template is="dom-repeat" items="{{files}}" as="file">
+ <template is="dom-repeat" items="{{_files}}" as="file">
<div class="row" selected$="[[_computeFileSelected(index, selectedIndex)]]">
<div class="positionIndicator">▶</div>
<div class="reviewed" hidden$="[[!_loggedIn]]" hidden>
@@ -151,6 +145,7 @@
</div>
</div>
</template>
+ <gr-rest-api-interface id="restAPI"></gr-rest-api-interface>
</template>
<script src="gr-file-list.js"></script>
</dom-module>
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
index ead5d74..947d982 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list.js
@@ -24,7 +24,6 @@
changeNum: String,
comments: Object,
drafts: Object,
- files: Array,
selectedIndex: {
type: Number,
notify: true,
@@ -33,6 +32,8 @@
type: Object,
value: function() { return document.body; },
},
+
+ _files: Array,
_loggedIn: {
type: Boolean,
value: false,
@@ -41,34 +42,34 @@
type: Array,
value: function() { return []; },
},
- _filesRequestPromise: Object, // Used for testing.
- _reviewedRequestPromise: Object, // Used for testing.
- _xhrPromise: Object, // Used for testing.
},
behaviors: [
Gerrit.KeyboardShortcutBehavior,
- Gerrit.RESTClientBehavior,
],
reload: function() {
if (!this.changeNum || !this.patchNum) {
return Promise.resolve();
}
- return Promise.all([
- this._filesRequestPromise =
- this.$.filesXHR.generateRequest().completes,
- app.accountReady.then(function() {
- this._loggedIn = app.loggedIn;
- if (!app.loggedIn) { return; }
- this._reviewedRequestPromise =
- this.$.reviewedXHR.generateRequest().completes;
- }.bind(this)),
- ]);
- },
- _computeFilesURL: function(changeNum, patchNum) {
- return this.changeBaseURL(changeNum, patchNum) + '/files';
+ var promises = [];
+ var _this = this;
+
+ promises.push(this._getFiles().then(function(files) {
+ _this._files = files;
+ }));
+ promises.push(this._getLoggedIn().then(function(loggedIn) {
+ return _this._loggedIn = loggedIn;
+ }).then(function(loggedIn) {
+ if (!loggedIn) { return; }
+
+ _this._getReviewedFiles().then(function(reviewed) {
+ _this._reviewed = reviewed;
+ });
+ }));
+
+ return Promise.all(promises);
},
_computeCommentsString: function(comments, patchNum, path) {
@@ -83,53 +84,63 @@
if (!comments) { return ''; }
var patchComments = (comments[path] || []).filter(function(c) {
- return c.patch_set == patchNum;
+ return parseInt(c.patch_set, 10) === parseInt(patchNum, 10);
});
var num = patchComments.length;
- if (num == 0) { return ''; }
+ if (num === 0) { return ''; }
return num + ' ' + noun + (num > 1 ? 's' : '');
},
- _computeReviewedURL: function(changeNum, patchNum) {
- return this.changeBaseURL(changeNum, patchNum) + '/files?reviewed';
- },
-
_computeReviewed: function(file, _reviewed) {
- return _reviewed.indexOf(file.__path) != -1;
+ return _reviewed.indexOf(file.__path) !== -1;
},
_handleReviewedChange: function(e) {
var path = Polymer.dom(e).rootTarget.getAttribute('data-path');
var index = this._reviewed.indexOf(path);
- var reviewed = index != -1;
+ var reviewed = index !== -1;
if (reviewed) {
this.splice('_reviewed', index, 1);
} else {
this.push('_reviewed', path);
}
- var method = reviewed ? 'DELETE' : 'PUT';
- var url = this.changeBaseURL(this.changeNum, this.patchNum) +
- '/files/' + encodeURIComponent(path) + '/reviewed';
- this._send(method, url).catch(function(err) {
+ this._saveReviewedState(path, !reviewed).catch(function(err) {
alert('Couldn’t change file review status. Check the console ' +
'and contact the PolyGerrit team for assistance.');
throw err;
}.bind(this));
},
- _handleResponse: function(e, req) {
- var result = e.detail.response;
- var paths = Object.keys(result).sort();
+ _saveReviewedState: function(path, reviewed) {
+ return this.$.restAPI.saveFileReviewed(this.changeNum, this.patchNum,
+ path, reviewed);
+ },
+
+ _getLoggedIn: function() {
+ return this.$.restAPI.getLoggedIn();
+ },
+
+ _getReviewedFiles: function() {
+ return this.$.restAPI.getReviewedFiles(this.changeNum, this.patchNum);
+ },
+
+ _getFiles: function() {
+ return this.$.restAPI.getChangeFiles(this.changeNum, this.patchNum).then(
+ this._normalizeFilesResponse.bind(this));
+ },
+
+ _normalizeFilesResponse: function(response) {
+ var paths = Object.keys(response).sort();
var files = [];
for (var i = 0; i < paths.length; i++) {
- var info = result[paths[i]];
+ var info = response[paths[i]];
info.__path = paths[i];
info.lines_inserted = info.lines_inserted || 0;
info.lines_deleted = info.lines_deleted || 0;
files.push(info);
}
- this.files = files;
+ return files;
},
_handleKey: function(e) {
@@ -139,7 +150,7 @@
case 74: // 'j'
e.preventDefault();
this.selectedIndex =
- Math.min(this.files.length - 1, this.selectedIndex + 1);
+ Math.min(this._files.length - 1, this.selectedIndex + 1);
break;
case 75: // 'k'
e.preventDefault();
@@ -147,7 +158,7 @@
break;
case 219: // '['
e.preventDefault();
- this._openSelectedFile(this.files.length - 1);
+ this._openSelectedFile(this._files.length - 1);
break;
case 221: // ']'
e.preventDefault();
@@ -166,11 +177,11 @@
this.selectedIndex = opt_index;
}
page.show(this._computeDiffURL(this.changeNum, this.patchNum,
- this.files[this.selectedIndex].__path));
+ this._files[this.selectedIndex].__path));
},
_computeFileSelected: function(index, selectedIndex) {
- return index == selectedIndex;
+ return index === selectedIndex;
},
_computeFileStatus: function(status) {
@@ -182,24 +193,15 @@
},
_computeFileDisplayName: function(path) {
- return path == COMMIT_MESSAGE_PATH ? 'Commit message' : path;
+ return path === COMMIT_MESSAGE_PATH ? 'Commit message' : path;
},
_computeClass: function(baseClass, path) {
var classes = [baseClass];
- if (path == COMMIT_MESSAGE_PATH) {
+ if (path === COMMIT_MESSAGE_PATH) {
classes.push('invisible');
}
return classes.join(' ');
},
-
- _send: function(method, url) {
- var xhr = document.createElement('gr-request');
- this._xhrPromise = xhr.send({
- method: method,
- url: url,
- });
- return this._xhrPromise;
- },
});
})();
diff --git a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
index 48e8cc7..21263c0 100644
--- a/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-file-list/gr-file-list_test.html
@@ -36,204 +36,93 @@
<script>
suite('gr-file-list tests', function() {
var element;
- var server;
+ var getLoggedInStub;
setup(function() {
element = fixture('basic');
- server = sinon.fakeServer.create();
- server.respondWith(
- 'GET',
- '/changes/42/revisions/1/files',
- [
- 200,
- {'Content-Type': 'application/json'},
- ')]}\'\n' +
- JSON.stringify({
- '/COMMIT_MSG': {
- status: 'A',
- lines_inserted: 9,
- size_delta: 317,
- size: 317
- },
- 'myfile.txt': {
- lines_inserted: 35,
- size_delta: 1146,
- size: 1167
- }
- }),
- ]
- );
- server.respondWith(
- 'GET',
- '/changes/42/revisions/2/files',
- [
- 200,
- {'Content-Type': 'application/json'},
- ')]}\'\n' +
- JSON.stringify({
- '/COMMIT_MSG': {
- status: 'A',
- lines_inserted: 9,
- size_delta: 317,
- size: 317
- },
- 'myfile.txt': {
- lines_inserted: 35,
- size_delta: 1146,
- size: 1167
- },
- 'file_added_in_rev2.txt': {
- lines_inserted: 98,
- size_delta: 234,
- size: 136
- }
- }),
- ]
- );
- server.respondWith(
- 'GET',
- '/changes/42/revisions/1/drafts',
- [
- 200,
- {'Content-Type': 'application/json'},
- ')]}\'\n' +
- '{}',
- ]
- );
- server.respondWith(
- 'GET',
- '/changes/42/revisions/2/drafts',
- [
- 200,
- {'Content-Type': 'application/json'},
- ')]}\'\n' +
- '{}',
- ]
- );
- server.respondWith(
- 'GET',
- '/changes/42/revisions/1/files?reviewed',
- [
- 200,
- {'Content-Type': 'application/json'},
- ')]}\'\n' +
- '["/COMMIT_MSG"]',
- ]
- );
- server.respondWith(
- 'GET',
- '/changes/42/revisions/2/files?reviewed',
- [
- 200,
- {'Content-Type': 'application/json'},
- ')]}\'\n' +
- '["/COMMIT_MSG","myfile.txt"]',
- ]
- );
- server.respondWith(
- 'PUT',
- '/changes/42/revisions/2/files/%2FCOMMIT_MSG/reviewed',
- [
- 201,
- {'Content-Type': 'application/json'},
- ')]}\'\n' +
- '""',
- ]
- );
- server.respondWith(
- 'DELETE',
- '/changes/42/revisions/2/files/%2FCOMMIT_MSG/reviewed',
- [
- 204,
- {'Content-Type': 'application/json'},
- '',
- ]
- );
-
- app.loggedIn = true;
+ getLoggedInStub = sinon.stub(element, '_getLoggedIn', function() {
+ return Promise.resolve(true);
+ });
});
teardown(function() {
- server.restore();
+ getLoggedInStub.restore();
});
- test('requests', function(done) {
- element.changeNum = '42';
- element.patchNum = '1';
- element.reload();
- server.respond();
-
- Promise.all([
- element._filesRequestPromise,
- element._reviewedRequestPromise,
- ]).then(function() {
- flushAsynchronousOperations();
- var filenames = element.files.map(function(f) {
- return f.__path;
- });
- assert.deepEqual(filenames, ['/COMMIT_MSG', 'myfile.txt']);
- assert.deepEqual(element._reviewed, ['/COMMIT_MSG']);
-
- element.patchNum = '2';
- element.reload();
- server.respond();
- Promise.all([
- element._filesRequestPromise,
- element._reviewedRequestPromise,
- ]).then(function() {
- flushAsynchronousOperations();
- filenames = element.files.map(function(f) {
- return f.__path;
+ test('get file list', function(done) {
+ var getChangeFilesStub = sinon.stub(element.$.restAPI, 'getChangeFiles',
+ function() {
+ return Promise.resolve({
+ '/COMMIT_MSG': {lines_inserted: 9},
+ 'tags.html': {lines_deleted: 123},
+ 'about.txt': {},
+ });
});
- assert.deepEqual(filenames,
- ['/COMMIT_MSG', 'file_added_in_rev2.txt', 'myfile.txt']);
- assert.deepEqual(element._reviewed, ['/COMMIT_MSG', 'myfile.txt']);
- done();
+
+ element._getFiles().then(function(files) {
+ var filenames = files.map(function(f) { return f.__path; });
+ assert.deepEqual(filenames, ['/COMMIT_MSG', 'about.txt', 'tags.html']);
+ assert.deepEqual(files[0], {
+ lines_inserted: 9,
+ lines_deleted: 0,
+ __path: '/COMMIT_MSG',
});
- });
- });
+ assert.deepEqual(files[1], {
+ lines_inserted: 0,
+ lines_deleted: 0,
+ __path: 'about.txt',
+ });
+ assert.deepEqual(files[2], {
+ lines_inserted: 0,
+ lines_deleted: 123,
+ __path: 'tags.html',
+ });
- test('keyboard shortcuts', function(done) {
- element.changeNum = '42';
- element.patchNum = '2';
- element.selectedIndex = 0;
- element.reload();
- server.respond();
-
- element._filesRequestPromise.then(function() {
- flushAsynchronousOperations();
- var elementItems = Polymer.dom(element.root).querySelectorAll(
- '.row:not(.header)');
- assert.equal(elementItems.length, 3);
- assert.isTrue(elementItems[0].hasAttribute('selected'));
- assert.isFalse(elementItems[1].hasAttribute('selected'));
- assert.isFalse(elementItems[2].hasAttribute('selected'));
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
- assert.equal(element.selectedIndex, 1);
- MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
-
- var showStub = sinon.stub(page, 'show');
- assert.equal(element.selectedIndex, 2);
- MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
- assert(showStub.lastCall.calledWith('/c/42/2/myfile.txt'),
- 'Should navigate to /c/42/2/myfile.txt');
-
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
- assert.equal(element.selectedIndex, 1);
- MockInteractions.pressAndReleaseKeyOn(element, 79); // 'o'
- assert(showStub.lastCall.calledWith('/c/42/2/file_added_in_rev2.txt'),
- 'Should navigate to /c/42/2/file_added_in_rev2.txt');
-
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
- MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
- assert.equal(element.selectedIndex, 0);
-
- showStub.restore();
done();
});
});
+ test('keyboard shortcuts', function() {
+ element._files = [
+ {__path: '/COMMIT_MSG'},
+ {__path: 'file_added_in_rev2.txt'},
+ {__path: 'myfile.txt'},
+ ];
+ element.changeNum = '42',
+ element.patchNum = '2';
+ element.selectedIndex = 0;
+
+ flushAsynchronousOperations();
+ var elementItems = Polymer.dom(element.root).querySelectorAll(
+ '.row:not(.header)');
+ assert.equal(elementItems.length, 3);
+ assert.isTrue(elementItems[0].hasAttribute('selected'));
+ assert.isFalse(elementItems[1].hasAttribute('selected'));
+ assert.isFalse(elementItems[2].hasAttribute('selected'));
+ MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
+ assert.equal(element.selectedIndex, 1);
+ MockInteractions.pressAndReleaseKeyOn(element, 74); // 'j'
+
+ var showStub = sinon.stub(page, 'show');
+ assert.equal(element.selectedIndex, 2);
+ MockInteractions.pressAndReleaseKeyOn(element, 13); // 'enter'
+ assert(showStub.lastCall.calledWith('/c/42/2/myfile.txt'),
+ 'Should navigate to /c/42/2/myfile.txt');
+
+ MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
+ assert.equal(element.selectedIndex, 1);
+ MockInteractions.pressAndReleaseKeyOn(element, 79); // 'o'
+ assert(showStub.lastCall.calledWith('/c/42/2/file_added_in_rev2.txt'),
+ 'Should navigate to /c/42/2/file_added_in_rev2.txt');
+
+ MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
+ MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
+ MockInteractions.pressAndReleaseKeyOn(element, 75); // 'k'
+ assert.equal(element.selectedIndex, 0);
+
+ showStub.restore();
+ });
+
test('comment filtering', function() {
var comments = {
'/COMMIT_MSG': [
@@ -284,50 +173,37 @@
'clazz invisible');
});
- test('file review status', function(done) {
+ test('file review status', function() {
+ element._files = [
+ {__path: '/COMMIT_MSG'},
+ {__path: 'file_added_in_rev2.txt'},
+ {__path: 'myfile.txt'},
+ ];
+ element._reviewed = ['/COMMIT_MSG', 'myfile.txt'];
element.changeNum = '42';
element.patchNum = '2';
- element.reload();
- server.respond();
+ element.selectedIndex = 0;
- Promise.all([
- element._filesRequestPromise,
- element._reviewedRequestPromise,
- ]).then(function() {
- flushAsynchronousOperations();
- var fileRows =
- Polymer.dom(element.root).querySelectorAll('.row:not(.header)');
- var commitMsg = fileRows[0].querySelector('input[type="checkbox"]');
- var fileAdded = fileRows[1].querySelector('input[type="checkbox"]');
- var myFile = fileRows[2].querySelector('input[type="checkbox"]');
+ flushAsynchronousOperations();
+ var fileRows =
+ Polymer.dom(element.root).querySelectorAll('.row:not(.header)');
+ var commitMsg = fileRows[0].querySelector('input[type="checkbox"]');
+ var fileAdded = fileRows[1].querySelector('input[type="checkbox"]');
+ var myFile = fileRows[2].querySelector('input[type="checkbox"]');
- assert.isTrue(commitMsg.checked);
- assert.isFalse(fileAdded.checked);
- assert.isTrue(myFile.checked);
+ assert.isTrue(commitMsg.checked);
+ assert.isFalse(fileAdded.checked);
+ assert.isTrue(myFile.checked);
- assert.equal(element._reviewed.length, 2);
+ var saveStub = sinon.stub(element, '_saveReviewedState',
+ function() { return Promise.resolve(); });
- MockInteractions.tap(commitMsg);
- server.respond();
- element._xhrPromise.then(function(req) {
- assert.equal(element._reviewed.length, 1);
- assert.equal(req.status, 204);
- assert.equal(req.url,
- '/changes/42/revisions/2/files/%2FCOMMIT_MSG/reviewed');
+ MockInteractions.tap(commitMsg);
+ assert.isTrue(saveStub.lastCall.calledWithExactly('/COMMIT_MSG', false));
+ MockInteractions.tap(commitMsg);
+ assert.isTrue(saveStub.lastCall.calledWithExactly('/COMMIT_MSG', true));
- MockInteractions.tap(commitMsg);
- server.respond();
- }).then(function() {
- element._xhrPromise.then(function(req) {
- assert.equal(element._reviewed.length, 2);
- assert.equal(req.status, 201);
- assert.equal(req.url,
- '/changes/42/revisions/2/files/%2FCOMMIT_MSG/reviewed');
-
- done();
- });
- });
- });
+ saveStub.restore();
});
});
</script>
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 72570d9..85d26e6 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
@@ -130,6 +130,48 @@
return this._sharedFetchPromises[url];
},
+ getChangeFiles: function(changeNum, patchNum) {
+ return this.fetchJSON(
+ this._changeBaseURL(changeNum, patchNum) + '/files');
+ },
+
+ getReviewedFiles: function(changeNum, patchNum) {
+ return this.fetchJSON(
+ this._changeBaseURL(changeNum, patchNum) + '/files?reviewed');
+ },
+
+ saveFileReviewed: function(changeNum, patchNum, path, reviewed, opt_errFn,
+ opt_ctx) {
+ var method = reviewed ? 'PUT' : 'DELETE';
+ var url = this._changeBaseURL(changeNum, patchNum) + '/files/' +
+ encodeURIComponent(path) + '/reviewed';
+
+ return this._save(method, url, null, opt_errFn, opt_ctx);
+ },
+
+ _save: function(method, url, opt_body, opt_errFn, opt_ctx) {
+ var headers = new Headers({
+ 'X-Gerrit-Auth': this._getCookie('XSRF_TOKEN'),
+ });
+
+ if (opt_body) {
+ headers.append('Content-Type', 'application/json');
+ options.body = body;
+ }
+ var options = {
+ method: method,
+ headers: headers,
+ credentials: 'same-origin',
+ };
+ return fetch(url, options).catch(function(err) {
+ if (opt_errFn) {
+ opt_errFn.call(opt_ctx || this);
+ } else {
+ throw err;
+ }
+ });
+ },
+
getDiff: function(changeNum, basePatchNum, patchNum, path,
opt_cancelCondition) {
var url = this._getDiffFetchURL(changeNum, patchNum, path);
@@ -165,7 +207,7 @@
opt_patchNum, opt_path) {
if (!opt_basePatchNum && !opt_patchNum && !opt_path) {
return this.fetchJSON(
- this._getDiffCommentsFetchURL(changeNum, null, '/drafts'));
+ this._getDiffCommentsFetchURL(changeNum, '/drafts'));
}
function onlyParent(c) { return c.side == PARENT_PATCH_NUM; }
@@ -212,5 +254,20 @@
return v;
},
+ _getCookie: function(name) {
+ var key = name + '=';
+ var cookies = document.cookie.split(';');
+ for (var i = 0; i < cookies.length; i++) {
+ var c = cookies[i];
+ while (c.charAt(0) == ' ') {
+ c = c.substring(1);
+ }
+ if (c.indexOf(key) == 0) {
+ return c.substring(key.length, c.length);
+ }
+ }
+ return '';
+ },
+
});
})();
