[Rt-commit] rt branch, 4.4/dashboard-subscription-user-group, created. rt-4.4.0rc1-10-gc1f32a3

Dustin Graves dustin at bestpractical.com
Mon Nov 30 19:02:42 EST 2015


The branch, 4.4/dashboard-subscription-user-group has been created
        at  c1f32a386263838d9423b1dcd98cf2d45cead475 (commit)

- Log -----------------------------------------------------------------
commit c1f32a386263838d9423b1dcd98cf2d45cead475
Author: Dustin Graves <dustin at bestpractical.com>
Date:   Tue Nov 17 22:45:06 2015 +0000

    change dashboard subscription recipient input to robust user/group search
    
    Fixes:T#158520

diff --git a/lib/RT/Dashboard/Mailer.pm b/lib/RT/Dashboard/Mailer.pm
index 276e93f..6836f4c 100644
--- a/lib/RT/Dashboard/Mailer.pm
+++ b/lib/RT/Dashboard/Mailer.pm
@@ -103,25 +103,61 @@ sub MailDashboards {
                 LocalTime    => [$hour, $dow, $dom],
             );
 
-            my $email = $subscription->SubValue('Recipient')
-                     || $user->EmailAddress;
-
-            eval {
-                $self->SendDashboard(
-                    %args,
-                    CurrentUser  => $currentuser,
-                    Email        => $email,
-                    Subscription => $subscription,
-                    From         => $from,
-                )
-            };
-            if ( $@ ) {
-                $RT::Logger->error("Caught exception: $@");
+            my $recipients = $subscription->SubValue('Recipients');
+            my $recipients_user = $recipients->{User};
+            my $recipients_group = $recipients->{Group};
+
+            my @emails = ();
+
+            # add users' emails to email list
+            if ($recipients_user) {
+                for my $user_id (@$recipients_user) {
+                    my $user = RT::User->new(RT->SystemUser);
+                    $user->Load($user_id);
+                    next unless $user->id;
+
+                    # don't duplicate email addresses
+                    next if grep { $_ eq $user->EmailAddress } @emails;
+
+                    push @emails, $user->EmailAddress;
+                }
             }
-            else {
-                my $counter = $subscription->SubValue('Counter') || 0;
-                $subscription->SetSubValues(Counter => $counter + 1)
-                    unless $args{DryRun};
+
+            # add emails for every group's members
+            if ($recipients_group) {
+                for my $group_id (@$recipients_group) {
+                    my $group = RT::Group->new(RT->SystemUser);
+                    $group->Load($group_id);
+                    next unless $group->id;
+
+                    my $users = $group->UserMembersObj;
+                    while (defined(my $user = $users->Next)) {
+                        # don't duplicate email addresses
+                        next if grep { $_ eq $user->EmailAddress } @emails;
+
+                        push @emails, $user->EmailAddress;
+                    }
+                }
+            }
+
+            for my $email (@emails) {
+                eval {
+                    $self->SendDashboard(
+                        %args,
+                        CurrentUser  => $currentuser,
+                        Email        => $email,
+                        Subscription => $subscription,
+                        From         => $from,
+                    )
+                };
+                if ( $@ ) {
+                    $RT::Logger->error("Caught exception: $@");
+                }
+                else {
+                    my $counter = $subscription->SubValue('Counter') || 0;
+                    $subscription->SetSubValues(Counter => $counter + 1)
+                        unless $args{DryRun};
+                }
             }
         }
     }
