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
Signed-off-by: Nicole C. Engard <nengard@bywatersolutions.com>

In use in production by two libraries: Middletown and Washoe
who give their sign off but don't have git to do so.

Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
This commit is contained in:
Kyle M Hall 2012-03-26 11:43:02 -04:00 committed by Paul Poulain
parent d29efac4f3
commit 47e4f3ed84
6 changed files with 22 additions and 20 deletions

View File

@ -2756,7 +2756,7 @@ sub SendCirculationAlert {
}
) or return;
my @transports = @{ $borrower_preferences->{transports} };
my @transports = keys %{ $borrower_preferences->{transports} };
# warn "no transports" unless @transports;
for (@transports) {
# warn "transport: $_";

View File

@ -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'};

View File

@ -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 );
}

View File

@ -1849,11 +1849,7 @@ sub _koha_notify_reserve {
my $messagingprefs;
if ( $to_address || $borrower->{'smsalertnumber'} ) {
$messagingprefs = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrowernumber, message_name => 'Hold_Filled' } );
return if ( !defined( $messagingprefs->{'letter_code'} ) );
$letter_code = $messagingprefs->{'letter_code'};
} else {
$letter_code = 'HOLD_PRINT';
$print_mode = 1;
}
@ -1869,9 +1865,8 @@ sub _koha_notify_reserve {
my $admin_email_address = $branch_details->{'branchemail'} || C4::Context->preference('KohaAdminEmailAddress');
my $letter = C4::Letters::GetPreparedLetter (
my %letter_params = (
module => 'reserves',
letter_code => $letter_code,
branchcode => $reserve->{branchcode},
tables => {
'branches' => $branch_details,
@ -1881,11 +1876,13 @@ sub _koha_notify_reserve {
'items', $reserve->{'itemnumber'},
},
substitute => { today => C4::Dates->new()->output() },
) or die "Could not find a letter called '$letter_code' in the 'reserves' module";
);
if ( $print_mode ) {
$letter_params{ 'letter_code' } = 'HOLD_PRINT';
my $letter = C4::Letters::GetPreparedLetter ( %letter_params ) or die "Could not find a letter called '$letter_params{'letter_code'}' in the 'reserves' module";
C4::Letters::EnqueueLetter( {
letter => $letter,
borrowernumber => $borrowernumber,
@ -1895,8 +1892,10 @@ sub _koha_notify_reserve {
return;
}
if ( grep { $_ eq 'email' } @{$messagingprefs->{transports}} ) {
# aka, 'email' in ->{'transports'}
if ( $to_address && defined $messagingprefs->{transports}->{'email'} ) {
$letter_params{ 'letter_code' } = $messagingprefs->{transports}->{'email'};
my $letter = C4::Letters::GetPreparedLetter ( %letter_params ) or die "Could not find a letter called '$letter_params{'letter_code'}' in the 'reserves' module";
C4::Letters::EnqueueLetter(
{ letter => $letter,
borrowernumber => $borrowernumber,
@ -1906,7 +1905,10 @@ sub _koha_notify_reserve {
);
}
if ( grep { $_ eq 'sms' } @{$messagingprefs->{transports}} ) {
if ( $borrower->{'smsalertnumber'} && defined $messagingprefs->{transports}->{'sms'} ) {
$letter_params{ 'letter_code' } = $messagingprefs->{transports}->{'sms'};
my $letter = C4::Letters::GetPreparedLetter ( %letter_params ) or die "Could not find a letter called '$letter_params{'letter_code'}' in the 'reserves' module";
C4::Letters::EnqueueLetter(
{ letter => $letter,
borrowernumber => $borrowernumber,

View File

@ -250,7 +250,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;

View File

@ -210,7 +210,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,
@ -264,7 +264,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,
@ -303,7 +303,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,