Merge "Consolidate render event dispatch in gr-diff"
diff --git a/.mailmap b/.mailmap
index c863847..b5c119c 100644
--- a/.mailmap
+++ b/.mailmap
@@ -6,6 +6,7 @@
Alex Ryazantsev <alex.ryazantsev@gmail.com> alex.ryazantsev <alex.ryazantsev@gmail.com>
Andrew Bonventre <andybons@chromium.org> <andybons@google.com>
Becky Siegel <beckysiegel@google.com> beckysiegel <beckysiegel@google.com>
+Ben Rohlfs <brohlfs@google.com> brohlfs <brohlfs@google.com>
Brad Larson <bklarson@gmail.com> <brad.larson@garmin.com>
Bruce Zu <bruce.zu.run10@gmail.com> <bruce.zu@sonyericsson.com>
Bruce Zu <bruce.zu.run10@gmail.com> <bruce.zu@sonymobile.com>
diff --git a/Documentation/intro-user.txt b/Documentation/intro-user.txt
index 544039b..17c9a61 100644
--- a/Documentation/intro-user.txt
+++ b/Documentation/intro-user.txt
@@ -548,7 +548,8 @@
----
Alternatively, click *Ready* from the Change screen.
-Only change owners, project owners and site administrators can mark changes as
+Change owners, project owners, site administrators and members of a group that
+was granted "Toggle Work In Progress state" permission can mark changes as
`work-in-progress` and `ready`.
[[wip-polygerrit]]
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 5d7a78b..cafd5ca 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -216,7 +216,8 @@
[[hashtag]]
hashtag:'HASHTAG'::
+
-Changes whose link:intro-user.html#hashtags[hashtag] matches 'HASHTAG' exactly.
+Changes whose link:intro-user.html#hashtags[hashtag] matches 'HASHTAG'.
+The match is case-insensitive.
[[ref]]
ref:'REF'::
diff --git a/WORKSPACE b/WORKSPACE
index 7aa4dce..c2629ba 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -47,11 +47,11 @@
# Golang support for PolyGerrit local dev server.
http_archive(
name = "io_bazel_rules_go",
- sha256 = "ee5fe78fe417c685ecb77a0a725dc9f6040ae5beb44a0ba4ddb55453aad23a8a",
- url = "https://github.com/bazelbuild/rules_go/releases/download/0.16.0/rules_go-0.16.0.tar.gz",
+ sha256 = "6776d68ebb897625dead17ae510eac3d5f6342367327875210df44dbe2aeeb19",
+ url = "https://github.com/bazelbuild/rules_go/releases/download/0.17.1/rules_go-0.17.1.tar.gz",
)
-load("@io_bazel_rules_go//go:def.bzl", "go_register_toolchains", "go_rules_dependencies")
+load("@io_bazel_rules_go//go:deps.bzl", "go_register_toolchains", "go_rules_dependencies")
go_rules_dependencies()
@@ -59,8 +59,8 @@
http_archive(
name = "bazel_gazelle",
- sha256 = "c0a5739d12c6d05b6c1ad56f2200cb0b57c5a70e03ebd2f7b87ce88cabf09c7b",
- urls = ["https://github.com/bazelbuild/bazel-gazelle/releases/download/0.14.0/bazel-gazelle-0.14.0.tar.gz"],
+ sha256 = "3c681998538231a2d24d0c07ed5a7658cb72bfb5fd4bf9911157c0e9ac6a2687",
+ urls = ["https://github.com/bazelbuild/bazel-gazelle/releases/download/0.17.0/bazel-gazelle-0.17.0.tar.gz"],
)
load("@bazel_gazelle//:deps.bzl", "gazelle_dependencies", "go_repository")
diff --git a/java/com/google/gerrit/common/data/Permission.java b/java/com/google/gerrit/common/data/Permission.java
index 2e9c2d6..3ba0ba7 100644
--- a/java/com/google/gerrit/common/data/Permission.java
+++ b/java/com/google/gerrit/common/data/Permission.java
@@ -46,6 +46,7 @@
public static final String REMOVE_REVIEWER = "removeReviewer";
public static final String SUBMIT = "submit";
public static final String SUBMIT_AS = "submitAs";
+ public static final String TOGGLE_WORK_IN_PROGRESS_STATE = "toggleWipState";
public static final String VIEW_PRIVATE_CHANGES = "viewPrivateChanges";
private static final List<String> NAMES_LC;
@@ -78,6 +79,7 @@
NAMES_LC.add(REMOVE_REVIEWER.toLowerCase());
NAMES_LC.add(SUBMIT.toLowerCase());
NAMES_LC.add(SUBMIT_AS.toLowerCase());
+ NAMES_LC.add(TOGGLE_WORK_IN_PROGRESS_STATE.toLowerCase());
NAMES_LC.add(VIEW_PRIVATE_CHANGES.toLowerCase());
LABEL_INDEX = NAMES_LC.indexOf(Permission.LABEL);
diff --git a/java/com/google/gerrit/server/change/WorkInProgressOp.java b/java/com/google/gerrit/server/change/WorkInProgressOp.java
index 02870fb..e5fdd8f 100644
--- a/java/com/google/gerrit/server/change/WorkInProgressOp.java
+++ b/java/com/google/gerrit/server/change/WorkInProgressOp.java
@@ -18,20 +18,14 @@
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
-import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.ChangeMessagesUtil;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.extensions.events.WorkInProgressStateChanged;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
-import com.google.gerrit.server.permissions.GlobalPermission;
-import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.update.BatchUpdateOp;
import com.google.gerrit.server.update.ChangeContext;
import com.google.gerrit.server.update.Context;
@@ -57,34 +51,6 @@
WorkInProgressOp create(boolean workInProgress, Input in);
}
- public static void checkPermissions(
- PermissionBackend permissionBackend, CurrentUser user, Change change)
- throws PermissionBackendException, AuthException {
- if (!user.isIdentifiedUser()) {
- throw new AuthException("Authentication required");
- }
-
- if (change.getOwner().equals(user.asIdentifiedUser().getAccountId())) {
- return;
- }
-
- try {
- permissionBackend.currentUser().check(GlobalPermission.ADMINISTRATE_SERVER);
- return;
- } catch (AuthException e) {
- // Skip.
- }
-
- try {
- permissionBackend
- .user(user)
- .project(change.getProject())
- .check(ProjectPermission.WRITE_CONFIG);
- } catch (AuthException exp) {
- throw new AuthException("not allowed to toggle work in progress");
- }
- }
-
private final ChangeMessagesUtil cmUtil;
private final EmailReviewComments.Factory email;
private final PatchSetUtil psUtil;
diff --git a/java/com/google/gerrit/server/permissions/ChangeControl.java b/java/com/google/gerrit/server/permissions/ChangeControl.java
index 250ca03..309bf51 100644
--- a/java/com/google/gerrit/server/permissions/ChangeControl.java
+++ b/java/com/google/gerrit/server/permissions/ChangeControl.java
@@ -176,6 +176,14 @@
return refControl.canForceEditTopicName();
}
+ /** Can this user toggle WorkInProgress state? */
+ private boolean canToggleWorkInProgressState() {
+ return isOwner()
+ || getProjectControl().isOwner()
+ || refControl.canPerform(Permission.TOGGLE_WORK_IN_PROGRESS_STATE)
+ || getProjectControl().isAdmin();
+ }
+
/** Can this user edit the description? */
private boolean canEditDescription() {
if (getChange().isNew()) {
@@ -299,6 +307,8 @@
return canRestore();
case SUBMIT:
return refControl.canSubmit(isOwner());
+ case TOGGLE_WORK_IN_PROGRESS_STATE:
+ return canToggleWorkInProgressState();
case REMOVE_REVIEWER:
case SUBMIT_AS:
diff --git a/java/com/google/gerrit/server/permissions/ChangePermission.java b/java/com/google/gerrit/server/permissions/ChangePermission.java
index ca1c460..2fba4ef 100644
--- a/java/com/google/gerrit/server/permissions/ChangePermission.java
+++ b/java/com/google/gerrit/server/permissions/ChangePermission.java
@@ -55,7 +55,8 @@
*/
REBASE,
SUBMIT,
- SUBMIT_AS("submit on behalf of other users");
+ SUBMIT_AS("submit on behalf of other users"),
+ TOGGLE_WORK_IN_PROGRESS_STATE;
private final String description;
diff --git a/java/com/google/gerrit/server/permissions/DefaultPermissionMappings.java b/java/com/google/gerrit/server/permissions/DefaultPermissionMappings.java
index ece29df..d56bc78 100644
--- a/java/com/google/gerrit/server/permissions/DefaultPermissionMappings.java
+++ b/java/com/google/gerrit/server/permissions/DefaultPermissionMappings.java
@@ -97,6 +97,9 @@
.put(ChangePermission.REBASE, Permission.REBASE)
.put(ChangePermission.SUBMIT, Permission.SUBMIT)
.put(ChangePermission.SUBMIT_AS, Permission.SUBMIT_AS)
+ .put(
+ ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE,
+ Permission.TOGGLE_WORK_IN_PROGRESS_STATE)
.build();
private static <T extends Enum<T>> void checkMapContainsAllEnumValues(
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 8afaa45..ff27bfa 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -340,8 +340,10 @@
return Response.withStatusCode(SC_BAD_REQUEST, output);
}
- WorkInProgressOp.checkPermissions(
- permissionBackend, revision.getUser(), revision.getChange());
+ revision
+ .getChangeResource()
+ .permissions()
+ .check(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE);
if (input.ready) {
output.ready = true;
diff --git a/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java b/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java
index 94914b0..aacf58b 100644
--- a/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java
+++ b/java/com/google/gerrit/server/restapi/change/SetReadyForReview.java
@@ -16,7 +16,6 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.or;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -25,48 +24,36 @@
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.WorkInProgressOp;
import com.google.gerrit.server.change.WorkInProgressOp.Input;
-import com.google.gerrit.server.permissions.GlobalPermission;
-import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
@Singleton
public class SetReadyForReview extends RetryingRestModifyView<ChangeResource, Input, Response<?>>
implements UiAction<ChangeResource> {
private final WorkInProgressOp.Factory opFactory;
- private final PermissionBackend permissionBackend;
- private final Provider<CurrentUser> user;
@Inject
- SetReadyForReview(
- RetryHelper retryHelper,
- WorkInProgressOp.Factory opFactory,
- PermissionBackend permissionBackend,
- Provider<CurrentUser> user) {
+ SetReadyForReview(RetryHelper retryHelper, WorkInProgressOp.Factory opFactory) {
super(retryHelper);
this.opFactory = opFactory;
- this.permissionBackend = permissionBackend;
- this.user = user;
}
@Override
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input)
throws RestApiException, UpdateException, PermissionBackendException {
- WorkInProgressOp.checkPermissions(permissionBackend, user.get(), rsrc.getChange());
+ rsrc.permissions().check(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE);
Change change = rsrc.getChange();
if (!change.isNew()) {
@@ -94,15 +81,6 @@
.setVisible(
and(
rsrc.getChange().isNew() && rsrc.getChange().isWorkInProgress(),
- or(
- rsrc.isUserOwner(),
- or(
- permissionBackend
- .currentUser()
- .testCond(GlobalPermission.ADMINISTRATE_SERVER),
- permissionBackend
- .currentUser()
- .project(rsrc.getProject())
- .testCond(ProjectPermission.WRITE_CONFIG)))));
+ rsrc.permissions().testCond(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE)));
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java b/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java
index d087e47..852813e 100644
--- a/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java
+++ b/java/com/google/gerrit/server/restapi/change/SetWorkInProgress.java
@@ -16,7 +16,6 @@
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.gerrit.extensions.conditions.BooleanCondition.and;
-import static com.google.gerrit.extensions.conditions.BooleanCondition.or;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -25,48 +24,36 @@
import com.google.gerrit.extensions.webui.UiAction;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.NotifyResolver;
import com.google.gerrit.server.change.WorkInProgressOp;
import com.google.gerrit.server.change.WorkInProgressOp.Input;
-import com.google.gerrit.server.permissions.GlobalPermission;
-import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.permissions.ProjectPermission;
import com.google.gerrit.server.update.BatchUpdate;
import com.google.gerrit.server.update.RetryHelper;
import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.inject.Inject;
-import com.google.inject.Provider;
import com.google.inject.Singleton;
@Singleton
public class SetWorkInProgress extends RetryingRestModifyView<ChangeResource, Input, Response<?>>
implements UiAction<ChangeResource> {
private final WorkInProgressOp.Factory opFactory;
- private final PermissionBackend permissionBackend;
- private final Provider<CurrentUser> user;
@Inject
- SetWorkInProgress(
- WorkInProgressOp.Factory opFactory,
- RetryHelper retryHelper,
- PermissionBackend permissionBackend,
- Provider<CurrentUser> user) {
+ SetWorkInProgress(WorkInProgressOp.Factory opFactory, RetryHelper retryHelper) {
super(retryHelper);
this.opFactory = opFactory;
- this.permissionBackend = permissionBackend;
- this.user = user;
}
@Override
protected Response<?> applyImpl(
BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input input)
throws RestApiException, UpdateException, PermissionBackendException {
- WorkInProgressOp.checkPermissions(permissionBackend, user.get(), rsrc.getChange());
+ rsrc.permissions().check(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE);
Change change = rsrc.getChange();
if (!change.isNew()) {
@@ -94,15 +81,6 @@
.setVisible(
and(
rsrc.getChange().isNew() && !rsrc.getChange().isWorkInProgress(),
- or(
- rsrc.isUserOwner(),
- or(
- permissionBackend
- .currentUser()
- .testCond(GlobalPermission.ADMINISTRATE_SERVER),
- permissionBackend
- .currentUser()
- .project(rsrc.getProject())
- .testCond(ProjectPermission.WRITE_CONFIG)))));
+ rsrc.permissions().testCond(ChangePermission.TOGGLE_WORK_IN_PROGRESS_STATE)));
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index d3e41f7..c38f686 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -277,7 +277,7 @@
requestScopeOperations.setApiUser(user.getId());
exception.expect(AuthException.class);
- exception.expectMessage("not allowed to toggle work in progress");
+ exception.expectMessage("toggle work in progress state not permitted");
gApi.changes().id(changeId).setWorkInProgress();
}
@@ -323,7 +323,7 @@
requestScopeOperations.setApiUser(user.getId());
exception.expect(AuthException.class);
- exception.expectMessage("not allowed to toggle work in progress");
+ exception.expectMessage("toggle work in progress state not permitted");
gApi.changes().id(changeId).setReadyForReview();
}
@@ -500,6 +500,37 @@
}
@Test
+ public void toggleWorkInProgressStateByNonOwnerWithPermission() throws Exception {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String refactor = "Needs some refactoring";
+ String ptal = "PTAL";
+
+ grant(
+ project,
+ "refs/heads/master",
+ Permission.TOGGLE_WORK_IN_PROGRESS_STATE,
+ false,
+ REGISTERED_USERS);
+
+ requestScopeOperations.setApiUser(user.getId());
+ gApi.changes().id(changeId).setWorkInProgress(refactor);
+
+ ChangeInfo info = gApi.changes().id(changeId).get();
+
+ assertThat(info.workInProgress).isTrue();
+ assertThat(Iterables.getLast(info.messages).message).contains(refactor);
+ assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_WIP);
+
+ gApi.changes().id(changeId).setReadyForReview(ptal);
+
+ info = gApi.changes().id(changeId).get();
+ assertThat(info.workInProgress).isNull();
+ assertThat(Iterables.getLast(info.messages).message).contains(ptal);
+ assertThat(Iterables.getLast(info.messages).tag).contains(ChangeMessagesUtil.TAG_SET_READY);
+ }
+
+ @Test
public void reviewAndStartReview() throws Exception {
PushOneCommit.Result r = createWorkInProgressChange();
r.assertOkStatus();
@@ -588,17 +619,33 @@
ReviewInput in = ReviewInput.noScore().setWorkInProgress(true);
requestScopeOperations.setApiUser(user.getId());
exception.expect(AuthException.class);
- exception.expectMessage("not allowed to toggle work in progress");
+ exception.expectMessage("toggle work in progress state not permitted");
gApi.changes().id(r.getChangeId()).current().review(in);
}
@Test
+ public void reviewWithWorkInProgressByNonOwnerWithPermission() throws Exception {
+ PushOneCommit.Result r = createChange();
+ ReviewInput in = ReviewInput.noScore().setWorkInProgress(true);
+ grant(
+ project,
+ "refs/heads/master",
+ Permission.TOGGLE_WORK_IN_PROGRESS_STATE,
+ false,
+ REGISTERED_USERS);
+ requestScopeOperations.setApiUser(user.getId());
+ gApi.changes().id(r.getChangeId()).current().review(in);
+ ChangeInfo info = gApi.changes().id(r.getChangeId()).get();
+ assertThat(info.workInProgress).isTrue();
+ }
+
+ @Test
public void reviewWithReadyByNonOwnerReturnsError() throws Exception {
PushOneCommit.Result r = createChange();
ReviewInput in = ReviewInput.noScore().setReady(true);
requestScopeOperations.setApiUser(user.getId());
exception.expect(AuthException.class);
- exception.expectMessage("not allowed to toggle work in progress");
+ exception.expectMessage("toggle work in progress state not permitted");
gApi.changes().id(r.getChangeId()).current().review(in);
}
diff --git a/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html b/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html
index fb0c685..e949a87 100644
--- a/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html
+++ b/polygerrit-ui/app/behaviors/gr-access-behavior/gr-access-behavior.html
@@ -120,6 +120,10 @@
id: 'submitAs',
name: 'Submit (On Behalf Of)',
},
+ toggleWipState: {
+ id: 'toggleWipState',
+ name: 'Toggle Work In Progress State',
+ },
viewDrafts: {
id: 'viewDrafts',
name: 'View Drafts',
diff --git a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
index 6699bd1..17f883f 100644
--- a/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
+++ b/polygerrit-ui/app/elements/core/gr-search-bar/gr-search-bar.js
@@ -65,6 +65,7 @@
'is:reviewed',
'is:reviewer',
'is:starred',
+ 'is:submittable',
'is:watched',
'is:wip',
'label:',
