opendev/gerrit
Code Issues Proposed changes

Merge "Throw an error when adding a robot to the attention set explictly"

Browse Source
This commit is contained in:
Gal Paikin
2020-07-30 07:59:57 +00:00
committed by Gerrit Code Review
parent d8cfdd0923 2eff775380
commit c574c09a9b
2 changed files with 22 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
java/com/google/gerrit/server/restapi/change/AddToAttentionSet.java
View File

@@ -19,10 +19,12 @@ import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestCollectionModifyView;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.account.RobotClassifier;
import com.google.gerrit.server.change.AddToAttentionSetOp;
import com.google.gerrit.server.change.AttentionSetEntryResource;
import com.google.gerrit.server.change.ChangeResource;
@@ -46,6 +48,7 @@ public class AddToAttentionSet
private final AccountLoader.Factory accountLoaderFactory;
private final PermissionBackend permissionBackend;
private final NotifyResolver notifyResolver;
private final RobotClassifier robotClassifier;
@Inject
AddToAttentionSet(
@@ -54,13 +57,15 @@ public class AddToAttentionSet
AddToAttentionSetOp.Factory opFactory,
AccountLoader.Factory accountLoaderFactory,
PermissionBackend permissionBackend,
NotifyResolver notifyResolver) {
NotifyResolver notifyResolver,
RobotClassifier robotClassifier) {
this.updateFactory = updateFactory;
this.accountResolver = accountResolver;
this.opFactory = opFactory;
this.accountLoaderFactory = accountLoaderFactory;
this.permissionBackend = permissionBackend;
this.notifyResolver = notifyResolver;
this.robotClassifier = robotClassifier;
}
@Override
@@ -69,6 +74,11 @@ public class AddToAttentionSet
AttentionSetUtil.validateInput(input);
Account.Id attentionUserId = accountResolver.resolve(input.user).asUnique().account().id();
if (robotClassifier.isRobot(attentionUserId)) {
throw new BadRequestException(
String.format(
"%s is a robot, and robots can't be added to the attention set.", input.user));
}
try {
permissionBackend
.absentUser(attentionUserId)

15
javatests/com/google/gerrit/acceptance/rest/change/AttentionSetIT.java
View File

@@ -963,11 +963,18 @@ public class AttentionSetIT extends AbstractDaemonTest {
accountCreator.create(
"robot1", "robot1@example.com", "Ro Bot", "Ro", "Non-Interactive Users");
PushOneCommit.Result r = createChange();
// TODO(paiking): Adding robots explicitly should throw an exception and only implicit additions
// should fail silently.
change(r).addToAttentionSet(new AttentionSetInput(robot.email(), "reason"));
change(r).addReviewer(robot.email());
// Throw an error when adding a robot explicitly.
BadRequestException exception =
assertThrows(
BadRequestException.class,
() -> change(r).addToAttentionSet(new AttentionSetInput(robot.email(), "reason")));
assertThat(exception.getMessage())
.isEqualTo(
"robot1@example.com is a robot, and robots can't be added to the attention set.");
// Robots are not added implicitly.
change(r).addReviewer(robot.email());
assertThat(r.getChange().attentionSet()).isEmpty();
}