From d05de8cfb1c6731ffa370d15c7243a8f973e149b Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Thu, 13 Aug 2015 16:52:46 +0300 Subject: [PATCH] Bug 14590 - Checkboxes should be disabled without valid contact information in messaging preferences This patch makes both client and server side validations, both in OPAC and Staff client. Test plan for Staff client: 1. Login to Staff client 2. Enable syspref ValidateEmailAddress and set ValidatePhoneNumber to "ipn" 3. Enable syspref TalkingTechItivaPhoneNotification and SMSSendDriver (write anything), and allow syspref EnhancedMessagingPreferences 4. Navigate to modify patron's information at memberentry.pl 5. Insert valid primary phone number, primary email address and SMS number 6. Check all messaging preferences and submit changes 7. Navigate to modify patron's information at memberentry.pl 8. Clear primary email, primary phone and SMS number fields and submit changes 9. Observe that the messaging preferences are disabled Test plan for OPAC: -1. Make sure sysprefs are set like in Staff client test step 2-3 1. Login to OPAC 2. Navigate to "your personal details" 3. Insert valid primary phone number, primary email address and SMS number 4. Submit changes 5. Login to Staff client 5.1. Navigate to Patrons 5.2. Approve information modification request 6. Back in OPAC, Navigate to "your messaging" 7. Check all messaging preferences and submit changes 8. Navigate to "your personal details" 9. Clear primary email, primary phone and submit changes 10. Repeat step 5 and 6 11. Observe that the messaging preferences are disabled for "Phone" and "Email" columns 12. Clear SMS number field and submit changes 13. Observe that all the messaging preferences are disabled This patch also includes a maintenance script at misc/maintenance/deleteMisconfiguredMessagingPrefs.pl that goes through all borrowers and automatically deletes their misconfigured messaging preferences. --- C4/Form/MessagingPreferences.pm | 39 +++- C4/Members.pm | 9 + C4/Members/Messaging.pm | 123 ++++++++++++ Koha/Borrower/Modifications.pm | 9 + .../prog/en/js/messaging-preference.js | 91 +++++++++ .../prog/en/modules/members/memberentrygen.tt | 65 +++++-- .../bootstrap/en/modules/opac-messaging.tt | 25 ++- .../opac-tmpl/bootstrap/js/messaging-preference.js | 91 +++++++++ members/memberentry.pl | 2 +- .../deleteMisconfiguredMessagingPrefs.pl | 215 +++++++++++++++++++++ opac/opac-messaging.pl | 2 +- t/db_dependent/MessagingPreferencesValidation.t | 180 +++++++++++++++++ 12 files changed, 826 insertions(+), 25 deletions(-) create mode 100644 koha-tmpl/intranet-tmpl/prog/en/js/messaging-preference.js create mode 100644 koha-tmpl/opac-tmpl/bootstrap/js/messaging-preference.js create mode 100755 misc/maintenance/deleteMisconfiguredMessagingPrefs.pl create mode 100644 t/db_dependent/MessagingPreferencesValidation.t diff --git a/C4/Form/MessagingPreferences.pm b/C4/Form/MessagingPreferences.pm index bb981a6..9c9ca5f 100644 --- a/C4/Form/MessagingPreferences.pm +++ b/C4/Form/MessagingPreferences.pm @@ -77,9 +77,41 @@ sub handle_form_action { # TODO: If a "NONE" box and another are checked somehow (javascript failed), we should pay attention to the "NONE" box my $prefs_set = 0; OPTION: foreach my $option ( @$messaging_options ) { - my $updater = { %{ $target_params }, - message_attribute_id => $option->{'message_attribute_id'} }; + my $updater = { message_attribute_id => $option->{'message_attribute_id'} }; + $borrowernumber = $target_params->{borrowernumber} unless $borrowernumber; + $updater->{borrowernumber} = $borrowernumber if defined $borrowernumber; + $updater->{categorycode} = $target_params->{categorycode} if defined $target_params->{categorycode} and not defined $borrowernumber; + + my @transport_methods = $query->param($option->{'message_attribute_id'}); + # Messaging preference validation. Make sure there is a valid contact information + # provided for every transport method. Otherwise remove the transport method, + # because the message cannot be delivered with this method! + if ((defined $query->param('email') && !$query->param('email') || + !defined $query->param('email') && !$target_params->{'email'} && exists $target_params->{'email'}) + && (my $transport_id = (List::MoreUtils::firstidx { $_ eq "email" } @transport_methods)) >-1) { + + splice(@transport_methods, $transport_id, 1);# splice the email transport method for this message + } + if ((defined $query->param('phone') && !$query->param('phone') || + !defined $query->param('phone') && !$target_params->{'phone'} && exists $target_params->{'phone'}) + && (my $transport_id = (List::MoreUtils::firstidx { $_ eq "phone" } @transport_methods)) >-1) { + + splice(@transport_methods, $transport_id, 1);# splice the phone transport method for this message + } + if ((defined $query->param('SMSnumber') && !$query->param('SMSnumber') || + !defined $query->param('SMSnumber') && !$target_params->{'smsalertnumber'} && exists $target_params->{'smsalertnumber'}) + && (my $transport_id = (List::MoreUtils::firstidx { $_ eq "sms" } @transport_methods)) >-1) { + + splice(@transport_methods, $transport_id, 1);# splice the sms transport method for this message + } + + if (@transport_methods > 0) { + $query->param($option->{'message_attribute_id'}, @transport_methods); + } else { + $query->delete($option->{'message_attribute_id'}); + } + # find the desired transports @{$updater->{'message_transport_types'}} = $query->param( $option->{'message_attribute_id'} ); next OPTION unless $updater->{'message_transport_types'}; @@ -108,7 +140,8 @@ sub handle_form_action { C4::Members::Messaging::SetMessagingPreferencesFromDefaults( $target_params ); } # show the success message - $template->param( settings_updated => 1 ); + $template->param( settings_updated => 1 ) if (defined $template); + } =head2 set_form_values diff --git a/C4/Members.pm b/C4/Members.pm index 5f71e89..e592a44 100644 --- a/C4/Members.pm +++ b/C4/Members.pm @@ -653,6 +653,7 @@ sub ModMember { $data{password} = hash_password($data{password}); } } + my $old_categorycode = GetBorrowerCategorycode( $data{borrowernumber} ); # get only the columns of a borrower @@ -686,6 +687,14 @@ sub ModMember { } } + # Validate messaging preferences if any of the following field has been removed + if ((not Koha::Validation::validate_email($data{email} or !$data{email}) or + not Koha::Validation::validate_phonenumber($data{phone}) or !$data{email}) and + not exists $data{smsalertnumber}) { + # Make sure there are no misconfigured preferences - if there is, delete them. + C4::Members::Messaging::DeleteAllMisconfiguredPreferences($data{borrowernumber}); + } + # If NorwegianPatronDBEnable is enabled, we set syncstatus to something that a # cronjob will use for syncing with NL if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) { diff --git a/C4/Members/Messaging.pm b/C4/Members/Messaging.pm index 76a3dad..0a182a9 100644 --- a/C4/Members/Messaging.pm +++ b/C4/Members/Messaging.pm @@ -20,6 +20,7 @@ package C4::Members::Messaging; use strict; use warnings; use C4::Context; +use Koha::Validation; use vars qw($VERSION); @@ -257,6 +258,128 @@ sub SetMessagingPreferencesFromDefaults { $default_pref->{borrowernumber} = $params->{borrowernumber}; SetMessagingPreference( $default_pref ); } + # Finally, delete all misconfigured preferences + DeleteAllMisconfiguredPreferences($params->{borrowernumber}); +} + +=head2 DeleteAllMisconfiguredPreferences + + C4::Members::Messaging::DeleteAllMisconfiguredPreferences( [ $borrowernumber ] ); + +Deletes all misconfigured preferences for ALL borrowers that have invalid contact information for +the transport types. Given a borrowernumber, deletes misconfigured preferences only for this borrower. + +return: returns array of arrayrefs to the deleted preferences (borrower_message_preference_id and message_transport_type) + +=cut + +sub DeleteAllMisconfiguredPreferences { + my $borrowernumber = shift; + + my @deleted_prefs; + + push(@deleted_prefs, DeleteMisconfiguredPreference("email", "email", "email", $borrowernumber)); + push(@deleted_prefs, DeleteMisconfiguredPreference("phone", "phone", "phone", $borrowernumber)); + push(@deleted_prefs, DeleteMisconfiguredPreference("sms", "smsalertnumber", "phone", $borrowernumber)); + + return @deleted_prefs; +} + +=head2 DeleteMisconfiguredPreference + + C4::Members::Messaging::DeleteMisconfiguredPreference( $type, $contact, $validator [, $borrowernumber ] ); + +Takes a messaging preference type and the primary contact method for it, and a string to define +the Koha::Validation that should be used to determine whether the preference is misconfigured. + +A messaging preference is misconfigured when it is linked with invalid contact information. +E.g. messaging type email expects the user to have a valid e-mail address in order to work. + +Deletes misconfigured preferences for ALL borrowers that have invalid contact information for +that transport type. Given a borrowernumber, deletes misconfigured preferences only for this borrower. + +return: returns array of arrayrefs to the deleted preferences (borrower_message_preference_id and message_transport_type) + +=cut + +sub DeleteMisconfiguredPreference { + my ($type, $contact, $validator, $borrowernumber) = @_; + + if (not defined $type or not defined $contact or not defined $validator) { + return 0; + } + + my @misconfigured_prefs; + + # Get all messaging preferences and borrower's contact information + my $dbh = C4::Context->dbh(); + my $query = " + + SELECT + borrower_message_preferences.borrower_message_preference_id, + borrower_message_transport_preferences.message_transport_type, + borrowers.$contact + + FROM + borrower_message_preferences, + borrower_message_transport_preferences, + borrowers + + WHERE + borrower_message_preferences.borrower_message_preference_id + = + borrower_message_transport_preferences.borrower_message_preference_id + + AND + borrowers.borrowernumber = borrower_message_preferences.borrowernumber + + AND + borrower_message_transport_preferences.message_transport_type = ? + "; + + $query .= " AND borrowers.borrowernumber = ?" if defined $borrowernumber; + + my $sth = $dbh->prepare($query); + + if (defined $borrowernumber){ + $sth->execute($type, $borrowernumber); + } else { + $sth->execute($type); + } + + while (my $ref = $sth->fetchrow_arrayref) { + if ($$ref[1] eq $type) { + if (not defined $$ref[2] or defined $$ref[2] and $$ref[2] eq "") { + my $valid_contact = 0; # delete prefs if empty contact + # push the misconfigured preferences into an array + push(@misconfigured_prefs, $$ref[0]) unless $valid_contact and $$ref[2]; + } + else { + my ($valid_contact, $err, $err_msg) = Koha::Validation::use_validator($validator, $$ref[2]); + # push the misconfigured preferences into an array + push(@misconfigured_prefs, $$ref[0]) unless $valid_contact and $$ref[2]; + } + } + } + + $sth = $dbh->prepare(" + + DELETE FROM + borrower_message_transport_preferences + + WHERE + borrower_message_preference_id = ? AND message_transport_type = ? + "); + + my @deleted_prefs; + foreach my $id (@misconfigured_prefs){ + # delete the misconfigured pref + $sth->execute($id, $type); + # push it into array that we will return + push (@deleted_prefs, [$id,$type]); + } + + return @deleted_prefs; } =head1 TABLES diff --git a/Koha/Borrower/Modifications.pm b/Koha/Borrower/Modifications.pm index 12cee66..2711fff 100644 --- a/Koha/Borrower/Modifications.pm +++ b/Koha/Borrower/Modifications.pm @@ -26,6 +26,7 @@ use Modern::Perl; use C4::Context; use C4::Debug; +use C4::Form::MessagingPreferences; sub new { my ( $class, %args ) = @_; @@ -191,6 +192,14 @@ sub ApproveModifications { borrowernumber => $data->{borrowernumber}, }); if( $rs->update($data) ) { + # Validate messaging preferences if any of the following field has been removed + if ((not Koha::Validation::validate_email($data->{email}) or + not Koha::Validation::validate_phonenumber($data->{phone}) or + not Koha::Validation::validate_phonenumber($data->{'smsalertnumber'}))) { + # Make sure there are no misconfigured preferences - if there is, delete them. + C4::Members::Messaging::DeleteAllMisconfiguredPreferences($borrowernumber); + } + $self->DelModifications( { borrowernumber => $borrowernumber } ); } } diff --git a/koha-tmpl/intranet-tmpl/prog/en/js/messaging-preference.js b/koha-tmpl/intranet-tmpl/prog/en/js/messaging-preference.js new file mode 100644 index 0000000..267a164 --- /dev/null +++ b/koha-tmpl/intranet-tmpl/prog/en/js/messaging-preference.js @@ -0,0 +1,91 @@ +/** + * + * Used in: koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt + * + * This component is also used in OPAC: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt + * For modifications, edit also OPAC version in: koha-tmpl/opac-tmpl/bootstrap/en/js/messaging-preference.js + * + * Disables and clears checkboxes from messaging preferences + * if there is either invalid or nonexistent contact information + * for the message transfer type. + * + * @param {HTMLInputElement} elem + * The contact field + * @param {string} id_attr + * Checkboxes' id attribute so that we can recognize them + */ +// Settings for messaging-preference.js +var patron_messaging_checkbox_preferences = { + email: { + checked_checkboxes: null, + is_enabled: true, + disabled_checkboxes: null + }, + sms: { + checked_checkboxes: null, + is_enabled: true, + disabled_checkboxes: null + }, + phone: { + checked_checkboxes: null, + is_enabled: true, + disabled_checkboxes: null + } +}; + +function disableCheckboxesWithInvalidPreferences(elem, id_attr) { + // Get checkbox preferences for the element + var checkbox_prefs = eval("patron_messaging_checkbox_preferences." + id_attr); + // Check if element is empty or not valid + + if (!$(elem).length || $(elem).val().length == 0 ||  !$(elem).valid()) { + + if (checkbox_prefs.is_enabled) { + // Save the state of checked checkboxes + checkbox_prefs.checked_checkboxes = $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]:checked"); + + // Save the state of automatically disabled checkboxes + // (We don't want to enable them once the e-mail is valid!) + checkbox_prefs.disabled_checkboxes = $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]:disabled"); + + // Clear patron messaging preferences checkboxes + $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]").removeAttr("checked"); + + // Then disable checkboxes from patron messaging perferences + $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]").attr("disabled", "disabled"); + + // Color table cell's background emphasize the disabled state of the checkbox + $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]").parent().css("background-color", "#E8F0F8"); + + // Show notice about missing contact in messaging preferences box + $("#required-" + id_attr).css("display", "block"); + + checkbox_prefs.is_enabled = false; + } + } else { + + if (!checkbox_prefs.is_enabled) { + // Enable patron messaging preferences checkboxes + $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]").removeAttr("disabled"); + + // Disable the checkboxes that were disabled by default + checkbox_prefs.disabled_checkboxes.each(function() { + $(this).attr("disabled", "disabled"); + $(this).removeAttr("checked"); + }); + + // Restore the state of checkboxes + checkbox_prefs.checked_checkboxes.each(function() { + $(this).attr("checked", "checked"); + }); + + // Remove the background color from table cell + $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]").parent().css("background-color", "#FFF"); + + // Remove notice about missing contact from messaging preferences box + $("#required-" + id_attr).css("display", "none"); + + checkbox_prefs.is_enabled = true; + } + } +} \ No newline at end of file diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt index a0890ab..c836bc5 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt @@ -37,7 +37,6 @@ var MSG_INCORRECT_PHONE = _("Please enter a valid phone number."); $.validator.addMethod('phone', function(value) { - value = value.trim(); if (!value.trim()) { return 1; } @@ -82,35 +81,59 @@ $("body, form input[type='submit'], form button[type='submit'], form a").addClass('waiting'); if (form.beenSubmitted) return false; - else - form.beenSubmitted = true; - [% IF ValidateEmailAddress %] + else { + form.beenSubmitted = true; + [% IF ValidateEmailAddress %] $("#email, #emailpro, #B_email").each(function(){ $(this).val($.trim($(this).val())); }); - [% END %] - [% IF ValidatePhoneNumber %] + [% END %] + [% IF ValidatePhoneNumber %] $("#phone, #phonepro, #B_phone, #SMSnumber").each(function(){ $(this).val($.trim($(this).val())); }); - [% END %] - form.submit(); + [% END %] + form.submit(); + } } }); [% IF ValidateEmailAddress %] - $("#email, #emailpro, #B_email").on("change", function(){ - $(this).val($.trim($(this).val())); - }); + [% IF !email %] + disableCheckboxesWithInvalidPreferences($("#email"), "email"); + [% END %] + $("#email").on("input", function(){ + disableCheckboxesWithInvalidPreferences($(this), "email"); + }); + $("#email, #emailpro, #B_email").on("change", function(){ + $(this).val($.trim($(this).val())); + disableCheckboxesWithInvalidPreferences($(this), "email"); + }); [% END %] + [% IF ValidatePhoneNumber %] - $("#SMSnumber").on("change", function(){ - $(this).val($.trim($(this).val())); - }); - $("#phone, #phonepro, #B_phone").on("change", function(){ - $(this).val($.trim($(this).val())); - }); + [% IF !smsalertnumber %] + disableCheckboxesWithInvalidPreferences($("#SMSnumber"), "sms"); + [% END %] + [% IF !phone %] + disableCheckboxesWithInvalidPreferences($("#phone"), "phone"); + [% END %] + $("#SMSnumber").on("input", function(){ + disableCheckboxesWithInvalidPreferences($(this), "sms"); + }); + $("#SMSnumber").on("change", function(){ + $(this).val($.trim($(this).val())); + disableCheckboxesWithInvalidPreferences($(this), "sms"); + }); + $("#phone").on("input", function(){ + disableCheckboxesWithInvalidPreferences($(this), "phone"); + }); + $("#phone, #phonepro, #B_phone").on("change", function(){ + $(this).val($.trim($(this).val())); + disableCheckboxesWithInvalidPreferences($(this), "phone"); + }); [% END %] + var mrform = $("#manual_restriction_form"); var mrlink = $("#add_manual_restriction"); mrform.hide(); @@ -215,6 +238,7 @@ //]]> + [% INCLUDE 'header.inc' %] @@ -1158,6 +1182,13 @@ [% END %] + [% IF ( ValidateEmailAddress ) %] +

is required in order to set Email preferences.

+ [% END %] + [% IF ( ValidatePhoneNumber ) %] + [% IF ( TalkingTechItivaPhone ) %]

is required in order to set Phone preferences.

[% END %] + [% IF ( SMSSendDriver ) %]

is required in order to set SMS preferences.

[% END %] + [% END %] [% INCLUDE 'messaging-preference-form.inc' %] [% IF ( SMSSendDriver ) %]

diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt index c2d3885..047f5e2 100644 --- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt +++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt @@ -24,6 +24,9 @@

Your messaging settings

+ [% IF ( ValidateEmailAddress ) %]

Primary email

is required in order to set Email preferences.
[% END %] + [% IF ( TalkingTechItivaPhone AND ValidatePhoneNumber ) %]

Primary phone

is required in order to set Phone preferences.
[% END %] + [% IF ( SMSSendDriver AND ValidatePhoneNumber ) %]

SMS number

is required in order to set SMS preferences.
[% END %] [% IF ( settings_updated ) %]

Settings updated

[% END %] @@ -201,14 +204,30 @@ } }); + [% IF ( ValidateEmailAddress ) %] + [% IF !BORROWER_INF.email %] + disableCheckboxesWithInvalidPreferences($("#email"), "email"); + [% END %] + [% END %] [% IF ( ValidatePhoneNumber ) %] - $("#SMSnumber").on("change", function(){ + [% IF !BORROWER_INF.smsalertnumber %] + disableCheckboxesWithInvalidPreferences($("#SMSnumber"), "sms"); + [% END %] + [% IF !BORROWER_INF.phone %] + disableCheckboxesWithInvalidPreferences($("#phone"), "phone"); + [% END %] + + $("#SMSnumber").on("input", function(){ + disableCheckboxesWithInvalidPreferences($(this), "sms"); + }); + $("#SMSnumber").on("change", function(){ $(this).val($.trim($(this).val())); - }); + disableCheckboxesWithInvalidPreferences($(this), "sms"); + }); [% END %] - }); //]]> + [% END %] diff --git a/koha-tmpl/opac-tmpl/bootstrap/js/messaging-preference.js b/koha-tmpl/opac-tmpl/bootstrap/js/messaging-preference.js new file mode 100644 index 0000000..bd28774 --- /dev/null +++ b/koha-tmpl/opac-tmpl/bootstrap/js/messaging-preference.js @@ -0,0 +1,91 @@ +/** + * + * Used in: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-messaging.tt + * + * This component is also used in Staff client: koha-tmpl/intranet-tmpl/prog/en/modules/memberentrygen.tt + * For modifications, edit also Staff client version in: koha-tmpl/intranet-tmpl/prog/en/js/messaging-preference.js + * + * Disables and clears checkboxes from messaging preferences + * if there is either invalid or nonexistent contact information + * for the message transfer type. + * + * @param {HTMLInputElement} elem + * The contact field + * @param {string} id_attr + * Checkboxes' id attribute so that we can recognize them + */ +// Settings for messaging-preference.js +var patron_messaging_checkbox_preferences = { + email: { + checked_checkboxes: null, + is_enabled: true, + disabled_checkboxes: null + }, + sms: { + checked_checkboxes: null, + is_enabled: true, + disabled_checkboxes: null + }, + phone: { + checked_checkboxes: null, + is_enabled: true, + disabled_checkboxes: null + } +}; + +function disableCheckboxesWithInvalidPreferences(elem, id_attr) { + // Get checkbox preferences for the element + var checkbox_prefs = eval("patron_messaging_checkbox_preferences." + id_attr); + // Check if element is empty or not valid + + if (!$(elem).length || $(elem).val().length == 0 ||  !$(elem).valid()) { + + if (checkbox_prefs.is_enabled) { + // Save the state of checked checkboxes + checkbox_prefs.checked_checkboxes = $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]:checked"); + + // Save the state of automatically disabled checkboxes + // (We don't want to enable them once the e-mail is valid!) + checkbox_prefs.disabled_checkboxes = $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]:disabled"); + + // Clear patron messaging preferences checkboxes + $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]").removeAttr("checked"); + + // Then disable checkboxes from patron messaging perferences + $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]").attr("disabled", "disabled"); + + // Color table cell's background emphasize the disabled state of the checkbox + $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]").parent().css("background-color", "#E8F0F8"); + + // Show notice about missing contact in messaging preferences box + $("#required-" + id_attr).css("display", "block"); + + checkbox_prefs.is_enabled = false; + } + } else { + + if (!checkbox_prefs.is_enabled) { + // Enable patron messaging preferences checkboxes + $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]").removeAttr("disabled"); + + // Disable the checkboxes that were disabled by default + checkbox_prefs.disabled_checkboxes.each(function() { + $(this).attr("disabled", "disabled"); + $(this).removeAttr("checked"); + }); + + // Restore the state of checkboxes + checkbox_prefs.checked_checkboxes.each(function() { + $(this).attr("checked", "checked"); + }); + + // Remove the background color from table cell + $("input[type='checkbox'][id^=" + id_attr + "][value=" + id_attr + "]").parent().css("background-color", "#FFF"); + + // Remove notice about missing contact from messaging preferences box + $("#required-" + id_attr).css("display", "none"); + + checkbox_prefs.is_enabled = true; + } + } +} \ No newline at end of file diff --git a/members/memberentry.pl b/members/memberentry.pl index bf41631..675eb90 100755 --- a/members/memberentry.pl +++ b/members/memberentry.pl @@ -433,7 +433,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes); } if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) { - C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template); + C4::Form::MessagingPreferences::handle_form_action($input, \%data, $template); } } print scalar ($destination eq "circ") ? diff --git a/misc/maintenance/deleteMisconfiguredMessagingPrefs.pl b/misc/maintenance/deleteMisconfiguredMessagingPrefs.pl new file mode 100755 index 0000000..4b37706 --- /dev/null +++ b/misc/maintenance/deleteMisconfiguredMessagingPrefs.pl @@ -0,0 +1,215 @@ +#!/usr/bin/perl + +#----------------------------------- +# Copyright 2015 Vaara-kirjastot +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it under the +# terms of the GNU General Public License as published by the Free Software +# Foundation; either version 2 of the License, or (at your option) any later +# version. +# +# Koha is distributed in the hope that it will be useful, but WITHOUT ANY +# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR +# A PARTICULAR PURPOSE. See the GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License along +# with Koha; if not, write to the Free Software Foundation, Inc., +# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +#----------------------------------- + +use strict; +use warnings; +use C4::Context; +use DBI; +use Email::Valid; +use POSIX qw(strftime); +use Getopt::Long; +use C4::Members::Messaging; + +my $helptext = " +This script deletes misconfigured messaging preferences from the Koha database. A preference is +misconfigured if the user has no valid contact information for that type of transport method. +E.g user wants messages via e-mail while he has not provided an e-mail address. + +Options: + -h|--help Prints this help documentation. + -b|--backup:s Filename for backup file. Required in 'restore' mode and optional in 'delete' mode. + Default value is messaging-prefs-backup_. In 'delete' mode, if nothing + gets deleted, the file will not be created. + -d|--delete Activates the deletion mode (cannot be used simultaneously with mode 'restore'). + -r|--restore Activates the restore mode (cannot be used simultaneously with mode 'delete'). + -m|--methods=s{1,3} Optional. Specifies the transfer methods/types. The three types are: email phone sms. + If not provided, all three types are used. + +Examples: + ./deleteMisconfiguredMessagingPrefs.pl -d -b + ./deleteMisconfiguredMessagingPrefs.pl -d -t email phone -b + ./deleteMisconfiguredMessagingPrefs.pl -d -t email phone sms -b my_backup_file + ./deleteMisconfiguredMessagingPrefs.pl -r -b my_backup_file +\n"; + +my ($help, $delete, $filename, $restore, @methods); + +GetOptions( + 'h|help' => \$help, + 'd|delete' => \$delete, + 'r|restore' => \$restore, + 'b|backup:s' => \$filename, + 'm|methods=s{1,3}' => \@methods +); + +my ($sec,$min,$hour,$mday,$mon,$year,$wday,$yday,$isdst) = localtime(); +my $are_we_doing_backups = 0; + + +if ($help || $restore && $delete || !$restore && !$delete) { + die $helptext; +} +if ($delete and $filename eq "" or length($filename) > 0) { + $are_we_doing_backups = 1; + $filename = (strftime "messaging-prefs-backup_%Y-%m-%d_%H-%M-%S", localtime) if ($filename eq ""); + # first, make sure we can do backups + open(my $file, ">>", $filename) or die "Could not open file for writing.\n"; +} + + + + + + + + + + + + +if ($delete) { + my @deleted_prefs; + + # check if user has specified messaging transport methods + if (@methods > 0 && @methods < 4) { + my $correct_modes = 0; + + foreach my $type (@methods){ + + if ($type eq "email" or $type eq "phone" or $type eq "sms") { + print "Deleting misconfigured $type messaging preferences\n"; + + # method exists, don't need to print warning anymore + $correct_modes = 1; + + # which contact info field (primary email / primary phone / smsalertnumber) + # should we check? + my $contact; + $contact = "email" if $type eq "email"; + $contact = "phone" if $type eq "phone"; + $contact = "smsalertnumber" if $type eq "sms"; + + # which validator should we use? + my $validator; + $validator = "email" if $type eq "email"; + $validator = "phone" if $type eq "phone" or $type eq "sms"; + + # delete the misconfigured prefs and keep a counting them + push(@deleted_prefs, C4::Members::Messaging::DeleteMisconfiguredPreference($type, $contact, $validator)); + } + } + + die "Missing or invalid in parameter --type values. See help.\n$helptext" if $correct_modes == 0; + + } else { + + # user did not specify any methods. so let's delete them all! + print "Deleting all misconfigured messaging preferences\n"; + push(@deleted_prefs, C4::Members::Messaging::DeleteAllMisconfiguredPreferences()); + + } + + BackupDeletedPrefs(@deleted_prefs) if $are_we_doing_backups; + + print "Deleted ".scalar(@deleted_prefs)." misconfigured messaging preferences in total.\n"; +} +elsif ($restore){ + if (@methods > 0) { + my $correct_modes = 0; + for (my $i=0; $i < @methods; $i++){ + if ($methods[$i] ne "email" and $methods[$i] ne "phone" and $methods[$i] ne "sms") { + print "Invalid type $methods[$i]. Valid types are: email phone sms\n"; + splice(@methods, $i); + } + } + die "Missing parameter --type values. See help.\n$helptext" if @methods == 0; + RestoreDeletedPreferences(@methods); + } else { + RestoreDeletedPreferences(); + } +} + + + + + + + + + + + + + + + +# restoring deleted prefs + +sub BackupDeletedPrefs { + my @deleted = @_; + + open(my $fh, ">>", $filename) or die "Could not open file for writing.\n"; + + for (my $i=0; $i < @deleted; $i++){ + say $fh $deleted[$i][0].",".$deleted[$i][1]; + } +} +sub RestoreDeletedPreferences { + my $count = 0; + my @methods = @_; + + my $dbh = C4::Context->dbh(); + open(my $fh, "<", $filename) or die "Could not open file for writing.\n"; + + my $query = "INSERT INTO borrower_message_transport_preferences (borrower_message_preference_id, message_transport_type) VALUES (?,?)"; + my $sth = $dbh->prepare($query); + + # check if user has specified methods (types) + if (@methods > 0) { + while (my $line = <$fh>) { + my @vars = split(',',$line); + my @remlinebreak = split('\\n', $vars[1]); + my $pref_id = $vars[0]; + my $type = $remlinebreak[0]; + + if (grep(/$type/,@methods)) { + $sth->execute($pref_id, $type) or $count--; + $count++; + } + } + } else { + # simply walk through each line and restore every single line + while (my $line = <$fh>) { + my @vars = split(',',$line); + next if @vars == 0; + my @remlinebreak = split('\\n', $vars[1]); + next if not defined $remlinebreak[0]; + my $pref_id = $vars[0]; + my $type = $remlinebreak[0]; + + $sth->execute($pref_id, $type) or $count--; + $count++; + } + } + + print "Restored $count preferences.\n"; + +} diff --git a/opac/opac-messaging.pl b/opac/opac-messaging.pl index f2eeec8..8fe37d7 100755 --- a/opac/opac-messaging.pl +++ b/opac/opac-messaging.pl @@ -64,7 +64,7 @@ if ( defined $query->param('modify') && $query->param('modify') eq 'yes' ) { $borrower = GetMemberDetails( $borrowernumber ); } - C4::Form::MessagingPreferences::handle_form_action($query, { borrowernumber => $borrowernumber }, $template); + C4::Form::MessagingPreferences::handle_form_action($query, $borrower, $template); } C4::Form::MessagingPreferences::set_form_values({ borrowernumber => $borrower->{'borrowernumber'} }, $template); diff --git a/t/db_dependent/MessagingPreferencesValidation.t b/t/db_dependent/MessagingPreferencesValidation.t new file mode 100644 index 0000000..9b4780a --- /dev/null +++ b/t/db_dependent/MessagingPreferencesValidation.t @@ -0,0 +1,180 @@ +#!/usr/bin/env perl + +# Copyright 2015 Open Source Freedom Fighters +# +# This file is part of Koha. +# +# Koha is free software; you can redistribute it and/or modify it +# under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# Koha is distributed in the hope that it will be useful, but +# WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Koha; if not, see . +$ENV{KOHA_PAGEOBJECT_DEBUG} = 1; +use Modern::Perl; + +use Test::More; +use Try::Tiny; #Even Selenium::Remote::Driver uses Try::Tiny :) + +use Koha::Auth::PermissionManager; + +use t::lib::Page::Mainpage; +use t::lib::Page::Opac::OpacMain; +use t::lib::Page::Opac::OpacMemberentry; +use t::lib::Page::Members::Memberentry; +use t::lib::Page::Members::Moremember; + +use t::lib::TestObjects::BorrowerFactory; +use t::lib::TestObjects::SystemPreferenceFactory; + +##Setting up the test context +my $testContext = {}; + +my $password = '1234'; +my $borrowerFactory = t::lib::TestObjects::BorrowerFactory->new(); +my $borrowers = $borrowerFactory->createTestGroup([ + {firstname => 'Testone', + surname => 'Testtwo', + cardnumber => '1A01', + branchcode => 'CPL', + userid => 'normal_user', + password => $password, + }, + {firstname => 'Testthree', + surname => 'Testfour', + cardnumber => 'superuberadmin', + branchcode => 'CPL', + userid => 'god', + password => $password, + }, + ], undef, $testContext); + +my $systempreferences = t::lib::TestObjects::SystemPreferenceFactory->createTestGroup([ + {preference => 'ValidateEmailAddress', + value => 1 + }, + {preference => 'ValidatePhoneNumber', + value => 'ipn', + }, + {preference => 'TalkingTechItivaPhoneNotification', + value => 1 + }, + {preference => 'SMSSendDriver', + value => 'test' + }, + ], undef, $testContext); + +my $permissionManager = Koha::Auth::PermissionManager->new(); +$permissionManager->grantPermissions($borrowers->{'superuberadmin'}, {superlibrarian => 'superlibrarian'}); + +eval { + + # staff client + my $memberentry = t::lib::Page::Members::Memberentry->new({borrowernumber => $borrowers->{'superuberadmin'}->borrowernumber, op => 'modify', destination => 'circ', categorycode => 'PT'}); + # opac + my $main = t::lib::Page::Opac::OpacMain->new({borrowernumber => $borrowers->{'superuberadmin'}->borrowernumber}); + + # set valid contacts and check preferences checkboxes + $memberentry->doPasswordLogin($borrowers->{'superuberadmin'}->userid(), $password) + ->setEmail("valid\@email.com") + ->checkPreferences(1, "email") + ->setPhone("+3585012345678") + ->checkPreferences(1, "phone") + ->setSMSNumber("+3585012345678") + ->checkPreferences(1, "sms") + ->submitForm(1) # expecting success + ->navigateToDetails() + # make sure everything is now checked on moremember.pl details page + ->checkMessagingPreferencesSet(1, "email", "sms", "phone"); + + $main # check that they are also checked in OPAC + ->doPasswordLogin($borrowers->{'superuberadmin'}->userid(), $password) + ->navigateYourMessaging() + ->checkMessagingPreferencesSet(1, "email", "sms", "phone"); + + # go to edit patron and set invalid contacts. + $memberentry + ->navigateEditPatron() + ->setEmail("invalidemail.com") + ->checkPreferences(0, "email") + ->setPhone("+3585012asd345678") + ->checkPreferences(0, "phone") + ->setSMSNumber("+358501asd2345678") + ->checkPreferences(0, "sms") + # check messaging preferences: they should be unchecked + ->checkMessagingPreferencesSet(0, "email", "sms", "phone") + ->submitForm(0) # also confirm that we cant submit the preferences + ->navigateToDetails() + + # go to library use and just simply submit the form without any changes + ->navigateToLibraryUseEdit() + ->submitForm(1) + # all the preferences should be still set + ->navigateToDetails() + ->checkMessagingPreferencesSet(1, "email", "sms", "phone") + + # go to smsnumber edit and make sure everything is checked + ->navigateToSMSnumberEdit() + ->checkMessagingPreferencesSet(1, "email", "sms", "phone") + ->submitForm(1) + ->navigateToDetails() + ->checkMessagingPreferencesSet(1, "email", "sms", "phone") + + # go to patron information edit and clear email and phone + ->navigateToPatronInformationEdit() + ->clearMessagingContactFields("email", "phone") + ->submitForm(1) + ->navigateToDetails() + # this should remove our messaging preferences for phone and email + ->checkMessagingPreferencesSet(0, "email", "phone") + ->checkMessagingPreferencesSet(1, "sms"); # ... but not for sms (it's still set) + + $main # check the preferences also from OPAC + ->navigateYourMessaging() + ->checkMessagingPreferencesSet(0, "email", "phone") + ->checkMessagingPreferencesSet(1, "sms"); + + # go to smsnumber edit and see that email and phone are disabled + $memberentry + ->navigateToSMSnumberEdit() + ->checkMessagingPreferencesSet(0, "email", "phone") + ->clearMessagingContactFields("SMSnumber") # uncheck all sms preferences + ->submitForm(1) + ->navigateToDetails() + ->checkMessagingPreferencesSet(0, "email", "phone", "sms"); + + $main # check the preferences also from OPAC + ->navigateYourMessaging() + ->checkMessagingPreferencesSet(0, "email", "phone", "sms"); + +}; +if ($@) { #Catch all leaking errors and gracefully terminate. + warn $@; + tearDown(); + exit 1; +} + +##All tests done, tear down test context +tearDown(); +done_testing; + +sub tearDown { + t::lib::TestObjects::ObjectFactory->tearDownTestContext($testContext); +} + + + + + + + + +###################################################### + ### STARTING TEST IMPLEMENTATIONS ### +###################################################### \ No newline at end of file -- 1.9.1