From a40bb124fa647b45be1288f079b45e8a9311ee8e Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Fri, 17 Jan 2020 09:35:44 +0100 Subject: [PATCH] Include patch set number into message when setting description for patch set When a description for a patch set is set/removed, a change message with the new/old description is added to the change. However it doesn't say for which patch set the description was set/removed, which makes the message not very useful, as you cannot distinguish between messages for different patch sets. Signed-off-by: Edwin Kempin Change-Id: Iaa43c1897a236592c756535346689971a478dc96 --- .../server/restapi/change/PutDescription.java | 22 +++++++++++-------- .../acceptance/api/revision/RevisionIT.java | 6 ++--- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/java/com/google/gerrit/server/restapi/change/PutDescription.java b/java/com/google/gerrit/server/restapi/change/PutDescription.java index d84ab3e24f..f442a42875 100644 --- a/java/com/google/gerrit/server/restapi/change/PutDescription.java +++ b/java/com/google/gerrit/server/restapi/change/PutDescription.java @@ -87,17 +87,21 @@ public class PutDescription if (oldDescription.equals(newDescription)) { return false; } - String summary; - if (oldDescription.isEmpty()) { - summary = "Description set to \"" + newDescription + "\""; - } else if (newDescription.isEmpty()) { - summary = "Description \"" + oldDescription + "\" removed"; - } else { - summary = "Description changed to \"" + newDescription + "\""; - } - update.setPsDescription(newDescription); + String summary; + if (oldDescription.isEmpty()) { + summary = + String.format("Description of patch set %d set to \"%s\"", psId.get(), newDescription); + } else if (newDescription.isEmpty()) { + summary = + String.format( + "Description \"%s\" removed from patch set %d", oldDescription, psId.get()); + } else { + summary = + String.format( + "Description of patch set %d changed to \"%s\"", psId.get(), newDescription); + } ChangeMessage cmsg = ChangeMessagesUtil.newMessage( psId, ctx.getUser(), ctx.getWhen(), summary, ChangeMessagesUtil.TAG_SET_DESCRIPTION); diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java index d29f1cd4d2..e6b2190dd3 100644 --- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java +++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java @@ -1325,19 +1325,19 @@ public class RevisionIT extends AbstractDaemonTest { gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).description("foo"); assertDescription(r, "foo"); assertThat(Iterables.getLast(gApi.changes().id(r.getChangeId()).get().messages).message) - .isEqualTo("Description set to \"foo\""); + .isEqualTo("Description of patch set 1 set to \"foo\""); // update description gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).description("bar"); assertDescription(r, "bar"); assertThat(Iterables.getLast(gApi.changes().id(r.getChangeId()).get().messages).message) - .isEqualTo("Description changed to \"bar\""); + .isEqualTo("Description of patch set 1 changed to \"bar\""); // remove description gApi.changes().id(r.getChangeId()).revision(r.getCommit().name()).description(""); assertDescription(r, ""); assertThat(Iterables.getLast(gApi.changes().id(r.getChangeId()).get().messages).message) - .isEqualTo("Description \"bar\" removed"); + .isEqualTo("Description \"bar\" removed from patch set 1"); } @Test