diff --git a/share/html/Dashboards/Elements/SubscriptionRecipients b/share/html/Dashboards/Elements/SubscriptionRecipients
new file mode 100644
index 0000000..071c304
--- /dev/null
+++ b/share/html/Dashboards/Elements/SubscriptionRecipients
@@ -0,0 +1,221 @@
+%# BEGIN BPS TAGGED BLOCK {{{
+%#
+%# COPYRIGHT:
+%#
+%# This software is Copyright (c) 1996-2015 Best Practical Solutions, LLC
+%#                                          <sales at 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
+%# http://www.gnu.org/licenses/old-licenses/gpl-2.0.html.
+%#
+%#
+%# 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 }}}
+<%ARGS>
+$UserField      => 'Name'
+$UserOp         => '='
+$UserString     => undef
+$PrivilegedOnly => undef
+
+$GroupField     => 'Name'
+$GroupOp        => '='
+$GroupString    => undef
+
+$Recipients => undef
+$IsFirstSubscription => undef
+</%ARGS>
+
+<%INIT>
+my ($recipients_user, $recipients_group);
+my ($Users, $Groups);
+
+if ($Recipients) {
+    $recipients_user = $Recipients->{User};
+    $recipients_group = $Recipients->{Group};
+}
+
+if ($UserString) {
+    $Users = RT::Users->new($session{'CurrentUser'});
+    $Users->Limit(FIELD => $UserField, VALUE => $UserString, OPERATOR => $UserOp, CASESENSITIVE => 0);
+    $Users->LimitToPrivileged if $PrivilegedOnly;
+
+    if ($recipients_user) {
+        $Users->Limit(FIELD => "id", VALUE => $_, OPERATOR => '!=') foreach @$recipients_user;
+    }
+}
+
+if ($GroupString) {
+    $Groups = RT::Groups->new($session{'CurrentUser'});
+    $Groups->LimitToUserDefinedGroups;
+    $Groups->Limit(FIELD => $GroupField, VALUE => $GroupString, OPERATOR => $GroupOp, CASESENSITIVE => 0);
+    if ($recipients_group) {
+        $Groups->Limit(FIELD => "id", VALUE => $_, OPERATOR => '!=') foreach @$recipients_group;
+    }
+}
+</%INIT>
+
+<table>
+<tr>
+<td>
+
+<table width="100%">
+<tr>
+<td valign="top">
+<&|/l&>Find people whose</&><br />
+% if ($UserString) {
+<& /Elements/SelectUsers, UserString => $UserString, UserField => $UserField, UserOp => $UserOp &>
+% } else {
+<& /Elements/SelectUsers &>
+% }
+<input type="submit" class="button" name="OnlySearchForPeople" value="<&|/l&>Go!</&>" />
+
+<br />
+
+<&|/l&>Find groups whose</&><br />
+% if ($GroupString) {
+<& /Elements/SelectGroups, GroupString => $GroupString, GroupField => $GroupField, GroupOp => $GroupOp &>
+% } else {
+<& /Elements/SelectGroups &>
+% }
+<input type="submit" class="button" name="OnlySearchForGroup" value="<&|/l&>Go!</&>" />
+</td>
+</tr>
+</table>
+
+% my $has_users = $Users && $Users->Count;
+% my $has_groups = $Groups && $Groups->Count;
+
+% if ($UserString || $GroupString) {
+<h4><&|/l&>Search Results</&>:</h4>
+<ul>
+
+% if ($has_users) {
+% while (my $u = $Users->Next ) {
+<li>
+<input type="checkbox" class="checkbox" name="Dashboard-Subscription-User-<%$u->id%>" value="1" />
+<input type="hidden" name="Dashboard-Subscription-User-<%$u->id%>" value="0" />
+<& '/Elements/ShowUser', User => $u, style=>'verbose' &>
+% }
+% }
+
+% if ($has_groups) {
+% while (my $g = $Groups->Next ) {
+<li>
+<input type="checkbox" class="checkbox" name="Dashboard-Subscription-Group-<%$g->id%>" value="1" />
+<input type="hidden" name="Dashboard-Subscription-Group-<%$g->id%>" value="0" />
+<%$g->Name%> (<%$g->Description%>)
+</li>
+% }
+% }
+
+% unless ($has_users or $has_groups) {
+<li><i><&|/l&>none</&></i></li>
+% }
+
+</ul>
+% }
+
+</td>
+<td>
+
+<h4><&|/l&>Current Recipients</&>:</h4>
+<ul>
+
+% my $current_user_id = $session{CurrentUser}->id;
+% my $current_user_subscribed = $recipients_user && grep { $_ == $current_user_id } @$recipients_user;
+<li>
+<input type="checkbox" class="checkbox" name="Dashboard-Subscription-User-<%$current_user_id%>" value="1" <% $current_user_subscribed || $IsFirstSubscription ? 'checked' : '' %> />
+<input type="hidden" name="Dashboard-Subscription-User-<%$current_user_id%>" value="0" />
+% if ( $session{CurrentUser}->HasRight( Right => 'AdminUsers', Object => $RT::System ) &&
+%      $session{CurrentUser}->HasRight( Right => 'ShowConfigTab', Object =>$RT::System ) ) {
+<a href="<% RT->Config->Get('WebPath') %>/Admin/Users/Modify.html?id=<% $current_user_id %>">
+<& /Elements/ShowUser, User => $session{CurrentUser} &></a>
+% } else {
+<& /Elements/ShowUser, User => $session{CurrentUser} &>
+% }
+<& /Elements/ShowUserEmailFrequency, User => $session{CurrentUser} &>
+</li>
+
+% if ($recipients_user && @$recipients_user) {
+% for my $user_id (@$recipients_user) {
+% next if $user_id == $session{'CurrentUser'}->id; # already listed current user
+% my $user = RT::User->new( $session{'CurrentUser'} );
+% $user->Load($user_id);
+% next unless $user->id;
+<li>
+<input type="checkbox" class="checkbox" name="Dashboard-Subscription-User-<%$user_id%>" value="1" checked />
+<input type="hidden" name="Dashboard-Subscription-User-<%$user_id%>" value="0" />
+% if ( $session{CurrentUser}->HasRight( Right => 'AdminUsers', Object => $RT::System ) &&
+%      $session{CurrentUser}->HasRight( Right => 'ShowConfigTab', Object =>$RT::System ) ) {
+<a href="<% RT->Config->Get('WebPath') %>/Admin/Users/Modify.html?id=<% $user_id %>">
+<& /Elements/ShowUser, User => $user &></a>
+% } else {
+<& /Elements/ShowUser, User => $user &>
+% }
+<& /Elements/ShowUserEmailFrequency, User => $user &>
+</li>
+% }
+% }
+
+% if ($recipients_group && @$recipients_group) {
+% for my $group_id (@$recipients_group) {
+% my $group = RT::Group->new( $session{'CurrentUser'} );
+% $group->Load($group_id);
+% next unless $group->id;
+<li>
+<input type="checkbox" class="checkbox" name="Dashboard-Subscription-Group-<%$group_id%>" value="1" checked />
+<input type="hidden" name="Dashboard-Subscription-Group-<%$group_id%>" value="0" />
+
+
+% if ( $session{CurrentUser}->HasRight( Right => 'AdminGroup', Object => $RT::System ) &&
+%      $session{CurrentUser}->HasRight( Right => 'ShowConfigTab', Object =>$RT::System ) ) {
+<a href="<% RT->Config->Get('WebPath') %>/Admin/Groups/Modify.html?id=<% $group_id %>">
+<% $group->Name %>
+</a>
+% } else {
+<% $group->Name %>
+% }
+</li>
+% }
+% }
+
+</ul>
+
+</td>
+</tr>
+</table>
+
diff --git a/share/html/Dashboards/Subscription.html b/share/html/Dashboards/Subscription.html
index d55076d..e2cbfee 100644
--- a/share/html/Dashboards/Subscription.html
+++ b/share/html/Dashboards/Subscription.html
@@ -173,15 +173,17 @@
 %   }
 </select>
 </td></tr>
