@@ -, +, @@ C4::Members::Messaging::GetMessagingPreferences - EnhancedMessagingPreferences ---- GENERAL ---- (grep -r 'GetMessagingPreferences'). ---- UNIT TESTS ---- --- C4/Circulation.pm | 6 ++- C4/Form/MessagingPreferences.pm | 9 ++-- C4/Members/Messaging.pm | 63 ------------------------- C4/Reserves.pm | 13 +++-- Koha/Patron/Category.pm | 14 +++--- circ/returns.pl | 9 +++- misc/cronjobs/advance_notices.pl | 44 ++++++++--------- t/db_dependent/Koha/Patron/Categories.t | 6 +-- 8 files changed, 56 insertions(+), 108 deletions(-) --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -48,6 +48,7 @@ use Koha::Illrequests; use Koha::Items; use Koha::Patrons; use Koha::Patron::Debarments; +use Koha::Patron::Message::Preferences; use Koha::Database; use Koha::Libraries; use Koha::Account::Lines; @@ -3493,14 +3494,15 @@ sub SendCirculationAlert { CHECKOUT => 'Item_Checkout', RENEWAL => 'Item_Checkout', ); - my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences({ + return unless my $borrower_preferences = Koha::Patron::Message::Preferences->find_with_message_name({ borrowernumber => $borrower->{borrowernumber}, message_name => $message_name{$type}, }); + my $issues_table = ( $type eq 'CHECKOUT' || $type eq 'RENEWAL' ) ? 'issues' : 'old_issues'; my $schema = Koha::Database->new->schema; - my @transports = keys %{ $borrower_preferences->{transports} }; + my @transports = keys %{ $borrower_preferences->message_transport_types }; # From the MySQL doc: # LOCK TABLES is not transaction-safe and implicitly commits any active transaction before attempting to lock the tables. --- a/C4/Form/MessagingPreferences.pm +++ a/C4/Form/MessagingPreferences.pm @@ -133,21 +133,22 @@ sub set_form_values { # walk through the options and update them with these borrower_preferences my $messaging_options = Koha::Patron::Message::Preferences->get_options; PREF: foreach my $option ( @$messaging_options ) { - my $pref = C4::Members::Messaging::GetMessagingPreferences( { %{ $target_params }, message_name => $option->{'message_name'} } ); + my $pref = Koha::Patron::Message::Preferences->find( { %{ $target_params }, message_attribute_id => $option->{'message_attribute_id'} } ); $option->{ $option->{'message_name'} } = 1; # make a hashref of the days, selecting one. if ( $option->{'takes_days'} ) { - my $days_in_advance = $pref->{'days_in_advance'} ? $pref->{'days_in_advance'} : 0; + my $days_in_advance = $pref && $pref->days_in_advance ? $pref->days_in_advance : 0; $option->{days_in_advance} = $days_in_advance; @{$option->{'select_days'}} = map { { day => $_, selected => $_ == $days_in_advance } } ( 0..MAX_DAYS_IN_ADVANCE ); } - foreach my $transport ( keys %{$pref->{'transports'}} ) { + next unless $pref; + foreach my $transport ( keys %{$pref->message_transport_types} ) { $option->{'transports_'.$transport} = 1; } - $option->{'digest'} = 1 if $pref->{'wants_digest'}; + $option->{'digest'} = 1 if $pref->wants_digest; } $template->param(messaging_preferences => $messaging_options); } --- a/C4/Members/Messaging.pm +++ a/C4/Members/Messaging.pm @@ -35,69 +35,6 @@ This module lets you modify a patron's messaging preferences. =head1 FUNCTIONS -=head2 GetMessagingPreferences - - my $preferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrower->{'borrowernumber'}, - message_name => 'DUE' } ); - - my $preferences = C4::Members::Messaging::GetMessagingPreferences( { categorycode => 'LIBRARY', - message_name => 'DUE' } ); - -returns: a hashref of messaging preferences for a borrower or patron category for a particlar message_name - -Requires either a borrowernumber or a categorycode key, but not both. - -=cut - -sub GetMessagingPreferences { - my $params = shift; - - return unless exists $params->{message_name}; - return unless exists $params->{borrowernumber} xor exists $params->{categorycode}; # yes, xor - my $sql = <<'END_SQL'; -SELECT borrower_message_preferences.*, - borrower_message_transport_preferences.message_transport_type, - message_attributes.message_name, - message_attributes.takes_days, - message_transports.is_digest, - message_transports.letter_module, - message_transports.letter_code -FROM borrower_message_preferences -LEFT JOIN borrower_message_transport_preferences -ON borrower_message_transport_preferences.borrower_message_preference_id = borrower_message_preferences.borrower_message_preference_id -LEFT JOIN message_attributes -ON message_attributes.message_attribute_id = borrower_message_preferences.message_attribute_id -LEFT JOIN message_transports -ON message_transports.message_attribute_id = message_attributes.message_attribute_id -AND message_transports.message_transport_type = borrower_message_transport_preferences.message_transport_type -AND message_transports.is_digest = borrower_message_preferences.wants_digest -WHERE message_attributes.message_name = ? -END_SQL - - my @bind_params = ( $params->{'message_name'} ); - if ( exists $params->{'borrowernumber'} ) { - $sql .= " AND borrower_message_preferences.borrowernumber = ? "; - push @bind_params, $params->{borrowernumber}; - } else { - $sql .= " AND borrower_message_preferences.categorycode = ? "; - push @bind_params, $params->{categorycode}; - } - - my $sth = C4::Context->dbh->prepare($sql); - $sth->execute(@bind_params); - my $return; - my %transports; # helps build a list of unique message_transport_types - ROW: while ( my $row = $sth->fetchrow_hashref() ) { - next ROW unless $row->{'message_attribute_id'}; - $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'}; - next unless defined $row->{'message_transport_type'}; - $return->{'transports'}->{ $row->{'message_transport_type'} } = $row->{'letter_code'}; - } - return $return; -} - =head2 SetMessagingPreference This method defines how a user (or a default for a patron category) wants to get a certain --- a/C4/Reserves.pm +++ a/C4/Reserves.pm @@ -44,6 +44,7 @@ use Koha::ItemTypes; use Koha::Items; use Koha::Libraries; use Koha::Old::Hold; +use Koha::Patron::Message::Preferences; use Koha::Patrons; use Carp; @@ -1758,10 +1759,10 @@ sub _koha_notify_reserve { # Try to get the borrower's email address my $to_address = $patron->notice_email_address; - my $messagingprefs = C4::Members::Messaging::GetMessagingPreferences( { - borrowernumber => $borrowernumber, - message_name => 'Hold_Filled' - } ); + my $messagingpref = Koha::Patron::Message::Preferences->find_with_message_name({ + borrowernumber => $borrowernumber, + message_name => 'Hold_Filled', + }); my $library = Koha::Libraries->find( $hold->branchcode )->unblessed; @@ -1801,7 +1802,9 @@ sub _koha_notify_reserve { } ); }; - while ( my ( $mtt, $letter_code ) = each %{ $messagingprefs->{transports} } ) { + my $transports = $messagingpref->message_transport_types if $messagingpref; + foreach my $mtt (keys %{$transports}) { + my $letter_code = $transports->{$mtt}; next if ( ( $mtt eq 'email' and not $to_address ) # No email address or ( $mtt eq 'sms' and not $patron->smsalertnumber ) # No SMS number --- a/Koha/Patron/Category.pm +++ a/Koha/Patron/Category.pm @@ -84,19 +84,17 @@ sub default_messaging { my $messaging_options = Koha::Patron::Message::Preferences->get_options; my @messaging; foreach my $option (@$messaging_options) { - my $pref = C4::Members::Messaging::GetMessagingPreferences( - { - categorycode => $self->categorycode, - message_name => $option->{message_name} - } - ); - next unless $pref->{transports}; + my $pref = Koha::Patron::Message::Preferences->find_with_message_name({ + categorycode => $self->categorycode, + message_name => $option->{message_name}, + }); + next if !$pref || $pref && !$pref->message_transport_types; my $brief_pref = { message_attribute_id => $option->{message_attribute_id}, message_name => $option->{message_name}, $option->{'message_name'} => 1, }; - foreach my $transport ( keys %{ $pref->{transports} } ) { + foreach my $transport ( keys %{ $pref->message_transport_types } ) { push @{ $brief_pref->{transports} }, { transport => $transport }; } push @messaging, $brief_pref; --- a/circ/returns.pl +++ a/circ/returns.pl @@ -56,6 +56,7 @@ use Koha::DateUtils; use Koha::Holds; use Koha::Items; use Koha::Patrons; +use Koha::Patron::Message::Preferences; my $query = new CGI; @@ -410,7 +411,11 @@ if ( $messages->{'WrongTransfer'} and not $messages->{'WasTransfered'}) { if ( $messages->{'ResFound'}) { my $reserve = $messages->{'ResFound'}; my $patron = Koha::Patrons->find( $reserve->{borrowernumber} ); - my $holdmsgpreferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $reserve->{'borrowernumber'}, message_name => 'Hold_Filled' } ); + my $holdmsgpreferences = Koha::Patron::Message::Preferences->find_with_message_name({ + borrowernumber => $reserve->{'borrowernumber'}, + message_name => 'Hold_Filled', + }); + my $mtts = $holdmsgpreferences->message_transport_types if defined $holdmsgpreferences; my $branchCheck = ( $userenv_branch eq $reserve->{branchcode} ); if ( $reserve->{'ResFound'} eq "Reserved" && C4::Context->preference('HoldsAutoFill') ) { my $item = Koha::Items->find( $itemnumber ); @@ -465,7 +470,7 @@ if ( $messages->{'ResFound'}) { itemnumber => $reserve->{'itemnumber'}, reservenotes => $reserve->{'reservenotes'}, reserve_id => $reserve->{reserve_id}, - bormessagepref => $holdmsgpreferences->{'transports'}, + bormessagepref => $mtts, ); } --- a/misc/cronjobs/advance_notices.pl +++ a/misc/cronjobs/advance_notices.pl @@ -61,6 +61,7 @@ use C4::Log; use Koha::Items; use Koha::Libraries; use Koha::Patrons; +use Koha::Patron::Message::Preferences; =head1 NAME @@ -261,11 +262,13 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { my $borrower_preferences; if ( 0 == $upcoming->{'days_until_due'} ) { # This item is due today. Send an 'item due' message. - $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $upcoming->{'borrowernumber'}, - message_name => 'item_due' } ); + $borrower_preferences = Koha::Patron::Message::Preferences->find_with_message_name({ + borrowernumber => $upcoming->{'borrowernumber'}, + message_name => 'item_due', + }); next unless $borrower_preferences; - if ( $borrower_preferences->{'wants_digest'} ) { + if ( $borrower_preferences->wants_digest ) { # cache this one to process after we've run through all of the items. if ($digest_per_branch) { $due_digest->{ $upcoming->{branchcode} }->{ $upcoming->{borrowernumber} }->{email} = $from_address; @@ -284,7 +287,7 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { } ## Get branch info for borrowers home library. - foreach my $transport ( keys %{$borrower_preferences->{'transports'}} ) { + foreach my $transport ( keys %{$borrower_preferences->message_transport_types} ) { my $letter = parse_letter( { letter_code => $letter_type, borrowernumber => $upcoming->{'borrowernumber'}, branchcode => $upcoming->{'branchcode'}, @@ -298,12 +301,14 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { } } } else { - $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $upcoming->{'borrowernumber'}, - message_name => 'advance_notice' } ); - next UPCOMINGITEM unless $borrower_preferences && exists $borrower_preferences->{'days_in_advance'}; - next UPCOMINGITEM unless $borrower_preferences->{'days_in_advance'} == $upcoming->{'days_until_due'}; + $borrower_preferences = Koha::Patron::Message::Preferences->find_with_message_name({ + borrowernumber => $upcoming->{'borrowernumber'}, + message_name => 'advance_notice', + }); + next UPCOMINGITEM unless $borrower_preferences && defined $borrower_preferences->days_in_advance; + next UPCOMINGITEM unless $borrower_preferences->days_in_advance == $upcoming->{'days_until_due'}; - if ( $borrower_preferences->{'wants_digest'} ) { + if ( $borrower_preferences->wants_digest ) { # cache this one to process after we've run through all of the items. if ($digest_per_branch) { $upcoming_digest->{ $upcoming->{branchcode} }->{ $upcoming->{borrowernumber} }->{email} = $from_address; @@ -315,14 +320,14 @@ UPCOMINGITEM: foreach my $upcoming ( @$upcoming_dues ) { } else { my $item = Koha::Items->find( $upcoming->{itemnumber} ); my $letter_type = 'PREDUE'; - $sth->execute($upcoming->{'borrowernumber'},$upcoming->{'itemnumber'},$borrower_preferences->{'days_in_advance'}); + $sth->execute($upcoming->{'borrowernumber'},$upcoming->{'itemnumber'},$borrower_preferences->days_in_advance); my $titles = ""; while ( my $item_info = $sth->fetchrow_hashref()) { $titles .= C4::Letters::get_item_content( { item => $item_info, item_content_fields => \@item_content_fields } ); } ## Get branch info for borrowers home library. - foreach my $transport ( keys %{$borrower_preferences->{'transports'}} ) { + foreach my $transport ( keys %{$borrower_preferences->message_transport_types} ) { my $letter = parse_letter( { letter_code => $letter_type, borrowernumber => $upcoming->{'borrowernumber'}, branchcode => $upcoming->{'branchcode'}, @@ -380,7 +385,7 @@ if ($digest_per_branch) { get_item_info => sub { my $params = shift; $params->{sth}->execute($params->{borrowernumber}, - $params->{borrower_preferences}->{'days_in_advance'}); + $params->{borrower_preferences}->days_in_advance); return sub { $params->{sth}->fetchrow_hashref; }; @@ -413,7 +418,7 @@ if ($digest_per_branch) { get_item_info => sub { my $params = shift; $params->{sth}->execute($params->{borrowernumber}, - $params->{borrower_preferences}->{'days_in_advance'}); + $params->{borrower_preferences}->days_in_advance); return sub { $params->{sth}->fetchrow_hashref; }; @@ -549,13 +554,10 @@ sub send_digests { $branchcode = $branch_info{'branches.branchcode'}; } - my $borrower_preferences = - C4::Members::Messaging::GetMessagingPreferences( - { - borrowernumber => $borrowernumber, - message_name => $params->{message_name} - } - ); + my $borrower_preferences = Koha::Patron::Message::Preferences->find_with_message_name({ + borrowernumber => $borrowernumber, + message_name => $params->{message_name}, + }); next PATRON unless $borrower_preferences; # how could this happen? @@ -569,7 +571,7 @@ sub send_digests { $titles .= C4::Letters::get_item_content( { item => $item_info, item_content_fields => \@item_content_fields } ); } - foreach my $transport ( keys %{ $borrower_preferences->{'transports'} } ) { + foreach my $transport ( keys %{ $borrower_preferences->message_transport_types } ) { my $letter = parse_letter( { letter_code => $params->{letter_code}, --- a/t/db_dependent/Koha/Patron/Categories.t +++ a/t/db_dependent/Koha/Patron/Categories.t @@ -22,12 +22,12 @@ use Modern::Perl; use Test::More tests => 11; use C4::Context; -use C4::Members::Messaging; use Koha::Database; use Koha::DateUtils; use Koha::Patron; use Koha::Patron::Category; use Koha::Patron::Categories; +use Koha::Patron::Message::Preferences; use t::lib::Dates; use t::lib::TestBuilder; use t::lib::Mocks; @@ -123,13 +123,13 @@ subtest 'default_messaging tests' => sub { is_digest => 1, } }); - C4::Members::Messaging::SetMessagingPreference({ + Koha::Patron::Message::Preference->new({ categorycode => $category->categorycode, message_attribute_id => $attr->{'message_attribute_id'}, message_transport_types => [ qw( email ) ], days_in_advance => 5, wants_digest => 1 - }); + })->store; my $default_messaging = $category->default_messaging; is(@$default_messaging, 1, "One default messaging found."); $default_messaging = $default_messaging->[0]; --