Bug 4054

Summary: Double-clicking the 'place hold' button can result in duplicate holds
Product: Koha Reporter: Owen Leonard <oleonard>
Component: Hold requestsAssignee: Galen Charlton <gmcharlt>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: enhancement    
Priority: P5 - low CC: charles.farmer, eric.begin, francois.charbonnier, josef.moravec, kyle.m.hall, marjorie.barry-vila, nengard, paul.poulain, pelletiermaxime, philippe.blouin, veron, wizzyrea
Version: 3.8   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14891
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 7556    
Attachments: Bug 4054 - Double-clicking the 'place hold' button can result in duplicate holds
Bug 4054 - Double-clicking the 'place hold' button can result in duplicate holds
Bug 4054 - Double-clicking the 'place hold' button can result in duplicate holds
Bug 4054 - Double-clicking the 'place hold' button can result in duplicate holds - Part 2: OPAC
[Signed Off] Bug 4054 - Double-clicking the 'place hold' button can result in duplicate holds - Part 2: OPAC

Description Chris Cormack 2010-05-21 01:22:37 UTC


---- Reported by oleonard@myacpl.org 2010-01-21 14:48:08 ----

Testing in the staff client I find that if I double-click the "place hold" button I can accidentally create duplicate holds. The duplicates have the same priority assigned to them.

At one time (2.x? dev_week?) we tried using JavaScript to prevent double-clicks, and I found that to be buggy. There are some cases where the first click triggers the JS event but *not* the submit.

Is there a server-side fix that could prevent this?



---- Additional Comments From oleonard@myacpl.org 2010-01-21 14:55:40 ----

Further testing suggests that when an item is allocated for the hold[s], each of the duplicate holds is shown as being fulfilled. The patron's circ screen will even show duplicate "holds waiting" messages which will all disappear after the patron checks out the item. So perhaps this bug is really only cosmetic?



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 01:22 UTC  ---

This bug was previously known as _bug_ 4054 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=4054

Actual time not defined. Setting to 0.0
Setting qa contact to the default for this product.
   This bug either had no qa contact or an invalid one.

Comment 1 Liz Rea 2010-08-17 19:39:53 UTC
http://screencast.com/t/N2RjNmNh

Shows double clicking the place hold button causing a duplicate hold.
Comment 2 Kyle M Hall 2012-02-14 15:57:55 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2012-02-14 16:01:07 UTC Comment hidden (obsolete)
Comment 4 Liz Rea 2012-02-14 17:10:21 UTC
Created attachment 7640 [details] [review]
Bug 4054 - Double-clicking the 'place hold' button can result in duplicate holds

This commit adds a new jquery function to staff-global.js, preventDoubleFormSubmit().
When used thusly: $('#form-id').preventDoubleFormSubmit();
It will prevent a submitted form from being submitted a second time.
It is currently only added to the Place Hold form in reserve/request.pl

http://bugs.koha-community.org/show_bug.cgi?id=4045
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Tested:
Double clicking no longer causes identical double holds to be placed.
passes prove t xt t/db_dependent in line with current master.

Reminder: this will need to be added to the OPAC as well.
Comment 5 Kyle M Hall 2012-02-14 18:51:15 UTC Comment hidden (obsolete)
Comment 6 Liz Rea 2012-02-14 21:44:19 UTC
Created attachment 7647 [details] [review]
[Signed Off] Bug 4054 - Double-clicking the 'place hold' button can result in duplicate holds - Part 2: OPAC

This commit adds a new jquery function to functions.js, preventDoubleFormSubmit().
When used thusly: $('#form-id').preventDoubleFormSubmit();
It will prevent a submitted form from being submitted a second time.
This commit uses said function to prevent double processing a hold
request from the opac.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Rapid clicking on the place hold button no longer causes duplicate holds!
Passes prove t xt t/db_dependent in line with current master.
Comment 7 Paul Poulain 2012-02-17 14:33:39 UTC
QA comment:
nothing to say about the code itself.
A question though : couldn't we say the same thing for all forms ? if the librarian hit the "same new patron" twice, it will result in 2 patrons.

