From 14d1804652daf9c0b83139c3373f4cdaf46b5863 Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Wed, 29 Jan 2020 12:44:57 +0100
Subject: [PATCH] Bug 21466: Keep all AVs for a given framework

There was a major (and silly) issue in the previous version, only one AV
was kept as we erased the hash value at the end of each iteration:
$invalid_locations_per_framework->{$framework->frameworkcode } =
  { items => $items, av_category => $mss->authorised_value, kohafield => $kohafield };

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
---
 .../search_for_data_inconsistencies.pl        | 59 +++++++++++--------
 1 file changed, 34 insertions(+), 25 deletions(-)

diff --git a/misc/maintenance/search_for_data_inconsistencies.pl b/misc/maintenance/search_for_data_inconsistencies.pl
index 2c7b3c9e3f..1790495e88 100755
--- a/misc/maintenance/search_for_data_inconsistencies.pl
+++ b/misc/maintenance/search_for_data_inconsistencies.pl
@@ -125,55 +125,64 @@ use Koha::ItemTypes;
 
 {
     my $frameworks = Koha::BiblioFrameworks->search;
-    my $invalid_locations_per_framework;
+    my $invalid_av_per_framework = {};
     while ( my $framework = $frameworks->next ) {
         my $msss = Koha::MarcSubfieldStructures->search({ frameworkcode => $framework->frameworkcode, authorised_value => { '!=' => [ -and => ( undef, '' ) ]} });
         while ( my $mss = $msss->next ) {
             my $kohafield = $mss->kohafield;
-            next if grep {/$kohafield/} qw( branches itemtypes cn_source ); # internal categories
+            my $av = $mss->authorised_value;
+            next if grep {$_ eq $av} qw( branches itemtypes cn_source ); # internal categories
+
             my $avs = Koha::AuthorisedValues->search_by_koha_field(
                 {
                     frameworkcode => $framework->frameworkcode,
                     kohafield     => $kohafield,
                 }
             );
-            (my $tmp_kohafield = $kohafield) =~ s|items|me|; # replace items.attr with me.attr
+            my $tmp_kohafield = $kohafield;
+            if ( $tmp_kohafield =~ /^biblioitems/ ) {
+                $tmp_kohafield =~ s|biblioitems|biblioitem|;
+            } else {
+                $tmp_kohafield =~ s|items|me|;
+            }
+            # replace items.attr with me.attr
             my $items = Koha::Items->search(
                 {
                     $tmp_kohafield =>
-                      { -not_in => [ $avs->get_column('authorised_value') ] },
+                      {
+                          -not_in => [ $avs->get_column('authorised_value'), '' ],
+                          '!='    => undef,
+                      },
                     'biblio.frameworkcode' => $framework->frameworkcode
                 },
                 { join => [ 'biblioitem', 'biblio' ] }
             );
             if ( $items->count ) {
-                $invalid_locations_per_framework->{ $framework->frameworkcode } =
-                  { items => $items, av_category => $mss->authorised_value, kohafield => $kohafield };
+                $invalid_av_per_framework->{ $framework->frameworkcode }->{$av} =
+                  { items => $items, kohafield => $kohafield };
             }
         }
     }
-    if (%$invalid_locations_per_framework) {
+    if (%$invalid_av_per_framework) {
         new_section('Wrong values linked to authorised values');
-        for my $frameworkcode ( keys %$invalid_locations_per_framework ) {
+        for my $frameworkcode ( keys %$invalid_av_per_framework ) {
             my $output;
-            my $items =
-              $invalid_locations_per_framework->{$frameworkcode}->{items};
-            my $av_category =
-              $invalid_locations_per_framework->{$frameworkcode}->{av_category};
-            my $kohafield =
-              $invalid_locations_per_framework->{$frameworkcode}->{kohafield};
-            my ( $table, $column ) = split '\.', $kohafield;
-            while ( my $i = $items->next ) {
-                my $value = $table eq 'items' ? $i->$column : $i->biblioitem->$column;
-                $output .= " {" . $i->itemnumber . " => " . $value . "}";
+            while ( my ( $av_category, $v ) = each %{$invalid_av_per_framework->{$frameworkcode}} ) {
+                my $items     = $v->{items};
+                my $kohafield = $v->{kohafield};
+                my ( $table, $column ) = split '\.', $kohafield;
+                while ( my $i = $items->next ) {
+                    my $value = $table eq 'items' ? $i->$column : $i->biblioitem->$column;
+                    $output .= " {" . $i->itemnumber . " => " . $value . "}";
+                }
+                new_item(
+                    sprintf(
+                        "The Framework *%s* is using the authorised value's category *%s*, "
+                        . "but the following %s do not have a value defined ({itemnumber => value }):\n%s",
+                        $frameworkcode, $av_category, $kohafield, $output
+                    )
+                );
             }
-            new_item(
-                sprintf(
-                    "The Framework *%s* is using the authorised value's category *%s*, "
-                    . "but the following %s do not have a value defined ({itemnumber => value }):\n%s",
-                    $frameworkcode, $av_category, $kohafield, $output
-                )
-            );
         }
     }
 }
-- 
2.39.5