fixed roles management in the person page

diff --git a/planetlab/common/actions.php b/planetlab/common/actions.php
index ae98281..66d6c0a 100644 (file)
--- a/planetlab/common/actions.php
+++ b/planetlab/common/actions.php
@@ -167,19 +167,23 @@ switch ($action) {
  case 'remove-roles-from-person' : {
    $role_ids=$_POST['role_ids'];
    if ( ! $role_ids) {
-     drupal_set_message("action=$action - No role selected");
-     return;
-   }
-   foreach( $role_ids as $role_id)  {
-     $api->DeleteRoleFromPerson( intval( $role_id ), intval( $person_id ) );
+     drupal_set_error("You have not selected role(s) to remove");
+   } else {
+     foreach( $role_ids as $role_id)  
+       if ( $api->DeleteRoleFromPerson( intval( $role_id ), intval( $person_id ) ) != 1 ) 
+        drupal_set_error ("Could not remove role $role_id from person $person_id");
    }
-   plc_redirect (l_person($person_id));
+   plc_redirect (l_person_roles($person_id));
  }
      
  case 'add-role-to-person' : {
    $role_id=$_POST['role_id'];
-   $api->AddRoleToPerson( intval( $role_id ), intval( $person_id ) );
-   plc_redirect (l_person($person_id));
+   if ( ! $role_id) {
+     drupal_set_error ("You have not selected a role to add");
+   } else if ($api->AddRoleToPerson( intval( $role_id ), intval( $person_id ) ) != 1) {
+     drupal_set_error("Could not add role $role_id to person $person_id");
+   }
+   plc_redirect (l_person_roles($person_id));
  }
 
  case 'enable-person' : {

diff --git a/planetlab/includes/plc_functions.php b/planetlab/includes/plc_functions.php
index 3e69f23..76c81e7 100644 (file)
--- a/planetlab/includes/plc_functions.php
+++ b/planetlab/includes/plc_functions.php
@@ -119,6 +119,7 @@ function l_sliver_t ($node_id,$slice_id,$text) {
 function l_persons ()                  { return "/db/persons/index.php&active_line_tab=All Accounts"; }
 function l_persons_peer ($peer_id)     { return "/db/persons/index.php?peerscope=$peer_id&active_line_tab=Local Accounts"; }
 function l_person ($person_id)         { return "/db/persons/index.php?id=$person_id"; }
+function l_person_roles ($person_id)   { return "/db/persons/index.php?id=$person_id&show_details=0&show_roles=1"; }
 function l_person_t ($person_id,$text) { return href (l_person($person_id),$text); }
 function l_persons_site ($site_id)     { return "/db/persons/index.php?site_id=$site_id"; }
 function l_persons_slice ($slice_id)   { return "/db/persons/index.php?slice_id=$slice_id"; }
@@ -305,6 +306,19 @@ function plc_role_global_hash ($api) {
   return $hash;
 }
 
+// because GetRoles does not correctly support filters, it's really painful to do this
+function sort_roles ($r1, $r2) { return $r2['role_id'] - $r1['role_id']; }
+function roles_except ($roles, $exception_ids) {
+  $result=array();
+  if ($roles) foreach ($roles as $role) {
+      if ( ! in_array ($role['role_id'],$exception_ids) ) {
+       $result[]=$role;
+      }
+    }
+  usort($result,sort_roles);
+  return $result;
+}
+
 //////////////////////////////////////////////////////////// nodegroups
 // hash by 'tagname=value'
 function plc_nodegroup_global_hash ($api,$tagnames=NULL) {

diff --git a/planetlab/persons/person.php b/planetlab/persons/person.php
index 7268311..d79020f 100644 (file)
--- a/planetlab/persons/person.php
+++ b/planetlab/persons/person.php
@@ -141,10 +141,10 @@ $details->form_start(l_actions(),array("action"=>"update-person",
 $details->start();
 
 
-$details->th_td("Title",$title,"title",array('width'=>5));
+$details->th_td("Title",$title,"title",array('width'=>10));
 $details->th_td("First Name",$first_name,"first_name");
 $details->th_td("Last Name",$last_name,"last_name");
-$details->th_td(href("mailto:$email","Email"),$email,"email");
+$details->th_td(href("mailto:$email","Email"),$email,"email",array("width"=>30));
 $details->th_td("Phone",$phone,"phone");
 $details->th_td("URL",$url,"url",array('width'=>40));
 $details->th_td("Bio",$bio,"bio",array('input_type'=>'textarea','height'=>4));
@@ -313,13 +313,13 @@ if ($local_peer) {
  }
 //////////////////// roles
 if ($local_peer) {
-  $toggle=new PlekitToggle ('roles','Roles',array('visible'=>get_arg('show_roles',false)));
+  $toggle=new PlekitToggle ('roles',count_english($roles,"role"),array('visible'=>get_arg('show_roles',false)));
   $toggle->start();
 
   if (! $roles) plc_warning ("This user has no role !");
 
   $is_pi_of_the_site = ( plc_in_site($site_ids[0]) && plc_is_pi() );
-  $can_manage_roles= ( $local_peer && plc_is_admin() || $is_pi_of_the_site );
+  $can_manage_roles= ( ($local_peer && plc_is_admin()) || $is_pi_of_the_site );
   $table_options=array("search_area"=>false,"notes_area"=>false);
 
   $headers=array("Role"=>"string");
@@ -344,6 +344,8 @@ if ($local_peer) {
 
   // footers : the remove and add buttons
   if ($can_manage_roles) {
+
+    // remove
     $table->tfoot_start();
     if ($roles) {
       $table->row_start();
@@ -352,22 +354,26 @@ if ($local_peer) {
       $table->row_end();
     }
 
-    $table->row_start();
-    if ($is_pi_of_the_site) {
-      // pi's can only add/remove tech (40) and user (30) roles.
-        $role_ids_to_add = array_diff(array(30, 40), $role_ids);
-        if ($role_ids_to_add) {
-            $selectors=$form->role_selectors($api, $role_ids_to_add);
-        }
-    } else {
-      $selectors=$form->role_selectors_excluding($api,$role_ids);
+    // add
+    // compute the roles that can be added
+    if (plc_is_admin()) 
+      // all roles
+      $exclude_role_ids=array();
+    else
+      // all roles except admin and pi
+      $exclude_role_ids=array(10,20);
+    $possible_roles = roles_except($api->GetRoles(),$exclude_role_ids);
+    $roles_to_add = roles_except ($possible_roles,$role_ids);
+    if ( $roles_to_add ) {
+      $selectors=$form->role_selectors($roles_to_add);
+      $table->row_start();
+      $add_role_left_area=$form->select_html("role_id",$selectors,array('label'=>"Choose role"));
+      // add a role : the button
+      $add_role_right_area=$form->submit_html("add-role-to-person","Add role");
+      $table->cell ($add_role_left_area . $add_role_right_area,
+                   array('hfill'=>true,'align'=>'right'));
+      $table->row_end();
     }
-    $add_role_left_area=$form->select_html("role_id",$selectors,array('label'=>"Choose role"));
-    // add a role : the button
-    $add_role_right_area=$form->submit_html("add-role-to-person","Add role");
-    $table->cell ($add_role_left_area . $add_role_right_area,
-                 array('hfill'=>true,'align'=>'right'));
-    $table->row_end();
   }
   $table->end();
   $toggle->end();

diff --git a/planetlab/tags/tag.php b/planetlab/tags/tag.php
index 010150c..db09b8c 100644 (file)
--- a/planetlab/tags/tag.php
+++ b/planetlab/tags/tag.php
@@ -94,7 +94,7 @@ $details->th_td("Description",$description,"description",array('width'=>40));
 
 if ($can_update) {
 // select the option corresponding with min_role_id
-  $selectors = $details->form()->role_selectors($api,"",$min_role_id);
+  $selectors = $details->form()->role_selectors($api->GetRoles(),$min_role_id);
   $select_field = $details->form()->select_html("min_role_id",$selectors);
   // xxx would need to turn role_id into role name
   $details->th_td("Min role",$select_field,"min_role_id",array('input_type'=>'select','value'=>$min_role_id));

diff --git a/plekit/php/form.php b/plekit/php/form.php
index d4ee499..4d5e9fd 100644 (file)
--- a/plekit/php/form.php
+++ b/plekit/php/form.php
@@ -128,40 +128,23 @@ class PlekitForm {
 
   // helper function to handle role-oriented selectors
   // because GetRoles does not correctly support filters, it's really painful to do this
-  static public function role_selectors($api,$role_ids=NULL,$current=NULL) {
+  static public function role_selectors($roles,$current_id=NULL) {
     function role_selector ($role) { return array('display'=>$role['name'],"value"=>$role['role_id']); }
-    function role_id ($role) { return $role['role_id']; }
-
-    $all_roles=$api->GetRoles();
-    if ( ! $role_ids)
-      $role_ids=array_map("role_id",$all_roles);
-
     $selectors=array();
     // preserve input order
-    foreach ($role_ids as $role_id) {
-      foreach ($all_roles as $all_role) {
-       if ($all_role['role_id'] == $role_id) {
-         $selector=role_selector($all_role);
-         if ($role_id == $current) 
+    if ( ! $roles) {
+      drupal_set_message('WARNING: empty roles in role_selectors');
+    } else {
+      foreach ($roles as $role) {
+       $selector=role_selector($role);
+       if ($role['role_id'] == $current_id) 
            $selector['selected']=true;
-         $selectors []= $selector;
-       }
+       $selectors []= $selector;
       }
     }
     return $selectors;
   }
 
-  static public function role_selectors_excluding ($api,$exclude_role_ids=NULL,$current=NULL) {
-    if ( ! $exclude_role_ids ) $exclude_role_ids = array();
-    $all_roles=$api->GetRoles();
-    $role_ids = array();
-    foreach ($all_roles as $role) {
-      if ( ! in_array ($role['role_id'],$exclude_role_ids)) {
-       $role_ids [] = $role['role_id'];
-      }
-    }
-    return PlekitForm::role_selectors($api,$role_ids,$current);    
-  }
 }
 
 // a form with a single button