-
-<tr><td class="label">
-<&|/l&>Recipient</&>:
-</td><td class="value">
-<input name="Recipient" id="Recipient" size="30" value="<%$fields{Recipient} ? $fields{Recipient} : ''%>" />
-<div class="hints"><% loc("Leave blank to send to your current email address ([_1])", $session{'CurrentUser'}->EmailAddress) %></div>
-</td></tr>
 </table>
 </&>
+
+<&| /Widgets/TitleBox, title => loc('Recipients') &>
+<& Elements/SubscriptionRecipients,
+    UserField => $UserField, UserString => $UserString, UserOp => $UserOp,
+    GroupString => $GroupString, GroupOp => $GroupOp, GroupField => $GroupField,
+    Recipients => $fields{Recipients},
+    IsFirstSubscription => $SubscriptionObj ? 0 : 1 &>
+</&>
+
 </td>
 </tr>
 </table>
@@ -222,7 +224,7 @@ my %fields = (
     Dow         => 'Monday',
     Dom         => 1,
     Rows        => 20,
-    Recipient   => '',
+    Recipients  => { User => [], Group => [] },
     Fow         => 1,
     Counter     => 0,
 );
@@ -241,53 +243,109 @@ for my $field (keys %fields) {
         if defined($ARGS{$field}) || $ARGS{$field.'-Magic'};
 }
 