I think that librarians can be properly teached, but patron (thus OPAC) can't, so I'm for securing all OPAC forms, but not necessarly all staff ones
Comment 8 Liz Rea 2012-02-17 18:24:06 UTC
Yep, that's a great point - this code (as I understand it) can be used on any form id - we probably need a separate bug for that. :)
Comment 9 Kyle M Hall 2012-02-17 18:47:46 UTC
(In reply to comment #8)

This could be added to every form on every page with a bit of jquery.

> Yep, that's a great point - this code (as I understand it) can be used on any
> form id - we probably need a separate bug for that. :)
Comment 10 Paul Poulain 2012-02-20 14:29:42 UTC
more an ENH than a bugfix, versionning it for 3.8
Comment 11 Maxime Pelletier 2012-11-08 21:05:03 UTC
I just ran in a problem with this new function preventDoubleFormSubmit(). If you place a hold and you have an error in the function check() then you won't be able to submit the form.
The only idea I have to fix this is to add a setTimeout :

setTimeout(function(){$('#hold-request-form')[0].beenSubmitted = false;}, 2000)

after "this.beenSubmitted = true;" in preventDoubleFormSubmit(). This way you can submit the form again in 2 seconds.
Anyone has a better idea ? If not I will send a patch.
Comment 12 Paul Poulain 2012-11-12 13:00:51 UTC
(In reply to comment #11)
> I just ran in a problem with this new function preventDoubleFormSubmit(). If
> you place a hold and you have an error in the function check() then you
> won't be able to submit the form.
> The only idea I have to fix this is to add a setTimeout :
> 
> setTimeout(function(){$('#hold-request-form')[0].beenSubmitted = false;},
> 2000)
> 
> after "this.beenSubmitted = true;" in preventDoubleFormSubmit(). This way
> you can submit the form again in 2 seconds.
> Anyone has a better idea ? If not I will send a patch.

Good catch maxime ! my opinion is that it's an acceptable way to fix this problem, but I see a -uncommon- problem with it:
 * I click to validate
 * server very high load and/or network prolem, so no answer for 3 or 4 seconds
 * click again (with your proposal it would be possible)
 * 2 hold placed 
Would it be possible to declare "unbusmitted" a form if the check() is false ? That would be a better option
Comment 13 Nicole C. Engard 2015-05-27 18:27:58 UTC
I want to bring this back to people's attention.
Comment 14 Marc Véron 2015-12-22 10:22:45 UTC
(In reply to Liz Rea from comment #1)
> http://screencast.com/t/N2RjNmNh
> 
> Shows double clicking the place hold button causing a duplicate hold.

There is a question in the screencast:
'If I can't place holds, why could I do it?'

Maybe this is related to
- place hold on next available item ( bilio level hold)
- 'ReservesControlBranch' is set to "Check the [item's home library] to see if the patron can place a hold on the item." 

See bug 15404
Comment 15 Marc Véron 2015-12-22 11:34:34 UTC
There seems to be some code to prevent double click submit:

koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt line 179
    $('#hold-request-form').preventDoubleFormSubmit();

Hmm, if I have a JavaScript alert e.g. 
"- This patron had already placed a hold on this item"
and I click OK, I get an endless wait cursor!

This issue goes away if I remove the line with the preventDoubleFormSubmit.
Comment 16 Owen Leonard 2016-08-10 15:22:36 UTC
(In reply to Marc Véron from comment #15)

> Hmm, if I have a JavaScript alert e.g. 
> "- This patron had already placed a hold on this item"
> and I click OK, I get an endless wait cursor!

This issue is still valid, but is unrelated to this bug report. The original bug reported here is fixed in master.