opendev/gerrit
Code Issues Proposed changes

Change group ownership to be another group

Browse Source 
Instead of marking individual group members as owning that group
we designate another group whose members can manage this group.
The reference can be a self-reference, permitting a group to
control its own membership, or it can be to another group, so
the management is delegated away from the members.

With this pattern it may be common to see "foo" and "foo-owners"
or "foo-managers" be created, with the latter group having the
ownership status over the former group.

When a group is initially created it manages itself, so the user
who created the group can be made a member of it and may control
access to the group, even if they are not a site admin.

Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2008-12-30 10:03:24 -08:00
parent 0aca7299e4
commit bd523456fc
14 changed files with 232 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
appjar/src/main/java/com/google/gerrit/client/admin/AccountGroupDetail.java
View File

@@ -32,6 +32,7 @@ public class AccountGroupDetail {
protected AccountInfoCache accounts;
protected AccountGroup group;
protected List<AccountGroupMember> members;
protected AccountGroup ownerGroup;
public AccountGroupDetail() {
}
@@ -39,6 +40,12 @@ public class AccountGroupDetail {
public void load(final ReviewDb db, final AccountInfoCacheFactory acc,
final AccountGroup g) throws OrmException {
group = g;
if (group.getId().equals(group.getOwnerGroupId())) {
ownerGroup = group;
} else {
ownerGroup = db.accountGroups().get(group.getOwnerGroupId());
}
members = db.accountGroupMembers().byGroup(group.getId()).toList();
for (final AccountGroupMember m : members) {
acc.want(m.getAccountId());

76
appjar/src/main/java/com/google/gerrit/client/admin/AccountGroupScreen.java
View File

@@ -20,6 +20,7 @@ import com.google.gerrit.client.reviewdb.AccountGroup;
import com.google.gerrit.client.reviewdb.AccountGroupMember;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.ui.AccountDashboardLink;
import com.google.gerrit.client.ui.AccountGroupSuggestOracle;
import com.google.gerrit.client.ui.AccountScreen;
import com.google.gerrit.client.ui.AccountSuggestOracle;
import com.google.gerrit.client.ui.FancyFlexTable;
@@ -49,10 +50,15 @@ public class AccountGroupScreen extends AccountScreen {
private AccountInfoCache accounts = AccountInfoCache.empty();
private MemberTable members;
private Panel groupNamePanel;
private Panel groupNamePanel;
private TextBox groupNameTxt;
private Button saveName;
private Panel ownerPanel;
private TextBox ownerTxtBox;
private SuggestBox ownerTxt;
private Button saveOwner;
private TextArea descTxt;
private Button saveDesc;
@@ -73,6 +79,7 @@ public class AccountGroupScreen extends AccountScreen {
enableForm(false);
saveName.setEnabled(false);
saveOwner.setEnabled(false);
saveDesc.setEnabled(false);
super.onLoad();
@@ -81,6 +88,7 @@ public class AccountGroupScreen extends AccountScreen {
public void onSuccess(final AccountGroupDetail result) {
enableForm(true);
saveName.setEnabled(false);
saveOwner.setEnabled(false);
saveDesc.setEnabled(false);
display(result);
}
@@ -88,6 +96,7 @@ public class AccountGroupScreen extends AccountScreen {
}
private void enableForm(final boolean on) {
ownerTxtBox.setEnabled(on);
groupNameTxt.setEnabled(on);
descTxt.setEnabled(on);
addMember.setEnabled(on);
@@ -97,6 +106,7 @@ public class AccountGroupScreen extends AccountScreen {
private void initUI() {
initName();
initOwner();
initDescription();
initMemberList();
}
@@ -126,6 +136,37 @@ public class AccountGroupScreen extends AccountScreen {
new TextSaveButtonListener(groupNameTxt, saveName);
}
private void initOwner() {
ownerPanel = new VerticalPanel();
final Label ownerHdr = new Label(Util.C.headingOwner());
ownerHdr.setStyleName("gerrit-SmallHeading");
ownerPanel.add(ownerHdr);
ownerTxtBox = new TextBox();
ownerTxtBox.setVisibleLength(60);
ownerTxt = new SuggestBox(new AccountGroupSuggestOracle(), ownerTxtBox);
ownerPanel.add(ownerTxt);
saveOwner = new Button(Util.C.buttonChangeGroupOwner());
saveOwner.addClickListener(new ClickListener() {
public void onClick(Widget sender) {
final String newOwner = ownerTxt.getText().trim();
if (newOwner.length() > 0) {
Util.GROUP_SVC.changeGroupOwner(groupId, newOwner,
new GerritCallback<VoidResult>() {
public void onSuccess(final VoidResult result) {
saveOwner.setEnabled(false);
}
});
}
}
});
ownerPanel.add(saveOwner);
add(ownerPanel);
new TextSaveButtonListener(ownerTxtBox, saveOwner);
}
private void initDescription() {
final VerticalPanel vp = new VerticalPanel();
final Label descHdr = new Label(Util.C.headingDescription());
@@ -211,14 +252,19 @@ public class AccountGroupScreen extends AccountScreen {
private void display(final AccountGroupDetail result) {
if (GroupAdminService.ADMIN_GROUP.equals(result.group.getNameKey())) {
ownerTxtBox.setEnabled(false);
ownerPanel.setVisible(false);
groupNameTxt.setEnabled(false);
groupNamePanel.setVisible(false);
} else {
ownerPanel.setVisible(true);
groupNamePanel.setVisible(true);
}
setTitleText(Util.M.group(result.group.getName()));
groupNameTxt.setText(result.group.getName());
ownerTxt.setText(result.ownerGroup.getName());
descTxt.setText(result.group.getDescription());
accounts = result.accounts;
members.display(result.members);
@@ -258,7 +304,6 @@ public class AccountGroupScreen extends AccountScreen {
MemberTable() {
table.setText(0, 2, Util.C.columnMember());
table.setText(0, 3, Util.C.columnEmailAddress());
table.setText(0, 4, Util.C.columnOwner());
table.addTableListener(new TableListener() {
public void onCellClicked(SourcesTableEvents sender, int row, int cell) {
if (cell != 1 && getRowItem(row) != null) {
@@ -271,7 +316,6 @@ public class AccountGroupScreen extends AccountScreen {
fmt.addStyleName(0, 1, S_ICON_HEADER);
fmt.addStyleName(0, 2, S_DATA_HEADER);
fmt.addStyleName(0, 3, S_DATA_HEADER);
fmt.addStyleName(0, 4, S_ICON_HEADER);
}
@Override
@@ -354,34 +398,10 @@ public class AccountGroupScreen extends AccountScreen {
table.setWidget(row, 2, AccountDashboardLink.link(accounts, accountId));
table.setText(row, 3, accounts.get(accountId).getPreferredEmail());
final CheckBox owner = new CheckBox();
owner.setChecked(k.isGroupOwner());
owner.addClickListener(new ClickListener() {
public void onClick(Widget sender) {
final boolean oldValue = k.isGroupOwner();
final boolean newValue = owner.isChecked();
Util.GROUP_SVC.changeGroupOwner(k.getKey(), newValue,
new GerritCallback<VoidResult>() {
public void onSuccess(final VoidResult result) {
k.setGroupOwner(newValue);
}
@Override
public void onFailure(final Throwable caught) {
owner.setChecked(oldValue);
k.setGroupOwner(oldValue);
super.onFailure(caught);
}
});
}
});
table.setWidget(row, 4, owner);
final FlexCellFormatter fmt = table.getFlexCellFormatter();
fmt.addStyleName(row, 1, S_ICON_CELL);
fmt.addStyleName(row, 2, S_DATA_CELL);
fmt.addStyleName(row, 3, S_DATA_CELL);
fmt.addStyleName(row, 4, S_ICON_CELL);
setRowItem(row, k);
}

3
appjar/src/main/java/com/google/gerrit/client/admin/AdminConstants.java
View File

@@ -24,14 +24,15 @@ public interface AdminConstants extends Constants {
String buttonSaveDescription();
String buttonRenameGroup();
String buttonCreateGroup();
String buttonChangeGroupOwner();
String headingOwner();
String headingDescription();
String headingMembers();
String headingCreateGroup();
String columnMember();
String columnEmailAddress();
String columnOwner();
String columnGroupName();
String columnGroupDescription();

3
appjar/src/main/java/com/google/gerrit/client/admin/AdminConstants.properties
View File

@@ -5,14 +5,15 @@ buttonAddGroupMember = Add
buttonRenameGroup = Rename Group
buttonSaveDescription = Save Description
buttonCreateGroup = Create Group
buttonChangeGroupOwner = Change Owner
headingOwner = Owners
headingDescription = Description
headingMembers = Members
headingCreateGroup = Create New Group
columnMember = Member
columnEmailAddress = Email Address
columnOwner = Owner
columnGroupName = Name
columnGroupDescription = Description

8
appjar/src/main/java/com/google/gerrit/client/admin/GroupAdminService.java
View File

@@ -43,6 +43,10 @@ public interface GroupAdminService extends RemoteJsonService {
void changeGroupDescription(AccountGroup.Id groupId, String description,
AsyncCallback<VoidResult> callback);
@SignInRequired
void changeGroupOwner(AccountGroup.Id groupId, String newOwnerName,
AsyncCallback<VoidResult> callback);
@SignInRequired
void renameGroup(AccountGroup.Id groupId, String newName,
AsyncCallback<VoidResult> callback);
@@ -54,8 +58,4 @@ public interface GroupAdminService extends RemoteJsonService {
@SignInRequired
void deleteGroupMembers(Set<AccountGroupMember.Key> keys,
AsyncCallback<VoidResult> callback);
@SignInRequired
void changeGroupOwner(AccountGroupMember.Key key, boolean owner,
AsyncCallback<VoidResult> callback);
}

88
appjar/src/main/java/com/google/gerrit/client/admin/GroupAdminServiceImpl.java
View File

@@ -30,6 +30,7 @@ import com.google.gwtorm.client.SchemaFactory;
import com.google.gwtorm.client.Transaction;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
@@ -49,9 +50,7 @@ public class GroupAdminServiceImpl extends BaseServiceImplementation implements
if (amAdmin(db)) {
result = db.accountGroups().all().toList();
} else {
final Set<AccountGroup.Id> mine = myOwnedGroups(db);
result =
new ArrayList<AccountGroup>(db.accountGroups().get(mine).toList());
result = myOwnedGroups(db);
Collections.sort(result, new Comparator<AccountGroup>() {
public int compare(final AccountGroup a, final AccountGroup b) {
return a.getName().compareTo(b.getName());
@@ -89,7 +88,6 @@ public class GroupAdminServiceImpl extends BaseServiceImplementation implements
final AccountGroupMember m =
new AccountGroupMember(new AccountGroupMember.Key(RpcUtil
.getAccountId(), group.getId()));
m.setGroupOwner(true);
final Transaction txn = db.beginTransaction();
db.accountGroups().insert(Collections.singleton(group), txn);
@@ -133,6 +131,29 @@ public class GroupAdminServiceImpl extends BaseServiceImplementation implements
});
}
public void changeGroupOwner(final AccountGroup.Id groupId,
final String newOwnerName, final AsyncCallback<VoidResult> callback) {
run(callback, new Action<VoidResult>() {
public VoidResult run(final ReviewDb db) throws OrmException, Failure {
assertAmGroupOwner(db, groupId);
final AccountGroup group = db.accountGroups().get(groupId);
if (group == null) {
throw new Failure(new NoSuchEntityException());
}
final AccountGroup owner =
db.accountGroups().get(new AccountGroup.NameKey(newOwnerName));
if (owner == null) {
throw new Failure(new NoSuchEntityException());
}
group.setOwnerGroupId(owner.getId());
db.accountGroups().update(Collections.singleton(group));
return VoidResult.INSTANCE;
}
});
}
public void renameGroup(final AccountGroup.Id groupId, final String newName,
final AsyncCallback<VoidResult> callback) {
run(callback, new Action<VoidResult>() {
@@ -188,7 +209,7 @@ public class GroupAdminServiceImpl extends BaseServiceImplementation implements
final AsyncCallback<VoidResult> callback) {
run(callback, new Action<VoidResult>() {
public VoidResult run(final ReviewDb db) throws OrmException, Failure {
final Set<AccountGroup.Id> owned = myOwnedGroups(db);
final Set<AccountGroup.Id> owned = ids(myOwnedGroups(db));
Boolean amAdmin = null;
for (final AccountGroupMember.Key k : keys) {
if (!owned.contains(k.getAccountGroupId())) {
@@ -211,53 +232,50 @@ public class GroupAdminServiceImpl extends BaseServiceImplementation implements
});
}
public void changeGroupOwner(final AccountGroupMember.Key key,
final boolean owner, final AsyncCallback<VoidResult> callback) {
run(callback, new Action<VoidResult>() {
public VoidResult run(final ReviewDb db) throws OrmException, Failure {
assertAmGroupOwner(db, key.getAccountGroupId());
final AccountGroupMember m = db.accountGroupMembers().get(key);
if (m == null) {
throw new Failure(new NoSuchEntityException());
}
if (m.isGroupOwner() != owner) {
m.setGroupOwner(owner);
db.accountGroupMembers().update(Collections.singleton(m));
}
return VoidResult.INSTANCE;
}
});
private static boolean amInGroup(final ReviewDb db,
final AccountGroup.Id groupId) throws OrmException {
return db.accountGroupMembers().get(
new AccountGroupMember.Key(RpcUtil.getAccountId(), groupId)) != null;
}
private static boolean amAdmin(final ReviewDb db) throws OrmException {
final AccountGroup admin = db.accountGroups().get(ADMIN_GROUP);
if (admin == null) {
return false;
}
return db.accountGroupMembers().get(
new AccountGroupMember.Key(RpcUtil.getAccountId(), admin.getId())) != null;
return admin != null && amInGroup(db, admin.getId());
}
private static void assertAmGroupOwner(final ReviewDb db,
final AccountGroup.Id groupId) throws OrmException, Failure {
final AccountGroupMember m =
db.accountGroupMembers().get(
new AccountGroupMember.Key(RpcUtil.getAccountId(), groupId));
if ((m == null || !m.isGroupOwner()) && !amAdmin(db)) {
final AccountGroup group = db.accountGroups().get(groupId);
if (group == null) {
throw new Failure(new NoSuchEntityException());
}
if (!amInGroup(db, group.getOwnerGroupId()) && !amAdmin(db)) {
throw new Failure(new NoSuchEntityException());
}
}
private static Set<AccountGroup.Id> myOwnedGroups(final ReviewDb db)
throws OrmException {
private static Set<AccountGroup.Id> ids(
final Collection<AccountGroup> groupList) {
final HashSet<AccountGroup.Id> r = new HashSet<AccountGroup.Id>();
for (final AccountGroupMember m : db.accountGroupMembers().owned(
RpcUtil.getAccountId())) {
r.add(m.getAccountGroupId());
for (final AccountGroup group : groupList) {
r.add(group.getId());
}
return r;
}
private static List<AccountGroup> myOwnedGroups(final ReviewDb db)
throws OrmException {
final List<AccountGroup> own = new ArrayList<AccountGroup>();
for (final AccountGroupMember m : db.accountGroupMembers().byAccount(
RpcUtil.getAccountId()).toList()) {
for (final AccountGroup g : db.accountGroups().ownedByGroup(
m.getAccountGroupId())) {
own.add(g);
}
}
return own;
}
private static Account findAccount(final ReviewDb db, final String nameOrEmail)
throws OrmException, Failure {
final Account r = Account.find(db, nameOrEmail);

20
appjar/src/main/java/com/google/gerrit/client/reviewdb/AccountGroup.java
View File

@@ -74,12 +74,23 @@ public final class AccountGroup {
}
}
/** Unique name of this group within the system. */
@Column
protected NameKey name;
/** Unique identity, to link entities as {@link #name} can change. */
@Column
protected Id groupId;
/**
* Identity of the group whose members can manage this group.
* <p>
* This can be a self-reference to indicate the group's members manage itself.
*/
@Column
protected Id ownerGroupId;
/** A textual description of the group's purpose. */
@Column(length = Integer.MAX_VALUE, notNull = false)
protected String description;
@@ -90,6 +101,7 @@ public final class AccountGroup {
final AccountGroup.Id newId) {
name = newName;
groupId = newId;
ownerGroupId = groupId;
}
public AccountGroup.Id getId() {
@@ -115,4 +127,12 @@ public final class AccountGroup {
public void setDescription(final String d) {
description = d;
}
public AccountGroup.Id getOwnerGroupId() {
return ownerGroupId;
}
public void setOwnerGroupId(final AccountGroup.Id id) {
ownerGroupId = id;
}
}

8
appjar/src/main/java/com/google/gerrit/client/reviewdb/AccountGroupAccess.java
View File

@@ -31,4 +31,12 @@ public interface AccountGroupAccess extends
@Query("ORDER BY name")
ResultSet<AccountGroup> all() throws OrmException;
@Query("WHERE ownerGroupId = ?")
ResultSet<AccountGroup> ownedByGroup(AccountGroup.Id groupId)
throws OrmException;
@Query("WHERE name.name >= ? AND name.name <= ? ORDER BY name LIMIT ?")
ResultSet<AccountGroup> suggestByName(String nameA, String nameB, int limit)
throws OrmException;
}

14
appjar/src/main/java/com/google/gerrit/client/reviewdb/AccountGroupMember.java
View File

@@ -41,7 +41,7 @@ public final class AccountGroupMember {
return accountId;
}
public AccountGroup.Id getAccountGroupId(){
public AccountGroup.Id getAccountGroupId() {
return groupId;
}
@@ -54,10 +54,6 @@ public final class AccountGroupMember {
@Column(name = Column.NONE)
protected Key key;
/** If true, this user owns this group and can edit the membership. */
@Column
protected boolean owner;
protected AccountGroupMember() {
}
@@ -76,12 +72,4 @@ public final class AccountGroupMember {
public AccountGroup.Id getAccountGroupId() {
return key.groupId;
}
public boolean isGroupOwner() {
return owner;
}
public void setGroupOwner(final boolean o) {
owner = o;
}
}

3
appjar/src/main/java/com/google/gerrit/client/reviewdb/AccountGroupMemberAccess.java
View File

@@ -28,9 +28,6 @@ public interface AccountGroupMemberAccess extends
@Query("WHERE key.accountId = ?")
ResultSet<AccountGroupMember> byAccount(Account.Id id) throws OrmException;
@Query("WHERE key.accountId = ? AND owner = 'Y'")
ResultSet<AccountGroupMember> owned(Account.Id id) throws OrmException;
@Query("WHERE key.groupId = ?")
ResultSet<AccountGroupMember> byGroup(AccountGroup.Id id) throws OrmException;
}

62
appjar/src/main/java/com/google/gerrit/client/ui/AccountGroupSuggestOracle.java Normal file
View File

@@ -0,0 +1,62 @@
// Copyright 2008 Google Inc.
//
// 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.client.ui;
import com.google.gerrit.client.RpcStatus;
import com.google.gerrit.client.reviewdb.AccountGroup;
import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gwt.user.client.ui.SuggestOracle;
import java.util.ArrayList;
import java.util.List;
/** Suggestion Oracle for AccountGroup entities. */
public class AccountGroupSuggestOracle extends SuggestOracle {
@Override
public void requestSuggestions(final Request req, final Callback callback) {
RpcStatus.hide(new Runnable() {
public void run() {
SuggestUtil.SVC.suggestAccountGroup(req.getQuery(), req.getLimit(),
new GerritCallback<List<AccountGroup>>() {
public void onSuccess(final List<AccountGroup> result) {
final ArrayList<AccountGroupSuggestion> r =
new ArrayList<AccountGroupSuggestion>(result.size());
for (final AccountGroup p : result) {
r.add(new AccountGroupSuggestion(p));
}
callback.onSuggestionsReady(req, new Response(r));
}
});
}
});
}
private static class AccountGroupSuggestion implements
SuggestOracle.Suggestion {
private final AccountGroup info;
AccountGroupSuggestion(final AccountGroup k) {
info = k;
}
public String getDisplayString() {
return info.getName();
}
public String getReplacementString() {
return info.getName();
}
}
}

5
appjar/src/main/java/com/google/gerrit/client/ui/SuggestService.java
View File

@@ -15,6 +15,7 @@
package com.google.gerrit.client.ui;
import com.google.gerrit.client.data.AccountInfo;
import com.google.gerrit.client.reviewdb.AccountGroup;
import com.google.gerrit.client.reviewdb.Project;
import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwtjsonrpc.client.AllowCrossSiteRequest;
@@ -30,4 +31,8 @@ public interface SuggestService extends RemoteJsonService {
@AllowCrossSiteRequest
void suggestAccount(String query, int limit,
AsyncCallback<List<AccountInfo>> callback);
@AllowCrossSiteRequest
void suggestAccountGroup(String query, int limit,
AsyncCallback<List<AccountGroup>> callback);
}

14
appjar/src/main/java/com/google/gerrit/client/ui/SuggestServiceImpl.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.client.ui;
import com.google.gerrit.client.data.AccountInfo;
import com.google.gerrit.client.reviewdb.Account;
import com.google.gerrit.client.reviewdb.AccountGroup;
import com.google.gerrit.client.reviewdb.Project;
import com.google.gerrit.client.reviewdb.ReviewDb;
import com.google.gerrit.client.rpc.BaseServiceImplementation;
@@ -73,4 +74,17 @@ public class SuggestServiceImpl extends BaseServiceImplementation implements
}
});
}
public void suggestAccountGroup(final String query, final int limit,
final AsyncCallback<List<AccountGroup>> callback) {
run(callback, new Action<List<AccountGroup>>() {
public List<AccountGroup> run(final ReviewDb db) throws OrmException {
final String a = query;
final String b = a + "\uffff";
final int max = 10;
final int n = limit <= 0 ? max : Math.min(limit, max);
return db.accountGroups().suggestByName(a, b, n).toList();
}
});
}
}