[Rt-commit] rt branch, 4.2/add-reassignticket-right, created. rt-4.1.6-415-g0941adb

Jim Brandt jbrandt at bestpractical.com
Fri Apr 12 13:22:43 EDT 2013


The branch, 4.2/add-reassignticket-right has been created
        at  0941adb7c1d77528bff787e7593698c627c602f9 (commit)

- Log -----------------------------------------------------------------
commit 12c1c7ac81db7a3236a4b0ddb06dd711a66c5c0a
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri Apr 5 13:38:23 2013 -0400

    Add a ReassignTicket right to allow owner changes on owned tickets
    
    Users have frequently requested the ability to change the owner on
    a ticket that already has an owner. Previously RT required you to own
    a ticket to change the owner, so you had to steal a ticket owned by
    someone else, then set the owner. The new ReassignTicket right allows
    users to change owner directly (reassign tickets).

diff --git a/lib/RT/Queue.pm b/lib/RT/Queue.pm
index 86d05c9..e89e015 100644
--- a/lib/RT/Queue.pm
+++ b/lib/RT/Queue.pm
@@ -135,6 +135,7 @@ our $RIGHTS = {
     DeleteTicket        => 'Delete tickets',                                            # loc_pair
     TakeTicket          => 'Take tickets',                                              # loc_pair
     StealTicket         => 'Steal tickets',                                             # loc_pair
+    ReassignTicket      => 'Modify ticket owner on owned tickets',                      # loc_pair
 
     ForwardMessage      => 'Forward messages outside of RT',                            # loc_pair
 };
