diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties index bbcc5c32a1..ffa749df84 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties @@ -1,3 +1,5 @@ +# Changes to this file should also be made in +# gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties accountDashboardTitle = Code Review Dashboard for {0} changesOpenInProject = Open Changes In {0} changesMergedInProject = Merged Changes In {0} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/AccountInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/AccountInfo.java index 56362ce120..437947c27d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/AccountInfo.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/AccountInfo.java @@ -28,6 +28,7 @@ import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; +import java.util.Collection; import java.util.List; import java.util.Map; @@ -89,6 +90,14 @@ public class AccountInfo { } } + public void fill(Collection infos) + throws OrmException { + for (AccountInfo info : infos) { + put(info); + } + fill(); + } + private void fill(AccountInfo info, Account account) { info.name = account.getFullName(); if (detailed) { @@ -98,7 +107,7 @@ public class AccountInfo { } } - private transient Account.Id _id; + transient Account.Id _id; protected AccountInfo(Account.Id id) { _id = id; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeMessages.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeMessages.java new file mode 100644 index 0000000000..5cb64acb48 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeMessages.java @@ -0,0 +1,28 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.change; + +import org.eclipse.jgit.nls.NLS; +import org.eclipse.jgit.nls.TranslationBundle; + +public class ChangeMessages extends TranslationBundle { + public static ChangeMessages get() { + return NLS.getBundleFor(ChangeMessages.class); + } + + public String groupIsNotAllowed; + public String groupHasTooManyMembers; + public String groupManyMembersConfirmation; +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java index 48c054adb6..68cc1b44ba 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListReviewers.java @@ -21,7 +21,6 @@ import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.account.AccountCache; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -29,17 +28,14 @@ import com.google.inject.Provider; import java.util.Map; class ListReviewers implements RestReadView { - private final AccountCache accountCache; private final Provider dbProvider; private final ReviewerJson json; private final ReviewerResource.Factory resourceFactory; @Inject - ListReviewers(AccountCache accountCache, - Provider dbProvider, + ListReviewers(Provider dbProvider, ReviewerResource.Factory resourceFactory, ReviewerJson json) { - this.accountCache = accountCache; this.dbProvider = dbProvider; this.resourceFactory = resourceFactory; this.json = json; @@ -55,8 +51,7 @@ class ListReviewers implements RestReadView { : db.patchSetApprovals().byChange(changeId)) { Account.Id accountId = patchSetApproval.getAccountId(); if (!reviewers.containsKey(accountId)) { - Account account = accountCache.get(accountId).getAccount(); - reviewers.put(accountId, resourceFactory.create(rsrc, account)); + reviewers.put(accountId, resourceFactory.create(rsrc, accountId)); } } return json.format(reviewers.values()); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java index b00b3d278e..94331aa583 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java @@ -46,10 +46,11 @@ public class Module extends RestApiModule { delete(CHANGE_KIND, "topic").to(PutTopic.class); post(CHANGE_KIND, "abandon").to(Abandon.class); post(CHANGE_KIND, "restore").to(Restore.class); - child(CHANGE_KIND, "reviewers").to(Reviewers.class); post(CHANGE_KIND, "revert").to(Revert.class); post(CHANGE_KIND, "submit").to(Submit.CurrentRevision.class); + post(CHANGE_KIND, "reviewers").to(PostReviewers.class); + child(CHANGE_KIND, "reviewers").to(Reviewers.class); get(REVIEWER_KIND).to(GetReviewer.class); delete(REVIEWER_KIND).to(DeleteReviewer.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java new file mode 100644 index 0000000000..f83ab64bc2 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java @@ -0,0 +1,288 @@ +// Copyright (C) 2013 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.server.change; + +import com.google.common.base.Objects; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; +import com.google.gerrit.common.ChangeHooks; +import com.google.gerrit.common.data.ApprovalTypes; +import com.google.gerrit.common.errors.EmailException; +import com.google.gerrit.common.errors.NoSuchGroupException; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.DefaultInput; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; +import com.google.gerrit.extensions.restapi.RestModifyView; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.reviewdb.client.AccountGroup; +import com.google.gerrit.reviewdb.client.ApprovalCategory; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.PatchSetApproval; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.account.AccountCache; +import com.google.gerrit.server.account.GroupCache; +import com.google.gerrit.server.account.GroupMembers; +import com.google.gerrit.server.change.PostReviewers.Input; +import com.google.gerrit.server.change.ReviewerJson.PostResult; +import com.google.gerrit.server.change.ReviewerJson.ReviewerInfo; +import com.google.gerrit.server.config.GerritServerConfig; +import com.google.gerrit.server.mail.AddReviewerSender; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.NoSuchProjectException; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import org.eclipse.jgit.lib.Config; + +import java.text.MessageFormat; +import java.util.List; +import java.util.Set; + +class PostReviewers implements RestModifyView { + public final static int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10; + public final static int DEFAULT_MAX_REVIEWERS = 20; + + static class Input { + @DefaultInput + String reviewer; + Boolean confirmed; + + boolean confirmed() { + return Objects.firstNonNull(confirmed, false); + } + } + + private final Reviewers.Parser parser; + private final ReviewerResource.Factory reviewerFactory; + private final AddReviewerSender.Factory addReviewerSenderFactory; + private final GroupCache groupCache; + private final GroupMembers.Factory groupMembersFactory; + private final AccountInfo.Loader.Factory accountLoaderFactory; + private final Provider db; + private final IdentifiedUser currentUser; + private final IdentifiedUser.GenericFactory identifiedUserFactory; + private final ApprovalCategory.Id addReviewerCategoryId; + private final Config cfg; + private final ChangeHooks hooks; + private final AccountCache accountCache; + private final ReviewerJson json; + + @Inject + PostReviewers(Reviewers.Parser parser, + ReviewerResource.Factory reviewerFactory, + AddReviewerSender.Factory addReviewerSenderFactory, + GroupCache groupCache, + GroupMembers.Factory groupMembersFactory, + AccountInfo.Loader.Factory accountLoaderFactory, + Provider db, + IdentifiedUser currentUser, + IdentifiedUser.GenericFactory identifiedUserFactory, + ApprovalTypes approvalTypes, + @GerritServerConfig Config cfg, + ChangeHooks hooks, + AccountCache accountCache, + ReviewerJson json) { + this.parser = parser; + this.reviewerFactory = reviewerFactory; + this.addReviewerSenderFactory = addReviewerSenderFactory; + this.groupCache = groupCache; + this.groupMembersFactory = groupMembersFactory; + this.accountLoaderFactory = accountLoaderFactory; + this.db = db; + this.currentUser = currentUser; + this.identifiedUserFactory = identifiedUserFactory; + this.cfg = cfg; + this.hooks = hooks; + this.accountCache = accountCache; + this.json = json; + + this.addReviewerCategoryId = Iterables.getLast( + approvalTypes.getApprovalTypes()).getCategory().getId(); + } + + @Override + public PostResult apply(ChangeResource rsrc, Input input) + throws ResourceNotFoundException, AuthException, OrmException, + EmailException { + Account.Id accountId = parser.parse(rsrc, input.reviewer); + try { + if (accountId != null) { + return putAccount(reviewerFactory.create(rsrc, accountId)); + } else { + return putGroup(rsrc, input); + } + } catch (NoSuchChangeException e) { + throw new ResourceNotFoundException(e.getMessage()); + } catch (NoSuchGroupException e) { + throw new ResourceNotFoundException(e.getMessage()); + } catch (NoSuchProjectException e) { + throw new ResourceNotFoundException(e.getMessage()); + } + } + + private PostResult putAccount(ReviewerResource rsrc) throws OrmException, + EmailException, NoSuchChangeException { + PostResult result = new PostResult(); + addReviewers(rsrc, result, ImmutableSet.of(rsrc.getUser())); + return result; + } + + private PostResult putGroup(ChangeResource rsrc, Input input) + throws ResourceNotFoundException, AuthException, NoSuchGroupException, + NoSuchProjectException, OrmException, NoSuchChangeException, + EmailException { + AccountGroup group = + groupCache.get(new AccountGroup.NameKey(input.reviewer)); + if (group == null) { + throw new ResourceNotFoundException(input.reviewer); + } + PostResult result = new PostResult(); + if (!isLegalReviewerGroup(group.getGroupUUID())) { + result.error = MessageFormat.format( + ChangeMessages.get().groupIsNotAllowed, group.getName()); + return result; + } + + Set reviewers = Sets.newLinkedHashSet(); + ChangeControl control = rsrc.getControl(); + Set members = groupMembersFactory.create(control.getCurrentUser()) + .listAccounts(group.getGroupUUID(), control.getProject().getNameKey()); + + // if maxAllowed is set to 0, it is allowed to add any number of + // reviewers + int maxAllowed = + cfg.getInt("addreviewer", "maxAllowed", DEFAULT_MAX_REVIEWERS); + if (maxAllowed > 0 && members.size() > maxAllowed) { + result.error = MessageFormat.format( + ChangeMessages.get().groupHasTooManyMembers, group.getName()); + return result; + } + + // if maxWithoutCheck is set to 0, we never ask for confirmation + int maxWithoutConfirmation = + cfg.getInt("addreviewer", "maxWithoutConfirmation", + DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK); + if (!input.confirmed() && maxWithoutConfirmation > 0 + && members.size() > maxWithoutConfirmation) { + result.confirm = true; + result.error = MessageFormat.format( + ChangeMessages.get().groupManyMembersConfirmation, + group.getName(), members.size()); + return result; + } + + for (Account member : members) { + if (member.isActive()) { + IdentifiedUser user = identifiedUserFactory.create(member.getId()); + // Does not account for draft status as a user might want to let a + // reviewer see a draft. + if (control.forUser(user).isRefVisible()) { + reviewers.add(user); + } + } + } + + addReviewers(rsrc, result, reviewers); + return result; + } + + private void addReviewers(ChangeResource rsrc, PostResult result, + Set reviewers) throws OrmException, EmailException, + NoSuchChangeException { + if (reviewers.isEmpty()) { + result.reviewers = ImmutableList.of(); + return; + } + + PatchSet.Id psid = rsrc.getChange().currentPatchSetId(); + Set existing = Sets.newHashSet(); + for (PatchSetApproval psa : db.get().patchSetApprovals().byPatchSet(psid)) { + existing.add(psa.getAccountId()); + } + + result.reviewers = Lists.newArrayListWithCapacity(reviewers.size()); + List toInsert = + Lists.newArrayListWithCapacity(reviewers.size()); + for (IdentifiedUser user : reviewers) { + Account.Id id = user.getAccountId(); + if (existing.contains(id)) { + continue; + } + ChangeControl control = rsrc.getControl().forUser(user); + PatchSetApproval psa = dummyApproval(control.getChange(), psid, id); + result.reviewers.add(json.format( + new ReviewerInfo(id), control, ImmutableList.of(psa))); + toInsert.add(psa); + } + db.get().patchSetApprovals().insert(toInsert); + accountLoaderFactory.create(true).fill(result.reviewers); + postAdd(rsrc.getChange(), result); + } + + private void postAdd(Change change, PostResult result) + throws OrmException, EmailException { + if (result.reviewers.isEmpty()) { + return; + } + + // Execute hook for added reviewers + // + PatchSet patchSet = db.get().patchSets().get(change.currentPatchSetId()); + for (AccountInfo info : result.reviewers) { + Account account = accountCache.get(info._id).getAccount(); + hooks.doReviewerAddedHook(change, account, patchSet, db.get()); + } + + // Email the reviewers + // + // The user knows they added themselves, don't bother emailing them. + List added = + Lists.newArrayListWithCapacity(result.reviewers.size()); + for (AccountInfo info : result.reviewers) { + if (!info._id.equals(currentUser.getAccountId())) { + added.add(info._id); + } + } + if (!added.isEmpty()) { + AddReviewerSender cm; + + cm = addReviewerSenderFactory.create(change); + cm.setFrom(currentUser.getAccountId()); + cm.addReviewers(added); + cm.send(); + } + } + + public static boolean isLegalReviewerGroup(AccountGroup.UUID groupUUID) { + return !(AccountGroup.ANONYMOUS_USERS.equals(groupUUID) + || AccountGroup.REGISTERED_USERS.equals(groupUUID)); + } + + private PatchSetApproval dummyApproval(Change change, PatchSet.Id patchSetId, + Account.Id reviewerId) { + PatchSetApproval dummyApproval = + new PatchSetApproval(new PatchSetApproval.Key(patchSetId, reviewerId, + addReviewerCategoryId), (short) 0); + dummyApproval.cache(change); + return dummyApproval; + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java index 9479e67a99..e5fd78648f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java @@ -22,7 +22,6 @@ import com.google.gerrit.common.data.ApprovalTypes; import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.ApprovalCategoryValue; -import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -58,7 +57,7 @@ public class ReviewerJson { List infos = Lists.newArrayListWithCapacity(rsrcs.size()); AccountInfo.Loader loader = accountLoaderFactory.create(true); for (ReviewerResource rsrc : rsrcs) { - ReviewerInfo info = formatOne(rsrc); + ReviewerInfo info = format(rsrc, null); loader.put(info); infos.add(info); } @@ -70,17 +69,15 @@ public class ReviewerJson { return format(ImmutableList. of(rsrc)); } - private ReviewerInfo formatOne(ReviewerResource rsrc) throws OrmException { - Account.Id id = rsrc.getUser().getAccountId(); - ReviewerInfo out = new ReviewerInfo(id); + public ReviewerInfo format(ReviewerInfo out, ChangeControl control, + List approvals) throws OrmException { + PatchSet.Id psId = control.getChange().currentPatchSetId(); - Change change = rsrc.getChange(); - PatchSet.Id psId = change.currentPatchSetId(); + if (approvals == null) { + approvals = db.get().patchSetApprovals() + .byPatchSetUser(psId, out._id).toList(); + } - List approvals = db.get().patchSetApprovals() - .byPatchSetUser(psId, id).toList(); - - ChangeControl control = rsrc.getControl().forUser(rsrc.getUser()); FunctionState fs = functionState.create(control, psId, approvals); for (ApprovalType at : approvalTypes.getApprovalTypes()) { CategoryFunction.forCategory(at.getCategory()).run(at, fs); @@ -106,6 +103,12 @@ public class ReviewerJson { return out; } + private ReviewerInfo format(ReviewerResource rsrc, + List approvals) throws OrmException { + return format(new ReviewerInfo(rsrc.getUser().getAccountId()), + rsrc.getUserControl(), approvals); + } + public static class ReviewerInfo extends AccountInfo { final String kind = "gerritcodereview#reviewer"; Map approvals; @@ -114,4 +117,10 @@ public class ReviewerJson { super(id); } } + + public static class PostResult { + List reviewers; + Boolean confirm; + String error; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerResource.java index 23fba4720f..98a4277327 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerResource.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerResource.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.change; import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.project.ChangeControl; import com.google.inject.TypeLiteral; import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.AssistedInject; @@ -27,7 +28,7 @@ public class ReviewerResource extends ChangeResource { static interface Factory { ReviewerResource create(ChangeResource rsrc, IdentifiedUser user); - ReviewerResource create(ChangeResource rsrc, Account account); + ReviewerResource create(ChangeResource rsrc, Account.Id id); } private final IdentifiedUser user; @@ -42,11 +43,19 @@ public class ReviewerResource extends ChangeResource { @AssistedInject ReviewerResource(IdentifiedUser.GenericFactory userFactory, @Assisted ChangeResource rsrc, - @Assisted Account account) { - this(rsrc, userFactory.create(account.getId())); + @Assisted Account.Id id) { + this(rsrc, userFactory.create(id)); } public IdentifiedUser getUser() { return user; } + + /** + * @return the control for the reviewer's user (as opposed to the caller's + * user as returned by {@link #getControl()}). + */ + public ChangeControl getUserControl() { + return getControl().forUser(user); + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java index e75f54cd35..2129d15aa7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Reviewers.java @@ -28,7 +28,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.account.AccountCache; import com.google.gerrit.server.account.AccountResolver; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -40,23 +39,50 @@ public class Reviewers implements ChildCollection { private final DynamicMap> views; private final Provider dbProvider; - private final AccountResolver resolver; + private final Parser parser; private final ReviewerResource.Factory resourceFactory; - private final AccountCache accountCache; private final Provider list; + static class Parser { + private final AccountResolver resolver; + + @Inject + Parser(AccountResolver resolver) { + this.resolver = resolver; + } + + Account.Id parse(ChangeResource rsrc, String id) + throws OrmException, AuthException { + if (id.equals("self")) { + CurrentUser user = rsrc.getControl().getCurrentUser(); + if (user instanceof IdentifiedUser) { + return ((IdentifiedUser) user).getAccountId(); + } else if (user instanceof AnonymousUser) { + throw new AuthException("Authentication required"); + } else { + return null; + } + } else { + Set matches = resolver.findAll(id); + if (matches.size() != 1) { + return null; + } + return Iterables.getOnlyElement(matches); + } + } + } + + @Inject Reviewers(Provider dbProvider, - AccountResolver resolver, + Parser parser, ReviewerResource.Factory resourceFactory, DynamicMap> views, - AccountCache accountCache, Provider list) { this.dbProvider = dbProvider; - this.resolver = resolver; + this.parser = parser; this.resourceFactory = resourceFactory; this.views = views; - this.accountCache = accountCache; this.list = list; } @@ -73,28 +99,10 @@ public class Reviewers implements @Override public ReviewerResource parse(ChangeResource rsrc, IdString id) throws OrmException, ResourceNotFoundException, AuthException { - Account.Id accountId; - if (id.equals("self")) { - CurrentUser user = rsrc.getControl().getCurrentUser(); - if (user instanceof IdentifiedUser) { - accountId = ((IdentifiedUser) user).getAccountId(); - } else if (user instanceof AnonymousUser) { - throw new AuthException("Authentication required"); - } else { - throw new ResourceNotFoundException(id); - } - } else { - Set matches = resolver.findAll(id.get()); - if (matches.size() != 1) { - throw new ResourceNotFoundException(id); - } - accountId = Iterables.getOnlyElement(matches); - } - + Account.Id accountId = parser.parse(rsrc, id.get()); // See if the id exists as a reviewer for this change - if (fetchAccountIds(rsrc).contains(accountId)) { - Account account = accountCache.get(accountId).getAccount(); - return resourceFactory.create(rsrc, account); + if (accountId != null && fetchAccountIds(rsrc).contains(accountId)) { + return resourceFactory.create(rsrc, accountId); } throw new ResourceNotFoundException(id); } diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties b/gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties new file mode 100644 index 0000000000..816c9536a9 --- /dev/null +++ b/gerrit-server/src/main/resources/com/google/gerrit/server/change/ChangeMessages.properties @@ -0,0 +1,5 @@ +# Changes to this file should also be made in +# gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeMessages.properties +groupIsNotAllowed = The group {0} cannot be added as reviewer. +groupHasTooManyMembers = The group {0} has too many members to add them all as reviewers. +groupManyMembersConfirmation = The group {0} has {1} members. Do you want to add them all as reviewers?