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
This commit is contained in:
David Ostrovsky
2015-11-01 22:49:28 +01:00
committed by David Pursehouse
parent 5f8ce2c486
commit 5d8f62c938
9 changed files with 82 additions and 5 deletions

View File

@@ -1946,6 +1946,9 @@ Whether to show the change sizes as colored bars in the change table.
Whether to show change number in the change table. Whether to show change number in the change table.
|`mute_common_path_prefixes` |not set if `false`| |`mute_common_path_prefixes` |not set if `false`|
Whether to mute common path prefixes in file names in the file table. 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` || |`review_category_strategy` ||
The strategy used to displayed info in the review category column. The strategy used to displayed info in the review category column.
Allowed values are `NONE`, `NAME`, `EMAIL`, `USERNAME`, `ABBREV`. Allowed values are `NONE`, `NAME`, `EMAIL`, `USERNAME`, `ABBREV`.
@@ -2001,6 +2004,9 @@ Whether to show the change sizes as colored bars in the change table.
Whether to show change number in the change table. Whether to show change number in the change table.
|`mute_common_path_prefixes` |optional| |`mute_common_path_prefixes` |optional|
Whether to mute common path prefixes in file names in the file table. 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| |`review_category_strategy` |optional|
The strategy used to displayed info in the review category column. The strategy used to displayed info in the review category column.
Allowed values are `NONE`, `NAME`, `EMAIL`, `USERNAME`, `ABBREV`. Allowed values are `NONE`, `NAME`, `EMAIL`, `USERNAME`, `ABBREV`.

View File

@@ -72,6 +72,7 @@ public class GeneralPreferencesIT extends AbstractDaemonTest {
assertThat(o.legacycidInChangeTable).isNull(); assertThat(o.legacycidInChangeTable).isNull();
assertThat(o.muteCommonPathPrefixes).isEqualTo( assertThat(o.muteCommonPathPrefixes).isEqualTo(
d.muteCommonPathPrefixes); d.muteCommonPathPrefixes);
assertThat(o.signedOffBy).isNull();
assertThat(o.reviewCategoryStrategy).isEqualTo( assertThat(o.reviewCategoryStrategy).isEqualTo(
d.getReviewCategoryStrategy()); d.getReviewCategoryStrategy());
assertThat(o.diffView).isEqualTo(d.getDiffView()); assertThat(o.diffView).isEqualTo(d.getDiffView());
@@ -93,6 +94,7 @@ public class GeneralPreferencesIT extends AbstractDaemonTest {
i.sizeBarInChangeTable ^= true; i.sizeBarInChangeTable ^= true;
i.legacycidInChangeTable ^= true; i.legacycidInChangeTable ^= true;
i.muteCommonPathPrefixes ^= true; i.muteCommonPathPrefixes ^= true;
i.signedOffBy ^= true;
i.reviewCategoryStrategy = ReviewCategoryStrategy.ABBREV; i.reviewCategoryStrategy = ReviewCategoryStrategy.ABBREV;
i.diffView = DiffView.UNIFIED_DIFF; i.diffView = DiffView.UNIFIED_DIFF;
i.my = new ArrayList<>(); i.my = new ArrayList<>();
@@ -117,6 +119,7 @@ public class GeneralPreferencesIT extends AbstractDaemonTest {
assertThat(o.sizeBarInChangeTable).isNull(); assertThat(o.sizeBarInChangeTable).isNull();
assertThat(o.legacycidInChangeTable).isEqualTo(i.legacycidInChangeTable); assertThat(o.legacycidInChangeTable).isEqualTo(i.legacycidInChangeTable);
assertThat(o.muteCommonPathPrefixes).isNull(); assertThat(o.muteCommonPathPrefixes).isNull();
assertThat(o.signedOffBy).isEqualTo(i.signedOffBy);
assertThat(o.reviewCategoryStrategy).isEqualTo( assertThat(o.reviewCategoryStrategy).isEqualTo(
i.getReviewCategoryStrategy()); i.getReviewCategoryStrategy());
assertThat(o.diffView).isEqualTo(i.getDiffView()); assertThat(o.diffView).isEqualTo(i.getDiffView());

View File

@@ -17,11 +17,13 @@ package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume; import static com.google.common.truth.TruthJUnit.assume;
import static java.util.concurrent.TimeUnit.SECONDS; 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.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest; 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.ChangeStatus;
import com.google.gerrit.extensions.client.GeneralPreferencesInfo;
import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput; import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -42,7 +44,6 @@ import org.junit.AfterClass;
import org.junit.BeforeClass; import org.junit.BeforeClass;
import org.junit.Test; import org.junit.Test;
@NoHttpd
public class CreateChangeIT extends AbstractDaemonTest { public class CreateChangeIT extends AbstractDaemonTest {
@ConfigSuite.Config @ConfigSuite.Config
public static Config allowDraftsDisabled() { public static Config allowDraftsDisabled() {
@@ -59,7 +60,6 @@ public class CreateChangeIT extends AbstractDaemonTest {
TestTimeUtil.useSystemTime(); TestTimeUtil.useSystemTime();
} }
@Test @Test
public void createEmptyChange_MissingBranch() throws Exception { public void createEmptyChange_MissingBranch() throws Exception {
ChangeInput ci = new ChangeInput(); ChangeInput ci = new ChangeInput();
@@ -89,6 +89,17 @@ public class CreateChangeIT extends AbstractDaemonTest {
assertCreateSucceeds(newChangeInput(ChangeStatus.NEW)); assertCreateSucceeds(newChangeInput(ChangeStatus.NEW));
} }
@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 @Test
public void createDraftChange() throws Exception { public void createDraftChange() throws Exception {
assume().that(isAllowDrafts()).isTrue(); assume().that(isAllowDrafts()).isTrue();
@@ -164,4 +175,21 @@ public class CreateChangeIT extends AbstractDaemonTest {
} }
return draft ? ChangeStatus.DRAFT : ChangeStatus.NEW; 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();
}
} }

View File

@@ -119,6 +119,7 @@ public class GeneralPreferencesInfo {
public Boolean legacycidInChangeTable; public Boolean legacycidInChangeTable;
public ReviewCategoryStrategy reviewCategoryStrategy; public ReviewCategoryStrategy reviewCategoryStrategy;
public Boolean muteCommonPathPrefixes; public Boolean muteCommonPathPrefixes;
public Boolean signedOffBy;
public List<MenuItem> my; public List<MenuItem> my;
public Map<String, String> urlAliases; public Map<String, String> urlAliases;
public EmailStrategy emailStrategy; public EmailStrategy emailStrategy;
@@ -178,6 +179,7 @@ public class GeneralPreferencesInfo {
p.sizeBarInChangeTable = true; p.sizeBarInChangeTable = true;
p.legacycidInChangeTable = false; p.legacycidInChangeTable = false;
p.muteCommonPathPrefixes = true; p.muteCommonPathPrefixes = true;
p.signedOffBy = false;
return p; return p;
} }
} }

View File

@@ -51,6 +51,7 @@ public class GeneralPreferences extends JavaScriptObject {
p.sizeBarInChangeTable(d.sizeBarInChangeTable); p.sizeBarInChangeTable(d.sizeBarInChangeTable);
p.legacycidInChangeTable(d.legacycidInChangeTable); p.legacycidInChangeTable(d.legacycidInChangeTable);
p.muteCommonPathPrefixes(d.muteCommonPathPrefixes); p.muteCommonPathPrefixes(d.muteCommonPathPrefixes);
p.signedOffBy(d.signedOffBy);
p.reviewCategoryStrategy(d.getReviewCategoryStrategy()); p.reviewCategoryStrategy(d.getReviewCategoryStrategy());
p.diffView(d.getDiffView()); p.diffView(d.getDiffView());
p.emailStrategy(d.emailStrategy); p.emailStrategy(d.emailStrategy);
@@ -109,6 +110,9 @@ public class GeneralPreferences extends JavaScriptObject {
public final native boolean muteCommonPathPrefixes() public final native boolean muteCommonPathPrefixes()
/*-{ return this.mute_common_path_prefixes || false }-*/; /*-{ return this.mute_common_path_prefixes || false }-*/;
public final native boolean signedOffBy()
/*-{ return this.signed_off_by || false }-*/;
public final ReviewCategoryStrategy reviewCategoryStrategy() { public final ReviewCategoryStrategy reviewCategoryStrategy() {
String s = reviewCategeoryStrategyRaw(); String s = reviewCategeoryStrategyRaw();
return s != null ? ReviewCategoryStrategy.valueOf(s) : ReviewCategoryStrategy.NONE; return s != null ? ReviewCategoryStrategy.valueOf(s) : ReviewCategoryStrategy.NONE;
@@ -176,6 +180,9 @@ public class GeneralPreferences extends JavaScriptObject {
public final native void muteCommonPathPrefixes(boolean s) public final native void muteCommonPathPrefixes(boolean s)
/*-{ this.mute_common_path_prefixes = 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) { public final void reviewCategoryStrategy(ReviewCategoryStrategy s) {
reviewCategoryStrategyRaw(s != null ? s.toString() : null); reviewCategoryStrategyRaw(s != null ? s.toString() : null);
} }

View File

@@ -42,6 +42,7 @@ public interface AccountConstants extends Constants {
String showSizeBarInChangeTable(); String showSizeBarInChangeTable();
String showLegacycidInChangeTable(); String showLegacycidInChangeTable();
String muteCommonPathPrefixes(); String muteCommonPathPrefixes();
String signedOffBy();
String myMenu(); String myMenu();
String myMenuInfo(); String myMenuInfo();
String myMenuName(); String myMenuName();

View File

@@ -28,6 +28,7 @@ showRelativeDateInChangeTable = Show Relative Dates In Changes Table
showSizeBarInChangeTable = Show Change Sizes As Colored Bars showSizeBarInChangeTable = Show Change Sizes As Colored Bars
showLegacycidInChangeTable = Show Change Number In Changes Table showLegacycidInChangeTable = Show Change Number In Changes Table
muteCommonPathPrefixes = Mute Common Path Prefixes In File List muteCommonPathPrefixes = Mute Common Path Prefixes In File List
signedOffBy = Insert Signed-off-by Footer For Inline Edit Changes
myMenu = My Menu myMenu = My Menu
myMenuInfo = \ myMenuInfo = \
Menu items for the 'My' top level menu. \ Menu items for the 'My' top level menu. \

View File

@@ -54,6 +54,7 @@ public class MyPreferencesScreen extends SettingsScreen {
private CheckBox sizeBarInChangeTable; private CheckBox sizeBarInChangeTable;
private CheckBox legacycidInChangeTable; private CheckBox legacycidInChangeTable;
private CheckBox muteCommonPathPrefixes; private CheckBox muteCommonPathPrefixes;
private CheckBox signedOffBy;
private ListBox maximumPageSize; private ListBox maximumPageSize;
private ListBox dateFormat; private ListBox dateFormat;
private ListBox timeFormat; private ListBox timeFormat;
@@ -152,9 +153,10 @@ public class MyPreferencesScreen extends SettingsScreen {
sizeBarInChangeTable = new CheckBox(Util.C.showSizeBarInChangeTable()); sizeBarInChangeTable = new CheckBox(Util.C.showSizeBarInChangeTable());
legacycidInChangeTable = new CheckBox(Util.C.showLegacycidInChangeTable()); legacycidInChangeTable = new CheckBox(Util.C.showLegacycidInChangeTable());
muteCommonPathPrefixes = new CheckBox(Util.C.muteCommonPathPrefixes()); muteCommonPathPrefixes = new CheckBox(Util.C.muteCommonPathPrefixes());
signedOffBy = new CheckBox(Util.C.signedOffBy());
boolean flashClippy = !UserAgent.hasJavaScriptClipboard() && UserAgent.Flash.isInstalled(); 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; int row = 0;
formGrid.setText(row, labelIdx, ""); formGrid.setText(row, labelIdx, "");
@@ -197,6 +199,9 @@ public class MyPreferencesScreen extends SettingsScreen {
formGrid.setText(row, labelIdx, Util.C.emailFieldLabel()); formGrid.setText(row, labelIdx, Util.C.emailFieldLabel());
formGrid.setWidget(row, fieldIdx, emailStrategy); formGrid.setWidget(row, fieldIdx, emailStrategy);
formGrid.setText(row, labelIdx, "");
formGrid.setWidget(row, fieldIdx, signedOffBy);
row++; row++;
formGrid.setText(row, labelIdx, Util.C.diffViewLabel()); formGrid.setText(row, labelIdx, Util.C.diffViewLabel());
@@ -228,6 +233,7 @@ public class MyPreferencesScreen extends SettingsScreen {
e.listenTo(sizeBarInChangeTable); e.listenTo(sizeBarInChangeTable);
e.listenTo(legacycidInChangeTable); e.listenTo(legacycidInChangeTable);
e.listenTo(muteCommonPathPrefixes); e.listenTo(muteCommonPathPrefixes);
e.listenTo(signedOffBy);
e.listenTo(diffView); e.listenTo(diffView);
e.listenTo(reviewCategoryStrategy); e.listenTo(reviewCategoryStrategy);
e.listenTo(emailStrategy); e.listenTo(emailStrategy);
@@ -260,6 +266,7 @@ public class MyPreferencesScreen extends SettingsScreen {
sizeBarInChangeTable.setEnabled(on); sizeBarInChangeTable.setEnabled(on);
legacycidInChangeTable.setEnabled(on); legacycidInChangeTable.setEnabled(on);
muteCommonPathPrefixes.setEnabled(on); muteCommonPathPrefixes.setEnabled(on);
signedOffBy.setEnabled(on);
reviewCategoryStrategy.setEnabled(on); reviewCategoryStrategy.setEnabled(on);
diffView.setEnabled(on); diffView.setEnabled(on);
emailStrategy.setEnabled(on); emailStrategy.setEnabled(on);
@@ -277,6 +284,7 @@ public class MyPreferencesScreen extends SettingsScreen {
sizeBarInChangeTable.setValue(p.sizeBarInChangeTable()); sizeBarInChangeTable.setValue(p.sizeBarInChangeTable());
legacycidInChangeTable.setValue(p.legacycidInChangeTable()); legacycidInChangeTable.setValue(p.legacycidInChangeTable());
muteCommonPathPrefixes.setValue(p.muteCommonPathPrefixes()); muteCommonPathPrefixes.setValue(p.muteCommonPathPrefixes());
signedOffBy.setValue(p.signedOffBy());
setListBox(reviewCategoryStrategy, setListBox(reviewCategoryStrategy,
GeneralPreferencesInfo.ReviewCategoryStrategy.NONE, GeneralPreferencesInfo.ReviewCategoryStrategy.NONE,
p.reviewCategoryStrategy()); p.reviewCategoryStrategy());
@@ -363,6 +371,7 @@ public class MyPreferencesScreen extends SettingsScreen {
p.sizeBarInChangeTable(sizeBarInChangeTable.getValue()); p.sizeBarInChangeTable(sizeBarInChangeTable.getValue());
p.legacycidInChangeTable(legacycidInChangeTable.getValue()); p.legacycidInChangeTable(legacycidInChangeTable.getValue());
p.muteCommonPathPrefixes(muteCommonPathPrefixes.getValue()); p.muteCommonPathPrefixes(muteCommonPathPrefixes.getValue());
p.signedOffBy(signedOffBy.getValue());
p.reviewCategoryStrategy(getListBox(reviewCategoryStrategy, p.reviewCategoryStrategy(getListBox(reviewCategoryStrategy,
ReviewCategoryStrategy.NONE, ReviewCategoryStrategy.NONE,
ReviewCategoryStrategy.values())); ReviewCategoryStrategy.values()));

View File

@@ -14,11 +14,14 @@
package com.google.gerrit.server.change; 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.base.Strings;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
import com.google.gerrit.common.TimeUtil; import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.data.Capable; import com.google.gerrit.common.data.Capable;
import com.google.gerrit.extensions.client.ChangeStatus; 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.ChangeInfo;
import com.google.gerrit.extensions.common.ChangeInput; import com.google.gerrit.extensions.common.ChangeInput;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
@@ -41,6 +44,9 @@ import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.Sequences; 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.config.GerritServerConfig;
import com.google.gerrit.server.git.BatchUpdate; import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
@@ -79,8 +85,10 @@ import java.util.TimeZone;
public class CreateChange implements public class CreateChange implements
RestModifyView<TopLevelResource, ChangeInput> { RestModifyView<TopLevelResource, ChangeInput> {
private final String anonymousCowardName;
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final GitRepositoryManager gitManager; private final GitRepositoryManager gitManager;
private final AccountCache accountCache;
private final Sequences seq; private final Sequences seq;
private final TimeZone serverTimeZone; private final TimeZone serverTimeZone;
private final Provider<CurrentUser> user; private final Provider<CurrentUser> user;
@@ -93,8 +101,10 @@ public class CreateChange implements
private final boolean allowDrafts; private final boolean allowDrafts;
@Inject @Inject
CreateChange(Provider<ReviewDb> db, CreateChange(@AnonymousCowardName String anonymousCowardName,
Provider<ReviewDb> db,
GitRepositoryManager gitManager, GitRepositoryManager gitManager,
AccountCache accountCache,
Sequences seq, Sequences seq,
@GerritPersonIdent PersonIdent myIdent, @GerritPersonIdent PersonIdent myIdent,
Provider<CurrentUser> user, Provider<CurrentUser> user,
@@ -105,8 +115,10 @@ public class CreateChange implements
BatchUpdate.Factory updateFactory, BatchUpdate.Factory updateFactory,
PatchSetUtil psUtil, PatchSetUtil psUtil,
@GerritServerConfig Config config) { @GerritServerConfig Config config) {
this.anonymousCowardName = anonymousCowardName;
this.db = db; this.db = db;
this.gitManager = gitManager; this.gitManager = gitManager;
this.accountCache = accountCache;
this.seq = seq; this.seq = seq;
this.serverTimeZone = myIdent.getTimeZone(); this.serverTimeZone = myIdent.getTimeZone();
this.user = user; this.user = user;
@@ -204,6 +216,9 @@ public class CreateChange implements
Timestamp now = TimeUtil.nowTs(); Timestamp now = TimeUtil.nowTs();
IdentifiedUser me = user.get().asIdentifiedUser(); IdentifiedUser me = user.get().asIdentifiedUser();
PersonIdent author = me.newCommitterIdent(now, serverTimeZone); PersonIdent author = me.newCommitterIdent(now, serverTimeZone);
AccountState account = accountCache.get(me.getAccountId());
GeneralPreferencesInfo info =
account.getAccount().getGeneralPreferencesInfo();
try (ObjectInserter oi = git.newObjectInserter()) { try (ObjectInserter oi = git.newObjectInserter()) {
ObjectId treeId = ObjectId treeId =
@@ -211,6 +226,11 @@ public class CreateChange implements
ObjectId id = ChangeIdUtil.computeChangeId(treeId, ObjectId id = ChangeIdUtil.computeChangeId(treeId,
mergeTip, author, author, input.subject); mergeTip, author, author, input.subject);
String commitMessage = ChangeIdUtil.insertId(input.subject, id); 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); RevCommit c = newCommit(oi, rw, author, mergeTip, commitMessage);