[Rt-commit] rt branch, 5.0/hide-pencil-button-if-no-rights, created. rt-5.0.0alpha1-11-g935e5c01e

[Michel Rodriguez]

The branch, 5.0/hide-pencil-button-if-no-rights has been created
        at  935e5c01ec3678fd16032b2b48dc70e36967bda0 (commit)

- Log -----------------------------------------------------------------
commit 05b82dbe42eb67dd98461086401ad9463b9bf7ec
Author: michel <michel at bestpractical.com>
Date:   Fri Feb 28 09:13:26 2020 +0100

    Hide the pencil icon if the user doesn't have the rights.
    
    On the ticket display page.
    
    I restructured the variables that determine whether each portlet should
    show the icon and link the title to the edit form so the 2 features
    can share them without duplicating the tests on the conditions.

diff --git a/share/html/Ticket/Elements/ShowSummary b/share/html/Ticket/Elements/ShowSummary
index 0c5ffdab9..bc1861ac3 100644
--- a/share/html/Ticket/Elements/ShowSummary
+++ b/share/html/Ticket/Elements/ShowSummary
@@ -58,12 +58,12 @@ my $modify_inline
     . '<a class="inline-edit-toggle cancel hidden" href="#">'
     . qq{<span class="fas fa-times icon-bordered fa-2x" alt="$cancel_label" data-toggle="tooltip" data-placement="top" data-original-title="$cancel_label"></span>}
     . '</a>';
-my $modify_basics = sprintf( $modify_inline, $m->interp->apply_escapes( $modify_url, 'h' ) );
+my $modify_basics = $can_modify ? sprintf( $modify_inline, $m->interp->apply_escapes( $modify_url, 'h' ) ) : '';
 my $modify_behavior = $InlineEdit ? ($inline_edit_behavior{Basics} || $inline_edit_behavior{_default} || 'link') : 'hide';
 </%PERL>
 
     <&| /Widgets/TitleBox, title => loc('The Basics'),
-        (($can_modify || $can_modify_cf) ? (title_href => $modify_url) : ()),
+        ($can_modify_basics ? (title_href => $modify_url) : ()),
         ($modify_behavior =~ /^(link|click)$/ ? (titleright_raw => $modify_basics) : ()),
         class => (join " ", 'ticket-info-basics', ($modify_behavior eq 'always' ? 'editing' : ())),
         data => { 'inline-edit-behavior' => $modify_behavior },
