From 95e476ecb6b8a5f029631193b40d39f79a5d5ee5 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Tue, 14 Dec 2021 15:35:25 +0100
Subject: [PATCH] Bug 29695: Remove GetColumnDefs

C4::Templates::GetColumnDefs can easily be replaced using the new
Koha::Database::Columns module.

Test plan:
Go to the import patron tool and confirm that you see the same list of
patron's attribute on the import patron form.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
---
 C4/Templates.pm                               | 47 -------------------
 .../prog/en/modules/tools/import_borrowers.tt | 29 ++++++------
 .../prog/js/pages/preferences.js              |  2 +-
 t/db_dependent/Templates.t                    | 35 ++++++--------
 tools/import_borrowers.pl                     |  6 +--
 5 files changed, 31 insertions(+), 88 deletions(-)

diff --git a/C4/Templates.pm b/C4/Templates.pm
index d41c9b534e..9658c51117 100644
--- a/C4/Templates.pm
+++ b/C4/Templates.pm
@@ -348,51 +348,4 @@ sub getlanguagecookie {
     return $cookie;
 }
 
-=head2 GetColumnDefs
-
-    my $columns = GetColumnDefs( $cgi )
-
-It is passed a CGI object and returns a hash of hashes containing
-the column names and descriptions for each table defined in the
-columns.def file corresponding to the CGI object.
-
-=cut
-
-sub GetColumnDefs {
-
-    my $query = shift;
-
-    my $columns = {};
-
-    my $htdocs = C4::Context->config('intrahtdocs');
-    my $columns_file = 'columns.def';
-
-    # Get theme and language to build the path to columns.def
-    my ($theme, $lang, $availablethemes) =
-        themelanguage($htdocs, 'about.tt', 'intranet', $query);
-    # Build columns.def path
-    my $path = "$htdocs/$theme/$lang/$columns_file";
-    my $fh;
-    if ( ! open ( $fh, q{<:encoding(utf-8)}, $path ) )  {
-        carp "Error opening $path. Check your templates.";
-        return;
-    }
-    # Loop through the columns.def file
-    while ( my $input = <$fh> ){
-        chomp $input;
-        if ( $input =~ m|<field name="(.*)">(.*)</field>| ) {
-            my ( $table, $column ) =  split( '\.', $1);
-            my $description        = $2;
-            # Initialize the table array if needed.
-            @{$columns->{ $table }} = () if ! defined $columns->{ $table };
-            # Push field and description
-            push @{$columns->{ $table }},
-                { field => $column, description => $description };
-        }
-    }
-    close $fh;
-
-    return $columns;
-}
-
 1;
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt
index a0bb8409b7..b11e4617df 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt
@@ -246,36 +246,35 @@
             </legend>
 
             <ol class="default_values" style="display:none;">
-                [% FOREACH borrower_field IN borrower_fields %]
-
-                    [% SWITCH borrower_field.field %]
+                [% FOREACH borrower_db_column IN borrower_fields.keys.sort %]
+                    [% SWITCH borrower_db_column %]
                     [% CASE 'branchcode' %]
                         <li>
-                            <label class="description" for="branchcode">[% borrower_field.description | html %]: </label>
+                            <label class="description" for="branchcode">[% borrower_fields.$borrower_db_column | html %]: </label>
                             <select id="branchcode" name="branchcode">
                                 <option value="" selected="selected"></option>
                                 [% FOREACH library IN Branches.all() %]
                                     <option value="[% library.branchcode | html %]">[% library.branchname | html %]</option>
                                 [% END %]
                             </select>
-                            <span class="field_hint">[% borrower_field.field | html %]</span>
+                            <span class="field_hint">[% borrower_db_column | html %]</span>
                         </li>
                     [% CASE 'categorycode' %]
                         <li>
-                            <label class="description" for="categorycode">[% borrower_field.description | html %]: </label>
+                            <label class="description" for="categorycode">[% borrower_fields.$borrower_db_column | html %]: </label>
                             <select id="categorycode" name="categorycode">
                                 <option value="" selected="selected"></option>
                                 [% FOREACH category IN categories %]
                                     <option value="[% category.categorycode | html %]">[% category.description | html %]</option>
                                 [% END %]
                             </select>
-                            <span class="field_hint">[% borrower_field.field | html %]</span>
+                            <span class="field_hint">[% borrower_db_column | html %]</span>
                         </li>
                     [% CASE %]
                         <li>
-                            <label class="description" for="[% borrower_field.field | html %]">[% borrower_field.description | html %]: </label>
-                            <input id="[% borrower_field.field | html %]" name="[% borrower_field.field | html %]" type="text" />
-                            <span class="field_hint">[% borrower_field.field | html %]</span>
+                            <label class="description" for="[% borrower_db_column| html %]">[% borrower_fields.$borrower_db_column | html %]: </label>
+                            <input id="[% borrower_db_column | html %]" name="[% borrower_db_column | html %]" type="text" />
+                            <span class="field_hint">[% borrower_db_column | html %]</span>
                         </li>
                     [% END %]
                 [% END %]
@@ -298,11 +297,11 @@
 
             <ol class="preserve_values" style="display:none;">
                 <div class="hint">Selected fields will be preserved from original patron record when overwriting existing patron.</div>
-                [% FOREACH borrower_field IN borrower_fields %]
+                [% FOREACH borrower_db_column IN borrower_fields.keys.sort %]
                         <li>
