From 9d94396f7596fe30a5cd84ee7ddc7e8ae3af59d2 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Thu, 17 Apr 2014 13:30:44 +0200 Subject: [PATCH] Bug 12100: ensure that messaging preferences displays saved Days in Advance If you have enhanced messaging preference, the Days in Advance combo value (in Patron Messaging Preferences) is saved in the database but not retrieved when you have not enabled the Email checkbox (or checkbox for any other transport) next to it. This patch does the following: [1] It replaces a JOIN by a LEFT JOIN that is the actual reason of the problem described. [2] Removes a FIXME by saving a hardcoded 30 into a constant. [3] Fixes a typo in the neighborhood. [4] Removes a superfluous comma in the map statement. [5] Simplifies code for the selected field of the days combo. It should just be a boolean. The text selected="selected" is in the template. Test plan: [1] Enable enhanced messaging preferences. [2] Fill in Days in advance for Advance notice but uncheck Email. [3] Save the preferences. [4] The member home screen does not display the number of days (until you decide to apply this patch :) Followed test plan. Works as expected. Signed-off-by: Marc Veron Signed-off-by: Kyle M Hall Signed-off-by: Galen Charlton --- C4/Form/MessagingPreferences.pm | 10 ++++++---- C4/Members/Messaging.pm | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/C4/Form/MessagingPreferences.pm b/C4/Form/MessagingPreferences.pm index fa3c303483..328c55e704 100644 --- a/C4/Form/MessagingPreferences.pm +++ b/C4/Form/MessagingPreferences.pm @@ -25,9 +25,11 @@ use C4::Context; use C4::Members::Messaging; use C4::Debug; +use constant MAX_DAYS_IN_ADVANCE => 30; + =head1 NAME -C4::Form::MessagingPreferences - manage messaging prefernces form +C4::Form::MessagingPreferences - manage messaging preferences form =head1 SYNOPSIS @@ -134,10 +136,10 @@ sub set_form_values { if ( $option->{'takes_days'} ) { my $days_in_advance = $pref->{'days_in_advance'} ? $pref->{'days_in_advance'} : 0; $option->{days_in_advance} = $days_in_advance; - @{$option->{'select_days'}} = map {; { + @{$option->{'select_days'}} = map { { day => $_, - selected => $_ == $days_in_advance ? 'selected="selected"' :'' } - } ( 0..30 ); # FIXME: 30 is a magic number. + selected => $_ == $days_in_advance } + } ( 0..MAX_DAYS_IN_ADVANCE ); } foreach my $transport ( keys %{$pref->{'transports'}} ) { $option->{'transports_'.$transport} = 1; diff --git a/C4/Members/Messaging.pm b/C4/Members/Messaging.pm index 033c6944c8..3cb6ae8244 100644 --- a/C4/Members/Messaging.pm +++ b/C4/Members/Messaging.pm @@ -74,7 +74,7 @@ LEFT JOIN borrower_message_transport_preferences ON borrower_message_transport_preferences.borrower_message_preference_id = borrower_message_preferences.borrower_message_preference_id LEFT JOIN message_attributes ON message_attributes.message_attribute_id = borrower_message_preferences.message_attribute_id -JOIN message_transports +LEFT JOIN message_transports ON message_transports.message_attribute_id = message_attributes.message_attribute_id AND message_transports.message_transport_type = borrower_message_transport_preferences.message_transport_type WHERE message_attributes.message_name = ? -- 2.39.5