From ad5cf1ebf11f0e730bf89ae3b219d9d53a5b2b96 Mon Sep 17 00:00:00 2001
From: Ingo Schommer <ingo@silverstripe.com>
Date: Mon, 5 Mar 2012 14:40:40 +0100
Subject: [PATCH] ENHANCEMENT Using ListboxField/chosen.js to assign roles in
 Group->getCMSFields(). Improved explanation of groups in the user interface.

---
 admin/code/SecurityAdmin.php | 15 ---------------
 lang/en_US.php               |  4 ++--
 security/Group.php           | 29 +++++++++++++++++++----------
 3 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/admin/code/SecurityAdmin.php b/admin/code/SecurityAdmin.php
index 2e9723dd8..ea10d401b 100755
--- a/admin/code/SecurityAdmin.php
+++ b/admin/code/SecurityAdmin.php
@@ -63,21 +63,6 @@ class SecurityAdmin extends LeftAndMain implements PermissionProvider {
 						)
 					)
 				);
-				if(Permission::check('APPLY_ROLES')) { 
-					$fields->addFieldToTab(
-						'Root.Roles',
-						new LiteralField(
-							'RolesAddEditLink', 
-							sprintf(
-								'<p class="add-role"><a href="%s">%s</a></p>',
-								$this->Link('show/root'),
-								// TODO This should include #Root_Roles to switch directly to the tab,
-								// but tabstrip.js doesn't display tabs when directly adressed through a URL pragma
-								_t('Group.RolesAddEditLink', 'Add/edit roles')
-							)
-						)
-					);
-				}
 		
 				// Filter permissions
 				$permissionField = $form->Fields()->dataFieldByName('Permissions');
diff --git a/lang/en_US.php b/lang/en_US.php
index 93ed18602..1625f8676 100644
--- a/lang/en_US.php
+++ b/lang/en_US.php
@@ -281,7 +281,7 @@ $lang['en_US']['Group']['Parent'] = array(
 	PR_MEDIUM,
 	'One group has one parent group'
 );
-$lang['en_US']['Group']['RolesAddEditLink'] = 'Add/edit roles';
+$lang['en_US']['Group']['RolesAddEditLink'] = 'Manage roles';
 $lang['en_US']['Group']['SINGULARNAME'] = array(
 	'Group',
 	50,
@@ -730,7 +730,7 @@ $lang['en_US']['SecurityAdmin']['MemberListCaution'] = 'Caution: Removing member
 $lang['en_US']['SecurityAdmin']['NEWGROUP'] = 'New Group';
 $lang['en_US']['SecurityAdmin']['PERMISSIONS'] = 'Permissions';
 $lang['en_US']['SecurityAdmin']['ROLES'] = 'Roles';
-$lang['en_US']['SecurityAdmin']['ROLESDESCRIPTION'] = 'This section allows you to add roles to this group. Roles are logical groupings of permissions, which can be editied in the Roles tab';
+$lang['en_US']['SecurityAdmin']['ROLESDESCRIPTION'] = 'Roles are predefined sets of permissions, and can be assigned to groups.<br />They are inherited from parent groups if required.';
 $lang['en_US']['SecurityAdmin']['SGROUPS'] = 'Security Groups';
 $lang['en_US']['SecurityAdmin']['TABIMPORT'] = 'Import';
 $lang['en_US']['SecurityAdmin']['TABROLES'] = 'Roles';
diff --git a/security/Group.php b/security/Group.php
index ffd7f8f4c..9bd01156f 100755
--- a/security/Group.php
+++ b/security/Group.php
@@ -123,9 +123,17 @@ class Group extends DataObject {
 				new LiteralField( 
 					"",  
 					"<p>" .  
-					_t('SecurityAdmin.ROLESDESCRIPTION', 
-						"This section allows you to add roles to this group. Roles are logical groupings of permissions, which can be editied in the Roles tab" 
-					) .  
+					_t(
+						'SecurityAdmin.ROLESDESCRIPTION', 
+						"Roles are predefined sets of permissions, and can be assigned to groups.<br />They are inherited from parent groups if required."
+					) . '<br />' . 
+					sprintf(
+						'<a href="%s" class="add-role">%s</a>',
+						singleton('SecurityAdmin')->Link('show/root#Root_Roles'),
+						// TODO This should include #Root_Roles to switch directly to the tab,
+						// but tabstrip.js doesn't display tabs when directly adressed through a URL pragma
+						_t('Group.RolesAddEditLink', 'Manage roles')
+					) .
 					 "</p>" 
 				) 
 			);
@@ -139,13 +147,14 @@ class Group extends DataObject {
 				$ancestorRoles = $ancestor->Roles();
 				if($ancestorRoles) $inheritedRoles->merge($ancestorRoles);
 			}
-			$fields->findOrMakeTab('Root.Roles', 'Root.' . _t('SecurityAdmin.ROLES', 'Roles'));
-			$fields->addFieldToTab(
-				'Root.Roles',
-				$rolesField = new CheckboxSetField('Roles', 'Roles', $allRoles)
-			);
-			$rolesField->setDefaultItems($inheritedRoles->column('ID'));
-			$rolesField->setDisabledItems($inheritedRoles->column('ID'));
+			$groupRoleIDs = $groupRoles->column('ID') + $inheritedRoles->column('ID');
+			$rolesField = Object::create('ListboxField', 'Roles', false, $allRoles->map()->toArray())
+					->setMultiple(true)
+					->setDefaultItems($groupRoleIDs)
+					->setAttribute('data-placeholder', _t('Group.AddRole', 'Add a role for this group'))
+					->setDisabledItems($inheritedRoles->column('ID'));	
+			if(!$allRoles->Count()) $rolesField->setAttribute('data-placeholder', _t('Group.NoRoles', 'No roles found'));
+			$fields->addFieldToTab('Root.Roles', $rolesField);
 		} 
 		
 		$fields->push($idField = new HiddenField("ID"));