Bugzilla – Attachment 8639 Details for
Bug 4246
Koha support for Itiva Talking Tech phone notification service
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
3.6.x - Talking Tech Support - Phase I - Followup - Fix Messaging Preferences
36x---Talking-Tech-Support---Phase-I---Followup---.patch (text/plain), 11.41 KB, created by
Kyle M Hall
on 2012-03-26 17:05:34 UTC
(
hide
)
Description:
3.6.x - Talking Tech Support - Phase I - Followup - Fix Messaging Preferences
Filename:
MIME Type:
Creator:
Kyle M Hall
Created:
2012-03-26 17:05:34 UTC
Size:
11.41 KB
patch
obsolete
>From 6968b4542b13a8f30087f29e8930e75c7f031b65 Mon Sep 17 00:00:00 2001 >From: Kyle M Hall <kyle@bywatersolutions.com> >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>>/$today/g; >- $letter->{'content'} =~ s/<<today>>/$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>>/$today/g; >+ $letter->{'content'} =~ s/<<today>>/$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>>/$today/g; >+ $letter->{'content'} =~ s/<<today>>/$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>>/$today/g; >+ $letter->{'content'} =~ s/<<today>>/$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
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 4246
:
7964
|
7965
|
8012
|
8014
|
8636
|
8637
|
8638
|
8639
|
9487
|
9488
|
9501
|
10062