From 5f5e89c0db155b12bafc55bf0caf91e451996195 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Thu, 27 Oct 2016 16:08:50 +0300 Subject: [PATCH] Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences Unit tests: 1. Run t/db_dependent/Circulation.t 2. Run t/db_dependent/Reserves.t 3. Run t/db_dependent/Koha/Patron/Categories.t --- 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 | 8 +++-- misc/cronjobs/advance_notices.pl | 47 ++++++++++++++---------- t/db_dependent/Koha/Patron/Categories.t | 6 ++-- 8 files changed, 60 insertions(+), 106 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 174b3ac..5044ee3 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -48,6 +48,7 @@ use Koha::IssuingRules; use Koha::Items; use Koha::Patrons; use Koha::Patron::Debarments; +use Koha::Patron::Message::Preferences; use Koha::Database; use Koha::Libraries; use Koha::Holds; @@ -3368,14 +3369,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. diff --git a/C4/Form/MessagingPreferences.pm b/C4/Form/MessagingPreferences.pm index 888a47d..53885ed 100644 --- a/C4/Form/MessagingPreferences.pm +++ b/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); } diff --git a/C4/Members/Messaging.pm b/C4/Members/Messaging.pm index 6019eed..d28de2c 100644 --- a/C4/Members/Messaging.pm +++ b/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 diff --git a/C4/Reserves.pm b/C4/Reserves.pm index 5520954..cd1405f 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -46,6 +46,7 @@ use Koha::Libraries; use Koha::IssuingRules; use Koha::Items; use Koha::ItemTypes; +use Koha::Patron::Message::Preferences; use Koha::Patrons; use List::MoreUtils qw( firstidx any ); @@ -1847,10 +1848,10 @@ sub _koha_notify_reserve { # Try to get the borrower's email address my $to_address = C4::Members::GetNoticeEmailAddress($borrowernumber); - 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; @@ -1890,7 +1891,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 $borrower->{smsalertnumber} ) # No SMS number diff --git a/Koha/Patron/Category.pm b/Koha/Patron/Category.pm index a90123e..8e9b590 100644 --- a/Koha/Patron/Category.pm +++ b/Koha/Patron/Category.pm @@ -83,19 +83,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; diff --git a/circ/returns.pl b/circ/returns.pl index d1b95e8..942dd11 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -52,6 +52,7 @@ use Koha::DateUtils; use Koha::Calendar; use Koha::Checkouts; use Koha::BiblioFrameworks; +use Koha::Patron::Message::Preferences; my $query = new CGI; @@ -436,7 +437,10 @@ if ( $messages->{'WrongTransfer'} and not $messages->{'WasTransfered'}) { if ( $messages->{'ResFound'}) { my $reserve = $messages->{'ResFound'}; my $borr = C4::Members::GetMember( borrowernumber => $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', + }); if ( $reserve->{'ResFound'} eq "Waiting" or $reserve->{'ResFound'} eq "Reserved" ) { if ( $reserve->{'ResFound'} eq "Waiting" ) { $template->param( @@ -478,7 +482,7 @@ if ( $messages->{'ResFound'}) { itemnumber => $reserve->{'itemnumber'}, reservenotes => $reserve->{'reservenotes'}, reserve_id => $reserve->{reserve_id}, - bormessagepref => $holdmsgpreferences->{'transports'}, + bormessagepref => $holdmsgpreferences->message_transport_types, ); } # else { ; } # error? } diff --git a/misc/cronjobs/advance_notices.pl b/misc/cronjobs/advance_notices.pl index 855492e..4142c82 100755 --- a/misc/cronjobs/advance_notices.pl +++ b/misc/cronjobs/advance_notices.pl @@ -58,6 +58,7 @@ use C4::Overdues; use Koha::DateUtils; use C4::Log; use Koha::Libraries; +use Koha::Patron::Message::Preferences; =head1 NAME @@ -241,11 +242,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. $due_digest->{ $upcoming->{borrowernumber} }->{email} = $from_address; $due_digest->{ $upcoming->{borrowernumber} }->{count}++; @@ -260,7 +263,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'}, @@ -274,19 +277,21 @@ 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'}; - - if ( $borrower_preferences->{'wants_digest'} ) { + $borrower_preferences = Koha::Patron::Message::Preferences->find_with_message_name({ + borrowernumber => $upcoming->{'borrowernumber'}, + message_name => 'advance_notice', + }); + next UPCOMINGITEM unless $borrower_preferences && $borrower_preferences->days_in_advance; + next UPCOMINGITEM unless $borrower_preferences->days_in_advance == $upcoming->{'days_until_due'}; + + if ( $borrower_preferences->wants_digest ) { # cache this one to process after we've run through all of the items. $upcoming_digest->{ $upcoming->{borrowernumber} }->{email} = $from_address; $upcoming_digest->{ $upcoming->{borrowernumber} }->{count}++; } else { my $biblio = C4::Biblio::GetBiblioFromItemNumber( $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()) { my @item_info = map { $_ =~ /^date|date$/ ? format_date($item_info->{$_}) : $item_info->{$_} || '' } @item_content_fields; @@ -294,7 +299,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'}, @@ -345,14 +350,16 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$upcoming_digest ) { my $count = $digest->{count}; my $from_address = $digest->{email}; - my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrowernumber, - message_name => 'advance_notice' } ); + my $borrower_preferences = Koha::Patron::Message::Preferences->find_with_message_name({ + borrowernumber => $borrowernumber, + message_name => 'advance_notice', + }); next PATRON unless $borrower_preferences; # how could this happen? my $letter_type = 'PREDUEDGST'; - $sth->execute($borrowernumber,$borrower_preferences->{'days_in_advance'}); + $sth->execute($borrowernumber,$borrower_preferences->days_in_advance); my $titles = ""; while ( my $item_info = $sth->fetchrow_hashref()) { my @item_info = map { $_ =~ /^date|date$/ ? format_date($item_info->{$_}) : $item_info->{$_} || '' } @item_content_fields; @@ -362,7 +369,7 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$upcoming_digest ) { ## Get branch info for borrowers home library. my %branch_info = get_branch_info( $borrowernumber ); - foreach my $transport ( keys %{ $borrower_preferences->{'transports'} } ) { + foreach my $transport ( keys %{ $borrower_preferences->message_transport_types } ) { my $letter = parse_letter( { letter_code => $letter_type, @@ -404,8 +411,10 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$due_digest ) { my $count = $digest->{count}; my $from_address = $digest->{email}; - my $borrower_preferences = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrowernumber, - message_name => 'item_due' } ); + my $borrower_preferences = Koha::Patron::Message::Preferences->find_with_message_name({ + borrowernumber => $borrowernumber, + message_name => 'item_due', + }); next PATRON unless $borrower_preferences; # how could this happen? my $letter_type = 'DUEDGST'; @@ -419,7 +428,7 @@ PATRON: while ( my ( $borrowernumber, $digest ) = each %$due_digest ) { ## Get branch info for borrowers home library. my %branch_info = get_branch_info( $borrowernumber ); - for my $transport ( keys %{ $borrower_preferences->{'transports'} } ) { + for my $transport ( keys %{ $borrower_preferences->message_transport_types } ) { my $letter = parse_letter( { letter_code => $letter_type, diff --git a/t/db_dependent/Koha/Patron/Categories.t b/t/db_dependent/Koha/Patron/Categories.t index 4d81768..e62a614 100644 --- a/t/db_dependent/Koha/Patron/Categories.t +++ b/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::TestBuilder; use t::lib::Mocks; @@ -122,13 +122,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]; -- 2.7.4