See bug 25907 and comment 21 from bug 25969 To recreate: 1 - Place a hold for pickup at library A 2 - Confirm the hold and print slip 3 - Checkin the item again 3 - Confirm the hold and print slip 4 - The modal returns, do it again 5 - and again 6 - and again
Created attachment 111337 [details] [review] Bug 26627: Print and confirming a hold can cause an infinite loop Test plan: 1) Place a hold for pickup at library A 2) Confirm the hold and print slip 3) Checkin the item again 4) Confirm the hold and print slip 5) The modal returns, do it again 6) Apply this patch 7) Reload the page 8) Repeat steps 1-3 9) No loop!
Nick, why did you link this bug report with bug 26485?
(In reply to Jonathan Druart from comment #2) > Nick, why did you link this bug report with bug 26485? I tested them all together, without the dependencies I was still seeing issues
Created attachment 111635 [details] [review] Bug 26627: Print and confirming a hold can cause an infinite loop Test plan: 1) Place a hold for pickup at library A 2) Confirm the hold and print slip 3) Checkin the item again 4) Confirm the hold and print slip 5) The modal returns, do it again 6) Apply this patch 7) Reload the page 8) Repeat steps 1-3 9) No loop! Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Created attachment 111805 [details] [review] Bug 26627: Print and confirming a hold can cause an infinite loop Test plan: 1) Place a hold for pickup at library A 2) Confirm the hold and print slip 3) Checkin the item again 4) Confirm the hold and print slip 5) The modal returns, do it again 6) Apply this patch 7) Reload the page 8) Repeat steps 1-3 9) No loop! Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Interesting bug! Note: it has to be "print slip..." with just confirm the loop doesn't happen.
Pushed to master for 20.11, thanks to everybody involved!
The fix here seems to have created another problem, it causes the button "Print slip, transfer, and confirm" to not do anything when clicked. In Javascript console I see Uncaught TypeError: this.form.barcode is undefined /cgi-bin/koha/circ/returns.pl:31317
Created attachment 113093 [details] [review] Bug 26627: (QA follow-up) Remove barcode input by id
It looks like the first patch is "Pushed to master", but nothin has been done about the second patch from Kyle. Should it go on a separate bug, or should the status here be reset to "Needs signoff"? Joonas: Have you tested if the second patch solves your problem? BTW: This needs to be backported to 20.05.x, I think.
I just tested and the QA follow-up here didn't solve the issue, in browser console I get: document.getElementById(...) is null So I'm creating first a hold to item and select a pickup location to something other than where the item is currently, then I check-in the item and I want to confirm the hold and print slip & transfer but this happens. Let me see if I will find a fix for this.
My guess is that this would work if the hold was item level, but I made biblio level hold meaning there is no specific barcode attached to the hold, thus the error.
Created attachment 113237 [details] [review] Bug 26627: (QA follow-up) Remove only the loop causing input field The loop reported originally happened only in this one specific form which this commit now removes the input field from. There is no need to remove the barcode from all the forms (even ones that don't have it). Because the JavaScript tried to remove the barcode input field from other forms as well we got following error when clicking "Print slip, transfer, and confirm" for a hold that needed transfer: Uncaught TypeError: this.form.barcode is undefined Test plan: 1) Create biblio level hold that needs a transfer 2) Check-in the item and click "Print slip, transfer, and confirm" 3) Notice nothing happens, apply patch & restart, notice the slip will be now printed 4) Follow the test plan from commit a71596fb39 (Bug 26627: Print and confirming a hold can cause an infinite loop) Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
I re-did now basically the original patch by removing the unneeded HTML input directly from the HTML code instead of using Javascript to do it. Asking now for someone else to sign-off this as well.
(In reply to Joonas Kylmälä from comment #14) > I re-did now basically the original patch by removing the unneeded HTML > input directly from the HTML code instead of using Javascript to do it. > Asking now for someone else to sign-off this as well. This patch has effectively undone bug 25907 and would create a regression.
Created attachment 113249 [details] [review] Bug 26627: (QA follow-up) Remove barcode input by id
I think this latest version should solve all the problems! Would you mind testing? Thanks!
(In reply to Kyle M Hall from comment #16) > Created attachment 113249 [details] [review] [review] > Bug 26627: (QA follow-up) Remove barcode input by id Thanks, I was just commenting the following and your patch seems to be what I wanted: > I think we should have separate forms for each one of the actions (confirm, > print and confirm, cancel). Though it would be nice to have a fast fix for this > without huge possibility of regression so maybe adding and if check whether the > HTML element this.form.barcode exists would be good enough solution for now. I will test now your patch and sign-it off.
Created attachment 113250 [details] [review] Bug 26627: (QA follow-up) Remove barcode input by id Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Works now as expected, signing-off.
(In reply to Joonas Kylmälä from comment #19) > Created attachment 113250 [details] [review] [review] > Bug 26627: (QA follow-up) Remove barcode input by id > > Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi> Patch pushed to master for 20.11
backported to 20.05.x for 20.05.06
missing dependencies, not backported to 19.11.x