Bug 16117 - Log borrower cardnumbers and item barcodes which are not valid
Summary: Log borrower cardnumbers and item barcodes which are not valid
Status: Patch doesn't apply
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-03-21 20:59 UTC by Barton Chittenden
Modified: 2018-04-22 10:36 UTC (History)
10 users (show)

See Also:
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 (15.34 KB, patch)
2017-07-28 17:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16117 - Feature request: Log borrower cardnumbers and item barcodes which are not valid (15.47 KB, patch)
2017-07-28 19:46 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 16117: Log borrower cardnumbers and item barcodes which are not valid (13.38 KB, patch)
2018-03-23 13:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 16117: Give methods better names (5.56 KB, patch)
2018-03-23 13:39 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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