From 5ca9844dcdf6f36c94618d2c91aa1c3d1399ee4d Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Thu, 14 Mar 2024 09:58:05 +0100
Subject: [PATCH] Bug 29948: OPACAuthorIdentifiersAndInformation

This patch merge the previous patch attempt with the existing OPACAuthorIdentifiers syspref.

It creates a new syspref OPACAuthorIdentifiersAndInformation and remove
OPACAuthorIdentifiers.
"identifiers" become an entry of the new syspref.

Test plan:
Select some entries in OPACAuthorIdentifiersAndInformation
Edit an authority record and enter data in the corresponding subfield
(the list is on the syspref entry)
Go to the biblio detail page or the authority detail page at the OPAC
and notice that the info you selected are correctly displayed.

Sponsored-by: Orex Digital
---
 Koha/Authority.pm                             | 45 +++++++------------
 .../data/mysql/atomicupdate/bug_29948.pl      | 22 ++++++---
 .../en/modules/admin/preferences/opac.pref    | 15 +++----
 .../en/includes/authority-identifiers.inc     | 27 -----------
 .../en/includes/authority-information.inc     | 20 +++++++++
 .../bootstrap/en/modules/opac-auth-detail.tt  | 14 +++---
 .../bootstrap/en/modules/opac-detail.tt       | 21 ++-------
 opac/opac-authoritiesdetail.pl                |  6 +--
 opac/opac-detail.pl                           | 24 ++--------
 t/db_dependent/Koha/Authorities.t             | 27 ++++++-----
 10 files changed, 86 insertions(+), 135 deletions(-)
 delete mode 100644 koha-tmpl/opac-tmpl/bootstrap/en/includes/authority-identifiers.inc

diff --git a/Koha/Authority.pm b/Koha/Authority.pm
index 52f0b9127db..ed2ab678bd9 100644
--- a/Koha/Authority.pm
+++ b/Koha/Authority.pm
@@ -113,39 +113,15 @@ sub controlled_indicators {
     });
 }
 
-=head3 get_identifiers
+=head3 get_identifiers_and_information
 
-    my $identifiers = $author->get_identifiers;
+    my $information = $author->get_identifiers_and_information;
 
-Return a list of identifiers of the authors which are in 024$2$a
+Return a list of information of the authors (syspref OPACAuthorIdentifiersAndInformation)
 
 =cut
 
