RT Reminders

Hi Jesse,

just saw you “touched” :wink: the reminders

I fixed some bugs and reworked the layout
for reminders in my extension:
http://github.com/cloos/RT-Extension-ReminderImproved

You should checkout my changes and integrate them in
3.9 to make the reminder feature useful.

Thanks!

-Chris

Much appreciated. I’ll be sure to have a look.

-jesseOn Wed 29.Sep’10 at 14:59:11 +0200, Christian Loos wrote:

Hi Jesse,

just saw you “touched” :wink: the reminders
halve the # of SQL queries the MyReminders portlet does. · bestpractical/rt@e3e391e · GitHub

I fixed some bugs and reworked the layout
for reminders in my extension:
http://github.com/cloos/RT-Extension-ReminderImproved

You should checkout my changes and integrate them in
3.9 to make the reminder feature useful.

Thanks!

-Chris

I know I should have dealt with this for 3.9, but customer commitments
meant that we didn’t manage to. I’m sorry about that.

On a positive note, I intend for the 4.2 cycle to be relatively short.On Wed, Sep 29, 2010 at 02:59:11PM +0200, Christian Loos wrote:

Hi Jesse,

just saw you “touched” :wink: the reminders
halve the # of SQL queries the MyReminders portlet does. · bestpractical/rt@e3e391e · GitHub

I fixed some bugs and reworked the layout
for reminders in my extension:
http://github.com/cloos/RT-Extension-ReminderImproved

You should checkout my changes and integrate them in
3.9 to make the reminder feature useful.

Thanks!

-Chris

It’s very sad, to have a feature in RT since 3.6 that is still broken.

Would it help if I could send you patches this week with, at least, my
bugfixes, so that they can go into RT 4.0 or should I wait after the
release of RT 4.0?Am 08.12.2010 18:40, schrieb Jesse Vincent:

I know I should have dealt with this for 3.9, but customer commitments
meant that we didn’t manage to. I’m sorry about that.

On a positive note, I intend for the 4.2 cycle to be relatively short.

It’s very sad, to have a feature in RT since 3.6 that is still broken.

Would it help if I could send you patches this week with, at least, my
bugfixes, so that they can go into RT 4.0 or should I wait after the
release of RT 4.0?

Patches would certainly be easier to deal with than “here’s a thing on github”

If you have the cycles to do that, I’d find time to take a look.

Hi Jesse,

attached 7 patches against 3.9-trunk.

-ChrisAm 13.12.2010 22:42, schrieb Jesse Vincent:

Patches would certainly be easier to deal with than “here’s a thing on github”

If you have the cycles to do that, I’d find time to take a look.

0001-reminder-code-cleanups.patch (3.66 KB)

0002-only-record-transaction-on-success.patch (1.64 KB)

0003-don-t-record-link-transaction-on-reminder-create.patch (1.15 KB)

0004-create-ShowReminders-element-and-use-it-in-MyReminde.patch (6 KB)

0005-create-reminder-view-tool-and-link-it-to-MyReminders.patch (4.17 KB)

0006-more-constistent-naming-of-save-button.patch (812 Bytes)

0007-redesign-ticket-reminder-element.patch (7.95 KB)

Hi!

I have some quibbles with some of the styling:

  • In the full-page edit UI, we should be using a more RTish key:value layout
    rather than the multicolumn table

  • I’m not sure “Reminder” is the right header for the table, maybe “Summary”? not sure.

  • The table looks a little weird in context

Additionally, we should work toward killing the inline mason components. (I know they’re not your fault. They were mine 8 years ago)

It looks like you may not have run tests after applying these patches, as they generated some warnings.

All that being said, this is much better than what we had before. Thanks! Applied.

Best,
JesseOn Thu, Dec 16, 2010 at 04:33:26PM +0100, Christian Loos wrote:

Hi Jesse,

attached 7 patches against 3.9-trunk.

-Chris

Am 13.12.2010 22:42, schrieb Jesse Vincent:

Patches would certainly be easier to deal with than “here’s a thing on github”

If you have the cycles to do that, I’d find time to take a look.

From 11034a1e490c426fc55957072ea4fc9a607bad9e Mon Sep 17 00:00:00 2001
From: Christian Loos cloos@netcologne.de
Date: Thu, 16 Dec 2010 13:05:08 +0100
Subject: [PATCH 1/7] reminder code cleanups


