Merge "Fix links in accounts REST API documentation"
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 1a17305..b12b9be 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -653,6 +653,35 @@
}
@Test
+ public void removeReviewerNoVotes() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ gApi.changes()
+ .id(changeId)
+ .addReviewer(user.getId().toString());
+
+ // ReviewerState will vary between ReviewDb and NoteDb; we just care that it
+ // shows up somewhere.
+ Iterable<AccountInfo> reviewers = Iterables.concat(
+ gApi.changes().id(changeId).get().reviewers.values());
+ assertThat(reviewers).hasSize(1);
+ assertThat(reviewers.iterator().next()._accountId)
+ .isEqualTo(user.getId().get());
+
+ gApi.changes()
+ .id(changeId)
+ .reviewer(user.getId().toString())
+ .remove();
+ assertThat(gApi.changes().id(changeId).get().reviewers.isEmpty());
+
+ exception.expect(ResourceNotFoundException.class);
+ gApi.changes()
+ .id(changeId)
+ .reviewer(user.getId().toString())
+ .remove();
+ }
+
+ @Test
public void removeReviewer() throws Exception {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
index 28d7dcd8..a2a59e9 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java
@@ -128,6 +128,10 @@
public boolean updateChange(ChangeContext ctx)
throws AuthException, ResourceNotFoundException, OrmException {
Account.Id reviewerId = reviewer.getId();
+ if (!approvalsUtil.getReviewers(ctx.getDb(), ctx.getNotes()).all()
+ .contains(reviewerId)) {
+ throw new ResourceNotFoundException();
+ }
currChange = ctx.getChange();
currPs = psUtil.current(dbProvider.get(), ctx.getNotes());
@@ -156,9 +160,6 @@
throw new AuthException("delete not permitted");
}
}
- if (del.isEmpty()) {
- throw new ResourceNotFoundException();
- }
ctx.getDb().patchSetApprovals().delete(del);
ChangeUpdate update = ctx.getUpdate(currPs.getId());
update.removeReviewer(reviewerId);
diff --git a/polygerrit-ui/app/behaviors/gr-tooltip-behavior/gr-tooltip-behavior.js b/polygerrit-ui/app/behaviors/gr-tooltip-behavior/gr-tooltip-behavior.js
index b176466..3702c84 100644
--- a/polygerrit-ui/app/behaviors/gr-tooltip-behavior/gr-tooltip-behavior.js
+++ b/polygerrit-ui/app/behaviors/gr-tooltip-behavior/gr-tooltip-behavior.js
@@ -14,7 +14,7 @@
(function(window) {
'use strict';
- var BOTTOM_OFFSET = 10;
+ var BOTTOM_OFFSET = 7.2; // Height of the arrow in tooltip.
/** @polymerBehavior Gerrit.TooltipBehavior */
var TooltipBehavior = {
@@ -84,29 +84,19 @@
},
_positionTooltip: function(tooltip) {
- var offset = this._getOffset(this);
- var top = offset.top;
- var left = offset.left;
-
- top -= this.offsetHeight + BOTTOM_OFFSET;
- left -= (tooltip.offsetWidth / 2) - (this.offsetWidth / 2);
- left = Math.max(0, left);
- top = Math.max(0, top);
-
- tooltip.style.left = left + 'px';
- tooltip.style.top = top + 'px';
- },
-
- _getOffset: function(el) {
- var top = 0;
- var left = 0;
- for (var node = el; node; node = node.offsetParent) {
- if (node.offsetTop) { top += node.offsetTop; }
- if (node.offsetLeft) { left += node.offsetLeft; }
+ var rect = this.getBoundingClientRect();
+ var boxRect = tooltip.getBoundingClientRect();
+ var parentRect = tooltip.parentElement.getBoundingClientRect();
+ var top = rect.top - parentRect.top - boxRect.height - BOTTOM_OFFSET;
+ var left =
+ rect.left - parentRect.left + (rect.width - boxRect.width) / 2;
+ if (left < 0) {
+ tooltip.updateStyles({
+ '--gr-tooltip-arrow-center-offset': left + 'px',
+ });
}
- top += window.scrollY;
- left += window.scrollX;
- return {top: top, left: left};
+ tooltip.style.left = Math.max(0, left) + 'px';
+ tooltip.style.top = Math.max(0, top) + 'px';
},
};
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
index 19f9e85..a0a9305 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.html
@@ -17,6 +17,7 @@
<link rel="import" href="../../../bower_components/polymer/polymer.html">
<link rel="import" href="../../../behaviors/rest-client-behavior.html">
<link rel="import" href="../../shared/gr-account-link/gr-account-link.html">
+<link rel="import" href="../../shared/gr-label/gr-label.html">
<link rel="import" href="../../shared/gr-date-formatter/gr-date-formatter.html">
<link rel="import" href="../../shared/gr-editable-label/gr-editable-label.html">
<link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
@@ -141,7 +142,12 @@
as="label">
<div class="labelValueContainer">
<span class$="[[label.className]]">
- <span class="labelValue">[[label.value]]</span>
+ <gr-label
+ has-tooltip
+ title="[[_computeValueTooltip(label.value, labelName)]]"
+ class="labelValue">
+ [[label.value]]
+ </gr-label>
<gr-account-link account="[[label.account]]"></gr-account-link>
</span>
</div>
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
index 379487d..e24cc4a 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata.js
@@ -117,6 +117,11 @@
return result;
},
+ _computeValueTooltip: function(score, labelName) {
+ var values = this.change.labels[labelName].values;
+ return values[score];
+ },
+
_handleTopicChanged: function(e, topic) {
if (!topic.length) { topic = null; }
this.$.restAPI.setChangeTopic(this.change.id, topic);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
index 557e7b8..832665b 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html
@@ -21,7 +21,9 @@
<div class="contentWrapper">
<content></content>
</div>
- <gr-diff-processor id="processor"></gr-diff-processor>
+ <gr-diff-processor
+ id="processor"
+ groups="{{_groups}}"></gr-diff-processor>
</template>
<script src="../gr-diff/gr-diff-line.js"></script>
<script src="../gr-diff/gr-diff-group.js"></script>
@@ -53,19 +55,34 @@
baseImage: Object,
revisionImage: Object,
_builder: Object,
+ _groups: Array,
},
get diffElement() {
return this.queryEffectiveChildren('#diffTable');
},
+ observers: [
+ '_groupsChanged(_groups.splices)',
+ ],
+
render: function(diff, comments, prefs) {
+ // Stop the processor (if it's running).
+ this.$.processor.cancel();
+
this._builder = this._getDiffBuilder(diff, comments, prefs);
this.$.processor.context = prefs.context;
this.$.processor.keyLocations = this._getCommentLocations(comments);
- this.$.processor.process(diff.content)
- .then(this._renderDiff.bind(this));
+
+ this._clearDiffContent();
+
+ this.$.processor.process(diff.content).then(function() {
+ if (this.isImageDiff) {
+ this._builder.renderDiffImages();
+ }
+ this.fire('render');
+ }.bind(this));
},
getLineElByChild: function(node) {
@@ -183,10 +200,6 @@
this._builder.emitGroup(group, sectionEl);
},
- emitDiff: function() {
- this._builder.emitDiff();
- },
-
showContext: function(newGroups, sectionEl) {
var groups = this._builder.groups;
// TODO(viktard): Polyfill findIndex for IE10.
@@ -219,19 +232,6 @@
throw Error('Unsupported diff view mode: ' + this.viewMode);
},
- _renderDiff: function(groups) {
- this._builder.groups = groups;
-
- this._clearDiffContent();
- this.emitDiff();
- if (this.isImageDiff) {
- this._builder.renderDiffImages();
- }
- this.async(function() {
- this.fire('render');
- }, 1);
- },
-
_clearDiffContent: function() {
this.diffElement.innerHTML = null;
},
@@ -252,6 +252,18 @@
}
return result;
},
+
+ _groupsChanged: function(changeRecord) {
+ if (!changeRecord) { return; }
+ changeRecord.indexSplices.forEach(function(splice) {
+ var group;
+ for (var i = 0; i < splice.addedCount; i++) {
+ group = splice.object[splice.index + i];
+ this._builder.groups.push(group);
+ this._builder.emitGroup(group);
+ }
+ }, this);
+ },
});
})();
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index b1256db..f68825c 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -59,12 +59,6 @@
var PARTIAL_CONTEXT_AMOUNT = 10;
- GrDiffBuilder.prototype.emitDiff = function() {
- for (var i = 0; i < this.groups.length; i++) {
- this.emitGroup(this.groups[i]);
- }
- };
-
GrDiffBuilder.prototype.buildSectionElement = function(group) {
throw Error('Subclasses must implement buildGroupElement');
};
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
index 60654a8..4c5ee62 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor_test.html
@@ -98,28 +98,39 @@
assert.equal(cursorElement.diffRow, firstDeltaRow);
});
- test('diff cursor functionality (unified)', function() {
- diffElement.viewMode = 'UNIFIED_DIFF';
- cursorElement.reInitCursor();
+ suite('unified diff', function() {
- // The cursor has been initialized to the first delta.
- assert.isOk(cursorElement.diffRow);
+ setup(function(done) {
+ // We must allow the diff to re-render after setting the viewMode.
+ var renderHandler = function() {
+ diffElement.removeEventListener('render', renderHandler);
+ cursorElement.reInitCursor();
+ done();
+ };
+ diffElement.addEventListener('render', renderHandler);
+ diffElement.viewMode = 'UNIFIED_DIFF';
+ });
- var firstDeltaRow = diffElement.$$('.section.delta .diff-row');
- assert.equal(cursorElement.diffRow, firstDeltaRow);
+ test('diff cursor functionality (unified)', function() {
+ // The cursor has been initialized to the first delta.
+ assert.isOk(cursorElement.diffRow);
- firstDeltaRow = diffElement.$$('.section.delta .diff-row');
- assert.equal(cursorElement.diffRow, firstDeltaRow);
+ var firstDeltaRow = diffElement.$$('.section.delta .diff-row');
+ assert.equal(cursorElement.diffRow, firstDeltaRow);
- cursorElement.moveDown();
+ firstDeltaRow = diffElement.$$('.section.delta .diff-row');
+ assert.equal(cursorElement.diffRow, firstDeltaRow);
- assert.notEqual(cursorElement.diffRow, firstDeltaRow);
- assert.equal(cursorElement.diffRow, firstDeltaRow.nextSibling);
+ cursorElement.moveDown();
- cursorElement.moveUp();
+ assert.notEqual(cursorElement.diffRow, firstDeltaRow);
+ assert.equal(cursorElement.diffRow, firstDeltaRow.nextSibling);
- assert.notEqual(cursorElement.diffRow, firstDeltaRow.nextSibling);
- assert.equal(cursorElement.diffRow, firstDeltaRow);
+ cursorElement.moveUp();
+
+ assert.notEqual(cursorElement.diffRow, firstDeltaRow.nextSibling);
+ assert.equal(cursorElement.diffRow, firstDeltaRow);
+ });
});
test('cursor side functionality', function() {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
index 0a54665..f0fc649 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-processor/gr-diff-processor.js
@@ -58,6 +58,8 @@
type: Object,
value: function() { return {left: {}, right: {}}; },
},
+
+ _nextStepHandle: Number,
},
/**
@@ -82,6 +84,7 @@
// If we are done, resolve the promise.
if (state.sectionIndex >= content.length) {
resolve(this.groups);
+ this._nextStepHandle = undefined;
return;
}
@@ -95,7 +98,7 @@
// Increment the index and recurse.
state.sectionIndex++;
- this.async(nextStep, 1);
+ this._nextStepHandle = this.async(nextStep, 1);
};
nextStep.call(this);
@@ -103,6 +106,16 @@
},
/**
+ * Cancel any jobs that are running.
+ */
+ cancel: function() {
+ if (this._nextStepHandle !== undefined) {
+ this.cancelAsync(this._nextStepHandle);
+ this._nextStepHandle = undefined;
+ }
+ },
+
+ /**
* Process the next section of the diff.
*/
_processNext: function(state, content) {
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
index 32de7d5..d4cced0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.js
@@ -36,7 +36,10 @@
changeNum: String,
patchRange: Object,
path: String,
- prefs: Object,
+ prefs: {
+ type: Object,
+ observer: '_prefsObserver',
+ },
projectConfig: {
type: Object,
observer: '_projectConfigChanged',
@@ -57,6 +60,7 @@
viewMode: {
type: String,
value: DiffViewMode.SIDE_BY_SIDE,
+ observer: '_viewModeObserver',
},
_diff: Object,
_comments: Object,
@@ -64,10 +68,6 @@
_revisionImage: Object,
},
- observers: [
- '_prefsChanged(prefs.*, viewMode)',
- ],
-
listeners: {
'thread-discard': '_handleThreadDiscard',
'comment-discard': '_handleCommentDiscard',
@@ -303,8 +303,28 @@
});
},
- _prefsChanged: function(prefsChangeRecord) {
- var prefs = prefsChangeRecord.base;
+ _prefsObserver: function(newPrefs, oldPrefs) {
+ // Scan the preference objects one level deep to see if they differ.
+ var differ = !oldPrefs;
+ if (newPrefs && oldPrefs) {
+ for (var key in newPrefs) {
+ if (newPrefs[key] !== oldPrefs[key]) {
+ differ = true;
+ }
+ }
+ }
+
+ if (differ) {
+ this._prefsChanged(newPrefs);
+ }
+ },
+
+ _viewModeObserver: function() {
+ this._prefsChanged(this.prefs);
+ },
+
+ _prefsChanged: function(prefs) {
+ if (!prefs) { return; }
this.customStyle['--content-width'] = prefs.line_length + 'ch';
this.updateStyles();
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
index 5936bee..c12d653 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-js-api-interface_test.html
@@ -42,7 +42,7 @@
getAccount: function() {
return Promise.resolve({name: 'Judy Hopps'});
},
- })
+ });
element = fixture('basic');
errorStub = sinon.stub(console, 'error');
Gerrit.install(function(p) { plugin = p; }, '0.1',
diff --git a/polygerrit-ui/app/elements/shared/gr-label/gr-label.html b/polygerrit-ui/app/elements/shared/gr-label/gr-label.html
new file mode 100644
index 0000000..04b12e7
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-label/gr-label.html
@@ -0,0 +1,23 @@
+<!--
+Copyright (C) 2016 The Android Open Source Project
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+-->
+<link rel="import" href="../../../bower_components/polymer/polymer.html">
+<link rel="import" href="../../../behaviors/gr-tooltip-behavior/gr-tooltip-behavior.html">
+<dom-module id="gr-label">
+ <template strip-whitespace>
+ <content></content>
+ </template>
+ <script src="gr-label.js"></script>
+</dom-module>
diff --git a/polygerrit-ui/app/elements/shared/gr-label/gr-label.js b/polygerrit-ui/app/elements/shared/gr-label/gr-label.js
new file mode 100644
index 0000000..37e1f77
--- /dev/null
+++ b/polygerrit-ui/app/elements/shared/gr-label/gr-label.js
@@ -0,0 +1,24 @@
+// Copyright (C) 2016 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+(function() {
+ 'use strict';
+
+ Polymer({
+ is: 'gr-label',
+
+ behaviors: [
+ Gerrit.TooltipBehavior,
+ ],
+ });
+})();
diff --git a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.html b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.html
index 64ef9b2..57a7272 100644
--- a/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.html
+++ b/polygerrit-ui/app/elements/shared/gr-tooltip/gr-tooltip.html
@@ -20,7 +20,8 @@
<template>
<style>
:host {
- --gr-tooltip-arrow-size: .6em;
+ --gr-tooltip-arrow-size: .5em;
+ --gr-tooltip-arrow-center-offset: 0;
background-color: #333;
box-shadow: 0 1px 3px rgba(0, 0, 0, .3);
@@ -38,6 +39,7 @@
height: 0;
position: absolute;
left: calc(50% - var(--gr-tooltip-arrow-size));
+ margin-left: var(--gr-tooltip-arrow-center-offset);
width: 0;
}
</style>
@@ -46,4 +48,3 @@
</template>
<script src="gr-tooltip.js"></script>
</dom-module>
-
