From 006d52bb8675382b7599f34df2319438bcc8cbb6 Mon Sep 17 00:00:00 2001 From: Gal Paikin Date: Thu, 19 Dec 2019 11:22:09 +0100 Subject: [PATCH] Cut down to below 73 characters when reverting changes Whenever a change is reverted, "Revert" is added and the subject of the change that is being reverted is copied. However, if the subject is around 73 characters and the word "Revert" is added, then the subject will be above 73 characters. This change checks the length of the subject, and if the subject length is above 63, it will shorten it to 60 and add "..." in the end. Thus, the final length will always be below 73. Change-Id: I07e36ef60f8add83f34f41b01c0d2d41bfc9b448 --- Documentation/rest-api-changes.txt | 8 +++- .../google/gerrit/server/git/CommitUtil.java | 8 ++-- .../restapi/change/RevertSubmission.java | 3 ++ .../acceptance/api/change/RevertIT.java | 43 +++++++++++++++++++ 4 files changed, 58 insertions(+), 4 deletions(-) diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 4b7a659b7e..1396cf5a07 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -1457,6 +1457,10 @@ response body. Reverts a change. +The subject of the newly created change will be +'Revert ""'. If the subject of the change reverted is +above 63 characters, it will be cut down to 59 characters with "..." in the end. + The request body does not need to include a link:#revert-input[ RevertInput] entity if no review comment is added. @@ -1515,7 +1519,9 @@ the error message is contained in the response body. Creates open revert changes for all of the changes of a certain submission. -The subject of each revert change will be "Revert 63) { + subject = subject.substring(0, 59) + "..."; + } if (message == null) { message = MessageFormat.format( - ChangeMessages.get().revertChangeDefaultMessage, - changeToRevert.getSubject(), - patch.commitId().name()); + ChangeMessages.get().revertChangeDefaultMessage, subject, patch.commitId().name()); } if (generatedChangeId != null) { revertCommitBuilder.setMessage(ChangeIdUtil.insertId(message, generatedChangeId, true)); diff --git a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java index df4c83e2c6..a7d19d1f5d 100644 --- a/java/com/google/gerrit/server/restapi/change/RevertSubmission.java +++ b/java/com/google/gerrit/server/restapi/change/RevertSubmission.java @@ -313,6 +313,9 @@ public class RevertSubmission private String getMessage(RevertInput revertInput, ChangeNotes changeNotes) { String subject = changeNotes.getChange().getSubject(); + if (subject.length() > 63) { + subject = subject.substring(0, 59) + "..."; + } if (revertInput.message == null) { return MessageFormat.format( ChangeMessages.get().revertChangeDefaultMessage, diff --git a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java index f1ea3192a9..7703b1ceb5 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/RevertIT.java @@ -283,6 +283,27 @@ public class RevertIT extends AbstractDaemonTest { .isEqualTo(revertInput.message); } + @Test + public void revertChangeWithLongSubject() throws Exception { + String changeTitle = + "This change has a very long title and therefore it will be cut to 50 characters when the" + + " revert change will revert this change"; + String result = createChange(changeTitle, "a.txt", "message").getChangeId(); + gApi.changes().id(result).current().review(ReviewInput.approve()); + gApi.changes().id(result).current().submit(); + RevertInput revertInput = new RevertInput(); + ChangeInfo revertChange = gApi.changes().id(result).revert(revertInput).get(); + assertThat(revertChange.subject) + .isEqualTo(String.format("Revert \"%s...\"", changeTitle.substring(0, 59))); + assertThat(gApi.changes().id(revertChange.id).current().commit(false).message) + .isEqualTo( + String.format( + "Revert \"%s...\"\n\nThis reverts commit %s.\n\nChange-Id: %s\n", + changeTitle.substring(0, 59), + gApi.changes().id(result).get().currentRevision, + revertChange.changeId)); + } + @Test public void revertNotifications() throws Exception { PushOneCommit.Result r = createChange(); @@ -704,6 +725,28 @@ public class RevertIT extends AbstractDaemonTest { gApi.changes().id(result).get().currentRevision, revertChange.changeId)); } + @Test + public void revertSubmissionRevertsChangeWithLongSubject() throws Exception { + String changeTitle = + "This change has a very long title and therefore it will be cut to 50 characters when the" + + " revert change will revert this change"; + String result = createChange(changeTitle, "a.txt", "message").getChangeId(); + gApi.changes().id(result).current().review(ReviewInput.approve()); + gApi.changes().id(result).current().submit(); + RevertInput revertInput = new RevertInput(); + ChangeInfo revertChange = + gApi.changes().id(result).revertSubmission(revertInput).revertChanges.get(0); + assertThat(revertChange.subject) + .isEqualTo(String.format("Revert \"%s...\"", changeTitle.substring(0, 59))); + assertThat(gApi.changes().id(revertChange.id).current().commit(false).message) + .isEqualTo( + String.format( + "Revert \"%s...\"\n\nThis reverts commit %s.\n\nChange-Id: %s\n", + changeTitle.substring(0, 59), + gApi.changes().id(result).get().currentRevision, + revertChange.changeId)); + } + @Test @GerritConfig(name = "change.submitWholeTopic", value = "true") public void revertSubmissionDifferentRepositoriesWithDependantChange() throws Exception {