-sub get_identifiers {
-    my ( $self ) = @_;
-
-    my $record = $self->record;
-
-    my @identifiers;
-    for my $field ( $record->field('024') ) {
-        my $sf_2 = $field->subfield('2');
-        my $sf_a = $field->subfield('a');
-        next unless $sf_2 && $sf_a;
-        push @identifiers, {source => $sf_2, number => $sf_a, };
-    }
-
-    return \@identifiers;
-}
-
-=head3 get_information
-
-    my $information = $author->get_information;
-
-Return a list of information of the authors (syspref OPACAuthorInformation)
-
-=cut
-
-sub get_information {
+sub get_identifiers_and_information {
     my ($self) = @_;
 
     my $record = $self->record;
@@ -154,8 +130,17 @@ sub get_information {
     return if C4::Context->preference('marcflavour') eq 'UNIMARC';
 
     my $information;
-    for my $info ( split ',', C4::Context->preference('OPACAuthorInformation') ) {
-        if ( $info eq 'activity' ) {
+    for my $info ( split ',', C4::Context->preference('OPACAuthorIdentifiersAndInformation') ) {
+        if ( $info eq 'identifiers' ) {
+
+            # identifiers (024$2$a)
+            for my $field ( $record->field('024') ) {
+                my $sf_2 = $field->subfield('2');
+                my $sf_a = $field->subfield('a');
+                next unless $sf_2 && $sf_a;
+                push @{ $information->{identifiers} }, { source => $sf_2, number => $sf_a, };
+            }
+        } elsif ( $info eq 'activity' ) {
 
             # activity: Activity (372$a$s$t)
             for my $field ( $record->field('372') ) {
diff --git a/installer/data/mysql/atomicupdate/bug_29948.pl b/installer/data/mysql/atomicupdate/bug_29948.pl
index 3b8f0a682b3..1dc6362e605 100755
--- a/installer/data/mysql/atomicupdate/bug_29948.pl
+++ b/installer/data/mysql/atomicupdate/bug_29948.pl
@@ -1,16 +1,24 @@
 use Modern::Perl;
 
 return {
-    bug_number => "29948",
+    bug_number  => "29948",
     description => "Display author information for researchers",
-    up => sub {
+    up          => sub {
         my ($args) = @_;
-        my ($dbh, $out) = @$args{qw(dbh out)};
-        $dbh->do(q{
+        my ( $dbh, $out ) = @$args{qw(dbh out)};
+        my ($OPACAuthorIdentifiers) =
+            $dbh->selectrow_array(q{SELECT value FROM systempreferences WHERE variable="OPACAuthorIdentifiers"});
+        my $value = $OPACAuthorIdentifiers ? 'identifiers' : '';
+
+        $dbh->do(
+            q{
             INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES
-            ('OPACAuthorInformation','0','','Display author information on the OPAC detail page','multiple_sortable')
-        });
+            ('OPACAuthorIdentifiersAndInformation', ?, '', 'Display author information on the OPAC detail page','multiple_sortable')
+        }, undef, $value
+        );
+        say $out "Added new system preference 'OPACAuthorIdentifiersAndInformation'";
 
-        say $out "Added new system preference 'OPACAuthorInformation'";
+        $dbh->do(q{DELETE FROM systempreferences WHERE variable="OPACAuthorIdentifiers"});
+        say $out "  Removed system preference 'OPACAuthorIdentifiers'";
     },
 };
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
index e45f637a82f..14122b4b6aa 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
@@ -524,27 +524,22 @@ OPAC:
                   1: Enable
                   0: Disable
             - "interface for browsing all holdings (Elasticsearch only)."
-        -
-            - pref: OPACAuthorIdentifiers
-              default: 0
-              choices:
-                  1: Display
-                  0: "Don't display"
-            - "identifiers for authors and contributors to the detail pages in the OPAC."
-            - "This feature requires authorities with 024$2 and 024$a."
-            - "Valid source codes in $2 are currently: orcid, scopus, loop, rid and viaf."
         -
             - "Display the following information for authors and contributors to the detail pages in the OPAC."
-            - pref: OPACAuthorInformation
+            - pref: OPACAuthorIdentifiersAndInformation
               multiple:
                 activity: Activity (372$a$s$t)
                 address: Address (371$a$b$d$e)
                 associated_group: Associated group (373$a$s$t$u$v$0)
                 email_address: Electronic mail address (371$m)
+                identifiers: Identifiers (024$2$a)
                 occupation: Occupation (374$a$s$t$u$v$0)
                 place_of_birth: Place of birth (370$a)
                 place_of_death: Place of death (370$b)
                 uri: URI (371$u)
+            - "<p>Indentifiers: identifiers for authors and contributors to the detail pages in the OPAC."
+            - "This feature requires authorities with 024$2 and 024$a."
+            - "Valid source codes in $2 are currently: orcid, scopus, loop, rid and viaf.</p>"
         -
             - "Calculate the amount a patron has 'saved' by using the library based on replacement prices, and display:"
             - pref: OPACShowSavings
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/authority-identifiers.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/authority-identifiers.inc
deleted file mode 100644
index 85d68c09bec..00000000000
--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/authority-identifiers.inc
+++ /dev/null
@@ -1,27 +0,0 @@
-[% SWITCH identifier.source.lower %]
-[% CASE 'orcid' %]
-    <li>
-        <span>ORCID: </span>
-        <a href="https://orcid.org/[% identifier.number | url %]">[% identifier.number | html %]</a>
-    </li>
-[% CASE 'scopus' %]
-    <li>
-        <span>ScopusID: </span>
-        <a href="https://www.scopus.com/authid/detail.uri?authorId=[% identifier.number | url %]">[% identifier.number | html %]</a>
-    </li>
-[% CASE 'loop' %]
-    <li>
-        <span>loop: </span>
-        <a href="https://loop.frontiersin.org/people/[% identifier.number | url %]">[% identifier.number | html %]</a>
-    </li>
-[% CASE 'rid' %]
-    <li>
-        <span>Publons: </span>
-        <a href="https://publons.com/researcher/[% identifier.number | url %]">[% identifier.number | html %]</a>
-    </li>
-[% CASE 'viaf' %]
-    <li>
-        <span>VIAF: </span>
-        <a href="https://viaf.org/viaf/[% identifier.number | url %]">[% identifier.number | html %]</a>
-    </li>
-[% END %]
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/authority-information.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/authority-information.inc
index 9f1f5c6d71e..5e1b748da6c 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/authority-information.inc
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/authority-information.inc
@@ -1,5 +1,25 @@
 [% FOREACH info IN information %]
     [% SWITCH info_type %]
+    [% CASE 'identifiers' %]
+        <li>
+        [% SWITCH info.source.lower %]
+            [% CASE 'orcid' %]
+                <span>ORCID: </span>
+                <a href="https://orcid.org/[% info.number | url %]">[% info.number | html %]</a>
+            [% CASE 'scopus' %]
+                <span>ScopusID: </span>
+                <a href="https://www.scopus.com/authid/detail.uri?authorId=[% info.number | url %]">[% info.number | html %]</a>
+            [% CASE 'loop' %]
+                <span>loop: </span>
+                <a href="https://loop.frontiersin.org/people/[% info.number | url %]">[% info.number | html %]</a>
+            [% CASE 'rid' %]
+                <span>Publons: </span>
+                <a href="https://publons.com/researcher/[% info.number | url %]">[% info.number | html %]</a>
+            [% CASE 'viaf' %]
+                <span>VIAF: </span>
+                <a href="https://viaf.org/viaf/[% info.number | url %]">[% info.number | html %]</a>
+            [% END %]
+        </li>
     [% CASE 'activity' %]
         <li>
             <span>
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth-detail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth-detail.tt
index 661a9bc2b2e..e5babfd0d6f 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth-detail.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-auth-detail.tt
@@ -38,12 +38,12 @@
                             [% INCLUDE 'navigation.inc' %]
                         </div>
                     </div>
-                    [% IF Koha.Preference( 'OPACAuthorIdentifiers' ) && author_identifiers.size %]
+                    [% IF Koha.Preference( 'OPACAuthorIdentifiersAndInformation' ) && author_information.size %]
                         <div class="col-lg-5 order-first order-md-first order-lg-2">
                     [% ELSE %]
                         <div class="col-lg-10 order-first order-md-first order-lg-2">
                     [% END %]
-                [% ELSIF Koha.Preference( 'OPACAuthorIdentifiers' ) && author_identifiers.size %]
+                [% ELSIF Koha.Preference( 'OPACAuthorIdentifiersAndInformation' ) && author_information.size %]
                     <div class="col-lg-6 order-first order-md-first order-lg-2">
                 [% ELSE %]
                     <div class="col order-first order-md-first order-lg-2">
@@ -176,17 +176,17 @@
 
                     </div> <!-- / .#userauthdetails -->
                 </div> <!-- / .col-lg-10/12 -->
-                [% IF Koha.Preference( 'OPACAuthorIdentifiers' ) && author_identifiers.size %]
+                [% IF Koha.Preference( 'OPACAuthorIdentifiersAndInformation' ) && author_information.size %]
                     [% IF ( OpacNav || OpacNavBottom ) %]
                         <div class="col-lg-5 order-lg-3">
                     [% ELSE %]
                         <div class="col-lg-6 order-lg-3">
                     [% END %]
-                        <div class="author_identifier">
-                            <span>Identifiers</span>
+                        <div class="author_identifier_info">
+                            <span>Identifiers/information</span>
                             <ul>
-                            [% FOR identifier IN author_identifiers %]
-                                [% PROCESS "authority-identifiers.inc" identifier => identifier %]
+                            [% FOR info IN author_information.keys %]
+                                [% PROCESS "authority-information.inc" information => author_information.$info, info_type => info %]
                             [% END %]
                             </ul>
                         </div>
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
index ad67e42d999..7b86e36e8fb 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
@@ -493,14 +493,10 @@
                             [% END %]
                         [% END %]
 
-                        [% IF Koha.Preference( 'OPACAuthorIdentifiers' ) && author_identifiers.size || Koha.Preference( 'OPACAuthorInformation' ) && author_information.size %]
+                        [% IF Koha.Preference( 'OPACAuthorIdentifiersAndInformation' ) && author_information.size %]
                             [% WRAPPER tab_item tabname= "author_identifiers_info" %]
-                                [% IF Koha.Preference( 'OPACAuthorIdentifiers' ) && author_identifiers.size && Koha.Preference( 'OPACAuthorInformation' ) && author_information.size %]
+                                [% IF Koha.Preference( 'OPACAuthorIdentifiersAndInformation' ) && author_information.size %]
                                     <span>Author identifiers/information</span>
-                                [% ELSIF Koha.Preference( 'OPACAuthorIdentifiers' ) && author_identifiers.size %]
-                                    <span>Author identifiers</span>
-                                [% ELSIF Koha.Preference( 'OPACAuthorInformation' ) && author_information.size %]
-                                    <span>Author information</span>
                                 [% END %]
                             [% END %]
                         [% END %]
@@ -947,7 +943,7 @@
                             [% END # /tab_panel#images %]
                         [% END # / IF OPACLocalCoverImages && localimages.size %]
 
-                        [% IF Koha.Preference( 'OPACAuthorIdentifiers' ) && author_identifiers.size || Koha.Preference( 'OPACAuthorInformation' ) && author_information.size %]
+                        [% IF Koha.Preference( 'OPACAuthorIdentifiersAndInformation' ) && author_information.size %]
                             [% WRAPPER tab_panel tabname="author_identifiers_info" %]
                                 [% FOR author IN author_information %]
                                     <div class="author_information">
@@ -959,17 +955,6 @@
                                         </ul>
                                     </div>
                                 [% END %]
-
-                                [% FOR author IN author_identifiers %]
-                                    <div class="author_identifier">
-                                        <span class="author_name"><a href="/cgi-bin/koha/opac-authoritiesdetail.pl?authid=[% author.authid | uri %]">[% author.name | html %]</a></span>
-                                        <ul>
-                                        [% FOR identifier IN author.identifiers %]
-                                            [% PROCESS "authority-identifiers.inc" identifier => identifier %]
-                                        [% END %]
-                                        </ul>
-                                    </div>
-                                [% END %]
                             [% END # /tab_panel#author_identifiers_info %]
                         [% END %]
 
diff --git a/opac/opac-authoritiesdetail.pl b/opac/opac-authoritiesdetail.pl
index 26d22f3a437..c8993d1f01b 100755
--- a/opac/opac-authoritiesdetail.pl
+++ b/opac/opac-authoritiesdetail.pl
@@ -188,10 +188,10 @@ if ($show_marc) {
         $template->param(summary => $summary);
     }
 
-    if ( C4::Context->preference('OPACAuthorIdentifiers') ) {
+    if ( C4::Context->preference('OPACAuthorIdentifiersAndInformation') ) {
         my $authority = Koha::Authorities->find($authid);
-        my $identifiers = $authority->get_identifiers;
-        $template->param( author_identifiers => $identifiers );
+        my $information = $authority->get_identifiers_and_information;
+        $template->param( author_information => $information );
     }
 }
 
diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
index f8f7c3666de..31114e7ba55 100755
--- a/opac/opac-detail.pl
+++ b/opac/opac-detail.pl
@@ -1227,38 +1227,20 @@ $template->param(
     'OpacLocationBranchToDisplay' => C4::Context->preference('OpacLocationBranchToDisplay'),
 );
 
-if ( C4::Context->preference('OPACAuthorIdentifiers') ) {
-    my @author_identifiers;
-    for my $author ( @{ $biblio->get_marc_authors } ) {
-        my $authid    = $author->{authoritylink};
-        my $authority = Koha::Authorities->find($authid);
-        next unless $authority;
-        my $identifiers = $authority->get_identifiers;
-        next unless $identifiers && @$identifiers;
-        my ($name) =
-          map  { $_->{value} }
-          grep { $_->{code} eq 'a' ? $_ : () }
-          @{ $author->{MARCAUTHOR_SUBFIELDS_LOOP} };
-        push @author_identifiers,
-          { authid => $authid, name => $name, identifiers => $identifiers };
-    }
-    $template->param( author_identifiers => \@author_identifiers );
-}
-
-if ( C4::Context->preference('OPACAuthorInformation') ) {
+if ( C4::Context->preference('OPACAuthorIdentifiersAndInformation') ) {
     my @author_information;
     for my $author ( @{ $biblio->get_marc_authors } ) {
         my $authid    = $author->{authoritylink};
         my $authority = Koha::Authorities->find($authid);
         next unless $authority;
-        my $information = $authority->get_information;
+        my $information = $authority->get_identifiers_and_information;
         next unless $information;
         my ($name) =
           map  { $_->{value} }
           grep { $_->{code} eq 'a' ? $_ : () }
           @{ $author->{MARCAUTHOR_SUBFIELDS_LOOP} };
         push @author_information,
-          { authid => $authid, name => $name, information => $information };
+          { authid => $authid, name => $name, information => $information};
     }
     $template->param( author_information => \@author_information );
 }
diff --git a/t/db_dependent/Koha/Authorities.t b/t/db_dependent/Koha/Authorities.t
index 422307c7b97..111a68dbd20 100755
--- a/t/db_dependent/Koha/Authorities.t
+++ b/t/db_dependent/Koha/Authorities.t
@@ -242,10 +242,11 @@ sub few_marc_records {
     return [ $marc ];
 }
 
-subtest 'get_identifiers' => sub {
+subtest 'get_identifiers_and_information' => sub {
     plan tests => 1;
 
     t::lib::Mocks::mock_preference( 'marcflavour', 'MARC21' );
+    t::lib::Mocks::mock_preference( 'OPACAuthorIdentifiersAndInformation', 'identifiers' );
     my $record = MARC::Record->new();
     $record->add_fields(
         [
@@ -271,17 +272,19 @@ subtest 'get_identifiers' => sub {
     my $authid = C4::AuthoritiesMarc::AddAuthority($record, undef, 'PERSO_NAME');
     my $authority = Koha::Authorities->find($authid);
     is_deeply(
-        $authority->get_identifiers,
-        [
-            {
-                source  => 'orcid',
-                number  => '0000-0002-1234-5678',
-            },
-            {
-                source => 'scopus',
-                number => '01234567890',
-            }
-        ]
+        $authority->get_identifiers_and_information,
+        {
+            identifiers => [
+                {
+                    source => 'orcid',
+                    number => '0000-0002-1234-5678',
+                },
+                {
+                    source => 'scopus',
+                    number => '01234567890',
+                }
+            ]
+        }
     );
 };
 
-- 
2.34.1