From c29ba45693da7f2b9d03bb2f91061db568a488ab Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Thu, 27 Oct 2016 16:17:47 +0300 Subject: [PATCH] Bug 18595: Move C4::Members::Messaging::SetMessagingPreference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch moves C4::Members::Messaging::SetMessagingPreference to Koha::Patron::MessagePreference 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 "SetMessagingPreference" (grep -r 'SetMessagingPreference'). 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 4. prove t/db_dependent/api/v1/patrons.t Works as expected. QA tools fails with: FAIL C4/Members/Messaging.pm pod empty section in previous paragraph Can be fixed in a followup. Signed-off-by: Marc Véron QA tools pass now OK Signed-off-by: Marc Véron Signed-off-by: David Bourgault Rebased-by: Emmi Takkinen --- C4/Form/MessagingPreferences.pm | 13 +++- C4/Members/Messaging.pm | 86 ------------------------- t/db_dependent/Circulation.t | 5 +- t/db_dependent/Illrequests.t | 9 +-- t/db_dependent/Koha/Patron/Categories.t | 5 +- t/db_dependent/api/v1/patrons.t | 16 +++-- 6 files changed, 34 insertions(+), 100 deletions(-) diff --git a/C4/Form/MessagingPreferences.pm b/C4/Form/MessagingPreferences.pm index 55a0f2f2a4..10854479fe 100644 --- a/C4/Form/MessagingPreferences.pm +++ b/C4/Form/MessagingPreferences.pm @@ -97,7 +97,18 @@ sub handle_form_action { } } - C4::Members::Messaging::SetMessagingPreference( $updater ); + $updater->{'days_in_advance'} = undef unless exists $updater->{'days_in_advance'}; + $updater->{'wants_digest'} = 0 unless exists $updater->{'wants_digest'}; + my $preference = Koha::Patron::MessagePreferences->find({ + borrowernumber => $updater->{'borrowernumber'}, + message_attribute_id => $option->{'message_attribute_id'}, + categorycode => $categorycode || $updater->{'categorycode'}, + }); + unless ($preference) { + Koha::Patron::MessagePreference->new($updater)->store; + } else { + $preference->set($updater)->store; + } if ($query->param( $option->{'message_attribute_id'})){ $prefs_set = 1; diff --git a/C4/Members/Messaging.pm b/C4/Members/Messaging.pm index d28de2ccd7..a065636da2 100644 --- a/C4/Members/Messaging.pm +++ b/C4/Members/Messaging.pm @@ -33,92 +33,6 @@ C4::Members::Messaging - manage patron messaging preferences This module lets you modify a patron's messaging preferences. -=head1 FUNCTIONS - -=head2 SetMessagingPreference - -This method defines how a user (or a default for a patron category) wants to get a certain -message delivered. The list of valid message types can be delivered can be found in the -C table, and the list of valid message transports can be -found in the C table. - - C4::Members::Messaging::SetMessagingPreference( { borrowernumber => $borrower->{'borrowernumber'} - message_attribute_id => $message_attribute_id, - message_transport_types => [ qw( email sms ) ], - days_in_advance => 5 - wants_digest => 1 } ) - -returns nothing useful. - -=cut - -sub SetMessagingPreference { - my $params = shift; - - unless (exists $params->{borrowernumber} xor exists $params->{categorycode}) { # yes, xor - warn "SetMessagingPreference called without exactly one of borrowernumber or categorycode"; - return; - } - foreach my $required ( qw( message_attribute_id message_transport_types ) ) { - if ( ! exists $params->{ $required } ) { - warn "SetMessagingPreference called without required parameter: $required"; - return; - } - } - $params->{'days_in_advance'} = undef unless exists ( $params->{'days_in_advance'} ); - $params->{'wants_digest'} = 0 unless exists ( $params->{'wants_digest'} ); - - my $dbh = C4::Context->dbh(); - - my $delete_sql = <<'END_SQL'; -DELETE FROM borrower_message_preferences - WHERE message_attribute_id = ? -END_SQL - my @bind_params = ( $params->{'message_attribute_id'} ); - if ( exists $params->{'borrowernumber'} ) { - $delete_sql .= " AND borrowernumber = ? "; - push @bind_params, $params->{borrowernumber}; - } else { - $delete_sql .= " AND categorycode = ? "; - push @bind_params, $params->{categorycode}; - } - my $sth = $dbh->prepare( $delete_sql ); - my $deleted = $sth->execute( @bind_params ); - - if ( $params->{'message_transport_types'} ) { - my $insert_bmp = <<'END_SQL'; -INSERT INTO borrower_message_preferences -(borrower_message_preference_id, borrowernumber, categorycode, message_attribute_id, days_in_advance, wants_digest) -VALUES -(NULL, ?, ?, ?, ?, ?) -END_SQL - - $sth = C4::Context->dbh()->prepare($insert_bmp); - # set up so that we can easily construct the insert SQL - $params->{'borrowernumber'} = undef unless exists ( $params->{'borrowernumber'} ); - $params->{'categorycode'} = undef unless exists ( $params->{'categorycode'} ); - my $success = $sth->execute( $params->{'borrowernumber'}, - $params->{'categorycode'}, - $params->{'message_attribute_id'}, - $params->{'days_in_advance'}, - $params->{'wants_digest'} ); - # my $borrower_message_preference_id = $dbh->last_insert_id(); - my $borrower_message_preference_id = $dbh->{'mysql_insertid'}; - - my $insert_bmtp = <<'END_SQL'; -INSERT INTO borrower_message_transport_preferences -(borrower_message_preference_id, message_transport_type) -VALUES -(?, ?) -END_SQL - $sth = C4::Context->dbh()->prepare($insert_bmtp); - foreach my $transport ( @{$params->{'message_transport_types'}}) { - my $success = $sth->execute( $borrower_message_preference_id, $transport ); - } - } - return; -} - =head1 TABLES =head2 message_queue diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index e32a89ee6f..6e94957e05 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -39,7 +39,6 @@ use C4::Items qw( ModItemTransfer ); use C4::Log; use C4::Reserves qw( AddReserve ModReserve ModReserveCancelAll ModReserveAffect CheckReserves GetOtherReserves ); use C4::Overdues qw( CalcFine UpdateFine get_chargeable_units ); -use C4::Members::Messaging qw( SetMessagingPreference ); use Koha::DateUtils qw( dt_from_string output_pref ); use C4::Message; use Koha::Database; @@ -5674,11 +5673,11 @@ subtest "SendCirculationAlert" => sub { my $library = $builder->build_object({ class => 'Koha::Libraries' }); set_userenv( $library->unblessed); my $patron = $builder->build_object({ class => 'Koha::Patrons' }); - C4::Members::Messaging::SetMessagingPreference({ + Koha::Patron::MessagePreference->new({ borrowernumber => $patron->id, message_transport_types => ['sms'], message_attribute_id => 5 - }); + })->store; my $item = $builder->build_sample_item(); my $checkin_notice = $builder->build_object({ class => 'Koha::Notice::Templates', diff --git a/t/db_dependent/Illrequests.t b/t/db_dependent/Illrequests.t index 23e257a51e..27660dce15 100755 --- a/t/db_dependent/Illrequests.t +++ b/t/db_dependent/Illrequests.t @@ -26,6 +26,7 @@ use Koha::Illrequestattributes; use Koha::Illrequest::Config; use Koha::Biblios; use Koha::Patrons; +use Koha::Patron::MessagePreferences; use Koha::ItemTypes; use Koha::Items; use Koha::Libraries; @@ -1015,11 +1016,11 @@ subtest 'Helpers' => sub { #send_patron_notice my $attr = Koha::Patron::MessagePreference::Attributes->find({ message_name => 'Ill_ready' }); - C4::Members::Messaging::SetMessagingPreference({ + Koha::Patron::MessagePreference->new({ borrowernumber => $patron->{borrowernumber}, message_attribute_id => $attr->message_attribute_id, message_transport_types => ['email'] - }); + })->store; my $return_patron = $illrq_obj->send_patron_notice('ILL_PICKUP_READY'); my $notice = $schema->resultset('MessageQueue')->search({ letter_code => 'ILL_PICKUP_READY', @@ -1035,11 +1036,11 @@ subtest 'Helpers' => sub { # ill update notice, passes additional text parameter my $attr_update = Koha::Patron::MessagePreference::Attributes->find({ message_name => 'Ill_update' }); - C4::Members::Messaging::SetMessagingPreference({ + Koha::Patron::MessagePreference->new({ borrowernumber => $patron->{borrowernumber}, message_attribute_id => $attr_update->message_attribute_id, message_transport_types => ['email'] - }); + })->store; my $return_patron_update = $illrq_obj->send_patron_notice('ILL_REQUEST_UPDATE', 'Some additional text'); my $notice_update = $schema->resultset('MessageQueue')->search({ letter_code => 'ILL_REQUEST_UPDATE', diff --git a/t/db_dependent/Koha/Patron/Categories.t b/t/db_dependent/Koha/Patron/Categories.t index d009ee3108..7cc08b64f2 100755 --- a/t/db_dependent/Koha/Patron/Categories.t +++ b/t/db_dependent/Koha/Patron/Categories.t @@ -115,7 +115,10 @@ subtest 'default_messaging tests' => sub { enrolmentperiod => undef, enrolmentperioddate => $next_month, })->store; - my $attr = $builder->build({ source => 'MessageAttribute', }); + my $attr = $builder->build({ + source => 'MessageAttribute', + value => { takes_days => 1 }, + }); my $transport = $builder->build({ source => 'MessageTransport', value => { diff --git a/t/db_dependent/api/v1/patrons.t b/t/db_dependent/api/v1/patrons.t index 117e94d5cc..437a75181e 100755 --- a/t/db_dependent/api/v1/patrons.t +++ b/t/db_dependent/api/v1/patrons.t @@ -27,7 +27,6 @@ use t::lib::Mocks; use t::lib::Dates; use C4::Auth; -use C4::Members::Messaging; use Koha::Database; use Koha::DateUtils qw(dt_from_string output_pref); use Koha::Exceptions::Patron; @@ -35,6 +34,7 @@ use Koha::Exceptions::Patron::Attribute; use Koha::Old::Patrons; use Koha::Patron::Attributes; use Koha::Patron::Debarments qw( AddDebarment ); +use Koha::Patron::MessagePreferences; use JSON qw(encode_json); @@ -549,12 +549,12 @@ subtest 'add() tests' => sub { t::lib::Mocks::mock_preference( 'EnhancedMessagingPreferences', 1 ); - C4::Members::Messaging::SetMessagingPreference({ + Koha::Patron::MessagePreference->new({ categorycode => 'ST', message_attribute_id => 1, message_transport_types => ['email'], wants_digest => 1 - }); + })->store; my $patron_id = $t->post_ok( "//$userid:$password@/api/v1/patrons" => json => { @@ -568,11 +568,17 @@ subtest 'add() tests' => sub { )->status_is(201, 'Patron added')->tx->res->json->{patron_id}; my $messaging_preferences = Koha::Patron::MessagePreferences->find_with_message_name({ borrowernumber => $patron_id, message_name => 'Item_Due' }); - + my $mtts = $messaging_preferences->message_transport_types; + $messaging_preferences = $messaging_preferences->unblessed; + $messaging_preferences->{'transports'} = $mtts; is_deeply( $messaging_preferences, { - letter_code => 'DUEDGST', + borrower_message_preference_id => $messaging_preferences->{borrower_message_preference_id}, + borrowernumber => $patron_id, + categorycode => undef, + days_in_advance => undef, + message_attribute_id => 1, wants_digest => 1, transports => { email => 'DUEDGST' } } , -- 2.25.1