Summary: | Transfer silently not canceled when TransfersBlockCirc = "don't block" and AutomaticConfirmTransfer = "do automatically confirm" | ||
---|---|---|---|
Product: | Koha | Reporter: | Victor Grousset/tuxayo <victor> |
Component: | Circulation | Assignee: | Emily Lamancusa (emlam) <emily.lamancusa> |
Status: | Signed Off --- | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | david, emily.lamancusa, gmcharlt, kyle.m.hall, oleonard |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38789 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=38588 |
||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 38588 | ||
Bug Blocks: | 38842 | ||
Attachments: |
Bug 38793: Don't dismiss modal before submitting form
Bug 38793: Don't dismiss modal before submitting form Bug 38793: Don't dismiss modal before submitting form Bug 38793: Don't dismiss modal before submitting form |
Description
Victor Grousset/tuxayo
2024-12-28 07:40:52 UTC
Created attachment 176189 [details] [review] Bug 38793: Don't dismiss modal before submitting form Remove the bs-data-dismiss attribute from the "cancel transfer" button on the Wrong Transfer modal. The "cancel transfer" button's JS works by adding a hidden input and then submitting the form - we don't want Bootstrap to dismiss the modal before the embedded form has been submitted. To test: 0. Set up system preferences: TransfersBlockCirc - Don't block AutomaticConfirmTransfer - Do 1. Find or create an item that belongs to a branch other than the logged-in branch 2. Check in the item to trigger a Return to Home transfer 3. Click OK on the modal to confirm the transfer 4. Check in the item again to trigger a Wrong Transfer modal 5. Click the "Cancel transfer" button on the modal 6. Navigate to the bib for that item and check the item status --> The transfer wasn't cancelled - the item is still in transit 7. Apply patch and refresh the page 8. Repeat steps 4-6 --> Item shows as available - the transfer was cancelled successfully Turns out Bootstrap was dismissing the modal before the form submitted the request to cancel the transfer...which caused AutomaticConfirmTransfer to confirm it in the absence of other input. The test plan produces internal server errors without the patches from 38588, so I'm adding that as a dependency. I had a go at testing. However, I'm not noticing any difference in behavour before and after the patch. Testing notes - both before and after the patch:[1] - before step 6 the item is showing as in transit (as expected after the first check in) - after the second check in and selecting "Cancel transfer (X)": . the current library is showing as Centerville (as expected) . the home library is showing as Midway (as expected) . not shown as in transit I had to apply bug 38588 before starting the test plan, to get rid of the error trace. [1] Used record 165 - Higher-order Perl : transforming programs with programs, item barcode 39999000007077 (before check ins, current and home library shown as Midway) (In reply to David Nind from comment #3) > I had a go at testing. > > However, I'm not noticing any difference in behavour before and after the > patch. > > Testing notes - both before and after the patch:[1] > - before step 6 the item is showing as in transit (as expected after the > first check in) > - after the second check in and selecting "Cancel transfer (X)": > . the current library is showing as Centerville (as expected) > . the home library is showing as Midway (as expected) > . not shown as in transit > > I had to apply bug 38588 before starting the test plan, to get rid of the > error trace. > > [1] Used record 165 - Higher-order Perl : transforming programs with > programs, item barcode 39999000007077 (before check ins, current and home > library shown as Midway) Thanks for testing! Hmmmm...that's what Victor said too, but I still reproduce it with bug 38588 applied and this bug not applied. I only reproduce it with the system preference settings from step 0, though, not with default settings. You're still seeing the above after changing the sysprefs? If so, maybe it's browser-based? I'm using Firefox. (In reply to Emily Lamancusa (emlam) from comment #4) > Thanks for testing! Hmmmm...that's what Victor said too, but I still > reproduce it with bug 38588 applied and this bug not applied. I only > reproduce it with the system preference settings from step 0, though, not > with default settings. You're still seeing the above after changing the > sysprefs? If so, maybe it's browser-based? I'm using Firefox. I was testing using Chromium. I just tested with Firefox, and I got the behavour as described in the test plan. It seems of that it works differently in different browsers - I tried to see if there were any console errors, but couldn't see any. Can confirm that system preferences as per step 0 of the test plan were set. I don't know enough to identify what might be causing this - maybe Owen might have an idea. (In reply to Emily Lamancusa (emlam) from comment #4) > Thanks for testing! Hmmmm...that's what Victor said too It was for another ticket. Or maybe it's applicable here? Because here I get everything as expected with Firefox. But yes with Chromium I can't reproduce the issue, there is indeed a difference in behavior! 😱 (In reply to Victor Grousset/tuxayo from comment #6) > (In reply to Emily Lamancusa (emlam) from comment #4) > > Thanks for testing! Hmmmm...that's what Victor said too > > It was for another ticket. Or maybe it's applicable here? > Because here I get everything as expected with Firefox. > > But yes with Chromium I can't reproduce the issue, there is indeed a > difference in behavior! 😱 It was something you mentioned on bug 38806, yeah... Though now that I go back and reread, I'm not sure if we were talking about the same thing ^^" I'll update the test plan in the patch to note that it seems to be specific to Firefox (I couldn't replicate it in Chrome or Edge either). Owen, would you mind taking a look? Created attachment 176368 [details] [review] Bug 38793: Don't dismiss modal before submitting form Remove the bs-data-dismiss attribute from the "cancel transfer" button on the Wrong Transfer modal. The "cancel transfer" button's JS works by adding a hidden input and then submitting the form - we don't want Bootstrap to dismiss the modal before the embedded form has been submitted. Note: This bug seems to be browser-dependent. So far it has been reproduced with Firefox, but not Chrome/Chromium or Microsoft Edge To test: 1. Log into the staff client using Firefox 2. Set up system preferences: TransfersBlockCirc - Don't block AutomaticConfirmTransfer - Do 3. Find or create an item that belongs to a branch other than the logged-in branch 4. Check in the item to trigger a Return to Home transfer 5. Click OK on the modal to confirm the transfer 6. Check in the item again to trigger a Wrong Transfer modal 7. Click the "Cancel transfer" button on the modal 8. Navigate to the bib for that item and check the item status --> The transfer wasn't cancelled - the item is still in transit 9. Apply patch and refresh the page 10. Repeat steps 6-8 --> Item shows as available - the transfer was cancelled successfully Created attachment 176387 [details] [review] Bug 38793: Don't dismiss modal before submitting form Remove the bs-data-dismiss attribute from the "cancel transfer" button on the Wrong Transfer modal. The "cancel transfer" button's JS works by adding a hidden input and then submitting the form - we don't want Bootstrap to dismiss the modal before the embedded form has been submitted. Note: This bug seems to be browser-dependent. So far it has been reproduced with Firefox, but not Chrome/Chromium or Microsoft Edge To test: 1. Log into the staff client using Firefox 2. Set up system preferences: TransfersBlockCirc - Don't block AutomaticConfirmTransfer - Do 3. Find or create an item that belongs to a branch other than the logged-in branch 4. Check in the item to trigger a Return to Home transfer 5. Click OK on the modal to confirm the transfer 6. Check in the item again to trigger a Wrong Transfer modal 7. Click the "Cancel transfer" button on the modal 8. Navigate to the bib for that item and check the item status --> The transfer wasn't cancelled - the item is still in transit 9. Apply patch and refresh the page 10. Repeat steps 6-8 --> Item shows as available - the transfer was cancelled successfully Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 176404 [details] [review] Bug 38793: Don't dismiss modal before submitting form Remove the bs-data-dismiss attribute from the "cancel transfer" button on the Wrong Transfer modal. The "cancel transfer" button's JS works by adding a hidden input and then submitting the form - we don't want Bootstrap to dismiss the modal before the embedded form has been submitted. Note: This bug seems to be browser-dependent. So far it has been reproduced with Firefox, but not Chrome/Chromium or Microsoft Edge To test: 1. Log into the staff client using Firefox 2. Set up system preferences: TransfersBlockCirc - Don't block AutomaticConfirmTransfer - Do 3. Find or create an item that belongs to a branch other than the logged-in branch 4. Check in the item to trigger a Return to Home transfer 5. Click OK on the modal to confirm the transfer 6. Check in the item again to trigger a Wrong Transfer modal 7. Click the "Cancel transfer" button on the modal 8. Navigate to the bib for that item and check the item status --> The transfer wasn't cancelled - the item is still in transit 9. Apply patch and refresh the page 10. Repeat steps 6-8 --> Item shows as available - the transfer was cancelled successfully Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Owen Leonard <oleonard@myacpl.org> |