From 20e74039a306dfb279b551f704b46708c95a96ec Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Fri, 28 Aug 2015 09:13:09 +0200 Subject: [PATCH] Bug 14683: [QA Follow-up] Mixup between mobile and smsalertnumber This is an issue discussed on older reports already in the past. Column mobile in borrowers is actually 'Other phone', not necessary a mobile number. The name of the field is confusing. (Renaming it is outside the scope of this report.) The field that we are editing here is smsalertnumber. It should not be compared with mobile at all. What could be the side-effect of this correction? === First, the change is only relevant for libraries with pref SMSSendDriver enabled. In the past patrons editing their message preferences saw mobile (read: other phone) in their smsalertnumber field (if the latter was still empty). If they saved it, it was copied to smsalertnumber. This change does not affect these patrons. They just have the same number in two columns. No big deal. What if a patron does not yet have a smsalertnumber? In that case no sms is sent in Letters.pm. So no change in behavior. If he submits opac-messaging now, he will no longer copy his other phone to smsalert [we cannot assume that it was mobile anyway!]. If he enters a mobile number, it will be saved correctly in the right field. Conclusion: this change will not break things or hurt anyone. It only prevents unwanted copying other phone to smsalertnumber. Also modified the compare to prevent uninitialized warnings. And removed a commented warn. Test plan: [1] Add, edit or delete the SMS number on opac-messaging regardless of the value of Other Phone (in the badly named mobile field). Signed-off-by: Marcel de Rooy Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi (cherry picked from commit 9b8d7168beb27342c4c483a0812e3a6789fabced) Signed-off-by: Chris Cormack (cherry picked from commit bc720464216eca9d5283596af23d2c2c926e7093) Signed-off-by: Liz Rea --- opac/opac-messaging.pl | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/opac/opac-messaging.pl b/opac/opac-messaging.pl index da0f86135e..a58540d7ae 100755 --- a/opac/opac-messaging.pl +++ b/opac/opac-messaging.pl @@ -50,10 +50,10 @@ my $borrower = GetMemberDetails( $borrowernumber ); my $messaging_options = C4::Members::Messaging::GetMessagingOptions(); if ( defined $query->param('modify') && $query->param('modify') eq 'yes' ) { - - if ( $query->param('SMSnumber') ne $borrower->{'mobile'} ) { + my $sms = $query->param('SMSnumber'); + if ( defined $sms && ( $borrower->{'smsalertnumber'} // '' ) ne $sms ) { ModMember( borrowernumber => $borrowernumber, - smsalertnumber => $query->param('SMSnumber') ); + smsalertnumber => $sms ); $borrower = GetMemberDetails( $borrowernumber ); } @@ -62,10 +62,9 @@ if ( defined $query->param('modify') && $query->param('modify') eq 'yes' ) { 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, - SMSnumber => $borrower->{'smsalertnumber'} ? $borrower->{'smsalertnumber'} : $borrower->{'mobile'}, + SMSnumber => $borrower->{'smsalertnumber'}, SMSSendDriver => C4::Context->preference("SMSSendDriver"), TalkingTechItivaPhone => C4::Context->preference("TalkingTechItivaPhoneNotification") ); -- 2.39.5