From 6968b4542b13a8f30087f29e8930e75c7f031b65 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Mon, 26 Mar 2012 13:04:29 -0400 Subject: [PATCH] 3.6.x - Talking Tech Support - Phase I - Followup - Fix Messaging Preferences There is a flaw in C4::Members::Messaging::GetMessagingPreferences where the system assumes that every transport will use the same letter. This is not necessarily true. Even with the default preferences of just 'email' and 'sms', we should be able to have different letters for each, as one has a maximum character length ( sms ) and one does not. GetMessagingPreferences currently uses the letter code of the last result of its query as the letter code for every transport type. The returned data is a hashref with a key 'transport_types' that is an array of transport_types this borrower has selected for the given alert. This commit modifies GetMessagingPreferences such that the the 'transport_types' array is now a hash where the name of the transport type is now a key to the value of the letter code set for that transport type. It also modifies code calling GetMessagingPreferences where necessary, and as a side benefit will correctly get the letter codes for email and sms correctly, if they are defined differently. http://bugs.koha-community.org/show_bug.cgi?id=4246 Conflicts: C4/Circulation.pm C4/Reserves.pm --- C4/Circulation.pm | 2 +- C4/Form/MessagingPreferences.pm | 2 +- C4/Members/Messaging.pm | 6 ++-- C4/Reserves.pm | 66 ++++++++++++++++++++++++++++---------- admin/categorie.pl | 2 +- misc/cronjobs/advance_notices.pl | 6 ++-- 6 files changed, 58 insertions(+), 26 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index feb71b6..944e3c1 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2611,7 +2611,7 @@ sub SendCirculationAlert { C4::Letters::parseletter($letter, 'biblioitems', $item->{biblionumber}); C4::Letters::parseletter($letter, 'borrowers', $borrower->{borrowernumber}); C4::Letters::parseletter($letter, 'branches', $branch); - my @transports = @{ $borrower_preferences->{transports} }; + my @transports = keys %{ $borrower_preferences->{transports} }; # warn "no transports" unless @transports; for (@transports) { # warn "transport: $_"; diff --git a/C4/Form/MessagingPreferences.pm b/C4/Form/MessagingPreferences.pm index 85ae9e8..fa3c303 100644 --- a/C4/Form/MessagingPreferences.pm +++ b/C4/Form/MessagingPreferences.pm @@ -139,7 +139,7 @@ sub set_form_values { selected => $_ == $days_in_advance ? 'selected="selected"' :'' } } ( 0..30 ); # FIXME: 30 is a magic number. } - foreach my $transport ( @{$pref->{'transports'}} ) { + foreach my $transport ( keys %{$pref->{'transports'}} ) { $option->{'transports_'.$transport} = 1; } $option->{'digest'} = 1 if $pref->{'wants_digest'}; diff --git a/C4/Members/Messaging.pm b/C4/Members/Messaging.pm index 2b39f11..1a9e3c4 100644 --- a/C4/Members/Messaging.pm +++ b/C4/Members/Messaging.pm @@ -98,9 +98,8 @@ END_SQL $return->{'days_in_advance'} = $row->{'days_in_advance'} if defined $row->{'days_in_advance'}; $return->{'wants_digest'} = $row->{'wants_digest'} if defined $row->{'wants_digest'}; $return->{'letter_code'} = $row->{'letter_code'}; - $transports{$row->{'message_transport_type'}} = 1; + $return->{'transports'}->{ $row->{'message_transport_type'} } = $row->{'letter_code'}; } - @{$return->{'transports'}} = keys %transports; return $return; } @@ -248,8 +247,9 @@ sub SetMessagingPreferencesFromDefaults { # FIXME - except for setting the borrowernumber, it really ought to be possible # to have the output of GetMessagingPreferences be able to be the input # to SetMessagingPreference + my @message_transport_types = keys %{ $default_pref->{transports} }; $default_pref->{message_attribute_id} = $option->{'message_attribute_id'}; - $default_pref->{message_transport_types} = $default_pref->{transports}; + $default_pref->{message_transport_types} = \@message_transport_types; $default_pref->{borrowernumber} = $params->{borrowernumber}; SetMessagingPreference( $default_pref ); } diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 359bbad..f51a515 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -1720,23 +1720,24 @@ sub _koha_notify_reserve { my $admin_email_address = $branch_details->{'branchemail'} || C4::Context->preference('KohaAdminEmailAddress'); - my $letter = getletter( 'reserves', $letter_code ); - die "Could not find a letter called '$letter_code' in the 'reserves' module" unless( $letter ); - C4::Letters::parseletter( $letter, 'branches', $reserve->{'branchcode'} ); - C4::Letters::parseletter( $letter, 'borrowers', $borrowernumber ); - C4::Letters::parseletter( $letter, 'biblio', $biblionumber ); - C4::Letters::parseletter( $letter, 'reserves', $borrowernumber, $biblionumber ); + if ( $print_mode ) { + my $letter = getletter( 'reserves', 'HOLD_PRINT' ); + die "Could not find a letter called '$letter_code' in the 'reserves' module" unless( $letter ); - if ( $reserve->{'itemnumber'} ) { - C4::Letters::parseletter( $letter, 'items', $reserve->{'itemnumber'} ); - } - my $today = C4::Dates->new()->output(); - $letter->{'title'} =~ s/<>/$today/g; - $letter->{'content'} =~ s/<>/$today/g; - $letter->{'content'} =~ s/<<[a-z0-9_]+\.[a-z0-9]+>>//g; #remove any stragglers + C4::Letters::parseletter( $letter, 'branches', $reserve->{'branchcode'} ); + C4::Letters::parseletter( $letter, 'borrowers', $borrowernumber ); + C4::Letters::parseletter( $letter, 'biblio', $biblionumber ); + C4::Letters::parseletter( $letter, 'reserves', $borrowernumber, $biblionumber ); + + if ( $reserve->{'itemnumber'} ) { + C4::Letters::parseletter( $letter, 'items', $reserve->{'itemnumber'} ); + } + my $today = C4::Dates->new()->output(); + $letter->{'title'} =~ s/<>/$today/g; + $letter->{'content'} =~ s/<>/$today/g; + $letter->{'content'} =~ s/<<[a-z0-9_]+\.[a-z0-9]+>>//g; #remove any stragglers - if ( $print_mode ) { C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $borrowernumber, @@ -1746,8 +1747,23 @@ sub _koha_notify_reserve { return; } - if ( grep { $_ eq 'email' } @{$messagingprefs->{transports}} ) { - # aka, 'email' in ->{'transports'} + if ( defined $messagingprefs->{transports}->{'email'} ) { + my $letter = getletter( 'reserves', $messagingprefs->{transports}->{'email'} ); + die "Could not find a letter called '$messagingprefs->{transports}->{'email'}' in the 'reserves' module" unless( $messagingprefs->{transports}->{'email'} ); + + C4::Letters::parseletter( $letter, 'branches', $reserve->{'branchcode'} ); + C4::Letters::parseletter( $letter, 'borrowers', $borrowernumber ); + C4::Letters::parseletter( $letter, 'biblio', $biblionumber ); + C4::Letters::parseletter( $letter, 'reserves', $borrowernumber, $biblionumber ); + + if ( $reserve->{'itemnumber'} ) { + C4::Letters::parseletter( $letter, 'items', $reserve->{'itemnumber'} ); + } + my $today = C4::Dates->new()->output(); + $letter->{'title'} =~ s/<>/$today/g; + $letter->{'content'} =~ s/<>/$today/g; + $letter->{'content'} =~ s/<<[a-z0-9_]+\.[a-z0-9]+>>//g; #remove any stragglers + C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $borrowernumber, @@ -1757,7 +1773,23 @@ sub _koha_notify_reserve { ); } - if ( grep { $_ eq 'sms' } @{$messagingprefs->{transports}} ) { + if ( defined $messagingprefs->{transports}->{'sms'} ) { + my $letter = getletter( 'reserves', $messagingprefs->{transports}->{'sms'} ); + die "Could not find a letter called '$messagingprefs->{transports}->{'sms'}' in the 'reserves' module" unless( $messagingprefs->{transports}->{'sms'} ); + + C4::Letters::parseletter( $letter, 'branches', $reserve->{'branchcode'} ); + C4::Letters::parseletter( $letter, 'borrowers', $borrowernumber ); + C4::Letters::parseletter( $letter, 'biblio', $biblionumber ); + C4::Letters::parseletter( $letter, 'reserves', $borrowernumber, $biblionumber ); + + if ( $reserve->{'itemnumber'} ) { + C4::Letters::parseletter( $letter, 'items', $reserve->{'itemnumber'} ); + } + my $today = C4::Dates->new()->output(); + $letter->{'title'} =~ s/<>/$today/g; + $letter->{'content'} =~ s/<>/$today/g; + $letter->{'content'} =~ s/<<[a-z0-9_]+\.[a-z0-9]+>>//g; #remove any stragglers + C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $borrowernumber, diff --git a/admin/categorie.pl b/admin/categorie.pl index 521346b..265f772 100755 --- a/admin/categorie.pl +++ b/admin/categorie.pl @@ -249,7 +249,7 @@ sub _get_brief_messaging_prefs { message_name => $option->{'message_name'}, $option->{'message_name'} => 1 }; - foreach my $transport ( @{$pref->{'transports'}} ) { + foreach my $transport ( keys %{$pref->{'transports'}} ) { push @{ $brief_pref->{'transports'} }, { transport => $transport }; } push @$results, $brief_pref; diff --git a/misc/cronjobs/advance_notices.pl b/misc/cronjobs/advance_notices.pl index 1fa358f..4262e46 100755 --- a/misc/cronjobs/advance_notices.pl +++ b/misc/cronjobs/advance_notices.pl @@ -215,7 +215,7 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { print $letter->{'content'}; } else { - foreach my $transport ( @{$borrower_preferences->{'transports'}} ) { + foreach my $transport ( keys %{$borrower_preferences->{'transports'}} ) { C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $upcoming->{'borrowernumber'}, from_address => $from_address, @@ -270,7 +270,7 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$upcoming_digest ) { print $letter->{'content'}; } else { - foreach my $transport ( @{$borrower_preferences->{'transports'}} ) { + foreach my $transport ( keys %{$borrower_preferences->{'transports'}} ) { C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $borrowernumber, from_address => $from_address, @@ -310,7 +310,7 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$due_digest ) { print $letter->{'content'}; } else { - foreach my $transport ( @{$borrower_preferences->{'transports'}} ) { + foreach my $transport ( keys %{$borrower_preferences->{'transports'}} ) { C4::Letters::EnqueueLetter( { letter => $letter, borrowernumber => $borrowernumber, from_address => $from_address, -- 1.7.2.5