Bug 16117

Summary: Log borrower cardnumbers and item barcodes which are not valid
Product: Koha Reporter: Barton Chittenden <barton>
Component: CirculationAssignee: Kyle M Hall <kyle>
Status: Patch doesn't apply --- QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: amy, gmcharlt, jonathan.druart, josef.moravec, katrin.fischer, kyle.m.hall, kyle, m.de.rooy, nicole, veron
Version: master   
Hardware: All   
OS: All   
Whiteboard:
Change sponsored?: --- Patch complexity: Medium patch
Bot Control: --- When did the bot last check this:
Who signed the patch off: Text to go in the release notes:
Attachments: Bug 16117 - Feature request: Log borrower cardnumbers and item barcodes which are not valid
Bug 16117 - Feature request: Log borrower cardnumbers and item barcodes which are not valid
Bug 16117: Log borrower cardnumbers and item barcodes which are not valid
Bug 16117: Give methods better names

Description Barton Chittenden 2016-03-21 20:59:55 UTC
On any circ screen where a barcode could be scanned (circ.pl, checkin.pl, checkout and return boxes), we should have the option of logging failed barcode scans, for the purpose of trouble-shooting.
Comment 1 Marc Véron 2016-03-30 12:36:59 UTC
As well valid for current master?
Comment 2 Kyle M Hall 2017-07-28 17:00:47 UTC
Created attachment 65317 [details] [review]
Bug 16117 - Feature request: Log borrower cardnumbers and item barcodes which are not valid

We should be able to log invalid patrons and items for trouble-shooting purposes. It
is especially helpful when determining if there are issues with barcode scanners.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Attempt to check out to an invalid patron cardnumber
4) Attempt to checkout a non-existent barcode
5) Attempt to check in a non-existant barcode
6) Note nothing new is added to the statistics table
7) Enable the new system preferences LogInvalidItems and LogInvalidPatrons
8) Repeat steps 4-6
9) Note each action creates a new line in the statistics table
Comment 3 Marc Véron 2017-07-28 19:46:23 UTC
Created attachment 65319 [details] [review]
Bug 16117 - Feature request: Log borrower cardnumbers and item barcodes which are not valid

We should be able to log invalid patrons and items for trouble-shooting purposes. It
is especially helpful when determining if there are issues with barcode scanners.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Attempt to check out to an invalid patron cardnumber
4) Attempt to checkout a non-existent barcode
5) Attempt to check in a non-existant barcode
6) Note nothing new is added to the statistics table
7) Enable the new system preferences LogInvalidItems and LogInvalidPatrons
8) Repeat steps 4-6
9) Note each action creates a new line in the statistics table

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 4 Jonathan Druart 2017-08-09 17:53:14 UTC
Just a thought:
  Koha::Statistics->invalid_item( { item => $barcode } );
sounds like it will return something, not it will actually write.

I would suggest to rename the method with something more meaningful.
Waiting for another QA opinion.
Comment 5 Josef Moravec 2017-12-03 10:39:42 UTC
(In reply to Jonathan Druart from comment #4)
> Just a thought:
>   Koha::Statistics->invalid_item( { item => $barcode } );
> sounds like it will return something, not it will actually write.
> 
> I would suggest to rename the method with something more meaningful.
> Waiting for another QA opinion.

I agree, something like "log_invalid_item" or similar would be better.
Comment 6 Marcel de Rooy 2018-03-09 07:22:28 UTC
See last two comments
Comment 7 Katrin Fischer 2018-03-12 13:29:24 UTC
Also: Patch doesn't apply!
Comment 8 Kyle M Hall 2018-03-23 13:39:10 UTC
Created attachment 73177 [details] [review]
Bug 16117: Log borrower cardnumbers and item barcodes which are not valid

We should be able to log invalid patrons and items for trouble-shooting purposes. It
is especially helpful when determining if there are issues with barcode scanners.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Attempt to check out to an invalid patron cardnumber
4) Attempt to checkout a non-existent barcode
5) Attempt to check in a non-existant barcode
6) Note nothing new is added to the statistics table
7) Enable the new system preferences LogInvalidItems and LogInvalidPatrons
8) Repeat steps 4-6
9) Note each action creates a new line in the statistics table

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 9 Kyle M Hall 2018-03-23 13:39:18 UTC
Created attachment 73178 [details] [review]
Bug 16117: Give methods better names
Comment 10 Katrin Fischer 2018-04-22 10:36:07 UTC
Patch doesn't apply, please rebase!

73178 - Bug 16117: Give methods better names

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 16117: Log borrower cardnumbers and item barcodes which are not valid
Using index info to reconstruct a base tree...
M	C4/Circulation.pm
M	circ/circulation.pl
M	installer/data/mysql/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
M	t/db_dependent/Stats.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Stats.t
CONFLICT (content): Merge conflict in t/db_dependent/Stats.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/circulation.pref
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging circ/circulation.pl
Auto-merging C4/Circulation.pm
CONFLICT (content): Merge conflict in C4/Circulation.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 16117: Log borrower cardnumbers and item barcodes which are not valid
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Bug-16117-Log-borrower-cardnumbers-and-item-barcod-t7ci5h.patch