Attachment 42051 Details for Bug 14683 – Bug 14683: [QA Follow-up] Mixup between mobile and smsalertnumber

[patch] Bug 14683: [QA Follow-up] Mixup between mobile and smsalertnumber

Bug-14683-QA-Follow-up-Mixup-between-mobile-and-sm.patch (text/plain), 3.44 KB, created by Marcel de Rooy on 2015-08-28 08:17:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcel de Rooy

Created: 2015-08-28 08:17:50 UTC

Size: 3.44 KB

patch

obsolete

>From 95ac051a7e4a45852e900c3694e7ee115a822c2c Mon Sep 17 00:00:00 2001
>From: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>Date: Fri, 28 Aug 2015 09:13:09 +0200
>Subject: [PATCH] Bug 14683: [QA Follow-up] Mixup between mobile and
> smsalertnumber
>Content-Type: text/plain; charset=utf-8
>
>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 <m.de.rooy@rijksmuseum.nl>
>---
> 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 b3b192c..622f9ce 100755
>--- a/opac/opac-messaging.pl
>+++ b/opac/opac-messaging.pl
>@@ -49,10 +49,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 );
>     }
> 
>@@ -61,10 +61,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") );
> 
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 14683: 41664 | 42050 | 42051 | 42162 | 42187 | 42188 | 42189