[Rt-commit] rt branch, 4.0/display-actions-on-abort, created. rt-4.0.6-254-g02eecea

Alex Vandiver alexmv at bestpractical.com
Tue Aug 7 20:55:38 EDT 2012


The branch, 4.0/display-actions-on-abort has been created
        at  02eecea7167172eb283b63ecb54da979fd1be7b2 (commit)

- Log -----------------------------------------------------------------
commit e0e9e5e910f042a151463f67f9896022496893d0
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Aug 7 18:48:36 2012 -0400

    Allow the passing of actions through to Abort
    
    When a change causes a ticket to no longer be viewable to the user, the
    results of the change are now at least displayed to the user.

diff --git a/share/html/Elements/Error b/share/html/Elements/Error
index 87dfd02..04998c7 100755
--- a/share/html/Elements/Error
+++ b/share/html/Elements/Error
@@ -52,6 +52,8 @@
 <& /Elements/Tabs &>
 % }
 
+<& /Elements/ListActions, actions => $Actions &>
+
 <div class="error">
 <%$Why%>
 <br />
@@ -64,6 +66,7 @@ $m->abort();
 </%cleanup>
 
 <%args>
+$Actions => []
 $Code => undef
 $Details => ''
 $Title => loc("RT Error")
diff --git a/share/html/Ticket/Display.html b/share/html/Ticket/Display.html
index 5e84a50..0b86e0f 100755
--- a/share/html/Ticket/Display.html
+++ b/share/html/Ticket/Display.html
@@ -177,10 +177,12 @@ if ($ARGS{'id'} eq 'new') {
         push @Actions, ProcessObjectCustomFieldUpdates(ARGSRef => \%ARGS, TicketObj => $TicketObj );
         push @Actions, ProcessTicketReminders( ARGSRef => \%ARGS, TicketObj => $TicketObj );
 
-        # XXX: we shouldn't block actions here if user has no right to see the ticket,
-        # but we should allow him to see actions he has done
         unless ($TicketObj->CurrentUserHasRight('ShowTicket')) {
-            Abort("No permission to view ticket");
+            if (@Actions) {
+                Abort("A change was applied successfully, but you no longer have permissions to view the ticket", Actions => \@Actions);
+            } else {
+                Abort("No permission to view ticket");
+            }
         }
         if ( $ARGS{'MarkAsSeen'} ) {
             $TicketObj->SetAttribute(
diff --git a/share/html/Ticket/Modify.html b/share/html/Ticket/Modify.html
index d779b12..752b99d 100755
--- a/share/html/Ticket/Modify.html
+++ b/share/html/Ticket/Modify.html
@@ -83,10 +83,13 @@ push @results, ProcessObjectCustomFieldUpdates(Object => $TicketObj, ARGSRef =>
 
 $TicketObj->ApplyTransactionBatch;
 
-# TODO: display the results, even if we can't display the ticket
 unless ($TicketObj->CurrentUserHasRight('ShowTicket')) {
-    Abort("No permission to view ticket");
-} 
+    if (@results) {
+        Abort("A change was applied successfully, but you no longer have permissions to view the ticket", Actions => \@results);
+    } else {
+        Abort("No permission to view ticket");
+    }
+}
 
 </%INIT>
 <%ARGS>
diff --git a/share/html/Ticket/ModifyAll.html b/share/html/Ticket/ModifyAll.html
index 8619cd5..a645c81 100755
--- a/share/html/Ticket/ModifyAll.html
+++ b/share/html/Ticket/ModifyAll.html
@@ -215,10 +215,12 @@ unless ($OnlySearchForPeople or $OnlySearchForGroup or $ARGS{'AddMoreAttach'} )
 $Ticket->ApplyTransactionBatch;
 
 # If they've gone and moved the ticket to somewhere they can't see, etc...
-# TODO: display the results, even if we can't display the ticket.
-
 unless ($Ticket->CurrentUserHasRight('ShowTicket')) {
-   Abort("No permission to view ticket");
+    if (@results) {
+        Abort("A change was applied successfully, but you no longer have permissions to view the ticket", Actions => \@results);
+    } else {
+        Abort("No permission to view ticket");
+    }
 }
 
 
diff --git a/share/html/m/ticket/show b/share/html/m/ticket/show
index f6ffe88..2b41fd8 100644
--- a/share/html/m/ticket/show
+++ b/share/html/m/ticket/show
@@ -112,10 +112,12 @@ if ($ARGS{'id'} eq 'new') {
     push @Actions, ProcessObjectCustomFieldUpdates(ARGSRef => \%ARGS, TicketObj => $Ticket );
     push @Actions, ProcessTicketReminders( ARGSRef => \%ARGS, TicketObj => $Ticket );
 
-    # XXX: we shouldn't block actions here if user has no right to see the ticket,
-    # but we should allow him to see actions he has done
     unless ($Ticket->CurrentUserHasRight('ShowTicket')) {
-        Abort("No permission to view ticket");
+        if (@Actions) {
+            Abort("A change was applied successfully, but you no longer have permissions to view the ticket", Actions => \@Actions);
+        } else {
+            Abort("No permission to view ticket");
+        }
     }
     if ( $ARGS{'MarkAsSeen'} ) {
         $Ticket->SetAttribute(

commit 02eecea7167172eb283b63ecb54da979fd1be7b2
Author: Alex Vandiver <alexmv at bestpractical.com>
Date:   Tue Aug 7 20:15:27 2012 -0400

    Descriptions of just-created transactions should always be readable
    
    If a user had permissions to execute a change, they should always have
    permission to read the description of the transaction that is returned.
    This case is triggered when a user changes a ticket's queue to one they
    do not have rights in, for example; without it, the UI merely displays
    the unhelpful status change "Permission denied" in @Actions, despite an
    action clearly having happened.
    
    While the simplest method for ensuring that the transaction can be read
    would be to load it as the system user, this causes the returned message
    to be in the system user's localization, not the current user's.  Add an
    explicit flag which allows ->CurrentUserCanSee to be overridden for this
    one object.

diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 91711e4..cf2d667 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -3491,6 +3491,9 @@ sub _Set {
                                                OldValue  => $Old,
                                                TimeTaken => $args{'TimeTaken'},
         );
+        # Ensure that we can read the transaction, even if the change
+        # just made the ticket unreadable to us
+        $TransObj->{_readable} = 1;
         return ( $Trans, scalar $TransObj->BriefDescription );
     }
     else {
diff --git a/lib/RT/Transaction.pm b/lib/RT/Transaction.pm
index 5b3641f..9c8f1bd 100644
--- a/lib/RT/Transaction.pm
+++ b/lib/RT/Transaction.pm
@@ -1066,6 +1066,8 @@ sub CurrentUserCanSee {
         $cf->SetContextObject( $self->Object );
         $cf->Load( $cf_id );
         return 0 unless $cf->CurrentUserHasRight('SeeCustomField');
+    } elsif ($self->{ _readable }) {
+        return 1;
     }
     # Defer to the object in question
     return $self->Object->CurrentUserCanSee("Transaction");

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


More information about the Rt-commit mailing list