From 92cbbd59d52f440942e1148c6ee68c1f5febd71f Mon Sep 17 00:00:00 2001
From: Thibaud Guillot <thibaud.guillot@biblibre.com>
Date: Mon, 14 Sep 2026 14:01:23 +0000
Subject: [PATCH] Bug 32748: (QA follow-up) Fix duplicate AV when limited to
 several libraries at once

When an authorised value is limited to more than one library, the
no_limit query in GetAuthorisedValues joins authorised_values_branches
and returns one row per associated library, so the value was listed
several times in the item edition dropdown.

Group by the authorised value so each one is only returned once, and
aggregate the restricted flag with MIN() so the value stays
unrestricted as soon as one of its libraries matches the current one.

Sponsored-by: BibLibre
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
---
 C4/Koha.pm            |  4 +++-
 t/db_dependent/Koha.t | 41 ++++++++++++++++++++++++++++++++++-------
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/C4/Koha.pm b/C4/Koha.pm
index 2e9f85e659..7c9c129082 100644
--- a/C4/Koha.pm
+++ b/C4/Koha.pm
@@ -523,7 +523,7 @@ sub GetAuthorisedValues {
     my $select_clause = 'SELECT av.*';
     my @where_args;
     if ( $branch_limit && $no_limit ) {
-        $select_clause .= ', IF (branchcode = ? OR branchcode IS NULL, 0, 1) as restricted';
+        $select_clause .= ', MIN(IF (branchcode = ? OR branchcode IS NULL, 0, 1)) as restricted';
         push @where_args, $branch_limit;
     }
 
@@ -548,6 +548,8 @@ sub GetAuthorisedValues {
     if ( @where_strings > 0 ) {
         $query .= " WHERE " . join( " AND ", @where_strings );
     }
+    $query .= ' GROUP BY av.id, av.category, av.authorised_value, av.lib, av.lib_opac, av.imageurl'
+        if $branch_limit && $no_limit;
     $query .= ' ORDER BY category, '
         . (
         $opac
diff --git a/t/db_dependent/Koha.t b/t/db_dependent/Koha.t
index 51222ef2c8..4e5490129f 100755
--- a/t/db_dependent/Koha.t
+++ b/t/db_dependent/Koha.t
@@ -174,7 +174,7 @@ subtest 'Authorized Values Tests' => sub {
 };
 
 subtest 'GetAuthorisedValues no_limit and cache key tests (bug 32748)' => sub {
-    plan tests => 6;
+    plan tests => 7;
 
     Koha::AuthorisedValueCategory->new( { category_name => 'BUG32748' } )->store;
 
@@ -202,6 +202,28 @@ subtest 'GetAuthorisedValues no_limit and cache key tests (bug 32748)' => sub {
         }
     );
 
+    # An AV limited to several libraries at once, one of them being the current
+    # library: it must be listed only once, and not flagged as restricted.
+    my $av_multi = Koha::AuthorisedValue->new(
+        {
+            category         => 'BUG32748',
+            authorised_value => 'MULTI',
+            lib              => 'Multi',
+        }
+    )->store;
+    $schema->resultset('AuthorisedValuesBranch')->create(
+        {
+            av_id      => $av_multi->id,
+            branchcode => $branch1->{branchcode},
+        }
+    );
+    $schema->resultset('AuthorisedValuesBranch')->create(
+        {
+            av_id      => $av_multi->id,
+            branchcode => $branch2->{branchcode},
+        }
+    );
+
     t::lib::Mocks::mock_userenv( { branchcode => $branch1->{branchcode} } );
 
     my $cache = Koha::Caches->get_instance;
@@ -209,17 +231,22 @@ subtest 'GetAuthorisedValues no_limit and cache key tests (bug 32748)' => sub {
     $cache->clear_from_cache("AuthorisedValues-BUG32748-0-$branch1->{branchcode}-1");
 
     my $limited = GetAuthorisedValues('BUG32748');
-    is( scalar @$limited, 1, 'Branch-limited call returns only the unrestricted value' );
-    is(
-        $limited->[0]{authorised_value}, 'UNRESTRICTED',
-        'Branch-limited call excludes the value restricted to another library'
+    is( scalar @$limited, 2, 'Branch-limited call returns the unrestricted value and the multi-library one' );
+    is_deeply(
+        [ sort map { $_->{authorised_value} } @$limited ],
+        [ 'MULTI', 'UNRESTRICTED' ],
+        'Branch-limited call excludes the value restricted to another library only'
     );
 
     my $unlimited = GetAuthorisedValues( 'BUG32748', undef, { no_limit => 1 } );
-    is( scalar @$unlimited, 2, 'no_limit call returns both values' );
+    is( scalar @$unlimited, 3, 'no_limit call returns each value once, including the multi-library one' );
     my %restricted_by_av = map { $_->{authorised_value} => $_->{restricted} } @$unlimited;
     is( $restricted_by_av{UNRESTRICTED}, 0, 'Unrestricted value is flagged as not restricted' );
     is( $restricted_by_av{RESTRICTED},   1, 'Value tied to another library is flagged as restricted' );
+    is(
+        $restricted_by_av{MULTI}, 0,
+        'Value tied to the current library and another one is not flagged as restricted (bug 32748)'
+    );
 
     # Regression test for the cache key bug reported on bug 32748: a plain call must
     # not be served the cached result of a no_limit call (or vice versa), since the
@@ -227,7 +254,7 @@ subtest 'GetAuthorisedValues no_limit and cache key tests (bug 32748)' => sub {
     my $limited_again = GetAuthorisedValues('BUG32748');
     is_deeply(
         [ sort map { $_->{authorised_value} } @$limited_again ],
-        ['UNRESTRICTED'],
+        [ 'MULTI', 'UNRESTRICTED' ],
         'Branch-limited call is not polluted by a previous no_limit call sharing category/opac/branch'
     );
 };