Bugzilla – Attachment 110155 Details for
Bug 18595
Move C4::Members::Messaging to Koha namespace
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences
0005-Bug-18595-Move-C4-Members-Messaging-GetMessagingPref.patch (text/plain), 19.88 KB, created by
Emmi Takkinen
on 2020-09-16 06:57:55 UTC
(
hide
)
Description:
Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences
Filename:
MIME Type:
Creator:
Emmi Takkinen
Created:
2020-09-16 06:57:55 UTC
Size:
19.88 KB
patch
obsolete
>From 6a9250d96dba33b8fdd35d6127229f1d9f740ed7 Mon Sep 17 00:00:00 2001 >From: Lari Taskula <lari.taskula@jns.fi> >Date: Thu, 27 Oct 2016 16:08:50 +0300 >Subject: [PATCH 5/9] Bug 18595: Move > C4::Members::Messaging::GetMessagingPreferences >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >This patch moves C4::Members::Messaging::GetMessagingPreferences to >Koha::Patron::Message::Preference(s) >The patch makes a heavy touch on core messaging preference features, >but should not introduce any visible changes for end user. Keep this in mind >if testing. If you notice any changes in Koha, something is probably wrong! > >To test: > >Make sure you have enabled the following system preferences: >- EnhancedMessagingPreferences > >---- GENERAL ---- >1. Apply patch >2. Search Koha source code for references of "GetMessagingPreferences" > (grep -r 'GetMessagingPreferences'). >3. Observe no results in source code. > >---- UNIT TESTS ---- >1. prove t/db_dependent/Circulation.t >2. prove t/db_dependent/Reserves.t >3. prove t/db_dependent/Koha/Patron/Categories.t > >Signed-off-by: Marc Véron <veron@veron.ch> > >Signed-off-by: David Bourgault <david.bourgault@inlibro.com> >--- > C4/Circulation.pm | 6 ++- > C4/Form/MessagingPreferences.pm | 9 ++-- > C4/Members/Messaging.pm | 62 ------------------------- > 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(+), 107 deletions(-) > >diff --git a/C4/Circulation.pm b/C4/Circulation.pm >index 3571051e84..18c9cd3a9f 100644 >--- a/C4/Circulation.pm >+++ b/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; >@@ -3433,14 +3434,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 a6c938a64a..77d9ef5c9c 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 92ea6515da..d28de2ccd7 100644 >--- a/C4/Members/Messaging.pm >+++ b/C4/Members/Messaging.pm >@@ -35,68 +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; >- 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 8d7dcbc611..d0160877c4 100644 >--- a/C4/Reserves.pm >+++ b/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 Koha::Plugins; > >@@ -1788,10 +1789,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; > >@@ -1831,7 +1832,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 >diff --git a/Koha/Patron/Category.pm b/Koha/Patron/Category.pm >index 7fb6105831..a416c6734e 100644 >--- a/Koha/Patron/Category.pm >+++ b/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; >diff --git a/circ/returns.pl b/circ/returns.pl >index 18c36dd724..f03e7e35c5 100755 >--- a/circ/returns.pl >+++ b/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; > >@@ -407,7 +408,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 ); >@@ -457,7 +462,7 @@ if ( $messages->{'ResFound'}) { > itemnumber => $reserve->{'itemnumber'}, > reservenotes => $reserve->{'reservenotes'}, > reserve_id => $reserve->{reserve_id}, >- bormessagepref => $holdmsgpreferences->{'transports'}, >+ bormessagepref => $mtts, > ); > } > >diff --git a/misc/cronjobs/advance_notices.pl b/misc/cronjobs/advance_notices.pl >index d8dcf73a51..bd60635511 100755 >--- a/misc/cronjobs/advance_notices.pl >+++ b/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}, >diff --git a/t/db_dependent/Koha/Patron/Categories.t b/t/db_dependent/Koha/Patron/Categories.t >index eb6467db83..fe8c32a4db 100755 >--- 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::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]; >-- >2.17.1 >
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 18595
:
63435
|
63436
|
63437
|
63438
|
63439
|
63440
|
63441
|
63794
|
63795
|
63796
|
63797
|
63798
|
63799
|
63800
|
63813
|
63815
|
63816
|
63817
|
63818
|
63819
|
63820
|
63821
|
63822
|
63823
|
63824
|
63825
|
63826
|
63827
|
63828
|
63829
|
69441
|
69442
|
69443
|
69444
|
69445
|
69446
|
69447
|
69448
|
69449
|
69450
|
69451
|
69452
|
69453
|
69667
|
69668
|
69669
|
69670
|
69671
|
69672
|
69673
|
69674
|
93873
|
93874
|
93875
|
93876
|
93877
|
93878
|
93879
|
93880
|
93908
|
94753
|
94754
|
103444
|
103445
|
103446
|
103447
|
103448
|
103449
|
103450
|
103451
|
103452
|
110151
|
110152
|
110153
|
110154
|
110155
|
110156
|
110157
|
110158
|
110159
|
112147
|
112148
|
112149
|
112150
|
112151
|
112152
|
112153
|
112154
|
112155
|
112690
|
113126
|
113192
|
113221
|
113222
|
113769
|
146541
|
146542
|
146543
|
146544
|
146545
|
146546
|
146547
|
146548
|
146549
|
146550
|
146551
|
146552