@@ -165,6 +166,7 @@ our $RIGHT_CATEGORIES = {
     DeleteTicket        => 'Staff',
     TakeTicket          => 'Staff',
     StealTicket         => 'Staff',
+    ReassignTicket      => 'Staff',
     ForwardMessage      => 'Staff',
 };
 
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index b5eff48..586acd4 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -2097,6 +2097,10 @@ sub SetOwner {
 
 sub _CurrentUserHasRightToSetOwner {
     my $self = shift;
+
+    # ReassignTicket unconditionally allows you to SetOwner
+    return 1 if $self->CurrentUserHasRight('ReassignTicket');
+
     # must have ModifyTicket rights
     # or TakeTicket/StealTicket and $NewOwner is self
     # see if it's a take
@@ -2133,10 +2137,11 @@ sub _IsProposedOwnerChangeValid {
     my $OldOwnerObj = $self->OwnerObj;
 
     # If we're not stealing and the ticket has an owner and it's not
-    # the current user
+    # the current user, and the current user can't reassign tickets
     if (     $Type ne 'Steal' and $Type ne 'Force'
          and $OldOwnerObj->Id != RT->Nobody->Id
-         and $OldOwnerObj->Id != $self->CurrentUser->Id ) {
+         and $OldOwnerObj->Id != $self->CurrentUser->Id
+         and not $self->CurrentUserHasRight('ReassignTicket') ) {
         if ( $NewOwnerObj->id == $self->CurrentUser->id) {
             return ( 0, $self->loc("You can only take tickets that are unowned") )
         }
diff --git a/t/api/ticket.t b/t/api/ticket.t
index 57e6183..4db8d7e 100644
--- a/t/api/ticket.t
+++ b/t/api/ticket.t
@@ -205,20 +205,23 @@ is ($t1->Requestors->MembersObj->Count, 2);
 
 }
 
+diag "Test owner changes";
 {
 
 my $root = RT::User->new(RT->SystemUser);
 $root->Load('root');
 ok ($root->Id, "Loaded the root user");
 my $t = RT::Ticket->new(RT->SystemUser);
-$t->Load(1);
+my ($val, $msg) = $t->Create( Subject => 'Owner test 1', Queue => 'General');
+ok( $t->Id, "Created a new ticket with id $val: $msg");
+
 $t->SetOwner('root');
 is ($t->OwnerObj->Name, 'root' , "Root owns the ticket");
 $t->Steal();
 is ($t->OwnerObj->id, RT->SystemUser->id , "SystemUser owns the ticket");
 my $txns = RT::Transactions->new(RT->SystemUser);
 $txns->OrderBy(FIELD => 'id', ORDER => 'DESC');
-$txns->Limit(FIELD => 'ObjectId', VALUE => '1');
+$txns->Limit(FIELD => 'ObjectId', VALUE => $t->Id);
 $txns->Limit(FIELD => 'ObjectType', VALUE => 'RT::Ticket');
 $txns->Limit(FIELD => 'Type', OPERATOR => '!=',  VALUE => 'EmailRecord');
 
@@ -226,6 +229,37 @@ my $steal  = $txns->First;
 is($steal->OldValue , $root->Id , "Stolen from root");
 is($steal->NewValue , RT->SystemUser->Id , "Stolen by the systemuser");
 
+ok(my $user1 = RT::User->new(RT->SystemUser), "Creating a user1 rt::user");
+($val, $msg) = $user1->Create(Name => 'User1', EmailAddress => 'user1 at example.com');
+ok( $val, "Created new user with id: $val");
+ok( $user1->Id,  "Found the user1 rt user");
+
+my $t1 = RT::Ticket->new($user1);
+($val, $msg) = $t1->Load($t->Id);
+ok( $t1->Id, "Loaded ticket with id $val");
+
+($val, $msg) = $t1->SetOwner('root');
+ok( !$val, "user1 can't set owner to root: $msg");
+is ($t->OwnerObj->id, RT->SystemUser->id , "SystemUser still owns ticket " . $t1->Id);
+
+my $queue = RT::Queue->new(RT->SystemUser);
+$queue->Load("General");
+
+($val, $msg) = $user1->PrincipalObj->GrantRight(
+         Object => $queue, Right => 'ModifyTicket'
+     );
+
+($val, $msg) = $t1->SetOwner('root');
+ok( !$val, "With ModifyTicket user1 can't set owner to root: $msg");
+is ($t->OwnerObj->id, RT->SystemUser->id , "SystemUser still owns ticket " . $t1->Id);
+
+($val, $msg) = $user1->PrincipalObj->GrantRight(
+         Object => $queue, Right => 'ReassignTicket'
+     );
+
+($val, $msg) = $t1->SetOwner('root');
+ok( $val, "With ReassignTicket user1 reassigned ticket " . $t1->Id . " to root: $msg");
+is ($t1->OwnerObj->Name, 'root' , "Root owns ticket " . $t1->Id);
 
 }
 

commit 5f29c1b2257d8e3e1da5a4789e111a9347e7fe51
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri Apr 12 11:32:58 2013 -0400

    Change _CurrentUserHasRightToSetOwner to public CurrentUserCanSetOwner
    
    Make public and rename to CurrentUserCanSetOwner to make it
    consistent with other CurrentUserCan method names.

diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 586acd4..f34b0be 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -2066,7 +2066,7 @@ sub SetOwner {
         return ( 0, $self->loc("That user does not exist") );
     }
 
-    if ( !$self->_CurrentUserHasRightToSetOwner ) {
+    if ( !$self->CurrentUserCanSetOwner ) {
         $RT::Handle->Rollback();
         return ( 0, $self->loc("Permission Denied") );
     }
@@ -2095,7 +2095,36 @@ sub SetOwner {
     return ( $val, $msg );
 }
 
-sub _CurrentUserHasRightToSetOwner {
+=head2 CurrentUserCanSetOwner
+
+Check if the current user can set the owner of the current ticket.
+Owner change can be granted through a few different RT rights:
+
+=over
+
+=item *
+
+ReassignTicket unconditionally grants the right to set owner.
+
+=item *
+
+If you own the ticket and have ModifyTicket, you can set the owner.
+
+=item *
+
+If the ticket is not owned (owned by Nobody) and you have ModifyTicket
+and TakeTicket, you can Take the ticket (set owner to yourself).
+
+=item *
+
+If the ticket is owned by someone else and you have ModifyTicket
+and StealTicket, you can Steal the ticket (set owner to yourself).
+
+=back
+
+=cut
+
+sub CurrentUserCanSetOwner {
     my $self = shift;
 
     # ReassignTicket unconditionally allows you to SetOwner

commit 0941adb7c1d77528bff787e7593698c627c602f9
Author: Jim Brandt <jbrandt at bestpractical.com>
Date:   Fri Apr 12 13:15:47 2013 -0400

    Replace individual rights checks with CurrentUserCanSetOwner
    
    CurrentUserCanSetOwner in RT::Ticket handles the various cases
    for changing owner based on Stealing and Taking tickets.
    With the new ReassignTicket right, it's no longer the case that
    OwnTicket is needed to have the rights to assign tickets to
    others.

diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index a2f7743..c9d9e98 100644
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -604,7 +604,7 @@ my $build_main_nav = sub {
                 $tabs->child( history => title => loc('History'), path => "/Ticket/History.html?id=" . $id );
 
                 my %can = %{ $obj->CurrentUser->PrincipalObj->HasRights( Object => $obj ) };
-                $can{'_ModifyOwner'} = $can{'OwnTicket'} || $can{'TakeTicket'} || $can{'StealTicket'};
+                $can{'_ModifyOwner'} = $obj->CurrentUserCanSetOwner();
                 my $can = sub {
                     unless ($_[0] eq 'ExecuteCode') {
                         return $can{$_[0]} || $can{'SuperUser'};
diff --git a/share/html/Ticket/Elements/ShowSummary b/share/html/Ticket/Elements/ShowSummary
index 98dde90..b7981e3 100644
--- a/share/html/Ticket/Elements/ShowSummary
+++ b/share/html/Ticket/Elements/ShowSummary
@@ -104,7 +104,5 @@ $Attachments => undef
 <%INIT>
 my $can_modify = $Ticket->CurrentUserHasRight('ModifyTicket');
 my $can_modify_cf = $Ticket->CurrentUserHasRight('ModifyCustomField');
-my $can_modify_owner = $Ticket->CurrentUserHasRight('OwnTicket')
-                    || $Ticket->CurrentUserHasRight('TakeTicket')
-                    || $Ticket->CurrentUserHasRight('StealTicket');
+my $can_modify_owner = $Ticket->CurrentUserCanSetOwner();
 </%INIT>

-----------------------------------------------------------------------


More information about the Rt-commit mailing list