From 131002e6230f2b69f0d6007b75fbc90444745b68 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Mon, 4 Jul 2016 15:03:18 +0100
Subject: [PATCH] Bug 16847: Remove C4::Members::GetTitles
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

This subroutine just reads the content of a pref, split it, add an
empty string and returns.
It is not really useful and the code in the script (memberentry.pl) uses
the only occurrence of CGI::popup_menu
Let's remove it and build the dropdown list in the template.

Test plan:
1/ Empty BorrowersTitles, edit a patron and confirm that the "Salutation"
does not appear
2/ Fill BorrowersTitles with "Mr|Mrs|Miss|Ms", edit a patron and confirm
that the "Salutation" dropdown list is correctly filled.
The default option should be selected if you are editing a patron with a
title defined.
This should also be tested at the OPAC.

Followed test pan, works as expected in Staff and OPAC
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
---
 C4/Members.pm                                 | 21 -------------------
 .../prog/en/modules/members/memberentrygen.tt | 13 ++++++++++--
 .../bootstrap/en/modules/opac-memberentry.tt  |  4 ++--
 members/memberentry.pl                        | 10 +--------
 opac/opac-memberentry.pl                      |  1 -
 5 files changed, 14 insertions(+), 35 deletions(-)

diff --git a/C4/Members.pm b/C4/Members.pm
index 53ca664248..061172fee9 100644
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -71,7 +71,6 @@ BEGIN {
 
         &GetAge
         &GetSortDetails
-        &GetTitles
 
         &GetHideLostItemsPreference
 
@@ -1640,26 +1639,6 @@ EOF
     return 0;
 }
 
-=head2 GetTitles (OUEST-PROVENCE)
-
-  ($borrowertitle)= &GetTitles();
-
-Looks up the different title . Returns array  with all borrowers title
-
-=cut
-
-sub GetTitles {
-    my @borrowerTitle = split (/,|\|/,C4::Context->preference('BorrowersTitles'));
-    unshift( @borrowerTitle, "" );
-    my $count=@borrowerTitle;
-    if ($count == 1){
-        return ();
-    }
-    else {
-        return ( \@borrowerTitle);
-    }
-}
-
 =head2 GetHideLostItemsPreference
 
   $hidelostitemspref = &GetHideLostItemsPreference($borrowernumber);
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
index 5dfd5a2745..20725bd8c8 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/memberentrygen.tt
@@ -254,7 +254,7 @@ $(document).ready(function() {
 		<ol>
 		[% UNLESS ( I ) %]
         [% UNLESS notitle %]
-		[% IF ( title_cgipopup ) %]
+        [% IF Koha.Preference('BorrowersTitles') %]
             <li>
             [% IF ( mandatorytitle ) %]
                 <label for="btitle" class="required">
@@ -262,7 +262,16 @@ $(document).ready(function() {
                 <label for="btitle">
             [% END %]
             Salutation: </label>
-            [% borrotitlepopup %]
+            <select id="btitle" name="title">
+                <option value=""></option>
+                [% FOREACH t IN Koha.Preference('BorrowersTitles').split('\|') %]
+                    [% IF btitle == t %]
+                        <option value="[% t %]" selected="selected">[% t %]</option>
+                    [% ELSE %]
+                        <option value="[% t %]">[% t %]</option>
+                    [% END %]
+                [% END %]
+            </select>
             [% IF ( mandatorytitle ) %]<span class="required">Required</span>[% END %]
             </li>
 		[% END %]
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
index b70ab9e75c..ca0358dfb5 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-memberentry.tt
@@ -203,7 +203,7 @@
                                 <legend id="identity_legend">Identity</legend>
 
                                 <ol>
-                                    [% UNLESS hidden.defined('title') || !member_titles %]
+                                    [% UNLESS hidden.defined('title') || !Koha.Preference('BorrowersTitles') %]
                                         <li>
                                             [% IF mandatory.defined('title') %]
                                                 <label for="borrower_title" class="required">
@@ -213,7 +213,7 @@
                                             Salutation:</label>
 
                                             <select id="borrower_title" name="borrower_title">
-                                                [% FOREACH mt IN member_titles %]
+                                                [% FOREACH mt IN Koha.Preference('BorrowersTitles').split('\|') %]
                                                     [% IF mt == borrower.title %]
                                                         <option value="[% mt %]" selected="selected">[% mt %]</option>
                                                     [% ELSE %]
diff --git a/members/memberentry.pl b/members/memberentry.pl
index 0987168886..32b2402258 100755
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -546,14 +546,6 @@ $template->param(
 
 my $default_borrowertitle = '';
 unless ( $op eq 'duplicate' ) { $default_borrowertitle=$data{'title'} }
-my($borrowertitle)=GetTitles();
-$template->param( title_cgipopup => 1) if ($borrowertitle);
-my $borrotitlepopup = CGI::popup_menu(-name=>'title',
-        -id => 'btitle',
-        -values=>$borrowertitle,
-        -override => 1,
-        -default=>$default_borrowertitle
-        );    
 
 my @relationships = split /,|\|/, C4::Context->preference('borrowerRelationship');
 my @relshipdata;
@@ -681,7 +673,7 @@ $template->param(
   borrowernumber  => $borrowernumber, #register number
   guarantorid => ($borrower_data->{'guarantorid'} || $guarantorid),
   relshiploop => \@relshipdata,
-  borrotitlepopup => $borrotitlepopup,
+  btitle=> $default_borrowertitle,
   guarantorinfo   => $guarantorinfo,
   flagloop  => \@flagdata,
   category_type =>$category_type,
diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl
index de157efbf4..b73ee1b0bc 100755
--- a/opac/opac-memberentry.pl
+++ b/opac/opac-memberentry.pl
@@ -68,7 +68,6 @@ $template->param(
     action            => $action,
     hidden            => GetHiddenFields( $mandatory, 'registration' ),
     mandatory         => $mandatory,
-    member_titles     => GetTitles() || undef,
     branches          => GetBranchesLoop(),
     OPACPatronDetails => C4::Context->preference('OPACPatronDetails'),
 );
-- 
2.39.5