Bug 41074 - Last patron links are shuffled and wrong patrons removed
Summary: Last patron links are shuffled and wrong patrons removed
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Nick Clemens (kidclamp)
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 39022
Blocks:
  Show dependency treegraph
 
Reported: 2025-10-22 18:00 UTC by Nick Clemens (kidclamp)
Modified: 2025-10-24 13:59 UTC (History)
2 users (show)

See Also:
GIT URL:
Initiative type: ---
Sponsorship status: ---
Crowdfunding goal: 0
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.11.00
Circulation function:


Attachments
Bug 41074: Don't reverse previous_patrons arrary (2.05 KB, patch)
2025-10-22 18:02 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 41074: Don't reverse previous_patrons arrary (2.14 KB, patch)
2025-10-24 07:50 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2025-10-22 18:00:07 UTC
On bug 39022 some foolhardy developer added a reverse of the previous_patrons array to correct a problem with the ordering on the page. This worked well when 1 previous patron was retained, but has the affect of shuffling the stored list with each addition when multiple are retained.

To test:
1 - Write a report to get some patron
SELECT cardnumber,surname FROM borrowers WHERE cardnumber <>'' ORDER BY rand() LIMit 6
2 - Enabled 'showLastPatron' and set count to 3
    http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=showLastPatron
3 - Copy the barcodes in order from the report and search for them in patron search, you'll want two tabs so you can keep report open
4 - Notice at 4th patron that list order has changed
5 - When 5th patron is loaded, the 4th is removed from the list
Comment 1 Nick Clemens (kidclamp) 2025-10-22 18:02:48 UTC
Created attachment 188301 [details] [review]
Bug 41074: Don't reverse previous_patrons arrary

This patch alters the code to append instead of prepend, removing the need to reverse the list

To test:
1 - Write a report to get some patron
SELECT cardnumber,surname FROM borrowers WHERE cardnumber <>'' ORDER BY rand() LIMit 6
2 - Enabled 'showLastPatron' and set count to 3
    http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=showLastPatron
3 - Copy the barcodes in order from the report and search for them in patron search, you'll want two tabs so you can keep report open
4 - Notice at 4th patron that list order has changed
5 - When 5th patron is loaded, the 4th is removed from the list
6 - Apply patch
7 - Clear the list
8 - Repeat the test plan, but note that list order does not change
9 - Confirm the patrons are removed from the list in expected order when adding new
10 - Change the count to 1 and search for several patrons, ensuring the last patron link is always correct
Comment 2 Nick Clemens (kidclamp) 2025-10-22 18:03:31 UTC
trivial, moving to QA
Comment 3 Marcel de Rooy 2025-10-24 07:50:05 UTC
Created attachment 188402 [details] [review]
Bug 41074: Don't reverse previous_patrons arrary

This patch alters the code to append instead of prepend, removing the need to reverse the list

To test:
1 - Write a report to get some patron
SELECT cardnumber,surname FROM borrowers WHERE cardnumber <>'' ORDER BY rand() LIMit 6
2 - Enabled 'showLastPatron' and set count to 3
    http://localhost:8081/cgi-bin/koha/admin/preferences.pl?op=search&searchfield=showLastPatron
3 - Copy the barcodes in order from the report and search for them in patron search, you'll want two tabs so you can keep report open
4 - Notice at 4th patron that list order has changed
5 - When 5th patron is loaded, the 4th is removed from the list
6 - Apply patch
7 - Clear the list
8 - Repeat the test plan, but note that list order does not change
9 - Confirm the patrons are removed from the list in expected order when adding new
10 - Change the count to 1 and search for several patrons, ensuring the last patron link is always correct

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Marcel de Rooy 2025-10-24 07:51:19 UTC
(In reply to Nick Clemens (kidclamp) from comment #2)
> trivial, moving to QA

Trivial is a bit strong. Although this feature seems to be recent, its code is not very clear.
Comment 5 Lucas Gass (lukeg) 2025-10-24 13:59:19 UTC
Nice work everyone!

Pushed to main for 25.11