From e0a08fd9ca45ef74318fc166bf31b7284f9e1498 Mon Sep 17 00:00:00 2001 From: Olivier V Date: Mon, 28 Oct 2024 11:16:47 -0400 Subject: [PATCH] Bug 36027: (follow-up) Fixed code duplication and add mandatory options An option is now mandatory for the script to lunch. 1. Apply the patch 2. Run the command without any argument and confirm it blocks you and shows the help message. 3. Run the command with one or more check arguments (ex: ./search_for_data_inconsistencies.pl --check-title --check-loop), confirm that it displayed the specified inconsistences. 4. Run the command with one or more skip arguments (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title), confirm that it displayed the inconsistences unsless the skip ones. 5. Run the command with check and skip options (ex: ./search_for_data_inconsistencies.pl --skip-auth --skip-title --check-loop), confirm that it displayed the specified check options and that a warning message indicate that the skip options have been ignored. --- .../search_for_data_inconsistencies.pl | 112 ++---------------- 1 file changed, 8 insertions(+), 104 deletions(-) diff --git a/misc/maintenance/search_for_data_inconsistencies.pl b/misc/maintenance/search_for_data_inconsistencies.pl index 3ada8a5253..ba967e8793 100755 --- a/misc/maintenance/search_for_data_inconsistencies.pl +++ b/misc/maintenance/search_for_data_inconsistencies.pl @@ -66,7 +66,6 @@ GetOptions( 'skip-loop' => sub { $options{'skip-loop'} = 1 }, 'help' => sub { $options{'help'} = 1; }, ) or pod2usage(2); # Print usage if invalid options are provided - # Call handle_invalid_options subroutine if invalid options are provided Getopt::Long::Configure('pass_through'); GetOptions( @@ -127,16 +126,10 @@ sub set_skip_options { # Set all check options to 1 if none are provided, considering skip options sub set_all_check_options_if_none_provided { my $any_check_option_provided = grep { $options{$_} } grep { /^check-/ } keys %options; - if ( !$any_check_option_provided ) { - foreach my $key ( keys %options ) { - if ( $key =~ /^check-/ ) { - my $skip_key = $key; - $skip_key =~ s/^check-/skip-/; - # Set check option to 1 unless the corresponding skip option indicated - $options{$key} = 1 unless $options{$skip_key}; - } - } + if ( !$any_check_option_provided ) { + handle_invalid_options("No options selected"); + die; } } @@ -271,19 +264,19 @@ sub CheckItemsStatus { } else { $biblioitemnumber = $record->subfield( $biblioitem_tag, $biblioitem_subfield ); } - if ( $biblionumber != $biblio->biblionumber ) { + if (not (defined $biblionumber) and $biblionumber != $biblio->biblionumber ) { push @ids_not_in_marc, { biblionumber => $biblio->biblionumber, biblionumber_in_marc => $biblionumber, }; } - if ( $biblioitemnumber != $biblio->biblioitem->biblioitemnumber ) { + if ( not (defined $biblioitemnumber) and $biblioitemnumber != $biblio->biblioitem->biblioitemnumber ) { push @ids_not_in_marc, { - biblionumber => $biblio->biblionumber, - biblioitemnumber => $biblio->biblioitem->biblioitemnumber, - biblioitemnumber_in_marc => $biblionumber, + biblionumber => $biblio->biblionumber, + biblioitemnumber => $biblio->biblioitem->biblioitemnumber, + biblioitemnumber_in_marc => $biblionumber, }; } } @@ -615,95 +608,6 @@ sub CheckAgeForCategory { } } -sub CheckRelationshipsLoop { - my @loop_borrowers_relationships; - my @guarantor_ids = Koha::Patron::Relationships->_resultset->get_column('guarantor_id')->all(); - my @guarantee_ids = Koha::Patron::Relationships->_resultset->get_column('guarantee_id')->all(); - - foreach my $guarantor_id (@guarantor_ids) { - foreach my $guarantee_id (@guarantee_ids) { - if ( $guarantor_id == $guarantee_id ) { - - my $relation_guarantor_id; - my $relation_guarantee_id; - my $size_list; - my $tmp_garantor_id = $guarantor_id; - my @guarantor_ids; - - do { - my @relationship_for_go = Koha::Patron::Relationships->search( - { - -or => [ - 'guarantor_id' => { '=', $tmp_garantor_id }, - ] - }, - )->as_list; - $size_list = scalar @relationship_for_go; - - foreach my $relation (@relationship_for_go) { - $relation_guarantor_id = $relation->guarantor_id; - unless ( grep { $_ == $relation_guarantor_id } @guarantor_ids ) { - push @guarantor_ids, $relation_guarantor_id; - } - $relation_guarantee_id = $relation->guarantee_id; - - my @relationship_for_go = Koha::Patron::Relationships->search( - { - -or => [ - 'guarantor_id' => { '=', $relation_guarantee_id }, - ] - }, - )->as_list; - $size_list = scalar @relationship_for_go; - - if ( $guarantor_id == $relation_guarantee_id ) { - last; - } - - foreach my $relation (@relationship_for_go) { - $relation_guarantor_id = $relation->guarantor_id; - unless ( grep { $_ == $relation_guarantor_id } @guarantor_ids ) { - push @guarantor_ids, $relation_guarantor_id; - } - $relation_guarantee_id = $relation->guarantee_id; - - if ( $guarantor_id == $relation_guarantee_id ) { - last; - } - } - if ( $guarantor_id == $relation_guarantee_id ) { - last; - } - } - - $tmp_garantor_id = $relation_guarantee_id; - } while ( $guarantor_id != $relation_guarantee_id && $size_list > 0 ); - - if ( $guarantor_id == $relation_guarantee_id ) { - unless ( grep { join( "", sort @$_ ) eq join( "", sort @guarantor_ids ) } - @loop_borrowers_relationships ) - { - push @loop_borrowers_relationships, \@guarantor_ids; - } - } - } - } - } - - if ( scalar @loop_borrowers_relationships > 0 ) { - new_section("The list of guarantors who are also guarantee."); - my $count = 0; - foreach my $table (@loop_borrowers_relationships) { - $count++; - print "Loop $count, borrowers id : "; - foreach my $borrower_id (@$table) { - print "$borrower_id , "; - } - print "\n"; - } - new_hint("Relationships that form guarantor loops must be deleted"); - } -} sub CheckRelationshipsLoop { my @loop_borrowers_relationships; -- 2.34.1