Update url in emails to use new comment route

Gerrit added support for new url type /comment/{commentId} in 274190.
This new url route redirects user to the diff against the latest
patchset instead of base.
Update the url in the emails to use this route. The file link inside
the emails is removed as the support for file level route does not
exist.

Change-Id: Ia622714ff8c02f4326c7de495a867b498c72fd38
This commit is contained in:
Dhruv Srivastava
2020-08-21 14:33:32 +02:00
parent 5612d7580e
commit f45a2c58eb
5 changed files with 25 additions and 54 deletions

View File

@@ -57,17 +57,10 @@ public interface UrlFormatter {
return getChangeViewUrl(change.getProject(), change.getId()).map(url -> url + "?tab=findings");
}
/** Returns the URL for viewing a file in a given patch set of a change. */
default Optional<String> getPatchFileView(Change change, int patchsetId, String filename) {
/** Returns the URL for viewing a comment in a file for a change. */
default Optional<String> getInlineCommentView(Change change, String uuid) {
return getChangeViewUrl(change.getProject(), change.getId())
.map(url -> url + "/" + patchsetId + "/" + filename);
}
/** Returns the URL for viewing a comment in a file in a given patch set of a change. */
default Optional<String> getInlineCommentView(
Change change, int patchsetId, String filename, short side, int startLine) {
return getPatchFileView(change, patchsetId, filename)
.map(url -> url + String.format("@%s%d", side == 0 ? "a" : "", startLine));
.map(url -> url + "/comment/" + uuid);
}
/** Returns a URL pointing to the settings page. */

View File

@@ -23,7 +23,6 @@ import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.HumanComment;
import com.google.gerrit.entities.KeyUtil;
import com.google.gerrit.entities.NotifyConfig.NotifyType;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.Project;
@@ -73,20 +72,9 @@ public class CommentSender extends ReplyToChangeSender {
public PatchFile fileData;
public List<Comment> comments = new ArrayList<>();
/** @return a web link to the given patch set and file. */
public String getFileLink() {
return args.urlFormatter
.get()
.getPatchFileView(change, patchSetId, KeyUtil.encode(filename))
.orElse(null);
}
/** @return a web link to a comment within a given patch set and file. */
public String getCommentLink(short side, int startLine) {
return args.urlFormatter
.get()
.getInlineCommentView(change, patchSetId, KeyUtil.encode(filename), side, startLine)
.orElse(null);
/** @return a web link to a comment for a change. */
public String getCommentLink(String uuid) {
return args.urlFormatter.get().getInlineCommentView(change, uuid).orElse(null);
}
/** @return a web link to the comment tab view of a change. */
@@ -389,9 +377,6 @@ public class CommentSender extends ReplyToChangeSender {
for (CommentSender.FileCommentGroup group : getGroupedInlineComments(repo)) {
Map<String, Object> groupData = new HashMap<>();
if (!group.filename.equals(Patch.PATCHSET_LEVEL)) {
groupData.put("link", group.getFileLink());
}
groupData.put("title", group.getTitle());
groupData.put("patchSetId", group.patchSetId);
@@ -426,10 +411,8 @@ public class CommentSender extends ReplyToChangeSender {
} else {
commentData.put("link", group.getCommentsTabLink());
}
} else if (comment.lineNbr == 0) {
commentData.put("link", group.getFileLink());
} else {
commentData.put("link", group.getCommentLink(comment.side, startLine));
commentData.put("link", group.getCommentLink(comment.key.uuid));
}
// Set robot comment data.

View File

@@ -1103,12 +1103,6 @@ public class CommentsIT extends AbstractDaemonTest {
+ "\n"
+ "comments\n"
+ "\n"
+ url
+ "c/"
+ project.get()
+ "/+/"
+ c
+ "/1/a.txt \n"
+ "File a.txt:\n"
+ "\n"
+ url
@@ -1116,7 +1110,9 @@ public class CommentsIT extends AbstractDaemonTest {
+ project.get()
+ "/+/"
+ c
+ "/1/a.txt@a1 \n"
+ "/comment/"
+ ps1List.get(0).id
+ " \n"
+ "PS1, Line 1: initial\n"
+ "what happened to this?\n"
+ "\n"
@@ -1126,17 +1122,13 @@ public class CommentsIT extends AbstractDaemonTest {
+ project.get()
+ "/+/"
+ c
+ "/1/a.txt@1 \n"
+ "/comment/"
+ ps1List.get(1).id
+ " \n"
+ "PS1, Line 1: boring\n"
+ "Is it that bad?\n"
+ "\n"
+ "\n"
+ url
+ "c/"
+ project.get()
+ "/+/"
+ c
+ "/2/a.txt \n"
+ "File a.txt:\n"
+ "\n"
+ url
@@ -1144,7 +1136,9 @@ public class CommentsIT extends AbstractDaemonTest {
+ project.get()
+ "/+/"
+ c
+ "/2/a.txt@a1 \n"
+ "/comment/"
+ ps2List.get(0).id
+ " \n"
+ "PS2, Line 1: initial content\n"
+ "comment 1 on base\n"
+ "\n"
@@ -1154,7 +1148,9 @@ public class CommentsIT extends AbstractDaemonTest {
+ project.get()
+ "/+/"
+ c
+ "/2/a.txt@a2 \n"
+ "/comment/"
+ ps2List.get(1).id
+ " \n"
+ "PS2, Line 2: \n"
+ "comment 2 on base\n"
+ "\n"
@@ -1164,7 +1160,9 @@ public class CommentsIT extends AbstractDaemonTest {
+ project.get()
+ "/+/"
+ c
+ "/2/a.txt@1 \n"
+ "/comment/"
+ ps2List.get(2).id
+ " \n"
+ "PS2, Line 1: interesting\n"
+ "better now\n"
+ "\n"
@@ -1174,7 +1172,9 @@ public class CommentsIT extends AbstractDaemonTest {
+ project.get()
+ "/+/"
+ c
+ "/2/a.txt@2 \n"
+ "/comment/"
+ ps2List.get(3).id
+ " \n"
+ "PS2, Line 2: cntent\n"
+ "typo: content\n"
+ "\n"

View File

@@ -39,9 +39,6 @@
{/if}
{for $group in $commentFiles}
// Insert a space before the newline so that Gmail does not mistakenly link
// the following line with the file link. See issue 9201.
{if $group.link}{$group.link}{sp}{/if}{\n}
{$group.title}:{\n}
{\n}

View File

@@ -111,9 +111,7 @@
{for $group in $commentFiles}
<li style="{$fileLiStyle}">
<p>
{if $group.link}<a href="{$group.link}">{/if}
{$group.title}:
{if $group.link}</a>{/if}
</p>
<ul style="{$ulStyle}">