-                            <label class="description" for="preserve_existing_[% borrower_field.field | html %]">[% borrower_field.description | html %]: </label>
-                            <input name="preserve_existing" id="preserve_existing_[%  borrower_field.field | html %]" value="[%  borrower_field.field | html %]" type="checkbox">
-                            <span class="field_hint">[% borrower_field.field | html %]</span>
+                            <label class="description" for="preserve_existing_[% borrower_db_column | html %]">[% borrower_fields.$borrower_db_column | html %]: </label>
+                            <input name="preserve_existing" id="preserve_existing_[%  borrower_db_column | html %]" value="[%  borrower_db_column | html %]" type="checkbox">
+                            <span class="field_hint">[% borrower_db_column | html %]</span>
                         </li>
                 [% END %]
             </ol>
@@ -364,7 +363,7 @@
             OR choose which fields you want to supply from the following list:
             <ul>
                 <li>
-                    [% FOREACH columnkey IN borrower_fields %]'[% columnkey.field | html %]', [% END %]
+                    [% FOREACH columnkey IN borrower_fields.keys.sort %]'[% columnkey | html %]', [% END %]
                 </li>
             </ul>
         </li>
diff --git a/koha-tmpl/intranet-tmpl/prog/js/pages/preferences.js b/koha-tmpl/intranet-tmpl/prog/js/pages/preferences.js
index 75a6587ac5..b60fbe57c2 100644
--- a/koha-tmpl/intranet-tmpl/prog/js/pages/preferences.js
+++ b/koha-tmpl/intranet-tmpl/prog/js/pages/preferences.js
@@ -256,7 +256,7 @@ $( document ).ready( function () {
                     style = "";
                     disabled = "";
                 }
-                items.push('<label class="' + style +'"><input class="dbcolumn_selection" type="checkbox" id="' + key + '"' + checked + disabled + ' name="pref" value="' + val + '" /> ' + key + '</label>');
+                items.push('<label class="' + style +'"><input class="dbcolumn_selection" type="checkbox" id="' + key + '"' + checked + disabled + ' name="pref" value="' + key + '" /> ' + val + ' (' + key + ')</label>');
             });
             $("<div/>", {
                 "class": "columns-2",
diff --git a/t/db_dependent/Templates.t b/t/db_dependent/Templates.t
index b6c3e7e4ec..ebab637078 100755
--- a/t/db_dependent/Templates.t
+++ b/t/db_dependent/Templates.t
@@ -19,7 +19,7 @@ use Modern::Perl;
 
 use CGI;
 
-use Test::More tests => 8;
+use Test::More tests => 5;
 use Test::Deep;
 use Test::MockModule;
 use Test::Warn;
@@ -30,28 +30,21 @@ use t::lib::Mocks;
 use C4::Auth qw( get_template_and_user );
 
 BEGIN {
-    use_ok('C4::Templates', qw( GetColumnDefs getlanguagecookie setlanguagecookie themelanguage gettemplate param output availablethemes badtemplatecheck ));
-    can_ok( 'C4::Templates',
-         qw/ GetColumnDefs
-             getlanguagecookie
-             setlanguagecookie
-             themelanguage
-             gettemplate
-             _get_template_file
-             param
-             output /);
+    use_ok('C4::Templates', qw( getlanguagecookie setlanguagecookie themelanguage gettemplate param output availablethemes badtemplatecheck ));
+    can_ok(
+        'C4::Templates',
+        qw(
+          getlanguagecookie
+          setlanguagecookie
+          themelanguage
+          gettemplate
+          _get_template_file
+          param
+          output
+          )
+    );
 }
 
-my $query   = CGI->new();
-my $columns = C4::Templates::GetColumnDefs( $query );
-
-is( ref( $columns ) eq 'HASH', 1, 'GetColumnDefs returns a hashref' );
-# get the tables names, sorted
-my @keys = sort keys %{$columns};
-is( scalar @keys, 6, 'GetColumnDefs correctly returns the 5 tables defined in columns.def' );
-my @tables = qw( biblio biblioitems borrowers items statistics subscription );
-cmp_deeply( \@keys, \@tables, 'GetColumnDefs returns the expected tables');
-
 subtest 'Testing themelanguage' => sub {
     plan tests => 12;
     my $testing_language;
diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
index 949f40261d..1241306421 100755
--- a/tools/import_borrowers.pl
+++ b/tools/import_borrowers.pl
@@ -38,7 +38,7 @@ use Modern::Perl;
 
 use C4::Auth qw( get_template_and_user );
 use C4::Output qw( output_and_exit output_html_with_http_headers );
-use C4::Templates;
+use Koha::Database::Columns;
 use Koha::Patrons;
 use Koha::DateUtils qw( dt_from_string );
 use Koha::Token;
@@ -78,9 +78,7 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
 # get the patron categories and pass them to the template
 my @patron_categories = Koha::Patron::Categories->search_with_library_limits({}, {order_by => ['description']});
 $template->param( categories => \@patron_categories );
-my $columns = C4::Templates::GetColumnDefs( $input )->{borrowers};
-$columns = [ grep { $_->{field} ne 'borrowernumber' ? $_ : () } @$columns ];
-$template->param( borrower_fields => $columns );
+$template->param( borrower_fields => Koha::Database::Columns->columns->{borrowers} );
 
 if ( $input->param('sample') ) {
     our $csv = Text::CSV->new( { binary => 1 } );    # binary needed for non-ASCII Unicode
-- 
2.20.1