diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt index 38af68e436..837755560e 100644 --- a/Documentation/intro-user.txt +++ b/Documentation/intro-user.txt @@ -827,6 +827,12 @@ commands that need to be copied frequently, such as the Change-Ids, commit IDs and download commands. Note that this option is only shown if the Flash plugin is available and the JavaScript Clipboard API is unavailable. +- [[work-in-progress-by-default]]`Set new changes work-in-progress`: ++ +Whether new changes are uploaded as work-in-progress per default. This +preference just sets the default; the behavior can still be overridden using a +link:user-upload.html#wip[push option]. + [[my-menu]] In addition it is possible to customize the menu entries of the `My` menu. This can be used to make the navigation to frequently used diff --git a/Documentation/rest-api-accounts.txt b/Documentation/rest-api-accounts.txt index e7549d9f4d..95dd4f2fcc 100644 --- a/Documentation/rest-api-accounts.txt +++ b/Documentation/rest-api-accounts.txt @@ -1249,6 +1249,7 @@ any account. "review_category_strategy": "ABBREV", "mute_common_path_prefixes": true, "publish_comments_on_push": true, + "work_in_progress_by_default": true, "default_base_for_merges": "FIRST_PARENT", "my": [ { @@ -1355,6 +1356,7 @@ link:#preferences-info[PreferencesInfo] entity. "review_category_strategy": "NAME", "diff_view": "SIDE_BY_SIDE", "publish_comments_on_push": true, + "work_in_progress_by_default": true, "mute_common_path_prefixes": true, "my": [ { @@ -2645,6 +2647,9 @@ Allowed values are `AUTO_MERGE` and `FIRST_PARENT`. |`publish_comments_on_push` |not set if `false`| Whether to link:user-upload.html#publish-comments[publish draft comments] on push by default. +|`work_in_progress_by_default` |not set if `false`| +Whether to link:user-upload.html#wip[set work-in-progress] on +push or on create changes online by default. |============================================ [[preferences-input]] diff --git a/Documentation/user-upload.txt b/Documentation/user-upload.txt index da21809c4e..1e76df5c97 100644 --- a/Documentation/user-upload.txt +++ b/Documentation/user-upload.txt @@ -309,6 +309,11 @@ flag from a change on push, explicitly specify the `ready` option: Only change owners, project owners and site administrators can specify `work-in-progress` and `ready` options on push. +The default for this option can be set as a +link:intro-user.html#work-in-progress-by-default[user preference]. If the +preference is set so the default behavior is to create `work-in-progress` +changes, this can be overridden with the `ready` option. + [[message]] ==== Message diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/WorkInProgressByDefaultIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/WorkInProgressByDefaultIT.java index 0f4a9dcd5c..34d87d0030 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/WorkInProgressByDefaultIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/WorkInProgressByDefaultIT.java @@ -19,12 +19,14 @@ import static com.google.common.truth.Truth.assertThat; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.extensions.api.projects.ConfigInput; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.extensions.client.InheritableBoolean; import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInput; import com.google.gerrit.reviewdb.client.Project; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.junit.TestRepository; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -38,6 +40,14 @@ public class WorkInProgressByDefaultIT extends AbstractDaemonTest { project2 = createProject("project-2", project1); } + @After + public void tearDown() throws Exception { + setApiUser(admin); + GeneralPreferencesInfo prefs = gApi.accounts().id(admin.id.get()).getPreferences(); + prefs.workInProgressByDefault = false; + gApi.accounts().id(admin.id.get()).setPreferences(prefs); + } + @Test public void createChangeWithWorkInProgressByDefaultForProjectDisabled() throws Exception { ChangeInfo info = @@ -52,6 +62,13 @@ public class WorkInProgressByDefaultIT extends AbstractDaemonTest { assertThat(gApi.changes().create(input).get().workInProgress).isTrue(); } + @Test + public void createChangeWithWorkInProgressByDefaultForUserEnabled() throws Exception { + setWorkInProgressByDefaultForUser(); + ChangeInput input = new ChangeInput(project2.get(), "master", "empty change"); + assertThat(gApi.changes().create(input).get().workInProgress).isTrue(); + } + @Test public void createChangeBypassWorkInProgressByDefaultForProjectEnabled() throws Exception { setWorkInProgressByDefaultForProject(project2); @@ -60,6 +77,14 @@ public class WorkInProgressByDefaultIT extends AbstractDaemonTest { assertThat(gApi.changes().create(input).get().workInProgress).isNull(); } + @Test + public void createChangeBypassWorkInProgressByDefaultForUserEnabled() throws Exception { + setWorkInProgressByDefaultForUser(); + ChangeInput input = new ChangeInput(project2.get(), "master", "empty change"); + input.workInProgress = false; + assertThat(gApi.changes().create(input).get().workInProgress).isNull(); + } + @Test public void createChangeWithWorkInProgressByDefaultForProjectInherited() throws Exception { setWorkInProgressByDefaultForProject(project1); @@ -74,6 +99,12 @@ public class WorkInProgressByDefaultIT extends AbstractDaemonTest { assertThat(createChange(project2).getChange().change().isWorkInProgress()).isTrue(); } + @Test + public void pushWithWorkInProgressByDefaultForUserEnabled() throws Exception { + setWorkInProgressByDefaultForUser(); + assertThat(createChange(project2).getChange().change().isWorkInProgress()).isTrue(); + } + @Test public void pushBypassWorkInProgressByDefaultForProjectEnabled() throws Exception { setWorkInProgressByDefaultForProject(project2); @@ -82,6 +113,14 @@ public class WorkInProgressByDefaultIT extends AbstractDaemonTest { .isFalse(); } + @Test + public void pushBypassWorkInProgressByDefaultForUserEnabled() throws Exception { + setWorkInProgressByDefaultForUser(); + assertThat( + createChange(project2, "refs/for/master%ready").getChange().change().isWorkInProgress()) + .isFalse(); + } + @Test public void pushWithWorkInProgressByDefaultForProjectDisabled() throws Exception { assertThat(createChange(project2).getChange().change().isWorkInProgress()).isFalse(); @@ -99,6 +138,12 @@ public class WorkInProgressByDefaultIT extends AbstractDaemonTest { gApi.projects().name(p.get()).config(input); } + private void setWorkInProgressByDefaultForUser() throws Exception { + GeneralPreferencesInfo prefs = gApi.accounts().id(admin.id.get()).getPreferences(); + prefs.workInProgressByDefault = true; + gApi.accounts().id(admin.id.get()).setPreferences(prefs); + } + private PushOneCommit.Result createChange(Project.NameKey p) throws Exception { return createChange(p, "refs/for/master"); } 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 10455e36d2..a94a63da58 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,7 @@ import com.google.gerrit.extensions.api.changes.NotifyHandling; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.changes.SubmitInput; import com.google.gerrit.extensions.api.projects.ConfigInput; +import com.google.gerrit.extensions.client.GeneralPreferencesInfo; import com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy; import com.google.gerrit.extensions.client.InheritableBoolean; import com.google.gerrit.extensions.client.ReviewerState; @@ -950,6 +951,30 @@ public class ChangeNotificationsIT extends AbstractNotificationTest { assertThat(sender).notSent(); } + @Test + public void createWipChangeWithWorkInProgressByDefaultForUser() throws Exception { + // Make sure owner user is created + StagedChange sc = stageReviewableChange(); + // All was cleaned already + assertThat(sender).notSent(); + + // Toggle workInProgress flag for owner + GeneralPreferencesInfo prefs = gApi.accounts().id(sc.owner.id.get()).getPreferences(); + prefs.workInProgressByDefault = true; + gApi.accounts().id(sc.owner.id.get()).setPreferences(prefs); + + // Create another change without notification that should be wip + StagedPreChange spc = stagePreChange("refs/for/master"); + Truth.assertThat(gApi.changes().id(spc.changeId).get().workInProgress).isTrue(); + assertThat(sender).notSent(); + + // Clean up workInProgressByDefault by owner + prefs = gApi.accounts().id(sc.owner.id.get()).getPreferences(); + Truth.assertThat(prefs.workInProgressByDefault).isTrue(); + prefs.workInProgressByDefault = false; + gApi.accounts().id(sc.owner.id.get()).setPreferences(prefs); + } + @Test public void createReviewableChangeWithNotifyOwnerReviewers() throws Exception { stagePreChange("refs/for/master%notify=OWNER_REVIEWERS"); 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 9dcba5e4b9..1f16d8d847 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 @@ -158,6 +158,7 @@ public class GeneralPreferencesInfo { public EmailFormat emailFormat; public DefaultBase defaultBaseForMerges; public Boolean publishCommentsOnPush; + public Boolean workInProgressByDefault; public boolean isShowInfoInReviewCategory() { return getReviewCategoryStrategy() != ReviewCategoryStrategy.NONE; @@ -227,6 +228,7 @@ public class GeneralPreferencesInfo { p.signedOffBy = false; p.defaultBaseForMerges = DefaultBase.FIRST_PARENT; p.publishCommentsOnPush = false; + p.workInProgressByDefault = 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 1dcb284ed1..fbdf52cbfb 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 @@ -151,6 +151,9 @@ public class GeneralPreferences extends JavaScriptObject { public final native boolean publishCommentsOnPush() /*-{ return this.publish_comments_on_push || false }-*/; + public final native boolean + workInProgressByDefault() /*-{ return this.work_in_progress_by_default || false }-*/; + public final native JsArray my() /*-{ return this.my; }-*/; public final native void changesPerPage(int n) /*-{ this.changes_per_page = n }-*/; @@ -230,6 +233,9 @@ public class GeneralPreferences extends JavaScriptObject { public final native void publishCommentsOnPush( boolean p) /*-{ this.publish_comments_on_push = p }-*/; + public final native void workInProgressByDefault( + boolean p) /*-{ this.work_in_progress_by_default = p }-*/; + public final void setMyMenus(List myMenus) { initMy(); for (TopMenuItem n : myMenus) { 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 0b32cd5839..3e21619256 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 @@ -69,6 +69,8 @@ public interface AccountConstants extends Constants { String publishCommentsOnPush(); + String workInProgressByDefault(); + String myMenu(); String myMenuInfo(); 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 deba4f8086..59b8b3dc59 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 @@ -39,6 +39,7 @@ 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 publishCommentsOnPush = Publish Draft Comments When a Change Is Updated by Push +workInProgressByDefault = Set all new changes work-in-progress by default 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 f3490658ac..afb87188d4 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 @@ -56,6 +56,7 @@ public class MyPreferencesScreen extends SettingsScreen { private CheckBox muteCommonPathPrefixes; private CheckBox signedOffBy; private CheckBox publishCommentsOnPush; + private CheckBox workInProgressByDefault; private ListBox maximumPageSize; private ListBox dateFormat; private ListBox timeFormat; @@ -163,9 +164,10 @@ public class MyPreferencesScreen extends SettingsScreen { muteCommonPathPrefixes = new CheckBox(Util.C.muteCommonPathPrefixes()); signedOffBy = new CheckBox(Util.C.signedOffBy()); publishCommentsOnPush = new CheckBox(Util.C.publishCommentsOnPush()); + workInProgressByDefault = new CheckBox(Util.C.workInProgressByDefault()); boolean flashClippy = !UserAgent.hasJavaScriptClipboard() && UserAgent.Flash.isInstalled(); - final Grid formGrid = new Grid(15 + (flashClippy ? 1 : 0), 2); + final Grid formGrid = new Grid(16 + (flashClippy ? 1 : 0), 2); int row = 0; @@ -229,6 +231,10 @@ public class MyPreferencesScreen extends SettingsScreen { formGrid.setWidget(row, fieldIdx, publishCommentsOnPush); row++; + formGrid.setText(row, labelIdx, ""); + formGrid.setWidget(row, fieldIdx, workInProgressByDefault); + row++; + if (flashClippy) { formGrid.setText(row, labelIdx, ""); formGrid.setWidget(row, fieldIdx, useFlashClipboard); @@ -264,6 +270,7 @@ public class MyPreferencesScreen extends SettingsScreen { e.listenTo(muteCommonPathPrefixes); e.listenTo(signedOffBy); e.listenTo(publishCommentsOnPush); + e.listenTo(workInProgressByDefault); e.listenTo(diffView); e.listenTo(reviewCategoryStrategy); e.listenTo(emailStrategy); @@ -303,6 +310,7 @@ public class MyPreferencesScreen extends SettingsScreen { muteCommonPathPrefixes.setEnabled(on); signedOffBy.setEnabled(on); publishCommentsOnPush.setEnabled(on); + workInProgressByDefault.setEnabled(on); reviewCategoryStrategy.setEnabled(on); diffView.setEnabled(on); emailStrategy.setEnabled(on); @@ -329,6 +337,7 @@ public class MyPreferencesScreen extends SettingsScreen { muteCommonPathPrefixes.setValue(p.muteCommonPathPrefixes()); signedOffBy.setValue(p.signedOffBy()); publishCommentsOnPush.setValue(p.publishCommentsOnPush()); + workInProgressByDefault.setValue(p.workInProgressByDefault()); setListBox( reviewCategoryStrategy, GeneralPreferencesInfo.ReviewCategoryStrategy.NONE, @@ -421,6 +430,7 @@ public class MyPreferencesScreen extends SettingsScreen { p.muteCommonPathPrefixes(muteCommonPathPrefixes.getValue()); p.signedOffBy(signedOffBy.getValue()); p.publishCommentsOnPush(publishCommentsOnPush.getValue()); + p.workInProgressByDefault(workInProgressByDefault.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 e17b8b6491..bbc3de0027 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 @@ -188,11 +188,6 @@ public class CreateChange throw new MethodNotAllowedException("private changes are disabled"); } - boolean isWorkInProgress = - input.workInProgress == null - ? rsrc.getProjectState().isWorkInProgressByDefault() - : input.workInProgress; - contributorAgreements.check(rsrc.getNameKey(), rsrc.getUser()); Project.NameKey project = rsrc.getNameKey(); @@ -243,6 +238,12 @@ public class CreateChange AccountState account = accountCache.get(me.getAccountId()); GeneralPreferencesInfo info = account.getAccount().getGeneralPreferencesInfo(); + boolean isWorkInProgress = + input.workInProgress == null + ? rsrc.getProjectState().isWorkInProgressByDefault() + || MoreObjects.firstNonNull(info.workInProgressByDefault, false) + : input.workInProgress; + // Add a Change-Id line if there isn't already one String commitMessage = subject; if (ChangeIdUtil.indexOfChangeId(commitMessage, "\n") == -1) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index 41f229a07c..c4a3620ccc 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -2154,7 +2154,9 @@ class ReceiveCommits { return; } magicBranch.workInProgress = - projectCache.get(project.getNameKey()).isWorkInProgressByDefault(); + projectCache.get(project.getNameKey()).isWorkInProgressByDefault() + || firstNonNull( + user.getAccount().getGeneralPreferencesInfo().workInProgressByDefault, false); } private void addOps(BatchUpdate bu) throws RestApiException { diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html index addb9da2ee..49ba141042 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.html @@ -190,6 +190,16 @@ limitations under the License. on-change="_handlePublishCommentsOnPushChanged"> +
+ Set new changes to "work in progress" by default + + + +
Insert Signed-off-by Footer For Inline Edit Changes diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js index 115a612f7d..3b0f5091d2 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view.js @@ -22,6 +22,7 @@ 'diff_view', 'expand_inline_diffs', 'publish_comments_on_push', + 'work_in_progress_by_default', 'signed_off_by', 'email_format', ]; @@ -265,6 +266,11 @@ this.$.publishCommentsOnPush.checked); }, + _handleWorkInProgressByDefault() { + this.set('_localPrefs.work_in_progress_by_default', + this.$.workInProgressByDefault.checked); + }, + _handleInsertSignedOff() { this.set('_localPrefs.signed_off_by', this.$.insertSignedOff.checked); }, diff --git a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html index 6b998bf96c..01a3dc2dea 100644 --- a/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html +++ b/polygerrit-ui/app/elements/settings/gr-settings-view/gr-settings-view_test.html @@ -172,6 +172,9 @@ limitations under the License. .firstElementChild.checked, false); assert.equal(valueOf('Publish comments on push', 'preferences') .firstElementChild.checked, false); + assert.equal(valueOf( + 'Set new changes to "work in progress" by default', 'preferences') + .firstElementChild.checked, false); assert.equal(valueOf( 'Insert Signed-off-by Footer For Inline Edit Changes', 'preferences') .firstElementChild.checked, false); @@ -232,6 +235,30 @@ limitations under the License. }); }); + test('set new changes work-in-progress', done => { + const newChangesWorkInProgress = + valueOf('Set new changes to "work in progress" by default', + 'preferences').firstElementChild; + MockInteractions.tap(newChangesWorkInProgress); + + assert.isFalse(element._menuChanged); + assert.isTrue(element._prefsChanged); + + stub('gr-rest-api-interface', { + savePreferences(prefs) { + assert.equal(prefs.work_in_progress_by_default, true); + return Promise.resolve(); + }, + }); + + // Save the change. + element._handleSavePreferences().then(() => { + assert.isFalse(element._prefsChanged); + assert.isFalse(element._menuChanged); + done(); + }); + }); + test('diff preferences', done => { // Rendered with the expected preferences selected. assert.equal(valueOf('Context', 'diffPreferences')