[Rt-commit] rt branch, 4.4/dashboard-subscription-user-group, repushed
Dustin Graves
dustin at bestpractical.com
Tue Dec 1 23:00:25 EST 2015
The branch 4.4/dashboard-subscription-user-group was deleted and repushed:
was c1f32a386263838d9423b1dcd98cf2d45cead475
now d3ef4303055170c2486171fd62087d374bb5a8de
1: c712931 = 1: c712931 Fix extraneous whitespace warnings from po linter
2: dea515e = 2: dea515e Updated translations from Launchpad
3: 5cb67b2 = 3: 5cb67b2 Avoid using a stale dbh in rt-setup-database
4: d99f77e = 4: d99f77e failing and warning externalauth tests
5: 3091165 = 5: 3091165 One last missing undef $m in ldap_privileged.t
6: 3a80169 = 6: 3a80169 skip when ::Authen::ExternalAuth can't load like other externalauth tests do
7: 5d5e872 = 7: 5d5e872 French i18n updates
8: 6c0c5b2 = 8: 6c0c5b2 Add "HeldBy" -> "Held By" en localization from Assets
9: e641431 = 9: e641431 po mismerge
--: ------- > 10: 5ea7017 Fix mismerge of _DurationAsString
10: 615120d = 11: 615120d Fix failing test in ldapimport/user-import.t
11: 2f14fe9 = 12: 2f14fe9 Remove calls to removed ->screendebug
12: 4a861ef = 13: 4a861ef Switch ExternalAuth example in RT_Config to JSGantt
13: ec6ce9c = 14: ec6ce9c Fix My_DBI being removed from ExternalServices due to "db" vs "dbi"
--: ------- > 15: e525b1a Avoid returning undef for default catalog
--: ------- > 16: 719dfea Avoid undef warnings in upgrading dashboards
--: ------- > 17: b9c4bdf Unify our upgrading cored extensions documentation
--: ------- > 18: b6d9c46 Install rt-ldapimport
--: ------- > 19: 5f68191 Updated translations from Launchpad
--: ------- > 20: f968229 Handle non-ckeditor for syncing scrips
14: c1f32a3 ! 21: d3ef430 change dashboard subscription recipient input to robust user/group search
@@ -4,9 +4,65 @@
Fixes:T#158520
+diff --git a/etc/upgrade/4.4.1/content b/etc/upgrade/4.4.1/content
+new file mode 100644
+--- /dev/null
++++ b/etc/upgrade/4.4.1/content
+@@
++use strict;
++use warnings;
++
++our @Initial = (
++ # migrate old Recipient field to new Recipients format
++ sub {
++ $RT::Logger->info("Going to migrate dashboard subscription recipients");
++
++ my $attrs = RT::Attributes->new( RT->SystemUser );
++ $attrs->Limit( FIELD => 'ObjectType', VALUE => 'RT::User' );
++ $attrs->Limit( FIELD => 'Name', VALUE => 'Subscription' );
++
++ while ( my $attr = $attrs->Next ) {
++ my $recipient = $attr->SubValue('Recipient');
++ next unless $recipient;
++
++ my %fields = ( Recipients => { Users => [], Groups => [] } );
++
++ foreach (RT::EmailParser->ParseEmailAddress($recipient)) {
++ my $addr = $_->address;
++
++ 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}->{Users} }, $user->id unless grep { $_ == $user->id } @{ $fields{Recipients}->{Users} };
++ }
++
++ my ($ok, $msg) = $attr->SetSubValues(%fields);
++ $RT::Logger->error("Couldn't update subscription: $msg") unless $ok;
++
++ ($ok, $msg) = $attr->DeleteSubValue('Recipient');
++ $RT::Logger->error("Couldn't delete Recipient field from subscription: $msg") unless $ok;
++ }
++ return 1;
++ },
++);
++
+
diff --git a/lib/RT/Dashboard/Mailer.pm b/lib/RT/Dashboard/Mailer.pm
--- a/lib/RT/Dashboard/Mailer.pm
+++ b/lib/RT/Dashboard/Mailer.pm
+@@
+ use File::Temp 'tempdir';
+ use HTML::Scrubber;
+ use URI::QueryParam;
++use List::MoreUtils 'uniq';
+
+ sub MailDashboards {
+ my $self = shift;
@@
LocalTime => [$hour, $dow, $dom],
);
@@ -26,47 +82,34 @@
- if ( $@ ) {
- $RT::Logger->error("Caught exception: $@");
+ my $recipients = $subscription->SubValue('Recipients');
-+ my $recipients_user = $recipients->{User};
-+ my $recipients_group = $recipients->{Group};
-+
-+ my @emails = ();
++ my $recipients_users = $recipients->{Users};
++ my $recipients_groups = $recipients->{Groups};
++
++ 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;
-+
++ for my $user_id (@{ $recipients_users || [] }) {
++ my $user = RT::User->new(RT->SystemUser);
++ $user->Load($user_id);
++ next unless $user->id;
++
++ push @emails, $user->EmailAddress;
++ }
++
++ # add emails for every group's members
++ for my $group_id (@{ $recipients_groups || [] }) {
++ my $group = RT::Group->new(RT->SystemUser);
++ $group->Load($group_id);
++ next unless $group->id;
++
++ my $users = $group->UserMembersObj;
++ while (my $user = $users->Next) {
+ 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) {
++ my $email_success = 0;
++ for my $email (uniq @emails) {
+ eval {
+ $self->SendDashboard(
+ %args,
@@ -80,13 +123,15 @@
+ $RT::Logger->error("Caught exception: $@");
+ }
+ else {
-+ my $counter = $subscription->SubValue('Counter') || 0;
-+ $subscription->SetSubValues(Counter => $counter + 1)
-+ unless $args{DryRun};
++ $email_success = 1;
+ }
}
- }
- }
+- else {
++
++ if ($email_success) {
+ 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
@@ -155,12 +200,12 @@
+</%ARGS>
+
+<%INIT>
-+my ($recipients_user, $recipients_group);
++my ($recipients_users, $recipients_groups);
+my ($Users, $Groups);
+
+if ($Recipients) {
-+ $recipients_user = $Recipients->{User};
-+ $recipients_group = $Recipients->{Group};
++ $recipients_users = $Recipients->{Users};
++ $recipients_groups = $Recipients->{Groups};
+}
+
+if ($UserString) {
@@ -168,18 +213,16 @@
+ $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;
-+ }
++ my $excluded_users = [ $session{'CurrentUser'}->id ];
++ push @$excluded_users, @{ $recipients_users || [] };
++ $Users->Limit(FIELD => 'id', VALUE => $excluded_users, OPERATOR => 'NOT IN');
+}
+
+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;
-+ }
++ $Groups->Limit(FIELD => 'id', VALUE => $recipients_groups, OPERATOR => 'NOT IN') if @{ $recipients_groups || [] };
+}
+</%INIT>
+
@@ -211,14 +254,11 @@
+</tr>
+</table>
+
-+% my $has_users = $Users && $Users->Count;
-+% my $has_groups = $Groups && $Groups->Count;
-+
+% if ($UserString || $GroupString) {
-+<h4><&|/l&>Search Results</&>:</h4>
++<strong><&|/l&>Search Results</&>:</strong>
+<ul>
+
-+% if ($has_users) {
++% if ($Users) {
+% while (my $u = $Users->Next ) {
+<li>
+<input type="checkbox" class="checkbox" name="Dashboard-Subscription-User-<%$u->id%>" value="1" />
@@ -227,7 +267,7 @@
+% }
+% }
+
-+% if ($has_groups) {
++% if ($Groups) {
+% while (my $g = $Groups->Next ) {
+<li>
+<input type="checkbox" class="checkbox" name="Dashboard-Subscription-Group-<%$g->id%>" value="1" />
@@ -237,7 +277,7 @@
+% }
+% }
+
-+% unless ($has_users or $has_groups) {
++% unless (($Users && $Users->Count) || ($Groups && $Groups->Count)) {
+<li><i><&|/l&>none</&></i></li>
+% }
+
@@ -245,13 +285,13 @@
+% }
+
+</td>
-+<td>
-+
-+<h4><&|/l&>Current Recipients</&>:</h4>
++<td class="current-recipients">
++
++<strong><&|/l&>Current Recipients</&>:</strong>
+<ul>
+
+% my $current_user_id = $session{CurrentUser}->id;
-+% my $current_user_subscribed = $recipients_user && grep { $_ == $current_user_id } @$recipients_user;
++% my $current_user_subscribed = $recipients_users && grep { $_ == $current_user_id } @$recipients_users;
+<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" />
@@ -265,8 +305,7 @@
+<& /Elements/ShowUserEmailFrequency, User => $session{CurrentUser} &>
+</li>
+
-+% if ($recipients_user && @$recipients_user) {
-+% for my $user_id (@$recipients_user) {
++% for my $user_id (@{ $recipients_users || [] }) {
+% next if $user_id == $session{'CurrentUser'}->id; # already listed current user
+% my $user = RT::User->new( $session{'CurrentUser'} );
+% $user->Load($user_id);
@@ -284,10 +323,8 @@
+<& /Elements/ShowUserEmailFrequency, User => $user &>
+</li>
+% }
-+% }
-+
-+% if ($recipients_group && @$recipients_group) {
-+% for my $group_id (@$recipients_group) {
++
++% for my $group_id (@{ $recipients_groups || [] }) {
+% my $group = RT::Group->new( $session{'CurrentUser'} );
+% $group->Load($group_id);
+% next unless $group->id;
@@ -305,7 +342,6 @@
+<% $group->Name %>
+% }
+</li>
-+% }
+% }
+
+</ul>
@@ -348,7 +384,7 @@
Dom => 1,
Rows => 20,
- Recipient => '',
-+ Recipients => { User => [], Group => [] },
++ Recipients => { Users => [], Groups => [] },
Fow => 1,
Counter => 0,
);
@@ -356,9 +392,27 @@
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');
++# update recipients
++for my $key (keys %ARGS) {
++ next unless $key =~ /^Dashboard-Subscription-(User|Group)-(\d+)$/;
++
++ my ($mode, $type, $id) = ('', $1.'s', $2);
++
++ # find out proper value for user/group checkbox
++ my $add_keep_recipient = ref $ARGS{$key} eq 'ARRAY' ?
++ grep { $_ } @{ $ARGS{$key} } :
++ $ARGS{$key};
++
++ my $record; # hold user/group object
++ if ($type eq 'Users') {
++ my $user = RT::User->new($session{CurrentUser});
++ $user->Load( $id );
++ $record = $user;
++ } elsif ($type eq 'Groups') {
++ my $group = RT::Group->new($session{CurrentUser});
++ $group->Load( $id );
++ $record = $group;
++ }
-# this'll be defined on submit
-if (defined $ARGS{Save}) {
@@ -370,55 +424,16 @@
- 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} };
+- }
++ my @recipients = @{ $fields{Recipients}->{$type} };
++ my $is_prev_recipient = grep { $_ == $id } @recipients;
++
++ if ($add_keep_recipient and not $is_prev_recipient) { # Add User/Group
++ push @recipients, $id;
++ push @results, loc("$type [_1] added to dashboard subscription recipients.", $record->Name);
++ } elsif (not $add_keep_recipient and $is_prev_recipient) { # Remove User/Group
++ @recipients = grep { $_ != $id } @recipients;
++ push @results, loc("$type [_1] removed from dashboard subscription recipients.", $record->Name);
}
- if ($ok) {
@@ -427,7 +442,8 @@
- $id = delete $fields{'DashboardId'}; # immutable
- ($ok, $msg) = $SubscriptionObj->SetSubValues(%fields);
- $fields{'DashboardId'} = $id;
-+ push @results, $msg if $msg;
++ use List::MoreUtils 'uniq';
++ @{ $fields{Recipients}->{$type} } = uniq @recipients;
+}
- $msg = loc("Subscription updated") if $ok;
@@ -439,10 +455,6 @@
+ $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;
@@ -462,8 +474,6 @@
+ );
+ 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 {
@@ -489,6 +499,8 @@
+ push @results, loc('Subscription could not be created: [_1]', $msg);
}
}
++ push @results, loc("Warning: This dashboard has no recipients")
++ unless @{ $fields{Recipients}->{Users} } || @{ $fields{Recipients}->{Groups} };
+} elsif (defined $ARGS{OnlySearchForPeople}) {
+ $GroupString = undef;
+ $GroupField = undef;
@@ -514,3 +526,17 @@
+$GroupString => undef
</%ARGS>
+
+diff --git a/share/static/css/base/misc.css b/share/static/css/base/misc.css
+--- a/share/static/css/base/misc.css
++++ b/share/static/css/base/misc.css
+@@
+ text-decoration: underline !important;
+ color: white !important;
+ }
++
++/* dashboard subscription */
++
++td.current-recipients {
++ vertical-align: top;
++}
More information about the rt-commit
mailing list