@@ -89,17 +89,17 @@ my $modify_behavior = $InlineEdit ? ($inline_edit_behavior{Basics} || $inline_ed
 % $m->callback( %ARGS, CallbackName => 'AfterBasics' );
     <& /Elements/ShowCustomFieldCustomGroupings,
         Object       => $Ticket,
-        title_href   => ($can_modify || $can_modify_cf) ? RT->Config->Get('WebPath')."/Ticket/Modify.html" : "",
-        InlineEdit   => $InlineEdit,
+        title_href   => $can_modify_basics ? RT->Config->Get('WebPath')."/Ticket/Modify.html" : "",
+        InlineEdit   => ( $can_modify_cf && $InlineEdit ),
         &>
 
 <%PERL>
 my $people_url = RT->Config->Get('WebPath')."/Ticket/ModifyPeople.html?id=".$Ticket->Id;
-my $people_inline = sprintf( $modify_inline, $m->interp->apply_escapes( $people_url, 'h' ) );
+my $people_inline = $can_modify_people ? sprintf( $modify_inline, $m->interp->apply_escapes( $people_url, 'h' ) ) : '';
 my $people_behavior = $InlineEdit ? ($inline_edit_behavior{People} || $inline_edit_behavior{_default} || 'link') : 'hide';
 </%PERL>
     <&| /Widgets/TitleBox, title => loc('People'),
-        (($can_modify || $can_modify_owner || $can_modify_people) ? (title_href => RT->Config->Get('WebPath')."/Ticket/ModifyPeople.html?id=".$Ticket->Id) : ()),
+        ($can_modify_people ? (title_href => RT->Config->Get('WebPath')."/Ticket/ModifyPeople.html?id=".$Ticket->Id) : ()),
         class => (join " ", 'ticket-info-people', ($people_behavior eq 'always' ? 'editing' : ())),
         ($people_behavior =~ /^(link|click)$/ ? (titleright_raw => $people_inline) : ()),
         data => { 'inline-edit-behavior' => $people_behavior },
@@ -144,7 +144,7 @@ my $people_behavior = $InlineEdit ? ($inline_edit_behavior{People} || $inline_ed
 
 <%PERL>
 my $dates_url = RT->Config->Get('WebPath')."/Ticket/ModifyDates.html?id=".$Ticket->Id;
-my $dates_inline = sprintf( $modify_inline, $m->interp->apply_escapes( $dates_url, 'h' ) );
+my $dates_inline = $can_modify ? sprintf( $modify_inline, $m->interp->apply_escapes( $dates_url, 'h' ) ) : '';
 my $dates_behavior = $InlineEdit ? ($inline_edit_behavior{Dates} || $inline_edit_behavior{_default} || 'link') : 'hide';
 </%PERL>
     <&| /Widgets/TitleBox, title => loc("Dates"),
@@ -174,7 +174,7 @@ my $dates_behavior = $InlineEdit ? ($inline_edit_behavior{Dates} || $inline_edit
 <& /Ticket/Elements/ShowAssets, Ticket => $Ticket &>
 <%PERL>
 my $links_url = RT->Config->Get('WebPath')."/Ticket/ModifyLinks.html?id=".$Ticket->Id;
-my $links_inline = sprintf( $modify_inline, $m->interp->apply_escapes( $links_url, 'h' ) );
+my $links_inline = $can_modify ? sprintf( $modify_inline, $m->interp->apply_escapes( $links_url, 'h' ) ) : '';
 my $links_behavior = $InlineEdit ? ($inline_edit_behavior{Links} || $inline_edit_behavior{_default} || 'link') : 'hide';
 my $alt = loc('Graph ticket links');
 my $links_graph = '<a href="' . RT->Config->Get('WebPath') . '/Ticket/Graphs/index.html?id=' . $Ticket->id . '"><span class="fas fa-project-diagram icon-bordered fa-2x" alt="' . $alt . '" data-toggle="tooltip" data-placement="top" data-original-title="' . $alt . '"></span></a>';
@@ -226,8 +226,9 @@ $InlineEdit => 0
 <%INIT>
 my $can_modify = $Ticket->CurrentUserHasRight('ModifyTicket');
 my $can_modify_cf = $Ticket->CurrentUserHasRight('ModifyCustomField');
-my $can_modify_owner = $Ticket->CurrentUserCanSetOwner();
-my $can_modify_people = $Ticket->CurrentUserHasRight('Watch')
+my $can_modify_basics = $can_modify || $can_modify_cf;
+my $can_modify_people = $Ticket->CurrentUserCanSetOwner();
+                     || $Ticket->CurrentUserHasRight('Watch')
                      || $Ticket->CurrentUserHasRight('WatchAsAdminCc');
 
 my $edit_label = $m->interp->apply_escapes( loc("Edit"), 'h' );

commit 935e5c01ec3678fd16032b2b48dc70e36967bda0
Author: michel <michel at bestpractical.com>
Date:   Fri Feb 28 09:17:20 2020 +0100

    Fix a bug in determining whether the user can edit the people info.
    
    $Ticket->CurrentUserCanSetOwner() returns ($result, $message), so
    result needs to be captured separatly. The method cannot be used
    as a boolean in tests.

diff --git a/share/html/Ticket/Elements/ShowSummary b/share/html/Ticket/Elements/ShowSummary
index bc1861ac3..c74c33df8 100644
--- a/share/html/Ticket/Elements/ShowSummary
+++ b/share/html/Ticket/Elements/ShowSummary
@@ -227,7 +227,11 @@ $InlineEdit => 0
 my $can_modify = $Ticket->CurrentUserHasRight('ModifyTicket');
 my $can_modify_cf = $Ticket->CurrentUserHasRight('ModifyCustomField');
 my $can_modify_basics = $can_modify || $can_modify_cf;
-my $can_modify_people = $Ticket->CurrentUserCanSetOwner();
+
+# contrary to other methods, CurrentUserCanSetOwner does not return a boolen
+my ( $can_set_owner, $msg ) = $Ticket->CurrentUserCanSetOwner();
+
+my $can_modify_people = $can_set_owner
                      || $Ticket->CurrentUserHasRight('Watch')
                      || $Ticket->CurrentUserHasRight('WatchAsAdminCc');
 

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