+# migrate old Recipient field to new Recipients format
+if ($SubscriptionObj && $SubscriptionObj->SubValue('Recipient')) {
+    my $old_recipient = $SubscriptionObj->SubValue('Recipient');
 
-# this'll be defined on submit
-if (defined $ARGS{Save}) {
-    my $ok = 1;
-
-    # validation
-    if ($fields{Recipient}) {
-        my @addresses = Email::Address->parse($fields{Recipient});
-        if (@addresses == 0) {
-            push @results, loc('Recipient must be an email address');
-            $ok = 0;
+    my @old_recipients = split(',', $old_recipient);
+
+    foreach my $addr (@old_recipients) {
+        $addr =~ s/^\s+|\s+$//g; # trim whitespace
+
+        my $user = RT::User->new($session{CurrentUser});
+        $user->LoadByEmail($addr);
+
+        unless ($user->id) { # we need to create user
+            my $principal = $RT::System->CanonicalizePrincipal(User => $addr);
+            $user->Load($principal->Object->id);
         }
+
+        push @{ $fields{Recipients}->{User} }, $user->id unless grep { $_ == $user->id } @{ $fields{Recipients}->{User} };
+    }
+}
+
+# update recipients
+for my $key (keys %ARGS) {
+    next unless $key =~ /^Dashboard-Subscription-(User|Group)-(\d+)$/;
+
+    my ($mode, $type, $id) = ('', $1, $2);
+
+    # find out proper value for user/group checkbox
+    my $add_keep_recipient = ref $ARGS{$key} eq 'ARRAY' ?
+        grep { !!$_ } @{ $ARGS{$key} } :
+        !!$ARGS{$key};
+
+    my $object; # hold user/group object
+    if ($type eq 'User') { # User
+        my $user = RT::User->new($session{CurrentUser});
+        $user->Load( $id );
+        $object = $user;
+    } else { # Group
+        my $group = RT::Group->new($session{CurrentUser});
+        $group->Load( $id );
+        $object = $group;
+    }
+
+    my $msg = '';
+
+    my $prev_subscribed = grep { $_ == $id } @{ $fields{Recipients}->{$type} };
+
+    if ($add_keep_recipient and not $prev_subscribed) { # Add User/Group
+        push @{ $fields{Recipients}->{$type} }, $id;
+        $msg = loc("$type [_1] added to dashboard subscription recipients.", $object->Name);
+    } elsif (not $add_keep_recipient and $prev_subscribed) { # Remove User/Group
+        $msg = loc("$type [_1] removed from dashboard subscription recipients.", $object->Name);
+        @{ $fields{Recipients}->{$type} } = grep { $_ != $id } @{ $fields{Recipients}->{$type} };
     }
 
-    if ($ok) {
-        # update
-        if ($SubscriptionObj) {
-            $id = delete $fields{'DashboardId'}; # immutable
-            ($ok, $msg) = $SubscriptionObj->SetSubValues(%fields);
-            $fields{'DashboardId'} = $id;
+    push @results, $msg if $msg;
+}
 
-            $msg = loc("Subscription updated") if $ok;
-            push @results, $msg;
+# this'll be defined on submit
+if (defined $ARGS{Save}) {
+    # update
+    if ($SubscriptionObj) {
+        $id = delete $fields{'DashboardId'}; # immutable
+        ($ok, $msg) = $SubscriptionObj->SetSubValues(%fields);
+        $fields{'DashboardId'} = $id;
+
+        # delete legacy Recipient field
+        $SubscriptionObj->DeleteSubValue('Recipient')
+            if ($SubscriptionObj->SubValue('Recipient'));
+
+        $msg = loc("Subscription updated") if $ok;
+        push @results, $msg;
+    }
+    # create
+    else {
+        Abort(loc("Unable to subscribe to dashboard [_1]: Permission Denied", $id))
+            unless $Dashboard->CurrentUserCanSubscribe;
+
+        $SubscriptionObj = RT::Attribute->new($session{CurrentUser});
+        ($ok, $msg) = $SubscriptionObj->Create(
+            Name        => 'Subscription',
+            Description => 'Subscription to dashboard ' . $id,
+            ContentType => 'storable',
+            Object      => $session{'CurrentUser'}->UserObj,
+            Content     => \%fields,
+        );
+        if ($ok) {
+            push @results, loc("Subscribed to dashboard [_1]", $Dashboard->Name);
+            push @results, loc("Warning: You have no recipients, so you will not receive this dashboard until you have one added")
+                unless @{ $fields{Recipients}->{User} } || @{ $fields{Recipients}->{Group} };
         }
-        # create
         else {
-            Abort(loc("Unable to subscribe to dashboard [_1]: Permission Denied", $id))
-                unless $Dashboard->CurrentUserCanSubscribe;
-
-            $SubscriptionObj = RT::Attribute->new($session{CurrentUser});
-            ($ok, $msg) = $SubscriptionObj->Create(
-                Name        => 'Subscription',
-                Description => 'Subscription to dashboard ' . $id,
-                ContentType => 'storable',
-                Object      => $session{'CurrentUser'}->UserObj,
-                Content     => \%fields,
-            );
-            if ($ok) {
-                push @results, loc("Subscribed to dashboard [_1]", $Dashboard->Name);
-                push @results, loc("Warning: you have no email address set, so you will not receive this dashboard until you have it set")
-                    unless $session{'CurrentUser'}->EmailAddress || $fields{Recipient};
-            }
-            else {
-                push @results, loc('Subscription could not be created: [_1]', $msg);
-            }
+            push @results, loc('Subscription could not be created: [_1]', $msg);
         }
     }
+} elsif (defined $ARGS{OnlySearchForPeople}) {
+    $GroupString = undef;
+    $GroupField = undef;
+    $GroupOp = undef;
+} elsif (defined $ARGS{OnlySearchForGroup}) {
+    $UserString = undef;
+    $UserField = undef;
+    $UserOp = undef;
 }
 
 if ($SubscriptionObj) {
@@ -305,6 +363,12 @@ $Hour        => undef
 $Dow         => undef
 $Dom         => undef
 $Rows        => undef
-$Recipient   => undef
+
+$UserField => undef
+$UserOp => undef
+$UserString => undef
+$GroupField => undef
+$GroupOp => undef
+$GroupString => undef
 </%ARGS>
 

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


More information about the rt-commit mailing list