Add WIP awareness to PutMessage notifications
This change adds notification integration tests for this new API
endpoint. For convenience, PutMessage.Input is moved to common
extensions and renamed MessageInput, so it can be incorporated into the
ChangeApi interface.
Bug: Issue 6615
Change-Id: If419d713e4586038f04cc079245fed825acfb647
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index d64054a..adc882e 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -3154,6 +3154,17 @@
}
----
+.Notifications
+
+An email will be sent using the "newpatchset" template.
+
+[options="header",cols="1,1"]
+|=============================
+|WIP State |Default
+|Ready for review|owner, reviewers, CCs, stars, NEW_PATCHSETS watchers
+|Work in progress|owner
+|=============================
+
[[revision-endpoints]]
== Revision Endpoints
@@ -5958,7 +5969,7 @@
Notify handling that defines to whom email notifications should be sent
after the commit message was updated. +
Allowed values are `NONE`, `OWNER`, `OWNER_REVIEWERS` and `ALL`. +
-If not set, the default is `ALL`.
+If not set, the default is `OWNER` for WIP changes and `ALL` otherwise.
|`notify_details`|optional|
Additional information about whom to notify about the update as a map
of recipient type to link:#notify-info[NotifyInfo] entity.
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
index 1845103..5e0caf2 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/mail/ChangeNotificationsIT.java
@@ -43,6 +43,8 @@
import com.google.gerrit.extensions.api.changes.SubmitInput;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy;
import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.common.CommitInfo;
+import com.google.gerrit.extensions.common.CommitMessageInput;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.project.Util;
import org.junit.Before;
@@ -1473,10 +1475,9 @@
assume().that(notesMigration.readChanges()).isTrue();
StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master", other);
- // TODO(logan): This should include owner but currently doesn't because
- // it's sent *from* the owner.
assertThat(sender)
.sent("newpatchset", sc)
+ .notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
.to(sc.reviewer, other)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
@@ -1494,7 +1495,6 @@
.sent("newpatchset", sc)
.notTo(sc.owner) // TODO(logan): This email shouldn't come from the owner.
.to(sc.reviewer, sc.ccer, other)
- .notTo(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1507,7 +1507,8 @@
pushTo(sc, "refs/for/master", other, EmailStrategy.CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("newpatchset", sc)
- .to(sc.owner, sc.reviewer, other)
+ .notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
+ .to(sc.reviewer, other)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.bcc(sc.starrer)
@@ -1522,7 +1523,8 @@
pushTo(sc, "refs/for/master", other, EmailStrategy.CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("newpatchset", sc)
- .to(sc.owner, sc.reviewer, sc.ccer, other)
+ .notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
+ .to(sc.reviewer, sc.ccer, other)
.bcc(sc.starrer)
.bcc(NEW_PATCHSETS)
.noOneElse();
@@ -1533,10 +1535,9 @@
assume().that(notesMigration.readChanges()).isTrue();
StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=OWNER_REVIEWERS", other);
- // TODO(logan): This should include owner but currently doesn't because
- // it's sent *from* the owner.
assertThat(sender)
.sent("newpatchset", sc)
+ .notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
.to(sc.reviewer)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
@@ -1549,9 +1550,11 @@
assume().that(notesMigration.readChanges()).isFalse();
StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=OWNER_REVIEWERS", other);
- // TODO(logan): This should include owner but currently doesn't because
- // it's sent *from* the owner.
- assertThat(sender).sent("newpatchset", sc).to(sc.reviewer, sc.ccer).noOneElse();
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
+ .to(sc.reviewer, sc.ccer)
+ .noOneElse();
}
@Test
@@ -1562,7 +1565,8 @@
pushTo(sc, "refs/for/master%notify=OWNER_REVIEWERS", other, EmailStrategy.CC_ON_OWN_COMMENTS);
assertThat(sender)
.sent("newpatchset", sc)
- .to(sc.owner, sc.reviewer)
+ .notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
+ .to(sc.reviewer)
.cc(sc.ccer)
.cc(sc.reviewerByEmail, sc.ccerByEmail)
.noOneElse();
@@ -1574,7 +1578,11 @@
assume().that(notesMigration.readChanges()).isFalse();
StagedChange sc = stageReviewableChange();
pushTo(sc, "refs/for/master%notify=OWNER_REVIEWERS", other, EmailStrategy.CC_ON_OWN_COMMENTS);
- assertThat(sender).sent("newpatchset", sc).to(sc.owner, sc.reviewer, sc.ccer).noOneElse();
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.reviewer, sc.ccer)
+ .notTo(sc.owner) // TODO(logan): This shouldn't be sent *from* the owner.
+ .noOneElse();
}
@Test
@@ -1797,10 +1805,246 @@
private void pushTo(StagedChange sc, String ref, TestAccount by, EmailStrategy emailStrategy)
throws Exception {
- setEmailStrategy(sc.owner, emailStrategy);
+ setEmailStrategy(by, emailStrategy);
pushFactory.create(db, by.getIdent(), sc.repo, sc.changeId).to(ref).assertOkStatus();
}
+ @Test
+ public void editCommitMessageEditByOwnerOnReviewableChangeInNoteDb() throws Exception {
+ assume().that(notesMigration.readChanges()).isTrue();
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, sc.owner);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.reviewer)
+ .cc(sc.ccer)
+ .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .bcc(sc.starrer)
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageEditByOwnerOnReviewableChangeInReviewDb() throws Exception {
+ assume().that(notesMigration.readChanges()).isFalse();
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, sc.owner);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.reviewer, sc.ccer)
+ .bcc(sc.starrer)
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageEditByOtherOnReviewableChangeInNoteDb() throws Exception {
+ assume().that(notesMigration.readChanges()).isTrue();
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.owner, sc.reviewer)
+ .cc(sc.ccer)
+ .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .bcc(sc.starrer)
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageEditByOtherOnReviewableChangeInReviewDb() throws Exception {
+ assume().that(notesMigration.readChanges()).isFalse();
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.owner, sc.reviewer, sc.ccer)
+ .bcc(sc.starrer)
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnReviewableChangeOwnerSelfCcInNoteDb() throws Exception {
+ assume().that(notesMigration.readChanges()).isTrue();
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other, CC_ON_OWN_COMMENTS);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.owner, sc.reviewer, other)
+ .cc(sc.ccer)
+ .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .bcc(sc.starrer)
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnReviewableChangeOwnerSelfCcInReviewDb() throws Exception {
+ assume().that(notesMigration.readChanges()).isFalse();
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other, CC_ON_OWN_COMMENTS);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.owner, sc.reviewer, sc.ccer, other)
+ .bcc(sc.starrer)
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnReviewableChangeNotifyOwnerReviewersInNoteDb()
+ throws Exception {
+ assume().that(notesMigration.readChanges()).isTrue();
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other, OWNER_REVIEWERS);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.owner, sc.reviewer)
+ .cc(sc.ccer)
+ .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnReviewableChangeNotifyOwnerReviewersInReviewDb()
+ throws Exception {
+ assume().that(notesMigration.readChanges()).isFalse();
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other, OWNER_REVIEWERS);
+ assertThat(sender).sent("newpatchset", sc).to(sc.owner, sc.reviewer, sc.ccer).noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnReviewableChangeOwnerSelfCcNotifyOwnerReviewersInNoteDb()
+ throws Exception {
+ assume().that(notesMigration.readChanges()).isTrue();
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other, OWNER_REVIEWERS, CC_ON_OWN_COMMENTS);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.owner, sc.reviewer)
+ .cc(sc.ccer, other)
+ .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnReviewableChangeOwnerSelfCcNotifyOwnerReviewersInReviewDb()
+ throws Exception {
+ assume().that(notesMigration.readChanges()).isFalse();
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other, OWNER_REVIEWERS, CC_ON_OWN_COMMENTS);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.owner, sc.reviewer, sc.ccer)
+ .cc(other)
+ .noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnReviewableChangeNotifyOwner() throws Exception {
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other, OWNER);
+ assertThat(sender).sent("newpatchset", sc).to(sc.owner).noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnReviewableChangeOwnerSelfCcNotifyOwner() throws Exception {
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other, OWNER, CC_ON_OWN_COMMENTS);
+ assertThat(sender).sent("newpatchset", sc).to(sc.owner).cc(other).noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnReviewableChangeNotifyNone() throws Exception {
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other, NONE);
+ assertThat(sender).notSent();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnReviewableChangeOwnerSelfCcNotifyNone() throws Exception {
+ StagedChange sc = stageReviewableChange();
+ editCommitMessage(sc, other, NONE, CC_ON_OWN_COMMENTS);
+ assertThat(sender).notSent();
+ }
+
+ @Test
+ public void editCommitMessageOnWipChange() throws Exception {
+ StagedChange sc = stageWipChange();
+ editCommitMessage(sc, sc.owner);
+ assertThat(sender).notSent();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnWipChange() throws Exception {
+ StagedChange sc = stageWipChange();
+ editCommitMessage(sc, other);
+ assertThat(sender).sent("newpatchset", sc).to(sc.owner).noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageByOtherOnWipChangeSelfCc() throws Exception {
+ StagedChange sc = stageWipChange();
+ editCommitMessage(sc, other, CC_ON_OWN_COMMENTS);
+ assertThat(sender).sent("newpatchset", sc).to(sc.owner).cc(other).noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageOnWipChangeNotifyAllInNoteDb() throws Exception {
+ assume().that(notesMigration.readChanges()).isTrue();
+ StagedChange sc = stageWipChange();
+ editCommitMessage(sc, sc.owner, ALL);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.reviewer)
+ .cc(sc.ccer)
+ .cc(sc.reviewerByEmail, sc.ccerByEmail)
+ .bcc(sc.starrer)
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
+ }
+
+ @Test
+ public void editCommitMessageOnWipChangeNotifyAllInReviewDb() throws Exception {
+ assume().that(notesMigration.readChanges()).isFalse();
+ StagedChange sc = stageWipChange();
+ editCommitMessage(sc, sc.owner, ALL);
+ assertThat(sender)
+ .sent("newpatchset", sc)
+ .to(sc.reviewer, sc.ccer)
+ .bcc(sc.starrer)
+ .bcc(NEW_PATCHSETS)
+ .noOneElse();
+ }
+
+ private void editCommitMessage(StagedChange sc, TestAccount by) throws Exception {
+ editCommitMessage(sc, by, null, ENABLED);
+ }
+
+ private void editCommitMessage(StagedChange sc, TestAccount by, @Nullable NotifyHandling notify)
+ throws Exception {
+ editCommitMessage(sc, by, notify, ENABLED);
+ }
+
+ private void editCommitMessage(StagedChange sc, TestAccount by, EmailStrategy emailStrategy)
+ throws Exception {
+ editCommitMessage(sc, by, null, emailStrategy);
+ }
+
+ private void editCommitMessage(
+ StagedChange sc, TestAccount by, @Nullable NotifyHandling notify, EmailStrategy emailStrategy)
+ throws Exception {
+ setEmailStrategy(by, emailStrategy);
+ CommitInfo commit = gApi.changes().id(sc.changeId).revision("current").commit(false);
+ CommitMessageInput in = new CommitMessageInput();
+ in.message = "update\n" + commit.message;
+ in.notify = notify;
+ gApi.changes().id(sc.changeId).setMessage(in);
+ }
+
/*
* RestoredSender tests.
*/
@@ -1914,7 +2158,6 @@
assertThat(sender)
.sent("revert", sc)
- .notTo(sc.owner)
.cc(sc.reviewer, sc.ccer, admin)
.bcc(ALL_COMMENTS)
.noOneElse(); // TODO(logan): Why not starrer/reviewers-by-email?
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
index aad10a9..f713ad2 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/ChangeApi.java
@@ -19,6 +19,7 @@
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.common.CommitMessageInput;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.MergePatchSetInput;
import com.google.gerrit.extensions.common.RobotCommentInfo;
@@ -193,6 +194,9 @@
/** Create a new patch set with a new commit message. */
void setMessage(String message) throws RestApiException;
+ /** Create a new patch set with a new commit message. */
+ void setMessage(CommitMessageInput in) throws RestApiException;
+
/** Set hashtags on a change */
void setHashtags(HashtagsInput input) throws RestApiException;
@@ -439,6 +443,11 @@
}
@Override
+ public void setMessage(CommitMessageInput in) throws RestApiException {
+ throw new NotImplementedException();
+ }
+
+ @Override
public EditInfo getEdit() throws RestApiException {
throw new NotImplementedException();
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/CommitMessageInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/CommitMessageInput.java
new file mode 100644
index 0000000..1e23cb4
--- /dev/null
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/CommitMessageInput.java
@@ -0,0 +1,30 @@
+// Copyright (C) 2017 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.extensions.common;
+
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.api.changes.NotifyHandling;
+import com.google.gerrit.extensions.api.changes.NotifyInfo;
+import com.google.gerrit.extensions.api.changes.RecipientType;
+import com.google.gerrit.extensions.restapi.DefaultInput;
+import java.util.Map;
+
+public class CommitMessageInput {
+ @DefaultInput public String message;
+
+ @Nullable public NotifyHandling notify;
+
+ public Map<RecipientType, NotifyInfo> notifyDetails;
+}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
index 4f4af54..dfa0e7c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java
@@ -39,6 +39,7 @@
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.common.CommitMessageInput;
import com.google.gerrit.extensions.common.EditInfo;
import com.google.gerrit.extensions.common.MergePatchSetInput;
import com.google.gerrit.extensions.common.RobotCommentInfo;
@@ -515,11 +516,16 @@
}
@Override
- public void setMessage(String in) throws RestApiException {
+ public void setMessage(String msg) throws RestApiException {
+ CommitMessageInput in = new CommitMessageInput();
+ in.message = msg;
+ setMessage(in);
+ }
+
+ @Override
+ public void setMessage(CommitMessageInput in) throws RestApiException {
try {
- PutMessage.Input input = new PutMessage.Input();
- input.message = in;
- putMessage.apply(change, input);
+ putMessage.apply(change, in);
} catch (Exception e) {
throw asRestApiException("Cannot edit commit message", e);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
index 6463bed..5c3b611 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -19,6 +19,7 @@
import static com.google.gerrit.server.notedb.ReviewerStateInternal.CC;
import static com.google.gerrit.server.notedb.ReviewerStateInternal.REVIEWER;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
@@ -180,7 +181,7 @@
}
public PatchSetInserter setNotify(NotifyHandling notify) {
- this.notify = notify;
+ this.notify = Preconditions.checkNotNull(notify);
return this;
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java
index 95e29ea..d9d4f43 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutMessage.java
@@ -17,11 +17,9 @@
import com.google.gerrit.common.FooterConstants;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
-import com.google.gerrit.extensions.api.changes.NotifyInfo;
-import com.google.gerrit.extensions.api.changes.RecipientType;
+import com.google.gerrit.extensions.common.CommitMessageInput;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -46,7 +44,6 @@
import java.io.IOException;
import java.sql.Timestamp;
import java.util.List;
-import java.util.Map;
import java.util.TimeZone;
import javax.inject.Inject;
import javax.inject.Provider;
@@ -63,15 +60,7 @@
@Singleton
public class PutMessage
- extends RetryingRestModifyView<ChangeResource, PutMessage.Input, Response<?>> {
-
- public static class Input {
- @DefaultInput public String message;
-
- public NotifyHandling notify = NotifyHandling.ALL;
-
- public Map<RecipientType, NotifyInfo> notifyDetails;
- }
+ extends RetryingRestModifyView<ChangeResource, CommitMessageInput, Response<?>> {
private final GitRepositoryManager repositoryManager;
private final Provider<CurrentUser> currentUserProvider;
@@ -106,7 +95,7 @@
@Override
protected Response<String> applyImpl(
- BatchUpdate.Factory updateFactory, ChangeResource resource, Input input)
+ BatchUpdate.Factory updateFactory, ChangeResource resource, CommitMessageInput input)
throws IOException, UnchangedCommitMessageException, RestApiException, UpdateException,
PermissionBackendException, OrmException, ConfigInvalidException {
PatchSet ps = psUtil.current(db.get(), resource.getNotes());
@@ -127,6 +116,11 @@
resource.getChange().getKey().get(),
sanitizedCommitMessage);
+ NotifyHandling notify = input.notify;
+ if (notify == null) {
+ notify = resource.getChange().isWorkInProgress() ? NotifyHandling.OWNER : NotifyHandling.ALL;
+ }
+
try (Repository repository = repositoryManager.openRepository(resource.getProject());
RevWalk revWalk = new RevWalk(repository);
ObjectInserter objectInserter = repository.newObjectInserter()) {
@@ -152,7 +146,7 @@
inserter.setMessage(
String.format("Patch Set %s: Commit message was updated.", psId.getId()));
inserter.setDescription("Edit commit message");
- inserter.setNotify(input.notify);
+ inserter.setNotify(notify);
inserter.setAccountsToNotify(notifyUtil.resolveAccounts(input.notifyDetails));
bu.addOp(resource.getChange().getId(), inserter);
bu.execute();
