From 926495fd71f283549abeb5951d4683a041646c1f 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 Signed-off-by: Kyle M Hall --- .../search_for_data_inconsistencies.pl | 317 ++++++++++++++++-- 1 file changed, 283 insertions(+), 34 deletions(-) diff --git a/misc/maintenance/search_for_data_inconsistencies.pl b/misc/maintenance/search_for_data_inconsistencies.pl index 8cdeb7d796..2f56ba2294 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")} @@ -431,6 +463,220 @@ sub CheckAgeForCategory { } { + use Koha::Database; + my $schema = Koha::Database->new->schema; + + # Loop over all the DBIx::Class classes + for my $class ( sort values %{ $schema->{class_mappings} } ) { + + # Retrieve the resultset so we can access the columns info + my $rs = $schema->resultset($class); + my $columns = $rs->result_source->columns_info; + + # Loop over the columns + while ( my ( $column, $info ) = each %$columns ) { + + # Next if data type is not date/datetime/timestamp + my $data_type = $info->{data_type}; + next unless grep { $data_type =~ m{^$_$} } qw( timestamp datetime date ); + + # Count the invalid dates + my $invalid_dates = $rs->search( { $column => '0000-00-00' } )->count; + + next unless $invalid_dates; + + new_section( + "Column " . $rs->result_source->name . "." . $column . " contains $invalid_dates invalid dates" ); + + if ( $invalid_dates > 0 ) { + new_hint("You may change the dates with script: misc/cronjobs/fix_invalid_dates.pl (-c -v)"); + } + + } + } +} + +{ + 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; + 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; my @guarantor_ids = Koha::Patron::Relationships->_resultset->get_column('guarantor_id')->all(); my @guarantee_ids = Koha::Patron::Relationships->_resultset->get_column('guarantee_id')->all(); @@ -551,23 +797,26 @@ 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 * Any date fields in the database (timestamp, datetime, date) set to 0000-00-00 =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.34.1