[Rt-commit] rt branch, master, updated. rt-4.0.0rc4-77-g9adb5f8

? sunnavy sunnavy at bestpractical.com
Thu Feb 3 07:51:02 EST 2011


The branch, master has been updated
       via  9adb5f8242a004f01cb26c6155443b696dbb841e (commit)
       via  9f3bef51768e288d729972fa76c3b0c1f2a4688d (commit)
       via  648ab58b6a96443bc743d8aa980be32e138e094f (commit)
       via  c22b2f12fe0797ec0eacc7dbf29add53d6ac5f93 (commit)
       via  78dacb9fdda9505012cbab9df90ffbc49285f82a (commit)
      from  b8b8a8813b86673501bd1b2c18b10b181f09f35c (commit)

Summary of changes:
 lib/RT/Interface/Web.pm                |    9 ++++-
 lib/RT/Reminders.pm                    |    3 +-
 share/html/Elements/Tabs               |    2 +-
 share/html/Ticket/Elements/Reminders   |    9 ++++-
 share/html/Ticket/Elements/ShowSummary |    2 +
 share/html/Ticket/Reminders.html       |    6 +++-
 t/web/reminders-permissions.t          |   59 ++++++++++++++++++++++++++++++++
 7 files changed, 84 insertions(+), 6 deletions(-)
 create mode 100644 t/web/reminders-permissions.t

- Log -----------------------------------------------------------------
commit 78dacb9fdda9505012cbab9df90ffbc49285f82a
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu Feb 3 19:37:55 2011 +0800

    apparently we messed up return values of Ticket->Create

diff --git a/lib/RT/Reminders.pm b/lib/RT/Reminders.pm
index c80fe83..58e19c9 100644
--- a/lib/RT/Reminders.pm
+++ b/lib/RT/Reminders.pm
@@ -117,7 +117,8 @@ sub Add {
     );
 
     my $reminder = RT::Ticket->new($self->CurrentUser);
