opendev/gerrit
Code Issues Proposed changes

Persist diff context length on server

Browse Source 
This code will eventually live within a diff preference pane
once options other than context are supported.

Change-Id: I3c752a18d163a55cb3398c0a46544593b0a9faed
This commit is contained in:
Andrew Bonventre 2016-01-06 10:43:30 -05:00 committed by Dave Borowitz
parent 5c1caa4f62
commit b3caf48ffa
4 changed files with 123 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
polygerrit-ui/app/elements/gr-app.html
View File

@ -87,6 +87,9 @@ limitations under the License.
<gr-ajax auto url="/accounts/self/detail" last-response="{{account}}"></gr-ajax>
<gr-ajax auto url="/config/server/info" last-response="{{config}}"></gr-ajax>
<gr-ajax auto url="/config/server/version" last-response="{{version}}"></gr-ajax>
<gr-ajax id="diffPreferencesXHR"
url="/accounts/self/preferences.diff"
last-response="{{_diffPreferences}}"></gr-ajax>
<header role="banner">
<a href="/" class="bigTitle">PolyGerrit</a>
<div class="headerRightItems">
@ -109,7 +112,7 @@ limitations under the License.
params="[[params]]"></gr-change-view>
</template>
<template is="dom-if" if="{{_showDiffView}}" restamp="true">
<gr-diff-view params="[[params]]"></gr-diff-view>
<gr-diff-view prefs="{{_diffPreferences}}" params="[[params]]"></gr-diff-view>
</template>
</main>
<footer role="contentinfo">
@ -175,6 +178,8 @@ limitations under the License.
type: String,
observer: '_routeChanged',
},
_diffPreferences: Object,
},
get loggedIn() {
@ -185,6 +190,15 @@ limitations under the License.
this._resolveAccountReady();
this.$.accountContainer.classList.toggle('loggedIn', this.loggedIn);
this.$.accountContainer.classList.toggle('loggedOut', !this.loggedIn);
if (this.loggedIn) {
this.$.diffPreferencesXHR.generateRequest();
} else {
// These defaults should match the defaults in
// gerrit-extension-api/src/main/jcg/gerrit/extensions/client/DiffPreferencesInfo.java
this._diffPreferences = {
context: 10,
};
}
},
_configChanged: function(config) {

5
polygerrit-ui/app/elements/gr-diff-view.html
View File

@ -120,6 +120,7 @@ limitations under the License.
<gr-diff id="diff"
auto
change-num="[[_changeNum]]"
prefs="{{prefs}}"
patch-range="[[_patchRange]]"
path="[[_path]]"
available-patches="[[_computeAvailablePatches(_change.revisions)]]"
@ -138,6 +139,10 @@ limitations under the License.
],
properties: {
prefs: {
type: Object,
notify: true,
},
keyEventTarget: {
type: Object,
value: function() {

88
polygerrit-ui/app/elements/gr-diff.html
View File

@ -68,7 +68,7 @@ limitations under the License.
change-num="[[changeNum]]"
patch-range="[[patchRange]]"
available-patches="[[availablePatches]]"></gr-patch-range-select>
<div class="contextControl">
<div class="contextControl" hidden$="[[!prefs.context]]" hidden>
Context:
<select id="contextSelect" on-change="_handleContextSelectChange">
<option value="3">3 lines</option>
@ -77,7 +77,7 @@ limitations under the License.
<option value="50">50 lines</option>
<option value="75">75 lines</option>
<option value="100">100 lines</option>
<option value="ALL">Whole file</option>
<option value="-1">Whole file</option>
</select>
</div>
</div>
@ -138,10 +138,18 @@ limitations under the License.
type: Number,
value: 80,
},
_context: {
type: Number,
value: 10,
observer: '_contextChanged',
prefs: {
type: Object,
notify: true,
},
_prefsReady: {
type: Object,
readOnly: true,
value: function() {
return new Promise(function(resolve) {
this._resolvePrefsReady = resolve;
}.bind(this));
},
},
_baseComments: Array,
_comments: Array,
@ -170,11 +178,17 @@ limitations under the License.
type: Boolean,
value: false,
},
_initialRenderComplete: {
type: Boolean,
value: false,
},
_diffRequestsPromise: Object, // Used for testing.
_diffPreferencesPromise: Object, // Used for testing.
},
observers: [
'_diffOptionsChanged(changeNum, patchRange, path)'
'_prefsChanged(prefs.*)',
'_diffOptionsChanged(changeNum, patchRange, path)',
],
ready: function() {
@ -188,18 +202,25 @@ limitations under the License.
this.$.rightDiff.scrollToLine(lineNum);
},
_contextChanged: function(context) {
if (context == Infinity) {
this.$.contextSelect.value = 'ALL';
} else {
this.$.contextSelect.value = context;
_prefsChanged: function(changeRecord) {
var prefs = changeRecord.base;
this.$.contextSelect.value = prefs.context;
if (this._initialRenderComplete) {
this._render();
}
this._resolvePrefsReady(prefs);
},
_diffOptionsChanged: function(changeNum, patchRange, path) {
if (!this.auto) { return; }
var promises = [this.$.diffXHR.generateRequest().completes];
var promises = [
this._prefsReady,
this.$.diffXHR.generateRequest().completes
];
var basePatchNum = patchRange.basePatchNum;
var patchNum = patchRange.patchNum;
@ -224,6 +245,8 @@ limitations under the License.
this.async(function() {
this.fire('render', null, {bubbles: false});
}.bind(this), 1);
this._initialRenderComplete = true;
},
_getCommentsAndDrafts: function(basePatchNum, loggedIn) {
@ -302,12 +325,28 @@ limitations under the License.
_handleContextSelectChange: function(e) {
var selectEl = Polymer.dom(e).rootTarget;
if (selectEl.value == 'ALL') {
this._context = Infinity;
} else {
this._context = parseInt(selectEl.value, 10);
}
this._render();
this.set('prefs.context', parseInt(selectEl.value, 10));
app.accountReady.then(function() {
if (!this._loggedIn) { return; }
this._diffPreferencesPromise =
this._saveDiffPreferences().catch(function(err) {
alert('Oops. Something went wrong. Check the console and bug the ' +
'PolyGerrit team for assistance.');
throw err;
});
}.bind(this));
},
_saveDiffPreferences: function() {
var xhr = document.createElement('gr-request');
this._diffPreferencesPromise = xhr.send({
method: 'PUT',
url: '/accounts/self/preferences.diff',
body: this.prefs,
});
return this._diffPreferencesPromise;
},
_handleExpandContext: function(e) {
@ -424,13 +463,18 @@ limitations under the License.
}
};
var content = this._diffResponse.content;
var context = this.prefs.context;
if (context == -1) {
// Show the entire file.
context = Infinity;
}
for (var i = 0; i < content.length; i++) {
if (i == 0) {
ctx.skipRange = [0, this._context];
ctx.skipRange = [0, context];
} else if (i == content.length - 1) {
ctx.skipRange = [this._context, 0];
ctx.skipRange = [context, 0];
} else {
ctx.skipRange = [this._context, this._context];
ctx.skipRange = [context, context];
}
ctx.diffChunkIndex = i;
this._addDiffChunk(ctx, content[i], leftSide, rightSide);

40
polygerrit-ui/app/test/gr-diff-test.html
View File

@ -48,6 +48,9 @@ limitations under the License.
element = fixture('basic');
element.changeNum = 42;
element.path = 'sieve.go';
element.prefs = {
context: 10,
};
server = sinon.fakeServer.create();
server.respondWith(
@ -184,6 +187,18 @@ limitations under the License.
}),
]
);
server.respondWith(
'PUT',
'/accounts/self/preferences.diff',
[
200,
{ 'Content-Type': 'application/json' },
')]}\'\n' +
JSON.stringify({ context: 25 }),
]
);
});
teardown(function() {
@ -225,7 +240,7 @@ limitations under the License.
});
test('comment rendering', function(done) {
element._context = Infinity;
element.prefs.context = -1;
element._loggedIn = true;
element.patchRange = {
basePatchNum: 1,
@ -394,7 +409,7 @@ limitations under the License.
});
test('context', function() {
element._context = 3;
element.prefs.context = 3;
element._diffResponse = {
content: [
{
@ -468,7 +483,26 @@ limitations under the License.
assert.equal(leftContext.start, 17);
assert.equal(leftContext.end, 19);
});
test('save context', function(done) {
element._loggedIn = false;
var contextSelectEl = element.$.contextSelect;
assert.ok(contextSelectEl);
contextSelectEl.value = '50';
element.fire('change', {}, {node: contextSelectEl});
assert.isTrue(element._diffPreferencesPromise == null);
element._loggedIn = true;
contextSelectEl.value = '25';
element.fire('change', {}, {node: contextSelectEl});
server.respond();
element._diffPreferencesPromise.then(function(req) {
assert.equal(req.xhr.requestBody, JSON.stringify(element.prefs));
done();
});
});
});
</script>