Bugzilla – Attachment 63798 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
Bug-18595-Move-C4MembersMessagingGetMessagingPrefe.patch (text/plain), 20.46 KB, created by
Lari Taskula
on 2017-05-29 11:16:19 UTC
(
hide
)
Description:
Bug 18595: Move C4::Members::Messaging::GetMessagingPreferences
Filename:
MIME Type:
Creator:
Lari Taskula
Created:
2017-05-29 11:16:19 UTC
Size:
20.46 KB
patch
obsolete
>From e688bc9991f92e481a83aa87d9b75b7b44baaa2f 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] Bug 18595: Move > C4::Members::Messaging::GetMessagingPreferences > >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. > >---- INTRANET ---- >1. todo > >---- 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 >--- > 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 a439021..85014b4 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; >@@ -3376,14 +3377,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 7e24ff4..3054e3e 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 ); >@@ -1848,10 +1849,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; > >@@ -1892,7 +1893,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 2274737..a753007 100755 >--- a/misc/cronjobs/advance_notices.pl >+++ b/misc/cronjobs/advance_notices.pl >@@ -59,6 +59,7 @@ use Koha::DateUtils; > use C4::Log; > use Koha::Libraries; > use Koha::Patrons; >+use Koha::Patron::Message::Preferences; > > =head1 NAME > >@@ -242,11 +243,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}++; >@@ -261,7 +264,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'}, >@@ -275,19 +278,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; >@@ -295,7 +300,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'}, >@@ -346,14 +351,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; >@@ -363,7 +370,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, >@@ -405,8 +412,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'; >@@ -420,7 +429,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
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