Bug 14364 made a change that seems advised against by the code. koha_notify_reserve uses the inbound_email_address for the 'from' field, it should use 'from_email_address'
That is correct, the From should be branchemail/KohaAdminEmailAddress.
Created attachment 144552 [details] [review] Bug 32455: Use from_email_address for 'from' field for hold notices This patch updates two occurrences where the inbound library email is used as the from address To test: 1 - Set a unique 'Email' and 'Reply to' address for a library 2 - Find a patron of that library, ensure they have an email 3 - Ensure their messaging preference for holds is 'email' 4 - Set system preference ReservesMaxPickupDelay to -1 5 - Set system preference ExpireReservesMaxPickUpDelay to Allow 6 - Set system preference ExpireReservesAutoFill to Do 7 - Place and fill a hold for that patron at that library 8 - Check the patron's notification tab, confirm the from address is the 'Reply to' 9 - Place a hold for another patron on the same item 10 - Run the expired holds cronjob: perl misc/cronjobs/holds/cancel_expired_holds.pl --reason=whatever 11 - Check the message_queue - notice the from address is the 'reply to' 12 - Apply patch 13 - Repeat 1-11, confirm the from addresses are correct now
Please provide tests.
Created attachment 144579 [details] [review] Bug 32455: Unit tests
Created attachment 144580 [details] [review] Bug 32455: Cleanup and expand ExpireReservesAutoFill.t The tests here reused data, and deleted the tables before starting, with a little shifting we can use specific data and avoid mass deletion To test: prove -v t/db_dependent/Holds/ExpireReservesAutoFill.t
Created attachment 145783 [details] [review] Bug 32455: Use from_email_address for 'from' field for hold notices This patch updates two occurrences where the inbound library email is used as the from address To test: 1 - Set a unique 'Email' and 'Reply to' address for a library 2 - Find a patron of that library, ensure they have an email 3 - Ensure their messaging preference for holds is 'email' 4 - Set system preference ReservesMaxPickupDelay to -1 5 - Set system preference ExpireReservesMaxPickUpDelay to Allow 6 - Set system preference ExpireReservesAutoFill to Do 7 - Place and fill a hold for that patron at that library 8 - Check the patron's notification tab, confirm the from address is the 'Reply to' 9 - Place a hold for another patron on the same item 10 - Run the expired holds cronjob: perl misc/cronjobs/holds/cancel_expired_holds.pl --reason=whatever 11 - Check the message_queue - notice the from address is the 'reply to' 12 - Apply patch 13 - Repeat 1-11, confirm the from addresses are correct now Signed-off-by: David Nind <david@davidnind.com>
Created attachment 145784 [details] [review] Bug 32455: Unit tests Signed-off-by: David Nind <david@davidnind.com>
Created attachment 145785 [details] [review] Bug 32455: Cleanup and expand ExpireReservesAutoFill.t The tests here reused data, and deleted the tables before starting, with a little shifting we can use specific data and avoid mass deletion To test: prove -v t/db_dependent/Holds/ExpireReservesAutoFill.t Signed-off-by: David Nind <david@davidnind.com>
Created attachment 145940 [details] [review] Bug 32455: Use from_email_address for 'from' field for hold notices This patch updates two occurrences where the inbound library email is used as the from address To test: 1 - Set a unique 'Email' and 'Reply to' address for a library 2 - Find a patron of that library, ensure they have an email 3 - Ensure their messaging preference for holds is 'email' 4 - Set system preference ReservesMaxPickupDelay to -1 5 - Set system preference ExpireReservesMaxPickUpDelay to Allow 6 - Set system preference ExpireReservesAutoFill to Do 7 - Place and fill a hold for that patron at that library 8 - Check the patron's notification tab, confirm the from address is the 'Reply to' 9 - Place a hold for another patron on the same item 10 - Run the expired holds cronjob: perl misc/cronjobs/holds/cancel_expired_holds.pl --reason=whatever 11 - Check the message_queue - notice the from address is the 'reply to' 12 - Apply patch 13 - Repeat 1-11, confirm the from addresses are correct now Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 145941 [details] [review] Bug 32455: Unit tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 145942 [details] [review] Bug 32455: Cleanup and expand ExpireReservesAutoFill.t The tests here reused data, and deleted the tables before starting, with a little shifting we can use specific data and avoid mass deletion To test: prove -v t/db_dependent/Holds/ExpireReservesAutoFill.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Trivial fix, great unit test improvements, no regressions and QA scripts are happy. Passing QA
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work, thanks everyone! Pushed to 22.11.x for the next release.
Missing dependencies for 22.05.x, no backport