Make adding of signed-off-by footer for online changes customizable
Now that we migrated general user preferences to Git backend, we can
easily introduce new customization option: Signed-off-by footer for
changes created with inline edit feature:
* "Create Change" button on project screen
* "Edit Config" button on project screen
* "Follow-Up" button on change screen
Change-Id: Ifa94c18d5351196fbc741c6b16d0b129e2b4a6cb
diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt
index 408e46b..820efab 100644
--- a/Documentation/rest-api-accounts.txt
+++ b/Documentation/rest-api-accounts.txt
@@ -1946,6 +1946,9 @@
Whether to show change number in the change table.
|`mute_common_path_prefixes` |not set if `false`|
Whether to mute common path prefixes in file names in the file table.
+|`signed_off_by` |not set if `false`|
+Whether to insert Signed-off-by footer in changes created with the
+inline edit feature.
|`review_category_strategy` ||
The strategy used to displayed info in the review category column.
Allowed values are `NONE`, `NAME`, `EMAIL`, `USERNAME`, `ABBREV`.
@@ -2001,6 +2004,9 @@
Whether to show change number in the change table.
|`mute_common_path_prefixes` |optional|
Whether to mute common path prefixes in file names in the file table.
+|`signed_off_by` |optional|
+Whether to insert Signed-off-by footer in changes created with the
+inline edit feature.
|`review_category_strategy` |optional|
The strategy used to displayed info in the review category column.
Allowed values are `NONE`, `NAME`, `EMAIL`, `USERNAME`, `ABBREV`.
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/GeneralPreferencesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/GeneralPreferencesIT.java
index 6b8f6ce..7c601b1 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/GeneralPreferencesIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/GeneralPreferencesIT.java
@@ -72,6 +72,7 @@
assertThat(o.legacycidInChangeTable).isNull();
assertThat(o.muteCommonPathPrefixes).isEqualTo(
d.muteCommonPathPrefixes);
+ assertThat(o.signedOffBy).isNull();
assertThat(o.reviewCategoryStrategy).isEqualTo(
d.getReviewCategoryStrategy());
assertThat(o.diffView).isEqualTo(d.getDiffView());
@@ -93,6 +94,7 @@
i.sizeBarInChangeTable ^= true;
i.legacycidInChangeTable ^= true;
i.muteCommonPathPrefixes ^= true;
+ i.signedOffBy ^= true;
i.reviewCategoryStrategy = ReviewCategoryStrategy.ABBREV;
i.diffView = DiffView.UNIFIED_DIFF;
i.my = new ArrayList<>();
@@ -117,6 +119,7 @@
assertThat(o.sizeBarInChangeTable).isNull();
assertThat(o.legacycidInChangeTable).isEqualTo(i.legacycidInChangeTable);
assertThat(o.muteCommonPathPrefixes).isNull();
+ assertThat(o.signedOffBy).isEqualTo(i.signedOffBy);
assertThat(o.reviewCategoryStrategy).isEqualTo(
i.getReviewCategoryStrategy());
assertThat(o.diffView).isEqualTo(i.getDiffView());
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
index 063a795..14208e96e 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java
@@ -17,11 +17,13 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume;
import static java.util.concurrent.TimeUnit.SECONDS;
+import static org.eclipse.jgit.lib.Constants.SIGNED_OFF_BY_TAG;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -42,7 +44,6 @@
import org.junit.BeforeClass;
import org.junit.Test;
-@NoHttpd
public class CreateChangeIT extends AbstractDaemonTest {
@ConfigSuite.Config
public static Config allowDraftsDisabled() {
@@ -59,7 +60,6 @@
TestTimeUtil.useSystemTime();
}
-
@Test
public void createEmptyChange_MissingBranch() throws Exception {
ChangeInput ci = new ChangeInput();
@@ -90,6 +90,17 @@
}
@Test
+ public void createNewChangeSignedOffByFooter() throws Exception {
+ assume().that(isAllowDrafts()).isTrue();
+ setSignedOffByFooter();
+ ChangeInfo info = assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));
+ String message = info.revisions.get(info.currentRevision).commit.message;
+ assertThat(message.contains(
+ String.format("%s Adminitrstaor %s", SIGNED_OFF_BY_TAG,
+ admin.getIdent().getEmailAddress())));
+ }
+
+ @Test
public void createDraftChange() throws Exception {
assume().that(isAllowDrafts()).isTrue();
assertCreateSucceeds(newChangeInput(ChangeStatus.DRAFT));
@@ -164,4 +175,21 @@
}
return draft ? ChangeStatus.DRAFT : ChangeStatus.NEW;
}
+
+ // TODO(davido): Expose setting of account preferences in the API
+ private void setSignedOffByFooter() throws Exception {
+ RestResponse r = adminSession.get("/accounts/" + admin.email
+ + "/preferences");
+ r.assertOK();
+ GeneralPreferencesInfo i =
+ newGson().fromJson(r.getReader(), GeneralPreferencesInfo.class);
+ i.signedOffBy = true;
+
+ r = adminSession.put("/accounts/" + admin.email + "/preferences", i);
+ r.assertOK();
+ GeneralPreferencesInfo o = newGson().fromJson(r.getReader(),
+ GeneralPreferencesInfo.class);
+
+ assertThat(o.signedOffBy).isTrue();
+ }
}
diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
index 8ffcfb9..cb3ab59 100644
--- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
+++ b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/client/GeneralPreferencesInfo.java
@@ -119,6 +119,7 @@
public Boolean legacycidInChangeTable;
public ReviewCategoryStrategy reviewCategoryStrategy;
public Boolean muteCommonPathPrefixes;
+ public Boolean signedOffBy;
public List<MenuItem> my;
public Map<String, String> urlAliases;
public EmailStrategy emailStrategy;
@@ -178,6 +179,7 @@
p.sizeBarInChangeTable = true;
p.legacycidInChangeTable = false;
p.muteCommonPathPrefixes = true;
+ p.signedOffBy = false;
return p;
}
}
diff --git a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GeneralPreferences.java b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GeneralPreferences.java
index c54fd1c..40e6278 100644
--- a/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GeneralPreferences.java
+++ b/gerrit-gwtui-common/src/main/java/com/google/gerrit/client/info/GeneralPreferences.java
@@ -51,6 +51,7 @@
p.sizeBarInChangeTable(d.sizeBarInChangeTable);
p.legacycidInChangeTable(d.legacycidInChangeTable);
p.muteCommonPathPrefixes(d.muteCommonPathPrefixes);
+ p.signedOffBy(d.signedOffBy);
p.reviewCategoryStrategy(d.getReviewCategoryStrategy());
p.diffView(d.getDiffView());
p.emailStrategy(d.emailStrategy);
@@ -109,6 +110,9 @@
public final native boolean muteCommonPathPrefixes()
/*-{ return this.mute_common_path_prefixes || false }-*/;
+ public final native boolean signedOffBy()
+ /*-{ return this.signed_off_by || false }-*/;
+
public final ReviewCategoryStrategy reviewCategoryStrategy() {
String s = reviewCategeoryStrategyRaw();
return s != null ? ReviewCategoryStrategy.valueOf(s) : ReviewCategoryStrategy.NONE;
@@ -176,6 +180,9 @@
public final native void muteCommonPathPrefixes(boolean s)
/*-{ this.mute_common_path_prefixes = s }-*/;
+ public final native void signedOffBy(boolean s)
+ /*-{ this.signed_off_by = s }-*/;
+
public final void reviewCategoryStrategy(ReviewCategoryStrategy s) {
reviewCategoryStrategyRaw(s != null ? s.toString() : null);
}
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.java
index e49b95f..549923c 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.java
@@ -42,6 +42,7 @@
String showSizeBarInChangeTable();
String showLegacycidInChangeTable();
String muteCommonPathPrefixes();
+ String signedOffBy();
String myMenu();
String myMenuInfo();
String myMenuName();
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.properties
index 9a00ded..0ed2620 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.properties
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/AccountConstants.properties
@@ -28,6 +28,7 @@
showSizeBarInChangeTable = Show Change Sizes As Colored Bars
showLegacycidInChangeTable = Show Change Number In Changes Table
muteCommonPathPrefixes = Mute Common Path Prefixes In File List
+signedOffBy = Insert Signed-off-by Footer For Inline Edit Changes
myMenu = My Menu
myMenuInfo = \
Menu items for the 'My' top level menu. \
diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyPreferencesScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyPreferencesScreen.java
index 2a00853..f605abf 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyPreferencesScreen.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/account/MyPreferencesScreen.java
@@ -54,6 +54,7 @@
private CheckBox sizeBarInChangeTable;
private CheckBox legacycidInChangeTable;
private CheckBox muteCommonPathPrefixes;
+ private CheckBox signedOffBy;
private ListBox maximumPageSize;
private ListBox dateFormat;
private ListBox timeFormat;
@@ -152,9 +153,10 @@
sizeBarInChangeTable = new CheckBox(Util.C.showSizeBarInChangeTable());
legacycidInChangeTable = new CheckBox(Util.C.showLegacycidInChangeTable());
muteCommonPathPrefixes = new CheckBox(Util.C.muteCommonPathPrefixes());
+ signedOffBy = new CheckBox(Util.C.signedOffBy());
boolean flashClippy = !UserAgent.hasJavaScriptClipboard() && UserAgent.Flash.isInstalled();
- final Grid formGrid = new Grid(11 + (flashClippy ? 1 : 0), 2);
+ final Grid formGrid = new Grid(12 + (flashClippy ? 1 : 0), 2);
int row = 0;
formGrid.setText(row, labelIdx, "");
@@ -197,6 +199,9 @@
formGrid.setText(row, labelIdx, Util.C.emailFieldLabel());
formGrid.setWidget(row, fieldIdx, emailStrategy);
+
+ formGrid.setText(row, labelIdx, "");
+ formGrid.setWidget(row, fieldIdx, signedOffBy);
row++;
formGrid.setText(row, labelIdx, Util.C.diffViewLabel());
@@ -228,6 +233,7 @@
e.listenTo(sizeBarInChangeTable);
e.listenTo(legacycidInChangeTable);
e.listenTo(muteCommonPathPrefixes);
+ e.listenTo(signedOffBy);
e.listenTo(diffView);
e.listenTo(reviewCategoryStrategy);
e.listenTo(emailStrategy);
@@ -260,6 +266,7 @@
sizeBarInChangeTable.setEnabled(on);
legacycidInChangeTable.setEnabled(on);
muteCommonPathPrefixes.setEnabled(on);
+ signedOffBy.setEnabled(on);
reviewCategoryStrategy.setEnabled(on);
diffView.setEnabled(on);
emailStrategy.setEnabled(on);
@@ -277,6 +284,7 @@
sizeBarInChangeTable.setValue(p.sizeBarInChangeTable());
legacycidInChangeTable.setValue(p.legacycidInChangeTable());
muteCommonPathPrefixes.setValue(p.muteCommonPathPrefixes());
+ signedOffBy.setValue(p.signedOffBy());
setListBox(reviewCategoryStrategy,
GeneralPreferencesInfo.ReviewCategoryStrategy.NONE,
p.reviewCategoryStrategy());
@@ -363,6 +371,7 @@
p.sizeBarInChangeTable(sizeBarInChangeTable.getValue());
p.legacycidInChangeTable(legacycidInChangeTable.getValue());
p.muteCommonPathPrefixes(muteCommonPathPrefixes.getValue());
+ p.signedOffBy(signedOffBy.getValue());
p.reviewCategoryStrategy(getListBox(reviewCategoryStrategy,
ReviewCategoryStrategy.NONE,
ReviewCategoryStrategy.values()));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
index 52f6b7d..73c2f7c 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
@@ -14,11 +14,14 @@
package com.google.gerrit.server.change;
+import static org.eclipse.jgit.lib.Constants.SIGNED_OFF_BY_TAG;
+
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.Capable;
import com.google.gerrit.extensions.client.ChangeStatus;
+import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.restapi.AuthException;
@@ -41,6 +44,9 @@
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.Sequences;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.account.AccountState;
+import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.GitRepositoryManager;
@@ -79,8 +85,10 @@
public class CreateChange implements
RestModifyView<TopLevelResource, ChangeInput> {
+ private final String anonymousCowardName;
private final Provider<ReviewDb> db;
private final GitRepositoryManager gitManager;
+ private final AccountCache accountCache;
private final Sequences seq;
private final TimeZone serverTimeZone;
private final Provider<CurrentUser> user;
@@ -93,8 +101,10 @@
private final boolean allowDrafts;
@Inject
- CreateChange(Provider<ReviewDb> db,
+ CreateChange(@AnonymousCowardName String anonymousCowardName,
+ Provider<ReviewDb> db,
GitRepositoryManager gitManager,
+ AccountCache accountCache,
Sequences seq,
@GerritPersonIdent PersonIdent myIdent,
Provider<CurrentUser> user,
@@ -105,8 +115,10 @@
BatchUpdate.Factory updateFactory,
PatchSetUtil psUtil,
@GerritServerConfig Config config) {
+ this.anonymousCowardName = anonymousCowardName;
this.db = db;
this.gitManager = gitManager;
+ this.accountCache = accountCache;
this.seq = seq;
this.serverTimeZone = myIdent.getTimeZone();
this.user = user;
@@ -204,6 +216,9 @@
Timestamp now = TimeUtil.nowTs();
IdentifiedUser me = user.get().asIdentifiedUser();
PersonIdent author = me.newCommitterIdent(now, serverTimeZone);
+ AccountState account = accountCache.get(me.getAccountId());
+ GeneralPreferencesInfo info =
+ account.getAccount().getGeneralPreferencesInfo();
try (ObjectInserter oi = git.newObjectInserter()) {
ObjectId treeId =
@@ -211,6 +226,11 @@
ObjectId id = ChangeIdUtil.computeChangeId(treeId,
mergeTip, author, author, input.subject);
String commitMessage = ChangeIdUtil.insertId(input.subject, id);
+ if (Boolean.TRUE.equals(info.signedOffBy)) {
+ commitMessage += String.format("%s%s",
+ SIGNED_OFF_BY_TAG,
+ account.getAccount().getNameEmail(anonymousCowardName));
+ }
RevCommit c = newCommit(oi, rw, author, mergeTip, commitMessage);
