[Koha-patches] [PATCH 04/10] bug 3222: new module to handle messaging preferences form
Daniel Sweeney
daniel.sweeney at liblime.com
Wed May 20 18:35:52 CEST 2009
From: Galen Charlton <galen.charlton at liblime.com>
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 <daniel.sweeney at liblime.com>
---
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 0000000..264158e
--- /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:
+
+ <!-- TMPL_INCLUDE NAME="messaging-preference-form.inc" -->
+
+=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<categorycode> key or a C<borrowernumber> 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<categorycode> key or a C<borrowernumber> 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<C4::Members::Messaging>, F<admin/categorie.pl>, F<opac/opac-messaging.pl>, F<members/messaging.pl>
+
+=head1 AUTHOR
+
+Koha Developement team <info at koha.org>
+
+Galen Charlton <galen.charlton at liblime.com> refactoring code by Andrew Moore.
+
+=cut
+
+1;
diff --git a/members/messaging.pl b/members/messaging.pl
index 3b598ca..90d1542 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 3292e44..61c5a31 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") );
--
1.5.6.5
More information about the Koha-patches
mailing list