lib/RT/Reminders.pm | 70 +++++++++++++++++++++++±-------------------------
1 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/lib/RT/Reminders.pm b/lib/RT/Reminders.pm
index 13dd12a…70cf67c 100644
— a/lib/RT/Reminders.pm
+++ b/lib/RT/Reminders.pm
@@ -52,7 +52,6 @@ use base qw/RT::Base/;

our $REMINDER_QUEUE = ‘General’;

sub new {
my $class = shift;
my $self = {};
@@ -61,7 +60,6 @@ sub new {
return($self);
}

sub Ticket {
my $self = shift;
$self->{‘ticket’} = shift if (@);
@@ -74,10 +72,9 @@ sub TicketObj {
$self->{’_ticketobj’} = RT::Ticket->new($self->CurrentUser);
$self->{‘_ticketobj’}->Load($self->Ticket);
}

  •    return $self->{'_ticketobj'};
    
  • return $self->{‘_ticketobj’};
    }

=head2 Collection

Returns an RT::Tickets object containing reminders for this object’s “Ticket”
@@ -88,8 +85,8 @@ sub Collection {
my $self = shift;
my $col = RT::Tickets->new($self->CurrentUser);

  • my $query = 'Type = "reminder" AND RefersTo = "'.$self->Ticket.'"';
    
  • my $query = ‘Type = “reminder” AND RefersTo = "’.$self->Ticket.‘"’;

  • $col->FromSQL($query);

    $col->OrderBy( FIELD => ‘Due’ );
    @@ -107,55 +104,56 @@ Takes
    Owner
    Due

=cut

sub Add {
my $self = shift;

  • my %args = ( Subject => undef,
  •             Owner => undef,
    
  •             Due => undef,
    
  •             @_);
    
  • my %args = (

  •    Subject => undef,
    
  •    Owner => undef,
    
  •    Due => undef,
    
  •    @_
    
  • );

    my $reminder = RT::Ticket->new($self->CurrentUser);

  • $reminder->Create( Subject => $args{‘Subject’},
  •                   Owner => $args{'Owner'},
    
  •                   Due => $args{'Due'},
    
  •                   RefersTo => $self->Ticket,
    
  •                   Type => 'reminder',
    
  •                   Queue => $self->TicketObj->Queue,
    
  •               );
    
  • $self->TicketObj->_NewTransaction(Type => ‘AddReminder’,
  •                                Field => 'RT::Ticket',
    
  •                               NewValue => $reminder->id);
    
  • $reminder->Create(
  •    Subject => $args{'Subject'},
    
  •    Owner => $args{'Owner'},
    
  •    Due => $args{'Due'},
    
  •    RefersTo => $self->Ticket,
    
  •    Type => 'reminder',
    
  •    Queue => $self->TicketObj->Queue,
    
  • );
  • $self->TicketObj->_NewTransaction(
  •    Type => 'AddReminder',
    
  •    Field => 'RT::Ticket',
    
  •    NewValue => $reminder->id
    
  • );
    }

sub Open {
my $self = shift;

  • my $reminder = shift;
  • my $reminder = shift;

    $reminder->SetStatus(‘open’);

  • $self->TicketObj->_NewTransaction(Type => ‘OpenReminder’,
  •                                Field => 'RT::Ticket',
    
  •                               NewValue => $reminder->id);
    
  • $self->TicketObj->_NewTransaction(
  •    Type => 'OpenReminder',
    
  •    Field => 'RT::Ticket',
    
  •    NewValue => $reminder->id
    
  • );
    }

sub Resolve {
my $self = shift;
my $reminder = shift;
$reminder->SetStatus(‘resolved’);

  • $self->TicketObj->_NewTransaction(Type => ‘ResolveReminder’,
  •                                Field => 'RT::Ticket',
    
  •                               NewValue => $reminder->id);
    
  • $self->TicketObj->_NewTransaction(
  •    Type => 'ResolveReminder',
    
  •    Field => 'RT::Ticket',
    
  •    NewValue => $reminder->id
    
  • );
    }
  • RT::Base->_ImportOverlays();

+RT::Base->_ImportOverlays();

1;

1.7.1

From 4b33ccc72a9a3b8199990b5439834afe999d73a2 Mon Sep 17 00:00:00 2001
From: Christian Loos cloos@netcologne.de
Date: Thu, 16 Dec 2010 13:17:15 +0100
Subject: [PATCH 2/7] only record transaction on success


lib/RT/Reminders.pm | 15 ++++++++±-----
1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/lib/RT/Reminders.pm b/lib/RT/Reminders.pm
index 70cf67c…49d8183 100644
— a/lib/RT/Reminders.pm
+++ b/lib/RT/Reminders.pm
@@ -116,7 +116,7 @@ sub Add {
);

 my $reminder = RT::Ticket->new($self->CurrentUser);
  • $reminder->Create(
  • my ( $status, $msg ) = $reminder->Create(
    Subject => $args{‘Subject’},
    Owner => $args{‘Owner’},
    Due => $args{‘Due’},
    @@ -128,30 +128,33 @@ sub Add {
    Type => ‘AddReminder’,
    Field => ‘RT::Ticket’,
    NewValue => $reminder->id
  • );
  • ) if $status;
  • return ( $status, $msg );
    }

sub Open {
my $self = shift;
my $reminder = shift;

  • $reminder->SetStatus(‘open’);
  • my ( $status, $msg ) = $reminder->SetStatus(‘open’);
    $self->TicketObj->_NewTransaction(
    Type => ‘OpenReminder’,
    Field => ‘RT::Ticket’,
    NewValue => $reminder->id
  • );
  • ) if $status;
  • return ( $status, $msg );
    }

sub Resolve {
my $self = shift;
my $reminder = shift;

  • $reminder->SetStatus(‘resolved’);
  • my ( $status, $msg ) = $reminder->SetStatus(‘resolved’);
    $self->TicketObj->_NewTransaction(
    Type => ‘ResolveReminder’,
    Field => ‘RT::Ticket’,
    NewValue => $reminder->id
  • );
  • ) if $status;
  • return ( $status, $msg );
    }

