From bd17eee3294247e7a2660d73fa9fe970445fe860 Mon Sep 17 00:00:00 2001 From: Alexandre Noel Date: Fri, 14 Jun 2024 16:21:05 -0400 Subject: [PATCH] Bug 36027: search_for_data_inconsistencies.pl - make each section optional We can specify which sections to search for data inconsistencies using check options, or which sections to skip using skip options. TEST PLAN: Make sure there are data inconsistencies present in at least two categories to validate the results of the command. 1. Apply the patch 2. Run the command without any argument and confirm that every inconsistences are reported. 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. Signed-off-by: David Nind --- .../search_for_data_inconsistencies.pl | 193 +++++++++++++++--- 1 file changed, 159 insertions(+), 34 deletions(-) diff --git a/misc/maintenance/search_for_data_inconsistencies.pl b/misc/maintenance/search_for_data_inconsistencies.pl index d1f371c19a..ddb86758fd 100755 --- a/misc/maintenance/search_for_data_inconsistencies.pl +++ b/misc/maintenance/search_for_data_inconsistencies.pl @@ -36,12 +36,14 @@ our %options = ( 'check-framework' => 0, 'check-title' => 0, 'check-age' => 0, - 'skip-branch' => 1, - 'skip-auth' => 1, - 'skip-status' => 1, - 'skip-framework' => 1, - 'skip-title' => 1, - 'skip-age' => 1, + 'check-loop' => 0, + 'skip-branch' => 0, + 'skip-auth' => 0, + 'skip-status' => 0, + 'skip-framework' => 0, + 'skip-title' => 0, + 'skip-age' => 0, + 'skip-loop' => 0, 'help' => 0, ); @@ -53,12 +55,14 @@ GetOptions( 'check-framework' => sub { $options{'check-framework'} = 1 }, 'check-title' => sub { $options{'check-title'} = 1 }, 'check-age' => sub { $options{'check-age'} = 1 }, - 'skip-branch' => sub { $options{'skip-branch'} = 0 }, - 'skip-auth' => sub { $options{'skip-auth'} = 0 }, - 'skip-status' => sub { $options{'skip-status'} = 0 }, - 'skip-framework' => sub { $options{'skip-framework'} = 0 }, - 'skip-title' => sub { $options{'skip-title'} = 0 }, - 'skip-age' => sub { $options{'skip-age'} = 0 }, + 'check-loop' => sub { $options{'check-loop'} = 1 }, + 'skip-branch' => sub { $options{'skip-branch'} = 1 }, + 'skip-auth' => sub { $options{'skip-auth'} = 1 }, + 'skip-status' => sub { $options{'skip-status'} = 1 }, + 'skip-framework' => sub { $options{'skip-framework'} = 1 }, + 'skip-title' => sub { $options{'skip-title'} = 1 }, + 'skip-age' => sub { $options{'skip-age'} = 1 }, + 'skip-loop' => sub { $options{'skip-loop'} = 1 }, 'help' => sub { $options{'help'} = 1; }, ) or pod2usage(2); # Print usage if invalid options are provided @@ -74,14 +78,18 @@ GetOptions( # Print usage and exit if --help flag is provided pod2usage(1) if $options{'help'}; -# Run tests based on options +# Set skip options based on check options set_skip_options(); -CheckItemsBranch() if $options{'check-branch'} || $options{'skip-branch'}; -CheckItemsAuthHeader() if $options{'check-auth'} || $options{'skip-auth'}; -CheckItemsStatus() if $options{'check-status'} || $options{'skip-status'}; -CheckItemsFramework() if $options{'check-framework'} || $options{'skip-framework'}; -CheckItemsTitle() if $options{'check-title'} || $options{'skip-title'}; -CheckAgeForCategory() if $options{'check-age'} || $options{'skip-age'}; +# Set all check options to 1 if none are provided, unless specified skip options +set_all_check_options_if_none_provided(); +# Run checks based on options +CheckItemsBranch() if $options{'check-branch'} && !$options{'skip-branch'}; +CheckItemsAuthHeader() if $options{'check-auth'} && !$options{'skip-auth'}; +CheckItemsStatus() if $options{'check-status'} && !$options{'skip-status'}; +CheckItemsFramework() if $options{'check-framework'} && !$options{'skip-framework'}; +CheckItemsTitle() if $options{'check-title'} && !$options{'skip-title'}; +CheckAgeForCategory() if $options{'check-age'} && !$options{'skip-age'}; +CheckRelationshipsLoop() if $options{'check-loop'} && !$options{'skip-loop'}; # Handle invalid options sub handle_invalid_options { @@ -92,15 +100,39 @@ sub handle_invalid_options { # Set skip options based on check options sub set_skip_options { - my $has_check_option = grep { $options{$_} == 1 } grep { /^check-/ } keys %options; + # If at least one check option is provided, set all skip options to 0 + my $has_check_option = grep { $options{$_} } grep { /^check-/ } keys %options; if ($has_check_option) { + # if a least one skip option is provided, print a warning + my $has_skip_option = grep { $options{$_} == 1 } grep { /^skip-/ } keys %options; + if ($has_skip_option) { + print("Warning : skip options are ignored when check options are provided\n") + } + # Set all skip options to 0 foreach my $key (keys %options) { - $options{$key} = 0 if $key =~ /^skip-/; + if ($key =~ /^skip-/) { + $options{$key} = 0; + } } } return %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}; + } + } + } +} + sub CheckItemsBranch { my $items = Koha::Items->search({ -or => { homebranch => undef, holdingbranch => undef }}); if ( $items->count ) { new_section("Not defined items.homebranch and/or items.holdingbranch")} @@ -486,6 +518,96 @@ 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 new_section { my ( $name ) = @_; say "\n== $name =="; @@ -517,22 +639,25 @@ Catch data inconsistencies in Koha database: * Bibliographic records without a title * Invalid MARCXML in bibliographic records * Patrons with invalid category types due to lower and upper age limits + * Relationships that form guarantor loops =head2 OPTIONS - --check-branch Check for items without home or holding library - --check-auth Check for authority records with invalid authority type - --check-status Check for bibliographic records and items without an item type or with an invalid item type - --check-framework Check for invalid values in fields where the framework limits to an authorized value category - --check-title Check for bibliographic records without a title - --check-age Check for patrons with invalid age for category - --skip-branch Skip checking for items without home or holding library - --skip-auth Skip checking for authority records with invalid authority type - --skip-status Skip checking for bibliographic records and items without an item type or with an invalid item type - --skip-framework Skip checking for invalid values in fields where the framework limits to an authorized value category - --skip-title Skip checking for bibliographic records without a title - --skip-age Skip checking for patrons with invalid age for category - --help Print usage information + --check-branch Check for items without home or holding library + --check-auth Check for authority records with invalid authority type + --check-status Check for bibliographic records and items without an item type or with an invalid item type + --check-framework Check for invalid values in fields where the framework limits to an authorized value category + --check-title Check for bibliographic records without a title + --check-age Check for patrons with invalid age for category + --check-loop Check for relationships that form guarantor loops + --skip-branch Skip checking for items without home or holding library + --skip-auth Skip checking for authority records with invalid authority type + --skip-status Skip checking for bibliographic records and items without an item type or with an invalid item type + --skip-framework Skip checking for invalid values in fields where the framework limits to an authorized value category + --skip-title Skip checking for bibliographic records without a title + --skip-age Skip checking for patrons with invalid age for category + --skip-loop Skip checking for relationships that form guarantor loops + --help Print usage information Note: If no options are provided, all tests will be run. -- 2.39.2