From 224a0a1bac9d8510894e97e9c1e61b747e693796 Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Wed, 20 May 2009 11:35:52 -0500 Subject: [PATCH] bug 3222: new module to handle messaging preferences form Define and use a new module, C4::Form::MessagingPreferences, to handle displaying and processing the messaging preferences form. This change reduces code duplication between OPAC and staff. Signed-off-by: Daniel Sweeney Signed-off-by: Galen Charlton --- C4/Form/MessagingPreferences.pm | 166 ++++++++++++++++++++++++++++++++ members/messaging.pl | 58 +---------- opac/opac-messaging.pl | 59 +----------- 3 files changed, 175 insertions(+), 108 deletions(-) create mode 100755 C4/Form/MessagingPreferences.pm diff --git a/C4/Form/MessagingPreferences.pm b/C4/Form/MessagingPreferences.pm new file mode 100755 index 0000000000..264158e054 --- /dev/null +++ b/C4/Form/MessagingPreferences.pm @@ -0,0 +1,166 @@ +package C4::Form::MessagingPreferences; + +# Copyright 2008-2009 LibLime +# +# 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., 59 Temple Place, +# Suite 330, Boston, MA 02111-1307 USA + +use strict; +use warnings; + +use CGI; +use C4::Context; +use C4::Members::Messaging; +use C4::Debug; + +=head1 NAME + +C4::Form::MessagingPreferences - manage messaging prefernces form + +=head1 SYNOPSIS + +In script: + + use C4::Form::MessagingPreferences; + C4::Form::MessagingPreferences::set_form_value({ borrowernumber => 51 }, $template); + C4::Form::MessagingPreferences::handle_form_action($input, { categorycode => 'CPL' }, $template); + +In HTML template: + + + +=head1 DESCRIPTION + +This module manages input and output for the messaging preferences form +that is used in the staff patron editor, the staff patron category editor, +and the OPAC patron messaging prefereneces form. It in its current form, +it essentially serves only to eliminate copy-and-paste code, but suggests +at least one approach for reconciling functionality that does mostly +the same thing in staff and OPAC. + +=head1 FUNCTIONS + +=head2 handle_form_action + + C4::Form::MessagingPreferences::handle_form_action($input, { categorycode => 'CPL' }, $template); + +Processes CGI parameters and updates the target patron or patron category's +preferences. + +C<$input> is the CGI query object. + +C<$target_params> is a hashref containing either a C key or a C key +identifying the patron or patron category whose messaging preferences are to be updated. + +C<$template> is the HTML::Template::Pro object for the response; this routine +adds a settings_updated template variable. + +=cut + +sub handle_form_action { + my ($query, $target_params, $template) = @_; + my $messaging_options = C4::Members::Messaging::GetMessagingOptions(); + + # TODO: If a "NONE" box and another are checked somehow (javascript failed), we should pay attention to the "NONE" box + + OPTION: foreach my $option ( @$messaging_options ) { + my $updater = { %{ $target_params }, + message_attribute_id => $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'}; + + if ( $option->{'has_digest'} ) { + if ( List::Util::first { $_ == $option->{'message_attribute_id'} } $query->param( 'digest' ) ) { + $updater->{'wants_digest'} = 1; + } + } + + if ( $option->{'takes_days'} ) { + if ( defined $query->param( $option->{'message_attribute_id'} . '-DAYS' ) ) { + $updater->{'days_in_advance'} = $query->param( $option->{'message_attribute_id'} . '-DAYS' ); + } + } + + C4::Members::Messaging::SetMessagingPreference( $updater ); + } + + # show the success message + $template->param( settings_updated => 1 ); +} + +=head2 set_form_values + + C4::Form::MessagingPreferences::set_form_value({ borrowernumber => 51 }, $template); + +Retrieves the messaging preferences for the specified patron or patron category +and fills the corresponding template variables. + +C<$target_params> is a hashref containing either a C key or a C key +identifying the patron or patron category. + +C<$template> is the HTML::Template::Pro object for the response. + +=cut + +sub set_form_values { + my ($target_params, $template) = @_; + # walk through the options and update them with these borrower_preferences + my $messaging_options = C4::Members::Messaging::GetMessagingOptions(); + PREF: foreach my $option ( @$messaging_options ) { + my $pref = C4::Members::Messaging::GetMessagingPreferences( { %{ $target_params }, + message_name => $option->{'message_name'} } ); + # make a hashref of the days, selecting one. + if ( $option->{'takes_days'} ) { + my $days_in_advance = $pref->{'days_in_advance'} ? $pref->{'days_in_advance'} : 0; + @{$option->{'select_days'}} = map {; { + day => $_, + selected => $_ == $days_in_advance ? 'selected="selected"' :'' } + } ( 0..30 ); # FIXME: 30 is a magic number. + } + foreach my $transport ( @{$pref->{'transports'}} ) { + $option->{'transport-'.$transport} = 'checked="checked"'; + } + $option->{'digest'} = 'checked="checked"' if $pref->{'wants_digest'}; + } + $template->param(messaging_preferences => $messaging_options); +} + +=head1 TODO + +=over 4 + +=item Reduce coupling between processing CGI parameters and updating the messaging preferences + +=item Handle when form input is invalid + +=item Generalize into a system of form handler clases + +=back + +=head1 SEE ALSO + +L, F, F, F + +=head1 AUTHOR + +Koha Developement team + +Galen Charlton refactoring code by Andrew Moore. + +=cut + +1; diff --git a/members/messaging.pl b/members/messaging.pl index 3b598cade7..90d154231c 100755 --- a/members/messaging.pl +++ b/members/messaging.pl @@ -33,6 +33,7 @@ use C4::Letters; use C4::Biblio; use C4::Reserves; use C4::Branch; # GetBranchName +use C4::Form::MessagingPreferences; use Data::Dumper; @@ -64,9 +65,6 @@ $template->param( $borrower ); my $borrower = GetMemberDetails( $borrowernumber ); -my $messaging_options = C4::Members::Messaging::GetMessagingOptions(); -my $messaging_preferences; - if ( defined $query->param('modify') && $query->param('modify') eq 'yes' ) { # If they've modified the SMS number, record it. @@ -75,54 +73,10 @@ if ( defined $query->param('modify') && $query->param('modify') eq 'yes' ) { smsalertnumber => $query->param('SMSnumber') ); $borrower = GetMemberDetails( $borrowernumber ); } - - # TODO: If a "NONE" box and another are checked somehow (javascript failed), we should pay attention to the "NONE" box - - # warn( Data::Dumper->Dump( [ $messaging_options ], [ 'messaging_options' ] ) ); - OPTION: foreach my $option ( @$messaging_options ) { - # warn( Data::Dumper->Dump( [ $option ], [ 'option' ] ) ); - my $updater = { borrowernumber => $borrower->{'borrowernumber'}, - message_attribute_id => $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'}; - - if ( $option->{'has_digest'} ) { - if ( List::Util::first { $_ == $option->{'message_attribute_id'} } $query->param( 'digest' ) ) { - $updater->{'wants_digest'} = 1; - } - } - - if ( $option->{'takes_days'} ) { - if ( defined $query->param( $option->{'message_attribute_id'} . '-DAYS' ) ) { - $updater->{'days_in_advance'} = $query->param( $option->{'message_attribute_id'} . '-DAYS' ); - } - } - - #warn( 'calling SetMessaginPreferencse with ' . Data::Dumper->Dump( [ $updater ], [ 'updater' ] ) ); - C4::Members::Messaging::SetMessagingPreference( $updater ); - } - - # show the success message - $template->param( settings_updated => 1 ); + C4::Form::MessagingPreferences::handle_form_action($query, { borrowernumber => $borrowernumber }, $template); } -# walk through the options and update them with these borrower_preferences -PREF: foreach my $option ( @$messaging_options ) { - my $pref = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrower->{'borrowernumber'}, - message_name => $option->{'message_name'} } ); - #warn( Data::Dumper->Dump( [ $pref ], [ 'pref' ] ) ); - # make a hashref of the days, selecting one. - if ( $option->{'takes_days'} ) { - @{$option->{'select_days'}} = map {; { day => $_, - selected => $_ == $pref->{'days_in_advance'} ? 'SELECTED' :'' } } ( 0..30 ); # FIXME: 30 is a magic number. - } - foreach my $transport ( @{$pref->{'transports'}} ) { - $option->{'transport-'.$transport} = 'checked="checked"'; - } - $option->{'digest'} = 'checked="checked"' if $pref->{'wants_digest'}; -} +C4::Form::MessagingPreferences::set_form_values({ borrowernumber => $borrowernumber }, $template); if ( $borrower->{'category_type'} eq 'C') { my ( $catcodes, $labels ) = GetborCatFromCatType( 'A', 'WHERE category_type = ?' ); @@ -138,7 +92,6 @@ $template->param( picture => 1 ) if $picture; my $message_queue = C4::Letters::GetQueuedMessages( { borrowernumber => $query->param('borrowernumber') } ); $template->param( messagingview => 1, - messaging_preferences => [ $messaging_preferences ], message_queue => $message_queue, DHTMLcalendar_dateformat => C4::Dates->DHTMLcalendar(), borrowernumber => $borrowernumber, @@ -150,12 +103,11 @@ $template->param( messagingview => 1, SMSSendDriver => C4::Context->preference("SMSSendDriver") ); -$messaging_preferences->{'SMSnumber'}{'value'} = defined $borrower->{'smsalertnumber'} - ? $borrower->{'smsalertnumber'} : $borrower->{'mobile'}; +#$messaging_preferences->{'SMSnumber'}{'value'} = defined $borrower->{'smsalertnumber'} +# ? $borrower->{'smsalertnumber'} : $borrower->{'mobile'}; $template->param( BORROWER_INFO => [ $borrower ], messagingview => 1, - messaging_preferences => $messaging_options, is_child => ($borrower->{'category_type'} eq 'C'), SMSnumber => defined $borrower->{'smsalertnumber'} ? $borrower->{'smsalertnumber'} : $borrower->{'mobile'} ); diff --git a/opac/opac-messaging.pl b/opac/opac-messaging.pl index 3292e4405e..61c5a31ae0 100755 --- a/opac/opac-messaging.pl +++ b/opac/opac-messaging.pl @@ -31,6 +31,7 @@ use C4::Dates qw/format_date/; use C4::Members; use C4::Members::Messaging; use C4::Branch; +use C4::Form::MessagingPreferences; my $query = CGI->new(); @@ -57,66 +58,14 @@ if ( defined $query->param('modify') && $query->param('modify') eq 'yes' ) { $borrower = GetMemberDetails( $borrowernumber ); } - # TODO: If a "NONE" box and another are checked somehow (javascript failed), we should pay attention to the "NONE" box - - # warn( Data::Dumper->Dump( [ $messaging_options ], [ 'messaging_options' ] ) ); - OPTION: foreach my $option ( @$messaging_options ) { - # warn( Data::Dumper->Dump( [ $option ], [ 'option' ] ) ); - my $updater = { borrowernumber => $borrower->{'borrowernumber'}, - message_attribute_id => $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'}; - - if ( $option->{'has_digest'} ) { - if ( List::Util::first { $_ == $option->{'message_attribute_id'} } $query->param( 'digest' ) ) { - $updater->{'wants_digest'} = 1; - } - } - - if ( $option->{'takes_days'} ) { - if ( defined $query->param( $option->{'message_attribute_id'} . '-DAYS' ) ) { - $updater->{'days_in_advance'} = $query->param( $option->{'message_attribute_id'} . '-DAYS' ); - } - } - - # warn( 'calling SetMessaginPreferencse with ' . Data::Dumper->Dump( [ $updater ], [ 'updater' ] ) ); - C4::Members::Messaging::SetMessagingPreference( $updater ); - } - - # show the success message - $template->param( settings_updated => 1 ); -} - -$messaging_options = C4::Members::Messaging::GetMessagingOptions(); -# walk through the options and update them with these borrower_preferences -PREF: foreach my $option ( @$messaging_options ) { - my $pref = C4::Members::Messaging::GetMessagingPreferences( { borrowernumber => $borrower->{'borrowernumber'}, - message_name => $option->{'message_name'} } ); - # warn( Data::Dumper->Dump( [ $pref ], [ 'pref' ] ) ); - # make a hashref of the days, selecting one. - if ( $option->{'takes_days'} ) { - foreach my $day ( 0..30 ) { # FIXME: 30 is a magic number. - my $dayrow = { day => $day, - selected => '' }; - if ( defined $pref->{'days_in_advance'} && $pref->{'days_in_advance'} == $day ) { - $dayrow->{'selected'} = 'SELECTED'; - } - push @{$option->{'select_days'}}, $dayrow - } - } - foreach my $transport ( @{$pref->{'transports'}} ) { - $option->{'transport-'.$transport} = 'checked="checked"'; - } - - $option->{'digest'} = $pref->{'wants_digest'} ? 'checked="checked"' : ''; + C4::Form::MessagingPreferences::handle_form_action($query, { borrowernumber => $borrowernumber }, $template); } +C4::Form::MessagingPreferences::set_form_values({ borrowernumber => $borrower->{'borrowernumber'} }, $template); + # warn( Data::Dumper->Dump( [ $messaging_options ], [ 'messaging_options' ] ) ); $template->param( BORROWER_INFO => [ $borrower ], messagingview => 1, - messaging_preferences => $messaging_options, SMSnumber => defined $borrower->{'smsalertnumber'} ? $borrower->{'smsalertnumber'} : $borrower->{'mobile'}, SMSSendDriver => C4::Context->preference("SMSSendDriver") ); -- 2.39.5