Merge "Missing labels list in change metadata and long-label polish"
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
index 1f27d83..0f6e89e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -299,13 +299,8 @@
return Sets.newHashSet(a.trackingFooters.extract(footers).values());
});
- /** List of labels on the current patch set. */
- @Deprecated
- public static final FieldDef<ChangeData, Iterable<String>> LABEL =
- exact(ChangeQueryBuilder.FIELD_LABEL).buildRepeatable(cd -> getLabels(cd, false));
-
/** List of labels on the current patch set including change owner votes. */
- public static final FieldDef<ChangeData, Iterable<String>> LABEL2 =
+ public static final FieldDef<ChangeData, Iterable<String>> LABEL =
exact("label2").buildRepeatable(cd -> getLabels(cd, true));
private static Iterable<String> getLabels(ChangeData cd, boolean owners) throws OrmException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index ba7c1ec..35d84b8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -22,75 +22,53 @@
public class ChangeSchemaDefinitions extends SchemaDefinitions<ChangeData> {
@Deprecated
- static final Schema<ChangeData> V32 =
+ static final Schema<ChangeData> V39 =
schema(
- ChangeField.LEGACY_ID,
+ ChangeField.ADDED,
+ ChangeField.APPROVAL,
+ ChangeField.ASSIGNEE,
+ ChangeField.AUTHOR,
+ ChangeField.CHANGE,
+ ChangeField.COMMENT,
+ ChangeField.COMMENTBY,
+ ChangeField.COMMIT,
+ ChangeField.COMMITTER,
+ ChangeField.COMMIT_MESSAGE,
+ ChangeField.DELETED,
+ ChangeField.DELTA,
+ ChangeField.DRAFTBY,
+ ChangeField.EDITBY,
+ ChangeField.EXACT_COMMIT,
+ ChangeField.EXACT_TOPIC,
+ ChangeField.FILE_PART,
+ ChangeField.FUZZY_TOPIC,
+ ChangeField.GROUP,
+ ChangeField.HASHTAG,
+ ChangeField.HASHTAG_CASE_AWARE,
ChangeField.ID,
- ChangeField.STATUS,
+ ChangeField.LABEL,
+ ChangeField.LEGACY_ID,
+ ChangeField.MERGEABLE,
+ ChangeField.OWNER,
+ ChangeField.PATCH_SET,
+ ChangeField.PATH,
ChangeField.PROJECT,
ChangeField.PROJECTS,
ChangeField.REF,
- ChangeField.EXACT_TOPIC,
- ChangeField.FUZZY_TOPIC,
- ChangeField.UPDATED,
- ChangeField.FILE_PART,
- ChangeField.PATH,
- ChangeField.OWNER,
- ChangeField.COMMIT,
- ChangeField.TR,
- ChangeField.LABEL,
- ChangeField.COMMIT_MESSAGE,
- ChangeField.COMMENT,
- ChangeField.CHANGE,
- ChangeField.APPROVAL,
- ChangeField.MERGEABLE,
- ChangeField.ADDED,
- ChangeField.DELETED,
- ChangeField.DELTA,
- ChangeField.HASHTAG,
- ChangeField.COMMENTBY,
- ChangeField.PATCH_SET,
- ChangeField.GROUP,
- ChangeField.SUBMISSIONID,
- ChangeField.EDITBY,
+ ChangeField.REF_STATE,
+ ChangeField.REF_STATE_PATTERN,
ChangeField.REVIEWEDBY,
- ChangeField.EXACT_COMMIT,
- ChangeField.AUTHOR,
- ChangeField.COMMITTER,
- ChangeField.DRAFTBY,
- ChangeField.HASHTAG_CASE_AWARE,
+ ChangeField.REVIEWER,
ChangeField.STAR,
ChangeField.STARBY,
- ChangeField.REVIEWER);
-
- @Deprecated static final Schema<ChangeData> V33 = schema(V32, ChangeField.ASSIGNEE);
-
- @Deprecated
- static final Schema<ChangeData> V34 =
- new Schema.Builder<ChangeData>()
- .add(V33)
- .remove(ChangeField.LABEL)
- .add(ChangeField.LABEL2)
- .build();
-
- @Deprecated
- static final Schema<ChangeData> V35 =
- schema(
- V34,
- ChangeField.SUBMIT_RECORD,
+ ChangeField.STATUS,
ChangeField.STORED_SUBMIT_RECORD_LENIENT,
- ChangeField.STORED_SUBMIT_RECORD_STRICT);
-
- @Deprecated
- static final Schema<ChangeData> V36 =
- schema(V35, ChangeField.REF_STATE, ChangeField.REF_STATE_PATTERN);
-
- @Deprecated static final Schema<ChangeData> V37 = schema(V36);
-
- @Deprecated
- static final Schema<ChangeData> V38 = schema(V37, ChangeField.UNRESOLVED_COMMENT_COUNT);
-
- @Deprecated static final Schema<ChangeData> V39 = schema(V38);
+ ChangeField.STORED_SUBMIT_RECORD_STRICT,
+ ChangeField.SUBMISSIONID,
+ ChangeField.SUBMIT_RECORD,
+ ChangeField.TR,
+ ChangeField.UNRESOLVED_COMMENT_COUNT,
+ ChangeField.UPDATED);
@Deprecated static final Schema<ChangeData> V40 = schema(V39, ChangeField.PRIVATE);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index 1189e87..bb251cb 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -42,7 +42,7 @@
private final AccountGroup.UUID group;
EqualsLabelPredicate(LabelPredicate.Args args, String label, int expVal, Account.Id account) {
- super(args.field, ChangeField.formatLabel(label, expVal, account));
+ super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account));
this.ccFactory = args.ccFactory;
this.projectCache = args.projectCache;
this.userFactory = args.userFactory;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
index 2fbaa1e..9fdbcef 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -19,8 +19,6 @@
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.index.FieldDef;
-import com.google.gerrit.server.index.change.ChangeField;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.query.OrPredicate;
@@ -37,7 +35,6 @@
private static final int MAX_LABEL_VALUE = 4;
static class Args {
- final FieldDef<ChangeData, ?> field;
final ProjectCache projectCache;
final ChangeControl.GenericFactory ccFactory;
final IdentifiedUser.GenericFactory userFactory;
@@ -47,7 +44,6 @@
final AccountGroup.UUID group;
private Args(
- FieldDef<ChangeData, ?> field,
ProjectCache projectCache,
ChangeControl.GenericFactory ccFactory,
IdentifiedUser.GenericFactory userFactory,
@@ -55,7 +51,6 @@
String value,
Set<Account.Id> accounts,
AccountGroup.UUID group) {
- this.field = field;
this.projectCache = projectCache;
this.ccFactory = ccFactory;
this.userFactory = userFactory;
@@ -80,7 +75,6 @@
private final String value;
- @SuppressWarnings("deprecation")
LabelPredicate(
ChangeQueryBuilder.Arguments a,
String value,
@@ -89,7 +83,6 @@
super(
predicates(
new Args(
- a.getSchema().getField(ChangeField.LABEL2, ChangeField.LABEL).get(),
a.projectCache,
a.changeControlGenericFactory,
a.userFactory,
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 8ad1ae2..5f11219 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
@@ -136,7 +136,7 @@
placeholder="Add assignee..."
accounts="{{_assignee}}"
change="[[change]]"
- readonly="[[!mutable]]"
+ readonly="[[_computeAssigneeReadOnly(mutable, change)]]"
allow-any-user></gr-account-list>
</span>
</section>
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 f8c189a..6caed12 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
@@ -152,6 +152,12 @@
return !mutable || !change.actions.topic || !change.actions.topic.enabled;
},
+ _computeAssigneeReadOnly: function(mutable, change) {
+ return !mutable ||
+ !change.actions.assignee ||
+ !change.actions.assignee.enabled;
+ },
+
_computeTopicPlaceholder: function(_topicReadOnly) {
return _topicReadOnly ? 'No Topic' : 'Click to add topic';
},
@@ -239,8 +245,8 @@
status = this.change.status.toLowerCase();
}
return '/q/project:' + this.encodeURL(project, false) +
- ' branch:' + this.encodeURL(branch, false) +
- ' status:' + this.encodeURL(status, false);
+ ' branch:' + this.encodeURL(branch, false) +
+ ' status:' + this.encodeURL(status, false);
},
_computeTopicHref: function(topic) {
diff --git a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
index c32f16b..e8e22f9 100644
--- a/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
+++ b/polygerrit-ui/app/elements/change/gr-change-metadata/gr-change-metadata_test.html
@@ -201,6 +201,7 @@
element.change = {
_number: 'the number',
change_id: 'the id',
+ actions: [],
topic: 'the topic',
status: 'NEW',
submit_type: 'CHERRY_PICK',
@@ -287,8 +288,15 @@
_account_id: 1,
name: 'bojack',
};
+ var change = {
+ actions: {
+ assignee: {enabled: false},
+ },
+ assignee: dummyAccount,
+ };
var deleteStub;
var setStub;
+
setup(function() {
deleteStub = sandbox.stub(element.$.restAPI, 'deleteAssignee');
setStub = sandbox.stub(element.$.restAPI, 'setAssignee');
@@ -315,6 +323,17 @@
element.set('_assignee', []);
assert.isTrue(deleteStub.calledOnce);
});
+
+ test('_computeAssigneeReadOnly', function() {
+ var mutable = false;
+ assert.isTrue(element._computeAssigneeReadOnly(mutable, change));
+ mutable = true;
+ assert.isTrue(element._computeAssigneeReadOnly(mutable, change));
+ change.actions.assignee.enabled = true;
+ assert.isFalse(element._computeAssigneeReadOnly(mutable, change));
+ mutable = false;
+ assert.isTrue(element._computeAssigneeReadOnly(mutable, change));
+ });
});
});
});
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js
index 870e7ea..1e609c9 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager.js
@@ -20,6 +20,7 @@
var SIGN_IN_WIDTH_PX = 690;
var SIGN_IN_HEIGHT_PX = 500;
var TOO_MANY_FILES = 'too many files to find conflicts';
+ var AUTHENTICATION_REQUIRED = 'Authentication required\n';
Polymer({
is: 'gr-error-manager',
@@ -73,21 +74,21 @@
},
_handleServerError: function(e) {
- if (e.detail.response.status === 403) {
- this._getLoggedIn().then(function(loggedIn) {
- if (loggedIn) {
- // The app was logged at one point and is now getting auth errors.
- // This indicates the auth token is no longer valid.
- this._showAuthErrorAlert('Auth error', 'Refresh credentials.');
- }
- }.bind(this));
- } else {
- e.detail.response.text().then(function(text) {
- if (!this._shouldSuppressError(text)) {
- this._showAlert('Server error: ' + text);
- }
- }.bind(this));
- }
+ Promise.all([
+ e.detail.response.text(), this._getLoggedIn()
+ ]).then(function(values) {
+ var text = values[0];
+ var loggedIn = values[1];
+ if (e.detail.response.status === 403 &&
+ loggedIn &&
+ text === AUTHENTICATION_REQUIRED) {
+ // The app was logged at one point and is now getting auth errors.
+ // This indicates the auth token is no longer valid.
+ this._showAuthErrorAlert('Auth error', 'Refresh credentials.');
+ } else if (!this._shouldSuppressError(text)) {
+ this._showAlert('Server error: ' + text);
+ }
+ }.bind(this));
},
_handleShowAlert: function(e) {
diff --git a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html
index 8bab82e..191d67a 100644
--- a/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html
+++ b/polygerrit-ui/app/elements/core/gr-error-manager/gr-error-manager_test.html
@@ -49,10 +49,31 @@
sandbox.restore();
});
- test('show auth error', function(done) {
+ test('does not show auth error on 403 by default', function(done) {
var showAuthErrorStub = sandbox.stub(element, '_showAuthErrorAlert');
- element.fire('server-error', {response: {status: 403}});
- element.$.restAPI.getLoggedIn.lastCall.returnValue.then(function() {
+ var responseText = Promise.resolve('server says no.');
+ element.fire('server-error',
+ {response: {status: 403, text: function() { return responseText; }}}
+ );
+ Promise.all([
+ element.$.restAPI.getLoggedIn.lastCall.returnValue,
+ responseText,
+ ]).then(function() {
+ assert.isFalse(showAuthErrorStub.calledOnce);
+ done();
+ });
+ });
+
+ test('shows auth error on 403 and Authentication required', function(done) {
+ var showAuthErrorStub = sandbox.stub(element, '_showAuthErrorAlert');
+ var responseText = Promise.resolve('Authentication required\n');
+ element.fire('server-error',
+ {response: {status: 403, text: function() { return responseText; }}}
+ );
+ Promise.all([
+ element.$.restAPI.getLoggedIn.lastCall.returnValue,
+ responseText,
+ ]).then(function() {
assert.isTrue(showAuthErrorStub.calledOnce);
done();
});
@@ -71,7 +92,10 @@
element.fire('server-error', {response: {status: 500, text: textSpy}});
assert.isTrue(textSpy.called);
- textSpy.lastCall.returnValue.then(function() {
+ Promise.all([
+ element.$.restAPI.getLoggedIn.lastCall.returnValue,
+ textSpy.lastCall.returnValue,
+ ]).then(function() {
assert.isTrue(showAlertStub.calledOnce);
assert.isTrue(showAlertStub.lastCall.calledWithExactly(
'Server error: ZOMG'));
@@ -87,7 +111,10 @@
element.fire('server-error', {response: {status: 500, text: textSpy}});
assert.isTrue(textSpy.called);
- textSpy.lastCall.returnValue.then(function() {
+ Promise.all([
+ element.$.restAPI.getLoggedIn.lastCall.returnValue,
+ textSpy.lastCall.returnValue,
+ ]).then(function() {
assert.isFalse(showAlertStub.called);
done();
});
@@ -112,8 +139,14 @@
function() { return Promise.resolve(true); });
var toastSpy = sandbox.spy(element, '_createToastAlert');
var windowOpen = sandbox.stub(window, 'open');
- element.fire('server-error', {response: {status: 403}});
- element.$.restAPI.getLoggedIn.lastCall.returnValue.then(function() {
+ var responseText = Promise.resolve('Authentication required\n');
+ element.fire('server-error',
+ {response: {status: 403, text: function() { return responseText; }}}
+ );
+ Promise.all([
+ element.$.restAPI.getLoggedIn.lastCall.returnValue,
+ responseText,
+ ]).then(function() {
assert.isTrue(toastSpy.called);
var toast = toastSpy.lastCall.returnValue;
assert.isOk(toast);
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
index 838938b..368c613 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder-image.js
@@ -45,7 +45,7 @@
tr.appendChild(this._createElement('td'));
tr.appendChild(this._createImageCell(
- this._revisionImage, 'right', section));
+ this._revisionImage, 'right', section));
section.appendChild(tr);
};
@@ -61,13 +61,13 @@
this._updateImageLabel(section, className, image);
}.bind(this);
imageEl.src = 'data:' + image.type + ';base64, ' + image.body;
- imageEl.addEventListener('error', function(e) {
+ imageEl.addEventListener('error', function() {
imageEl.remove();
td.textContent = '[Image failed to load]';
});
td.appendChild(imageEl);
- return td;
}
+ return td;
};
GrDiffBuilderImage.prototype._updateImageLabel =
@@ -86,7 +86,8 @@
var addNamesInLabel = false;
- if (this._baseImage._name !== this._revisionImage._name) {
+ if (this._baseImage && this._revisionImage &&
+ this._baseImage._name !== this._revisionImage._name) {
addNamesInLabel = true;
}
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
index ad7a260..3db2295 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff_test.html
@@ -338,7 +338,6 @@
function() { return Promise.resolve(mockComments); }));
element.patchRange = {basePatchNum: 'PARENT', patchNum: 1};
-
});
test('renders image diffs with same file name', function(done) {
@@ -500,6 +499,84 @@
element.reload();
});
});
+
+ test('renders added image', function(done) {
+ var mockDiff = {
+ meta_b: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'ADDED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index 0000000..f9c2f2c 100644',
+ '--- /dev/null',
+ '+++ b/carrot.jpg',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ stubs.push(sandbox.stub(element, '_getDiff',
+ function() { return Promise.resolve(mockDiff); }));
+
+ element.addEventListener('render', function() {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diffBuilder._builder, GrDiffBuilderImage);
+
+ var leftImage = element.$.diffTable.querySelector('td.left img');
+ var rightImage = element.$.diffTable.querySelector('td.right img');
+
+ assert.isNotOk(leftImage);
+ assert.isOk(rightImage);
+ done();
+ });
+
+ element.$.restAPI.getDiffPreferences().then(function(prefs) {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
+
+ test('renders removed image', function(done) {
+ var mockDiff = {
+ meta_a: {name: 'carrot.jpg', content_type: 'image/jpeg',
+ lines: 560},
+ intraline_status: 'OK',
+ change_type: 'DELETED',
+ diff_header: [
+ 'diff --git a/carrot.jpg b/carrot.jpg',
+ 'index f9c2f2c..0000000 100644',
+ '--- a/carrot.jpg',
+ '+++ /dev/null',
+ 'Binary files differ',
+ ],
+ content: [{skip: 66}],
+ binary: true,
+ };
+ stubs.push(sandbox.stub(element, '_getDiff',
+ function() { return Promise.resolve(mockDiff); }));
+
+ element.addEventListener('render', function() {
+ // Recognizes that it should be an image diff.
+ assert.isTrue(element.isImageDiff);
+ assert.instanceOf(
+ element.$.diffBuilder._builder, GrDiffBuilderImage);
+
+ var leftImage = element.$.diffTable.querySelector('td.left img');
+ var rightImage = element.$.diffTable.querySelector('td.right img');
+
+ assert.isOk(leftImage);
+ assert.isNotOk(rightImage);
+ done();
+ });
+
+ element.$.restAPI.getDiffPreferences().then(function(prefs) {
+ element.prefs = prefs;
+ element.reload();
+ });
+ });
});
test('_handleTap lineNum', function(done) {
