From 743198b2524223ee14816b5d722a9c50f57f7d85 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Tue, 10 Sep 2024 10:39:13 +0200
Subject: [PATCH] Bug 37870: Fix sort order of class sources

There are several things going on here.
The tests are failing randomly when some additional class sources are in the
DB. This should never happen on Jenkins, but it happened, see the third
patch of this patch set (spoiler: tests not run within a txn)

There were also a sorting problem: by default sort will show
uppercases first:
  A, B, C, a, b, c
However we want:
  a, A, b, B, c, C
which is what fc does (https://perldoc.perl.org/functions/fc)

Test plan:
0. Checkout the main branch, without patches from this patchset.
1. Run t/db_dependent/ClassSources.t several times
=> Notice that new entries in the DB table 'class_sources' are created
2. Run t/db_dependent/Koha/UI/Form/Builder/Biblio.t and
t/db_dependent/Koha/UI/Form/Builder/Item.t
=> They fail (if not, run t/db_dependent/ClassSources.t again)
3. Apply the patches
4. Run t/db_dependent/ClassSources.t
=> No more additional entries in DB, tests are correctly run within
transactions
5. Run t/db_dependent/Koha/UI/Form/Builder/Biblio.t and
   t/db_dependent/Koha/UI/Form/Builder/Item.t several times
=> They always pass

Note that the sort should actually be done on the description, not the
code. But that's for another bug...
---
 Koha/UI/Form/Builder/Biblio.pm               |  4 +++-
 Koha/UI/Form/Builder/Item.pm                 |  4 +++-
 t/db_dependent/Koha/UI/Form/Builder/Biblio.t | 12 +++++------
 t/db_dependent/Koha/UI/Form/Builder/Item.t   | 21 ++++++++++----------
 4 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/Koha/UI/Form/Builder/Biblio.pm b/Koha/UI/Form/Builder/Biblio.pm
index b0603dedde3..13ecc67ff61 100644
--- a/Koha/UI/Form/Builder/Biblio.pm
+++ b/Koha/UI/Form/Builder/Biblio.pm
@@ -16,6 +16,8 @@ package Koha::UI::Form::Builder::Biblio;
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 use Modern::Perl;
+use feature qw(fc);
+
 use C4::Context;
 use C4::ClassSource qw( GetClassSources );
 use Koha::DateUtils qw( dt_from_string );
@@ -333,7 +335,7 @@ sub build_authorized_values_list {
 
         my $default_source = C4::Context->preference("DefaultClassificationSource");
 
-        foreach my $class_source (sort keys %$class_sources) {
+        foreach my $class_source (sort {fc($a) cmp fc($b)} keys %$class_sources) {
             next unless $class_sources->{$class_source}->{'used'} or
                         ($value and $class_source eq $value) or
                         ($class_source eq $default_source);
diff --git a/Koha/UI/Form/Builder/Item.pm b/Koha/UI/Form/Builder/Item.pm
index 2ab9fb3bb50..1ab305b2fa3 100644
--- a/Koha/UI/Form/Builder/Item.pm
+++ b/Koha/UI/Form/Builder/Item.pm
@@ -16,8 +16,10 @@ package Koha::UI::Form::Builder::Item;
 # along with Koha; if not, see <http://www.gnu.org/licenses>.
 
 use Modern::Perl;
+use feature qw(fc);
 use List::Util qw( any );
 use MARC::Record;
+
 use C4::Context;
 use C4::Biblio qw( GetFrameworkCode GetMarcStructure IsMarcStructureInternal );
 use C4::Koha qw( GetAuthorisedValues );
@@ -242,7 +244,7 @@ sub generate_subfield_form {
             my $default_source =
               C4::Context->preference("DefaultClassificationSource");
 
-            foreach my $class_source ( sort keys %$class_sources ) {
+            foreach my $class_source (sort {fc($a) cmp fc($b)} keys %$class_sources) {
                 next
                   unless $class_sources->{$class_source}->{'used'}
                   or ( $value and $class_source eq $value )
diff --git a/t/db_dependent/Koha/UI/Form/Builder/Biblio.t b/t/db_dependent/Koha/UI/Form/Builder/Biblio.t
index ed5d582ef03..b331d06b370 100755
--- a/t/db_dependent/Koha/UI/Form/Builder/Biblio.t
+++ b/t/db_dependent/Koha/UI/Form/Builder/Biblio.t
@@ -138,13 +138,13 @@ subtest 'generate_subfield_form class sources' => sub {
         },
     );
 
-    my @class_sources = Koha::ClassSources->search({used => 1}, {order_by => 'cn_source'})->as_list;
-    my %labels = map { $_->cn_source => $_->description } @class_sources;
-    my @values = ('', map { $_->cn_source } @class_sources);
+    my @class_sources = Koha::ClassSources->search( { used => 1 }, { order_by => 'cn_source' } )->as_list;
+    my %labels        = map { $_->cn_source => $_->description } @class_sources;
+    my @values        = ( '', map { $_->cn_source } @class_sources );
 
-    is($subfield->{marc_value}->{type}, 'select');
-    is_deeply($subfield->{marc_value}->{labels}, \%labels);
-    is_deeply($subfield->{marc_value}->{values}, \@values);
+    is( $subfield->{marc_value}->{type}, 'select' );
+    is_deeply( $subfield->{marc_value}->{labels}, \%labels );
+    is_deeply( $subfield->{marc_value}->{values}, \@values );
 };
 
 subtest 'generate_subfield_form authorised value' => sub {
diff --git a/t/db_dependent/Koha/UI/Form/Builder/Item.t b/t/db_dependent/Koha/UI/Form/Builder/Item.t
index de82c7741cf..92a7411797a 100755
--- a/t/db_dependent/Koha/UI/Form/Builder/Item.t
+++ b/t/db_dependent/Koha/UI/Form/Builder/Item.t
@@ -23,6 +23,7 @@ use utf8;
 
 use List::MoreUtils qw( uniq );
 
+use Koha::ClassSources;
 use Koha::Libraries;
 use Koha::MarcSubfieldStructures;
 use Koha::UI::Form::Builder::Item;
@@ -99,16 +100,16 @@ subtest 'authorised values' => sub {
     };
 
     subtest 'cn_source' => sub {
-        plan tests => 2;
-        my ( $subfield ) = grep { $_->{kohafield} eq 'items.cn_source' } @$subfields;
-        is_deeply( $subfield->{marc_value}->{values}, [ '', 'ddc', 'lcc' ] );
-        is_deeply(
-            $subfield->{marc_value}->{labels},
-            {
-                ddc => "Dewey Decimal Classification",
-                lcc => "Library of Congress Classification",
-            }
-        );
+        plan tests => 3;
+        my ($subfield) = grep { $_->{kohafield} eq 'items.cn_source' } @$subfields;
+
+        my @class_sources = Koha::ClassSources->search( { used => 1 }, { order_by => 'cn_source' } )->as_list;
+        my %labels        = map { $_->cn_source => $_->description } @class_sources;
+        my @values        = ( '', map { $_->cn_source } @class_sources );
+
+        is( $subfield->{marc_value}->{type}, 'select' );
+        is_deeply( $subfield->{marc_value}->{labels}, \%labels );
+        is_deeply( $subfield->{marc_value}->{values}, \@values );
     };
     subtest 'branches' => sub {
         plan tests => 2;
-- 
2.34.1