opendev/gerrit
Code Issues Proposed changes

Don't show error on ACL modification if a section is added more than once

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 multiple sections for
the same ref were added. This is now fixed by merging the expected
sections before doing the verification.

Change-Id: I8236bfa33ab5cdf215770b2cf5f47bd798ee0b94
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2013-01-03 08:57:34 +01:00
parent c486dd357b
commit 0371240169
3 changed files with 74 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -14,12 +14,14 @@
package com.google.gerrit.client.admin;
import static com.google.gerrit.common.ProjectAccessUtil.mergeSections;
import static com.google.gerrit.common.ProjectAccessUtil.removeEmptyPermissionsAndSections;
import com.google.gerrit.client.Gerrit;
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;
@@ -198,7 +200,7 @@ public class ProjectAccessScreen extends ProjectScreen {
private Set<String> getDiffs(ProjectAccess wantedAccess,
ProjectAccess newAccess) {
final List<AccessSection> wantedSections =
removeEmptyPermissionsAndSections(wantedAccess.getLocal());
mergeSections(removeEmptyPermissionsAndSections(wantedAccess.getLocal()));
final HashSet<AccessSection> same =
new HashSet<AccessSection>(wantedSections);
final HashSet<AccessSection> different =
@@ -216,29 +218,6 @@ 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();