Add group option that makes the group visible to all registered users

Normally a group is only visible to the group owners and the Gerrit
administrators. In certain environments (corporate or open source) it
can make sense to have groups that are visible to all registered
users. E.g.:
- a user needs access rights for a certain project, if this user can
  see the project owner group, he knows whom to contact to request
  the access rights
- a user needs support from a Gerrit administrator, if this user can
  see the administrator group, he knows whom to contact

This change adds a new group option that allows to make the group
visible to all registered users. Modifying the group is still only
allowed for the group owners. By default a newly created group will
only be visible to the group owners.

Change-Id: I2de0084a7842d73618ca48fa95804c22d5bb90cb
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2011-04-07 11:04:25 +02:00
parent 745ed373a3
commit b66c43fc0f
17 changed files with 204 additions and 31 deletions

View File

@@ -29,7 +29,7 @@ import java.util.Set;
@RpcImpl(version = Version.V2_0)
public interface GroupAdminService extends RemoteJsonService {
@SignInRequired
void ownedGroups(AsyncCallback<List<AccountGroup>> callback);
void visibleGroups(AsyncCallback<List<AccountGroup>> callback);
@SignInRequired
void createGroup(String newName, AsyncCallback<AccountGroup.Id> callback);
@@ -41,6 +41,10 @@ public interface GroupAdminService extends RemoteJsonService {
void changeGroupDescription(AccountGroup.Id groupId, String description,
AsyncCallback<VoidResult> callback);
@SignInRequired
void changeGroupOptions(AccountGroup.Id groupId, GroupOptions groupOptions,
AsyncCallback<VoidResult> callback);
@SignInRequired
void changeGroupOwner(AccountGroup.Id groupId, String newOwnerName,
AsyncCallback<VoidResult> callback);

View File

@@ -24,6 +24,7 @@ public class GroupDetail {
public AccountGroup group;
public List<AccountGroupMember> members;
public AccountGroup ownerGroup;
public boolean canModify;
public GroupDetail() {
}
@@ -43,4 +44,8 @@ public class GroupDetail {
public void setOwnerGroup(AccountGroup g) {
ownerGroup = g;
}
public void setCanModify(final boolean canModify) {
this.canModify = canModify;
}
}

View File

@@ -0,0 +1,36 @@
// Copyright (C) 2011 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.common.data;
import com.google.gerrit.reviewdb.AccountGroup;
/**
* Options for an {@link AccountGroup}.
*/
public class GroupOptions {
private boolean visibleToAll;
protected GroupOptions() {
}
public GroupOptions(final boolean visibleToAll) {
this.visibleToAll = visibleToAll;
}
public boolean isVisibleToAll() {
return visibleToAll;
}
}

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.client.ui.RPCSuggestOracle;
import com.google.gerrit.client.ui.SmallHeading;
import com.google.gerrit.common.data.AccountInfoCache;
import com.google.gerrit.common.data.GroupDetail;
import com.google.gerrit.common.data.GroupOptions;
import com.google.gerrit.reviewdb.Account;
import com.google.gerrit.reviewdb.AccountGroup;
import com.google.gerrit.reviewdb.AccountGroupMember;
@@ -84,6 +85,10 @@ public class AccountGroupScreen extends AccountScreen {
private Button externalNameSearch;
private Grid externalMatches;
private Panel groupOptionsPanel;
private CheckBox visibleToAllCheckBox;
private Button saveGroupOptions;
public AccountGroupScreen(final AccountGroup.Id toShow) {
groupId = toShow;
}
@@ -95,6 +100,14 @@ public class AccountGroupScreen extends AccountScreen {
this) {
@Override
protected void preDisplay(final GroupDetail result) {
enableForm(result.canModify);
saveName.setVisible(result.canModify);
saveOwner.setVisible(result.canModify);
saveDesc.setVisible(result.canModify);
saveGroupOptions.setVisible(result.canModify);
delMember.setVisible(result.canModify);
members.setEnabled(result.canModify);
saveType.setVisible(result.canModify);
display(result);
}
});
@@ -106,11 +119,23 @@ public class AccountGroupScreen extends AccountScreen {
initName();
initOwner();
initDescription();
initGroupOptions();
initGroupType();
initMemberList();
initExternal();
}
private void enableForm(final boolean canModify) {
groupNameTxt.setEnabled(canModify);
ownerTxtBox.setEnabled(canModify);
descTxt.setEnabled(canModify);
typeSelect.setEnabled(canModify);
addMemberBox.setEnabled(canModify);
externalNameFilter.setEnabled(canModify);
externalNameSearch.setEnabled(canModify);
visibleToAllCheckBox.setEnabled(canModify);
}
private void initName() {
final VerticalPanel groupNamePanel = new VerticalPanel();
groupNameTxt = new NpTextBox();
@@ -199,6 +224,35 @@ public class AccountGroupScreen extends AccountScreen {
new OnEditEnabler(saveDesc, descTxt);
}
private void initGroupOptions() {
groupOptionsPanel = new VerticalPanel();
groupOptionsPanel.add(new SmallHeading(Util.C.headingGroupOptions()));
visibleToAllCheckBox = new CheckBox(Util.C.isVisibleToAll());
groupOptionsPanel.add(visibleToAllCheckBox);
saveGroupOptions = new Button(Util.C.buttonSaveGroupOptions());
saveGroupOptions.setEnabled(false);
saveGroupOptions.addClickHandler(new ClickHandler() {
@Override
public void onClick(final ClickEvent event) {
final GroupOptions groupOptions =
new GroupOptions(visibleToAllCheckBox.getValue());
Util.GROUP_SVC.changeGroupOptions(groupId, groupOptions,
new GerritCallback<VoidResult>() {
public void onSuccess(final VoidResult result) {
saveGroupOptions.setEnabled(false);
}
});
}
});
groupOptionsPanel.add(saveGroupOptions);
add(groupOptionsPanel);
new OnEditEnabler(saveGroupOptions, visibleToAllCheckBox);
}
private void initGroupType() {
typeSystem = new Label(Util.C.groupType_SYSTEM());
@@ -445,6 +499,8 @@ public class AccountGroupScreen extends AccountScreen {
}
setType(group.getType());
visibleToAllCheckBox.setValue(group.isVisibleToAll());
}
void doAddNew() {
@@ -474,6 +530,8 @@ public class AccountGroupScreen extends AccountScreen {
}
private class MemberTable extends FancyFlexTable<AccountGroupMember> {
private boolean enabled = true;
MemberTable() {
table.setText(0, 2, Util.C.columnMember());
table.setText(0, 3, Util.C.columnEmailAddress());
@@ -484,6 +542,16 @@ public class AccountGroupScreen extends AccountScreen {
fmt.addStyleName(0, 3, Gerrit.RESOURCES.css().dataHeader());
}
void setEnabled(final boolean enabled) {
this.enabled = enabled;
for (int row = 1; row < table.getRowCount(); row++) {
final AccountGroupMember k = getRowItem(row);
if (k != null) {
((CheckBox) table.getWidget(row, 1)).setEnabled(enabled);
}
}
}
void deleteChecked() {
final HashSet<AccountGroupMember.Key> ids =
new HashSet<AccountGroupMember.Key>();
@@ -531,7 +599,9 @@ public class AccountGroupScreen extends AccountScreen {
void populate(final int row, final AccountGroupMember k) {
final Account.Id accountId = k.getAccountId();
table.setWidget(row, 1, new CheckBox());
CheckBox checkBox = new CheckBox();
table.setWidget(row, 1, checkBox);
checkBox.setEnabled(enabled);
table.setWidget(row, 2, AccountDashboardLink.link(accounts, accountId));
table.setText(row, 3, accounts.get(accountId).getPreferredEmail());

View File

@@ -37,6 +37,9 @@ public interface AdminConstants extends Constants {
String useContributorAgreements();
String useSignedOffBy();
String requireChangeID();
String headingGroupOptions();
String isVisibleToAll();
String buttonSaveGroupOptions();
String headingOwner();
String headingParentProjectName();

View File

@@ -18,6 +18,9 @@ useContentMerge = Automatically resolve conflicts
useContributorAgreements = Require a valid contributor agreement to upload
useSignedOffBy = Require <a href="http://gerrit.googlecode.com/svn/documentation/2.0/user-signedoffby.html#Signed-off-by" target="_blank"><code>Signed-off-by</code></a> in commit message
requireChangeID = Require <a href="http://gerrit.googlecode.com/svn/documentation/2.0/user-changeid.html" target="_blank"><code>Change-Id</code></a> in commit message
headingGroupOptions = Group Options
isVisibleToAll = Make group visible to all registered users.
buttonSaveGroupOptions = Save Group Options
headingOwner = Owners
headingParentProjectName = Rights Inherit From

View File

@@ -49,7 +49,7 @@ public class GroupListScreen extends AccountScreen {
protected void onLoad() {
super.onLoad();
Util.GROUP_SVC
.ownedGroups(new ScreenLoadCallback<List<AccountGroup>>(this) {
.visibleGroups(new ScreenLoadCallback<List<AccountGroup>>(this) {
@Override
protected void preDisplay(final List<AccountGroup> result) {
groups.display(result);

View File

@@ -46,6 +46,6 @@ class CreateGroup extends Handler<AccountGroup.Id> {
public AccountGroup.Id call() throws OrmException, NameAlreadyUsedException {
final PerformCreateGroup performCreateGroup = performCreateGroupFactory.create();
final Account.Id me = user.getAccountId();
return performCreateGroup.createGroup(groupName, null, null, me);
return performCreateGroup.createGroup(groupName, null, false, null, me);
}
}

View File

@@ -16,6 +16,7 @@ package com.google.gerrit.httpd.rpc.account;
import com.google.gerrit.common.data.GroupAdminService;
import com.google.gerrit.common.data.GroupDetail;
import com.google.gerrit.common.data.GroupOptions;
import com.google.gerrit.common.errors.InactiveAccountException;
import com.google.gerrit.common.errors.NameAlreadyUsedException;
import com.google.gerrit.common.errors.NoSuchAccountException;
@@ -80,7 +81,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
this.groupDetailFactory = groupDetailFactory;
}
public void ownedGroups(final AsyncCallback<List<AccountGroup>> callback) {
public void visibleGroups(final AsyncCallback<List<AccountGroup>> callback) {
run(callback, new Action<List<AccountGroup>>() {
public List<AccountGroup> run(ReviewDb db) throws OrmException {
final IdentifiedUser user = identifiedUser.get();
@@ -88,22 +89,11 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
if (user.isAdministrator()) {
result = db.accountGroups().all().toList();
} else {
final HashSet<AccountGroup.Id> seen = new HashSet<AccountGroup.Id>();
result = new ArrayList<AccountGroup>();
for (final AccountGroup.Id myGroup : user.getEffectiveGroups()) {
for (AccountGroup group : db.accountGroups().ownedByGroup(myGroup)) {
final AccountGroup.Id id = group.getId();
if (!seen.add(id)) {
continue;
}
try {
GroupControl c = groupControlFactory.controlFor(id);
if (c.isOwner()) {
result.add(c.getAccountGroup());
}
} catch (NoSuchGroupException e) {
continue;
}
for(final AccountGroup group : db.accountGroups().all().toList()) {
final GroupControl c = groupControlFactory.controlFor(group);
if (c.isVisible()) {
result.add(c.getAccountGroup());
}
}
}
@@ -141,6 +131,20 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
});
}
public void changeGroupOptions(final AccountGroup.Id groupId,
final GroupOptions groupOptions, final AsyncCallback<VoidResult> callback) {
run(callback, new Action<VoidResult>() {
public VoidResult run(final ReviewDb db) throws OrmException, Failure {
final AccountGroup group = db.accountGroups().get(groupId);
assertAmGroupOwner(db, group);
group.setVisibleToAll(groupOptions.isVisibleToAll());
db.accountGroups().update(Collections.singleton(group));
groupCache.evict(group);
return VoidResult.INSTANCE;
}
});
}
public void changeGroupOwner(final AccountGroup.Id groupId,
final String newOwnerName, final AsyncCallback<VoidResult> callback) {
run(callback, new Action<VoidResult>() {

View File

@@ -72,6 +72,7 @@ class GroupDetailFactory extends Handler<GroupDetail> {
break;
}
detail.setAccounts(aic.create());
detail.setCanModify(control.isOwner());
return detail;
}

View File

@@ -168,6 +168,9 @@ public final class AccountGroup {
@Column(id = 6, notNull = false)
protected ExternalNameKey externalName;
@Column(id = 7)
protected boolean visibleToAll;
protected AccountGroup() {
}
@@ -176,6 +179,7 @@ public final class AccountGroup {
name = newName;
groupId = newId;
ownerGroupId = groupId;
visibleToAll = false;
setType(Type.INTERNAL);
}
@@ -226,4 +230,12 @@ public final class AccountGroup {
public void setExternalNameKey(final ExternalNameKey k) {
externalName = k;
}
public void setVisibleToAll(final boolean visibleToAll) {
this.visibleToAll = visibleToAll;
}
public boolean isVisibleToAll() {
return visibleToAll;
}
}

View File

@@ -31,8 +31,4 @@ public interface AccountGroupAccess extends
@Query
ResultSet<AccountGroup> all() throws OrmException;
@Query("WHERE ownerGroupId = ?")
ResultSet<AccountGroup> ownedByGroup(AccountGroup.Id groupId)
throws OrmException;
}

View File

@@ -42,6 +42,10 @@ public class GroupControl {
return new GroupControl(user.get(), group);
}
public GroupControl controlFor(final AccountGroup group) {
return new GroupControl(user.get(), group);
}
public GroupControl validateFor(final AccountGroup.Id groupId)
throws NoSuchGroupException {
final GroupControl c = controlFor(groupId);
@@ -70,7 +74,7 @@ public class GroupControl {
/** Can this user see this group exists? */
public boolean isVisible() {
return isOwner();
return group.isVisibleToAll() || isOwner();
}
public boolean isOwner() {
@@ -88,6 +92,6 @@ public class GroupControl {
}
public boolean canSee(Account.Id id) {
return isOwner();
return isVisible();
}
}

View File

@@ -54,6 +54,9 @@ public class PerformCreateGroup {
* @param groupName the name for the new group
* @param groupDescription the description of the new group, <code>null</code>
* if no description
* @param visibleToAll <code>true</code> to make the group visible to all
* registered users, if <code>false</code> the group is only visible to
* the group owners and Gerrit administrators
* @param ownerGroupId the group that should own the new group, if
* <code>null</code> the new group will own itself
* @param initialMembers initial members to be added to the new group
@@ -64,13 +67,14 @@ public class PerformCreateGroup {
* name already exists
*/
public AccountGroup.Id createGroup(final String groupName,
final String groupDescription, final AccountGroup.Id ownerGroupId,
final Account.Id... initialMembers) throws OrmException,
NameAlreadyUsedException {
final String groupDescription, final boolean visibleToAll,
final AccountGroup.Id ownerGroupId, final Account.Id... initialMembers)
throws OrmException, NameAlreadyUsedException {
final AccountGroup.Id groupId =
new AccountGroup.Id(db.nextAccountGroupId());
final AccountGroup.NameKey nameKey = new AccountGroup.NameKey(groupName);
final AccountGroup group = new AccountGroup(nameKey, groupId);
group.setVisibleToAll(visibleToAll);
if (ownerGroupId != null) {
group.setOwnerGroupId(ownerGroupId);
}

View File

@@ -32,7 +32,7 @@ import java.util.List;
/** A version of the database schema. */
public abstract class SchemaVersion {
/** The current schema version. */
private static final Class<? extends SchemaVersion> C = Schema_48.class;
private static final Class<? extends SchemaVersion> C = Schema_49.class;
public static class Module extends AbstractModule {
@Override

View File

@@ -0,0 +1,26 @@
// Copyright (C) 2011 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.schema;
import com.google.inject.Inject;
import com.google.inject.Provider;
public class Schema_49 extends SchemaVersion {
@Inject
Schema_49(Provider<Schema_48> prior) {
super(prior);
}
}

View File

@@ -55,6 +55,9 @@ public class AdminCreateGroup extends BaseCommand {
initialMembers.add(id);
}
@Option(name = "--visible-to-all", usage = "to make the group visible to all registered users")
private boolean visibleToAll;
@Inject
private PerformCreateGroup.Factory performCreateGroupFactory;
@@ -73,7 +76,9 @@ public class AdminCreateGroup extends BaseCommand {
final PerformCreateGroup performCreateGroup =
performCreateGroupFactory.create();
try {
performCreateGroup.createGroup(groupName, groupDescription, ownerGroupId, initialMembers.toArray(new Account.Id[initialMembers.size()]));
performCreateGroup.createGroup(groupName, groupDescription, visibleToAll,
ownerGroupId,
initialMembers.toArray(new Account.Id[initialMembers.size()]));
} catch (NameAlreadyUsedException e) {
throw die(e);
}