Bug 13415 - Prevent librarians from scanning multiple items into a single barcode field
Summary: Prevent librarians from scanning multiple items into a single barcode field
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low minor
Assignee: Kyle M Hall (khall)
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-12-09 12:18 UTC by Kyle M Hall (khall)
Modified: 2015-12-03 22:02 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 13415 - Prevent librarians from scanning multiple items into a single barcode field (3.01 KB, patch)
2014-12-09 12:45 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 13415 - Prevent librarians from scanning multiple items into a single barcode field (3.07 KB, patch)
2014-12-09 12:47 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[SIGNED OFF] Bug 13415 - Prevent librarians from scanning multiple items into a single barcode field (3.15 KB, patch)
2014-12-09 15:18 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 13415 [QA Followup] - Change bind to on, revert barcode disable (1.66 KB, patch)
2014-12-12 13:07 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
[PASSED QA] Bug 13415 - Prevent librarians from scanning multiple items into a single barcode field (3.20 KB, patch)
2014-12-14 20:23 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 13415 [QA Followup] - Change bind to on, revert barcode disable (1.79 KB, patch)
2014-12-14 20:26 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 2014-12-09 12:18:08 UTC
It appears that some librarians will attempt to scan items as fast as they can without checking to make sure the checkout was submitted and the page reloaded properly. This can cause multiple barcodes to be scanned into the barcode field if the person is fast enough.
Comment 1 Kyle M Hall (khall) 2014-12-09 12:45:40 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall (khall) 2014-12-09 12:47:24 UTC Comment hidden (obsolete)
Comment 3 Martin Renvoize (ashimema) 2014-12-09 15:18:42 UTC Comment hidden (obsolete)
Comment 4 Martin Renvoize (ashimema) 2014-12-09 15:19:23 UTC
Works well for me, and code looks fine.  Signing Off
Comment 5 Owen Leonard 2014-12-09 15:44:23 UTC
-    [% IF NEEDSCONFIRMATION %]
            <input type="text" name="barcode" id="barcode" class="barcode focus" size="14" disabled="disabled" />
-    [% ELSE %]
-       <input type="text" name="barcode" id="barcode" class="barcode focus" size="14" />
-    [% END %]

This change means that all circulation transactions must wait until document.ready() fires. Is this what we want? 

+    $('#mainform').bind('submit',function() {
+        $('#barcode').on('keypress',function(event) {

Is there a reason to use bind() on #mainform instead of on()? "As of jQuery 1.7, the .on() method is the preferred method for attaching event handlers to a document." https://api.jquery.com/bind/
Comment 6 Kyle M Hall (khall) 2014-12-12 13:04:34 UTC
I agree, I think I was a bit too aggressive. There is really no need to prevent a barcode from being scanned before the page loads completely. I'll also change the bind to on, as you are correct about that as well.

(In reply to Owen Leonard from comment #5)
> -    [% IF NEEDSCONFIRMATION %]
>             <input type="text" name="barcode" id="barcode" class="barcode
> focus" size="14" disabled="disabled" />
> -    [% ELSE %]
> -       <input type="text" name="barcode" id="barcode" class="barcode focus"
> size="14" />
> -    [% END %]
> 
> This change means that all circulation transactions must wait until
> document.ready() fires. Is this what we want? 
> 
> +    $('#mainform').bind('submit',function() {
> +        $('#barcode').on('keypress',function(event) {
> 
> Is there a reason to use bind() on #mainform instead of on()? "As of jQuery
> 1.7, the .on() method is the preferred method for attaching event handlers
> to a document." https://api.jquery.com/bind/
Comment 7 Kyle M Hall (khall) 2014-12-12 13:07:03 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2014-12-14 20:23:49 UTC
Created attachment 34380 [details] [review]
[PASSED QA] Bug 13415 - Prevent librarians from scanning multiple items into a single barcode field

It appears that some librarians will attempt to scan items as fast as
they can without checking to make sure the checkout was submitted and
the page reloaded properly. This can cause multiple barcodes to be
scanned into the barcode field if the person is fast enough.
This causes the checkout of both ( or more! ) items to fail.

Test Plan:
1) Apply this patch
2) Attempt to scan two or more items into the barcode field
   You can emulate this with a keybaord by hitting a few keys, then
   enter, then a few more keys, then enter again. The barcodes don't
   need to be valid for this test.
3) Note the dialog stating the barcode has been submitted and to please
   wait.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 9 Katrin Fischer 2014-12-14 20:26:23 UTC
Created attachment 34381 [details] [review]
[PASSED QA] Bug 13415 [QA Followup] - Change bind to on, revert barcode disable

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Tested according to test plan, passes tests and QA script.
Comment 10 Tomás Cohen Arazi (tcohen) 2014-12-19 21:01:58 UTC
Patch pushed to master.

Thanks Kyle!
Comment 11 Chris Cormack 2014-12-19 23:56:52 UTC
Template changes held for 3.18.3
Comment 12 Chris Cormack 2014-12-25 19:22:20 UTC
Pushed to 3.18.x is 3.18.3