@@ -, +, @@ are not valid --- C4/Circulation.pm | 3 + Koha/Statistics.pm | 70 ++++++++++++++++++- circ/circulation.pl | 2 + .../data/mysql/atomicupdate/bug_16117.pl | 19 +++++ installer/data/mysql/mandatory/sysprefs.sql | 2 + .../admin/preferences/circulation.pref | 12 ++++ opac/sco/sco-main.pl | 6 +- t/db_dependent/Stats.t | 54 +++++++++++++- 8 files changed, 164 insertions(+), 4 deletions(-) create mode 100755 installer/data/mysql/atomicupdate/bug_16117.pl --- a/C4/Circulation.pm +++ a/C4/Circulation.pm @@ -63,6 +63,7 @@ use Koha::SearchEngine::Indexer; use Koha::Exceptions::Checkout; use Koha::Plugins; use Koha::Recalls; +use Koha::Statistics; use Carp qw( carp ); use List::MoreUtils qw( any ); use Scalar::Util qw( looks_like_number blessed ); @@ -782,6 +783,7 @@ sub CanBookBeIssued { # MANDATORY CHECKS - unless item exists, nothing else matters unless ( $item_object ) { + Koha::Statistics->log_invalid_item( { item => $barcode } ); $issuingimpossible{UNKNOWN_BARCODE} = 1; } return ( \%issuingimpossible, \%needsconfirmation ) if %issuingimpossible; @@ -2095,6 +2097,7 @@ sub AddReturn { # get information on item my $item = Koha::Items->find({ barcode => $barcode }); unless ($item) { + Koha::Statistics->log_invalid_item( { item => $barcode } ); return ( 0, { BadBarcode => $barcode } ); # no barcode means no item or borrower. bail out. } --- a/Koha/Statistics.pm +++ a/Koha/Statistics.pm @@ -21,6 +21,7 @@ use Modern::Perl; use Koha::Database; use Koha::Statistic; +use Koha::DateUtils qw(dt_from_string); use base qw(Koha::Objects); @@ -32,9 +33,76 @@ Koha::Statistics - Koha Statistic Object set class =head2 Class Methods +=head3 log_invalid_patron + +Koha::Statistics->log_invalid_patron( { patron => $cardnumber } ); + +=cut + +sub log_invalid_patron { + return unless C4::Context->preference('LogInvalidPatrons'); + + my ( $class, $params ) = @_; + + my $patron = $params->{patron}; + + return $class->_log_invalid_value( + { + type => 'patron', + value => $patron + } + ); +} + +=head3 log_invalid_item + +Koha::Statistics->log_invalid_item( { item => $barcode } ); + =cut -=head3 type +sub log_invalid_item { + return unless C4::Context->preference('LogInvalidItems'); + + my ( $class, $params ) = @_; + + my $item = $params->{item}; + + return $class->_log_invalid_value( + { + type => 'item', + value => $item + } + ); +} + +=head3 invalid_value + +Koha::Statistics->invalid_value( { type => 'patron', value => $patron } ); + +=cut + +sub _log_invalid_value { + my ( $class, $params ) = @_; + + my $type = $params->{type}; + my $value = $params->{value}; + + my $branch = C4::Context->userenv ? C4::Context->userenv->{branch} : undef; + my $dt = dt_from_string(); + my $borrowernumber = C4::Context->userenv->{'number'}; + + return Koha::Statistic->new( + { + type => "invalid_$type", + other => $value, + datetime => $dt, + branch => $branch, + borrowernumber => $borrowernumber + } + )->store(); +} + +=head3 _type =cut --- a/circ/circulation.pl +++ a/circ/circulation.pl @@ -54,6 +54,7 @@ use Koha::SearchEngine; use Koha::SearchEngine::Search; use Koha::Patron::Modifications; use Koha::Token; +use Koha::Statistics; use List::MoreUtils qw( uniq ); @@ -221,6 +222,7 @@ if ($findborrower) { if ( $patron ) { $borrowernumber = $patron->borrowernumber; } else { + Koha::Statistics->log_invalid_patron( { patron => $findborrower } ); print $query->redirect( "/cgi-bin/koha/members/member.pl?quicksearch=1&circsearch=1&searchmember=" . uri_escape_utf8($findborrower) ); exit; } --- a/installer/data/mysql/atomicupdate/bug_16117.pl +++ a/installer/data/mysql/atomicupdate/bug_16117.pl @@ -0,0 +1,19 @@ +use Modern::Perl; + +return { + bug_number => "16117", + description => "Log invalid borrower cardnumbers and item barcodes", + up => sub { + my ($args) = @_; + my ($dbh, $out) = @$args{qw(dbh out)}; + + $dbh->do(q{ + INSERT IGNORE INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `type` ) VALUES + ('LogInvalidItems','0','','Log scanned invalid item identifiers as statistics','YesNo'), + ('LogInvalidPatrons','0','','Log scanned invalid patron identifiers as statistics','YesNo'); + }); + # sysprefs + say $out "Added new system preference 'LogInvalidItems'"; + say $out "Added new system preference 'LogInvalidPatrons'"; + }, +}; --- a/installer/data/mysql/mandatory/sysprefs.sql +++ a/installer/data/mysql/mandatory/sysprefs.sql @@ -357,6 +357,8 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('LocalHoldsPriorityItemControl', 'holdingbranch', 'holdingbranch|homebranch', 'decides if the feature operates using the item''s home or holding library.', 'Choice'), ('LocalHoldsPriorityPatronControl', 'PickupLibrary', 'HomeLibrary|PickupLibrary', 'decides if the feature operates using the library set as the patron''s home library, or the library set as the pickup library for the given hold.', 'Choice'), ('LockExpiredDelay','','','Delay for locking expired patrons (empty means no locking)','Integer'), +('LogInvalidItems','0','','Log scanned invalid item identifiers as statistics','YesNo'), +('LogInvalidPatrons','0','','Log scanned invalid patron identifiers as statistics','YesNo'), ('makePreviousSerialAvailable','0','','make previous serial automatically available when collecting a new serial. Please note that the item-level_itypes syspref must be set to specific item.','YesNo'), ('Mana','2',NULL,'request to Mana Webservice. Mana centralize common information between other Koha to facilitate the creation of new subscriptions, vendors, report queries etc... You can search, share, import and comment the content of Mana.','Choice'), ('MARCAuthorityControlField008','|| aca||aabn | a|a d',NULL,'Define the contents of MARC21 authority control field 008 position 06-39','Textarea'), --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref +++ a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref @@ -19,6 +19,18 @@ Circulation: 0: "Don't enable" - the automatic redirection to another patron when a patron barcode is scanned instead of a book. - This should not be enabled if you have overlapping patron and book barcodes. + - + - pref: LogInvalidPatrons + choices: + yes: Log + no: Do not log + - scanned invalid patron identifiers as statistics. + - + - pref: LogInvalidItems + choices: + yes: Log + no: Do not log + - scanned invalid item identifiers as statistics. - - pref: PatronAutoComplete choices: --- a/opac/sco/sco-main.pl +++ a/opac/sco/sco-main.pl @@ -131,6 +131,7 @@ my $patron; if ( $patronid ) { Koha::Plugins->call( 'patron_barcode_transform', \$patronid ); $patron = Koha::Patrons->find( { cardnumber => $patronid } ); + Koha::Statistics->log_invalid_patron( { patron => $patronid } ) unless $patron; } undef $jwt unless $patron; @@ -142,8 +143,9 @@ my $return_only = 0; if ( $patron && $op eq "returnbook" && $allowselfcheckreturns ) { my $success = 1; - my $item = Koha::Items->find( { barcode => $barcode } ); + Koha::Statistics->log_invalid_item( { item => $barcode } ) unless $item; + if ( $success && C4::Context->preference("CircConfirmItemParts") ) { if ( defined($item) && $item->materials ) @@ -167,6 +169,7 @@ if ( $patron && $op eq "returnbook" && $allowselfcheckreturns ) { elsif ( $patron && ( $op eq 'checkout' ) ) { my $item = Koha::Items->find( { barcode => $barcode } ); + Koha::Statistics->log_invalid_item( { item => $barcode } ) unless $item; my $impossible = {}; my $needconfirm = {}; ( $impossible, $needconfirm ) = CanBookBeIssued( @@ -277,6 +280,7 @@ elsif ( $patron && ( $op eq 'checkout' ) ) { if ( $patron && ( $op eq 'renew' ) ) { my $item = Koha::Items->find({ barcode => $barcode }); + Koha::Statistics->log_invalid_item( { item => $barcode } ) unless $item; if ( $patron->checkouts->find( { itemnumber => $item->itemnumber } ) ) { my ($status,$renewerror) = CanBookBeRenewed( $patron, $item->checkout ); --- a/t/db_dependent/Stats.t +++ a/t/db_dependent/Stats.t @@ -4,10 +4,15 @@ use Modern::Perl; use C4::Stats qw( UpdateStats ); use Koha::Database; -use Test::More tests => 19; +use Test::More tests => 31; +use Test::MockModule; +use t::lib::TestBuilder; +use t::lib::Mocks; BEGIN { use_ok('C4::Stats', qw( UpdateStats )); + use_ok('Koha::Statistic'); + use_ok('Koha::Statistics'); } can_ok( 'C4::Stats', @@ -162,4 +167,49 @@ $line = ${ $sth->fetchall_arrayref( {} ) }[0]; is( $line->{location}, undef, "UpdateStats sets location to NULL if undef is passed in." ); -# More tests to write! +my $builder = t::lib::TestBuilder->new; +my $library = $builder->build( { source => 'Branch' } ); +my $branchcode = $library->{branchcode}; +my $context = Test::MockModule->new('C4::Context'); +$context->mock( + 'userenv', + sub { + return { + flags => 1, + id => 'my_userid', + branch => $branchcode, + number => '-1', + }; + } +); +# Test Koha::Statistic->log_invalid_patron +$dbh->do(q{DELETE FROM statistics}); +t::lib::Mocks::mock_preference( "LogInvalidPatrons", 0 ); +Koha::Statistics->log_invalid_patron( { patron => 'InvalidCardnumber' } ); +is( Koha::Statistics->search()->count(), 0, 'No stat line added if system preference LogInvalidPatrons is disabled' ); + +t::lib::Mocks::mock_preference( "LogInvalidPatrons", 1 ); +Koha::Statistics->log_invalid_patron( { patron => 'InvalidCardnumber' } ); +my $stat = Koha::Statistics->search()->next(); +is( $stat->type, 'invalid_patron', 'Type set to invalid_patron' ); +is( $stat->borrowernumber, '-1', 'Associated library id set correctly' ); +is( $stat->other, 'InvalidCardnumber', 'Invalid cardnumber is set correctly' ); +is( $stat->branch, $branchcode, 'Branchcode is set correctly' ); + +# Test Koha::Statistic->log_invalid_item +$dbh->do(q{DELETE FROM statistics}); +t::lib::Mocks::mock_preference( "LogInvalidItems", 0 ); +Koha::Statistics->log_invalid_item( { item => 'InvalidBarcode' } ); +is( Koha::Statistics->search()->count(), 0, 'No stat line added if system preference LogInvalidItems is disabled' ); + +t::lib::Mocks::mock_preference( "LogInvalidItems", 1 ); +Koha::Statistics->log_invalid_item( { item => 'InvalidBarcode' } ); +$stat = Koha::Statistics->search()->next(); +is( $stat->type, 'invalid_item', 'Type set to invalid_item' ); +is( $stat->borrowernumber, '-1', 'Associated library id set correctly' ); +is( $stat->other, 'InvalidBarcode', 'Invalid barcode is set correctly' ); +is( $stat->branch, $branchcode, 'Branchcode is set correctly' ); + + +#End transaction +$dbh->rollback; --