@@ -, +, @@ are not valid --- C4/Circulation.pm | 3 + Koha/Statistic.pm | 2 +- Koha/Statistics.pm | 73 ++++++++++++++++++- 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/Koha/Statistics.t | 59 ++++++++++++++- 9 files changed, 174 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 ); @@ -778,6 +779,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; @@ -2122,6 +2124,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/Statistic.pm +++ a/Koha/Statistic.pm @@ -28,7 +28,7 @@ use Koha::PseudonymizedTransaction; use base qw(Koha::Object); our @allowed_accounts_types = qw( writeoff payment ); -our @allowed_circulation_types = qw( renew issue localuse return onsite_checkout recall item_found item_lost ); +our @allowed_circulation_types = qw( renew issue localuse return onsite_checkout recall item_found item_lost invalid_item invalid_patron); our @mandatory_accounts_keys = qw( type branch borrowernumber value ); # note that amount is mapped to value our @mandatory_circulation_keys = qw( type branch borrowernumber itemnumber ccode itemtype ); --- 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,79 @@ 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, + itemnumber => "", + ccode => "", + itemtype => "", + datetime => $dt, + branch => $branch, + borrowernumber => $borrowernumber + } + )->store(); +} + +=head3 _type =cut --- a/circ/circulation.pl +++ a/circ/circulation.pl @@ -55,6 +55,7 @@ use Koha::SearchEngine; use Koha::SearchEngine::Search; use Koha::Patron::Modifications; use Koha::Token; +use Koha::Statistics; use List::MoreUtils qw( uniq ); @@ -240,6 +241,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 @@ -366,6 +366,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/Koha/Statistics.t +++ a/t/db_dependent/Koha/Statistics.t @@ -19,7 +19,7 @@ use Modern::Perl; -use Test::More tests => 3; +use Test::More tests => 4; use Test::Exception; use C4::Context; @@ -27,6 +27,8 @@ use Koha::Database; use Koha::Statistics; use t::lib::TestBuilder; +use t::lib::Mocks; +use Test::MockModule; our $schema = Koha::Database->new->schema; our $builder = t::lib::TestBuilder->new; @@ -159,3 +161,58 @@ sub insert_and_fetch { # FIXME discard_changes would be nicer, but we dont have a PK (yet) } + +subtest 'Log borrower cardnumbers and item barcodes which are not valid' => sub { + plan tests => 10; + $schema->storage->txn_begin; + + #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 + my $dbh = $schema->storage->dbh; + $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' ); + + $schema->storage->txn_rollback; +}; --