[Bug 36139] New: Bug 35518 follow-up: fix AutoSwitchPatron
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Bug ID: 36139 Summary: Bug 35518 follow-up: fix AutoSwitchPatron Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: trivial Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: michael.hafen@washk12.org QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Bug 35518 moved some code blocks to after the call to get_user_and_template() so that userenv would be populated before it was needed. This caused a couple variables to be set before the AutoSwitchPatron block could prevent them from being set. Which broke AutoSwitchPatron functionality. This clears two variable so that AuthSwitchPatron works again. Test plan: 1. Ensure AutoSwitchPatron is turned on. 2. Select the card number of two patron accounts. 3. Find the first patron in circulation. 4. Enter the second patron's card number in the item barcode field to switch patrons. 5. Observe the error about item barcode not existing, and the patron did not switch. 6. Apply patch and restart services. 7. Enter the second patron's card number in the item barcode field again. 8. Observe that the patron was switched with no error about an invalid barcode. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Michael Hafen <michael.hafen@washk12.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |35518 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 [Bug 35518] Call to C4::Context->userenv happens before it's gets populated breaks code logic in circulation -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 --- Comment #1 from Michael Hafen <michael.hafen@washk12.org> --- Created attachment 162310 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=162310&action=edit Bug 35518 Follow-up to fix AutoSwitchPatron - clear variables -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #2 from Jonathan Druart <jonathan.druart@gmail.com> --- Is this ready for signoff? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Michael Hafen <michael.hafen@washk12.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff --- Comment #3 from Michael Hafen <michael.hafen@washk12.org> --- Yes, sorry. I'll change the status. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Patch doesn't apply CC| |david@davidnind.com --- Comment #4 from David Nind <david@davidnind.com> --- The patch(es) didn't apply for me - I'm confused why there are four patches shown when applying, versus only one listed under the attachments in the bug. See [1]. In addition, before the patches are applied the current behaviour in master doesn't match with what is described in the bug. For me, the patron successfully switches, but the barcode error is displayed (tested using koha-testing-docker with the sample data): 1. Enable the AutoSwitchPatron system preference. 2. Go to check out an item to a patron: enter name, such as Mary, in the check out search box. 3. On the checkout page, enter the card number for another patron instead of an item barcode, for example: Lisa - 23529000197047. 4. The patron switches to Lisa. 5. Two messages are displayed: Patron was automatically switched by reading the patron card during checking out. Ensure you are working with the right patron. Barcode not found The barcode was not found: 23529000197047 Add record using fast cataloging [1] Result of trying to apply the patch(es): git bz apply 35518 Bug 35518 - Call to C4::Context->userenv happens before it's gets populated breaks code logic in circulation 161929 - Bug 35518: Check authentication and set userenv before fetching userenv variables 161930 - Bug 35518: Tidy the moved blocks 162216 - Bug 35518: follow up - clear the barcode list in AutoSwitchPatron block 162218 - Bug 35518: Follow-up for AutoSwitchPatron - clear $patron variable too Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 35518: Check authentication and set userenv before fetching userenv variables Using index info to reconstruct a base tree... M circ/circulation.pl Falling back to patching base and 3-way merge... Auto-merging circ/circulation.pl CONFLICT (content): Merge conflict in circ/circulation.pl error: Failed to merge in the changes. Patch failed at 0001 Bug 35518: Check authentication and set userenv before fetching userenv variables -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |michael.hafen@washk12.org |ity.org | --- Comment #5 from David Nind <david@davidnind.com> --- I've also changed the assignee. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 --- Comment #6 from Michael Hafen <michael.hafen@washk12.org> --- There are four patches because the `git bz apply` command references the parent bug, which is already in master as commits 90b6f68616 and b577b65670. Please try `git bz apply 36139` instead. The patch file here squashes the last two patches on the parent bug (those two should probably be removed from there or something, since I mistakenly added them after the patch had been pushed). This will at least take care of the 'Barcode not found' error, and I think clearing the $patron variable should be harmless at that point in the file. (Looking over the code again, I don't know why it wasn't switching patrons for me, the $patron variable clearly gets reset in the $findborrower block below.) I'll leave the status as is, just in case there is anything else that causes the patch to not apply. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff --- Comment #7 from David Nind <david@davidnind.com> --- (In reply to Michael Hafen from comment #6)
There are four patches because the `git bz apply` command references the parent bug, which is already in master as commits 90b6f68616 and b577b65670. Please try `git bz apply 36139` instead. The patch file here squashes the last two patches on the parent bug (those two should probably be removed from there or something, since I mistakenly added them after the patch had been pushed). This will at least take care of the 'Barcode not found' error, and I think clearing the $patron variable should be harmless at that point in the file. (Looking over the code again, I don't know why it wasn't switching patrons for me, the $patron variable clearly gets reset in the $findborrower block below.) I'll leave the status as is, just in case there is anything else that causes the patch to not apply.
Hi Michael. My apologies! I was reading things incorrectly, and as you spotted I applied the wrong patch! The patch does apply, now that I have used the correct bug number! Things now work as expected: with AutoSwitchPatron enabled, the patron is swicthed and the only message that is now displayed is: Patron was automatically switched by reading the patron card during checking out. Ensure you are working with the right patron. Sign off on the way. David -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 --- Comment #8 from David Nind <david@davidnind.com> --- Created attachment 163287 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163287&action=edit Bug 36139: Bug 35518 Follow-up to fix AutoSwitchPatron - clear variables Bug 35518 moved some code blocks to after the call to get_user_and_template() so that userenv would be populated before it was needed. This caused a couple variables to be set before the AutoSwitchPatron block could prevent them from being set. Which broke AutoSwitchPatron functionality. This clears two variable so that AuthSwitchPatron works again. The AutoSwitchPatron clears the $borrowernumber variable to switch patrons. With the AuthSwitchPatron block moved, the $patron variable still gets set, and the patron doesn't get switched. The clears the $patron variable too. Also clear the barcode list. The AutoSwitchPatron block got moved, and now the @$barcodes variable gets filled and not cleared. Leading to a 'Barcode not found' error when the patron is auto switched. Test plan: 1. Ensure AutoSwitchPatron is turned on. 2. Select the card number of two patron accounts. 3. Find the first patron in circulation. 4. Enter the second patron's card number in the item barcode field to switch patrons. 5. Observe the error about item barcode not existing, and the patron did not switch. 6. Apply patch and restart services. 7. Enter the second patron's card number in the item barcode field again. 8. Observe that the patron was switched with no error about an invalid barcode. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #162310|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This fixes an issue when release notes| |the AutoSwitchPatron system | |preference is enabled (the | |issue was caused by bug | |35518 - added to Koha | |24.05.00, 23.11.03, and | |23.05.09). | | | |If you went to | |check out an item to a | |patron, and then entered | |another patron's card | |number in the item bar | |code, it was correctly: | |- | |switching to that patron | |- | |showing a message to say | |that the patron was | |switched. | | | |However, it was | |also incorrectly showing a | |"Barcode not found" message | |- this is now fixed, and is | |no longer displayed. --- Comment #9 from David Nind <david@davidnind.com> --- Testing notes (using koha-testing-docker (KTD): 1. Enable the AutoSwitchPatron system preference. 2. Go to check out an item to a patron: enter name, such as Mary, in the check out search box. 3. On the check out page, enter the card number for another patron instead of an item barcode, for example: Lisa - 23529000197047. 4. The patron correctly switches to Lisa. 5. Two messages are displayed: Patron was automatically switched by reading the patron card during checking out. Ensure you are working with the right patron. Barcode not found The barcode was not found: 23529000197047 Add record using fast cataloging 6. Only the first message should be displayed. 7. Apply the patch and repeat. 8. The barcode not found message is no longer displayed. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 --- Comment #10 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 163708 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=163708&action=edit Bug 36139: Bug 35518 (follow-up) to fix AutoSwitchPatron - clear variables Bug 35518 moved some code blocks to after the call to get_user_and_template() so that userenv would be populated before it was needed. This caused a couple variables to be set before the AutoSwitchPatron block could prevent them from being set. Which broke AutoSwitchPatron functionality. This clears two variable so that AuthSwitchPatron works again. The AutoSwitchPatron clears the $borrowernumber variable to switch patrons. With the AuthSwitchPatron block moved, the $patron variable still gets set, and the patron doesn't get switched. The clears the $patron variable too. Also clear the barcode list. The AutoSwitchPatron block got moved, and now the @$barcodes variable gets filled and not cleared. Leading to a 'Barcode not found' error when the patron is auto switched. Test plan: 1. Ensure AutoSwitchPatron is turned on. 2. Select the card number of two patron accounts. 3. Find the first patron in circulation. 4. Enter the second patron's card number in the item barcode field to switch patrons. 5. Observe the error about item barcode not existing, and the patron did not switch. 6. Apply patch and restart services. 7. Enter the second patron's card number in the item barcode field again. 8. Observe that the patron was switched with no error about an invalid barcode. kidclamp amended patch - tidy and fix commmit message Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hm, that must have been a close miss - patch doesn't apply anymore. Can you please rebase? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #163287|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Pushed for 24.05! Well done everyone, thank you! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz --- Comment #13 from Aleisha Amohia <aleisha@catalyst.net.nz> --- This fixes a bug in 23.11 and 23.05. Can we please ensure it is backported? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |rel_23_05_candidate, | |rel_23_11_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m Version(s)| |24.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|24.05.00 |24.05.00,23.11.05 released in| | Status|Pushed to main |Pushed to stable --- Comment #14 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to 23.11.x for 23.11.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 --- Comment #15 from Michael Hafen <michael.hafen@washk12.org> --- This patch must NOT be back ported to 23.05.x. these changes are already in that branch due to a mistake on my part. There is a note on the parent bug about it. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Resolution|--- |FIXED Status|Pushed to stable |RESOLVED --- Comment #16 from Lucas Gass <lucas@bywatersolutions.com> --- (In reply to Michael Hafen from comment #15)
This patch must NOT be back ported to 23.05.x. these changes are already in that branch due to a mistake on my part. There is a note on the parent bug about it.
Michael, you got it! No backport for 23.05.x. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=36139 Bug 36139 depends on bug 35518, which changed state. Bug 35518 Summary: Call to C4::Context->userenv happens before it's gets populated breaks code logic in circulation https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=35518 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to oldstable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org