Bug 36139 - Bug 35518 follow-up: fix AutoSwitchPatron
Summary: Bug 35518 follow-up: fix AutoSwitchPatron
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Bugs List
QA Contact: Testopia
URL:
Keywords: rel_23_05_candidate, rel_23_11_candidate
Depends on: 35518
Blocks:
  Show dependency treegraph
 
Reported: 2024-02-20 21:18 UTC by Michael Hafen
Modified: 2024-12-23 08:07 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This fixes an issue when 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.
Version(s) released in:
24.05.00,23.11.05
Circulation function:


Attachments
Bug 35518 Follow-up to fix AutoSwitchPatron - clear variables (2.03 KB, patch)
2024-02-20 21:24 UTC, Michael Hafen
Details | Diff | Splinter Review
Bug 36139: Bug 35518 Follow-up to fix AutoSwitchPatron - clear variables (2.04 KB, patch)
2024-03-15 22:47 UTC, David Nind
Details | Diff | Splinter Review
Bug 36139: Bug 35518 (follow-up) to fix AutoSwitchPatron - clear variables (2.10 KB, patch)
2024-03-22 14:07 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Hafen 2024-02-20 21:18:25 UTC
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.
Comment 1 Michael Hafen 2024-02-20 21:24:27 UTC
Created attachment 162310 [details] [review]
Bug 35518 Follow-up to fix AutoSwitchPatron - clear variables
Comment 2 Jonathan Druart 2024-03-14 14:41:52 UTC
Is this ready for signoff?
Comment 3 Michael Hafen 2024-03-14 17:02:56 UTC
Yes, sorry.  I'll change the status.
Comment 4 David Nind 2024-03-15 20:33:01 UTC Comment hidden (obsolete)
Comment 5 David Nind 2024-03-15 20:33:49 UTC
I've also changed the assignee.
Comment 6 Michael Hafen 2024-03-15 22:07:13 UTC Comment hidden (obsolete)
Comment 7 David Nind 2024-03-15 22:46:19 UTC Comment hidden (obsolete)
Comment 8 David Nind 2024-03-15 22:47:07 UTC
Created attachment 163287 [details] [review]
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>
Comment 9 David Nind 2024-03-15 23:03:11 UTC
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.
Comment 10 Nick Clemens (kidclamp) 2024-03-22 14:07:19 UTC
Created attachment 163708 [details] [review]
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>
Comment 11 Katrin Fischer 2024-03-22 14:16:56 UTC
Hm, that must have been a close miss - patch doesn't apply anymore. Can you please rebase?
Comment 12 Katrin Fischer 2024-03-22 14:50:11 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 13 Aleisha Amohia 2024-04-24 00:09:35 UTC
This fixes a bug in 23.11 and 23.05. Can we please ensure it is backported?
Comment 14 Fridolin Somers 2024-04-24 05:37:08 UTC
Pushed to 23.11.x for 23.11.05
Comment 15 Michael Hafen 2024-04-24 14:42:14 UTC
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.
Comment 16 Lucas Gass (lukeg) 2024-05-13 20:10:14 UTC
(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.