Merge "Fix ignoring change when project is watched"
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index d4fa912..13fca66 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1948,7 +1948,7 @@
--
Update star labels on a change. The star labels to be added/removed
-must be specified in the request body as link:#star-input[StarInput]
+must be specified in the request body as link:#stars-input[StarsInput]
entity. Starred changes are returned for the search query `has:stars`.
.Request
diff --git a/WORKSPACE b/WORKSPACE
index 4b8c90a..2a24c5a 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -130,22 +130,22 @@
sha1 = "cdb2dcb4e22b83d6b32b93095f644c3462739e82",
)
-load("//lib/jgit:jgit.bzl", "JGIT_VERS")
+load("//lib/jgit:jgit.bzl", "JGIT_VERS", "JGIT_REPO", "JGIT_SHA1", "JGIT_SRC_SHA1", "JGIT_SERVLET_SHA1", "JGIT_ARCHIVE_SHA1", "JGIT_JUNIT_SHA1")
maven_jar(
name = "jgit",
artifact = "org.eclipse.jgit:org.eclipse.jgit:" + JGIT_VERS,
- repository = GERRIT,
- sha1 = "a2b5970b853f8fee64589fc1103c0ceb7677ba63",
- src_sha1 = "765f955774c36c226aa41fba7c20119451de2db7",
+ repository = JGIT_REPO,
+ sha1 = JGIT_SHA1,
+ src_sha1 = JGIT_SRC_SHA1,
unsign = True,
)
maven_jar(
name = "jgit_servlet",
artifact = "org.eclipse.jgit:org.eclipse.jgit.http.server:" + JGIT_VERS,
- repository = GERRIT,
- sha1 = "d3aa54bd610db9a5c246aa8fef13989982c98628",
+ repository = JGIT_REPO,
+ sha1 = JGIT_SERVLET_SHA1,
unsign = True,
)
@@ -159,15 +159,15 @@
maven_jar(
name = "jgit_archive",
artifact = "org.eclipse.jgit:org.eclipse.jgit.archive:" + JGIT_VERS,
- repository = GERRIT,
- sha1 = "a728cf277396f1227c5a8dffcf5dee0188fc0821",
+ repository = JGIT_REPO,
+ sha1 = JGIT_ARCHIVE_SHA1,
)
maven_jar(
name = "jgit_junit",
artifact = "org.eclipse.jgit:org.eclipse.jgit.junit:" + JGIT_VERS,
- repository = GERRIT,
- sha1 = "6c2b2f192c95d25a2e1576aee5d1169dd8bd2266",
+ repository = JGIT_REPO,
+ sha1 = JGIT_JUNIT_SHA1,
unsign = True,
)
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index a1bd747..31ca9df 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -44,6 +44,7 @@
import com.google.gerrit.extensions.client.InheritableBoolean;
import com.google.gerrit.extensions.client.ListChangesOption;
import com.google.gerrit.extensions.client.ProjectWatchInfo;
+import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.common.EditInfo;
@@ -505,6 +506,8 @@
cr = ci.labels.get("Code-Review");
assertThat(Iterables.getLast(ci.messages).message)
.isEqualTo("Uploaded patch set 2: Code-Review+2.");
+ // Check that the user who pushed the change was added as a reviewer since they added a vote
+ assertThatUserIsOnlyReviewer(ci, admin);
assertThat(cr.all).hasSize(1);
assertThat(cr.all.get(0).name).isEqualTo("Administrator");
@@ -524,6 +527,36 @@
assertThat(Iterables.getLast(ci.messages).message).isEqualTo("Uploaded patch set 3.");
}
+ @Test
+ public void pushNewPatchSetForMasterWithApprovals() throws Exception {
+ PushOneCommit.Result r = pushTo("refs/for/master");
+ r.assertOkStatus();
+
+ PushOneCommit push =
+ pushFactory.create(
+ db,
+ admin.getIdent(),
+ testRepo,
+ PushOneCommit.SUBJECT,
+ "b.txt",
+ "anotherContent",
+ r.getChangeId());
+ r = push.to("refs/for/master/%l=Code-Review+2");
+
+ ChangeInfo ci = get(r.getChangeId());
+ LabelInfo cr = ci.labels.get("Code-Review");
+ assertThat(Iterables.getLast(ci.messages).message)
+ .isEqualTo("Uploaded patch set 2: Code-Review+2.");
+
+ // Check that the user who pushed the new patch set was added as a reviewer since they added
+ // a vote
+ assertThatUserIsOnlyReviewer(ci, admin);
+
+ assertThat(cr.all).hasSize(1);
+ assertThat(cr.all.get(0).name).isEqualTo("Administrator");
+ assertThat(cr.all.get(0).value).isEqualTo(2);
+ }
+
/**
* There was a bug that allowed a user with Forge Committer Identity access right to upload a
* commit and put *votes on behalf of another user* on it. This test checks that this is not
@@ -566,6 +599,8 @@
assertThat(cr.all.get(indexUser).value.intValue()).isEqualTo(0);
assertThat(Iterables.getLast(ci.messages).message)
.isEqualTo("Uploaded patch set 1: Code-Review+1.");
+ // Check that the user who pushed the change was added as a reviewer since they added a vote
+ assertThatUserIsOnlyReviewer(ci, admin);
}
@Test
@@ -594,6 +629,8 @@
assertThat(cr.all).hasSize(1);
cr = ci.labels.get("Custom-Label");
assertThat(cr.all).hasSize(1);
+ // Check that the user who pushed the change was added as a reviewer since they added a vote
+ assertThatUserIsOnlyReviewer(ci, admin);
}
@Test
@@ -1366,6 +1403,13 @@
assertThat(info.status).isEqualTo(ChangeStatus.NEW);
}
+ private void assertThatUserIsOnlyReviewer(ChangeInfo ci, TestAccount reviewer) {
+ assertThat(ci.reviewers).isNotNull();
+ assertThat(ci.reviewers.keySet()).containsExactly(ReviewerState.REVIEWER);
+ assertThat(ci.reviewers.get(ReviewerState.REVIEWER).iterator().next().email)
+ .isEqualTo(reviewer.email);
+ }
+
private void pushWithReviewerInFooter(String nameEmail, TestAccount expectedReviewer)
throws Exception {
int n = 5;
diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/AuthInfo.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/AuthInfo.java
index 2f6ef79..c0ece8c 100644
--- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/AuthInfo.java
+++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/AuthInfo.java
@@ -67,8 +67,11 @@
public final List<AgreementInfo> contributorAgreements() {
List<AgreementInfo> agreements = new ArrayList<>();
- for (AgreementInfo a : Natives.asList(_contributorAgreements())) {
- agreements.add(a);
+ JsArray<AgreementInfo> contributorAgreements = _contributorAgreements();
+ if (contributorAgreements != null) {
+ for (AgreementInfo a : Natives.asList(contributorAgreements)) {
+ agreements.add(a);
+ }
}
return agreements;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
index 8710e91..d013120 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/DeleteActive.java
@@ -25,12 +25,13 @@
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.DeleteActive.Input;
+import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Collections;
+import java.util.concurrent.atomic.AtomicBoolean;
@RequiresCapability(GlobalCapability.MODIFY_ACCOUNT)
@Singleton
@@ -52,18 +53,34 @@
@Override
public Response<?> apply(AccountResource rsrc, Input input)
throws RestApiException, OrmException, IOException {
- Account a = dbProvider.get().accounts().get(rsrc.getUser().getAccountId());
- if (a == null) {
- throw new ResourceNotFoundException("account not found");
- }
- if (!a.isActive()) {
- throw new ResourceConflictException("account not active");
- }
if (self.get() == rsrc.getUser()) {
throw new ResourceConflictException("cannot deactivate own account");
}
- a.setActive(false);
- dbProvider.get().accounts().update(Collections.singleton(a));
+
+ AtomicBoolean alreadyInactive = new AtomicBoolean(false);
+ Account a =
+ dbProvider
+ .get()
+ .accounts()
+ .atomicUpdate(
+ rsrc.getUser().getAccountId(),
+ new AtomicUpdate<Account>() {
+ @Override
+ public Account update(Account a) {
+ if (!a.isActive()) {
+ alreadyInactive.set(true);
+ } else {
+ a.setActive(false);
+ }
+ return a;
+ }
+ });
+ if (a == null) {
+ throw new ResourceNotFoundException("account not found");
+ }
+ if (alreadyInactive.get()) {
+ throw new ResourceConflictException("account not active");
+ }
byIdCache.evict(a.getId());
return Response.none();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
index cbddefd..32c5345 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutActive.java
@@ -22,12 +22,14 @@
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.PutActive.Input;
+import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections;
+import java.util.concurrent.atomic.AtomicBoolean;
@RequiresCapability(GlobalCapability.MODIFY_ACCOUNT)
@Singleton
@@ -46,16 +48,29 @@
@Override
public Response<String> apply(AccountResource rsrc, Input input)
throws ResourceNotFoundException, OrmException, IOException {
- Account a = dbProvider.get().accounts().get(rsrc.getUser().getAccountId());
+ AtomicBoolean alreadyActive = new AtomicBoolean(false);
+ Account a =
+ dbProvider
+ .get()
+ .accounts()
+ .atomicUpdate(
+ rsrc.getUser().getAccountId(),
+ new AtomicUpdate<Account>() {
+ @Override
+ public Account update(Account a) {
+ if (a.isActive()) {
+ alreadyActive.set(true);
+ } else {
+ a.setActive(true);
+ }
+ return a;
+ }
+ });
if (a == null) {
throw new ResourceNotFoundException("account not found");
}
- if (a.isActive()) {
- return Response.ok("");
- }
- a.setActive(true);
dbProvider.get().accounts().update(Collections.singleton(a));
byIdCache.evict(a.getId());
- return Response.created("");
+ return alreadyActive.get() ? Response.ok("") : Response.created("");
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
index 726508f..443a549 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutName.java
@@ -27,12 +27,12 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.PutName.Input;
+import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Collections;
@Singleton
public class PutName implements RestModifyView<AccountResource, Input> {
@@ -77,12 +77,23 @@
throw new MethodNotAllowedException("realm does not allow editing name");
}
- Account a = dbProvider.get().accounts().get(user.getAccountId());
+ String newName = input.name;
+ Account a =
+ dbProvider
+ .get()
+ .accounts()
+ .atomicUpdate(
+ user.getAccountId(),
+ new AtomicUpdate<Account>() {
+ @Override
+ public Account update(Account a) {
+ a.setFullName(newName);
+ return a;
+ }
+ });
if (a == null) {
throw new ResourceNotFoundException("account not found");
}
- a.setFullName(input.name);
- dbProvider.get().accounts().update(Collections.singleton(a));
byIdCache.evict(a.getId());
return Strings.isNullOrEmpty(a.getFullName())
? Response.<String>none()
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
index 3c80d2c..ec60fb3 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutPreferred.java
@@ -23,12 +23,14 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.PutPreferred.Input;
+import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Collections;
+import java.util.concurrent.atomic.AtomicBoolean;
@Singleton
public class PutPreferred implements RestModifyView<AccountResource.Email, Input> {
@@ -56,16 +58,29 @@
public Response<String> apply(IdentifiedUser user, String email)
throws ResourceNotFoundException, OrmException, IOException {
- Account a = dbProvider.get().accounts().get(user.getAccountId());
+ AtomicBoolean alreadyPreferred = new AtomicBoolean(false);
+ Account a =
+ dbProvider
+ .get()
+ .accounts()
+ .atomicUpdate(
+ user.getAccountId(),
+ new AtomicUpdate<Account>() {
+ @Override
+ public Account update(Account a) {
+ if (email.equals(a.getPreferredEmail())) {
+ alreadyPreferred.set(true);
+ } else {
+ a.setPreferredEmail(email);
+ }
+ return a;
+ }
+ });
if (a == null) {
throw new ResourceNotFoundException("account not found");
}
- if (email.equals(a.getPreferredEmail())) {
- return Response.ok("");
- }
- a.setPreferredEmail(email);
dbProvider.get().accounts().update(Collections.singleton(a));
byIdCache.evict(a.getId());
- return Response.created("");
+ return alreadyPreferred.get() ? Response.ok("") : Response.created("");
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
index d826899..ff541fd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/PutStatus.java
@@ -25,12 +25,12 @@
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.PutStatus.Input;
+import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
import java.io.IOException;
-import java.util.Collections;
@Singleton
public class PutStatus implements RestModifyView<AccountResource, Input> {
@@ -70,12 +70,23 @@
input = new Input();
}
- Account a = dbProvider.get().accounts().get(user.getAccountId());
+ String newStatus = input.status;
+ Account a =
+ dbProvider
+ .get()
+ .accounts()
+ .atomicUpdate(
+ user.getAccountId(),
+ new AtomicUpdate<Account>() {
+ @Override
+ public Account update(Account a) {
+ a.setStatus(Strings.nullToEmpty(newStatus));
+ return a;
+ }
+ });
if (a == null) {
throw new ResourceNotFoundException("account not found");
}
- a.setStatus(Strings.nullToEmpty(input.status));
- dbProvider.get().accounts().update(Collections.singleton(a));
byIdCache.evict(a.getId());
return Strings.isNullOrEmpty(a.getStatus()) ? Response.none() : Response.ok(a.getStatus());
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
index 9983928..e929382 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -17,6 +17,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.gerrit.reviewdb.client.Change.INITIAL_PATCH_SET_ID;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import static java.util.stream.Collectors.toSet;
import com.google.common.base.MoreObjects;
@@ -390,6 +391,12 @@
Collections.<Account.Id>emptySet());
approvalsUtil.addApprovalsForNewPatchSet(
db, update, labelTypes, patchSet, ctx.getControl(), approvals);
+ // Check if approvals are changing in with this update. If so, add current user to reviewers.
+ // Note that this is done separately as addReviewers is filtering out the change owner as
+ // reviewer which is needed in several other code paths.
+ if (!approvals.isEmpty()) {
+ update.putReviewer(ctx.getAccountId(), REVIEWER);
+ }
if (message != null) {
changeMessage =
ChangeMessagesUtil.newMessage(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
index 3cd3c1e..a3bd97c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReplaceOp.java
@@ -17,6 +17,7 @@
import static com.google.gerrit.common.FooterConstants.CHANGE_ID;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromReviewers;
+import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
@@ -280,6 +281,14 @@
info,
recipients.getReviewers(),
oldRecipients.getAll());
+
+ // Check if approvals are changing in with this update. If so, add current user to reviewers.
+ // Note that this is done separately as addReviewers is filtering out the change owner as
+ // reviewer which is needed in several other code paths.
+ if (magicBranch != null && !magicBranch.labels.isEmpty()) {
+ update.putReviewer(ctx.getAccountId(), REVIEWER);
+ }
+
recipients.add(oldRecipients);
String approvalMessage =
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index 2b90306..730b710 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -147,23 +147,22 @@
//
removeUser(fromUser);
}
-
- // Check the preferences of all recipients. If any user has disabled
- // his email notifications then drop him from recipients' list.
- // In addition, check if users only want to receive plaintext email.
- for (Account.Id id : rcptTo) {
- Account thisUser = args.accountCache.get(id).getAccount();
- GeneralPreferencesInfo prefs = thisUser.getGeneralPreferencesInfo();
- if (prefs == null || prefs.getEmailStrategy() == DISABLED) {
- removeUser(thisUser);
- } else if (useHtml() && prefs.getEmailFormat() == EmailFormat.PLAINTEXT) {
- removeUser(thisUser);
- smtpRcptToPlaintextOnly.add(
- new Address(thisUser.getFullName(), thisUser.getPreferredEmail()));
- }
- if (smtpRcptTo.isEmpty() && smtpRcptToPlaintextOnly.isEmpty()) {
- return;
- }
+ }
+ // Check the preferences of all recipients. If any user has disabled
+ // his email notifications then drop him from recipients' list.
+ // In addition, check if users only want to receive plaintext email.
+ for (Account.Id id : rcptTo) {
+ Account thisUser = args.accountCache.get(id).getAccount();
+ GeneralPreferencesInfo prefs = thisUser.getGeneralPreferencesInfo();
+ if (prefs == null || prefs.getEmailStrategy() == DISABLED) {
+ removeUser(thisUser);
+ } else if (useHtml() && prefs.getEmailFormat() == EmailFormat.PLAINTEXT) {
+ removeUser(thisUser);
+ smtpRcptToPlaintextOnly.add(
+ new Address(thisUser.getFullName(), thisUser.getPreferredEmail()));
+ }
+ if (smtpRcptTo.isEmpty() && smtpRcptToPlaintextOnly.isEmpty()) {
+ return;
}
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
index e956be3..4488c71 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/ChangeArgumentParser.java
@@ -80,10 +80,13 @@
List<ChangeControl> matched =
useIndex ? changeFinder.find(id, currentUser) : changeFromNotesFactory(id, currentUser);
List<ChangeControl> toAdd = new ArrayList<>(changes.size());
+ boolean canMaintainServer =
+ currentUser.isIdentifiedUser()
+ && currentUser.asIdentifiedUser().getCapabilities().canMaintainServer();
for (ChangeControl ctl : matched) {
if (!changes.containsKey(ctl.getId())
&& inProject(projectControl, ctl.getProject())
- && ctl.isVisible(db)) {
+ && (canMaintainServer || ctl.isVisible(db))) {
toAdd.add(ctl);
}
}
diff --git a/lib/GUAVA_VERSION b/lib/GUAVA_VERSION
deleted file mode 100644
index b5f47b3..0000000
--- a/lib/GUAVA_VERSION
+++ /dev/null
@@ -1 +0,0 @@
-include_defs('//lib/guava.bzl')
diff --git a/lib/JGIT_VERSION b/lib/JGIT_VERSION
deleted file mode 100644
index 87a625f..0000000
--- a/lib/JGIT_VERSION
+++ /dev/null
@@ -1,4 +0,0 @@
-include_defs('//lib/jgit/jgit.bzl')
-include_defs('//lib/maven.defs')
-
-REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL.
diff --git a/lib/jgit/jgit.bzl b/lib/jgit/jgit.bzl
index 99f0432..ac843d8 100644
--- a/lib/jgit/jgit.bzl
+++ b/lib/jgit/jgit.bzl
@@ -1,5 +1,19 @@
+load("//tools/bzl:maven_jar.bzl", "GERRIT", "MAVEN_LOCAL", "MAVEN_CENTRAL")
+
JGIT_VERS = "4.6.0.201612231935-r.30-gd3148f300"
DOC_VERS = "4.6.0.201612231935-r" # Set to JGIT_VERS unless using a snapshot
JGIT_DOC_URL = "http://download.eclipse.org/jgit/site/" + DOC_VERS + "/apidocs"
+
+JGIT_REPO = GERRIT # Leave here even if set to MAVEN_CENTRAL.
+
+JGIT_SHA1 = "a2b5970b853f8fee64589fc1103c0ceb7677ba63"
+
+JGIT_SRC_SHA1 = "765f955774c36c226aa41fba7c20119451de2db7"
+
+JGIT_SERVLET_SHA1 = "d3aa54bd610db9a5c246aa8fef13989982c98628"
+
+JGIT_ARCHIVE_SHA1 = "a728cf277396f1227c5a8dffcf5dee0188fc0821"
+
+JGIT_JUNIT_SHA1 = "6c2b2f192c95d25a2e1576aee5d1169dd8bd2266"
diff --git a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
index acbb367..8e2b192 100644
--- a/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
+++ b/polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.html
@@ -141,7 +141,9 @@
</td>
<td class="cell updated"
hidden$="[[isColumnHidden('Updated', visibleChangeTableColumns)]]">
- <gr-date-formatter date-str="[[change.updated]]"></gr-date-formatter>
+ <gr-date-formatter
+ has-tooltip
+ date-str="[[change.updated]]"></gr-date-formatter>
</td>
<td class="cell size u-monospace"
hidden$="[[isColumnHidden('Size', visibleChangeTableColumns)]]">
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 4e362e5..932b914 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
@@ -99,6 +99,7 @@
<span class="title">Updated</span>
<span class="value">
<gr-date-formatter
+ has-tooltip
date-str="[[change.updated]]"></gr-date-formatter>
</span>
</section>
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
index 3f5f64f..14361f4 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.html
@@ -73,14 +73,14 @@
</div>
<span
id="messageControlsContainer"
- hidden$="[[_computeShowHideTextHidden(_visibleMessages.length, _processedMessages, _hideAutomated)]]">
+ hidden$="[[_computeShowHideTextHidden(_visibleMessages, _processedMessages, _hideAutomated, _visibleMessages.length)]]">
<gr-button id="oldMessagesBtn" link on-tap="_handleShowAllTap">
- [[_computeNumMessagesText(_visibleMessages.length, _processedMessages, _hideAutomated)]]
+ [[_computeNumMessagesText(_visibleMessages, _processedMessages, _hideAutomated, _visibleMessages.length)]]
</gr-button>
/
<gr-button id="incrementMessagesBtn" link
on-tap="_handleIncrementShownMessages">
- [[_computeIncrementText(_visibleMessages.length, _processedMessages, _hideAutomated)]]
+ [[_computeIncrementText(_visibleMessages, _processedMessages, _hideAutomated, _visibleMessages.length)]]
</gr-button>
</span>
<template
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
index 0c3e126..0d58d96 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list.js
@@ -255,9 +255,9 @@
* remaining in the list and the number of messages needed to display five
* more visible messages in the list.
*/
- _getDelta: function(numVisible, messages, hideAutomated) {
+ _getDelta: function(visibleMessages, messages, hideAutomated) {
var delta = MESSAGES_INCREMENT;
- var msgsRemaining = messages.length - numVisible;
+ var msgsRemaining = messages.length - visibleMessages.length;
if (hideAutomated) {
var counter = 0;
var i;
@@ -273,29 +273,34 @@
* Gets the number of messages that would be visible, but do not currently
* exist in _visibleMessages.
*/
- _numRemaining: function(numVisible, messages, hideAutomated) {
- var total = hideAutomated ?
- messages.filter(function(msg) {
- return !this._isAutomated(msg);
- }.bind(this)).length :
- messages.length;
- return total - numVisible;
+ _numRemaining: function(visibleMessages, messages, hideAutomated) {
+ if (hideAutomated) {
+ return this._getHumanMessages(messages).length -
+ this._getHumanMessages(visibleMessages).length;
+ }
+ return messages.length - visibleMessages.length;
},
- _computeIncrementText: function(numVisible, messages, hideAutomated) {
- var delta = this._getDelta(numVisible, messages, hideAutomated);
+ _computeIncrementText: function(visibleMessages, messages, hideAutomated) {
+ var delta = this._getDelta(visibleMessages, messages, hideAutomated);
delta = Math.min(
- this._numRemaining(numVisible, messages, hideAutomated), delta);
+ this._numRemaining(visibleMessages, messages, hideAutomated), delta);
return 'Show ' + Math.min(MESSAGES_INCREMENT, delta) + ' more';
},
- _computeShowHideTextHidden: function(numVisible, messages, hideAutomated) {
- if (numVisible >= messages.length) { return true; }
- if (!hideAutomated) {
- return numVisible >= messages.length;
+ _getHumanMessages: function(messages) {
+ return messages.filter(function(msg) {
+ return !this._isAutomated(msg);
+ }.bind(this));
+ },
+
+ _computeShowHideTextHidden: function(visibleMessages, messages,
+ hideAutomated) {
+ if (hideAutomated) {
+ messages = this._getHumanMessages(messages);
+ visibleMessages = this._getHumanMessages(visibleMessages);
}
- var hiddenMessages = messages.slice(0, messages.length - numVisible);
- return this._hasAutomatedMessages(hiddenMessages);
+ return visibleMessages.length >= messages.length;
},
_handleShowAllTap: function() {
@@ -304,9 +309,9 @@
},
_handleIncrementShownMessages: function() {
- var len = this._visibleMessages.length;
- var delta = this._getDelta(len, this._processedMessages,
+ var delta = this._getDelta(this._visibleMessages, this._processedMessages,
this._hideAutomated);
+ var len = this._visibleMessages.length;
var newMessages = this._processedMessages.slice(-(len + delta), -len);
// Add newMessages to the beginning of _visibleMessages
this.splice.apply(this, ['_visibleMessages', 0, 0].concat(newMessages));
@@ -317,8 +322,9 @@
this._visibleMessages = messages.slice(-MAX_INITIAL_SHOWN_MESSAGES);
},
- _computeNumMessagesText: function(numVisible, messages, hideAutomated) {
- var total = this._numRemaining(numVisible, messages, hideAutomated);
+ _computeNumMessagesText: function(visibleMessages, messages,
+ hideAutomated) {
+ var total = this._numRemaining(visibleMessages, messages, hideAutomated);
return total === 1 ? 'Show 1 message' : 'Show all ' + total + ' messages';
},
});
diff --git a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
index b8acae3..7026d3c 100644
--- a/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
+++ b/polygerrit-ui/app/elements/change/gr-messages-list/gr-messages-list_test.html
@@ -128,6 +128,20 @@
assert.isTrue(element.$.messageControlsContainer.hasAttribute('hidden'));
});
+ test('message count still respects non-automated on toggle', function() {
+ element.messages = _.times(10, randomMessage)
+ .concat(_.times(11, randomAutomated));
+ flushAsynchronousOperations();
+
+ assert.equal(element.$.oldMessagesBtn.innerText, 'Show 1 message');
+ assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
+ MockInteractions.tap(element.$.automatedMessageToggle);
+ flushAsynchronousOperations();
+
+ assert.equal(element.$.oldMessagesBtn.innerText, 'Show 1 message');
+ assert.isFalse(element.$.messageControlsContainer.hasAttribute('hidden'));
+ });
+
test('show all messages respects expand', function() {
element.messages = _.times(10, randomAutomated)
.concat(_.times(11, randomMessage));
@@ -450,19 +464,34 @@
test('_getDelta', function() {
var messages = [randomMessage()];
- assert.equal(element._getDelta(0, messages, false), 1);
- assert.equal(element._getDelta(0, messages, true), 1);
+ assert.equal(element._getDelta([], messages, false), 1);
+ assert.equal(element._getDelta([], messages, true), 1);
messages = _.times(7, randomMessage);
- assert.equal(element._getDelta(0, messages, false), 5);
- assert.equal(element._getDelta(0, messages, true), 5);
+ assert.equal(element._getDelta([], messages, false), 5);
+ assert.equal(element._getDelta([], messages, true), 5);
messages = _.times(4, randomMessage)
.concat(_.times(2, randomAutomated))
.concat(_.times(3, randomMessage));
- assert.equal(element._getDelta(2, messages, false), 5);
- assert.equal(element._getDelta(2, messages, true), 7);
+
+ var dummyArr = _.times(2, randomMessage);
+ assert.equal(element._getDelta(dummyArr, messages, false), 5);
+ assert.equal(element._getDelta(dummyArr, messages, true), 7);
+ });
+
+ test('_getHumanMessages', function() {
+ assert.equal(
+ element._getHumanMessages(_.times(5, randomAutomated)).length, 0);
+ assert.equal(
+ element._getHumanMessages(_.times(5, randomMessage)).length, 5);
+
+ var messages = _.shuffle(_.times(5, randomMessage)
+ .concat(_.times(5, randomAutomated)));
+ messages = element._getHumanMessages(messages);
+ assert.equal(messages.length, 5);
+ assert.isFalse(element._hasAutomatedMessages(messages));
});
});
</script>
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
index e3a3327..749ca12 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff-comment/gr-diff-comment.html
@@ -189,7 +189,9 @@
<span class="collapsedContent">[[comment.message]]</span>
</div>
<a class="date" href$="[[_computeLinkToComment(comment)]]" on-tap="_handleLinkTap">
- <gr-date-formatter date-str="[[comment.updated]]"></gr-date-formatter>
+ <gr-date-formatter
+ has-tooltip
+ date-str="[[comment.updated]]"></gr-date-formatter>
</a>
<div class="show-hide">
<label class="show-hide">
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
index c135caa..e40ccf3 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-cursor/gr-diff-cursor.js
@@ -127,7 +127,10 @@
},
moveToNextChunk: function() {
- this.$.cursorManager.next(this._isFirstRowOfChunk.bind(this));
+ this.$.cursorManager.next(this._isFirstRowOfChunk.bind(this),
+ function(target) {
+ return target.parentNode.scrollHeight;
+ });
this._fixSide();
},
diff --git a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.html b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.html
index 1ff1f96..defcf18 100644
--- a/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.html
+++ b/polygerrit-ui/app/elements/settings/gr-account-info/gr-account-info.html
@@ -37,6 +37,7 @@
<span class="title">Registered</span>
<span class="value">
<gr-date-formatter
+ has-tooltip
date-str="[[_account.registered_on]]"></gr-date-formatter>
</span>
</section>
diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
index c3542ba..54aafb8 100644
--- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
+++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html
@@ -157,8 +157,8 @@
<select
is="gr-select"
bind-value="{{_localPrefs.time_format}}">
- <option value="HHMM_12">4:10 PM (PST)</option>
- <option value="HHMM_24">16:10 (PST)</option>
+ <option value="HHMM_12">4:10 PM</option>
+ <option value="HHMM_24">16:10</option>
</select>
</span>
</section>
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
index 63a1a7d..9bfdcfb 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager.js
@@ -35,6 +35,10 @@
notify: true,
observer: '_scrollToTarget',
},
+ /**
+ * The height of content intended to be included with the target.
+ */
+ _targetHeight: Number,
/**
* The index of the current target (if any). -1 otherwise.
@@ -76,8 +80,8 @@
this.unsetCursor();
},
- next: function(opt_condition) {
- this._moveCursor(1, opt_condition);
+ next: function(opt_condition, opt_getTargetHeight) {
+ this._moveCursor(1, opt_condition, opt_getTargetHeight);
},
previous: function(opt_condition) {
@@ -109,6 +113,7 @@
this._unDecorateTarget();
this.index = -1;
this.target = null;
+ this._targetHeight = null;
},
isAtStart: function() {
@@ -136,9 +141,12 @@
* @param {Function} opt_condition Optional stop condition. If a condition
* is passed the cursor will continue to move in the specified direction
* until the condition is met.
+ * @param {Function} opt_getTargetHeight Optional function to calculate the
+ * height of the target's 'section'. The height of the target itself is
+ * sometimes different, used by the diff cursor.
* @private
*/
- _moveCursor: function(delta, opt_condition) {
+ _moveCursor: function(delta, opt_condition, opt_getTargetHeight) {
if (!this.stops.length) {
this.unsetCursor();
return;
@@ -153,6 +161,12 @@
newTarget = this.stops[newIndex];
}
+ if (opt_getTargetHeight) {
+ this._targetHeight = opt_getTargetHeight(newTarget);
+ } else {
+ this._targetHeight = newTarget.scrollHeight;
+ }
+
this.index = newIndex;
this.target = newTarget;
@@ -245,22 +259,35 @@
top < window.pageYOffset + window.innerHeight;
},
+ _calculateScrollToValue: function(top, target) {
+ return top - (window.innerHeight / 3) + (target.offsetHeight / 2);
+ },
+
_scrollToTarget: function() {
if (!this.target || this.scrollBehavior === ScrollBehavior.NEVER) {
return;
}
var top = this._getTop(this.target);
+ var bottomIsVisible = this._targetHeight ?
+ this._targetIsVisible(top + this._targetHeight) : true;
+ var scrollToValue = this._calculateScrollToValue(top, this.target);
+
if (this._targetIsVisible(top)) {
- return;
+ // Don't scroll if either the bottom is visible or if the position that
+ // would get scrolled to is higher up than the current position. this
+ // woulld cause less of the target content to be displayed than is
+ // already.
+ if (bottomIsVisible || scrollToValue < window.scrollY) {
+ return;
+ }
}
// Scroll the element to the middle of the window. Dividing by a third
// instead of half the inner height feels a bit better otherwise the
// element appears to be below the center of the window even when it
// isn't.
- window.scrollTo(0, top - (window.innerHeight / 3) +
- (this.target.offsetHeight / 2));
+ window.scrollTo(0, scrollToValue);
},
});
})();
diff --git a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.html b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.html
index 0f48501..d7496c7 100644
--- a/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-cursor-manager/gr-cursor-manager_test.html
@@ -125,6 +125,23 @@
assert.equal(element.index, -1);
});
+
+ test('_moveCursor', function() {
+ // Initialize the cursor with its stops.
+ element.stops = list.querySelectorAll('li');
+ // Select the first stop.
+ element.setCursor(list.children[0]);
+ var getTargetHeight = sinon.stub();
+
+ // Move the cursor without an optional get target height function.
+ element._moveCursor(1);
+ assert.isFalse(getTargetHeight.called);
+
+ // Move the cursor with an optional get target height function.
+ element._moveCursor(1, null, getTargetHeight);
+ assert.isTrue(getTargetHeight.called);
+ });
+
test('opt_noScroll', function() {
sandbox.stub(element, '_targetIsVisible', function() { return false; });
var scrollStub = sandbox.stub(window, 'scrollTo');
@@ -182,5 +199,63 @@
element.next();
assert.isTrue(element.target.focus.called);
});
+
+ suite('_scrollToTarget', function() {
+ var scrollStub;
+ setup(function() {
+ element.stops = list.querySelectorAll('li');
+ element.scrollBehavior = 'keep-visible';
+
+ // There is a target which has a targetNext
+ element.setCursor(list.children[0]);
+ element._moveCursor(1);
+ scrollStub = sandbox.stub(window, 'scrollTo');
+ window.innerHeight = 60;
+ });
+
+ test('Called when top and bottom not visible', function() {
+ sandbox.stub(element, '_targetIsVisible', function() {
+ return false;
+ });
+ element._scrollToTarget();
+ assert.isTrue(scrollStub.called);
+ });
+
+ test('Not called when top and bottom visible', function() {
+ sandbox.stub(element, '_targetIsVisible', function() {
+ return true;
+ });
+ element._scrollToTarget();
+ assert.isFalse(scrollStub.called);
+ });
+
+ test('Called when top is visible, bottom is not, and scroll is lower',
+ function() {
+ var visibleStub = sandbox.stub(element, '_targetIsVisible', function() {
+ return visibleStub.callCount == 2;
+ });
+ window.scrollY = 15;
+ sandbox.stub(element, '_calculateScrollToValue', function() {
+ return 20;
+ });
+ element._scrollToTarget();
+ assert.isTrue(scrollStub.called);
+ assert.equal(visibleStub.callCount, 2);
+ });
+
+ test('Called when top is visible, bottom is not, and scroll is higher',
+ function() {
+ var visibleStub = sandbox.stub(element, '_targetIsVisible', function() {
+ return visibleStub.callCount == 2;
+ });
+ window.scrollY = 25;
+ sandbox.stub(element, '_calculateScrollToValue', function() {
+ return 20;
+ });
+ element._scrollToTarget();
+ assert.isFalse(scrollStub.called);
+ assert.equal(visibleStub.callCount, 2);
+ });
+ });
});
</script>
diff --git a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js
index fe0dad0..6f4cd3d 100644
--- a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js
+++ b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter.js
@@ -65,10 +65,6 @@
this._loadPreferences();
},
- _getTzString: function() {
- return ' ' + new Date().toString().split(' ').pop();
- },
-
_getUtcOffsetString: function() {
return ' UTC' + moment().format('Z');
},
@@ -150,7 +146,7 @@
var now = new Date();
var format = TimeFormats.MONTH_DAY_YEAR;
if (this._isWithinDay(now, date)) {
- return date.format(timeFormat) + this._getTzString();
+ format = timeFormat;
} else if (this._isWithinHalfYear(now, date)) {
format = TimeFormats.MONTH_DAY;
}
diff --git a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.html b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.html
index 43aea17..5955cc5 100644
--- a/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.html
+++ b/polygerrit-ui/app/elements/shared/gr-date-formatter/gr-date-formatter_test.html
@@ -82,7 +82,6 @@
{time_format: 'HHMM_24', relative_date_in_change_table: false}
).then(function() {
element = fixture('basic');
- sandbox.stub(element, '_getTzString').returns('');
sandbox.stub(element, '_getUtcOffsetString').returns('');
return element._loadPreferences();
});
@@ -125,7 +124,6 @@
{time_format: 'HHMM_12'}
).then(function() {
element = fixture('basic');
- sandbox.stub(element, '_getTzString').returns('');
sandbox.stub(element, '_getUtcOffsetString').returns('');
return element._loadPreferences();
});
@@ -144,7 +142,6 @@
{time_format: 'HHMM_12', relative_date_in_change_table: true}
).then(function() {
element = fixture('basic');
- sandbox.stub(element, '_getTzString').returns('');
sandbox.stub(element, '_getUtcOffsetString').returns('');
return element._loadPreferences();
});
@@ -169,7 +166,6 @@
{time_format: 'HHMM_12', relative_date_in_change_table: true}
).then(function() {
element = fixture('basic');
- sandbox.stub(element, '_getTzString').returns('');
return element._loadPreferences();
});
});
@@ -184,7 +180,6 @@
setup(function() {
return stubRestAPI(null).then(function() {
element = fixture('basic');
- sandbox.stub(element, '_getTzString').returns('');
return element._loadPreferences();
});
});
