[Rt-commit] rt branch, 4.2/fix-take-and-steal-links-display-logic, created. rt-4.2.1-69-g206a159

[? sunnavy]

The branch, 4.2/fix-take-and-steal-links-display-logic has been created
        at  206a15950661ebcb497401ab5ec8d2082e9494e7 (commit)

- Log -----------------------------------------------------------------
commit 206a15950661ebcb497401ab5ec8d2082e9494e7
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Mon Dec 30 14:50:06 2013 +0800

    fix the logic of the display of Take/Steal links
    
    as long as user has ReassignTicket, RT::Ticket::CurrentUserCanSetOwner will
    return true for both "Take" and "Steal" types of owner set, so it's not enough.
    
    see also #28211

diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index 555edd2..03d24ea 100644
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -684,13 +684,23 @@ my $build_main_nav = sub {
                     $actions->child( $key => title => loc( $key ), path => $url);
                 }
 
-                my ($can_take, $tmsg) = $obj->CurrentUserCanSetOwner( Type => 'Take' );
-                my ($can_steal, $smsg) = $obj->CurrentUserCanSetOwner( Type => 'Steal' );
-                if ( $can_take ){
-                    $actions->child( take => title => loc('Take'), path => "/Ticket/Display.html?Action=Take;id=" . $id );
-                }
-                elsif ( $can_steal ){
-                    $actions->child( steal => title => loc('Steal'), path => "/Ticket/Display.html?Action=Steal;id=" . $id );
+                if ( $obj->CurrentUserHasRight('OwnTicket') ) {
+                    my ($can_take, $tmsg) = $obj->CurrentUserCanSetOwner( Type => 'Take' );
+                    if ( $can_take && $obj->OwnerObj->id != RT->Nobody->id ) {
+                        $can_take = 0;
+                    }
+
+                    my ($can_steal, $smsg) = $obj->CurrentUserCanSetOwner( Type => 'Steal' );
+                    if ( $can_steal && ( $obj->OwnerObj->id == RT->Nobody->id || $obj->OwnerObj->id == $session{CurrentUser}->id )) {
+                        $can_steal = 0;
+                    }
+
+                    if ( $can_take ){
+                        $actions->child( take => title => loc('Take'), path => "/Ticket/Display.html?Action=Take;id=" . $id );
+                    }
+                    elsif ( $can_steal ){
+                        $actions->child( steal => title => loc('Steal'), path => "/Ticket/Display.html?Action=Steal;id=" . $id );
+                    }
                 }
 
                 # TODO needs a "Can extract article into a class applied to this queue" check
diff --git a/t/web/ticket_owner.t b/t/web/ticket_owner.t
index 37716ea..782e68f 100644
--- a/t/web/ticket_owner.t
+++ b/t/web/ticket_owner.t
@@ -440,6 +440,10 @@ diag "user can assign ticket to new owner with ReassignTicket right";
     ok $id, 'created a ticket #' . $id or diag "error: $msg";
     is $ticket->Owner, RT->Nobody->id, 'correct owner';
 
+    $agent_c->goto_ticket($id);
+    ok !($agent_c->find_all_links( text => 'Take' ))[0], 'no Take link';
+    ok !($agent_c->find_all_links( text => 'Steal' ))[0], 'no Steal link';
+
     $agent_a->goto_ticket($id);
     $agent_a->content_lacks('Taken', 'no Taken');
     $agent_a->follow_link_ok( { text => 'Basics' }, 'Ticket -> Basics' );
@@ -454,6 +458,8 @@ diag "user can assign ticket to new owner with ReassignTicket right";
          qr{user_a\s*-\s*Taken}, 'got user_a Taken message' );
 
     $agent_c->goto_ticket($id);
+    ok !($agent_c->find_all_links( text => 'Take' ))[0], 'no Take link';
+    ok !($agent_c->find_all_links( text => 'Steal' ))[0], 'no Steal link';
     $agent_c->follow_link_ok( { text => 'Basics' }, 'Ticket -> Basics' );
     my $form = $agent_c->form_name('TicketModify');
     is $form->value('Owner'), $user_a->id, 'correct owner selected';
@@ -466,7 +472,49 @@ diag "user can assign ticket to new owner with ReassignTicket right";
         'got set message in Basics' );
     $agent_c->goto_ticket($id);
     $agent_c->content_like( qr{Owner forcibly changed}, 'got owner forcibly changed message' );
+    ok !($agent_c->find_all_links( text => 'Take' ))[0], 'no Take link';
+}
+
+ok(
+    RT::Test->add_rights(
+        { Principal => $user_c, Right => [qw(OwnTicket)] },
+    ),
+    'add rights'
+);
+diag "user can take/steal ticket with ReassignTicket+OwnTicket right";
+{
+    my $ticket = RT::Ticket->new($user_a);
+    my ( $id, $txn, $msg ) = $ticket->Create(
+        Queue   => $queue->id,
+        Subject => 'test',
+    );
+    ok $id, 'created a ticket #' . $id or diag "error: $msg";
+    is $ticket->Owner, RT->Nobody->id, 'correct owner';
 
+    $agent_c->goto_ticket($id);
+    ok( ($agent_c->find_all_links( text => 'Take' ))[0], 'has Take link' );
+    ok !($agent_c->find_all_links( text => 'Steal' ))[0], 'no Steal link';
+
+    $agent_a->goto_ticket($id);
+    $agent_a->content_lacks('Taken', 'no Taken');
+    $agent_a->follow_link_ok( { text => 'Basics' }, 'Ticket -> Basics' );
+    $agent_a->submit_form(
+        form_name => 'TicketModify',
+        fields    => { Owner => $user_a->id },
+    );
+    $agent_a->content_contains( 'Owner changed from Nobody to user_a',
+        'got set message in Basics' );
+    $agent_a->goto_ticket($id);
+    like($agent_a->dom->at('.transaction.people .description')->all_text,
+         qr{user_a\s*-\s*Taken}, 'got user_a Taken message' );
+
+    $agent_c->goto_ticket($id);
+    ok !($agent_c->find_all_links( text => 'Take' ))[0], 'no Take link';
+    ok( ($agent_c->find_all_links( text => 'Steal' ))[0], 'has Steal link' );
+    $agent_c->follow_link_ok( { text => 'Steal' }, 'Ticket -> Steal' );
+    $agent_c->content_contains( 'Owner changed from user_a to user_c', 'steal message' );
+    ok !($agent_c->find_all_links( text => 'Take' ))[0], 'no Take link';
+    ok !($agent_c->find_all_links( text => 'Steal' ))[0], 'no Steal link';
 }
 
 

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