A bunch of work on the admin cli (acl editor now works)rt-1.3.38

a couple API changes to standardize method names across classes
template editing via the web should work better now.

16 files changed, 646 insertions, 163 deletions

diff --git a/bin/rtadmin b/bin/rtadmin
index b5c1f9d33c..48aa6b6f69 100644
--- a/bin/rtadmin
+++ b/bin/rtadmin
@@ -33,6 +33,59 @@ PickMode();
 # {{{ Help
 
 sub Help {
+
+# {{{ help_acl
+my $help_acl ="
+        Access control
+	--grant-right <right> 
+	--revoke-right <right>
+	  --userid <user>
+          --groupid <group>
+	--list-rights";
+# }}}
+
+# {{{ help_keyword_sel
+my $help_keyword_sel = "
+       Keyword Selections
+       --add-keyword-select [name]
+          --root <keyword>
+          --single 
+          --multiple
+          --depth  <int>
+        --delete-keyword-select [name]";
+
+# }}}
+
+# {{{ help_scrip
+my $help_scrip = "
+        Scrips
+        --create-scrip
+           --condition <condition name or id>
+           --action <action name or id>
+           --template <template name or id>
+
+        --delete-scrip <id>
+	--list-scrips";
+# }}}
+
+# {{{ help_template  
+my $help_template = "      
+        Templates
+        --delete-template [<id>|<name>]
+        --display-template [<id>|<name>]
+
+         --create-template
+         --modify-template [<id>|<name>]
+           Flags for --create-template and --modify-template
+           --template-name
+           --template-description
+           --template-edit-content
+         
+         --list-templates";
+
+# }}}
+
+
 print <<EOF;
 
 USAGE:  rtadmin --user <userid> [Userflags]
@@ -114,89 +167,45 @@ Queue Configuration for --queue <queueid>
 	--add-admincc <email address>
 	--delete-admincc <email address>
 	
-	--add-keyword-select [name]
-            --root <keyword>
-            --single 
-            --multiple
-            --depth  <int>
-            
-	--delete-keyword-select [name]
-	
-
-	--scrip [<id>] (id needed for delete or modify)
-           --delete
-           --create
-           --condition <condition name or id>
-           --action <action name or id>
-           --template <template name or id>
 
-	--list-scrips
+$help_acl
 
-        --delete-template [<id>|<name>]
-        --display-template [<id>|<name>]
+$help_keyword_sel
 
-         --create-template
-         --modify-template [<id>|<name>]
-           Flags for --create-template and --modify-template
-           --template-name
-           --template-description
-           --template-edit
-         
-         --list-templates
+$help_template
 
-	--grant-user <userid> <right>
-	--grant-group <groupid> <right>
+$help_scrip
 
-	--list-user-rights <user>
-	--list-users-with-right <right>
 
-	--list-group-rights <group>
-	--list-groups-with-right <right>
+System configuration for --system 
 
-	--revoke-user <userid> <right>
-	--revoke-group <groupid> <right>
+$help_acl
 
+$help_keyword_sel
 
-System configuration for --system 
-	--add-keyword-select [name] [root keyword] [<single|multiple>] [depth]
-	--delete-keyword-select [name]
-	--list-keyword-selects	
+$help_template
 
-	--list-templates
+$help_scrip
 
-	--add-scrip <condition> <action> <template>
-	--delete-scrip <condition> <action> <template>
-	--list-scrips
 
-	--grant-user <userid> <right>
-	--grant-group <groupid> <right>
-	
-	--list-user-rights <user>
-	--list-users-with-right
+Keyword configuration for --keyword  <fully qualified name>
+	--list-children [<optional-depth>]
+  	--create
+	--delete
+	--name
+	--description
 
-	--list-group-rights <group>
-	--list-groups-with-right
-	
-	--revoke-user <userid> <right>
-	--revoke-group <groupid> <right>	
+EOF
 
 
 
-Template Configuration for --template 
 
 
 
 
 
-Keyword configuration for --keyword  <fully qualified name>
-	--list-children [<optional-depth>]
-  	--create
-	--delete
-	--name
-	--description
 
 
-EOF
 
 }
 
@@ -326,7 +335,6 @@ sub AdminUser {
 	}		
 	    
 	