-    my ( $status, $msg ) = $reminder->Create(
+    # the 2nd return value is txn id, which is useless here
+    my ( $status, undef, $msg ) = $reminder->Create(
         Subject => $args{'Subject'},
         Owner => $args{'Owner'},
         Due => $args{'Due'},

commit c22b2f12fe0797ec0eacc7dbf29add53d6ac5f93
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu Feb 3 19:39:03 2011 +0800

    show reminder added msg only if it was added successfully

diff --git a/lib/RT/Interface/Web.pm b/lib/RT/Interface/Web.pm
index 7b3558c..806d27d 100644
--- a/lib/RT/Interface/Web.pm
+++ b/lib/RT/Interface/Web.pm
@@ -1990,12 +1990,17 @@ sub ProcessTicketReminders {
           Format => 'unknown',
           Value => $args->{'NewReminder-Due'}
         );
-        my ( $add_id, $msg, $txnid ) = $Ticket->Reminders->Add(
+        my ( $status, $msg ) = $Ticket->Reminders->Add(
             Subject => $args->{'NewReminder-Subject'},
             Owner   => $args->{'NewReminder-Owner'},
             Due     => $due_obj->ISO
         );
-        push @results, loc("Reminder '[_1]' added", $args->{'NewReminder-Subject'});
+        if ( $status ) {
+            push @results, loc("Reminder '[_1]' added", $args->{'NewReminder-Subject'});
+        }
+        else {
+            push @results, $msg;
+        }
     }
     return @results;
 }

commit 648ab58b6a96443bc743d8aa980be32e138e094f
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu Feb 3 19:40:40 2011 +0800

    it's good to show users the action results

diff --git a/share/html/Ticket/Reminders.html b/share/html/Ticket/Reminders.html
index 4508f5a..639e15f 100755
--- a/share/html/Ticket/Reminders.html
+++ b/share/html/Ticket/Reminders.html
@@ -49,6 +49,8 @@
 <& /Elements/Tabs &>
     
 % $m->callback(CallbackName => 'BeforeActionList', ARGSRef => \%ARGS, Ticket => $Ticket);
+
+<& /Elements/ListActions, actions => \@actions &>
     
 <form action="<%RT->Config->Get('WebPath')%>/Ticket/Reminders.html" name="UpdateReminders" id="UpdateReminders" method="post">
 <&|/Widgets/TitleBox, title => loc("Reminders"),
@@ -65,7 +67,7 @@
 <%INIT>
 my $Ticket = LoadTicket($id);
 
-ProcessTicketReminders( TicketObj => $Ticket, ARGSRef => \%ARGS );
+my @actions = ProcessTicketReminders( TicketObj => $Ticket, ARGSRef => \%ARGS );
 </%INIT>
 <%ARGS>
 $id => undef

commit 9f3bef51768e288d729972fa76c3b0c1f2a4688d
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu Feb 3 19:41:32 2011 +0800

    don't show reminders in ticket page unless the user has ModifyTicket right

diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index e85b059..ebe221d 100755
--- a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -473,7 +473,7 @@ my $build_admin_menu = sub {
             $tabs->child( jumbo => title => loc('Jumbo'), path => "/Ticket/ModifyAll.html?id=" . $id, );
             #}
 
-            if ( RT->Config->Get('EnableReminders') ) {
+            if ( RT->Config->Get('EnableReminders') && $can->('ModifyTicket') ) {
                 $tabs->child( reminders => title => loc('Reminders'), path => "/Ticket/Reminders.html?id=" . $id, );
             }
 
diff --git a/share/html/Ticket/Elements/Reminders b/share/html/Ticket/Elements/Reminders
index bc2e7c4..d618a87 100644
--- a/share/html/Ticket/Elements/Reminders
+++ b/share/html/Ticket/Elements/Reminders
@@ -75,7 +75,9 @@ my $reminder_collection = $count_reminders->Collection;
 % if ( $Edit ) {
 <th class="collection-as-table" colspan="5"><&|/l&>Reminders</&></th>
 % } else {
+% if ( $Ticket->CurrentUserHasRight('ModifyTicket') ) {
 <th class="collection-as-table"></th>
+% }
 <th class="collection-as-table"><&|/l&>Reminder</&></th>
 <th class="collection-as-table"><&|/l&>Due</&></th>
 <th class="collection-as-table"><&|/l&>Owner</&></th>
@@ -97,7 +99,7 @@ my $reminder_collection = $count_reminders->Collection;
 % }
 % }
 </table>
-% if ( $visible > 0 ) {
+% if ( $visible > 0 && $Ticket->CurrentUserHasRight('ModifyTicket') ) {
 <i><&|/l&>(Check box to complete)</&></i><br /><br />
 % }
 % } else {
@@ -111,8 +113,11 @@ my $reminder_collection = $count_reminders->Collection;
 % }
 % }
 
+% if ( $Ticket->CurrentUserHasRight('ModifyTicket') ) {
 <&|/l&>New reminder:</&>
 <& SELF:NewReminder, Ticket => $Ticket &>
+% }
+
 <%method NewReminder>
 <%args>
 $Ticket
@@ -160,7 +165,9 @@ $Index
 % my $dueobj = $Reminder->DueObj;
 % my $overdue = $dueobj->Unix > 0 && $dueobj->Diff < 0 ? 1 : 0;
 <tr class="<% $Index%2 ? 'oddline' : 'evenline' %>">
+% if ( $Ticket->CurrentUserHasRight('ModifyTicket') ) {
 <td class="collection-as-table"><input type="checkbox" value="1" name="Complete-Reminder-<% $Reminder->id %>" <% $Reminder->Status eq 'resolved' ? 'checked="checked"' : '' |n %> /></td>
+% }
 <td class="collection-as-table"><% $Reminder->Subject %></td>
 <td class="collection-as-table"><% $overdue ? '<span class="overdue">' : '' |n %><% $dueobj->AgeAsString || loc('Not set') %><% $overdue ? '</span>' : '' |n %></td>
 <td class="collection-as-table"><& /Elements/ShowUser, User => $Reminder->OwnerObj &></td>
diff --git a/share/html/Ticket/Elements/ShowSummary b/share/html/Ticket/Elements/ShowSummary
index 8d35fdf..31f55af 100755
--- a/share/html/Ticket/Elements/ShowSummary
+++ b/share/html/Ticket/Elements/ShowSummary
@@ -77,7 +77,9 @@
         <table><tr><td>
             <form action="<%RT->Config->Get('WebPath')%>/Ticket/Display.html" name="UpdateReminders" id="UpdateReminders" method="post">
                 <& /Ticket/Elements/Reminders, Ticket => $Ticket, ShowCompleted => 0 &>
+% if ( $Ticket->CurrentUserHasRight('ModifyTicket') ) {
                 <div align="right"><input type="submit" class="button" value="<&|/l&>Save</&>" /></div>
+% }
             </form>
         </td></tr></table>
     </&>
diff --git a/share/html/Ticket/Reminders.html b/share/html/Ticket/Reminders.html
index 639e15f..32fd9b9 100755
--- a/share/html/Ticket/Reminders.html
+++ b/share/html/Ticket/Reminders.html
@@ -66,6 +66,8 @@
 
 <%INIT>
 my $Ticket = LoadTicket($id);
+Abort( loc("Permission Denied") )
+  unless $Ticket->CurrentUserHasRight('ModifyTicket');
 
 my @actions = ProcessTicketReminders( TicketObj => $Ticket, ARGSRef => \%ARGS );
 </%INIT>

commit 9adb5f8242a004f01cb26c6155443b696dbb841e
Author: sunnavy <sunnavy at bestpractical.com>
Date:   Thu Feb 3 20:48:27 2011 +0800

    reminders permission test

diff --git a/t/web/reminders-permissions.t b/t/web/reminders-permissions.t
new file mode 100644
index 0000000..ed7abc3
--- /dev/null
+++ b/t/web/reminders-permissions.t
@@ -0,0 +1,59 @@
+#!/usr/bin/env perl
+use strict;
+use warnings;
+use RT::Test tests => 22;
+
+my $user_a = RT::Test->load_or_create_user(
+    Name     => 'user_a',
+    Password => 'password',
+);
+
+ok( $user_a && $user_a->id, 'created user_a' );
+ok(
+    RT::Test->add_rights(
+        {
+            Principal => $user_a,
+            Right     => [
+                qw/SeeQueue CreateTicket ShowTicket/
+            ]
+        },
+    ),
+    'add basic rights for user_a'
+);
+
+my $ticket = RT::Test->create_ticket(
+    Subject => 'test reminder permission',
+    Queue   => 'General',
+);
+
+ok( $ticket->id, 'created a ticket' );
+
+my ($baseurl, $m) = RT::Test->started_ok;
+ok($m->login( user_a => 'password'), 'logged in as user_a');
+
+$m->goto_ticket($ticket->id);
+$m->content_lacks('New reminder:', 'can not create a new reminder');
+ok( !$m->find_link(id => 'page-reminders'), 'no like to Reminders page' );
+$m->get_ok( $baseurl . '/Ticket/Reminders.html?id=' . $ticket->id );
+$m->title_is("RT Error", 'got rt error');
+$m->warning_like(qr/Permission Denied/, 'got permission denied warning');
+$m->content_contains('Permission Denied', 'got permission denied msg');
+
+ok(
+    RT::Test->add_rights(
+        {
+            Principal => $user_a,
+            Right     => [
+                qw/ModifyTicket/
+            ]
+        },
+    ),
+    'add basic rights for user_a'
+);
+$m->goto_ticket($ticket->id);
+$m->content_contains('New reminder:', 'can create a new reminder');
+$m->follow_link_ok({id => 'page-reminders'});
+$m->title_is("Reminders for ticket #" . $ticket->id);
+$m->content_contains('New reminder:', 'can create a new reminder');
+$m->content_lacks('Permission Denied', 'no permission denied msg');
+

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


More information about the Rt-commit mailing list