Attachment 42464 Details for Bug 14590 – [Signed-off] Bug 14590 - Checkboxes should be disabled without valid contact information in messaging preferences

[patch] [Signed-off] Bug 14590 - Checkboxes should be disabled without valid contact information in messaging preferences

Signed-off-Bug-14590---Checkboxes-should-be-disabl.patch (text/plain), 42.37 KB, created by Marc Véron on 2015-09-09 13:11:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marc Véron

Created: 2015-09-09 13:11:35 UTC

Size: 42.37 KB

patch

obsolete

>From cd1b8f2af23ceb598f57ce4dea6e7b21fbcee1d4 Mon Sep 17 00:00:00 2001
>From: Lari Taskula <larit@student.uef.fi>
>Date: Thu, 13 Aug 2015 16:52:46 +0300
>Subject: [PATCH] [Signed-off] Bug 14590 - Checkboxes should be disabled
> without valid contact information in messaging preferences
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>This patch makes both client and server side validations, both in OPAC and Staff client.
>
>Followed test plan (comment #9), works as expected
>Signed-off-by: Marc VÃ©ron <veron@veron.ch>
>---
> C4/Form/MessagingPreferences.pm                    |   34 +++-
> 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, 822 insertions(+), 24 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..e868490 100644
>--- a/C4/Form/MessagingPreferences.pm
>+++ b/C4/Form/MessagingPreferences.pm
>@@ -77,9 +77,38 @@ 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 }, 
>+        my $updater = { borrowernumber          => $target_params->{'borrowernumber'},
>                         message_attribute_id    => $option->{'message_attribute_id'} };
>         
>+        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 +137,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 0b8e7d7..aff08c2 100644
>--- a/C4/Members.pm
>+++ b/C4/Members.pm
>@@ -656,6 +656,7 @@ sub ModMember {
>             $data{password} = hash_password($data{password});
>         }
>     }
>+
>     my $old_categorycode = GetBorrowerCategorycode( $data{borrowernumber} );
> 
>     # get only the columns of a borrower
>@@ -689,6 +690,14 @@ sub ModMember {
>             }
>         }
> 
>+        # 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})) 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 2a34c2e..c41aa84 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();
>@@ -214,6 +237,7 @@
> //]]>
> </script>
> <script type="text/javascript" src="[% themelang %]/js/members.js"></script>
>+<script type="text/javascript" src="[% themelang %]/js/messaging-preference.js"></script>
> </head>
> <body id="pat_memberentrygen" class="pat">
> [% INCLUDE 'header.inc' %]
>@@ -1157,6 +1181,13 @@
>     </script>
>     [% END %]
>     <input type="hidden" name="setting_messaging_prefs" value="1" />
>+    [% IF ( ValidateEmailAddress ) %]
>+    <p id="required-email" style="[% IF email %]display:none;[% END %]"><label for="email">Primary email</label> is required in order to set Email preferences.</p>
>+    [% END %]
>+    [% IF ( ValidatePhoneNumber ) %]
>+    [% IF ( TalkingTechItivaPhone ) %]<p id="required-phone" style="[% IF phone %]display:none;[% END %]"><label for="phone">Primary phone</label> is required in order to set Phone preferences.</p>[% END %]
>+    [% IF ( SMSSendDriver ) %]<p id="required-sms" style="[% IF smsalertnumber %]display:none;[% END %]"><label for="SMSnumber">SMS Number</label> is required in order to set SMS preferences.</p>[% END %]
>+    [% END %]
>     [% INCLUDE 'messaging-preference-form.inc' %]
>     [% IF ( SMSSendDriver ) %]
>         <p><label for="SMSnumber">SMS number:</label>
>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 @@
>             <div class="span10">
>                 <div id="usermessaging">
>                     <h3>Your messaging settings</h3>
>+                        [% IF ( ValidateEmailAddress ) %]<div id="required-email" class="alert" style="[% IF BORROWER_INF.email %]display:none;[% END %]"><h4>Primary email</h4> is required in order to set Email preferences.</div>[% END %]
>+                        [% IF ( TalkingTechItivaPhone AND ValidatePhoneNumber ) %]<div id="required-phone" class="alert" style="[% IF BORROWER_INF.phone %]display:none;[% END %]"><h4>Primary phone</h4> is required in order to set Phone preferences.</div>[% END %]
>+                        [% IF ( SMSSendDriver AND ValidatePhoneNumber ) %]<div id="required-sms" class="alert" style="[% IF BORROWER_INF.smsalertnumber %]display:none;[% END %]"><h4>SMS number</h4> is required in order to set SMS preferences.</div>[% END %]
>                     [% IF ( settings_updated ) %]
>                         <div class="alert alert-success"><h4>Settings updated</h4></div>
>                     [% 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 %]
>-
>   });
> //]]>
> </script>
> <script type="text/javascript" src="/opac-tmpl/bootstrap/lib/jquery/plugins/jquery.validate.min.js"></script>
>+<script type="text/javascript" src="[% interface %]/[% theme %]/js/messaging-preference.js"></script>
> [% 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 4d98a55..6991c97 100755
>--- a/members/memberentry.pl
>+++ b/members/memberentry.pl
>@@ -437,7 +437,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_<date and time>. 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 7e23cd7..dd7cb3a 100755
>--- a/opac/opac-messaging.pl
>+++ b/opac/opac-messaging.pl
>@@ -65,7 +65,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 <http://www.gnu.org/licenses>.
>+$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.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 14590: 41496 | 41497 | 41526 | 41528 | 41700 | 41701 | 41884 | 41991 | 42457 | 42464 | 44704 | 62040 | 62043 | 62044 | 62071