-	print "About to display, if we can do that\n";
 	#Check if we need to display the user
 	if ($args{'display'}) {
 	    foreach my $attrib (@attributes) {
@@ -418,6 +426,9 @@ sub AdminQueue {
 	}	
        	
         AdminTemplates($queue_obj->Id());
+        AdminScrips($queue_obj->Id());
+	AdminRights($queue_obj->Id());
+	
 
 	#Check if we need to delete the queue
 	if ($args{'delete'}) {
@@ -431,8 +442,6 @@ sub AdminQueue {
 # }}}
 
 
-
-
 sub AdminKeyword {
     my $keyword = shift;
 }
@@ -444,10 +453,15 @@ sub AdminGroup {
 
 sub AdminSystem {
     print "In AdminSystem\n";
+
     AdminTemplates(0);
+    AdminScrips(0);
+    AdminRights(0);
 }
 
 
+# {{{ sub AdminTemplates
+
 sub AdminTemplates {
     my $queue = shift;
     #Queue = 0 means 'global';
@@ -469,7 +483,7 @@ sub AdminTemplates {
 	    $templates->LimitToQueue($queue);
 	}	
 	else {
-	    $templates->LimitToSystem();
+	    $templates->LimitToGlobal();
 	}	
 	while (my $template = $templates->Next) {
 	    print $template->Id.": ".$template->Name." - " . $template->Description ."\n";
@@ -478,6 +492,7 @@ sub AdminTemplates {
     else {
 	print "Did not want to list templates\n";
     }
+
     # }}}
 
     require RT::Template;	
@@ -541,3 +556,190 @@ sub AdminTemplates {
 	print $template->Name . "\n". $template->Description ."\n". $template->Content."\n";
     }	
 }	
+
+# }}}
+
+# {{{ sub AdminScrips
+
+sub AdminScrips {
+    my $queue = shift;
+    #Queue = 0 means 'global';
+
+    my %args;
+
+    
+    GetOptions(\%args, 'list-scrips', 'create-scrip','modify-scrip=s',
+	       'scrip-action=s', 'scrip-template=s', 'scrip-condition=s',
+	       'delete-scrip=s');
+
+    
+    # {{{ List entries
+    if ($args{'list-scrips'}) {
+	print "Scrips for $queue\n";
+	require RT::Scrips;
+	my $scrips = new RT::Scrips($CurrentUser);
+	if ($queue != 0) {
+	    $scrips->LimitToQueue($queue);
+	}	
+	else {
+	    $scrips->LimitToGlobal();
+	}	
+	while (my $scrip = $scrips->Next) {
+	    print $scrip->Id.": If ".
+	      $scrip->ConditionObj->Name." then " .
+		$scrip->ActionObj->Name." with template " .
+		  $scrip->TemplateObj->Name."\n";
+	}	
+    }
+    # }}}
+
+    require RT::Scrip;
+    my $scrip = new RT::Scrip($CurrentUser);
+    if ($args{'delete-scrip'}) {
+	$scrip->Load($args{'delete-scrip'});
+	unless ($scrip->id) {
+	    print "Couldn't load scrip";
+	      return(undef);
+	}
+	my ($val, $msg) = $scrip->Delete();
+	print "$msg\n";
+    }
+    elsif ($args{'create-scrip'}) {
+	my ($val, $msg) = $scrip->Create( ScripAction => $args{'scrip-action'},
+					  ScripCondition => $args{'scrip-condition'},
+					  Template => $args{'scrip-template'},
+					  Queue => $queue);
+
+	print "$msg\n";
+    }	
+}	
+
+
+# }}}
+
+# {{{ sub AdminRights
+
+sub AdminRights {
+    my $queue = shift;
+    #Queue = 0 means 'global';
+
+    my ($scope, $appliesto);
+    if ($queue == 0) {
+	$scope = 'System';
+	$appliesto = 0;
+    }	
+    else {
+	$scope =  'Queue';
+	$appliesto = $queue;
+    }	
+
+    my %args;    
+    GetOptions(\%args, 
+	       'grant-right|add-right|new-right|create-right=s@',
+	       'revoke-right|del-right|delete-right=s@',
+	       'list-rights', 'userid=s@', 'groupid=s@',
+	      );
+    
+    
+    # {{{ List entries
+    if ($args{'list-rights'}) {
+	require RT::ACL;
+	my $acl = new RT::ACL($CurrentUser);
+	if ($queue != 0) {
+	    $acl->LimitToQueue($queue);
+	}	
+	else {
+	    $acl->LimitToSystem();
+	}	
+	while (my $ace = $acl->Next) {
+	    print $ace->RightScope;
+	    
+	    #Print the queue name if we have it.
+	    print " " . $ace->AppliesToObj->Name if (defined $ace->AppliesToObj);
+	    
+	    print ": ". $ace->PrincipalType . " " .$ace->PrincipalObj->Name .
+	      " has right " . $ace->RightName ."\n";
+	      
+	}	
+    }
+    # }}}
+
+    require RT::ACE;    
+
+    # {{{ Build up an array of principals
+    my (@principals);
+    my $i = 0;
+    foreach my $group (@{$args{'groupid'}}) { 
+
+
+	my $princ = new RT::Group($CurrentUser);
+	$princ->Load("$group");
+	if ($princ->id) {
+	    $principals[$i]->{'type'} = 'Group';
+	    $principals[$i]->{'id'} = $princ->id();
+	    $i++;
+	}	
+	else {
+	    print "Could not find group $group\n";
+	}	
+    }	
+
+
+    foreach my $user (@{$args{'userid'}}) { 
+	my $princ = new RT::User($CurrentUser);
+	$princ->Load("$user");
+	if ($princ->id) {
+	    $principals[$i]->{'type'} = 'User';
+	    $principals[$i]->{'id'} = $princ->id();
+	    $i++;
+	}	
+	else {
+	    print "Could not find user $user.\n";
+	    }
+    }
+    # }}}
+
+
+    foreach my $principal (@principals) {
+
+	# {{{ Delete rights that need deleting
+	foreach my $right (@{$args{'revoke-right'}}) {
+	    my $ace = new RT::ACE($CurrentUser);
+	    $RT::Logger->debug("Trying to delete a right: $right \n");
+	    my ($val, $msg) = $ace->LoadByValues( RightName => $right,
+						  RightScope => $scope,
+						  PrincipalType => $principal->{'type'},
+						  PrincipalId => $principal->{'id'},
+						  RightAppliesTo => $appliesto);
+	    
+	    unless ($val) {
+		print "Right $right not found for" . $principal->{'type'} . " " .
+		  $principal->{'id'} . " in scope $scope ($appliesto)\n";
+		next;
+	    }	
+	    $ace->Delete;
+	    #TODO: check some return values
+	    print "ACE deleted\n";
+	    
+	}
+	# }}}
+	
+	# {{{ grant rights that need granting
+	foreach my $right (@{$args{'grant-right'}}) {
+	    my $ace = new RT::ACE($CurrentUser);
+	    my ($val, $msg) = $ace->Create(RightName => $right,
+					   PrincipalType => $principal->{'type'},
+					   PrincipalId => $principal->{'id'},
+					   RightScope => $scope,
+					   RightAppliesTo => $appliesto);
+	    
+	    print $msg . "\n";
+	}
+
+	# }}}
+    }	
+
+}
+
+
+# }}}
diff --git a/lib/RT/ACE.pm b/lib/RT/ACE.pm
index 7c92e1497f..a1cc875b09 100755
--- a/lib/RT/ACE.pm
+++ b/lib/RT/ACE.pm
@@ -23,9 +23,9 @@ use RT::Record;
 use vars qw (%SCOPES
    	     %QUEUERIGHTS
 	     %SYSTEMRIGHTS
-	    ); 
-
+	     %LOWERCASERIGHTNAMES
 
+	    ); 
 
 %SCOPES = (
 	   System => 'System-level right',
@@ -39,8 +39,8 @@ use vars qw (%SCOPES
 %QUEUERIGHTS = ( 
 		SeeQueue => 'Can this principal see this queue',
 		AdminQueue => 'Create, delete and modify queues', 
-		
-		ModifyACL => 'Modify this queue\'s ACL',
+		ShowACL => 'Display Access Control List',
+		ModifyACL => 'Modify Access Control List',
 		ModifyQueueWatchers => 'Modify the queue watchers',
                 ModifyKeywordSelects => 'Modify keyword selections for this queue',
 
@@ -71,7 +71,6 @@ use vars qw (%SCOPES
 		 AdminGroups => 'Create, delete and modify groups',
 		 AdminUsers => 'Create, Delete and Modify users',
 		 ModifySelf => 'Modify one\'s own RT account',
-		 ModifySystemACL => 'Modify system ACLs',
 
 		);
 
@@ -85,9 +84,20 @@ use vars qw (%SCOPES
 			               AdminCc => 'The administrative CC of a ticket',
 			 );
 
-
 # }}}
 
+# {{{ We need to build a hash of all rights, keyed by lower case names
+
+#since you can't do case insensitive hash lookups
+
+foreach $right (keys %QUEUERIGHTS) {
+    $LOWERCASERIGHTNAMES{lc $right}=$right;
+}
+foreach $right (keys %SYSTEMRIGHTS) {
+    $LOWERCASERIGHTNAMES{lc $right}=$right;
+}
+
+# }}}
 
 # {{{ sub _Init
 sub _Init  {
@@ -97,8 +107,62 @@ sub _Init  {
 }
 # }}}
 
+# {{{ sub LoadByValues
+
+=head2 LoadByValues PARAMHASH
+
+Load an ACE by specifying a paramhash with the following fields:
+
+              PrincipalId => undef,
+	      PrincipalType => undef,
+	      RightName => undef,
+	      RightScope => undef,
+	      RightAppliesTo => undef,
+
+=cut
+
+sub LoadByValues {
+  my $self = shift;
+  my %args = (PrincipalId => undef,
+	      PrincipalType => undef,
+	      RightName => undef,
+	      RightScope => undef,
+	      RightAppliesTo => undef,
+	      @_);
+  
+  $self->LoadByCols (PrincipalId => $args{'PrincipalId'},
+		     PrincipalType => $args{'PrincipalType'},
+		     RightName => $args{'RightName'},
+		     RightScope => $args{'RightScope'},
+		     RightAppliesTo => $args{'RightAppliesTo'}
+		    );
+  
+  #If we couldn't load it.
+  unless ($self->Id) {
+      return (0, "ACE not found");
+  }
+  # if we could
+  return ($self->Id, "ACE Loaded");
+  
+}
+
+# }}}
+
+
 # {{{ sub Create
 
+=head2 Create <PARAMS>
+
+PARAMS is a parameter hash with the following elements:
+
+   PrincipalType => "Queue"|"User"
+   PrincipalId => an intentifier you can use to ->Load a user or group
+   RightName => the name of a right. in any case
+   RightScope => "System" | "Queue"
+   RightAppliesTo => a queue id or undef
+
+=cut
+
 sub Create {
     my $self = shift;
     my %args = ( PrincipalId => undef,
@@ -109,23 +173,48 @@ sub Create {
 		 @_
 	       );
     
+    # {{{ Validate the principal
+    my ($princ_obj);
+    if ($args{'PrincipalType'} eq 'Group') {
+	$princ_obj = new RT::User($RT::SystemUser);
+	
+    }	
+    elsif ($args{'PrincipalType'} eq 'User') {
+	require RT::Group;
+	$princ_obj = new RT::Group($RT::SystemUser);
+    }
+    else {
+	return (0, 'Principal type '.$args{'PrincipalType'} . ' is invalid.');
+    }	
+    
+    $princ_obj->Load($args{'PrincipalId'});
+    my $princ_id = $princ_obj->Id();
+    
+    unless ($princ_id) {
+	return (0, 'Principal '.$args{'PrincipalId'}.' not found.');
+    }
+
+    # }}}
+
+    # {{{ Check the scope
     if ($args{'RightScope'} eq 'System') {
 	
-	unless ($self->CurrentUser->HasSystemRight('ModifySystemACL')) {
+	unless ($self->CurrentUser->HasSystemRight('ModifyACL')) {
 	    $RT::Logger->error("No permission to grant rights");
 	    return(undef);
 	}
 	
-	#TODO check if it's a valid RightName/Principaltype
+
     }
     elsif ($args{'RightScope'} eq 'Queue') {
    	
-    unless ($self->CurrentUser->HasQueueRight( Queue => $args{'RightAppliesTo'},
-						  Right => 'ModifyQueueACL')) {
+	unless ($self->CurrentUser->HasQueueRight( Queue => $args{'RightAppliesTo'},
+						   Right => 'ModifyACL')) {
 	    return (0, 'No permission to grant rights');
 	}
+	#TODO allow loading of queues by name.
+
 	
-	#TODO check if it's a valid RightName/Principaltype
 	
     }
     #If it's not a scope we recognise, something scary is happening.
@@ -134,9 +223,38 @@ sub Create {
 			 $args{'RightScope'}." Bailing. \n");
 	return(0,"System error. Unable to grant rights.");
     }
+
+    # }}}
+
+    # {{{ Canonicalize and check the right name
+    $args{'RightName'} = $self->CanonicalizeRightName($args{'RightName'});
     
+    #check if it's a valid RightName
+    if ($args{'RightScope'} eq 'Queue') {
+	unless (exists $QUEUERIGHTS{$args{'RightName'}}) {
+	    return(0, 'Invalid right');
+	}	
+	}	
+    elsif ($args{'RightScope' eq 'System'}) {
+	unless (exists $SYSTEMRIGHTS{$args{'RightName'}}) {
+	    return(0, 'Invalid right');
+	}		    
+    }	
+    # }}}
+    
+    # Make sure the right doesn't already exist.
+    $self->LoadByCols (PrincipalId => $princ_id,
+		       PrincipalType => $args{'PrincipalType'},
+		       RightName => $args{'RightName'},
+		       RightScope => $args {'RightScope'},
+		       RightAppliesTo => $args{'RightAppliesTo'}
+		      );
+    if ($self->Id) {
+	return (0, 'That user already has that right');
+    }	
+
     $RT::Logger->debug("$self ->Create Granting ". $args{'RightName'} ." to ".  $args{'PrincipalId'}."\n");
-    my $id = $self->SUPER::Create( PrincipalId => $args{'PrincipalId'},
+    my $id = $self->SUPER::Create( PrincipalId => $princ_id,
 				   PrincipalType => $args{'PrincipalType'},
 				   RightName => $args{'RightName'},
 				   RightScope => $args {'RightScope'},
@@ -152,6 +270,7 @@ sub Create {
 	return(undef);
     }
 }
+
 # }}}
 
 # {{{ sub _BootstrapRight 
@@ -188,6 +307,29 @@ sub _BootstrapRight {
 
 # }}}
 
+# {{{ sub CanonicalizeRightName
+
+=head2 CanonicalizeRightName <RIGHT>
+
+Takes a queue or system right name in any case and returns it in
+the correct case. If it's not found, will return undef.
+
+=cut
+
+sub CanonicalizeRightName {
+    my $self = shift;
+    my $right = shift;
+    $right = lc $right;
+    if (exists $LOWERCASERIGHTNAMES{"$right"}) {
+	return ($LOWERCASERIGHTNAMES{"$right"});
+    }
+    else {
+	return (undef);
+    }
+}
+
+# }}}
+
 # {{{ sub QueueRights
 
 =head2 QueueRights
@@ -232,6 +374,69 @@ sub _Accessible  {
 }
 # }}}
 
+# {{{ sub AppliesToObj
+
+=head2 AppliesToObj
+
+If the AppliesTo is a queue, returns the queue object. If it's 
+the system object, returns undef. If the user has no rights, returns undef.
+
+=cut
+
+sub AppliesToObj {
+    my $self = shift;
+    if ($self->RightScope eq 'Queue') {
+	my $appliesto_obj = new RT::Queue($self->CurrentUser);
+	$appliesto_obj->Load($self->RightAppliesTo);
+	return($appliesto_obj);
+    }
+    elsif ($self->RightScope eq 'System') {
+	return (undef);
+    }	
+    else {
+	$RT::Logger->warning("$self -> AppliesToObj called for an object ".
+			     "of an unknown scope:" . $self->RightScope);
+	return(undef);
+    }
+}	
+
+# }}}
+
+# {{{ sub PrincipalObj
+
+=head2 PrincipalObj
+
+If the AppliesTo is a group, returns the group object.
+If the AppliesTo is a user, returns the user object.
+Otherwise, it logs a warning and returns undef.
+
+=cut
+
+sub PrincipalObj {
+    my $self = shift;
+    my ($princ_obj);
+
+    if ($self->PrincipalType eq 'Group') {
+	use RT::Group;
+	$princ_obj = new RT::Group($self->CurrentUser);
+    }
+    elsif ($self->PrincipalType eq 'User') {
+	$princ_obj = new RT::User($self->CurrentUser);
+    }
+    else {
+	$RT::Logger->warning("$self -> PrincipalObj called for an object ".
+			     "of an unknown principal type:" . 
+			     $self->PrincipalType ."\n");
+	return(undef);
+    }
+#    $RT::Logger->debug("Loading Principal ".$self->PrincipalId ."\n");
+    $princ_obj->Load($self->PrincipalId);
+    return($princ_obj);
+
+}	
+
+# }}}
+
 # {{{ sub _Set
 
 sub _Set {
@@ -240,13 +445,34 @@ sub _Set {
 }
 
 # }}}
+
+# {{{ sub CurrentUserHasRight
+sub CurrentUserHasRight {
+    my $self = shift;
+    my $right = shift;
+    if ($self->RightScope eq 'System') {
+	return $self->CurrentUser->HasSystemRight($right);
+    }
+    elsif ($self->RightScope eq 'Queue') {
+	return $self->CurrentUser->HasQueueRight( Queue => $self->RightAppliesTo,
+						  Right => $right );
+    }	
+    else {
+	$RT::Logger->warning("$self: Trying to check an acl for a scope we ".
+			     "don't understand:" . $self->RightScope ."\n");
+	return undef;
+    }
+}	
+# }}}
+
+
 1;
 
 __DATA__
 
 # {{{ POD
 
-=head1 RT::ACE
+=head1 Out of date docs
 
 =head2 Table Structure
 
@@ -345,7 +571,7 @@ Modify Queue Attributes for <queue>
 
 Modify Queue ACL for queue <queue>
 
-	Name: ModifyQueueACL
+	Name: ModifyACL
 	Principals: <user> <group>
 
 
@@ -393,7 +619,7 @@ Modify Self
 
 Modify System ACL
 
-	Name: ModifySystemACL		  
+	Name: ModifyACL		  
 	Principals: <user> <group>
 
 =head1 The Principal Side of the ACE
diff --git a/lib/RT/ACL.pm b/lib/RT/ACL.pm
index b53665b42b..7f72b60628 100755
--- a/lib/RT/ACL.pm
+++ b/lib/RT/ACL.pm
@@ -42,7 +42,6 @@ sub Limit  {
 }
 # }}}
 
-
 # {{{ sub NewItem 
 sub NewItem  {
   my $self = shift;
@@ -63,20 +62,49 @@ Deals with collections of RT::ACE objects
 
 =head2 Next
 
-List off the ACL that's been specified
+Hand out the next ACE that was found
+
+=cut
+
+
+# {{{ sub Next 
+sub Next {
+    my $self = shift;
+    
+    my $ACE = $self->SUPER::Next();
+    if ((defined($ACE)) and (ref($ACE))) {
+	
+	if ( $ACE->CurrentUserHasRight('ShowACL') or
+	     $ACE->CurrentUserHasRight('ModifyACL')
+	   ) {
+	    return($ACE);
+	}
+	
+	#If the user doesn't have the right to show this ACE
+	else {	
+	    return($self->Next());
+	}
+    }
+    #if there never was any ACE
+    else {
+	return(undef);
+    }	
+    
+}
+
+# }}}
+
 
 =head1 Limit the ACL to a specific scope
 
-There are three real scopes right now:
+There are two real scopes right now:
 
 =item Queue is for rights that apply to a single queue
 
-=item AllQueues is for rights that apply to all queues
-
 =item System is for rights that apply to the System (rights that aren't queue related)
 
 
-=head2 LimitScopeToQueue
+=head2 LimitToQueue
 
 Takes a single queueid as its argument.
 
@@ -84,51 +112,33 @@ Limit the ACL to just a given queue when supplied with an integer queue id.
 
 =cut
 
-sub LimitScopeToQueue {
-  my $self = shift;
-  my $queue = shift;
-  
-  
-  
-  $self->Limit( FIELD =>'RightScope',
-                ENTRYAGGREGATOR => 'OR',
-		VALUE => 'Queue');
-  $self->Limit( FIELD =>'RightScope',
-                ENTRYAGGREGATOR => 'OR',
+sub LimitToQueue {
+    my $self = shift;
+    my $queue = shift;
+    
+    
+    
+    $self->Limit( FIELD =>'RightScope',
+		  ENTRYAGGREGATOR => 'OR',
+		  VALUE => 'Queue');
+    $self->Limit( FIELD =>'RightScope',
+		  ENTRYAGGREGATOR => 'OR',
 		VALUE => 'Ticket');
+    
+    $self->Limit(ENTRYAGGREGATOR => 'OR',
+		 FIELD => 'RightAppliesTo',
+		 VALUE => $queue );
   
-  $self->Limit(ENTRYAGGREGATOR => 'OR',
-	       FIELD => 'RightAppliesTo',
-	       VALUE => $queue );
-  
-}
-
-=head2 LimitScopeToAllQueues
-
-Takes no arguments
-Limit the ACL to global queue rights. (Rights granted across all queues)
-
-=cut
-
-sub LimitScopeToAllQueues {
-  my $self = shift;
-  
-  $self->Limit( FIELD =>'RightScope',
-		VALUE => 'Queue');
-  
-  $self->Limit(ENTRYAGGREGATOR => 'OR',
-	       FIELD => 'RightAppliesTo',
-	       VALUE => 0 );
 }
 
 
-=head2 LimitScopeToSystem()
+=head2 LimitToSystem()
 
 Limit the ACL to system rights
 
 =cut 
 
-sub LimitScopeToSystem {
+sub LimitToSystem {
   my $self = shift;
   
   $self->Limit( FIELD =>'RightScope',
@@ -198,6 +208,7 @@ sub LimitPrincipalToGroup {
 	       VALUE => $group );
 
 }
+
 =head2 LimitPrincipalToType($type)
 
 Takes a single argument, $type.
diff --git a/lib/RT/Interface/CLI.pm b/lib/RT/Interface/CLI.pm
index 7b1e519b46..1a2b5caeb8 100644
--- a/lib/RT/Interface/CLI.pm
+++ b/lib/RT/Interface/CLI.pm
@@ -7,8 +7,8 @@ use strict;
 
 
 BEGIN {
-    use Exporter   ();
-    our ($VERSION, @ISA, @EXPORT, @EXPORT_OK, %EXPORT_TAGS);
+    use Exporter ();
+    use vars qw ($VERSION  @ISA @EXPORT @EXPORT_OK %EXPORT_TAGS);
     
     # set the version for version checking
     $VERSION = do { my @r = (q$Revision$ =~ /\d+/g); sprintf "%d."."%02d" x $#r, @r }; # must be all one line, for MakeMaker
diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 0362a786aa..7c6da9990a 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -234,7 +234,7 @@ sub ProcessSearchQuery {
 	($args{ARGS}->{'ValueOfResultsPerPage'})) {
 	$session{'tickets'}->Rows($args{ARGS}->{'ValueOfResultsPerPage'});
     }
-    
+
     # }}}
     # {{{ Limit owner
     if ($args{ARGS}->{'ValueOfOwner'} ne '' ) {
@@ -381,9 +381,9 @@ sub ProcessACLChanges {
 	    
 	    my $CurrentACL = new RT::ACL($session{'CurrentUser'});
 	    if ($Scope eq 'Queue') {
-		$CurrentACL->LimitScopeToQueue($AppliesTo);
+		$CurrentACL->LimitToQueue($AppliesTo);
 	    } elsif ($Scope eq 'System') {
-		$CurrentACL->LimitScopeToSystem();
+		$CurrentACL->LimitToSystem();
 	    }
 	    
 	    $CurrentACL->LimitPrincipalToType($PrincipalType);
diff --git a/lib/RT/Scrip.pm b/lib/RT/Scrip.pm
index 681f269e7a..2b46812b11 100755
--- a/lib/RT/Scrip.pm
+++ b/lib/RT/Scrip.pm
@@ -61,23 +61,39 @@ sub Create  {
     
     
     #TODO +++ validate input 
+    #TODO: Allow loading Template, ScripAction and ScripCondition by name
+
+    require RT::ScripAction;
+    my $action = new RT::ScripAction($self->CurrentUser);
+    $action->Load($args{'ScripAction'});
+    return (0, "Action ".$args{'ScripAction'}." not found") unless $action->Id;
+
+    require RT::Template;
+    my $template = new RT::Template($self->CurrentUser);
+    $template->Load($args{'Template'});
+    return (0, 'Template not found') unless $template->Id;
+
+    require RT::ScripCondition;
+    my $condition = new RT::ScripCondition($self->CurrentUser);
+    $condition->Load($args{'ScripCondition'});
+    return (0, 'Condition not found') unless $condition->Id;
+
 
     unless ($self->CurrentUserHasRight('ModifyScrips')) {
-	return (undef);
+	return (0, 'Permission Denied');
     }
     
     my $id = $self->SUPER::Create(Queue => $args{'Queue'},
-				  Template => $args{'Template'},
-				  ScripCondition => $args{'ScripCondition'},
+				  Template => $template->Id,
+				  ScripCondition => $condition->id,
 				  Stage => $args{'Stage'},
-				  ScripAction => $args{'ScripAction'}
+				  ScripAction => $action->Id
 				 );
-    return ($id); 
+    return ($id, 'Scrip Created'); 
 }
 
 # }}}
 
-
 # {{{ sub QueueObj
 
 =head2 QueueObj
@@ -289,9 +305,9 @@ sub HasRight {
                  Principal => undef,
                  @_ );
     
-    if ($self->SUPER::_Value('Queue') > 0) {
+    if ((defined $self->SUPER::_Value('Queue')) and ($self->SUPER::_Value('Queue') != 0)) {
         return ( $args{'Principal'}->HasQueueRight(
-                      Right => $args{'Right'},
+		      Right => $args{'Right'},
                       Queue => $self->SUPER::_Value('Queue'),
                       Principal => $args{'Principal'}
                       ) 
@@ -299,7 +315,7 @@ sub HasRight {
 
     }
     else {
-        return( $args{'Principal'}->HasSystemRight( Right => $args{'Right'}) );
+        return( $args{'Principal'}->HasSystemRight( $args{'Right'}) );
     }
 }
 # }}}
diff --git a/lib/RT/ScripAction.pm b/lib/RT/ScripAction.pm
index ba44c4b356..76f1a39716 100755
--- a/lib/RT/ScripAction.pm
+++ b/lib/RT/ScripAction.pm
@@ -76,24 +76,27 @@ sub Load  {
     my $self = shift;
     my $identifier = shift;
     
-    my $template = shift;
+    
+    
     
     if (!$identifier) {
 	return (undef);
     }	    
     
-  if ($identifier !~ /\D/) {
-      $self->SUPER::LoadById($identifier);
-  }
+    if ($identifier !~ /\D/) {
+	$self->SUPER::LoadById($identifier);
+    }
     else {
-	$RT::Logger->crit("$self -> Load called with a bogus id '$identifier'\n");
-	return(undef);
+	$self->LoadByCol('Name', $identifier);
+	
     }
-    
-    # Set the template Id to the passed in template
+
+    if (@_) {
+	# Set the template Id to the passed in template    
+	my $template = shift;
+	
 	$self->{'Template'} = $template;
-    
-    
+    }
 }
 # }}}
 
diff --git a/lib/RT/ScripCondition.pm b/lib/RT/ScripCondition.pm
index 0b2242b180..9d740927c9 100755
--- a/lib/RT/ScripCondition.pm
+++ b/lib/RT/ScripCondition.pm
@@ -85,8 +85,7 @@ sub Load  {
       $self->SUPER::LoadById($identifier);
   }
     else {
-	$RT::Logger->crit("$self -> Load called with a bogus id '$identifier'\n");
-	return(undef);
+	$self->LoadByCol('Name', $identifier);
     }
 }
 # }}}
diff --git a/lib/RT/Scrips.pm b/lib/RT/Scrips.pm
index 4d795810c2..ba47f19cbf 100755
--- a/lib/RT/Scrips.pm
+++ b/lib/RT/Scrips.pm
@@ -78,6 +78,7 @@ another call to this method or LimitToQueue
 
 =cut
 
+
 sub LimitToGlobal  {
    my $self = shift;
  
diff --git a/lib/RT/Template.pm b/lib/RT/Template.pm
index bd3a38009c..e0ebe59700 100755
--- a/lib/RT/Template.pm
+++ b/lib/RT/Template.pm
@@ -105,6 +105,35 @@ sub _Value  {
 
 # }}}
 
+# {{{ sub Load
+
+=head2 Load <identifer>
+
+Load a template, either by number or by name
+
+=cut
+
+sub Load  {
+    my $self = shift;
+    my $identifier = shift;
+    
+    
+    
+    
+    if (!$identifier) {
+	return (undef);
+    }	    
+    
+    if ($identifier !~ /\D/) {
+	$self->SUPER::LoadById($identifier);
+    }
+    else {
+	$self->LoadByCol('Name', $identifier);
+	
+    }
+}
+# }}}
+
 # {{{ sub Create
 
 =head2 Create
@@ -158,6 +187,7 @@ sub Create {
     return ($result);
 
 }
+
 # }}}
 
 # {{{ sub MIMEObj
@@ -245,7 +275,6 @@ sub QueueObj {
     return ($self->{'queue'});
 }
 
-
 # }}}
 
 # {{{ sub CurrentUserHasQueueRight
diff --git a/lib/RT/Templates.pm b/lib/RT/Templates.pm
index f5717b3fee..5d91d41da2 100755
--- a/lib/RT/Templates.pm
+++ b/lib/RT/Templates.pm
@@ -55,16 +55,16 @@ sub LimitToNotInQueue {
                 );
 }
 # }}}
-# {{{ LimitToSystem
+# {{{ LimitToGlobal
 
-=head2 LimitToSystem
+=head2 LimitToGlobal
 
 Takes no arguments. Limits the returned set to "Global" templates
 which can be used with any queue.
 
 =cut
 
-sub LimitToSystem {
+sub LimitToGlobal {
     my $self = shift;
     my $queue_id = shift;
     $self->Limit(FIELD => 'Queue',
diff --git a/lib/RT/User.pm b/lib/RT/User.pm
index e588c8f196..69d08241ce 100755
--- a/lib/RT/User.pm
+++ b/lib/RT/User.pm
@@ -640,10 +640,7 @@ sub HasQueueRight {
     }
 
     unless ($QueueId) {
-	use Data::Dumper;
-	$RT::Logger->debug( "\n\n\n".Dumper(%args)."\n");
-    	require Carp;
-	$RT::Logger->debug( Carp::cluck() . "$self ->HasQueueRight found no valid queue id.");
+	$RT::Logger->debug( "$self->HasQueueRight Couldn't find a queue id");
     }
 
     #If the user wants to create a ticket, that would make them a requestor
@@ -774,8 +771,6 @@ sub _HasRight {
     	return(undef);
     }
     elsif (!defined $args{'AppliesTo'}) {
-        use Carp;
-        $RT::Logger->debug(Carp::cluck."\n");
     	$RT::Logger->debug("_HasRight called without an AppliesTo object\n");
     	return(undef);
     }
diff --git a/webrt/Admin/Elements/SelectTemplate b/webrt/Admin/Elements/SelectTemplate
index 8d0aa1264b..76550dc4e1 100755
--- a/webrt/Admin/Elements/SelectTemplate
+++ b/webrt/Admin/Elements/SelectTemplate
@@ -25,7 +25,7 @@ $PrimaryTemplates->LimitToQueue($DefaultQueue);
 }
 
 my $OtherTemplates = RT::Templates->new($session{'CurrentUser'});
-$OtherTemplates->LimitToSystem($DefaultQueue);
+$OtherTemplates->LimitToGlobal($DefaultQueue);
 
 </%INIT>
 <%ARGS>
diff --git a/webrt/Admin/Global/Template.html b/webrt/Admin/Global/Template.html
index 477f65aa3c..881a4c8374 100755
--- a/webrt/Admin/Global/Template.html
+++ b/webrt/Admin/Global/Template.html
@@ -1,11 +1,11 @@
 <& /Admin/Elements/Header, title => "Modify template ".$TemplateObj->id&>
-<& /Admin/Elements/QueueTabs, id => $id &>
+<& /Admin/Elements/SystemTabs &>
 <& /Elements/ListActions, actions => @results &>
 
 <& /Elements/TitleBoxStart, title => "Modify Template ".$TemplateObj->id &>
 
 <FORM METHOD=POST ACTION="Template.html">
-<INPUT TYPE=HIDDEN name="id" value="<%$TemplateObj->id%>">
+<INPUT TYPE=HIDDEN name="template" value="<%$TemplateObj->id%>">
 Name: <input name="Name" VALUE="<%$TemplateObj->Name%>" SIZE=20><BR>
 Description <input name="Description" VALUE="<%$TemplateObj->Description%>" SIZE=80><BR>
 
@@ -46,6 +46,5 @@ foreach $field (@fields) {
 
 </%INIT>
 <%ARGS>
-$id => undef
 $template => undef
 </%ARGS>
diff --git a/webrt/Admin/Global/Templates.html b/webrt/Admin/Global/Templates.html
index 95170a1dfa..6556fcc35a 100755
--- a/webrt/Admin/Global/Templates.html
+++ b/webrt/Admin/Global/Templates.html
@@ -8,7 +8,7 @@
 
 %while  (my $TemplateObj = $Templates->Next) { 
 
-<LI><A HREF="Template.html?id=<%$id%>&template=<%$TemplateObj->id()%>"><%$TemplateObj->id()%>/<%$TemplateObj->Name%>: <%$TemplateObj->Description%></a><BR>
+<LI><A HREF="Template.html?template=<%$TemplateObj->id()%>"><%$TemplateObj->id()%>/<%$TemplateObj->Name%>: <%$TemplateObj->Description%></a><BR>
 
 %}
 
@@ -16,7 +16,7 @@
 <%INIT>
 
 my $Templates = RT::Templates->new($session{'CurrentUser'});
-$Templates->LimitToSystem();
+$Templates->LimitToGlobal();
 
 </%INIT>
 <%ARGS>
diff --git a/webrt/Admin/Queues/Template.html b/webrt/Admin/Queues/Template.html
index 0f2b93608a..be0d1f14c5 100755
--- a/webrt/Admin/Queues/Template.html
+++ b/webrt/Admin/Queues/Template.html
@@ -5,7 +5,9 @@
 <& /Elements/TitleBoxStart, title => "Modify Template ".$TemplateObj->id &>
 
 <FORM METHOD=POST ACTION="Template.html">
-<INPUT TYPE=HIDDEN name="id" value="<%$TemplateObj->id%>">
+<INPUT TYPE=HIDDEN name="template" value="<%$TemplateObj->id%>">
+%# hang onto the queue id
+<INPUT TYPE=HIDDEN name="id" value="<%$id%>">
 Name: <input name="Name" VALUE="<%$TemplateObj->Name%>" SIZE=20><BR>
 Description: <input name="Description" VALUE="<%$TemplateObj->Description%>" SIZE=80><BR>