https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125129 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125129&action=edit Bug 29062: Use primary key issued_id to fetch old_issues for letters The code currently uses itemnumber to fetch old_issues for notices. This doesn't seem to be used in any current notices except the CHECKINSLIP: SELECTY letter.code,content FROM letter WHERE content LIKE 'old\\_%'\G For issues we use itemnumber, however, issues has a constraint to limit issues for an itemnumber to 1 Old issues has no such constraint, we try to rectify this in the old code by adding 'ORDER BY returndate DESC LIMIT 1" As the code is not used by default and buggy I think we can make a change to using 'issue_id' as the key and announcing the change - it prevents leaky data To test: 1. Check something out to patron A. Check it in. 2. Check something out to patron B. Check it in. 3. Check something out to patron C. Check it in and print the check-in slip. (Leave the checkin paghe open) 4. You will see the checkin repeat itself 3 times, one for each line in old_issues. 5. Apply patch and restart_all 6. Click the 'Print checkin slip' button again 7. You see a single checkin 8. Checkout a different item to patron A. Check it in and print the check-in slip 9. See the correct checkins -- 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=29062 Lucas Gass <lucas@bywatersolutions.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=29062 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl Status|Signed Off |Failed QA --- Comment #3 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- This might be hard to find another occurrence. But look at this code in SendCirculationAlert my $issues_table = ( $type eq 'CHECKOUT' || $type eq 'RENEWAL' ) ? 'issues' : 'old_issues'; [etc] tables => { $issues_table => $item->{itemnumber}, This should no longer work. How do you know if there no other occurrences like that one? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 --- Comment #5 from Nick Clemens <nick@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #3)

This might be hard to find another occurrence. But look at this code in SendCirculationAlert This should no longer work.

Fixed

How do you know if there no other occurrences like that one?

according to tools/letters.pl only CHECKIN and CHECKINSLIP have the old_issues table, infact, in printslip we don't even pass the old_issues table, but we send it using the 'loops' parameter I added a patch that fixes that call, I could find no others, and we don't indicate support elsewhere The sample notices make no use of the old_issues table, so another option would be to remove it from the options all together -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #6)

+ $issues_id = $item->{itemnumber};

I am having serious doubts about that change ;)

Especially the way it reads btw. Maybe find a good rename.. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125712|0 |1 is obsolete| | --- Comment #8 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125749 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125749&action=edit Bug 29062: (follow-up) Adjust SendCirculationAlert -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #125154|0 |1 is obsolete| | --- Comment #9 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 125806 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125806&action=edit Bug 29062: Use primary key issued_id to fetch old_issues for letters The code currently uses itemnumber to fetch old_issues for notices. This doesn't seem to be used in any current notices except the CHECKINSLIP: SELECTY letter.code,content FROM letter WHERE content LIKE 'old\\_%'\G For issues we use itemnumber, however, issues has a constraint to limit issues for an itemnumber to 1 Old issues has no such constraint, we try to rectify this in the old code by adding 'ORDER BY returndate DESC LIMIT 1" As the code is not used by default and buggy I think we can make a change to using 'issue_id' as the key and announcing the change - it prevents leaky data To test: 1. Check something out to patron A. Check it in. 2. Check something out to patron B. Check it in. 3. Check something out to patron C. Check it in and print the check-in slip. (Leave the checkin paghe open) 4. You will see the checkin repeat itself 3 times, one for each line in old_issues. 5. Apply patch and restart_all 6. Click the 'Print checkin slip' button again 7. You see a single checkin 8. Checkout a different item to patron A. Check it in and print the check-in slip 9. See the correct checkins Signed-off-by: Lucas Gass <lucas@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 --- Comment #11 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Created attachment 125809 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125809&action=edit Bug 29062: (QA follow-up) Add comment Wrapped it in a TODO ;) Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 --- Comment #13 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #12)

Missing test coverage.

I was permissive here for the current lack of tests of SendCirculationAlert. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 --- Comment #14 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 125879 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=125879&action=edit Bug 29062: Unit tests -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29062 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |21.11.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=29062 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.11.00 |21.11.00,21.05.05 released in| | Status|Pushed to master |Pushed to stable CC| |kyle@bywatersolutions.com --- Comment #17 from Kyle M Hall <kyle@bywatersolutions.com> --- Pushed to 21.05.x for 21.05.05 -- You are receiving this mail because: You are watching all bug changes.