Merge "Markdown highlighting - remove emphasis css rules"
diff --git a/Documentation/config-mail.txt b/Documentation/config-mail.txt
index 85006dc..bc45956 100644
--- a/Documentation/config-mail.txt
+++ b/Documentation/config-mail.txt
@@ -221,6 +221,10 @@
The original subject limited to 72 characters, with an ellipsis if it exceeds
that.
+$change.sizeBucket::
++
+Human-readable size bucket of the current change.
+
$change.ownerEmail::
+
The email address of the owner of the change.
diff --git a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
index 6c9a2b1..da033c1 100644
--- a/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
+++ b/java/com/google/gerrit/acceptance/AbstractNotificationTest.java
@@ -98,6 +98,7 @@
protected static class FakeEmailSenderSubject extends Subject {
private final FakeEmailSender fakeEmailSender;
+ private String emailTitle;
private Message message;
private StagedUsers users;
private Map<RecipientType, List<String>> recipients = new HashMap<>();
@@ -145,6 +146,10 @@
? ((StringEmailHeader) header).getString()
: header));
}
+ EmailHeader titleHeader = message.headers().get("Subject");
+ if (titleHeader instanceof StringEmailHeader) {
+ emailTitle = ((StringEmailHeader) titleHeader).getString();
+ }
return this;
}
@@ -190,6 +195,15 @@
return rcpt(users.supportReviewersByEmail ? BCC : null, emails);
}
+ public FakeEmailSenderSubject title(String expectedEmailTitle) {
+ if (!emailTitle.equals(expectedEmailTitle)) {
+ failWithoutActual(
+ fact("Expected email title", expectedEmailTitle),
+ fact("but actual title is", emailTitle));
+ }
+ return this;
+ }
+
private FakeEmailSenderSubject rcpt(@Nullable RecipientType type, String[] emails) {
for (String email : emails) {
rcpt(type, email);
diff --git a/java/com/google/gerrit/entities/ChangeSizeBucket.java b/java/com/google/gerrit/entities/ChangeSizeBucket.java
new file mode 100644
index 0000000..4d01ebd
--- /dev/null
+++ b/java/com/google/gerrit/entities/ChangeSizeBucket.java
@@ -0,0 +1,64 @@
+// Copyright (C) 2022 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.
+package com.google.gerrit.entities;
+
+/**
+ * A human-readable change size bucket.
+ *
+ * <p>Should be kept in sync with
+ * polygerrit-ui/app/elements/change-list/gr-change-list-item/gr-change-list-item.ts
+ */
+public class ChangeSizeBucket {
+
+ /**
+ * The upper bounds for the different size buckets.
+ *
+ * <p>Same as gr-change-list-item.ts::ChangeSize.
+ */
+ private enum BucketThresholds {
+ XS(10),
+ SMALL(50),
+ MEDIUM(250),
+ LARGE(1000);
+
+ public final long delta;
+
+ BucketThresholds(long delta) {
+ this.delta = delta;
+ }
+ }
+
+ /**
+ * Gets the correlative size bucket for the given change delta.
+ *
+ * <p>Same as gr-change-list-item.ts::computeChangeSize().
+ *
+ * @param delta the total number of changed lines (additions+deletions) of the change.
+ * @return a short human-readable size bucket.
+ */
+ public static String getChangeSizeBucket(long delta) {
+ if (delta == 0) {
+ return "NoOp";
+ } else if (delta < BucketThresholds.XS.delta) {
+ return "XS";
+ } else if (delta < BucketThresholds.SMALL.delta) {
+ return "S";
+ } else if (delta < BucketThresholds.MEDIUM.delta) {
+ return "M";
+ } else if (delta < BucketThresholds.LARGE.delta) {
+ return "L";
+ }
+ return "XL";
+ }
+}
diff --git a/java/com/google/gerrit/server/mail/send/ChangeEmail.java b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
index 63b9c70..cba6e47 100644
--- a/java/com/google/gerrit/server/mail/send/ChangeEmail.java
+++ b/java/com/google/gerrit/server/mail/send/ChangeEmail.java
@@ -26,6 +26,7 @@
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.ChangeSizeBucket;
import com.google.gerrit.entities.NotifyConfig.NotifyType;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.PatchSet;
@@ -74,6 +75,7 @@
/** Sends an email to one or more interested parties. */
public abstract class ChangeEmail extends NotificationEmail {
+
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
protected static ChangeData newChangeData(
@@ -231,6 +233,18 @@
setHeader(FieldName.SUBJECT, textTemplate("ChangeSubject"));
}
+ private int getInsertionsCount() {
+ return listModifiedFiles().values().stream()
+ .map(FileDiffOutput::insertions)
+ .reduce(0, Integer::sum);
+ }
+
+ private int getDeletionsCount() {
+ return listModifiedFiles().values().stream()
+ .map(FileDiffOutput::deletions)
+ .reduce(0, Integer::sum);
+ }
+
/** Get a link to the change; null if the server doesn't know its own address. */
@Nullable
public String getChangeUrl() {
@@ -285,10 +299,6 @@
fileDiff.oldPath(), fileDiff.newPath(), fileDiff.changeType()))
.append("\n");
}
- Integer insertions =
- modifiedFiles.values().stream().map(FileDiffOutput::insertions).reduce(0, Integer::sum);
- Integer deletions =
- modifiedFiles.values().stream().map(FileDiffOutput::deletions).reduce(0, Integer::sum);
detail.append(
MessageFormat.format(
"" //
@@ -297,8 +307,8 @@
+ "{2,choice,0#0 deletions|1#1 deletion|1<{2} deletions}(-)" //
+ "\n",
modifiedFiles.size() - 1, //
- insertions, //
- deletions));
+ getInsertionsCount(), //
+ getDeletionsCount()));
detail.append("\n");
}
return detail.toString();
@@ -309,29 +319,35 @@
}
/** Get the patch list corresponding to patch set patchSetId of this change. */
- protected Map<String, FileDiffOutput> listModifiedFiles(int patchSetId)
- throws DiffNotAvailableException {
- PatchSet ps;
- if (patchSetId == patchSet.number()) {
- ps = patchSet;
- } else {
- try {
+ protected Map<String, FileDiffOutput> listModifiedFiles(int patchSetId) {
+ try {
+ PatchSet ps;
+ if (patchSetId == patchSet.number()) {
+ ps = patchSet;
+ } else {
ps = args.patchSetUtil.get(changeData.notes(), PatchSet.id(change.getId(), patchSetId));
- } catch (StorageException e) {
- throw new DiffNotAvailableException("Failed to get patchSet", e);
}
+ return args.diffOperations.listModifiedFilesAgainstParent(
+ change.getProject(), ps.commitId(), /* parentNum= */ 0, DiffOptions.DEFAULTS);
+ } catch (StorageException | DiffNotAvailableException e) {
+ logger.atSevere().withCause(e).log("Failed to get modified files");
+ return new HashMap<>();
}
- return args.diffOperations.listModifiedFilesAgainstParent(
- change.getProject(), ps.commitId(), /* parentNum= */ 0, DiffOptions.DEFAULTS);
}
/** Get the patch list corresponding to this patch set. */
- protected Map<String, FileDiffOutput> listModifiedFiles() throws DiffNotAvailableException {
+ protected Map<String, FileDiffOutput> listModifiedFiles() {
if (patchSet != null) {
- return args.diffOperations.listModifiedFilesAgainstParent(
- change.getProject(), patchSet.commitId(), /* parentNum= */ 0, DiffOptions.DEFAULTS);
+ try {
+ return args.diffOperations.listModifiedFilesAgainstParent(
+ change.getProject(), patchSet.commitId(), /* parentNum= */ 0, DiffOptions.DEFAULTS);
+ } catch (DiffNotAvailableException e) {
+ logger.atSevere().withCause(e).log("Failed to get modified files");
+ }
+ } else {
+ logger.atSevere().log("no patchSet specified");
}
- throw new DiffNotAvailableException("no patchSet specified");
+ return new HashMap<>();
}
/** Get the project entity the change is in; null if its been deleted. */
@@ -497,6 +513,9 @@
changeData.put("ownerName", getNameFor(change.getOwner()));
changeData.put("ownerEmail", getNameEmailFor(change.getOwner()));
changeData.put("changeNumber", Integer.toString(change.getChangeId()));
+ changeData.put(
+ "sizeBucket",
+ ChangeSizeBucket.getChangeSizeBucket(getInsertionsCount() + getDeletionsCount()));
soyContext.put("change", changeData);
Map<String, Object> patchSetData = new HashMap<>();
@@ -579,16 +598,11 @@
/** Show patch set as unified difference. */
public String getUnifiedDiff() {
Map<String, FileDiffOutput> modifiedFiles;
- try {
- modifiedFiles = listModifiedFiles();
- if (modifiedFiles.isEmpty()) {
- // Octopus merges are not well supported for diff output by Gerrit.
- // Currently these always have a null oldId in the PatchList.
- return "[Octopus merge; cannot be formatted as a diff.]\n";
- }
- } catch (DiffNotAvailableException e) {
- logger.atSevere().withCause(e).log("Cannot format patch");
- return "";
+ modifiedFiles = listModifiedFiles();
+ if (modifiedFiles.isEmpty()) {
+ // Octopus merges are not well supported for diff output by Gerrit.
+ // Currently these always have a null oldId in the PatchList.
+ return "[Empty change (potentially Octopus merge); cannot be formatted as a diff.]\n";
}
int maxSize = args.settings.maximumDiffSize;
diff --git a/java/com/google/gerrit/server/mail/send/CommentSender.java b/java/com/google/gerrit/server/mail/send/CommentSender.java
index 81e4f3e..ce5438b 100644
--- a/java/com/google/gerrit/server/mail/send/CommentSender.java
+++ b/java/com/google/gerrit/server/mail/send/CommentSender.java
@@ -37,7 +37,6 @@
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.mail.receive.Protocol;
-import com.google.gerrit.server.patch.DiffNotAvailableException;
import com.google.gerrit.server.patch.PatchFile;
import com.google.gerrit.server.patch.filediff.FileDiffOutput;
import com.google.gerrit.server.util.LabelVote;
@@ -60,13 +59,16 @@
/** Send comments, after the author of them hit used Publish Comments in the UI. */
public class CommentSender extends ReplyToChangeSender {
+
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
public interface Factory {
+
CommentSender create(Project.NameKey project, Change.Id changeId);
}
private class FileCommentGroup {
+
public String filename;
public int patchSetId;
public PatchFile fileData;
@@ -199,12 +201,7 @@
currentGroup.filename = c.key.filename;
currentGroup.patchSetId = c.key.patchSetId;
// Get the modified files:
- Map<String, FileDiffOutput> modifiedFiles = null;
- try {
- modifiedFiles = listModifiedFiles(c.key.patchSetId);
- } catch (DiffNotAvailableException e) {
- logger.atSevere().withCause(e).log("Failed to get modified files");
- }
+ Map<String, FileDiffOutput> modifiedFiles = listModifiedFiles(c.key.patchSetId);
groups.add(currentGroup);
if (modifiedFiles != null && !modifiedFiles.isEmpty()) {
diff --git a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
index c6103f4..6013862 100644
--- a/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
@@ -64,6 +64,7 @@
import org.junit.Test;
public class ChangeNotificationsIT extends AbstractNotificationTest {
+
@Inject private ProjectOperations projectOperations;
@Inject private RequestScopeOperations requestScopeOperations;
@@ -599,6 +600,7 @@
}
private interface Adder {
+
void addReviewer(String changeId, String reviewer, @Nullable NotifyHandling notify)
throws Exception;
}
@@ -1000,6 +1002,15 @@
}
@Test
+ public void verifyTitle() throws Exception {
+ StagedPreChange spc = stagePreChange("refs/for/master");
+ assertThat(sender)
+ .sent("newchange", spc)
+ .title(String.format("[S] Change in %s[master]: test commit", project));
+ assertThat(sender).didNotSend();
+ }
+
+ @Test
public void createWipChange() throws Exception {
stagePreChange("refs/for/master%wip");
assertThat(sender).didNotSend();
@@ -1291,6 +1302,7 @@
}
private interface Stager {
+
StagedChange stage() throws Exception;
}
diff --git a/javatests/com/google/gerrit/entities/ChangeSizeBucketTest.java b/javatests/com/google/gerrit/entities/ChangeSizeBucketTest.java
new file mode 100644
index 0000000..a0fff22
--- /dev/null
+++ b/javatests/com/google/gerrit/entities/ChangeSizeBucketTest.java
@@ -0,0 +1,32 @@
+// Copyright (C) 2022 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.
+
+package com.google.gerrit.entities;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import org.junit.Test;
+
+public class ChangeSizeBucketTest {
+
+ @Test
+ public void getChangeSizeBucket() {
+ assertThat(ChangeSizeBucket.getChangeSizeBucket(0)).isEqualTo("NoOp");
+ assertThat(ChangeSizeBucket.getChangeSizeBucket(1)).isEqualTo("XS");
+ assertThat(ChangeSizeBucket.getChangeSizeBucket(10)).isEqualTo("S");
+ assertThat(ChangeSizeBucket.getChangeSizeBucket(50)).isEqualTo("M");
+ assertThat(ChangeSizeBucket.getChangeSizeBucket(250)).isEqualTo("L");
+ assertThat(ChangeSizeBucket.getChangeSizeBucket(1000)).isEqualTo("XL");
+ }
+}
diff --git a/resources/com/google/gerrit/server/mail/ChangeSubject.soy b/resources/com/google/gerrit/server/mail/ChangeSubject.soy
index 28d04d0..ba422a4 100644
--- a/resources/com/google/gerrit/server/mail/ChangeSubject.soy
+++ b/resources/com/google/gerrit/server/mail/ChangeSubject.soy
@@ -25,10 +25,13 @@
{@param change: ?}
{@param shortProjectName: ?}
{@param instanceAndProjectName: ?}
- {@param addInstanceNameInSubject: ?} /** boolean */
+ {@param addInstanceNameInSubject: ?} /** boolean */
+
{if not $addInstanceNameInSubject}
- Change in {$shortProjectName}[{$branch.shortName}]: {$change.shortSubject}
+ [{$change.sizeBucket}] Change in {$shortProjectName}[{$branch.shortName}]: {$change
+ .shortSubject}
{else}
- Change in {$instanceAndProjectName}[{$branch.shortName}]: {$change.shortSubject}
+ [{$change.sizeBucket}] Change in {$instanceAndProjectName}[{$branch.shortName}]: {$change
+ .shortSubject}
{/if}
{/template}