RT::Base->_ImportOverlays();

1.7.1

From 6349338cfbe0d7da3d24dd50257d442a9477ceb0 Mon Sep 17 00:00:00 2001
From: Christian Loos cloos@netcologne.de
Date: Thu, 16 Dec 2010 13:36:21 +0100
Subject: [PATCH 3/7] don’t record link transaction on reminder create

Don’t record an link transaction on reminder create because the reminder
themself records an transaction of type ‘AddReminder’

lib/RT/Ticket_Overlay.pm | 2 ±
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/lib/RT/Ticket_Overlay.pm b/lib/RT/Ticket_Overlay.pm
index e95481f…22dbf20 100755
— a/lib/RT/Ticket_Overlay.pm
+++ b/lib/RT/Ticket_Overlay.pm
@@ -636,7 +636,7 @@ sub Create {
my ( $wval, $wmsg ) = $self->_AddLink(
Type => $LINKTYPEMAP{$type}->{‘Type’},
$LINKTYPEMAP{$type}->{‘Mode’} => $link,

  •            Silent                        => !$args{'_RecordTransaction'},
    
  •            Silent                        => !$args{'_RecordTransaction'} || $self->Type eq 'reminder',
               'Silent'. ( $LINKTYPEMAP{$type}->{'Mode'} eq 'Base'? 'Target': 'Base' )
                                             => 1,
           );
    


1.7.1

From b27ccc7508a047aeff42a093cce8521be843e787 Mon Sep 17 00:00:00 2001
From: Christian Loos cloos@netcologne.de
Date: Thu, 16 Dec 2010 14:46:15 +0100
Subject: [PATCH 4/7] create ShowReminders element and use it in MyReminders


share/html/Elements/MyReminders | 31 ±----------
share/html/Elements/ShowReminders | 101 +++++++++++++++++++++++++++++++++++++
2 files changed, 102 insertions(+), 30 deletions(-)
create mode 100644 share/html/Elements/ShowReminders

diff --git a/share/html/Elements/MyReminders b/share/html/Elements/MyReminders
index 00ab113…8b20fbe 100755
— a/share/html/Elements/MyReminders
+++ b/share/html/Elements/MyReminders
@@ -49,40 +49,11 @@
<&|/Widgets/TitleBox,
class => ‘reminders’,
title => loc(“Reminders”) &>
-


-<%perl>
-my $i =0;
-while (my $reminder = $reminders->Next) {
-$i++;
-my $targets = RT::Tickets->new($session{‘CurrentUser’});
-$targets->FromSQL("ReferredToBy = ".$reminder->id);

-if (my $ticket= $targets->First) {
+<& /Elements/ShowReminders, OnlyOverdue => 1 &>

-</%perl>
-


-
-
-% }
-% else {

-<&|/l, $reminder->id &>Couldn’t find a ticket for reminder [_1].</&>

-<&|/l&>Please contact your administrator.</&>

-% }}
-

<%$reminder->Subject%>

-
-#<%$ticket->id%>: <%$ticket->Subject%>

-<& /Elements/ShowUser, User => $reminder->OwnerObj &>
-<%$reminder->DueObj->Unix >0 ? '• '.$reminder->DueObj->AgeAsString : ‘’ |n %>
-

-

</&>

<%init>
return unless RT->Config->Get(‘EnableReminders’);
-my $reminders = RT::Tickets->new($session{‘CurrentUser’});
-$reminders->FromSQL(‘(Owner = “Nobody” OR Owner = "’.$session{‘CurrentUser’}->id.‘")’ .

  • ’ AND Type = “reminder” AND (Status = “new” OR Status = “open”)');
    -$reminders->OrderBy(FIELD => ‘Due’, ORDER => ‘ASC’);
    </%init>
    diff --git a/share/html/Elements/ShowReminders b/share/html/Elements/ShowReminders
    new file mode 100644
    index 0000000…51c1614
    — /dev/null
    +++ b/share/html/Elements/ShowReminders
    @@ -0,0 +1,101 @@
    +%# BEGIN BPS TAGGED BLOCK {{{
    +%#
    +%# COPYRIGHT:
    +%#
    +%# This software is Copyright (c) 1996-2010 Best Practical Solutions, LLC
    +%# jesse@bestpractical.com
    +%#
    +%# (Except where explicitly superseded by other copyright notices)
    +%#
    +%#
    +%# LICENSE:
    +%#
    +%# This work is made available to you under the terms of Version 2 of
    +%# the GNU General Public License. A copy of that license should have
    +%# been provided with this software, but in any event can be snarfed
    +%# from www.gnu.org.
    +%#
    +%# This work is distributed in the hope that it will be useful, but
    +%# WITHOUT ANY WARRANTY; without even the implied warranty of
    +%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
    +%# General Public License for more details.
    +%#
    +%# You should have received a copy of the GNU General Public License
    +%# along with this program; if not, write to the Free Software
    +%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
    +%# 02110-1301 or visit their web page on the internet at
    +%# GNU General Public License v2.0 - GNU Project - Free Software Foundation.
    +%#
    +%#
    +%# CONTRIBUTION SUBMISSION POLICY:
    +%#
    +%# (The following paragraph is not intended to limit the rights granted
    +%# to you to modify and distribute this software under the terms of
    +%# the GNU General Public License and is only of importance to you if
    +%# you choose to contribute your changes and enhancements to the
    +%# community by submitting them to Best Practical Solutions, LLC.)
    +%#
    +%# By intentionally submitting any modifications, corrections or
    +%# derivatives to this work, or any other work intended for use with
    +%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
    +%# you are the copyright holder for those contributions and you grant
    +%# Best Practical Solutions, LLC a nonexclusive, worldwide, irrevocable,
    +%# royalty-free, perpetual, license to use, copy, create derivative
    +%# works based on those contributions, and sublicense and distribute
    +%# those contributions and any derivatives thereof.
    +%#
    +%# END BPS TAGGED BLOCK }}}
    +% if ( $reminders->Count ) {
    +
    +
    +
    +
    +
    +
    +<%PERL>
    +my $i =0;
    +while ( my $reminder = $reminders->Next ) {
    +$i++;
    +my $dueobj = $reminder->DueObj;
    +my $overdue = $dueobj->Unix > 0 && $dueobj->Diff < 0 ? 1 : 0;

    +my $targets = RT::Tickets->new($session{‘CurrentUser’});
    +$targets->FromSQL( "ReferredToBy = " . $reminder->id );
    +
    +if ( my $ticket= $targets->First ) {
    +</%PERL>
    +


    +
    +
    +
    +% } else {
    +<td colspan=“3” class="collection-as-table>
    +
    Couldn’t find Ticket for reminder <% $reminder->id %>. Please contact administrator.

    +
    +% }
    +
    +% }
    +
    <&|/l&>Reminder</&><&|/l&>Due</&><&|/l&>Ticket</&>

    +<% $reminder->Subject %>
    +

    +<% $overdue ? ‘’ : ‘’ |n %><% $dueobj->AgeAsString || loc(‘Not set’) %><% $overdue ? ‘’ : ‘’ |n %>
    +

    +#<% $ticket->Id %>: <% $ticket->Subject %>
    +

    +% }
    +
    +<%INIT>
    +my $reminders = RT::Tickets->new($session{‘CurrentUser’});
    +my $tsql = ‘Type = “reminder”’ .

    •       ' AND ( Owner = "Nobody" OR Owner ="' . $session{'CurrentUser'}->id . '")' .
      
    •       ' AND ( Status = "new" OR Status = "open" )';
      

    +$tsql .= ’ AND Due < “now”’ if $OnlyOverdue;
    +
    +$reminders->FromSQL($tsql);
    +$reminders->OrderBy( FIELD => ‘Due’, ORDER => ‘ASC’ );
    +</%INIT>
    +
    +<%ARGS>
    +$OnlyOverdue => 0
    +</%ARGS>

    1.7.1

From 7f4c4e01fa7fec82b485a4ac69c0f27a449230c2 Mon Sep 17 00:00:00 2001
From: Christian Loos cloos@netcologne.de
Date: Thu, 16 Dec 2010 15:19:26 +0100
Subject: [PATCH 5/7] create reminder view tool and link it to MyReminders


share/html/Elements/MyReminders | 3 ±
share/html/Elements/Tabs | 6 ++++
share/html/Tools/MyReminders.html | 55 +++++++++++++++++++++++++++++++++++++
3 files changed, 63 insertions(+), 1 deletions(-)
create mode 100644 share/html/Tools/MyReminders.html

diff --git a/share/html/Elements/MyReminders b/share/html/Elements/MyReminders
index 8b20fbe…c3e4450 100755
— a/share/html/Elements/MyReminders
+++ b/share/html/Elements/MyReminders
@@ -48,7 +48,8 @@
%# DEPRECATED
<&|/Widgets/TitleBox,
class => ‘reminders’,

  • title => loc(“Reminders”) &>
  • title => loc(“Reminders”),
  • title_href => RT->Config->Get(‘WebPath’) . ‘/Tools/MyReminders.html’ &>

<& /Elements/ShowReminders, OnlyOverdue => 1 &>

diff --git a/share/html/Elements/Tabs b/share/html/Elements/Tabs
index f9b7b86…7613fd1 100755
— a/share/html/Elements/Tabs
+++ b/share/html/Elements/Tabs
@@ -111,6 +111,12 @@ if ( $request_path !~ m{^/SelfService/} ) {
$tools->child( my_day => title => loc(‘My Day’), path => ‘/Tools/MyDay.html’, sort_order => 5,
description => loc(‘Easy updating of your open tickets’) );

  • if ( RT->Config->Get(‘EnableReminders’) )
  • {
  •    $tools->child( my_reminders => title => loc('My Reminders'), path => '/Tools/MyReminders.html', sort_order => 6,
    
  •                   description => loc('Easy viewing of your reminders') );
    
  • }
  • $tools->child( offline => title => loc(‘Offline’), path => ‘/Tools/Offline.html’, sort_order => 10,
    description => loc(‘Create tickets offline’) );

diff --git a/share/html/Tools/MyReminders.html b/share/html/Tools/MyReminders.html
new file mode 100644
index 0000000…5415cdf
— /dev/null
+++ b/share/html/Tools/MyReminders.html
@@ -0,0 +1,55 @@
+%# BEGIN BPS TAGGED BLOCK {{{
+%#
+%# COPYRIGHT:
+%#
+%# This software is Copyright (c) 1996-2010 Best Practical Solutions, LLC
+%# jesse@bestpractical.com
+%#
+%# (Except where explicitly superseded by other copyright notices)
+%#
+%#
+%# LICENSE:
+%#
+%# This work is made available to you under the terms of Version 2 of
+%# the GNU General Public License. A copy of that license should have
+%# been provided with this software, but in any event can be snarfed
+%# from www.gnu.org.
+%#
+%# This work is distributed in the hope that it will be useful, but
+%# WITHOUT ANY WARRANTY; without even the implied warranty of
+%# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+%# General Public License for more details.
+%#
+%# You should have received a copy of the GNU General Public License
+%# along with this program; if not, write to the Free Software
+%# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+%# 02110-1301 or visit their web page on the internet at
+%# GNU General Public License v2.0 - GNU Project - Free Software Foundation.
+%#
+%#
+%# CONTRIBUTION SUBMISSION POLICY:
+%#
+%# (The following paragraph is not intended to limit the rights granted
+%# to you to modify and distribute this software under the terms of
+%# the GNU General Public License and is only of importance to you if
+%# you choose to contribute your changes and enhancements to the
+%# community by submitting them to Best Practical Solutions, LLC.)
+%#
+%# By intentionally submitting any modifications, corrections or
+%# derivatives to this work, or any other work intended for use with
+%# Request Tracker, to Best Practical Solutions, LLC, you confirm that
+%# you are the copyright holder for those contributions and you grant
+%# Best Practical Solutions, LLC a nonexclusive, worldwide, irrevocable,
+%# royalty-free, perpetual, license to use, copy, create derivative
+%# works based on those contributions, and sublicense and distribute
+%# those contributions and any derivatives thereof.
+%#
+%# END BPS TAGGED BLOCK }}}
+<& /Elements/Header, Title => loc(‘My reminders’) &>
+<& /Elements/Tabs &>
+
+<& /Elements/ShowReminders, OnlyOverdue => 0 &>
+
+<%INIT>
+return unless RT->Config->Get(‘EnableReminders’);
+</%INIT>

1.7.1

From e3bff5646a328360358d7897323851b60d2e6735 Mon Sep 17 00:00:00 2001
From: Christian Loos cloos@netcologne.de
Date: Thu, 16 Dec 2010 15:26:56 +0100
Subject: [PATCH 6/7] more constistent naming of save button


share/html/Ticket/Reminders.html | 4 ++±
1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/share/html/Ticket/Reminders.html b/share/html/Ticket/Reminders.html
index 64fef40…fae8105 100755
— a/share/html/Ticket/Reminders.html
+++ b/share/html/Ticket/Reminders.html
@@ -57,7 +57,9 @@

<& /Ticket/Elements/Reminders, Ticket => $Ticket, ShowCompleted => 1, Edit => 1 &>
</&>
-<& /Elements/Submit, Label => loc(‘Save’) &>
+<& /Elements/Submit,

  • Label => loc(‘Save Changes’),
  • Caption => loc(“If you’ve updated anything above, be sure to”) &>


1.7.1

From 1a52b4ab3efa11ccc9cbd7f8708a2afc55a09875 Mon Sep 17 00:00:00 2001
From: Christian Loos cloos@netcologne.de
Date: Thu, 16 Dec 2010 16:25:18 +0100
Subject: [PATCH 7/7] redesign ticket reminder element


share/html/Ticket/Elements/Reminders | 115 +++++++++++++++++++±-------------
1 files changed, 69 insertions(+), 46 deletions(-)

diff --git a/share/html/Ticket/Elements/Reminders b/share/html/Ticket/Elements/Reminders
index 43da890…33aa480 100644
— a/share/html/Ticket/Elements/Reminders
+++ b/share/html/Ticket/Elements/Reminders
@@ -76,23 +76,26 @@ if ( $request_args->{‘update-reminders’} ) {
$reminder->SetOwner( $request_args->{ ‘Reminder-Owner-’ . $reminder->id } , “Force” ) ;
}

  •    if ( exists( $request_args->{ 'Reminder-Due-' . $reminder->id } ) && ( $reminder->DueObj->Date ne $request_args->{ 'Reminder-Due-' . $reminder->id } )) {
    
  •        $reminder->SetDue( $request_args->{ 'Reminder-Due-' . $reminder->id } ) ;
    
  •    if ( exists( $request_args->{ 'Reminder-Due-' . $reminder->id } ) && $request_args->{ 'Reminder-Due-' . $reminder->id } ne '' ) {
    
  •        my $DateObj = RT::Date->new( $session{'CurrentUser'} );
    
  •        $DateObj->Set(
    
  •            Format => 'unknown',
    
  •            Value  => $request_args->{ 'Reminder-Due-' . $reminder->id }
    
  •        );
    
  •        if ( defined $DateObj->Unix && $DateObj->Unix != $reminder->DueObj->Unix ) {
    
  •            $reminder->SetDue( $DateObj->ISO );
    
  •        }
       }
    
    }
    }

if ( $request_args->{‘NewReminder-Subject’} ) {
my $due_obj = RT::Date->new( $session{‘CurrentUser’} );

  • my $date = Time::ParseDate::parsedate(
  •    $request_args->{'NewReminder-Due'},
    
  •    UK            => RT->Config->Get('DateDayBeforeMonth'),
    
  •    PREFER_PAST   => 0,
    
  •    PREFER_FUTURE => 1
    
  • $due_obj->Set(
  •  Format => 'unknown',
    
  •  Value => $request_args->{'NewReminder-Due'}
    
    );
  • $due_obj->Set( Value => $date, Format => ‘unix’ );
    my ( $add_id, $msg, $txnid ) = $Ticket->Reminders->Add(
  •    Subject => $request_args->{'NewReminder-Subject'},
       Owner   => $request_args->{'NewReminder-Owner'},
       Due     => $due_obj->ISO
    

@@ -105,42 +108,51 @@ $reminder_collection = $Ticket->Reminders->Collection;
</%init>


-


-% while (my $reminder = $reminder_collection->Next) {
-% if ($reminder->Status eq ‘resolved’ && !$ShowCompleted) {
-
-% } elsif ($Edit) {
-<& SELF:EditEntry, Reminder => $reminder, Ticket => $Ticket &>
-% } else {
-<& SELF:ShowEntry, Reminder => $reminder, Ticket => $Ticket &>
-% }
+<table border=“0” cellpadding=“1” cellspacing=“0” class=“collection-as-table”<% $Edit ? ’ style=“width: auto;”’ : ‘’ |n %>>
+
+% if ( $Edit ) {
+<&|/l&>Reminders</&>
+% } else {
+
+<&|/l&>Reminder</&>
+<&|/l&>Due</&>
+<&|/l&>Owner</&>
+% }
+
+% my ( $i, $visible ) = 0;
+% while ( my $reminder = $reminder_collection->Next ) {
+% $i++;
+% if ( $reminder->Status eq ‘resolved’ && !$ShowCompleted ) {
+
+% $i++;
+% } elsif ($Edit) {
+<& SELF:EditEntry, Reminder => $reminder, Ticket => $Ticket, Index => $i &>
+% $visible++;
+% } else {
+<& SELF:ShowEntry, Reminder => $reminder, Ticket => $Ticket, Index => $i &>
+% $visible++;
% }
-% if ($reminder_collection->Count) {
-<&|/l&>(Check box to delete)</&>


% }
-

-

+
+% if ( $visible > 0 ) {
+<&|/l&>(Check box to complete)</&>


+% }
<&|/l&>New reminder:</&>
<& SELF:NewReminder, Ticket => $Ticket &>
-

<%method NewReminder>
<%args>
$Ticket
</%args>

- - - + + + - - - - - - + + + + +
<&|/l&>Subject:
<&|/l&>Subject:
-<&|/l&>Owner: -<& /Elements/SelectOwner, Name => 'NewReminder-Owner', QueueObj => $Ticket->QueueObj, Default=>$session{'CurrentUser'}->id, DefaultValue => 0 &> -
<&|/l&>Due:<& /Elements/SelectDate, Name => "NewReminder-Due", Default => "" &>
<&|/l&>Owner:<& /Elements/SelectOwner, Name => 'NewReminder-Owner', QueueObj => $Ticket->QueueObj, Default=>$session{'CurrentUser'}->id, DefaultValue => 0 &><&|/l&>Due:<& /Elements/SelectDate, Name => "NewReminder-Due", Default => "" &>
@@ -148,22 +160,33 @@ $Ticket <%args> $Reminder $Ticket +$Index -Status eq 'resolved' ? 'checked="checked"' : '' %> /> - • - <& /Elements/SelectOwner, Name => 'Reminder-Owner-'.$Reminder->id, Queue => $Ticket->QueueObj, Default => $Reminder->Owner, DefaultValue => 0 &> - <& /Elements/SelectDate, Name => 'Reminder-Due-'.$Reminder->id, Default => $Reminder->DueObj->Date &> - (<%$Reminder->DueObj->Unix>0 ? $Reminder->DueObj->AgeAsString : '' %>)
+ +Status eq 'resolved' ? 'checked="checked"' : '' |n %> /> +<&|/l&>Subject: + + + +  +<&|/l&>Owner: +<& /Elements/SelectOwner, Name => 'Reminder-Owner-'.$Reminder->id, QueueObj => $Ticket->QueueObj, Default => $Reminder->Owner, DefaultValue => 0 &> +<&|/l&>Due: +<& /Elements/SelectDate, Name => 'Reminder-Due-'.$Reminder->id &> (<% $Reminder->DueObj->AsString %>) + <%method ShowEntry> <%args> $Reminder $Ticket +$Index -Status eq 'resolved' ? 'checked="checked"' : '' %> /> - <%$Reminder->Subject%> • \ - <& /Elements/ShowUser, User => $Reminder->OwnerObj &> \ - <%$Reminder->DueObj->Unix>0 ? "• ". $Reminder->DueObj->AgeAsString : '' |n%>
+% my $dueobj = $Reminder->DueObj; +% my $overdue = $dueobj->Unix > 0 && $dueobj->Diff < 0 ? 1 : 0; + +Status eq 'resolved' ? 'checked="checked"' : '' |n %> /> +<% $Reminder->Subject %> +<% $overdue ? '' : '' |n %><% $dueobj->AgeAsString || loc('Not set') %><% $overdue ? '' : '' |n %> +<& /Elements/ShowUser, User => $Reminder->OwnerObj &> + -- 1.7.1

Christian

There turn out to have been multiple issues with this refactor:

http://issues.bestpractical.com/Ticket/Display.html?id=16273
https://github.com/bestpractical/rt/tree/4.0/show-all-reminders

You may want to backport them to your original extension

-kevinOn Fri, Dec 17, 2010 at 01:25:23PM -0500, Jesse Vincent wrote:

Hi!

I have some quibbles with some of the styling:

  • In the full-page edit UI, we should be using a more RTish key:value layout
    rather than the multicolumn table

  • I’m not sure “Reminder” is the right header for the table, maybe “Summary”? not sure.

  • The table looks a little weird in context

Additionally, we should work toward killing the inline mason components. (I know they’re not your fault. They were mine 8 years ago)

It looks like you may not have run tests after applying these patches, as they generated some warnings.

All that being said, this is much better than what we had before. Thanks! Applied.

Best,
Jesse

On Thu, Dec 16, 2010 at 04:33:26PM +0100, Christian Loos wrote:

Hi Jesse,

attached 7 patches against 3.9-trunk.

-Chris

Am 13.12.2010 22:42, schrieb Jesse Vincent:

Patches would certainly be easier to deal with than “here’s a thing on github”

If you have the cycles to do that, I’d find time to take a look.

Hi Kevin,

thanks for the note.
I always watching the changes at the RT github repo and will update my
extension.

-ChrisAm 12.01.2011 03:52, schrieb Kevin Falcone:

Christian

There turn out to have been multiple issues with this refactor:

http://issues.bestpractical.com/Ticket/Display.html?id=16273
https://github.com/bestpractical/rt/tree/4.0/show-all-reminders

You may want to backport them to your original extension

-kevin

  • In the full-page edit UI, we should be using a more RTish key:value layout
    rather than the multicolumn table

  • I’m not sure “Reminder” is the right header for the table, maybe “Summary”? not sure.

  • The table looks a little weird in context

How about this patch?

0001-improved-Ticket-Reminders-layout.patch (4.91 KB)

Am 17.12.2010 19:25, schrieb Jesse Vincent:

  • In the full-page edit UI, we should be using a more RTish key:value layout
    rather than the multicolumn table

  • I’m not sure “Reminder” is the right header for the table, maybe “Summary”? not sure.

  • The table looks a little weird in context

How about this patch?

It looks like this patch mixes:

  • Adding a new CSS file
  • Whitespace changes in code that have no effect on output
  • Layout changes to the HTML of reminders

That probably wants to be 3 separate patches.

It looks like this patch mixes:

  • Adding a new CSS file
  • Whitespace changes in code that have no effect on output
  • Layout changes to the HTML of reminders

That probably wants to be 3 separate patches.

Attached the separate patches.

0001-put-the-due-date-in-an-separate-row.patch (1.04 KB)

0002-new-css-for-reminder.patch (3.25 KB)

0003-use-the-new-reminder.css.patch (1.37 KB)