From 6fe3e99bc9b831f611b1c4c5dec4326e095a46c7 Mon Sep 17 00:00:00 2001
From: "Shawn O. Pearce" <sop@google.com>
Date: Tue, 5 Jun 2012 09:01:42 -0700
Subject: [PATCH] Don't hyperlink non-internal groups

When an external group (such as LDAP) is used in a permission rule,
don't attempt to link to the group in the internal account system UI.
The group won't load successfully. Instead just display the name and
put the UUID into a tooltip to show the full DN.

Change-Id: I8fbb67a36da5d5fcb69b4f90bfedbc2638896886
---
 .../client/admin/PermissionRuleEditor.java     | 18 ++++++++++++------
 .../gerrit/reviewdb/client/AccountGroup.java   |  6 ++++++
 .../server/account/InternalGroupBackend.java   |  3 +--
 3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java
index e4cced7aa2..5dd8b3c536 100644
--- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java
+++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/admin/PermissionRuleEditor.java
@@ -25,6 +25,7 @@ import com.google.gerrit.common.data.GroupReference;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.common.data.PermissionRange;
 import com.google.gerrit.common.data.PermissionRule;
+import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gwt.core.client.GWT;
 import com.google.gwt.dom.client.DivElement;
 import com.google.gwt.dom.client.SpanElement;
@@ -178,16 +179,21 @@ public class PermissionRuleEditor extends Composite implements
   @Override
   public void setValue(PermissionRule value) {
     GroupReference ref = value.getGroup();
-    if (ref.getUUID() != null) {
+
+    boolean link;
+    if (ref.getUUID() != null && AccountGroup.isInternalGroup(ref.getUUID())) {
+      groupNameLink.setText(ref.getName());
       groupNameLink.setTargetHistoryToken(Dispatcher.toGroup(ref.getUUID()));
+      link = true;
+    } else {
+      groupNameSpan.setInnerText(ref.getName());
+      groupNameSpan.setTitle(ref.getUUID() != null ? ref.getUUID().get() : "");
+      link = false;
     }
 
-    groupNameLink.setText(ref.getName());
-    groupNameSpan.setInnerText(ref.getName());
     deletedGroupName.setInnerText(ref.getName());
-
-    groupNameLink.setVisible(ref.getUUID() != null);
-    UIObject.setVisible(groupNameSpan, ref.getUUID() == null);
+    groupNameLink.setVisible(link);
+    UIObject.setVisible(groupNameSpan, !link);
   }
 
   @Override
diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java
index 9f67fe8cf1..2ea659db61 100644
--- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java
+++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/AccountGroup.java
@@ -79,6 +79,12 @@ public final class AccountGroup {
     }
   }
 
+  /** @return true if the UUID is for a group managed within Gerrit. */
+  public static boolean isInternalGroup(AccountGroup.UUID uuid) {
+    return uuid.get().startsWith("global:")
+        || uuid.get().matches("[0-9a-f]{40}");
+  }
+
   /** Synthetic key to link to within the database */
   public static class Id extends IntKey<com.google.gwtorm.client.Key<?>> {
     private static final long serialVersionUID = 1L;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalGroupBackend.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalGroupBackend.java
index 2335ded935..ad65499a5a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalGroupBackend.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/InternalGroupBackend.java
@@ -57,8 +57,7 @@ public class InternalGroupBackend implements GroupBackend {
 
   @Override
   public boolean handles(AccountGroup.UUID uuid) {
-    return uuid.get().startsWith("global:")
-        || uuid.get().matches("[0-9a-f]{40}");
+    return AccountGroup.isInternalGroup(uuid);
   }
 
   @Override