opendev/gerrit
Code Issues Proposed changes

Don't show error on ACL modification if empty permissions are added

Browse Source 
Even if the RPC to change the access rights of a project returns
successfully it might be that not all ACL modifications were saved
successfully since modifications to refs which are not owned are
ignored. This is why on client side there is a check to verify if all
modifications were successfully saved and if not the user is informed
about failed modifications with an error message.

This error message was incorrectly displayed if a permission without
rules was added. This is now fixed by removing empty permissions and
sections from the expected ACLs before doing the verification.

Bug: issue 1662
Change-Id: I3da941b4eba0255329dc461760471f6f1418d23e
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2013-01-03 08:56:57 +01:00
parent 6802493d7b
commit c486dd357b
1 changed files with 30 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/ProjectAccessScreen.java
View File

@@ -19,6 +19,7 @@ import com.google.gerrit.client.rpc.GerritCallback;
import com.google.gerrit.client.rpc.ScreenLoadCallback;
import com.google.gerrit.common.PageLinks;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.ProjectAccess;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
@@ -39,6 +40,7 @@ import com.google.gwtexpui.globalkey.client.NpTextArea;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
public class ProjectAccessScreen extends ProjectScreen {
@@ -195,12 +197,14 @@ public class ProjectAccessScreen extends ProjectScreen {
private Set<String> getDiffs(ProjectAccess wantedAccess,
ProjectAccess newAccess) {
final List<AccessSection> wantedSections =
removeEmptyPermissionsAndSections(wantedAccess.getLocal());
final HashSet<AccessSection> same =
new HashSet<AccessSection>(wantedAccess.getLocal());
new HashSet<AccessSection>(wantedSections);
final HashSet<AccessSection> different =
new HashSet<AccessSection>(wantedAccess.getLocal().size()
new HashSet<AccessSection>(wantedSections.size()
+ newAccess.getLocal().size());
different.addAll(wantedAccess.getLocal());
different.addAll(wantedSections);
different.addAll(newAccess.getLocal());
same.retainAll(newAccess.getLocal());
different.removeAll(same);
@@ -212,6 +216,29 @@ public class ProjectAccessScreen extends ProjectScreen {
return differentNames;
}
private List<AccessSection> removeEmptyPermissionsAndSections(
final List<AccessSection> src) {
final Set<AccessSection> sectionsToRemove = new HashSet<AccessSection>();
for (final AccessSection section : src) {
final Set<Permission> permissionsToRemove = new HashSet<Permission>();
for (final Permission permission : section.getPermissions()) {
if (permission.getRules().isEmpty()) {
permissionsToRemove.add(permission);
}
}
for (final Permission permissionToRemove : permissionsToRemove) {
section.remove(permissionToRemove);
}
if (section.getPermissions().isEmpty()) {
sectionsToRemove.add(section);
}
}
for (final AccessSection sectionToRemove : sectionsToRemove) {
src.remove(sectionToRemove);
}
return src;
}
@Override
public void onFailure(Throwable caught) {
error.clear();